public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Lendacky, Thomas via groups.io" <thomas.lendacky=amd.com@groups.io>
To: "Yao, Jiewen" <jiewen.yao@intel.com>,
	"devel@edk2.groups.io" <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: [edk2-devel] [PATCH v2 00/23] Provide SEV-SNP support for running under an SVSM
Date: Wed, 28 Feb 2024 10:19:41 -0600	[thread overview]
Message-ID: <30de7630-870b-41d4-9da3-5486c8fc44fe@amd.com> (raw)
In-Reply-To: <MW4PR11MB5872D32CACF8BA6B0574A60C8C582@MW4PR11MB5872.namprd11.prod.outlook.com>

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?

> 
> 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.

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 (#116121): https://edk2.groups.io/g/devel/message/116121
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]
-=-=-=-=-=-=-=-=-=-=-=-



  reply	other threads:[~2024-02-28 16:19 UTC|newest]

Thread overview: 53+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-22 17:29 [edk2-devel] [PATCH v2 00/23] Provide SEV-SNP support for running under an SVSM Lendacky, Thomas via groups.io
2024-02-22 17:29 ` [edk2-devel] [PATCH v2 01/23] OvmfPkg/BaseMemEncryptLib: Fix error check from AsmRmpAdjust() Lendacky, Thomas via groups.io
2024-02-27  9:46   ` Gerd Hoffmann
2024-02-22 17:29 ` [edk2-devel] [PATCH v2 02/23] MdePkg: GHCB APIC ID retrieval support definitions Lendacky, Thomas via groups.io
2024-02-23  0:16   ` Ni, Ray
2024-02-27 10:02     ` Gerd Hoffmann
2024-02-22 17:29 ` [edk2-devel] [PATCH v2 03/23] OvmfPkg/PlatformPei: Retrieve APIC IDs from the hypervisor Lendacky, Thomas via groups.io
2024-02-27 10:03   ` Gerd Hoffmann
2024-02-22 17:29 ` [edk2-devel] [PATCH v2 04/23] UefiCpuPkg/MpInitLib: Always use AP Create if PcdSevSnpApicIds is set Lendacky, Thomas via groups.io
2024-02-27 10:11   ` Gerd Hoffmann
2024-02-22 17:29 ` [edk2-devel] [PATCH v2 05/23] OvmfPkg/BaseMemEncryptSevLib: Fix uncrustify errors Lendacky, Thomas via groups.io
2024-02-27 10:12   ` Gerd Hoffmann
2024-02-22 17:29 ` [edk2-devel] [PATCH v2 06/23] OvmfPkg/BaseMemEncryptSevLib: Calculate memory size for Page State Change Lendacky, Thomas via groups.io
2024-02-27 10:17   ` Gerd Hoffmann
2024-02-22 17:29 ` [edk2-devel] [PATCH v2 07/23] MdePkg: Avoid hardcoded value for number of Page State Change entries Lendacky, Thomas via groups.io
2024-02-27 10:18   ` Gerd Hoffmann
2024-02-27 15:52     ` Lendacky, Thomas via groups.io
2024-02-22 17:29 ` [edk2-devel] [PATCH v2 08/23] OvmfPkg/BaseMemEncryptSevLib: Re-organize page state change support Lendacky, Thomas via groups.io
2024-02-27 11:07   ` Gerd Hoffmann
2024-02-22 17:29 ` [edk2-devel] [PATCH v2 09/23] OvmfPkg/BaseMemEncryptSevLib: Maximize Page State Change efficiency Lendacky, Thomas via groups.io
2024-02-27 11:19   ` Gerd Hoffmann
2024-02-22 17:29 ` [edk2-devel] [PATCH v2 10/23] MdePkg/Register/Amd: Define the SVSM related information Lendacky, Thomas via groups.io
2024-02-22 17:29 ` [edk2-devel] [PATCH v2 11/23] MdePkg/BaseLib: Add a new VMGEXIT instruction invocation for SVSM Lendacky, Thomas via groups.io
2024-02-27 11:50   ` Gerd Hoffmann
2024-02-22 17:29 ` [edk2-devel] [PATCH v2 12/23] UefiCpuPkg/CcSvsmLib: Create the CcSvsmLib library to support an SVSM Lendacky, Thomas via groups.io
2024-02-27 11:53   ` Gerd Hoffmann
2024-02-22 17:29 ` [edk2-devel] [PATCH v2 13/23] UefiPayloadPkg: Prepare UefiPayloadPkg to use the CcSvsmLib library Lendacky, Thomas via groups.io
2024-02-27 11:54   ` Gerd Hoffmann
2024-02-22 17:29 ` [edk2-devel] [PATCH v2 14/23] Ovmfpkg/CcSvsmLib: Create CcSvsmLib to handle SVSM related services Lendacky, Thomas via groups.io
2024-02-28  8:40   ` Gerd Hoffmann
2024-02-28 15:51     ` Lendacky, Thomas via groups.io
2024-03-01 10:59       ` Gerd Hoffmann
2024-02-22 17:29 ` [edk2-devel] [PATCH v2 15/23] UefiCpuPkg/MpInitLib: Use CcSvsmSnpVmsaRmpAdjust() to set/clear VMSA Lendacky, Thomas via groups.io
2024-02-28  8:42   ` Gerd Hoffmann
2024-02-22 17:29 ` [edk2-devel] [PATCH v2 16/23] OvmfPkg/BaseMemEncryptSevLib: Use CcSvsmSnpPvalidate() to validate pages Lendacky, Thomas via groups.io
2024-02-28  8:43   ` Gerd Hoffmann
2024-02-22 17:29 ` [edk2-devel] [PATCH v2 17/23] OvmfPkg: Create a calling area used to communicate with the SVSM Lendacky, Thomas via groups.io
2024-02-28  8:44   ` Gerd Hoffmann
2024-02-22 17:29 ` [edk2-devel] [PATCH v2 18/23] OvmfPkg/CcSvsmLib: Add support for the SVSM_CORE_PVALIDATE call Lendacky, Thomas via groups.io
2024-02-28  8:50   ` Gerd Hoffmann
2024-02-28 15:58     ` Lendacky, Thomas via groups.io
2024-02-22 17:29 ` [edk2-devel] [PATCH v2 19/23] OvmfPkg/BaseMemEncryptSevLib: Maximize Page State Change efficiency Lendacky, Thomas via groups.io
2024-02-28  8:50   ` Gerd Hoffmann
2024-02-22 17:29 ` [edk2-devel] [PATCH v2 20/23] OvmfPkg/CcSvsmLib: Add support for the SVSM create/delete vCPU calls Lendacky, Thomas via groups.io
2024-02-28  8:52   ` Gerd Hoffmann
2024-02-22 17:30 ` [edk2-devel] [PATCH v2 21/23] UefiCpuPkg/MpInitLib: AP creation support under an SVSM Lendacky, Thomas via groups.io
2024-02-22 17:30 ` [edk2-devel] [PATCH v2 22/23] Ovmfpkg/CcExitLib: Provide SVSM discovery support Lendacky, Thomas via groups.io
2024-02-28  8:54   ` Gerd Hoffmann
2024-02-22 17:30 ` [edk2-devel] [PATCH v2 23/23] OvmfPkg/BaseMemEncryptLib: Check for presence of an SVSM when not at VMPL0 Lendacky, Thomas via groups.io
2024-02-28  6:14 ` [edk2-devel] [PATCH v2 00/23] Provide SEV-SNP support for running under an SVSM Yao, Jiewen
2024-02-28 16:19   ` Lendacky, Thomas via groups.io [this message]
2024-02-29 14:06     ` Yao, Jiewen
2024-02-29 14:36       ` Lendacky, Thomas via groups.io

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=30de7630-870b-41d4-9da3-5486c8fc44fe@amd.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox