[edk2-devel] [PATCH V2 4/4] OvmfPkg/ResetVector: Update ResetVector to support Tdx
Yao, Jiewen
jiewen.yao at intel.com
Sun Jul 25 06:00:51 UTC 2021
Hi Min, Brijesh, James
I feel very frustrated when I review the existing OVMF reset vector.
A big problem is that this code mixed too many SEV stuff, and we are trying to add more TDX stuff in *one* file, without clear isolation.
Take PageTables64.asm as example, here are the symbols. (* means it is newly added.)
=================
CheckSevFeatures:
GetSevEncBit:
SevEncBitLowHlt:
SevSaveMask:
NoSev:
NoSevEsVcHlt:
NoSevPass:
SevExit:
IsSevEsEnabled:
SevEsDisabled:
SetCr3ForPageTables64:
CheckSev: (*)
SevNotActive:
clearPageTablesMemoryLoop:
pageTableEntriesLoop:
tdClearTdxPageTablesMemoryLoop: (*)
IsSevEs: (*)
pageTableEntries4kLoop:
clearGhcbMemoryLoop:
SetCr3:
SevEsIdtNotCpuid:
SevEsIdtNoCpuidResponse:
SevEsIdtTerminate:
SevEsIdtHlt:
SevEsIdtVmmComm:
NextReg:
VmmDone:
=================
In order to better maintain the ResetVector, may I propose some refinement:
1) The main function only contains the non-TEE function, where TEE == SEV + TDX.
2) The TEE related code is moved to TEE specific standalone file, such *Sev.asm and *Tdx.Asm.
3) We need handle different cases with different pattern.
I hope the patter can be used consistently. As such, the reviewer can easily understand what it is for.
3.1) If TEE function is a hook, then the main function calls TEE function directly. The TEE function need implement a TEE check function (such as IsSev, or IsTdx). For example:
====================
OneTimeCall PreMainFunctionHookSev
OneTimeCall PreMainFunctionHookTdx
MainFunction:
XXXXXX
OneTimeCall PostMainFunctionHookSev
OneTimeCall PostMainFunctionHookTdx
====================
3.2) If TEE function is a replacement for non-TEE function. The main function can call TEE replacement function, then check the return status to decide next step. For example:
====================
OneTimeCall MainFunctionSev
Jz MainFunctionEnd
OneTimeCall MainFunctionTdx
Jz MainFunctionEnd
MainFunction:
XXXXXX
MainFunctionEnd:
====================
4) If we found it is too hard to write code in above patter, we can discuss case by case.
> -----Original Message-----
> From: Xu, Min M <min.m.xu at intel.com>
> Sent: Thursday, July 22, 2021 1:52 PM
> To: devel at edk2.groups.io
> Cc: Xu, Min M <min.m.xu at intel.com>; Ard Biesheuvel
> <ardb+tianocore at kernel.org>; Brijesh Singh <brijesh.singh at amd.com>; Erdem
> Aktas <erdemaktas at google.com>; James Bottomley <jejb at linux.ibm.com>;
> Yao, Jiewen <jiewen.yao at intel.com>; Tom Lendacky
> <thomas.lendacky at amd.com>
> Subject: [PATCH V2 4/4] OvmfPkg/ResetVector: Update ResetVector to support
> Tdx
>
> RFC: https://bugzilla.tianocore.org/show_bug.cgi?id=3429
>
> In Tdx all CPUs "reset" to run on 32-bit protected mode with flat
> descriptor (paging disabled). But in Non-Td guest the initial state of
> CPUs is 16-bit real mode. To resolve this conflict, BITS 16/32 is used
> in the very beginning of ResetVector. It will check the 32-bit protected
> mode or 16-bit real mode, then jump to the corresponding entry point.
> This is done in OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm.
>
> ReloadFlat32.asm load the GDT and set the CR0, then jump to Flat-32 mode.
>
> InitTdx.asm is called to record the Tdx signature ('TDXG') and other tdx
> information in a TDX_WORK_AREA which can be used by the other routines in
> ResetVector.
>
> Init32.asm is 32-bit initialization code in OvmfPkg. It puts above
> ReloadFlat32 and InitTdx together to do the initializaiton for Tdx.
>
> After that Tdx jumps to 64-bit long mode by doing following tasks:
> 1. SetCr3ForPageTables64
> For OVMF, some initial page tables is built at:
> PcdOvmfSecPageTablesBase - (PcdOvmfSecPageTablesBase + 0x6000)
> This page table supports the 4-level page table.
> But Tdx support 4-level and 5-level page table based on the CPU GPA width.
> 48bit is 4-level paging, 52-bit is 5-level paging.
> If 5-level page table is supported (GPAW is 52), then a top level
> page directory pointers (1 * 256TB entry) is generated in the
> TdxPageTable.
> 2. Set Cr4
> Enable PAE.
> 3. Adjust Cr3
> If GPAW is 48, then Cr3 is PT_ADDR (0). If GPAW is 52, then Cr3 is
> TDX_PT_ADDR (0).
>
> Tdx MailBox [0x10, 0x800] is reserved for OS. So we initialize piece of this
> area ([0x10, 0x20]) to record the Tdx flag ('TDXG') and other Tdx info so that
> they can be used in the following flow.
>
> After all above is successfully done, Tdx jump to SecEntry.
>
> Cc: Ard Biesheuvel <ardb+tianocore at kernel.org>
> Cc: Brijesh Singh <brijesh.singh at amd.com>
> Cc: Erdem Aktas <erdemaktas at google.com>
> Cc: James Bottomley <jejb at linux.ibm.com>
> Cc: Jiewen Yao <jiewen.yao at intel.com>
> Cc: Tom Lendacky <thomas.lendacky at amd.com>
> Signed-off-by: Min Xu <min.m.xu at intel.com>
> ---
> OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm | 21 ++++++++
> OvmfPkg/ResetVector/Ia32/Flat32ToFlat64.asm | 47 ++++++++++++++++
> OvmfPkg/ResetVector/Ia32/Init32.asm | 34 ++++++++++++
> OvmfPkg/ResetVector/Ia32/InitTdx.asm | 57 ++++++++++++++++++++
> OvmfPkg/ResetVector/Ia32/PageTables64.asm | 41 ++++++++++++++
> OvmfPkg/ResetVector/Ia32/ReloadFlat32.asm | 44 +++++++++++++++
> OvmfPkg/ResetVector/ResetVector.nasmb | 18 +++++++
> 7 files changed, 262 insertions(+)
> create mode 100644 OvmfPkg/ResetVector/Ia32/Init32.asm
> create mode 100644 OvmfPkg/ResetVector/Ia32/InitTdx.asm
> create mode 100644 OvmfPkg/ResetVector/Ia32/ReloadFlat32.asm
>
> diff --git a/OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm
> b/OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm
> index ac86ce69ebe8..a390ed81d021 100644
> --- a/OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm
> +++ b/OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm
> @@ -155,10 +155,31 @@ resetVector:
> ;
> ; This is where the processor will begin execution
> ;
> +; In IA32 we follow the standard reset vector flow. While in X64, Td guest
> +; may be supported. Td guest requires the startup mode to be 32-bit
> +; protected mode but the legacy VM startup mode is 16-bit real mode.
> +; To make NASM generate such shared entry code that behaves correctly in
> +; both 16-bit and 32-bit mode, more BITS directives are added.
> +;
> +%ifdef ARCH_IA32
> +
> nop
> nop
> jmp EarlyBspInitReal16
>
> +%else
> +
> + smsw ax
> + test al, 1
> + jz .Real
> +BITS 32
> + jmp Main32
> +BITS 16
> +.Real:
> + jmp EarlyBspInitReal16
> +
> +%endif
> +
> ALIGN 16
>
> fourGigabytes:
> diff --git a/OvmfPkg/ResetVector/Ia32/Flat32ToFlat64.asm
> b/OvmfPkg/ResetVector/Ia32/Flat32ToFlat64.asm
> index c6d0d898bcd1..2206ca719593 100644
> --- a/OvmfPkg/ResetVector/Ia32/Flat32ToFlat64.asm
> +++ b/OvmfPkg/ResetVector/Ia32/Flat32ToFlat64.asm
> @@ -17,6 +17,9 @@ Transition32FlatTo64Flat:
>
> OneTimeCall SetCr3ForPageTables64
>
> + cmp dword[TDX_WORK_AREA], 0x47584454 ; 'TDXG'
> + jz TdxTransition32FlatTo64Flat
> +
> mov eax, cr4
> bts eax, 5 ; enable PAE
> mov cr4, eax
> @@ -65,10 +68,54 @@ EnablePaging:
> bts eax, 31 ; set PG
> mov cr0, eax ; enable paging
>
> + jmp _jumpTo64Bit
> +
> +;
> +; Tdx Transition from 32Flat to 64Flat
> +;
> +TdxTransition32FlatTo64Flat:
> +
> + mov eax, cr4
> + bts eax, 5 ; enable PAE
> +
> + ;
> + ; byte[TDX_WORK_AREA_PAGELEVEL5] holds the indicator whether 52bit is
> supported.
> + ; if it is the case, need to set LA57 and use 5-level paging
> + ;
> + cmp byte[TDX_WORK_AREA_PAGELEVEL5], 0
> + jz .set_cr4
> + bts eax, 12
> +.set_cr4:
> + mov cr4, eax
> + mov ebx, cr3
> +
> + ;
> + ; if la57 is not set, we are ok
> + ; if using 5-level paging, adjust top-level page directory
> + ;
> + bt eax, 12
> + jnc .set_cr3
> + mov ebx, TDX_PT_ADDR (0)
> +.set_cr3:
> + mov cr3, ebx
> +
> + mov eax, cr0
> + bts eax, 31 ; set PG
> + mov cr0, eax ; enable paging
> +
> +_jumpTo64Bit:
> jmp LINEAR_CODE64_SEL:ADDR_OF(jumpTo64BitAndLandHere)
> +
> BITS 64
> jumpTo64BitAndLandHere:
>
> + ;
> + ; For Td guest we are done and jump to the end
> + ;
> + mov eax, TDX_WORK_AREA
> + cmp dword [eax], 0x47584454 ; 'TDXG'
> + jz GoodCompare
> +
> ;
> ; Check if the second step of the SEV-ES mitigation is to be performed.
> ;
> diff --git a/OvmfPkg/ResetVector/Ia32/Init32.asm
> b/OvmfPkg/ResetVector/Ia32/Init32.asm
> new file mode 100644
> index 000000000000..772adc51e531
> --- /dev/null
> +++ b/OvmfPkg/ResetVector/Ia32/Init32.asm
> @@ -0,0 +1,34 @@
> +;------------------------------------------------------------------------------
> +; @file
> +; 32-bit initialization for Tdx
> +;
> +; Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
> +; SPDX-License-Identifier: BSD-2-Clause-Patent
> +;
> +;------------------------------------------------------------------------------
> +
> +BITS 32
> +
> +;
> +; Modified: EBP
> +;
> +; @param[in] EBX [6:0] CPU supported GPA width
> +; [7:7] 5 level page table support
> +; @param[in] ECX [31:0] TDINITVP - Untrusted Configuration
> +; @param[in] EDX [31:0] VCPUID
> +; @param[in] ESI [31:0] VCPU_Index
> +;
> +Init32:
> + ;
> + ; Save EBX in EBP because EBX will be changed in ReloadFlat32
> + ;
> + mov ebp, ebx
> +
> + OneTimeCall ReloadFlat32
> +
> + ;
> + ; Init Tdx
> + ;
> + OneTimeCall InitTdx
> +
> + OneTimeCallRet Init32
> diff --git a/OvmfPkg/ResetVector/Ia32/InitTdx.asm
> b/OvmfPkg/ResetVector/Ia32/InitTdx.asm
> new file mode 100644
> index 000000000000..de8273da6a0c
> --- /dev/null
> +++ b/OvmfPkg/ResetVector/Ia32/InitTdx.asm
> @@ -0,0 +1,57 @@
> +;------------------------------------------------------------------------------
> +; @file
> +; Initialize TDX_WORK_AREA to record the Tdx flag ('TDXG') and other Tdx info
> +; so that the following codes can use these information.
> +;
> +; Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
> +; SPDX-License-Identifier: BSD-2-Clause-Patent
> +;
> +;------------------------------------------------------------------------------
> +
> +BITS 32
> +
> +;
> +; Modified: EBP
> +;
> +InitTdx:
> + ;
> + ; In Td guest, BSP/AP shares the same entry point
> + ; BSP builds up the page table, while APs shouldn't do the same task.
> + ; Instead, APs just leverage the page table which is built by BSP.
> + ; APs will wait until the page table is ready.
> + ; In Td guest, vCPU 0 is treated as the BSP, the others are APs.
> + ; ESI indicates the vCPU ID.
> + ;
> + cmp esi, 0
> + je tdBspEntry
> +
> +apWait:
> + cmp byte[TDX_WORK_AREA_PGTBL_READY], 0
> + je apWait
> + jmp doneTdxInit
> +
> +tdBspEntry:
> + ;
> + ; It is of Tdx Guest
> + ; Save the Tdx info in TDX_WORK_AREA so that the following code can use
> + ; these information.
> + ;
> + mov dword [TDX_WORK_AREA], 0x47584454 ; 'TDXG'
> +
> + ;
> + ; EBP[6:0] CPU supported GPA width
> + ;
> + and ebp, 0x3f
> + cmp ebp, 52
> + jl NotPageLevel5
> + mov byte[TDX_WORK_AREA_PAGELEVEL5], 1
> +
> +NotPageLevel5:
> + ;
> + ; ECX[31:0] TDINITVP - Untrusted Configuration
> + ;
> + mov DWORD[TDX_WORK_AREA_INITVP], ecx
> + mov DWORD[TDX_WORK_AREA_INFO], ebp
> +
> +doneTdxInit:
> + OneTimeCallRet InitTdx
> diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> index 5fae8986d9da..508df6cf5967 100644
> --- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> +++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> @@ -218,6 +218,24 @@ SevEsDisabled:
> ;
> SetCr3ForPageTables64:
>
> + ;
> + ; Check Td guest
> + ;
> + cmp dword[TDX_WORK_AREA], 0x47584454 ; 'TDXG'
> + jnz CheckSev
> +
> + xor edx, edx
> +
> + ;
> + ; In Td guest, BSP builds the page table and set the flag of
> + ; TDX_WORK_AREA_PGTBL_READY. APs check this flag and then set
> + ; cr3 directly.
> + ;
> + cmp byte[TDX_WORK_AREA_PGTBL_READY], 1
> + jz SetCr3
> + jmp SevNotActive
> +
> +CheckSev:
> OneTimeCall CheckSevFeatures
> xor edx, edx
> test eax, eax
> @@ -277,6 +295,29 @@ pageTableEntriesLoop:
> mov [(ecx * 8 + PT_ADDR (0x2000 - 8)) + 4], edx
> loop pageTableEntriesLoop
>
> + ;
> + ; If it is Td guest, TdxExtraPageTable should be initialized as well
> + ;
> + cmp dword[TDX_WORK_AREA], 0x47584454 ; 'TDXG'
> + jnz IsSevEs
> +
> + xor eax, eax
> + mov ecx, 0x400
> +tdClearTdxPageTablesMemoryLoop:
> + mov dword [ecx * 4 + TDX_PT_ADDR (0) - 4], eax
> + loop tdClearTdxPageTablesMemoryLoop
> +
> + xor edx, edx
> + ;
> + ; Top level Page Directory Pointers (1 * 256TB entry)
> + ;
> + mov dword[TDX_PT_ADDR (0)], PT_ADDR (0) + PAGE_PDP_ATTR
> + mov dword[TDX_PT_ADDR (4)], edx
> +
> + mov byte[TDX_WORK_AREA_PGTBL_READY], 1
> + jmp SetCr3
> +
> +IsSevEs:
> OneTimeCall IsSevEsEnabled
> test eax, eax
> jz SetCr3
> diff --git a/OvmfPkg/ResetVector/Ia32/ReloadFlat32.asm
> b/OvmfPkg/ResetVector/Ia32/ReloadFlat32.asm
> new file mode 100644
> index 000000000000..06d44142625a
> --- /dev/null
> +++ b/OvmfPkg/ResetVector/Ia32/ReloadFlat32.asm
> @@ -0,0 +1,44 @@
> +;------------------------------------------------------------------------------
> +; @file
> +; Load the GDT and set the CR0/CR4, then jump to Flat 32 protected mode.
> +;
> +; Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
> +; SPDX-License-Identifier: BSD-2-Clause-Patent
> +;
> +;------------------------------------------------------------------------------
> +
> +%define SEC_DEFAULT_CR0 0x00000023
> +%define SEC_DEFAULT_CR4 0x640
> +
> +BITS 32
> +
> +;
> +; Modified: EAX, EBX, CR0, CR4, DS, ES, FS, GS, SS
> +;
> +ReloadFlat32:
> +
> + cli
> + mov ebx, ADDR_OF(gdtr)
> + lgdt [ebx]
> +
> + mov eax, SEC_DEFAULT_CR0
> + mov cr0, eax
> +
> + jmp LINEAR_CODE_SEL:dword ADDR_OF(jumpToFlat32BitAndLandHere)
> +BITS 32
> +jumpToFlat32BitAndLandHere:
> +
> + mov eax, SEC_DEFAULT_CR4
> + mov cr4, eax
> +
> + debugShowPostCode POSTCODE_32BIT_MODE
> +
> + mov ax, LINEAR_SEL
> + mov ds, ax
> + mov es, ax
> + mov fs, ax
> + mov gs, ax
> + mov ss, ax
> +
> + OneTimeCallRet ReloadFlat32
> +
> diff --git a/OvmfPkg/ResetVector/ResetVector.nasmb
> b/OvmfPkg/ResetVector/ResetVector.nasmb
> index b653fe87abd6..3ec163613477 100644
> --- a/OvmfPkg/ResetVector/ResetVector.nasmb
> +++ b/OvmfPkg/ResetVector/ResetVector.nasmb
> @@ -106,6 +106,21 @@
> %define TDX_EXTRA_PAGE_TABLE_BASE FixedPcdGet32
> (PcdOvmfSecGhcbPageTableBase)
> %define TDX_EXTRA_PAGE_TABLE_SIZE FixedPcdGet32
> (PcdOvmfSecGhcbPageTableSize)
>
> + ;
> + ; TdMailboxBase [0x10, 0x800] is reserved for OS.
> + ; Td guest initialize piece of this area (TdMailboxBase [0x10,0x20]) to
> + ; record the Td guest info so that this information can be used in the
> + ; following ResetVector flow.
> + ;
> + %define TD_MAILBOX_WORKAREA_OFFSET 0x10
> + %define TDX_WORK_AREA (TDX_MAILBOX_MEMORY_BASE +
> TD_MAILBOX_WORKAREA_OFFSET)
> + %define TDX_WORK_AREA_PAGELEVEL5 (TDX_WORK_AREA + 4)
> + %define TDX_WORK_AREA_PGTBL_READY (TDX_WORK_AREA + 5)
> + %define TDX_WORK_AREA_INITVP (TDX_WORK_AREA + 8)
> + %define TDX_WORK_AREA_INFO (TDX_WORK_AREA + 8 + 4)
> +
> + %define TDX_PT_ADDR(Offset) (TDX_EXTRA_PAGE_TABLE_BASE + (Offset))
> +
> %define PT_ADDR(Offset) (FixedPcdGet32 (PcdOvmfSecPageTablesBase) +
> (Offset))
>
> %define GHCB_PT_ADDR (FixedPcdGet32 (PcdOvmfSecGhcbPageTableBase))
> @@ -117,6 +132,9 @@
> %define SEV_ES_VC_TOP_OF_STACK (FixedPcdGet32
> (PcdOvmfSecPeiTempRamBase) + FixedPcdGet32 (PcdOvmfSecPeiTempRamSize))
>
> %include "X64/TdxMetadata.asm"
> + %include "Ia32/Init32.asm"
> + %include "Ia32/InitTdx.asm"
> + %include "Ia32/ReloadFlat32.asm"
>
> %include "Ia32/Flat32ToFlat64.asm"
> %include "Ia32/PageTables64.asm"
> --
> 2.29.2.windows.2
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#78151): https://edk2.groups.io/g/devel/message/78151
Mute This Topic: https://groups.io/mt/84373830/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