public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [edk2-devel] [PATCH v3 0/6] OvmfPkg: Add support for 5-level paging
@ 2024-02-20  9:06 Gerd Hoffmann
  2024-02-20  9:06 ` [edk2-devel] [PATCH v3 1/6] MdeModulePkg/DxeIplPeim: fix PcdUse5LevelPageTable assert Gerd Hoffmann
                   ` (7 more replies)
  0 siblings, 8 replies; 14+ messages in thread
From: Gerd Hoffmann @ 2024-02-20  9:06 UTC (permalink / raw)
  To: devel
  Cc: Liming Gao, Michael Roth, Oliver Steffen, Erdem Aktas,
	Tom Lendacky, Laszlo Ersek, Min Xu, Ard Biesheuvel, Jiewen Yao,
	Gerd Hoffmann

Patch #1 + #2 fix MdeModulePkg/DxeIplPeim to not assert in case a
5-level enabled build runs in 4-level paging mode.

Patch #2 - #4 update OvmfPkg ResetVector, adding support for 5-level
paging (setup 5-level page tables in case both la57 and gigabyte pages
are supported by the vCPU).

Patch #5 updates PlatformInitLib for 5-level paging support (update
PhysBits calculation).

Known issues / limitations:
 - BaseMemEncryptSevLib must be updated to also support 5-level
   paging for full 5-level paging support in SEV mode.

The patch series does *not* enable 5-level paging by default.
Building with 5-level paging support can be done by compiling
OVMF with '--pcd PcdUse5LevelPageTable=TRUE'.

v3:
 - add resetvector fixes for sev and tdx
v2 changes:
 - fix sev/tdx handling with 5-level paging.
 - more comments for 5-level page table setup.
 - improve PAGE_* naming (new patch #3).
 - rename Page5LevelSupported to Page5LevelEnabled (new patch #2).
 - pick up some review tags.

Gerd Hoffmann (6):
  MdeModulePkg/DxeIplPeim: fix PcdUse5LevelPageTable assert
  MdeModulePkg/DxeIplPeim: rename variable
  OvmfPkg/ResetVector: improve page table flag names
  OvmfPkg/ResetVector: SEV: keep #vc handler installed longer
  OvmfPkg/ResetVector: add 5-level paging support
  OvmfPkg/PlatformInitLib: add 5-level paging support

 OvmfPkg/ResetVector/ResetVector.inf           |   1 +
 .../Core/DxeIplPeim/X64/VirtualMemory.c       |  24 +--
 OvmfPkg/Library/PlatformInitLib/MemDetect.c   |  57 ++++--
 OvmfPkg/ResetVector/Ia32/AmdSev.asm           |   7 +-
 OvmfPkg/ResetVector/Ia32/IntelTdx.asm         |  17 +-
 OvmfPkg/ResetVector/Ia32/PageTables64.asm     | 170 +++++++++++++++---
 OvmfPkg/ResetVector/ResetVector.nasmb         |   1 +
 7 files changed, 224 insertions(+), 53 deletions(-)

-- 
2.43.2



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



^ permalink raw reply	[flat|nested] 14+ messages in thread

* [edk2-devel] [PATCH v3 1/6] MdeModulePkg/DxeIplPeim: fix PcdUse5LevelPageTable assert
  2024-02-20  9:06 [edk2-devel] [PATCH v3 0/6] OvmfPkg: Add support for 5-level paging Gerd Hoffmann
@ 2024-02-20  9:06 ` Gerd Hoffmann
  2024-02-20  9:06 ` [edk2-devel] [PATCH v3 2/6] MdeModulePkg/DxeIplPeim: rename variable Gerd Hoffmann
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Gerd Hoffmann @ 2024-02-20  9:06 UTC (permalink / raw)
  To: devel
  Cc: Liming Gao, Michael Roth, Oliver Steffen, Erdem Aktas,
	Tom Lendacky, Laszlo Ersek, Min Xu, Ard Biesheuvel, Jiewen Yao,
	Gerd Hoffmann

PcdUse5LevelPageTable documentation says:

  Indicates if 5-Level Paging will be enabled in long mode. 5-Level
  Paging will not be enabled when the PCD is TRUE but CPU doesn't support
  5-Level Paging.

So running in 4-level paging mode with PcdUse5LevelPageTable=TRUE is
possible.  The only invalid combination is 5-level paging being active
with PcdUse5LevelPageTable=FALSE.

Fix the ASSERT accordingly.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
 MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
index 980c2002d4f5..1d240e95966e 100644
--- a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
+++ b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
@@ -745,7 +745,9 @@ CreateIdentityMappingPageTables (
     //
     Cr4.UintN         = AsmReadCr4 ();
     Page5LevelSupport = (Cr4.Bits.LA57 != 0);
-    ASSERT (PcdGetBool (PcdUse5LevelPageTable) == Page5LevelSupport);
+    if (Page5LevelSupport) {
+      ASSERT (PcdGetBool (PcdUse5LevelPageTable));
+    }
   } else {
     //
     // If cpu runs in 32bit protected mode PEI, Page table Level in DXE is decided by PCD and feature capability.
-- 
2.43.2



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



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [edk2-devel] [PATCH v3 2/6] MdeModulePkg/DxeIplPeim: rename variable
  2024-02-20  9:06 [edk2-devel] [PATCH v3 0/6] OvmfPkg: Add support for 5-level paging Gerd Hoffmann
  2024-02-20  9:06 ` [edk2-devel] [PATCH v3 1/6] MdeModulePkg/DxeIplPeim: fix PcdUse5LevelPageTable assert Gerd Hoffmann
@ 2024-02-20  9:06 ` Gerd Hoffmann
  2024-02-20  9:06 ` [edk2-devel] [PATCH v3 3/6] OvmfPkg/ResetVector: improve page table flag names Gerd Hoffmann
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Gerd Hoffmann @ 2024-02-20  9:06 UTC (permalink / raw)
  To: devel
  Cc: Liming Gao, Michael Roth, Oliver Steffen, Erdem Aktas,
	Tom Lendacky, Laszlo Ersek, Min Xu, Ard Biesheuvel, Jiewen Yao,
	Gerd Hoffmann

Rename Page5LevelSupported to Page5LevelEnabled.

The variable is set to true in case 5-paging level is enabled (64-bit
PEI) or will be enabled (32-bit PEI), it does *not* tell whenever the
5-level paging is supported by the CPU.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
 .../Core/DxeIplPeim/X64/VirtualMemory.c       | 22 +++++++++----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
index 1d240e95966e..df6196a41cd5 100644
--- a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
+++ b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
@@ -696,7 +696,7 @@ CreateIdentityMappingPageTables (
   UINTN                                        TotalPagesNum;
   UINTN                                        BigPageAddress;
   VOID                                         *Hob;
-  BOOLEAN                                      Page5LevelSupport;
+  BOOLEAN                                      Page5LevelEnabled;
   BOOLEAN                                      Page1GSupport;
   PAGE_TABLE_1G_ENTRY                          *PageDirectory1GEntry;
   UINT64                                       AddressEncMask;
@@ -744,15 +744,15 @@ CreateIdentityMappingPageTables (
     // If cpu has already run in 64bit long mode PEI, Page table Level in DXE must align with previous level.
     //
     Cr4.UintN         = AsmReadCr4 ();
-    Page5LevelSupport = (Cr4.Bits.LA57 != 0);
-    if (Page5LevelSupport) {
+    Page5LevelEnabled = (Cr4.Bits.LA57 != 0);
+    if (Page5LevelEnabled) {
       ASSERT (PcdGetBool (PcdUse5LevelPageTable));
     }
   } else {
     //
     // If cpu runs in 32bit protected mode PEI, Page table Level in DXE is decided by PCD and feature capability.
     //
-    Page5LevelSupport = FALSE;
+    Page5LevelEnabled = FALSE;
     if (PcdGetBool (PcdUse5LevelPageTable)) {
       AsmCpuidEx (
         CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS,
@@ -763,12 +763,12 @@ CreateIdentityMappingPageTables (
         NULL
         );
       if (EcxFlags.Bits.FiveLevelPage != 0) {
-        Page5LevelSupport = TRUE;
+        Page5LevelEnabled = TRUE;
       }
     }
   }
 
-  DEBUG ((DEBUG_INFO, "AddressBits=%u 5LevelPaging=%u 1GPage=%u\n", PhysicalAddressBits, Page5LevelSupport, Page1GSupport));
+  DEBUG ((DEBUG_INFO, "AddressBits=%u 5LevelPaging=%u 1GPage=%u\n", PhysicalAddressBits, Page5LevelEnabled, Page1GSupport));
 
   //
   // IA-32e paging translates 48-bit linear addresses to 52-bit physical addresses
@@ -776,7 +776,7 @@ CreateIdentityMappingPageTables (
   //  due to either unsupported by HW, or disabled by PCD.
   //
   ASSERT (PhysicalAddressBits <= 52);
-  if (!Page5LevelSupport && (PhysicalAddressBits > 48)) {
+  if (!Page5LevelEnabled && (PhysicalAddressBits > 48)) {
     PhysicalAddressBits = 48;
   }
 
@@ -811,7 +811,7 @@ CreateIdentityMappingPageTables (
   //
   // Substract the one page occupied by PML5 entries if 5-Level Paging is disabled.
   //
-  if (!Page5LevelSupport) {
+  if (!Page5LevelEnabled) {
     TotalPagesNum--;
   }
 
@@ -831,7 +831,7 @@ CreateIdentityMappingPageTables (
   // By architecture only one PageMapLevel4 exists - so lets allocate storage for it.
   //
   PageMap = (VOID *)BigPageAddress;
-  if (Page5LevelSupport) {
+  if (Page5LevelEnabled) {
     //
     // By architecture only one PageMapLevel5 exists - so lets allocate storage for it.
     //
@@ -853,7 +853,7 @@ CreateIdentityMappingPageTables (
     PageMapLevel4Entry = (VOID *)BigPageAddress;
     BigPageAddress    += SIZE_4KB;
 
-    if (Page5LevelSupport) {
+    if (Page5LevelEnabled) {
       //
       // Make a PML5 Entry
       //
@@ -947,7 +947,7 @@ CreateIdentityMappingPageTables (
     ZeroMem (PageMapLevel4Entry, (512 - IndexOfPml4Entries) * sizeof (PAGE_MAP_AND_DIRECTORY_POINTER));
   }
 
-  if (Page5LevelSupport) {
+  if (Page5LevelEnabled) {
     Cr4.UintN     = AsmReadCr4 ();
     Cr4.Bits.LA57 = 1;
     AsmWriteCr4 (Cr4.UintN);
-- 
2.43.2



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



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [edk2-devel] [PATCH v3 3/6] OvmfPkg/ResetVector: improve page table flag names
  2024-02-20  9:06 [edk2-devel] [PATCH v3 0/6] OvmfPkg: Add support for 5-level paging Gerd Hoffmann
  2024-02-20  9:06 ` [edk2-devel] [PATCH v3 1/6] MdeModulePkg/DxeIplPeim: fix PcdUse5LevelPageTable assert Gerd Hoffmann
  2024-02-20  9:06 ` [edk2-devel] [PATCH v3 2/6] MdeModulePkg/DxeIplPeim: rename variable Gerd Hoffmann
@ 2024-02-20  9:06 ` Gerd Hoffmann
  2024-02-20  9:06 ` [edk2-devel] [PATCH v3 4/6] OvmfPkg/ResetVector: SEV: keep #vc handler installed longer Gerd Hoffmann
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Gerd Hoffmann @ 2024-02-20  9:06 UTC (permalink / raw)
  To: devel
  Cc: Liming Gao, Michael Roth, Oliver Steffen, Erdem Aktas,
	Tom Lendacky, Laszlo Ersek, Min Xu, Ard Biesheuvel, Jiewen Yao,
	Gerd Hoffmann

Add comments, rename some of the PAGE_* flags and combined attributes.
Specifically use "LARGEPAGE" instead of "2M" because that bit is used
for both 2M and 1G large pages.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
 OvmfPkg/ResetVector/Ia32/PageTables64.asm | 39 +++++++++++++----------
 1 file changed, 22 insertions(+), 17 deletions(-)

diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
index 317cad430f29..6fec6f2beeea 100644
--- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm
+++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
@@ -10,6 +10,7 @@
 
 BITS    32
 
+; common for all levels
 %define PAGE_PRESENT            0x01
 %define PAGE_READ_WRITE         0x02
 %define PAGE_USER_SUPERVISOR    0x04
@@ -17,25 +18,29 @@ BITS    32
 %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
+
+; page table entries (level 1)
+%define PAGE_PTE_PAT           0x080
+
+; page directory entries (level 2+)
+%define PAGE_PDE_LARGEPAGE     0x080
+%define PAGE_PDE_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 + \
-                          PAGE_READ_WRITE + \
-                          PAGE_PRESENT)
+%define PAGE_PDE_LARGEPAGE_ATTR (PAGE_PDE_LARGEPAGE + \
+                                 PAGE_ACCESSED + \
+                                 PAGE_DIRTY + \
+                                 PAGE_READ_WRITE + \
+                                 PAGE_PRESENT)
 
-%define PAGE_PDP_ATTR (PAGE_ACCESSED + \
-                       PAGE_READ_WRITE + \
-                       PAGE_PRESENT)
+%define PAGE_PDE_DIRECTORY_ATTR (PAGE_ACCESSED + \
+                                 PAGE_READ_WRITE + \
+                                 PAGE_PRESENT)
 
 %define TDX_BSP         1
 %define TDX_AP          2
@@ -84,19 +89,19 @@ clearPageTablesMemoryLoop:
     ;
     ; Top level Page Directory Pointers (1 * 512GB entry)
     ;
-    mov     dword[PT_ADDR (0)], PT_ADDR (0x1000) + PAGE_PDP_ATTR
+    mov     dword[PT_ADDR (0)], PT_ADDR (0x1000) + PAGE_PDE_DIRECTORY_ATTR
     mov     dword[PT_ADDR (4)], edx
 
     ;
     ; Next level Page Directory Pointers (4 * 1GB entries => 4GB)
     ;
-    mov     dword[PT_ADDR (0x1000)], PT_ADDR (0x2000) + PAGE_PDP_ATTR
+    mov     dword[PT_ADDR (0x1000)], PT_ADDR (0x2000) + PAGE_PDE_DIRECTORY_ATTR
     mov     dword[PT_ADDR (0x1004)], edx
-    mov     dword[PT_ADDR (0x1008)], PT_ADDR (0x3000) + PAGE_PDP_ATTR
+    mov     dword[PT_ADDR (0x1008)], PT_ADDR (0x3000) + PAGE_PDE_DIRECTORY_ATTR
     mov     dword[PT_ADDR (0x100C)], edx
-    mov     dword[PT_ADDR (0x1010)], PT_ADDR (0x4000) + PAGE_PDP_ATTR
+    mov     dword[PT_ADDR (0x1010)], PT_ADDR (0x4000) + PAGE_PDE_DIRECTORY_ATTR
     mov     dword[PT_ADDR (0x1014)], edx
-    mov     dword[PT_ADDR (0x1018)], PT_ADDR (0x5000) + PAGE_PDP_ATTR
+    mov     dword[PT_ADDR (0x1018)], PT_ADDR (0x5000) + PAGE_PDE_DIRECTORY_ATTR
     mov     dword[PT_ADDR (0x101C)], edx
 
     ;
@@ -107,7 +112,7 @@ pageTableEntriesLoop:
     mov     eax, ecx
     dec     eax
     shl     eax, 21
-    add     eax, PAGE_2M_PDE_ATTR
+    add     eax, PAGE_PDE_LARGEPAGE_ATTR
     mov     [ecx * 8 + PT_ADDR (0x2000 - 8)], eax
     mov     [(ecx * 8 + PT_ADDR (0x2000 - 8)) + 4], edx
     loop    pageTableEntriesLoop
-- 
2.43.2



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



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [edk2-devel] [PATCH v3 4/6] OvmfPkg/ResetVector: SEV: keep #vc handler installed longer
  2024-02-20  9:06 [edk2-devel] [PATCH v3 0/6] OvmfPkg: Add support for 5-level paging Gerd Hoffmann
                   ` (2 preceding siblings ...)
  2024-02-20  9:06 ` [edk2-devel] [PATCH v3 3/6] OvmfPkg/ResetVector: improve page table flag names Gerd Hoffmann
@ 2024-02-20  9:06 ` Gerd Hoffmann
  2024-02-20 16:48   ` Laszlo Ersek
  2024-02-20 19:56   ` Lendacky, Thomas via groups.io
  2024-02-20  9:06 ` [edk2-devel] [PATCH v3 5/6] OvmfPkg/ResetVector: add 5-level paging support Gerd Hoffmann
                   ` (3 subsequent siblings)
  7 siblings, 2 replies; 14+ messages in thread
From: Gerd Hoffmann @ 2024-02-20  9:06 UTC (permalink / raw)
  To: devel
  Cc: Liming Gao, Michael Roth, Oliver Steffen, Erdem Aktas,
	Tom Lendacky, Laszlo Ersek, Min Xu, Ard Biesheuvel, Jiewen Yao,
	Gerd Hoffmann

When running in SEV mode do not uninstall the #vc handler in
CheckSevFeatures.   Keep it active and uninstall it later in
SevClearPageEncMaskForGhcbPage.

This allows using the cpuid instruction in SetCr3ForPageTables64,
which is needed to check for la57 & 1G page support.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 OvmfPkg/ResetVector/Ia32/AmdSev.asm | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/OvmfPkg/ResetVector/Ia32/AmdSev.asm b/OvmfPkg/ResetVector/Ia32/AmdSev.asm
index 043c88a7abbe..02f287f1d934 100644
--- a/OvmfPkg/ResetVector/Ia32/AmdSev.asm
+++ b/OvmfPkg/ResetVector/Ia32/AmdSev.asm
@@ -158,6 +158,11 @@ SevClearPageEncMaskForGhcbPage:
     cmp       byte[WORK_AREA_GUEST_TYPE], 1
     jnz       SevClearPageEncMaskForGhcbPageExit
 
+    ; Clear exception handlers and stack
+    mov       eax, ADDR_OF(IdtrClear)
+    lidt      [cs:eax]
+    mov       esp, 0
+
     ; Check if SEV-ES is enabled
     mov       ecx, 1
     bt        [SEV_ES_WORK_AREA_STATUS_MSR], ecx
@@ -332,7 +337,6 @@ NoSevEsVcHlt:
 NoSevPass:
     xor       eax, eax
 
-SevExit:
     ;
     ; Clear exception handlers and stack
     ;
@@ -342,6 +346,7 @@ SevExit:
     pop       eax
     mov       esp, 0
 
+SevExit:
     OneTimeCallRet CheckSevFeatures
 
 ; Start of #VC exception handling routines
-- 
2.43.2



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



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [edk2-devel] [PATCH v3 5/6] OvmfPkg/ResetVector: add 5-level paging support
  2024-02-20  9:06 [edk2-devel] [PATCH v3 0/6] OvmfPkg: Add support for 5-level paging Gerd Hoffmann
                   ` (3 preceding siblings ...)
  2024-02-20  9:06 ` [edk2-devel] [PATCH v3 4/6] OvmfPkg/ResetVector: SEV: keep #vc handler installed longer Gerd Hoffmann
@ 2024-02-20  9:06 ` Gerd Hoffmann
  2024-02-20 17:45   ` Laszlo Ersek
  2024-02-20 22:18   ` Lendacky, Thomas via groups.io
  2024-02-20  9:06 ` [edk2-devel] [PATCH v3 6/6] OvmfPkg/PlatformInitLib: " Gerd Hoffmann
                   ` (2 subsequent siblings)
  7 siblings, 2 replies; 14+ messages in thread
From: Gerd Hoffmann @ 2024-02-20  9:06 UTC (permalink / raw)
  To: devel
  Cc: Liming Gao, Michael Roth, Oliver Steffen, Erdem Aktas,
	Tom Lendacky, Laszlo Ersek, Min Xu, Ard Biesheuvel, Jiewen Yao,
	Gerd Hoffmann

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
 
+%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
+
+PageTablesReady:
+    ; TDX will do some PostBuildPages task, such as setting
+    ; byte[TDX_WORK_AREA_PGTBL_READY].
+    OneTimeCall   TdxPostBuildPageTables
+
+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))
-- 
2.43.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115646): https://edk2.groups.io/g/devel/message/115646
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]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [edk2-devel] [PATCH v3 6/6] OvmfPkg/PlatformInitLib: add 5-level paging support
  2024-02-20  9:06 [edk2-devel] [PATCH v3 0/6] OvmfPkg: Add support for 5-level paging Gerd Hoffmann
                   ` (4 preceding siblings ...)
  2024-02-20  9:06 ` [edk2-devel] [PATCH v3 5/6] OvmfPkg/ResetVector: add 5-level paging support Gerd Hoffmann
@ 2024-02-20  9:06 ` Gerd Hoffmann
  2024-02-21  6:42 ` [edk2-devel] [PATCH v3 0/6] OvmfPkg: Add support for 5-level paging Min Xu
  2024-02-21 13:31 ` Ard Biesheuvel
  7 siblings, 0 replies; 14+ messages in thread
From: Gerd Hoffmann @ 2024-02-20  9:06 UTC (permalink / raw)
  To: devel
  Cc: Liming Gao, Michael Roth, Oliver Steffen, Erdem Aktas,
	Tom Lendacky, Laszlo Ersek, Min Xu, Ard Biesheuvel, Jiewen Yao,
	Gerd Hoffmann

Adjust physical address space logic for la57 mode (5-level paging).
With a larger logical address space we can identity-map a larger
physical address space.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
 OvmfPkg/Library/PlatformInitLib/MemDetect.c | 57 ++++++++++++++-------
 1 file changed, 38 insertions(+), 19 deletions(-)

diff --git a/OvmfPkg/Library/PlatformInitLib/MemDetect.c b/OvmfPkg/Library/PlatformInitLib/MemDetect.c
index f042517bb64a..0f9658fc34fa 100644
--- a/OvmfPkg/Library/PlatformInitLib/MemDetect.c
+++ b/OvmfPkg/Library/PlatformInitLib/MemDetect.c
@@ -628,11 +628,12 @@ PlatformAddressWidthFromCpuid (
   IN     BOOLEAN                QemuQuirk
   )
 {
-  UINT32   RegEax, RegEbx, RegEcx, RegEdx, Max;
-  UINT8    PhysBits;
-  CHAR8    Signature[13];
-  BOOLEAN  Valid         = FALSE;
-  BOOLEAN  Page1GSupport = FALSE;
+  UINT32    RegEax, RegEbx, RegEcx, RegEdx, Max;
+  UINT8     PhysBits;
+  CHAR8     Signature[13];
+  IA32_CR4  Cr4;
+  BOOLEAN   Valid         = FALSE;
+  BOOLEAN   Page1GSupport = FALSE;
 
   ZeroMem (Signature, sizeof (Signature));
 
@@ -670,30 +671,48 @@ PlatformAddressWidthFromCpuid (
     }
   }
 
+  Cr4.UintN = AsmReadCr4 ();
+
   DEBUG ((
     DEBUG_INFO,
-    "%a: Signature: '%a', PhysBits: %d, QemuQuirk: %a, Valid: %a\n",
+    "%a: Signature: '%a', PhysBits: %d, QemuQuirk: %a, la57: %a, Valid: %a\n",
     __func__,
     Signature,
     PhysBits,
     QemuQuirk ? "On" : "Off",
+    Cr4.Bits.LA57 ? "On" : "Off",
     Valid ? "Yes" : "No"
     ));
 
   if (Valid) {
-    if (PhysBits > 46) {
-      /*
-       * Avoid 5-level paging altogether for now, which limits
-       * PhysBits to 48.  Also avoid using address bit 48, due to sign
-       * extension we can't identity-map these addresses (and lots of
-       * places in edk2 assume we have everything identity-mapped).
-       * So the actual limit is 47.
-       *
-       * Also some older linux kernels apparently have problems handling
-       * phys-bits > 46 correctly, so use that as limit.
-       */
-      DEBUG ((DEBUG_INFO, "%a: limit PhysBits to 46 (avoid 5-level paging)\n", __func__));
-      PhysBits = 46;
+    /*
+     * Due to the sign extension we can use only the lower half of the
+     * virtual address space to identity-map physical address space,
+     * which gives us a 47 bit wide address space with 4 paging levels
+     * and a 56 bit wide address space with 5 paging levels.
+     */
+    if (Cr4.Bits.LA57) {
+      if (PhysBits > 48) {
+        /*
+         * Some Intel CPUs support 5-level paging, have more than 48
+         * phys-bits but support only 4-level EPT, which effectively
+         * limits guest phys-bits to 48.  Until we have some way to
+         * communicate that limitation from hypervisor to guest, limit
+         * phys-bits to 48 unconditionally.
+         */
+        DEBUG ((DEBUG_INFO, "%a: limit PhysBits to 48 (5-level paging)\n", __func__));
+        PhysBits = 48;
+      }
+    } else {
+      if (PhysBits > 46) {
+        /*
+         * Some older linux kernels apparently have problems handling
+         * phys-bits > 46 correctly, so use that instead of 47 as
+         * limit.
+         */
+        DEBUG ((DEBUG_INFO, "%a: limit PhysBits to 46 (4-level paging)\n", __func__));
+        PhysBits = 46;
+      }
     }
 
     if (!Page1GSupport && (PhysBits > 40)) {
-- 
2.43.2



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



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [edk2-devel] [PATCH v3 4/6] OvmfPkg/ResetVector: SEV: keep #vc handler installed longer
  2024-02-20  9:06 ` [edk2-devel] [PATCH v3 4/6] OvmfPkg/ResetVector: SEV: keep #vc handler installed longer Gerd Hoffmann
@ 2024-02-20 16:48   ` Laszlo Ersek
  2024-02-20 19:56   ` Lendacky, Thomas via groups.io
  1 sibling, 0 replies; 14+ messages in thread
From: Laszlo Ersek @ 2024-02-20 16:48 UTC (permalink / raw)
  To: devel, kraxel
  Cc: Liming Gao, Michael Roth, Oliver Steffen, Erdem Aktas,
	Tom Lendacky, Min Xu, Ard Biesheuvel, Jiewen Yao

On 2/20/24 10:06, Gerd Hoffmann wrote:
> When running in SEV mode do not uninstall the #vc handler in
> CheckSevFeatures.   Keep it active and uninstall it later in
> SevClearPageEncMaskForGhcbPage.
> 
> This allows using the cpuid instruction in SetCr3ForPageTables64,
> which is needed to check for la57 & 1G page support.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  OvmfPkg/ResetVector/Ia32/AmdSev.asm | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)

I'll let Tom review this :)

Acked-by: Laszlo Ersek <lersek@redhat.com>


> 
> diff --git a/OvmfPkg/ResetVector/Ia32/AmdSev.asm b/OvmfPkg/ResetVector/Ia32/AmdSev.asm
> index 043c88a7abbe..02f287f1d934 100644
> --- a/OvmfPkg/ResetVector/Ia32/AmdSev.asm
> +++ b/OvmfPkg/ResetVector/Ia32/AmdSev.asm
> @@ -158,6 +158,11 @@ SevClearPageEncMaskForGhcbPage:
>      cmp       byte[WORK_AREA_GUEST_TYPE], 1
>      jnz       SevClearPageEncMaskForGhcbPageExit
>  
> +    ; Clear exception handlers and stack
> +    mov       eax, ADDR_OF(IdtrClear)
> +    lidt      [cs:eax]
> +    mov       esp, 0
> +
>      ; Check if SEV-ES is enabled
>      mov       ecx, 1
>      bt        [SEV_ES_WORK_AREA_STATUS_MSR], ecx
> @@ -332,7 +337,6 @@ NoSevEsVcHlt:
>  NoSevPass:
>      xor       eax, eax
>  
> -SevExit:
>      ;
>      ; Clear exception handlers and stack
>      ;
> @@ -342,6 +346,7 @@ SevExit:
>      pop       eax
>      mov       esp, 0
>  
> +SevExit:
>      OneTimeCallRet CheckSevFeatures
>  
>  ; Start of #VC exception handling routines



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



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [edk2-devel] [PATCH v3 5/6] OvmfPkg/ResetVector: add 5-level paging support
  2024-02-20  9:06 ` [edk2-devel] [PATCH v3 5/6] OvmfPkg/ResetVector: add 5-level paging support Gerd Hoffmann
@ 2024-02-20 17:45   ` Laszlo Ersek
  2024-02-20 18:32     ` Laszlo Ersek
  2024-02-20 22:18   ` Lendacky, Thomas via groups.io
  1 sibling, 1 reply; 14+ messages in thread
From: Laszlo Ersek @ 2024-02-20 17:45 UTC (permalink / raw)
  To: devel, kraxel
  Cc: Liming Gao, Michael Roth, Oliver Steffen, Erdem Aktas,
	Tom Lendacky, Min Xu, Ard Biesheuvel, Jiewen Yao

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 <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(-)

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 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
>  
> +%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
> +
> +PageTablesReady:
> +    ; TDX will do some PostBuildPages task, such as setting
> +    ; byte[TDX_WORK_AREA_PGTBL_READY].
> +    OneTimeCall   TdxPostBuildPageTables
> +
> +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 (#115664): https://edk2.groups.io/g/devel/message/115664
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]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [edk2-devel] [PATCH v3 5/6] OvmfPkg/ResetVector: add 5-level paging support
  2024-02-20 17:45   ` Laszlo Ersek
@ 2024-02-20 18:32     ` Laszlo Ersek
  0 siblings, 0 replies; 14+ messages in thread
From: Laszlo Ersek @ 2024-02-20 18:32 UTC (permalink / raw)
  To: devel, kraxel
  Cc: Liming Gao, Michael Roth, Oliver Steffen, Erdem Aktas,
	Tom Lendacky, Min Xu, Ard Biesheuvel, Jiewen Yao

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/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [edk2-devel] [PATCH v3 4/6] OvmfPkg/ResetVector: SEV: keep #vc handler installed longer
  2024-02-20  9:06 ` [edk2-devel] [PATCH v3 4/6] OvmfPkg/ResetVector: SEV: keep #vc handler installed longer Gerd Hoffmann
  2024-02-20 16:48   ` Laszlo Ersek
@ 2024-02-20 19:56   ` Lendacky, Thomas via groups.io
  1 sibling, 0 replies; 14+ messages in thread
From: Lendacky, Thomas via groups.io @ 2024-02-20 19:56 UTC (permalink / raw)
  To: Gerd Hoffmann, devel
  Cc: Liming Gao, Michael Roth, Oliver Steffen, Erdem Aktas,
	Laszlo Ersek, Min Xu, Ard Biesheuvel, Jiewen Yao

On 2/20/24 03:06, Gerd Hoffmann wrote:
> When running in SEV mode do not uninstall the #vc handler in
> CheckSevFeatures.   Keep it active and uninstall it later in
> SevClearPageEncMaskForGhcbPage.
> 
> This allows using the cpuid instruction in SetCr3ForPageTables64,
> which is needed to check for la57 & 1G page support.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

I think a comment should be added above where the #VC handler is 
established to document that the #VC handler is removed at the end of this 
function if SEV is not active or that it remains installed to support 
CPUID calls, e.g. to check for 5-level paging support, and is removed 
later in SevClearPageEncMaskForGhcbPage().

With that,

Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>

> ---
>   OvmfPkg/ResetVector/Ia32/AmdSev.asm | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/OvmfPkg/ResetVector/Ia32/AmdSev.asm b/OvmfPkg/ResetVector/Ia32/AmdSev.asm
> index 043c88a7abbe..02f287f1d934 100644
> --- a/OvmfPkg/ResetVector/Ia32/AmdSev.asm
> +++ b/OvmfPkg/ResetVector/Ia32/AmdSev.asm
> @@ -158,6 +158,11 @@ SevClearPageEncMaskForGhcbPage:
>       cmp       byte[WORK_AREA_GUEST_TYPE], 1
>       jnz       SevClearPageEncMaskForGhcbPageExit
>   
> +    ; Clear exception handlers and stack
> +    mov       eax, ADDR_OF(IdtrClear)
> +    lidt      [cs:eax]
> +    mov       esp, 0
> +
>       ; Check if SEV-ES is enabled
>       mov       ecx, 1
>       bt        [SEV_ES_WORK_AREA_STATUS_MSR], ecx
> @@ -332,7 +337,6 @@ NoSevEsVcHlt:
>   NoSevPass:
>       xor       eax, eax
>   
> -SevExit:
>       ;
>       ; Clear exception handlers and stack
>       ;
> @@ -342,6 +346,7 @@ SevExit:
>       pop       eax
>       mov       esp, 0
>   
> +SevExit:
>       OneTimeCallRet CheckSevFeatures
>   
>   ; Start of #VC exception handling routines


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



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [edk2-devel] [PATCH v3 5/6] OvmfPkg/ResetVector: add 5-level paging support
  2024-02-20  9:06 ` [edk2-devel] [PATCH v3 5/6] OvmfPkg/ResetVector: add 5-level paging support Gerd Hoffmann
  2024-02-20 17:45   ` Laszlo Ersek
@ 2024-02-20 22:18   ` Lendacky, Thomas via groups.io
  1 sibling, 0 replies; 14+ messages in thread
From: Lendacky, Thomas via groups.io @ 2024-02-20 22:18 UTC (permalink / raw)
  To: Gerd Hoffmann, devel
  Cc: Liming Gao, Michael Roth, Oliver Steffen, Erdem Aktas,
	Laszlo Ersek, Min Xu, Ard Biesheuvel, Jiewen Yao

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]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [edk2-devel] [PATCH v3 0/6] OvmfPkg: Add support for 5-level paging
  2024-02-20  9:06 [edk2-devel] [PATCH v3 0/6] OvmfPkg: Add support for 5-level paging Gerd Hoffmann
                   ` (5 preceding siblings ...)
  2024-02-20  9:06 ` [edk2-devel] [PATCH v3 6/6] OvmfPkg/PlatformInitLib: " Gerd Hoffmann
@ 2024-02-21  6:42 ` Min Xu
  2024-02-21 13:31 ` Ard Biesheuvel
  7 siblings, 0 replies; 14+ messages in thread
From: Min Xu @ 2024-02-21  6:42 UTC (permalink / raw)
  To: Gerd Hoffmann, devel@edk2.groups.io
  Cc: Liming Gao, Michael Roth, Oliver Steffen, Aktas, Erdem,
	Tom Lendacky, Laszlo Ersek, Ard Biesheuvel, Yao, Jiewen,
	Xu, Min M, Sun, CepingX

On Tuesday, February 20, 2024 5:07 PM, Gerd Hoffmann wrote:
> Subject: [PATCH v3 0/6] OvmfPkg: Add support for 5-level paging
> 
> Patch #1 + #2 fix MdeModulePkg/DxeIplPeim to not assert in case a 5-level
> enabled build runs in 4-level paging mode.
> 
> Patch #2 - #4 update OvmfPkg ResetVector, adding support for 5-level paging
> (setup 5-level page tables in case both la57 and gigabyte pages are supported
> by the vCPU).
> 
> Patch #5 updates PlatformInitLib for 5-level paging support (update PhysBits
> calculation).
> 
> Known issues / limitations:
>  - BaseMemEncryptSevLib must be updated to also support 5-level
>    paging for full 5-level paging support in SEV mode.
> 
> The patch series does *not* enable 5-level paging by default.
> Building with 5-level paging support can be done by compiling OVMF with '--
> pcd PcdUse5LevelPageTable=TRUE'.
> 
> v3:
>  - add resetvector fixes for sev and tdx
> v2 changes:
>  - fix sev/tdx handling with 5-level paging.
>  - more comments for 5-level page table setup.
>  - improve PAGE_* naming (new patch #3).
>  - rename Page5LevelSupported to Page5LevelEnabled (new patch #2).
>  - pick up some review tags.
> 
> Gerd Hoffmann (6):
>   MdeModulePkg/DxeIplPeim: fix PcdUse5LevelPageTable assert
>   MdeModulePkg/DxeIplPeim: rename variable
>   OvmfPkg/ResetVector: improve page table flag names
>   OvmfPkg/ResetVector: SEV: keep #vc handler installed longer
>   OvmfPkg/ResetVector: add 5-level paging support
>   OvmfPkg/PlatformInitLib: add 5-level paging support
> 
>  OvmfPkg/ResetVector/ResetVector.inf           |   1 +
>  .../Core/DxeIplPeim/X64/VirtualMemory.c       |  24 +--
>  OvmfPkg/Library/PlatformInitLib/MemDetect.c   |  57 ++++--
>  OvmfPkg/ResetVector/Ia32/AmdSev.asm           |   7 +-
>  OvmfPkg/ResetVector/Ia32/IntelTdx.asm         |  17 +-
>  OvmfPkg/ResetVector/Ia32/PageTables64.asm     | 170 +++++++++++++++---
>  OvmfPkg/ResetVector/ResetVector.nasmb         |   1 +
>  7 files changed, 224 insertions(+), 53 deletions(-)
> 

Test the patch-set in TDX (OvmfPkgX64 and Intel/IntelTdx) and both passed.

Tested-by: Min Xu <min.m.xu@intel.com>


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



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [edk2-devel] [PATCH v3 0/6] OvmfPkg: Add support for 5-level paging
  2024-02-20  9:06 [edk2-devel] [PATCH v3 0/6] OvmfPkg: Add support for 5-level paging Gerd Hoffmann
                   ` (6 preceding siblings ...)
  2024-02-21  6:42 ` [edk2-devel] [PATCH v3 0/6] OvmfPkg: Add support for 5-level paging Min Xu
@ 2024-02-21 13:31 ` Ard Biesheuvel
  7 siblings, 0 replies; 14+ messages in thread
From: Ard Biesheuvel @ 2024-02-21 13:31 UTC (permalink / raw)
  To: Gerd Hoffmann
  Cc: devel, Liming Gao, Michael Roth, Oliver Steffen, Erdem Aktas,
	Tom Lendacky, Laszlo Ersek, Min Xu, Ard Biesheuvel, Jiewen Yao

On Tue, 20 Feb 2024 at 10:06, Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> Patch #1 + #2 fix MdeModulePkg/DxeIplPeim to not assert in case a
> 5-level enabled build runs in 4-level paging mode.
>
> Patch #2 - #4 update OvmfPkg ResetVector, adding support for 5-level
> paging (setup 5-level page tables in case both la57 and gigabyte pages
> are supported by the vCPU).
>
> Patch #5 updates PlatformInitLib for 5-level paging support (update
> PhysBits calculation).
>
> Known issues / limitations:
>  - BaseMemEncryptSevLib must be updated to also support 5-level
>    paging for full 5-level paging support in SEV mode.
>
> The patch series does *not* enable 5-level paging by default.
> Building with 5-level paging support can be done by compiling
> OVMF with '--pcd PcdUse5LevelPageTable=TRUE'.
>
> v3:
>  - add resetvector fixes for sev and tdx
> v2 changes:
>  - fix sev/tdx handling with 5-level paging.
>  - more comments for 5-level page table setup.
>  - improve PAGE_* naming (new patch #3).
>  - rename Page5LevelSupported to Page5LevelEnabled (new patch #2).
>  - pick up some review tags.
>
> Gerd Hoffmann (6):
>   MdeModulePkg/DxeIplPeim: fix PcdUse5LevelPageTable assert
>   MdeModulePkg/DxeIplPeim: rename variable
>   OvmfPkg/ResetVector: improve page table flag names
>   OvmfPkg/ResetVector: SEV: keep #vc handler installed longer
>   OvmfPkg/ResetVector: add 5-level paging support
>   OvmfPkg/PlatformInitLib: add 5-level paging support
>

I'm fine with all this but I haven't looked in great detail, so

Acked-by: Ard Biesheuvel <ardb@kernel.org>


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



^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2024-02-21 13:31 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-02-20  9:06 [edk2-devel] [PATCH v3 0/6] OvmfPkg: Add support for 5-level paging Gerd Hoffmann
2024-02-20  9:06 ` [edk2-devel] [PATCH v3 1/6] MdeModulePkg/DxeIplPeim: fix PcdUse5LevelPageTable assert Gerd Hoffmann
2024-02-20  9:06 ` [edk2-devel] [PATCH v3 2/6] MdeModulePkg/DxeIplPeim: rename variable Gerd Hoffmann
2024-02-20  9:06 ` [edk2-devel] [PATCH v3 3/6] OvmfPkg/ResetVector: improve page table flag names Gerd Hoffmann
2024-02-20  9:06 ` [edk2-devel] [PATCH v3 4/6] OvmfPkg/ResetVector: SEV: keep #vc handler installed longer Gerd Hoffmann
2024-02-20 16:48   ` Laszlo Ersek
2024-02-20 19:56   ` Lendacky, Thomas via groups.io
2024-02-20  9:06 ` [edk2-devel] [PATCH v3 5/6] OvmfPkg/ResetVector: add 5-level paging support Gerd Hoffmann
2024-02-20 17:45   ` Laszlo Ersek
2024-02-20 18:32     ` Laszlo Ersek
2024-02-20 22:18   ` Lendacky, Thomas via groups.io
2024-02-20  9:06 ` [edk2-devel] [PATCH v3 6/6] OvmfPkg/PlatformInitLib: " Gerd Hoffmann
2024-02-21  6:42 ` [edk2-devel] [PATCH v3 0/6] OvmfPkg: Add support for 5-level paging Min Xu
2024-02-21 13:31 ` Ard Biesheuvel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox