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

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

[Gerd Hoffmann]

Patch #1 + #2 fix MdeModulePkg/DxeIplPeim to not assert in case a
5-level enabled build runs in 4-level paging mode.

Patch #3 updates PlatformInitLib for 5-level paging support (update
PhysBits calculation).

v4:
 - drop OvmfPkg/ResetVecor changes, they will be sent as
   separate patch series.
 - add Ard's ack.
v3:
 - add resetvector fixes for sev and tdx
v2 changes:
 - fix sev/tdx handling with 5-level paging.
 - more comments for 5-level page table setup.
 - improve PAGE_* naming (new patch #3).
 - rename Page5LevelSupported to Page5LevelEnabled (new patch #2).
 - pick up some review tags.

Gerd Hoffmann (3):
  MdeModulePkg/DxeIplPeim: fix PcdUse5LevelPageTable assert
  MdeModulePkg/DxeIplPeim: rename variable
  OvmfPkg/PlatformInitLib: add 5-level paging support

 .../Core/DxeIplPeim/X64/VirtualMemory.c       | 24 +++----
 OvmfPkg/Library/PlatformInitLib/MemDetect.c   | 63 +++++++++++++------
 2 files changed, 57 insertions(+), 30 deletions(-)

-- 
2.43.2



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

[edk2-devel] [PATCH v4 1/3] MdeModulePkg/DxeIplPeim: fix PcdUse5LevelPageTable assert

[Gerd Hoffmann]

PcdUse5LevelPageTable documentation says:

  Indicates if 5-Level Paging will be enabled in long mode. 5-Level
  Paging will not be enabled when the PCD is TRUE but CPU doesn't support
  5-Level Paging.

So running in 4-level paging mode with PcdUse5LevelPageTable=TRUE is
possible.  The only invalid combination is 5-level paging being active
with PcdUse5LevelPageTable=FALSE.

Fix the ASSERT accordingly.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Acked-by: Ard Biesheuvel <ardb@kernel.org>
---
 MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
index 980c2002d4f5..1d240e95966e 100644
--- a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
+++ b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
@@ -745,7 +745,9 @@ CreateIdentityMappingPageTables (
     //
     Cr4.UintN         = AsmReadCr4 ();
     Page5LevelSupport = (Cr4.Bits.LA57 != 0);
-    ASSERT (PcdGetBool (PcdUse5LevelPageTable) == Page5LevelSupport);
+    if (Page5LevelSupport) {
+      ASSERT (PcdGetBool (PcdUse5LevelPageTable));
+    }
   } else {
     //
     // If cpu runs in 32bit protected mode PEI, Page table Level in DXE is decided by PCD and feature capability.
-- 
2.43.2



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

[edk2-devel] [PATCH v4 2/3] MdeModulePkg/DxeIplPeim: rename variable

[Gerd Hoffmann]

Rename Page5LevelSupported to Page5LevelEnabled.

The variable is set to true in case 5-paging level is enabled (64-bit
PEI) or will be enabled (32-bit PEI), it does *not* tell whenever the
5-level paging is supported by the CPU.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Acked-by: Ard Biesheuvel <ardb@kernel.org>
---
 .../Core/DxeIplPeim/X64/VirtualMemory.c       | 22 +++++++++----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
index 1d240e95966e..df6196a41cd5 100644
--- a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
+++ b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
@@ -696,7 +696,7 @@ CreateIdentityMappingPageTables (
   UINTN                                        TotalPagesNum;
   UINTN                                        BigPageAddress;
   VOID                                         *Hob;
-  BOOLEAN                                      Page5LevelSupport;
+  BOOLEAN                                      Page5LevelEnabled;
   BOOLEAN                                      Page1GSupport;
   PAGE_TABLE_1G_ENTRY                          *PageDirectory1GEntry;
   UINT64                                       AddressEncMask;
@@ -744,15 +744,15 @@ CreateIdentityMappingPageTables (
     // If cpu has already run in 64bit long mode PEI, Page table Level in DXE must align with previous level.
     //
     Cr4.UintN         = AsmReadCr4 ();
-    Page5LevelSupport = (Cr4.Bits.LA57 != 0);
-    if (Page5LevelSupport) {
+    Page5LevelEnabled = (Cr4.Bits.LA57 != 0);
+    if (Page5LevelEnabled) {
       ASSERT (PcdGetBool (PcdUse5LevelPageTable));
     }
   } else {
     //
     // If cpu runs in 32bit protected mode PEI, Page table Level in DXE is decided by PCD and feature capability.
     //
-    Page5LevelSupport = FALSE;
+    Page5LevelEnabled = FALSE;
     if (PcdGetBool (PcdUse5LevelPageTable)) {
       AsmCpuidEx (
         CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS,
@@ -763,12 +763,12 @@ CreateIdentityMappingPageTables (
         NULL
         );
       if (EcxFlags.Bits.FiveLevelPage != 0) {
-        Page5LevelSupport = TRUE;
+        Page5LevelEnabled = TRUE;
       }
     }
   }
 
-  DEBUG ((DEBUG_INFO, "AddressBits=%u 5LevelPaging=%u 1GPage=%u\n", PhysicalAddressBits, Page5LevelSupport, Page1GSupport));
+  DEBUG ((DEBUG_INFO, "AddressBits=%u 5LevelPaging=%u 1GPage=%u\n", PhysicalAddressBits, Page5LevelEnabled, Page1GSupport));
 
   //
   // IA-32e paging translates 48-bit linear addresses to 52-bit physical addresses
@@ -776,7 +776,7 @@ CreateIdentityMappingPageTables (
   //  due to either unsupported by HW, or disabled by PCD.
   //
   ASSERT (PhysicalAddressBits <= 52);
-  if (!Page5LevelSupport && (PhysicalAddressBits > 48)) {
+  if (!Page5LevelEnabled && (PhysicalAddressBits > 48)) {
     PhysicalAddressBits = 48;
   }
 
@@ -811,7 +811,7 @@ CreateIdentityMappingPageTables (
   //
   // Substract the one page occupied by PML5 entries if 5-Level Paging is disabled.
   //
-  if (!Page5LevelSupport) {
+  if (!Page5LevelEnabled) {
     TotalPagesNum--;
   }
 
@@ -831,7 +831,7 @@ CreateIdentityMappingPageTables (
   // By architecture only one PageMapLevel4 exists - so lets allocate storage for it.
   //
   PageMap = (VOID *)BigPageAddress;
-  if (Page5LevelSupport) {
+  if (Page5LevelEnabled) {
     //
     // By architecture only one PageMapLevel5 exists - so lets allocate storage for it.
     //
@@ -853,7 +853,7 @@ CreateIdentityMappingPageTables (
     PageMapLevel4Entry = (VOID *)BigPageAddress;
     BigPageAddress    += SIZE_4KB;
 
-    if (Page5LevelSupport) {
+    if (Page5LevelEnabled) {
       //
       // Make a PML5 Entry
       //
@@ -947,7 +947,7 @@ CreateIdentityMappingPageTables (
     ZeroMem (PageMapLevel4Entry, (512 - IndexOfPml4Entries) * sizeof (PAGE_MAP_AND_DIRECTORY_POINTER));
   }
 
-  if (Page5LevelSupport) {
+  if (Page5LevelEnabled) {
     Cr4.UintN     = AsmReadCr4 ();
     Cr4.Bits.LA57 = 1;
     AsmWriteCr4 (Cr4.UintN);
-- 
2.43.2



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

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

[Gerd Hoffmann]

Adjust physical address space logic for la57 mode (5-level paging).
With a larger logical address space we can identity-map a larger
physical address space.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Acked-by: Ard Biesheuvel <ardb@kernel.org>
---
 OvmfPkg/Library/PlatformInitLib/MemDetect.c | 63 ++++++++++++++-------
 1 file changed, 44 insertions(+), 19 deletions(-)

diff --git a/OvmfPkg/Library/PlatformInitLib/MemDetect.c b/OvmfPkg/Library/PlatformInitLib/MemDetect.c
index f042517bb64a..7b6e5102ad55 100644
--- a/OvmfPkg/Library/PlatformInitLib/MemDetect.c
+++ b/OvmfPkg/Library/PlatformInitLib/MemDetect.c
@@ -628,11 +628,12 @@ PlatformAddressWidthFromCpuid (
   IN     BOOLEAN                QemuQuirk
   )
 {
-  UINT32   RegEax, RegEbx, RegEcx, RegEdx, Max;
-  UINT8    PhysBits;
-  CHAR8    Signature[13];
-  BOOLEAN  Valid         = FALSE;
-  BOOLEAN  Page1GSupport = FALSE;
+  UINT32    RegEax, RegEbx, RegEcx, RegEdx, Max;
+  UINT8     PhysBits;
+  CHAR8     Signature[13];
+  IA32_CR4  Cr4;
+  BOOLEAN   Valid         = FALSE;
+  BOOLEAN   Page1GSupport = FALSE;
 
   ZeroMem (Signature, sizeof (Signature));
 
@@ -670,30 +671,54 @@ PlatformAddressWidthFromCpuid (
     }
   }
 
+  Cr4.UintN = AsmReadCr4 ();
+
   DEBUG ((
     DEBUG_INFO,
-    "%a: Signature: '%a', PhysBits: %d, QemuQuirk: %a, Valid: %a\n",
+    "%a: Signature: '%a', PhysBits: %d, QemuQuirk: %a, la57: %a, Valid: %a\n",
     __func__,
     Signature,
     PhysBits,
     QemuQuirk ? "On" : "Off",
+    Cr4.Bits.LA57 ? "On" : "Off",
     Valid ? "Yes" : "No"
     ));
 
   if (Valid) {
-    if (PhysBits > 46) {
-      /*
-       * Avoid 5-level paging altogether for now, which limits
-       * PhysBits to 48.  Also avoid using address bit 48, due to sign
-       * extension we can't identity-map these addresses (and lots of
-       * places in edk2 assume we have everything identity-mapped).
-       * So the actual limit is 47.
-       *
-       * Also some older linux kernels apparently have problems handling
-       * phys-bits > 46 correctly, so use that as limit.
-       */
-      DEBUG ((DEBUG_INFO, "%a: limit PhysBits to 46 (avoid 5-level paging)\n", __func__));
-      PhysBits = 46;
+    /*
+     * Due to the sign extension we can use only the lower half of the
+     * virtual address space to identity-map physical address space,
+     * which gives us a 47 bit wide address space with 4 paging levels
+     * and a 56 bit wide address space with 5 paging levels.
+     */
+    if (Cr4.Bits.LA57) {
+      if (PhysBits > 48) {
+        /*
+         * Some Intel CPUs support 5-level paging, have more than 48
+         * phys-bits but support only 4-level EPT, which effectively
+         * limits guest phys-bits to 48.
+         *
+         * AMD Processors have a different but somewhat related
+         * problem: They can handle guest phys-bits larger than 48
+         * only in case the host runs in 5-level paging mode.
+         *
+         * Until we have some way to communicate that kind of
+         * limitations from hypervisor to guest, limit phys-bits
+         * to 48 unconditionally.
+         */
+        DEBUG ((DEBUG_INFO, "%a: limit PhysBits to 48 (5-level paging)\n", __func__));
+        PhysBits = 48;
+      }
+    } else {
+      if (PhysBits > 46) {
+        /*
+         * Some older linux kernels apparently have problems handling
+         * phys-bits > 46 correctly, so use that instead of 47 as
+         * limit.
+         */
+        DEBUG ((DEBUG_INFO, "%a: limit PhysBits to 46 (4-level paging)\n", __func__));
+        PhysBits = 46;
+      }
     }
 
     if (!Page1GSupport && (PhysBits > 40)) {
-- 
2.43.2



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

[edk2-devel] GuestPhysAddrSize questions (was: Re: [PATCH v4 3/3] OvmfPkg/PlatformInitLib: add 5-level paging) support

[Gerd Hoffmann]

  Hi,

> +    if (Cr4.Bits.LA57) {
> +      if (PhysBits > 48) {
> +        /*
> +         * Some Intel CPUs support 5-level paging, have more than 48
> +         * phys-bits but support only 4-level EPT, which effectively
> +         * limits guest phys-bits to 48.
> +         *
> +         * AMD Processors have a different but somewhat related
> +         * problem: They can handle guest phys-bits larger than 48
> +         * only in case the host runs in 5-level paging mode.
> +         *
> +         * Until we have some way to communicate that kind of
> +         * limitations from hypervisor to guest, limit phys-bits
> +         * to 48 unconditionally.
> +         */

So I'm looking for some communication path.  One option would be to use
some bits in the KVM cpuid leaves.  Another possible candidate is cpuid
leaf 0x80000008.

From the AMD APM (revision 3.35):

  CPUID Fn8000_0008_EAX Long Mode Size Identifiers
  ------------------------------------------------

  The value returned in EAX provides information about the maximum host
  and guest physical and linear address width (in bits) supported by the
  processor.

  Bits   FieldName        Description

  31:24  —                Reserved

  23:16 GuestPhysAddrSize Maximum guest physical address size in bits.
                          This number applies only to guests using nested
                          paging. When this field is zero, refer to the
                          PhysAddrSize field for the maximum guest
                          physical address size. See “Secure Virtual
                          Machine” in APM Volume 2.

  15:8  LinAddrSize       Maximum linear address size in bits.

  7:0   PhysAddrSize      Maximum physical address size in bits. When
                          GuestPhysAddrSize is zero, this field also
                          indicates the maximum guest physical address
                          size.

The description of the GuestPhysAddrSize is somewhat vague.  Is this a
value the hypervisor should use to figure how much address space it can
give to guests?  Or is this a value the hypervisor can set to inform the
guest about the available address space (which would be a solution to
the problem outlined in the comment above)?  Or both?

In case GuestPhysAddrSize should not be used this way:  Is it possible
allocate the currently reserved bits 31:24 for that purpose?

Tom?  Michael?

thanks & take care,
  Gerd



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

Re: [edk2-devel] GuestPhysAddrSize questions

[Lendacky, Thomas via groups.io]

On 2/22/24 05:24, Gerd Hoffmann wrote:
>    Hi,
> 
>> +    if (Cr4.Bits.LA57) {
>> +      if (PhysBits > 48) {
>> +        /*
>> +         * Some Intel CPUs support 5-level paging, have more than 48
>> +         * phys-bits but support only 4-level EPT, which effectively
>> +         * limits guest phys-bits to 48.
>> +         *
>> +         * AMD Processors have a different but somewhat related
>> +         * problem: They can handle guest phys-bits larger than 48
>> +         * only in case the host runs in 5-level paging mode.
>> +         *
>> +         * Until we have some way to communicate that kind of
>> +         * limitations from hypervisor to guest, limit phys-bits
>> +         * to 48 unconditionally.
>> +         */
> 
> So I'm looking for some communication path.  One option would be to use
> some bits in the KVM cpuid leaves.  Another possible candidate is cpuid
> leaf 0x80000008.
> 
>  From the AMD APM (revision 3.35):
> 
>    CPUID Fn8000_0008_EAX Long Mode Size Identifiers
>    ------------------------------------------------
> 
>    The value returned in EAX provides information about the maximum host
>    and guest physical and linear address width (in bits) supported by the
>    processor.
> 
>    Bits   FieldName        Description
> 
>    31:24  —                Reserved
> 
>    23:16 GuestPhysAddrSize Maximum guest physical address size in bits.
>                            This number applies only to guests using nested
>                            paging. When this field is zero, refer to the
>                            PhysAddrSize field for the maximum guest
>                            physical address size. See “Secure Virtual
>                            Machine” in APM Volume 2.
> 
>    15:8  LinAddrSize       Maximum linear address size in bits.
> 
>    7:0   PhysAddrSize      Maximum physical address size in bits. When
>                            GuestPhysAddrSize is zero, this field also
>                            indicates the maximum guest physical address
>                            size.
> 
> The description of the GuestPhysAddrSize is somewhat vague.  Is this a
> value the hypervisor should use to figure how much address space it can
> give to guests?  Or is this a value the hypervisor can set to inform the
> guest about the available address space (which would be a solution to
> the problem outlined in the comment above)?  Or both?

I believe the main purpose of GuestPhysAddrSize was for software use (for 
nested virtualization) and that the hardware itself has always returned 
zero for that value. So you should be able to use that field. Adding 
@Paolo for his thoughts.

Thanks,
Tom

> 
> In case GuestPhysAddrSize should not be used this way:  Is it possible
> allocate the currently reserved bits 31:24 for that purpose?
> 
> Tom?  Michael?
> 
> thanks & take care,
>    Gerd
> 


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

Re: [edk2-devel] GuestPhysAddrSize questions

[Paolo Bonzini]

On 2/22/24 16:44, Tom Lendacky wrote:
> On 2/22/24 05:24, Gerd Hoffmann wrote:
>>    Hi,
>>
>>> +    if (Cr4.Bits.LA57) {
>>> +      if (PhysBits > 48) {
>>> +        /*
>>> +         * Some Intel CPUs support 5-level paging, have more than 48
>>> +         * phys-bits but support only 4-level EPT, which effectively
>>> +         * limits guest phys-bits to 48.
>>> +         *
>>> +         * AMD Processors have a different but somewhat related
>>> +         * problem: They can handle guest phys-bits larger than 48
>>> +         * only in case the host runs in 5-level paging mode.
>>> +         *
>>> +         * Until we have some way to communicate that kind of
>>> +         * limitations from hypervisor to guest, limit phys-bits
>>> +         * to 48 unconditionally.
>>> +         */
>>
>> So I'm looking for some communication path.  One option would be to use
>> some bits in the KVM cpuid leaves.  Another possible candidate is cpuid
>> leaf 0x80000008.
>>
>>  From the AMD APM (revision 3.35):
>>
>>    CPUID Fn8000_0008_EAX Long Mode Size Identifiers
>>    ------------------------------------------------
>>
>>    The value returned in EAX provides information about the maximum host
>>    and guest physical and linear address width (in bits) supported by the
>>    processor.
>>
>>    Bits   FieldName        Description
>>
>>    31:24  —                Reserved
>>
>>    23:16 GuestPhysAddrSize Maximum guest physical address size in bits.
>>                            This number applies only to guests using 
>> nested
>>                            paging. When this field is zero, refer to the
>>                            PhysAddrSize field for the maximum guest
>>                            physical address size. See “Secure Virtual
>>                            Machine” in APM Volume 2.
>>
>>    15:8  LinAddrSize       Maximum linear address size in bits.
>>
>>    7:0   PhysAddrSize      Maximum physical address size in bits. When
>>                            GuestPhysAddrSize is zero, this field also
>>                            indicates the maximum guest physical address
>>                            size.
>>
>> The description of the GuestPhysAddrSize is somewhat vague.  Is this a
>> value the hypervisor should use to figure how much address space it can
>> give to guests?  Or is this a value the hypervisor can set to inform the
>> guest about the available address space (which would be a solution to
>> the problem outlined in the comment above)?  Or both?
> 
> I believe the main purpose of GuestPhysAddrSize was for software use 
> (for nested virtualization) and that the hardware itself has always 
> returned zero for that value. So you should be able to use that field. 
> Adding @Paolo for his thoughts.

I have already discussed this with Gerd, so I don't really have many 
thoughts to add.

Anyhow, basically we would like GuestPhysAddrSize to be "redefined" as

    Maximum usable physical address size in bits.  Physical addresses
    above this size should not be used, but will not produce a "reserved"
    page fault.  When this field is zero, all bits up to PhysAddrSize are
    usable.  This field is expected to be nonzero only on guests where
    the hypervisor is using nested paging.

Also, to clarify the hardware behavior, if hCR4.LA57=0 and host 
PhysAddrSize==52, then will guest physical addresses above 2^48

1) cause a reserved #PF in the guest, or

2) cause a non-present NPF exit in the hypervisor?

I remember that several years ago we had a discussion on hCR4.LA57=0 
reducing the address space compared to MAXPHYADDR, but I cannot find the 
emails and also at the time I didn't notice GuestPhysAddrSize.

Paolo



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

Re: [edk2-devel] GuestPhysAddrSize questions

[Paolo Bonzini]

On Thu, Feb 22, 2024 at 5:13 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
> Also, to clarify the hardware behavior, if hCR4.LA57=0 and host
> PhysAddrSize==52, then will guest physical addresses above 2^48
>
> 1) cause a reserved #PF in the guest, or
>
> 2) cause a non-present NPF exit in the hypervisor?
>
> I remember that several years ago we had a discussion on hCR4.LA57=0
> reducing the address space compared to MAXPHYADDR, but I cannot find the
> emails and also at the time I didn't notice GuestPhysAddrSize.

Found them! They say that "according to the design document, CPU will
report #NPF if the guest references a PA that is greater than 48 bits
while the hypervisor is in 4-level nested paging mode". That's nice,
because it's the same behavior as the affected Intel processors.

Paolo

> Anyhow, basically we would like GuestPhysAddrSize to be "redefined" as
>
>     Maximum usable physical address size in bits.  Physical addresses
>     above this size should not be used, but will not produce a "reserved"
>     page fault.  When this field is zero, all bits up to PhysAddrSize are
>     usable.  This field is expected to be nonzero only on guests where
>     the hypervisor is using nested paging.



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

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

[Laszlo Ersek]

Hi Liming,

can you please check the first two patches in the series? They should be
easy to review.

Thanks!
Laszlo

On 2/22/24 11:54, Gerd Hoffmann wrote:
> Patch #1 + #2 fix MdeModulePkg/DxeIplPeim to not assert in case a
> 5-level enabled build runs in 4-level paging mode.
> 
> Patch #3 updates PlatformInitLib for 5-level paging support (update
> PhysBits calculation).
> 
> v4:
>  - drop OvmfPkg/ResetVecor changes, they will be sent as
>    separate patch series.
>  - add Ard's ack.
> v3:
>  - add resetvector fixes for sev and tdx
> v2 changes:
>  - fix sev/tdx handling with 5-level paging.
>  - more comments for 5-level page table setup.
>  - improve PAGE_* naming (new patch #3).
>  - rename Page5LevelSupported to Page5LevelEnabled (new patch #2).
>  - pick up some review tags.
> 
> Gerd Hoffmann (3):
>   MdeModulePkg/DxeIplPeim: fix PcdUse5LevelPageTable assert
>   MdeModulePkg/DxeIplPeim: rename variable
>   OvmfPkg/PlatformInitLib: add 5-level paging support
> 
>  .../Core/DxeIplPeim/X64/VirtualMemory.c       | 24 +++----
>  OvmfPkg/Library/PlatformInitLib/MemDetect.c   | 63 +++++++++++++------
>  2 files changed, 57 insertions(+), 30 deletions(-)
> 



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

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

[Laszlo Ersek]

On 2/22/24 11:54, Gerd Hoffmann wrote:
> Patch #1 + #2 fix MdeModulePkg/DxeIplPeim to not assert in case a
> 5-level enabled build runs in 4-level paging mode.
> 
> Patch #3 updates PlatformInitLib for 5-level paging support (update
> PhysBits calculation).
> 
> v4:
>  - drop OvmfPkg/ResetVecor changes, they will be sent as
>    separate patch series.
>  - add Ard's ack.
> v3:
>  - add resetvector fixes for sev and tdx
> v2 changes:
>  - fix sev/tdx handling with 5-level paging.
>  - more comments for 5-level page table setup.
>  - improve PAGE_* naming (new patch #3).
>  - rename Page5LevelSupported to Page5LevelEnabled (new patch #2).
>  - pick up some review tags.
> 
> Gerd Hoffmann (3):
>   MdeModulePkg/DxeIplPeim: fix PcdUse5LevelPageTable assert
>   MdeModulePkg/DxeIplPeim: rename variable
>   OvmfPkg/PlatformInitLib: add 5-level paging support
> 
>  .../Core/DxeIplPeim/X64/VirtualMemory.c       | 24 +++----
>  OvmfPkg/Library/PlatformInitLib/MemDetect.c   | 63 +++++++++++++------
>  2 files changed, 57 insertions(+), 30 deletions(-)
> 

Series merged as commit range d9a6e7b0b8a6..adebfe121ca0, via
<https://github.com/tianocore/edk2/pull/5424>.



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

[edk2-devel] 回复: [PATCH v4 1/3] MdeModulePkg/DxeIplPeim: fix PcdUse5LevelPageTable assert

[gaoliming via groups.io]

Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>

> -----邮件原件-----
> 发件人: Gerd Hoffmann <kraxel@redhat.com>
> 发送时间: 2024年2月22日 18:54
> 收件人: devel@edk2.groups.io
> 抄送: Michael Roth <michael.roth@amd.com>; Jiewen Yao
> <jiewen.yao@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Laszlo
> Ersek <lersek@redhat.com>; Tom Lendacky <thomas.lendacky@amd.com>;
> Paolo Bonzini <pbonzini@redhat.com>; Ard Biesheuvel
> <ardb+tianocore@kernel.org>; Gerd Hoffmann <kraxel@redhat.com>; Min Xu
> <min.m.xu@intel.com>; Erdem Aktas <erdemaktas@google.com>; Oliver
> Steffen <osteffen@redhat.com>; Ard Biesheuvel <ardb@kernel.org>
> 主题: [PATCH v4 1/3] MdeModulePkg/DxeIplPeim: fix
> PcdUse5LevelPageTable assert
> 
> PcdUse5LevelPageTable documentation says:
> 
>   Indicates if 5-Level Paging will be enabled in long mode. 5-Level
>   Paging will not be enabled when the PCD is TRUE but CPU doesn't support
>   5-Level Paging.
> 
> So running in 4-level paging mode with PcdUse5LevelPageTable=TRUE is
> possible.  The only invalid combination is 5-level paging being active
> with PcdUse5LevelPageTable=FALSE.
> 
> Fix the ASSERT accordingly.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> Acked-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>  MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
> b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
> index 980c2002d4f5..1d240e95966e 100644
> --- a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
> +++ b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
> @@ -745,7 +745,9 @@ CreateIdentityMappingPageTables (
>      //
>      Cr4.UintN         = AsmReadCr4 ();
>      Page5LevelSupport = (Cr4.Bits.LA57 != 0);
> -    ASSERT (PcdGetBool (PcdUse5LevelPageTable) == Page5LevelSupport);
> +    if (Page5LevelSupport) {
> +      ASSERT (PcdGetBool (PcdUse5LevelPageTable));
> +    }
>    } else {
>      //
>      // If cpu runs in 32bit protected mode PEI, Page table Level in DXE
is
> decided by PCD and feature capability.
> --
> 2.43.2





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

[edk2-devel] 回复: [PATCH v4 2/3] MdeModulePkg/DxeIplPeim: rename variable

[gaoliming via groups.io]

Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>

> -----邮件原件-----
> 发件人: Gerd Hoffmann <kraxel@redhat.com>
> 发送时间: 2024年2月22日 18:54
> 收件人: devel@edk2.groups.io
> 抄送: Michael Roth <michael.roth@amd.com>; Jiewen Yao
> <jiewen.yao@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Laszlo
> Ersek <lersek@redhat.com>; Tom Lendacky <thomas.lendacky@amd.com>;
> Paolo Bonzini <pbonzini@redhat.com>; Ard Biesheuvel
> <ardb+tianocore@kernel.org>; Gerd Hoffmann <kraxel@redhat.com>; Min Xu
> <min.m.xu@intel.com>; Erdem Aktas <erdemaktas@google.com>; Oliver
> Steffen <osteffen@redhat.com>; Ard Biesheuvel <ardb@kernel.org>
> 主题: [PATCH v4 2/3] MdeModulePkg/DxeIplPeim: rename variable
> 
> Rename Page5LevelSupported to Page5LevelEnabled.
> 
> The variable is set to true in case 5-paging level is enabled (64-bit
> PEI) or will be enabled (32-bit PEI), it does *not* tell whenever the
> 5-level paging is supported by the CPU.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> Acked-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>  .../Core/DxeIplPeim/X64/VirtualMemory.c       | 22 +++++++++----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
> b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
> index 1d240e95966e..df6196a41cd5 100644
> --- a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
> +++ b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
> @@ -696,7 +696,7 @@ CreateIdentityMappingPageTables (
>    UINTN                                        TotalPagesNum;
>    UINTN                                        BigPageAddress;
>    VOID                                         *Hob;
> -  BOOLEAN
> Page5LevelSupport;
> +  BOOLEAN
> Page5LevelEnabled;
>    BOOLEAN                                      Page1GSupport;
>    PAGE_TABLE_1G_ENTRY
> *PageDirectory1GEntry;
>    UINT64                                       AddressEncMask;
> @@ -744,15 +744,15 @@ CreateIdentityMappingPageTables (
>      // If cpu has already run in 64bit long mode PEI, Page table Level in
DXE
> must align with previous level.
>      //
>      Cr4.UintN         = AsmReadCr4 ();
> -    Page5LevelSupport = (Cr4.Bits.LA57 != 0);
> -    if (Page5LevelSupport) {
> +    Page5LevelEnabled = (Cr4.Bits.LA57 != 0);
> +    if (Page5LevelEnabled) {
>        ASSERT (PcdGetBool (PcdUse5LevelPageTable));
>      }
>    } else {
>      //
>      // If cpu runs in 32bit protected mode PEI, Page table Level in DXE
is
> decided by PCD and feature capability.
>      //
> -    Page5LevelSupport = FALSE;
> +    Page5LevelEnabled = FALSE;
>      if (PcdGetBool (PcdUse5LevelPageTable)) {
>        AsmCpuidEx (
>          CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS,
> @@ -763,12 +763,12 @@ CreateIdentityMappingPageTables (
>          NULL
>          );
>        if (EcxFlags.Bits.FiveLevelPage != 0) {
> -        Page5LevelSupport = TRUE;
> +        Page5LevelEnabled = TRUE;
>        }
>      }
>    }
> 
> -  DEBUG ((DEBUG_INFO, "AddressBits=%u 5LevelPaging=%u 1GPage=%u\n",
> PhysicalAddressBits, Page5LevelSupport, Page1GSupport));
> +  DEBUG ((DEBUG_INFO, "AddressBits=%u 5LevelPaging=%u
> 1GPage=%u\n", PhysicalAddressBits, Page5LevelEnabled, Page1GSupport));
> 
>    //
>    // IA-32e paging translates 48-bit linear addresses to 52-bit physical
> addresses
> @@ -776,7 +776,7 @@ CreateIdentityMappingPageTables (
>    //  due to either unsupported by HW, or disabled by PCD.
>    //
>    ASSERT (PhysicalAddressBits <= 52);
> -  if (!Page5LevelSupport && (PhysicalAddressBits > 48)) {
> +  if (!Page5LevelEnabled && (PhysicalAddressBits > 48)) {
>      PhysicalAddressBits = 48;
>    }
> 
> @@ -811,7 +811,7 @@ CreateIdentityMappingPageTables (
>    //
>    // Substract the one page occupied by PML5 entries if 5-Level Paging is
> disabled.
>    //
> -  if (!Page5LevelSupport) {
> +  if (!Page5LevelEnabled) {
>      TotalPagesNum--;
>    }
> 
> @@ -831,7 +831,7 @@ CreateIdentityMappingPageTables (
>    // By architecture only one PageMapLevel4 exists - so lets allocate
> storage for it.
>    //
>    PageMap = (VOID *)BigPageAddress;
> -  if (Page5LevelSupport) {
> +  if (Page5LevelEnabled) {
>      //
>      // By architecture only one PageMapLevel5 exists - so lets allocate
> storage for it.
>      //
> @@ -853,7 +853,7 @@ CreateIdentityMappingPageTables (
>      PageMapLevel4Entry = (VOID *)BigPageAddress;
>      BigPageAddress    += SIZE_4KB;
> 
> -    if (Page5LevelSupport) {
> +    if (Page5LevelEnabled) {
>        //
>        // Make a PML5 Entry
>        //
> @@ -947,7 +947,7 @@ CreateIdentityMappingPageTables (
>      ZeroMem (PageMapLevel4Entry, (512 - IndexOfPml4Entries) * sizeof
> (PAGE_MAP_AND_DIRECTORY_POINTER));
>    }
> 
> -  if (Page5LevelSupport) {
> +  if (Page5LevelEnabled) {
>      Cr4.UintN     = AsmReadCr4 ();
>      Cr4.Bits.LA57 = 1;
>      AsmWriteCr4 (Cr4.UintN);
> --
> 2.43.2





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

Re: [edk2-devel] GuestPhysAddrSize questions

[Gerd Hoffmann]

  Hi,

> >    23:16 GuestPhysAddrSize Maximum guest physical address size in bits.
> >                            This number applies only to guests using nested
> >                            paging. When this field is zero, refer to the
> >                            PhysAddrSize field for the maximum guest
> >                            physical address size. See “Secure Virtual
> >                            Machine” in APM Volume 2.

> I believe the main purpose of GuestPhysAddrSize was for software use (for
> nested virtualization) and that the hardware itself has always returned zero
> for that value. So you should be able to use that field. Adding @Paolo for
> his thoughts.

Posted patches for kernel
https://lore.kernel.org/kvm/20240301101410.356007-1-kraxel@redhat.com/
and qemu
https://lore.kernel.org/kvm/20240301101713.356759-1-kraxel@redhat.com/
(sorry forgot to Cc you).

Reviewers mentioned this is meant for nested guests, i.e. (if I
understand this correctly) the l0 hypervisor can use that to tell
the l1 hypervisor what the l2 guest phys-bits should be.

Is this nested virtualization use documented somewhere?  Tried to
search for GuestPhysAddrSize or Fn8000_0008_EAX in APM Volume 2,
found nothing.

Is there any case where the phys-bits limits for an l1 guest and
l2 guest would be different?

thanks & take care,
  Gerd



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

Re: [edk2-devel] GuestPhysAddrSize questions

[Lendacky, Thomas via groups.io]

On 3/4/24 07:09, Gerd Hoffmann wrote:
>    Hi,
> 
>>>     23:16 GuestPhysAddrSize Maximum guest physical address size in bits.
>>>                             This number applies only to guests using nested
>>>                             paging. When this field is zero, refer to the
>>>                             PhysAddrSize field for the maximum guest
>>>                             physical address size. See “Secure Virtual
>>>                             Machine” in APM Volume 2.
> 
>> I believe the main purpose of GuestPhysAddrSize was for software use (for
>> nested virtualization) and that the hardware itself has always returned zero
>> for that value. So you should be able to use that field. Adding @Paolo for
>> his thoughts.
> 
> Posted patches for kernel
> https://lore.kernel.org/kvm/20240301101410.356007-1-kraxel@redhat.com/
> and qemu
> https://lore.kernel.org/kvm/20240301101713.356759-1-kraxel@redhat.com/
> (sorry forgot to Cc you).
> 
> Reviewers mentioned this is meant for nested guests, i.e. (if I
> understand this correctly) the l0 hypervisor can use that to tell
> the l1 hypervisor what the l2 guest phys-bits should be.
> 
> Is this nested virtualization use documented somewhere?  Tried to
> search for GuestPhysAddrSize or Fn8000_0008_EAX in APM Volume 2,
> found nothing.

Right, and I don't think you'll see anything added to the APM that will 
state how it can be used by software. The APM is an architectural 
definition and won't talk about hypervisors and using nested paging, etc.

> 
> Is there any case where the phys-bits limits for an l1 guest and
> l2 guest would be different?

I haven't really thought about this before and all the implications that 
may or may not be in play, so I don't think I can really answer that.

Thanks,
Tom

> 
> thanks & take care,
>    Gerd
> 


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

Re: [edk2-devel] GuestPhysAddrSize questions

[Paolo Bonzini]

On Mon, Mar 4, 2024 at 6:24 PM Tom Lendacky <thomas.lendacky@amd.com> wrote:
>
> On 3/4/24 07:09, Gerd Hoffmann wrote:
> >    Hi,
> >
> >>>     23:16 GuestPhysAddrSize Maximum guest physical address size in bits.
> >>>                             This number applies only to guests using nested
> >>>                             paging. When this field is zero, refer to the
> >>>                             PhysAddrSize field for the maximum guest
> >>>                             physical address size. See “Secure Virtual
> >>>                             Machine” in APM Volume 2.
> >
> >> I believe the main purpose of GuestPhysAddrSize was for software use (for
> >> nested virtualization) and that the hardware itself has always returned zero
> >> for that value. So you should be able to use that field. Adding @Paolo for
> >> his thoughts.
> >
> > Reviewers mentioned this is meant for nested guests, i.e. (if I
> > understand this correctly) the l0 hypervisor can use that to tell
> > the l1 hypervisor what the l2 guest phys-bits should be.
> >
> > Is this nested virtualization use documented somewhere?  Tried to
> > search for GuestPhysAddrSize or Fn8000_0008_EAX in APM Volume 2,
> > found nothing.
>
> Right, and I don't think you'll see anything added to the APM that will
> state how it can be used by software. The APM is an architectural
> definition and won't talk about hypervisors and using nested paging, etc.

I don't think that's a problem. The problem is that the definition in
the APM is ambiguous and can mean one of three things:

1) it can be a suggested value for PhysAddrSize (bits 7:0) of guests
that use nested paging. This would imply that in a nested page guest,
with GuestPhysAddrSize=48, setting bits 51:48 would cause a
#PF(reserved) exception.

2) it can be equivalent to LinAddrSize (bits 15:8) but for nested page
tables. This would imply that, with GuestPhysAddrSize=48, VMRUN would
fail if hCR4.LA57=1.

3) it can be what I propose above: the architecture defined a
situation that can only happen when using nested paging (on AMD:
host_CR4.LA57=0, PhysAddrSize=52), and GuestPhysAddrSize is an
architectural way to explain the situation to guests.

The message above suggests that the intended meaning is (1). That is
because "the l0 hypervisor can use that to tell the l1 hypervisor what
the l2 guest phys-bits should be" is exactly the same as "the
processor can use that to tell the hypervisor what the guest phys-bits
should be" (just shifted one level down).

However, there are no processors that implement (1) or (2), so my
suggestion is to clarify that the intended meaning is (3). Do you
agree that the above proposal is a plausible interpretation of what is
already in the APM, but clearer? And do you think there is a way for
the clarification to make it into the APM?

Paolo



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