public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v3 1/5] UefiCpuPkg/ResetVector: Rename macros about page table.
@ 2023-04-28  6:42 Zhiguang Liu
  2023-04-28  6:42 ` [PATCH v3 2/5] UefiCpuPkg/ResetVector: Simplify page table creation in ResetVector Zhiguang Liu
                   ` (5 more replies)
  0 siblings, 6 replies; 12+ messages in thread
From: Zhiguang Liu @ 2023-04-28  6:42 UTC (permalink / raw)
  To: devel
  Cc: Zhiguang Liu, Eric Dong, Ray Ni, Rahul Kumar, Gerd Hoffmann,
	Debkumar De, Catharine West

This patch only renames macro, with no code logic impacted.
Two purpose to rename macro:
1. Align some macro name in PageTables1G.asm and PageTables2M.asm, so
that these two files can be easily combined later.
2. Some Macro names such as PDP are not accurate, since 4 level page
entry also uses this macro. PG_NLE (no leaf entry) is better.

Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Debkumar De <debkumar.de@intel.com>
Cc: Catharine West <catharine.west@intel.com>
Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
---
 .../ResetVector/Vtf0/X64/PageTables1G.asm     | 24 ++++++++++----
 .../ResetVector/Vtf0/X64/PageTables2M.asm     | 33 +++++++++++--------
 2 files changed, 37 insertions(+), 20 deletions(-)

diff --git a/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables1G.asm b/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables1G.asm
index 19bd3d5a92..97e90777c8 100644
--- a/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables1G.asm
+++ b/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables1G.asm
@@ -2,7 +2,7 @@
 ; @file
 ; Emits Page Tables for 1:1 mapping of the addresses 0 - 0x8000000000 (512GB)
 ;
-; Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
+; Copyright (c) 2021 - 2023, Intel Corporation. All rights reserved.<BR>
 ; SPDX-License-Identifier: BSD-2-Clause-Patent
 ; Linear-Address Translation to a 1-GByte Page
 ;
@@ -12,11 +12,18 @@ BITS    64
 
 %define ALIGN_TOP_TO_4K_FOR_PAGING
 
-%define PAGE_PDP_ATTR (PAGE_ACCESSED + \
+;
+; Page table no-leaf entry attribute
+;
+%define PAGE_NLE_ATTR (PAGE_ACCESSED + \
                         PAGE_READ_WRITE + \
                         PAGE_PRESENT)
 
-%define PAGE_PDP_1G_ATTR (PAGE_ACCESSED + \
+;
+; Page table big leaf page attribute
+; Big leaf page contains PDPTE 1GB page and PDE 2MB page
+;
+%define PAGE_BLP_ATTR (PAGE_ACCESSED + \
                         PAGE_READ_WRITE + \
                         PAGE_DIRTY + \
                         PAGE_PRESENT + \
@@ -25,10 +32,13 @@ BITS    64
 %define PGTBLS_OFFSET(x) ((x) - TopLevelPageDirectory)
 %define PGTBLS_ADDR(x) (ADDR_OF(TopLevelPageDirectory) + (x))
 
-%define PDP(offset) (ADDR_OF(TopLevelPageDirectory) + (offset) + \
-                    PAGE_PDP_ATTR)
+;
+; Page table no-leaf entry
+;
+%define PG_NLE(offset) (ADDR_OF(TopLevelPageDirectory) + (offset) + \
+                    PAGE_NLE_ATTR)
 
-%define PDP_1G(x) ((x << 30) + PAGE_PDP_1G_ATTR)
+%define PDP_1G(x) ((x << 30) + PAGE_BLP_ATTR)
 
 ALIGN 16
 
@@ -37,7 +47,7 @@ TopLevelPageDirectory:
     ;
     ; Top level Page Directory Pointers (1 * 512GB entry)
     ;
-    DQ      PDP(0x1000)
+    DQ      PG_NLE(0x1000)
 
     TIMES 0x1000-PGTBLS_OFFSET($) DB 0
     ;
diff --git a/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables2M.asm b/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables2M.asm
index b97df384ac..e46694c799 100644
--- a/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables2M.asm
+++ b/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables2M.asm
@@ -2,7 +2,7 @@
 ; @file
 ; Emits Page Tables for 1:1 mapping of the addresses 0 - 0x100000000 (4GB)
 ;
-; Copyright (c) 2008 - 2014, Intel Corporation. All rights reserved.<BR>
+; Copyright (c) 2008 - 2023, Intel Corporation. All rights reserved.<BR>
 ; SPDX-License-Identifier: BSD-2-Clause-Patent
 ;
 ;------------------------------------------------------------------------------
@@ -11,29 +11,36 @@ BITS    64
 
 %define ALIGN_TOP_TO_4K_FOR_PAGING
 
-%define PAGE_2M_PDE_ATTR (PAGE_SIZE + \
+;
+; Page table big leaf page attribute
+; Big leaf page contains PDPTE 1GB page and PDE 2MB page
+;
+%define PAGE_BLP_ATTR   (PAGE_SIZE + \
                           PAGE_ACCESSED + \
                           PAGE_DIRTY + \
                           PAGE_READ_WRITE + \
                           PAGE_PRESENT)
 
-%define PAGE_PDP_ATTR (PAGE_ACCESSED + \
-                       PAGE_READ_WRITE + \
-                       PAGE_PRESENT)
+;
+; Page table no-leaf entry attribute
+;
+%define PAGE_NLE_ATTR (PAGE_ACCESSED + \
+                        PAGE_READ_WRITE + \
+                        PAGE_PRESENT)
 
 %define PGTBLS_OFFSET(x) ((x) - TopLevelPageDirectory)
 %define PGTBLS_ADDR(x) (ADDR_OF(TopLevelPageDirectory) + (x))
 
-%define PDP(offset) (ADDR_OF(TopLevelPageDirectory) + (offset) + \
-                     PAGE_PDP_ATTR)
-%define PTE_2MB(x) ((x << 21) + PAGE_2M_PDE_ATTR)
+%define PG_NLE(offset) (ADDR_OF(TopLevelPageDirectory) + (offset) + \
+                    PAGE_NLE_ATTR)
+%define PTE_2MB(x) ((x << 21) + PAGE_BLP_ATTR)
 
 TopLevelPageDirectory:
 
     ;
     ; Top level Page Directory Pointers (1 * 512GB entry)
     ;
-    DQ      PDP(0x1000)
+    DQ      PG_NLE(0x1000)
 
 
     ;
@@ -41,10 +48,10 @@ TopLevelPageDirectory:
     ;
     TIMES 0x1000-PGTBLS_OFFSET($) DB 0
 
-    DQ      PDP(0x2000)
-    DQ      PDP(0x3000)
-    DQ      PDP(0x4000)
-    DQ      PDP(0x5000)
+    DQ      PG_NLE(0x2000)
+    DQ      PG_NLE(0x3000)
+    DQ      PG_NLE(0x4000)
+    DQ      PG_NLE(0x5000)
 
     ;
     ; Page Table Entries (2048 * 2MB entries => 4GB)
-- 
2.31.1.windows.1


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

* [PATCH v3 2/5] UefiCpuPkg/ResetVector: Simplify page table creation in ResetVector
  2023-04-28  6:42 [PATCH v3 1/5] UefiCpuPkg/ResetVector: Rename macros about page table Zhiguang Liu
@ 2023-04-28  6:42 ` Zhiguang Liu
  2023-04-28  9:05   ` Ni, Ray
  2023-04-28  6:42 ` [PATCH v3 3/5] UefiCpuPkg/ResetVector: Combine PageTables1G.asm and PageTables2M.asm Zhiguang Liu
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 12+ messages in thread
From: Zhiguang Liu @ 2023-04-28  6:42 UTC (permalink / raw)
  To: devel
  Cc: Zhiguang Liu, Eric Dong, Ray Ni, Rahul Kumar, Gerd Hoffmann,
	Debkumar De, Catharine West

Currently, page table creation has many hard-code values about the
offset to the start of page table. To simplify it, add Labels such
as Pml4, Pdp and Pd, so that we can remove many hard-code values

Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Debkumar De <debkumar.de@intel.com>
Cc: Catharine West <catharine.west@intel.com>
Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
---
 .../ResetVector/Vtf0/Ia32/PageTables64.asm    |  4 +--
 .../ResetVector/Vtf0/X64/PageTables1G.asm     | 18 ++++------
 .../ResetVector/Vtf0/X64/PageTables2M.asm     | 34 ++++++++-----------
 3 files changed, 24 insertions(+), 32 deletions(-)

diff --git a/UefiCpuPkg/ResetVector/Vtf0/Ia32/PageTables64.asm b/UefiCpuPkg/ResetVector/Vtf0/Ia32/PageTables64.asm
index 87a4125d4b..f188da20ba 100644
--- a/UefiCpuPkg/ResetVector/Vtf0/Ia32/PageTables64.asm
+++ b/UefiCpuPkg/ResetVector/Vtf0/Ia32/PageTables64.asm
@@ -2,7 +2,7 @@
 ; @file
 ; Sets the CR3 register for 64-bit paging
 ;
-; Copyright (c) 2008 - 2013, Intel Corporation. All rights reserved.<BR>
+; Copyright (c) 2008 - 2023, Intel Corporation. All rights reserved.<BR>
 ; SPDX-License-Identifier: BSD-2-Clause-Patent
 ;
 ;------------------------------------------------------------------------------
@@ -17,7 +17,7 @@ SetCr3ForPageTables64:
     ;
     ; These pages are built into the ROM image in X64/PageTables.asm
     ;
-    mov     eax, ADDR_OF(TopLevelPageDirectory)
+    mov     eax, ADDR_OF(Pml4)
     mov     cr3, eax
 
     OneTimeCallRet SetCr3ForPageTables64
diff --git a/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables1G.asm b/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables1G.asm
index 97e90777c8..2b0de6020c 100644
--- a/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables1G.asm
+++ b/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables1G.asm
@@ -29,35 +29,31 @@ BITS    64
                         PAGE_PRESENT + \
                         PAGE_SIZE)
 
-%define PGTBLS_OFFSET(x) ((x) - TopLevelPageDirectory)
-%define PGTBLS_ADDR(x) (ADDR_OF(TopLevelPageDirectory) + (x))
-
 ;
 ; Page table no-leaf entry
 ;
-%define PG_NLE(offset) (ADDR_OF(TopLevelPageDirectory) + (offset) + \
+%define PG_NLE(address) (ADDR_OF(address) + \
                     PAGE_NLE_ATTR)
 
 %define PDP_1G(x) ((x << 30) + PAGE_BLP_ATTR)
 
 ALIGN 16
 
-TopLevelPageDirectory:
-
+Pml4:
     ;
-    ; Top level Page Directory Pointers (1 * 512GB entry)
+    ; PML4 (1 * 512GB entry)
     ;
-    DQ      PG_NLE(0x1000)
+    DQ      PG_NLE(Pdp)
+    TIMES   0x1000 - ($ - Pml4) DB 0
 
-    TIMES 0x1000-PGTBLS_OFFSET($) DB 0
+Pdp:
     ;
-    ; Next level Page Directory Pointers (512 * 1GB entries => 512GB)
+    ; Page-directory pointer table (512 * 1GB entries => 512GB)
     ;
 %assign i 0
 %rep      512
     DQ    PDP_1G(i)
     %assign i i+1
 %endrep
-    TIMES 0x2000-PGTBLS_OFFSET($) DB 0
 
 EndOfPageTables:
diff --git a/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables2M.asm b/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables2M.asm
index e46694c799..cdf0fb41b7 100644
--- a/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables2M.asm
+++ b/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables2M.asm
@@ -28,36 +28,32 @@ BITS    64
                         PAGE_READ_WRITE + \
                         PAGE_PRESENT)
 
-%define PGTBLS_OFFSET(x) ((x) - TopLevelPageDirectory)
-%define PGTBLS_ADDR(x) (ADDR_OF(TopLevelPageDirectory) + (x))
-
-%define PG_NLE(offset) (ADDR_OF(TopLevelPageDirectory) + (offset) + \
+%define PG_NLE(address) (ADDR_OF(address) + \
                     PAGE_NLE_ATTR)
 %define PTE_2MB(x) ((x << 21) + PAGE_BLP_ATTR)
 
-TopLevelPageDirectory:
-
+Pml4:
     ;
-    ; Top level Page Directory Pointers (1 * 512GB entry)
+    ; PML4 (1 * 512GB entry)
     ;
-    DQ      PG_NLE(0x1000)
-
+    DQ      PG_NLE(Pdp)
+    TIMES   0x1000 - ($ - Pml4) DB 0
 
+Pdp:
     ;
-    ; Next level Page Directory Pointers (4 * 1GB entries => 4GB)
+    ; Page-directory pointer table (4 * 1GB entries => 4GB)
     ;
-    TIMES 0x1000-PGTBLS_OFFSET($) DB 0
-
-    DQ      PG_NLE(0x2000)
-    DQ      PG_NLE(0x3000)
-    DQ      PG_NLE(0x4000)
-    DQ      PG_NLE(0x5000)
+    DQ      PG_NLE(Pd)
+    DQ      PG_NLE(Pd + 0x1000)
+    DQ      PG_NLE(Pd + 0x2000)
+    DQ      PG_NLE(Pd + 0x3000)
+    TIMES   0x1000 - ($ - Pdp) DB 0
 
+Pd:
     ;
-    ; Page Table Entries (2048 * 2MB entries => 4GB)
+    ; Page-Directory (2048 * 2MB entries => 4GB)
+    ; Four pages below, each is pointed by one entry in Pdp.
     ;
-    TIMES 0x2000-PGTBLS_OFFSET($) DB 0
-
 %assign i 0
 %rep    0x800
     DQ      PTE_2MB(i)
-- 
2.31.1.windows.1


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

* [PATCH v3 3/5] UefiCpuPkg/ResetVector: Combine PageTables1G.asm and PageTables2M.asm
  2023-04-28  6:42 [PATCH v3 1/5] UefiCpuPkg/ResetVector: Rename macros about page table Zhiguang Liu
  2023-04-28  6:42 ` [PATCH v3 2/5] UefiCpuPkg/ResetVector: Simplify page table creation in ResetVector Zhiguang Liu
@ 2023-04-28  6:42 ` Zhiguang Liu
  2023-04-28  9:09   ` Ni, Ray
  2023-04-28  6:42 ` [PATCH v3 4/5] UefiCpuPkg/ResetVector: Modify Page Table in ResetVector Zhiguang Liu
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 12+ messages in thread
From: Zhiguang Liu @ 2023-04-28  6:42 UTC (permalink / raw)
  To: devel
  Cc: Zhiguang Liu, Eric Dong, Ray Ni, Rahul Kumar, Gerd Hoffmann,
	Debkumar De, Catharine West

Combine PageTables1G.asm and PageTables2M.asm to reuse code.

Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Debkumar De <debkumar.de@intel.com>
Cc: Catharine West <catharine.west@intel.com>
Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
---
 UefiCpuPkg/ResetVector/Vtf0/Vtf0.nasmb        |  8 +--
 .../X64/{PageTables1G.asm => PageTables.asm}  | 38 ++++++++---
 .../ResetVector/Vtf0/X64/PageTables2M.asm     | 63 -------------------
 3 files changed, 33 insertions(+), 76 deletions(-)
 rename UefiCpuPkg/ResetVector/Vtf0/X64/{PageTables1G.asm => PageTables.asm} (58%)
 delete mode 100644 UefiCpuPkg/ResetVector/Vtf0/X64/PageTables2M.asm

diff --git a/UefiCpuPkg/ResetVector/Vtf0/Vtf0.nasmb b/UefiCpuPkg/ResetVector/Vtf0/Vtf0.nasmb
index bdea1fb875..136361e62c 100644
--- a/UefiCpuPkg/ResetVector/Vtf0/Vtf0.nasmb
+++ b/UefiCpuPkg/ResetVector/Vtf0/Vtf0.nasmb
@@ -2,7 +2,7 @@
 ; @file
 ; This file includes all other code files to assemble the reset vector code
 ;
-; Copyright (c) 2008 - 2013, Intel Corporation. All rights reserved.<BR>
+; Copyright (c) 2008 - 2023, Intel Corporation. All rights reserved.<BR>
 ; SPDX-License-Identifier: BSD-2-Clause-Patent
 ;
 ;------------------------------------------------------------------------------
@@ -38,11 +38,7 @@
 %include "PageTables.inc"
 
 %ifdef ARCH_X64
-  %ifdef PAGE_TABLE_1G
-    %include "X64/PageTables1G.asm"
-  %else
-    %include "X64/PageTables2M.asm"
-  %endif
+  %include "X64/PageTables.asm"
 %endif
 
 %ifdef DEBUG_PORT80
diff --git a/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables1G.asm b/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables.asm
similarity index 58%
rename from UefiCpuPkg/ResetVector/Vtf0/X64/PageTables1G.asm
rename to UefiCpuPkg/ResetVector/Vtf0/X64/PageTables.asm
index 2b0de6020c..469fed0006 100644
--- a/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables1G.asm
+++ b/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables.asm
@@ -1,10 +1,11 @@
 ;------------------------------------------------------------------------------
 ; @file
-; Emits Page Tables for 1:1 mapping of the addresses 0 - 0x8000000000 (512GB)
+; Emits Page Tables for 1:1 mapping.
+; If using 1G page table, map addresses 0 - 0x8000000000 (512GB),
+; else, map addresses 0 - 0x100000000 (4GB)
 ;
 ; Copyright (c) 2021 - 2023, Intel Corporation. All rights reserved.<BR>
 ; SPDX-License-Identifier: BSD-2-Clause-Patent
-; Linear-Address Translation to a 1-GByte Page
 ;
 ;------------------------------------------------------------------------------
 
@@ -36,6 +37,7 @@ BITS    64
                     PAGE_NLE_ATTR)
 
 %define PDP_1G(x) ((x << 30) + PAGE_BLP_ATTR)
+%define PTE_2MB(x) ((x << 21) + PAGE_BLP_ATTR)
 
 ALIGN 16
 
@@ -46,14 +48,36 @@ Pml4:
     DQ      PG_NLE(Pdp)
     TIMES   0x1000 - ($ - Pml4) DB 0
 
+%ifdef PAGE_TABLE_1G
 Pdp:
     ;
     ; Page-directory pointer table (512 * 1GB entries => 512GB)
     ;
-%assign i 0
-%rep      512
-    DQ    PDP_1G(i)
-    %assign i i+1
-%endrep
+    %assign i 0
+    %rep      512
+        DQ    PDP_1G(i)
+        %assign i i+1
+    %endrep
+%else
+Pdp:
+    ;
+    ; Page-directory pointer table (4 * 1GB entries => 4GB)
+    ;
+    DQ      PG_NLE(Pd)
+    DQ      PG_NLE(Pd + 0x1000)
+    DQ      PG_NLE(Pd + 0x2000)
+    DQ      PG_NLE(Pd + 0x3000)
+    TIMES   0x1000 - ($ - Pdp) DB 0
 
+Pd:
+    ;
+    ; Page-Directory (2048 * 2MB entries => 4GB)
+    ; Four pages below, each is pointed by one entry in Pdp.
+    ;
+    %assign i 0
+    %rep    0x800
+        DQ      PTE_2MB(i)
+        %assign i i+1
+    %endrep
+%endif
 EndOfPageTables:
diff --git a/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables2M.asm b/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables2M.asm
deleted file mode 100644
index cdf0fb41b7..0000000000
--- a/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables2M.asm
+++ /dev/null
@@ -1,63 +0,0 @@
-;------------------------------------------------------------------------------
-; @file
-; Emits Page Tables for 1:1 mapping of the addresses 0 - 0x100000000 (4GB)
-;
-; Copyright (c) 2008 - 2023, Intel Corporation. All rights reserved.<BR>
-; SPDX-License-Identifier: BSD-2-Clause-Patent
-;
-;------------------------------------------------------------------------------
-
-BITS    64
-
-%define ALIGN_TOP_TO_4K_FOR_PAGING
-
-;
-; Page table big leaf page attribute
-; Big leaf page contains PDPTE 1GB page and PDE 2MB page
-;
-%define PAGE_BLP_ATTR   (PAGE_SIZE + \
-                          PAGE_ACCESSED + \
-                          PAGE_DIRTY + \
-                          PAGE_READ_WRITE + \
-                          PAGE_PRESENT)
-
-;
-; Page table no-leaf entry attribute
-;
-%define PAGE_NLE_ATTR (PAGE_ACCESSED + \
-                        PAGE_READ_WRITE + \
-                        PAGE_PRESENT)
-
-%define PG_NLE(address) (ADDR_OF(address) + \
-                    PAGE_NLE_ATTR)
-%define PTE_2MB(x) ((x << 21) + PAGE_BLP_ATTR)
-
-Pml4:
-    ;
-    ; PML4 (1 * 512GB entry)
-    ;
-    DQ      PG_NLE(Pdp)
-    TIMES   0x1000 - ($ - Pml4) DB 0
-
-Pdp:
-    ;
-    ; Page-directory pointer table (4 * 1GB entries => 4GB)
-    ;
-    DQ      PG_NLE(Pd)
-    DQ      PG_NLE(Pd + 0x1000)
-    DQ      PG_NLE(Pd + 0x2000)
-    DQ      PG_NLE(Pd + 0x3000)
-    TIMES   0x1000 - ($ - Pdp) DB 0
-
-Pd:
-    ;
-    ; Page-Directory (2048 * 2MB entries => 4GB)
-    ; Four pages below, each is pointed by one entry in Pdp.
-    ;
-%assign i 0
-%rep    0x800
-    DQ      PTE_2MB(i)
-    %assign i i+1
-%endrep
-
-EndOfPageTables:
-- 
2.31.1.windows.1


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

* [PATCH v3 4/5] UefiCpuPkg/ResetVector: Modify Page Table in ResetVector
  2023-04-28  6:42 [PATCH v3 1/5] UefiCpuPkg/ResetVector: Rename macros about page table Zhiguang Liu
  2023-04-28  6:42 ` [PATCH v3 2/5] UefiCpuPkg/ResetVector: Simplify page table creation in ResetVector Zhiguang Liu
  2023-04-28  6:42 ` [PATCH v3 3/5] UefiCpuPkg/ResetVector: Combine PageTables1G.asm and PageTables2M.asm Zhiguang Liu
@ 2023-04-28  6:42 ` Zhiguang Liu
  2023-04-28  9:12   ` Ni, Ray
  2023-05-01  7:47   ` [edk2-devel] " Ard Biesheuvel
  2023-04-28  6:42 ` [PATCH v3 5/5] UefiCpuPkg/ResetVector: Support 5 level page table " Zhiguang Liu
                   ` (2 subsequent siblings)
  5 siblings, 2 replies; 12+ messages in thread
From: Zhiguang Liu @ 2023-04-28  6:42 UTC (permalink / raw)
  To: devel
  Cc: Zhiguang Liu, Eric Dong, Ray Ni, Rahul Kumar, Gerd Hoffmann,
	Debkumar De, Catharine West

In ResetVector, if create page table, its highest address is fixed
because after page table, code layout is fixed(4K for normal code,
and another 4K only contains reset vector code).
Today's implementation organizes the page table as following if 1G
page table is used:
  4G-16K: PML4 page (PML4[0] points to 4G-12K)
  4G-12K: PDP page
  CR3 is set to 4G-16K
When 2M page table is used, the layout is as following:
  4G-32K: PML4 page (PML4[0] points to 4G-28K)
  4G-28K: PDP page (PDP entries point to PD pages)
  4G-24K: PD page mapping 0-1G
  4G-20K: PD page mapping 1-2G
  4G-16K: PD page mapping 2-3G
  4G-12K: PD page mapping 3-4G
  CR3 is set to 4G-32K
CR3 doesn't point to a fixed location which is a bit hard to debug at
runtime.

The new page table layout will always put PML4 in highest address
When 1G page table is used, the layout is as following:
  4G-16K: PDP page
  4G-12K: PML4 page (PML4[0] points to 4G-16K)
When 2M page table is used, the layout is as following:
  4G-32K: PD page mapping 0-1G
  4G-28K: PD page mapping 1-2G
  4G-24K: PD page mapping 2-3G
  4G-20K: PD page mapping 3-4G
  4G-16K: PDP page (PDP entries point to PD pages)
  4G-12K: PML4 page (PML4[0] points to 4G-16K)
CR3 is always set to 4G-12K
So, this patch can improve biodegradability by make sure the init
CR3 pointing to a fixed address(4G-12K).

Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Debkumar De <debkumar.de@intel.com>
Cc: Catharine West <catharine.west@intel.com>
Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
---
 .../ResetVector/Vtf0/X64/PageTables.asm       | 32 +++++++++----------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables.asm b/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables.asm
index 469fed0006..4ff68cddef 100644
--- a/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables.asm
+++ b/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables.asm
@@ -41,13 +41,6 @@ BITS    64
 
 ALIGN 16
 
-Pml4:
-    ;
-    ; PML4 (1 * 512GB entry)
-    ;
-    DQ      PG_NLE(Pdp)
-    TIMES   0x1000 - ($ - Pml4) DB 0
-
 %ifdef PAGE_TABLE_1G
 Pdp:
     ;
@@ -59,6 +52,16 @@ Pdp:
         %assign i i+1
     %endrep
 %else
+Pd:
+    ;
+    ; Page-Directory (2048 * 2MB entries => 4GB)
+    ; Four pages below, each is pointed by one entry in Pdp.
+    ;
+    %assign i 0
+    %rep    0x800
+        DQ      PTE_2MB(i)
+        %assign i i+1
+    %endrep
 Pdp:
     ;
     ; Page-directory pointer table (4 * 1GB entries => 4GB)
@@ -69,15 +72,12 @@ Pdp:
     DQ      PG_NLE(Pd + 0x3000)
     TIMES   0x1000 - ($ - Pdp) DB 0
 
-Pd:
+%endif
+
+Pml4:
     ;
-    ; Page-Directory (2048 * 2MB entries => 4GB)
-    ; Four pages below, each is pointed by one entry in Pdp.
+    ; PML4 (1 * 512GB entry)
     ;
-    %assign i 0
-    %rep    0x800
-        DQ      PTE_2MB(i)
-        %assign i i+1
-    %endrep
-%endif
+    DQ      PG_NLE(Pdp)
+    TIMES   0x1000 - ($ - Pml4) DB 0
 EndOfPageTables:
-- 
2.31.1.windows.1


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

* [PATCH v3 5/5] UefiCpuPkg/ResetVector: Support 5 level page table in ResetVector
  2023-04-28  6:42 [PATCH v3 1/5] UefiCpuPkg/ResetVector: Rename macros about page table Zhiguang Liu
                   ` (2 preceding siblings ...)
  2023-04-28  6:42 ` [PATCH v3 4/5] UefiCpuPkg/ResetVector: Modify Page Table in ResetVector Zhiguang Liu
@ 2023-04-28  6:42 ` Zhiguang Liu
  2023-04-28  9:15   ` Ni, Ray
  2023-04-28  9:03 ` [PATCH v3 1/5] UefiCpuPkg/ResetVector: Rename macros about page table Ni, Ray
       [not found] ` <175A0DD0D612EB33.26969@groups.io>
  5 siblings, 1 reply; 12+ messages in thread
From: Zhiguang Liu @ 2023-04-28  6:42 UTC (permalink / raw)
  To: devel
  Cc: Zhiguang Liu, Eric Dong, Ray Ni, Rahul Kumar, Gerd Hoffmann,
	Debkumar De, Catharine West

Add a macro USE_5_LEVEL_PAGE_TABLE to determine whether to create
5 level page table.
If macro USE_5_LEVEL_PAGE_TABLE is defined, PML5Table is created
at (4G-12K), while PML4Table is at (4G-16K). In runtime check, if
5level paging is supported, use PML5Table, otherwise, use PML4Table.
If macro USE_5_LEVEL_PAGE_TABLE is not defined, to save space, 5level
paging is not created, and 4level paging is at (4G-12K) and be used.

Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Debkumar De <debkumar.de@intel.com>
Cc: Catharine West <catharine.west@intel.com>
Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
---
 .../ResetVector/Vtf0/Ia32/Flat32ToFlat64.asm  | 25 +++++++++++++++++--
 .../ResetVector/Vtf0/Ia32/PageTables64.asm    | 24 ------------------
 UefiCpuPkg/ResetVector/Vtf0/Vtf0.nasmb        |  1 -
 .../ResetVector/Vtf0/X64/PageTables.asm       |  9 +++++++
 4 files changed, 32 insertions(+), 27 deletions(-)
 delete mode 100644 UefiCpuPkg/ResetVector/Vtf0/Ia32/PageTables64.asm

diff --git a/UefiCpuPkg/ResetVector/Vtf0/Ia32/Flat32ToFlat64.asm b/UefiCpuPkg/ResetVector/Vtf0/Ia32/Flat32ToFlat64.asm
index 6891397c2a..f119f941a5 100644
--- a/UefiCpuPkg/ResetVector/Vtf0/Ia32/Flat32ToFlat64.asm
+++ b/UefiCpuPkg/ResetVector/Vtf0/Ia32/Flat32ToFlat64.asm
@@ -2,7 +2,7 @@
 ; @file
 ; Transition from 32 bit flat protected mode into 64 bit flat protected mode
 ;
-; Copyright (c) 2008 - 2018, Intel Corporation. All rights reserved.<BR>
+; Copyright (c) 2008 - 2023, Intel Corporation. All rights reserved.<BR>
 ; SPDX-License-Identifier: BSD-2-Clause-Patent
 ;
 ;------------------------------------------------------------------------------
@@ -14,7 +14,28 @@ BITS    32
 ;
 Transition32FlatTo64Flat:
 
-    OneTimeCall SetCr3ForPageTables64
+%ifdef USE_5_LEVEL_PAGE_TABLE
+    mov     eax, 0
+    cpuid
+    cmp     eax, 07h                    ; check if basic CPUID leaf contains leaf 07
+    jb      NotSupport5LevelPaging      ; 5level paging not support, downgrade to 4level paging
+    mov     eax, 07h                    ; check cpuid leaf 7, subleaf 0
+    mov     ecx, 0
+    cpuid
+    bt      ecx, 16                     ; [Bits 16] Supports 5-level paging if 1.
+    jnc     NotSupport5LevelPaging      ; 5level paging not support, downgrade to 4level paging
+    mov     eax, ADDR_OF(Pml5)
+    mov     cr3, eax
+    mov     eax, cr4
+    bts     eax, 12                     ; Set LA57=1.
+    mov     cr4, eax
+    jmp     SetCr3Done
+NotSupport5LevelPaging:
+%endif
+
+    mov     eax, ADDR_OF(Pml4)
+    mov     cr3, eax
+SetCr3Done:
 
     mov     eax, cr4
     bts     eax, 5                      ; enable PAE
diff --git a/UefiCpuPkg/ResetVector/Vtf0/Ia32/PageTables64.asm b/UefiCpuPkg/ResetVector/Vtf0/Ia32/PageTables64.asm
deleted file mode 100644
index f188da20ba..0000000000
--- a/UefiCpuPkg/ResetVector/Vtf0/Ia32/PageTables64.asm
+++ /dev/null
@@ -1,24 +0,0 @@
-;------------------------------------------------------------------------------
-; @file
-; Sets the CR3 register for 64-bit paging
-;
-; Copyright (c) 2008 - 2023, Intel Corporation. All rights reserved.<BR>
-; SPDX-License-Identifier: BSD-2-Clause-Patent
-;
-;------------------------------------------------------------------------------
-
-BITS    32
-
-;
-; Modified:  EAX
-;
-SetCr3ForPageTables64:
-
-    ;
-    ; These pages are built into the ROM image in X64/PageTables.asm
-    ;
-    mov     eax, ADDR_OF(Pml4)
-    mov     cr3, eax
-
-    OneTimeCallRet SetCr3ForPageTables64
-
diff --git a/UefiCpuPkg/ResetVector/Vtf0/Vtf0.nasmb b/UefiCpuPkg/ResetVector/Vtf0/Vtf0.nasmb
index 136361e62c..5a6563bd34 100644
--- a/UefiCpuPkg/ResetVector/Vtf0/Vtf0.nasmb
+++ b/UefiCpuPkg/ResetVector/Vtf0/Vtf0.nasmb
@@ -54,7 +54,6 @@
 
 %ifdef ARCH_X64
 %include "Ia32/Flat32ToFlat64.asm"
-%include "Ia32/PageTables64.asm"
 %endif
 
 %include "Ia16/Real16ToFlat32.asm"
diff --git a/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables.asm b/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables.asm
index 4ff68cddef..5aa229eb14 100644
--- a/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables.asm
+++ b/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables.asm
@@ -80,4 +80,13 @@ Pml4:
     ;
     DQ      PG_NLE(Pdp)
     TIMES   0x1000 - ($ - Pml4) DB 0
+
+%ifdef USE_5_LEVEL_PAGE_TABLE
+Pml5:
+    ;
+    ; Pml5 table (only first entry is present, pointing to Pml4)
+    ;
+    DQ      PG_NLE(Pml4)
+    TIMES   0x1000 - ($ - Pml5) DB 0
+%endif
 EndOfPageTables:
-- 
2.31.1.windows.1


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

* Re: [PATCH v3 1/5] UefiCpuPkg/ResetVector: Rename macros about page table.
  2023-04-28  6:42 [PATCH v3 1/5] UefiCpuPkg/ResetVector: Rename macros about page table Zhiguang Liu
                   ` (3 preceding siblings ...)
  2023-04-28  6:42 ` [PATCH v3 5/5] UefiCpuPkg/ResetVector: Support 5 level page table " Zhiguang Liu
@ 2023-04-28  9:03 ` Ni, Ray
       [not found] ` <175A0DD0D612EB33.26969@groups.io>
  5 siblings, 0 replies; 12+ messages in thread
From: Ni, Ray @ 2023-04-28  9:03 UTC (permalink / raw)
  To: Liu, Zhiguang, devel@edk2.groups.io
  Cc: Dong, Eric, Kumar, Rahul R, Gerd Hoffmann, De, Debkumar,
	West, Catharine



> -----Original Message-----
> From: Liu, Zhiguang <zhiguang.liu@intel.com>
> Sent: Friday, April 28, 2023 2:42 PM
> To: devel@edk2.groups.io
> Cc: Liu, Zhiguang <zhiguang.liu@intel.com>; Dong, Eric
> <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Kumar, Rahul R
> <rahul.r.kumar@intel.com>; Gerd Hoffmann <kraxel@redhat.com>; De,
> Debkumar <debkumar.de@intel.com>; West, Catharine
> <catharine.west@intel.com>
> Subject: [PATCH v3 1/5] UefiCpuPkg/ResetVector: Rename macros about
> page table.
> 
> This patch only renames macro, with no code logic impacted.
> Two purpose to rename macro:
> 1. Align some macro name in PageTables1G.asm and PageTables2M.asm, so
> that these two files can be easily combined later.
> 2. Some Macro names such as PDP are not accurate, since 4 level page
> entry also uses this macro. PG_NLE (no leaf entry) is better.
> 
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Debkumar De <debkumar.de@intel.com>
> Cc: Catharine West <catharine.west@intel.com>
> Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
> ---
>  .../ResetVector/Vtf0/X64/PageTables1G.asm     | 24 ++++++++++----
>  .../ResetVector/Vtf0/X64/PageTables2M.asm     | 33 +++++++++++--------
>  2 files changed, 37 insertions(+), 20 deletions(-)
> 
> diff --git a/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables1G.asm
> b/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables1G.asm
> index 19bd3d5a92..97e90777c8 100644
> --- a/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables1G.asm
> +++ b/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables1G.asm
> @@ -2,7 +2,7 @@
>  ; @file
>  ; Emits Page Tables for 1:1 mapping of the addresses 0 - 0x8000000000
> (512GB)
>  ;
> -; Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
> +; Copyright (c) 2021 - 2023, Intel Corporation. All rights reserved.<BR>
>  ; SPDX-License-Identifier: BSD-2-Clause-Patent
>  ; Linear-Address Translation to a 1-GByte Page
>  ;
> @@ -12,11 +12,18 @@ BITS    64
> 
>  %define ALIGN_TOP_TO_4K_FOR_PAGING
> 
> -%define PAGE_PDP_ATTR (PAGE_ACCESSED + \
> +;
> +; Page table no-leaf entry attribute

1. non-leaf entry attribute

> +;
> +%define PAGE_NLE_ATTR (PAGE_ACCESSED + \
>                          PAGE_READ_WRITE + \
>                          PAGE_PRESENT)
> 
> -%define PAGE_PDP_1G_ATTR (PAGE_ACCESSED + \
> +;
> +; Page table big leaf page attribute

2. Page table big leaf entry attribute

> +; Big leaf page contains PDPTE 1GB page and PDE 2MB page

3. PDPTE 1GB entry or PDE 2MB entry

> +;
> +%define PAGE_BLP_ATTR (PAGE_ACCESSED + \

4. PAGE_BLE_ATTR


>                          PAGE_READ_WRITE + \
>                          PAGE_DIRTY + \
>                          PAGE_PRESENT + \
> @@ -25,10 +32,13 @@ BITS    64
>  %define PGTBLS_OFFSET(x) ((x) - TopLevelPageDirectory)
>  %define PGTBLS_ADDR(x) (ADDR_OF(TopLevelPageDirectory) + (x))
> 
> -%define PDP(offset) (ADDR_OF(TopLevelPageDirectory) + (offset) + \
> -                    PAGE_PDP_ATTR)
> +;
> +; Page table no-leaf entry
> +;
> +%define PG_NLE(offset) (ADDR_OF(TopLevelPageDirectory) + (offset) + \

5. PAGE_NLE
Similar comments to the following changes.


> +                    PAGE_NLE_ATTR)
> 
> -%define PDP_1G(x) ((x << 30) + PAGE_PDP_1G_ATTR)
> +%define PDP_1G(x) ((x << 30) + PAGE_BLP_ATTR)
> 
>  ALIGN 16
> 
> @@ -37,7 +47,7 @@ TopLevelPageDirectory:
>      ;
>      ; Top level Page Directory Pointers (1 * 512GB entry)
>      ;
> -    DQ      PDP(0x1000)
> +    DQ      PG_NLE(0x1000)
> 
>      TIMES 0x1000-PGTBLS_OFFSET($) DB 0
>      ;
> diff --git a/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables2M.asm
> b/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables2M.asm
> index b97df384ac..e46694c799 100644
> --- a/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables2M.asm
> +++ b/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables2M.asm
> @@ -2,7 +2,7 @@
>  ; @file
>  ; Emits Page Tables for 1:1 mapping of the addresses 0 - 0x100000000 (4GB)
>  ;
> -; Copyright (c) 2008 - 2014, Intel Corporation. All rights reserved.<BR>
> +; Copyright (c) 2008 - 2023, Intel Corporation. All rights reserved.<BR>
>  ; SPDX-License-Identifier: BSD-2-Clause-Patent
>  ;
>  ;------------------------------------------------------------------------------
> @@ -11,29 +11,36 @@ BITS    64
> 
>  %define ALIGN_TOP_TO_4K_FOR_PAGING
> 
> -%define PAGE_2M_PDE_ATTR (PAGE_SIZE + \
> +;
> +; Page table big leaf page attribute
> +; Big leaf page contains PDPTE 1GB page and PDE 2MB page
> +;
> +%define PAGE_BLP_ATTR   (PAGE_SIZE + \
>                            PAGE_ACCESSED + \
>                            PAGE_DIRTY + \
>                            PAGE_READ_WRITE + \
>                            PAGE_PRESENT)
> 
> -%define PAGE_PDP_ATTR (PAGE_ACCESSED + \
> -                       PAGE_READ_WRITE + \
> -                       PAGE_PRESENT)
> +;
> +; Page table no-leaf entry attribute
> +;
> +%define PAGE_NLE_ATTR (PAGE_ACCESSED + \
> +                        PAGE_READ_WRITE + \
> +                        PAGE_PRESENT)
> 
>  %define PGTBLS_OFFSET(x) ((x) - TopLevelPageDirectory)
>  %define PGTBLS_ADDR(x) (ADDR_OF(TopLevelPageDirectory) + (x))
> 
> -%define PDP(offset) (ADDR_OF(TopLevelPageDirectory) + (offset) + \
> -                     PAGE_PDP_ATTR)
> -%define PTE_2MB(x) ((x << 21) + PAGE_2M_PDE_ATTR)
> +%define PG_NLE(offset) (ADDR_OF(TopLevelPageDirectory) + (offset) + \
> +                    PAGE_NLE_ATTR)
> +%define PTE_2MB(x) ((x << 21) + PAGE_BLP_ATTR)
> 
>  TopLevelPageDirectory:
> 
>      ;
>      ; Top level Page Directory Pointers (1 * 512GB entry)
>      ;
> -    DQ      PDP(0x1000)
> +    DQ      PG_NLE(0x1000)
> 
> 
>      ;
> @@ -41,10 +48,10 @@ TopLevelPageDirectory:
>      ;
>      TIMES 0x1000-PGTBLS_OFFSET($) DB 0
> 
> -    DQ      PDP(0x2000)
> -    DQ      PDP(0x3000)
> -    DQ      PDP(0x4000)
> -    DQ      PDP(0x5000)
> +    DQ      PG_NLE(0x2000)
> +    DQ      PG_NLE(0x3000)
> +    DQ      PG_NLE(0x4000)
> +    DQ      PG_NLE(0x5000)
> 
>      ;
>      ; Page Table Entries (2048 * 2MB entries => 4GB)
> --
> 2.31.1.windows.1


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

* Re: [PATCH v3 2/5] UefiCpuPkg/ResetVector: Simplify page table creation in ResetVector
  2023-04-28  6:42 ` [PATCH v3 2/5] UefiCpuPkg/ResetVector: Simplify page table creation in ResetVector Zhiguang Liu
@ 2023-04-28  9:05   ` Ni, Ray
  0 siblings, 0 replies; 12+ messages in thread
From: Ni, Ray @ 2023-04-28  9:05 UTC (permalink / raw)
  To: Liu, Zhiguang, devel@edk2.groups.io
  Cc: Dong, Eric, Kumar, Rahul R, Gerd Hoffmann, De, Debkumar,
	West, Catharine

The changes look good to me.
Please change the macro per comments to patch #1, then update this patch accordingly.

> -----Original Message-----
> From: Liu, Zhiguang <zhiguang.liu@intel.com>
> Sent: Friday, April 28, 2023 2:42 PM
> To: devel@edk2.groups.io
> Cc: Liu, Zhiguang <zhiguang.liu@intel.com>; Dong, Eric
> <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Kumar, Rahul R
> <rahul.r.kumar@intel.com>; Gerd Hoffmann <kraxel@redhat.com>; De,
> Debkumar <debkumar.de@intel.com>; West, Catharine
> <catharine.west@intel.com>
> Subject: [PATCH v3 2/5] UefiCpuPkg/ResetVector: Simplify page table
> creation in ResetVector
> 
> Currently, page table creation has many hard-code values about the
> offset to the start of page table. To simplify it, add Labels such
> as Pml4, Pdp and Pd, so that we can remove many hard-code values
> 
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Debkumar De <debkumar.de@intel.com>
> Cc: Catharine West <catharine.west@intel.com>
> Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
> ---
>  .../ResetVector/Vtf0/Ia32/PageTables64.asm    |  4 +--
>  .../ResetVector/Vtf0/X64/PageTables1G.asm     | 18 ++++------
>  .../ResetVector/Vtf0/X64/PageTables2M.asm     | 34 ++++++++-----------
>  3 files changed, 24 insertions(+), 32 deletions(-)
> 
> diff --git a/UefiCpuPkg/ResetVector/Vtf0/Ia32/PageTables64.asm
> b/UefiCpuPkg/ResetVector/Vtf0/Ia32/PageTables64.asm
> index 87a4125d4b..f188da20ba 100644
> --- a/UefiCpuPkg/ResetVector/Vtf0/Ia32/PageTables64.asm
> +++ b/UefiCpuPkg/ResetVector/Vtf0/Ia32/PageTables64.asm
> @@ -2,7 +2,7 @@
>  ; @file
>  ; Sets the CR3 register for 64-bit paging
>  ;
> -; Copyright (c) 2008 - 2013, Intel Corporation. All rights reserved.<BR>
> +; Copyright (c) 2008 - 2023, Intel Corporation. All rights reserved.<BR>
>  ; SPDX-License-Identifier: BSD-2-Clause-Patent
>  ;
>  ;------------------------------------------------------------------------------
> @@ -17,7 +17,7 @@ SetCr3ForPageTables64:
>      ;
>      ; These pages are built into the ROM image in X64/PageTables.asm
>      ;
> -    mov     eax, ADDR_OF(TopLevelPageDirectory)
> +    mov     eax, ADDR_OF(Pml4)
>      mov     cr3, eax
> 
>      OneTimeCallRet SetCr3ForPageTables64
> diff --git a/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables1G.asm
> b/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables1G.asm
> index 97e90777c8..2b0de6020c 100644
> --- a/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables1G.asm
> +++ b/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables1G.asm
> @@ -29,35 +29,31 @@ BITS    64
>                          PAGE_PRESENT + \
>                          PAGE_SIZE)
> 
> -%define PGTBLS_OFFSET(x) ((x) - TopLevelPageDirectory)
> -%define PGTBLS_ADDR(x) (ADDR_OF(TopLevelPageDirectory) + (x))
> -
>  ;
>  ; Page table no-leaf entry
>  ;
> -%define PG_NLE(offset) (ADDR_OF(TopLevelPageDirectory) + (offset) + \
> +%define PG_NLE(address) (ADDR_OF(address) + \
>                      PAGE_NLE_ATTR)
> 
>  %define PDP_1G(x) ((x << 30) + PAGE_BLP_ATTR)
> 
>  ALIGN 16
> 
> -TopLevelPageDirectory:
> -
> +Pml4:
>      ;
> -    ; Top level Page Directory Pointers (1 * 512GB entry)
> +    ; PML4 (1 * 512GB entry)
>      ;
> -    DQ      PG_NLE(0x1000)
> +    DQ      PG_NLE(Pdp)
> +    TIMES   0x1000 - ($ - Pml4) DB 0
> 
> -    TIMES 0x1000-PGTBLS_OFFSET($) DB 0
> +Pdp:
>      ;
> -    ; Next level Page Directory Pointers (512 * 1GB entries => 512GB)
> +    ; Page-directory pointer table (512 * 1GB entries => 512GB)
>      ;
>  %assign i 0
>  %rep      512
>      DQ    PDP_1G(i)
>      %assign i i+1
>  %endrep
> -    TIMES 0x2000-PGTBLS_OFFSET($) DB 0
> 
>  EndOfPageTables:
> diff --git a/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables2M.asm
> b/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables2M.asm
> index e46694c799..cdf0fb41b7 100644
> --- a/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables2M.asm
> +++ b/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables2M.asm
> @@ -28,36 +28,32 @@ BITS    64
>                          PAGE_READ_WRITE + \
>                          PAGE_PRESENT)
> 
> -%define PGTBLS_OFFSET(x) ((x) - TopLevelPageDirectory)
> -%define PGTBLS_ADDR(x) (ADDR_OF(TopLevelPageDirectory) + (x))
> -
> -%define PG_NLE(offset) (ADDR_OF(TopLevelPageDirectory) + (offset) + \
> +%define PG_NLE(address) (ADDR_OF(address) + \
>                      PAGE_NLE_ATTR)
>  %define PTE_2MB(x) ((x << 21) + PAGE_BLP_ATTR)
> 
> -TopLevelPageDirectory:
> -
> +Pml4:
>      ;
> -    ; Top level Page Directory Pointers (1 * 512GB entry)
> +    ; PML4 (1 * 512GB entry)
>      ;
> -    DQ      PG_NLE(0x1000)
> -
> +    DQ      PG_NLE(Pdp)
> +    TIMES   0x1000 - ($ - Pml4) DB 0
> 
> +Pdp:
>      ;
> -    ; Next level Page Directory Pointers (4 * 1GB entries => 4GB)
> +    ; Page-directory pointer table (4 * 1GB entries => 4GB)
>      ;
> -    TIMES 0x1000-PGTBLS_OFFSET($) DB 0
> -
> -    DQ      PG_NLE(0x2000)
> -    DQ      PG_NLE(0x3000)
> -    DQ      PG_NLE(0x4000)
> -    DQ      PG_NLE(0x5000)
> +    DQ      PG_NLE(Pd)
> +    DQ      PG_NLE(Pd + 0x1000)
> +    DQ      PG_NLE(Pd + 0x2000)
> +    DQ      PG_NLE(Pd + 0x3000)
> +    TIMES   0x1000 - ($ - Pdp) DB 0
> 
> +Pd:
>      ;
> -    ; Page Table Entries (2048 * 2MB entries => 4GB)
> +    ; Page-Directory (2048 * 2MB entries => 4GB)
> +    ; Four pages below, each is pointed by one entry in Pdp.
>      ;
> -    TIMES 0x2000-PGTBLS_OFFSET($) DB 0
> -
>  %assign i 0
>  %rep    0x800
>      DQ      PTE_2MB(i)
> --
> 2.31.1.windows.1


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

* Re: [edk2-devel] [PATCH v3 1/5] UefiCpuPkg/ResetVector: Rename macros about page table.
       [not found] ` <175A0DD0D612EB33.26969@groups.io>
@ 2023-04-28  9:08   ` Ni, Ray
  0 siblings, 0 replies; 12+ messages in thread
From: Ni, Ray @ 2023-04-28  9:08 UTC (permalink / raw)
  To: devel@edk2.groups.io, Ni, Ray, Liu, Zhiguang
  Cc: Dong, Eric, Kumar, Rahul R, Gerd Hoffmann, De, Debkumar,
	West, Catharine

6. PDP_1G -> PDPTE_1GB

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ni, Ray
> Sent: Friday, April 28, 2023 5:03 PM
> To: Liu, Zhiguang <zhiguang.liu@intel.com>; devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>; Kumar, Rahul R
> <rahul.r.kumar@intel.com>; Gerd Hoffmann <kraxel@redhat.com>; De,
> Debkumar <debkumar.de@intel.com>; West, Catharine
> <catharine.west@intel.com>
> Subject: Re: [edk2-devel] [PATCH v3 1/5] UefiCpuPkg/ResetVector: Rename
> macros about page table.
> 
> 
> 
> > -----Original Message-----
> > From: Liu, Zhiguang <zhiguang.liu@intel.com>
> > Sent: Friday, April 28, 2023 2:42 PM
> > To: devel@edk2.groups.io
> > Cc: Liu, Zhiguang <zhiguang.liu@intel.com>; Dong, Eric
> > <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Kumar, Rahul R
> > <rahul.r.kumar@intel.com>; Gerd Hoffmann <kraxel@redhat.com>; De,
> > Debkumar <debkumar.de@intel.com>; West, Catharine
> > <catharine.west@intel.com>
> > Subject: [PATCH v3 1/5] UefiCpuPkg/ResetVector: Rename macros about
> > page table.
> >
> > This patch only renames macro, with no code logic impacted.
> > Two purpose to rename macro:
> > 1. Align some macro name in PageTables1G.asm and PageTables2M.asm,
> so
> > that these two files can be easily combined later.
> > 2. Some Macro names such as PDP are not accurate, since 4 level page
> > entry also uses this macro. PG_NLE (no leaf entry) is better.
> >
> > Cc: Eric Dong <eric.dong@intel.com>
> > Cc: Ray Ni <ray.ni@intel.com>
> > Cc: Rahul Kumar <rahul1.kumar@intel.com>
> > Cc: Gerd Hoffmann <kraxel@redhat.com>
> > Cc: Debkumar De <debkumar.de@intel.com>
> > Cc: Catharine West <catharine.west@intel.com>
> > Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
> > ---
> >  .../ResetVector/Vtf0/X64/PageTables1G.asm     | 24 ++++++++++----
> >  .../ResetVector/Vtf0/X64/PageTables2M.asm     | 33 +++++++++++--------
> >  2 files changed, 37 insertions(+), 20 deletions(-)
> >
> > diff --git a/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables1G.asm
> > b/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables1G.asm
> > index 19bd3d5a92..97e90777c8 100644
> > --- a/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables1G.asm
> > +++ b/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables1G.asm
> > @@ -2,7 +2,7 @@
> >  ; @file
> >  ; Emits Page Tables for 1:1 mapping of the addresses 0 - 0x8000000000
> > (512GB)
> >  ;
> > -; Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
> > +; Copyright (c) 2021 - 2023, Intel Corporation. All rights reserved.<BR>
> >  ; SPDX-License-Identifier: BSD-2-Clause-Patent
> >  ; Linear-Address Translation to a 1-GByte Page
> >  ;
> > @@ -12,11 +12,18 @@ BITS    64
> >
> >  %define ALIGN_TOP_TO_4K_FOR_PAGING
> >
> > -%define PAGE_PDP_ATTR (PAGE_ACCESSED + \
> > +;
> > +; Page table no-leaf entry attribute
> 
> 1. non-leaf entry attribute
> 
> > +;
> > +%define PAGE_NLE_ATTR (PAGE_ACCESSED + \
> >                          PAGE_READ_WRITE + \
> >                          PAGE_PRESENT)
> >
> > -%define PAGE_PDP_1G_ATTR (PAGE_ACCESSED + \
> > +;
> > +; Page table big leaf page attribute
> 
> 2. Page table big leaf entry attribute
> 
> > +; Big leaf page contains PDPTE 1GB page and PDE 2MB page
> 
> 3. PDPTE 1GB entry or PDE 2MB entry
> 
> > +;
> > +%define PAGE_BLP_ATTR (PAGE_ACCESSED + \
> 
> 4. PAGE_BLE_ATTR
> 
> 
> >                          PAGE_READ_WRITE + \
> >                          PAGE_DIRTY + \
> >                          PAGE_PRESENT + \
> > @@ -25,10 +32,13 @@ BITS    64
> >  %define PGTBLS_OFFSET(x) ((x) - TopLevelPageDirectory)
> >  %define PGTBLS_ADDR(x) (ADDR_OF(TopLevelPageDirectory) + (x))
> >
> > -%define PDP(offset) (ADDR_OF(TopLevelPageDirectory) + (offset) + \
> > -                    PAGE_PDP_ATTR)
> > +;
> > +; Page table no-leaf entry
> > +;
> > +%define PG_NLE(offset) (ADDR_OF(TopLevelPageDirectory) + (offset) + \
> 
> 5. PAGE_NLE
> Similar comments to the following changes.
> 
> 
> > +                    PAGE_NLE_ATTR)
> >
> > -%define PDP_1G(x) ((x << 30) + PAGE_PDP_1G_ATTR)
> > +%define PDP_1G(x) ((x << 30) + PAGE_BLP_ATTR)
> >
> >  ALIGN 16
> >
> > @@ -37,7 +47,7 @@ TopLevelPageDirectory:
> >      ;
> >      ; Top level Page Directory Pointers (1 * 512GB entry)
> >      ;
> > -    DQ      PDP(0x1000)
> > +    DQ      PG_NLE(0x1000)
> >
> >      TIMES 0x1000-PGTBLS_OFFSET($) DB 0
> >      ;
> > diff --git a/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables2M.asm
> > b/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables2M.asm
> > index b97df384ac..e46694c799 100644
> > --- a/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables2M.asm
> > +++ b/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables2M.asm
> > @@ -2,7 +2,7 @@
> >  ; @file
> >  ; Emits Page Tables for 1:1 mapping of the addresses 0 - 0x100000000 (4GB)
> >  ;
> > -; Copyright (c) 2008 - 2014, Intel Corporation. All rights reserved.<BR>
> > +; Copyright (c) 2008 - 2023, Intel Corporation. All rights reserved.<BR>
> >  ; SPDX-License-Identifier: BSD-2-Clause-Patent
> >  ;
> >  ;------------------------------------------------------------------------------
> > @@ -11,29 +11,36 @@ BITS    64
> >
> >  %define ALIGN_TOP_TO_4K_FOR_PAGING
> >
> > -%define PAGE_2M_PDE_ATTR (PAGE_SIZE + \
> > +;
> > +; Page table big leaf page attribute
> > +; Big leaf page contains PDPTE 1GB page and PDE 2MB page
> > +;
> > +%define PAGE_BLP_ATTR   (PAGE_SIZE + \
> >                            PAGE_ACCESSED + \
> >                            PAGE_DIRTY + \
> >                            PAGE_READ_WRITE + \
> >                            PAGE_PRESENT)
> >
> > -%define PAGE_PDP_ATTR (PAGE_ACCESSED + \
> > -                       PAGE_READ_WRITE + \
> > -                       PAGE_PRESENT)
> > +;
> > +; Page table no-leaf entry attribute
> > +;
> > +%define PAGE_NLE_ATTR (PAGE_ACCESSED + \
> > +                        PAGE_READ_WRITE + \
> > +                        PAGE_PRESENT)
> >
> >  %define PGTBLS_OFFSET(x) ((x) - TopLevelPageDirectory)
> >  %define PGTBLS_ADDR(x) (ADDR_OF(TopLevelPageDirectory) + (x))
> >
> > -%define PDP(offset) (ADDR_OF(TopLevelPageDirectory) + (offset) + \
> > -                     PAGE_PDP_ATTR)
> > -%define PTE_2MB(x) ((x << 21) + PAGE_2M_PDE_ATTR)
> > +%define PG_NLE(offset) (ADDR_OF(TopLevelPageDirectory) + (offset) + \
> > +                    PAGE_NLE_ATTR)
> > +%define PTE_2MB(x) ((x << 21) + PAGE_BLP_ATTR)
> >
> >  TopLevelPageDirectory:
> >
> >      ;
> >      ; Top level Page Directory Pointers (1 * 512GB entry)
> >      ;
> > -    DQ      PDP(0x1000)
> > +    DQ      PG_NLE(0x1000)
> >
> >
> >      ;
> > @@ -41,10 +48,10 @@ TopLevelPageDirectory:
> >      ;
> >      TIMES 0x1000-PGTBLS_OFFSET($) DB 0
> >
> > -    DQ      PDP(0x2000)
> > -    DQ      PDP(0x3000)
> > -    DQ      PDP(0x4000)
> > -    DQ      PDP(0x5000)
> > +    DQ      PG_NLE(0x2000)
> > +    DQ      PG_NLE(0x3000)
> > +    DQ      PG_NLE(0x4000)
> > +    DQ      PG_NLE(0x5000)
> >
> >      ;
> >      ; Page Table Entries (2048 * 2MB entries => 4GB)
> > --
> > 2.31.1.windows.1
> 
> 
> 
> 
> 


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

* Re: [PATCH v3 3/5] UefiCpuPkg/ResetVector: Combine PageTables1G.asm and PageTables2M.asm
  2023-04-28  6:42 ` [PATCH v3 3/5] UefiCpuPkg/ResetVector: Combine PageTables1G.asm and PageTables2M.asm Zhiguang Liu
@ 2023-04-28  9:09   ` Ni, Ray
  0 siblings, 0 replies; 12+ messages in thread
From: Ni, Ray @ 2023-04-28  9:09 UTC (permalink / raw)
  To: Liu, Zhiguang, devel@edk2.groups.io
  Cc: Dong, Eric, Kumar, Rahul R, Gerd Hoffmann, De, Debkumar,
	West, Catharine

The changes look good to me.
Please change the macro per comments to patch #1, then update this patch accordingly.

> -----Original Message-----
> From: Liu, Zhiguang <zhiguang.liu@intel.com>
> Sent: Friday, April 28, 2023 2:42 PM
> To: devel@edk2.groups.io
> Cc: Liu, Zhiguang <zhiguang.liu@intel.com>; Dong, Eric
> <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Kumar, Rahul R
> <rahul.r.kumar@intel.com>; Gerd Hoffmann <kraxel@redhat.com>; De,
> Debkumar <debkumar.de@intel.com>; West, Catharine
> <catharine.west@intel.com>
> Subject: [PATCH v3 3/5] UefiCpuPkg/ResetVector: Combine
> PageTables1G.asm and PageTables2M.asm
> 
> Combine PageTables1G.asm and PageTables2M.asm to reuse code.
> 
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Debkumar De <debkumar.de@intel.com>
> Cc: Catharine West <catharine.west@intel.com>
> Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
> ---
>  UefiCpuPkg/ResetVector/Vtf0/Vtf0.nasmb        |  8 +--
>  .../X64/{PageTables1G.asm => PageTables.asm}  | 38 ++++++++---
>  .../ResetVector/Vtf0/X64/PageTables2M.asm     | 63 -------------------
>  3 files changed, 33 insertions(+), 76 deletions(-)
>  rename UefiCpuPkg/ResetVector/Vtf0/X64/{PageTables1G.asm =>
> PageTables.asm} (58%)
>  delete mode 100644 UefiCpuPkg/ResetVector/Vtf0/X64/PageTables2M.asm
> 
> diff --git a/UefiCpuPkg/ResetVector/Vtf0/Vtf0.nasmb
> b/UefiCpuPkg/ResetVector/Vtf0/Vtf0.nasmb
> index bdea1fb875..136361e62c 100644
> --- a/UefiCpuPkg/ResetVector/Vtf0/Vtf0.nasmb
> +++ b/UefiCpuPkg/ResetVector/Vtf0/Vtf0.nasmb
> @@ -2,7 +2,7 @@
>  ; @file
>  ; This file includes all other code files to assemble the reset vector code
>  ;
> -; Copyright (c) 2008 - 2013, Intel Corporation. All rights reserved.<BR>
> +; Copyright (c) 2008 - 2023, Intel Corporation. All rights reserved.<BR>
>  ; SPDX-License-Identifier: BSD-2-Clause-Patent
>  ;
>  ;------------------------------------------------------------------------------
> @@ -38,11 +38,7 @@
>  %include "PageTables.inc"
> 
>  %ifdef ARCH_X64
> -  %ifdef PAGE_TABLE_1G
> -    %include "X64/PageTables1G.asm"
> -  %else
> -    %include "X64/PageTables2M.asm"
> -  %endif
> +  %include "X64/PageTables.asm"
>  %endif
> 
>  %ifdef DEBUG_PORT80
> diff --git a/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables1G.asm
> b/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables.asm
> similarity index 58%
> rename from UefiCpuPkg/ResetVector/Vtf0/X64/PageTables1G.asm
> rename to UefiCpuPkg/ResetVector/Vtf0/X64/PageTables.asm
> index 2b0de6020c..469fed0006 100644
> --- a/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables1G.asm
> +++ b/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables.asm
> @@ -1,10 +1,11 @@
>  ;------------------------------------------------------------------------------
>  ; @file
> -; Emits Page Tables for 1:1 mapping of the addresses 0 - 0x8000000000
> (512GB)
> +; Emits Page Tables for 1:1 mapping.
> +; If using 1G page table, map addresses 0 - 0x8000000000 (512GB),
> +; else, map addresses 0 - 0x100000000 (4GB)
>  ;
>  ; Copyright (c) 2021 - 2023, Intel Corporation. All rights reserved.<BR>
>  ; SPDX-License-Identifier: BSD-2-Clause-Patent
> -; Linear-Address Translation to a 1-GByte Page
>  ;
>  ;------------------------------------------------------------------------------
> 
> @@ -36,6 +37,7 @@ BITS    64
>                      PAGE_NLE_ATTR)
> 
>  %define PDP_1G(x) ((x << 30) + PAGE_BLP_ATTR)
> +%define PTE_2MB(x) ((x << 21) + PAGE_BLP_ATTR)
> 
>  ALIGN 16
> 
> @@ -46,14 +48,36 @@ Pml4:
>      DQ      PG_NLE(Pdp)
>      TIMES   0x1000 - ($ - Pml4) DB 0
> 
> +%ifdef PAGE_TABLE_1G
>  Pdp:
>      ;
>      ; Page-directory pointer table (512 * 1GB entries => 512GB)
>      ;
> -%assign i 0
> -%rep      512
> -    DQ    PDP_1G(i)
> -    %assign i i+1
> -%endrep
> +    %assign i 0
> +    %rep      512
> +        DQ    PDP_1G(i)
> +        %assign i i+1
> +    %endrep
> +%else
> +Pdp:
> +    ;
> +    ; Page-directory pointer table (4 * 1GB entries => 4GB)
> +    ;
> +    DQ      PG_NLE(Pd)
> +    DQ      PG_NLE(Pd + 0x1000)
> +    DQ      PG_NLE(Pd + 0x2000)
> +    DQ      PG_NLE(Pd + 0x3000)
> +    TIMES   0x1000 - ($ - Pdp) DB 0
> 
> +Pd:
> +    ;
> +    ; Page-Directory (2048 * 2MB entries => 4GB)
> +    ; Four pages below, each is pointed by one entry in Pdp.
> +    ;
> +    %assign i 0
> +    %rep    0x800
> +        DQ      PTE_2MB(i)
> +        %assign i i+1
> +    %endrep
> +%endif
>  EndOfPageTables:
> diff --git a/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables2M.asm
> b/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables2M.asm
> deleted file mode 100644
> index cdf0fb41b7..0000000000
> --- a/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables2M.asm
> +++ /dev/null
> @@ -1,63 +0,0 @@
> -;------------------------------------------------------------------------------
> -; @file
> -; Emits Page Tables for 1:1 mapping of the addresses 0 - 0x100000000 (4GB)
> -;
> -; Copyright (c) 2008 - 2023, Intel Corporation. All rights reserved.<BR>
> -; SPDX-License-Identifier: BSD-2-Clause-Patent
> -;
> -;------------------------------------------------------------------------------
> -
> -BITS    64
> -
> -%define ALIGN_TOP_TO_4K_FOR_PAGING
> -
> -;
> -; Page table big leaf page attribute
> -; Big leaf page contains PDPTE 1GB page and PDE 2MB page
> -;
> -%define PAGE_BLP_ATTR   (PAGE_SIZE + \
> -                          PAGE_ACCESSED + \
> -                          PAGE_DIRTY + \
> -                          PAGE_READ_WRITE + \
> -                          PAGE_PRESENT)
> -
> -;
> -; Page table no-leaf entry attribute
> -;
> -%define PAGE_NLE_ATTR (PAGE_ACCESSED + \
> -                        PAGE_READ_WRITE + \
> -                        PAGE_PRESENT)
> -
> -%define PG_NLE(address) (ADDR_OF(address) + \
> -                    PAGE_NLE_ATTR)
> -%define PTE_2MB(x) ((x << 21) + PAGE_BLP_ATTR)
> -
> -Pml4:
> -    ;
> -    ; PML4 (1 * 512GB entry)
> -    ;
> -    DQ      PG_NLE(Pdp)
> -    TIMES   0x1000 - ($ - Pml4) DB 0
> -
> -Pdp:
> -    ;
> -    ; Page-directory pointer table (4 * 1GB entries => 4GB)
> -    ;
> -    DQ      PG_NLE(Pd)
> -    DQ      PG_NLE(Pd + 0x1000)
> -    DQ      PG_NLE(Pd + 0x2000)
> -    DQ      PG_NLE(Pd + 0x3000)
> -    TIMES   0x1000 - ($ - Pdp) DB 0
> -
> -Pd:
> -    ;
> -    ; Page-Directory (2048 * 2MB entries => 4GB)
> -    ; Four pages below, each is pointed by one entry in Pdp.
> -    ;
> -%assign i 0
> -%rep    0x800
> -    DQ      PTE_2MB(i)
> -    %assign i i+1
> -%endrep
> -
> -EndOfPageTables:
> --
> 2.31.1.windows.1


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

* Re: [PATCH v3 4/5] UefiCpuPkg/ResetVector: Modify Page Table in ResetVector
  2023-04-28  6:42 ` [PATCH v3 4/5] UefiCpuPkg/ResetVector: Modify Page Table in ResetVector Zhiguang Liu
@ 2023-04-28  9:12   ` Ni, Ray
  2023-05-01  7:47   ` [edk2-devel] " Ard Biesheuvel
  1 sibling, 0 replies; 12+ messages in thread
From: Ni, Ray @ 2023-04-28  9:12 UTC (permalink / raw)
  To: Liu, Zhiguang, devel@edk2.groups.io
  Cc: Dong, Eric, Kumar, Rahul R, Gerd Hoffmann, De, Debkumar,
	West, Catharine

The changes look good to me.
Please change the macro per comments to patch #1, then update this patch accordingly.

> -----Original Message-----
> From: Liu, Zhiguang <zhiguang.liu@intel.com>
> Sent: Friday, April 28, 2023 2:42 PM
> To: devel@edk2.groups.io
> Cc: Liu, Zhiguang <zhiguang.liu@intel.com>; Dong, Eric
> <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Kumar, Rahul R
> <rahul.r.kumar@intel.com>; Gerd Hoffmann <kraxel@redhat.com>; De,
> Debkumar <debkumar.de@intel.com>; West, Catharine
> <catharine.west@intel.com>
> Subject: [PATCH v3 4/5] UefiCpuPkg/ResetVector: Modify Page Table in
> ResetVector
> 
> In ResetVector, if create page table, its highest address is fixed
> because after page table, code layout is fixed(4K for normal code,
> and another 4K only contains reset vector code).
> Today's implementation organizes the page table as following if 1G
> page table is used:
>   4G-16K: PML4 page (PML4[0] points to 4G-12K)
>   4G-12K: PDP page
>   CR3 is set to 4G-16K
> When 2M page table is used, the layout is as following:
>   4G-32K: PML4 page (PML4[0] points to 4G-28K)
>   4G-28K: PDP page (PDP entries point to PD pages)
>   4G-24K: PD page mapping 0-1G
>   4G-20K: PD page mapping 1-2G
>   4G-16K: PD page mapping 2-3G
>   4G-12K: PD page mapping 3-4G
>   CR3 is set to 4G-32K
> CR3 doesn't point to a fixed location which is a bit hard to debug at
> runtime.
> 
> The new page table layout will always put PML4 in highest address
> When 1G page table is used, the layout is as following:
>   4G-16K: PDP page
>   4G-12K: PML4 page (PML4[0] points to 4G-16K)
> When 2M page table is used, the layout is as following:
>   4G-32K: PD page mapping 0-1G
>   4G-28K: PD page mapping 1-2G
>   4G-24K: PD page mapping 2-3G
>   4G-20K: PD page mapping 3-4G
>   4G-16K: PDP page (PDP entries point to PD pages)
>   4G-12K: PML4 page (PML4[0] points to 4G-16K)
> CR3 is always set to 4G-12K
> So, this patch can improve biodegradability by make sure the init
> CR3 pointing to a fixed address(4G-12K).
> 
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Debkumar De <debkumar.de@intel.com>
> Cc: Catharine West <catharine.west@intel.com>
> Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
> ---
>  .../ResetVector/Vtf0/X64/PageTables.asm       | 32 +++++++++----------
>  1 file changed, 16 insertions(+), 16 deletions(-)
> 
> diff --git a/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables.asm
> b/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables.asm
> index 469fed0006..4ff68cddef 100644
> --- a/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables.asm
> +++ b/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables.asm
> @@ -41,13 +41,6 @@ BITS    64
> 
>  ALIGN 16
> 
> -Pml4:
> -    ;
> -    ; PML4 (1 * 512GB entry)
> -    ;
> -    DQ      PG_NLE(Pdp)
> -    TIMES   0x1000 - ($ - Pml4) DB 0
> -
>  %ifdef PAGE_TABLE_1G
>  Pdp:
>      ;
> @@ -59,6 +52,16 @@ Pdp:
>          %assign i i+1
>      %endrep
>  %else
> +Pd:
> +    ;
> +    ; Page-Directory (2048 * 2MB entries => 4GB)
> +    ; Four pages below, each is pointed by one entry in Pdp.
> +    ;
> +    %assign i 0
> +    %rep    0x800
> +        DQ      PTE_2MB(i)
> +        %assign i i+1
> +    %endrep
>  Pdp:
>      ;
>      ; Page-directory pointer table (4 * 1GB entries => 4GB)
> @@ -69,15 +72,12 @@ Pdp:
>      DQ      PG_NLE(Pd + 0x3000)
>      TIMES   0x1000 - ($ - Pdp) DB 0
> 
> -Pd:
> +%endif
> +
> +Pml4:
>      ;
> -    ; Page-Directory (2048 * 2MB entries => 4GB)
> -    ; Four pages below, each is pointed by one entry in Pdp.
> +    ; PML4 (1 * 512GB entry)
>      ;
> -    %assign i 0
> -    %rep    0x800
> -        DQ      PTE_2MB(i)
> -        %assign i i+1
> -    %endrep
> -%endif
> +    DQ      PG_NLE(Pdp)
> +    TIMES   0x1000 - ($ - Pml4) DB 0
>  EndOfPageTables:
> --
> 2.31.1.windows.1


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

* Re: [PATCH v3 5/5] UefiCpuPkg/ResetVector: Support 5 level page table in ResetVector
  2023-04-28  6:42 ` [PATCH v3 5/5] UefiCpuPkg/ResetVector: Support 5 level page table " Zhiguang Liu
@ 2023-04-28  9:15   ` Ni, Ray
  0 siblings, 0 replies; 12+ messages in thread
From: Ni, Ray @ 2023-04-28  9:15 UTC (permalink / raw)
  To: Liu, Zhiguang, devel@edk2.groups.io
  Cc: Dong, Eric, Kumar, Rahul R, Gerd Hoffmann, De, Debkumar,
	West, Catharine

The changes look good to me.
Please change the macro per comments to patch #1, then update this patch accordingly.

> -----Original Message-----
> From: Liu, Zhiguang <zhiguang.liu@intel.com>
> Sent: Friday, April 28, 2023 2:42 PM
> To: devel@edk2.groups.io
> Cc: Liu, Zhiguang <zhiguang.liu@intel.com>; Dong, Eric
> <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Kumar, Rahul R
> <rahul.r.kumar@intel.com>; Gerd Hoffmann <kraxel@redhat.com>; De,
> Debkumar <debkumar.de@intel.com>; West, Catharine
> <catharine.west@intel.com>
> Subject: [PATCH v3 5/5] UefiCpuPkg/ResetVector: Support 5 level page table
> in ResetVector
> 
> Add a macro USE_5_LEVEL_PAGE_TABLE to determine whether to create
> 5 level page table.
> If macro USE_5_LEVEL_PAGE_TABLE is defined, PML5Table is created
> at (4G-12K), while PML4Table is at (4G-16K). In runtime check, if
> 5level paging is supported, use PML5Table, otherwise, use PML4Table.
> If macro USE_5_LEVEL_PAGE_TABLE is not defined, to save space, 5level
> paging is not created, and 4level paging is at (4G-12K) and be used.
> 
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Debkumar De <debkumar.de@intel.com>
> Cc: Catharine West <catharine.west@intel.com>
> Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
> ---
>  .../ResetVector/Vtf0/Ia32/Flat32ToFlat64.asm  | 25 +++++++++++++++++--
>  .../ResetVector/Vtf0/Ia32/PageTables64.asm    | 24 ------------------
>  UefiCpuPkg/ResetVector/Vtf0/Vtf0.nasmb        |  1 -
>  .../ResetVector/Vtf0/X64/PageTables.asm       |  9 +++++++
>  4 files changed, 32 insertions(+), 27 deletions(-)
>  delete mode 100644 UefiCpuPkg/ResetVector/Vtf0/Ia32/PageTables64.asm
> 
> diff --git a/UefiCpuPkg/ResetVector/Vtf0/Ia32/Flat32ToFlat64.asm
> b/UefiCpuPkg/ResetVector/Vtf0/Ia32/Flat32ToFlat64.asm
> index 6891397c2a..f119f941a5 100644
> --- a/UefiCpuPkg/ResetVector/Vtf0/Ia32/Flat32ToFlat64.asm
> +++ b/UefiCpuPkg/ResetVector/Vtf0/Ia32/Flat32ToFlat64.asm
> @@ -2,7 +2,7 @@
>  ; @file
>  ; Transition from 32 bit flat protected mode into 64 bit flat protected mode
>  ;
> -; Copyright (c) 2008 - 2018, Intel Corporation. All rights reserved.<BR>
> +; Copyright (c) 2008 - 2023, Intel Corporation. All rights reserved.<BR>
>  ; SPDX-License-Identifier: BSD-2-Clause-Patent
>  ;
>  ;------------------------------------------------------------------------------
> @@ -14,7 +14,28 @@ BITS    32
>  ;
>  Transition32FlatTo64Flat:
> 
> -    OneTimeCall SetCr3ForPageTables64
> +%ifdef USE_5_LEVEL_PAGE_TABLE
> +    mov     eax, 0
> +    cpuid
> +    cmp     eax, 07h                    ; check if basic CPUID leaf contains leaf 07
> +    jb      NotSupport5LevelPaging      ; 5level paging not support, downgrade
> to 4level paging
> +    mov     eax, 07h                    ; check cpuid leaf 7, subleaf 0
> +    mov     ecx, 0
> +    cpuid
> +    bt      ecx, 16                     ; [Bits 16] Supports 5-level paging if 1.
> +    jnc     NotSupport5LevelPaging      ; 5level paging not support, downgrade
> to 4level paging
> +    mov     eax, ADDR_OF(Pml5)
> +    mov     cr3, eax
> +    mov     eax, cr4
> +    bts     eax, 12                     ; Set LA57=1.
> +    mov     cr4, eax
> +    jmp     SetCr3Done
> +NotSupport5LevelPaging:
> +%endif
> +
> +    mov     eax, ADDR_OF(Pml4)
> +    mov     cr3, eax
> +SetCr3Done:
> 
>      mov     eax, cr4
>      bts     eax, 5                      ; enable PAE
> diff --git a/UefiCpuPkg/ResetVector/Vtf0/Ia32/PageTables64.asm
> b/UefiCpuPkg/ResetVector/Vtf0/Ia32/PageTables64.asm
> deleted file mode 100644
> index f188da20ba..0000000000
> --- a/UefiCpuPkg/ResetVector/Vtf0/Ia32/PageTables64.asm
> +++ /dev/null
> @@ -1,24 +0,0 @@
> -;------------------------------------------------------------------------------
> -; @file
> -; Sets the CR3 register for 64-bit paging
> -;
> -; Copyright (c) 2008 - 2023, Intel Corporation. All rights reserved.<BR>
> -; SPDX-License-Identifier: BSD-2-Clause-Patent
> -;
> -;------------------------------------------------------------------------------
> -
> -BITS    32
> -
> -;
> -; Modified:  EAX
> -;
> -SetCr3ForPageTables64:
> -
> -    ;
> -    ; These pages are built into the ROM image in X64/PageTables.asm
> -    ;
> -    mov     eax, ADDR_OF(Pml4)
> -    mov     cr3, eax
> -
> -    OneTimeCallRet SetCr3ForPageTables64
> -
> diff --git a/UefiCpuPkg/ResetVector/Vtf0/Vtf0.nasmb
> b/UefiCpuPkg/ResetVector/Vtf0/Vtf0.nasmb
> index 136361e62c..5a6563bd34 100644
> --- a/UefiCpuPkg/ResetVector/Vtf0/Vtf0.nasmb
> +++ b/UefiCpuPkg/ResetVector/Vtf0/Vtf0.nasmb
> @@ -54,7 +54,6 @@
> 
>  %ifdef ARCH_X64
>  %include "Ia32/Flat32ToFlat64.asm"
> -%include "Ia32/PageTables64.asm"
>  %endif
> 
>  %include "Ia16/Real16ToFlat32.asm"
> diff --git a/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables.asm
> b/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables.asm
> index 4ff68cddef..5aa229eb14 100644
> --- a/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables.asm
> +++ b/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables.asm
> @@ -80,4 +80,13 @@ Pml4:
>      ;
>      DQ      PG_NLE(Pdp)
>      TIMES   0x1000 - ($ - Pml4) DB 0
> +
> +%ifdef USE_5_LEVEL_PAGE_TABLE
> +Pml5:
> +    ;
> +    ; Pml5 table (only first entry is present, pointing to Pml4)
> +    ;
> +    DQ      PG_NLE(Pml4)
> +    TIMES   0x1000 - ($ - Pml5) DB 0
> +%endif
>  EndOfPageTables:
> --
> 2.31.1.windows.1


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

* Re: [edk2-devel] [PATCH v3 4/5] UefiCpuPkg/ResetVector: Modify Page Table in ResetVector
  2023-04-28  6:42 ` [PATCH v3 4/5] UefiCpuPkg/ResetVector: Modify Page Table in ResetVector Zhiguang Liu
  2023-04-28  9:12   ` Ni, Ray
@ 2023-05-01  7:47   ` Ard Biesheuvel
  1 sibling, 0 replies; 12+ messages in thread
From: Ard Biesheuvel @ 2023-05-01  7:47 UTC (permalink / raw)
  To: devel, zhiguang.liu
  Cc: Eric Dong, Ray Ni, Rahul Kumar, Gerd Hoffmann, Debkumar De,
	Catharine West

On Fri, 28 Apr 2023 at 08:43, Zhiguang Liu <zhiguang.liu@intel.com> wrote:
>
> In ResetVector, if create page table, its highest address is fixed
> because after page table, code layout is fixed(4K for normal code,
> and another 4K only contains reset vector code).
> Today's implementation organizes the page table as following if 1G
> page table is used:
>   4G-16K: PML4 page (PML4[0] points to 4G-12K)
>   4G-12K: PDP page
>   CR3 is set to 4G-16K
> When 2M page table is used, the layout is as following:
>   4G-32K: PML4 page (PML4[0] points to 4G-28K)
>   4G-28K: PDP page (PDP entries point to PD pages)
>   4G-24K: PD page mapping 0-1G
>   4G-20K: PD page mapping 1-2G
>   4G-16K: PD page mapping 2-3G
>   4G-12K: PD page mapping 3-4G
>   CR3 is set to 4G-32K
> CR3 doesn't point to a fixed location which is a bit hard to debug at
> runtime.
>
> The new page table layout will always put PML4 in highest address
> When 1G page table is used, the layout is as following:
>   4G-16K: PDP page
>   4G-12K: PML4 page (PML4[0] points to 4G-16K)
> When 2M page table is used, the layout is as following:
>   4G-32K: PD page mapping 0-1G
>   4G-28K: PD page mapping 1-2G
>   4G-24K: PD page mapping 2-3G
>   4G-20K: PD page mapping 3-4G
>   4G-16K: PDP page (PDP entries point to PD pages)
>   4G-12K: PML4 page (PML4[0] points to 4G-16K)
> CR3 is always set to 4G-12K
> So, this patch can improve biodegradability

I am pretty sure you meant to type something else here


> by make sure the init
> CR3 pointing to a fixed address(4G-12K).
>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Debkumar De <debkumar.de@intel.com>
> Cc: Catharine West <catharine.west@intel.com>
> Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
> ---
>  .../ResetVector/Vtf0/X64/PageTables.asm       | 32 +++++++++----------
>  1 file changed, 16 insertions(+), 16 deletions(-)
>
> diff --git a/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables.asm b/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables.asm
> index 469fed0006..4ff68cddef 100644
> --- a/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables.asm
> +++ b/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables.asm
> @@ -41,13 +41,6 @@ BITS    64
>
>  ALIGN 16
>
> -Pml4:
> -    ;
> -    ; PML4 (1 * 512GB entry)
> -    ;
> -    DQ      PG_NLE(Pdp)
> -    TIMES   0x1000 - ($ - Pml4) DB 0
> -
>  %ifdef PAGE_TABLE_1G
>  Pdp:
>      ;
> @@ -59,6 +52,16 @@ Pdp:
>          %assign i i+1
>      %endrep
>  %else
> +Pd:
> +    ;
> +    ; Page-Directory (2048 * 2MB entries => 4GB)
> +    ; Four pages below, each is pointed by one entry in Pdp.
> +    ;
> +    %assign i 0
> +    %rep    0x800
> +        DQ      PTE_2MB(i)
> +        %assign i i+1
> +    %endrep
>  Pdp:
>      ;
>      ; Page-directory pointer table (4 * 1GB entries => 4GB)
> @@ -69,15 +72,12 @@ Pdp:
>      DQ      PG_NLE(Pd + 0x3000)
>      TIMES   0x1000 - ($ - Pdp) DB 0
>
> -Pd:
> +%endif
> +
> +Pml4:
>      ;
> -    ; Page-Directory (2048 * 2MB entries => 4GB)
> -    ; Four pages below, each is pointed by one entry in Pdp.
> +    ; PML4 (1 * 512GB entry)
>      ;
> -    %assign i 0
> -    %rep    0x800
> -        DQ      PTE_2MB(i)
> -        %assign i i+1
> -    %endrep
> -%endif
> +    DQ      PG_NLE(Pdp)
> +    TIMES   0x1000 - ($ - Pml4) DB 0
>  EndOfPageTables:
> --
> 2.31.1.windows.1
>
>
>
> 
>
>

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

end of thread, other threads:[~2023-05-01  7:48 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-04-28  6:42 [PATCH v3 1/5] UefiCpuPkg/ResetVector: Rename macros about page table Zhiguang Liu
2023-04-28  6:42 ` [PATCH v3 2/5] UefiCpuPkg/ResetVector: Simplify page table creation in ResetVector Zhiguang Liu
2023-04-28  9:05   ` Ni, Ray
2023-04-28  6:42 ` [PATCH v3 3/5] UefiCpuPkg/ResetVector: Combine PageTables1G.asm and PageTables2M.asm Zhiguang Liu
2023-04-28  9:09   ` Ni, Ray
2023-04-28  6:42 ` [PATCH v3 4/5] UefiCpuPkg/ResetVector: Modify Page Table in ResetVector Zhiguang Liu
2023-04-28  9:12   ` Ni, Ray
2023-05-01  7:47   ` [edk2-devel] " Ard Biesheuvel
2023-04-28  6:42 ` [PATCH v3 5/5] UefiCpuPkg/ResetVector: Support 5 level page table " Zhiguang Liu
2023-04-28  9:15   ` Ni, Ray
2023-04-28  9:03 ` [PATCH v3 1/5] UefiCpuPkg/ResetVector: Rename macros about page table Ni, Ray
     [not found] ` <175A0DD0D612EB33.26969@groups.io>
2023-04-28  9:08   ` [edk2-devel] " Ni, Ray

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