Re: [PATCH v11 00/32] Add AMD Secure Nested Paging (SEV-SNP) support

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

Yes. I will try my best to merge.

I checked the patch set but I did not find the "R-B" from UefiCpuPkg maintainer. Neither from email nor from you v11.

Did I miss something?

Thank you
Yao Jiewen


> -----Original Message-----
> From: Brijesh Singh <brijesh.singh@amd.com>
> Sent: Saturday, October 23, 2021 12:13 PM
> 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 v11 00/32] Add AMD Secure Nested Paging (SEV-SNP) support
> 
> Hi Gerd and Jiewen,
> 
> CI was a bit unstable during my v10 submission, so, I was not able to
> run it to the completion. Finally, I managed to get the CI going,
> and it reported few Windows 32-bit build errors. The v11 fixes those build
> errors. Please consider this for the merge.
> 
> Thank you so much for all your support in reviewing the series.
> 
> -----------------------------------------------------------------------------
> 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/snp-v11
> 
> 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 v10:
>  * fix 'unresolved external symbol __allshl' link error when building I32 for
> VS2017.
> 
> Changes since v9:
>  * Move CCAttrs Pcd define in MdePkg
>  * Add comment to indicate that allocating the identity map PT is temporary until
> we get lazy validation
> 
> Changes since v8:
>  * drop the generic metadata and make it specific to SEV.
> 
> Changes since v7:
>  * Move SEV specific changes in MpLib in AmdSev file
>  * Update the GHCB register function to not restore the GHCB MSR because
>    we were already in the MSR protocol mode.
>  * Drop the SNP name from PcdSnpSecPreValidate.
>  * Add new section for GHCB memory in the OVMF metadata.
> 
> Change since v6:
>  * Drop the SNP boot block GUID and switch to using the Metadata guided
> structure
>    proposed by Min in TDX series.
>  * Exclude the GHCB page from the pre-validated region. It simplifies the reset
>    vector code where we do not need to unvalidate the GHCB page.
>  * Now that GHCB page is not validated so move the VMPL check from reset
> vector
>    code to the MemEncryptSevLib on the first page validation.
>  * Introduce the ConfidentialComputingGuestAttr PCD to communicate which
>    memory encryption is active so that MpInitLib can make use of it.
>  * Drop the SEVES specific PCD as the information can be communicated via
>    the ConfidentialComputingGuestAttr.
>  * Move the SNP specific AP creation function in AmdSev.c.
>  * Define the SNP Blob GUID in a new file.
> 
> 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 (28):
>   OvmfPkg/SecMain: move SEV specific routines in AmdSev.c
>   UefiCpuPkg/MpInitLib: move SEV specific routines in AmdSev.c
>   OvmfPkg/ResetVector: move clearing GHCB in SecMain
>   OvmfPkg/ResetVector: introduce SEV metadata descriptor for VMM use
>   OvmfPkg: reserve SNP secrets page
>   OvmfPkg: reserve CPUID page
>   OvmfPkg/ResetVector: pre-validate the data pages used in SEC phase
>   OvmfPkg/MemEncryptSevLib: add MemEncryptSevSnpEnabled()
>   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/MemEncryptSevLib: add function to check the VMPL0
>   OvmfPkg/BaseMemEncryptSevLib: skip the pre-validated system RAM
>   OvmfPkg/MemEncryptSevLib: add support to validate > 4GB memory in PEI
>     phase
>   OvmfPkg/SecMain: validate the memory used for decompressing Fv
>   OvmfPkg/PlatformPei: validate the system RAM when SNP is active
>   UefiCpuPkg: Define ConfidentialComputingGuestAttr
>   OvmfPkg/PlatformPei: set PcdConfidentialComputingAttr when SEV is
>     active
>   UefiCpuPkg/MpInitLib: use PcdConfidentialComputingAttr to check SEV
>     status
>   UefiCpuPkg: add PcdGhcbHypervisorFeatures
>   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
> 
>  MdePkg/MdePkg.dec                             |   4 +
>  OvmfPkg/OvmfPkg.dec                           |  18 +
>  UefiCpuPkg/UefiCpuPkg.dec                     |   5 +
>  OvmfPkg/AmdSev/AmdSevX64.dsc                  |   8 +-
>  OvmfPkg/Bhyve/BhyveX64.dsc                    |   5 +-
>  OvmfPkg/OvmfPkgIa32.dsc                       |   4 +
>  OvmfPkg/OvmfPkgIa32X64.dsc                    |   9 +-
>  OvmfPkg/OvmfPkgX64.dsc                        |   8 +-
>  OvmfPkg/OvmfXen.dsc                           |   5 +-
>  OvmfPkg/OvmfPkgX64.fdf                        |   6 +
>  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           |   7 +
>  OvmfPkg/ResetVector/ResetVector.inf           |   5 +
>  OvmfPkg/Sec/SecMain.inf                       |   4 +
>  UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf |   6 +-
>  UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf |   6 +-
>  .../Include/ConfidentialComputingGuestAttr.h  |  25 +
>  MdePkg/Include/Register/Amd/Ghcb.h            |   2 +-
>  .../Guid/ConfidentialComputingSevSnpBlob.h    |  33 ++
>  OvmfPkg/Include/Library/MemEncryptSevLib.h    |  26 +
>  .../X64/SnpPageStateChange.h                  |  36 ++
>  .../BaseMemEncryptSevLib/X64/VirtualMemory.h  |  24 +
>  OvmfPkg/PlatformPei/Platform.h                |   5 +
>  OvmfPkg/Sec/AmdSev.h                          |  95 ++++
>  UefiCpuPkg/Library/MpInitLib/MpLib.h          |  93 ++++
>  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             | 127 +++++
>  .../X64/SecSnpSystemRamValidate.c             |  82 ++++
>  .../X64/SnpPageStateChangeInternal.c          | 294 ++++++++++++
>  OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c | 444 ++++++++++++++++--
>  OvmfPkg/PlatformPei/AmdSev.c                  | 231 +++++++++
>  OvmfPkg/PlatformPei/MemDetect.c               |   2 +
>  OvmfPkg/Sec/AmdSev.c                          | 298 ++++++++++++
>  OvmfPkg/Sec/SecMain.c                         | 158 +------
>  UefiCpuPkg/Library/MpInitLib/AmdSev.c         | 239 ++++++++++
>  UefiCpuPkg/Library/MpInitLib/DxeMpLib.c       |  16 +-
>  UefiCpuPkg/Library/MpInitLib/Ia32/AmdSev.c    |  70 +++
>  UefiCpuPkg/Library/MpInitLib/MpLib.c          | 345 +++++---------
>  UefiCpuPkg/Library/MpInitLib/PeiMpLib.c       |   4 +-
>  UefiCpuPkg/Library/MpInitLib/X64/AmdSev.c     | 261 ++++++++++
>  OvmfPkg/FvmainCompactScratchEnd.fdf.inc       |   5 +
>  OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm  |  17 +
>  OvmfPkg/ResetVector/Ia32/AmdSev.asm           |  86 +++-
>  OvmfPkg/ResetVector/ResetVector.nasmb         |  18 +
>  OvmfPkg/ResetVector/X64/OvmfSevMetadata.asm   |  74 +++
>  UefiCpuPkg/Library/MpInitLib/MpEqu.inc        |   2 +
>  UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm  | 200 ++++++++
>  UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 100 +---
>  59 files changed, 3329 insertions(+), 528 deletions(-)
>  create mode 100644 MdePkg/Include/ConfidentialComputingGuestAttr.h
>  create mode 100644
> OvmfPkg/Include/Guid/ConfidentialComputingSevSnpBlob.h
>  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/AmdSev.c
>  create mode 100644 UefiCpuPkg/Library/MpInitLib/Ia32/AmdSev.c
>  create mode 100644 UefiCpuPkg/Library/MpInitLib/X64/AmdSev.c
>  create mode 100644 OvmfPkg/ResetVector/X64/OvmfSevMetadata.asm
>  create mode 100644 UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm
> 
> --
> 2.25.1