[edk2-devel] [PATCH v2 0/5] OvmfPkg: Add support for 5-level paging

[edk2-devel] [PATCH v2 0/5] OvmfPkg: Add support for 5-level paging

[Gerd Hoffmann]

Patch #1 has been submitted separately last week.
Intewl raised concerns that removing or renaming
the PCD breaks platforms, so I'm just doing the
minimal fix here.

Patch #2 + #3 update OvmfPkg ResetVector and
PlatformInitLib for 5-level paging support.

Tom, Min: can you test this patch set with SEV / 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 (5):
  MdeModulePkg/DxeIplPeim: fix PcdUse5LevelPageTable assert
  MdeModulePkg/DxeIplPeim: rename variable
  OvmfPkg/ResetVector: improve page table flag names
  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/PageTables64.asm     | 116 +++++++++++++++---
 OvmfPkg/ResetVector/ResetVector.nasmb         |   1 +
 5 files changed, 152 insertions(+), 47 deletions(-)

-- 
2.43.0



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

[edk2-devel] [PATCH v2 1/5] MdeModulePkg/DxeIplPeim: fix PcdUse5LevelPageTable assert

[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.0



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

[edk2-devel] [PATCH v2 2/5] MdeModulePkg/DxeIplPeim: rename variable

[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>
---
 .../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.0



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

[edk2-devel] [PATCH v2 3/5] OvmfPkg/ResetVector: improve page table flag names

[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>
---
 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.0



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

[edk2-devel] [PATCH v2 4/5] OvmfPkg/ResetVector: add 5-level paging support

[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, with gigabyte pages
we need three of them, with 2M pages we would need seven.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 OvmfPkg/ResetVector/ResetVector.inf       |  1 +
 OvmfPkg/ResetVector/Ia32/PageTables64.asm | 77 +++++++++++++++++++++++
 OvmfPkg/ResetVector/ResetVector.nasmb     |  1 +
 3 files changed, 79 insertions(+)

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/PageTables64.asm b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
index 6fec6f2beeea..cf64c88b6cda 100644
--- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm
+++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
@@ -86,6 +86,82 @@ 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,
+    ; due to the use of gigabyte pages we need three pages
+    ; and everything fits in.
+    ;
+    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 (0x2000) + PAGE_PDE_DIRECTORY_ATTR
+    mov     dword[PT_ADDR (0x1004)], edx
+
+    ; level 3 (four 1GB pages for the lowest 4G)
+    mov     dword[PT_ADDR (0x2000)], (0 << 30) + PAGE_PDE_LARGEPAGE_ATTR
+    mov     dword[PT_ADDR (0x2004)], edx
+    mov     dword[PT_ADDR (0x2008)], (1 << 30) + PAGE_PDE_LARGEPAGE_ATTR
+    mov     dword[PT_ADDR (0x200c)], edx
+    mov     dword[PT_ADDR (0x2010)], (2 << 30) + PAGE_PDE_LARGEPAGE_ATTR
+    mov     dword[PT_ADDR (0x2014)], edx
+    mov     dword[PT_ADDR (0x2018)], (3 << 30) + PAGE_PDE_LARGEPAGE_ATTR
+    mov     dword[PT_ADDR (0x201c)], edx
+
+    ; set la57 bit in cr4
+    mov     eax, cr4
+    bts     eax, 12
+    mov     cr4, eax
+
+    ; done
+    jmp     PageTablesReady
+
+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,6 +193,7 @@ pageTableEntriesLoop:
     mov     [(ecx * 8 + PT_ADDR (0x2000 - 8)) + 4], edx
     loop    pageTableEntriesLoop
 
+PageTablesReady:
     ; Clear the C-bit from the GHCB page if the SEV-ES is enabled.
     OneTimeCall   SevClearPageEncMaskForGhcbPage
 
diff --git a/OvmfPkg/ResetVector/ResetVector.nasmb b/OvmfPkg/ResetVector/ResetVector.nasmb
index 5832aaa8abf7..16b3eee57671 100644
--- a/OvmfPkg/ResetVector/ResetVector.nasmb
+++ b/OvmfPkg/ResetVector/ResetVector.nasmb
@@ -49,6 +49,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.0



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

[edk2-devel] [PATCH v2 5/5] OvmfPkg/PlatformInitLib: add 5-level paging support

[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.0



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

Re: [edk2-devel] [PATCH v2 2/5] MdeModulePkg/DxeIplPeim: rename variable

[Laszlo Ersek]

On 1/30/24 13:32, Gerd Hoffmann wrote:
> 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>
> ---
>  .../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);

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

Thanks!
Laszlo



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

Re: [edk2-devel] [PATCH v2 3/5] OvmfPkg/ResetVector: improve page table flag names

[Laszlo Ersek]

On 1/30/24 13:32, Gerd Hoffmann wrote:
> 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>
> ---
>  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

Interesting: PAGE_PAT and PAGE_2M_PAT appear unused!

But the patch is still fine; thank you for it!

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

Laszlo


>  
>  %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



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

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

[Laszlo Ersek]

On 1/30/24 13:32, 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, with gigabyte pages
> we need three of them, with 2M pages we would need seven.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  OvmfPkg/ResetVector/ResetVector.inf       |  1 +
>  OvmfPkg/ResetVector/Ia32/PageTables64.asm | 77 +++++++++++++++++++++++
>  OvmfPkg/ResetVector/ResetVector.nasmb     |  1 +
>  3 files changed, 79 insertions(+)
> 
> 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/PageTables64.asm b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> index 6fec6f2beeea..cf64c88b6cda 100644
> --- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> +++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> @@ -86,6 +86,82 @@ 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,
> +    ; due to the use of gigabyte pages we need three pages
> +    ; and everything fits in.
> +    ;
> +    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 (0x2000) + PAGE_PDE_DIRECTORY_ATTR
> +    mov     dword[PT_ADDR (0x1004)], edx
> +
> +    ; level 3 (four 1GB pages for the lowest 4G)
> +    mov     dword[PT_ADDR (0x2000)], (0 << 30) + PAGE_PDE_LARGEPAGE_ATTR
> +    mov     dword[PT_ADDR (0x2004)], edx
> +    mov     dword[PT_ADDR (0x2008)], (1 << 30) + PAGE_PDE_LARGEPAGE_ATTR
> +    mov     dword[PT_ADDR (0x200c)], edx
> +    mov     dword[PT_ADDR (0x2010)], (2 << 30) + PAGE_PDE_LARGEPAGE_ATTR
> +    mov     dword[PT_ADDR (0x2014)], edx
> +    mov     dword[PT_ADDR (0x2018)], (3 << 30) + PAGE_PDE_LARGEPAGE_ATTR
> +    mov     dword[PT_ADDR (0x201c)], edx
> +
> +    ; set la57 bit in cr4
> +    mov     eax, cr4
> +    bts     eax, 12
> +    mov     cr4, eax
> +
> +    ; done
> +    jmp     PageTablesReady
> +
> +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,6 +193,7 @@ pageTableEntriesLoop:
>      mov     [(ecx * 8 + PT_ADDR (0x2000 - 8)) + 4], edx
>      loop    pageTableEntriesLoop
>  
> +PageTablesReady:
>      ; Clear the C-bit from the GHCB page if the SEV-ES is enabled.
>      OneTimeCall   SevClearPageEncMaskForGhcbPage
>  
> diff --git a/OvmfPkg/ResetVector/ResetVector.nasmb b/OvmfPkg/ResetVector/ResetVector.nasmb
> index 5832aaa8abf7..16b3eee57671 100644
> --- a/OvmfPkg/ResetVector/ResetVector.nasmb
> +++ b/OvmfPkg/ResetVector/ResetVector.nasmb
> @@ -49,6 +49,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))

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

Thanks!



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

Re: [edk2-devel] [PATCH v2 0/5] OvmfPkg: Add support for 5-level paging

[Laszlo Ersek]

On 1/30/24 13:31, Gerd Hoffmann wrote:
> Patch #1 has been submitted separately last week.
> Intewl raised concerns that removing or renaming
> the PCD breaks platforms, so I'm just doing the
> minimal fix here.
> 
> Patch #2 + #3 update OvmfPkg ResetVector and
> PlatformInitLib for 5-level paging support.
> 
> Tom, Min: can you test this patch set with SEV / TDX?

I'll merge the series when those tests succeed. (Please keep me CC'd.)

Thanks
Laszlo

> 
> 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 (5):
>   MdeModulePkg/DxeIplPeim: fix PcdUse5LevelPageTable assert
>   MdeModulePkg/DxeIplPeim: rename variable
>   OvmfPkg/ResetVector: improve page table flag names
>   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/PageTables64.asm     | 116 +++++++++++++++---
>  OvmfPkg/ResetVector/ResetVector.nasmb         |   1 +
>  5 files changed, 152 insertions(+), 47 deletions(-)
> 



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

Re: [edk2-devel] [PATCH v2 3/5] OvmfPkg/ResetVector: improve page table flag names

[Pedro Falcato]

On Tue, Jan 30, 2024 at 12:32 PM Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> 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>
> ---
>  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

Technically, if we want to be pedantic, these are PDPTEs (or PDPs I
guess), so PDE is misleading here.

-- 
Pedro


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

Re: [edk2-devel] [PATCH v2 3/5] OvmfPkg/ResetVector: improve page table flag names

[Laszlo Ersek]

On 1/30/24 20:46, Pedro Falcato wrote:
> On Tue, Jan 30, 2024 at 12:32 PM Gerd Hoffmann <kraxel@redhat.com> wrote:
>>
>> 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>
>> ---
>>  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
> 
> Technically, if we want to be pedantic, these are PDPTEs (or PDPs I
> guess), so PDE is misleading here.

I remembered the fact that the SDM used different terms for different
table levels, but honestly, I've never been able to keep all those in my
mind for longer than 5 minutes. I recall them as fairly arbitrary. So I
consciously ignored this; in my book, this doesn't count as misleading.
But, I have nothing against more detailed macro names, if Gerd's willing
to respin!

Laszlo



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

Re: [edk2-devel] [PATCH v2 0/5] OvmfPkg: Add support for 5-level paging

[Min Xu]

On Tuesday, January 30, 2024 8:32 PM, Gerd Hoffmann wrote:
> 
> Patch #1 has been submitted separately last week.
> Intewl raised concerns that removing or renaming the PCD breaks platforms,
> so I'm just doing the minimal fix here.
> 
> Patch #2 + #3 update OvmfPkg ResetVector and PlatformInitLib for 5-level
> paging support.
> 
> Tom, Min: can you test this patch set with SEV / TDX?
> 
We test the patch in TDX with PcdUse5LevelPageTable TRUE. It failed if the td-guest is configured with multiple vCpus. We're investigating what happened.

Thanks
Min


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

Re: [edk2-devel] [PATCH v2 3/5] OvmfPkg/ResetVector: improve page table flag names

[Gerd Hoffmann]

  Hi,

> > Technically, if we want to be pedantic, these are PDPTEs (or PDPs I
> > guess), so PDE is misleading here.
> 
> I remembered the fact that the SDM used different terms for different
> table levels, but honestly, I've never been able to keep all those in my
> mind for longer than 5 minutes. I recall them as fairly arbitrary.

The names made sense back in the 90-ies when we had only two or three
(with PAE) paging levels on x86.  Nowdays with up to 5 levels not so
much.  I likewise can't remember all the names, so I very much prefer
to work with level numbers.

The bits for the lowest level are slightly different from all levels
above, and I've used the level 1 + level 2 names (page table entry and
page directory entry) to name them.

take care,
  Gerd



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

Re: [edk2-devel] [PATCH v2 0/5] OvmfPkg: Add support for 5-level paging

[Gerd Hoffmann]

On Wed, Jan 31, 2024 at 06:19:38AM +0000, Xu, Min M wrote:
> On Tuesday, January 30, 2024 8:32 PM, Gerd Hoffmann wrote:
> > 
> > Patch #1 has been submitted separately last week.
> > Intewl raised concerns that removing or renaming the PCD breaks platforms,
> > so I'm just doing the minimal fix here.
> > 
> > Patch #2 + #3 update OvmfPkg ResetVector and PlatformInitLib for 5-level
> > paging support.
> > 
> > Tom, Min: can you test this patch set with SEV / TDX?
> > 
> We test the patch in TDX with PcdUse5LevelPageTable TRUE. It failed if the td-guest is configured with multiple vCpus. We're investigating what happened.

My first guess would be we need to somehow propagate to the APs that
cr4.la57 must be set.

Maybe use
TDX_WORK_AREA_PGTBL_READY_4_LEVEL +
TDX_WORK_AREA_PGTBL_READY_5_LEVEL ?

Or add a field for cr4 to the work area?

take care,
  Gerd



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

Re: [edk2-devel] [PATCH v2 3/5] OvmfPkg/ResetVector: improve page table flag names

[Laszlo Ersek]

On 1/31/24 09:14, Gerd Hoffmann wrote:
>   Hi,
> 
>>> Technically, if we want to be pedantic, these are PDPTEs (or PDPs I
>>> guess), so PDE is misleading here.
>>
>> I remembered the fact that the SDM used different terms for different
>> table levels, but honestly, I've never been able to keep all those in my
>> mind for longer than 5 minutes. I recall them as fairly arbitrary.
> 
> The names made sense back in the 90-ies when we had only two or three
> (with PAE) paging levels on x86.  Nowdays with up to 5 levels not so
> much.  I likewise can't remember all the names, so I very much prefer
> to work with level numbers.
> 
> The bits for the lowest level are slightly different from all levels
> above,

Right; I've not checked recently (and I certainly can't just recall the
details), but this has been my "operating impression" anyway, and I felt
that your macro names were a good match:

> and I've used the level 1 + level 2 names (page table entry and
> page directory entry) to name them.

Pedro, what say you?

Thanks,
Laszlo



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

Re: [edk2-devel] [PATCH v2 0/5] OvmfPkg: Add support for 5-level paging

[Laszlo Ersek]

Hi Liming,

On 1/30/24 13:31, Gerd Hoffmann wrote:

> 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.

can you please check patches #1 and #2?

Patch #1 is very small, and #2, while larger, is trivial.

Thanks!
Laszlo



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

Re: [edk2-devel] [PATCH v2 3/5] OvmfPkg/ResetVector: improve page table flag names

[Pedro Falcato]

On Wed, Jan 31, 2024 at 11:23 AM Laszlo Ersek <lersek@redhat.com> wrote:
>
> On 1/31/24 09:14, Gerd Hoffmann wrote:
> >   Hi,
> >
> >>> Technically, if we want to be pedantic, these are PDPTEs (or PDPs I
> >>> guess), so PDE is misleading here.
> >>
> >> I remembered the fact that the SDM used different terms for different
> >> table levels, but honestly, I've never been able to keep all those in my
> >> mind for longer than 5 minutes. I recall them as fairly arbitrary.
> >
> > The names made sense back in the 90-ies when we had only two or three
> > (with PAE) paging levels on x86.  Nowdays with up to 5 levels not so
> > much.  I likewise can't remember all the names, so I very much prefer
> > to work with level numbers.
> >
> > The bits for the lowest level are slightly different from all levels
> > above,
>
> Right; I've not checked recently (and I certainly can't just recall the
> details), but this has been my "operating impression" anyway, and I felt
> that your macro names were a good match:
>
> > and I've used the level 1 + level 2 names (page table entry and
> > page directory entry) to name them.
>
> Pedro, what say you?

It was just a random comment, do whatever you think is more
maintainable/worth it. It's not like it makes much of a difference, I
just wanted the define to be unambiguous.
Something generic like HUGE_PAGE_ATTR would also work IMO (even if
hugepage is a super overloaded and confusing term in the kernel world
- ooh boy let's not get into that :P).

-- 
Pedro


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

Re: [edk2-devel] [PATCH v2 0/5] OvmfPkg: Add support for 5-level paging

[Min Xu]

On Wednesday, January 31, 2024 4:24 PM, Gerd Hoffmann wrote:
> On Wed, Jan 31, 2024 at 06:19:38AM +0000, Xu, Min M wrote:
> > On Tuesday, January 30, 2024 8:32 PM, Gerd Hoffmann wrote:
> > >
> > > Patch #1 has been submitted separately last week.
> > > Intewl raised concerns that removing or renaming the PCD breaks
> > > platforms, so I'm just doing the minimal fix here.
> > >
> > > Patch #2 + #3 update OvmfPkg ResetVector and PlatformInitLib for
> > > 5-level paging support.
> > >
> > > Tom, Min: can you test this patch set with SEV / TDX?
> > >
> > We test the patch in TDX with PcdUse5LevelPageTable TRUE. It failed if the
> td-guest is configured with multiple vCpus. We're investigating what
> happened.
> 
> My first guess would be we need to somehow propagate to the APs that
> cr4.la57 must be set.
Yes, we make a quick poc that set the LA57 for APs. It works in multiple vCPUs scenario.

Thanks
Min


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

Re: [edk2-devel] [PATCH v2 0/5] OvmfPkg: Add support for 5-level paging

[Gerd Hoffmann]

On Thu, Feb 01, 2024 at 05:44:30AM +0000, Xu, Min M wrote:
> > My first guess would be we need to somehow propagate to the APs that
> > cr4.la57 must be set.
> Yes, we make a quick poc that set the LA57 for APs. It works in multiple vCPUs scenario.

Good.  How can we move forward with this?  Do you want work on a fix?
Should I give it a try?  In case of the latter:  Is the design idea
to have two TDX_WORK_AREA_PGTBL_READY states ok?

take care,
  Gerd



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

Re: [edk2-devel] [PATCH v2 0/5] OvmfPkg: Add support for 5-level paging

[Gerd Hoffmann]

On Thu, Feb 01, 2024 at 05:44:30AM +0000, Xu, Min M wrote:
> On Wednesday, January 31, 2024 4:24 PM, Gerd Hoffmann wrote:
> > On Wed, Jan 31, 2024 at 06:19:38AM +0000, Xu, Min M wrote:
> > > On Tuesday, January 30, 2024 8:32 PM, Gerd Hoffmann wrote:
> > > >
> > > > Patch #1 has been submitted separately last week.
> > > > Intewl raised concerns that removing or renaming the PCD breaks
> > > > platforms, so I'm just doing the minimal fix here.
> > > >
> > > > Patch #2 + #3 update OvmfPkg ResetVector and PlatformInitLib for
> > > > 5-level paging support.
> > > >
> > > > Tom, Min: can you test this patch set with SEV / TDX?
> > > >
> > > We test the patch in TDX with PcdUse5LevelPageTable TRUE. It failed if the
> > td-guest is configured with multiple vCpus. We're investigating what
> > happened.
> > 
> > My first guess would be we need to somehow propagate to the APs that
> > cr4.la57 must be set.
> Yes, we make a quick poc that set the LA57 for APs. It works in multiple vCPUs scenario.

Pushed branch with two experimental patches:
  https://github.com/kraxel/edk2/commits/devel/paging-5lvl/

One for tdx, writing different values into TDX_WORK_AREA_PGTBL_READY for
4-level and 5-level paging modes.

One for sev, which adds a 2-level page directory for the first gigabyte
and places it at the same physical address it has in 4-level paging.
With that SevClearPageEncMaskForGhcbPage should work unmodified in
5-level mode.

Test results are welcome.

thanks & take care,
  Gerd



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

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

[Lendacky, Thomas via groups.io]

On 1/30/24 06:32, 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, with gigabyte pages
> we need three of them, with 2M pages we would need seven.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>   OvmfPkg/ResetVector/ResetVector.inf       |  1 +
>   OvmfPkg/ResetVector/Ia32/PageTables64.asm | 77 +++++++++++++++++++++++
>   OvmfPkg/ResetVector/ResetVector.nasmb     |  1 +
>   3 files changed, 79 insertions(+)
> 
> 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/PageTables64.asm b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> index 6fec6f2beeea..cf64c88b6cda 100644
> --- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> +++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> @@ -86,6 +86,82 @@ 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

Because of these CPUID instructions, this won't work for SEV-ES / SEV-SNP. 
To use these we'll need to have a (special 32-bit) #VC handler in place. 
Currently that is done in only in OvmfPkg/ResetVector/Ia32/AmdSev.asm for 
the CheckSevFeatures function, where the #VC handler is established at the 
beginning of the function, but it is removed when leaving the function.

The SEV support in general needs looking into in order to support 5-level 
paging. At the time the SEV support was developed, there wasn't a page 
table library and so there is some 4-level page table manipulation support 
in the BaseMemEncryptSevLib that really needs to be converted to use the 
page table library.

I don't have an objection to the series, as long as PcdUse5LevelPageTable 
is not set to TRUE by default for the Ovmf packages.

Thanks,
Tom

> +    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,
> +    ; due to the use of gigabyte pages we need three pages
> +    ; and everything fits in.
> +    ;
> +    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 (0x2000) + PAGE_PDE_DIRECTORY_ATTR
> +    mov     dword[PT_ADDR (0x1004)], edx
> +
> +    ; level 3 (four 1GB pages for the lowest 4G)
> +    mov     dword[PT_ADDR (0x2000)], (0 << 30) + PAGE_PDE_LARGEPAGE_ATTR
> +    mov     dword[PT_ADDR (0x2004)], edx
> +    mov     dword[PT_ADDR (0x2008)], (1 << 30) + PAGE_PDE_LARGEPAGE_ATTR
> +    mov     dword[PT_ADDR (0x200c)], edx
> +    mov     dword[PT_ADDR (0x2010)], (2 << 30) + PAGE_PDE_LARGEPAGE_ATTR
> +    mov     dword[PT_ADDR (0x2014)], edx
> +    mov     dword[PT_ADDR (0x2018)], (3 << 30) + PAGE_PDE_LARGEPAGE_ATTR
> +    mov     dword[PT_ADDR (0x201c)], edx
> +
> +    ; set la57 bit in cr4
> +    mov     eax, cr4
> +    bts     eax, 12
> +    mov     cr4, eax
> +
> +    ; done
> +    jmp     PageTablesReady
> +
> +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,6 +193,7 @@ pageTableEntriesLoop:
>       mov     [(ecx * 8 + PT_ADDR (0x2000 - 8)) + 4], edx
>       loop    pageTableEntriesLoop
>   
> +PageTablesReady:
>       ; Clear the C-bit from the GHCB page if the SEV-ES is enabled.
>       OneTimeCall   SevClearPageEncMaskForGhcbPage
>   
> diff --git a/OvmfPkg/ResetVector/ResetVector.nasmb b/OvmfPkg/ResetVector/ResetVector.nasmb
> index 5832aaa8abf7..16b3eee57671 100644
> --- a/OvmfPkg/ResetVector/ResetVector.nasmb
> +++ b/OvmfPkg/ResetVector/ResetVector.nasmb
> @@ -49,6 +49,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 (#114949): https://edk2.groups.io/g/devel/message/114949
Mute This Topic: https://groups.io/mt/104052208/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v2 0/5] OvmfPkg: Add support for 5-level paging

[Lendacky, Thomas via groups.io]

On 1/30/24 13:15, Laszlo Ersek wrote:
> On 1/30/24 13:31, Gerd Hoffmann wrote:
>> Patch #1 has been submitted separately last week.
>> Intewl raised concerns that removing or renaming
>> the PCD breaks platforms, so I'm just doing the
>> minimal fix here.
>>
>> Patch #2 + #3 update OvmfPkg ResetVector and
>> PlatformInitLib for 5-level paging support.
>>
>> Tom, Min: can you test this patch set with SEV / TDX?
> 
> I'll merge the series when those tests succeed. (Please keep me CC'd.)

I replied to the patch 4/5 with some comments.

As long as the PcdUse5LevelPageTable PCD is FALSE, this series doesn't 
appear to affect SEV.

When PcdUse5LevelPageTable is set to TRUE, the CPUID calls in 
OvmfPkg/ResetVector/Ia32/PageTables64.asm will cause an SEV-ES/SEV-SNP 
guest to crash because there is no #VC handler established.

When PcdUse5LevelPageTable is set to TRUE, an SEV guest will ASSERT:
   AmdSevDxe:SetMemoryEncDec: bad PML4 for Physical=0x8000000000
   ASSERT_EFI_ERROR (Status = No mapping)

The SEV code in BaseMemEncryptSevLib was written before the page table 
library support was developed, and as such, only supports 4-level paging. 
So the SEV page table support needs updating.

Thanks,
Tom

> 
> Thanks
> Laszlo
> 
>>
>> 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 (5):
>>    MdeModulePkg/DxeIplPeim: fix PcdUse5LevelPageTable assert
>>    MdeModulePkg/DxeIplPeim: rename variable
>>    OvmfPkg/ResetVector: improve page table flag names
>>    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/PageTables64.asm     | 116 +++++++++++++++---
>>   OvmfPkg/ResetVector/ResetVector.nasmb         |   1 +
>>   5 files changed, 152 insertions(+), 47 deletions(-)
>>
> 


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

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

[Gerd Hoffmann]

  Hi,

> Because of these CPUID instructions, this won't work for SEV-ES / SEV-SNP.
> To use these we'll need to have a (special 32-bit) #VC handler in place.
> Currently that is done in only in OvmfPkg/ResetVector/Ia32/AmdSev.asm for
> the CheckSevFeatures function, where the #VC handler is established at the
> beginning of the function, but it is removed when leaving the function.

Noted.  Uninstalling the exception handler later (probably just before
entering long mode) should be possible I think.

> The SEV support in general needs looking into in order to support 5-level
> paging. At the time the SEV support was developed, there wasn't a page table
> library and so there is some 4-level page table manipulation support in the
> BaseMemEncryptSevLib that really needs to be converted to use the page table
> library.

Right, I remember, and I think TDX has the same problem.

> I don't have an objection to the series, as long as PcdUse5LevelPageTable is
> not set to TRUE by default for the Ovmf packages.

The patch series does not add it to the Ovmf*.dsc files, and
MdeModulePkg/MdeModulePkg.dec declares with with default being FALSE, so
yes, it is disabled by default.

take care,
  Gerd



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

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

[Laszlo Ersek]

On 2/1/24 16:44, Lendacky, Thomas via groups.io wrote:
> On 1/30/24 06:32, 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, with gigabyte pages
>> we need three of them, with 2M pages we would need seven.
>>
>> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
>> ---
>>   OvmfPkg/ResetVector/ResetVector.inf       |  1 +
>>   OvmfPkg/ResetVector/Ia32/PageTables64.asm | 77 +++++++++++++++++++++++
>>   OvmfPkg/ResetVector/ResetVector.nasmb     |  1 +
>>   3 files changed, 79 insertions(+)
>>
>> 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/PageTables64.asm
>> b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
>> index 6fec6f2beeea..cf64c88b6cda 100644
>> --- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm
>> +++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
>> @@ -86,6 +86,82 @@ 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
> 
> Because of these CPUID instructions, this won't work for SEV-ES /
> SEV-SNP. To use these we'll need to have a (special 32-bit) #VC handler
> in place. Currently that is done in only in
> OvmfPkg/ResetVector/Ia32/AmdSev.asm for the CheckSevFeatures function,
> where the #VC handler is established at the beginning of the function,
> but it is removed when leaving the function.
> 
> The SEV support in general needs looking into in order to support
> 5-level paging. At the time the SEV support was developed, there wasn't
> a page table library and so there is some 4-level page table
> manipulation support in the BaseMemEncryptSevLib that really needs to be
> converted to use the page table library.
> 
> I don't have an objection to the series, as long as
> PcdUse5LevelPageTable is not set to TRUE by default for the Ovmf packages.

Well, I do have a slight objection:

> 
>> +    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,
>> +    ; due to the use of gigabyte pages we need three pages
>> +    ; and everything fits in.
>> +    ;
>> +    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 (0x2000) +
>> PAGE_PDE_DIRECTORY_ATTR
>> +    mov     dword[PT_ADDR (0x1004)], edx
>> +
>> +    ; level 3 (four 1GB pages for the lowest 4G)
>> +    mov     dword[PT_ADDR (0x2000)], (0 << 30) + PAGE_PDE_LARGEPAGE_ATTR
>> +    mov     dword[PT_ADDR (0x2004)], edx
>> +    mov     dword[PT_ADDR (0x2008)], (1 << 30) + PAGE_PDE_LARGEPAGE_ATTR
>> +    mov     dword[PT_ADDR (0x200c)], edx
>> +    mov     dword[PT_ADDR (0x2010)], (2 << 30) + PAGE_PDE_LARGEPAGE_ATTR
>> +    mov     dword[PT_ADDR (0x2014)], edx
>> +    mov     dword[PT_ADDR (0x2018)], (3 << 30) + PAGE_PDE_LARGEPAGE_ATTR
>> +    mov     dword[PT_ADDR (0x201c)], edx
>> +
>> +    ; set la57 bit in cr4
>> +    mov     eax, cr4
>> +    bts     eax, 12
>> +    mov     cr4, eax
>> +
>> +    ; done
>> +    jmp     PageTablesReady

Note this jump here...

>> +
>> +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,6 +193,7 @@ pageTableEntriesLoop:
>>       mov     [(ecx * 8 + PT_ADDR (0x2000 - 8)) + 4], edx
>>       loop    pageTableEntriesLoop
>>   +PageTablesReady:
>>       ; Clear the C-bit from the GHCB page if the SEV-ES is enabled.
>>       OneTimeCall   SevClearPageEncMaskForGhcbPage

Landing here.

I requested this; see point (4) at
<https://edk2.groups.io/g/devel/message/114745>.

But knowing (now!) that the neighborhood (= the 5 level paging setup)
isn't compatible with / reachable under SEV-ES in the first place, this
jump only seems wishful thinking.

The best I could propose: jump again to SetCr3 (like in v1), but add a
comment that it's not a mistake, but intentional (because the stuff
doesn't work on SEV-ES anyway).

Thanks
Laszlo


>>   diff --git a/OvmfPkg/ResetVector/ResetVector.nasmb
>> b/OvmfPkg/ResetVector/ResetVector.nasmb
>> index 5832aaa8abf7..16b3eee57671 100644
>> --- a/OvmfPkg/ResetVector/ResetVector.nasmb
>> +++ b/OvmfPkg/ResetVector/ResetVector.nasmb
>> @@ -49,6 +49,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 (#114982): https://edk2.groups.io/g/devel/message/114982
Mute This Topic: https://groups.io/mt/104052208/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v2 0/5] OvmfPkg: Add support for 5-level paging

[Min Xu]

On Thursday, February 1, 2024 10:15 PM, Gerd Hoffmann wrote:
> On Thu, Feb 01, 2024 at 05:44:30AM +0000, Xu, Min M wrote:
> > On Wednesday, January 31, 2024 4:24 PM, Gerd Hoffmann wrote:
> > > On Wed, Jan 31, 2024 at 06:19:38AM +0000, Xu, Min M wrote:
> > > > On Tuesday, January 30, 2024 8:32 PM, Gerd Hoffmann wrote:
> > > > >
> > > > > Patch #1 has been submitted separately last week.
> > > > > Intewl raised concerns that removing or renaming the PCD breaks
> > > > > platforms, so I'm just doing the minimal fix here.
> > > > >
> > > > > Patch #2 + #3 update OvmfPkg ResetVector and PlatformInitLib for
> > > > > 5-level paging support.
> > > > >
> > > > > Tom, Min: can you test this patch set with SEV / TDX?
> > > > >
> > > > We test the patch in TDX with PcdUse5LevelPageTable TRUE. It
> > > > failed if the
> > > td-guest is configured with multiple vCpus. We're investigating what
> > > happened.
> > >
> > > My first guess would be we need to somehow propagate to the APs that
> > > cr4.la57 must be set.
> > Yes, we make a quick poc that set the LA57 for APs. It works in multiple
> vCPUs scenario.
> 
> Pushed branch with two experimental patches:
>   https://github.com/kraxel/edk2/commits/devel/paging-5lvl/
> 
> One for tdx, writing different values into TDX_WORK_AREA_PGTBL_READY for
> 4-level and 5-level paging modes.
> 
> One for sev, which adds a 2-level page directory for the first gigabyte and
> places it at the same physical address it has in 4-level paging.
> With that SevClearPageEncMaskForGhcbPage should work unmodified in 5-
> level mode.
> 
> Test results are welcome.
> 
Thanks Gerd, it works in TDX. One typo in the patch that pageTableEntriesLoopla57  and pageTableEntriesLoopLa57.

Thanks
Min


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

Re: [edk2-devel] [PATCH v2 0/5] OvmfPkg: Add support for 5-level paging

[Gerd Hoffmann]

On Fri, Feb 02, 2024 at 08:30:23AM +0000, Xu, Min M wrote:
> > Pushed branch with two experimental patches:
> >   https://github.com/kraxel/edk2/commits/devel/paging-5lvl/
> > 
> > One for tdx, writing different values into TDX_WORK_AREA_PGTBL_READY for
> > 4-level and 5-level paging modes.
> > 
> > One for sev, which adds a 2-level page directory for the first gigabyte and
> > places it at the same physical address it has in 4-level paging.
> > With that SevClearPageEncMaskForGhcbPage should work unmodified in 5-
> > level mode.
> > 
> > Test results are welcome.
> 
> Thanks Gerd, it works in TDX. One typo in the patch that pageTableEntriesLoopla57  and pageTableEntriesLoopLa57.

Thanks for testing.

Added a second sev patch to the branch, rearranging the #vc
handler uninstall to make the la57 cpuid checks work on sev.

@Tom: Would be great if you can give this a test

thanks & take care,
  Gerd

PS: I'll be offline next week.



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

Re: [edk2-devel] [PATCH v2 0/5] OvmfPkg: Add support for 5-level paging

[Lendacky, Thomas via groups.io]

On 2/2/24 02:44, Gerd Hoffmann wrote:
> On Fri, Feb 02, 2024 at 08:30:23AM +0000, Xu, Min M wrote:
>>> Pushed branch with two experimental patches:
>>>    https://github.com/kraxel/edk2/commits/devel/paging-5lvl/
>>>
>>> One for tdx, writing different values into TDX_WORK_AREA_PGTBL_READY for
>>> 4-level and 5-level paging modes.
>>>
>>> One for sev, which adds a 2-level page directory for the first gigabyte and
>>> places it at the same physical address it has in 4-level paging.
>>> With that SevClearPageEncMaskForGhcbPage should work unmodified in 5-
>>> level mode.
>>>
>>> Test results are welcome.
>>
>> Thanks Gerd, it works in TDX. One typo in the patch that pageTableEntriesLoopla57  and pageTableEntriesLoopLa57.
> 
> Thanks for testing.
> 
> Added a second sev patch to the branch, rearranging the #vc
> handler uninstall to make the la57 cpuid checks work on sev.
> 
> @Tom: Would be great if you can give this a test

So this gets us past the ASM code CPUID instructions, however, the
SEV support is still assuming 4-level paging and so:

SEV ASSERT:
   AmdSevDxe:SetMemoryEncDec: bad PML4 for Physical=0x8000000000

   ASSERT_EFI_ERROR (Status = No mapping)
   ASSERT [AmdSevDxe] /root/kernels/ovmf-gerd-build-X64/OvmfPkg/AmdSevDxe/AmdSevDxe.c(235): !(((INTN)(RETURN_STATUS)(Status)) < 0)

SEV-ES and SEV-SNP ASSERT:
   PlatformPei:SetMemoryEncDec: bad PDPE for Physical=0x7FE5C000

   ASSERT_RETURN_ERROR (Status = No mapping)
   ASSERT [PlatformPei] /root/kernels/ovmf-gerd-build-X64/OvmfPkg/PlatformPei/AmdSev.c(254): !(((INTN)(RETURN_STATUS)(Status)) < 0)

FYI, this uncovered an SNP CPUID table processing bug in the ResetVector
code and so I sent a fix to the mailing list.

Thanks,
Tom

> 
> thanks & take care,
>    Gerd
> 
> PS: I'll be offline next week.
> 


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

Re: [edk2-devel] [PATCH v2 0/5] OvmfPkg: Add support for 5-level paging

[Gerd Hoffmann]

On Fri, Feb 02, 2024 at 01:28:47PM -0600, Lendacky, Thomas via groups.io wrote:
> On 2/2/24 02:44, Gerd Hoffmann wrote:
> > On Fri, Feb 02, 2024 at 08:30:23AM +0000, Xu, Min M wrote:
> > > > Pushed branch with two experimental patches:
> > > >    https://github.com/kraxel/edk2/commits/devel/paging-5lvl/
> > > > 
> > > > One for tdx, writing different values into TDX_WORK_AREA_PGTBL_READY for
> > > > 4-level and 5-level paging modes.
> > > > 
> > > > One for sev, which adds a 2-level page directory for the first gigabyte and
> > > > places it at the same physical address it has in 4-level paging.
> > > > With that SevClearPageEncMaskForGhcbPage should work unmodified in 5-
> > > > level mode.
> > > > 
> > > > Test results are welcome.
> > > 
> > > Thanks Gerd, it works in TDX. One typo in the patch that pageTableEntriesLoopla57  and pageTableEntriesLoopLa57.
> > 
> > Thanks for testing.
> > 
> > Added a second sev patch to the branch, rearranging the #vc
> > handler uninstall to make the la57 cpuid checks work on sev.
> > 
> > @Tom: Would be great if you can give this a test
> 
> So this gets us past the ASM code CPUID instructions, however, the
> SEV support is still assuming 4-level paging and so:

Thanks for testing.  So, with the patch series the ResetVector is ready
for 5-level paging.  Good start.  I'll go over the series and will send
an update later (not urgent, it's post-freeze material anyway).

> SEV ASSERT:
>   AmdSevDxe:SetMemoryEncDec: bad PML4 for Physical=0x8000000000

As expected.

> FYI, this uncovered an SNP CPUID table processing bug in the ResetVector
> code and so I sent a fix to the mailing list.

I see the fix already merged.  Nice.

take care,
  Gerd



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