public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [edk2-devel] [PATCH v2 0/2] MdeModulePkg: 5-level paging fixes
@ 2024-01-25  8:20 Gerd Hoffmann
  2024-01-25  8:20 ` [edk2-devel] [PATCH v2 1/2] MdeModulePkg: rename PcdUse5LevelPageTable to PcdEnable5LevelPageTable Gerd Hoffmann
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Gerd Hoffmann @ 2024-01-25  8:20 UTC (permalink / raw)
  To: devel
  Cc: Liming Gao, László Érsek, Oliver Steffen,
	Gerd Hoffmann

- rename PcdUse5LevelPageTable to PcdEnable5LevelPageTable
- relax 5-level paging assert

Gerd Hoffmann (2):
  MdeModulePkg: rename PcdUse5LevelPageTable to PcdEnable5LevelPageTable
  MdeModulePkg/VirtualMemory: fix 5-level paging assert

 MdeModulePkg/MdeModulePkg.dec                    | 2 +-
 MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf          | 2 +-
 MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c | 6 ++++--
 MdeModulePkg/MdeModulePkg.uni                    | 4 ++--
 4 files changed, 8 insertions(+), 6 deletions(-)

-- 
2.43.0



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



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

* [edk2-devel] [PATCH v2 1/2] MdeModulePkg: rename PcdUse5LevelPageTable to PcdEnable5LevelPageTable
  2024-01-25  8:20 [edk2-devel] [PATCH v2 0/2] MdeModulePkg: 5-level paging fixes Gerd Hoffmann
@ 2024-01-25  8:20 ` Gerd Hoffmann
  2024-01-25 12:01   ` Ni, Ray
  2024-01-25  8:20 ` [edk2-devel] [PATCH v2 2/2] MdeModulePkg/VirtualMemory: fix 5-level paging assert Gerd Hoffmann
  2024-01-25 23:22 ` [edk2-devel] [PATCH v2 0/2] MdeModulePkg: 5-level paging fixes Laszlo Ersek
  2 siblings, 1 reply; 8+ messages in thread
From: Gerd Hoffmann @ 2024-01-25  8:20 UTC (permalink / raw)
  To: devel
  Cc: Liming Gao, László Érsek, Oliver Steffen,
	Gerd Hoffmann

The PCD will allow but not require 5-level paging.  Whenever 5-level
paging is used or not will be decided by the ResetVector, by looking
at CPU capabilities.  Rename the PCD to make that clear.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 MdeModulePkg/MdeModulePkg.dec                    | 2 +-
 MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf          | 2 +-
 MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c | 4 ++--
 MdeModulePkg/MdeModulePkg.uni                    | 4 ++--
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index a2cd83345f5b..80047f029d36 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -2105,7 +2105,7 @@ [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
   #   TRUE  - 5-Level Paging will be enabled.<BR>
   #   FALSE - 5-Level Paging will not be enabled.<BR>
   # @Prompt Enable 5-Level Paging support in long mode.
-  gEfiMdeModulePkgTokenSpaceGuid.PcdUse5LevelPageTable|FALSE|BOOLEAN|0x0001105F
+  gEfiMdeModulePkgTokenSpaceGuid.PcdEnable5LevelPageTable|FALSE|BOOLEAN|0x0001105F
 
   ## Capsule In Ram is to use memory to deliver the capsules that will be processed after system
   #  reset.<BR><BR>
diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
index f1990eac7760..d73add2d814d 100644
--- a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
+++ b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
@@ -104,7 +104,7 @@ [Pcd.IA32,Pcd.X64]
   gEfiMdeModulePkgTokenSpaceGuid.PcdNullPointerDetectionPropertyMask    ## CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask               ## CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard                       ## CONSUMES
-  gEfiMdeModulePkgTokenSpaceGuid.PcdUse5LevelPageTable                  ## SOMETIMES_CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdEnable5LevelPageTable               ## SOMETIMES_CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdGhcbBase                            ## CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdGhcbSize                            ## CONSUMES
 
diff --git a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
index 980c2002d4f5..46528e4f719d 100644
--- a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
+++ b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
@@ -745,13 +745,13 @@ CreateIdentityMappingPageTables (
     //
     Cr4.UintN         = AsmReadCr4 ();
     Page5LevelSupport = (Cr4.Bits.LA57 != 0);
-    ASSERT (PcdGetBool (PcdUse5LevelPageTable) == Page5LevelSupport);
+    ASSERT (PcdGetBool (PcdEnable5LevelPageTable) == Page5LevelSupport);
   } else {
     //
     // If cpu runs in 32bit protected mode PEI, Page table Level in DXE is decided by PCD and feature capability.
     //
     Page5LevelSupport = FALSE;
-    if (PcdGetBool (PcdUse5LevelPageTable)) {
+    if (PcdGetBool (PcdEnable5LevelPageTable)) {
       AsmCpuidEx (
         CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS,
         CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS_SUB_LEAF_INFO,
diff --git a/MdeModulePkg/MdeModulePkg.uni b/MdeModulePkg/MdeModulePkg.uni
index a17d34d60b21..7a98dc64832b 100644
--- a/MdeModulePkg/MdeModulePkg.uni
+++ b/MdeModulePkg/MdeModulePkg.uni
@@ -1332,9 +1332,9 @@
                                                                                    "required to be accessed in PcdDxe driver entry point. So, its value must be set in PEI phase."
                                                                                    "It can't depend on EFI variable service, and can't be DynamicExHii PCD."
 
-#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdUse5LevelPageTable_PROMPT  #language en-US "Enable 5-Level Paging support in long mode"
+#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdEnable5LevelPageTable_PROMPT  #language en-US "Enable 5-Level Paging support in long mode"
 
-#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdUse5LevelPageTable_HELP  #language en-US "Indicates if 5-Level Paging will be enabled in long mode. 5-Level Paging will not be enabled"
+#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdEnable5LevelPageTable_HELP  #language en-US "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."
                                                                                     " TRUE  - 5-Level Paging will be enabled."
                                                                                     " FALSE - 5-Level Paging will not be enabled."
-- 
2.43.0



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



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

* [edk2-devel] [PATCH v2 2/2] MdeModulePkg/VirtualMemory: fix 5-level paging assert
  2024-01-25  8:20 [edk2-devel] [PATCH v2 0/2] MdeModulePkg: 5-level paging fixes Gerd Hoffmann
  2024-01-25  8:20 ` [edk2-devel] [PATCH v2 1/2] MdeModulePkg: rename PcdUse5LevelPageTable to PcdEnable5LevelPageTable Gerd Hoffmann
@ 2024-01-25  8:20 ` Gerd Hoffmann
  2024-01-25 23:22 ` [edk2-devel] [PATCH v2 0/2] MdeModulePkg: 5-level paging fixes Laszlo Ersek
  2 siblings, 0 replies; 8+ messages in thread
From: Gerd Hoffmann @ 2024-01-25  8:20 UTC (permalink / raw)
  To: devel
  Cc: Liming Gao, László Érsek, Oliver Steffen,
	Gerd Hoffmann

The ResetVector decides at runtime (depending on CPU capabilities)
whenever it uses 5-level paging or not.  Firmware builds with 5-level
paging enabled (PcdEnable5LevelPageTable=TRUE) may run run in 4-level
paging mode.  The code handles that just fine, by looking at the la57
bit in cr4 instead of checking PcdEnable5LevelPageTable.

So the ASSERT is too strict.  Change it to ensure the build was
configured with PcdEnable5LevelPageTable=TRUE when running in 5-level
paging mode.

Signed-off-by: Gerd Hoffmann <kraxel@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 46528e4f719d..9e194ef8cc55 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 (PcdEnable5LevelPageTable) == Page5LevelSupport);
+    if (Page5LevelSupport) {
+      ASSERT (PcdGetBool (PcdEnable5LevelPageTable));
+    }
   } 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 (#114375): https://edk2.groups.io/g/devel/message/114375
Mute This Topic: https://groups.io/mt/103950406/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



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

* Re: [edk2-devel] [PATCH v2 1/2] MdeModulePkg: rename PcdUse5LevelPageTable to PcdEnable5LevelPageTable
  2024-01-25  8:20 ` [edk2-devel] [PATCH v2 1/2] MdeModulePkg: rename PcdUse5LevelPageTable to PcdEnable5LevelPageTable Gerd Hoffmann
@ 2024-01-25 12:01   ` Ni, Ray
  2024-01-25 12:17     ` Gerd Hoffmann
  0 siblings, 1 reply; 8+ messages in thread
From: Ni, Ray @ 2024-01-25 12:01 UTC (permalink / raw)
  To: devel@edk2.groups.io, kraxel@redhat.com
  Cc: Liming Gao, László Érsek, Oliver Steffen

Rename an existing PCD might break lots of platform builds.

When 5-level paging capability was added to ResetVector, I also considered to remove the PcdUse5LevelPageTable reference from C code.

Let me think about it...

Thanks,
Ray
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Gerd
> Hoffmann
> Sent: Thursday, January 25, 2024 4:21 PM
> To: devel@edk2.groups.io
> Cc: Liming Gao <gaoliming@byosoft.com.cn>; László Érsek
> <lersek@redhat.com>; Oliver Steffen <osteffen@redhat.com>; Gerd Hoffmann
> <kraxel@redhat.com>
> Subject: [edk2-devel] [PATCH v2 1/2] MdeModulePkg: rename
> PcdUse5LevelPageTable to PcdEnable5LevelPageTable
> 
> The PCD will allow but not require 5-level paging.  Whenever 5-level
> paging is used or not will be decided by the ResetVector, by looking
> at CPU capabilities.  Rename the PCD to make that clear.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  MdeModulePkg/MdeModulePkg.dec                    | 2 +-
>  MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf          | 2 +-
>  MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c | 4 ++--
>  MdeModulePkg/MdeModulePkg.uni                    | 4 ++--
>  4 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/MdeModulePkg/MdeModulePkg.dec
> b/MdeModulePkg/MdeModulePkg.dec
> index a2cd83345f5b..80047f029d36 100644
> --- a/MdeModulePkg/MdeModulePkg.dec
> +++ b/MdeModulePkg/MdeModulePkg.dec
> @@ -2105,7 +2105,7 @@ [PcdsFixedAtBuild, PcdsPatchableInModule,
> PcdsDynamic, PcdsDynamicEx]
>    #   TRUE  - 5-Level Paging will be enabled.<BR>
>    #   FALSE - 5-Level Paging will not be enabled.<BR>
>    # @Prompt Enable 5-Level Paging support in long mode.
> -
> gEfiMdeModulePkgTokenSpaceGuid.PcdUse5LevelPageTable|FALSE|BOOLEA
> N|0x0001105F
> +
> gEfiMdeModulePkgTokenSpaceGuid.PcdEnable5LevelPageTable|FALSE|BOOLE
> AN|0x0001105F
> 
>    ## Capsule In Ram is to use memory to deliver the capsules that will be
> processed after system
>    #  reset.<BR><BR>
> diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
> b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
> index f1990eac7760..d73add2d814d 100644
> --- a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
> +++ b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
> @@ -104,7 +104,7 @@ [Pcd.IA32,Pcd.X64]
>    gEfiMdeModulePkgTokenSpaceGuid.PcdNullPointerDetectionPropertyMask
> ## CONSUMES
>    gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask               ##
> CONSUMES
>    gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard                       ##
> CONSUMES
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdUse5LevelPageTable                  ##
> SOMETIMES_CONSUMES
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdEnable5LevelPageTable               ##
> SOMETIMES_CONSUMES
>    gEfiMdeModulePkgTokenSpaceGuid.PcdGhcbBase                            ##
> CONSUMES
>    gEfiMdeModulePkgTokenSpaceGuid.PcdGhcbSize                            ##
> CONSUMES
> 
> diff --git a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
> b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
> index 980c2002d4f5..46528e4f719d 100644
> --- a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
> +++ b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
> @@ -745,13 +745,13 @@ CreateIdentityMappingPageTables (
>      //
>      Cr4.UintN         = AsmReadCr4 ();
>      Page5LevelSupport = (Cr4.Bits.LA57 != 0);
> -    ASSERT (PcdGetBool (PcdUse5LevelPageTable) == Page5LevelSupport);
> +    ASSERT (PcdGetBool (PcdEnable5LevelPageTable) == Page5LevelSupport);
>    } else {
>      //
>      // If cpu runs in 32bit protected mode PEI, Page table Level in DXE is decided
> by PCD and feature capability.
>      //
>      Page5LevelSupport = FALSE;
> -    if (PcdGetBool (PcdUse5LevelPageTable)) {
> +    if (PcdGetBool (PcdEnable5LevelPageTable)) {
>        AsmCpuidEx (
>          CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS,
>          CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS_SUB_LEAF_INFO,
> diff --git a/MdeModulePkg/MdeModulePkg.uni
> b/MdeModulePkg/MdeModulePkg.uni
> index a17d34d60b21..7a98dc64832b 100644
> --- a/MdeModulePkg/MdeModulePkg.uni
> +++ b/MdeModulePkg/MdeModulePkg.uni
> @@ -1332,9 +1332,9 @@
>                                                                                     "required to be accessed in PcdDxe
> driver entry point. So, its value must be set in PEI phase."
>                                                                                     "It can't depend on EFI variable
> service, and can't be DynamicExHii PCD."
> 
> -#string
> STR_gEfiMdeModulePkgTokenSpaceGuid_PcdUse5LevelPageTable_PROMPT
> #language en-US "Enable 5-Level Paging support in long mode"
> +#string
> STR_gEfiMdeModulePkgTokenSpaceGuid_PcdEnable5LevelPageTable_PROMP
> T  #language en-US "Enable 5-Level Paging support in long mode"
> 
> -#string
> STR_gEfiMdeModulePkgTokenSpaceGuid_PcdUse5LevelPageTable_HELP
> #language en-US "Indicates if 5-Level Paging will be enabled in long mode. 5-
> Level Paging will not be enabled"
> +#string
> STR_gEfiMdeModulePkgTokenSpaceGuid_PcdEnable5LevelPageTable_HELP
> #language en-US "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."
>                                                                                      " TRUE  - 5-Level Paging will be
> enabled."
>                                                                                      " FALSE - 5-Level Paging will not be
> enabled."
> --
> 2.43.0
> 
> 
> 
> 
> 



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



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

* Re: [edk2-devel] [PATCH v2 1/2] MdeModulePkg: rename PcdUse5LevelPageTable to PcdEnable5LevelPageTable
  2024-01-25 12:01   ` Ni, Ray
@ 2024-01-25 12:17     ` Gerd Hoffmann
  2024-01-26  2:49       ` Zhiguang Liu
  0 siblings, 1 reply; 8+ messages in thread
From: Gerd Hoffmann @ 2024-01-25 12:17 UTC (permalink / raw)
  To: Ni, Ray
  Cc: devel@edk2.groups.io, Liming Gao, László Érsek,
	Oliver Steffen

On Thu, Jan 25, 2024 at 12:01:46PM +0000, Ni, Ray wrote:
> Rename an existing PCD might break lots of platform builds.
> 
> When 5-level paging capability was added to ResetVector, I also
> considered to remove the PcdUse5LevelPageTable reference from C code.
> 
> Let me think about it...

Purging this completely from the source code is fine with me too.

From OVMF point of view this is not needed, all backward compatibility
concerns for old guests without 5-level paging support can be solved by
just removing la57 capability from the vCPU (qemu -cpu host,la57=off).

take care,
  Gerd



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



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

* Re: [edk2-devel] [PATCH v2 0/2] MdeModulePkg: 5-level paging fixes
  2024-01-25  8:20 [edk2-devel] [PATCH v2 0/2] MdeModulePkg: 5-level paging fixes Gerd Hoffmann
  2024-01-25  8:20 ` [edk2-devel] [PATCH v2 1/2] MdeModulePkg: rename PcdUse5LevelPageTable to PcdEnable5LevelPageTable Gerd Hoffmann
  2024-01-25  8:20 ` [edk2-devel] [PATCH v2 2/2] MdeModulePkg/VirtualMemory: fix 5-level paging assert Gerd Hoffmann
@ 2024-01-25 23:22 ` Laszlo Ersek
  2 siblings, 0 replies; 8+ messages in thread
From: Laszlo Ersek @ 2024-01-25 23:22 UTC (permalink / raw)
  To: devel, kraxel; +Cc: Liming Gao, Oliver Steffen

On 1/25/24 09:20, Gerd Hoffmann wrote:
> - rename PcdUse5LevelPageTable to PcdEnable5LevelPageTable
> - relax 5-level paging assert
> 
> Gerd Hoffmann (2):
>   MdeModulePkg: rename PcdUse5LevelPageTable to PcdEnable5LevelPageTable
>   MdeModulePkg/VirtualMemory: fix 5-level paging assert
> 
>  MdeModulePkg/MdeModulePkg.dec                    | 2 +-
>  MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf          | 2 +-
>  MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c | 6 ++++--
>  MdeModulePkg/MdeModulePkg.uni                    | 4 ++--
>  4 files changed, 8 insertions(+), 6 deletions(-)
> 

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

Feel free to keep the R-b if Ray suggests a different approach for the
PCD renaming (e.g., keeping the existing name, or replacing "PcdGetBool
(PcdUse5LevelPageTable)" with constant TRUE altogether)

Laszlo



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



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

* Re: [edk2-devel] [PATCH v2 1/2] MdeModulePkg: rename PcdUse5LevelPageTable to PcdEnable5LevelPageTable
  2024-01-25 12:17     ` Gerd Hoffmann
@ 2024-01-26  2:49       ` Zhiguang Liu
  2024-01-26 13:19         ` Gerd Hoffmann
  0 siblings, 1 reply; 8+ messages in thread
From: Zhiguang Liu @ 2024-01-26  2:49 UTC (permalink / raw)
  To: devel@edk2.groups.io, kraxel@redhat.com, Ni, Ray
  Cc: Liming Gao, László Érsek, Oliver Steffen

Hi Gerd,
For the PCD, it current has below usage.
1) for 32-bit PEI and 64-bit DXE, this PCD will decide if use 5 level paging in DXE.
2) for 64-bit PEI and DXE, reset vector chooses if use 5 level paging in early phase. But BIOS can still switch paging mode based on this PCD later.
Since this PCD can be dynamic, we can let user to choose different paging mode in BIOS setup menu, and use this paging mode on reboot.

You may want to use one BIOS to support machine with different la57 capability, I assume two possible ways:
1) make the PCD as dynamic, and set it based on la57 capability.
2) Remove the ASSERT in DxeIpl

Both are fine for me.

Thanks
Zhiguang

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Gerd
> Hoffmann
> Sent: Thursday, January 25, 2024 8:17 PM
> To: Ni, Ray <ray.ni@intel.com>
> Cc: devel@edk2.groups.io; Liming Gao <gaoliming@byosoft.com.cn>; László
> Érsek <lersek@redhat.com>; Oliver Steffen <osteffen@redhat.com>
> Subject: Re: [edk2-devel] [PATCH v2 1/2] MdeModulePkg: rename
> PcdUse5LevelPageTable to PcdEnable5LevelPageTable
> 
> On Thu, Jan 25, 2024 at 12:01:46PM +0000, Ni, Ray wrote:
> > Rename an existing PCD might break lots of platform builds.
> >
> > When 5-level paging capability was added to ResetVector, I also
> > considered to remove the PcdUse5LevelPageTable reference from C code.
> >
> > Let me think about it...
> 
> Purging this completely from the source code is fine with me too.
> 
> From OVMF point of view this is not needed, all backward compatibility
> concerns for old guests without 5-level paging support can be solved by just
> removing la57 capability from the vCPU (qemu -cpu host,la57=off).
> 
> take care,
>   Gerd
> 
> 
> 
> 
> 



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



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

* Re: [edk2-devel] [PATCH v2 1/2] MdeModulePkg: rename PcdUse5LevelPageTable to PcdEnable5LevelPageTable
  2024-01-26  2:49       ` Zhiguang Liu
@ 2024-01-26 13:19         ` Gerd Hoffmann
  0 siblings, 0 replies; 8+ messages in thread
From: Gerd Hoffmann @ 2024-01-26 13:19 UTC (permalink / raw)
  To: Liu, Zhiguang
  Cc: devel@edk2.groups.io, Ni, Ray, Liming Gao,
	László Érsek, Oliver Steffen

On Fri, Jan 26, 2024 at 02:49:13AM +0000, Liu, Zhiguang wrote:
> Hi Gerd,
> For the PCD, it current has below usage.
> 1) for 32-bit PEI and 64-bit DXE, this PCD will decide if use 5 level paging in DXE.

Yes.  That is the only real use of the PCD today.  I expect the days of
32bit BEI / 64bit DXE firmware builds are numbered though.  There has
been a steady stream of patches from intel to push edk2 to full 64-bit
support, and there also is the x86s proposal[1].

> 2) for 64-bit PEI and DXE, reset vector chooses if use 5 level paging
> in early phase. But BIOS can still switch paging mode based on this
> PCD later.

I don't think edk2 implements the paging mode switch.  It's also not
that easy to do because the paging mode can only be switched with paging
turned off (which implies long mode turned off too).

> Since this PCD can be dynamic, we can let user to choose different
> paging mode in BIOS setup menu, and use this paging mode on reboot.

Is there any (pure 64-bit) firmware actually implementing this today?

> You may want to use one BIOS to support machine with different la57 capability, I assume two possible ways:
> 1) make the PCD as dynamic, and set it based on la57 capability.

You mean la57 state I assume (i.e. cr4.la57)?

i.e. for the 32-PEI / 64-DXE builds the PCD decides which mode will be
used, and for pure 64-bit builds the PCD will be set according to the
mode the CPU is running in?

take care,
  Gerd



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



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

end of thread, other threads:[~2024-01-26 13:19 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-01-25  8:20 [edk2-devel] [PATCH v2 0/2] MdeModulePkg: 5-level paging fixes Gerd Hoffmann
2024-01-25  8:20 ` [edk2-devel] [PATCH v2 1/2] MdeModulePkg: rename PcdUse5LevelPageTable to PcdEnable5LevelPageTable Gerd Hoffmann
2024-01-25 12:01   ` Ni, Ray
2024-01-25 12:17     ` Gerd Hoffmann
2024-01-26  2:49       ` Zhiguang Liu
2024-01-26 13:19         ` Gerd Hoffmann
2024-01-25  8:20 ` [edk2-devel] [PATCH v2 2/2] MdeModulePkg/VirtualMemory: fix 5-level paging assert Gerd Hoffmann
2024-01-25 23:22 ` [edk2-devel] [PATCH v2 0/2] MdeModulePkg: 5-level paging fixes Laszlo Ersek

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