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

Laszlo Ersek lersek at redhat.com
Tue Apr 6 12:16:54 UTC 2021


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 at amd.com>
>> Sent: Wednesday, March 24, 2021 11:32 PM
>> To: devel at edk2.groups.io
>> Cc: Brijesh Singh <brijesh.singh at amd.com>; James Bottomley
>> <jejb at linux.ibm.com>; Xu, Min M <min.m.xu at intel.com>; Yao, Jiewen
>> <jiewen.yao at intel.com>; Tom Lendacky <thomas.lendacky at amd.com>;
>> Justen, Jordan L <jordan.l.justen at intel.com>; Ard Biesheuvel
>> <ardb+tianocore at kernel.org>; Laszlo Ersek <lersek at 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 at linux.ibm.com>
>> Cc: Min Xu <min.m.xu at intel.com>
>> Cc: Jiewen Yao <jiewen.yao at intel.com>
>> Cc: Tom Lendacky <thomas.lendacky at amd.com>
>> Cc: Jordan Justen <jordan.l.justen at intel.com>
>> Cc: Ard Biesheuvel <ardb+tianocore at kernel.org>
>> Cc: Laszlo Ersek <lersek at redhat.com>
>> Signed-off-by: Brijesh Singh <brijesh.singh at 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/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-





More information about the edk2-devel-archive mailing list