Some feedback: 1) 0002-MdePkg-GHCB-APIC-ID-retrieval-support-definitions
MdePkg only contains the definition in the standard. Question: Is EFI_APIC_IDS_GUID definition in some AMD/SVSM specification? 2) 0012-UefiCpuPkg-CcSvsmLib-Create-the-CcSvsmLib-library-to-support-an-SVSM I am not sure the position of SVSM. If the SVSM interface is AMD specific, the it should be AmdSvsmLib. If the SVSM interface is generic, then we should define everything in a generic way. It is very confusing to mix a generic CcSvsm lib with AMD specific <Register/Amd/Ghcb.h>. Thank you Yao, Jiewen > -----Original Message----- > From: Tom Lendacky <thomas.lenda...@amd.com> > Sent: Friday, February 23, 2024 1:30 AM > To: devel@edk2.groups.io > Cc: Ard Biesheuvel <ardb+tianoc...@kernel.org>; Aktas, Erdem > <erdemak...@google.com>; Gerd Hoffmann <kra...@redhat.com>; Yao, Jiewen > <jiewen....@intel.com>; Laszlo Ersek <ler...@redhat.com>; Liming Gao > <gaolim...@byosoft.com.cn>; Kinney, Michael D <michael.d.kin...@intel.com>; > Xu, Min M <min.m...@intel.com>; Liu, Zhiguang <zhiguang....@intel.com>; > Kumar, Rahul R <rahul.r.ku...@intel.com>; Ni, Ray <ray...@intel.com>; Michael > Roth <michael.r...@amd.com> > Subject: [PATCH v2 00/23] Provide SEV-SNP support for running under an SVSM > > > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4654 > > This series adds SEV-SNP support for running OVMF under an Secure VM > Service Module (SVSM) at a less privileged VM Privilege Level (VMPL). > By running at a less priviledged VMPL, the SVSM can be used to provide > services, e.g. a virtual TPM, for the guest OS within the SEV-SNP > confidential VM (CVM) rather than trust such services from the hypervisor. > > Currently, OVMF expects to run at the highest VMPL, VMPL0, and there are > certain SNP related operations that require that VMPL level. Specifically, > the PVALIDATE instruction and the RMPADJUST instruction when setting the > the VMSA attribute of a page (used when starting APs). > > If OVMF is to run at a less privileged VMPL, e.g. VMPL2, then it must > use an SVSM (which is running at VMPL0) to perform the operations that > it is no longer able to perform. > > When running under an SVSM, OVMF must know the APIC IDs of the vCPUs that > it will be starting. As a result, the GHCB APIC ID retrieval action must > be performed. Since this service can also work with SEV-SNP running at > VMPL0, the patches to make use of this feature are near the beginning of > the series. > > How OVMF interacts with and uses the SVSM is documented in the SVSM > specification [1] and the GHCB specification [2]. > > This support creates a new CcSvsmLib library that is used by MpInitLib. > This requires an update to the edk2-platform DSC files to add the new > library. The edk2-platform change would be needed after patch 12, but > before patch 15. > > This series introduces support to run OVMF under an SVSM. It consists > of: > - Retrieving the list of vCPU APIC IDs and starting up all APs without > performing a broadcast SIPI > - Reorganizing the page state change support to not directly use the > GHCB buffer since an SVSM will use the calling area buffer, instead > - Detecting the presence of an SVSM > - When not running at VMPL0, invoking the SVSM for page validation and > VMSA page creation/deletion > - Detecting and allowing OVMF to run in a VMPL other than 0 when an > SVSM is present > > The series is based off of commit: > > 2ca8d5597443 ("UefiCpuPkg/PiSmmCpuDxeSmm: Check BspIndex first before > lock cmpxchg") > > [1] https://www.amd.com/content/dam/amd/en/documents/epyc-technical- > docs/specifications/58019.pdf > [2] https://www.amd.com/content/dam/amd/en/documents/epyc-technical- > docs/specifications/56421.pdf > > --- > > Changes in v2: > - Move the APIC IDs retrieval support to the beginning of the patch series > - Use a GUIDed HOB to hold the APIC ID list instead of a PCD > - Split up Page State Change reorganization into multiple patches > - Created CcSvsmLib library instead of extending CcExitLib > - This will require a corresponding update to edk2-platform DSC files > - Removed Ray Ni's Acked-by since it is not a minor change > - Variable name changes and other misc changes > > Tom Lendacky (23): > OvmfPkg/BaseMemEncryptLib: Fix error check from AsmRmpAdjust() > MdePkg: GHCB APIC ID retrieval support definitions > OvmfPkg/PlatformPei: Retrieve APIC IDs from the hypervisor > UefiCpuPkg/MpInitLib: Always use AP Create if PcdSevSnpApicIds is set > OvmfPkg/BaseMemEncryptSevLib: Fix uncrustify errors > OvmfPkg/BaseMemEncryptSevLib: Calculate memory size for Page State > Change > MdePkg: Avoid hardcoded value for number of Page State Change entries > OvmfPkg/BaseMemEncryptSevLib: Re-organize page state change support > OvmfPkg/BaseMemEncryptSevLib: Maximize Page State Change efficiency > MdePkg/Register/Amd: Define the SVSM related information > MdePkg/BaseLib: Add a new VMGEXIT instruction invocation for SVSM > UefiCpuPkg/CcSvsmLib: Create the CcSvsmLib library to support an SVSM > UefiPayloadPkg: Prepare UefiPayloadPkg to use the CcSvsmLib library > Ovmfpkg/CcSvsmLib: Create CcSvsmLib to handle SVSM related services > UefiCpuPkg/MpInitLib: Use CcSvsmSnpVmsaRmpAdjust() to set/clear VMSA > OvmfPkg/BaseMemEncryptSevLib: Use CcSvsmSnpPvalidate() to validate > pages > OvmfPkg: Create a calling area used to communicate with the SVSM > OvmfPkg/CcSvsmLib: Add support for the SVSM_CORE_PVALIDATE call > OvmfPkg/BaseMemEncryptSevLib: Maximize Page State Change efficiency > OvmfPkg/CcSvsmLib: Add support for the SVSM create/delete vCPU calls > UefiCpuPkg/MpInitLib: AP creation support under an SVSM > Ovmfpkg/CcExitLib: Provide SVSM discovery support > OvmfPkg/BaseMemEncryptLib: Check for presence of an SVSM when not at > VMPL0 > > MdePkg/MdePkg.dec | 5 > +- > OvmfPkg/OvmfPkg.dec | 4 + > UefiCpuPkg/UefiCpuPkg.dec | 5 > +- > OvmfPkg/AmdSev/AmdSevX64.dsc | 1 + > OvmfPkg/Bhyve/BhyveX64.dsc | 1 + > OvmfPkg/CloudHv/CloudHvX64.dsc | 1 + > OvmfPkg/IntelTdx/IntelTdxX64.dsc | 1 + > OvmfPkg/Microvm/MicrovmX64.dsc | 1 + > OvmfPkg/OvmfPkgIa32.dsc | 1 + > OvmfPkg/OvmfPkgIa32X64.dsc | 3 > +- > OvmfPkg/OvmfPkgX64.dsc | 1 + > OvmfPkg/OvmfXen.dsc | 1 + > UefiCpuPkg/UefiCpuPkg.dsc | 4 > +- > UefiPayloadPkg/UefiPayloadPkg.dsc | 1 + > OvmfPkg/AmdSev/AmdSevX64.fdf | 9 > +- > OvmfPkg/OvmfPkgX64.fdf | 3 + > MdePkg/Library/BaseLib/BaseLib.inf | 2 + > OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf | 3 > +- > OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf | 3 > +- > OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLib.inf | 3 > +- > OvmfPkg/Library/CcExitLib/CcExitLib.inf | 3 > +- > OvmfPkg/Library/CcExitLib/SecCcExitLib.inf | 3 > +- > OvmfPkg/Library/CcSvsmLib/CcSvsmLib.inf | 38 > ++ > OvmfPkg/PlatformPei/PlatformPei.inf | 3 + > OvmfPkg/ResetVector/ResetVector.inf | 2 + > UefiCpuPkg/Library/CcSvsmLibNull/CcSvsmLibNull.inf | 27 > ++ > UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf | 2 + > UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf | 2 + > MdePkg/Include/Library/BaseLib.h | 39 > ++ > MdePkg/Include/Register/Amd/Fam17Msr.h | 19 > +- > MdePkg/Include/Register/Amd/Ghcb.h | 23 > +- > MdePkg/Include/Register/Amd/Msr.h | 3 > +- > MdePkg/Include/Register/Amd/Svsm.h | 101 > ++++ > MdePkg/Include/Register/Amd/SvsmMsr.h | 35 > ++ > OvmfPkg/Include/WorkArea.h | 9 > +- > OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChange.h | 6 > +- > UefiCpuPkg/Include/Library/CcSvsmLib.h | 101 > ++++ > UefiCpuPkg/Library/MpInitLib/MpLib.h | 29 > +- > OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c | > 11 +- > OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c | 27 > +- > OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiSnpSystemRamValidate.c | > 22 +- > OvmfPkg/Library/BaseMemEncryptSevLib/X64/SecSnpSystemRamValidate.c | > 31 +- > OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChangeInternal.c | > 206 ++++---- > OvmfPkg/Library/CcExitLib/CcExitVcHandler.c | 29 > +- > OvmfPkg/Library/CcSvsmLib/CcSvsmLib.c | 500 > ++++++++++++++++++++ > OvmfPkg/PlatformPei/AmdSev.c | 102 > +++- > UefiCpuPkg/Library/CcSvsmLibNull/CcSvsmLibNull.c | 108 > +++++ > UefiCpuPkg/Library/MpInitLib/Ia32/AmdSev.c | 21 > +- > UefiCpuPkg/Library/MpInitLib/MpLib.c | 9 > +- > UefiCpuPkg/Library/MpInitLib/X64/AmdSev.c | 134 > ++++-- > MdePkg/Library/BaseLib/Ia32/VmgExitSvsm.nasm | 39 > ++ > MdePkg/Library/BaseLib/X64/VmgExitSvsm.nasm | 94 > ++++ > OvmfPkg/ResetVector/ResetVector.nasmb | 6 > +- > OvmfPkg/ResetVector/X64/OvmfSevMetadata.asm | 11 > +- > UefiCpuPkg/Library/CcSvsmLibNull/CcSvsmLibNull.uni | 13 + > 55 files changed, 1628 insertions(+), 233 deletions(-) > create mode 100644 OvmfPkg/Library/CcSvsmLib/CcSvsmLib.inf > create mode 100644 UefiCpuPkg/Library/CcSvsmLibNull/CcSvsmLibNull.inf > create mode 100644 MdePkg/Include/Register/Amd/Svsm.h > create mode 100644 MdePkg/Include/Register/Amd/SvsmMsr.h > create mode 100644 UefiCpuPkg/Include/Library/CcSvsmLib.h > create mode 100644 OvmfPkg/Library/CcSvsmLib/CcSvsmLib.c > create mode 100644 UefiCpuPkg/Library/CcSvsmLibNull/CcSvsmLibNull.c > create mode 100644 MdePkg/Library/BaseLib/Ia32/VmgExitSvsm.nasm > create mode 100644 MdePkg/Library/BaseLib/X64/VmgExitSvsm.nasm > create mode 100644 UefiCpuPkg/Library/CcSvsmLibNull/CcSvsmLibNull.uni > > -- > 2.42.0 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#116092): https://edk2.groups.io/g/devel/message/116092 Mute This Topic: https://groups.io/mt/104512925/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
