Re: [edk2-devel] [PATCH v3 5/6] OvmfPkg/ResetVector: add 5-level paging support

["Lendacky, Thomas via groups.io" <thomas.lendacky=amd.com@groups.io>]

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 <kraxel@redhat.com>
> ---
>   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 @@ BITS    32
>                                    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
> +    je        CheckSev
>       cmp       eax, TDX_BSP
>       je        ClearOvmfPageTables
> +%if PG_5_LEVEL
>       cmp       eax, TDX_AP
>       je        SetCr3
> +    ; 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
>       loop    clearPageTablesMemoryLoop
>   
> +%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 0x80000001
> +    mov     eax, 0x80000000
> +    cpuid
> +    cmp     eax, 0x80000001
> +    jb      Paging4Lvl
> +
> +    ; check for 1g pages
> +    mov     eax, 0x80000001
> +    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.
> +    ;
> +    debugShowPostCode 0x51      ; 5-level paging
> +
> +    ; restore GetSevCBitMaskAbove31 result
> +    mov     edx, edi
> +
> +    ; level 5
> +    mov     dword[PT_ADDR (0)], PT_ADDR (0x1000) + PAGE_PDE_DIRECTORY_ATTR
> +    mov     dword[PT_ADDR (4)], edx
> +
> +    ; level 4
> +    mov     dword[PT_ADDR (0x1000)], PT_ADDR (0x3000) + PAGE_PDE_DIRECTORY_ATTR
> +    mov     dword[PT_ADDR (0x1004)], edx
> +
> +    ; level 3 (1x -> level 2, 3x 1GB)
> +    mov     dword[PT_ADDR (0x3000)], PT_ADDR (0x2000) + PAGE_PDE_DIRECTORY_ATTR
> +    mov     dword[PT_ADDR (0x3004)], edx
> +    mov     dword[PT_ADDR (0x3008)], (1 << 30) + PAGE_PDE_LARGEPAGE_ATTR
> +    mov     dword[PT_ADDR (0x300c)], edx
> +    mov     dword[PT_ADDR (0x3010)], (2 << 30) + PAGE_PDE_LARGEPAGE_ATTR
> +    mov     dword[PT_ADDR (0x3014)], edx
> +    mov     dword[PT_ADDR (0x3018)], (3 << 30) + PAGE_PDE_LARGEPAGE_ATTR
> +    mov     dword[PT_ADDR (0x301c)], edx
> +
> +    ;
> +    ; level 2 (512 * 2MB entries => 1GB)
> +    ;
> +    mov     ecx, 0x200
> +pageTableEntriesLoopLa57:
> +    mov     eax, ecx
> +    dec     eax
> +    shl     eax, 21
> +    add     eax, PAGE_PDE_LARGEPAGE_ATTR
> +    mov     [ecx * 8 + PT_ADDR (0x2000 - 8)], eax
> +    mov     [(ecx * 8 + PT_ADDR (0x2000 - 8)) + 4], edx
> +    loop    pageTableEntriesLoopLa57
> +
> +    ; set la57 bit in cr4
> +    mov     eax, cr4
> +    bts     eax, 12
> +    mov     cr4, eax
> +
> +    ; done
> +    jmp     PageTablesReadyLa57
> +
> +Paging4Lvl:
> +    debugShowPostCode 0x41      ; 4-level paging
> +
> +    ; restore GetSevCBitMaskAbove31 result
> +    mov     edx, edi
> +
> +%endif ; PG_5_LEVEL
> +
>       ;
>       ; Top level Page Directory Pointers (1 * 512GB entry)
>       ;
> @@ -117,13 +228,25 @@ pageTableEntriesLoop:
>       mov     [(ecx * 8 + PT_ADDR (0x2000 - 8)) + 4], edx
>       loop    pageTableEntriesLoop

I think there is supposed to be a jmp PageTablesReady here so that you 
don't accidentally end up at PageTablesReadyLa57.

or...

>   
> +%if PG_5_LEVEL
> +
> +PageTablesReadyLa57:
> +    ; TDX will do some PostBuildPages task, such as setting
> +    ; byte[TDX_WORK_AREA_PGTBL_READY].
> +    OneTimeCall   TdxPostBuildPageTablesLa57
> +    jmp SevPostBuildPageTables
> +
> +%endif

Move this block just before the "jmp PageTablesReadyLa57"

> +
> +PageTablesReady:
> +    ; TDX will do some PostBuildPages task, such as setting
> +    ; byte[TDX_WORK_AREA_PGTBL_READY].
> +    OneTimeCall   TdxPostBuildPageTables

And this block would end up just after the "loop pageEntriesLoop" 
statement and you can get rid of the PageTablesReady label.

Thanks,
Tom

> +
> +SevPostBuildPageTables:
>       ; Clear the C-bit from the GHCB page if the SEV-ES is enabled.
>       OneTimeCall   SevClearPageEncMaskForGhcbPage
>   
> -    ; TDX will do some PostBuildPages task, such as setting
> -    ; byte[TDX_WORK_AREA_PGTBL_READY].
> -    OneTimeCall   TdxPostBuildPageTables
> -
>   SetCr3:
>       ;
>       ; Set CR3 now that the paging structures are available
> diff --git a/OvmfPkg/ResetVector/ResetVector.nasmb b/OvmfPkg/ResetVector/ResetVector.nasmb
> index 366a70fb9992..2bd80149e58b 100644
> --- a/OvmfPkg/ResetVector/ResetVector.nasmb
> +++ b/OvmfPkg/ResetVector/ResetVector.nasmb
> @@ -53,6 +53,7 @@
>   
>   %define WORK_AREA_GUEST_TYPE          (FixedPcdGet32 (PcdOvmfWorkAreaBase))
>   %define PT_ADDR(Offset)               (FixedPcdGet32 (PcdOvmfSecPageTablesBase) + (Offset))
> +%define PG_5_LEVEL                    (FixedPcdGetBool (PcdUse5LevelPageTable))
>   
>   %define GHCB_PT_ADDR                  (FixedPcdGet32 (PcdOvmfSecGhcbPageTableBase))
>   %define GHCB_BASE                     (FixedPcdGet32 (PcdOvmfSecGhcbBase))


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115681): https://edk2.groups.io/g/devel/message/115681
Mute This Topic: https://groups.io/mt/104464309/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-