[PATCH v6 00/29] Add AMD Secure Nested Paging (SEV-SNP) support


Brijesh Singh
 

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3275

SEV-SNP builds upon existing SEV and SEV-ES functionality while adding
new hardware-based memory protections. SEV-SNP adds strong memory integrity
protection to help prevent malicious hypervisor-based attacks like data
replay, memory re-mapping and more in order to create an isolated memory
encryption environment.

This series provides the basic building blocks to support booting the SEV-SNP
VMs, it does not cover all the security enhancement introduced by the SEV-SNP
such as interrupt protection.

Many of the integrity guarantees of SEV-SNP are enforced through a new
structure called the Reverse Map Table (RMP). Adding a new page to SEV-SNP
VM requires a 2-step process. First, the hypervisor assigns a page to the
guest using the new RMPUPDATE instruction. This transitions the page to
guest-invalid. Second, the guest validates the page using the new PVALIDATE
instruction. The SEV-SNP VMs can use the new "Page State Change Request NAE"
defined in the GHCB specification to ask hypervisor to add or remove page
from the RMP table.

Each page assigned to the SEV-SNP VM can either be validated or unvalidated,
as indicated by the Validated flag in the page's RMP entry. There are two
approaches that can be taken for the page validation: Pre-validation and
Lazy Validation.

Under pre-validation, the pages are validated prior to first use. And under
lazy validation, pages are validated when first accessed. An access to a
unvalidated page results in a #VC exception, at which time the exception
handler may validate the page. Lazy validation requires careful tracking of
the validated pages to avoid validating the same GPA more than once. The
recently introduced "Unaccepted" memory type can be used to communicate the
unvalidated memory ranges to the Guest OS.

At this time we only support the pre-validation. OVMF detects all the available
system RAM in the PEI phase. When SEV-SNP is enabled, the memory is validated
before it is made available to the EDK2 core.

Now that series contains all the basic support required to launch SEV-SNP
guest. We are still missing the Interrupt security feature provided by the
SNP. The feature will be added after the base support is accepted.

Additional resources
---------------------
SEV-SNP whitepaper
https://www.amd.com/system/files/TechDocs/SEV-SNP-strengthening-vm-isolation-with-integrity-protection-and-more.pdf

APM 2: https://www.amd.com/system/files/TechDocs/24593.pdf (section 15.36)

The complete source is available at
https://github.com/AMDESE/ovmf/tree/sev-snp-rfc-5

GHCB spec:
https://developer.amd.com/wp-content/resources/56421.pdf

SEV-SNP firmware specification:
https://www.amd.com/system/files/TechDocs/56860.pdf

Change since v5:
* When possible use the CPUID value from CPUID page
* Move the SEV specific functions from SecMain.c in AmdSev.c
* Rebase to the latest code
* Add the review feedback from Yao.

Change since v4:
* Use the correct MSR for the SEV_STATUS
* Add VMPL-0 check

Change since v3:
* ResetVector: move all SEV specific code in AmdSev.asm and add macros to keep
the code readable.
* Drop extending the EsWorkArea to contain SNP specific state.
* Drop the GhcbGpa library and call the VmgExit directly to register GHCB GPA.
* Install the CC blob config table from AmdSevDxe instead of extending the
AmdSev/SecretsDxe for it.
* Add the separate PCDs for the SNP Secrets.

Changes since v2:
* Add support for the AP creation.
* Use the module-scoping override to make AmdSevDxe use the IO port for PCI reads.
* Use the reserved memory type for CPUID and Secrets page.
*
Changes since v1:
* Drop the interval tree support to detect the pre-validated overlap region.
* Use an array to keep track of pre-validated regions.
* Add support to query the Hypervisor feature and verify that SNP feature is supported.
* Introduce MemEncryptSevClearMmioPageEncMask() to clear the C-bit from MMIO ranges.
* Pull the SevSecretDxe and SevSecretPei into OVMF package build.
* Extend the SevSecretDxe to expose confidential computing blob location through
EFI configuration table.

Brijesh Singh (25):
OvmfPkg: reserve SNP secrets page
OvmfPkg: reserve CPUID page for SEV-SNP
OvmfPkg/ResetVector: introduce SEV-SNP boot block GUID
OvmfPkg/ResetVector: invalidate the GHCB page
OvmfPkg/ResetVector: check the vmpl level
OvmfPkg/ResetVector: pre-validate the data pages used in SEC phase
UefiCpuPkg: Define the SEV-SNP specific dynamic PCDs
OvmfPkg/MemEncryptSevLib: add MemEncryptSevSnpEnabled()
OvmfPkg/SecMain: move SEV specific routines in AmdSev.c
OvmfPkg/SecMain: register GHCB gpa for the SEV-SNP guest
OvmfPkg/PlatformPei: register GHCB gpa for the SEV-SNP guest
OvmfPkg/AmdSevDxe: do not use extended PCI config space
OvmfPkg/MemEncryptSevLib: add support to validate system RAM
OvmfPkg/BaseMemEncryptSevLib: skip the pre-validated system RAM
OvmfPkg/MemEncryptSevLib: add support to validate > 4GB memory in PEI
phase
OvmfPkg/SecMain: pre-validate the memory used for decompressing Fv
OvmfPkg/PlatformPei: validate the system RAM when SNP is active
OvmfPkg/PlatformPei: set the SEV-SNP enabled PCD
OvmfPkg/PlatformPei: set the Hypervisor Features PCD
MdePkg/GHCB: increase the GHCB protocol max version
UefiCpuPkg/MpLib: add support to register GHCB GPA when SEV-SNP is
enabled
OvmfPkg/MemEncryptSevLib: change the page state in the RMP table
OvmfPkg/MemEncryptSevLib: skip page state change for Mmio address
OvmfPkg/PlatformPei: mark cpuid and secrets memory reserved in EFI map
OvmfPkg/AmdSev: expose the SNP reserved pages through configuration
table

Michael Roth (3):
OvmfPkg/ResetVector: use SEV-SNP-validated CPUID values
OvmfPkg/VmgExitLib: use SEV-SNP-validated CPUID values
UefiCpuPkg/MpInitLib: use BSP to do extended topology check

Tom Lendacky (1):
UefiCpuPkg/MpInitLib: Use SEV-SNP AP Creation NAE event to launch APs

OvmfPkg/OvmfPkg.dec | 23 +
UefiCpuPkg/UefiCpuPkg.dec | 11 +
OvmfPkg/AmdSev/AmdSevX64.dsc | 5 +-
OvmfPkg/Bhyve/BhyveX64.dsc | 5 +-
OvmfPkg/OvmfPkgIa32.dsc | 1 +
OvmfPkg/OvmfPkgIa32X64.dsc | 6 +-
OvmfPkg/OvmfPkgX64.dsc | 5 +-
OvmfPkg/OvmfXen.dsc | 5 +-
OvmfPkg/OvmfPkgX64.fdf | 12 +-
OvmfPkg/AmdSevDxe/AmdSevDxe.inf | 7 +
.../DxeMemEncryptSevLib.inf | 3 +
.../PeiMemEncryptSevLib.inf | 7 +
.../SecMemEncryptSevLib.inf | 3 +
OvmfPkg/Library/VmgExitLib/SecVmgExitLib.inf | 2 +
OvmfPkg/Library/VmgExitLib/VmgExitLib.inf | 3 +
OvmfPkg/PlatformPei/PlatformPei.inf | 10 +
OvmfPkg/ResetVector/ResetVector.inf | 6 +
OvmfPkg/Sec/SecMain.inf | 4 +
UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf | 4 +
UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf | 4 +
MdePkg/Include/Register/Amd/Ghcb.h | 2 +-
.../Guid/ConfidentialComputingSecret.h | 18 +
OvmfPkg/Include/Library/MemEncryptSevLib.h | 26 +
.../X64/SnpPageStateChange.h | 31 ++
.../BaseMemEncryptSevLib/X64/VirtualMemory.h | 19 +
OvmfPkg/Sec/AmdSev.h | 95 ++++
UefiCpuPkg/Library/MpInitLib/MpLib.h | 20 +
OvmfPkg/AmdSevDxe/AmdSevDxe.c | 23 +
.../DxeMemEncryptSevLibInternal.c | 27 ++
.../Ia32/MemEncryptSevLib.c | 17 +
.../PeiMemEncryptSevLibInternal.c | 27 ++
.../SecMemEncryptSevLibInternal.c | 19 +
.../X64/DxeSnpSystemRamValidate.c | 40 ++
.../X64/PeiDxeVirtualMemory.c | 167 ++++++-
.../X64/PeiSnpSystemRamValidate.c | 126 +++++
.../X64/SecSnpSystemRamValidate.c | 36 ++
.../X64/SnpPageStateChangeInternal.c | 295 ++++++++++++
OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c | 444 ++++++++++++++++--
OvmfPkg/PlatformPei/AmdSev.c | 192 ++++++++
OvmfPkg/PlatformPei/MemDetect.c | 21 +
OvmfPkg/Sec/AmdSev.c | 267 +++++++++++
OvmfPkg/Sec/SecMain.c | 160 +------
UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 11 +-
.../MpInitLib/Ia32/SevSnpRmpAdjustInternal.c | 31 ++
UefiCpuPkg/Library/MpInitLib/MpLib.c | 286 ++++++++++-
.../MpInitLib/X64/SevSnpRmpAdjustInternal.c | 44 ++
OvmfPkg/FvmainCompactScratchEnd.fdf.inc | 5 +
OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm | 28 ++
OvmfPkg/ResetVector/Ia32/AmdSev.asm | 307 +++++++++++-
OvmfPkg/ResetVector/ResetVector.nasmb | 6 +
UefiCpuPkg/Library/MpInitLib/MpEqu.inc | 2 +
UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 78 +++
52 files changed, 2771 insertions(+), 225 deletions(-)
create mode 100644 OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChange.h
create mode 100644 OvmfPkg/Sec/AmdSev.h
create mode 100644 OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c
create mode 100644 OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiSnpSystemRamValidate.c
create mode 100644 OvmfPkg/Library/BaseMemEncryptSevLib/X64/SecSnpSystemRamValidate.c
create mode 100644 OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChangeInternal.c
create mode 100644 OvmfPkg/Sec/AmdSev.c
create mode 100644 UefiCpuPkg/Library/MpInitLib/Ia32/SevSnpRmpAdjustInternal.c
create mode 100644 UefiCpuPkg/Library/MpInitLib/X64/SevSnpRmpAdjustInternal.c

--
2.17.1


Yao, Jiewen
 

Thank you Brijesh
It took me a while to review this series. Here is my feedback.
I am not sure what you prefer, to put all comment together? Or reply 29 email separately?
Let me put them together in this version. If you prefer a different way, please let me know.

My strategy is same as previous. I will focus on common part and review as detail as possible.
For SEV specific thing, I will ACK and let AMD people make decision unless I have big concern on the design.
You can add my A-B and R-B in next version.


0001-OvmfPkg-reserve-SNP-secrets-page
Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>

0002-OvmfPkg-reserve-CPUID-page-for-SEV-SNP
Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>

0003-OvmfPkg-ResetVector-introduce-SEV-SNP-boot-block-GUID
I am still thinking if it is possible to move all SEV define GUID blob to a standalone file, and TDX define GUID blob to another file.
Anyway, that can be done later.
Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>

0004-OvmfPkg-ResetVector-invalidate-the-GHCB-page
Acked-by: Jiewen Yao <Jiewen.yao@intel.com>

0005-OvmfPkg-ResetVector-check-the-vmpl-level
Acked-by: Jiewen Yao <Jiewen.yao@intel.com>

0006-OvmfPkg-ResetVector-pre-validate-the-data-pages-used-in-SEC-phase
Acked-by: Jiewen Yao <Jiewen.yao@intel.com>

0007-OvmfPkg-ResetVector-use-SEV-SNP-validated-CPUID-values
Acked-by: Jiewen Yao <Jiewen.yao@intel.com>

0008-UefiCpuPkg-Define-the-SEV-SNP-specific-dynamic-PCDs
I really don't like the idea to use BOOL PcdSevEsIsEnabled and PcdSevSnpIsEnabled.
Can we define *one* PCD - such as PcdConfidentialComputingCategory?
We can assign range 0x0000~0xFFFF to AMD SEV, 0x10000~0x1FFFF to Intel TDX.
Then SEV=0x0000, SEV-ES=0x0001, SEV-SNP=0x0002, and TDX=0x10000 later.
I really don't want to keep adding PCD endlessly in the future, like PcdSevXXXIsEnabled, PcdSevYYYIsEnabled, PcdTdxIsEnabled, PcdTdx20Enabled, PcdTdx30Enabled, ......


0009-OvmfPkg-MemEncryptSevLib-add-MemEncryptSevSnpEnabled()
I am not sure since we have PCD in 0008, why we need to expose the function - MemEncryptSevSnpIsEnabled() and MemEncryptSevEsIsEnabled()?
Should we always use PCD anywhere else?
Anyway, Acked-by: Jiewen Yao <Jiewen.yao@intel.com>

0010-OvmfPkg-SecMain-move-SEV-specific-routines-in-AmdSev.c
Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>

0011-OvmfPkg-SecMain-register-GHCB-gpa-for-the-SEV-SNP-guest
Acked-by: Jiewen Yao <Jiewen.yao@intel.com>

0012-OvmfPkg-VmgExitLib-use-SEV-SNP-validated-CPUID-values
Acked-by: Jiewen Yao <Jiewen.yao@intel.com>

0013-OvmfPkg-PlatformPei-register-GHCB-gpa-for-the-SEV-SNP-guest
Acked-by: Jiewen Yao <Jiewen.yao@intel.com>

0014-OvmfPkg-AmdSevDxe-do-not-use-extended-PCI-config-space
Acked-by: Jiewen Yao <Jiewen.yao@intel.com>

0015-OvmfPkg-MemEncryptSevLib-add-support-to-validate-system-RAM
Acked-by: Jiewen Yao <Jiewen.yao@intel.com>

0016-OvmfPkg-BaseMemEncryptSevLib-skip-the-pre-validated-system-RAM
Acked-by: Jiewen Yao <Jiewen.yao@intel.com>

0017-OvmfPkg-MemEncryptSevLib-add-support-to-validate-4GB-memory-in-PEI-phase
Acked-by: Jiewen Yao <Jiewen.yao@intel.com>

0018-OvmfPkg-SecMain-pre-validate-the-memory-used-for-decompressing-Fv
Acked-by: Jiewen Yao <Jiewen.yao@intel.com>

0019-OvmfPkg-PlatformPei-validate-the-system-RAM-when-SNP-is-active
Acked-by: Jiewen Yao <Jiewen.yao@intel.com>

0020-OvmfPkg-PlatformPei-set-the-SEV-SNP-enabled-PCD
See 0008

0021-OvmfPkg-PlatformPei-set-the-Hypervisor-Features-PCD
Acked-by: Jiewen Yao <Jiewen.yao@intel.com>

0022-MdePkg-GHCB-increase-the-GHCB-protocol-max-version
Acked-by: Jiewen Yao <Jiewen.yao@intel.com>

0023-UefiCpuPkg-MpLib-add-support-to-register-GHCB-GPA-when-SEV-SNP-is-enabled
1) See 0008.
2) For MpFuncs.nasm, I recommend to move AmdSev specific initialization to a standalone file, such as Sev.nasm

0024-UefiCpuPkg-MpInitLib-use-BSP-to-do-extended-topology-check
See 0023

0025-OvmfPkg-MemEncryptSevLib-change-the-page-state-in-the-RMP-table
Acked-by: Jiewen Yao <Jiewen.yao@intel.com>

0026-OvmfPkg-MemEncryptSevLib-skip-page-state-change-for-Mmio-address
Acked-by: Jiewen Yao <Jiewen.yao@intel.com>

0027-OvmfPkg-PlatformPei-mark-cpuid-and-secrets-memory-reserved-in-EFI-map
Would you please move SEV specific init to another Sev.c?
Also I found MemEncryptSevEsIsEnabled() and MemEncryptSevSnpIsEnabled() are there.
I suggest just use one API
MemEncryptSevEsIsEnabled() {
DoSevInitializeRamRegions()
}
Then you can check more in DoSevInitializeRamRegions().
DoSevInitializeRamRegions() {
MemEncryptSevSnpIsEnabled() {
}
}

0028-OvmfPkg-AmdSev-expose-the-SNP-reserved-pages-through-configuration-table
I am not convinced to include SEV specific data structure in a generic structure in ConfidentialComputingSecret.h.
I recommend moving it to SEV specific file.

0029-UefiCpuPkg-MpInitLib-Use-SEV-SNP-AP-Creation-NAE-event-to-launch-APs
See 0008, 0023.
I recommend to move SevSnpCreateSaveArea() to Sev.c.

Thank you
Yao Jiewen

-----Original Message-----
From: Brijesh Singh <brijesh.singh@amd.com>
Sent: Thursday, September 2, 2021 12:16 AM
To: devel@edk2.groups.io
Cc: James Bottomley <jejb@linux.ibm.com>; Xu, Min M <min.m.xu@intel.com>;
Yao, Jiewen <jiewen.yao@intel.com>; Tom Lendacky
<thomas.lendacky@amd.com>; Justen, Jordan L <jordan.l.justen@intel.com>;
Ard Biesheuvel <ardb+tianocore@kernel.org>; Erdem Aktas
<erdemaktas@google.com>; Michael Roth <Michael.Roth@amd.com>; Gerd
Hoffmann <kraxel@redhat.com>; Brijesh Singh <brijesh.singh@amd.com>
Subject: [PATCH v6 00/29] Add AMD Secure Nested Paging (SEV-SNP) support

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3275

SEV-SNP builds upon existing SEV and SEV-ES functionality while adding
new hardware-based memory protections. SEV-SNP adds strong memory
integrity
protection to help prevent malicious hypervisor-based attacks like data
replay, memory re-mapping and more in order to create an isolated memory
encryption environment.

This series provides the basic building blocks to support booting the SEV-SNP
VMs, it does not cover all the security enhancement introduced by the SEV-SNP
such as interrupt protection.

Many of the integrity guarantees of SEV-SNP are enforced through a new
structure called the Reverse Map Table (RMP). Adding a new page to SEV-SNP
VM requires a 2-step process. First, the hypervisor assigns a page to the
guest using the new RMPUPDATE instruction. This transitions the page to
guest-invalid. Second, the guest validates the page using the new PVALIDATE
instruction. The SEV-SNP VMs can use the new "Page State Change Request
NAE"
defined in the GHCB specification to ask hypervisor to add or remove page
from the RMP table.

Each page assigned to the SEV-SNP VM can either be validated or unvalidated,
as indicated by the Validated flag in the page's RMP entry. There are two
approaches that can be taken for the page validation: Pre-validation and
Lazy Validation.

Under pre-validation, the pages are validated prior to first use. And under
lazy validation, pages are validated when first accessed. An access to a
unvalidated page results in a #VC exception, at which time the exception
handler may validate the page. Lazy validation requires careful tracking of
the validated pages to avoid validating the same GPA more than once. The
recently introduced "Unaccepted" memory type can be used to communicate
the
unvalidated memory ranges to the Guest OS.

At this time we only support the pre-validation. OVMF detects all the available
system RAM in the PEI phase. When SEV-SNP is enabled, the memory is validated
before it is made available to the EDK2 core.

Now that series contains all the basic support required to launch SEV-SNP
guest. We are still missing the Interrupt security feature provided by the
SNP. The feature will be added after the base support is accepted.

Additional resources
---------------------
SEV-SNP whitepaper
https://www.amd.com/system/files/TechDocs/SEV-SNP-strengthening-vm-
isolation-with-integrity-protection-and-more.pdf

APM 2: https://www.amd.com/system/files/TechDocs/24593.pdf (section 15.36)

The complete source is available at
https://github.com/AMDESE/ovmf/tree/sev-snp-rfc-5

GHCB spec:
https://developer.amd.com/wp-content/resources/56421.pdf

SEV-SNP firmware specification:
https://www.amd.com/system/files/TechDocs/56860.pdf

Change since v5:
* When possible use the CPUID value from CPUID page
* Move the SEV specific functions from SecMain.c in AmdSev.c
* Rebase to the latest code
* Add the review feedback from Yao.

Change since v4:
* Use the correct MSR for the SEV_STATUS
* Add VMPL-0 check

Change since v3:
* ResetVector: move all SEV specific code in AmdSev.asm and add macros to
keep
the code readable.
* Drop extending the EsWorkArea to contain SNP specific state.
* Drop the GhcbGpa library and call the VmgExit directly to register GHCB GPA.
* Install the CC blob config table from AmdSevDxe instead of extending the
AmdSev/SecretsDxe for it.
* Add the separate PCDs for the SNP Secrets.

Changes since v2:
* Add support for the AP creation.
* Use the module-scoping override to make AmdSevDxe use the IO port for PCI
reads.
* Use the reserved memory type for CPUID and Secrets page.
*
Changes since v1:
* Drop the interval tree support to detect the pre-validated overlap region.
* Use an array to keep track of pre-validated regions.
* Add support to query the Hypervisor feature and verify that SNP feature is
supported.
* Introduce MemEncryptSevClearMmioPageEncMask() to clear the C-bit from
MMIO ranges.
* Pull the SevSecretDxe and SevSecretPei into OVMF package build.
* Extend the SevSecretDxe to expose confidential computing blob location
through
EFI configuration table.

Brijesh Singh (25):
OvmfPkg: reserve SNP secrets page
OvmfPkg: reserve CPUID page for SEV-SNP
OvmfPkg/ResetVector: introduce SEV-SNP boot block GUID
OvmfPkg/ResetVector: invalidate the GHCB page
OvmfPkg/ResetVector: check the vmpl level
OvmfPkg/ResetVector: pre-validate the data pages used in SEC phase
UefiCpuPkg: Define the SEV-SNP specific dynamic PCDs
OvmfPkg/MemEncryptSevLib: add MemEncryptSevSnpEnabled()
OvmfPkg/SecMain: move SEV specific routines in AmdSev.c
OvmfPkg/SecMain: register GHCB gpa for the SEV-SNP guest
OvmfPkg/PlatformPei: register GHCB gpa for the SEV-SNP guest
OvmfPkg/AmdSevDxe: do not use extended PCI config space
OvmfPkg/MemEncryptSevLib: add support to validate system RAM
OvmfPkg/BaseMemEncryptSevLib: skip the pre-validated system RAM
OvmfPkg/MemEncryptSevLib: add support to validate > 4GB memory in PEI
phase
OvmfPkg/SecMain: pre-validate the memory used for decompressing Fv
OvmfPkg/PlatformPei: validate the system RAM when SNP is active
OvmfPkg/PlatformPei: set the SEV-SNP enabled PCD
OvmfPkg/PlatformPei: set the Hypervisor Features PCD
MdePkg/GHCB: increase the GHCB protocol max version
UefiCpuPkg/MpLib: add support to register GHCB GPA when SEV-SNP is
enabled
OvmfPkg/MemEncryptSevLib: change the page state in the RMP table
OvmfPkg/MemEncryptSevLib: skip page state change for Mmio address
OvmfPkg/PlatformPei: mark cpuid and secrets memory reserved in EFI map
OvmfPkg/AmdSev: expose the SNP reserved pages through configuration
table

Michael Roth (3):
OvmfPkg/ResetVector: use SEV-SNP-validated CPUID values
OvmfPkg/VmgExitLib: use SEV-SNP-validated CPUID values
UefiCpuPkg/MpInitLib: use BSP to do extended topology check

Tom Lendacky (1):
UefiCpuPkg/MpInitLib: Use SEV-SNP AP Creation NAE event to launch APs

OvmfPkg/OvmfPkg.dec | 23 +
UefiCpuPkg/UefiCpuPkg.dec | 11 +
OvmfPkg/AmdSev/AmdSevX64.dsc | 5 +-
OvmfPkg/Bhyve/BhyveX64.dsc | 5 +-
OvmfPkg/OvmfPkgIa32.dsc | 1 +
OvmfPkg/OvmfPkgIa32X64.dsc | 6 +-
OvmfPkg/OvmfPkgX64.dsc | 5 +-
OvmfPkg/OvmfXen.dsc | 5 +-
OvmfPkg/OvmfPkgX64.fdf | 12 +-
OvmfPkg/AmdSevDxe/AmdSevDxe.inf | 7 +
.../DxeMemEncryptSevLib.inf | 3 +
.../PeiMemEncryptSevLib.inf | 7 +
.../SecMemEncryptSevLib.inf | 3 +
OvmfPkg/Library/VmgExitLib/SecVmgExitLib.inf | 2 +
OvmfPkg/Library/VmgExitLib/VmgExitLib.inf | 3 +
OvmfPkg/PlatformPei/PlatformPei.inf | 10 +
OvmfPkg/ResetVector/ResetVector.inf | 6 +
OvmfPkg/Sec/SecMain.inf | 4 +
UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf | 4 +
UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf | 4 +
MdePkg/Include/Register/Amd/Ghcb.h | 2 +-
.../Guid/ConfidentialComputingSecret.h | 18 +
OvmfPkg/Include/Library/MemEncryptSevLib.h | 26 +
.../X64/SnpPageStateChange.h | 31 ++
.../BaseMemEncryptSevLib/X64/VirtualMemory.h | 19 +
OvmfPkg/Sec/AmdSev.h | 95 ++++
UefiCpuPkg/Library/MpInitLib/MpLib.h | 20 +
OvmfPkg/AmdSevDxe/AmdSevDxe.c | 23 +
.../DxeMemEncryptSevLibInternal.c | 27 ++
.../Ia32/MemEncryptSevLib.c | 17 +
.../PeiMemEncryptSevLibInternal.c | 27 ++
.../SecMemEncryptSevLibInternal.c | 19 +
.../X64/DxeSnpSystemRamValidate.c | 40 ++
.../X64/PeiDxeVirtualMemory.c | 167 ++++++-
.../X64/PeiSnpSystemRamValidate.c | 126 +++++
.../X64/SecSnpSystemRamValidate.c | 36 ++
.../X64/SnpPageStateChangeInternal.c | 295 ++++++++++++
OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c | 444 ++++++++++++++++--
OvmfPkg/PlatformPei/AmdSev.c | 192 ++++++++
OvmfPkg/PlatformPei/MemDetect.c | 21 +
OvmfPkg/Sec/AmdSev.c | 267 +++++++++++
OvmfPkg/Sec/SecMain.c | 160 +------
UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 11 +-
.../MpInitLib/Ia32/SevSnpRmpAdjustInternal.c | 31 ++
UefiCpuPkg/Library/MpInitLib/MpLib.c | 286 ++++++++++-
.../MpInitLib/X64/SevSnpRmpAdjustInternal.c | 44 ++
OvmfPkg/FvmainCompactScratchEnd.fdf.inc | 5 +
OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm | 28 ++
OvmfPkg/ResetVector/Ia32/AmdSev.asm | 307 +++++++++++-
OvmfPkg/ResetVector/ResetVector.nasmb | 6 +
UefiCpuPkg/Library/MpInitLib/MpEqu.inc | 2 +
UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 78 +++
52 files changed, 2771 insertions(+), 225 deletions(-)
create mode 100644
OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChange.h
create mode 100644 OvmfPkg/Sec/AmdSev.h
create mode 100644
OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c
create mode 100644
OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiSnpSystemRamValidate.c
create mode 100644
OvmfPkg/Library/BaseMemEncryptSevLib/X64/SecSnpSystemRamValidate.c
create mode 100644
OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChangeInternal.c
create mode 100644 OvmfPkg/Sec/AmdSev.c
create mode 100644
UefiCpuPkg/Library/MpInitLib/Ia32/SevSnpRmpAdjustInternal.c
create mode 100644
UefiCpuPkg/Library/MpInitLib/X64/SevSnpRmpAdjustInternal.c

--
2.17.1


Min Xu
 

On September 7, 2021 10:37 AM, Jiewen Yao wrote:

0008-UefiCpuPkg-Define-the-SEV-SNP-specific-dynamic-PCDs
I really don't like the idea to use BOOL PcdSevEsIsEnabled and
PcdSevSnpIsEnabled.
Can we define *one* PCD - such as PcdConfidentialComputingCategory?
We can assign range 0x0000~0xFFFF to AMD SEV, 0x10000~0x1FFFF to Intel TDX.
Then SEV=0x0000, SEV-ES=0x0001, SEV-SNP=0x0002, and TDX=0x10000 later.
I really don't want to keep adding PCD endlessly in the future, like
PcdSevXXXIsEnabled, PcdSevYYYIsEnabled, PcdTdxIsEnabled, PcdTdx20Enabled,
PcdTdx30Enabled, ......
We have CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER definition in OvmfPkg\Include\WorkArea.h like below:
typedef struct _CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER {
UINT8 GuestType; // 0 - legacy guest, 1 - SEV guest, 2 - tdx guest
UINT8 Reserved1[3];
} CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER;

Can we define the PcdConfidentialComputingCategory like below:
## This dynamic PCD indicates the Confidential Computing Category
# [7:0] Confidential Computing Category (0 - Non-Cc, 1 - AmdSev, 2 - IntelTdx)
# [15:8] Sub-Category (defined by each vendor, SEV-ES, SEV-SNP, or TDX-1.0, TDX-2.0, etc)
# [31:16] Reserved
# @Prompt Confidential Computing Category
gUefiCpuPkgTokenSpaceGuid.PcdConfidentialComputingCategory|0|UINT32|0x60000018

Thanks!
Min


Yao, Jiewen
 

Yes, that is good idea.

-----Original Message-----
From: Xu, Min M <min.m.xu@intel.com>
Sent: Wednesday, September 8, 2021 10:30 AM
To: Yao, Jiewen <jiewen.yao@intel.com>; Brijesh Singh
<brijesh.singh@amd.com>; devel@edk2.groups.io
Cc: James Bottomley <jejb@linux.ibm.com>; Tom Lendacky
<thomas.lendacky@amd.com>; Justen, Jordan L <jordan.l.justen@intel.com>;
Ard Biesheuvel <ardb+tianocore@kernel.org>; Erdem Aktas
<erdemaktas@google.com>; Michael Roth <Michael.Roth@amd.com>; Gerd
Hoffmann <kraxel@redhat.com>
Subject: RE: [PATCH v6 00/29] Add AMD Secure Nested Paging (SEV-SNP)
support

On September 7, 2021 10:37 AM, Jiewen Yao wrote:

0008-UefiCpuPkg-Define-the-SEV-SNP-specific-dynamic-PCDs
I really don't like the idea to use BOOL PcdSevEsIsEnabled and
PcdSevSnpIsEnabled.
Can we define *one* PCD - such as PcdConfidentialComputingCategory?
We can assign range 0x0000~0xFFFF to AMD SEV, 0x10000~0x1FFFF to Intel
TDX.
Then SEV=0x0000, SEV-ES=0x0001, SEV-SNP=0x0002, and TDX=0x10000 later.
I really don't want to keep adding PCD endlessly in the future, like
PcdSevXXXIsEnabled, PcdSevYYYIsEnabled, PcdTdxIsEnabled, PcdTdx20Enabled,
PcdTdx30Enabled, ......
We have CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER definition in
OvmfPkg\Include\WorkArea.h like below:
typedef struct _CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER {
UINT8 GuestType; // 0 - legacy guest, 1 - SEV guest, 2 - tdx guest
UINT8 Reserved1[3];
} CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER;

Can we define the PcdConfidentialComputingCategory like below:
## This dynamic PCD indicates the Confidential Computing Category
# [7:0] Confidential Computing Category (0 - Non-Cc, 1 - AmdSev, 2 -
IntelTdx)
# [15:8] Sub-Category (defined by each vendor, SEV-ES, SEV-SNP, or TDX-1.0,
TDX-2.0, etc)
# [31:16] Reserved
# @Prompt Confidential Computing Category

gUefiCpuPkgTokenSpaceGuid.PcdConfidentialComputingCategory|0|UINT32|0x
60000018

Thanks!
Min


Brijesh Singh
 

Thank you so much Yao for reviewing the patches. Based on some comments from Gerd I may update code around the reset vector area (mainly use the metadata format etc). For your comments regarding the introducing a new PcdConfidentialComputingCategory I will look to see what I can come up with and in UefiCpuPkg I will try to move all the SEV specific functions in new files (where applicable).

thanks
Brijesh

On 9/6/21 9:36 PM, Yao, Jiewen wrote:
Thank you Brijesh
It took me a while to review this series. Here is my feedback.
I am not sure what you prefer, to put all comment together? Or reply 29 email separately?
Let me put them together in this version. If you prefer a different way, please let me know.
My strategy is same as previous. I will focus on common part and review as detail as possible.
For SEV specific thing, I will ACK and let AMD people make decision unless I have big concern on the design.
You can add my A-B and R-B in next version.
0001-OvmfPkg-reserve-SNP-secrets-page
Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>
0002-OvmfPkg-reserve-CPUID-page-for-SEV-SNP
Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>
0003-OvmfPkg-ResetVector-introduce-SEV-SNP-boot-block-GUID
I am still thinking if it is possible to move all SEV define GUID blob to a standalone file, and TDX define GUID blob to another file.
Anyway, that can be done later.
Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>
0004-OvmfPkg-ResetVector-invalidate-the-GHCB-page
Acked-by: Jiewen Yao <Jiewen.yao@intel.com>
0005-OvmfPkg-ResetVector-check-the-vmpl-level
Acked-by: Jiewen Yao <Jiewen.yao@intel.com>
0006-OvmfPkg-ResetVector-pre-validate-the-data-pages-used-in-SEC-phase
Acked-by: Jiewen Yao <Jiewen.yao@intel.com>
0007-OvmfPkg-ResetVector-use-SEV-SNP-validated-CPUID-values
Acked-by: Jiewen Yao <Jiewen.yao@intel.com>
0008-UefiCpuPkg-Define-the-SEV-SNP-specific-dynamic-PCDs
I really don't like the idea to use BOOL PcdSevEsIsEnabled and PcdSevSnpIsEnabled.
Can we define *one* PCD - such as PcdConfidentialComputingCategory?
We can assign range 0x0000~0xFFFF to AMD SEV, 0x10000~0x1FFFF to Intel TDX.
Then SEV=0x0000, SEV-ES=0x0001, SEV-SNP=0x0002, and TDX=0x10000 later.
I really don't want to keep adding PCD endlessly in the future, like PcdSevXXXIsEnabled, PcdSevYYYIsEnabled, PcdTdxIsEnabled, PcdTdx20Enabled, PcdTdx30Enabled, ......
0009-OvmfPkg-MemEncryptSevLib-add-MemEncryptSevSnpEnabled()
I am not sure since we have PCD in 0008, why we need to expose the function - MemEncryptSevSnpIsEnabled() and MemEncryptSevEsIsEnabled()?
Should we always use PCD anywhere else?
Anyway, Acked-by: Jiewen Yao <Jiewen.yao@intel.com>
0010-OvmfPkg-SecMain-move-SEV-specific-routines-in-AmdSev.c
Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>
0011-OvmfPkg-SecMain-register-GHCB-gpa-for-the-SEV-SNP-guest
Acked-by: Jiewen Yao <Jiewen.yao@intel.com>
0012-OvmfPkg-VmgExitLib-use-SEV-SNP-validated-CPUID-values
Acked-by: Jiewen Yao <Jiewen.yao@intel.com>
0013-OvmfPkg-PlatformPei-register-GHCB-gpa-for-the-SEV-SNP-guest
Acked-by: Jiewen Yao <Jiewen.yao@intel.com>
0014-OvmfPkg-AmdSevDxe-do-not-use-extended-PCI-config-space
Acked-by: Jiewen Yao <Jiewen.yao@intel.com>
0015-OvmfPkg-MemEncryptSevLib-add-support-to-validate-system-RAM
Acked-by: Jiewen Yao <Jiewen.yao@intel.com>
0016-OvmfPkg-BaseMemEncryptSevLib-skip-the-pre-validated-system-RAM
Acked-by: Jiewen Yao <Jiewen.yao@intel.com>
0017-OvmfPkg-MemEncryptSevLib-add-support-to-validate-4GB-memory-in-PEI-phase
Acked-by: Jiewen Yao <Jiewen.yao@intel.com>
0018-OvmfPkg-SecMain-pre-validate-the-memory-used-for-decompressing-Fv
Acked-by: Jiewen Yao <Jiewen.yao@intel.com>
0019-OvmfPkg-PlatformPei-validate-the-system-RAM-when-SNP-is-active
Acked-by: Jiewen Yao <Jiewen.yao@intel.com>
0020-OvmfPkg-PlatformPei-set-the-SEV-SNP-enabled-PCD
See 0008
0021-OvmfPkg-PlatformPei-set-the-Hypervisor-Features-PCD
Acked-by: Jiewen Yao <Jiewen.yao@intel.com>
0022-MdePkg-GHCB-increase-the-GHCB-protocol-max-version
Acked-by: Jiewen Yao <Jiewen.yao@intel.com>
0023-UefiCpuPkg-MpLib-add-support-to-register-GHCB-GPA-when-SEV-SNP-is-enabled
1) See 0008.
2) For MpFuncs.nasm, I recommend to move AmdSev specific initialization to a standalone file, such as Sev.nasm
0024-UefiCpuPkg-MpInitLib-use-BSP-to-do-extended-topology-check
See 0023
0025-OvmfPkg-MemEncryptSevLib-change-the-page-state-in-the-RMP-table
Acked-by: Jiewen Yao <Jiewen.yao@intel.com>
0026-OvmfPkg-MemEncryptSevLib-skip-page-state-change-for-Mmio-address
Acked-by: Jiewen Yao <Jiewen.yao@intel.com>
0027-OvmfPkg-PlatformPei-mark-cpuid-and-secrets-memory-reserved-in-EFI-map
Would you please move SEV specific init to another Sev.c?
Also I found MemEncryptSevEsIsEnabled() and MemEncryptSevSnpIsEnabled() are there.
I suggest just use one API
MemEncryptSevEsIsEnabled() {
DoSevInitializeRamRegions()
}
Then you can check more in DoSevInitializeRamRegions().
DoSevInitializeRamRegions() {
MemEncryptSevSnpIsEnabled() {
}
}
0028-OvmfPkg-AmdSev-expose-the-SNP-reserved-pages-through-configuration-table
I am not convinced to include SEV specific data structure in a generic structure in ConfidentialComputingSecret.h.
I recommend moving it to SEV specific file.
0029-UefiCpuPkg-MpInitLib-Use-SEV-SNP-AP-Creation-NAE-event-to-launch-APs
See 0008, 0023.
I recommend to move SevSnpCreateSaveArea() to Sev.c.
Thank you
Yao Jiewen

-----Original Message-----
From: Brijesh Singh <brijesh.singh@amd.com>
Sent: Thursday, September 2, 2021 12:16 AM
To: devel@edk2.groups.io
Cc: James Bottomley <jejb@linux.ibm.com>; Xu, Min M <min.m.xu@intel.com>;
Yao, Jiewen <jiewen.yao@intel.com>; Tom Lendacky
<thomas.lendacky@amd.com>; Justen, Jordan L <jordan.l.justen@intel.com>;
Ard Biesheuvel <ardb+tianocore@kernel.org>; Erdem Aktas
<erdemaktas@google.com>; Michael Roth <Michael.Roth@amd.com>; Gerd
Hoffmann <kraxel@redhat.com>; Brijesh Singh <brijesh.singh@amd.com>
Subject: [PATCH v6 00/29] Add AMD Secure Nested Paging (SEV-SNP) support

BZ: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D3275&;data=04%7C01%7Cbrijesh.singh%40amd.com%7C33df27781053475362e208d971a85cee%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637665791405981353%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=8vfBxVawRoEeDCR0DHJhfhTgPr66704twMGZ8%2BY%2BLGI%3D&amp;reserved=0

SEV-SNP builds upon existing SEV and SEV-ES functionality while adding
new hardware-based memory protections. SEV-SNP adds strong memory
integrity
protection to help prevent malicious hypervisor-based attacks like data
replay, memory re-mapping and more in order to create an isolated memory
encryption environment.

This series provides the basic building blocks to support booting the SEV-SNP
VMs, it does not cover all the security enhancement introduced by the SEV-SNP
such as interrupt protection.

Many of the integrity guarantees of SEV-SNP are enforced through a new
structure called the Reverse Map Table (RMP). Adding a new page to SEV-SNP
VM requires a 2-step process. First, the hypervisor assigns a page to the
guest using the new RMPUPDATE instruction. This transitions the page to
guest-invalid. Second, the guest validates the page using the new PVALIDATE
instruction. The SEV-SNP VMs can use the new "Page State Change Request
NAE"
defined in the GHCB specification to ask hypervisor to add or remove page
from the RMP table.

Each page assigned to the SEV-SNP VM can either be validated or unvalidated,
as indicated by the Validated flag in the page's RMP entry. There are two
approaches that can be taken for the page validation: Pre-validation and
Lazy Validation.

Under pre-validation, the pages are validated prior to first use. And under
lazy validation, pages are validated when first accessed. An access to a
unvalidated page results in a #VC exception, at which time the exception
handler may validate the page. Lazy validation requires careful tracking of
the validated pages to avoid validating the same GPA more than once. The
recently introduced "Unaccepted" memory type can be used to communicate
the
unvalidated memory ranges to the Guest OS.

At this time we only support the pre-validation. OVMF detects all the available
system RAM in the PEI phase. When SEV-SNP is enabled, the memory is validated
before it is made available to the EDK2 core.

Now that series contains all the basic support required to launch SEV-SNP
guest. We are still missing the Interrupt security feature provided by the
SNP. The feature will be added after the base support is accepted.

Additional resources
---------------------
SEV-SNP whitepaper
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.amd.com%2Fsystem%2Ffiles%2FTechDocs%2FSEV-SNP-strengthening-vm-&;data=04%7C01%7Cbrijesh.singh%40amd.com%7C33df27781053475362e208d971a85cee%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637665791405981353%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=tobk2zHk1ziA6nZ9bvwNrohRuIN7bTEh5ZXFNzwTTX0%3D&amp;reserved=0
isolation-with-integrity-protection-and-more.pdf

APM 2: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.amd.com%2Fsystem%2Ffiles%2FTechDocs%2F24593.pdf&;data=04%7C01%7Cbrijesh.singh%40amd.com%7C33df27781053475362e208d971a85cee%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637665791405981353%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=%2FDTzbh8F6CtvvC263r7xJGX6WAQ8yCAuKLkPM7GwBvQ%3D&amp;reserved=0 (section 15.36)

The complete source is available at
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FAMDESE%2Fovmf%2Ftree%2Fsev-snp-rfc-5&;data=04%7C01%7Cbrijesh.singh%40amd.com%7C33df27781053475362e208d971a85cee%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637665791405981353%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=6lvmuqOQbvNoXG50qK5QGYG6XEdojJ%2BHlkKrODZRAHY%3D&amp;reserved=0

GHCB spec:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdeveloper.amd.com%2Fwp-content%2Fresources%2F56421.pdf&;data=04%7C01%7Cbrijesh.singh%40amd.com%7C33df27781053475362e208d971a85cee%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637665791405981353%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=Q1oa5gB3CthKPkentzFJE3B3LfBpZq%2B4y8EzPTlPzl8%3D&amp;reserved=0

SEV-SNP firmware specification:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.amd.com%2Fsystem%2Ffiles%2FTechDocs%2F56860.pdf&;data=04%7C01%7Cbrijesh.singh%40amd.com%7C33df27781053475362e208d971a85cee%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637665791405981353%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=7cLoMR52WAvMe%2Fr4rKGGYx2wadopvXKnSGi%2FghyEdJA%3D&amp;reserved=0

Change since v5:
* When possible use the CPUID value from CPUID page
* Move the SEV specific functions from SecMain.c in AmdSev.c
* Rebase to the latest code
* Add the review feedback from Yao.

Change since v4:
* Use the correct MSR for the SEV_STATUS
* Add VMPL-0 check

Change since v3:
* ResetVector: move all SEV specific code in AmdSev.asm and add macros to
keep
the code readable.
* Drop extending the EsWorkArea to contain SNP specific state.
* Drop the GhcbGpa library and call the VmgExit directly to register GHCB GPA.
* Install the CC blob config table from AmdSevDxe instead of extending the
AmdSev/SecretsDxe for it.
* Add the separate PCDs for the SNP Secrets.

Changes since v2:
* Add support for the AP creation.
* Use the module-scoping override to make AmdSevDxe use the IO port for PCI
reads.
* Use the reserved memory type for CPUID and Secrets page.
*
Changes since v1:
* Drop the interval tree support to detect the pre-validated overlap region.
* Use an array to keep track of pre-validated regions.
* Add support to query the Hypervisor feature and verify that SNP feature is
supported.
* Introduce MemEncryptSevClearMmioPageEncMask() to clear the C-bit from
MMIO ranges.
* Pull the SevSecretDxe and SevSecretPei into OVMF package build.
* Extend the SevSecretDxe to expose confidential computing blob location
through
EFI configuration table.

Brijesh Singh (25):
OvmfPkg: reserve SNP secrets page
OvmfPkg: reserve CPUID page for SEV-SNP
OvmfPkg/ResetVector: introduce SEV-SNP boot block GUID
OvmfPkg/ResetVector: invalidate the GHCB page
OvmfPkg/ResetVector: check the vmpl level
OvmfPkg/ResetVector: pre-validate the data pages used in SEC phase
UefiCpuPkg: Define the SEV-SNP specific dynamic PCDs
OvmfPkg/MemEncryptSevLib: add MemEncryptSevSnpEnabled()
OvmfPkg/SecMain: move SEV specific routines in AmdSev.c
OvmfPkg/SecMain: register GHCB gpa for the SEV-SNP guest
OvmfPkg/PlatformPei: register GHCB gpa for the SEV-SNP guest
OvmfPkg/AmdSevDxe: do not use extended PCI config space
OvmfPkg/MemEncryptSevLib: add support to validate system RAM
OvmfPkg/BaseMemEncryptSevLib: skip the pre-validated system RAM
OvmfPkg/MemEncryptSevLib: add support to validate > 4GB memory in PEI
phase
OvmfPkg/SecMain: pre-validate the memory used for decompressing Fv
OvmfPkg/PlatformPei: validate the system RAM when SNP is active
OvmfPkg/PlatformPei: set the SEV-SNP enabled PCD
OvmfPkg/PlatformPei: set the Hypervisor Features PCD
MdePkg/GHCB: increase the GHCB protocol max version
UefiCpuPkg/MpLib: add support to register GHCB GPA when SEV-SNP is
enabled
OvmfPkg/MemEncryptSevLib: change the page state in the RMP table
OvmfPkg/MemEncryptSevLib: skip page state change for Mmio address
OvmfPkg/PlatformPei: mark cpuid and secrets memory reserved in EFI map
OvmfPkg/AmdSev: expose the SNP reserved pages through configuration
table

Michael Roth (3):
OvmfPkg/ResetVector: use SEV-SNP-validated CPUID values
OvmfPkg/VmgExitLib: use SEV-SNP-validated CPUID values
UefiCpuPkg/MpInitLib: use BSP to do extended topology check

Tom Lendacky (1):
UefiCpuPkg/MpInitLib: Use SEV-SNP AP Creation NAE event to launch APs

OvmfPkg/OvmfPkg.dec | 23 +
UefiCpuPkg/UefiCpuPkg.dec | 11 +
OvmfPkg/AmdSev/AmdSevX64.dsc | 5 +-
OvmfPkg/Bhyve/BhyveX64.dsc | 5 +-
OvmfPkg/OvmfPkgIa32.dsc | 1 +
OvmfPkg/OvmfPkgIa32X64.dsc | 6 +-
OvmfPkg/OvmfPkgX64.dsc | 5 +-
OvmfPkg/OvmfXen.dsc | 5 +-
OvmfPkg/OvmfPkgX64.fdf | 12 +-
OvmfPkg/AmdSevDxe/AmdSevDxe.inf | 7 +
.../DxeMemEncryptSevLib.inf | 3 +
.../PeiMemEncryptSevLib.inf | 7 +
.../SecMemEncryptSevLib.inf | 3 +
OvmfPkg/Library/VmgExitLib/SecVmgExitLib.inf | 2 +
OvmfPkg/Library/VmgExitLib/VmgExitLib.inf | 3 +
OvmfPkg/PlatformPei/PlatformPei.inf | 10 +
OvmfPkg/ResetVector/ResetVector.inf | 6 +
OvmfPkg/Sec/SecMain.inf | 4 +
UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf | 4 +
UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf | 4 +
MdePkg/Include/Register/Amd/Ghcb.h | 2 +-
.../Guid/ConfidentialComputingSecret.h | 18 +
OvmfPkg/Include/Library/MemEncryptSevLib.h | 26 +
.../X64/SnpPageStateChange.h | 31 ++
.../BaseMemEncryptSevLib/X64/VirtualMemory.h | 19 +
OvmfPkg/Sec/AmdSev.h | 95 ++++
UefiCpuPkg/Library/MpInitLib/MpLib.h | 20 +
OvmfPkg/AmdSevDxe/AmdSevDxe.c | 23 +
.../DxeMemEncryptSevLibInternal.c | 27 ++
.../Ia32/MemEncryptSevLib.c | 17 +
.../PeiMemEncryptSevLibInternal.c | 27 ++
.../SecMemEncryptSevLibInternal.c | 19 +
.../X64/DxeSnpSystemRamValidate.c | 40 ++
.../X64/PeiDxeVirtualMemory.c | 167 ++++++-
.../X64/PeiSnpSystemRamValidate.c | 126 +++++
.../X64/SecSnpSystemRamValidate.c | 36 ++
.../X64/SnpPageStateChangeInternal.c | 295 ++++++++++++
OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c | 444 ++++++++++++++++--
OvmfPkg/PlatformPei/AmdSev.c | 192 ++++++++
OvmfPkg/PlatformPei/MemDetect.c | 21 +
OvmfPkg/Sec/AmdSev.c | 267 +++++++++++
OvmfPkg/Sec/SecMain.c | 160 +------
UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 11 +-
.../MpInitLib/Ia32/SevSnpRmpAdjustInternal.c | 31 ++
UefiCpuPkg/Library/MpInitLib/MpLib.c | 286 ++++++++++-
.../MpInitLib/X64/SevSnpRmpAdjustInternal.c | 44 ++
OvmfPkg/FvmainCompactScratchEnd.fdf.inc | 5 +
OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm | 28 ++
OvmfPkg/ResetVector/Ia32/AmdSev.asm | 307 +++++++++++-
OvmfPkg/ResetVector/ResetVector.nasmb | 6 +
UefiCpuPkg/Library/MpInitLib/MpEqu.inc | 2 +
UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 78 +++
52 files changed, 2771 insertions(+), 225 deletions(-)
create mode 100644
OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChange.h
create mode 100644 OvmfPkg/Sec/AmdSev.h
create mode 100644
OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c
create mode 100644
OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiSnpSystemRamValidate.c
create mode 100644
OvmfPkg/Library/BaseMemEncryptSevLib/X64/SecSnpSystemRamValidate.c
create mode 100644
OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChangeInternal.c
create mode 100644 OvmfPkg/Sec/AmdSev.c
create mode 100644
UefiCpuPkg/Library/MpInitLib/Ia32/SevSnpRmpAdjustInternal.c
create mode 100644
UefiCpuPkg/Library/MpInitLib/X64/SevSnpRmpAdjustInternal.c

--
2.17.1


Min Xu
 

On September 9, 2021 3:46 AM, Brijesh Singh wrote:

Thank you so much Yao for reviewing the patches. Based on some comments
from Gerd I may update code around the reset vector area (mainly use the
metadata format etc). For your comments regarding the introducing a new
PcdConfidentialComputingCategory I will look to see what I can come up with
and in UefiCpuPkg I will try to move all the SEV specific functions in new files
(where applicable).
Hi, Brijesh
if you are considering to introduce a new PcdConfidentialComputingCategory
as Jiewen suggested below:

0008-UefiCpuPkg-Define-the-SEV-SNP-specific-dynamic-PCDs
I really don't like the idea to use BOOL PcdSevEsIsEnabled and
PcdSevSnpIsEnabled.
Can we define *one* PCD - such as PcdConfidentialComputingCategory?
We can assign range 0x0000~0xFFFF to AMD SEV, 0x10000~0x1FFFF to Intel
TDX.
Then SEV=0x0000, SEV-ES=0x0001, SEV-SNP=0x0002, and TDX=0x10000
later.
I really don't want to keep adding PCD endlessly in the future, like
PcdSevXXXIsEnabled, PcdSevYYYIsEnabled, PcdTdxIsEnabled,
PcdTdx20Enabled, PcdTdx30Enabled, ......
I also have some suggestions.

As we have below definition in WorkArea.h
typedef struct _CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER {
UINT8 GuestType;
UINT8 Reserved1[3];
} CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER;

Can we update above CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER to below:
typedef struct _CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER {
UINT8 GuestType;
UINT8 SubType; // subtype which indicates SEV-ES, SEV-NP, or TDX 1.0, TDX 2.0 etc.
UINT8 Reserved1[2];
} CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER;

The PcdConfidentialComputingCategory can be defined as UINT32, like below:
## This dynamic PCD indicates the Confidential Computing Category
# [7:0] Confidential Computing Category (0 - Non-Cc, 1 - AmdSev, 2 - IntelTdx)
# [15:8] Sub-Category (defined by each vendor, SEV-ES, SEV-SNP, or TDX-1.0, TDX-2.0, etc)
# [31:16] Reserved
# @Prompt Confidential Computing Category
gUefiCpuPkgTokenSpaceGuid.PcdConfidentialComputingCategory|0|UINT32|0x60000018

So that we simply copy the CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER to PcdConfidentialComputingCategory.
What's your thought?

Thanks!
Min


Brijesh Singh
 

Hi Min,

On 9/8/21 7:31 PM, Xu, Min M wrote:
On September 9, 2021 3:46 AM, Brijesh Singh wrote:
Thank you so much Yao for reviewing the patches. Based on some comments
from Gerd I may update code around the reset vector area (mainly use the
metadata format etc). For your comments regarding the introducing a new
PcdConfidentialComputingCategory I will look to see what I can come up with
and in UefiCpuPkg I will try to move all the SEV specific functions in new files
(where applicable).
Hi, Brijesh
if you are considering to introduce a new PcdConfidentialComputingCategory
as Jiewen suggested below:
0008-UefiCpuPkg-Define-the-SEV-SNP-specific-dynamic-PCDs
I really don't like the idea to use BOOL PcdSevEsIsEnabled and
PcdSevSnpIsEnabled.
Can we define *one* PCD - such as PcdConfidentialComputingCategory?
We can assign range 0x0000~0xFFFF to AMD SEV, 0x10000~0x1FFFF to Intel
TDX.
Then SEV=0x0000, SEV-ES=0x0001, SEV-SNP=0x0002, and TDX=0x10000
later.
I really don't want to keep adding PCD endlessly in the future, like
PcdSevXXXIsEnabled, PcdSevYYYIsEnabled, PcdTdxIsEnabled,
PcdTdx20Enabled, PcdTdx30Enabled, ......
I also have some suggestions.

As we have below definition in WorkArea.h
typedef struct _CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER {
UINT8 GuestType;
UINT8 Reserved1[3];
} CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER;

Can we update above CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER to below:
typedef struct _CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER {
UINT8 GuestType;
UINT8 SubType; // subtype which indicates SEV-ES, SEV-NP, or TDX 1.0, TDX 2.0 etc.
UINT8 Reserved1[2];
} CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER;

The PcdConfidentialComputingCategory can be defined as UINT32, like below:
## This dynamic PCD indicates the Confidential Computing Category
# [7:0] Confidential Computing Category (0 - Non-Cc, 1 - AmdSev, 2 - IntelTdx)
# [15:8] Sub-Category (defined by each vendor, SEV-ES, SEV-SNP, or TDX-1.0, TDX-2.0, etc)
# [31:16] Reserved
# @Prompt Confidential Computing Category
gUefiCpuPkgTokenSpaceGuid.PcdConfidentialComputingCategory|0|UINT32|0x60000018

So that we simply copy the CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER to PcdConfidentialComputingCategory.
What's your thought?
I am not sure if its a good idea to pack a header like above in a 32-bit
PCD. The caller need to unpack the 32-bit number and perform a bitshit
etc. Additionally we also need to check for reserved bits being set to
zero etc. I am more inclined toward something like this:

enum {

   /* The guest is running with memory encryption disabled. */

    CC_ATTR_UNDEF,

  /* The guest is running with active AMD SEV memory encryption. */

   CC_ATTR_AMD_SEV,

  /* The guest is running with active AMD SEV-ES memory encryption. */

  CC_ATTR_AMD_SEV_ES,

  /* The guest is running with active AMD SEV-SNP memory encryption. */

  CC_ATTR_AMD_SEV_SNP,

 /* The guest is running with active Intel TDX memory encryption. */

 CC_ATTR_INTEL_TDX,

 /* The guest is running with active Intel SGX memory encryption. */

 CC_ATTR_INTEL_SGX,

} ConfidentialComputingAttr;

The PcdConfidentialComputingAttr will be set to zero. The OVMF will set
this dynamic PCD in PEI phase. The UefiCpuPkg will provide a new
function that can be used by anyone to check the CC guest type.

BOOLEAN

CcPlatformHas(enum ConfidentialComputingAttr attr)

{

    UINT32 Val;

    Val = PcdGet32 (PcdConfidentialComputingAttr);

    return val == attr;

}


Thanks!
Min


Gerd Hoffmann
 

Hi,

I am not sure if its a good idea to pack a header like above in a 32-bit
PCD. The caller need to unpack the 32-bit number and perform a bitshit
etc. Additionally we also need to check for reserved bits being set to
zero etc. I am more inclined toward something like this:

enum {
Well, various places probably just need to know whenever they should
call into the sev or the tdx library, so grouping stuff makes sense to
me. We don't need bitfields for that though, could also be done this
way:

enum {
NOT_ENCRYPTED = 0,
AMD_SEV = 0x100,
AMD_SEV_ES,
[ ... ]
INTEL_TDX = 0x200,
[ ... ]
}

So if you need the exact mode you can compare values as-is, if you want
figure which vendor library should be called you'll just mask out the
least significant 8 bits.

take care,
Gerd


Brijesh Singh
 

On 9/9/21 6:22 AM, Gerd Hoffmann wrote:
Hi,

I am not sure if its a good idea to pack a header like above in a 32-bit
PCD. The caller need to unpack the 32-bit number and perform a bitshit
etc. Additionally we also need to check for reserved bits being set to
zero etc. I am more inclined toward something like this:

enum {
Well, various places probably just need to know whenever they should
call into the sev or the tdx library, so grouping stuff makes sense to
me. We don't need bitfields for that though, could also be done this
way:

enum {
NOT_ENCRYPTED = 0,
AMD_SEV = 0x100,
AMD_SEV_ES,
[ ... ]
INTEL_TDX = 0x200,
[ ... ]
}

So if you need the exact mode you can compare values as-is, if you want
figure which vendor library should be called you'll just mask out the
least significant 8 bits.
Yes, this also works fine.

thanks


Min Xu
 

On September 9, 2021 7:40 PM, Brijesh Singh wrote:
On 9/9/21 6:22 AM, Gerd Hoffmann wrote:
Hi,

I am not sure if its a good idea to pack a header like above in a
32-bit PCD. The caller need to unpack the 32-bit number and perform a
bitshit etc. Additionally we also need to check for reserved bits
being set to zero etc. I am more inclined toward something like this:

enum {
Well, various places probably just need to know whenever they should
call into the sev or the tdx library, so grouping stuff makes sense to
me. We don't need bitfields for that though, could also be done this
way:

enum {
NOT_ENCRYPTED = 0,
AMD_SEV = 0x100,
AMD_SEV_ES,
[ ... ]
INTEL_TDX = 0x200,
[ ... ]
}

So if you need the exact mode you can compare values as-is, if you
want figure which vendor library should be called you'll just mask out
the least significant 8 bits.
Yes, this also works fine.
Agree. So let's follow this way.

Thanks!
Min


Yao, Jiewen
 

I do not see any conflict here.

You can use below definition
enum {
CC_ATTR_AMD_SEV = 0x0001,
CC_ATTR_AMD_SEV_ES = 0x0101,
CC_ATTR_AMD_SEV_SNP = 0x0201,
CC_ATTR_INTEL_TDX = 0x0002,
} ConfidentialComputingAttr;

BTW: Please remove SGX, we don’t need it here.

Thank you
Yao Jiewen

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Brijesh
Singh via groups.io
Sent: Thursday, September 9, 2021 6:51 PM
To: Xu, Min M <min.m.xu@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>;
devel@edk2.groups.io
Cc: James Bottomley <jejb@linux.ibm.com>; Tom Lendacky
<thomas.lendacky@amd.com>; Justen, Jordan L <jordan.l.justen@intel.com>;
Ard Biesheuvel <ardb+tianocore@kernel.org>; Erdem Aktas
<erdemaktas@google.com>; Michael Roth <Michael.Roth@amd.com>; Gerd
Hoffmann <kraxel@redhat.com>
Subject: Re: [edk2-devel] [PATCH v6 00/29] Add AMD Secure Nested Paging
(SEV-SNP) support

Hi Min,

On 9/8/21 7:31 PM, Xu, Min M wrote:
On September 9, 2021 3:46 AM, Brijesh Singh wrote:
Thank you so much Yao for reviewing the patches. Based on some comments
from Gerd I may update code around the reset vector area (mainly use the
metadata format etc). For your comments regarding the introducing a new
PcdConfidentialComputingCategory I will look to see what I can come up
with
and in UefiCpuPkg I will try to move all the SEV specific functions in new files
(where applicable).
Hi, Brijesh
if you are considering to introduce a new PcdConfidentialComputingCategory
as Jiewen suggested below:
0008-UefiCpuPkg-Define-the-SEV-SNP-specific-dynamic-PCDs
I really don't like the idea to use BOOL PcdSevEsIsEnabled and
PcdSevSnpIsEnabled.
Can we define *one* PCD - such as PcdConfidentialComputingCategory?
We can assign range 0x0000~0xFFFF to AMD SEV, 0x10000~0x1FFFF to Intel
TDX.
Then SEV=0x0000, SEV-ES=0x0001, SEV-SNP=0x0002, and TDX=0x10000
later.
I really don't want to keep adding PCD endlessly in the future, like
PcdSevXXXIsEnabled, PcdSevYYYIsEnabled, PcdTdxIsEnabled,
PcdTdx20Enabled, PcdTdx30Enabled, ......
I also have some suggestions.

As we have below definition in WorkArea.h
typedef struct _CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER {
UINT8 GuestType;
UINT8 Reserved1[3];
} CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER;

Can we update above CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER to
below:
typedef struct _CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER {
UINT8 GuestType;
UINT8 SubType; // subtype which indicates SEV-ES, SEV-NP,
or TDX 1.0, TDX 2.0 etc.
UINT8 Reserved1[2];
} CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER;

The PcdConfidentialComputingCategory can be defined as UINT32, like below:
## This dynamic PCD indicates the Confidential Computing Category
# [7:0] Confidential Computing Category (0 - Non-Cc, 1 - AmdSev, 2 -
IntelTdx)
# [15:8] Sub-Category (defined by each vendor, SEV-ES, SEV-SNP, or TDX-1.0,
TDX-2.0, etc)
# [31:16] Reserved
# @Prompt Confidential Computing Category
gUefiCpuPkgTokenSpaceGuid.PcdConfidentialComputingCategory|0|UINT32|0x
60000018

So that we simply copy the
CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER to
PcdConfidentialComputingCategory.
What's your thought?
I am not sure if its a good idea to pack a header like above in a 32-bit
PCD. The caller need to unpack the 32-bit number and perform a bitshit
etc. Additionally we also need to check for reserved bits being set to
zero etc. I am more inclined toward something like this:

enum {

   /* The guest is running with memory encryption disabled. */

    CC_ATTR_UNDEF,

  /* The guest is running with active AMD SEV memory encryption. */

   CC_ATTR_AMD_SEV,

  /* The guest is running with active AMD SEV-ES memory encryption. */

  CC_ATTR_AMD_SEV_ES,

  /* The guest is running with active AMD SEV-SNP memory encryption. */

  CC_ATTR_AMD_SEV_SNP,

 /* The guest is running with active Intel TDX memory encryption. */

 CC_ATTR_INTEL_TDX,

 /* The guest is running with active Intel SGX memory encryption. */

 CC_ATTR_INTEL_SGX,

} ConfidentialComputingAttr;

The PcdConfidentialComputingAttr will be set to zero. The OVMF will set
this dynamic PCD in PEI phase. The UefiCpuPkg will provide a new
function that can be used by anyone to check the CC guest type.

BOOLEAN

CcPlatformHas(enum ConfidentialComputingAttr attr)

{

    UINT32 Val;

    Val = PcdGet32 (PcdConfidentialComputingAttr);

    return val == attr;

}


Thanks!
Min



Brijesh Singh
 

Hi Yao,

I am going through implementing your feedback. I have covered most of
it. But your comment on moving some of the changes from MpFunc.nasm to
Sev.nasm may make code harder to read. It is mainly because the GPA
registration and Topo check are not self-contained routines. They depend
on some previous register states and need to jump to different paths
based on the condition. All those jump are in generic AP init routines.

As you can see, the changes in MpFunc.nasm are more petite, and if it's
a big issue, we can revisit it later and devise a proposal to move the
big chunk of generic code in a separate file then have TDX and SEV call
into it. At that time, we can add OneTimeCall or OneTimeReturn macros
etc., to jump between different files to make it more readable. Would
you please let me know if that is acceptable?

-Brijesh

On 9/6/21 9:36 PM, Yao, Jiewen wrote:

0023-UefiCpuPkg-MpLib-add-support-to-register-GHCB-GPA-when-SEV-SNP-is-enabled
1) See 0008.
2) For MpFuncs.nasm, I recommend to move AmdSev specific initialization to a standalone file, such as Sev.nasm

0024-UefiCpuPkg-MpInitLib-use-BSP-to-do-extended-topology-check
See 0023


Yao, Jiewen
 

Hi Brijesh
I think it is OK to leave MpFunc.nasm in this series.
We can revisit later.

Thank you
Yao Jiewen

-----Original Message-----
From: Brijesh Singh <brijesh.singh@amd.com>
Sent: Monday, September 13, 2021 6:56 AM
To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io
Cc: James Bottomley <jejb@linux.ibm.com>; Xu, Min M <min.m.xu@intel.com>;
Tom Lendacky <thomas.lendacky@amd.com>; Justen, Jordan L
<jordan.l.justen@intel.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>;
Erdem Aktas <erdemaktas@google.com>; Michael Roth
<Michael.Roth@amd.com>; Gerd Hoffmann <kraxel@redhat.com>
Subject: Re: [PATCH v6 00/29] Add AMD Secure Nested Paging (SEV-SNP)
support

Hi Yao,

I am going through implementing your feedback. I have covered most of
it. But your comment on moving some of the changes from MpFunc.nasm to
Sev.nasm may make code harder to read. It is mainly because the GPA
registration and Topo check are not self-contained routines. They depend
on some previous register states and need to jump to different paths
based on the condition. All those jump are in generic AP init routines.

As you can see, the changes in MpFunc.nasm are more petite, and if it's
a big issue, we can revisit it later and devise a proposal to move the
big chunk of generic code in a separate file then have TDX and SEV call
into it. At that time, we can add OneTimeCall or OneTimeReturn macros
etc., to jump between different files to make it more readable. Would
you please let me know if that is acceptable?

-Brijesh

On 9/6/21 9:36 PM, Yao, Jiewen wrote:

0023-UefiCpuPkg-MpLib-add-support-to-register-GHCB-GPA-when-SEV-SNP-
is-enabled
1) See 0008.
2) For MpFuncs.nasm, I recommend to move AmdSev specific initialization to a
standalone file, such as Sev.nasm

0024-UefiCpuPkg-MpInitLib-use-BSP-to-do-extended-topology-check
See 0023