On 11/3/23 16:30, Wu, Jiaxin wrote: > The SmmCpuSyncLib instance is included in UefiCpuLibs.dsc.inc. > This patch is to specify SmmCpuSyncLib instance in OvmfPkg by > using "!include UefiCpuPkg/UefiCpuLibs.dsc.inc". > > Change-Id: I2ab1737425e26a7bfc4f564b3b7f15ca5c2268fb > Cc: Ard Biesheuvel <ardb+tianoc...@kernel.org> > Cc: Jiewen Yao <jiewen....@intel.com> > Cc: Jordan Justen <jordan.l.jus...@intel.com> > Cc: Eric Dong <eric.d...@intel.com> > Cc: Ray Ni <ray...@intel.com> > Cc: Zeng Star <star.z...@intel.com> > Cc: Rahul Kumar <rahul1.ku...@intel.com> > Cc: Gerd Hoffmann <kra...@redhat.com> > Signed-off-by: Jiaxin Wu <jiaxin...@intel.com> > --- > OvmfPkg/CloudHv/CloudHvX64.dsc | 1 + > OvmfPkg/OvmfPkgIa32.dsc | 1 + > OvmfPkg/OvmfPkgIa32X64.dsc | 1 + > OvmfPkg/OvmfPkgX64.dsc | 1 + > 4 files changed, 4 insertions(+) > > diff --git a/OvmfPkg/CloudHv/CloudHvX64.dsc b/OvmfPkg/CloudHv/CloudHvX64.dsc > index c23c7eaf6c..e65767fe16 100644 > --- a/OvmfPkg/CloudHv/CloudHvX64.dsc > +++ b/OvmfPkg/CloudHv/CloudHvX64.dsc > @@ -122,10 +122,11 @@ > # Library Class section - list of all Library Classes needed by this > Platform. > # > > ################################################################################ > > !include MdePkg/MdeLibs.dsc.inc > +!include UefiCpuPkg/UefiCpuLibs.dsc.inc > > [LibraryClasses] > PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf > TimerLib|OvmfPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.inf > ResetSystemLib|OvmfPkg/Library/ResetSystemLib/BaseResetSystemLib.inf > diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc > index ed3a19feeb..07d16e6935 100644 > --- a/OvmfPkg/OvmfPkgIa32.dsc > +++ b/OvmfPkg/OvmfPkgIa32.dsc > @@ -125,10 +125,11 @@ > # Library Class section - list of all Library Classes needed by this > Platform. > # > > ################################################################################ > > !include MdePkg/MdeLibs.dsc.inc > +!include UefiCpuPkg/UefiCpuLibs.dsc.inc > > [LibraryClasses] > PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf > TimerLib|OvmfPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.inf > ResetSystemLib|OvmfPkg/Library/ResetSystemLib/BaseResetSystemLib.inf > diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc > index 16ca139b29..8d243b7075 100644 > --- a/OvmfPkg/OvmfPkgIa32X64.dsc > +++ b/OvmfPkg/OvmfPkgIa32X64.dsc > @@ -130,10 +130,11 @@ > # Library Class section - list of all Library Classes needed by this > Platform. > # > > ################################################################################ > > !include MdePkg/MdeLibs.dsc.inc > +!include UefiCpuPkg/UefiCpuLibs.dsc.inc > > [LibraryClasses] > PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf > TimerLib|OvmfPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.inf > ResetSystemLib|OvmfPkg/Library/ResetSystemLib/BaseResetSystemLib.inf > diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc > index dc1a0942aa..6343789152 100644 > --- a/OvmfPkg/OvmfPkgX64.dsc > +++ b/OvmfPkg/OvmfPkgX64.dsc > @@ -143,10 +143,11 @@ > # Library Class section - list of all Library Classes needed by this > Platform. > # > > ################################################################################ > > !include MdePkg/MdeLibs.dsc.inc > +!include UefiCpuPkg/UefiCpuLibs.dsc.inc > > [LibraryClasses] > PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf > TimerLib|OvmfPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.inf > ResetSystemLib|OvmfPkg/Library/ResetSystemLib/BaseResetSystemLib.inf
I am undecided about this patch. More precisely, I'm undecided that this is the right way to introduce "UefiCpuLibs.dsc.inc". In my opinion,"UefiCpuLibs.dsc.inc" should be a general-purpose include file. But, at this moment, the include file only resolves the "SmmCpuSyncLib" class -- and that resolution is useless for any IA32/X64 platform that does not use the SMM machinery. In particular, the resolution is useless even for OVMF X64 (for example) if it is built without SMM_REQUIRE. Furthermore, OVMF X64 uses a bunch of other UefiCpuPkg libraries: BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf CcExitLibNull/CcExitLibNull.inf CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.inf CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.inf CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib.inf CpuExceptionHandlerLib/SmmCpuExceptionHandlerLib.inf CpuPageTableLib/CpuPageTableLib.inf MicrocodeLib/MicrocodeLib.inf MmSaveStateLib/AmdMmSaveStateLib.inf MpInitLib/DxeMpInitLib.inf MpInitLib/PeiMpInitLib.inf MpInitLibUp/MpInitLibUp.inf MtrrLib/MtrrLib.inf SmmCpuRendezvousLib/SmmCpuRendezvousLib.inf It's weird to put SmmCpuSyncLib in the new DSC include file, but not CpuPageTableLib or MtrrLib (for example), which seem much more generic. For now I'd suggest avoiding "UefiCpuLibs.dsc.inc" altogether, and resolving just "SmmCpuSyncLib" in the OVMF DSC files -- and only when SMM_REQUIRE is defined on the build command line (i.e., when PiSmmCpuDxeSmm is built into the firmware platform). We can certainly introduce "UefiCpuLibs.dsc.inc" later, but for that: - we need to collect all UefiCpuPkg library classes and instances that are commonly used (and then probably use multiple module type sections!). Example: "OvmfPkg/Include/Dsc/OvmfTpmLibs.dsc.inc" - some of those resolutions should be conditional, and so we'd have to properly prefix macro names like SMM_REQUIRE with something like UEFI_CPU_. Basically introduce a namespace for those build time macros. Example: "NetworkPkg/NetworkDefines.dsc.inc", which uses the NETWORK_ macro prefix for enabling various features. Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#110840): https://edk2.groups.io/g/devel/message/110840 Mute This Topic: https://groups.io/mt/102366302/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
