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

["Brijesh Singh" <brijesh.singh@amd.com>]

Hi Laszlo,

On 4/8/21 4:58 AM, Laszlo Ersek wrote:
> Hi Brijesh,
>
> On 03/24/21 16:31, Brijesh Singh wrote:
>> 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%7Cb8ee3abf81aa4e5b1e6008d8fa74dbbc%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637534728122143364%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=G2AQ%2FCks3%2BbczHJXMwqlqpWpoBJmb0pmxb1VNLw6t%2BA%3D&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.
> Can you describe this in a bit more detail, before I look at the
> individual patches? Specifically, what existing logic in the PEI phase
> was taken, and extended, and how?

One of the key requirement is that the guest private pages much be
validated before the access. If guest tries to access the pages before
the validation then it will result in #VC (page-not-validated)
exception. To avoid the #VC, we propose the validating the memory before
the access. We will incrementally add the support to lazy validate (i.e
validate on access).

Let me try explaining a bit, the page validation process consist of two
steps:

1. Add the pages in the RMP table -- must be done by the hypervisor
using the RMPUPDATE instruction. The guest can use VMGEXIT NAEs to ask
hypervisor to add or remove pages from the RMP table.

2. Guest issue the PVALIDATE instruction -- this sets the validate bit
in the RMP table.

Similar to SEV, the OVMF_CODE.fd is encrypted through the SNP firmware
before the launch. The SNP firmware also validates the memory page after
encrypting. This allows us to boot the initial entry code without guest
going through the validation process.

The OVMF reset vector uses few data pages (e.g page table, early Sec
stack). Access to these data pages will result in #VC. There are two
approaches we can take to validate these data pages:

1. Ask SNP firmware to pre-validate it -- SNP firmware provides an
special command that can be used to pre-validate the pages without
affecting the measurement.

2. Enhance the reset vector code to validate the pages.

For now I choose #1.

The pre-validation performed by the SNP firmware is sufficient to boot
through the SEC phase. The SEC phase later decompress the Fv to a new
memory location. Now we need the OVMF to take over the validation
procedure.  The series extends the MemEncryptSevLib to add a new helper
MemEncryptSevSnpValidateRam(). The helper is used to validate the system
RAM. See patch #12. SEC phase calls the MemEncryptSevSnpValidateRam() to
validate the output buffer used for the decompression. This was
sufficient to boot into the PEI phase, see patch #13. The PEI detects
all the available system RAM. After the memory detection is completed
the PlatformPei calls the AmdSevSnpInitialize(). The initialization
routine iterate through the HOB and calls the
MemEncryptSevSnpValidateRam() to validate all the system RAM. Is it
possible the more system ram can be detected after the PlatformPei is
completed ?

One of the important thing is we should *never* validate the pages
twice. The MemEncryptSevSnpValidateRam() uses a interval search tree to
keep the record of what has been validated. Before validating the range,
it lookup in its tree and if it finds that range is already validated
then do nothing. If it detects an overlap then it will validate only non
overlapping regions -- see patch #14.

The patch #18 extend the MemEncrypt{Set,Clear}PageEncMask() to call the
SNP page state change during the C-bit toggle.

Please let me know if you have any questions. We can hash out the design
here before you taking a closure look at the code.

>
> If there is a particular patch whose commit message is closely related
> to my question, can you point it out? Patch#15 perhaps? (Doesn't seem
> like a big patch; for some reason I'd expect something more complex, but
> perhaps that's only because it builds upon the many earlier patches.)
>
> Thanks,
> Laszlo
>
>> This series does not implements the following SEV-SNP features yet:
>>
>> * CPUID filtering
>> * AP bring up using the new SEV-SNP NAE
>> * Lazy validation
>> * Interrupt security
>>
>> The series is based on commit:
>> e542e05d4f UefiCpuPkg/SmmCpuFeaturesLib: Abstract PcdCpuMaxLogicalProcessorNumber
>>
>> 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-isolation-with-integrity-protection-and-more.pdf&data=04%7C01%7Cbrijesh.singh%40amd.com%7Cb8ee3abf81aa4e5b1e6008d8fa74dbbc%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637534728122143364%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=uUqlVHhWQ6geGDaNHwxGMpoSpIamB%2F1vHH69h%2FEGUro%3D&reserved=0
>>  
>> 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%7Cb8ee3abf81aa4e5b1e6008d8fa74dbbc%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637534728122143364%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=R2kU42jvCDZat8kGZ5gDDz2nFXIawHfXdRW1aovhNK8%3D&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-1&data=04%7C01%7Cbrijesh.singh%40amd.com%7Cb8ee3abf81aa4e5b1e6008d8fa74dbbc%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637534728122143364%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=yreFg2hr%2F82WYEjxqCmb7pXUdtrRCJRYPrPHgfrWjM8%3D&reserved=0
>>
>> GHCB spec v2:
>>   The draft specification is posted on AMD-SEV-SNP mailing list:
>>    https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.suse.com%2Fmailman%2Fprivate%2Famd-sev-snp%2F&data=04%7C01%7Cbrijesh.singh%40amd.com%7Cb8ee3abf81aa4e5b1e6008d8fa74dbbc%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637534728122143364%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=PFV2mA7T%2Fbl2zP5j52kNdT%2FavDMRLWDEDqz6JGusEFg%3D&reserved=0
>>
>>   Copy of the spec is also available at 
>>   https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FAMDESE%2FAMDSEV%2Fblob%2Fsev-snp-devel%2Fdocs%2F56421-Guest_Hypervisor_Communication_Block_Standardization.pdf&data=04%7C01%7Cbrijesh.singh%40amd.com%7Cb8ee3abf81aa4e5b1e6008d8fa74dbbc%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637534728122143364%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=U3oKRe5m0NxI0xqv1gBoyh%2BEEX1LVeCWR42rvPh6XZ8%3D&reserved=0
>>
>> GHCB spec v1:
>> SEV-SNP firmware specification:
>>  https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdeveloper.amd.com%2Fsev%2F&data=04%7C01%7Cbrijesh.singh%40amd.com%7Cb8ee3abf81aa4e5b1e6008d8fa74dbbc%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637534728122143364%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=G%2BttkORsTJchJ1Fy1iNlD%2B%2BqiQZuwI8md5vhJjEb%2Fn4%3D&reserved=0
>>   
>> Cc: James Bottomley <jejb@linux.ibm.com>
>> Cc: Min Xu <min.m.xu@intel.com>
>> Cc: Jiewen Yao <jiewen.yao@intel.com>
>> Cc: Tom Lendacky <thomas.lendacky@amd.com>
>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
>> Cc: Laszlo Ersek <lersek@redhat.com>
>>
>> Brijesh Singh (19):
>>   OvmfPkg: Reserve the Secrets and Cpuid page for the SEV-SNP guest
>>   OvmfPkg: validate the data pages used in the SEC phase
>>   MdePkg: Expand the SEV MSR to include the SNP definition
>>   OvmfPkg/MemEncryptSevLib: add MemEncryptSevSnpEnabled()
>>   MdePkg: Define the GHCB GPA structure
>>   UefiCpuPkg/MpLib: add support to register GHCB GPA when SEV-SNP is
>>     enabled
>>   OvmfPkg: Add a library to support registering GHCB GPA
>>   OvmfPkg: register GHCB gpa for the SEV-SNP guest
>>   MdePkg: Add AsmPvalidate() support
>>   OvmfPkg: Define the Page State Change VMGEXIT structures
>>   OvmfPkg/ResetVector: Invalidate the GHCB page
>>   OvmfPkg/MemEncryptSevLib: Add support to validate system RAM
>>   OvmfPkg/SecMain: Validate the data/code pages used for the PEI phase
>>   OvmfPkg/MemEncryptSevLib: Add support to validate RAM in PEI phase
>>   OvmfPkg/PlatformPei: Validate the system RAM when SNP is active
>>   OvmfPkg/MemEncryptSevLib: Add support to validate > 4GB memory in PEI
>>     phase
>>   OvmfPkg/VmgExitLib: Allow PMBASE register access in Dxe phase
>>   OvmfPkg/MemEncryptSevLib: Validate the memory during set or clear enc
>>     attribute
>>   OvmfPkg/MemEncryptSevLib: Skip page state change for non RAM region
>>
>>  MdePkg/Include/Library/BaseLib.h              |  37 +++
>>  MdePkg/Include/Register/Amd/Fam17Msr.h        |  31 ++-
>>  MdePkg/Include/Register/Amd/Ghcb.h            |  39 ++-
>>  MdePkg/Library/BaseLib/BaseLib.inf            |   1 +
>>  MdePkg/Library/BaseLib/X64/Pvalidate.nasm     |  43 +++
>>  OvmfPkg/Include/Library/GhcbRegisterLib.h     |  27 ++
>>  OvmfPkg/Include/Library/MemEncryptSevLib.h    |  30 +++
>>  .../DxeMemEncryptSevLib.inf                   |   7 +
>>  .../DxeMemEncryptSevLibInternal.c             |  27 ++
>>  .../Ia32/SnpPageStateChange.c                 |  17 ++
>>  .../PeiMemEncryptSevLib.inf                   |   9 +
>>  .../PeiMemEncryptSevLibInternal.c             |  47 ++++
>>  .../SecMemEncryptSevLib.inf                   |   4 +
>>  .../SecMemEncryptSevLibInternal.c             |  39 +++
>>  .../BaseMemEncryptSevLib/SnpPageStateChange.h |  37 +++
>>  .../X64/PeiDxeSnpSetPageState.c               |  63 +++++
>>  .../X64/PeiDxeVirtualMemory.c                 | 151 ++++++++++-
>>  .../X64/PeiSnpSystemRamValidate.c             | 129 +++++++++
>>  .../X64/SecSnpSystemRamValidate.c             |  23 ++
>>  .../X64/SnpPageStateChangeInternal.c          | 254 ++++++++++++++++++
>>  .../X64/SnpPageStateTrack.c                   | 119 ++++++++
>>  .../X64/SnpPageStateTrack.h                   |  36 +++
>>  .../X64/SnpSetPageState.h                     |  27 ++
>>  .../BaseMemEncryptSevLib/X64/VirtualMemory.h  |  19 ++
>>  .../Library/GhcbRegisterLib/GhcbRegisterLib.c |  97 +++++++
>>  .../GhcbRegisterLib/GhcbRegisterLib.inf       |  33 +++
>>  OvmfPkg/Library/VmgExitLib/SecVmgExitLib.inf  |   4 +
>>  OvmfPkg/Library/VmgExitLib/VmgExitLib.inf     |   7 +
>>  OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c |  45 ++++
>>  OvmfPkg/OvmfPkg.dec                           |  12 +
>>  OvmfPkg/OvmfPkgX64.dsc                        |   1 +
>>  OvmfPkg/OvmfPkgX64.fdf                        |  33 ++-
>>  OvmfPkg/PlatformPei/AmdSev.c                  |  52 ++++
>>  OvmfPkg/PlatformPei/PlatformPei.inf           |   2 +
>>  OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm  |  24 ++
>>  OvmfPkg/ResetVector/Ia32/PageTables64.asm     | 106 ++++++++
>>  OvmfPkg/ResetVector/ResetVector.inf           |   5 +
>>  OvmfPkg/ResetVector/ResetVector.nasmb         |   4 +
>>  OvmfPkg/Sec/SecMain.c                         | 102 +++++++
>>  OvmfPkg/Sec/SecMain.inf                       |   2 +
>>  UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf |   1 +
>>  UefiCpuPkg/Library/MpInitLib/MpEqu.inc        |   1 +
>>  UefiCpuPkg/Library/MpInitLib/MpLib.c          |   2 +
>>  UefiCpuPkg/Library/MpInitLib/MpLib.h          |   2 +
>>  UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf |   1 +
>>  UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm |  51 ++++
>>  UefiCpuPkg/UefiCpuPkg.dec                     |   6 +
>>  47 files changed, 1790 insertions(+), 19 deletions(-)
>>  create mode 100644 MdePkg/Library/BaseLib/X64/Pvalidate.nasm
>>  create mode 100644 OvmfPkg/Include/Library/GhcbRegisterLib.h
>>  create mode 100644 OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/SnpPageStateChange.c
>>  create mode 100644 OvmfPkg/Library/BaseMemEncryptSevLib/SnpPageStateChange.h
>>  create mode 100644 OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeSnpSetPageState.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/Library/BaseMemEncryptSevLib/X64/SnpPageStateTrack.c
>>  create mode 100644 OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateTrack.h
>>  create mode 100644 OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpSetPageState.h
>>  create mode 100644 OvmfPkg/Library/GhcbRegisterLib/GhcbRegisterLib.c
>>  create mode 100644 OvmfPkg/Library/GhcbRegisterLib/GhcbRegisterLib.inf
>>