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|BOOLEAN|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|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpCpuidSize
+
+0x001000|0x001000
+gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsSize
+
+0x002000|0x006000
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesSize
-0x006000|0x001000
+0x008000|0x001000
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageSize
-0x007000|0x001000
+0x009000|0x001000
gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress|gUefiOvmfPkgTokenSpaceGuid.PcdGuidedExtractHandlerTableSize
-0x008000|0x001000
+0x00A000|0x001000
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbPageTableBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbPageTableSize
-0x009000|0x002000
+0x00B000|0x002000
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbSize
-0x00B000|0x001000
-gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase|gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaSize
-
-0x00C000|0x001000
+0x00D000|0x001000
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBackupBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBackupSize
+0x00F000|0x001000
+gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase|gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaSize
+
0x010000|0x010000
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamBase|gUefiOvmfPkgTokenSpaceGuid.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
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#73217): https://edk2.groups.io/g/devel/message/73217
Mute This Topic: https://groups.io/mt/81584577/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
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. 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|BOOLEAN > |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|gUefiOvmfPkgTokenS > paceGu > +id.PcdOvmfSnpCpuidSize > + > +0x001000|0x001000 > +gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsBase|gUefiOvmfPkgToken > Space > +Guid.PcdOvmfSnpSecretsSize > + > +0x002000|0x006000 > > gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesBase|gUefiOvmfPkgTok > enSpaceGuid.PcdOvmfSecPageTablesSize > > -0x006000|0x001000 > +0x008000|0x001000 > > gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageBase|gUefiOvmfPkgTo > kenSpaceGuid.PcdOvmfLockBoxStorageSize > > -0x007000|0x001000 > +0x009000|0x001000 > > gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress|gUefiOvm > fPkgTokenSpaceGuid.PcdGuidedExtractHandlerTableSize > > -0x008000|0x001000 > +0x00A000|0x001000 > > gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbPageTableBase|gUefiOvmfPkg > TokenSpaceGuid.PcdOvmfSecGhcbPageTableSize > > -0x009000|0x002000 > +0x00B000|0x002000 > > gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBase|gUefiOvmfPkgTokenSpa > ceGuid.PcdOvmfSecGhcbSize > > -0x00B000|0x001000 > - > gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase|gUefiCpuPkgTokenSpac > eGuid.PcdSevEsWorkAreaSize > - > -0x00C000|0x001000 > +0x00D000|0x001000 > > gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBackupBase|gUefiOvmfPkgTo > kenSpaceGuid.PcdOvmfSecGhcbBackupSize > > +0x00F000|0x001000 > +gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase|gUefiCpuPkgTokenSpa > ceGui > +d.PcdSevEsWorkAreaSize > + > 0x010000|0x010000 > > gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamBase|gUefiOvmfPkgTo > 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 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#73712): https://edk2.groups.io/g/devel/message/73712 Mute This Topic: https://groups.io/mt/81584577/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
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|BOOLEAN >> |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|gUefiOvmfPkgTokenS >> paceGu >> +id.PcdOvmfSnpCpuidSize >> + >> +0x001000|0x001000 >> +gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsBase|gUefiOvmfPkgToken >> Space >> +Guid.PcdOvmfSnpSecretsSize >> + >> +0x002000|0x006000 >> >> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesBase|gUefiOvmfPkgTok >> enSpaceGuid.PcdOvmfSecPageTablesSize >> >> -0x006000|0x001000 >> +0x008000|0x001000 >> >> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageBase|gUefiOvmfPkgTo >> kenSpaceGuid.PcdOvmfLockBoxStorageSize >> >> -0x007000|0x001000 >> +0x009000|0x001000 >> >> gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress|gUefiOvm >> fPkgTokenSpaceGuid.PcdGuidedExtractHandlerTableSize >> >> -0x008000|0x001000 >> +0x00A000|0x001000 >> >> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbPageTableBase|gUefiOvmfPkg >> TokenSpaceGuid.PcdOvmfSecGhcbPageTableSize >> >> -0x009000|0x002000 >> +0x00B000|0x002000 >> >> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBase|gUefiOvmfPkgTokenSpa >> ceGuid.PcdOvmfSecGhcbSize >> >> -0x00B000|0x001000 >> - >> gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase|gUefiCpuPkgTokenSpac >> eGuid.PcdSevEsWorkAreaSize >> - >> -0x00C000|0x001000 >> +0x00D000|0x001000 >> >> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBackupBase|gUefiOvmfPkgTo >> kenSpaceGuid.PcdOvmfSecGhcbBackupSize >> >> +0x00F000|0x001000 >> +gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase|gUefiCpuPkgTokenSpa >> ceGui >> +d.PcdSevEsWorkAreaSize >> + >> 0x010000|0x010000 >> >> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamBase|gUefiOvmfPkgTo >> 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 > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#73723): https://edk2.groups.io/g/devel/message/73723 Mute This Topic: https://groups.io/mt/81584577/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
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
> >
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#73748): https://edk2.groups.io/g/devel/message/73748
Mute This Topic: https://groups.io/mt/81584577/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
On Wed, 2021-04-07 at 00:21 +0000, Xu, Min M wrote:
> 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
Well, then use the rel8 jump like the compiler would in this situation:
smsw ax
test al, 1
jz 1f
jmp EarlyBspPmEntry
1:
jmp EarlyBspInitReal16
So now both entries can be 32k away.
James
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#73750): https://edk2.groups.io/g/devel/message/73750
Mute This Topic: https://groups.io/mt/81584577/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
On 04/07/21 02:44, James Bottomley wrote:
> On Wed, 2021-04-07 at 00:21 +0000, Xu, Min M wrote:
>> 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
>
> Well, then use the rel8 jump like the compiler would in this situation:
>
> smsw ax
> test al, 1
> jz 1f
> jmp EarlyBspPmEntry
> 1:
> jmp EarlyBspInitReal16
>
> So now both entries can be 32k away.
The problem is that we need NASM to generate such *shared* entry code
that behaves correctly when executed in either 16-bit or 32-bit mode.
The rel8 near jumps ("short jumps") are like that -- for example, the
"74 cb" opcode decodes to the same "JZ rel8" in both modes.
But the rel16 ("non-short") near jumps turn into rel32 near jumps when
decoded in 32-bit mode. For example, "E9 cw" decodes to "JMP rel16" in
16-bit mode, but it gets parsed as "E9 cd" (= "JMP rel32") in 32-bit mode.
So the idea is to add more BITS directives, for covering the non-short
near jumps themselves:
> ; instructions up to and including the rel8 JZ decode identically
> ; between BITS 16 and BITS 32
> BITS 16
> smsw ax
> test al, 1
> jz Real
>
> ; the unconditional near jumps are mode-specific
> BITS 32
> jmp near EarlyBspPmEntry
> BITS 16
> Real:
> jmp near EarlyBspInitReal16
>
> ; --------------------
>
> BITS 16
> EarlyBspInitReal16:
> nop
>
> BITS 32
> EarlyBspPmEntry:
> nop
$ nasm -f bin jz.nasmb
Decoded (executed) in 16-bit mode:
$ ndisasm -b 16 -k 7,5 -k 0x10,1 jz
00000000 0F01E0 smsw ax
00000003 A801 test al,0x1
00000005 7405 jz 0xc ; taken
00000007 skipping 0x5 bytes
0000000C E90000 jmp word 0xf
0000000F 90 nop
00000010 skipping 0x1 bytes
Decoded (executed) in 32-bit mode:
$ ndisasm -b 32 -k 0xc,4 jz
00000000 0F01E0 smsw eax
00000003 A801 test al,0x1
00000005 7405 jz 0xc ; not taken
00000007 E904000000 jmp dword 0x10
0000000C skipping 0x4 bytes
00000010 90 nop
With the garbage *not* hidden:
$ ndisasm -b 16 -s 0xc jz
00000000 0F01E0 smsw ax
00000003 A801 test al,0x1
00000005 7405 jz 0xc ; taken
00000007 E90400 jmp word 0xe ; garbage
0000000A 0000 add [bx+si],al ; garbage
0000000C E90000 jmp word 0xf
0000000F 90 nop
00000010 90 nop ; garbage
$ ndisasm -b 32 -s 0x10 jz
00000000 0F01E0 smsw eax
00000003 A801 test al,0x1
00000005 7405 jz 0xc ; not taken
00000007 E904000000 jmp dword 0x10
0000000C E9 db 0xe9 ; garbage
0000000D 0000 add [eax],al ; garbage
0000000F 90 nop ; garbage
00000010 90 nop
Thanks
Laszlo
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#73785): https://edk2.groups.io/g/devel/message/73785
Mute This Topic: https://groups.io/mt/81584577/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
On Wed, 2021-04-07 at 17:02 +0200, Laszlo Ersek wrote:
> On 04/07/21 02:44, James Bottomley wrote:
> > On Wed, 2021-04-07 at 00:21 +0000, Xu, Min M wrote:
> > > 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
> >
> > Well, then use the rel8 jump like the compiler would in this
> > situation:
> >
> > smsw ax
> > test al, 1
> > jz 1f
> > jmp EarlyBspPmEntry
> > 1:
> > jmp EarlyBspInitReal16
> >
> > So now both entries can be 32k away.
>
> The problem is that we need NASM to generate such *shared* entry code
> that behaves correctly when executed in either 16-bit or 32-bit mode.
>
> The rel8 near jumps ("short jumps") are like that -- for example, the
> "74 cb" opcode decodes to the same "JZ rel8" in both modes.
>
> But the rel16 ("non-short") near jumps turn into rel32 near jumps
> when decoded in 32-bit mode. For example, "E9 cw" decodes to "JMP
> rel16" in 16-bit mode, but it gets parsed as "E9 cd" (= "JMP rel32")
> in 32-bit mode.
>
> So the idea is to add more BITS directives, for covering the non-
> short near jumps themselves:
Absolutely ... sorry, I should have said this was just the first thing
I thought of. The key point is don't do a rel8 jump over the guid
table, use the rel8 jump within the reset vector to sort out the final
destination and use the wider jumps to go over the guid table. As you
say, we have to be very careful about the wider jumps given the
differences between the entry modes.
James
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#73786): https://edk2.groups.io/g/devel/message/73786
Mute This Topic: https://groups.io/mt/81584577/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
On Wednesday, April 7, 2021 11:03 PM, Laszlo wrote:
> On 04/07/21 02:44, James Bottomley wrote:
> > On Wed, 2021-04-07 at 00:21 +0000, Xu, Min M wrote:
> >> 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
> >
> > Well, then use the rel8 jump like the compiler would in this situation:
> >
> > smsw ax
> > test al, 1
> > jz 1f
> > jmp EarlyBspPmEntry
> > 1:
> > jmp EarlyBspInitReal16
> >
> > So now both entries can be 32k away.
>
> The problem is that we need NASM to generate such *shared* entry code that
> behaves correctly when executed in either 16-bit or 32-bit mode.
>
> The rel8 near jumps ("short jumps") are like that -- for example, the
> "74 cb" opcode decodes to the same "JZ rel8" in both modes.
>
> But the rel16 ("non-short") near jumps turn into rel32 near jumps when
> decoded in 32-bit mode. For example, "E9 cw" decodes to "JMP rel16" in 16-bit
> mode, but it gets parsed as "E9 cd" (= "JMP rel32") in 32-bit mode.
>
> So the idea is to add more BITS directives, for covering the non-short near
> jumps themselves:
Yes this is the root cause. TDX requires the startup mode to be 32-bit
protected mode while the legacy VM startup mode is 16-bit real mode.
Add more BITS directives can work round this. I have tried it and it works.
So my initial solution is to use *jmp rel8* because it works in both 16-bit
and 32-bit mode. But *jmp rel8* depends on the distance which should
be less than 128 bytes. If more metadata is added in the ResetVector.asm
then we have to use the BITS solution.
>
> > ; instructions up to and including the rel8 JZ decode identically ;
> > between BITS 16 and BITS 32 BITS 16
> > smsw ax
> > test al, 1
> > jz Real
> >
> > ; the unconditional near jumps are mode-specific BITS 32
> > jmp near EarlyBspPmEntry
> > BITS 16
> > Real:
> > jmp near EarlyBspInitReal16
> >
> > ; --------------------
> >
> > BITS 16
> > EarlyBspInitReal16:
> > nop
> >
> > BITS 32
> > EarlyBspPmEntry:
> > nop
>
> $ nasm -f bin jz.nasmb
>
> Decoded (executed) in 16-bit mode:
>
> $ ndisasm -b 16 -k 7,5 -k 0x10,1 jz
> 00000000 0F01E0 smsw ax
> 00000003 A801 test al,0x1
> 00000005 7405 jz 0xc ; taken
> 00000007 skipping 0x5 bytes
> 0000000C E90000 jmp word 0xf
> 0000000F 90 nop
> 00000010 skipping 0x1 bytes
>
> Decoded (executed) in 32-bit mode:
>
> $ ndisasm -b 32 -k 0xc,4 jz
> 00000000 0F01E0 smsw eax
> 00000003 A801 test al,0x1
> 00000005 7405 jz 0xc ; not taken
> 00000007 E904000000 jmp dword 0x10
> 0000000C skipping 0x4 bytes
> 00000010 90 nop
>
>
> With the garbage *not* hidden:
>
> $ ndisasm -b 16 -s 0xc jz
>
> 00000000 0F01E0 smsw ax
> 00000003 A801 test al,0x1
> 00000005 7405 jz 0xc ; taken
> 00000007 E90400 jmp word 0xe ; garbage
> 0000000A 0000 add [bx+si],al ; garbage
> 0000000C E90000 jmp word 0xf
> 0000000F 90 nop
> 00000010 90 nop ; garbage
>
> $ ndisasm -b 32 -s 0x10 jz
>
> 00000000 0F01E0 smsw eax
> 00000003 A801 test al,0x1
> 00000005 7405 jz 0xc ; not taken
> 00000007 E904000000 jmp dword 0x10
> 0000000C E9 db 0xe9 ; garbage
> 0000000D 0000 add [eax],al ; garbage
> 0000000F 90 nop ; garbage
> 00000010 90 nop
>
> Thanks
> Laszlo
>
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#73816): https://edk2.groups.io/g/devel/message/73816
Mute This Topic: https://groups.io/mt/81584577/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
On 4/8/21 1:24 AM, Xu, Min M wrote:
> On Wednesday, April 7, 2021 11:03 PM, Laszlo wrote:
>> On 04/07/21 02:44, James Bottomley wrote:
>>> On Wed, 2021-04-07 at 00:21 +0000, Xu, Min M wrote:
>>>> 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
>>>
>>> Well, then use the rel8 jump like the compiler would in this situation:
>>>
>>> smsw ax
>>> test al, 1
>>> jz 1f
>>> jmp EarlyBspPmEntry
>>> 1:
>>> jmp EarlyBspInitReal16
>>>
>>> So now both entries can be 32k away.
>>
>> The problem is that we need NASM to generate such *shared* entry code that
>> behaves correctly when executed in either 16-bit or 32-bit mode.
>>
>> The rel8 near jumps ("short jumps") are like that -- for example, the
>> "74 cb" opcode decodes to the same "JZ rel8" in both modes.
>>
>> But the rel16 ("non-short") near jumps turn into rel32 near jumps when
>> decoded in 32-bit mode. For example, "E9 cw" decodes to "JMP rel16" in 16-bit
>> mode, but it gets parsed as "E9 cd" (= "JMP rel32") in 32-bit mode.
>>
>> So the idea is to add more BITS directives, for covering the non-short near
>> jumps themselves:
>
> Yes this is the root cause. TDX requires the startup mode to be 32-bit
> protected mode while the legacy VM startup mode is 16-bit real mode.
> Add more BITS directives can work round this. I have tried it and it works.
>
> So my initial solution is to use *jmp rel8* because it works in both 16-bit
> and 32-bit mode. But *jmp rel8* depends on the distance which should
> be less than 128 bytes. If more metadata is added in the ResetVector.asm
> then we have to use the BITS solution.
To me, it sounds like the BITS solution should be the approach you use
from the start.
Thanks,
Tom
>
>>
>>> ; instructions up to and including the rel8 JZ decode identically ;
>>> between BITS 16 and BITS 32 BITS 16
>>> smsw ax
>>> test al, 1
>>> jz Real
>>>
>>> ; the unconditional near jumps are mode-specific BITS 32
>>> jmp near EarlyBspPmEntry
>>> BITS 16
>>> Real:
>>> jmp near EarlyBspInitReal16
>>>
>>> ; --------------------
>>>
>>> BITS 16
>>> EarlyBspInitReal16:
>>> nop
>>>
>>> BITS 32
>>> EarlyBspPmEntry:
>>> nop
>>
>> $ nasm -f bin jz.nasmb
>>
>> Decoded (executed) in 16-bit mode:
>>
>> $ ndisasm -b 16 -k 7,5 -k 0x10,1 jz
>> 00000000 0F01E0 smsw ax
>> 00000003 A801 test al,0x1
>> 00000005 7405 jz 0xc ; taken
>> 00000007 skipping 0x5 bytes
>> 0000000C E90000 jmp word 0xf
>> 0000000F 90 nop
>> 00000010 skipping 0x1 bytes
>>
>> Decoded (executed) in 32-bit mode:
>>
>> $ ndisasm -b 32 -k 0xc,4 jz
>> 00000000 0F01E0 smsw eax
>> 00000003 A801 test al,0x1
>> 00000005 7405 jz 0xc ; not taken
>> 00000007 E904000000 jmp dword 0x10
>> 0000000C skipping 0x4 bytes
>> 00000010 90 nop
>>
>>
>> With the garbage *not* hidden:
>>
>> $ ndisasm -b 16 -s 0xc jz
>>
>> 00000000 0F01E0 smsw ax
>> 00000003 A801 test al,0x1
>> 00000005 7405 jz 0xc ; taken
>> 00000007 E90400 jmp word 0xe ; garbage
>> 0000000A 0000 add [bx+si],al ; garbage
>> 0000000C E90000 jmp word 0xf
>> 0000000F 90 nop
>> 00000010 90 nop ; garbage
>>
>> $ ndisasm -b 32 -s 0x10 jz
>>
>> 00000000 0F01E0 smsw eax
>> 00000003 A801 test al,0x1
>> 00000005 7405 jz 0xc ; not taken
>> 00000007 E904000000 jmp dword 0x10
>> 0000000C E9 db 0xe9 ; garbage
>> 0000000D 0000 add [eax],al ; garbage
>> 0000000F 90 nop ; garbage
>> 00000010 90 nop
>>
>> Thanks
>> Laszlo
>>
>>
>>
>>
>>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#73850): https://edk2.groups.io/g/devel/message/73850
Mute This Topic: https://groups.io/mt/81584577/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
On 04/08/21 15:31, Tom Lendacky wrote:
> On 4/8/21 1:24 AM, Xu, Min M wrote:
>> On Wednesday, April 7, 2021 11:03 PM, Laszlo wrote:
>>> On 04/07/21 02:44, James Bottomley wrote:
>>>> On Wed, 2021-04-07 at 00:21 +0000, Xu, Min M wrote:
>>>>> 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
>>>>
>>>> Well, then use the rel8 jump like the compiler would in this situation:
>>>>
>>>> smsw ax
>>>> test al, 1
>>>> jz 1f
>>>> jmp EarlyBspPmEntry
>>>> 1:
>>>> jmp EarlyBspInitReal16
>>>>
>>>> So now both entries can be 32k away.
>>>
>>> The problem is that we need NASM to generate such *shared* entry code that
>>> behaves correctly when executed in either 16-bit or 32-bit mode.
>>>
>>> The rel8 near jumps ("short jumps") are like that -- for example, the
>>> "74 cb" opcode decodes to the same "JZ rel8" in both modes.
>>>
>>> But the rel16 ("non-short") near jumps turn into rel32 near jumps when
>>> decoded in 32-bit mode. For example, "E9 cw" decodes to "JMP rel16" in 16-bit
>>> mode, but it gets parsed as "E9 cd" (= "JMP rel32") in 32-bit mode.
>>>
>>> So the idea is to add more BITS directives, for covering the non-short near
>>> jumps themselves:
>>
>> Yes this is the root cause. TDX requires the startup mode to be 32-bit
>> protected mode while the legacy VM startup mode is 16-bit real mode.
>> Add more BITS directives can work round this. I have tried it and it works.
>>
>> So my initial solution is to use *jmp rel8* because it works in both 16-bit
>> and 32-bit mode. But *jmp rel8* depends on the distance which should
>> be less than 128 bytes. If more metadata is added in the ResetVector.asm
>> then we have to use the BITS solution.
>
> To me, it sounds like the BITS solution should be the approach you use
> from the start.
I agree; we have an extensible approach in place already (with the
GUIDed struct list), and QEMU already knows about one magic GPA (for
locating the head of the list). Using one conditional rel8 jump, and two
non-conditional mode-specific (rel16/rel32) jumps, doesn't cost much in
the assembly code, it's compatible with future GUID additions, and
shouldn't affect QEMU's GUIDed struct list traversal.
Thanks
Laszlo
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#73902): https://edk2.groups.io/g/devel/message/73902
Mute This Topic: https://groups.io/mt/81584577/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Hi Min, On 04/08/21 15:31, Lendacky, Thomas wrote: > On 4/8/21 1:24 AM, Xu, Min M wrote: >> Yes this is the root cause. TDX requires the startup mode to be 32-bit >> protected mode while the legacy VM startup mode is 16-bit real mode. >> Add more BITS directives can work round this. I have tried it and it works. >> >> So my initial solution is to use *jmp rel8* because it works in both 16-bit >> and 32-bit mode. But *jmp rel8* depends on the distance which should >> be less than 128 bytes. If more metadata is added in the ResetVector.asm >> then we have to use the BITS solution. > > To me, it sounds like the BITS solution should be the approach you use > from the start. BTW, have you considered using a separate ResetVector module for TDX? That would obviate this multi-mode trickery. (Most recently raised by Paolo.) I think TDX will need a separate platform DSC / FDF / fw binary anyway. I realize that's not a done deal yet; it may depend on who provides the firmware binary (guest owner or cloud owner) -- of course the guest owner will have to perform the attestation in either case, but the "provenance" question remains open, IIUC. And even if TDX gets its own firmware platform, it's a separate question whether the ResetVector module itself should be split. I'm inclined to think a separation at that level would make development and maintenance easier. (FWIW, Xen PVH has its own reset vector module, in OvmfPkg/XenResetVector.) Thanks Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#73903): https://edk2.groups.io/g/devel/message/73903 Mute This Topic: https://groups.io/mt/81584577/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
Hi Laszlo Thanks. We did provide a separate binary in the beginning - see https://github.com/tianocore/edk2-staging/tree/TDVF, with same goal - easy to maintain and develop. A clean solution, definitely. However, we got requirement to deliver one binary solution together with 1) normal OVMF, 2) AMD-SEV, 3) Intel-TDX. Now, we are struggling to merge them...... For DXE, we hope to isolate TDX driver whenever it is possible. But we only have one reset vector here. Sigh... > -----Original Message----- > From: Laszlo Ersek <lersek@redhat.com> > Sent: Friday, April 9, 2021 9:33 PM > To: Xu, Min M <min.m.xu@intel.com> > Cc: devel@edk2.groups.io; thomas.lendacky@amd.com; jejb@linux.ibm.com; > Brijesh Singh <brijesh.singh@amd.com>; Yao, Jiewen <jiewen.yao@intel.com>; > Justen, Jordan L <jordan.l.justen@intel.com>; Ard Biesheuvel > <ardb+tianocore@kernel.org>; Paolo Bonzini <pbonzini@redhat.com> > Subject: Re: [edk2-devel] [RFC PATCH 01/19] OvmfPkg: Reserve the Secrets and > Cpuid page for the SEV-SNP guest > > Hi Min, > > On 04/08/21 15:31, Lendacky, Thomas wrote: > > On 4/8/21 1:24 AM, Xu, Min M wrote: > > >> Yes this is the root cause. TDX requires the startup mode to be 32-bit > >> protected mode while the legacy VM startup mode is 16-bit real mode. > >> Add more BITS directives can work round this. I have tried it and it works. > >> > >> So my initial solution is to use *jmp rel8* because it works in both 16-bit > >> and 32-bit mode. But *jmp rel8* depends on the distance which should > >> be less than 128 bytes. If more metadata is added in the ResetVector.asm > >> then we have to use the BITS solution. > > > > To me, it sounds like the BITS solution should be the approach you use > > from the start. > > BTW, have you considered using a separate ResetVector module for TDX? > That would obviate this multi-mode trickery. (Most recently raised by > Paolo.) > > I think TDX will need a separate platform DSC / FDF / fw binary anyway. > I realize that's not a done deal yet; it may depend on who provides the > firmware binary (guest owner or cloud owner) -- of course the guest > owner will have to perform the attestation in either case, but the > "provenance" question remains open, IIUC. > > And even if TDX gets its own firmware platform, it's a separate question > whether the ResetVector module itself should be split. I'm inclined to > think a separation at that level would make development and maintenance > easier. > > (FWIW, Xen PVH has its own reset vector module, in OvmfPkg/XenResetVector.) > > Thanks > Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#73905): https://edk2.groups.io/g/devel/message/73905 Mute This Topic: https://groups.io/mt/81584577/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On 04/09/21 15:44, Yao, Jiewen wrote: > Hi Laszlo > Thanks. > > We did provide a separate binary in the beginning - see https://github.com/tianocore/edk2-staging/tree/TDVF, with same goal - easy to maintain and develop. A clean solution, definitely. > > However, we got requirement to deliver one binary solution together with 1) normal OVMF, 2) AMD-SEV, 3) Intel-TDX. > Now, we are struggling to merge them...... > > For DXE, we hope to isolate TDX driver whenever it is possible. > But we only have one reset vector here. Sigh... Can we please pry a little bit at that "one binary" requirement? Ultimately the "guest bundle" is going to be composed by much higher-level code, I expect (such as some userspace code, written in python or similar); selecting a firmware binary in such an environment is surely easier than handling this "polymorphism" in the most restrictive software environment imaginable (reset vector assembly code in the guest)? Thanks Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#73906): https://edk2.groups.io/g/devel/message/73906 Mute This Topic: https://groups.io/mt/81969494/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On 04/07/21 02:21, Xu, Min M wrote: > 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. Assuming a QEMU version has been released that looks for the chain of GUID-ed structs already, then I think such a change would break compatibility with that QEMU version. If we definitely need a separate spot to include more information in the flash, for QEMU's parsing, then please introduce a new GUIDed structure, which contains nothing but a pointer to that spot. Thanks Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#73780): https://edk2.groups.io/g/devel/message/73780 Mute This Topic: https://groups.io/mt/81584577/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On 04/07/21 15:22, Laszlo Ersek wrote: > On 04/07/21 02:21, Xu, Min M wrote: > >> 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. > > Assuming a QEMU version has been released that looks for the chain of > GUID-ed structs already, then I think such a change would break > compatibility with that QEMU version. > > If we definitely need a separate spot to include more information in the > flash, for QEMU's parsing, then please introduce a new GUIDed structure, > which contains nothing but a pointer to that spot. Ah, upon re-reading James's earlier message which he just referenced, namely at https://listman.redhat.com/archives/edk2-devel-archive/2020-November/msg01063.html it's clear that James had already proposed this: > That's not to say we can't have a larger table ... we definitely can, > but it can't be where it is now. we'd have to do something different > like make the table guid describe where the actual table is located > (as a 32 bit offset and length) so as not to break up the 16 bit jump > code. Thanks Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#73781): https://edk2.groups.io/g/devel/message/73781 Mute This Topic: https://groups.io/mt/81584577/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On April 7, 2021 9:23 PM, Laszlo wrote: > > On 04/07/21 02:21, Xu, Min M wrote: > > > 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. > > Assuming a QEMU version has been released that looks for the chain of GUID- > ed structs already, then I think such a change would break compatibility with > that QEMU version. Agree. The existing GUIDed structs should be kept. Otherwise the QEMU version Which has been released would be broken. > > If we definitely need a separate spot to include more information in the flash, > for QEMU's parsing, then please introduce a new GUIDed structure, which > contains nothing but a pointer to that spot. I suggest if new information is to be added in the flash in the future, then we'd better pack the information in a separate spot and place a pointer to that spot in a known location. Intel TDX has metadata too. I believe AMD SEV will add more information in the future. Even ARM may has its metadata. So putting the metadata in separate spot is more friendly and flexible. Since it is a pointer in a known location, for example, 0xffffffe0, then do we still need a GUIDed structure? > > Thanks > Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#73808): https://edk2.groups.io/g/devel/message/73808 Mute This Topic: https://groups.io/mt/81584577/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On Tue, 2021-04-06 at 14:16 +0200, Laszlo Ersek wrote: > 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. Why? I think the short jump can cover 32k as an offset. > 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? Yes, I explained this before: https://listman.redhat.com/archives/edk2-devel-archive/2020-November/msg01063.html Sorry, it's buried in the middle about why we can only have 32k worth of entries. However, the restriction is 32k not 128 bytes unless Intel is doing something strange. James -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#73749): https://edk2.groups.io/g/devel/message/73749 Mute This Topic: https://groups.io/mt/81584577/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
Hi James and Laszlo, I was planning to work to add the support to reserve the Secrets and CPUID page in E820 map and then create the EFI configuration table entry for it so that guest OS can reach to it. We have two packages "SecretDxe" and "SecretPei" in OvmfPkg/AmdSev. Any issues if I use them in the OvmfPkg.dsc ? Here is what I was thinking: 1) Rename the PcdSevLaunchSecretBase -> PcdSevSecretsBase 2) When SNP is enabled then VMM use this page as secrets page for the SNP 3) When SEV or SEV-ES is enabled then VMM uses this page as a launch secret page This will allow me to drop PcdOvmfSnpSecretsBase. This will not just save 4-bytes but also minimize the code duplication. Thoughts ? -Brijesh On 3/24/21 10:31 AM, Brijesh Singh wrote: > 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|BOOLEAN|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|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpCpuidSize > + > +0x001000|0x001000 > +gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsSize > + > +0x002000|0x006000 > gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesSize > > -0x006000|0x001000 > +0x008000|0x001000 > gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageSize > > -0x007000|0x001000 > +0x009000|0x001000 > gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress|gUefiOvmfPkgTokenSpaceGuid.PcdGuidedExtractHandlerTableSize > > -0x008000|0x001000 > +0x00A000|0x001000 > gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbPageTableBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbPageTableSize > > -0x009000|0x002000 > +0x00B000|0x002000 > gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbSize > > -0x00B000|0x001000 > -gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase|gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaSize > - > -0x00C000|0x001000 > +0x00D000|0x001000 > gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBackupBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBackupSize > > +0x00F000|0x001000 > +gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase|gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaSize > + > 0x010000|0x010000 > gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamBase|gUefiOvmfPkgTokenSpaceGuid.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" -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#73961): https://edk2.groups.io/g/devel/message/73961 Mute This Topic: https://groups.io/mt/81584577/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On 04/12/21 16:52, Brijesh Singh wrote: > Hi James and Laszlo, > > I was planning to work to add the support to reserve the Secrets and > CPUID page in E820 map and then create the EFI configuration table entry > for it so that guest OS can reach to it. We have two packages > "SecretDxe" and "SecretPei" in OvmfPkg/AmdSev. Any issues if I use them > in the OvmfPkg.dsc ? Here is what I was thinking: > > 1) Rename the PcdSevLaunchSecretBase -> PcdSevSecretsBase > > 2) When SNP is enabled then VMM use this page as secrets page for the SNP > > 3) When SEV or SEV-ES is enabled then VMM uses this page as a launch > secret page > > This will allow me to drop PcdOvmfSnpSecretsBase. This will not just > save 4-bytes but also minimize the code duplication. I'm pretty unhappy about needing a separate page for each such purpose. We're wasting room in MEMFD. The GUIDed structs that we expose to QEMU seem to be flexible enough to describe non-page-aligned addresses, right? Can we pack larger amounts of cruft into MEMFD pages? I'm not looking forward to the day when we run out of slack in MEMFD and we get to shift PEIFV / DXEFV. (Every time we need to increase the DXEFV size, the same risk exists -- which is why I've been thinking for a while now that OVMF includes too many features already.) This can introduce obscure changes to the UEFI memory map, which has caused compat problems in the past, for example with the "crash" utility. The feature creep in OVMF has gone off the rails in the last few years, really. (Not that I'm not guilty myself.) Thanks, Laszlo > > Thoughts ? > > -Brijesh > > On 3/24/21 10:31 AM, Brijesh Singh wrote: >> 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|BOOLEAN|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|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpCpuidSize >> + >> +0x001000|0x001000 >> +gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsSize >> + >> +0x002000|0x006000 >> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesSize >> >> -0x006000|0x001000 >> +0x008000|0x001000 >> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageSize >> >> -0x007000|0x001000 >> +0x009000|0x001000 >> gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress|gUefiOvmfPkgTokenSpaceGuid.PcdGuidedExtractHandlerTableSize >> >> -0x008000|0x001000 >> +0x00A000|0x001000 >> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbPageTableBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbPageTableSize >> >> -0x009000|0x002000 >> +0x00B000|0x002000 >> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbSize >> >> -0x00B000|0x001000 >> -gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase|gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaSize >> - >> -0x00C000|0x001000 >> +0x00D000|0x001000 >> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBackupBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBackupSize >> >> +0x00F000|0x001000 >> +gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase|gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaSize >> + >> 0x010000|0x010000 >> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamBase|gUefiOvmfPkgTokenSpaceGuid.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" > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#74024): https://edk2.groups.io/g/devel/message/74024 Mute This Topic: https://groups.io/mt/81584577/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On 4/13/21 4:49 AM, Laszlo Ersek wrote: > On 04/12/21 16:52, Brijesh Singh wrote: >> Hi James and Laszlo, >> >> I was planning to work to add the support to reserve the Secrets and >> CPUID page in E820 map and then create the EFI configuration table entry >> for it so that guest OS can reach to it. We have two packages >> "SecretDxe" and "SecretPei" in OvmfPkg/AmdSev. Any issues if I use them >> in the OvmfPkg.dsc ? Here is what I was thinking: >> >> 1) Rename the PcdSevLaunchSecretBase -> PcdSevSecretsBase >> >> 2) When SNP is enabled then VMM use this page as secrets page for the SNP >> >> 3) When SEV or SEV-ES is enabled then VMM uses this page as a launch >> secret page >> >> This will allow me to drop PcdOvmfSnpSecretsBase. This will not just >> save 4-bytes but also minimize the code duplication. > I'm pretty unhappy about needing a separate page for each such purpose. > We're wasting room in MEMFD. The GUIDed structs that we expose to QEMU > seem to be flexible enough to describe non-page-aligned addresses, > right? Can we pack larger amounts of cruft into MEMFD pages? > > I'm not looking forward to the day when we run out of slack in MEMFD and > we get to shift PEIFV / DXEFV. (Every time we need to increase the DXEFV > size, the same risk exists -- which is why I've been thinking for a > while now that OVMF includes too many features already.) This can > introduce obscure changes to the UEFI memory map, which has caused > compat problems in the past, for example with the "crash" utility. What's your take to move all SEV-specific reserved pages at the end of PcdOvmfDecompressionScratchEnd ? I have not tried yet, but I can give try to make sure the ES works after such moves. What is a general rule of thump to what goes in MEMFD ? Is this all the data pages accessed during the SEC phase ? If so, then we probably can't do everything after the PcdOvmfDecompressionScratchEnd. The only thing which we can quickly move out is a secret page. > The feature creep in OVMF has gone off the rails in the last few years, > really. (Not that I'm not guilty myself.) > > Thanks, > Laszlo > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#74282): https://edk2.groups.io/g/devel/message/74282 Mute This Topic: https://groups.io/mt/81584577/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On 04/19/21 23:42, Brijesh Singh wrote: > > On 4/13/21 4:49 AM, Laszlo Ersek wrote: >> On 04/12/21 16:52, Brijesh Singh wrote: >>> Hi James and Laszlo, >>> >>> I was planning to work to add the support to reserve the Secrets and >>> CPUID page in E820 map and then create the EFI configuration table entry >>> for it so that guest OS can reach to it. We have two packages >>> "SecretDxe" and "SecretPei" in OvmfPkg/AmdSev. Any issues if I use them >>> in the OvmfPkg.dsc ? Here is what I was thinking: >>> >>> 1) Rename the PcdSevLaunchSecretBase -> PcdSevSecretsBase >>> >>> 2) When SNP is enabled then VMM use this page as secrets page for the SNP >>> >>> 3) When SEV or SEV-ES is enabled then VMM uses this page as a launch >>> secret page >>> >>> This will allow me to drop PcdOvmfSnpSecretsBase. This will not just >>> save 4-bytes but also minimize the code duplication. >> I'm pretty unhappy about needing a separate page for each such purpose. >> We're wasting room in MEMFD. The GUIDed structs that we expose to QEMU >> seem to be flexible enough to describe non-page-aligned addresses, >> right? Can we pack larger amounts of cruft into MEMFD pages? >> >> I'm not looking forward to the day when we run out of slack in MEMFD and >> we get to shift PEIFV / DXEFV. (Every time we need to increase the DXEFV >> size, the same risk exists -- which is why I've been thinking for a >> while now that OVMF includes too many features already.) This can >> introduce obscure changes to the UEFI memory map, which has caused >> compat problems in the past, for example with the "crash" utility. > > > What's your take to move all SEV-specific reserved pages at the end of > PcdOvmfDecompressionScratchEnd ? I have not tried yet, but I can give > try to make sure the ES works after such moves. I don't recommend that, without a serious audit anyway. While we use PCDs in the source code rather than open-coded constants, and so it's relatively easy to change the value of a PCD, the source code may very well rely on the *relations* between PCDs. If you change PCDs such that their relations change (by moving around areas), things can break. If you absolutely need more room, I'd prefer enlarging the currently available gap in-place (pushing up everything that's currently above it). > What is a general rule of thump to what goes in MEMFD ? Is this all the > data pages accessed during the SEC phase ? More or less; it lets us lay out ranges, with symbolic names, that are needed very early (reset vector, SEC), during normal boot or ACPI S3 resume. If something can be allocated manually during PEI or later, yielding a variable address, then it arguably doesn't beling on MEMFD. > If so, then we probably can't > do everything after the PcdOvmfDecompressionScratchEnd. The only thing > which we can quickly move out is a secret page. IIRC, PcdOvmfDecompressionScratchEnd stands for the highest RAM address overwritten during ACPI S3 resume. I wouldn't like to put anything above it through MEMFD. Thanks Laszlo > > >> The feature creep in OVMF has gone off the rails in the last few years, >> really. (Not that I'm not guilty myself.) >> >> Thanks, >> Laszlo >> > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#74295): https://edk2.groups.io/g/devel/message/74295 Mute This Topic: https://groups.io/mt/81584577/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On 4/13/21 4:49 AM, Laszlo Ersek wrote: > On 04/12/21 16:52, Brijesh Singh wrote: >> Hi James and Laszlo, >> >> I was planning to work to add the support to reserve the Secrets and >> CPUID page in E820 map and then create the EFI configuration table entry >> for it so that guest OS can reach to it. We have two packages >> "SecretDxe" and "SecretPei" in OvmfPkg/AmdSev. Any issues if I use them >> in the OvmfPkg.dsc ? Here is what I was thinking: >> >> 1) Rename the PcdSevLaunchSecretBase -> PcdSevSecretsBase >> >> 2) When SNP is enabled then VMM use this page as secrets page for the SNP >> >> 3) When SEV or SEV-ES is enabled then VMM uses this page as a launch >> secret page >> >> This will allow me to drop PcdOvmfSnpSecretsBase. This will not just >> save 4-bytes but also minimize the code duplication. > I'm pretty unhappy about needing a separate page for each such purpose. > We're wasting room in MEMFD. The GUIDed structs that we expose to QEMU > seem to be flexible enough to describe non-page-aligned addresses, > right? Can we pack larger amounts of cruft into MEMFD pages? With the GUID approach we should be able to pack multiple fields into a page but unfortunately in the case of SEV-SNP both the CPUID and Secrets need to be a page size. Without SNP support the we reserve the following page for the SEV/SEV-ES: 1 page for Launch Secret. 2 pages for the GHCB 1 page for EsWorkArea Both the EsWorkArea and LaunchSecret does not need to be the page aligned or page sized. Since the SNP needs a full page for the secrets so I was inclined to use the same secrets page for both SEV and SNP. At the end all we need to do is reserve one extra page for CPUID to make the SNP work. In future the EsWorkArea page can be used to pack additional information without needed to reserve full page (if feature does not require page). > > I'm not looking forward to the day when we run out of slack in MEMFD and > we get to shift PEIFV / DXEFV. (Every time we need to increase the DXEFV > size, the same risk exists -- which is why I've been thinking for a > while now that OVMF includes too many features already.) This can > introduce obscure changes to the UEFI memory map, which has caused > compat problems in the past, for example with the "crash" utility. > > The feature creep in OVMF has gone off the rails in the last few years, > really. (Not that I'm not guilty myself.) > > Thanks, > Laszlo > >> Thoughts ? >> >> -Brijesh >> >> On 3/24/21 10:31 AM, 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%7C04be68371db9458cbdc108d8fe61713a%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637539041773579324%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=J62%2BVSCZRawAkzsi9xtS43cpowZxSCx%2BwcDYNwdF3qA%3D&reserved=0 >>> >>> 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|BOOLEAN|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|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpCpuidSize >>> + >>> +0x001000|0x001000 >>> +gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsSize >>> + >>> +0x002000|0x006000 >>> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesSize >>> >>> -0x006000|0x001000 >>> +0x008000|0x001000 >>> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageSize >>> >>> -0x007000|0x001000 >>> +0x009000|0x001000 >>> gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress|gUefiOvmfPkgTokenSpaceGuid.PcdGuidedExtractHandlerTableSize >>> >>> -0x008000|0x001000 >>> +0x00A000|0x001000 >>> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbPageTableBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbPageTableSize >>> >>> -0x009000|0x002000 >>> +0x00B000|0x002000 >>> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbSize >>> >>> -0x00B000|0x001000 >>> -gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase|gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaSize >>> - >>> -0x00C000|0x001000 >>> +0x00D000|0x001000 >>> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBackupBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBackupSize >>> >>> +0x00F000|0x001000 >>> +gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase|gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaSize >>> + >>> 0x010000|0x010000 >>> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamBase|gUefiOvmfPkgTokenSpaceGuid.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" -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#74034): https://edk2.groups.io/g/devel/message/74034 Mute This Topic: https://groups.io/mt/81584577/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On 04/13/21 13:29, Brijesh Singh wrote: > > On 4/13/21 4:49 AM, Laszlo Ersek wrote: >> On 04/12/21 16:52, Brijesh Singh wrote: >>> Hi James and Laszlo, >>> >>> I was planning to work to add the support to reserve the Secrets and >>> CPUID page in E820 map and then create the EFI configuration table entry >>> for it so that guest OS can reach to it. We have two packages >>> "SecretDxe" and "SecretPei" in OvmfPkg/AmdSev. Any issues if I use them >>> in the OvmfPkg.dsc ? Here is what I was thinking: >>> >>> 1) Rename the PcdSevLaunchSecretBase -> PcdSevSecretsBase >>> >>> 2) When SNP is enabled then VMM use this page as secrets page for the SNP >>> >>> 3) When SEV or SEV-ES is enabled then VMM uses this page as a launch >>> secret page >>> >>> This will allow me to drop PcdOvmfSnpSecretsBase. This will not just >>> save 4-bytes but also minimize the code duplication. >> I'm pretty unhappy about needing a separate page for each such purpose. >> We're wasting room in MEMFD. The GUIDed structs that we expose to QEMU >> seem to be flexible enough to describe non-page-aligned addresses, >> right? Can we pack larger amounts of cruft into MEMFD pages? > > With the GUID approach we should be able to pack multiple fields into a > page but unfortunately in the case of SEV-SNP both the CPUID and Secrets > need to be a page size. Without SNP support the we reserve the following > page for the SEV/SEV-ES: > > 1 page for Launch Secret. > > 2 pages for the GHCB > > 1 page for EsWorkArea > > Both the EsWorkArea and LaunchSecret does not need to be the page > aligned or page sized. Since the SNP needs a full page for the secrets > so I was inclined to use the same secrets page for both SEV and SNP. > At the end all we need to do is reserve one extra page for CPUID to > make the SNP work. > > In future the EsWorkArea page can be used to pack additional information > without needed to reserve full page (if feature does not require page). Thank you for doing this analysis. I would much welcome a separate series, just for "compressing" as many of the artifacts we now have down to as few pages as possible. Could you propose a series like that? Please CC Tom, James and Jiewen for review. For this, feel free to rename PCDs as needed, and also to introduce new (packed, if needed) structure types; probably under <OvmfPkg/Include/Guid>. If some constants have to be doubly-defined, for C code and for assembly code separately, for example, I'm fine with that -- normally, that's bad practice, and if we can avoid it, that's great; but being conservative with MEMFD is more important to me. (I hope I'm not going to eat my words a few months down the road; this is how I feel right now anyway.) Thanks! Laszlo > > >> >> I'm not looking forward to the day when we run out of slack in MEMFD and >> we get to shift PEIFV / DXEFV. (Every time we need to increase the DXEFV >> size, the same risk exists -- which is why I've been thinking for a >> while now that OVMF includes too many features already.) This can >> introduce obscure changes to the UEFI memory map, which has caused >> compat problems in the past, for example with the "crash" utility. >> >> The feature creep in OVMF has gone off the rails in the last few years, >> really. (Not that I'm not guilty myself.) >> >> Thanks, >> Laszlo >> >>> Thoughts ? >>> >>> -Brijesh >>> >>> On 3/24/21 10:31 AM, 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%7C04be68371db9458cbdc108d8fe61713a%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637539041773579324%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=J62%2BVSCZRawAkzsi9xtS43cpowZxSCx%2BwcDYNwdF3qA%3D&reserved=0 >>>> >>>> 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|BOOLEAN|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|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpCpuidSize >>>> + >>>> +0x001000|0x001000 >>>> +gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsSize >>>> + >>>> +0x002000|0x006000 >>>> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesSize >>>> >>>> -0x006000|0x001000 >>>> +0x008000|0x001000 >>>> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageSize >>>> >>>> -0x007000|0x001000 >>>> +0x009000|0x001000 >>>> gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress|gUefiOvmfPkgTokenSpaceGuid.PcdGuidedExtractHandlerTableSize >>>> >>>> -0x008000|0x001000 >>>> +0x00A000|0x001000 >>>> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbPageTableBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbPageTableSize >>>> >>>> -0x009000|0x002000 >>>> +0x00B000|0x002000 >>>> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbSize >>>> >>>> -0x00B000|0x001000 >>>> -gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase|gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaSize >>>> - >>>> -0x00C000|0x001000 >>>> +0x00D000|0x001000 >>>> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBackupBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBackupSize >>>> >>>> +0x00F000|0x001000 >>>> +gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase|gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaSize >>>> + >>>> 0x010000|0x010000 >>>> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamBase|gUefiOvmfPkgTokenSpaceGuid.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" > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#74043): https://edk2.groups.io/g/devel/message/74043 Mute This Topic: https://groups.io/mt/81584577/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2026 Red Hat, Inc.