Below: > -----Original Message----- > From: Tom Lendacky <thomas.lenda...@amd.com> > Sent: Thursday, February 29, 2024 12:20 AM > To: Yao, Jiewen <jiewen....@intel.com>; devel@edk2.groups.io > Cc: Ard Biesheuvel <ardb+tianoc...@kernel.org>; Aktas, Erdem > <erdemak...@google.com>; Gerd Hoffmann <kra...@redhat.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: Re: [PATCH v2 00/23] Provide SEV-SNP support for running under an > SVSM > > On 2/28/24 00:14, Yao, Jiewen wrote: > > 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? > > The structure is documented in the GHCB specification, but the GUID is not. > > Is the request to move the GUID to someplace other than MdePkg?
[Jiewen] Right. If the GUID is NOT in GHCB spec, then it should be in other place, such as OvmfPkg. > > > > > 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. > > I believe TDX is also looking at the SVSM for TDX partitioning, but I'm > not certain of that. > > > 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>. > > I can certainly change the name to be AMD specific fow now. It can always > be changed to something else later if need be, much like VmgExitLib was > changed to CcExitLib. [Jiewen] Yes, Intel is planning for SVSM. But it is NOT ready yet. It is hard for me to discuss it now. Maybe, please help me understand: Is CcSvsmLib a generic library / common protocol between OVMF and Coconut-SVSM? - Option 1 Or is CcSvsmLib an implementation specific library, and the current API cannot be shared with Intel TDX in future? - Option 2 I notice that some API is for option 1 - CcSvsmIsSvsmPresent(). But some API is for option 2 - CcSvsmSnpGetVmpl(), CcSvsmSnpGetCaa(), CcSvsmSnpPvalidate(), CcSvsmSnpVmsaRmpAdjust(). How do you plan if TDX need to support SVSM later? How do you plan if we need to add some generic interaction between OVMF and coconut-SVSM, such as vTPM? > > Thanks, > Tom > > > > > > > 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 (#116169): https://edk2.groups.io/g/devel/message/116169 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] -=-=-=-=-=-=-=-=-=-=-=-
