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

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

Hi Brijesh
Have you got R-B from UefiCpuPkg maintainer?



> -----Original Message-----
> From: Brijesh Singh <brijesh.singh@amd.com>
> Sent: Monday, October 25, 2021 7:54 AM
> To: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>
> Cc: brijesh.singh@amd.com; 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: [edk2-devel] [PATCH v11 00/32] Add AMD Secure Nested Paging
> (SEV-SNP) support
> 
> Thank Jiewen,
> 
> I have ping'ed UefiCpuPkg maintainer (Ray and Rahul) on every patch
> which touches the UefiCpuPkg. If maintainer wants me to rework on
> something then I will work accordingly. If they are okay with v11 then
> now the merge will create a conflict (due to the TDX patches merge
> commit). I have rebased my series to the recent master and have pushed
> it here: https://github.com/AMDESE/ovmf/tree/snp-v12. I can post the
> series if you prefer it.
> 
> thanks
> 
> On 10/23/21 8:46 PM, Yao, Jiewen via groups.io wrote:
> > 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://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.
> tianocore.org%2Fshow_bug.cgi%3Fid%3D3275&amp;data=04%7C01%7Cbrijesh.
> singh%40amd.com%7Cddc5570780ff4a91d0da08d9969026e2%7C3dd8961fe488
> 4e608e11a82d994e183d%7C0%7C0%7C637706369230826414%7CUnknown%7
> CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJ
> XVCI6Mn0%3D%7C3000&amp;sdata=L41krO6G221HaIsG92FloIzgCDqMLAAsU26
> jaEMF7yw%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.a
> md.com%2Fsystem%2Ffiles%2FTechDocs%2FSEV-SNP-strengthening-vm-
> &amp;data=04%7C01%7Cbrijesh.singh%40amd.com%7Cddc5570780ff4a91d0da
> 08d9969026e2%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C63770
> 6369230826414%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQ
> IjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=nVMSG%
> 2FvSS2Wa21lu1lGrHr9OYX8hL7FoAcQXBBiCztc%3D&amp;reserved=0
> >> isolation-with-integrity-protection-and-more.pdf
> >>
> >> APM 2:
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.a
> md.com%2Fsystem%2Ffiles%2FTechDocs%2F24593.pdf&amp;data=04%7C01%7
> Cbrijesh.singh%40amd.com%7Cddc5570780ff4a91d0da08d9969026e2%7C3dd8
> 961fe4884e608e11a82d994e183d%7C0%7C0%7C637706369230826414%7CUnk
> nown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1h
> aWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=G8Xg2glOGY2EjHpeQ3WM4gZCh
> uI0k8QcLDTbpJiTplg%3D&amp;reserved=0 (section 15.36)
> >>
> >> The complete source is available at
> >>
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.c
> om%2FAMDESE%2Fovmf%2Ftree%2Fsnp-
> v11&amp;data=04%7C01%7Cbrijesh.singh%40amd.com%7Cddc5570780ff4a91d
> 0da08d9969026e2%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C63
> 7706369230826414%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiL
> CJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=HMH
> Fq8G%2FPqdhzNW3Ashmc4%2Bmv1RcDULD4vniofhiS54%3D&amp;reserved=0
> >>
> >> GHCB spec:
> >>
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdevelop
> er.amd.com%2Fwp-
> content%2Fresources%2F56421.pdf&amp;data=04%7C01%7Cbrijesh.singh%40a
> md.com%7Cddc5570780ff4a91d0da08d9969026e2%7C3dd8961fe4884e608e11
> a82d994e183d%7C0%7C0%7C637706369230826414%7CUnknown%7CTWFpbGZ
> sb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0
> %3D%7C3000&amp;sdata=YiPgZU87fdnl5rJpD0E2ue9aTKbqUwizuBrKxom0FiU%
> 3D&amp;reserved=0
> >>
> >> SEV-SNP firmware specification:
> >>
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.a
> md.com%2Fsystem%2Ffiles%2FTechDocs%2F56860.pdf&amp;data=04%7C01%7
> Cbrijesh.singh%40amd.com%7Cddc5570780ff4a91d0da08d9969026e2%7C3dd8
> 961fe4884e608e11a82d994e183d%7C0%7C0%7C637706369230826414%7CUnk
> nown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1h
> aWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=bfQsY4%2BRnlFGuD3Bg%2BFPb3l
> RgSGgpomNocXswHqkm%2F4%3D&amp;reserved=0
> >>
> >> 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
> >
> >
> > 
> >
> >