Re: [edk2-devel] [PATCH edk2-platforms] Silicon/AMD/StyxSpiFvDxe: declare missing GUID dependency

2020-01-16 Thread Ard Biesheuvel
On Thu, 16 Jan 2020 at 22:55, Leif Lindholm wrote: > > On Wed, Jan 15, 2020 at 16:44:01 +0100, Ard Biesheuvel wrote: > > StyxSpiFvDxe depends on gEfiEventVirtualAddressChangeGuid, but got > > away with not declaring it in its INF because of a transitive > > dependency. However, this dependency

Re: [edk2-devel] [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: fix 2M->4K page splitting regression for PDEs

2020-01-16 Thread Laszlo Ersek
On 01/16/20 13:22, Ni, Ray wrote: > Laszlo, > Thanks for finding and fixing the bug. > > The code change for 5level paging was done many years ago > before mAddressEncMask was added. > > The two lines of *Pd assignment might be caused when resolving > the local merge conflict. Ah, that makes

[edk2-devel] [PATCH v1] UefiCpuPkg/MpInitLib: Fix possible uninitialized 'InitFlag' field

2020-01-16 Thread Wu, Hao A
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2474 Previous commit d786a17232: UefiCpuPkg/MpInitLib: Reduce the size when loading microcode patches Removed the below assignments for the 'InitFlag' field of CPU_MP_DATA structure in function MpInitLibInitialize() when APs are waken up to do

Re: [edk2-devel] [PATCH edk2-platforms 2/2] Maintainers.txt: update email address for Leif Lindholm

2020-01-16 Thread Michael D Kinney
Reviewed-by: Michael D Kinney > -Original Message- > From: Leif Lindholm > Sent: Tuesday, January 14, 2020 8:33 AM > To: devel@edk2.groups.io > Cc: Andy Hayes ; Ard > Biesheuvel ; Marcin Wojtas > ; Kinney, Michael D > ; Pete Batard > ; Leif Lindholm > Subject: [PATCH edk2-platforms

Re: [edk2-devel] [PATCH v3 0/1] SecurityPkg/BaseHashLib: Implement Unified Hash Calculation API

2020-01-16 Thread Sukerkar, Amol N
Hi Mike, I had an offline conversation with Jian and, now, I agree on the point that bitmap will not optimize the API. I will work on changing back to switch..case. Thanks, Amol -Original Message- From: Sukerkar, Amol N Sent: Wednesday, January 15, 2020 9:26 PM To: Wang, Jian J ;

Re: [edk2-devel] [PATCH v2 1/1] SecurityPkg/BaseHashLib: Implement Unified Hash Calculation API

2020-01-16 Thread Sukerkar, Amol N
Thanks, Liming! ~ Amol -Original Message- From: Gao, Liming Sent: Thursday, January 16, 2020 5:31 PM To: Sukerkar, Amol N ; devel@edk2.groups.io; Wang, Jian J Cc: Kinney, Michael D ; Yao, Jiewen ; Agrawal, Sachin ; Musti, Srinivas Subject: RE: [edk2-devel] [PATCH v2 1/1]

[edk2-devel] [Patch 1/1] BaseTools: Fixed a incremental build bug

2020-01-16 Thread Bob Feng
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2451 If removing a header file from source code and file system, the incremental build will fail. This patch is to fix this issue. Cc: Liming Gao Signed-off-by: Bob Feng --- BaseTools/Source/Python/AutoGen/GenMake.py | 9 +++--

Re: [edk2-devel] [PATCH v3 1/2] CryptoPkg/BaseCryptLib: replace HmacXxxInit API with HmacXxxSetKey

2020-01-16 Thread Wang, Jian J
Laszlo, Thanks for catching the problem. I'll fix them before push. Regards, Jian > -Original Message- > From: Laszlo Ersek > Sent: Thursday, January 16, 2020 5:08 PM > To: Wang, Jian J ; devel@edk2.groups.io > Cc: Lu, XiaoyuX > Subject: Re: [PATCH v3 1/2] CryptoPkg/BaseCryptLib:

[edk2-devel] [PATCH V2] BaseTools:fix Ecc tool issue for check StructPcd

2020-01-16 Thread Fan, ZhijuX
BZ:https://bugzilla.tianocore.org/show_bug.cgi?id=2142 gAdvancedFeaturePkgTokenSpaceGuid.PcdSmbiosType0BiosInformation| {0x0}|SMBIOS_TABLE_TYPE0|0x8001 { IndustryStandard/SmBios.h MdePkg/MdePkg.dec AdvancedFeaturePkg/AdvancedFeaturePkg.dec } If there's a

Re: [edk2-devel] [PATCH] BaseTools/Conf/gitattributes: fix "--function-context" for *.h and *.c

2020-01-16 Thread Leif Lindholm
Liming, Hence the "possibly" :) Although, I agree, grouping it with .aslc (which definitely *is* C), was a bit confusing. I couldn't actually find any .act in the tree - are those obsolete, or do we simply not have any in the tree? (If the latter, should we add something in order to support

Re: [edk2-devel] [PATCH] BaseTools/Conf/gitattributes: fix "--function-context" for *.h and *.c

2020-01-16 Thread Liming Gao
Leif: .act is same to .aslc. There are no cases in open source. Thanks Liming -Original Message- From: Leif Lindholm Sent: 2020年1月17日 8:51 To: Gao, Liming Cc: Laszlo Ersek ; devel@edk2.groups.io; Feng, Bob C Subject: Re: [PATCH] BaseTools/Conf/gitattributes: fix

Re: [edk2-devel] [edk2-staging/EdkRepo] [PATCH] EdkRepo: Remove unused string

2020-01-16 Thread Bjorge, Erik C
Reviewed-by: Erik Bjorge -Original Message- From: devel@edk2.groups.io On Behalf Of Nate DeSimone Sent: Thursday, January 16, 2020 4:47 PM To: Desimone, Ashley E ; devel@edk2.groups.io Cc: Pandya, Puja ; Bjorge, Erik C ; Bret Barkelew Subject: Re: [edk2-devel] [edk2-staging/EdkRepo]

Re: [edk2-devel][PATCH 0/3] Fmp Capsule Dependency implementation.

2020-01-16 Thread Xu, Wei6
Sorry. I haven't prepared a Wiki for it. But I can make one as soon as possible. BR, Wei -Original Message- From: Gao, Liming Sent: Friday, January 17, 2020 8:32 AM To: devel@edk2.groups.io; Gao, Liming ; Xu, Wei6 Subject: RE: [edk2-devel][PATCH 0/3] Fmp Capsule Dependency

Re: [edk2-devel] [edk2-staging/EdkRepo] [PATCH] EdkRepo: Remove unused string

2020-01-16 Thread Nate DeSimone
Reviewed-by: Nate DeSimone -Original Message- From: Desimone, Ashley E Sent: Friday, January 10, 2020 1:44 PM To: devel@edk2.groups.io Cc: Desimone, Nathaniel L ; Pandya, Puja ; Bjorge, Erik C ; Bret Barkelew Subject: [edk2-staging/EdkRepo] [PATCH] EdkRepo: Remove unused string

Re: [edk2-devel] [edk2-platform] FitGen: Fix the issue to run in X64 linux machine

2020-01-16 Thread Liming Gao
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2466 -Original Message- From: devel@edk2.groups.io On Behalf Of Liming Gao Sent: 2020年1月16日 15:11 To: devel@edk2.groups.io Cc: Oram, Isaac W Subject: [edk2-devel] [edk2-platform] FitGen: Fix the issue to run in X64 linux machine Cc:

Re: [edk2-devel][PATCH 0/3] Fmp Capsule Dependency implementation.

2020-01-16 Thread Liming Gao
Wei: Is there Wiki page to introduce how to enable Capsule Dependency and how to verify Capsule with Dependency? Thanks Liming -Original Message- From: devel@edk2.groups.io On Behalf Of Liming Gao Sent: 2020年1月16日 21:55 To: Xu, Wei6 ; devel@edk2.groups.io Subject: Re:

Re: [edk2-devel] [PATCH v2 1/1] SecurityPkg/BaseHashLib: Implement Unified Hash Calculation API

2020-01-16 Thread Liming Gao
Amol: I just add it into edk2 2020 Q1 stable tag feature planning. Thanks Liming -Original Message- From: Sukerkar, Amol N Sent: 2020年1月17日 0:58 To: Gao, Liming ; devel@edk2.groups.io; Wang, Jian J Cc: Kinney, Michael D ; Yao, Jiewen ; Agrawal, Sachin ; Musti, Srinivas ;

Re: [edk2-devel] [PATCH] BaseTools/Conf/gitattributes: fix "--function-context" for *.h and *.c

2020-01-16 Thread Liming Gao
Leif: Vfr is not C style source file. It can't be regarded as C source file. Laszlo: Is there such support for the assembly file, such as .nasm? Thanks Liming -Original Message- From: Leif Lindholm Sent: 2020年1月17日 5:55 To: Laszlo Ersek Cc: devel@edk2.groups.io; Feng, Bob C ;

Re: [edk2-devel] [PATCH 1/2] Maintainers.txt: update email address for Leif Lindholm

2020-01-16 Thread Leif Lindholm
On Thu, Jan 16, 2020 at 19:54:43 +0100, Philippe Mathieu-Daudé wrote: > > Phil, would it make sense for us to ask Leif to post an update to > > ".mailmap"? > > > > For example, what happens if we run "git shortlog" over a period that > > contains patches authored by *both* of Leif's email

Re: [edk2-devel] [PATCH] BaseTools/Conf/gitattributes: fix "--function-context" for *.h and *.c

2020-01-16 Thread Leif Lindholm
On Thu, Jan 16, 2020 at 19:49:29 +0100, Laszlo Ersek wrote: > The "--function-context" ("-W") option of git-diff displays the entire > body of a modified function, not just small modified hunks within the > function. It is useful for reviewers when the code changes to the function > are small, but

Re: [edk2-devel] [PATCH edk2-platforms] Silicon/AMD/StyxSpiFvDxe: declare missing GUID dependency

2020-01-16 Thread Leif Lindholm
On Wed, Jan 15, 2020 at 16:44:01 +0100, Ard Biesheuvel wrote: > StyxSpiFvDxe depends on gEfiEventVirtualAddressChangeGuid, but got > away with not declaring it in its INF because of a transitive > dependency. However, this dependency got dropped in core EDK2, > resulting in build failures of the

[edk2-devel] [PATCH 10/11] SecurityPkg/DxeImageVerificationHandler: fix imgexec info on memalloc fail

2020-01-16 Thread Laszlo Ersek
It makes no sense to call AddImageExeInfo() with (Signature == NULL) and (SignatureSize > 0). AddImageExeInfo() does not crash in such a case -- it avoids the CopyMem() call --, but it creates an invalid EFI_IMAGE_EXECUTION_INFO record. Namely, the "EFI_IMAGE_EXECUTION_INFO.InfoSize" field

[edk2-devel] [PATCH 09/11] SecurityPkg/DxeImageVerificationHandler: fix retval for (FileBuffer==NULL)

2020-01-16 Thread Laszlo Ersek
"FileBuffer" is a non-optional input (pointer) parameter to DxeImageVerificationHandler(). Normally, when an edk2 function receives a NULL argument for such a parameter, we return EFI_INVALID_PARAMETER or RETURN_INVALID_PARAMETER. However, those don't conform to the

[edk2-devel] [PATCH 08/11] SecurityPkg/DxeImageVerificationHandler: eliminate "Status" variable

2020-01-16 Thread Laszlo Ersek
The "Status" variable is set to EFI_ACCESS_DENIED at the top of the function. Then it is overwritten with EFI_SECURITY_VIOLATION under the "Failed" (earlier: "Done") label. We finally return "Status". The above covers the complete usage of "Status" in DxeImageVerificationHandler(). Remove the

[edk2-devel] [PATCH 00/11] SecurityPkg/DxeImageVerificationHandler: fix retval for "deny" policy

2020-01-16 Thread Laszlo Ersek
Ref:https://bugzilla.tianocore.org/show_bug.cgi?id=2129 Repo: https://github.com/lersek/edk2.git Branch: deny_execute_2129 The DxeImageVerificationHandler() function does not handle the DENY_EXECUTE_ON_SECURITY_VIOLATION policy correctly. When an image is rejected, and the platform sets

[edk2-devel] [PATCH 04/11] SecurityPkg/DxeImageVerificationHandler: narrow down PE/COFF hash status

2020-01-16 Thread Laszlo Ersek
Inside the "for" loop that scans the signatures of the image, we call HashPeImageByType(), and assign its return value to "Status". Beyond the immediate retval check, this assignment is useless (never consumed). That's because a subsequent access to "Status" may only be one of the following: -

[edk2-devel] [PATCH 11/11] SecurityPkg/DxeImageVerificationHandler: fix "defer" vs. "deny" policies

2020-01-16 Thread Laszlo Ersek
In DxeImageVerificationHandler(), we should return EFI_SECURITY_VIOLATION for a rejected image only if the platform sets DEFER_EXECUTE_ON_SECURITY_VIOLATION as the policy for the image's source. Otherwise, EFI_ACCESS_DENIED must be returned. Right now, EFI_SECURITY_VIOLATION is returned for all

[edk2-devel] [PATCH 01/11] SecurityPkg/DxeImageVerificationHandler: simplify "VerifyStatus"

2020-01-16 Thread Laszlo Ersek
In the DxeImageVerificationHandler() function, the "VerifyStatus" variable can only contain one of two values: EFI_SUCCESS and EFI_ACCESS_DENIED. Furthermore, the variable is only consumed with EFI_ERROR(). Therefore, using the EFI_STATUS type for the variable is unnecessary. Worse, given the

[edk2-devel] [PATCH 06/11] SecurityPkg/DxeImageVerificationHandler: remove superfluous Status setting

2020-01-16 Thread Laszlo Ersek
After the final "IsVerified" check, we set "Status" to EFI_ACCESS_DENIED. This is superfluous, as "Status" already carries EFI_ACCESS_DENIED value there, from the top of the function. Remove the assignment. Functionally, this change is a no-op. Cc: Chao Zhang Cc: Jian J Wang Cc: Jiewen Yao

[edk2-devel] [PATCH 07/11] SecurityPkg/DxeImageVerificationHandler: unnest AddImageExeInfo() call

2020-01-16 Thread Laszlo Ersek
Before the "Done" label at the end of DxeImageVerificationHandler(), we now have a single access to "Status": we set "Status" to EFI_ACCESS_DENIED at the top of the function. Therefore, the (Status != EFI_SUCCESS) condition is always true under the "Done" label. Accordingly, unnest the

[edk2-devel] [PATCH 05/11] SecurityPkg/DxeImageVerificationHandler: fix retval on memalloc failure

2020-01-16 Thread Laszlo Ersek
A SECURITY2_FILE_AUTHENTICATION_HANDLER function is not expected to return EFI_OUT_OF_RESOURCES. We should only return EFI_SUCCESS, EFI_SECURITY_VIOLATION, or EFI_ACCESS_DENIED. In case we run out of memory while preparing "SignatureList" for AddImageExeInfo(), we should simply stick with the

[edk2-devel] [PATCH 02/11] SecurityPkg/DxeImageVerificationHandler: remove "else" after return/break

2020-01-16 Thread Laszlo Ersek
In the code structure if (condition) { // // block1 // return; } else { // // block2 // } nesting "block2" in an "else" branch is superfluous, and harms readability. It can be transformed to: if (condition) { // // block1 // return; } //

[edk2-devel] [PATCH 03/11] SecurityPkg/DxeImageVerificationHandler: keep PE/COFF info status internal

2020-01-16 Thread Laszlo Ersek
The PeCoffLoaderGetImageInfo() function may return various error codes, such as RETURN_INVALID_PARAMETER and RETURN_UNSUPPORTED. Such error values should not be assigned to our "Status" variable in the DxeImageVerificationHandler() function, because "Status" generally stands for the main exit

Re: [edk2-devel] [PATCH 1/2] Maintainers.txt: update email address for Leif Lindholm

2020-01-16 Thread Philippe Mathieu-Daudé
On 1/14/20 9:27 PM, Laszlo Ersek wrote: CC Phil On 01/14/20 17:32, Leif Lindholm wrote: Leif now works at NUVIA Inc, update email address accordingly. Cc: Andrew Fish Cc: Ard Biesheuvel Cc: Laszlo Ersek Cc: Michael D Kinney Cc: Ray Ni Cc: Zhichao Gao Cc: Leif Lindholm Signed-off-by:

[edk2-devel] [PATCH] BaseTools/Conf/gitattributes: fix "--function-context" for *.h and *.c

2020-01-16 Thread Laszlo Ersek
The "--function-context" ("-W") option of git-diff displays the entire body of a modified function, not just small modified hunks within the function. It is useful for reviewers when the code changes to the function are small, but they could affect, or depend on, control flow that is far away in

Re: [edk2-devel] [PATCH v2 1/1] SecurityPkg/BaseHashLib: Implement Unified Hash Calculation API

2020-01-16 Thread Sukerkar, Amol N
Sure, Liming! Please add edk2 2020 Q1 stable tag to this feature. I will follow the schedule. Thanks, Amol -Original Message- From: Gao, Liming Sent: Wednesday, January 15, 2020 9:36 PM To: Sukerkar, Amol N ; devel@edk2.groups.io; Wang, Jian J Cc: Kinney, Michael D ; Yao, Jiewen ;

Re: [edk2-devel][PATCH 0/3] Fmp Capsule Dependency implementation.

2020-01-16 Thread Liming Gao
Wei: Thanks for your update. I think those unit tests are enough. Liming > -Original Message- > From: Xu, Wei6 > Sent: Thursday, January 16, 2020 9:31 PM > To: Gao, Liming ; devel@edk2.groups.io > Subject: RE: [edk2-devel][PATCH 0/3] Fmp Capsule Dependency implementation. > > Hi

Re: [edk2-devel][PATCH 0/3] Fmp Capsule Dependency implementation.

2020-01-16 Thread Xu, Wei6
Hi Liming, Thanks a lot for review. We did following test: On OVMF: 1. Unit test to verify the handling of all op-codes. 2. Unit test for updated APIs 3. Verify capsule update process with satisfied/unsatisfied/invalid dependency. On real dev machine: 1. Update Fmp Device which supports

Re: [edk2-devel][PATCH 0/3] Fmp Capsule Dependency implementation.

2020-01-16 Thread Liming Gao
Wei: The code change looks good. Reviewed-by: Liming Gao Please share which test have been done. Thanks Liming > -Original Message- > From: devel@edk2.groups.io On Behalf Of Xu, Wei6 > Sent: Friday, January 10, 2020 1:35 PM > To: devel@edk2.groups.io > Subject:

Re: [edk2-devel] [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: fix 2M->4K page splitting regression for PDEs

2020-01-16 Thread Ni, Ray
Laszlo, Thanks for finding and fixing the bug. The code change for 5level paging was done many years ago before mAddressEncMask was added. The two lines of *Pd assignment might be caused when resolving the local merge conflict. Reviewed-by: Ray Ni > -Original Message- > From: Laszlo

Re: [edk2-devel] [PATCH v3 2/2] CryptoPkg/BaseCryptLib: remove HmacXxxGetContextSize interface

2020-01-16 Thread Laszlo Ersek
On 01/16/20 07:10, Jian J Wang wrote: > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1792 > > Hmac(Md5|Sha1|Sha256)GetContextSize() use a deprecated macro > HMAC_MAX_MD_CBLOCK defined in openssl. They should be dropped to > avoid misuses in the future. For context allocation and release, >

Re: [edk2-devel] [PATCH v3 1/2] CryptoPkg/BaseCryptLib: replace HmacXxxInit API with HmacXxxSetKey

2020-01-16 Thread Laszlo Ersek
Hi Jian, On 01/16/20 07:10, Jian J Wang wrote: > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1792 > > HmacXxxInit() is supposed to be initialize user supplied buffer as HMAC > context, as well as user supplied key. Currently it has no real use cases. > > Due to BZ1792, the user has no

Re: [edk2-devel] [PATCH] UefiCpuPkg/Library/MpInitLib: Remove BSP index == 0 Assumption.

2020-01-16 Thread Laszlo Ersek
On 01/16/20 04:15, Dong, Eric wrote: > Hi Laszlo, > >> -Original Message- >> From: Laszlo Ersek [mailto:ler...@redhat.com] >> Sent: Wednesday, January 15, 2020 6:05 PM >> To: Dong, Eric ; devel@edk2.groups.io >> Cc: Ni, Ray >> Subject: Re: [PATCH] UefiCpuPkg/Library/MpInitLib: Remove BSP