On 2/29/24 08:06, Yao, Jiewen wrote:
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.
Sounds good. I'll move to the UefiCpuPkg since MpInitLib will be using it.
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?
There are definitely some things that will be common,
CcSvsmIsSvsmPresent() and CcSvsmSnpGetCaa(), and some things that will be
SNP or TDX specific. For example, the concept of turning a page into a
VMSA page or how the SVSM will be invoked will be different.
For now, I'll create an AMD specific library and then when TDX is ready to
support an SVSM we can look to see how or what needs to be changed. It
could be that they need to remain separate if there is not enough in common.
Thanks,
Tom
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 (#116170): https://edk2.groups.io/g/devel/message/116170
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]
-=-=-=-=-=-=-=-=-=-=-=-
