Re: [edk2-devel] [RFC PATCH v2 06/44] OvmfPkg: Create a GHCB page for use during Sec phase
On 9/25/19 3:09 AM, Laszlo Ersek wrote: > On 09/19/19 21:52, Lendacky, Thomas wrote: >> From: Tom Lendacky >> >> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2198 >> >> 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 to break down the 2MB >> region where the GHCB page lives into 4K pagetable entries. >> >> Create a new entry in the OVMF memory layout for the new page table >> page and for the SEC GHCB page. After breaking down the 2MB page, update >> the GHCB page table entry to remove the encryption mask. >> >> Cc: Jordan Justen >> Cc: Laszlo Ersek >> Cc: Ard Biesheuvel >> Signed-off-by: Tom Lendacky >> --- >> OvmfPkg/OvmfPkg.dec | 10 +++ >> OvmfPkg/OvmfPkgX64.fdf| 6 ++ >> OvmfPkg/ResetVector/ResetVector.inf | 4 ++ >> OvmfPkg/ResetVector/Ia32/PageTables64.asm | 79 +++ >> OvmfPkg/ResetVector/ResetVector.nasmb | 12 >> 5 files changed, 111 insertions(+) >> >> diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec >> index 9640360f6245..b9287a023c94 100644 >> --- a/OvmfPkg/OvmfPkg.dec >> +++ b/OvmfPkg/OvmfPkg.dec >> @@ -218,6 +218,16 @@ [PcdsFixedAtBuild] >># The value should be a multiple of 4KB. >>gUefiOvmfPkgTokenSpaceGuid.PcdHighPmmMemorySize|0x40|UINT32|0x31 >> >> + ## Specify the extra page table needed to mark the GHCB as unencrypted. >> + # The value should be a multiple of 4KB for each. >> + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbPageTableBase|0x0|UINT32|0x32 >> + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbPageTableSize|0x0|UINT32|0x33 >> + >> + ## Specify the GHCB base address and size. >> + # The value should be a multiple of 4KB for each. >> + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBase|0x0|UINT32|0x34 >> + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbSize|0x0|UINT32|0x35 >> + >> [PcdsDynamic, PcdsDynamicEx] >>gUefiOvmfPkgTokenSpaceGuid.PcdEmuVariableEvent|0|UINT64|2 >>gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashVariablesEnable|FALSE|BOOLEAN|0x10 >> diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf >> index 74407072563b..a567131a0591 100644 >> --- a/OvmfPkg/OvmfPkgX64.fdf >> +++ b/OvmfPkg/OvmfPkgX64.fdf >> @@ -76,6 +76,12 @@ [FD.MEMFD] >> 0x007000|0x001000 >> >> gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress|gUefiOvmfPkgTokenSpaceGuid.PcdGuidedExtractHandlerTableSize >> >> +0x008000|0x001000 >> +gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbPageTableBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbPageTableSize >> + >> +0x009000|0x001000 >> +gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbSize >> + >> 0x01|0x01 >> >> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamSize >> >> diff --git a/OvmfPkg/ResetVector/ResetVector.inf >> b/OvmfPkg/ResetVector/ResetVector.inf >> index 960b47cd0797..80c971354176 100644 >> --- a/OvmfPkg/ResetVector/ResetVector.inf >> +++ b/OvmfPkg/ResetVector/ResetVector.inf >> @@ -37,3 +37,7 @@ [Pcd] >>gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesSize >>gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamBase >>gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamSize >> + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbPageTableBase >> + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbPageTableSize >> + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBase >> + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbSize >> diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm >> b/OvmfPkg/ResetVector/Ia32/PageTables64.asm >> index 40f7814c1134..7e346661f2c8 100644 >> --- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm >> +++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm >> @@ -21,6 +21,11 @@ BITS32 >> %define PAGE_2M_MBO0x080 >> %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 + \ >> @@ -95,6 +100,37 @@ SevExit: >> >> OneTimeCallRet CheckSevFeature >> >> +; Check if Secure Encrypted Virtualization - Encrypted State (SEV-ES) >> feature >> +; is enabled. >> +; >> +; Modified: EAX, EBX, ECX, EDX > > (1) I think we should remove EDX from this list. It is restored at the > end of the routine. And, in pageTableEntries4kLoop, we rely on EDX > containing the encryption mask. Yup, I'll remove that one. > >> +; >> +; If SEV-ES is enabled then EAX will be non-zero. >> +; If SEV-ES is disabled then EAX will be zero. >> +; >> +CheckSevEsFeature: >> +xor eax, eax >> + >> +; SEV-ES can't be enabled if SEV isn't, so first check the encryption >> +; mask. >> +test
Re: [edk2-devel] [RFC PATCH v2 06/44] OvmfPkg: Create a GHCB page for use during Sec phase
On 09/19/19 21:52, Lendacky, Thomas wrote: > From: Tom Lendacky > > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2198 > > 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 to break down the 2MB > region where the GHCB page lives into 4K pagetable entries. > > Create a new entry in the OVMF memory layout for the new page table > page and for the SEC GHCB page. After breaking down the 2MB page, update > the GHCB page table entry to remove the encryption mask. > > Cc: Jordan Justen > Cc: Laszlo Ersek > Cc: Ard Biesheuvel > Signed-off-by: Tom Lendacky > --- > OvmfPkg/OvmfPkg.dec | 10 +++ > OvmfPkg/OvmfPkgX64.fdf| 6 ++ > OvmfPkg/ResetVector/ResetVector.inf | 4 ++ > OvmfPkg/ResetVector/Ia32/PageTables64.asm | 79 +++ > OvmfPkg/ResetVector/ResetVector.nasmb | 12 > 5 files changed, 111 insertions(+) > > diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec > index 9640360f6245..b9287a023c94 100644 > --- a/OvmfPkg/OvmfPkg.dec > +++ b/OvmfPkg/OvmfPkg.dec > @@ -218,6 +218,16 @@ [PcdsFixedAtBuild] ># The value should be a multiple of 4KB. >gUefiOvmfPkgTokenSpaceGuid.PcdHighPmmMemorySize|0x40|UINT32|0x31 > > + ## Specify the extra page table needed to mark the GHCB as unencrypted. > + # The value should be a multiple of 4KB for each. > + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbPageTableBase|0x0|UINT32|0x32 > + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbPageTableSize|0x0|UINT32|0x33 > + > + ## Specify the GHCB base address and size. > + # The value should be a multiple of 4KB for each. > + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBase|0x0|UINT32|0x34 > + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbSize|0x0|UINT32|0x35 > + > [PcdsDynamic, PcdsDynamicEx] >gUefiOvmfPkgTokenSpaceGuid.PcdEmuVariableEvent|0|UINT64|2 >gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashVariablesEnable|FALSE|BOOLEAN|0x10 > diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf > index 74407072563b..a567131a0591 100644 > --- a/OvmfPkg/OvmfPkgX64.fdf > +++ b/OvmfPkg/OvmfPkgX64.fdf > @@ -76,6 +76,12 @@ [FD.MEMFD] > 0x007000|0x001000 > > gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress|gUefiOvmfPkgTokenSpaceGuid.PcdGuidedExtractHandlerTableSize > > +0x008000|0x001000 > +gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbPageTableBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbPageTableSize > + > +0x009000|0x001000 > +gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbSize > + > 0x01|0x01 > > gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamSize > > diff --git a/OvmfPkg/ResetVector/ResetVector.inf > b/OvmfPkg/ResetVector/ResetVector.inf > index 960b47cd0797..80c971354176 100644 > --- a/OvmfPkg/ResetVector/ResetVector.inf > +++ b/OvmfPkg/ResetVector/ResetVector.inf > @@ -37,3 +37,7 @@ [Pcd] >gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesSize >gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamBase >gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamSize > + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbPageTableBase > + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbPageTableSize > + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBase > + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbSize > diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm > b/OvmfPkg/ResetVector/Ia32/PageTables64.asm > index 40f7814c1134..7e346661f2c8 100644 > --- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm > +++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm > @@ -21,6 +21,11 @@ BITS32 > %define PAGE_2M_MBO0x080 > %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 + \ > @@ -95,6 +100,37 @@ SevExit: > > OneTimeCallRet CheckSevFeature > > +; Check if Secure Encrypted Virtualization - Encrypted State (SEV-ES) feature > +; is enabled. > +; > +; Modified: EAX, EBX, ECX, EDX (1) I think we should remove EDX from this list. It is restored at the end of the routine. And, in pageTableEntries4kLoop, we rely on EDX containing the encryption mask. > +; > +; If SEV-ES is enabled then EAX will be non-zero. > +; If SEV-ES is disabled then EAX will be zero. > +; > +CheckSevEsFeature: > +xor eax, eax > + > +; SEV-ES can't be enabled if SEV isn't, so first check the encryption > +; mask. > +test edx, edx > +jzNoSevEs > + > +; Save current value of encryption mask > +mov ebx, edx > + > +; Check if SEV-ES is enabled > +; MSR_0xC0010131 - Bit 1 (SEV-ES
[edk2-devel] [RFC PATCH v2 06/44] OvmfPkg: Create a GHCB page for use during Sec phase
From: Tom Lendacky BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2198 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 to break down the 2MB region where the GHCB page lives into 4K pagetable entries. Create a new entry in the OVMF memory layout for the new page table page and for the SEC GHCB page. After breaking down the 2MB page, update the GHCB page table entry to remove the encryption mask. Cc: Jordan Justen Cc: Laszlo Ersek Cc: Ard Biesheuvel Signed-off-by: Tom Lendacky --- OvmfPkg/OvmfPkg.dec | 10 +++ OvmfPkg/OvmfPkgX64.fdf| 6 ++ OvmfPkg/ResetVector/ResetVector.inf | 4 ++ OvmfPkg/ResetVector/Ia32/PageTables64.asm | 79 +++ OvmfPkg/ResetVector/ResetVector.nasmb | 12 5 files changed, 111 insertions(+) diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec index 9640360f6245..b9287a023c94 100644 --- a/OvmfPkg/OvmfPkg.dec +++ b/OvmfPkg/OvmfPkg.dec @@ -218,6 +218,16 @@ [PcdsFixedAtBuild] # The value should be a multiple of 4KB. gUefiOvmfPkgTokenSpaceGuid.PcdHighPmmMemorySize|0x40|UINT32|0x31 + ## Specify the extra page table needed to mark the GHCB as unencrypted. + # The value should be a multiple of 4KB for each. + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbPageTableBase|0x0|UINT32|0x32 + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbPageTableSize|0x0|UINT32|0x33 + + ## Specify the GHCB base address and size. + # The value should be a multiple of 4KB for each. + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBase|0x0|UINT32|0x34 + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbSize|0x0|UINT32|0x35 + [PcdsDynamic, PcdsDynamicEx] gUefiOvmfPkgTokenSpaceGuid.PcdEmuVariableEvent|0|UINT64|2 gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashVariablesEnable|FALSE|BOOLEAN|0x10 diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf index 74407072563b..a567131a0591 100644 --- a/OvmfPkg/OvmfPkgX64.fdf +++ b/OvmfPkg/OvmfPkgX64.fdf @@ -76,6 +76,12 @@ [FD.MEMFD] 0x007000|0x001000 gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress|gUefiOvmfPkgTokenSpaceGuid.PcdGuidedExtractHandlerTableSize +0x008000|0x001000 +gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbPageTableBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbPageTableSize + +0x009000|0x001000 +gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbSize + 0x01|0x01 gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamSize diff --git a/OvmfPkg/ResetVector/ResetVector.inf b/OvmfPkg/ResetVector/ResetVector.inf index 960b47cd0797..80c971354176 100644 --- a/OvmfPkg/ResetVector/ResetVector.inf +++ b/OvmfPkg/ResetVector/ResetVector.inf @@ -37,3 +37,7 @@ [Pcd] gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesSize gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamBase gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamSize + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbPageTableBase + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbPageTableSize + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBase + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbSize diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm b/OvmfPkg/ResetVector/Ia32/PageTables64.asm index 40f7814c1134..7e346661f2c8 100644 --- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm +++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm @@ -21,6 +21,11 @@ BITS32 %define PAGE_2M_MBO0x080 %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 + \ @@ -95,6 +100,37 @@ SevExit: OneTimeCallRet CheckSevFeature +; Check if Secure Encrypted Virtualization - Encrypted State (SEV-ES) feature +; is enabled. +; +; Modified: EAX, EBX, ECX, EDX +; +; If SEV-ES is enabled then EAX will be non-zero. +; If SEV-ES is disabled then EAX will be zero. +; +CheckSevEsFeature: +xor eax, eax + +; SEV-ES can't be enabled if SEV isn't, so first check the encryption +; mask. +test edx, edx +jzNoSevEs + +; Save current value of encryption mask +mov ebx, edx + +; Check if SEV-ES is enabled +; MSR_0xC0010131 - Bit 1 (SEV-ES enabled) +mov ecx, 0xc0010131 +rdmsr +and eax, 2 + +; Restore encryption mask +mov edx, ebx + +NoSevEs: +OneTimeCallRet CheckSevEsFeature + ; ; Modified: EAX, EBX, ECX, EDX ; @@ -159,6 +195,49 @@ pageTableEntriesLoop: mov [(ecx * 8 + PT_ADDR (0x2000 - 8)) + 4], edx looppageTableEntriesLoop +OneTimeCall CheckSevEsFeature +testeax, eax +jz SetCr3 + +; +; The
