[edk2-devel] [RFC PATCH 04/28] OvmfPkg: Create a GHCB page for use during Sec phase
Lendacky, Thomas
thomas.lendacky at amd.com
Wed Aug 21 21:29:16 UTC 2019
On 8/21/19 9:25 AM, Laszlo Ersek via Groups.Io wrote:
> On 08/19/19 23:35, Lendacky, Thomas wrote:
>> From: Tom Lendacky <thomas.lendacky at amd.com>
>>
>> A GHCB page is needed during the Sec phase, so this new page must be
>> created. Since the GHCB must be marked as an un-encrypted, or shared,
>> page, an additional pagetable page is required so break down the 2MB
>> region where the GHCB page lives into 4K pagetable entries.
>>
>> Signed-off-by: Tom Lendacky <thomas.lendacky at amd.com>
>> ---
>> OvmfPkg/OvmfPkg.dec | 5 +++
>> OvmfPkg/OvmfPkgX64.fdf | 11 ++++---
>> OvmfPkg/PlatformPei/PlatformPei.inf | 2 ++
>> OvmfPkg/ResetVector/ResetVector.inf | 2 ++
>> UefiCpuPkg/Include/Register/Amd/Fam17Msr.h | 28 ++++++++++++++++
>> OvmfPkg/ResetVector/Ia32/PageTables64.asm | 37 +++++++++++++++++++++-
>> OvmfPkg/ResetVector/ResetVector.nasmb | 2 +-
>> 7 files changed, 81 insertions(+), 6 deletions(-)
>
> I've skipped patches 02 and 03 for now, because I'll have to go through
> them with a fine toothed comb -- in a subsequent submission, most
> probably. I'm just trying to provide formal comments, so that I do the
> actual review more easily, later.
>
> As I requested under the blurb, this patch should be split in at least
> three parts, if possible -- OvmfPkg/PlatformPei, OvmfPkg/ResetVector,
> UefiCpuPkg. (The DEC and FDF changes can be kept squashed with the
> OvmfPkg patch that seems more suitable for that.)
Ok.
>
> ... Having said that, why do you add PCDs to the PlatformPei INF file?
> The code in PlatformPei doesn't change. Could be a leftover from an
> earlier (abandoned) approach.
Yeah, most likely. I'll remove that.
Thanks,
Tom
>
> Thanks
> Laszlo
>
>>
>> diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
>> index 9640360f6245..2ead9a944af4 100644
>> --- a/OvmfPkg/OvmfPkg.dec
>> +++ b/OvmfPkg/OvmfPkg.dec
>> @@ -218,6 +218,11 @@ [PcdsFixedAtBuild]
>> # The value should be a multiple of 4KB.
>> gUefiOvmfPkgTokenSpaceGuid.PcdHighPmmMemorySize|0x400000|UINT32|0x31
>>
>> + ## Specify the GHCB base address and size.
>> + # The value should be a multiple of 4KB for each.
>> + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBase|0x0|UINT32|0x32
>> + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbSize|0x0|UINT32|0x33
>> +
>> [PcdsDynamic, PcdsDynamicEx]
>> gUefiOvmfPkgTokenSpaceGuid.PcdEmuVariableEvent|0|UINT64|2
>> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashVariablesEnable|FALSE|BOOLEAN|0x10
>> diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
>> index 74407072563b..2a2427092382 100644
>> --- a/OvmfPkg/OvmfPkgX64.fdf
>> +++ b/OvmfPkg/OvmfPkgX64.fdf
>> @@ -67,13 +67,16 @@ [FD.MEMFD]
>> BlockSize = 0x10000
>> NumBlocks = 0xC0
>>
>> -0x000000|0x006000
>> +0x000000|0x007000
>> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesSize
>>
>> -0x006000|0x001000
>> -gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageSize
>> -
>> 0x007000|0x001000
>> +gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbSize
>> +
>> +0x008000|0x001000
>> +gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageSize
>> +
>> +0x009000|0x001000
>> gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress|gUefiOvmfPkgTokenSpaceGuid.PcdGuidedExtractHandlerTableSize
>>
>> 0x010000|0x010000
>> diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf b/OvmfPkg/PlatformPei/PlatformPei.inf
>> index d9fd9c8f05b3..aed1f64b7c93 100644
>> --- a/OvmfPkg/PlatformPei/PlatformPei.inf
>> +++ b/OvmfPkg/PlatformPei/PlatformPei.inf
>> @@ -72,6 +72,8 @@ [Pcd]
>> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamSize
>> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesBase
>> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesSize
>> + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBase
>> + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbSize
>> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageBase
>> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageSize
>> gUefiOvmfPkgTokenSpaceGuid.PcdGuidedExtractHandlerTableSize
>> diff --git a/OvmfPkg/ResetVector/ResetVector.inf b/OvmfPkg/ResetVector/ResetVector.inf
>> index 960b47cd0797..d66f4dc29737 100644
>> --- a/OvmfPkg/ResetVector/ResetVector.inf
>> +++ b/OvmfPkg/ResetVector/ResetVector.inf
>> @@ -37,3 +37,5 @@ [Pcd]
>> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesSize
>> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamBase
>> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamSize
>> + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBase
>> + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbSize
>> diff --git a/UefiCpuPkg/Include/Register/Amd/Fam17Msr.h b/UefiCpuPkg/Include/Register/Amd/Fam17Msr.h
>> index 37b935dcdb30..55a5723e164e 100644
>> --- a/UefiCpuPkg/Include/Register/Amd/Fam17Msr.h
>> +++ b/UefiCpuPkg/Include/Register/Amd/Fam17Msr.h
>> @@ -17,6 +17,34 @@
>> #ifndef __FAM17_MSR_H__
>> #define __FAM17_MSR_H__
>>
>> +/**
>> + Secure Encrypted Virtualization - Encrypted State (SEV-ES) GHCB register
>> +
>> +**/
>> +#define MSR_SEV_ES_GHCB 0xc0010130
>> +
>> +/**
>> + MSR information returned for #MSR_SEV_ES_GHCB
>> +**/
>> +typedef union {
>> + struct {
>> + UINT32 GhcbNegotiateBit:1;
>> +
>> + UINT32 Reserved:31;
>> + } Bits;
>> +
>> + struct {
>> + UINT8 Reserved[3];
>> + UINT8 SevEncryptionBitPos;
>> + UINT16 SevEsProtocolMin;
>> + UINT16 SevEsProtocolMax;
>> + } GhcbProtocol;
>> +
>> + VOID *Ghcb;
>> +
>> + UINT64 GhcbPhysicalAddress;
>> +} MSR_SEV_ES_GHCB_REGISTER;
>> +
>> /**
>> Secure Encrypted Virtualization (SEV) status register
>>
>> diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
>> index c6071fe934de..fd4d5b1d8661 100644
>> --- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm
>> +++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
>> @@ -21,6 +21,11 @@ BITS 32
>> %define PAGE_2M_MBO 0x080
>> %define PAGE_2M_PAT 0x01000
>>
>> +%define PAGE_4K_PDE_ATTR (PAGE_ACCESSED + \
>> + PAGE_DIRTY + \
>> + PAGE_READ_WRITE + \
>> + PAGE_PRESENT)
>> +
>> %define PAGE_2M_PDE_ATTR (PAGE_2M_MBO + \
>> PAGE_ACCESSED + \
>> PAGE_DIRTY + \
>> @@ -120,7 +125,7 @@ SevNotActive:
>> ; more permanent location by DxeIpl.
>> ;
>>
>> - mov ecx, 6 * 0x1000 / 4
>> + mov ecx, 7 * 0x1000 / 4
>> xor eax, eax
>> clearPageTablesMemoryLoop:
>> mov dword[ecx * 4 + PT_ADDR (0) - 4], eax
>> @@ -157,6 +162,36 @@ pageTableEntriesLoop:
>> mov [(ecx * 8 + PT_ADDR (0x2000 - 8)) + 4], edx
>> loop pageTableEntriesLoop
>>
>> + ;
>> + ; The GHCB will live at 0x807000 (just after the page tables)
>> + ; and needs to be un-encrypted. This requires the 2MB page
>> + ; (index 4 in the first 1GB page) for this range be broken down
>> + ; into 512 4KB pages. All will be marked as encrypted, except
>> + ; for the GHCB.
>> + ;
>> + mov ecx, 4
>> + mov eax, PT_ADDR (0x6000) + PAGE_PDP_ATTR
>> + mov [ecx * 8 + PT_ADDR (0x2000)], eax
>> +
>> + mov ecx, 512
>> +pageTableEntries4kLoop:
>> + mov eax, ecx
>> + dec eax
>> + shl eax, 12
>> + add eax, 0x800000
>> + add eax, PAGE_4K_PDE_ATTR
>> + mov [ecx * 8 + PT_ADDR (0x6000 - 8)], eax
>> + mov [(ecx * 8 + PT_ADDR (0x6000 - 8)) + 4], edx
>> + loop pageTableEntries4kLoop
>> +
>> + ;
>> + ; Clear the encryption bit from the GHCB entry (index 7 in the
>> + ; new PTE table - (0x807000 - 0x800000) >> 12).
>> + ;
>> + mov ecx, 7
>> + xor edx, edx
>> + mov [(ecx * 8 + PT_ADDR (0x6000)) + 4], edx
>> +
>> ;
>> ; Set CR3 now that the paging structures are available
>> ;
>> diff --git a/OvmfPkg/ResetVector/ResetVector.nasmb b/OvmfPkg/ResetVector/ResetVector.nasmb
>> index 3b213cd05ab2..56d9b86ed943 100644
>> --- a/OvmfPkg/ResetVector/ResetVector.nasmb
>> +++ b/OvmfPkg/ResetVector/ResetVector.nasmb
>> @@ -49,7 +49,7 @@
>> %ifdef ARCH_X64
>> #include <AutoGen.h>
>>
>> - %if (FixedPcdGet32 (PcdOvmfSecPageTablesSize) != 0x6000)
>> + %if (FixedPcdGet32 (PcdOvmfSecPageTablesSize) != 0x7000)
>> %error "This implementation inherently depends on PcdOvmfSecPageTablesSize"
>> %endif
>>
>>
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#46179): https://edk2.groups.io/g/devel/message/46179
Mute This Topic: https://groups.io/mt/32966276/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