Re: [RFC PATCH 01/19] OvmfPkg: Reserve the Secrets and Cpuid page for the SEV-SNP guest

["Min Xu" <min.m.xu@intel.com>]

Hi, Laszlo

For Intel TDX supported guest, all processors start in 32-bit protected
mode, while for Non-Td guest, it starts in 16-bit real mode. To make the
ResetVector work on both Td-guest and Non-Td guest, ResetVector are
updated as below:
------------------------------------------------------------------
  ALIGN   16
  resetVector:
  ;
  ; Reset Vector
  ;
  ; This is where the processor will begin execution
  ;
      nop
      nop
      smsw    ax
      test    al, 1
      jnz     EarlyBspPmEntry
      jmp     EarlyBspInitReal16

  ALIGN   16
  fourGigabytes:
------------------------------------------------------------------

For Non-Td guest, jmp to EarlyBspInitReal16 in 16-bit real mode is ok.

For Td-guest, first jmp to EarlyBspPmEntry in 32-bit protected mode, then
in EarlyBspPmEntry jmp to MainTd which is the the main entry for Td-guest.
This requires the distance between ResetVector and EarlyBspPmEntry less
than 128 bytes.

Intel TDX also has metadata which is consumed by QEMU. We put the metadata
in a single file (TdxMetadata.asm) and put it at the end of ResetVectorVtf0.
Then a pointer is placed in a known location in ResetVector.nasm. In this way
QEMU can easily read the Metadata by the pointer.
------------------------------------------------------------------
ALIGN   8
;
; TDX Virtual Firmware injects metadata in VTF0.
; The address of the metadata is injected in this location (0xffffffe8)
;
    DD      (OVMF_IMAGE_SIZE_IN_KB * 1024 - (fourGigabytes - TdxMetadataGuid - 16))
;
; The VTF signature
;
; VTF-0 means that the VTF (Volume Top File) code does not require
; any fixups.
;
vtfSignature:
    DB      'V', 'T', 'F', 0
------------------------------------------------------------------

The space in ResetVector is very precious and we all want a known location so that QEMU
can find the metadata easily. Putting the metadata in a single file give the developers
more flexible (They can put anything they want). So I think a pointer (point to a metadata
file) in a known location maybe a better solution.

Thanks!

> -----Original Message-----
> From: Laszlo Ersek <lersek@redhat.com>
> Sent: Tuesday, April 6, 2021 8:17 PM
> To: Xu, Min M <min.m.xu@intel.com>; Brijesh Singh
> <brijesh.singh@amd.com>; devel@edk2.groups.io
> Cc: James Bottomley <jejb@linux.ibm.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>
> Subject: Re: [RFC PATCH 01/19] OvmfPkg: Reserve the Secrets and Cpuid page
> for the SEV-SNP guest
> 
> On 04/06/21 10:11, Xu, Min M wrote:
> > Hi, Singh
> > I have a concern about the sevSnpBlock in ResetVectorVtf0.asm.
> > Actually SEV has inserted 3 blocks in ResetVectorVtf0.asm and the
> > total bytes are
> > (26 + 22 + 20 = 68 bytes). If sevSnpBlock is added, then the total
> > bytes will be (68 +26 = 94 bytes).
> >
> > I am not sure whether there will be more blocks added in
> > ResetVectorVtf0.asm in the future. But I don't think
> > ResetVectorVtf0.asm is a good place to add these data blobs. Can these
> > data be packed into a single file, for example, SevMetadata.asm, then
> > a pointer is inserted in ResetVectorVtf0.asm which then points to the
> > SevMetadata. In this way we can keep ResetVectorVtf0.asm clean, small
> and straight forward.
> >
> > Another reason is that I am working on the Intel TDX which will update
> > the ResetVectorVtf0.asm as well. My change depends on the assumption
> > that the distance between ResetVector(0xfffffff0) and
> > EarlyBspInitReal16 is less than 128 bytes. The blocks in
> ResetVectorVtf0.asm make it impossible.
> 
> That's a problem. These info blocks are placed in the reset vector because
> then they can be found by QEMU easily -- they are not compressed, and they
> appear at a known location in the guest physical address space. (More
> precisely, a GUID-ed structure chain starts at a known location, and then
> QEMU can traverse the chain of structures, for learning various bits of
> information about the firmware.)
> 
> Do we absolutely need a short jump?
> 
> Thanks
> Laszlo
> 
> >
> > Thanks!
> >
> >> -----Original Message-----
> >> From: Brijesh Singh <brijesh.singh@amd.com>
> >> Sent: Wednesday, March 24, 2021 11:32 PM
> >> To: devel@edk2.groups.io
> >> Cc: Brijesh Singh <brijesh.singh@amd.com>; 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>; Laszlo Ersek <lersek@redhat.com>
> >> Subject: [RFC PATCH 01/19] OvmfPkg: Reserve the Secrets and Cpuid
> >> page for the SEV-SNP guest
> >>
> >> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3275
> >>
> >> During the SEV-SNP guest launch sequence, two special pages need to
> >> be inserted, the secrets page and cpuid page. The secrets page,
> >> contain the VM platform communication keys. The guest BIOS and OS can
> >> use this key to communicate with the SEV firmware to get the
> >> attestation report. The Cpuid page, contain the CPUIDs entries filtered
> through the AMD-SEV firmware.
> >>
> >> The VMM will locate the secrets and cpuid page addresses through a
> >> fixed GUID and pass them to SEV firmware to populate further.
> >> For more information about the page content, see the SEV-SNP spec.
> >>
> >> To simplify the pre-validation range calculation in the next patch,
> >> the CPUID and Secrets pages are moved to the start of the
> MEMFD_BASE_ADDRESS.
> >>
> >> 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>
> >> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> >> ---
> >>  OvmfPkg/OvmfPkg.dec                          |  8 +++++++
> >>  OvmfPkg/OvmfPkgX64.fdf                       | 24 ++++++++++++--------
> >>  OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm | 19 ++++++++++++++++
> >>  OvmfPkg/ResetVector/ResetVector.inf          |  4 ++++
> >>  OvmfPkg/ResetVector/ResetVector.nasmb        |  2 ++
> >>  5 files changed, 48 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec index
> >> 4348bb45c6..062926772d 100644
> >> --- a/OvmfPkg/OvmfPkg.dec
> >> +++ b/OvmfPkg/OvmfPkg.dec
> >> @@ -317,6 +317,14 @@
> >>
> gUefiOvmfPkgTokenSpaceGuid.PcdSevLaunchSecretBase|0x0|UINT32|0x42
> >>
> gUefiOvmfPkgTokenSpaceGuid.PcdSevLaunchSecretSize|0x0|UINT32|0x43
> >>
> >> +  ## The base address of the CPUID page used by SEV-SNP
> >> +  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpCpuidBase|0|UINT32|0x48
> >> +  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpCpuidSize|0|UINT32|0x49
> >> +
> >> +  ## The base address of the Secrets page used by SEV-SNP
> >> +
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsBase|0|UINT32|0x50
> >> +  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsSize|0|UINT32|0x51
> >> +
> >>  [PcdsDynamic, PcdsDynamicEx]
> >>    gUefiOvmfPkgTokenSpaceGuid.PcdEmuVariableEvent|0|UINT64|2
> >>
> >>
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashVariablesEnable|FALSE|BOOLE
> AN
> >> |0x10
> >> diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf index
> >> d519f85328..ea214600be 100644
> >> --- a/OvmfPkg/OvmfPkgX64.fdf
> >> +++ b/OvmfPkg/OvmfPkgX64.fdf
> >> @@ -67,27 +67,33 @@ ErasePolarity = 1
> >>  BlockSize     = 0x10000
> >>  NumBlocks     = 0xD0
> >>
> >> -0x000000|0x006000
> >> +0x000000|0x001000
> >>
> +gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpCpuidBase|gUefiOvmfPkgToken
> S
> >> paceGu
> >> +id.PcdOvmfSnpCpuidSize
> >> +
> >> +0x001000|0x001000
> >>
> +gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsBase|gUefiOvmfPkgToke
> n
> >> Space
> >> +Guid.PcdOvmfSnpSecretsSize
> >> +
> >> +0x002000|0x006000
> >>
> >>
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesBase|gUefiOvmfPkgTo
> k
> >> enSpaceGuid.PcdOvmfSecPageTablesSize
> >>
> >> -0x006000|0x001000
> >> +0x008000|0x001000
> >>
> >>
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageBase|gUefiOvmfPkgT
> o
> >> kenSpaceGuid.PcdOvmfLockBoxStorageSize
> >>
> >> -0x007000|0x001000
> >> +0x009000|0x001000
> >>
> >>
> gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress|gUefiOv
> m
> >> fPkgTokenSpaceGuid.PcdGuidedExtractHandlerTableSize
> >>
> >> -0x008000|0x001000
> >> +0x00A000|0x001000
> >>
> >>
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbPageTableBase|gUefiOvmfP
> kg
> >> TokenSpaceGuid.PcdOvmfSecGhcbPageTableSize
> >>
> >> -0x009000|0x002000
> >> +0x00B000|0x002000
> >>
> >>
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBase|gUefiOvmfPkgTokenSp
> a
> >> ceGuid.PcdOvmfSecGhcbSize
> >>
> >> -0x00B000|0x001000
> >> -
> >>
> gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase|gUefiCpuPkgTokenSpa
> c
> >> eGuid.PcdSevEsWorkAreaSize
> >> -
> >> -0x00C000|0x001000
> >> +0x00D000|0x001000
> >>
> >>
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBackupBase|gUefiOvmfPkgT
> o
> >> kenSpaceGuid.PcdOvmfSecGhcbBackupSize
> >>
> >> +0x00F000|0x001000
> >>
> +gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase|gUefiCpuPkgTokenSp
> a
> >> ceGui
> >> +d.PcdSevEsWorkAreaSize
> >> +
> >>  0x010000|0x010000
> >>
> >>
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamBase|gUefiOvmfPkg
> To
> >> kenSpaceGuid.PcdOvmfSecPeiTempRamSize
> >>
> >> diff --git a/OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm
> >> b/OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm
> >> index 9c0b5853a4..5456f02924 100644
> >> --- a/OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm
> >> +++ b/OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm
> >> @@ -47,6 +47,25 @@ TIMES (15 - ((guidedStructureEnd -
> >> guidedStructureStart
> >> + 15) % 16)) DB 0  ;
> >>  guidedStructureStart:
> >>
> >> +;
> >> +; SEV-SNP boot support
> >> +;
> >> +; sevSnpBlock:
> >> +;   For the initial boot of SEV-SNP guest, a Secrets and CPUID page must
> be
> >> +;   reserved by the BIOS at a RAM area defined by
> SEV_SNP_SECRETS_PAGE
> >> +;   and SEV_SNP_CPUID_PAGE. A VMM will locate this information using
> the
> >> +;   SEV-SNP boot block.
> >> +;
> >> +; GUID (SEV-SNP boot block): bd39c0c2-2f8e-4243-83e8-1b74cebcb7d9
> >> +;
> >> +sevSnpBootBlockStart:
> >> +    DD      SEV_SNP_SECRETS_PAGE
> >> +    DD      SEV_SNP_CPUID_PAGE
> >> +    DW      sevSnpBootBlockEnd - sevSnpBootBlockStart
> >> +    DB      0xC2, 0xC0, 0x39, 0xBD, 0x8e, 0x2F, 0x43, 0x42
> >> +    DB      0x83, 0xE8, 0x1B, 0x74, 0xCE, 0xBC, 0xB7, 0xD9
> >> +sevSnpBootBlockEnd:
> >> +
> >>  ;
> >>  ; SEV Secret block
> >>  ;
> >> diff --git a/OvmfPkg/ResetVector/ResetVector.inf
> >> b/OvmfPkg/ResetVector/ResetVector.inf
> >> index dc38f68919..d890bb6b29 100644
> >> --- a/OvmfPkg/ResetVector/ResetVector.inf
> >> +++ b/OvmfPkg/ResetVector/ResetVector.inf
> >> @@ -37,6 +37,10 @@
> >>    gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase
> >>    gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBase
> >>    gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbSize
> >> +  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpCpuidBase
> >> +  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpCpuidSize
> >> +  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsBase
> >> +  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsSize
> >>    gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbPageTableBase
> >>    gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbPageTableSize
> >>    gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesBase
> >> diff --git a/OvmfPkg/ResetVector/ResetVector.nasmb
> >> b/OvmfPkg/ResetVector/ResetVector.nasmb
> >> index 5fbacaed5f..2c194958f4 100644
> >> --- a/OvmfPkg/ResetVector/ResetVector.nasmb
> >> +++ b/OvmfPkg/ResetVector/ResetVector.nasmb
> >> @@ -75,6 +75,8 @@
> >>    %define SEV_ES_WORK_AREA (FixedPcdGet32 (PcdSevEsWorkAreaBase))
> >>    %define SEV_ES_WORK_AREA_RDRAND (FixedPcdGet32
> >> (PcdSevEsWorkAreaBase) + 8)
> >>    %define SEV_ES_WORK_AREA_ENC_MASK (FixedPcdGet32
> >> (PcdSevEsWorkAreaBase) + 16)
> >> +  %define SEV_SNP_SECRETS_PAGE FixedPcdGet32
> (PcdOvmfSnpSecretsBase)
> >> + %define SEV_SNP_CPUID_PAGE  FixedPcdGet32 (PcdOvmfSnpCpuidBase)
> >>    %define SEV_ES_VC_TOP_OF_STACK (FixedPcdGet32
> >> (PcdOvmfSecPeiTempRamBase) + FixedPcdGet32
> >> (PcdOvmfSecPeiTempRamSize))  %include "Ia32/Flat32ToFlat64.asm"
> >>  %include "Ia32/PageTables64.asm"
> >> --
> >> 2.17.1
> >