Re: [edk2-devel] [PATCH v2 1/2] MdeModulePkg: rename PcdUse5LevelPageTable to PcdEnable5LevelPageTable

2024-01-26 Thread Gerd Hoffmann
On Fri, Jan 26, 2024 at 02:49:13AM +, 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/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH v2 1/2] MdeModulePkg: rename PcdUse5LevelPageTable to PcdEnable5LevelPageTable

2024-01-25 Thread Zhiguang Liu
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  On Behalf Of Gerd
> Hoffmann
> Sent: Thursday, January 25, 2024 8:17 PM
> To: Ni, Ray 
> Cc: devel@edk2.groups.io; Liming Gao ; László
> Érsek ; Oliver Steffen 
> Subject: Re: [edk2-devel] [PATCH v2 1/2] MdeModulePkg: rename
> PcdUse5LevelPageTable to PcdEnable5LevelPageTable
> 
> On Thu, Jan 25, 2024 at 12:01:46PM +, 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/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH v2 1/2] MdeModulePkg: rename PcdUse5LevelPageTable to PcdEnable5LevelPageTable

2024-01-25 Thread Gerd Hoffmann
On Thu, Jan 25, 2024 at 12:01:46PM +, 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/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH v2 1/2] MdeModulePkg: rename PcdUse5LevelPageTable to PcdEnable5LevelPageTable

2024-01-25 Thread Ni, Ray
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  On Behalf Of Gerd
> Hoffmann
> Sent: Thursday, January 25, 2024 4:21 PM
> To: devel@edk2.groups.io
> Cc: Liming Gao ; László Érsek
> ; Oliver Steffen ; Gerd Hoffmann
> 
> 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 
> ---
>  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.
>#   FALSE - 5-Level Paging will not be enabled.
># @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.
> 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 sup

[edk2-devel] [PATCH v2 1/2] MdeModulePkg: rename PcdUse5LevelPageTable to PcdEnable5LevelPageTable

2024-01-25 Thread 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 
---
 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.
   #   FALSE - 5-Level Paging will not be enabled.
   # @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.
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/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: