Date   

[PATCH] IntelFsp2Pkg: Add search function for Config Editor

 

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3D3482

This patch adds a search function in the Config Editor GUI at
the top right corner. Once users key in the words to search,
it will look for the option containing the string in the
same page and display it.

Cc: Maurice Ma <maurice.ma@...>
Cc: Nate DeSimone <nathaniel.l.desimone@...>
Cc: Star Zeng <star.zeng@...>
Cc: Chasel Chiu <chasel.chiu@...>
Signed-off-by: Loo Tung Lun <tung.lun.loo@...>
---
IntelFsp2Pkg/Tools/ConfigEditor/ConfigEditor.py | 31 +++++++++++++++++++++=
++++++++++
IntelFsp2Pkg/Tools/ConfigEditor/GenYamlCfg.py | 25 +++++++++++++++++++--=
----
2 files changed, 50 insertions(+), 6 deletions(-)

diff --git a/IntelFsp2Pkg/Tools/ConfigEditor/ConfigEditor.py b/IntelFsp2Pkg=
/Tools/ConfigEditor/ConfigEditor.py
index a7f79bbc96..d58df385b1 100644
--- a/IntelFsp2Pkg/Tools/ConfigEditor/ConfigEditor.py
+++ b/IntelFsp2Pkg/Tools/ConfigEditor/ConfigEditor.py
@@ -811,6 +811,7 @@ class application(tkinter.Frame):
self.org_cfg_data_bin =3D None=0D
self.in_left =3D state()=0D
self.in_right =3D state()=0D
+ self.search_text =3D ''=0D
=0D
# Check if current directory contains a file with a .yaml extensio=
n=0D
# if not default self.last_dir to a Platform directory where it is=
=0D
@@ -835,6 +836,23 @@ class application(tkinter.Frame):
=0D
root.geometry("1200x800")=0D
=0D
+ # Search string=0D
+ fram =3D tkinter.Frame(root)=0D
+ # adding label to search box=0D
+ tkinter.Label(fram, text=3D'Text to find:').pack(side=3Dtkinter.LE=
FT)=0D
+ # adding of single line text box=0D
+ self.edit =3D tkinter.Entry(fram, width=3D30)=0D
+ # positioning of text box=0D
+ self.edit.pack(=0D
+ side=3Dtkinter.LEFT, fill=3Dtkinter.BOTH, expand=3D1, padx=3D(=
4, 4))=0D
+ # setting focus=0D
+ self.edit.focus_set()=0D
+ # adding of search button=0D
+ butt =3D tkinter.Button(fram, text=3D'Search', relief=3Dtkinter.GR=
OOVE,=0D
+ command=3Dself.search_bar)=0D
+ butt.pack(side=3Dtkinter.RIGHT, padx=3D(4, 4))=0D
+ fram.pack(side=3Dtkinter.TOP, anchor=3Dtkinter.SE)=0D
+=0D
paned =3D ttk.Panedwindow(root, orient=3Dtkinter.HORIZONTAL)=0D
paned.pack(fill=3Dtkinter.BOTH, expand=3DTrue, padx=3D(4, 4))=0D
=0D
@@ -943,6 +961,12 @@ class application(tkinter.Frame):
"Unsupported file '%s' !" % path)=0D
return=0D
=0D
+ def search_bar(self):=0D
+ # get data from text box=0D
+ self.search_text =3D self.edit.get()=0D
+ # Clear the page and update it according to search value=0D
+ self.refresh_config_data_page()=0D
+=0D
def set_object_name(self, widget, name):=0D
self.conf_list[id(widget)] =3D name=0D
=0D
@@ -999,6 +1023,12 @@ class application(tkinter.Frame):
widget.grid()=0D
widget.configure(state=3D'normal')=0D
=0D
+ if visible and self.search_text !=3D '':=0D
+ name =3D item['name']=0D
+ if name.lower().find(self.search_text.lower()) =3D=3D -1:=0D
+ visible =3D False=0D
+ widget.grid_remove()=0D
+=0D
return visible=0D
=0D
def update_widgets_visibility_on_page(self):=0D
@@ -1377,6 +1407,7 @@ class application(tkinter.Frame):
return None=0D
else:=0D
path =3D name=0D
+=0D
item =3D self.cfg_data_obj.get_item_by_path(path)=0D
return item=0D
=0D
diff --git a/IntelFsp2Pkg/Tools/ConfigEditor/GenYamlCfg.py b/IntelFsp2Pkg/T=
ools/ConfigEditor/GenYamlCfg.py
index 25fd9c547e..0d9505b97f 100644
--- a/IntelFsp2Pkg/Tools/ConfigEditor/GenYamlCfg.py
+++ b/IntelFsp2Pkg/Tools/ConfigEditor/GenYamlCfg.py
@@ -583,6 +583,7 @@ class CGenYamlCfg:
self._mode =3D ''=0D
self._debug =3D False=0D
self._macro_dict =3D {}=0D
+ self.bin_offset =3D []=0D
self.initialize()=0D
=0D
def initialize(self):=0D
@@ -1301,10 +1302,15 @@ option format '%s' !" % option)
if 'indx' not in cfgs:=0D
return=0D
act_cfg =3D self.get_item_by_index(cfgs['indx'])=0D
- if force or act_cfg['value'] =3D=3D '':=0D
+ actual_offset =3D act_cfg['offset'] - struct_info['offset']=0D
+ set_value =3D True=0D
+ for each in self.bin_offset:=0D
+ if actual_offset in range(each[0], (each[0] + each[2]) * 8=
):=0D
+ if each[1] < 0:=0D
+ set_value =3D False=0D
+ if set_value and force or act_cfg['value'] =3D=3D '':=0D
value =3D get_bits_from_bytes(full_bytes,=0D
- act_cfg['offset'] -=0D
- struct_info['offset'],=0D
+ actual_offset,=0D
act_cfg['length'])=0D
act_val =3D act_cfg['value']=0D
if act_val =3D=3D '':=0D
@@ -1424,8 +1430,8 @@ for '%s' !" % (act_cfg['value'], act_cfg['path']))
% seg[0])=0D
bin_segs.append([seg[0], pos, seg[2]])=0D
else:=0D
- raise Exception("Could not find '%s' in binary !"=0D
- % seg[0])=0D
+ bin_segs.append([seg[0], -1, seg[2]])=0D
+ continue=0D
=0D
return bin_segs=0D
=0D
@@ -1433,8 +1439,15 @@ for '%s' !" % (act_cfg['value'], act_cfg['path']))
# get cfg bin length=0D
cfg_bins =3D bytearray()=0D
bin_segs =3D self.get_bin_segment(bin_data)=0D
+ Dummy_offset =3D 0=0D
for each in bin_segs:=0D
- cfg_bins.extend(bin_data[each[1]:each[1] + each[2]])=0D
+ if each[1] !=3D -1:=0D
+ self.bin_offset.append([Dummy_offset, each[1], each[2]])=0D
+ cfg_bins.extend(bin_data[each[1]:each[1] + each[2]])=0D
+ else:=0D
+ self.bin_offset.append([Dummy_offset, each[1], each[2]])=0D
+ cfg_bins.extend(bytearray(each[2]))=0D
+ Dummy_offset +=3D each[2]=0D
return cfg_bins=0D
=0D
def save_current_to_bin(self):=0D
--=20
2.28.0.windows.1


Re: [PATCH] OvmfPkg/Bhyve: clean up TPM_ENABLE remnants

Laszlo Ersek
 

On 07/07/21 08:00, Kinney, Michael D wrote:
Hi Laszlo,

I did many experiments and could not get the exact behavior I proposed.

Here is the best I can do with the behavior of GitHub and Mergify:

1) I further simplified Mergify configuration so personal builds ('push'
label not set) are no longer auto closed. Any developer doing a
personal build that wants to abandon the change should manually close
the PR.
This sounds OK to me.

We may need to periodically review PRs that have been open
for an extended period of time and close them and developers can reopen
if that was a mistake.
Yes, I've been periodically checking PRs that were stuck open anyway.


2) The 'push' label always does the safest possible rebase and merge.
If many PRs are queued at a time, then each one is rebased in turn,
all CI checks are run and if all CI checks pass, then PR is
added with linear history to the base branch.
OK.


3) If a maintainer wants to manually send a sequence of PRs through
one at a time and review the state before sending the next one, then
I recommending sending each PR as a personal build (always rebase against
latest base branch before submitting PR). Review the commits and status
checks. If the PR looks good and passes all status checks and the state
from GitHub is that the PR is 'up-to-date' with the base branch, then
the maintainer can set the 'push' label and the PR is merged immediately
without re-running the status checks since there have been no changes.
So this is the key development. OK.


If other PRs were merged into the base branch while the PR status checks
were run, then the personal build will show that the branch is out-of-date
with the base branch. The maintainer can do a local rebase and review
the changes and do a forced push of the updated PR. This will trigger the
personal build again. If that passes and the branch is 'up-to-date', then
the 'push' label can be set to immediately merge. Repeat as needed.

NOTE: If there is a lot of PR activity from other maintainers at the same
time as this manual process is being used, then the manual process may
have to rebase and force push several times until there is a quiet window.
Makes sense.



The simplified Mergify configuration file is shown below.

queue_rules:
- name: default
conditions:
- base~=(^main|^master|^stable/)
- label=push

pull_request_rules:
- name: Automatically merge a PR when all required checks pass and 'push' label is present
conditions:
- base~=(^main|^master|^stable/)
- label=push
actions:
queue:
method: rebase
rebase_fallback: none
name: default

- name: Post a comment on a PR that can not be merged due to a merge conflict
conditions:
- base~=(^main|^master|^stable/)
- conflict
actions:
comment:
message: PR can not be merged due to conflict. Please rebase and resubmit


Let me know if this process of using personal builds and setting 'push' label
if PR is 'up-to-date' is acceptable. I have tested this process with a few
different scenarios in the edk2-codereview repo, and they all worked as expected.
Sounds all fine to me; thank you for working it out!
Laszlo



Best regards,

Mike

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Laszlo Ersek
Sent: Monday, June 28, 2021 5:23 AM
To: Kinney, Michael D <michael.d.kinney@...>; devel@edk2.groups.io; spbrogan@...; ardb@...
Cc: Peter Grehan <grehan@...>; Ard Biesheuvel <ardb+tianocore@...>; Justen, Jordan L
<jordan.l.justen@...>; Sean Brogan <sean.brogan@...>; Rebecca Cran <rebecca@...>
Subject: Re: [edk2-devel] [PATCH] OvmfPkg/Bhyve: clean up TPM_ENABLE remnants

On 06/24/21 00:07, Kinney, Michael D wrote:
Hi Laszlo,

I understand your point.

I am trying to balance the ease of use for developers, reducing overhead for maintainers, and
prevent bad commits.

I think you are saying that you want to make sure a maintainer carefully reviews changes
across multiple PRs that are in the same area of code. The CI checks will of course make
sure the code builds and passes the basic boot tests, but those tests do not have full
coverage so an interaction issue between two PRs that pass build and boot but have
unintended behavior side effects are what require detailed manual review.

I am going to remove the auto-rebase by default and add a optional label that can
be set by a maintainer to enable auto-rebase. If a maintainer is confident that
a set of PRs being submitted at the same time with the 'push' label are independent,
then the maintainer can also set 'auto-rebase'. If they are not confident, then
they can send PRs one at a time with only 'push' label and manually rebase each
additional PR and review the manual rebase to make sure there are no unintended
side effects.
Sounds great, thank you!
Laszlo


Any objections to this direction?

Thanks,

Mike

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Laszlo Ersek
Sent: Wednesday, June 23, 2021 12:45 PM
To: Kinney, Michael D <michael.d.kinney@...>; devel@edk2.groups.io; spbrogan@...; ardb@...
Cc: Peter Grehan <grehan@...>; Ard Biesheuvel <ardb+tianocore@...>; Justen, Jordan L
<jordan.l.justen@...>; Sean Brogan <sean.brogan@...>; Rebecca Cran <rebecca@...>
Subject: Re: [edk2-devel] [PATCH] OvmfPkg/Bhyve: clean up TPM_ENABLE remnants

On 06/23/21 20:44, Kinney, Michael D wrote:

Hi Laszlo,

Thank you for the test case.

I created 2 PRs against edk2-codereview using your patches.
I made minor update to commit messages to pass patch check.

https://github.com/tianocore/edk2-codereview/pull/18
https://github.com/tianocore/edk2-codereview/pull/19

Found another issue with PatchCheck for the Mergify merge commit and
fixed that.

Mergify did process #18 and merged it in after passing all CI. Mergify
rebased #19 successfully and merged it after passing all CI. I do not
think this was your expected result.
Indeed, my "desired" result at least would have been for mergify to
reject the rebase.

I looked more closely at the patches you provided. They were not
overlapping in the lines of Readme.rst. This is why no merge conflict
was detected.
More precisely, a contextual conflict *was* determined between the
patches, but that conflict was auto-resolved.

This is risky when done in an automated fashion. It is an extremely
convenient feature of git, when used interactively; that is, when the
auto-merge (automatic conflict resolution) is semantically verified by a
human. Git takes away the chore of conflict resolution, presents a
"likely good" end result, and a human only needs to *look* at the end
result, not *implement* it.

But that "human look" is exactly what's missing from mergify.

Basically what I'd like for mergify is to turn off automatic conflict
resolution.

More or less, speaking in terms of the stand-alone "patch" utility
<https://man7.org/linux/man-pages/man1/patch.1.html>, my preference is
to set the "fuzz factor" to zero.


One way a human reviews such context differences is with git-range-diff.
Continuing my previous example commands:

$ git range-diff --color master..b2 b1..b2-rebase

1: 02dc81e58bd6 ! 1: 2cf39d4b1790 world
@@ -6,8 +6,8 @@
--- a/ReadMe.rst
+++ b/ReadMe.rst
@@
-
A modern, feature-rich, cross-platform firmware development
+ HELLO
environment for the UEFI and PI specifications from www.uefi.org.
+ WORLD

This output shows that the "world" addition is the same (it is identical
between pre-rebase and post-rebase in the commit), but the context has
changed. During the rebase, the leading empty line of the context
disappeared, and a HELLO line in the middle of the leading context
appeared.

This result may or may not be semantically correct; it needs a human
decision. What if the original purpose of the "world" patch author was
to say WORLD but only without HELLO? When they looked at the code, there
was no HELLO yet.

git-range-diff is very powerful, but reading its output takes some
getting used to. (Colorization with the "--color" option is basically
required for understanding; I can't reproduce it in this email, alas.)

I don't want to obsess about this forever, I just want us all to be
aware that this risk exists.

Thanks,
Laszlo


I then created 2 new PRs that added text to the same line # in Readme.rst.

https://github.com/tianocore/edk2-codereview/pull/21
https://github.com/tianocore/edk2-codereview/pull/22

PR #21 passed all CI tests and was merged. Mergify then attempted to
rebase #22 and got a merge conflict and is still in the open state waiting
for the developer to manually handle the merge conflict.
This case is not worrisome; when there is a clear conflict that cannot be auto-resolved, I'm not concerned.

My concern is the sneaky contextual conflict that *appears* auto-resolvable, but is semantically broken. Those things
exist.

Thanks
Laszlo








Re: [PATCH v5 00/10] Secure Boot default keys

Grzegorz Bernacki <gjb@...>
 

Hi,

I created BZ #3481 (https://bugzilla.tianocore.org/show_bug.cgi?id=3481).
Please let me know if I filled it correctly
thanks,
greg


śr., 7 lip 2021 o 03:18 gaoliming <gaoliming@...> napisał(a):


Grzegorz Bernacki:
This is a new feature. Can you submit one BZ
(https://bugzilla.tianocore.org/) for it? Then, I can add it into edk2
stable tag feature planning.

Thanks
Liming
-----邮件原件-----
发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Grzegorz
Bernacki
发送时间: 2021年7月1日 17:18
收件人: devel@edk2.groups.io
抄送: leif@...; ardb+tianocore@...;
Samer.El-Haj-Mahmoud@...; sunny.Wang@...;
mw@...; upstream@...; jiewen.yao@...;
jian.j.wang@...; min.m.xu@...; lersek@...;
sami.mujawar@...; afish@...; ray.ni@...;
jordan.l.justen@...; rebecca@...; grehan@...;
thomas.abraham@...; chasel.chiu@...;
nathaniel.l.desimone@...; gaoliming@...;
eric.dong@...; michael.d.kinney@...; zailiang.sun@...;
yi.qian@...; graeme@...; rad@...; pete@...;
Grzegorz Bernacki <gjb@...>
主题: [edk2-devel] [PATCH v5 00/10] Secure Boot default keys

This patchset adds support for initialization of default
Secure Boot variables based on keys content embedded in
flash binary. This feature is active only if Secure Boot
is enabled and DEFAULT_KEY is defined. The patchset
consist also application to enroll keys from default
variables and secure boot menu change to allow user
to reset key content to default values.
Discussion on design can be found at:
https://edk2.groups.io/g/rfc/topic/82139806#600

Built with:
GCC
- RISC-V (U500, U540) [requires fixes in dsc to build]
- Intel (Vlv2TbltDevicePkg (X64/IA32), Quark, MinPlatformPkg,
EmulatorPkg (X64), Bhyve, OvmfPkg (X64/IA32))
- ARM (Sgi75,SbsaQemu,DeveloperBox, RPi3/RPi4)

RISC-V, Quark, Vlv2TbltDevicePkg, Bhyve requires additional fixes to be
built,
will be post on edk2 maillist later

VS2019
- Intel (OvmfPkgX64)

Test with:
GCC5/RPi4
VS2019/OvmfX64 (requires changes to enable feature)

Tests:
1. Try to enroll key in incorrect format.
2. Enroll with only PKDefault keys specified.
3. Enroll with all keys specified.
4. Enroll when keys are enrolled.
5. Reset keys values.
6. Running signed & unsigned app after enrollment.

Changes since v1:
- change names:
SecBootVariableLib => SecureBootVariableLib
SecBootDefaultKeysDxe => SecureBootDefaultKeysDxe
SecEnrollDefaultKeysApp => EnrollFromDefaultKeysApp
- change name of function CheckSetupMode to GetSetupMode
- remove ShellPkg dependecy from EnrollFromDefaultKeysApp
- rebase to master

Changes since v2:
- fix coding style for functions headers in SecureBootVariableLib.h
- add header to SecureBootDefaultKeys.fdf.inc
- remove empty line spaces in SecureBootDefaultKeysDxe files
- revert FAIL macro in EnrollFromDefaultKeysApp
- remove functions duplicates and add SecureBootVariableLib
to platforms which used it

Changes since v3:
- move SecureBootDefaultKeys.fdf.inc to ArmPlatformPkg
- leave duplicate of CreateTimeBasedPayload in PlatformVarCleanupLib
- fix typo in guid description

Changes since v4:
- reorder patches to make it bisectable
- split commits related to more than one platform
- move edk2-platform commits to separate patchset

Grzegorz Bernacki (10):
SecurityPkg: Create library for setting Secure Boot variables.
ArmVirtPkg: add SecureBootVariableLib class resolution
OvmfPkg: add SecureBootVariableLib class resolution
EmulatorPkg: add SecureBootVariableLib class resolution
SecurityPkg: Remove duplicated functions from SecureBootConfigDxe.
ArmPlatformPkg: Create include file for default key content.
SecurityPkg: Add SecureBootDefaultKeysDxe driver
SecurityPkg: Add EnrollFromDefaultKeys application.
SecurityPkg: Add new modules to Security package.
SecurityPkg: Add option to reset secure boot keys.

SecurityPkg/SecurityPkg.dec
| 14 +
ArmVirtPkg/ArmVirt.dsc.inc
| 1 +
EmulatorPkg/EmulatorPkg.dsc
| 1 +
OvmfPkg/Bhyve/BhyveX64.dsc
| 1 +
OvmfPkg/OvmfPkgIa32.dsc
| 1 +
OvmfPkg/OvmfPkgIa32X64.dsc
| 1 +
OvmfPkg/OvmfPkgX64.dsc
| 1 +
SecurityPkg/SecurityPkg.dsc
| 4 +
SecurityPkg/EnrollFromDefaultKeysApp/EnrollFromDefaultKeysApp.inf
| 47 +
SecurityPkg/Library/SecureBootVariableLib/SecureBootVariableLib.inf
| 79 ++

SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigD
xe.inf | 2 +

SecurityPkg/VariableAuthenticated/SecureBootDefaultKeysDxe/SecureBootD
efaultKeysDxe.inf | 45 +
SecurityPkg/Include/Library/SecureBootVariableLib.h
| 251 +++++

SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigN
vData.h | 2 +

SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfig.v
fr | 6 +
SecurityPkg/EnrollFromDefaultKeysApp/EnrollFromDefaultKeysApp.c
| 109 +++
SecurityPkg/Library/SecureBootVariableLib/SecureBootVariableLib.c
| 980 ++++++++++++++++++++

SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigI
mpl.c | 343 ++++---

SecurityPkg/VariableAuthenticated/SecureBootDefaultKeysDxe/SecureBootD
efaultKeysDxe.c | 68 ++
ArmPlatformPkg/SecureBootDefaultKeys.fdf.inc
| 70 ++
SecurityPkg/Library/SecureBootVariableLib/SecureBootVariableLib.uni
| 16 +

SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigS
trings.uni | 4 +

SecurityPkg/VariableAuthenticated/SecureBootDefaultKeysDxe/SecureBootD
efaultKeysDxe.uni | 16 +
23 files changed, 1874 insertions(+), 188 deletions(-)
create mode 100644
SecurityPkg/EnrollFromDefaultKeysApp/EnrollFromDefaultKeysApp.inf
create mode 100644
SecurityPkg/Library/SecureBootVariableLib/SecureBootVariableLib.inf
create mode 100644
SecurityPkg/VariableAuthenticated/SecureBootDefaultKeysDxe/SecureBootD
efaultKeysDxe.inf
create mode 100644 SecurityPkg/Include/Library/SecureBootVariableLib.h
create mode 100644
SecurityPkg/EnrollFromDefaultKeysApp/EnrollFromDefaultKeysApp.c
create mode 100644
SecurityPkg/Library/SecureBootVariableLib/SecureBootVariableLib.c
create mode 100644
SecurityPkg/VariableAuthenticated/SecureBootDefaultKeysDxe/SecureBootD
efaultKeysDxe.c
create mode 100644 ArmPlatformPkg/SecureBootDefaultKeys.fdf.inc
create mode 100644
SecurityPkg/Library/SecureBootVariableLib/SecureBootVariableLib.uni
create mode 100644
SecurityPkg/VariableAuthenticated/SecureBootDefaultKeysDxe/SecureBootD
efaultKeysDxe.uni

--
2.25.1











Re: QemuVideo: BochsID mismatch (got 0x0)

Lange Tang
 

Hi all:
  After testing, I found that once the QXL device shared a PciRoot slot with other devices, the following log would appear:
QemuVideo: BochsID mismatch (got 0x0)
Connect: PciRoot(0x0)....... : Not Found

  Is this a feature of QXL devices?


Best regards,
Lange



At 2021-07-07 00:03:43, "Lange Tang" <lange_tang@...> wrote:

Greeting!
  branch: edk2 master  ArmVirtPkg/ArmVirtQemu.dsc
  commit 17143c4837393d42c484b42d1789b85b2cff1aaf (origin/master, origin/HEAD)
    Author: Rebecca Cran <rebecca@...>
    Date:   Sun Jun 13 11:43:01 2021 +0800

   I clone edk2 code and build, run on arm64 platform for debug. Following is key log, :
  QemuVideo: BochsID mismatch (got 0x0)
Connect: PciRoot(0x0)/Pci(0x1,0x5)/Pci(0x0,0x0)/Pci(0x1,0x0): Not Found

At first, I thought it was caused by QEMU that not supporting BOCHS-DISPLAY, However, when I build QEMU support BOCHS--DISPLAY, above logs will still appear. Can you give me some advice? Thank you so much!


 



 


Re: [PATCH] OvmfPkg/Bhyve: clean up TPM_ENABLE remnants

Michael D Kinney
 

Hi Laszlo,

I did many experiments and could not get the exact behavior I proposed.

Here is the best I can do with the behavior of GitHub and Mergify:

1) I further simplified Mergify configuration so personal builds ('push'
label not set) are no longer auto closed. Any developer doing a
personal build that wants to abandon the change should manually close
the PR. We may need to periodically review PRs that have been open
for an extended period of time and close them and developers can reopen
if that was a mistake.

2) The 'push' label always does the safest possible rebase and merge.
If many PRs are queued at a time, then each one is rebased in turn,
all CI checks are run and if all CI checks pass, then PR is
added with linear history to the base branch.

3) If a maintainer wants to manually send a sequence of PRs through
one at a time and review the state before sending the next one, then
I recommending sending each PR as a personal build (always rebase against
latest base branch before submitting PR). Review the commits and status
checks. If the PR looks good and passes all status checks and the state
from GitHub is that the PR is 'up-to-date' with the base branch, then
the maintainer can set the 'push' label and the PR is merged immediately
without re-running the status checks since there have been no changes.

If other PRs were merged into the base branch while the PR status checks
were run, then the personal build will show that the branch is out-of-date
with the base branch. The maintainer can do a local rebase and review
the changes and do a forced push of the updated PR. This will trigger the
personal build again. If that passes and the branch is 'up-to-date', then
the 'push' label can be set to immediately merge. Repeat as needed.

NOTE: If there is a lot of PR activity from other maintainers at the same
time as this manual process is being used, then the manual process may
have to rebase and force push several times until there is a quiet window.


The simplified Mergify configuration file is shown below.

queue_rules:
- name: default
conditions:
- base~=(^main|^master|^stable/)
- label=push

pull_request_rules:
- name: Automatically merge a PR when all required checks pass and 'push' label is present
conditions:
- base~=(^main|^master|^stable/)
- label=push
actions:
queue:
method: rebase
rebase_fallback: none
name: default

- name: Post a comment on a PR that can not be merged due to a merge conflict
conditions:
- base~=(^main|^master|^stable/)
- conflict
actions:
comment:
message: PR can not be merged due to conflict. Please rebase and resubmit


Let me know if this process of using personal builds and setting 'push' label
if PR is 'up-to-date' is acceptable. I have tested this process with a few
different scenarios in the edk2-codereview repo, and they all worked as expected.

Best regards,

Mike

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Laszlo Ersek
Sent: Monday, June 28, 2021 5:23 AM
To: Kinney, Michael D <michael.d.kinney@...>; devel@edk2.groups.io; spbrogan@...; ardb@...
Cc: Peter Grehan <grehan@...>; Ard Biesheuvel <ardb+tianocore@...>; Justen, Jordan L
<jordan.l.justen@...>; Sean Brogan <sean.brogan@...>; Rebecca Cran <rebecca@...>
Subject: Re: [edk2-devel] [PATCH] OvmfPkg/Bhyve: clean up TPM_ENABLE remnants

On 06/24/21 00:07, Kinney, Michael D wrote:
Hi Laszlo,

I understand your point.

I am trying to balance the ease of use for developers, reducing overhead for maintainers, and
prevent bad commits.

I think you are saying that you want to make sure a maintainer carefully reviews changes
across multiple PRs that are in the same area of code. The CI checks will of course make
sure the code builds and passes the basic boot tests, but those tests do not have full
coverage so an interaction issue between two PRs that pass build and boot but have
unintended behavior side effects are what require detailed manual review.

I am going to remove the auto-rebase by default and add a optional label that can
be set by a maintainer to enable auto-rebase. If a maintainer is confident that
a set of PRs being submitted at the same time with the 'push' label are independent,
then the maintainer can also set 'auto-rebase'. If they are not confident, then
they can send PRs one at a time with only 'push' label and manually rebase each
additional PR and review the manual rebase to make sure there are no unintended
side effects.
Sounds great, thank you!
Laszlo


Any objections to this direction?

Thanks,

Mike

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Laszlo Ersek
Sent: Wednesday, June 23, 2021 12:45 PM
To: Kinney, Michael D <michael.d.kinney@...>; devel@edk2.groups.io; spbrogan@...; ardb@...
Cc: Peter Grehan <grehan@...>; Ard Biesheuvel <ardb+tianocore@...>; Justen, Jordan L
<jordan.l.justen@...>; Sean Brogan <sean.brogan@...>; Rebecca Cran <rebecca@...>
Subject: Re: [edk2-devel] [PATCH] OvmfPkg/Bhyve: clean up TPM_ENABLE remnants

On 06/23/21 20:44, Kinney, Michael D wrote:

Hi Laszlo,

Thank you for the test case.

I created 2 PRs against edk2-codereview using your patches.
I made minor update to commit messages to pass patch check.

https://github.com/tianocore/edk2-codereview/pull/18
https://github.com/tianocore/edk2-codereview/pull/19

Found another issue with PatchCheck for the Mergify merge commit and
fixed that.

Mergify did process #18 and merged it in after passing all CI. Mergify
rebased #19 successfully and merged it after passing all CI. I do not
think this was your expected result.
Indeed, my "desired" result at least would have been for mergify to
reject the rebase.

I looked more closely at the patches you provided. They were not
overlapping in the lines of Readme.rst. This is why no merge conflict
was detected.
More precisely, a contextual conflict *was* determined between the
patches, but that conflict was auto-resolved.

This is risky when done in an automated fashion. It is an extremely
convenient feature of git, when used interactively; that is, when the
auto-merge (automatic conflict resolution) is semantically verified by a
human. Git takes away the chore of conflict resolution, presents a
"likely good" end result, and a human only needs to *look* at the end
result, not *implement* it.

But that "human look" is exactly what's missing from mergify.

Basically what I'd like for mergify is to turn off automatic conflict
resolution.

More or less, speaking in terms of the stand-alone "patch" utility
<https://man7.org/linux/man-pages/man1/patch.1.html>, my preference is
to set the "fuzz factor" to zero.


One way a human reviews such context differences is with git-range-diff.
Continuing my previous example commands:

$ git range-diff --color master..b2 b1..b2-rebase

1: 02dc81e58bd6 ! 1: 2cf39d4b1790 world
@@ -6,8 +6,8 @@
--- a/ReadMe.rst
+++ b/ReadMe.rst
@@
-
A modern, feature-rich, cross-platform firmware development
+ HELLO
environment for the UEFI and PI specifications from www.uefi.org.
+ WORLD

This output shows that the "world" addition is the same (it is identical
between pre-rebase and post-rebase in the commit), but the context has
changed. During the rebase, the leading empty line of the context
disappeared, and a HELLO line in the middle of the leading context
appeared.

This result may or may not be semantically correct; it needs a human
decision. What if the original purpose of the "world" patch author was
to say WORLD but only without HELLO? When they looked at the code, there
was no HELLO yet.

git-range-diff is very powerful, but reading its output takes some
getting used to. (Colorization with the "--color" option is basically
required for understanding; I can't reproduce it in this email, alas.)

I don't want to obsess about this forever, I just want us all to be
aware that this risk exists.

Thanks,
Laszlo


I then created 2 new PRs that added text to the same line # in Readme.rst.

https://github.com/tianocore/edk2-codereview/pull/21
https://github.com/tianocore/edk2-codereview/pull/22

PR #21 passed all CI tests and was merged. Mergify then attempted to
rebase #22 and got a merge conflict and is still in the open state waiting
for the developer to manually handle the merge conflict.
This case is not worrisome; when there is a clear conflict that cannot be auto-resolved, I'm not concerned.

My concern is the sneaky contextual conflict that *appears* auto-resolvable, but is semantically broken. Those things
exist.

Thanks
Laszlo








Possible [BUG] PeiCore: HOBs after cache-as-RAM teardown

Benjamin Doron
 

Hi all,
I'm working on root-causing an issue where I'm unable to retrieve any debug logs after cache-as-RAM teardown on my MinPlatform board port for GSoC 2021. I'm using KabylakeOpenBoardPkg/PeiSerialPortLibSpiFlash.

My best guess at the moment is that it's related to HOBs, which this SerialPortLib uses. If I reinitialise the library stack, it largely works, but a HOB/context structure entry "CurrentWriteOffset" also looks like it's zeroed.

Also, GetFeatureImplemented() in MinPlatformPkg/Test/TestPointCheckLib/PeiTestPointCheckLib.c asserts with "Status = Not Found". Internally, the test point architecture uses HOBs. I don't think this should happen, so, if not for this, I would think that perhaps a pointer in the serial port library needed to be updated.

I can see that the heap is copied over to permanent memory by the PEI core dispatcher (https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c#L881). However, I can't see that the HOB list pointer is updated, so it looks like it (https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Pei/Hob/Hob.c#L44) still has the value that's copied over from the old PEI core's data (https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c#L348).

Does this look like a bug, or might there be something else that I'm missing?

Best regards,
Benjamin


回复: [edk2-devel] [PATCH] MdeModulePkg PiSmmCore: Change MemoryAttributes message to DEBUG_VERBOSE level

gaoliming
 

Reviewed-by: Liming Gao <gaoliming@...>

-----邮件原件-----
发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Yang Gang
发送时间: 2021年7月5日 9:20
收件人: devel@edk2.groups.io
抄送: Eric Dong <eric.dong@...>; Ray Ni <ray.ni@...>; Liming
Gao <gaoliming@...>
主题: [edk2-devel] [PATCH] MdeModulePkg PiSmmCore: Change
MemoryAttributes message to DEBUG_VERBOSE level

1. Reduce the debug message during boot.
2. Update SmmCore debug level of MemoryAttributesTable align to DxeCore.

Signed-off-by: Yang Gang <yanggang@...>
Cc: Eric Dong <eric.dong@...>
Cc: Ray Ni <ray.ni@...>
Cc: Liming Gao <gaoliming@...>
---
.../Core/PiSmmCore/MemoryAttributesTable.c | 26 +++++++++----------
1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c
b/MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c
index de8262ecb9..3e8a80dd7d 100644
--- a/MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c
+++ b/MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c
@@ -1208,10 +1208,10 @@ PublishMemoryAttributesTable (
ASSERT (Status == EFI_BUFFER_TOO_SMALL);

do {
- DEBUG ((DEBUG_INFO, "MemoryMapSize - 0x%x\n",
MemoryMapSize));
+ DEBUG ((DEBUG_VERBOSE, "MemoryMapSize - 0x%x\n",
MemoryMapSize));
MemoryMap = AllocatePool (MemoryMapSize);
ASSERT (MemoryMap != NULL);
- DEBUG ((DEBUG_INFO, "MemoryMap - 0x%x\n", MemoryMap));
+ DEBUG ((DEBUG_VERBOSE, "MemoryMap - 0x%x\n", MemoryMap));

Status = SmmCoreGetMemoryMapMemoryAttributesTable (
&MemoryMapSize,
@@ -1236,19 +1236,19 @@ PublishMemoryAttributesTable (
MemoryAttributesTable->NumberOfEntries =
(UINT32)RuntimeEntryCount;
MemoryAttributesTable->DescriptorSize = (UINT32)DescriptorSize;
MemoryAttributesTable->Reserved = 0;
- DEBUG ((DEBUG_INFO, "MemoryAttributesTable:\n"));
- DEBUG ((DEBUG_INFO, " Version - 0x%08x\n",
MemoryAttributesTable->Version));
- DEBUG ((DEBUG_INFO, " NumberOfEntries - 0x%08x\n",
MemoryAttributesTable->NumberOfEntries));
- DEBUG ((DEBUG_INFO, " DescriptorSize - 0x%08x\n",
MemoryAttributesTable->DescriptorSize));
+ DEBUG ((DEBUG_VERBOSE, "MemoryAttributesTable:\n"));
+ DEBUG ((DEBUG_VERBOSE, " Version - 0x%08x\n",
MemoryAttributesTable->Version));
+ DEBUG ((DEBUG_VERBOSE, " NumberOfEntries - 0x%08x\n",
MemoryAttributesTable->NumberOfEntries));
+ DEBUG ((DEBUG_VERBOSE, " DescriptorSize - 0x%08x\n",
MemoryAttributesTable->DescriptorSize));
MemoryAttributesEntry = (EFI_MEMORY_DESCRIPTOR
*)(MemoryAttributesTable + 1);
for (Index = 0; Index < MemoryMapSize/DescriptorSize; Index++) {
CopyMem (MemoryAttributesEntry, MemoryMap, DescriptorSize);
- DEBUG ((DEBUG_INFO, "Entry (0x%x)\n", MemoryAttributesEntry));
- DEBUG ((DEBUG_INFO, " Type - 0x%x\n",
MemoryAttributesEntry->Type));
- DEBUG ((DEBUG_INFO, " PhysicalStart - 0x%016lx\n",
MemoryAttributesEntry->PhysicalStart));
- DEBUG ((DEBUG_INFO, " VirtualStart - 0x%016lx\n",
MemoryAttributesEntry->VirtualStart));
- DEBUG ((DEBUG_INFO, " NumberOfPages - 0x%016lx\n",
MemoryAttributesEntry->NumberOfPages));
- DEBUG ((DEBUG_INFO, " Attribute - 0x%016lx\n",
MemoryAttributesEntry->Attribute));
+ DEBUG ((DEBUG_VERBOSE, "Entry (0x%x)\n",
MemoryAttributesEntry));
+ DEBUG ((DEBUG_VERBOSE, " Type - 0x%x\n",
MemoryAttributesEntry->Type));
+ DEBUG ((DEBUG_VERBOSE, " PhysicalStart - 0x%016lx\n",
MemoryAttributesEntry->PhysicalStart));
+ DEBUG ((DEBUG_VERBOSE, " VirtualStart - 0x%016lx\n",
MemoryAttributesEntry->VirtualStart));
+ DEBUG ((DEBUG_VERBOSE, " NumberOfPages - 0x%016lx\n",
MemoryAttributesEntry->NumberOfPages));
+ DEBUG ((DEBUG_VERBOSE, " Attribute - 0x%016lx\n",
MemoryAttributesEntry->Attribute));
MemoryAttributesEntry =
NEXT_MEMORY_DESCRIPTOR(MemoryAttributesEntry, DescriptorSize);

MemoryMap = NEXT_MEMORY_DESCRIPTOR(MemoryMap,
DescriptorSize);
@@ -1331,7 +1331,7 @@ SmmInstallMemoryAttributesTable (
{
SmmInstallImageRecord ();

- DEBUG ((DEBUG_INFO, "SMM MemoryProtectionAttribute - 0x%016lx\n",
mMemoryProtectionAttribute));
+ DEBUG ((DEBUG_VERBOSE, "SMM MemoryProtectionAttribute -
0x%016lx\n", mMemoryProtectionAttribute));
if ((mMemoryProtectionAttribute &
EFI_MEMORY_ATTRIBUTES_RUNTIME_MEMORY_PROTECTION_NON_EXEC
UTABLE_PE_DATA) == 0) {
return EFI_SUCCESS;
}
--
2.23.0.windows.1






-=-=-=-=-=-=
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#77464): https://edk2.groups.io/g/devel/message/77464
Mute This Topic: https://groups.io/mt/83989394/4905953
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub
[gaoliming@...]
-=-=-=-=-=-=


回复: [PATCH v2 0/4] Allow EccCheck to run on other repositories

gaoliming
 

This version is good to me. Reviewed-by: Liming Gao
<gaoliming@...>

Thanks
Liming

-----邮件原件-----
发件人: Pierre.Gondois@... <Pierre.Gondois@...>
发送时间: 2021年7月7日 4:56
收件人: devel@edk2.groups.io; Sean Brogan <sean.brogan@...>;
Bret Barkelew <Bret.Barkelew@...>; Michael D Kinney
<michael.d.kinney@...>; Liming Gao <gaoliming@...>;
Sami Mujawar <sami.mujawar@...>
主题: [PATCH v2 0/4] Allow EccCheck to run on other repositories

From: Pierre Gondois <Pierre.Gondois@...>

EccCheck currently makes some assumptions on its working
environment that prevent it from running it in other repositories.
For instance, the workspace is assumed to be pointing to the edk2
repository path, which can be wrong.
This patch-set aims to allow the EccCheck tool to run on the
edk2-platforms repository, with edk2 being a submodule of
edk2-platforms.

The changes can be seen at:
https://github.com/PierreARM/edk2/commits/1628_Ecc_environment_paths
_update_v2

v2:
- Use ';' path separator for Windows host [Liming]

Pierre Gondois (4):
.pytool/EccCheck: Locate BaseTools dir with EDK_TOOLS_PATH
.pytool/EccCheck: Rename edk2_path as workspace_path
.pytool/EccCheck: Check ecc_csv exists
.pytool/EccCheck: Set PACKAGES_PATH env var in Ecc

.pytool/Plugin/EccCheck/EccCheck.py | 46 +++++++++++++++--------------
1 file changed, 24 insertions(+), 22 deletions(-)

--
2.17.1


Re: [PATCH v4 2/3] Acpi: Install Acpi tables for Cloud hypervisor

Jianyong Wu
 

Hi Sami,

Thanks for your rework on my patch. I tried the change and it works well. You can do what you like on the patch set.

Thanks
Jianyong

-----Original Message-----
From: Sami Mujawar <Sami.Mujawar@...>
Sent: Tuesday, July 6, 2021 4:52 PM
To: Jianyong Wu <Jianyong.Wu@...>; devel@edk2.groups.io
Cc: lersek@...; ardb+tianocore@...; Justin He
<Justin.He@...>; nd <nd@...>
Subject: Re: [PATCH v4 2/3] Acpi: Install Acpi tables for Cloud hypervisor

Hi Jianyong,

I should have caught this earlier in my review. However, if you agree, I will
do the following changes before pushing the patch.

1. The subject line of the commit message does not confirm to the edk2
coding standard. It should have ‘ArmVirtPkg: <subject line for the patch>’
2. The ACPI table signature can be simplified further. Can you try the
following and let me know if it works, please?

diff --git a/ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpi.c
b/ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpi.c
index f5a47aa7f3cd..51b012676e7d 100644
--- a/ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpi.c
+++ b/ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpi.c
@@ -52,8 +52,8 @@ FindAcpiTableProtocol ( EFI_STATUS EFIAPI
InstallCloudHvAcpiTables (
- IN EFI_ACPI_TABLE_PROTOCOL *AcpiProtocol
- )
+ IN EFI_ACPI_TABLE_PROTOCOL *AcpiProtocol
+ )
{
UINTN InstalledKey;
UINTN TableSize;
@@ -97,11 +97,12 @@ InstallCloudHvAcpiTables (
//
// Get DSDT from FADT
//
- if (DsdtPtr == NULL
- && !AsciiStrnCmp ((CHAR8 *)&((EFI_ACPI_COMMON_HEADER
*)AcpiTablePtr)->Signature, "FACP", 4)) {
+ if ((DsdtPtr == NULL)
+ && (EFI_ACPI_6_3_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE ==
+ ((EFI_ACPI_COMMON_HEADER *)AcpiTablePtr)->Signature)) {
DsdtPtr = (UINT64 *)(((EFI_ACPI_6_3_FIXED_ACPI_DESCRIPTION_TABLE
*)AcpiTablePtr)->XDsdt);
}
- }
+ } // while

if (DsdtPtr == NULL) {
DEBUG ((DEBUG_ERROR, "%a: no DSDT found\n", __FUNCTION__));

Regards,

Sami Mujawar

On 05/07/2021, 11:07, "Jianyong Wu" <jianyong.wu@...> wrote:

There is no device like Fw-cfg in Qemu in Cloud Hypervisor, so a specific
Acpi handler is introduced here.

The handler implemented here is in a very simple way:
1. acquire the RSDP from the PCD variable in the top ".dsc";
2. get the XSDT address from RSDP structure;
3. get the ACPI tables following the XSDT structure and install them
one by one;
4. get DSDT address from FADT and install DSDT table.

Cc: Laszlo Ersek <lersek@...>
Cc: Sami Mujawar <sami.mujawar@...>

Signed-off-by: Jianyong Wu <jianyong.wu@...>
---
ArmVirtPkg/ArmVirtPkg.dec | 6 +
.../CloudHvAcpiPlatformDxe.inf | 47 ++++++
.../CloudHvAcpiPlatformDxe/CloudHvAcpi.c | 155
++++++++++++++++++
3 files changed, 208 insertions(+)
create mode 100644
ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpiPlatformDxe.inf
create mode 100644 ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpi.c

diff --git a/ArmVirtPkg/ArmVirtPkg.dec b/ArmVirtPkg/ArmVirtPkg.dec
index bf82f7f1f3f2..4e4d758015bc 100644
--- a/ArmVirtPkg/ArmVirtPkg.dec
+++ b/ArmVirtPkg/ArmVirtPkg.dec
@@ -66,6 +66,12 @@ [PcdsFixedAtBuild, PcdsPatchableInModule]
#
gArmVirtTokenSpaceGuid.PcdTerminalTypeGuidBuffer|{0x65, 0x60, 0xA6,
0xDF, 0x19, 0xB4, 0xD3, 0x11, 0x9A, 0x2D, 0x00, 0x90, 0x27, 0x3F, 0xC1,
0x4D}|VOID*|0x00000007

+ ##
+ # This is the physical address of Rsdp which is the core struct of Acpi.
+ # Cloud Hypervisor has no other way to pass Rsdp address to the guest
except use a PCD.
+ #
+
gArmVirtTokenSpaceGuid.PcdCloudHvAcpiRsdpBaseAddress|0x0|UINT64|0
x00000005
+
[PcdsDynamic]
#
# Whether to force disable ACPI, regardless of the fw_cfg settings
diff --git
a/ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpiPlatformDxe.inf
b/ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpiPlatformDxe.inf
new file mode 100644
index 000000000000..01de76486686
--- /dev/null
+++ b/ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpiPlatformDxe.inf
@@ -0,0 +1,47 @@
+## @file
+# ACPI Platform Driver for Cloud Hypervisor
+#
+# Copyright (c) 2021, ARM Limited. All rights reserved.<BR>
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+ INF_VERSION = 0x00010005
+ BASE_NAME = CloudHvgAcpiPlatform
+ FILE_GUID = 6c76e407-73f2-dc1c-938f-5d6c4691ea93
+ MODULE_TYPE = DXE_DRIVER
+ VERSION_STRING = 1.0
+ ENTRY_POINT = CloudHvAcpiPlatformEntryPoint
+
+#
+# The following information is for reference only and not required by the
build tools.
+#
+ VALID_ARCHITECTURES = AARCH64
+#
+
+[Sources]
+ CloudHvAcpi.c
+
+[Packages]
+ MdePkg/MdePkg.dec
+ MdeModulePkg/MdeModulePkg.dec
+ OvmfPkg/OvmfPkg.dec
+ ArmVirtPkg/ArmVirtPkg.dec
+
+[LibraryClasses]
+ BaseLib
+ DebugLib
+ MemoryAllocationLib
+ OrderedCollectionLib
+ UefiBootServicesTableLib
+ UefiDriverEntryPoint
+
+[Protocols]
+ gEfiAcpiTableProtocolGuid # PROTOCOL ALWAYS_CONSUMED
+
+[Pcd]
+ gArmVirtTokenSpaceGuid.PcdCloudHvAcpiRsdpBaseAddress
+
+[Depex]
+ gEfiAcpiTableProtocolGuid
diff --git a/ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpi.c
b/ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpi.c
new file mode 100644
index 000000000000..f5a47aa7f3cd
--- /dev/null
+++ b/ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpi.c
@@ -0,0 +1,155 @@
+/** @file
+ Install Acpi tables for Cloud Hypervisor
+
+ Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#include <Library/BaseLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <IndustryStandard/Acpi63.h>
+#include <Protocol/AcpiTable.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/UefiDriverEntryPoint.h>
+#include <Library/DebugLib.h>
+
+/**
+ Find Acpi table Protocol and return it
+
+ @return AcpiTable Protocol, which is used to handle Acpi Table, on
SUCCESS or NULL on FAILURE.
+
+**/
+STATIC
+EFI_ACPI_TABLE_PROTOCOL *
+FindAcpiTableProtocol (
+ VOID
+ )
+{
+ EFI_STATUS Status;
+ EFI_ACPI_TABLE_PROTOCOL *AcpiTable;
+
+ Status = gBS->LocateProtocol (
+ &gEfiAcpiTableProtocolGuid,
+ NULL,
+ (VOID**)&AcpiTable
+ );
+ ASSERT_EFI_ERROR (Status);
+ return AcpiTable;
+}
+
+/** Install Acpi tables for Cloud Hypervisor
+
+ @param [in] AcpiProtocol Acpi Protocol which is used to install Acpi
talbles
+
+ @return EFI_SUCCESS The table was successfully inserted.
+ @return EFI_INVALID_PARAMETER Either AcpiProtocol, AcpiTablePtr or
DsdtPtr is NULL
+ and the size field embedded in the ACPI table pointed
+ by AcpiTablePtr or DsdtPtr are not in sync.
+ @return EFI_OUT_OF_RESOURCES Insufficient resources exist to
complete the request.
+ @return EFI_NOT_FOUND DSDT table not found.
+**/
+EFI_STATUS
+EFIAPI
+InstallCloudHvAcpiTables (
+ IN EFI_ACPI_TABLE_PROTOCOL *AcpiProtocol
+ )
+{
+ UINTN InstalledKey;
+ UINTN TableSize;
+ UINTN AcpiTableLength;
+ UINT64 RsdpPtr;
+ UINT64 XsdtPtr;
+ UINT64 TableOffset;
+ UINT64 AcpiTablePtr;
+ UINT64 *DsdtPtr = NULL;
+ EFI_STATUS Status;
+
+ if (AcpiProtocol == NULL) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ RsdpPtr = PcdGet64 (PcdCloudHvAcpiRsdpBaseAddress);
+ XsdtPtr = ((EFI_ACPI_6_3_ROOT_SYSTEM_DESCRIPTION_POINTER
*)RsdpPtr)->XsdtAddress;
+ AcpiTableLength = ((EFI_ACPI_COMMON_HEADER *)XsdtPtr)->Length;
+ TableOffset = sizeof (EFI_ACPI_DESCRIPTION_HEADER);
+
+ while (TableOffset < AcpiTableLength) {
+ AcpiTablePtr = *(UINT64 *)(XsdtPtr + TableOffset);
+ TableSize = ((EFI_ACPI_COMMON_HEADER *)AcpiTablePtr)->Length;
+
+ //
+ // Install ACPI tables from XSDT
+ //
+ Status = AcpiProtocol->InstallAcpiTable (
+ AcpiProtocol,
+ (VOID *)AcpiTablePtr,
+ TableSize,
+ &InstalledKey
+ );
+
+ if (EFI_ERROR(Status)) {
+ return Status;
+ }
+
+ TableOffset += sizeof (UINT64);
+
+ //
+ // Get DSDT from FADT
+ //
+ if (DsdtPtr == NULL
+ && !AsciiStrnCmp ((CHAR8 *)&((EFI_ACPI_COMMON_HEADER
*)AcpiTablePtr)->Signature, "FACP", 4)) {
+ DsdtPtr = (UINT64
*)(((EFI_ACPI_6_3_FIXED_ACPI_DESCRIPTION_TABLE *)AcpiTablePtr)-
XDsdt);
+ }
+ }
+
+ if (DsdtPtr == NULL) {
+ DEBUG ((DEBUG_ERROR, "%a: no DSDT found\n", __FUNCTION__));
+ return EFI_NOT_FOUND;
+ }
+
+ //
+ // Install DSDT table
+ //
+ TableSize = ((EFI_ACPI_COMMON_HEADER *)DsdtPtr)->Length;
+ Status = AcpiProtocol->InstallAcpiTable (
+ AcpiProtocol,
+ DsdtPtr,
+ TableSize,
+ &InstalledKey
+ );
+
+ return Status;
+}
+
+/** Entry point for Cloud Hypervisor Platform Dxe
+
+ @param [in] ImageHandle Handle for this image.
+ @param [in] SystemTable Pointer to the EFI system table.
+
+ @return EFI_SUCCESS The table was successfully inserted.
+ @return EFI_INVALID_PARAMETER Either AcpiProtocol, AcpiTablePtr or
DsdtPtr is NULL
+ and the size field embedded in the ACPI table pointed to
+ by AcpiTablePtr or DsdtPtr are not in sync.
+ @return EFI_OUT_OF_RESOURCES Insufficient resources exist to
complete the request.
+ @return EFI_NOT_FOUND DSDT table not found
+**/
+EFI_STATUS
+EFIAPI
+CloudHvAcpiPlatformEntryPoint (
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE *SystemTable
+ )
+{
+ EFI_STATUS Status;
+
+ Status = InstallCloudHvAcpiTables (FindAcpiTableProtocol ());
+
+ if (EFI_ERROR(Status)) {
+ DEBUG ((DEBUG_ERROR, "%a: Fail to install Acpi table: %r\n",
__FUNCTION__,
+ Status));
+ CpuDeadLoop ();
+ }
+
+ return EFI_SUCCESS;
+}
--
2.17.1


回复: [edk2-devel] [PATCH v6 0/2] CryptoPkg/OpensslLib: Add native instruction support for X64

gaoliming
 

Christopher:
Discard COMMON section is added by edk2 commit 214a3b79417f64bf2faae74af42c1b9d23f50dc8. Please help evaluate its impact.

Thanks
Liming

-----邮件原件-----
发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Yao, Jiewen
发送时间: 2021年6月23日 20:42
收件人: Christopher Zurcher <christopher.zurcher@...>;
devel@edk2.groups.io; gaoliming@...
抄送: Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX
<xiaoyux.lu@...>; Kinney, Michael D <michael.d.kinney@...>;
'Ard Biesheuvel' <ard.biesheuvel@...>; Yao, Jiewen
<jiewen.yao@...>
主题: Re: [edk2-devel] [PATCH v6 0/2] CryptoPkg/OpensslLib: Add native
instruction support for X64

Hi Christopher
Thank you very much to resume this work. :-)

I have no problem to approve CryptoPkg. (I believe I already did that in last
year).

But since you updated base tool. We need base tool package maintainer to
approve that change.

A good practice is to split the patch from package level. Then we can let each
package maintainer approve its own package.

I will be waiting for the response from base tool owner to review "
BaseTools/Scripts/GccBase.lds " and give R-B.


Thank you
Yao Jiewen

-----Original Message-----
From: Christopher Zurcher <christopher.zurcher@...>
Sent: Tuesday, June 22, 2021 6:05 AM
To: devel@edk2.groups.io; gaoliming@...
Cc: Yao, Jiewen <jiewen.yao@...>; Wang, Jian J
<jian.j.wang@...>;
Lu, XiaoyuX <xiaoyux.lu@...>; Kinney, Michael D
<michael.d.kinney@...>; 'Ard Biesheuvel'
<ard.biesheuvel@...>
Subject: RE: [edk2-devel] [PATCH v6 0/2] CryptoPkg/OpensslLib: Add native
instruction support for X64

Yes this was discussed last year, sorry for the delay in follow-up, I was
changing
jobs.
The problem is that the assembly code provided by OpenSSL uses
"wrt ..imagebase" which is only supported by win64, not elf64. It was
requested
at the time that I include the OpenSSL-provided .S files as a GCC tool chain
alternative.

Thanks,
Christopher Zurcher

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
gaoliming
Sent: Sunday, June 20, 2021 18:34
To: devel@edk2.groups.io; christopher.zurcher@...
Cc: 'Jiewen Yao' <jiewen.yao@...>; 'Jian J Wang'
<jian.j.wang@...>; 'Xiaoyu Lu' <xiaoyux.lu@...>; 'Mike Kinney'
<michael.d.kinney@...>; 'Ard Biesheuvel'
<ard.biesheuvel@...>
Subject: 回复: [edk2-devel] [PATCH v6 0/2] CryptoPkg/OpensslLib: Add
native
instruction support for X64

Christopher:
Nasm should support GCC tool chain. Do you meet with the problem on
nasm
version assembly code?
So, you have to add GAS assembly code. This topic may be discussed last
year.
Can you give some detail for it?

Thanks
Liming
-----邮件原件-----
发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表
Christopher
Zurcher
发送时间: 2021年6月19日 10:09
收件人: devel@edk2.groups.io
抄送: Jiewen Yao <jiewen.yao@...>; Jian J Wang
<jian.j.wang@...>; Xiaoyu Lu <xiaoyux.lu@...>; Mike Kinney
<michael.d.kinney@...>; Ard Biesheuvel
<ard.biesheuvel@...>
主题: [edk2-devel] [PATCH v6 0/2] CryptoPkg/OpensslLib: Add native
instruction support for X64

From: Christopher Zurcher <christopher.zurcher@...>

V6 Changes:
Add GCC-compatible version of these modifications. Supporting GCC
build
of
native OpenSSL .S files requires removal of *(COMMON) from the
/DISCARD/
section of the GCC linker script.
The VS/CLANG portion of the patch is unchanged from the
previously-approved
patchset.

V5 Changes:
Move ApiHooks.c into X64 folder
Update process_files.pl to clean architecture-specific subfolders
without
removing them
Rebased INF file to merge latest changes regarding RngLib vs.
TimerLib

V4 Changes:
Add copyright header to uefi-asm.conf
Move [Sources.X64] block to cover entire X64-specific config

V3 Changes:
Added definitions for ptrdiff_t and wchar_t to CrtLibSupport.h for
LLVM/Clang build support.
Added -UWIN32 to GCC Flags for LLVM/Clang build support.
Added missing AES GCM assembly file.

V2 Changes:
Limit scope of assembly config to SHA and AES functions.
Removed IA32 native support (reduced config was causing build
failure
and
can be added in a later patch).
Removed XMM instructions from assembly generation.
Added automatic copyright header porting for generated assembly
files.

This patch adds support for building the native instruction algorithms
for the X64 architecture in OpensslLib. The process_files.pl script
was
modified
to parse the .asm file targets from the OpenSSL build config data
struct,
and
generate the necessary assembly files for the EDK2 build environment.

For the X64 variant, OpenSSL includes calls to a Windows error
handling
API,
and that function has been stubbed out in ApiHooks.c.

For all variants, a constructor is added to call the required CPUID
function
within OpenSSL to facilitate processor capability checks in the native
algorithms.

Additional native architecture variants should be simple to add by
following
the changes made for this architecture.

The OpenSSL assembly files are traditionally generated at build time
using
a
perl script. To avoid that burden on EDK2 users, these end-result
assembly files are generated during the configuration steps performed
by the
package
maintainer (through process_files.pl). The perl generator scripts
inside OpenSSL do not parse file comments as they are only meant to
create intermediate build files, so process_files.pl contains
additional hooks to preserve the copyright headers as well as clean up
tabs and line endings
to
comply with EDK2 coding standards. The resulting file headers align
with the generated .h files which are already included in the EDK2
repository.

Cc: Jiewen Yao <jiewen.yao@...>
Cc: Jian J Wang <jian.j.wang@...>
Cc: Xiaoyu Lu <xiaoyux.lu@...>
Cc: Mike Kinney <michael.d.kinney@...>
Cc: Ard Biesheuvel <ard.biesheuvel@...>

Christopher Zurcher (2):
CryptoPkg/OpensslLib: Add native instruction support for X64
CryptoPkg/OpensslLib: Commit the auto-generated assembly files for
X64

BaseTools/Scripts/GccBase.lds
| 1 -
CryptoPkg/CryptoPkg.ci.yaml
| 21 +-
CryptoPkg/Library/Include/CrtLibSupport.h
| 2 +
CryptoPkg/Library/Include/openssl/opensslconf.h
| 3 -
CryptoPkg/Library/OpensslLib/OpensslLib.inf
| 2 +-
CryptoPkg/Library/OpensslLib/OpensslLibConstructor.c
| 44 +
CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
| 2 +-
CryptoPkg/Library/OpensslLib/OpensslLibX64.inf
| 653 +++
CryptoPkg/Library/OpensslLib/OpensslLibX64Gcc.inf
| 653 +++
CryptoPkg/Library/OpensslLib/UefiAsm.conf
| 30 +
CryptoPkg/Library/OpensslLib/X64/ApiHooks.c
| 22 +
CryptoPkg/Library/OpensslLib/X64/crypto/aes/aesni-mb-x86_64.nasm
| 732 +++
CryptoPkg/Library/OpensslLib/X64/crypto/aes/aesni-sha1-x86_64.nasm
| 1916 ++++++++
CryptoPkg/Library/OpensslLib/X64/crypto/aes/aesni-sha256-x86_64.nasm
| 78 +
CryptoPkg/Library/OpensslLib/X64/crypto/aes/aesni-x86_64.nasm
| 5103 ++++++++++++++++++++
CryptoPkg/Library/OpensslLib/X64/crypto/aes/vpaes-x86_64.nasm
| 1173 +++++
CryptoPkg/Library/OpensslLib/X64/crypto/modes/aesni-gcm-x86_64.nasm
| 34 +
CryptoPkg/Library/OpensslLib/X64/crypto/modes/ghash-x86_64.nasm
| 1569 ++++++
CryptoPkg/Library/OpensslLib/X64/crypto/sha/sha1-mb-x86_64.nasm
| 3137 ++++++++++++
CryptoPkg/Library/OpensslLib/X64/crypto/sha/sha1-x86_64.nasm
| 2884 +++++++++++
CryptoPkg/Library/OpensslLib/X64/crypto/sha/sha256-mb-x86_64.nasm
| 3461 +++++++++++++
CryptoPkg/Library/OpensslLib/X64/crypto/sha/sha256-x86_64.nasm
| 3313 +++++++++++++
CryptoPkg/Library/OpensslLib/X64/crypto/sha/sha512-x86_64.nasm
| 1938 ++++++++
CryptoPkg/Library/OpensslLib/X64/crypto/x86_64cpuid.nasm
| 491 ++
CryptoPkg/Library/OpensslLib/X64Gcc/crypto/aes/aesni-mb-x86_64.S
| 552 +++
CryptoPkg/Library/OpensslLib/X64Gcc/crypto/aes/aesni-sha1-x86_64.S
| 1719 +++++++
CryptoPkg/Library/OpensslLib/X64Gcc/crypto/aes/aesni-sha256-x86_64.S
|
69 +
CryptoPkg/Library/OpensslLib/X64Gcc/crypto/aes/aesni-x86_64.S
| 4484 +++++++++++++++++
CryptoPkg/Library/OpensslLib/X64Gcc/crypto/aes/vpaes-x86_64.S
| 863 ++++
CryptoPkg/Library/OpensslLib/X64Gcc/crypto/modes/aesni-gcm-x86_64.S
| 29 +
CryptoPkg/Library/OpensslLib/X64Gcc/crypto/modes/ghash-x86_64.S
| 1386 ++++++
CryptoPkg/Library/OpensslLib/X64Gcc/crypto/sha/sha1-mb-x86_64.S
| 2962 ++++++++++++
CryptoPkg/Library/OpensslLib/X64Gcc/crypto/sha/sha1-x86_64.S
| 2631 ++++++++++
CryptoPkg/Library/OpensslLib/X64Gcc/crypto/sha/sha256-mb-x86_64.S
| 3286 +++++++++++++
CryptoPkg/Library/OpensslLib/X64Gcc/crypto/sha/sha256-x86_64.S
| 3097 ++++++++++++
CryptoPkg/Library/OpensslLib/X64Gcc/crypto/sha/sha512-x86_64.S
| 1811 +++++++
CryptoPkg/Library/OpensslLib/X64Gcc/crypto/x86_64cpuid.S
| 491 ++
CryptoPkg/Library/OpensslLib/process_files.pl
| 241 +-
38 files changed, 50828 insertions(+), 55 deletions(-) create mode
100644 CryptoPkg/Library/OpensslLib/OpensslLibConstructor.c
create mode 100644 CryptoPkg/Library/OpensslLib/OpensslLibX64.inf
create mode 100644
CryptoPkg/Library/OpensslLib/OpensslLibX64Gcc.inf
create mode 100644 CryptoPkg/Library/OpensslLib/UefiAsm.conf
create mode 100644 CryptoPkg/Library/OpensslLib/X64/ApiHooks.c
create mode 100644
CryptoPkg/Library/OpensslLib/X64/crypto/aes/aesni-mb-x86_64.nasm
create mode 100644
CryptoPkg/Library/OpensslLib/X64/crypto/aes/aesni-sha1-x86_64.nasm
create mode 100644
CryptoPkg/Library/OpensslLib/X64/crypto/aes/aesni-sha256-x86_64.nasm
create mode 100644
CryptoPkg/Library/OpensslLib/X64/crypto/aes/aesni-x86_64.nasm
create mode 100644
CryptoPkg/Library/OpensslLib/X64/crypto/aes/vpaes-x86_64.nasm
create mode 100644
CryptoPkg/Library/OpensslLib/X64/crypto/modes/aesni-gcm-x86_64.nasm
create mode 100644
CryptoPkg/Library/OpensslLib/X64/crypto/modes/ghash-x86_64.nasm
create mode 100644
CryptoPkg/Library/OpensslLib/X64/crypto/sha/sha1-mb-x86_64.nasm
create mode 100644
CryptoPkg/Library/OpensslLib/X64/crypto/sha/sha1-x86_64.nasm
create mode 100644
CryptoPkg/Library/OpensslLib/X64/crypto/sha/sha256-mb-x86_64.nasm
create mode 100644
CryptoPkg/Library/OpensslLib/X64/crypto/sha/sha256-x86_64.nasm
create mode 100644
CryptoPkg/Library/OpensslLib/X64/crypto/sha/sha512-x86_64.nasm
create mode 100644
CryptoPkg/Library/OpensslLib/X64/crypto/x86_64cpuid.nasm
create mode 100644
CryptoPkg/Library/OpensslLib/X64Gcc/crypto/aes/aesni-mb-x86_64.S
create mode 100644
CryptoPkg/Library/OpensslLib/X64Gcc/crypto/aes/aesni-sha1-x86_64.S
create mode 100644
CryptoPkg/Library/OpensslLib/X64Gcc/crypto/aes/aesni-sha256-x86_64.S
create mode 100644
CryptoPkg/Library/OpensslLib/X64Gcc/crypto/aes/aesni-x86_64.S
create mode 100644
CryptoPkg/Library/OpensslLib/X64Gcc/crypto/aes/vpaes-x86_64.S
create mode 100644
CryptoPkg/Library/OpensslLib/X64Gcc/crypto/modes/aesni-gcm-x86_64.S
create mode 100644
CryptoPkg/Library/OpensslLib/X64Gcc/crypto/modes/ghash-x86_64.S
create mode 100644
CryptoPkg/Library/OpensslLib/X64Gcc/crypto/sha/sha1-mb-x86_64.S
create mode 100644
CryptoPkg/Library/OpensslLib/X64Gcc/crypto/sha/sha1-x86_64.S
create mode 100644
CryptoPkg/Library/OpensslLib/X64Gcc/crypto/sha/sha256-mb-x86_64.S
create mode 100644
CryptoPkg/Library/OpensslLib/X64Gcc/crypto/sha/sha256-x86_64.S
create mode 100644
CryptoPkg/Library/OpensslLib/X64Gcc/crypto/sha/sha512-x86_64.S
create mode 100644
CryptoPkg/Library/OpensslLib/X64Gcc/crypto/x86_64cpuid.S

--
2.32.0.windows.1













回复: [edk2-devel] [PATCH v5 00/10] Secure Boot default keys

gaoliming
 

Grzegorz Bernacki:
This is a new feature. Can you submit one BZ
(https://bugzilla.tianocore.org/) for it? Then, I can add it into edk2
stable tag feature planning.

Thanks
Liming
-----邮件原件-----
发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Grzegorz
Bernacki
发送时间: 2021年7月1日 17:18
收件人: devel@edk2.groups.io
抄送: leif@...; ardb+tianocore@...;
Samer.El-Haj-Mahmoud@...; sunny.Wang@...;
mw@...; upstream@...; jiewen.yao@...;
jian.j.wang@...; min.m.xu@...; lersek@...;
sami.mujawar@...; afish@...; ray.ni@...;
jordan.l.justen@...; rebecca@...; grehan@...;
thomas.abraham@...; chasel.chiu@...;
nathaniel.l.desimone@...; gaoliming@...;
eric.dong@...; michael.d.kinney@...; zailiang.sun@...;
yi.qian@...; graeme@...; rad@...; pete@...;
Grzegorz Bernacki <gjb@...>
主题: [edk2-devel] [PATCH v5 00/10] Secure Boot default keys

This patchset adds support for initialization of default
Secure Boot variables based on keys content embedded in
flash binary. This feature is active only if Secure Boot
is enabled and DEFAULT_KEY is defined. The patchset
consist also application to enroll keys from default
variables and secure boot menu change to allow user
to reset key content to default values.
Discussion on design can be found at:
https://edk2.groups.io/g/rfc/topic/82139806#600

Built with:
GCC
- RISC-V (U500, U540) [requires fixes in dsc to build]
- Intel (Vlv2TbltDevicePkg (X64/IA32), Quark, MinPlatformPkg,
EmulatorPkg (X64), Bhyve, OvmfPkg (X64/IA32))
- ARM (Sgi75,SbsaQemu,DeveloperBox, RPi3/RPi4)

RISC-V, Quark, Vlv2TbltDevicePkg, Bhyve requires additional fixes to be
built,
will be post on edk2 maillist later

VS2019
- Intel (OvmfPkgX64)

Test with:
GCC5/RPi4
VS2019/OvmfX64 (requires changes to enable feature)

Tests:
1. Try to enroll key in incorrect format.
2. Enroll with only PKDefault keys specified.
3. Enroll with all keys specified.
4. Enroll when keys are enrolled.
5. Reset keys values.
6. Running signed & unsigned app after enrollment.

Changes since v1:
- change names:
SecBootVariableLib => SecureBootVariableLib
SecBootDefaultKeysDxe => SecureBootDefaultKeysDxe
SecEnrollDefaultKeysApp => EnrollFromDefaultKeysApp
- change name of function CheckSetupMode to GetSetupMode
- remove ShellPkg dependecy from EnrollFromDefaultKeysApp
- rebase to master

Changes since v2:
- fix coding style for functions headers in SecureBootVariableLib.h
- add header to SecureBootDefaultKeys.fdf.inc
- remove empty line spaces in SecureBootDefaultKeysDxe files
- revert FAIL macro in EnrollFromDefaultKeysApp
- remove functions duplicates and add SecureBootVariableLib
to platforms which used it

Changes since v3:
- move SecureBootDefaultKeys.fdf.inc to ArmPlatformPkg
- leave duplicate of CreateTimeBasedPayload in PlatformVarCleanupLib
- fix typo in guid description

Changes since v4:
- reorder patches to make it bisectable
- split commits related to more than one platform
- move edk2-platform commits to separate patchset

Grzegorz Bernacki (10):
SecurityPkg: Create library for setting Secure Boot variables.
ArmVirtPkg: add SecureBootVariableLib class resolution
OvmfPkg: add SecureBootVariableLib class resolution
EmulatorPkg: add SecureBootVariableLib class resolution
SecurityPkg: Remove duplicated functions from SecureBootConfigDxe.
ArmPlatformPkg: Create include file for default key content.
SecurityPkg: Add SecureBootDefaultKeysDxe driver
SecurityPkg: Add EnrollFromDefaultKeys application.
SecurityPkg: Add new modules to Security package.
SecurityPkg: Add option to reset secure boot keys.

SecurityPkg/SecurityPkg.dec
| 14 +
ArmVirtPkg/ArmVirt.dsc.inc
| 1 +
EmulatorPkg/EmulatorPkg.dsc
| 1 +
OvmfPkg/Bhyve/BhyveX64.dsc
| 1 +
OvmfPkg/OvmfPkgIa32.dsc
| 1 +
OvmfPkg/OvmfPkgIa32X64.dsc
| 1 +
OvmfPkg/OvmfPkgX64.dsc
| 1 +
SecurityPkg/SecurityPkg.dsc
| 4 +
SecurityPkg/EnrollFromDefaultKeysApp/EnrollFromDefaultKeysApp.inf
| 47 +
SecurityPkg/Library/SecureBootVariableLib/SecureBootVariableLib.inf
| 79 ++

SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigD
xe.inf | 2 +

SecurityPkg/VariableAuthenticated/SecureBootDefaultKeysDxe/SecureBootD
efaultKeysDxe.inf | 45 +
SecurityPkg/Include/Library/SecureBootVariableLib.h
| 251 +++++

SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigN
vData.h | 2 +

SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfig.v
fr | 6 +
SecurityPkg/EnrollFromDefaultKeysApp/EnrollFromDefaultKeysApp.c
| 109 +++
SecurityPkg/Library/SecureBootVariableLib/SecureBootVariableLib.c
| 980 ++++++++++++++++++++

SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigI
mpl.c | 343 ++++---

SecurityPkg/VariableAuthenticated/SecureBootDefaultKeysDxe/SecureBootD
efaultKeysDxe.c | 68 ++
ArmPlatformPkg/SecureBootDefaultKeys.fdf.inc
| 70 ++
SecurityPkg/Library/SecureBootVariableLib/SecureBootVariableLib.uni
| 16 +

SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigS
trings.uni | 4 +

SecurityPkg/VariableAuthenticated/SecureBootDefaultKeysDxe/SecureBootD
efaultKeysDxe.uni | 16 +
23 files changed, 1874 insertions(+), 188 deletions(-)
create mode 100644
SecurityPkg/EnrollFromDefaultKeysApp/EnrollFromDefaultKeysApp.inf
create mode 100644
SecurityPkg/Library/SecureBootVariableLib/SecureBootVariableLib.inf
create mode 100644
SecurityPkg/VariableAuthenticated/SecureBootDefaultKeysDxe/SecureBootD
efaultKeysDxe.inf
create mode 100644 SecurityPkg/Include/Library/SecureBootVariableLib.h
create mode 100644
SecurityPkg/EnrollFromDefaultKeysApp/EnrollFromDefaultKeysApp.c
create mode 100644
SecurityPkg/Library/SecureBootVariableLib/SecureBootVariableLib.c
create mode 100644
SecurityPkg/VariableAuthenticated/SecureBootDefaultKeysDxe/SecureBootD
efaultKeysDxe.c
create mode 100644 ArmPlatformPkg/SecureBootDefaultKeys.fdf.inc
create mode 100644
SecurityPkg/Library/SecureBootVariableLib/SecureBootVariableLib.uni
create mode 100644
SecurityPkg/VariableAuthenticated/SecureBootDefaultKeysDxe/SecureBootD
efaultKeysDxe.uni

--
2.25.1





Re: [PATCH v3] IntelFsp2Pkg: PatchFv parseInfFile function modification

Chiu, Chasel
 

Reviewed-by: Chasel Chiu <chasel.chiu@...>

-----Original Message-----
From: S, Ashraf Ali <ashraf.ali.s@...>
Sent: Wednesday, July 7, 2021 2:42 AM
To: devel@edk2.groups.io
Cc: S, Ashraf Ali <ashraf.ali.s@...>; Ni, Ray <ray.ni@...>; Chiu,
Chasel <chasel.chiu@...>; Desimone, Nathaniel L
<nathaniel.l.desimone@...>; Zeng, Star <star.zeng@...>
Subject: [PATCH v3] IntelFsp2Pkg: PatchFv parseInfFile function modification

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3476

parseInfFile currently reading the EFI_BASE_ADDRESS from INF, once the
address found still it's continues to read the complete inf file which is not
required. once the EFI_BASE_ADDRESS read from the INF no need to read the
INF further.
MSFT compiler can generate the map file address 8 or 16 based on which
architecture the INF is compiler. currently it's support for IA32, modified the
patchfv to support for all.
modification of few typo errors in parseModMapFile, getCurr function required

verification : Working Fine

Signed-off-by: Ashraf Ali S <ashraf.ali.s@...>
Cc: Ray Ni <ray.ni@...>
Cc: Chasel Chiu <chasel.chiu@...>
Cc: Nate DeSimone <nathaniel.l.desimone@...>
Cc: Star Zeng <star.zeng@...>
---
IntelFsp2Pkg/Tools/PatchFv.py | 18 +++++++++++++-----
1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/IntelFsp2Pkg/Tools/PatchFv.py b/IntelFsp2Pkg/Tools/PatchFv.py
index 112de4077a..eb130049b5 100644
--- a/IntelFsp2Pkg/Tools/PatchFv.py
+++ b/IntelFsp2Pkg/Tools/PatchFv.py
@@ -1,6 +1,6 @@
## @ PatchFv.py
#
-# Copyright (c) 2014 - 2019, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2014 - 2021, Intel Corporation. All rights
+reserved.<BR>
# SPDX-License-Identifier: BSD-2-Clause-Patent # ## @@ -304,10 +304,11 @@
class Symbols:
match = re.match("^EFI_BASE_ADDRESS\s*=\s*(0x[a-fA-F0-9]+)", rptLine)
if match is not None:
self.fdBase = int(match.group(1), 16) - fvOffset
+ break
rptLine = fdIn.readline()
fdIn.close()
if self.fdBase == 0xFFFFFFFF:
- raise Exception("Could not find EFI_BASE_ADDRESS in INF file!" % fvFile)
+ raise Exception("Could not find EFI_BASE_ADDRESS in INF
+ file!" % infFile)
return 0

#
@@ -402,6 +403,7 @@ class Symbols:
#
# retval 0 Parsed MOD MAP file successfully
# retval 1 There is no moduleEntryPoint in modSymbols
+ # retval 2 There is no offset for moduleEntryPoint in modSymbols
#
def parseModMapFile(self, moduleName, mapFile):
#
@@ -426,7 +428,7 @@ class Symbols:
else:
#MSFT
#0003:00000190 _gComBase 00007a50 SerialPo
- patchMapFileMatchString = "^\s[0-9a-fA-F]{4}:[0-9a-fA-
F]{8}\s+(\w+)\s+([0-9a-fA-F]{8}\s+)"
+ patchMapFileMatchString = "^\s[0-9a-fA-F]{4}:[0-9a-fA-
F]{8}\s+(\w+)\s+([0-9a-fA-F]{8,16}\s+)"
matchKeyGroupIndex = 1
matchSymbolGroupIndex = 2
prefix = ''
@@ -455,7 +457,13 @@ class Symbols:
continue

if not moduleEntryPoint in modSymbols:
- return 1
+ if matchSymbolGroupIndex == 2:
+ if not '_ModuleEntryPoint' in modSymbols:
+ return 1
+ else:
+ moduleEntryPoint = "_ModuleEntryPoint"
+ else:
+ return 1

modEntry = '%s:%s' % (moduleName,moduleEntryPoint)
if not modEntry in self.dictSymbolAddress:
@@ -498,7 +506,7 @@ class Symbols:
#
# Get current character
#
- # retval elf.string[self.index]
+ # retval self.string[self.index]
# retval '' Exception
#
def getCurr(self):
--
2.30.2.windows.1


Reminder: Community meeting this week

Soumya Guptha
 

Hi Team,

I would like to remind you on the community meeting this Thursday, July 8.

Typically, we have the call during the first week of each month. Due to the long weekend last week, I have moved the call to this Thursday.

Please attend the call.

 

Thanks,

Soumya

 

Soumya Guptha
Firmware Ecosystem Enabling Manager, SFP/IAGS

 


[PATCH v2 4/4] .pytool/EccCheck: Set PACKAGES_PATH env var in Ecc

PierreGondois
 

From: Pierre Gondois <Pierre.Gondois@...>

When running Ecc on other repositories (e.g.: edk2-platforms with
edk2 as a submodule), edk2 modules are referenced.
E.g.: MdePkg/..
The PACKAGES_PATH env var can be used to reference other directories
containing packages. Set it so that Ecc can find these packages.

Cc: Sean Brogan <sean.brogan@...>
Cc: Bret Barkelew <Bret.Barkelew@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Liming Gao <gaoliming@...>
Cc: Sami Mujawar <sami.mujawar@...>
Signed-off-by: Pierre Gondois <Pierre.Gondois@...>
---

Notes:
v2:
- Use ';' path separator for Windows host [Liming]

.pytool/Plugin/EccCheck/EccCheck.py | 1 +
1 file changed, 1 insertion(+)

diff --git a/.pytool/Plugin/EccCheck/EccCheck.py b/.pytool/Plugin/EccCheck/EccCheck.py
index 87f0e65a140f..2d0612269b2e 100644
--- a/.pytool/Plugin/EccCheck/EccCheck.py
+++ b/.pytool/Plugin/EccCheck/EccCheck.py
@@ -67,6 +67,7 @@ class EccCheck(ICiBuildPlugin):
env = shell_environment.GetEnvironment()
env.set_shell_var('PYTHONPATH', python_path)
env.set_shell_var('WORKSPACE', workspace_path)
+ env.set_shell_var('PACKAGES_PATH', os.pathsep.join(Edk2pathObj.PackagePathList))
self.ECC_PASS = True
self.ApplyConfig(pkgconfig, workspace_path, basetools_path, packagename)
modify_dir_list = self.GetModifyDir(packagename)
--
2.17.1


[PATCH v2 3/4] .pytool/EccCheck: Check ecc_csv exists

PierreGondois
 

From: Pierre Gondois <Pierre.Gondois@...>

'workspace_path' being an absolute path leads to 'ecc_csv' being
an absolute path. Then it won't be found among 'file' as they
are relative paths.

Cc: Sean Brogan <sean.brogan@...>
Cc: Bret Barkelew <Bret.Barkelew@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Liming Gao <gaoliming@...>
Cc: Sami Mujawar <sami.mujawar@...>
Signed-off-by: Pierre Gondois <Pierre.Gondois@...>
---
.pytool/Plugin/EccCheck/EccCheck.py | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/.pytool/Plugin/EccCheck/EccCheck.py b/.pytool/Plugin/EccCheck/EccCheck.py
index fff317f23110..87f0e65a140f 100644
--- a/.pytool/Plugin/EccCheck/EccCheck.py
+++ b/.pytool/Plugin/EccCheck/EccCheck.py
@@ -206,11 +206,10 @@ class EccCheck(ICiBuildPlugin):

def ParseEccReport(self, ecc_diff_range: Dict[str, List[Tuple[int, int]]], workspace_path: str) -> None:
ecc_log = os.path.join(workspace_path, "Ecc.log")
- ecc_csv = "Ecc.csv"
- file = os.listdir(workspace_path)
+ ecc_csv = os.path.join(workspace_path, "Ecc.csv")
row_lines = []
ignore_error_code = self.GetIgnoreErrorCode()
- if ecc_csv in file:
+ if os.path.exists(ecc_csv):
with open(ecc_csv) as csv_file:
reader = csv.reader(csv_file)
for row in reader:
--
2.17.1


[PATCH v2 2/4] .pytool/EccCheck: Rename edk2_path as workspace_path

PierreGondois
 

From: Pierre Gondois <Pierre.Gondois@...>

The edk2 path and the workspace path are identical when running
Ecc on edk2. When running Ecc on another repository
(e.g.: edk2-platforms with edk2 as a submodule of edk2-platforms),
these directories are different. Indeed, in the latter configuration,
Ecc must run git commands on the tested repository, i.e. the workspace
directory, edk2-platforms.

Thus, rename edk2_path as workspace_path.

Cc: Sean Brogan <sean.brogan@...>
Cc: Bret Barkelew <Bret.Barkelew@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Liming Gao <gaoliming@...>
Cc: Sami Mujawar <sami.mujawar@...>
Signed-off-by: Pierre Gondois <Pierre.Gondois@...>
---
.pytool/Plugin/EccCheck/EccCheck.py | 32 ++++++++++++++---------------
1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/.pytool/Plugin/EccCheck/EccCheck.py b/.pytool/Plugin/EccCheck/EccCheck.py
index bb93446441de..fff317f23110 100644
--- a/.pytool/Plugin/EccCheck/EccCheck.py
+++ b/.pytool/Plugin/EccCheck/EccCheck.py
@@ -61,19 +61,19 @@ class EccCheck(ICiBuildPlugin):
# - Junit Logger
# - output_stream the StringIO output stream from this plugin via logging
def RunBuildPlugin(self, packagename, Edk2pathObj, pkgconfig, environment, PLM, PLMHelper, tc, output_stream=None):
- edk2_path = Edk2pathObj.WorkspacePath
+ workspace_path = Edk2pathObj.WorkspacePath
basetools_path = environment.GetValue("EDK_TOOLS_PATH")
python_path = os.path.join(basetools_path, "Source", "Python")
env = shell_environment.GetEnvironment()
env.set_shell_var('PYTHONPATH', python_path)
- env.set_shell_var('WORKSPACE', edk2_path)
+ env.set_shell_var('WORKSPACE', workspace_path)
self.ECC_PASS = True
- self.ApplyConfig(pkgconfig, edk2_path, basetools_path, packagename)
+ self.ApplyConfig(pkgconfig, workspace_path, basetools_path, packagename)
modify_dir_list = self.GetModifyDir(packagename)
patch = self.GetDiff(packagename)
- ecc_diff_range = self.GetDiffRange(patch, packagename, edk2_path)
- self.GenerateEccReport(modify_dir_list, ecc_diff_range, edk2_path, basetools_path)
- ecc_log = os.path.join(edk2_path, "Ecc.log")
+ ecc_diff_range = self.GetDiffRange(patch, packagename, workspace_path)
+ self.GenerateEccReport(modify_dir_list, ecc_diff_range, workspace_path, basetools_path)
+ ecc_log = os.path.join(workspace_path, "Ecc.log")
self.RevertCode()
if self.ECC_PASS:
tc.SetSuccess()
@@ -178,24 +178,24 @@ class EccCheck(ICiBuildPlugin):
return comment_range

def GenerateEccReport(self, modify_dir_list: List[str], ecc_diff_range: Dict[str, List[Tuple[int, int]]],
- edk2_path: str, basetools_path: str) -> None:
+ workspace_path: str, basetools_path: str) -> None:
ecc_need = False
ecc_run = True
config = os.path.join(basetools_path, "Source", "Python", "Ecc", "config.ini")
exception = os.path.join(basetools_path, "Source", "Python", "Ecc", "exception.xml")
- report = os.path.join(edk2_path, "Ecc.csv")
+ report = os.path.join(workspace_path, "Ecc.csv")
for modify_dir in modify_dir_list:
- target = os.path.join(edk2_path, modify_dir)
+ target = os.path.join(workspace_path, modify_dir)
logging.info('Run ECC tool for the commit in %s' % modify_dir)
ecc_need = True
ecc_params = "-c {0} -e {1} -t {2} -r {3}".format(config, exception, target, report)
- return_code = RunCmd("Ecc", ecc_params, workingdir=edk2_path)
+ return_code = RunCmd("Ecc", ecc_params, workingdir=workspace_path)
if return_code != 0:
ecc_run = False
break
if not ecc_run:
logging.error('Fail to run ECC tool')
- self.ParseEccReport(ecc_diff_range, edk2_path)
+ self.ParseEccReport(ecc_diff_range, workspace_path)

if not ecc_need:
logging.info("Doesn't need run ECC check")
@@ -204,10 +204,10 @@ class EccCheck(ICiBuildPlugin):
RunCmd("git", revert_params)
return

- def ParseEccReport(self, ecc_diff_range: Dict[str, List[Tuple[int, int]]], edk2_path: str) -> None:
- ecc_log = os.path.join(edk2_path, "Ecc.log")
+ def ParseEccReport(self, ecc_diff_range: Dict[str, List[Tuple[int, int]]], workspace_path: str) -> None:
+ ecc_log = os.path.join(workspace_path, "Ecc.log")
ecc_csv = "Ecc.csv"
- file = os.listdir(edk2_path)
+ file = os.listdir(workspace_path)
row_lines = []
ignore_error_code = self.GetIgnoreErrorCode()
if ecc_csv in file:
@@ -236,10 +236,10 @@ class EccCheck(ICiBuildPlugin):
log.writelines(all_line)
return

- def ApplyConfig(self, pkgconfig: Dict[str, List[str]], edk2_path: str, basetools_path: str, pkg: str) -> None:
+ def ApplyConfig(self, pkgconfig: Dict[str, List[str]], workspace_path: str, basetools_path: str, pkg: str) -> None:
if "IgnoreFiles" in pkgconfig:
for a in pkgconfig["IgnoreFiles"]:
- a = os.path.join(edk2_path, pkg, a)
+ a = os.path.join(workspace_path, pkg, a)
a = a.replace(os.sep, "/")

logging.info("Ignoring Files {0}".format(a))
--
2.17.1


[PATCH v2 1/4] .pytool/EccCheck: Locate BaseTools dir with EDK_TOOLS_PATH

PierreGondois
 

From: Pierre Gondois <Pierre.Gondois@...>

The BaseTools directory is currently being located as a
sub-directory of the WORKSPACE env var. This might not be
true in other environments. Cf EDKII Build Specification,
s4.1.3 "Build Process Restrictions":
There is no restriction on the location of the EDK_TOOLS_PATH,
it may be located within a directory identified as the
WORKSPACE directory, or in any other location that is
accessible on the development workstation.

Locate the BaseTools directory using EDK_TOOLS_PATH instead.

Cc: Sean Brogan <sean.brogan@...>
Cc: Bret Barkelew <Bret.Barkelew@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Liming Gao <gaoliming@...>
Cc: Sami Mujawar <sami.mujawar@...>
Signed-off-by: Pierre Gondois <Pierre.Gondois@...>
---
.pytool/Plugin/EccCheck/EccCheck.py | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/.pytool/Plugin/EccCheck/EccCheck.py b/.pytool/Plugin/EccCheck/EccCheck.py
index eee1ff7a77b5..bb93446441de 100644
--- a/.pytool/Plugin/EccCheck/EccCheck.py
+++ b/.pytool/Plugin/EccCheck/EccCheck.py
@@ -1,5 +1,6 @@
# @file EccCheck.py
#
+# Copyright (c) 2021, Arm Limited. All rights reserved.<BR>
# Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
# SPDX-License-Identifier: BSD-2-Clause-Patent
##
@@ -61,16 +62,17 @@ class EccCheck(ICiBuildPlugin):
# - output_stream the StringIO output stream from this plugin via logging
def RunBuildPlugin(self, packagename, Edk2pathObj, pkgconfig, environment, PLM, PLMHelper, tc, output_stream=None):
edk2_path = Edk2pathObj.WorkspacePath
- python_path = os.path.join(edk2_path, "BaseTools", "Source", "Python")
+ basetools_path = environment.GetValue("EDK_TOOLS_PATH")
+ python_path = os.path.join(basetools_path, "Source", "Python")
env = shell_environment.GetEnvironment()
env.set_shell_var('PYTHONPATH', python_path)
env.set_shell_var('WORKSPACE', edk2_path)
self.ECC_PASS = True
- self.ApplyConfig(pkgconfig, edk2_path, packagename)
+ self.ApplyConfig(pkgconfig, edk2_path, basetools_path, packagename)
modify_dir_list = self.GetModifyDir(packagename)
patch = self.GetDiff(packagename)
ecc_diff_range = self.GetDiffRange(patch, packagename, edk2_path)
- self.GenerateEccReport(modify_dir_list, ecc_diff_range, edk2_path)
+ self.GenerateEccReport(modify_dir_list, ecc_diff_range, edk2_path, basetools_path)
ecc_log = os.path.join(edk2_path, "Ecc.log")
self.RevertCode()
if self.ECC_PASS:
@@ -176,11 +178,11 @@ class EccCheck(ICiBuildPlugin):
return comment_range

def GenerateEccReport(self, modify_dir_list: List[str], ecc_diff_range: Dict[str, List[Tuple[int, int]]],
- edk2_path: str) -> None:
+ edk2_path: str, basetools_path: str) -> None:
ecc_need = False
ecc_run = True
- config = os.path.join(edk2_path, "BaseTools", "Source", "Python", "Ecc", "config.ini")
- exception = os.path.join(edk2_path, "BaseTools", "Source", "Python", "Ecc", "exception.xml")
+ config = os.path.join(basetools_path, "Source", "Python", "Ecc", "config.ini")
+ exception = os.path.join(basetools_path, "Source", "Python", "Ecc", "exception.xml")
report = os.path.join(edk2_path, "Ecc.csv")
for modify_dir in modify_dir_list:
target = os.path.join(edk2_path, modify_dir)
@@ -234,7 +236,7 @@ class EccCheck(ICiBuildPlugin):
log.writelines(all_line)
return

- def ApplyConfig(self, pkgconfig: Dict[str, List[str]], edk2_path: str, pkg: str) -> None:
+ def ApplyConfig(self, pkgconfig: Dict[str, List[str]], edk2_path: str, basetools_path: str, pkg: str) -> None:
if "IgnoreFiles" in pkgconfig:
for a in pkgconfig["IgnoreFiles"]:
a = os.path.join(edk2_path, pkg, a)
@@ -251,7 +253,7 @@ class EccCheck(ICiBuildPlugin):

if "ExceptionList" in pkgconfig:
exception_list = pkgconfig["ExceptionList"]
- exception_xml = os.path.join(edk2_path, "BaseTools", "Source", "Python", "Ecc", "exception.xml")
+ exception_xml = os.path.join(basetools_path, "Source", "Python", "Ecc", "exception.xml")
try:
logging.info("Appending exceptions")
self.AppendException(exception_list, exception_xml)
--
2.17.1


[PATCH v2 0/4] Allow EccCheck to run on other repositories

PierreGondois
 

From: Pierre Gondois <Pierre.Gondois@...>

EccCheck currently makes some assumptions on its working
environment that prevent it from running it in other repositories.
For instance, the workspace is assumed to be pointing to the edk2
repository path, which can be wrong.
This patch-set aims to allow the EccCheck tool to run on the
edk2-platforms repository, with edk2 being a submodule of
edk2-platforms.

The changes can be seen at: https://github.com/PierreARM/edk2/commits/1628_Ecc_environment_paths_update_v2

v2:
- Use ';' path separator for Windows host [Liming]

Pierre Gondois (4):
.pytool/EccCheck: Locate BaseTools dir with EDK_TOOLS_PATH
.pytool/EccCheck: Rename edk2_path as workspace_path
.pytool/EccCheck: Check ecc_csv exists
.pytool/EccCheck: Set PACKAGES_PATH env var in Ecc

.pytool/Plugin/EccCheck/EccCheck.py | 46 +++++++++++++++--------------
1 file changed, 24 insertions(+), 22 deletions(-)

--
2.17.1


[PATCH v3] IntelFsp2Pkg: PatchFv parseInfFile function modification

Ashraf Ali S
 

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3476

parseInfFile currently reading the EFI_BASE_ADDRESS from INF, once the
address found still it's continues to read the complete inf file which
is not required. once the EFI_BASE_ADDRESS read from the INF no need to
read the INF further.
MSFT compiler can generate the map file address 8 or 16 based on which
architecture the INF is compiler. currently it's support for IA32,
modified the patchfv to support for all.
modification of few typo errors in parseModMapFile, getCurr function
required

verification : Working Fine

Signed-off-by: Ashraf Ali S <ashraf.ali.s@...>
Cc: Ray Ni <ray.ni@...>
Cc: Chasel Chiu <chasel.chiu@...>
Cc: Nate DeSimone <nathaniel.l.desimone@...>
Cc: Star Zeng <star.zeng@...>
---
IntelFsp2Pkg/Tools/PatchFv.py | 18 +++++++++++++-----
1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/IntelFsp2Pkg/Tools/PatchFv.py b/IntelFsp2Pkg/Tools/PatchFv.py
index 112de4077a..eb130049b5 100644
--- a/IntelFsp2Pkg/Tools/PatchFv.py
+++ b/IntelFsp2Pkg/Tools/PatchFv.py
@@ -1,6 +1,6 @@
## @ PatchFv.py
#
-# Copyright (c) 2014 - 2019, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2014 - 2021, Intel Corporation. All rights reserved.<BR>
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
##
@@ -304,10 +304,11 @@ class Symbols:
match = re.match("^EFI_BASE_ADDRESS\s*=\s*(0x[a-fA-F0-9]+)", rptLine)
if match is not None:
self.fdBase = int(match.group(1), 16) - fvOffset
+ break
rptLine = fdIn.readline()
fdIn.close()
if self.fdBase == 0xFFFFFFFF:
- raise Exception("Could not find EFI_BASE_ADDRESS in INF file!" % fvFile)
+ raise Exception("Could not find EFI_BASE_ADDRESS in INF file!" % infFile)
return 0

#
@@ -402,6 +403,7 @@ class Symbols:
#
# retval 0 Parsed MOD MAP file successfully
# retval 1 There is no moduleEntryPoint in modSymbols
+ # retval 2 There is no offset for moduleEntryPoint in modSymbols
#
def parseModMapFile(self, moduleName, mapFile):
#
@@ -426,7 +428,7 @@ class Symbols:
else:
#MSFT
#0003:00000190 _gComBase 00007a50 SerialPo
- patchMapFileMatchString = "^\s[0-9a-fA-F]{4}:[0-9a-fA-F]{8}\s+(\w+)\s+([0-9a-fA-F]{8}\s+)"
+ patchMapFileMatchString = "^\s[0-9a-fA-F]{4}:[0-9a-fA-F]{8}\s+(\w+)\s+([0-9a-fA-F]{8,16}\s+)"
matchKeyGroupIndex = 1
matchSymbolGroupIndex = 2
prefix = ''
@@ -455,7 +457,13 @@ class Symbols:
continue

if not moduleEntryPoint in modSymbols:
- return 1
+ if matchSymbolGroupIndex == 2:
+ if not '_ModuleEntryPoint' in modSymbols:
+ return 1
+ else:
+ moduleEntryPoint = "_ModuleEntryPoint"
+ else:
+ return 1

modEntry = '%s:%s' % (moduleName,moduleEntryPoint)
if not modEntry in self.dictSymbolAddress:
@@ -498,7 +506,7 @@ class Symbols:
#
# Get current character
#
- # retval elf.string[self.index]
+ # retval self.string[self.index]
# retval '' Exception
#
def getCurr(self):
--
2.30.2.windows.1


Re: [EXTERNAL] 回复: [edk2-devel] 回复: [edk2][PATCH V3] MdePkg : Add IPMI Macro and Structure Defintions to resolve build errors

manickavasakam karpagavinayagam
 

Yeah I can see the change. Thank you Liming for all your help.

 

-Manic

 

From: gaoliming <gaoliming@...>
Sent: Sunday, July 4, 2021 8:53 PM
To: devel@edk2.groups.io; gaoliming@...; lateefcs@...
Cc: Manickavasakam Karpagavinayagam <manickavasakamk@...>; isaac.w.oram@...; nathaniel.l.desimone@...; Felix Polyudov <Felixp@...>; Harikrishna Doppalapudi <Harikrishnad@...>; Manish Jha <manishj@...>; Zachary Bobroff <zacharyb@...>
Subject: [EXTERNAL]
回复: [edk2-devel] 回复: [edk2][PATCH V3] MdePkg : Add IPMI Macro and Structure Defintions to resolve build errors

 

 

**CAUTION: The e-mail below is from an external source. Please exercise caution before opening attachments, clicking links, or following guidance.**

This patch in MdePkg has been merged at 55dee4947b20103fc48858b18307bd2b114dc145 on edk2 master.

 

Thanks

Liming

发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 gaoliming
发送时间: 2021630 9:22
收件人: devel@edk2.groups.io; gaoliming@...; lateefcs@...
抄送: 'manickavasakam karpagavinayagam' <manickavasakamk@...>; isaac.w.oram@...; nathaniel.l.desimone@...; Felixp@...; Harikrishnad@...; manishj@...; zacharyb@...
主题: 回复: [edk2-devel] 回复: [edk2][PATCH V3] MdePkg : Add IPMI Macro and Structure Defintions to resolve build errors

 

If no other comment, I will create PR to merge this patch.

 

Thanks

Liming

发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 gaoliming
发送时间: 2021624 8:54
收件人: devel@edk2.groups.io; lateefcs@...
抄送: 'manickavasakam karpagavinayagam' <manickavasakamk@...>; isaac.w.oram@...; nathaniel.l.desimone@...; Felixp@...; Harikrishnad@...; manishj@...; zacharyb@...
主题: 回复: [edk2-devel] 回复: [edk2][PATCH V3] MdePkg : Add IPMI Macro and Structure Defintions to resolve build errors

 

AbduL:

 This is same to the definition of UINT8 CompletionCode without bitfield.

 

Thanks

Liming

发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 AbduL Lateef
发送时间: 2021623 19:43
收件人: devel@edk2.groups.io; gaoliming@...
抄送: manickavasakam karpagavinayagam <manickavasakamk@...>; isaac.w.oram@...; nathaniel.l.desimone@...; Felixp@...; Harikrishnad@...; manishj@...; zacharyb@...
主题: Re: [edk2-devel] 回复: [edk2][PATCH V3] MdePkg : Add IPMI Macro and Structure Defintions to resolve build errors

 

whats the use of bitfield:8 for UINT8 variable type?

+typedef struct {
+ UINT8 CompletionCode:8;
+} IPMI_SET_BOOT_OPTIONS_RESPONSE;

 

Thanks

AbduL

 

On Mon, Jun 21, 2021 at 7:19 AM gaoliming <gaoliming@...> wrote:

Thanks for you update.

Reviewed-by: Liming Gao <gaoliming@...>

Thanks
Liming
> -----邮件原件-----
> 发件人: manickavasakam karpagavinayagam <manickavasakamk@...>
> 发送时间: 2021618 23:38
> 收件人: devel@edk2.groups.io
> 抄送: isaac.w.oram@...; nathaniel.l.desimone@...;
> Felixp@...; Harikrishnad@...; manishj@...;
> zacharyb@...; manickavasakamk@...;
> gaoliming@...
> 主题: [edk2][PATCH V3] MdePkg : Add IPMI Macro and Structure Defintions
> to resolve build errors
>
> Build error reported for missing structures
> IPMI_SET_BOOT_OPTIONS_RESPONSE,
> EFI_IPMI_MSG_GET_BMC_EXEC_RSP and
> macros EFI_FIRMWARE_GET_BMC_EXECUTION_CONTEXT
> EFI_FIRMWARE_BMC_IN_FULL_RUNTIME/EFI_FIRMWARE_BMC_IN_FORCE
> D_UPDATE_MODE
> when using
> edk2-platforms\Features\Intel\OutOfBandManagement\IpmiFeaturePkg
>
> MdePkg : Rename IPMI Macro and Structure Defintions
>
> Rename the EFI_IPMI_MSG_GET_BMC_EXEC_RSPB,
> EFI_FIRMWARE_GET_BMC_EXECUTION_CONTEXT
> EFI_FIRMWARE_BMC_IN_FORCED_UPDATE_MODE to
> IPMI_MSG_GET_BMC_EXEC_RSPB,IPMI_GET_BMC_EXECUTION_CONTEXT
> IPMI_BMC_IN_FORCED_UPDATE_MODE
>
> Notes:
> V1 :
> Rename the EFI_IPMI_MSG_GET_BMC_EXEC_RSPB,
> EFI_FIRMWARE_GET_BMC_EXECUTION_CONTEXT
> EFI_FIRMWARE_BMC_IN_FORCED_UPDATE_MODE to
> IPMI_MSG_GET_BMC_EXEC_RSPB,IPMI_GET_BMC_EXECUTION_CONTEXT
> IPMI_BMC_IN_FORCED_UPDATE_MODE
>
> V2:
>
> Remove 0001-MdePkg-Add-IPMI-Macro-and-Structure-Defintions-to-re.patch
>
> V3:
>
> Add Signed-off-by information
>
> Signed-off-by: Manickavasakam Karpagavinayagam
> <manickavasakamk@...>
> ---
>  .../IndustryStandard/IpmiNetFnChassis.h        |  4 ++++
>  .../IndustryStandard/IpmiNetFnFirmware.h       | 18
> ++++++++++++++++++
>  2 files changed, 22 insertions(+)
>
> diff --git a/MdePkg/Include/IndustryStandard/IpmiNetFnChassis.h
> b/MdePkg/Include/IndustryStandard/IpmiNetFnChassis.h
> index 79db55523d..d7cdd3a865 100644
> --- a/MdePkg/Include/IndustryStandard/IpmiNetFnChassis.h
> +++ b/MdePkg/Include/IndustryStandard/IpmiNetFnChassis.h
> @@ -186,6 +186,10 @@ typedef struct {
>    UINT8                                  ParameterData[0];
>
>  } IPMI_SET_BOOT_OPTIONS_REQUEST;
>
>
>
> +typedef struct {
>
> +  UINT8   CompletionCode:8;
>
> +} IPMI_SET_BOOT_OPTIONS_RESPONSE;
>
> +
>
>  //
>
>  //  Definitions for Get System Boot options command
>
>  //
>
> diff --git a/MdePkg/Include/IndustryStandard/IpmiNetFnFirmware.h
> b/MdePkg/Include/IndustryStandard/IpmiNetFnFirmware.h
> index 2d892dbd5a..c4cbe2349b 100644
> --- a/MdePkg/Include/IndustryStandard/IpmiNetFnFirmware.h
> +++ b/MdePkg/Include/IndustryStandard/IpmiNetFnFirmware.h
> @@ -17,4 +17,22 @@
>  // All Firmware commands and their structure definitions to follow here
>
>  //
>
>
>
> +//
----------------------------------------------------------------------------
------------
>
> +//    Definitions for Get BMC Execution Context
>
> +//
----------------------------------------------------------------------------
------------
>
> +#define IPMI_GET_BMC_EXECUTION_CONTEXT  0x23
>
> +
>
> +//
>
> +//  Constants and Structure definitions for "Get Device ID" command to
> follow here
>
> +//
>
> +typedef struct {
>
> +  UINT8   CurrentExecutionContext;
>
> +  UINT8   PartitionPointer;
>
> +} IPMI_MSG_GET_BMC_EXEC_RSP;
>
> +
>
> +//
>
> +// Current Execution Context responses
>
> +//
>
> +#define IPMI_BMC_IN_FORCED_UPDATE_MODE  0x11
>
> +
>
>  #endif
>
> --
> 2.25.0.windows.1
>
>
> Please consider the environment before printing this email.
>
> The information contained in this message may be confidential and
> proprietary to American Megatrends (AMI).  This communication is intended
> to be read only by the individual or entity to whom it is addressed or by
their
> designee. If the reader of this message is not the intended recipient, you
are
> on notice that any distribution of this message, in any form, is strictly
> prohibited.  Please promptly notify the sender by reply e-mail or by
> telephone at 770-246-8600, and then delete or destroy all copies of the
> transmission.




 

--

Thanks and Regards
Abdul Lateef Attar
Bangalore

-The information contained in this message may be confidential and proprietary to American Megatrends (AMI). This communication is intended to be read only by the individual or entity to whom it is addressed or by their designee. If the reader of this message is not the intended recipient, you are on notice that any distribution of this message, in any form, is strictly prohibited. Please promptly notify the sender by reply e-mail or by telephone at 770-246-8600, and then delete or destroy all copies of the transmission.

19361 - 19380 of 96835