On 06/30/21 14:34, Grzegorz Bernacki wrote: > The edk2 patch > SecurityPkg: Create library for setting Secure Boot variables. > > removes generic functions from SecureBootConfigDxe and places > them into SecureBootVariableLib. This patch adds SecureBootVariableLib > mapping for ArmVirtPkg platform. > > Signed-off-by: Grzegorz Bernacki <g...@semihalf.com> > --- > ArmVirtPkg/ArmVirt.dsc.inc | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc > index d9abadbe70..11c1f53537 100644 > --- a/ArmVirtPkg/ArmVirt.dsc.inc > +++ b/ArmVirtPkg/ArmVirt.dsc.inc > @@ -168,6 +168,7 @@ > # > !if $(SECURE_BOOT_ENABLE) == TRUE > AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf > + > SecureBootVariableLib|SecurityPkg/Library/SecureBootVariableLib/SecureBootVariableLib.inf > > # re-use the UserPhysicalPresent() dummy implementation from the ovmf tree > PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf >
I know a new version is coming up, but one comment still: you should please make this series bisectable. That is, the series should build at every stage. That implies the following approach (each step below corresponds to a patch, or to a sequence of patches): - introduce the new library (class and instance(s)) first, in isolation; this will duplicate the internal functions of SecureBootConfigDxe - add lib class resolution(s) to all platforms in edk2 (and edk2-platforms, possibly) that include SecureBootConfigDxe - replace the internal functions of SecureBootConfigDxe with the new library dependency. Right now, ArmVirtPkg platforms will definitely not build against your patch set applied up to and including only patch#1, because at patch#1, SecureBootConfigDxe depends on SecureBootVariableLib, but ArmVirtPkg doesn't yet resolve that lib class to any instance. Also, I don't see any OvmfPkg patch in the series... hm, well, there are OvmfPkg modifications, but they have been squashed into patch#3, "Intel Platforms: add SecureBootVariableLib class resolution". Regardless of whether we call OvmfPkg an "Intel Platform" -- I wouldn't, BTW --, OvmfPkg DSC updates need to go in their own, isolated patch. Same for EmulatorPkg -- separate patch please. Thanks, Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#77372): https://edk2.groups.io/g/devel/message/77372 Mute This Topic: https://groups.io/mt/83891029/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
