Topics

A proposal to reduce incompatible case


Zhiguang Liu
 

Hi all,

As Michael mentioned, there are some platforms do not build and some is because incompatible code change like this one.
I think it is a burden for both contributor and maintainer to fix platform code when meeting such incompatible change.
I want to proposal one solution to minimum the effort of such code change.

We could add a package library instance dsc include file under each package, like XXXPkgLib.dsc.inc
It will specify the default library instance that will be used by modules in this package.
For example, we add MdeModulePkgLib.dsc.inc file in MdeModulePkg.
Some package already has similar dsc include file, such as ArmVirtPkg/ArmVirt.dsc.inc and NetworkPkg\Network.dsc.inc.
In platform dsc file, we include the XXXPkgLib.dsc.inc file at the beginning, if the platform uses component from the package.
We place the inc file in the beginning because we can override the library instance in other part of the platform dsc file.

Whenever the contributor adds a new library dependency in one module, he should also add a default library instance in the package library instance dsc include file.

For example, in this case,
Contributor will add the below information in UefiPayloadPkgLib.dsc.inc, SecurityPkgLib.dsc.inc and MdeModulePkgLib.dsc.inc

[LibraryClasses]
VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.inf
VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf
[LibraryClasses.common.DXE_RUNTIME_DRIVER]
VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLibRuntimeDxe.inf

If the platform already includes these inc files, the code change won't break any build.
If the platform wants to choose another library instance, it can specify in the dsc file, and will override the configuration in inc files.
This feature can even reduce the code in platform dsc file if platform choose to use default library instance.
The problem is that it may compiles redundant modules if the

Please give comments about this proposal.

Thanks
Zhiguang

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael
Kubacki
Sent: Friday, November 20, 2020 4:16 AM
To: devel@edk2.groups.io; awarkentin@...; Ard Biesheuvel
<ard.biesheuvel@...>; debtech@...
Cc: Bret Barkelew <bret@...>; Yao, Jiewen <jiewen.yao@...>;
Bi, Dandan <dandan.bi@...>; Chao Zhang <chao.b.zhang@...>;
Wang, Jian J <jian.j.wang@...>; Wu, Hao A <hao.a.wu@...>;
Liming Gao <liming.gao@...>; Justen, Jordan L
<jordan.l.justen@...>; Laszlo Ersek <lersek@...>; Andrew Fish
<afish@...>; Ni, Ray <ray.ni@...>; Bret Barkelew
<brbarkel@...>
Subject: Re: [edk2-devel] [PATCH v9 00/13] Add the VariablePolicy feature

While I'm not currently a maintainer in either repo, I believe the current
process is not ideal. I highlighted some of my observations
here: https://edk2.groups.io/g/devel/message/65902.

Again, I don't have a strong vested interest in this but I do think some level of a
more well defined process needs to be reached between repo maintiners to
ease feature development in the future.

Thanks,
Michael

On 11/19/2020 12:02 PM, Andrei Warkentin wrote:
Hi Bret,

To be honest, I don't recall seeing anything. Again, maybe I should
have been more proactive, but that's probably the net reality for most
people. It would be unreasonable to expect you to test every platform,
but it is very reasonable to assume that if you know you're adding
build breakage to every platform (that is trivial to fix), that you
would be taking care of it... Principle of least surprise. And yes, in
some weird corner case perhaps that would be insufficient (again, I
don't think anyone would expect you to compile test every platform),
but it would take care of 99% of obvious fall-out.

For reference, there are occasional clean-ups that happen to the edk2
tree, and I've never seen anyone claim "not my problem" to deal with
the obvious fall-out resulting from renames and such.

A
----------------------------------------------------------------------
--
*From:* devel@edk2.groups.io <devel@edk2.groups.io> on behalf of Bret
Barkelew via groups.io <debtech=gmail.com@groups.io>
*Sent:* Thursday, November 19, 2020 10:15 AM
*To:* Ard Biesheuvel <ard.biesheuvel@...>
*Cc:* Bret Barkelew <bret@...>; devel@edk2.groups.io
<devel@edk2.groups.io>; Jiewen Yao <jiewen.yao@...>; Dandan Bi
<dandan.bi@...>; Chao Zhang <chao.b.zhang@...>; Jian J
Wang <jian.j.wang@...>; Hao A Wu <hao.a.wu@...>; Liming
Gao <liming.gao@...>; Jordan Justen <jordan.l.justen@...>;
Laszlo Ersek <lersek@...>; Andrew Fish <afish@...>; Ray
Ni <ray.ni@...>; Bret Barkelew <brbarkel@...>
*Subject:* Re: [edk2-devel] [PATCH v9 00/13] Add the VariablePolicy
feature Those bugs and recommendations were sent out months ago.
Several platforms have staged the changes already.

You need to add the library class to your DSC.

--
[ Insert obscure pop-culture reference here. ]

On Nov 19, 2020, at 4:46 AM, Ard Biesheuvel <ard.biesheuvel@...>
wrote:

On 11/9/20 7:45 AM, Bret Barkelew wrote:
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://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgi
thub.com%2Ftianocore%2Ftianocore.github.io%2Fwiki%2FVariablePolicy-P
rotocol---Enhanced-Method-for-Managing-Variables%23platform-porting&
amp;data=04%7C01%7Cawarkentin%40vmware.com%7C594f15b45aaf476bff7e
08d
88cb57390%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C63741405
82471
28819%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luM
zIiLC
JBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=LLKZ7qeffR0WCvLbYuH
tQI
uwJGhXY0mVqB2w9B0q180%3D&amp;reserved=0
<https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit
hub.com%2Ftianocore%2Ftianocore.github.io%2Fwiki%2FVariablePolicy-Prot
ocol---Enhanced-Method-for-Managing-Variables%23platform-
porting&amp;d
ata=04%7C01%7Cawarkentin%40vmware.com%7C594f15b45aaf476bff7e08d88
cb573
90%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637414058247128
819%7CU
nknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik
1ha
WwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=LLKZ7qeffR0WCvLbYuHtQIuwJGhX
Y0mVqB2
w9B0q180%3D&amp;reserved=0>
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://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fed
k2.groups.io%2Fg%2Fdevel%2Fmessage%2F53712&amp;data=04%7C01%7Cawa
rke
ntin%40vmware.com%7C594f15b45aaf476bff7e08d88cb57390%7Cb39138ca3c
ee4
b4aa4d6cd83d9dd62f0%7C0%7C0%7C637414058247133820%7CUnknown%7CT
WFpbGZ
sb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0
%3D%7C1000&amp;sdata=GYY52rlsPxw07vfdu%2BVbWhzRjtHWXlIGveCTT
17mlfc%3
D&amp;reserved=0
<https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk
2.groups.io%2Fg%2Fdevel%2Fmessage%2F53712&amp;data=04%7C01%7Cawar
kenti
n%40vmware.com%7C594f15b45aaf476bff7e08d88cb57390%7Cb39138ca3cee
4b4aa4
d6cd83d9dd62f0%7C0%7C0%7C637414058247133820%7CUnknown%7CTWFpb
GZsb3d8ey
JWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7
C100
0&amp;sdata=GYY52rlsPxw07vfdu%2BVbWhzRjtHWXlIGveCTT17mlfc%3D&amp
;reser
ved=0>
(the code branches shared in that discussion are now out of date,
but the whitepapers and discussion are relevant).
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Dandan Bi <dandan.bi@...>
Cc: Chao Zhang <chao.b.zhang@...>
Cc: Jian J Wang <jian.j.wang@...>
Cc: Hao A Wu <hao.a.wu@...>
Cc: Liming Gao <liming.gao@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Laszlo Ersek <lersek@...>
Cc: Ard Biesheuvel <ard.biesheuvel@...>
Cc: Andrew Fish <afish@...>
Cc: Ray Ni <ray.ni@...>
Cc: Bret Barkelew <brbarkel@...>
Signed-off-by: Bret Barkelew <brbarkel@...>
This series has now made it into edk2, and has subsequently broken every
single platform in edk2-platforms. Is anyone intending to propose any fixes for
this?


v9 changes:
* Rebase
* Address the event ordering issues around MorLock at EndOfDxe
* Drop problematic tests
* Address ECC issues
v8 changes:
* Rebase
* Small tweaks from final PRs
* Drank a lot
* Enrolled several members and a steward in CatFacts
v7 changes:
* Address comments from Dandan about security of the MM handler
* Add readme
* Fix bug around hex characters in BOOT####, etc
* Add additional testing for hex characters
* Add additional testing for authenticated variables
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 (13):
   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/Library/VarCheckPolicyLib/VarCheckPolicyLib.c
| 346 ++++++++

MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.
c     | 396 ++++++++++

MdeModulePkg/Library/VariablePolicyLib/VariablePolicyExtraInitNull.c
|  46 ++

MdeModulePkg/Library/VariablePolicyLib/VariablePolicyExtraInitRuntime
Dxe.c |  85 ++
  MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.c
| 830 ++++++++++++++++++++
  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
|  60 ++

MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequestToLo
ck.
c     |  71 ++
  MdeModulePkg/Universal/Variable/RuntimeDxe/VariablePolicySmmDxe.c
| 573 ++++++++++++++
  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
|   7 +
  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDx
e.c
|  14 +
  SecurityPkg/Library/AuthVariableLib/AuthService.c
|  30 +-
  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/ReadMe.md
| 406 ++++++++++
  MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.inf
|  48 ++
  MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.uni
|  12 +

MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLibRuntimeDxe.in
f     |  51 ++
  MdeModulePkg/MdeModulePkg.ci.yaml
|   4 +-
  MdeModulePkg/MdeModulePkg.dec
|  26 +-
  MdeModulePkg/MdeModulePkg.dsc
|   9 +
  MdeModulePkg/MdeModulePkg.uni
|   7 +
  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
|   5 +
  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
|   4 +

MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.
inf
|  11 +
  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.i
nf
|   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 +
  43 files changed, 3845 insertions(+), 80 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/VariablePolicyExtraInitRuntime
Dxe.c
  create mode 100644
MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.c
  create mode 100644
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequestToLo
ck.
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/ReadMe.md
  create mode 100644
MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.inf
  create mode 100644
MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.uni
  create mode 100644
MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLibRuntimeDxe.in
f







Yao, Jiewen
 

I like this idea. MinPlatform also adopt the same strategy - define common stuff in a dsc include file @ https://github.com/tianocore/edk2-platforms/tree/master/Platform/Intel/MinPlatformPkg/Include/Dsc


A minor clarification:
For VariablePolicyLib, I think we just need add to MdeModulePkgLib.dsc.inc. We don’t need update UefiPayloadPkgLib.dsc.inc or SecurityPkgLib.dsc.inc, right? They are just consumer, not producer.

Thank you
Yao Jiewen

-----Original Message-----
From: Liu, Zhiguang <zhiguang.liu@...>
Sent: Friday, November 20, 2020 2:52 PM
To: devel@edk2.groups.io; @michael.kubacki;
awarkentin@...; Ard Biesheuvel <ard.biesheuvel@...>;
debtech@...; Feng, Bob C <bob.c.feng@...>; Tian, Hot
<@Hot>
Cc: Bret Barkelew <bret@...>; Yao, Jiewen
<jiewen.yao@...>; Bi, Dandan <dandan.bi@...>; Chao Zhang
<chao.b.zhang@...>; Wang, Jian J <jian.j.wang@...>; Wu, Hao
A <hao.a.wu@...>; Liming Gao <liming.gao@...>; Justen,
Jordan L <jordan.l.justen@...>; Laszlo Ersek <lersek@...>;
Andrew Fish <afish@...>; Ni, Ray <ray.ni@...>; Bret Barkelew
<brbarkel@...>; Kinney, Michael D
<michael.d.kinney@...>; Liming Gao <gaoliming@...>
Subject: A proposal to reduce incompatible case

Hi all,

As Michael mentioned, there are some platforms do not build and some is
because incompatible code change like this one.
I think it is a burden for both contributor and maintainer to fix platform code
when meeting such incompatible change.
I want to proposal one solution to minimum the effort of such code change.

We could add a package library instance dsc include file under each package,
like XXXPkgLib.dsc.inc
It will specify the default library instance that will be used by modules in this
package.
For example, we add MdeModulePkgLib.dsc.inc file in MdeModulePkg.
Some package already has similar dsc include file, such as
ArmVirtPkg/ArmVirt.dsc.inc and NetworkPkg\Network.dsc.inc.
In platform dsc file, we include the XXXPkgLib.dsc.inc file at the beginning, if
the platform uses component from the package.
We place the inc file in the beginning because we can override the library
instance in other part of the platform dsc file.

Whenever the contributor adds a new library dependency in one module, he
should also add a default library instance in the package library instance dsc
include file.

For example, in this case,
Contributor will add the below information in UefiPayloadPkgLib.dsc.inc,
SecurityPkgLib.dsc.inc and MdeModulePkgLib.dsc.inc

[LibraryClasses]

VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLi
b.inf

VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/Va
riablePolicyHelperLib.inf
[LibraryClasses.common.DXE_RUNTIME_DRIVER]

VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLi
bRuntimeDxe.inf

If the platform already includes these inc files, the code change won't break
any build.
If the platform wants to choose another library instance, it can specify in the
dsc file, and will override the configuration in inc files.
This feature can even reduce the code in platform dsc file if platform choose
to use default library instance.
The problem is that it may compiles redundant modules if the

Please give comments about this proposal.

Thanks
Zhiguang


-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
Michael
Kubacki
Sent: Friday, November 20, 2020 4:16 AM
To: devel@edk2.groups.io; awarkentin@...; Ard Biesheuvel
<ard.biesheuvel@...>; debtech@...
Cc: Bret Barkelew <bret@...>; Yao, Jiewen
<jiewen.yao@...>;
Bi, Dandan <dandan.bi@...>; Chao Zhang
<chao.b.zhang@...>;
Wang, Jian J <jian.j.wang@...>; Wu, Hao A <hao.a.wu@...>;
Liming Gao <liming.gao@...>; Justen, Jordan L
<jordan.l.justen@...>; Laszlo Ersek <lersek@...>; Andrew
Fish
<afish@...>; Ni, Ray <ray.ni@...>; Bret Barkelew
<brbarkel@...>
Subject: Re: [edk2-devel] [PATCH v9 00/13] Add the VariablePolicy feature

While I'm not currently a maintainer in either repo, I believe the current
process is not ideal. I highlighted some of my observations
here: https://edk2.groups.io/g/devel/message/65902.

Again, I don't have a strong vested interest in this but I do think some level
of a
more well defined process needs to be reached between repo maintiners
to
ease feature development in the future.

Thanks,
Michael

On 11/19/2020 12:02 PM, Andrei Warkentin wrote:
Hi Bret,

To be honest, I don't recall seeing anything. Again, maybe I should
have been more proactive, but that's probably the net reality for most
people. It would be unreasonable to expect you to test every platform,
but it is very reasonable to assume that if you know you're adding
build breakage to every platform (that is trivial to fix), that you
would be taking care of it... Principle of least surprise. And yes, in
some weird corner case perhaps that would be insufficient (again, I
don't think anyone would expect you to compile test every platform),
but it would take care of 99% of obvious fall-out.

For reference, there are occasional clean-ups that happen to the edk2
tree, and I've never seen anyone claim "not my problem" to deal with
the obvious fall-out resulting from renames and such.

A
----------------------------------------------------------------------
--
*From:* devel@edk2.groups.io <devel@edk2.groups.io> on behalf of
Bret
Barkelew via groups.io <debtech=gmail.com@groups.io>
*Sent:* Thursday, November 19, 2020 10:15 AM
*To:* Ard Biesheuvel <ard.biesheuvel@...>
*Cc:* Bret Barkelew <bret@...>; devel@edk2.groups.io
<devel@edk2.groups.io>; Jiewen Yao <jiewen.yao@...>; Dandan
Bi
<dandan.bi@...>; Chao Zhang <chao.b.zhang@...>; Jian J
Wang <jian.j.wang@...>; Hao A Wu <hao.a.wu@...>;
Liming
Gao <liming.gao@...>; Jordan Justen <jordan.l.justen@...>;
Laszlo Ersek <lersek@...>; Andrew Fish <afish@...>; Ray
Ni <ray.ni@...>; Bret Barkelew <brbarkel@...>
*Subject:* Re: [edk2-devel] [PATCH v9 00/13] Add the VariablePolicy
feature Those bugs and recommendations were sent out months ago.
Several platforms have staged the changes already.

You need to add the library class to your DSC.

--
[ Insert obscure pop-culture reference here. ]

On Nov 19, 2020, at 4:46 AM, Ard Biesheuvel <ard.biesheuvel@...>
wrote:

On 11/9/20 7:45 AM, Bret Barkelew wrote:
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://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgi
thub.com%2Ftianocore%2Ftianocore.github.io%2Fwiki%2FVariablePolicy-P
rotocol---Enhanced-Method-for-Managing-Variables%23platform-
porting&
amp;data=04%7C01%7Cawarkentin%40vmware.com%7C594f15b45aaf476bff
7e
08d
88cb57390%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C6374140
5
82471
28819%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2lu
M
zIiLC
JBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=LLKZ7qeffR0WCvLbY
uH
tQI
uwJGhXY0mVqB2w9B0q180%3D&amp;reserved=0
<https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit
hub.com%2Ftianocore%2Ftianocore.github.io%2Fwiki%2FVariablePolicy-
Prot
ocol---Enhanced-Method-for-Managing-Variables%23platform-
porting&amp;d
ata=04%7C01%7Cawarkentin%40vmware.com%7C594f15b45aaf476bff7e08d
88
cb573
90%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C63741405824712
8
819%7CU
nknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI
6Ik
1ha
WwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=LLKZ7qeffR0WCvLbYuHtQIuwJG
hX
Y0mVqB2
w9B0q180%3D&amp;reserved=0>
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://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fed
k2.groups.io%2Fg%2Fdevel%2Fmessage%2F53712&amp;data=04%7C01%7Ca
wa
rke
ntin%40vmware.com%7C594f15b45aaf476bff7e08d88cb57390%7Cb39138ca
3c
ee4
b4aa4d6cd83d9dd62f0%7C0%7C0%7C637414058247133820%7CUnknown%7
CT
WFpbGZ
sb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6M
n0
%3D%7C1000&amp;sdata=GYY52rlsPxw07vfdu%2BVbWhzRjtHWXlIGve
CTT
17mlfc%3
D&amp;reserved=0
<https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk
2.groups.io%2Fg%2Fdevel%2Fmessage%2F53712&amp;data=04%7C01%7Ca
war
kenti
n%40vmware.com%7C594f15b45aaf476bff7e08d88cb57390%7Cb39138ca3ce
e
4b4aa4
d6cd83d9dd62f0%7C0%7C0%7C637414058247133820%7CUnknown%7CTWF
pb
GZsb3d8ey
JWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%
7
C100
0&amp;sdata=GYY52rlsPxw07vfdu%2BVbWhzRjtHWXlIGveCTT17mlfc%3D&a
mp
;reser
ved=0>
(the code branches shared in that discussion are now out of date,
but the whitepapers and discussion are relevant).
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Dandan Bi <dandan.bi@...>
Cc: Chao Zhang <chao.b.zhang@...>
Cc: Jian J Wang <jian.j.wang@...>
Cc: Hao A Wu <hao.a.wu@...>
Cc: Liming Gao <liming.gao@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Laszlo Ersek <lersek@...>
Cc: Ard Biesheuvel <ard.biesheuvel@...>
Cc: Andrew Fish <afish@...>
Cc: Ray Ni <ray.ni@...>
Cc: Bret Barkelew <brbarkel@...>
Signed-off-by: Bret Barkelew <brbarkel@...>
This series has now made it into edk2, and has subsequently broken
every
single platform in edk2-platforms. Is anyone intending to propose any fixes
for
this?


v9 changes:
* Rebase
* Address the event ordering issues around MorLock at EndOfDxe
* Drop problematic tests
* Address ECC issues
v8 changes:
* Rebase
* Small tweaks from final PRs
* Drank a lot
* Enrolled several members and a steward in CatFacts
v7 changes:
* Address comments from Dandan about security of the MM handler
* Add readme
* Fix bug around hex characters in BOOT####, etc
* Add additional testing for hex characters
* Add additional testing for authenticated variables
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 (13):
   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/Library/VarCheckPolicyLib/VarCheckPolicyLib.c
| 346 ++++++++

MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLi
b.
c     | 396 ++++++++++

MdeModulePkg/Library/VariablePolicyLib/VariablePolicyExtraInitNull.c
|  46 ++

MdeModulePkg/Library/VariablePolicyLib/VariablePolicyExtraInitRunti
me
Dxe.c |  85 ++
  MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.c
| 830 ++++++++++++++++++++
  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
|  60 ++

MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequestT
oLo
ck.
c     |  71 ++
  MdeModulePkg/Universal/Variable/RuntimeDxe/VariablePolicySmmD
xe.c

| 573 ++++++++++++++
  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
|   7 +
  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntim
eDx
e.c
|  14 +
  SecurityPkg/Library/AuthVariableLib/AuthService.c
|  30 +-
  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/VariablePolicyHelperLi
b.
inf   |  35 +

MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLi
b.
uni   |  12 +
  MdeModulePkg/Library/VariablePolicyLib/ReadMe.md
| 406 ++++++++++
  MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.inf
|  48 ++
  MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.uni
|  12 +

MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLibRuntimeDxe
.in
f     |  51 ++
  MdeModulePkg/MdeModulePkg.ci.yaml
|   4 +-
  MdeModulePkg/MdeModulePkg.dec
|  26 +-
  MdeModulePkg/MdeModulePkg.dsc
|   9 +
  MdeModulePkg/MdeModulePkg.uni
|   7 +
  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe
.inf

|   5 +
  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
|   4 +

MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntime
Dxe.
inf
|  11 +
  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandalone
Mm.i
nf
|   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 +
  43 files changed, 3845 insertions(+), 80 deletions(-)
  create mode 100644
MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.c
  create mode 100644
MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLi
b.
c
  create mode 100644
MdeModulePkg/Library/VariablePolicyLib/VariablePolicyExtraInitNull.c
  create mode 100644
MdeModulePkg/Library/VariablePolicyLib/VariablePolicyExtraInitRunti
me
Dxe.c
  create mode 100644
MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.c
  create mode 100644
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequestT
oLo
ck.
c
  create mode 100644
MdeModulePkg/Universal/Variable/RuntimeDxe/VariablePolicySmmDx
e.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/VariablePolicyHelperLi
b.
inf
  create mode 100644
MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLi
b.
uni
  create mode 100644
MdeModulePkg/Library/VariablePolicyLib/ReadMe.md
  create mode 100644
MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.inf
  create mode 100644
MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.uni
  create mode 100644
MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLibRuntimeDxe
.in
f







Bret Barkelew
 

@Yao, Jiewen, that should be correct.

 

I’m not so sure that this is a great idea. If a platform is consuming entire core packages en masse, that feels like a recipe for disaster (or a platform that is so inconsequential that it need not be managed). My personal feeling about it is that the platforms should be consuming from EDK2 like… well… platforms. They should snap to stable releases and those EDK2 releases should come with sufficient change notes to explain what the platform will need to change when it snaps.

 

Either that, or we can come up with a formal way to add platform porting documentation in the commit notes on commits that will have a platfrom-breaking effect.

 

- Bret

 

From: Yao, Jiewen
Sent: Thursday, November 19, 2020 11:20 PM
To: Liu, Zhiguang; devel@edk2.groups.io; michael.kubacki@...; awarkentin@...; Ard Biesheuvel; debtech@...; Feng, Bob C; Tian, Hot
Cc: Bret Barkelew; Bi, Dandan; Chao Zhang; Wang, Jian J; Wu, Hao A; liming.gao; Justen, Jordan L; Laszlo Ersek; Andrew Fish; Ni, Ray; Bret Barkelew; Kinney, Michael D; Liming Gao
Subject: [EXTERNAL] RE: A proposal to reduce incompatible case

 

I like this idea. MinPlatform also adopt the same strategy - define common stuff in a dsc include file @ https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Ftianocore%2Fedk2-platforms%2Ftree%2Fmaster%2FPlatform%2FIntel%2FMinPlatformPkg%2FInclude%2FDsc&amp;data=04%7C01%7CBret.Barkelew%40microsoft.com%7C14c39c430e694a2610d708d88d24c2e3%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637414536339327026%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=G0TSIq0nQH5t7%2F1nBsuacydp928NBHBzcmx3KQi%2Fw%2Fc%3D&amp;reserved=0


A minor clarification:
For VariablePolicyLib, I think we just need add to MdeModulePkgLib.dsc.inc. We don’t need update UefiPayloadPkgLib.dsc.inc or SecurityPkgLib.dsc.inc, right? They are just consumer, not producer.

Thank you
Yao Jiewen


> -----Original Message-----
> From: Liu, Zhiguang <zhiguang.liu@...>
> Sent: Friday, November 20, 2020 2:52 PM
> To: devel@edk2.groups.io; michael.kubacki@...;
> awarkentin@...; Ard Biesheuvel <ard.biesheuvel@...>;
> debtech@...; Feng, Bob C <bob.c.feng@...>; Tian, Hot
> <hot.tian@...>
> Cc: Bret Barkelew <bret@...>; Yao, Jiewen
> <jiewen.yao@...>; Bi, Dandan <dandan.bi@...>; Chao Zhang
> <chao.b.zhang@...>; Wang, Jian J <jian.j.wang@...>; Wu, Hao
> A <hao.a.wu@...>; Liming Gao <liming.gao@...>; Justen,
> Jordan L <jordan.l.justen@...>; Laszlo Ersek <lersek@...>;
> Andrew Fish <afish@...>; Ni, Ray <ray.ni@...>; Bret Barkelew
> <brbarkel@...>; Kinney, Michael D
> <michael.d.kinney@...>; Liming Gao <gaoliming@...>
> Subject: A proposal to reduce incompatible case
>
> Hi all,
>
> As Michael mentioned, there are some platforms do not build and some is
> because incompatible code change like this one.
> I think it is a burden for both contributor and maintainer to fix platform code
> when meeting such incompatible change.
> I want to proposal one solution to minimum the effort of such code change.
>
> We could add a package library instance dsc include file under each package,
> like XXXPkgLib.dsc.inc
> It will specify the default library instance that will be used by modules in this
> package.
> For example, we add MdeModulePkgLib.dsc.inc file in MdeModulePkg.
> Some package already has similar dsc include file, such as
> ArmVirtPkg/ArmVirt.dsc.inc and NetworkPkg\Network.dsc.inc.
> In platform dsc file, we include the XXXPkgLib.dsc.inc file at the beginning,  if
> the platform uses component from the package.
> We place the inc file in the beginning because we can override the library
> instance in other part of the platform dsc file.
>
> Whenever the contributor adds a new library dependency in one module, he
> should also add a default library instance in the package library instance dsc
> include file.
>
> For example, in this case,
> Contributor will add the below information in UefiPayloadPkgLib.dsc.inc,
> SecurityPkgLib.dsc.inc and MdeModulePkgLib.dsc.inc
>
>  [LibraryClasses]
>
> VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLi
> b.inf
>
> VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/Va
> riablePolicyHelperLib.inf
>  [LibraryClasses.common.DXE_RUNTIME_DRIVER]
>
> VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLi
> bRuntimeDxe.inf
>
> If the platform already includes these inc files, the code change won't break
> any build.
> If the platform wants to choose another library instance, it can specify in the
> dsc file, and will override the configuration in inc files.
> This feature can even reduce the code in platform dsc file if platform choose
> to use default library instance.
> The problem is that it may compiles redundant modules if the
>
> Please give comments about this proposal.
>
> Thanks
> Zhiguang
>
>
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> Michael
> > Kubacki
> > Sent: Friday, November 20, 2020 4:16 AM
> > To: devel@edk2.groups.io; awarkentin@...; Ard Biesheuvel
> > <ard.biesheuvel@...>; debtech@...
> > Cc: Bret Barkelew <bret@...>; Yao, Jiewen
> <jiewen.yao@...>;
> > Bi, Dandan <dandan.bi@...>; Chao Zhang
> <chao.b.zhang@...>;
> > Wang, Jian J <jian.j.wang@...>; Wu, Hao A <hao.a.wu@...>;
> > Liming Gao <liming.gao@...>; Justen, Jordan L
> > <jordan.l.justen@...>; Laszlo Ersek <lersek@...>; Andrew
> Fish
> > <afish@...>; Ni, Ray <ray.ni@...>; Bret Barkelew
> > <brbarkel@...>
> > Subject: Re: [edk2-devel] [PATCH v9 00/13] Add the VariablePolicy feature
> >
> > While I'm not currently a maintainer in either repo, I believe the current
> > process is not ideal. I highlighted some of my observations
> > here: https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk2.groups.io%2Fg%2Fdevel%2Fmessage%2F65902&amp;data=04%7C01%7CBret.Barkelew%40microsoft.com%7C14c39c430e694a2610d708d88d24c2e3%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637414536339327026%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=J6hQ9rFYGEW4tLEXDMTugHKJX7JH%2FPnxHBZpEiU%2FOT4%3D&amp;reserved=0.
> >
> > Again, I don't have a strong vested interest in this but I do think some level
> of a
> > more well defined process needs to be reached between repo maintiners
> to
> > ease feature development in the future.
> >
> > Thanks,
> > Michael
> >
> > On 11/19/2020 12:02 PM, Andrei Warkentin wrote:
> > > Hi Bret,
> > >
> > > To be honest, I don't recall seeing anything. Again, maybe I should
> > > have been more proactive, but that's probably the net reality for most
> > > people. It would be unreasonable to expect you to test every platform,
> > > but it is very reasonable to assume that if you know you're adding
> > > build breakage to every platform (that is trivial to fix), that you
> > > would be taking care of it... Principle of least surprise. And yes, in
> > > some weird corner case perhaps that would be insufficient (again, I
> > > don't think anyone would expect you to compile test every platform),
> > > but it would take care of 99% of obvious fall-out.
> > >
> > > For reference, there are occasional clean-ups that happen to the edk2
> > > tree, and I've never seen anyone claim "not my problem" to deal with
> > > the obvious fall-out resulting from renames and such.
> > >
> > > A
> > > ----------------------------------------------------------------------
> > > --
> > > *From:* devel@edk2.groups.io <devel@edk2.groups.io> on behalf of
> Bret
> > > Barkelew via groups.io <debtech@...>
> > > *Sent:* Thursday, November 19, 2020 10:15 AM
> > > *To:* Ard Biesheuvel <ard.biesheuvel@...>
> > > *Cc:* Bret Barkelew <bret@...>; devel@edk2.groups.io
> > > <devel@edk2.groups.io>; Jiewen Yao <jiewen.yao@...>; Dandan
> Bi
> > > <dandan.bi@...>; Chao Zhang <chao.b.zhang@...>; Jian J
> > > Wang <jian.j.wang@...>; Hao A Wu <hao.a.wu@...>;
> Liming
> > > Gao <liming.gao@...>; Jordan Justen <jordan.l.justen@...>;
> > > Laszlo Ersek <lersek@...>; Andrew Fish <afish@...>; Ray
> > > Ni <ray.ni@...>; Bret Barkelew <brbarkel@...>
> > > *Subject:* Re: [edk2-devel] [PATCH v9 00/13] Add the VariablePolicy
> > > feature Those bugs and recommendations were sent out months ago.
> > > Several platforms have staged the changes already.
> > >
> > > You need to add the library class to your DSC.
> > >
> > > --
> > > [ Insert obscure pop-culture reference here. ]
> > >
> > >> On Nov 19, 2020, at 4:46 AM, Ard Biesheuvel <ard.biesheuvel@...>
> > wrote:
> > >>
> > >> On 11/9/20 7:45 AM, Bret Barkelew wrote:
> > >>> 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://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgi
> > >>>
> thub.com%2Ftianocore%2Ftianocore.github.io%2Fwiki%2FVariablePolicy-P
> > >>> rotocol---Enhanced-Method-for-Managing-Variables%23platform-
> porting&
> > >>>
> >
> amp;data=04%7C01%7Cawarkentin%40vmware.com%7C594f15b45aaf476bff
> 7e
> > 08d
> > >>>
> >
> 88cb57390%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C6374140
> 5
> > 82471
> > >>>
> >
> 28819%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2lu
> M
> > zIiLC
> > >>>
> >
> JBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=LLKZ7qeffR0WCvLbY
> uH
> > tQI
> > >>> uwJGhXY0mVqB2w9B0q180%3D&amp;reserved=0
> > >
> <https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit
> > > hub.com%2Ftianocore%2Ftianocore.github.io%2Fwiki%2FVariablePolicy-
> Prot
> > > ocol---Enhanced-Method-for-Managing-Variables%23platform-
> > porting&amp;d
> > >
> >
> ata=04%7C01%7Cawarkentin%40vmware.com%7C594f15b45aaf476bff7e08d
> 88
> > cb573
> > >
> >
> 90%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C63741405824712
> 8
> > 819%7CU
> > >
> >
> nknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI
> 6Ik
> > 1ha
> > >
> >
> WwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=LLKZ7qeffR0WCvLbYuHtQIuwJG
> hX
> > Y0mVqB2
> > > w9B0q180%3D&amp;reserved=0>
> > >>> 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://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fed
> > >>>
> >
> k2.groups.io%2Fg%2Fdevel%2Fmessage%2F53712&amp;data=04%7C01%7Ca
> wa
> > rke
> > >>>
> >
> ntin%40vmware.com%7C594f15b45aaf476bff7e08d88cb57390%7Cb39138ca
> 3c
> > ee4
> > >>>
> >
> b4aa4d6cd83d9dd62f0%7C0%7C0%7C637414058247133820%7CUnknown%7
> CT
> > WFpbGZ
> > >>>
> >
> sb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6M
> n0
> > >>> %3D%7C1000&amp;sdata=GYY52rlsPxw07vfdu%2BVbWhzRjtHWXlIGve
> CTT
> > 17mlfc%3
> > >>> D&amp;reserved=0
> > >
> <https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk
> > >
> >
> 2.groups.io%2Fg%2Fdevel%2Fmessage%2F53712&amp;data=04%7C01%7Ca
> war
> > kenti
> > >
> >
> n%40vmware.com%7C594f15b45aaf476bff7e08d88cb57390%7Cb39138ca3ce
> e
> > 4b4aa4
> > >
> >
> d6cd83d9dd62f0%7C0%7C0%7C637414058247133820%7CUnknown%7CTWF
> pb
> > GZsb3d8ey
> > >
> >
> JWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%
> 7
> > C100
> > >
> >
> 0&amp;sdata=GYY52rlsPxw07vfdu%2BVbWhzRjtHWXlIGveCTT17mlfc%3D&a
> mp
> > ;reser
> > > ved=0>
> > >>> (the code branches shared in that discussion are now out of date,
> > >>> but the whitepapers and discussion are relevant).
> > >>> Cc: Jiewen Yao <jiewen.yao@...>
> > >>> Cc: Dandan Bi <dandan.bi@...>
> > >>> Cc: Chao Zhang <chao.b.zhang@...>
> > >>> Cc: Jian J Wang <jian.j.wang@...>
> > >>> Cc: Hao A Wu <hao.a.wu@...>
> > >>> Cc: Liming Gao <liming.gao@...>
> > >>> Cc: Jordan Justen <jordan.l.justen@...>
> > >>> Cc: Laszlo Ersek <lersek@...>
> > >>> Cc: Ard Biesheuvel <ard.biesheuvel@...>
> > >>> Cc: Andrew Fish <afish@...>
> > >>> Cc: Ray Ni <ray.ni@...>
> > >>> Cc: Bret Barkelew <brbarkel@...>
> > >>> Signed-off-by: Bret Barkelew <brbarkel@...>
> > >>
> > >> This series has now made it into edk2, and has subsequently broken
> every
> > single platform in edk2-platforms. Is anyone intending to propose any fixes
> for
> > this?
> > >>
> > >>
> > >>> v9 changes:
> > >>> * Rebase
> > >>> * Address the event ordering issues around MorLock at EndOfDxe
> > >>> * Drop problematic tests
> > >>> * Address ECC issues
> > >>> v8 changes:
> > >>> * Rebase
> > >>> * Small tweaks from final PRs
> > >>> * Drank a lot
> > >>> * Enrolled several members and a steward in CatFacts
> > >>> v7 changes:
> > >>> * Address comments from Dandan about security of the MM handler
> > >>> * Add readme
> > >>> * Fix bug around hex characters in BOOT####, etc
> > >>> * Add additional testing for hex characters
> > >>> * Add additional testing for authenticated variables
> > >>> 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 (13):
> > >>>   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/Library/VarCheckPolicyLib/VarCheckPolicyLib.c
> > >>>| 346 ++++++++
> > >>>
> > >>>MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLi
> b.
> > >>>c     | 396 ++++++++++
> > >>>
> > >>>MdeModulePkg/Library/VariablePolicyLib/VariablePolicyExtraInitNull.c
> > >>>|  46 ++
> > >>>
> > >>>MdeModulePkg/Library/VariablePolicyLib/VariablePolicyExtraInitRunti
> me
> > >>>Dxe.c |  85 ++
> > >>>  MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.c
> > >>>| 830 ++++++++++++++++++++
> > >>>  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
> > >>>|  60 ++
> > >>>
> > >>>MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequestT
> oLo
> > ck.
> > >>>c     |  71 ++
> > >>>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariablePolicySmmD
> xe.c
> >
> > >>>| 573 ++++++++++++++
> > >>>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> >
> > >>>|   7 +
> > >>>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntim
> eDx
> > e.c
> > >>>|  14 +
> > >>>  SecurityPkg/Library/AuthVariableLib/AuthService.c
> > >>>|  30 +-
> > >>>  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/VariablePolicyHelperLi
> b.
> > >>>inf   |  35 +
> > >>>
> > >>>MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLi
> b.
> > >>>uni   |  12 +
> > >>>  MdeModulePkg/Library/VariablePolicyLib/ReadMe.md
> > >>>| 406 ++++++++++
> > >>>  MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.inf
> > >>>|  48 ++
> > >>>  MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.uni
> > >>>|  12 +
> > >>>
> > >>>MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLibRuntimeDxe
> .in
> > >>>f     |  51 ++
> > >>>  MdeModulePkg/MdeModulePkg.ci.yaml
> > >>>|   4 +-
> > >>>  MdeModulePkg/MdeModulePkg.dec
> > >>>|  26 +-
> > >>>  MdeModulePkg/MdeModulePkg.dsc
> > >>>|   9 +
> > >>>  MdeModulePkg/MdeModulePkg.uni
> > >>>|   7 +
> > >>>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe
> .inf
> >
> > >>>|   5 +
> > >>>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> >
> > >>>|   4 +
> > >>>
> > >>>MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntime
> Dxe.
> > inf
> > >>>|  11 +
> > >>>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandalone
> Mm.i
> > nf
> > >>>|   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 +
> > >>>  43 files changed, 3845 insertions(+), 80 deletions(-)
> > >>>  create mode 100644
> > >>>MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.c
> > >>>  create mode 100644
> > >>>MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLi
> b.
> > >>>c
> > >>>  create mode 100644
> > >>>MdeModulePkg/Library/VariablePolicyLib/VariablePolicyExtraInitNull.c
> > >>>  create mode 100644
> > >>>MdeModulePkg/Library/VariablePolicyLib/VariablePolicyExtraInitRunti
> me
> > >>>Dxe.c
> > >>>  create mode 100644
> > >>>MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.c
> > >>>  create mode 100644
> > >>>MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequestT
> oLo
> > ck.
> > >>>c
> > >>>  create mode 100644
> > >>>MdeModulePkg/Universal/Variable/RuntimeDxe/VariablePolicySmmDx
> e.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/VariablePolicyHelperLi
> b.
> > >>>inf
> > >>>  create mode 100644
> > >>>MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLi
> b.
> > >>>uni
> > >>>  create mode 100644
> > MdeModulePkg/Library/VariablePolicyLib/ReadMe.md
> > >>>  create mode 100644
> > >>>MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.inf
> > >>>  create mode 100644
> > >>>MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.uni
> > >>>  create mode 100644
> > >>>MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLibRuntimeDxe
> .in
> > >>>f
> > >>
> > >
> > >
> > >
> > >
> > >
> > >
> >
> >
> >
> >