On 11/20/2018 8:58 PM, Leif Lindholm wrote:
On Tue, Nov 20, 2018 at 08:40:28PM +0800, Ming Huang wrote:
On 11/20/2018 8:13 PM, Leif Lindholm wrote:
On Tue, Nov 20, 2018 at 05:01:46PM +0800, Ming Huang wrote:
Now that the generic Variable Runtime DXE code no longer distinguishes between gEfiVariableGuid and gEfiAuthenticatedVariableGuid in the varstore FV header. We can relax the check in the flashFvb driver to accept either GUID regardless of whether we are running a secure boot capable build or not.
We are still in a situation where D06 is not buildable with Secure Boot enabled though.
If you build with -D SECURE_BOOT_ENABLE=TRUE, you still end up with /work/git/edk2-platforms/Platform/Hisilicon/D06/D06.dsc(...): error 4000: Instance of library class [PlatformSecureLib] is not found in [/work/git/edk2/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf] [AARCH64] consumed by module [/work/git/edk2/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf]
And all Hisilicon platforms still use AuthVariableLib|MdeModulePkg/Library/AuthVariableLibNull/AuthVariableLibNull.inf regardless of Secure Boot setting.
So what problem does this patch solve? A runtime one?
This patch solve bug in FlashFvbDxe.
Yes, but what bug? What is the symptom? What _specific_ problem goes away by adding this patch? That information should have been in the original commit message. I have no information available to me as I now build -rc1 to suggest that this patch should be included.
The bug is that gEfiAuthenticatedVariableGuid should be used in FlashFvbDxe, not gEfiVariableGuid when enable secure boot.
Should I add a patch before this patch to solve build error with -D SECURE_BOOT_ENABLE=TRUE in v4?
That would require a sane implementation of PlatformSecureLib, implementing a real UserPhysicalPresent(). Can you turn that around within the next few hours?
My original idea is using OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib. There is not enough time to implement a real UserPhysicalPresent. This patch will add when we implement real secure boot in future.
Thanks.
Regards,
Leif
Thanks.
/ Leif
Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ming Huang ming.huang@linaro.org
Silicon/Hisilicon/Drivers/FlashFvbDxe/FlashFvbDxe.inf | 1 + Silicon/Hisilicon/Drivers/FlashFvbDxe/FlashFvbDxe.c | 5 +++-- 2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/Silicon/Hisilicon/Drivers/FlashFvbDxe/FlashFvbDxe.inf b/Silicon/Hisilicon/Drivers/FlashFvbDxe/FlashFvbDxe.inf index f8be4741ef7c..a0226e0d87c0 100644 --- a/Silicon/Hisilicon/Drivers/FlashFvbDxe/FlashFvbDxe.inf +++ b/Silicon/Hisilicon/Drivers/FlashFvbDxe/FlashFvbDxe.inf @@ -44,6 +44,7 @@ [LibraryClasses] UefiRuntimeLib [Guids]
- gEfiAuthenticatedVariableGuid gEfiSystemNvDataFvGuid gEfiVariableGuid
diff --git a/Silicon/Hisilicon/Drivers/FlashFvbDxe/FlashFvbDxe.c b/Silicon/Hisilicon/Drivers/FlashFvbDxe/FlashFvbDxe.c index e18cc9e06ec2..12baed41cd4e 100644 --- a/Silicon/Hisilicon/Drivers/FlashFvbDxe/FlashFvbDxe.c +++ b/Silicon/Hisilicon/Drivers/FlashFvbDxe/FlashFvbDxe.c @@ -189,7 +189,7 @@ InitializeFvAndVariableStoreHeaders ( // VARIABLE_STORE_HEADER // VariableStoreHeader = (VARIABLE_STORE_HEADER*)((UINTN)Headers + (UINTN)FirmwareVolumeHeader->HeaderLength);
- CopyGuid (&VariableStoreHeader->Signature, &gEfiVariableGuid);
- CopyGuid (&VariableStoreHeader->Signature, &gEfiAuthenticatedVariableGuid); VariableStoreHeader->Size = PcdGet32(PcdFlashNvStorageVariableSize) - FirmwareVolumeHeader->HeaderLength; VariableStoreHeader->Format = VARIABLE_STORE_FORMATTED; VariableStoreHeader->State = VARIABLE_STORE_HEALTHY;
@@ -258,7 +258,8 @@ ValidateFvHeader ( VariableStoreHeader = (VARIABLE_STORE_HEADER*)((UINTN)FwVolHeader + (UINTN)FwVolHeader->HeaderLength); // Check the Variable Store Guid
- if ( CompareGuid (&VariableStoreHeader->Signature, &gEfiVariableGuid) == FALSE )
- if (!CompareGuid (&VariableStoreHeader->Signature, &gEfiVariableGuid) &&
!CompareGuid (&VariableStoreHeader->Signature, &gEfiAuthenticatedVariableGuid))-- 2.9.5