1. Patchew
  2. EDK2
  3. [edk2-devel] [PATCH for-edk2-stable201905 0/6] work around a QEMU issue triggered by the original TianoCore#1814 fix
  4. View patch

[edk2-devel] [PATCH for-edk2-stable201905 6/6] OvmfPkg/PlatformPei: set 32-bit UC area at PciBase / PciExBarBase (pc/q35)

Laszlo Ersek posted 6 patches 5 years, 6 months ago
[edk2-devel] [PATCH for-edk2-stable201905 6/6] OvmfPkg/PlatformPei: set 32-bit UC area at PciBase / PciExBarBase (pc/q35)
Posted by Laszlo Ersek 5 years, 6 months ago 
(This is a replacement for commit 39b9a5ffe661 ("OvmfPkg/PlatformPei: fix
MTRR for low-RAM sizes that have many bits clear", 2019-05-16).)

Reintroduce the same logic as seen in commit 39b9a5ffe661 for the pc
(i440fx) board type.

For q35, the same approach doesn't work any longer, given that (a) we'd
like to keep the PCIEXBAR in the platform DSC a fixed-at-build PCD, and
(b) QEMU expects the PCIEXBAR to reside at a lower address than the 32-bit
PCI MMIO aperture.

Therefore, introduce a helper function for determining the 32-bit
"uncacheable" (MMIO) area base address:

- On q35, this function behaves statically. Furthermore, the MTRR setup
  exploits that the range [0xB000_0000, 0xFFFF_FFFF] can be marked UC with
  just two variable MTRRs (one at 0xB000_0000 (size 256MB), another at
  0xC000_0000 (size 1GB)).

- On pc (i440fx), the function behaves dynamically, implementing the same
  logic as commit 39b9a5ffe661 did. The PciBase value is adjusted to the
  value calculated, similarly to commit 39b9a5ffe661. A further
  simplification is that we show that the UC32 area size truncation to a
  whole power of two automatically guarantees a >=2GB base address.

Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1859
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 OvmfPkg/PlatformPei/Platform.h  |  7 +++
 OvmfPkg/PlatformPei/MemDetect.c | 59 ++++++++++++++++++--
 OvmfPkg/PlatformPei/Platform.c  |  5 +-
 3 files changed, 66 insertions(+), 5 deletions(-)

diff --git a/OvmfPkg/PlatformPei/Platform.h b/OvmfPkg/PlatformPei/Platform.h
index 81af8b71480f..2f3cebcd3a6a 100644
--- a/OvmfPkg/PlatformPei/Platform.h
+++ b/OvmfPkg/PlatformPei/Platform.h
@@ -62,6 +62,11 @@ GetSystemMemorySizeBelow4gb (
   VOID
   );
 
+VOID
+QemuUc32BaseInitialization (
+  VOID
+  );
+
 VOID
 InitializeRamRegions (
   VOID
@@ -114,4 +119,6 @@ extern UINT32 mMaxCpuCount;
 
 extern UINT16 mHostBridgeDevId;
 
+extern UINT32 mQemuUc32Base;
+
 #endif // _PLATFORM_PEI_H_INCLUDED_
diff --git a/OvmfPkg/PlatformPei/MemDetect.c b/OvmfPkg/PlatformPei/MemDetect.c
index e890e36408a6..d451989f31c9 100644
--- a/OvmfPkg/PlatformPei/MemDetect.c
+++ b/OvmfPkg/PlatformPei/MemDetect.c
@@ -14,6 +14,7 @@ Module Name:
 // The package level header files this module uses
 //
 #include <IndustryStandard/E820.h>
+#include <IndustryStandard/I440FxPiix4.h>
 #include <IndustryStandard/Q35MchIch9.h>
 #include <PiPei.h>
 
@@ -42,6 +43,8 @@ STATIC UINT32 mS3AcpiReservedMemorySize;
 
 STATIC UINT16 mQ35TsegMbytes;
 
+UINT32 mQemuUc32Base;
+
 VOID
 Q35TsegMbytesInitialization (
   VOID
@@ -98,6 +101,54 @@ Q35TsegMbytesInitialization (
 }
 
 
+VOID
+QemuUc32BaseInitialization (
+  VOID
+  )
+{
+  UINT32 LowerMemorySize;
+  UINT32 Uc32Size;
+
+  if (mXen) {
+    return;
+  }
+
+  if (mHostBridgeDevId == INTEL_Q35_MCH_DEVICE_ID) {
+    //
+    // On q35, the 32-bit area that we'll mark as UC, through variable MTRRs,
+    // starts at PcdPciExpressBaseAddress. The platform DSC is responsible for
+    // setting PcdPciExpressBaseAddress such that describing the
+    // [PcdPciExpressBaseAddress, 4GB) range require a very small number of
+    // variable MTRRs (preferably 1 or 2).
+    //
+    ASSERT (FixedPcdGet64 (PcdPciExpressBaseAddress) <= MAX_UINT32);
+    mQemuUc32Base = (UINT32)FixedPcdGet64 (PcdPciExpressBaseAddress);
+    return;
+  }
+
+  ASSERT (mHostBridgeDevId == INTEL_82441_DEVICE_ID);
+  //
+  // On i440fx, start with the [LowerMemorySize, 4GB) range. Make sure one
+  // variable MTRR suffices by truncating the size to a whole power of two,
+  // while keeping the end affixed to 4GB. This will round the base up.
+  //
+  LowerMemorySize = GetSystemMemorySizeBelow4gb ();
+  Uc32Size = GetPowerOfTwo32 ((UINT32)(SIZE_4GB - LowerMemorySize));
+  mQemuUc32Base = (UINT32)(SIZE_4GB - Uc32Size);
+  //
+  // Assuming that LowerMemorySize is at least 1 byte, Uc32Size is at most 2GB.
+  // Therefore mQemuUc32Base is at least 2GB.
+  //
+  ASSERT (mQemuUc32Base >= BASE_2GB);
+
+  if (mQemuUc32Base != LowerMemorySize) {
+    DEBUG ((DEBUG_VERBOSE, "%a: rounded UC32 base from 0x%x up to 0x%x, for "
+      "an UC32 size of 0x%x\n", __FUNCTION__, LowerMemorySize, mQemuUc32Base,
+      Uc32Size));
+  }
+}
+
+
 /**
   Iterate over the RAM entries in QEMU's fw_cfg E820 RAM map that start outside
   of the 32-bit address range.
@@ -688,11 +739,11 @@ QemuInitializeRam (
     ASSERT_EFI_ERROR (Status);
 
     //
-    // Set memory range from the "top of lower RAM" (RAM below 4GB) to 4GB as
-    // uncacheable
+    // Set the memory range from the start of the 32-bit MMIO area (32-bit PCI
+    // MMIO aperture on i440fx, PCIEXBAR on q35) to 4GB as uncacheable.
     //
-    Status = MtrrSetMemoryAttribute (LowerMemorySize,
-               SIZE_4GB - LowerMemorySize, CacheUncacheable);
+    Status = MtrrSetMemoryAttribute (mQemuUc32Base, SIZE_4GB - mQemuUc32Base,
+               CacheUncacheable);
     ASSERT_EFI_ERROR (Status);
   }
 }
diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c
index 0876316eefbc..3ba2459872d9 100644
--- a/OvmfPkg/PlatformPei/Platform.c
+++ b/OvmfPkg/PlatformPei/Platform.c
@@ -191,7 +191,8 @@ MemMapInitialization (
       ASSERT (PciExBarBase <= MAX_UINT32 - SIZE_256MB);
       PciBase = (UINT32)(PciExBarBase + SIZE_256MB);
     } else {
-      PciBase = (TopOfLowRam < BASE_2GB) ? BASE_2GB : TopOfLowRam;
+      ASSERT (TopOfLowRam <= mQemuUc32Base);
+      PciBase = mQemuUc32Base;
     }
 
     //
@@ -650,6 +651,8 @@ InitializePlatform (
 
   PublishPeiMemory ();
 
+  QemuUc32BaseInitialization ();
+
   InitializeRamRegions ();
 
   if (mXen) {
-- 
2.19.1.3.g30247aa5d201


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#41630): https://edk2.groups.io/g/devel/message/41630
Mute This Topic: https://groups.io/mt/31834722/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH for-edk2-stable201905 6/6] OvmfPkg/PlatformPei: set 32-bit UC area at PciBase / PciExBarBase (pc/q35)
Posted by Philippe Mathieu-Daudé 5 years, 5 months ago 
On 5/29/19 5:12 PM, Laszlo Ersek wrote:
> (This is a replacement for commit 39b9a5ffe661 ("OvmfPkg/PlatformPei: fix
> MTRR for low-RAM sizes that have many bits clear", 2019-05-16).)
> 
> Reintroduce the same logic as seen in commit 39b9a5ffe661 for the pc
> (i440fx) board type.
> 
> For q35, the same approach doesn't work any longer, given that (a) we'd
> like to keep the PCIEXBAR in the platform DSC a fixed-at-build PCD, and
> (b) QEMU expects the PCIEXBAR to reside at a lower address than the 32-bit
> PCI MMIO aperture.
> 
> Therefore, introduce a helper function for determining the 32-bit
> "uncacheable" (MMIO) area base address:
> 
> - On q35, this function behaves statically. Furthermore, the MTRR setup
>   exploits that the range [0xB000_0000, 0xFFFF_FFFF] can be marked UC with
>   just two variable MTRRs (one at 0xB000_0000 (size 256MB), another at
>   0xC000_0000 (size 1GB)).
> 
> - On pc (i440fx), the function behaves dynamically, implementing the same
>   logic as commit 39b9a5ffe661 did. The PciBase value is adjusted to the
>   value calculated, similarly to commit 39b9a5ffe661. A further
>   simplification is that we show that the UC32 area size truncation to a
>   whole power of two automatically guarantees a >=2GB base address.
> 
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1859
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>

Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>

> ---
>  OvmfPkg/PlatformPei/Platform.h  |  7 +++
>  OvmfPkg/PlatformPei/MemDetect.c | 59 ++++++++++++++++++--
>  OvmfPkg/PlatformPei/Platform.c  |  5 +-
>  3 files changed, 66 insertions(+), 5 deletions(-)
> 
> diff --git a/OvmfPkg/PlatformPei/Platform.h b/OvmfPkg/PlatformPei/Platform.h
> index 81af8b71480f..2f3cebcd3a6a 100644
> --- a/OvmfPkg/PlatformPei/Platform.h
> +++ b/OvmfPkg/PlatformPei/Platform.h
> @@ -62,6 +62,11 @@ GetSystemMemorySizeBelow4gb (
>    VOID
>    );
>  
> +VOID
> +QemuUc32BaseInitialization (
> +  VOID
> +  );
> +
>  VOID
>  InitializeRamRegions (
>    VOID
> @@ -114,4 +119,6 @@ extern UINT32 mMaxCpuCount;
>  
>  extern UINT16 mHostBridgeDevId;
>  
> +extern UINT32 mQemuUc32Base;
> +
>  #endif // _PLATFORM_PEI_H_INCLUDED_
> diff --git a/OvmfPkg/PlatformPei/MemDetect.c b/OvmfPkg/PlatformPei/MemDetect.c
> index e890e36408a6..d451989f31c9 100644
> --- a/OvmfPkg/PlatformPei/MemDetect.c
> +++ b/OvmfPkg/PlatformPei/MemDetect.c
> @@ -14,6 +14,7 @@ Module Name:
>  // The package level header files this module uses
>  //
>  #include <IndustryStandard/E820.h>
> +#include <IndustryStandard/I440FxPiix4.h>
>  #include <IndustryStandard/Q35MchIch9.h>
>  #include <PiPei.h>
>  
> @@ -42,6 +43,8 @@ STATIC UINT32 mS3AcpiReservedMemorySize;
>  
>  STATIC UINT16 mQ35TsegMbytes;
>  
> +UINT32 mQemuUc32Base;
> +
>  VOID
>  Q35TsegMbytesInitialization (
>    VOID
> @@ -98,6 +101,54 @@ Q35TsegMbytesInitialization (
>  }
>  
>  
> +VOID
> +QemuUc32BaseInitialization (
> +  VOID
> +  )
> +{
> +  UINT32 LowerMemorySize;
> +  UINT32 Uc32Size;
> +
> +  if (mXen) {
> +    return;
> +  }
> +
> +  if (mHostBridgeDevId == INTEL_Q35_MCH_DEVICE_ID) {
> +    //
> +    // On q35, the 32-bit area that we'll mark as UC, through variable MTRRs,
> +    // starts at PcdPciExpressBaseAddress. The platform DSC is responsible for
> +    // setting PcdPciExpressBaseAddress such that describing the
> +    // [PcdPciExpressBaseAddress, 4GB) range require a very small number of
> +    // variable MTRRs (preferably 1 or 2).
> +    //
> +    ASSERT (FixedPcdGet64 (PcdPciExpressBaseAddress) <= MAX_UINT32);
> +    mQemuUc32Base = (UINT32)FixedPcdGet64 (PcdPciExpressBaseAddress);
> +    return;
> +  }
> +
> +  ASSERT (mHostBridgeDevId == INTEL_82441_DEVICE_ID);
> +  //
> +  // On i440fx, start with the [LowerMemorySize, 4GB) range. Make sure one
> +  // variable MTRR suffices by truncating the size to a whole power of two,
> +  // while keeping the end affixed to 4GB. This will round the base up.
> +  //
> +  LowerMemorySize = GetSystemMemorySizeBelow4gb ();
> +  Uc32Size = GetPowerOfTwo32 ((UINT32)(SIZE_4GB - LowerMemorySize));
> +  mQemuUc32Base = (UINT32)(SIZE_4GB - Uc32Size);
> +  //
> +  // Assuming that LowerMemorySize is at least 1 byte, Uc32Size is at most 2GB.
> +  // Therefore mQemuUc32Base is at least 2GB.
> +  //
> +  ASSERT (mQemuUc32Base >= BASE_2GB);
> +
> +  if (mQemuUc32Base != LowerMemorySize) {
> +    DEBUG ((DEBUG_VERBOSE, "%a: rounded UC32 base from 0x%x up to 0x%x, for "
> +      "an UC32 size of 0x%x\n", __FUNCTION__, LowerMemorySize, mQemuUc32Base,
> +      Uc32Size));
> +  }
> +}
> +
> +
>  /**
>    Iterate over the RAM entries in QEMU's fw_cfg E820 RAM map that start outside
>    of the 32-bit address range.
> @@ -688,11 +739,11 @@ QemuInitializeRam (
>      ASSERT_EFI_ERROR (Status);
>  
>      //
> -    // Set memory range from the "top of lower RAM" (RAM below 4GB) to 4GB as
> -    // uncacheable
> +    // Set the memory range from the start of the 32-bit MMIO area (32-bit PCI
> +    // MMIO aperture on i440fx, PCIEXBAR on q35) to 4GB as uncacheable.
>      //
> -    Status = MtrrSetMemoryAttribute (LowerMemorySize,
> -               SIZE_4GB - LowerMemorySize, CacheUncacheable);
> +    Status = MtrrSetMemoryAttribute (mQemuUc32Base, SIZE_4GB - mQemuUc32Base,
> +               CacheUncacheable);
>      ASSERT_EFI_ERROR (Status);
>    }
>  }
> diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c
> index 0876316eefbc..3ba2459872d9 100644
> --- a/OvmfPkg/PlatformPei/Platform.c
> +++ b/OvmfPkg/PlatformPei/Platform.c
> @@ -191,7 +191,8 @@ MemMapInitialization (
>        ASSERT (PciExBarBase <= MAX_UINT32 - SIZE_256MB);
>        PciBase = (UINT32)(PciExBarBase + SIZE_256MB);
>      } else {
> -      PciBase = (TopOfLowRam < BASE_2GB) ? BASE_2GB : TopOfLowRam;
> +      ASSERT (TopOfLowRam <= mQemuUc32Base);
> +      PciBase = mQemuUc32Base;
>      }
>  
>      //
> @@ -650,6 +651,8 @@ InitializePlatform (
>  
>    PublishPeiMemory ();
>  
> +  QemuUc32BaseInitialization ();
> +
>    InitializeRamRegions ();
>  
>    if (mXen) {
> 

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#41810): https://edk2.groups.io/g/devel/message/41810
Mute This Topic: https://groups.io/mt/31834722/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.