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.lenda...@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.lenda...@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/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-