Hey I notice that there is duplicated code in variable driver (MdeModulePkg/Universal/Variable/Protected/ and MdeModulePkg/Universal/Variable/). That is not the best idea and it adds maintenance burden. I am not sure if the feature is ready for EDKII.
Another option is to create ProtectedVariablePkg in https://github.com/tianocore/edk2-platforms/tree/master/Features/Intel, and put code there. It can merge back from edk2-platforms to edk2, after we finalize the Variable driver interface and avoid code duplication. Thank you Yao, Jiewen > -----Original Message----- > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Yao, > Jiewen > Sent: Friday, December 9, 2022 4:04 PM > To: devel@edk2.groups.io; Vang, Judah <judah.v...@intel.com> > Cc: Yao, Jiewen <jiewen....@intel.com>; Kinney, Michael D > <michael.d.kin...@intel.com>; Wang, Jian J <jian.j.w...@intel.com> > Subject: Re: [edk2-devel] [PATCH v5 00/19] UEFI variable protection > > Hi > Since this is a big feature in SecurityPkg and MdeModulePkg, I proposal to > add *dedicated reviewer(s)* to support the maintenance work in EDKII. > > Something like: > > =============== > MdeModulePkg: Protected Variable > F: MdeModulePkg/Universal/Variable/Protected/ > F: <Please list all newly added file> > R: <Please give the reviewer name> > > > SecurityPkg: Protected Variable > F: SecurityPkg/Library/ProtectedVariableLib/ > F: <Please list all newly added file> > R: <Please give the reviewer name> > > =============== > > Please follow the style at > https://github.com/tianocore/edk2/blob/master/Maintainers.txt > > Thank you > Yao, Jiewen > > > > -----Original Message----- > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Judah > > Vang > > Sent: Sunday, November 6, 2022 3:35 PM > > To: devel@edk2.groups.io > > Subject: [edk2-devel] [PATCH v5 00/19] UEFI variable protection > > > > Patch 07 - Add PEI Variable Protection into a new directory and leave the > > existing PEI Variable unchanged. > > > > Patch 08 - Add RuntimeDxe Variable Protection into a new directory and > > keep existing Variable for RuntimeDxe unchanged. > > > > Patch 09 - Add reference to new Protected Variable libs. > > > > Patch 16 - Applied code review comments by adding PEIM to library class > > > > Patch 18 - Applied code review comments by removing unused API. > > > > Notes: > > The CryptoPkg changes are now being tracked separately. > > Patches 21 on is no longer needed due to reorganization of the new > > protected variable modules. > > > > Judah Vang (19): > > MdePkg: Add reference to new Ppi Guid > > MdeModulePkg: Update AUTH_VARIABLE_INFO struct > > MdeModulePkg: Add new ProtectedVariable GUIDs > > MdeModulePkg: Add new include files > > MdeModulePkg: Add new GUID for Variable Store Info > > MdeModulePkg: Add Null ProtectedVariable Library > > MdeModulePkg: Add new Variable functionality > > MdeModulePkg: Add support for Protected Variables > > MdeModulePkg: Reference Null ProtectedVariableLib > > SecurityPkg: Add new GUIDs for > > SecurityPkg: Add new KeyService types and defines > > SecurityPkg: Add new variable types and functions > > SecurityPkg: Update RPMC APIs with index > > SecurityPkg: Fix GetVariableKey API > > SecurityPkg: Add null encryption variable libs > > SecurityPkg: Add VariableKey library function > > SecurityPkg: Add EncryptionVariable lib with AES > > SecurityPkg: Add Protected Variable Services > > SecurityPkg: Add references to new *.inf files > > > > MdeModulePkg/MdeModulePkg.dec > > | 13 +- > > SecurityPkg/SecurityPkg.dec > > | 43 +- > > MdeModulePkg/MdeModulePkg.dsc > > | 20 +- > > MdeModulePkg/Test/MdeModulePkgHostTest.dsc > > | 8 + > > SecurityPkg/SecurityPkg.dsc > > | 13 +- > > > > > MdeModulePkg/Library/ProtectedVariableLibNull/ProtectedVariableLibNull > > .inf | 34 + > > MdeModulePkg/Universal/Variable/Protected/Pei/VariablePei.inf > > | 79 + > > > > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/RuntimeDxeUni > > tTest/VariableLockRequestToLockUnitTest.inf | 36 + > > > > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/VariableRuntim > > eDxe.inf | 151 + > > > > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/VariableSmm.i > > nf | 153 + > > > > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/VariableSmmR > > untimeDxe.inf | 119 + > > > > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/VariableStandal > > oneMm.inf | 143 + > > SecurityPkg/Library/EncryptionVariableLib/EncryptionVariableLib.inf > > | 43 + > > > > > SecurityPkg/Library/EncryptionVariableLibNull/EncryptionVariableLibNull.in > > f | 34 + > > SecurityPkg/Library/ProtectedVariableLib/DxeProtectedVariableLib.inf > > | 64 + > > SecurityPkg/Library/ProtectedVariableLib/PeiProtectedVariableLib.inf > > | 68 + > > SecurityPkg/Library/ProtectedVariableLib/SmmProtectedVariableLib.inf > > | 67 + > > > > > SecurityPkg/Library/ProtectedVariableLib/SmmRuntimeProtectedVariableLi > > b.inf | 62 + > > SecurityPkg/Library/VariableKeyLib/VariableKeyLib.inf > > | 36 + > > MdeModulePkg/Include/Guid/ProtectedVariable.h > > | 22 + > > MdeModulePkg/Include/Library/AuthVariableLib.h > > | 4 +- > > MdeModulePkg/Include/Library/EncryptionVariableLib.h > > | 165 + > > MdeModulePkg/Include/Library/ProtectedVariableLib.h > > | 607 +++ > > MdeModulePkg/Universal/Variable/Protected/Pei/Variable.h > > | 225 ++ > > MdeModulePkg/Universal/Variable/Protected/Pei/VariableParsing.h > > | 309 ++ > > MdeModulePkg/Universal/Variable/Protected/Pei/VariableStore.h > > | 116 + > > > > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/PrivilegePolym > > orphic.h | 158 + > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/Variable.h > > | 948 +++++ > > > > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/VariableNonVol > > atile.h | 67 + > > > > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/VariableParsing > > .h | 424 ++ > > > > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/VariableRuntim > > eCache.h | 51 + > > MdePkg/Include/Ppi/ReadOnlyVariable2.h > > | 4 +- > > SecurityPkg/Include/Library/RpmcLib.h > > | 15 +- > > SecurityPkg/Include/Library/VariableKeyLib.h > > | 37 +- > > SecurityPkg/Include/Ppi/KeyServicePpi.h > > | 57 + > > SecurityPkg/Library/EncryptionVariableLib/EncryptionVariable.h > > | 49 + > > SecurityPkg/Library/ProtectedVariableLib/ProtectedVariableInternal.h > > | 589 +++ > > MdeModulePkg/Library/ProtectedVariableLibNull/ProtectedVariable.c > > | 336 ++ > > MdeModulePkg/Universal/Variable/Protected/Pei/Variable.c > > | 628 +++ > > MdeModulePkg/Universal/Variable/Protected/Pei/VariableParsing.c > > | 941 +++++ > > MdeModulePkg/Universal/Variable/Protected/Pei/VariableStore.c > > | 307 ++ > > > > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/Measurement.c > > | 343 ++ > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/Reclaim.c > > | 504 +++ > > > > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/RuntimeDxeUni > > tTest/VariableLockRequestToLockUnitTest.c | 607 +++ > > > > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/SpeculationBar > > rierDxe.c | 27 + > > > > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/SpeculationBar > > rierSmm.c | 26 + > > > > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/TcgMorLockDxe > > .c | 153 + > > > > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/TcgMorLockSm > > m.c | 569 +++ > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/VarCheck.c > > | 101 + > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/Variable.c > > | 4037 ++++++++++++++++++++ > > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/VariableDxe.c > > | 670 ++++ > > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/VariableExLib.c > > | 417 ++ > > > > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/VariableLockRe > > questToLock.c | 96 + > > > > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/VariableNonVol > > atile.c | 537 +++ > > > > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/VariableParsing > > .c | 1110 ++++++ > > > > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/VariablePolicyS > > mmDxe.c | 575 +++ > > > > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/VariableRuntim > > eCache.c | 158 + > > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/VariableSmm.c > > | 1268 ++++++ > > > > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/VariableSmmR > > untimeDxe.c | 1895 +++++++++ > > > > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/VariableStandal > > oneMm.c | 89 + > > > > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/VariableTraditi > > onalMm.c | 130 + > > SecurityPkg/Library/EncryptionVariableLib/EncryptionVariable.c > > | 734 ++++ > > SecurityPkg/Library/EncryptionVariableLibNull/EncryptionVariable.c > > | 92 + > > SecurityPkg/Library/ProtectedVariableLib/ProtectedVariableCommon.c > > | 2103 ++++++++++ > > SecurityPkg/Library/ProtectedVariableLib/ProtectedVariableDxe.c > > | 163 + > > SecurityPkg/Library/ProtectedVariableLib/ProtectedVariablePei.c > > | 1327 +++++++ > > SecurityPkg/Library/ProtectedVariableLib/ProtectedVariableSmm.c > > | 209 + > > > > > SecurityPkg/Library/ProtectedVariableLib/ProtectedVariableSmmDxeComm > > on.c | 967 +++++ > > > > > SecurityPkg/Library/ProtectedVariableLib/ProtectedVariableSmmRuntime.c > > | 233 ++ > > SecurityPkg/Library/RpmcLibNull/RpmcLibNull.c > > | 8 +- > > SecurityPkg/Library/VariableKeyLib/VariableKeyLib.c > > | 59 + > > SecurityPkg/Library/VariableKeyLibNull/VariableKeyLibNull.c > > | 8 +- > > MdeModulePkg/Universal/Variable/Protected/Pei/PeiVariable.uni > > | 16 + > > MdeModulePkg/Universal/Variable/Protected/Pei/PeiVariableExtra.uni > > | 14 + > > > > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/VariableRuntim > > eDxe.uni | 22 + > > > > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/VariableRuntim > > eDxeExtra.uni | 14 + > > > > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/VariableSmm.u > > ni | 27 + > > > > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/VariableSmmEx > > tra.uni | 14 + > > > > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/VariableSmmR > > untimeDxe.uni | 23 + > > > > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/VariableSmmR > > untimeDxeExtra.uni | 14 + > > 80 files changed, 26556 insertions(+), 48 deletions(-) > > create mode 100644 > > > MdeModulePkg/Library/ProtectedVariableLibNull/ProtectedVariableLibNull > > .inf > > create mode 100644 > > MdeModulePkg/Universal/Variable/Protected/Pei/VariablePei.inf > > create mode 100644 > > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/RuntimeDxeUni > > tTest/VariableLockRequestToLockUnitTest.inf > > create mode 100644 > > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/VariableRuntim > > eDxe.inf > > create mode 100644 > > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/VariableSmm.i > > nf > > create mode 100644 > > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/VariableSmmR > > untimeDxe.inf > > create mode 100644 > > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/VariableStandal > > oneMm.inf > > create mode 100644 > > SecurityPkg/Library/EncryptionVariableLib/EncryptionVariableLib.inf > > create mode 100644 > > > SecurityPkg/Library/EncryptionVariableLibNull/EncryptionVariableLibNull.in > > f > > create mode 100644 > > SecurityPkg/Library/ProtectedVariableLib/DxeProtectedVariableLib.inf > > create mode 100644 > > SecurityPkg/Library/ProtectedVariableLib/PeiProtectedVariableLib.inf > > create mode 100644 > > SecurityPkg/Library/ProtectedVariableLib/SmmProtectedVariableLib.inf > > create mode 100644 > > > SecurityPkg/Library/ProtectedVariableLib/SmmRuntimeProtectedVariableLi > > b.inf > > create mode 100644 > SecurityPkg/Library/VariableKeyLib/VariableKeyLib.inf > > create mode 100644 MdeModulePkg/Include/Guid/ProtectedVariable.h > > create mode 100644 > > MdeModulePkg/Include/Library/EncryptionVariableLib.h > > create mode 100644 > > MdeModulePkg/Include/Library/ProtectedVariableLib.h > > create mode 100644 > > MdeModulePkg/Universal/Variable/Protected/Pei/Variable.h > > create mode 100644 > > MdeModulePkg/Universal/Variable/Protected/Pei/VariableParsing.h > > create mode 100644 > > MdeModulePkg/Universal/Variable/Protected/Pei/VariableStore.h > > create mode 100644 > > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/PrivilegePolym > > orphic.h > > create mode 100644 > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/Variable.h > > create mode 100644 > > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/VariableNonVol > > atile.h > > create mode 100644 > > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/VariableParsing > > .h > > create mode 100644 > > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/VariableRuntim > > eCache.h > > create mode 100644 SecurityPkg/Include/Ppi/KeyServicePpi.h > > create mode 100644 > > SecurityPkg/Library/EncryptionVariableLib/EncryptionVariable.h > > create mode 100644 > > SecurityPkg/Library/ProtectedVariableLib/ProtectedVariableInternal.h > > create mode 100644 > > MdeModulePkg/Library/ProtectedVariableLibNull/ProtectedVariable.c > > create mode 100644 > > MdeModulePkg/Universal/Variable/Protected/Pei/Variable.c > > create mode 100644 > > MdeModulePkg/Universal/Variable/Protected/Pei/VariableParsing.c > > create mode 100644 > > MdeModulePkg/Universal/Variable/Protected/Pei/VariableStore.c > > create mode 100644 > > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/Measurement.c > > create mode 100644 > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/Reclaim.c > > create mode 100644 > > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/RuntimeDxeUni > > tTest/VariableLockRequestToLockUnitTest.c > > create mode 100644 > > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/SpeculationBar > > rierDxe.c > > create mode 100644 > > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/SpeculationBar > > rierSmm.c > > create mode 100644 > > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/TcgMorLockDxe > > .c > > create mode 100644 > > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/TcgMorLockSm > > m.c > > create mode 100644 > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/VarCheck.c > > create mode 100644 > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/Variable.c > > create mode 100644 > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/VariableDxe.c > > create mode 100644 > > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/VariableExLib.c > > create mode 100644 > > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/VariableLockRe > > questToLock.c > > create mode 100644 > > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/VariableNonVol > > atile.c > > create mode 100644 > > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/VariableParsing > > .c > > create mode 100644 > > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/VariablePolicyS > > mmDxe.c > > create mode 100644 > > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/VariableRuntim > > eCache.c > > create mode 100644 > > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/VariableSmm.c > > create mode 100644 > > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/VariableSmmR > > untimeDxe.c > > create mode 100644 > > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/VariableStandal > > oneMm.c > > create mode 100644 > > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/VariableTraditi > > onalMm.c > > create mode 100644 > > SecurityPkg/Library/EncryptionVariableLib/EncryptionVariable.c > > create mode 100644 > > SecurityPkg/Library/EncryptionVariableLibNull/EncryptionVariable.c > > create mode 100644 > > SecurityPkg/Library/ProtectedVariableLib/ProtectedVariableCommon.c > > create mode 100644 > > SecurityPkg/Library/ProtectedVariableLib/ProtectedVariableDxe.c > > create mode 100644 > > SecurityPkg/Library/ProtectedVariableLib/ProtectedVariablePei.c > > create mode 100644 > > SecurityPkg/Library/ProtectedVariableLib/ProtectedVariableSmm.c > > create mode 100644 > > > SecurityPkg/Library/ProtectedVariableLib/ProtectedVariableSmmDxeComm > > on.c > > create mode 100644 > > > SecurityPkg/Library/ProtectedVariableLib/ProtectedVariableSmmRuntime.c > > create mode 100644 > SecurityPkg/Library/VariableKeyLib/VariableKeyLib.c > > create mode 100644 > > MdeModulePkg/Universal/Variable/Protected/Pei/PeiVariable.uni > > create mode 100644 > > MdeModulePkg/Universal/Variable/Protected/Pei/PeiVariableExtra.uni > > create mode 100644 > > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/VariableRuntim > > eDxe.uni > > create mode 100644 > > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/VariableRuntim > > eDxeExtra.uni > > create mode 100644 > > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/VariableSmm.u > > ni > > create mode 100644 > > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/VariableSmmEx > > tra.uni > > create mode 100644 > > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/VariableSmmR > > untimeDxe.uni > > create mode 100644 > > > MdeModulePkg/Universal/Variable/Protected/RuntimeDxe/VariableSmmR > > untimeDxeExtra.uni > > > > -- > > 2.35.1.windows.2 > > > > > > > > > > > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#97172): https://edk2.groups.io/g/devel/message/97172 Mute This Topic: https://groups.io/mt/94840817/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
