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....@intel.com>
> Sent: Friday, April 28, 2023 2:42 PM
> To: devel@edk2.groups.io
> Cc: Liu, Zhiguang <zhiguang....@intel.com>; Dong, Eric
> <eric.d...@intel.com>; Ni, Ray <ray...@intel.com>; Kumar, Rahul R
> <rahul.r.ku...@intel.com>; Gerd Hoffmann <kra...@redhat.com>; De,
> Debkumar <debkumar...@intel.com>; West, Catharine
> <catharine.w...@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.d...@intel.com>
> Cc: Ray Ni <ray...@intel.com>
> Cc: Rahul Kumar <rahul1.ku...@intel.com>
> Cc: Gerd Hoffmann <kra...@redhat.com>
> Cc: Debkumar De <debkumar...@intel.com>
> Cc: Catharine West <catharine.w...@intel.com>
> Signed-off-by: Zhiguang Liu <zhiguang....@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



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#103767): https://edk2.groups.io/g/devel/message/103767
Mute This Topic: https://groups.io/mt/98553401/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: 
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-


Reply via email to