Re: [PATCH edk2-platforms v3 1/5] Hisilicon/D0x: Fix secure boot bug in FlashFvbDxe

[Leif Lindholm <leif.lindholm@linaro.org>]

On Tue, Nov 20, 2018 at 10:29:57PM +0800, Ming Huang wrote:
> >>> 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.

OK, I will ask a third time: what _problem_ does this solve?
What is the symptom?
When someone uses the buggy firmware, what does not work for them?
This information _always_ needs to be in the commit message.

> >> 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.

If there is not enough time to implement a real PlatformSecureLib,
there is no need to have Secure Boot at all. Same thing goes for
secure variable store (to hardware devices that are not accessible
from Non-secure world).

> This patch will add when we implement real secure boot in future.

That sounds like the best thing to do.

Meanwhile, could you create a patch to get rid of the SECURE_BOOT
options completely from the .dsc/.fdf please? I don't like having it
in there when we know it doesn't build.

/
    Leif