Re: [edk2-devel] [PATCH v2 00/23] Provide SEV-SNP support for running under an SVSM

["Yao, Jiewen" <jiewen.yao@intel.com>]

Below:

> -----Original Message-----
> From: Tom Lendacky <thomas.lendacky@amd.com>
> Sent: Thursday, February 29, 2024 12:20 AM
> To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>; Aktas, Erdem
> <erdemaktas@google.com>; Gerd Hoffmann <kraxel@redhat.com>; Laszlo Ersek
> <lersek@redhat.com>; Liming Gao <gaoliming@byosoft.com.cn>; Kinney, Michael
> D <michael.d.kinney@intel.com>; Xu, Min M <min.m.xu@intel.com>; Liu,
> Zhiguang <zhiguang.liu@intel.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>;
> Ni, Ray <ray.ni@intel.com>; Michael Roth <michael.roth@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.lendacky@amd.com>
> >> Sent: Friday, February 23, 2024 1:30 AM
> >> To: devel@edk2.groups.io
> >> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>; Aktas, Erdem
> >> <erdemaktas@google.com>; Gerd Hoffmann <kraxel@redhat.com>; Yao,
> Jiewen
> >> <jiewen.yao@intel.com>; Laszlo Ersek <lersek@redhat.com>; Liming Gao
> >> <gaoliming@byosoft.com.cn>; Kinney, Michael D
> <michael.d.kinney@intel.com>;
> >> Xu, Min M <min.m.xu@intel.com>; Liu, Zhiguang <zhiguang.liu@intel.com>;
> >> Kumar, Rahul R <rahul.r.kumar@intel.com>; Ni, Ray <ray.ni@intel.com>;
> Michael
> >> Roth <michael.roth@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/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-