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 <j...@linux.ibm.com> > Sent: Monday, July 26, 2021 10:50 PM > To: Yao, Jiewen <jiewen....@intel.com>; devel@edk2.groups.io; > dovmu...@linux.ibm.com > Cc: Tobin Feldman-Fitzthum <to...@linux.ibm.com>; Tobin Feldman-Fitzthum > <to...@ibm.com>; Jim Cadden <jcad...@ibm.com>; Hubertus Franke > <fran...@us.ibm.com>; Ard Biesheuvel <ardb+tianoc...@kernel.org>; Justen, > Jordan L <jordan.l.jus...@intel.com>; Ashish Kalra <ashish.ka...@amd.com>; > Brijesh Singh <brijesh.si...@amd.com>; Erdem Aktas > <erdemak...@google.com>; Xu, Min M <min.m...@intel.com>; Tom Lendacky > <thomas.lenda...@amd.com>; Leif Lindholm <l...@nuviainc.com>; Sami > Mujawar <sami.muja...@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/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
