[edk2-devel] [PATCH 3/3] OvmfPkg/ResetVector: move the GHCB page setup in AmdSev.asm
Lendacky, Thomas via groups.io
thomas.lendacky=amd.com at groups.io
Mon Aug 9 17:03:25 UTC 2021
On 8/4/21 3:20 PM, Brijesh Singh wrote:
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3429
>
> While build the initial page table, the SetCr3ForPageTables64 checks
> whether SEV-ES is enabled. If so, clear the page encryption mask from the
> GHCB page. Move the logic to clear the page encryption mask in the
> AmdSev.asm.
>
> 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: Erdem Aktas <erdemaktas at google.com>
> Signed-off-by: Brijesh Singh <brijesh.singh at amd.com>
> ---
> OvmfPkg/ResetVector/Ia32/AmdSev.asm | 113 +++++++++++++++++-----
> OvmfPkg/ResetVector/Ia32/PageTables64.asm | 53 ++--------
> 2 files changed, 94 insertions(+), 72 deletions(-)
>
> diff --git a/OvmfPkg/ResetVector/Ia32/AmdSev.asm b/OvmfPkg/ResetVector/Ia32/AmdSev.asm
> index 87d81b01e263..fd2e6abcd4a0 100644
> --- a/OvmfPkg/ResetVector/Ia32/AmdSev.asm
> +++ b/OvmfPkg/ResetVector/Ia32/AmdSev.asm
> @@ -44,6 +44,27 @@ BITS 32
> ; The unexpected response code
> %define TERM_UNEXPECTED_RESP_CODE 2
>
> +%define PAGE_PRESENT 0x01
> +%define PAGE_READ_WRITE 0x02
> +%define PAGE_USER_SUPERVISOR 0x04
> +%define PAGE_WRITE_THROUGH 0x08
> +%define PAGE_CACHE_DISABLE 0x010
> +%define PAGE_ACCESSED 0x020
> +%define PAGE_DIRTY 0x040
> +%define PAGE_PAT 0x080
> +%define PAGE_GLOBAL 0x0100
> +%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_PDP_ATTR (PAGE_ACCESSED + \
> + PAGE_READ_WRITE + \
> + PAGE_PRESENT)
> +
>
> ; Macro is used to issue the MSR protocol based VMGEXIT. The caller is
> ; responsible to populate values in the EDX:EAX registers. After the vmmcall
> @@ -117,6 +138,72 @@ BITS 32
> SevEsUnexpectedRespTerminate:
> TerminateVmgExit TERM_UNEXPECTED_RESP_CODE
>
> +; If SEV-ES is enabled then initialize the make the GHCB page shared
s/the make/and make/ ?
> +SevClearPageEncMaskFromGHCBPage:
Just a nit, maybe SevClearPageEncMaskForGhcbPage?
> + ; Check if SEV is enabled
> + cmp byte[WORK_AREA_GUEST_TYPE], 1
> + jnz SevClearPageEncMaskFromGHCBPageExit
> +
> + ; Check if SEV-ES is enabled
> + cmp byte[SEV_ES_WORK_AREA], 1
> + jnz SevClearPageEncMaskFromGHCBPageExit
> +
> + ;
> + ; The initial GHCB will live at GHCB_BASE and needs to be un-encrypted.
> + ; This requires the 2MB page for this range be broken down into 512 4KB
> + ; pages. All will be marked encrypted, except for the GHCB.
> + ;
> + mov ecx, (GHCB_BASE >> 21)
> + mov eax, GHCB_PT_ADDR + PAGE_PDP_ATTR
> + mov [ecx * 8 + PT_ADDR (0x2000)], eax
> +
> + ;
> + ; Page Table Entries (512 * 4KB entries => 2MB)
> + ;
> + mov ecx, 512
> +pageTableEntries4kLoop:
> + mov eax, ecx
> + dec eax
> + shl eax, 12
> + add eax, GHCB_BASE & 0xFFE0_0000
> + add eax, PAGE_4K_PDE_ATTR
> + mov [ecx * 8 + GHCB_PT_ADDR - 8], eax
> + mov [(ecx * 8 + GHCB_PT_ADDR - 8) + 4], edx
> + loop pageTableEntries4kLoop
> +
> + ;
> + ; Clear the encryption bit from the GHCB entry
> + ;
> + mov ecx, (GHCB_BASE & 0x1F_FFFF) >> 12
> + mov [ecx * 8 + GHCB_PT_ADDR + 4], strict dword 0
> +
> + mov ecx, GHCB_SIZE / 4
> + xor eax, eax
> +clearGhcbMemoryLoop:
> + mov dword[ecx * 4 + GHCB_BASE - 4], eax
> + loop clearGhcbMemoryLoop
> +
> +SevClearPageEncMaskFromGHCBPageExit:
> + OneTimeCallRet SevClearPageEncMaskFromGHCBPage
> +
> +; Check if SEV is enabled, and get the C-bit mask above 31.
> +; Modified: EDX
> +;
> +; The value is returned in the EDX
> +GetSevCBitMaskAbove31:
> + ; Check if SEV is enabled
> + cmp byte[WORK_AREA_GUEST_TYPE], 1
> + jnz NoCbitValue
> +
> + mov edx, dword[SEV_ES_WORK_AREA_ENC_MASK + 4]
> + jmp GetSevCBitMaskAbove31Exit
> +
> +NoCbitValue:
> + xor edx, edx
How about moving the xor as the first line of this routine and jumping to
GetSevCBitMaskAbove31Exit if the first cmp is non-zero. Then you can just
do the move from SEV_ES_WORK_AREA_ENC_MASK + 4 and eliminate the extra jmp
statement and NoCbitValue label.
Thanks,
Tom
> +
> +GetSevCBitMaskAbove31Exit:
> + OneTimeCallRet GetSevCBitMaskAbove31
> +
> ; Check if Secure Encrypted Virtualization (SEV) features are enabled.
> ;
> ; Register usage is tight in this routine, so multiple calls for the
> @@ -249,32 +336,6 @@ SevExit:
>
> OneTimeCallRet CheckSevFeatures
>
> -; Check if Secure Encrypted Virtualization - Encrypted State (SEV-ES) feature
> -; is enabled.
> -;
> -; Modified: EAX
> -;
> -; If SEV-ES is enabled then EAX will be non-zero.
> -; If SEV-ES is disabled then EAX will be zero.
> -;
> -IsSevEsEnabled:
> - xor eax, eax
> -
> - ; During CheckSevFeatures, the WORK_AREA_GUEST_TYPE is set
> - ; to 1 if SEV is enabled.
> - cmp byte[WORK_AREA_GUEST_TYPE], 1
> - jne SevEsDisabled
> -
> - ; During CheckSevFeatures, the SEV_ES_WORK_AREA was set to 1 if
> - ; SEV-ES is enabled.
> - cmp byte[SEV_ES_WORK_AREA], 1
> - jne SevEsDisabled
> -
> - mov eax, 1
> -
> -SevEsDisabled:
> - OneTimeCallRet IsSevEsEnabled
> -
> ; Start of #VC exception handling routines
> ;
>
> diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> index f688909f1c7d..0e8ba4dde534 100644
> --- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> +++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> @@ -46,16 +46,13 @@ SetCr3ForPageTables64:
> ; work area when detected.
> mov byte[WORK_AREA_GUEST_TYPE], 0
>
> + ; Check whether the SEV is active and populate the SevEsWorkArea
> OneTimeCall CheckSevFeatures
> - xor edx, edx
> - test eax, eax
> - jz SevNotActive
>
> - ; If SEV is enabled, C-bit is always above 31
> - sub eax, 32
> - bts edx, eax
> -
> -SevNotActive:
> + ; If SEV is enabled, the C-bit position is always above 31.
> + ; The mask will be saved in the EDX and applied during the
> + ; the page table build below.
> + OneTimeCall GetSevCBitMaskAbove31
>
> ;
> ; For OVMF, build some initial page tables at
> @@ -105,44 +102,8 @@ pageTableEntriesLoop:
> mov [(ecx * 8 + PT_ADDR (0x2000 - 8)) + 4], edx
> loop pageTableEntriesLoop
>
> - OneTimeCall IsSevEsEnabled
> - test eax, eax
> - jz SetCr3
> -
> - ;
> - ; The initial GHCB will live at GHCB_BASE and needs to be un-encrypted.
> - ; This requires the 2MB page for this range be broken down into 512 4KB
> - ; pages. All will be marked encrypted, except for the GHCB.
> - ;
> - mov ecx, (GHCB_BASE >> 21)
> - mov eax, GHCB_PT_ADDR + PAGE_PDP_ATTR
> - mov [ecx * 8 + PT_ADDR (0x2000)], eax
> -
> - ;
> - ; Page Table Entries (512 * 4KB entries => 2MB)
> - ;
> - mov ecx, 512
> -pageTableEntries4kLoop:
> - mov eax, ecx
> - dec eax
> - shl eax, 12
> - add eax, GHCB_BASE & 0xFFE0_0000
> - add eax, PAGE_4K_PDE_ATTR
> - mov [ecx * 8 + GHCB_PT_ADDR - 8], eax
> - mov [(ecx * 8 + GHCB_PT_ADDR - 8) + 4], edx
> - loop pageTableEntries4kLoop
> -
> - ;
> - ; Clear the encryption bit from the GHCB entry
> - ;
> - mov ecx, (GHCB_BASE & 0x1F_FFFF) >> 12
> - mov [ecx * 8 + GHCB_PT_ADDR + 4], strict dword 0
> -
> - mov ecx, GHCB_SIZE / 4
> - xor eax, eax
> -clearGhcbMemoryLoop:
> - mov dword[ecx * 4 + GHCB_BASE - 4], eax
> - loop clearGhcbMemoryLoop
> + ; Clear the C-bit from the GHCB page if the SEV-ES is enabled.
> + OneTimeCall SevClearPageEncMaskFromGHCBPage
>
> SetCr3:
> ;
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#78980): https://edk2.groups.io/g/devel/message/78980
Mute This Topic: https://groups.io/mt/84670986/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