Actually, if you want to leave feedback in the CI test PR that I made for this version, I’m happy to discuss there as well. https://github.com/tianocore/edk2/pull/718/files - Bret From: Bret Barkelew via groups.io Sent: Wednesday, July 1, 2020 11:46 PM To: devel@edk2.groups.io; dandan.bi@intel.com; bret@corthon.com Cc: Yao, Jiewen; Zhang, Chao B; Wang, Jian J; Wu, Hao A; liming.gao; Justen, Jordan L; Laszlo Ersek; Ard Biesheuvel; Andrew Fish; Ni, Ray Subject: [EXTERNAL] Re: [edk2-devel] [PATCH v6 00/14] Add the VariablePolicy feature Certainly do: https://github.com/corthon/edk2/tree/var_policy_dev_submission_v6 I’ve already made a change from some of your feedback in that branch and will restructure the patches for v7 once all feedback is in. Thanks! - Bret From: Dandan Bi via groups.io Sent: Wednesday, July 1, 2020 7:13 PM To: devel@edk2.groups.io; bret@corthon.com Cc: Yao, Jiewen; Zhang, Chao B; Wang, Jian J; Wu, Hao A; liming.gao; Justen, Jordan L; Laszlo Ersek; Ard Biesheuvel; Andrew Fish; Ni, Ray Subject: [EXTERNAL] Re: [edk2-devel] [PATCH v6 00/14] Add the VariablePolicy feature Hi Bret, Thanks for the contribution. I have taken an overview of this patch series and have some small comments in the related patches, please check in sub-patch. I will review the patch series more in details and bring more comments back if have. Do you have a branch for these patches in GitHub? Which should be easy for review. Thanks, Dandan > -----Original Message----- > From: devel@edk2.groups.io On Behalf Of Bret > Barkelew > Sent: Tuesday, June 23, 2020 2:41 PM > To: devel@edk2.groups.io > Cc: Yao, Jiewen ; Zhang, Chao B > ; Wang, Jian J ; Wu, Hao > A ; Gao, Liming ; Justen, > Jordan L ; Laszlo Ersek ; > Ard Biesheuvel ; Andrew Fish > ; Ni, Ray > Subject: [edk2-devel] [PATCH v6 00/14] Add the VariablePolicy feature > > REF:https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D2522&data=02%7C01%7Cbret.barkelew%40microsoft.com%7C3fb90fd337a24d2c965d08d81e2d9081%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637292528333631839&sdata=7hyxOuoJb08Ncpj6Ww4ABCFx48tdOUKmGZq5uCxLHjo%3D&reserved=0 > > The 14 patches in this series add the VariablePolicy feature to the core, > deprecate Edk2VarLock (while adding a compatibility layer to reduce code > churn), and integrate the VariablePolicy libraries and protocols into Variable > Services. > > Since the integration requires multiple changes, including adding libraries, a > protocol, an SMI communication handler, and VariableServices integration, > the patches are broken up by individual library additions and then a final > integration. Security-sensitive changes like bypassing Authenticated Variable > enforcement are also broken out into individual patches so that attention can > be called directly to them. > > Platform porting instructions are described in this wiki entry: > https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Ftianocore%2Ftianocore.github.io%2Fwiki%2FVariablePolicy-&data=02%7C01%7Cbret.barkelew%40microsoft.com%7C3fb90fd337a24d2c965d08d81e2d9081%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637292528333631839&sdata=B%2Bl%2F43l6xTTzvLw00gyXElHqUlu%2BfwhuUJOIEh%2BHN6g%3D&reserved=0 > Protocol---Enhanced-Method-for-Managing-Variables#platform-porting > > Discussion of the feature can be found in multiple places throughout the last > year on the RFC channel, staging branches, and in devel. > > Most recently, this subject was discussed in this thread: > https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk2.groups.io%2Fg%2Fdevel%2Fmessage%2F53712&data=02%7C01%7Cbret.barkelew%40microsoft.com%7C3fb90fd337a24d2c965d08d81e2d9081%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637292528333631839&sdata=UMEtJ5hxVBmvcWPSLRwDZ6P6bOaE8N3v5sEgdzyPffw%3D&reserved=0 > (the code branches shared in that discussion are now out of date, but the > whitepapers and discussion are relevant). > > Cc: Jiewen Yao > Cc: Chao Zhang > Cc: Jian J Wang > Cc: Hao A Wu > Cc: Liming Gao > Cc: Jordan Justen > Cc: Laszlo Ersek > Cc: Ard Biesheuvel > Cc: Andrew Fish > Cc: Ray Ni > Cc: Bret Barkelew > Signed-off-by: Bret Barkelew > > v6 changes: > * Fix an issue with uninitialized Status in InitVariablePolicyLib() and > DeinitVariablePolicyLib() > * Fix GCC building in shell-based functional test > * Rebase on latest origin/master > > v5 changes: > * Fix the CONST mismatch in VariablePolicy.h and VariablePolicySmmDxe.c > * Fix EFIAPI mismatches in the functional unittest > * Rebase on latest origin/master > > v4 changes: > * Remove Optional PcdAllowVariablePolicyEnforcementDisable PCD from > platforms > * Rebase on master > * Migrate to new MmCommunicate2 protocol > * Fix an oversight in the default return value for > InitMmCommonCommBuffer > * Fix in VariablePolicyLib to allow ExtraInitRuntimeDxe to consume variables > > V3 changes: > * Address all non-unittest issues with ECC > * Make additional style changes > * Include section name in hunk headers in "ini-style" files > * Remove requirement for the EdkiiPiSmmCommunicationsRegionTable > driver > (now allocates its own buffer) > * Change names from VARIABLE_POLICY_PROTOCOL and > gVariablePolicyProtocolGuid > to EDKII_VARIABLE_POLICY_PROTOCOL and > gEdkiiVariablePolicyProtocolGuid > * Fix GCC warning about initializing externs > * Add UNI strings for new PCD > * Add patches for ArmVirtPkg, OvmfXen, and UefiPayloadPkg > * Reorder patches according to Liming's feedback about adding to platforms > before changing variable driver > > V2 changes: > * Fixed implementation for RuntimeDxe > * Add PCD to block DisableVariablePolicy > * Fix the DumpVariablePolicy pagination in SMM > > Bret Barkelew (14): > MdeModulePkg: Define the VariablePolicy protocol interface > MdeModulePkg: Define the VariablePolicyLib > MdeModulePkg: Define the VariablePolicyHelperLib > MdeModulePkg: Define the VarCheckPolicyLib and SMM interface > OvmfPkg: Add VariablePolicy engine to OvmfPkg platform > EmulatorPkg: Add VariablePolicy engine to EmulatorPkg platform > ArmVirtPkg: Add VariablePolicy engine to ArmVirtPkg platform > UefiPayloadPkg: Add VariablePolicy engine to UefiPayloadPkg platform > MdeModulePkg: Connect VariablePolicy business logic to > VariableServices > MdeModulePkg: Allow VariablePolicy state to delete protected variables > SecurityPkg: Allow VariablePolicy state to delete authenticated > variables > MdeModulePkg: Change TCG MOR variables to use VariablePolicy > MdeModulePkg: Drop VarLock from RuntimeDxe variable driver > MdeModulePkg: Add a shell-based functional test for VariablePolicy > > MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.c > | 320 +++ > MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.c > | 396 ++++ > MdeModulePkg/Library/VariablePolicyLib/VariablePolicyExtraInitNull.c > | 46 + > > MdeModulePkg/Library/VariablePolicyLib/VariablePolicyExtraInitRuntimeDx > e.c | 85 + > MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.c > | 816 +++++++ > > MdeModulePkg/Library/VariablePolicyLib/VariablePolicyUnitTest/VariablePo > licyUnitTest.c | 2440 ++++++++++++++++++++ > > MdeModulePkg/Test/ShellTest/VariablePolicyFuncTestApp/VariablePolicyFu > ncTestApp.c | 1978 ++++++++++++++++ > MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockDxe.c > | 52 +- > MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockSmm.c > | 60 +- > MdeModulePkg/Universal/Variable/RuntimeDxe/VarCheck.c > | 49 +- > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c > | 53 + > > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequstToLock > .c | 71 + > MdeModulePkg/Universal/Variable/RuntimeDxe/VariablePolicySmmDxe.c > | 642 +++++ > > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe. > c | 14 + > SecurityPkg/Library/AuthVariableLib/AuthService.c | 22 > +- > ArmVirtPkg/ArmVirt.dsc.inc | 4 + > EmulatorPkg/EmulatorPkg.dsc | 3 + > MdeModulePkg/Include/Guid/VarCheckPolicyMmi.h | > 54 + > MdeModulePkg/Include/Library/VariablePolicyHelperLib.h > | 164 ++ > MdeModulePkg/Include/Library/VariablePolicyLib.h | > 207 ++ > MdeModulePkg/Include/Protocol/VariablePolicy.h | > 157 ++ > MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.inf > | 42 + > MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.uni > | 12 + > MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf > | 35 + > MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.uni > | 12 + > MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.inf > | 44 + > MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.uni > | 12 + > MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLibRuntimeDxe.inf > | 51 + > > MdeModulePkg/Library/VariablePolicyLib/VariablePolicyUnitTest/VariablePo > licyUnitTest.inf | 40 + > MdeModulePkg/MdeModulePkg.ci.yaml | 4 +- > MdeModulePkg/MdeModulePkg.dec | 26 +- > MdeModulePkg/MdeModulePkg.dsc | 15 + > MdeModulePkg/MdeModulePkg.uni | 7 + > MdeModulePkg/Test/MdeModulePkgHostTest.dsc | > 11 + > MdeModulePkg/Test/ShellTest/VariablePolicyFuncTestApp/Readme.md > | 55 + > > MdeModulePkg/Test/ShellTest/VariablePolicyFuncTestApp/VariablePolicyFu > ncTestApp.inf | 42 + > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf > | 5 + > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf > | 4 + > > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.i > nf | 10 + > > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf > | 4 + > OvmfPkg/OvmfPkgIa32.dsc | 5 + > OvmfPkg/OvmfPkgIa32X64.dsc | 5 + > OvmfPkg/OvmfPkgX64.dsc | 5 + > OvmfPkg/OvmfXen.dsc | 4 + > SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf | > 2 + > UefiPayloadPkg/UefiPayloadPkgIa32.dsc | 4 + > UefiPayloadPkg/UefiPayloadPkgIa32X64.dsc | 4 + > 47 files changed, 8015 insertions(+), 78 deletions(-) create mode 100644 > MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.c > create mode 100644 > MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.c > create mode 100644 > MdeModulePkg/Library/VariablePolicyLib/VariablePolicyExtraInitNull.c > create mode 100644 > MdeModulePkg/Library/VariablePolicyLib/VariablePolicyExtraInitRuntimeDx > e.c > create mode 100644 > MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.c > create mode 100644 > MdeModulePkg/Library/VariablePolicyLib/VariablePolicyUnitTest/VariablePo > licyUnitTest.c > create mode 100644 > MdeModulePkg/Test/ShellTest/VariablePolicyFuncTestApp/VariablePolicyFu > ncTestApp.c > create mode 100644 > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequstToLock > .c > create mode 100644 > MdeModulePkg/Universal/Variable/RuntimeDxe/VariablePolicySmmDxe.c > create mode 100644 MdeModulePkg/Include/Guid/VarCheckPolicyMmi.h > create mode 100644 > MdeModulePkg/Include/Library/VariablePolicyHelperLib.h > create mode 100644 MdeModulePkg/Include/Library/VariablePolicyLib.h > create mode 100644 MdeModulePkg/Include/Protocol/VariablePolicy.h > create mode 100644 > MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.inf > create mode 100644 > MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.uni > create mode 100644 > MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf > create mode 100644 > MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.uni > create mode 100644 > MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.inf > create mode 100644 > MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.uni > create mode 100644 > MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLibRuntimeDxe.inf > create mode 100644 > MdeModulePkg/Library/VariablePolicyLib/VariablePolicyUnitTest/VariablePo > licyUnitTest.inf > create mode 100644 > MdeModulePkg/Test/ShellTest/VariablePolicyFuncTestApp/Readme.md > create mode 100644 > MdeModulePkg/Test/ShellTest/VariablePolicyFuncTestApp/VariablePolicyFu > ncTestApp.inf > > -- > 2.26.2.windows.1.8.g01c50adf56.20200515075929 > > >