On 11/20/2018 10:39 PM, Leif Lindholm wrote:
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.
OK, I will send the patch in next series.
Thanks.
/ Leif