[edk2-devel] [PATCH v4 00/11] Measured SEV boot with kernel/initrd/cmdline

Mon Jul 26 15:31:48 UTC 2021

Hi James
You are right that initially EDKII did recommend to put a lib under Library dir.
But with more and more feature, people start realizing that it is NOT efficient way to identify a *feature*.
We changed the direction later - if we can define a feature dir, we can lib to feature dir.

I used " find . -name *Lib*.inf -print", because I cannot assume the Lib is under Library dir for feature.

I can find lots of stuff. Most of them are under Library, below is NOT in *Pkg/Library/
=============
./edk2/ArmPkg/Drivers/ArmGic/ArmGicLib.inf
./edk2/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
./edk2/ArmPlatformPkg/PlatformPei/PlatformPeiLib.inf
./edk2/CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestBaseCryptLibHost.inf
./edk2/CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestBaseCryptLibShell.inf
./edk2/FmpDevicePkg/FmpDxe/FmpDxeLib.inf
./edk2/FmpDevicePkg/Test/UnitTest/Library/FmpDependencyLib/FmpDependencyLibUnitTestsHost.inf
./edk2/FmpDevicePkg/Test/UnitTest/Library/FmpDependencyLib/FmpDependencyLibUnitTestsUefi.inf
./edk2/MdePkg/Test/UnitTest/Library/BaseLib/BaseLibUnitTestsHost.inf
./edk2/MdePkg/Test/UnitTest/Library/BaseLib/BaseLibUnitTestsUefi.inf
./edk2/MdePkg/Test/UnitTest/Library/BaseSafeIntLib/TestBaseSafeIntLibDxe.inf
./edk2/MdePkg/Test/UnitTest/Library/BaseSafeIntLib/TestBaseSafeIntLibHost.inf
./edk2/MdePkg/Test/UnitTest/Library/BaseSafeIntLib/TestBaseSafeIntLibPei.inf
./edk2/MdePkg/Test/UnitTest/Library/BaseSafeIntLib/TestBaseSafeIntLibSmm.inf
./edk2/MdePkg/Test/UnitTest/Library/BaseSafeIntLib/TestBaseSafeIntLibUefiShell.inf
./edk2/OvmfPkg/Csm/CsmSupportLib/CsmSupportLib.inf
./edk2/OvmfPkg/Csm/LegacyBootMaintUiLib/LegacyBootMaintUiLib.inf
./edk2/OvmfPkg/Csm/LegacyBootManagerLib/LegacyBootManagerLib.inf
============

If I search on edk2-platform (https://github.com/tianocore/edk2-platforms), I can find more:
============
./Platform/Intel/CometlakeOpenBoardPkg/CometlakeURvp/Library/BaseFuncLib/BaseFuncLib.inf
./Platform/Intel/CometlakeOpenBoardPkg/CometlakeURvp/Library/BaseGpioCheckConflictLib/BaseGpioCheckConflictLib.inf
./Platform/Intel/CometlakeOpenBoardPkg/CometlakeURvp/Library/BaseGpioCheckConflictLibNull/BaseGpioCheckConflictLibNull.inf
./Platform/Intel/CometlakeOpenBoardPkg/CometlakeURvp/Library/BasePlatformHookLib/BasePlatformHookLib.inf
./Platform/Intel/CometlakeOpenBoardPkg/CometlakeURvp/Library/BoardAcpiLib/SmmBoardAcpiEnableLib.inf
./Platform/Intel/CometlakeOpenBoardPkg/CometlakeURvp/Library/BoardAcpiLib/SmmMultiBoardAcpiSupportLib.inf
./Platform/Intel/CometlakeOpenBoardPkg/CometlakeURvp/Library/BoardInitLib/PeiBoardInitPostMemLib.inf
./Platform/Intel/CometlakeOpenBoardPkg/CometlakeURvp/Library/BoardInitLib/PeiBoardInitPreMemLib.inf
./Platform/Intel/CometlakeOpenBoardPkg/CometlakeURvp/Library/BoardInitLib/PeiMultiBoardInitPostMemLib.inf
./Platform/Intel/CometlakeOpenBoardPkg/CometlakeURvp/Library/BoardInitLib/PeiMultiBoardInitPreMemLib.inf
./Platform/Intel/CometlakeOpenBoardPkg/CometlakeURvp/Library/DxePolicyBoardConfigLib/DxePolicyBoardConfigLib.inf
./Platform/Intel/CometlakeOpenBoardPkg/CometlakeURvp/Library/PeiPolicyBoardConfigLib/PeiPolicyBoardConfigLib.inf
./Platform/Intel/CometlakeOpenBoardPkg/Features/Tbt/Library/DxeTbtPolicyLib/DxeTbtPolicyLib.inf
./Platform/Intel/CometlakeOpenBoardPkg/Features/Tbt/Library/PeiDxeSmmTbtCommonLib/TbtCommonLib.inf
./Platform/Intel/CometlakeOpenBoardPkg/Features/Tbt/Library/PeiTbtPolicyLib/PeiTbtPolicyLib.inf
./Platform/Intel/CometlakeOpenBoardPkg/Features/Tbt/Library/Private/PeiDTbtInitLib/PeiDTbtInitLib.inf
./Platform/Intel/CometlakeOpenBoardPkg/FspWrapper/Library/PeiFspPolicyInitLib/PeiFspPolicyInitLib.inf
./Platform/Intel/CometlakeOpenBoardPkg/FspWrapper/Library/PeiSiliconPolicyUpdateLibFsp/PeiSiliconPolicyUpdateLibFsp.inf
./Platform/Intel/CometlakeOpenBoardPkg/Policy/Library/DxePolicyUpdateLib/DxePolicyUpdateLib.inf
./Platform/Intel/CometlakeOpenBoardPkg/Policy/Library/PeiPolicyInitLib/PeiPolicyInitLib.inf
./Platform/Intel/CometlakeOpenBoardPkg/Policy/Library/PeiPolicyUpdateLib/PeiPolicyUpdateLib.inf
./Platform/Intel/KabylakeOpenBoardPkg/Features/Tbt/Library/DxeTbtPolicyLib/DxeTbtPolicyLib.inf
./Platform/Intel/KabylakeOpenBoardPkg/Features/Tbt/Library/PeiDxeSmmTbtCommonLib/TbtCommonLib.inf
./Platform/Intel/KabylakeOpenBoardPkg/Features/Tbt/Library/PeiTbtPolicyLib/PeiTbtPolicyLib.inf
./Platform/Intel/KabylakeOpenBoardPkg/Features/Tbt/Library/Private/PeiDTbtInitLib/PeiDTbtInitLib.inf
./Platform/Intel/KabylakeOpenBoardPkg/FspWrapper/Library/PeiSiliconPolicyNotifyLib/PeiPreMemSiliconPolicyNotifyLib.inf
./Platform/Intel/KabylakeOpenBoardPkg/GalagoPro3/FspWrapper/Library/PeiSiliconPolicyNotifyLib/PeiPreMemSiliconPolicyNotifyLib.inf
./Platform/Intel/KabylakeOpenBoardPkg/GalagoPro3/FspWrapper/Library/PeiSiliconPolicyUpdateLibFsp/PeiSiliconPolicyUpdateLibFsp.inf
./Platform/Intel/KabylakeOpenBoardPkg/GalagoPro3/Library/BasePlatformHookLib/BasePlatformHookLib.inf
./Platform/Intel/KabylakeOpenBoardPkg/GalagoPro3/Library/BoardAcpiLib/DxeBoardAcpiTableLib.inf
./Platform/Intel/KabylakeOpenBoardPkg/GalagoPro3/Library/BoardAcpiLib/DxeMultiBoardAcpiSupportLib.inf
./Platform/Intel/KabylakeOpenBoardPkg/GalagoPro3/Library/BoardAcpiLib/SmmBoardAcpiEnableLib.inf
./Platform/Intel/KabylakeOpenBoardPkg/GalagoPro3/Library/BoardAcpiLib/SmmMultiBoardAcpiSupportLib.inf
./Platform/Intel/KabylakeOpenBoardPkg/GalagoPro3/Library/BoardInitLib/PeiBoardInitPostMemLib.inf
./Platform/Intel/KabylakeOpenBoardPkg/GalagoPro3/Library/BoardInitLib/PeiBoardInitPreMemLib.inf
./Platform/Intel/KabylakeOpenBoardPkg/GalagoPro3/Library/BoardInitLib/PeiMultiBoardInitPostMemLib.inf
./Platform/Intel/KabylakeOpenBoardPkg/GalagoPro3/Library/BoardInitLib/PeiMultiBoardInitPreMemLib.inf
./Platform/Intel/KabylakeOpenBoardPkg/GalagoPro3/Policy/Library/DxeSiliconPolicyUpdateLib/DxeSiliconPolicyUpdateLib.inf
./Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/FspWrapper/Library/PeiSiliconPolicyUpdateLibFsp/PeiSiliconPolicyUpdateLibFsp.inf
./Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/Library/BasePlatformHookLib/BasePlatformHookLib.inf
./Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/Library/BoardAcpiLib/DxeBoardAcpiTableLib.inf
./Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/Library/BoardAcpiLib/DxeMultiBoardAcpiSupportLib.inf
./Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/Library/BoardAcpiLib/SmmBoardAcpiEnableLib.inf
./Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/Library/BoardAcpiLib/SmmMultiBoardAcpiSupportLib.inf
./Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/Library/BoardInitLib/PeiBoardInitPostMemLib.inf
./Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/Library/BoardInitLib/PeiBoardInitPreMemLib.inf
./Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/Library/BoardInitLib/PeiMultiBoardInitPostMemLib.inf
./Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/Library/BoardInitLib/PeiMultiBoardInitPreMemLib.inf
./Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/Policy/Library/DxeSiliconPolicyUpdateLib/DxeSiliconPolicyUpdateLib.inf
./Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/Policy/Library/PeiSiliconPolicyUpdateLib/PeiSiliconPolicyUpdateLib.inf
./Platform/Intel/MinPlatformPkg/Acpi/Library/BoardAcpiEnableLibNull/BoardAcpiEnableLibNull.inf
./Platform/Intel/MinPlatformPkg/Acpi/Library/BoardAcpiTableLibNull/BoardAcpiTableLibNull.inf
./Platform/Intel/MinPlatformPkg/Acpi/Library/DxeAslUpdateLib/DxeAslUpdateLib.inf
./Platform/Intel/MinPlatformPkg/Acpi/Library/MultiBoardAcpiSupportLib/DxeMultiBoardAcpiSupportLib.inf
./Platform/Intel/MinPlatformPkg/Acpi/Library/MultiBoardAcpiSupportLib/SmmMultiBoardAcpiSupportLib.inf
./Platform/Intel/MinPlatformPkg/Bds/Library/BoardBootManagerLibNull/BoardBootManagerLibNull.inf
./Platform/Intel/MinPlatformPkg/Bds/Library/DxePlatformBootManagerLib/DxePlatformBootManagerLib.inf
./Platform/Intel/MinPlatformPkg/Flash/Library/SpiFlashCommonLibNull/SpiFlashCommonLibNull.inf
./Platform/Intel/MinPlatformPkg/FspWrapper/Library/DxeFspWrapperPlatformLib/DxeFspWrapperPlatformLib.inf
./Platform/Intel/MinPlatformPkg/FspWrapper/Library/PeiFspWrapperHobProcessLib/PeiFspWrapperHobProcessLib.inf
./Platform/Intel/MinPlatformPkg/FspWrapper/Library/PeiFspWrapperPlatformLib/PeiFspWrapperPlatformLib.inf
./Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecFspWrapperPlatformSecLib.inf
./Platform/Intel/MinPlatformPkg/Pci/Library/PciHostBridgeLibSimple/PciHostBridgeLibSimple.inf
./Platform/Intel/MinPlatformPkg/Pci/Library/PciSegmentInfoLibSimple/PciSegmentInfoLibSimple.inf
./Platform/Intel/MinPlatformPkg/PlatformInit/Library/BoardInitLibNull/BoardInitLibNull.inf
./Platform/Intel/MinPlatformPkg/PlatformInit/Library/MultiBoardInitSupportLib/DxeMultiBoardInitSupportLib.inf
./Platform/Intel/MinPlatformPkg/PlatformInit/Library/MultiBoardInitSupportLib/PeiMultiBoardInitSupportLib.inf
./Platform/Intel/MinPlatformPkg/PlatformInit/Library/PeiReportFvLib/PeiReportFvLib.inf
./Platform/Intel/MinPlatformPkg/PlatformInit/Library/ReportCpuHobLib/ReportCpuHobLib.inf
./Platform/Intel/MinPlatformPkg/PlatformInit/Library/SecBoardInitLibNull/SecBoardInitLibNull.inf
./Platform/Intel/MinPlatformPkg/PlatformInit/Library/SiliconPolicyInitLibNull/SiliconPolicyInitLibNull.inf
./Platform/Intel/MinPlatformPkg/PlatformInit/Library/SiliconPolicyUpdateLibNull/SiliconPolicyUpdateLibNull.inf
./Platform/Intel/MinPlatformPkg/Tcg/Library/PeiDxeTpmPlatformHierarchyLib/PeiDxeTpmPlatformHierarchyLib.inf
./Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestPointCheckLib.inf
./Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/PeiTestPointCheckLib.inf
./Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/SecTestPointCheckLib.inf
./Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/SmmTestPointCheckLib.inf
./Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLibNull/TestPointCheckLibNull.inf
./Platform/Intel/MinPlatformPkg/Test/Library/TestPointLib/DxeTestPointLib.inf
./Platform/Intel/MinPlatformPkg/Test/Library/TestPointLib/PeiTestPointLib.inf
./Platform/Intel/MinPlatformPkg/Test/Library/TestPointLib/SmmTestPointLib.inf
......
============

> -----Original Message-----
> From: James Bottomley <jejb at linux.ibm.com>
> Sent: Monday, July 26, 2021 10:50 PM
> To: Yao, Jiewen <jiewen.yao at intel.com>; devel at edk2.groups.io;
> dovmurik at linux.ibm.com
> Cc: Tobin Feldman-Fitzthum <tobin at linux.ibm.com>; Tobin Feldman-Fitzthum
> <tobin at ibm.com>; Jim Cadden <jcadden at ibm.com>; Hubertus Franke
> <frankeh at us.ibm.com>; Ard Biesheuvel <ardb+tianocore at kernel.org>; Justen,
> Jordan L <jordan.l.justen at intel.com>; Ashish Kalra <ashish.kalra at amd.com>;
> Brijesh Singh <brijesh.singh at amd.com>; Erdem Aktas
> <erdemaktas at google.com>; Xu, Min M <min.m.xu at intel.com>; Tom Lendacky
> <thomas.lendacky at amd.com>; Leif Lindholm <leif at nuviainc.com>; Sami
> Mujawar <sami.mujawar at arm.com>
> Subject: Re: [edk2-devel] [PATCH v4 00/11] Measured SEV boot with
> kernel/initrd/cmdline
> 
> On Mon, 2021-07-26 at 00:55 +0000, Yao, Jiewen wrote:
> > Hi James
> > "However, this ran into problems when it was decided AmdSev shouldn't
> > have it's own Library."
> >
> > I am not clear on the history. Would you please clarify why AmdSev
> > should not have its own library?
> 
> The history predates me.  It was already done for the Bhyve package
> which also has a modified PlatformBootManagerLib when I came along with
> this.  However, only having Library in the top level package seems to
> be a common edk2 pattern if you run a find.
> 
> > It looks not reasonable to me. AmdSev is just a feature. A feature
> > may have its own library. We have enough examples.
> 
> We do?  Running
> 
> find . -name Library -print
> 
> only turns up
> 
> ./FmpDevicePkg/Test/UnitTest/Library
> 
> As not following the top level package only pattern.
> 
> > Also, the instance name "Grub" is very confusing. I compared
> > PlatformBootManagerLib and PlatformBootManagerLibGrub. This is just a
> > customized PlatformBootManagerLib.
> 
> It's called Grub because it places Grub in the Fv for combined pre-
> attestation.  Either SEV or TDX could use this (Although TDX looks
> likely not to want to).
> 
> > For example, XEN feature removing and PIIX4 difference has nothing to
> > do with Grub...
> > =================
> >       PciWrite8 (PCI_LIB_ADDRESS (0, 1, 0, 0x60), 0x0b); // A
> >       PciWrite8 (PCI_LIB_ADDRESS (0, 1, 0, 0x61), 0x0b); // B
> >       PciWrite8 (PCI_LIB_ADDRESS (0, 1, 0, 0x62), 0x0a); // C
> >       PciWrite8 (PCI_LIB_ADDRESS (0, 1, 0, 0x63), 0x0a); // D
> > =================
> 
> It's part of the boot path stripping to make sure there's a hard
> failure if Grub fails to execute.  There's a Bugzilla requiring more of
> this because a grub only booting platform library needs fewer
> extraneous things which could constitute an attack surface for the
> injected secret.
> 
> > It is a big misleading. Can we move the PlatformBootManagerLibGrub To
> > AmdSev now?
> 
> I think you probably want to ask around older edk2 package maintainers
> and see if there's any reason for this pattern, which seems to be
> strongly enforced.  If no-one can remember, then likely it can be
> broken.
> 
> James
> 

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#78178): https://edk2.groups.io/g/devel/message/78178
Mute This Topic: https://groups.io/mt/84375116/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-