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 <[email protected]>
>>>> ---
>>>> 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))
>>>> {
>>>> DEBUG ((EFI_D_ERROR, "ValidateFvHeader: Variable Store Guid
>>>> non-compatible\n"));
>>>> return EFI_NOT_FOUND;
>>>> --
>>>> 2.9.5
>>>>
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel
