Re: [edk2-devel] [PATCH v3 5/6] OvmfPkg/ResetVector: add 5-level paging support
On 2/20/24 03:06, Gerd Hoffmann wrote: Compile the OVMF ResetVector with 5-level paging support in case PcdUse5LevelPageTable is TRUE. When enabled the ResetVector will check at runtime whenever support for 5-level paging and gigabyte pages is available. In case both features are supported it will run OVMF in 5-level paging mode, otherwise fallback to 4-level paging. Gigabyte pages are required to make sure we can fit the page tables into the available space. We have six pages available, four of them are used. The first gibabyte is mapped with 2M pages, the 1GB -> 4GB range uses gigabyte pages. See the source code comment for the exact layout. In case TDX is used the TDX_WORK_AREA_PGTBL_READY will carry the information whenever 5-level paging is used (2) or not (1), so the APs can pick the correct paging mode. Signed-off-by: Gerd Hoffmann --- OvmfPkg/ResetVector/ResetVector.inf | 1 + OvmfPkg/ResetVector/Ia32/IntelTdx.asm | 17 ++- OvmfPkg/ResetVector/Ia32/PageTables64.asm | 131 +- OvmfPkg/ResetVector/ResetVector.nasmb | 1 + 4 files changed, 145 insertions(+), 5 deletions(-) diff --git a/OvmfPkg/ResetVector/ResetVector.inf b/OvmfPkg/ResetVector/ResetVector.inf index a4154ca90c28..65f71b05a02e 100644 --- a/OvmfPkg/ResetVector/ResetVector.inf +++ b/OvmfPkg/ResetVector/ResetVector.inf @@ -64,3 +64,4 @@ [FixedPcd] gUefiOvmfPkgTokenSpaceGuid.PcdQemuHashTableSize gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsBase gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsSize + gEfiMdeModulePkgTokenSpaceGuid.PcdUse5LevelPageTable diff --git a/OvmfPkg/ResetVector/Ia32/IntelTdx.asm b/OvmfPkg/ResetVector/Ia32/IntelTdx.asm index 06794baef81d..3e50ca76aacf 100644 --- a/OvmfPkg/ResetVector/Ia32/IntelTdx.asm +++ b/OvmfPkg/ResetVector/Ia32/IntelTdx.asm @@ -179,7 +179,7 @@ InitTdx: ; ; Modified: EAX, EDX ; -; 0-NonTdx, 1-TdxBsp, 2-TdxAps +; 0-NonTdx, 1-TdxBsp, 2-TdxAps, 3-TdxApsLa57 ; CheckTdxFeaturesBeforeBuildPagetables: xor eax, eax @@ -204,6 +204,21 @@ TdxPostBuildPageTables: ExitTdxPostBuildPageTables: OneTimeCallRet TdxPostBuildPageTables +%if PG_5_LEVEL + +; +; Set byte[TDX_WORK_AREA_PGTBL_READY] to 2 +; +TdxPostBuildPageTablesLa57: +cmp byte[WORK_AREA_GUEST_TYPE], VM_GUEST_TDX +jne ExitTdxPostBuildPageTablesLa57 +mov byte[TDX_WORK_AREA_PGTBL_READY], 2 + +ExitTdxPostBuildPageTablesLa57: +OneTimeCallRet TdxPostBuildPageTablesLa57 + +%endif + ; ; Check if TDX is enabled ; diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm b/OvmfPkg/ResetVector/Ia32/PageTables64.asm index 6fec6f2beeea..21de75a40097 100644 --- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm +++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm @@ -42,8 +42,10 @@ BITS32 PAGE_READ_WRITE + \ PAGE_PRESENT) +%define NOT_TDX 0 %define TDX_BSP 1 %define TDX_AP 2 +%define TDX_AP_LA57 3 ; ; Modified: EAX, EBX, ECX, EDX @@ -55,11 +57,21 @@ SetCr3ForPageTables64: ; the page tables. APs will spin on until byte[TDX_WORK_AREA_PGTBL_READY] ; is set. OneTimeCall CheckTdxFeaturesBeforeBuildPagetables +cmp eax, NOT_TDX +jeCheckSev cmp eax, TDX_BSP jeClearOvmfPageTables +%if PG_5_LEVEL cmp eax, TDX_AP jeSetCr3 +; TDX_AP_LA57 -> set cr4.la57 +mov eax, cr4 +bts eax, 12 +mov cr4, eax +%endif +jmp SetCr3 +CheckSev: ; Check whether the SEV is active and populate the SevEsWorkArea OneTimeCall CheckSevFeatures @@ -86,6 +98,105 @@ clearPageTablesMemoryLoop: mov dword[ecx * 4 + PT_ADDR (0) - 4], eax loopclearPageTablesMemoryLoop +%if PG_5_LEVEL + +; save GetSevCBitMaskAbove31 result (cpuid changes edx) +mov edi, edx + +; check for cpuid leaf 0x07 +mov eax, 0x00 +cpuid +cmp eax, 0x07 +jb Paging4Lvl + +; check for la57 (aka 5-level paging) +mov eax, 0x07 +mov ecx, 0x00 +cpuid +bt ecx, 16 +jnc Paging4Lvl + +; check for cpuid leaf 0x8001 +mov eax, 0x8000 +cpuid +cmp eax, 0x8001 +jb Paging4Lvl + +; check for 1g pages +mov eax, 0x8001 +cpuid +bt edx, 26 +jnc Paging4Lvl + +; +; Use 5-level paging with gigabyte pages. +; +; We have 6 pages available for the early page tables, +; we use four of them: +;PT_ADDR(0) - level 5 directory +;PT_ADDR(0x1000) - level 4 directory +;PT_ADDR(0x2000) - level 2 directory (0 -> 1GB) +;PT_ADDR(0x3000) - level 3 directory +; +; The level 2 directory for the first gigabyte has the same +; physical address in both 4-level and 5-level paging mode, +; SevClearPageEncMaskForGhcbPage depends on this. +;
Re: [edk2-devel] [PATCH v3 5/6] OvmfPkg/ResetVector: add 5-level paging support
On 2/20/24 18:45, Laszlo Ersek wrote: > (I'm using quotes around "subroutines" and "call" because we don't have > a stack at this point yet, IIUC, so all our "one time calls" are > actually just normal jumps, with some NASM macro magic. That's fine, > we're only talking a handful of assembly instructions here, so > readability definitely trumps code path reuse. SECFV only contains > SecMain and ResetVector, and it's only 26% full -- 56K used, 152K free, > out of 208K total.) Sorry, those numbers were for IA32X64, which is not right for this discussion -- we need to look at X64. NOOPT GCC5 X64 is: SECFV [49%Full] 212992 (0x34000) total, 106000 (0x19e10) used, 106992 (0x1a1f0) free That should still be plenty roomy for disentangling all three code paths here. Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#115671): https://edk2.groups.io/g/devel/message/115671 Mute This Topic: https://groups.io/mt/104464309/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v3 5/6] OvmfPkg/ResetVector: add 5-level paging support
On 2/20/24 10:06, Gerd Hoffmann wrote:
> Compile the OVMF ResetVector with 5-level paging support in case
> PcdUse5LevelPageTable is TRUE.
>
> When enabled the ResetVector will check at runtime whenever support for
> 5-level paging and gigabyte pages is available. In case both features
> are supported it will run OVMF in 5-level paging mode, otherwise
> fallback to 4-level paging.
>
> Gigabyte pages are required to make sure we can fit the page tables into
> the available space. We have six pages available, four of them are
> used. The first gibabyte is mapped with 2M pages, the 1GB -> 4GB range
> uses gigabyte pages. See the source code comment for the exact layout.
>
> In case TDX is used the TDX_WORK_AREA_PGTBL_READY will carry the
> information whenever 5-level paging is used (2) or not (1), so the
> APs can pick the correct paging mode.
>
> Signed-off-by: Gerd Hoffmann
> ---
> OvmfPkg/ResetVector/ResetVector.inf | 1 +
> OvmfPkg/ResetVector/Ia32/IntelTdx.asm | 17 ++-
> OvmfPkg/ResetVector/Ia32/PageTables64.asm | 131 +-
> OvmfPkg/ResetVector/ResetVector.nasmb | 1 +
> 4 files changed, 145 insertions(+), 5 deletions(-)
I'm sorry, but this is awful. The stuff in "PageTables64.asm" is now the
definition of spaghetti code. I find it nearly impossible to follow the
code through the forest of jumps.
For example, we have a label called "PageTablesReady", but nothing jumps
to it.
At this point I'd much prefer if we *didn't* try to reuse common page
table building code in this fashion, between TDX, SEV, and no-CC. IMO it
would be better to factor the common code out to a new "subroutine", in
a separate file, and then create a minimal and succinct high-level file
that deals with nothing but control flow / feature detection between
TDX, SEV, and neither. The current code mixes that kind of feature
checking, 5-level paging macro checking, and actual page table building
/ CR configuration.
Basically I'm proposing to implement a high-level assembly file that
reads like the following C pseudo-code:
Val = CheckTdx ();
if (!Val) {
goto Sev;
}
SetupTdx ();
goto Done;
Sev:
Val = CheckSev ();
if (!Val) {
goto NoCC;
}
SetupSev ();
goto Done;
NoCC:
SetupNoCc ();
Done:
...
And then CheckTdx() and CheckSev() would be standalone, separate
"subroutines", and SetupTdx(), SetupSev(), SetupNoCc() *too* would be
standalone, separate "subroutines". If the latter three would like to do
some stuff commonly, then they should "call" common "sub-subroutines",
or if that's not possible, then -- for all I care -- even *triplicate*
the common code, using NASM macros!
(I'm using quotes around "subroutines" and "call" because we don't have
a stack at this point yet, IIUC, so all our "one time calls" are
actually just normal jumps, with some NASM macro magic. That's fine,
we're only talking a handful of assembly instructions here, so
readability definitely trumps code path reuse. SECFV only contains
SecMain and ResetVector, and it's only 26% full -- 56K used, 152K free,
out of 208K total.)
The prime candidates for those "sub-subroutines" (or macros) are
"ClearOvmfPageTables", the 5-level page tree population, and the 4-level
page tree population.
Because, again, this level of code reuse, while I'm sure is brilliant,
functionally correct, and frugal with reset vector footprint, is also
super hard to read and maintain.
That's my opinion anyway.
Laszlo
>
> diff --git a/OvmfPkg/ResetVector/ResetVector.inf
> b/OvmfPkg/ResetVector/ResetVector.inf
> index a4154ca90c28..65f71b05a02e 100644
> --- a/OvmfPkg/ResetVector/ResetVector.inf
> +++ b/OvmfPkg/ResetVector/ResetVector.inf
> @@ -64,3 +64,4 @@ [FixedPcd]
>gUefiOvmfPkgTokenSpaceGuid.PcdQemuHashTableSize
>gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsBase
>gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsSize
> + gEfiMdeModulePkgTokenSpaceGuid.PcdUse5LevelPageTable
> diff --git a/OvmfPkg/ResetVector/Ia32/IntelTdx.asm
> b/OvmfPkg/ResetVector/Ia32/IntelTdx.asm
> index 06794baef81d..3e50ca76aacf 100644
> --- a/OvmfPkg/ResetVector/Ia32/IntelTdx.asm
> +++ b/OvmfPkg/ResetVector/Ia32/IntelTdx.asm
> @@ -179,7 +179,7 @@ InitTdx:
> ;
> ; Modified: EAX, EDX
> ;
> -; 0-NonTdx, 1-TdxBsp, 2-TdxAps
> +; 0-NonTdx, 1-TdxBsp, 2-TdxAps, 3-TdxApsLa57
> ;
> CheckTdxFeaturesBeforeBuildPagetables:
> xor eax, eax
> @@ -204,6 +204,21 @@ TdxPostBuildPageTables:
> ExitTdxPostBuildPageTables:
> OneTimeCallRet TdxPostBuildPageTables
>
> +%if PG_5_LEVEL
> +
> +;
> +; Set byte[TDX_WORK_AREA_PGTBL_READY] to 2
> +;
> +TdxPostBuildPageTablesLa57:
> +cmp byte[WORK_AREA_GUEST_TYPE], VM_GUEST_TDX
> +jne ExitTdxPostBuildPageTablesLa57
> +mov byte[TDX_WORK_AREA_PGTBL_READY], 2
> +
> +ExitTdxPostBuildPageTablesLa57:
> +OneTimeCallRet TdxPostBuildPageTablesLa57
> +
> +%endif
> +
> ;
> ; Check if TDX is enabled
> ;
> diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm
>
[edk2-devel] [PATCH v3 5/6] OvmfPkg/ResetVector: add 5-level paging support
Compile the OVMF ResetVector with 5-level paging support in case PcdUse5LevelPageTable is TRUE. When enabled the ResetVector will check at runtime whenever support for 5-level paging and gigabyte pages is available. In case both features are supported it will run OVMF in 5-level paging mode, otherwise fallback to 4-level paging. Gigabyte pages are required to make sure we can fit the page tables into the available space. We have six pages available, four of them are used. The first gibabyte is mapped with 2M pages, the 1GB -> 4GB range uses gigabyte pages. See the source code comment for the exact layout. In case TDX is used the TDX_WORK_AREA_PGTBL_READY will carry the information whenever 5-level paging is used (2) or not (1), so the APs can pick the correct paging mode. Signed-off-by: Gerd Hoffmann --- OvmfPkg/ResetVector/ResetVector.inf | 1 + OvmfPkg/ResetVector/Ia32/IntelTdx.asm | 17 ++- OvmfPkg/ResetVector/Ia32/PageTables64.asm | 131 +- OvmfPkg/ResetVector/ResetVector.nasmb | 1 + 4 files changed, 145 insertions(+), 5 deletions(-) diff --git a/OvmfPkg/ResetVector/ResetVector.inf b/OvmfPkg/ResetVector/ResetVector.inf index a4154ca90c28..65f71b05a02e 100644 --- a/OvmfPkg/ResetVector/ResetVector.inf +++ b/OvmfPkg/ResetVector/ResetVector.inf @@ -64,3 +64,4 @@ [FixedPcd] gUefiOvmfPkgTokenSpaceGuid.PcdQemuHashTableSize gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsBase gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsSize + gEfiMdeModulePkgTokenSpaceGuid.PcdUse5LevelPageTable diff --git a/OvmfPkg/ResetVector/Ia32/IntelTdx.asm b/OvmfPkg/ResetVector/Ia32/IntelTdx.asm index 06794baef81d..3e50ca76aacf 100644 --- a/OvmfPkg/ResetVector/Ia32/IntelTdx.asm +++ b/OvmfPkg/ResetVector/Ia32/IntelTdx.asm @@ -179,7 +179,7 @@ InitTdx: ; ; Modified: EAX, EDX ; -; 0-NonTdx, 1-TdxBsp, 2-TdxAps +; 0-NonTdx, 1-TdxBsp, 2-TdxAps, 3-TdxApsLa57 ; CheckTdxFeaturesBeforeBuildPagetables: xor eax, eax @@ -204,6 +204,21 @@ TdxPostBuildPageTables: ExitTdxPostBuildPageTables: OneTimeCallRet TdxPostBuildPageTables +%if PG_5_LEVEL + +; +; Set byte[TDX_WORK_AREA_PGTBL_READY] to 2 +; +TdxPostBuildPageTablesLa57: +cmp byte[WORK_AREA_GUEST_TYPE], VM_GUEST_TDX +jne ExitTdxPostBuildPageTablesLa57 +mov byte[TDX_WORK_AREA_PGTBL_READY], 2 + +ExitTdxPostBuildPageTablesLa57: +OneTimeCallRet TdxPostBuildPageTablesLa57 + +%endif + ; ; Check if TDX is enabled ; diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm b/OvmfPkg/ResetVector/Ia32/PageTables64.asm index 6fec6f2beeea..21de75a40097 100644 --- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm +++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm @@ -42,8 +42,10 @@ BITS32 PAGE_READ_WRITE + \ PAGE_PRESENT) +%define NOT_TDX 0 %define TDX_BSP 1 %define TDX_AP 2 +%define TDX_AP_LA57 3 ; ; Modified: EAX, EBX, ECX, EDX @@ -55,11 +57,21 @@ SetCr3ForPageTables64: ; the page tables. APs will spin on until byte[TDX_WORK_AREA_PGTBL_READY] ; is set. OneTimeCall CheckTdxFeaturesBeforeBuildPagetables +cmp eax, NOT_TDX +jeCheckSev cmp eax, TDX_BSP jeClearOvmfPageTables +%if PG_5_LEVEL cmp eax, TDX_AP jeSetCr3 +; TDX_AP_LA57 -> set cr4.la57 +mov eax, cr4 +bts eax, 12 +mov cr4, eax +%endif +jmp SetCr3 +CheckSev: ; Check whether the SEV is active and populate the SevEsWorkArea OneTimeCall CheckSevFeatures @@ -86,6 +98,105 @@ clearPageTablesMemoryLoop: mov dword[ecx * 4 + PT_ADDR (0) - 4], eax loopclearPageTablesMemoryLoop +%if PG_5_LEVEL + +; save GetSevCBitMaskAbove31 result (cpuid changes edx) +mov edi, edx + +; check for cpuid leaf 0x07 +mov eax, 0x00 +cpuid +cmp eax, 0x07 +jb Paging4Lvl + +; check for la57 (aka 5-level paging) +mov eax, 0x07 +mov ecx, 0x00 +cpuid +bt ecx, 16 +jnc Paging4Lvl + +; check for cpuid leaf 0x8001 +mov eax, 0x8000 +cpuid +cmp eax, 0x8001 +jb Paging4Lvl + +; check for 1g pages +mov eax, 0x8001 +cpuid +bt edx, 26 +jnc Paging4Lvl + +; +; Use 5-level paging with gigabyte pages. +; +; We have 6 pages available for the early page tables, +; we use four of them: +;PT_ADDR(0) - level 5 directory +;PT_ADDR(0x1000) - level 4 directory +;PT_ADDR(0x2000) - level 2 directory (0 -> 1GB) +;PT_ADDR(0x3000) - level 3 directory +; +; The level 2 directory for the first gigabyte has the same +; physical address in both 4-level and 5-level paging mode, +; SevClearPageEncMaskForGhcbPage depends on this. +; +; The 1 GB -> 4 GB range is mapped using 1G pages in the +; level 3 directory. +
