public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH edk2-platforms 0/2] QemuOpenBoardPkg: PCI rework and small fixes
@ 2023-01-12 23:13 Pedro Falcato
  2023-01-12 23:13 ` [PATCH edk2-platforms 1/2] QemuOpenBoardPkg: Redo PCI bus initialization Pedro Falcato
  2023-01-12 23:13 ` [PATCH edk2-platforms 2/2] QemuOpenBoardPkg: Trivial code cleanup Pedro Falcato
  0 siblings, 2 replies; 10+ messages in thread
From: Pedro Falcato @ 2023-01-12 23:13 UTC (permalink / raw)
  To: devel; +Cc: Pedro Falcato, Isaac Oram, Theo Jehl

This patch-set contains a large rework of PCI (including migration to MinPlatformPkg's PCI host bus driver)
and trivial code cleanup (some of it included in the PCI patch).

More details about the changes themselves in the commit messages and code comments.

Cc: Isaac Oram <isaac.w.oram@intel.com>
Cc: Theo Jehl <theojehl76@gmail.com>

Pedro Falcato (2):
  QemuOpenBoardPkg: Redo PCI bus initialization
  QemuOpenBoardPkg: Trivial code cleanup

 .../Include/Dsc/Stage2.dsc.inc                |   4 +-
 .../QemuOpenBoardPkg/PlatformInitPei/Cpu.c    |   2 +-
 .../QemuOpenBoardPkg/PlatformInitPei/Memory.c |  31 ++-
 .../QemuOpenBoardPkg/PlatformInitPei/Pci.c    | 227 +++++++++++++++---
 .../QemuOpenBoardPkg/PlatformInitPei/Pcie.c   | 106 --------
 .../PlatformInitPei/PlatformInit.c            |  17 +-
 .../PlatformInitPei/PlatformInit.h            |  60 +++--
 .../PlatformInitPei/PlatformInitPei.inf       |  18 +-
 .../QemuOpenBoardPkg/QemuOpenBoardPkg.dsc     |  17 +-
 9 files changed, 299 insertions(+), 183 deletions(-)
 delete mode 100644 Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Pcie.c

-- 
2.39.0


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

* [PATCH edk2-platforms 1/2] QemuOpenBoardPkg: Redo PCI bus initialization
  2023-01-12 23:13 [PATCH edk2-platforms 0/2] QemuOpenBoardPkg: PCI rework and small fixes Pedro Falcato
@ 2023-01-12 23:13 ` Pedro Falcato
  2023-01-13  1:46   ` Isaac Oram
  2023-01-13  6:33   ` Gerd Hoffmann
  2023-01-12 23:13 ` [PATCH edk2-platforms 2/2] QemuOpenBoardPkg: Trivial code cleanup Pedro Falcato
  1 sibling, 2 replies; 10+ messages in thread
From: Pedro Falcato @ 2023-01-12 23:13 UTC (permalink / raw)
  To: devel; +Cc: Pedro Falcato, Isaac Oram, Theo Jehl, Gerd Hoffmann

Rework PCI MMIO space into something more robust and compatible with
various QEMU configurations. Also, set up 64-bit memory regions for
64-bit BARs and drop OvmfPkg's PciHostBridgeLib for MinPlatformPkg's
PciHostBridgeLibSimple, which does the job just fine.

(cc Gerd for possible comments given his experience with OVMF and qemu)

Signed-off-by: Pedro Falcato <pedro.falcato@gmail.com>
Cc: Isaac Oram <isaac.w.oram@intel.com>
Cc: Theo Jehl <theojehl76@gmail.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
---
 .../Include/Dsc/Stage2.dsc.inc                |   4 +-
 .../QemuOpenBoardPkg/PlatformInitPei/Memory.c |  22 +-
 .../QemuOpenBoardPkg/PlatformInitPei/Pci.c    | 227 +++++++++++++++---
 .../QemuOpenBoardPkg/PlatformInitPei/Pcie.c   | 106 --------
 .../PlatformInitPei/PlatformInit.c            |  17 +-
 .../PlatformInitPei/PlatformInit.h            |  32 ++-
 .../PlatformInitPei/PlatformInitPei.inf       |  18 +-
 .../QemuOpenBoardPkg/QemuOpenBoardPkg.dsc     |  17 +-
 8 files changed, 274 insertions(+), 169 deletions(-)
 delete mode 100644 Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Pcie.c

diff --git a/Platform/Qemu/QemuOpenBoardPkg/Include/Dsc/Stage2.dsc.inc b/Platform/Qemu/QemuOpenBoardPkg/Include/Dsc/Stage2.dsc.inc
index d2e41ce79fda..c240c38cabef 100644
--- a/Platform/Qemu/QemuOpenBoardPkg/Include/Dsc/Stage2.dsc.inc
+++ b/Platform/Qemu/QemuOpenBoardPkg/Include/Dsc/Stage2.dsc.inc
@@ -3,14 +3,14 @@
 #
 # @copyright
 # Copyright (C) 2022 Theo Jehl
+# Copyright (c) 2023 Pedro Falcato All rights reserved.
 #
 # SPDX-License-Identifier: BSD-2-Clause-Patent
 ##
 
 [LibraryClasses.Common]
   ResetSystemLib          | OvmfPkg/Library/ResetSystemLib/BaseResetSystemLib.inf
-  PciHostBridgeLib        | OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf
-  PciHostBridgeUtilityLib | OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf
+  PciHostBridgeLib        | MinPlatformPkg/Pci/Library/PciHostBridgeLibSimple/PciHostBridgeLibSimple.inf
   DxeHardwareInfoLib      | OvmfPkg/Library/HardwareInfoLib/DxeHardwareInfoLib.inf
 
 [LibraryClasses.Common.PEIM]
diff --git a/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Memory.c b/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Memory.c
index 21705256191b..4f312c36016e 100644
--- a/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Memory.c
+++ b/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Memory.c
@@ -2,6 +2,7 @@
   Memory probing and installation
 
   Copyright (c) 2022 Theo Jehl All rights reserved.
+  Copyright (c) 2023 Pedro Falcato All rights reserved.
   SPDX-License-Identifier: BSD-2-Clause-Patent
 **/
 
@@ -53,12 +54,27 @@ GetMemoryBelow4Gb (
       Size += E820Entry.Length;
     } else {
       ASSERT (Size == (UINT32)Size);
-      return (UINT32) Size;
+      return (UINT32)Size;
     }
   }
 
   ASSERT (Size == (UINT32)Size);
-  return (UINT32) Size;
+  return (UINT32)Size;
+}
+
+STATIC UINT64  mTopNonHoleAddr;
+
+/**
+  Return the largest reserved/DRAM/etc address.
+
+  @return Largest non-hole address.
+**/
+UINT64
+GetTopNonHoleAddr (
+  VOID
+  )
+{
+  return mTopNonHoleAddr;
 }
 
 /**
@@ -222,6 +238,8 @@ InstallMemory (
       E820Entry.BaseAddr + E820Entry.Length - 1,
       E820Entry.Type
       ));
+
+    mTopNonHoleAddr = MAX (mTopNonHoleAddr, E820Entry.BaseAddr + E820Entry.Length - 1);
   }
 
   ASSERT (LargestE820Entry.Length != 0);
diff --git a/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Pci.c b/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Pci.c
index 4e6b784d9890..d2724d205552 100644
--- a/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Pci.c
+++ b/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Pci.c
@@ -1,7 +1,8 @@
 /** @file Pci.c
-  PCI Initialization for PIIX4 QEMU
+  PCI Initialization for QEMU platforms
 
   Copyright (c) 2022 Theo Jehl All rights reserved.
+  Copyright (c) 2023 Pedro Falcato All rights reserved.
   SPDX-License-Identifier: BSD-2-Clause-Patent
 **/
 
@@ -9,6 +10,7 @@
 #include <IndustryStandard/Pci.h>
 #include <Library/PciCf8Lib.h>
 #include <Library/QemuOpenFwCfgLib.h>
+#include <IndustryStandard/Q35MchIch9.h>
 #include <IndustryStandard/MemoryMappedConfigurationSpaceAccessTable.h>
 #include <Library/DebugLib.h>
 #include <IndustryStandard/Acpi30.h>
@@ -16,55 +18,220 @@
 #include <Library/PcdLib.h>
 #include <Library/HobLib.h>
 
+typedef EFI_STATUS (*SETUP_PCI_CALLBACK)(
+  VOID
+  );
+
+typedef struct PlatformInfo {
+  UINT16                PciIoBase;
+  UINT16                PciIoSize;
+  BOOLEAN               HasPcie;
+  SETUP_PCI_CALLBACK    SetupPci;
+} QEMU_PLATFORM_INFO;
+
+/**
+  Set up PCI on the q35 platform.
+
+  @retval EFI_SUCCESS   Success.
+**/
+STATIC
+EFI_STATUS
+PciSetupQ35 (
+  VOID
+  );
+
+STATIC
+CONST
+QEMU_PLATFORM_INFO  PlatformData[] =
+{
+  { PIIX4_PCI_IO_BASE, PIIX4_PCI_IO_SIZE, FALSE, NULL        },
+  { Q35_PCI_IO_BASE,   Q35_PCI_IO_SIZE,   TRUE,  PciSetupQ35 }
+};
+
+/**
+  Get platform info for the given platform.
+  range PCDs. Asserts on bad QEMU_PLATFORMs.
+
+  @retval Pointer to the QEMU_PLATFORM_INFO.
+**/
+STATIC
+CONST
+QEMU_PLATFORM_INFO *
+GetPlatform (
+  IN QEMU_PLATFORM  Plat
+  )
+{
+  ASSERT (Plat < QEMU_PLATFORM_MAX);
+  return &PlatformData[Plat];
+}
+
 /**
-  Initialize PCI support for QEMU PIIX4 machine.
+  Initialise PCI QEMU platforms.
 
-  It also publishes PCI MMIO and IO ranges PCDs for OVMF PciHostBridgeLib.
+  Updates PCI IO port and MMIO range PCDs.
 
   @retval EFI_SUCCESS      Initialization was a success.
-  @retval EFI_UNSUPPORTED  Initialization failed (Memory below 4Gb probing failed).
 **/
 EFI_STATUS
 EFIAPI
-InitializePciPIIX4 (
+InitializePci (
+  IN QEMU_PLATFORM  Platform
+  )
+{
+  CONST QEMU_PLATFORM_INFO  *PlatformData;
+  UINT32                    Tolud;
+  UINT32                    MemBase;
+  UINT32                    MemSize;
+  UINT64                    Mem64Base;
+  UINT64                    Mem64Size;
+  UINT32                    EcamBase;
+
+  PlatformData = GetPlatform (Platform);
+
+  ASSERT (PlatformData != NULL);
+
+  Tolud = GetMemoryBelow4Gb ();
+
+  //
+  // Configure the IO port ranges PCDs based on the platform
+  //
+  PcdSet16S (PcdPciReservedIobase, PlatformData->PciIoBase);
+  PcdSet16S (PcdPciReservedIoLimit, PlatformData->PciIoBase + PlatformData->PciIoSize - 1);
+
+  //
+  // Set up the PCI MMIO ranges
+  // Some context around this logic (after diving into QEMU source):
+  // Traditionally, QEMU seems to want to use a split at around
+  // 0xe0000000 (3.5GiB). After that things are reserved for PCI MMIO.
+  // However, PIIX4 grew a gigabyte_align option that can readjust this
+  // down to 0xc0000000 (3GiB).
+  // Q35 logic seems to dock lowmem at around 0x80000000 (2GiB) if it can't
+  // fit the whole lowmem under 4GB. If it can, it limits lowmem to 0xb0000000.
+  //
+  // It's worth noting that QEMU also grew an option to change lowmem based on the
+  // user's preferences. Because of all of this, it's near impossible to hardcode
+  // a range, so we grab TOLUD (not in a literal way, since QEMU does not implement
+  // that register ;)) and calculate our PCI MMIO based on that. This also makes it so
+  // the DSDT built by QEMU will have correct _CRS ranges.
+  // hw/pci-host/q35.c explicitly says our PCI hole ranges from [TOLUD, IO APIC].
+  // As far as I can tell, we seem to be allowed to add a 64-bit resource range anywhere.
+  //
+
+  MemBase = Tolud;
+  MemSize = PCI_MMIO_TOP_ADDRESS - MemBase;
+
+  //
+  // Set things up in the following way:
+  // ----------------------------------
+  //        Mem32-decoding-region
+  // ----------------------------------
+  //            ECAM/MMCONFIG
+  // ----------------------------------
+  //   Misc top regions, 64-bit memory
+  // ----------------------------------
+  //        Mem64-decoding-region
+  // ----------------------------------
+  //
+
+  if (PlatformData->HasPcie) {
+    ASSERT (FixedPcdGet64 (PcdPciExpressBaseAddress) < BASE_4GB);
+    EcamBase = (UINT32)FixedPcdGet64 (PcdPciExpressBaseAddress);
+
+    BuildResourceDescriptorHob (
+      EFI_RESOURCE_MEMORY_MAPPED_IO,
+      EFI_RESOURCE_ATTRIBUTE_PRESENT |
+      EFI_RESOURCE_ATTRIBUTE_INITIALIZED |
+      EFI_RESOURCE_ATTRIBUTE_UNCACHEABLE |
+      EFI_RESOURCE_ATTRIBUTE_TESTED,
+      EcamBase,
+      SIZE_256MB
+      );
+
+    BuildMemoryAllocationHob (
+      EcamBase,
+      SIZE_256MB,
+      EfiMemoryMappedIO
+      );
+
+    //
+    // Adjust the PCI MEM window
+    //
+    MemSize = EcamBase - MemBase;
+
+    DEBUG ((DEBUG_INFO, "PlatformInitPei: Using ECAM @ [%x, %x]\n", EcamBase, EcamBase + SIZE_256MB - 1));
+  }
+
+  PcdSetBoolS (PcdPciNoExtendedConfigSpace, !PlatformData->HasPcie);
+
+  PcdSet32S (PcdPciReservedMemBase, MemBase);
+  PcdSet32S (PcdPciReservedMemLimit, MemBase + MemSize - 1);
+
+  DEBUG ((DEBUG_INFO, "PlatformInitPei: PCI Mem window @ [%x, %x]\n", MemBase, MemBase + MemSize - 1));
+
+  //
+  // For simplicity, start the Mem64 region after every other
+  // valid non-hole memory region.
+  // Note: Unclear if this has any disadvantages.
+  //
+  Mem64Base = GetTopNonHoleAddr ();
+
+  if (Mem64Base < BASE_4GB) {
+    Mem64Base = BASE_4GB;
+  }
+
+  Mem64Size = SIZE_32GB;
+
+  PcdSet64S (PcdPciReservedMemAbove4GBBase, Mem64Base);
+  PcdSet64S (PcdPciReservedMemAbove4GBLimit, Mem64Base + Mem64Size - 1);
+
+  DEBUG ((DEBUG_INFO, "PlatformInitPei: PCI Mem64 window @ [%lx, %lx]\n", Mem64Base, Mem64Base + Mem64Size - 1));
+
+  if (PlatformData->SetupPci) {
+    EFI_STATUS  Status;
+    Status = PlatformData->SetupPci ();
+
+    if (EFI_ERROR (Status)) {
+      return Status;
+    }
+  }
+
+  return EFI_SUCCESS;
+}
+
+/**
+  Set up PCI on the q35 platform.
+
+  @retval EFI_SUCCESS   Success.
+**/
+STATIC
+EFI_STATUS
+PciSetupQ35 (
   VOID
   )
 {
-  UINTN  PciIoBase;
-  UINTN  PciIoSize;
-  UINTN  PciMmio32Base;
-  UINTN  PciMmio32Size;
+  UINT64  PciExBarBase;
+
+  PciExBarBase = PcdGet64 (PcdPciExpressBaseAddress);
 
   //
-  //  Setup PCI IO ranges for 440FX/PIIX4 platform
+  // Disable the ECAM before re-programming it.
   //
-  PciIoBase = PIIX4_PCI_IO_BASE;
-  PciIoSize = PIIX4_PCI_IO_SIZE;
-
-  PcdSet64S (PcdPciIoBase, PciIoBase);
-  PcdSet64S (PcdPciIoSize, PciIoSize);
+  PciWrite32 (DRAMC_REGISTER_Q35 (MCH_PCIEXBAR_LOW), 0);
 
   //
-  //  QEMU only allow a maximum of 2.8Gb of real memory below 4G
-  //  PCI MMIO range below 4Gb starts at the end of real memory below 4G
+  // Now, program the PCIe ECAM into the MCH PCIEXBAR register. We do it in a high-low order
+  // as to avoid temporary misdecoding of addresses.
   //
-  PciMmio32Base = (UINTN)GetMemoryBelow4Gb ();
 
-  if (PciMmio32Base == 0) {
-    DEBUG ((DEBUG_ERROR, "Unable to detect memory below 4Gb\n"));
-    ASSERT (PciMmio32Base != 0);
-    return EFI_UNSUPPORTED;
-  }
-
-  DEBUG ((DEBUG_ERROR, "Memory below 4Gb: %x \n", PciMmio32Base));
+  PciWrite32 (DRAMC_REGISTER_Q35 (MCH_PCIEXBAR_HIGH), (UINT32)(RShiftU64 (PciExBarBase, 32)));
 
   //
-  // Maximum size being PCI_MMIO_TOP_ADDRESS - TopOfLowMem to avoid overlapping with IO-APIC and other hardware mmio ranges
+  // Finally, program the low bits and simultaneously enable it.
   //
-  PciMmio32Size = PCI_MMIO_TOP_ADDRESS - PciMmio32Base;
-
-  PcdSet64S (PcdPciMmio32Base, PciMmio32Base);
-  PcdSet64S (PcdPciMmio32Size, PciMmio32Size);
+  PciWrite32 (
+    DRAMC_REGISTER_Q35 (MCH_PCIEXBAR_LOW),
+    (UINT32)PciExBarBase | MCH_PCIEXBAR_BUS_FF | MCH_PCIEXBAR_EN
+    );
 
   return EFI_SUCCESS;
 }
diff --git a/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Pcie.c b/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Pcie.c
deleted file mode 100644
index 0a5f0ff398de..000000000000
--- a/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Pcie.c
+++ /dev/null
@@ -1,106 +0,0 @@
-/** @file Pcie.c
-  PCI Express initialization for QEMU Q35
-
-  Copyright (c) 2022 Theo Jehl All rights reserved.
-  SPDX-License-Identifier: BSD-2-Clause-Patent
-**/
-
-#include "PlatformInit.h"
-#include <IndustryStandard/Pci.h>
-#include <Library/PciCf8Lib.h>
-#include <IndustryStandard/Q35MchIch9.h>
-#include <Library/QemuOpenFwCfgLib.h>
-#include <IndustryStandard/QemuFwCfg.h>
-#include <IndustryStandard/MemoryMappedConfigurationSpaceAccessTable.h>
-#include <Library/DebugLib.h>
-#include <IndustryStandard/Acpi30.h>
-#include <Library/MemoryAllocationLib.h>
-#include <Library/PcdLib.h>
-#include <Library/HobLib.h>
-
-/**
-  Initialize PCI Express support for QEMU Q35 system.
-  It also publishes PCI MMIO and IO ranges PCDs for OVMF PciHostBridgeLib.
-
-  @retval EFI_SUCCESS Initialization was successful
-**/
-EFI_STATUS
-EFIAPI
-InitializePcie (
-  VOID
-  )
-{
-  UINTN  PciBase;
-  UINTN  PciSize;
-  UINTN  PciIoBase;
-  UINTN  PciIoSize;
-
-  union {
-    UINT64    Uint64;
-    UINT32    Uint32[2];
-  } PciExBarBase;
-
-  PciExBarBase.Uint64 = FixedPcdGet64 (PcdPciExpressBaseAddress);
-
-  //
-  // Build a reserved memory space for PCIE MMIO
-  //
-  BuildResourceDescriptorHob (
-    EFI_RESOURCE_MEMORY_RESERVED,
-    EFI_RESOURCE_ATTRIBUTE_PRESENT |
-    EFI_RESOURCE_ATTRIBUTE_INITIALIZED |
-    EFI_RESOURCE_ATTRIBUTE_UNCACHEABLE |
-    EFI_RESOURCE_ATTRIBUTE_TESTED,
-    PciExBarBase.Uint64,
-    SIZE_256MB
-    );
-
-  BuildMemoryAllocationHob (
-    PciExBarBase.Uint64,
-    SIZE_256MB,
-    EfiReservedMemoryType
-    );
-
-  //
-  // Clear lower 32 bits of register
-  //
-  PciWrite32 (DRAMC_REGISTER_Q35 (MCH_PCIEXBAR_LOW), 0);
-
-  //
-  // Program PCIE MMIO Base address in MCH PCIEXBAR register
-  //
-  PciWrite32 (DRAMC_REGISTER_Q35 (MCH_PCIEXBAR_HIGH), PciExBarBase.Uint32[1]);
-
-  //
-  // Enable 256Mb MMIO space
-  //
-  PciWrite32 (
-    DRAMC_REGISTER_Q35 (MCH_PCIEXBAR_LOW),
-    PciExBarBase.Uint32[0] | MCH_PCIEXBAR_BUS_FF | MCH_PCIEXBAR_EN
-    );
-
-  //
-  // Disable PCI/PCIe MMIO above 4Gb
-  //
-  PcdSet64S (PcdPciMmio64Size, 0);
-
-  //
-  // Set Pci MMIO space below 4GB
-  //
-  PciBase = (UINTN)(PcdGet64 (PcdPciExpressBaseAddress) + SIZE_256MB);
-  PciSize = PCI_MMIO_TOP_ADDRESS - PciBase;
-
-  PcdSet64S (PcdPciMmio32Base, PciBase);
-  PcdSet64S (PcdPciMmio32Size, PciSize);
-
-  //
-  // Set Pci IO port range
-  //
-  PciIoBase = Q35_PCI_IO_BASE;
-  PciIoSize = Q35_PCI_IO_SIZE;
-
-  PcdSet64S (PcdPciIoBase, PciIoBase);
-  PcdSet64S (PcdPciIoSize, PciIoSize);
-
-  return EFI_SUCCESS;
-}
diff --git a/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/PlatformInit.c b/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/PlatformInit.c
index 7849298b52d5..d23895afc967 100644
--- a/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/PlatformInit.c
+++ b/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/PlatformInit.c
@@ -2,6 +2,7 @@
   Platform initialization PEIM for QEMU
 
   Copyright (c) 2022 Theo Jehl All rights reserved.
+  Copyright (c) 2023 Pedro Falcato All rights reserved.
   SPDX-License-Identifier: BSD-2-Clause-Patent
 **/
 
@@ -36,8 +37,6 @@ PlatformInit (
   if (EFI_ERROR (Status)) {
     DEBUG ((DEBUG_ERROR, "Memory installation failed\n"));
     return Status;
-  } else {
-    DEBUG ((DEBUG_INFO, "Memory installation success\n"));
   }
 
   //
@@ -48,7 +47,7 @@ PlatformInit (
   EfiPlatformInfo = AllocateZeroPool (sizeof (EFI_HOB_PLATFORM_INFO));
   if (EfiPlatformInfo == NULL) {
     DEBUG ((DEBUG_ERROR, "Failed to allocate pool for EFI_HOB_PLATFORM_INFO\n"));
-    return EFI_UNSUPPORTED;
+    return EFI_OUT_OF_RESOURCES;
   }
 
   //
@@ -56,20 +55,14 @@ PlatformInit (
   //
   DeviceId = PciCf8Read16 (PCI_CF8_LIB_ADDRESS (0, 0, 0, PCI_DEVICE_ID_OFFSET));
   DEBUG ((DEBUG_INFO, "Building gUefiOvmfPkgPlatformInfoGuid with Host bridge dev ID %x \n", DeviceId));
-  (*EfiPlatformInfo).HostBridgeDevId = DeviceId;
+  EfiPlatformInfo->HostBridgeDevId = DeviceId;
 
   BuildGuidDataHob (&gUefiOvmfPkgPlatformInfoGuid, EfiPlatformInfo, sizeof (EFI_HOB_PLATFORM_INFO));
 
   PcdSet16S (PcdOvmfHostBridgePciDevId, DeviceId);
 
   //
-  // Initialize PCI or PCIe based on current emulated system
+  // Finally, initialize PCI MMIO ranges and possibly the MMCONFIG
   //
-  if (DeviceId == INTEL_Q35_MCH_DEVICE_ID) {
-    DEBUG ((DEBUG_INFO, "Q35: Initialize PCIe\n"));
-    return InitializePcie ();
-  } else {
-    DEBUG ((DEBUG_INFO, "PIIX4: Initialize PCI\n"));
-    return InitializePciPIIX4 ();
-  }
+  return InitializePci (DeviceId == INTEL_Q35_MCH_DEVICE_ID ? QEMU_PLATFORM_Q35 : QEMU_PLATFORM_PIIX4);
 }
diff --git a/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/PlatformInit.h b/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/PlatformInit.h
index 771d2f958c40..f4044df3dbf5 100644
--- a/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/PlatformInit.h
+++ b/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/PlatformInit.h
@@ -2,6 +2,7 @@
   Headers for PlatformInitPei PEIM
 
   Copyright (c) 2022 Theo Jehl All rights reserved.
+  Copyright (c) 2023 Pedro Falcato All rights reserved.
   SPDX-License-Identifier: BSD-2-Clause-Patent
 **/
 
@@ -38,16 +39,23 @@ InstallMemory (
   IN CONST EFI_PEI_SERVICES  **PeiServices
   );
 
-EFI_STATUS
-EFIAPI
-InitializePcie (
-  VOID
-  );
+typedef enum Platforms {
+  QEMU_PLATFORM_PIIX4 = 0,
+  QEMU_PLATFORM_Q35,
+  QEMU_PLATFORM_MAX
+} QEMU_PLATFORM;
+
+/**
+  Initialise PCI QEMU platforms.
+
+  Updates PCI IO port and MMIO range PCDs.
 
+  @retval EFI_SUCCESS      Initialization was a success.
+**/
 EFI_STATUS
 EFIAPI
-InitializePciPIIX4 (
-  VOID
+InitializePci (
+  QEMU_PLATFORM  Platform
   );
 
 EFI_STATUS
@@ -56,4 +64,14 @@ MaxCpuInit (
   VOID
   );
 
+/**
+  Return the largest reserved/DRAM/etc address.
+
+  @return Largest non-hole address.
+**/
+UINT64
+GetTopNonHoleAddr (
+  VOID
+  );
+
 #endif //QEMU_OPEN_BOARD_PKG_PLATFORM_INIT_H_
diff --git a/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/PlatformInitPei.inf b/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/PlatformInitPei.inf
index 43b1e13adfeb..5f42d46178f7 100644
--- a/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/PlatformInitPei.inf
+++ b/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/PlatformInitPei.inf
@@ -4,6 +4,7 @@
 #  Simple PEIM for QEMU PIIX4/Q35 Memory, SMP and PCI/PCI Express initialization
 #
 #  Copyright (c) 2022 Theo Jehl
+#  Copyright (c) 2023 Pedro Falcato All rights reserved.
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 
 [Defines]
@@ -17,14 +18,15 @@
 [Packages]
   OvmfPkg/OvmfPkg.dec
   MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
   QemuOpenBoardPkg/QemuOpenBoardPkg.dec
   UefiCpuPkg/UefiCpuPkg.dec
+  MinPlatformPkg/MinPlatformPkg.dec
 
 [Sources]
   PlatformInit.h
   PlatformInit.c
   Memory.c
-  Pcie.c
   Pci.c
   Cpu.c
 
@@ -43,13 +45,15 @@
   gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
   gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber
   gUefiCpuPkgTokenSpaceGuid.PcdCpuBootLogicalProcessorNumber
-  gUefiOvmfPkgTokenSpaceGuid.PcdPciIoBase
-  gUefiOvmfPkgTokenSpaceGuid.PcdPciIoSize
-  gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio32Base
-  gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio32Size
-  gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Base
-  gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Size
   gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
+  gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
+  gMinPlatformPkgTokenSpaceGuid.PcdPciReservedMemAbove4GBBase
+  gMinPlatformPkgTokenSpaceGuid.PcdPciReservedMemAbove4GBLimit
+  gMinPlatformPkgTokenSpaceGuid.PcdPciReservedMemBase
+  gMinPlatformPkgTokenSpaceGuid.PcdPciReservedMemLimit
+  gMinPlatformPkgTokenSpaceGuid.PcdPciReservedIobase
+  gMinPlatformPkgTokenSpaceGuid.PcdPciReservedIoLimit
+  gMinPlatformPkgTokenSpaceGuid.PcdPciNoExtendedConfigSpace
   gUefiOvmfPkgTokenSpaceGuid.PcdQ35TsegMbytes
 
 [FeaturePcd]
diff --git a/Platform/Qemu/QemuOpenBoardPkg/QemuOpenBoardPkg.dsc b/Platform/Qemu/QemuOpenBoardPkg/QemuOpenBoardPkg.dsc
index f5c317c83e6a..b2b82b84be4c 100644
--- a/Platform/Qemu/QemuOpenBoardPkg/QemuOpenBoardPkg.dsc
+++ b/Platform/Qemu/QemuOpenBoardPkg/QemuOpenBoardPkg.dsc
@@ -4,6 +4,7 @@
 #  Description file for QemuOpenBoardPkg
 #
 #  Copyright (c) 2022 Theo Jehl
+#  Copyright (c) 2023 Pedro Falcato All rights reserved.
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 ##
 
@@ -78,9 +79,7 @@
   gQemuOpenBoardPkgTokenSpaceGuid.PcdDebugIoPort                        | 0x402
   gEfiMdePkgTokenSpaceGuid.PcdFSBClock                                  | 100000000
 
-  # PCIe base address for Q35 machines
-  # QEMU usable memory below 4G cannot exceed 2.8Gb
-  gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress                     | 0xB0000000
+  gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress                       | 0xE0000000
 
   gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvModeEnable             | TRUE
   gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange  | FALSE
@@ -119,6 +118,18 @@
   gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber             | 0
   gUefiCpuPkgTokenSpaceGuid.PcdCpuBootLogicalProcessorNumber            | 0
 
+  # All of these MinPlatform PCI PCDs are filled in PlatformInitPei
+  # They must be dynamic since we don't know what platform (ICH9 or PIIX4) we're booting
+  # or how the memory map looks like.
+  gMinPlatformPkgTokenSpaceGuid.PcdPciReservedMemAbove4GBBase             | 0xffffffffffffffff
+  gMinPlatformPkgTokenSpaceGuid.PcdPciReservedMemAbove4GBLimit            | 0
+  gMinPlatformPkgTokenSpaceGuid.PcdPciReservedMemBase                     | 0x90000000
+  gMinPlatformPkgTokenSpaceGuid.PcdPciReservedMemLimit                    | 0
+  gMinPlatformPkgTokenSpaceGuid.PcdPciReservedIobase                      | 0x2000
+  gMinPlatformPkgTokenSpaceGuid.PcdPciReservedIoLimit                     | 0xffff
+
+  gMinPlatformPkgTokenSpaceGuid.PcdPciNoExtendedConfigSpace               | TRUE
+
   !if $(SMM_REQUIRED) == TRUE
     gUefiOvmfPkgTokenSpaceGuid.PcdQ35TsegMbytes                         | 8
     gUefiOvmfPkgTokenSpaceGuid.PcdQ35SmramAtDefaultSmbase               | FALSE
-- 
2.39.0


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

* [PATCH edk2-platforms 2/2] QemuOpenBoardPkg: Trivial code cleanup
  2023-01-12 23:13 [PATCH edk2-platforms 0/2] QemuOpenBoardPkg: PCI rework and small fixes Pedro Falcato
  2023-01-12 23:13 ` [PATCH edk2-platforms 1/2] QemuOpenBoardPkg: Redo PCI bus initialization Pedro Falcato
@ 2023-01-12 23:13 ` Pedro Falcato
  2023-01-13  1:48   ` Isaac Oram
  2023-01-13 12:58   ` Théo Jehl
  1 sibling, 2 replies; 10+ messages in thread
From: Pedro Falcato @ 2023-01-12 23:13 UTC (permalink / raw)
  To: devel; +Cc: Pedro Falcato, Isaac Oram, Theo Jehl

Small cleanups around PlatformInitPei.

Signed-off-by: Pedro Falcato <pedro.falcato@gmail.com>
Cc: Isaac Oram <isaac.w.oram@intel.com>
Cc: Theo Jehl <theojehl76@gmail.com>
---
 .../QemuOpenBoardPkg/PlatformInitPei/Cpu.c    |  2 +-
 .../QemuOpenBoardPkg/PlatformInitPei/Memory.c |  9 ++----
 .../PlatformInitPei/PlatformInit.h            | 28 ++++++++++++++-----
 3 files changed, 25 insertions(+), 14 deletions(-)

diff --git a/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Cpu.c b/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Cpu.c
index e203b2654226..2fc62a0a3e77 100644
--- a/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Cpu.c
+++ b/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Cpu.c
@@ -20,7 +20,7 @@
 /**
   Probe Qemu FW CFG device for current CPU count and report to MpInitLib.
 
-  @return EFI_SUCCESS      Detection was successful.
+  @retval EFI_SUCCESS      Detection was successful.
   @retval EFI_UNSUPPORTED  QEMU FW CFG device is not present.
  */
 EFI_STATUS
diff --git a/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Memory.c b/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Memory.c
index 4f312c36016e..223cace0ca98 100644
--- a/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Memory.c
+++ b/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Memory.c
@@ -86,8 +86,8 @@ GetTopNonHoleAddr (
 STATIC
 VOID
 ReserveMmioRegion (
-  EFI_PHYSICAL_ADDRESS  Start,
-  UINT64                Length
+  IN EFI_PHYSICAL_ADDRESS  Start,
+  IN UINT64                Length
   )
 {
   EFI_RESOURCE_TYPE            ResourceType;
@@ -121,7 +121,6 @@ InstallMemory (
   )
 {
   EFI_STATUS                   Status;
-  CONST EFI_PEI_SERVICES       **PeiServicesTable;
   EFI_E820_ENTRY64             E820Entry;
   EFI_E820_ENTRY64             LargestE820Entry;
   QEMU_FW_CFG_FILE             FwCfgFile;
@@ -250,9 +249,7 @@ InstallMemory (
     LargestE820Entry.BaseAddr + LargestE820Entry.Length - 1
     ));
 
-  PeiServicesTable = GetPeiServicesTablePointer ();
-
-  Status = (*PeiServices)->InstallPeiMemory (PeiServicesTable, LargestE820Entry.BaseAddr, LargestE820Entry.Length);
+  Status = (*PeiServices)->InstallPeiMemory (PeiServices, LargestE820Entry.BaseAddr, LargestE820Entry.Length);
 
   ASSERT_EFI_ERROR (Status);
 
diff --git a/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/PlatformInit.h b/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/PlatformInit.h
index f4044df3dbf5..f17df707188a 100644
--- a/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/PlatformInit.h
+++ b/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/PlatformInit.h
@@ -20,19 +20,27 @@
 
 #define PCI_MMIO_TOP_ADDRESS  0xFC000000
 
-EFI_STATUS
-EFIAPI
-PlatformInit (
-  IN       EFI_PEI_FILE_HANDLE  FileHandle,
-  IN CONST EFI_PEI_SERVICES     **PeiServices
-  );
+/**
+  Return the memory size below 4GB.
 
+  @return Size of memory below 4GB, in bytes.
+**/
 UINT32
 EFIAPI
 GetMemoryBelow4Gb (
   VOID
   );
 
+/**
+  Install EFI memory by probing QEMU FW CFG devices for valid E820 entries.
+  It also reserves space for MMIO regions such as VGA, BIOS and APIC.
+
+  @param[in] PeiServices      PEI Services pointer.
+
+  @retval EFI_SUCCESS     Memory initialization succeded.
+  @retval EFI_UNSUPPORTED Installation failed (etc/e820 file was not found).
+  @retval EFI_NOT_FOUND   QEMU FW CFG device is not present.
+**/
 EFI_STATUS
 EFIAPI
 InstallMemory (
@@ -58,6 +66,12 @@ InitializePci (
   QEMU_PLATFORM  Platform
   );
 
+/**
+  Probe Qemu FW CFG device for current CPU count and report to MpInitLib.
+
+  @retval EFI_SUCCESS      Detection was successful.
+  @retval EFI_UNSUPPORTED  QEMU FW CFG device is not present.
+ */
 EFI_STATUS
 EFIAPI
 MaxCpuInit (
@@ -74,4 +88,4 @@ GetTopNonHoleAddr (
   VOID
   );
 
-#endif //QEMU_OPEN_BOARD_PKG_PLATFORM_INIT_H_
+#endif // QEMU_OPEN_BOARD_PKG_PLATFORM_INIT_H_
-- 
2.39.0


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

* Re: [PATCH edk2-platforms 1/2] QemuOpenBoardPkg: Redo PCI bus initialization
  2023-01-12 23:13 ` [PATCH edk2-platforms 1/2] QemuOpenBoardPkg: Redo PCI bus initialization Pedro Falcato
@ 2023-01-13  1:46   ` Isaac Oram
  2023-01-13  6:33   ` Gerd Hoffmann
  1 sibling, 0 replies; 10+ messages in thread
From: Isaac Oram @ 2023-01-13  1:46 UTC (permalink / raw)
  To: devel@edk2.groups.io; +Cc: Theo Jehl, Gerd Hoffmann

Reviewed-by: Isaac Oram <isaac.w.oram@intel.com>

Minor comments that could be fixed:

Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Memory.c: 65:  We generally don't like includes, defines, etc in the middle of the code.  Convention is near the top of the file so easy to find.  Also generally good to initialize everything to a known value.  And probably add a comment for what this is for.  
The name strikes me as odd.  The traditional name might be TOLUM (top of low usable memory).  Or maybe you might find something like mMaxUsableMemoryAddress as more readable.
It might also be good to assert if it has not been initialized yet when the function is called.  It would be better to return a status.

Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Pci.c: Again, mixing globals with code.

Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/PlatformInit.h (49)
"Initialise"  Am I allowed to correct British English spellings?  Joking aside it looks like a typo to use both Initialise and InitializePci.  I would suggest that Initialize is more common in our codebase.

Platform\Qemu\QemuOpenBoardPkg\PlatformInitPei\PlatformInitPei.inf: 49:  Duplicate line of code.

C:\Source\Open\edk2-platforms\Platform\Qemu\QemuOpenBoardPkg\QemuOpenBoardPkg.dsc:  
The additions are the only | out of vertical alignment.
I typically see hex numbers in uppercase.  I think that makes it more readable, but I am not sure if that is a formal requirement of coding style.

Regards,
Isaac

-----Original Message-----
From: Pedro Falcato <pedro.falcato@gmail.com> 
Sent: Thursday, January 12, 2023 3:14 PM
To: devel@edk2.groups.io
Cc: Pedro Falcato <pedro.falcato@gmail.com>; Oram, Isaac W <isaac.w.oram@intel.com>; Theo Jehl <theojehl76@gmail.com>; Gerd Hoffmann <kraxel@redhat.com>
Subject: [PATCH edk2-platforms 1/2] QemuOpenBoardPkg: Redo PCI bus initialization

Rework PCI MMIO space into something more robust and compatible with various QEMU configurations. Also, set up 64-bit memory regions for 64-bit BARs and drop OvmfPkg's PciHostBridgeLib for MinPlatformPkg's PciHostBridgeLibSimple, which does the job just fine.

(cc Gerd for possible comments given his experience with OVMF and qemu)

Signed-off-by: Pedro Falcato <pedro.falcato@gmail.com>
Cc: Isaac Oram <isaac.w.oram@intel.com>
Cc: Theo Jehl <theojehl76@gmail.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
---
 .../Include/Dsc/Stage2.dsc.inc                |   4 +-
 .../QemuOpenBoardPkg/PlatformInitPei/Memory.c |  22 +-
 .../QemuOpenBoardPkg/PlatformInitPei/Pci.c    | 227 +++++++++++++++---
 .../QemuOpenBoardPkg/PlatformInitPei/Pcie.c   | 106 --------
 .../PlatformInitPei/PlatformInit.c            |  17 +-
 .../PlatformInitPei/PlatformInit.h            |  32 ++-
 .../PlatformInitPei/PlatformInitPei.inf       |  18 +-
 .../QemuOpenBoardPkg/QemuOpenBoardPkg.dsc     |  17 +-
 8 files changed, 274 insertions(+), 169 deletions(-)  delete mode 100644 Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Pcie.c

diff --git a/Platform/Qemu/QemuOpenBoardPkg/Include/Dsc/Stage2.dsc.inc b/Platform/Qemu/QemuOpenBoardPkg/Include/Dsc/Stage2.dsc.inc
index d2e41ce79fda..c240c38cabef 100644
--- a/Platform/Qemu/QemuOpenBoardPkg/Include/Dsc/Stage2.dsc.inc
+++ b/Platform/Qemu/QemuOpenBoardPkg/Include/Dsc/Stage2.dsc.inc
@@ -3,14 +3,14 @@
 #
 # @copyright
 # Copyright (C) 2022 Theo Jehl
+# Copyright (c) 2023 Pedro Falcato All rights reserved.
 #
 # SPDX-License-Identifier: BSD-2-Clause-Patent  ##
 
 [LibraryClasses.Common]
   ResetSystemLib          | OvmfPkg/Library/ResetSystemLib/BaseResetSystemLib.inf
-  PciHostBridgeLib        | OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf
-  PciHostBridgeUtilityLib | OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf
+  PciHostBridgeLib        | MinPlatformPkg/Pci/Library/PciHostBridgeLibSimple/PciHostBridgeLibSimple.inf
   DxeHardwareInfoLib      | OvmfPkg/Library/HardwareInfoLib/DxeHardwareInfoLib.inf
 
 [LibraryClasses.Common.PEIM]
diff --git a/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Memory.c b/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Memory.c
index 21705256191b..4f312c36016e 100644
--- a/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Memory.c
+++ b/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Memory.c
@@ -2,6 +2,7 @@
   Memory probing and installation
 
   Copyright (c) 2022 Theo Jehl All rights reserved.
+  Copyright (c) 2023 Pedro Falcato All rights reserved.
   SPDX-License-Identifier: BSD-2-Clause-Patent  **/
 
@@ -53,12 +54,27 @@ GetMemoryBelow4Gb (
       Size += E820Entry.Length;
     } else {
       ASSERT (Size == (UINT32)Size);
-      return (UINT32) Size;
+      return (UINT32)Size;
     }
   }
 
   ASSERT (Size == (UINT32)Size);
-  return (UINT32) Size;
+  return (UINT32)Size;
+}
+
+STATIC UINT64  mTopNonHoleAddr;
+
+/**
+  Return the largest reserved/DRAM/etc address.
+
+  @return Largest non-hole address.
+**/
+UINT64
+GetTopNonHoleAddr (
+  VOID
+  )
+{
+  return mTopNonHoleAddr;
 }
 
 /**
@@ -222,6 +238,8 @@ InstallMemory (
       E820Entry.BaseAddr + E820Entry.Length - 1,
       E820Entry.Type
       ));
+
+    mTopNonHoleAddr = MAX (mTopNonHoleAddr, E820Entry.BaseAddr + 
+ E820Entry.Length - 1);
   }
 
   ASSERT (LargestE820Entry.Length != 0); diff --git a/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Pci.c b/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Pci.c
index 4e6b784d9890..d2724d205552 100644
--- a/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Pci.c
+++ b/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Pci.c
@@ -1,7 +1,8 @@
 /** @file Pci.c
-  PCI Initialization for PIIX4 QEMU
+  PCI Initialization for QEMU platforms
 
   Copyright (c) 2022 Theo Jehl All rights reserved.
+  Copyright (c) 2023 Pedro Falcato All rights reserved.
   SPDX-License-Identifier: BSD-2-Clause-Patent  **/
 
@@ -9,6 +10,7 @@
 #include <IndustryStandard/Pci.h>
 #include <Library/PciCf8Lib.h>
 #include <Library/QemuOpenFwCfgLib.h>
+#include <IndustryStandard/Q35MchIch9.h>
 #include <IndustryStandard/MemoryMappedConfigurationSpaceAccessTable.h>
 #include <Library/DebugLib.h>
 #include <IndustryStandard/Acpi30.h>
@@ -16,55 +18,220 @@
 #include <Library/PcdLib.h>
 #include <Library/HobLib.h>
 
+typedef EFI_STATUS (*SETUP_PCI_CALLBACK)(
+  VOID
+  );
+
+typedef struct PlatformInfo {
+  UINT16                PciIoBase;
+  UINT16                PciIoSize;
+  BOOLEAN               HasPcie;
+  SETUP_PCI_CALLBACK    SetupPci;
+} QEMU_PLATFORM_INFO;
+
+/**
+  Set up PCI on the q35 platform.
+
+  @retval EFI_SUCCESS   Success.
+**/
+STATIC
+EFI_STATUS
+PciSetupQ35 (
+  VOID
+  );
+
+STATIC
+CONST
+QEMU_PLATFORM_INFO  PlatformData[] =
+{
+  { PIIX4_PCI_IO_BASE, PIIX4_PCI_IO_SIZE, FALSE, NULL        },
+  { Q35_PCI_IO_BASE,   Q35_PCI_IO_SIZE,   TRUE,  PciSetupQ35 }
+};
+
+/**
+  Get platform info for the given platform.
+  range PCDs. Asserts on bad QEMU_PLATFORMs.
+
+  @retval Pointer to the QEMU_PLATFORM_INFO.
+**/
+STATIC
+CONST
+QEMU_PLATFORM_INFO *
+GetPlatform (
+  IN QEMU_PLATFORM  Plat
+  )
+{
+  ASSERT (Plat < QEMU_PLATFORM_MAX);
+  return &PlatformData[Plat];
+}
+
 /**
-  Initialize PCI support for QEMU PIIX4 machine.
+  Initialise PCI QEMU platforms.
 
-  It also publishes PCI MMIO and IO ranges PCDs for OVMF PciHostBridgeLib.
+  Updates PCI IO port and MMIO range PCDs.
 
   @retval EFI_SUCCESS      Initialization was a success.
-  @retval EFI_UNSUPPORTED  Initialization failed (Memory below 4Gb probing failed).
 **/
 EFI_STATUS
 EFIAPI
-InitializePciPIIX4 (
+InitializePci (
+  IN QEMU_PLATFORM  Platform
+  )
+{
+  CONST QEMU_PLATFORM_INFO  *PlatformData;
+  UINT32                    Tolud;
+  UINT32                    MemBase;
+  UINT32                    MemSize;
+  UINT64                    Mem64Base;
+  UINT64                    Mem64Size;
+  UINT32                    EcamBase;
+
+  PlatformData = GetPlatform (Platform);
+
+  ASSERT (PlatformData != NULL);
+
+  Tolud = GetMemoryBelow4Gb ();
+
+  //
+  // Configure the IO port ranges PCDs based on the platform  //  
+ PcdSet16S (PcdPciReservedIobase, PlatformData->PciIoBase);  PcdSet16S 
+ (PcdPciReservedIoLimit, PlatformData->PciIoBase + 
+ PlatformData->PciIoSize - 1);
+
+  //
+  // Set up the PCI MMIO ranges
+  // Some context around this logic (after diving into QEMU source):
+  // Traditionally, QEMU seems to want to use a split at around  // 
+ 0xe0000000 (3.5GiB). After that things are reserved for PCI MMIO.
+  // However, PIIX4 grew a gigabyte_align option that can readjust this  
+ // down to 0xc0000000 (3GiB).
+  // Q35 logic seems to dock lowmem at around 0x80000000 (2GiB) if it 
+ can't  // fit the whole lowmem under 4GB. If it can, it limits lowmem to 0xb0000000.
+  //
+  // It's worth noting that QEMU also grew an option to change lowmem 
+ based on the  // user's preferences. Because of all of this, it's near 
+ impossible to hardcode  // a range, so we grab TOLUD (not in a literal 
+ way, since QEMU does not implement  // that register ;)) and calculate 
+ our PCI MMIO based on that. This also makes it so  // the DSDT built by QEMU will have correct _CRS ranges.
+  // hw/pci-host/q35.c explicitly says our PCI hole ranges from [TOLUD, IO APIC].
+  // As far as I can tell, we seem to be allowed to add a 64-bit resource range anywhere.
+  //
+
+  MemBase = Tolud;
+  MemSize = PCI_MMIO_TOP_ADDRESS - MemBase;
+
+  //
+  // Set things up in the following way:
+  // ----------------------------------
+  //        Mem32-decoding-region
+  // ----------------------------------
+  //            ECAM/MMCONFIG
+  // ----------------------------------
+  //   Misc top regions, 64-bit memory
+  // ----------------------------------
+  //        Mem64-decoding-region
+  // ----------------------------------
+  //
+
+  if (PlatformData->HasPcie) {
+    ASSERT (FixedPcdGet64 (PcdPciExpressBaseAddress) < BASE_4GB);
+    EcamBase = (UINT32)FixedPcdGet64 (PcdPciExpressBaseAddress);
+
+    BuildResourceDescriptorHob (
+      EFI_RESOURCE_MEMORY_MAPPED_IO,
+      EFI_RESOURCE_ATTRIBUTE_PRESENT |
+      EFI_RESOURCE_ATTRIBUTE_INITIALIZED |
+      EFI_RESOURCE_ATTRIBUTE_UNCACHEABLE |
+      EFI_RESOURCE_ATTRIBUTE_TESTED,
+      EcamBase,
+      SIZE_256MB
+      );
+
+    BuildMemoryAllocationHob (
+      EcamBase,
+      SIZE_256MB,
+      EfiMemoryMappedIO
+      );
+
+    //
+    // Adjust the PCI MEM window
+    //
+    MemSize = EcamBase - MemBase;
+
+    DEBUG ((DEBUG_INFO, "PlatformInitPei: Using ECAM @ [%x, %x]\n", 
+ EcamBase, EcamBase + SIZE_256MB - 1));  }
+
+  PcdSetBoolS (PcdPciNoExtendedConfigSpace, !PlatformData->HasPcie);
+
+  PcdSet32S (PcdPciReservedMemBase, MemBase);  PcdSet32S 
+ (PcdPciReservedMemLimit, MemBase + MemSize - 1);
+
+  DEBUG ((DEBUG_INFO, "PlatformInitPei: PCI Mem window @ [%x, %x]\n", 
+ MemBase, MemBase + MemSize - 1));
+
+  //
+  // For simplicity, start the Mem64 region after every other  // valid 
+ non-hole memory region.
+  // Note: Unclear if this has any disadvantages.
+  //
+  Mem64Base = GetTopNonHoleAddr ();
+
+  if (Mem64Base < BASE_4GB) {
+    Mem64Base = BASE_4GB;
+  }
+
+  Mem64Size = SIZE_32GB;
+
+  PcdSet64S (PcdPciReservedMemAbove4GBBase, Mem64Base);  PcdSet64S 
+ (PcdPciReservedMemAbove4GBLimit, Mem64Base + Mem64Size - 1);
+
+  DEBUG ((DEBUG_INFO, "PlatformInitPei: PCI Mem64 window @ [%lx, 
+ %lx]\n", Mem64Base, Mem64Base + Mem64Size - 1));
+
+  if (PlatformData->SetupPci) {
+    EFI_STATUS  Status;
+    Status = PlatformData->SetupPci ();
+
+    if (EFI_ERROR (Status)) {
+      return Status;
+    }
+  }
+
+  return EFI_SUCCESS;
+}
+
+/**
+  Set up PCI on the q35 platform.
+
+  @retval EFI_SUCCESS   Success.
+**/
+STATIC
+EFI_STATUS
+PciSetupQ35 (
   VOID
   )
 {
-  UINTN  PciIoBase;
-  UINTN  PciIoSize;
-  UINTN  PciMmio32Base;
-  UINTN  PciMmio32Size;
+  UINT64  PciExBarBase;
+
+  PciExBarBase = PcdGet64 (PcdPciExpressBaseAddress);
 
   //
-  //  Setup PCI IO ranges for 440FX/PIIX4 platform
+  // Disable the ECAM before re-programming it.
   //
-  PciIoBase = PIIX4_PCI_IO_BASE;
-  PciIoSize = PIIX4_PCI_IO_SIZE;
-
-  PcdSet64S (PcdPciIoBase, PciIoBase);
-  PcdSet64S (PcdPciIoSize, PciIoSize);
+  PciWrite32 (DRAMC_REGISTER_Q35 (MCH_PCIEXBAR_LOW), 0);
 
   //
-  //  QEMU only allow a maximum of 2.8Gb of real memory below 4G
-  //  PCI MMIO range below 4Gb starts at the end of real memory below 4G
+  // Now, program the PCIe ECAM into the MCH PCIEXBAR register. We do 
+ it in a high-low order  // as to avoid temporary misdecoding of addresses.
   //
-  PciMmio32Base = (UINTN)GetMemoryBelow4Gb ();
 
-  if (PciMmio32Base == 0) {
-    DEBUG ((DEBUG_ERROR, "Unable to detect memory below 4Gb\n"));
-    ASSERT (PciMmio32Base != 0);
-    return EFI_UNSUPPORTED;
-  }
-
-  DEBUG ((DEBUG_ERROR, "Memory below 4Gb: %x \n", PciMmio32Base));
+  PciWrite32 (DRAMC_REGISTER_Q35 (MCH_PCIEXBAR_HIGH), 
+ (UINT32)(RShiftU64 (PciExBarBase, 32)));
 
   //
-  // Maximum size being PCI_MMIO_TOP_ADDRESS - TopOfLowMem to avoid overlapping with IO-APIC and other hardware mmio ranges
+  // Finally, program the low bits and simultaneously enable it.
   //
-  PciMmio32Size = PCI_MMIO_TOP_ADDRESS - PciMmio32Base;
-
-  PcdSet64S (PcdPciMmio32Base, PciMmio32Base);
-  PcdSet64S (PcdPciMmio32Size, PciMmio32Size);
+  PciWrite32 (
+    DRAMC_REGISTER_Q35 (MCH_PCIEXBAR_LOW),
+    (UINT32)PciExBarBase | MCH_PCIEXBAR_BUS_FF | MCH_PCIEXBAR_EN
+    );
 
   return EFI_SUCCESS;
 }
diff --git a/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Pcie.c b/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Pcie.c
deleted file mode 100644
index 0a5f0ff398de..000000000000
--- a/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Pcie.c
+++ /dev/null
@@ -1,106 +0,0 @@
-/** @file Pcie.c
-  PCI Express initialization for QEMU Q35
-
-  Copyright (c) 2022 Theo Jehl All rights reserved.
-  SPDX-License-Identifier: BSD-2-Clause-Patent -**/
-
-#include "PlatformInit.h"
-#include <IndustryStandard/Pci.h>
-#include <Library/PciCf8Lib.h>
-#include <IndustryStandard/Q35MchIch9.h> -#include <Library/QemuOpenFwCfgLib.h> -#include <IndustryStandard/QemuFwCfg.h> -#include <IndustryStandard/MemoryMappedConfigurationSpaceAccessTable.h>
-#include <Library/DebugLib.h>
-#include <IndustryStandard/Acpi30.h>
-#include <Library/MemoryAllocationLib.h> -#include <Library/PcdLib.h> -#include <Library/HobLib.h>
-
-/**
-  Initialize PCI Express support for QEMU Q35 system.
-  It also publishes PCI MMIO and IO ranges PCDs for OVMF PciHostBridgeLib.
-
-  @retval EFI_SUCCESS Initialization was successful -**/ -EFI_STATUS -EFIAPI -InitializePcie (
-  VOID
-  )
-{
-  UINTN  PciBase;
-  UINTN  PciSize;
-  UINTN  PciIoBase;
-  UINTN  PciIoSize;
-
-  union {
-    UINT64    Uint64;
-    UINT32    Uint32[2];
-  } PciExBarBase;
-
-  PciExBarBase.Uint64 = FixedPcdGet64 (PcdPciExpressBaseAddress);
-
-  //
-  // Build a reserved memory space for PCIE MMIO
-  //
-  BuildResourceDescriptorHob (
-    EFI_RESOURCE_MEMORY_RESERVED,
-    EFI_RESOURCE_ATTRIBUTE_PRESENT |
-    EFI_RESOURCE_ATTRIBUTE_INITIALIZED |
-    EFI_RESOURCE_ATTRIBUTE_UNCACHEABLE |
-    EFI_RESOURCE_ATTRIBUTE_TESTED,
-    PciExBarBase.Uint64,
-    SIZE_256MB
-    );
-
-  BuildMemoryAllocationHob (
-    PciExBarBase.Uint64,
-    SIZE_256MB,
-    EfiReservedMemoryType
-    );
-
-  //
-  // Clear lower 32 bits of register
-  //
-  PciWrite32 (DRAMC_REGISTER_Q35 (MCH_PCIEXBAR_LOW), 0);
-
-  //
-  // Program PCIE MMIO Base address in MCH PCIEXBAR register
-  //
-  PciWrite32 (DRAMC_REGISTER_Q35 (MCH_PCIEXBAR_HIGH), PciExBarBase.Uint32[1]);
-
-  //
-  // Enable 256Mb MMIO space
-  //
-  PciWrite32 (
-    DRAMC_REGISTER_Q35 (MCH_PCIEXBAR_LOW),
-    PciExBarBase.Uint32[0] | MCH_PCIEXBAR_BUS_FF | MCH_PCIEXBAR_EN
-    );
-
-  //
-  // Disable PCI/PCIe MMIO above 4Gb
-  //
-  PcdSet64S (PcdPciMmio64Size, 0);
-
-  //
-  // Set Pci MMIO space below 4GB
-  //
-  PciBase = (UINTN)(PcdGet64 (PcdPciExpressBaseAddress) + SIZE_256MB);
-  PciSize = PCI_MMIO_TOP_ADDRESS - PciBase;
-
-  PcdSet64S (PcdPciMmio32Base, PciBase);
-  PcdSet64S (PcdPciMmio32Size, PciSize);
-
-  //
-  // Set Pci IO port range
-  //
-  PciIoBase = Q35_PCI_IO_BASE;
-  PciIoSize = Q35_PCI_IO_SIZE;
-
-  PcdSet64S (PcdPciIoBase, PciIoBase);
-  PcdSet64S (PcdPciIoSize, PciIoSize);
-
-  return EFI_SUCCESS;
-}
diff --git a/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/PlatformInit.c b/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/PlatformInit.c
index 7849298b52d5..d23895afc967 100644
--- a/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/PlatformInit.c
+++ b/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/PlatformInit.c
@@ -2,6 +2,7 @@
   Platform initialization PEIM for QEMU
 
   Copyright (c) 2022 Theo Jehl All rights reserved.
+  Copyright (c) 2023 Pedro Falcato All rights reserved.
   SPDX-License-Identifier: BSD-2-Clause-Patent  **/
 
@@ -36,8 +37,6 @@ PlatformInit (
   if (EFI_ERROR (Status)) {
     DEBUG ((DEBUG_ERROR, "Memory installation failed\n"));
     return Status;
-  } else {
-    DEBUG ((DEBUG_INFO, "Memory installation success\n"));
   }
 
   //
@@ -48,7 +47,7 @@ PlatformInit (
   EfiPlatformInfo = AllocateZeroPool (sizeof (EFI_HOB_PLATFORM_INFO));
   if (EfiPlatformInfo == NULL) {
     DEBUG ((DEBUG_ERROR, "Failed to allocate pool for EFI_HOB_PLATFORM_INFO\n"));
-    return EFI_UNSUPPORTED;
+    return EFI_OUT_OF_RESOURCES;
   }
 
   //
@@ -56,20 +55,14 @@ PlatformInit (
   //
   DeviceId = PciCf8Read16 (PCI_CF8_LIB_ADDRESS (0, 0, 0, PCI_DEVICE_ID_OFFSET));
   DEBUG ((DEBUG_INFO, "Building gUefiOvmfPkgPlatformInfoGuid with Host bridge dev ID %x \n", DeviceId));
-  (*EfiPlatformInfo).HostBridgeDevId = DeviceId;
+  EfiPlatformInfo->HostBridgeDevId = DeviceId;
 
   BuildGuidDataHob (&gUefiOvmfPkgPlatformInfoGuid, EfiPlatformInfo, sizeof (EFI_HOB_PLATFORM_INFO));
 
   PcdSet16S (PcdOvmfHostBridgePciDevId, DeviceId);
 
   //
-  // Initialize PCI or PCIe based on current emulated system
+  // Finally, initialize PCI MMIO ranges and possibly the MMCONFIG
   //
-  if (DeviceId == INTEL_Q35_MCH_DEVICE_ID) {
-    DEBUG ((DEBUG_INFO, "Q35: Initialize PCIe\n"));
-    return InitializePcie ();
-  } else {
-    DEBUG ((DEBUG_INFO, "PIIX4: Initialize PCI\n"));
-    return InitializePciPIIX4 ();
-  }
+  return InitializePci (DeviceId == INTEL_Q35_MCH_DEVICE_ID ? 
+ QEMU_PLATFORM_Q35 : QEMU_PLATFORM_PIIX4);
 }
diff --git a/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/PlatformInit.h b/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/PlatformInit.h
index 771d2f958c40..f4044df3dbf5 100644
--- a/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/PlatformInit.h
+++ b/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/PlatformInit.h
@@ -2,6 +2,7 @@
   Headers for PlatformInitPei PEIM
 
   Copyright (c) 2022 Theo Jehl All rights reserved.
+  Copyright (c) 2023 Pedro Falcato All rights reserved.
   SPDX-License-Identifier: BSD-2-Clause-Patent  **/
 
@@ -38,16 +39,23 @@ InstallMemory (
   IN CONST EFI_PEI_SERVICES  **PeiServices
   );
 
-EFI_STATUS
-EFIAPI
-InitializePcie (
-  VOID
-  );
+typedef enum Platforms {
+  QEMU_PLATFORM_PIIX4 = 0,
+  QEMU_PLATFORM_Q35,
+  QEMU_PLATFORM_MAX
+} QEMU_PLATFORM;
+
+/**
+  Initialise PCI QEMU platforms.
+
+  Updates PCI IO port and MMIO range PCDs.
 
+  @retval EFI_SUCCESS      Initialization was a success.
+**/
 EFI_STATUS
 EFIAPI
-InitializePciPIIX4 (
-  VOID
+InitializePci (
+  QEMU_PLATFORM  Platform
   );
 
 EFI_STATUS
@@ -56,4 +64,14 @@ MaxCpuInit (
   VOID
   );
 
+/**
+  Return the largest reserved/DRAM/etc address.
+
+  @return Largest non-hole address.
+**/
+UINT64
+GetTopNonHoleAddr (
+  VOID
+  );
+
 #endif //QEMU_OPEN_BOARD_PKG_PLATFORM_INIT_H_
diff --git a/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/PlatformInitPei.inf b/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/PlatformInitPei.inf
index 43b1e13adfeb..5f42d46178f7 100644
--- a/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/PlatformInitPei.inf
+++ b/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/PlatformInitPei.inf
@@ -4,6 +4,7 @@
 #  Simple PEIM for QEMU PIIX4/Q35 Memory, SMP and PCI/PCI Express initialization  #  #  Copyright (c) 2022 Theo Jehl
+#  Copyright (c) 2023 Pedro Falcato All rights reserved.
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 
 [Defines]
@@ -17,14 +18,15 @@
 [Packages]
   OvmfPkg/OvmfPkg.dec
   MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
   QemuOpenBoardPkg/QemuOpenBoardPkg.dec
   UefiCpuPkg/UefiCpuPkg.dec
+  MinPlatformPkg/MinPlatformPkg.dec
 
 [Sources]
   PlatformInit.h
   PlatformInit.c
   Memory.c
-  Pcie.c
   Pci.c
   Cpu.c
 
@@ -43,13 +45,15 @@
   gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
   gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber
   gUefiCpuPkgTokenSpaceGuid.PcdCpuBootLogicalProcessorNumber
-  gUefiOvmfPkgTokenSpaceGuid.PcdPciIoBase
-  gUefiOvmfPkgTokenSpaceGuid.PcdPciIoSize
-  gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio32Base
-  gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio32Size
-  gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Base
-  gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Size
   gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
+  gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
+  gMinPlatformPkgTokenSpaceGuid.PcdPciReservedMemAbove4GBBase
+  gMinPlatformPkgTokenSpaceGuid.PcdPciReservedMemAbove4GBLimit
+  gMinPlatformPkgTokenSpaceGuid.PcdPciReservedMemBase
+  gMinPlatformPkgTokenSpaceGuid.PcdPciReservedMemLimit
+  gMinPlatformPkgTokenSpaceGuid.PcdPciReservedIobase
+  gMinPlatformPkgTokenSpaceGuid.PcdPciReservedIoLimit
+  gMinPlatformPkgTokenSpaceGuid.PcdPciNoExtendedConfigSpace
   gUefiOvmfPkgTokenSpaceGuid.PcdQ35TsegMbytes
 
 [FeaturePcd]
diff --git a/Platform/Qemu/QemuOpenBoardPkg/QemuOpenBoardPkg.dsc b/Platform/Qemu/QemuOpenBoardPkg/QemuOpenBoardPkg.dsc
index f5c317c83e6a..b2b82b84be4c 100644
--- a/Platform/Qemu/QemuOpenBoardPkg/QemuOpenBoardPkg.dsc
+++ b/Platform/Qemu/QemuOpenBoardPkg/QemuOpenBoardPkg.dsc
@@ -4,6 +4,7 @@
 #  Description file for QemuOpenBoardPkg  #  #  Copyright (c) 2022 Theo Jehl
+#  Copyright (c) 2023 Pedro Falcato All rights reserved.
 #  SPDX-License-Identifier: BSD-2-Clause-Patent  ##
 
@@ -78,9 +79,7 @@
   gQemuOpenBoardPkgTokenSpaceGuid.PcdDebugIoPort                        | 0x402
   gEfiMdePkgTokenSpaceGuid.PcdFSBClock                                  | 100000000
 
-  # PCIe base address for Q35 machines
-  # QEMU usable memory below 4G cannot exceed 2.8Gb
-  gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress                     | 0xB0000000
+  gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress                       | 0xE0000000
 
   gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvModeEnable             | TRUE
   gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange  | FALSE @@ -119,6 +118,18 @@
   gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber             | 0
   gUefiCpuPkgTokenSpaceGuid.PcdCpuBootLogicalProcessorNumber            | 0
 
+  # All of these MinPlatform PCI PCDs are filled in PlatformInitPei  # 
+ They must be dynamic since we don't know what platform (ICH9 or PIIX4) 
+ we're booting  # or how the memory map looks like.
+  gMinPlatformPkgTokenSpaceGuid.PcdPciReservedMemAbove4GBBase             | 0xffffffffffffffff
+  gMinPlatformPkgTokenSpaceGuid.PcdPciReservedMemAbove4GBLimit            | 0
+  gMinPlatformPkgTokenSpaceGuid.PcdPciReservedMemBase                     | 0x90000000
+  gMinPlatformPkgTokenSpaceGuid.PcdPciReservedMemLimit                    | 0
+  gMinPlatformPkgTokenSpaceGuid.PcdPciReservedIobase                      | 0x2000
+  gMinPlatformPkgTokenSpaceGuid.PcdPciReservedIoLimit                     | 0xffff
+
+  gMinPlatformPkgTokenSpaceGuid.PcdPciNoExtendedConfigSpace               | TRUE
+
   !if $(SMM_REQUIRED) == TRUE
     gUefiOvmfPkgTokenSpaceGuid.PcdQ35TsegMbytes                         | 8
     gUefiOvmfPkgTokenSpaceGuid.PcdQ35SmramAtDefaultSmbase               | FALSE
--
2.39.0


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

* Re: [PATCH edk2-platforms 2/2] QemuOpenBoardPkg: Trivial code cleanup
  2023-01-12 23:13 ` [PATCH edk2-platforms 2/2] QemuOpenBoardPkg: Trivial code cleanup Pedro Falcato
@ 2023-01-13  1:48   ` Isaac Oram
  2023-01-13 12:58   ` Théo Jehl
  1 sibling, 0 replies; 10+ messages in thread
From: Isaac Oram @ 2023-01-13  1:48 UTC (permalink / raw)
  To: Pedro Falcato, devel@edk2.groups.io; +Cc: Theo Jehl

Reviewed-by: Isaac Oram <isaac.w.oram@intel.com>

-----Original Message-----
From: Pedro Falcato <pedro.falcato@gmail.com> 
Sent: Thursday, January 12, 2023 3:14 PM
To: devel@edk2.groups.io
Cc: Pedro Falcato <pedro.falcato@gmail.com>; Oram, Isaac W <isaac.w.oram@intel.com>; Theo Jehl <theojehl76@gmail.com>
Subject: [PATCH edk2-platforms 2/2] QemuOpenBoardPkg: Trivial code cleanup

Small cleanups around PlatformInitPei.

Signed-off-by: Pedro Falcato <pedro.falcato@gmail.com>
Cc: Isaac Oram <isaac.w.oram@intel.com>
Cc: Theo Jehl <theojehl76@gmail.com>
---
 .../QemuOpenBoardPkg/PlatformInitPei/Cpu.c    |  2 +-
 .../QemuOpenBoardPkg/PlatformInitPei/Memory.c |  9 ++----
 .../PlatformInitPei/PlatformInit.h            | 28 ++++++++++++++-----
 3 files changed, 25 insertions(+), 14 deletions(-)

diff --git a/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Cpu.c b/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Cpu.c
index e203b2654226..2fc62a0a3e77 100644
--- a/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Cpu.c
+++ b/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Cpu.c
@@ -20,7 +20,7 @@
 /**
   Probe Qemu FW CFG device for current CPU count and report to MpInitLib.
 
-  @return EFI_SUCCESS      Detection was successful.
+  @retval EFI_SUCCESS      Detection was successful.
   @retval EFI_UNSUPPORTED  QEMU FW CFG device is not present.
  */
 EFI_STATUS
diff --git a/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Memory.c b/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Memory.c
index 4f312c36016e..223cace0ca98 100644
--- a/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Memory.c
+++ b/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Memory.c
@@ -86,8 +86,8 @@ GetTopNonHoleAddr (
 STATIC
 VOID
 ReserveMmioRegion (
-  EFI_PHYSICAL_ADDRESS  Start,
-  UINT64                Length
+  IN EFI_PHYSICAL_ADDRESS  Start,
+  IN UINT64                Length
   )
 {
   EFI_RESOURCE_TYPE            ResourceType;
@@ -121,7 +121,6 @@ InstallMemory (
   )
 {
   EFI_STATUS                   Status;
-  CONST EFI_PEI_SERVICES       **PeiServicesTable;
   EFI_E820_ENTRY64             E820Entry;
   EFI_E820_ENTRY64             LargestE820Entry;
   QEMU_FW_CFG_FILE             FwCfgFile;
@@ -250,9 +249,7 @@ InstallMemory (
     LargestE820Entry.BaseAddr + LargestE820Entry.Length - 1
     ));
 
-  PeiServicesTable = GetPeiServicesTablePointer ();
-
-  Status = (*PeiServices)->InstallPeiMemory (PeiServicesTable, LargestE820Entry.BaseAddr, LargestE820Entry.Length);
+  Status = (*PeiServices)->InstallPeiMemory (PeiServices, LargestE820Entry.BaseAddr, LargestE820Entry.Length);
 
   ASSERT_EFI_ERROR (Status);
 
diff --git a/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/PlatformInit.h b/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/PlatformInit.h
index f4044df3dbf5..f17df707188a 100644
--- a/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/PlatformInit.h
+++ b/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/PlatformInit.h
@@ -20,19 +20,27 @@
 
 #define PCI_MMIO_TOP_ADDRESS  0xFC000000
 
-EFI_STATUS
-EFIAPI
-PlatformInit (
-  IN       EFI_PEI_FILE_HANDLE  FileHandle,
-  IN CONST EFI_PEI_SERVICES     **PeiServices
-  );
+/**
+  Return the memory size below 4GB.
 
+  @return Size of memory below 4GB, in bytes.
+**/
 UINT32
 EFIAPI
 GetMemoryBelow4Gb (
   VOID
   );
 
+/**
+  Install EFI memory by probing QEMU FW CFG devices for valid E820 entries.
+  It also reserves space for MMIO regions such as VGA, BIOS and APIC.
+
+  @param[in] PeiServices      PEI Services pointer.
+
+  @retval EFI_SUCCESS     Memory initialization succeded.
+  @retval EFI_UNSUPPORTED Installation failed (etc/e820 file was not found).
+  @retval EFI_NOT_FOUND   QEMU FW CFG device is not present.
+**/
 EFI_STATUS
 EFIAPI
 InstallMemory (
@@ -58,6 +66,12 @@ InitializePci (
   QEMU_PLATFORM  Platform
   );
 
+/**
+  Probe Qemu FW CFG device for current CPU count and report to MpInitLib.
+
+  @retval EFI_SUCCESS      Detection was successful.
+  @retval EFI_UNSUPPORTED  QEMU FW CFG device is not present.
+ */
 EFI_STATUS
 EFIAPI
 MaxCpuInit (
@@ -74,4 +88,4 @@ GetTopNonHoleAddr (
   VOID
   );
 
-#endif //QEMU_OPEN_BOARD_PKG_PLATFORM_INIT_H_
+#endif // QEMU_OPEN_BOARD_PKG_PLATFORM_INIT_H_
-- 
2.39.0


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

* Re: [PATCH edk2-platforms 1/2] QemuOpenBoardPkg: Redo PCI bus initialization
  2023-01-12 23:13 ` [PATCH edk2-platforms 1/2] QemuOpenBoardPkg: Redo PCI bus initialization Pedro Falcato
  2023-01-13  1:46   ` Isaac Oram
@ 2023-01-13  6:33   ` Gerd Hoffmann
  2023-01-30 11:27     ` Pedro Falcato
  1 sibling, 1 reply; 10+ messages in thread
From: Gerd Hoffmann @ 2023-01-13  6:33 UTC (permalink / raw)
  To: Pedro Falcato; +Cc: devel, Isaac Oram, Theo Jehl

> diff --git a/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Memory.c b/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Memory.c
> index 21705256191b..4f312c36016e 100644
> --- a/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Memory.c
> +++ b/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Memory.c

There is OvmfPkg/Library/PlatformInitLib which you could use instead of
reinventing the wheel ...

If there are changes needed to make PlatformInitLib work for you feel
free to propose patches (same goes for eventually moving code from
OvmfPkg/PlatformPei to the Library).

> +  // It's worth noting that QEMU also grew an option to change lowmem based on the
> +  // user's preferences. Because of all of this, it's near impossible to hardcode
> +  // a range, so we grab TOLUD (not in a literal way, since QEMU does not implement
> +  // that register ;)) and calculate our PCI MMIO based on that. This also makes it so
> +  // the DSDT built by QEMU will have correct _CRS ranges.
> +  // hw/pci-host/q35.c explicitly says our PCI hole ranges from [TOLUD, IO APIC].
> +  // As far as I can tell, we seem to be allowed to add a 64-bit resource range anywhere.

There is a etc/reserved-memory-end fw_cfg FwCfg file.  If present it
the 64-bit resource range should be placed above the address specified
there (qemu uses that to reserve address space if needed, happens for
example when you enable memory hotplug).

The patch should be splitted up into smaller pieces to make it easier
to review the changes.

take care,
  Gerd


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

* Re: [PATCH edk2-platforms 2/2] QemuOpenBoardPkg: Trivial code cleanup
  2023-01-12 23:13 ` [PATCH edk2-platforms 2/2] QemuOpenBoardPkg: Trivial code cleanup Pedro Falcato
  2023-01-13  1:48   ` Isaac Oram
@ 2023-01-13 12:58   ` Théo Jehl
  1 sibling, 0 replies; 10+ messages in thread
From: Théo Jehl @ 2023-01-13 12:58 UTC (permalink / raw)
  To: Pedro Falcato; +Cc: devel, Isaac Oram

[-- Attachment #1: Type: text/plain, Size: 4306 bytes --]

Reviewed-by: Theo Jehl <theojehl76@gmail.com>

Le ven. 13 janv. 2023 à 00:14, Pedro Falcato <pedro.falcato@gmail.com> a
écrit :

> Small cleanups around PlatformInitPei.
>
> Signed-off-by: Pedro Falcato <pedro.falcato@gmail.com>
> Cc: Isaac Oram <isaac.w.oram@intel.com>
> Cc: Theo Jehl <theojehl76@gmail.com>
> ---
>  .../QemuOpenBoardPkg/PlatformInitPei/Cpu.c    |  2 +-
>  .../QemuOpenBoardPkg/PlatformInitPei/Memory.c |  9 ++----
>  .../PlatformInitPei/PlatformInit.h            | 28 ++++++++++++++-----
>  3 files changed, 25 insertions(+), 14 deletions(-)
>
> diff --git a/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Cpu.c
> b/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Cpu.c
> index e203b2654226..2fc62a0a3e77 100644
> --- a/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Cpu.c
> +++ b/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Cpu.c
> @@ -20,7 +20,7 @@
>  /**
>    Probe Qemu FW CFG device for current CPU count and report to MpInitLib.
>
> -  @return EFI_SUCCESS      Detection was successful.
> +  @retval EFI_SUCCESS      Detection was successful.
>    @retval EFI_UNSUPPORTED  QEMU FW CFG device is not present.
>   */
>  EFI_STATUS
> diff --git a/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Memory.c
> b/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Memory.c
> index 4f312c36016e..223cace0ca98 100644
> --- a/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Memory.c
> +++ b/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Memory.c
> @@ -86,8 +86,8 @@ GetTopNonHoleAddr (
>  STATIC
>  VOID
>  ReserveMmioRegion (
> -  EFI_PHYSICAL_ADDRESS  Start,
> -  UINT64                Length
> +  IN EFI_PHYSICAL_ADDRESS  Start,
> +  IN UINT64                Length
>    )
>  {
>    EFI_RESOURCE_TYPE            ResourceType;
> @@ -121,7 +121,6 @@ InstallMemory (
>    )
>  {
>    EFI_STATUS                   Status;
> -  CONST EFI_PEI_SERVICES       **PeiServicesTable;
>    EFI_E820_ENTRY64             E820Entry;
>    EFI_E820_ENTRY64             LargestE820Entry;
>    QEMU_FW_CFG_FILE             FwCfgFile;
> @@ -250,9 +249,7 @@ InstallMemory (
>      LargestE820Entry.BaseAddr + LargestE820Entry.Length - 1
>      ));
>
> -  PeiServicesTable = GetPeiServicesTablePointer ();
> -
> -  Status = (*PeiServices)->InstallPeiMemory (PeiServicesTable,
> LargestE820Entry.BaseAddr, LargestE820Entry.Length);
> +  Status = (*PeiServices)->InstallPeiMemory (PeiServices,
> LargestE820Entry.BaseAddr, LargestE820Entry.Length);
>
>    ASSERT_EFI_ERROR (Status);
>
> diff --git a/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/PlatformInit.h
> b/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/PlatformInit.h
> index f4044df3dbf5..f17df707188a 100644
> --- a/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/PlatformInit.h
> +++ b/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/PlatformInit.h
> @@ -20,19 +20,27 @@
>
>  #define PCI_MMIO_TOP_ADDRESS  0xFC000000
>
> -EFI_STATUS
> -EFIAPI
> -PlatformInit (
> -  IN       EFI_PEI_FILE_HANDLE  FileHandle,
> -  IN CONST EFI_PEI_SERVICES     **PeiServices
> -  );
> +/**
> +  Return the memory size below 4GB.
>
> +  @return Size of memory below 4GB, in bytes.
> +**/
>  UINT32
>  EFIAPI
>  GetMemoryBelow4Gb (
>    VOID
>    );
>
> +/**
> +  Install EFI memory by probing QEMU FW CFG devices for valid E820
> entries.
> +  It also reserves space for MMIO regions such as VGA, BIOS and APIC.
> +
> +  @param[in] PeiServices      PEI Services pointer.
> +
> +  @retval EFI_SUCCESS     Memory initialization succeded.
> +  @retval EFI_UNSUPPORTED Installation failed (etc/e820 file was not
> found).
> +  @retval EFI_NOT_FOUND   QEMU FW CFG device is not present.
> +**/
>  EFI_STATUS
>  EFIAPI
>  InstallMemory (
> @@ -58,6 +66,12 @@ InitializePci (
>    QEMU_PLATFORM  Platform
>    );
>
> +/**
> +  Probe Qemu FW CFG device for current CPU count and report to MpInitLib.
> +
> +  @retval EFI_SUCCESS      Detection was successful.
> +  @retval EFI_UNSUPPORTED  QEMU FW CFG device is not present.
> + */
>  EFI_STATUS
>  EFIAPI
>  MaxCpuInit (
> @@ -74,4 +88,4 @@ GetTopNonHoleAddr (
>    VOID
>    );
>
> -#endif //QEMU_OPEN_BOARD_PKG_PLATFORM_INIT_H_
> +#endif // QEMU_OPEN_BOARD_PKG_PLATFORM_INIT_H_
> --
> 2.39.0
>
>

[-- Attachment #2: Type: text/html, Size: 5308 bytes --]

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

* Re: [PATCH edk2-platforms 1/2] QemuOpenBoardPkg: Redo PCI bus initialization
  2023-01-13  6:33   ` Gerd Hoffmann
@ 2023-01-30 11:27     ` Pedro Falcato
  2023-03-09  3:48       ` [edk2-devel] " Isaac Oram
  0 siblings, 1 reply; 10+ messages in thread
From: Pedro Falcato @ 2023-01-30 11:27 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: devel, Isaac Oram, Theo Jehl

On Fri, Jan 13, 2023 at 6:34 AM Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> > diff --git a/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Memory.c b/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Memory.c
> > index 21705256191b..4f312c36016e 100644
> > --- a/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Memory.c
> > +++ b/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Memory.c
>
> There is OvmfPkg/Library/PlatformInitLib which you could use instead of
> reinventing the wheel ...
>
> If there are changes needed to make PlatformInitLib work for you feel
> free to propose patches (same goes for eventually moving code from
> OvmfPkg/PlatformPei to the Library).

Gerd,

Thank you for the comments.

That is an interesting idea, however I believe
OvmfPkg/Library/PlatformInitLib is ATM a bit too tightly coupled with
the rest of OVMF itself.
For instance, this work was partially inspired by your recent-ish work
in similar OvmfPkg code and also drove me to remove the dependency on
PciHostBridgeLib, et al (replacing it with the "standard"
MinPlatformPkg PCI libs). My solution is based on dynamic PCD
war-crimes but it Just Works(tm).
Changing it in OvmfPkg would be too invasive, I feel; it's not like my
solution is good or anything.

And because of the way the repos are split, there is no way to
currently dedup OVMF's PciHostBridgeLib by also making OVMF depend on
MinPlatformPkg's, because why would there be, right?

Anyway, doing deep changes to OVMF for QemuOpenBoardPkg is
uncomfortable for me because its place in Tiano is not defined. OVMF
and Intel folks have said very few things about QOBP.
At the moment, it's the typical "GSoC project stagnating in
edk2-platforms/-staging" that Tianocore seems to love so much. I don't
see much point in it being this way.
QemuOBP has been a nice fun exercise in reducing code duplication,
which shows a lot of promise, but it also doesn't support some of the
things OvmfPkg does, like PEI-less booting (and TDX/SEV but those
could certainly be worked in).

If this kicks off discussion on the future of QemuOpenBoardPkg, I'll be happy.

Until that happens, I am reluctant to change OVMF internals for an
obscure QEMU PlatformPkg hidden in the best worst repo, edk2-platforms
:)

> > +  // It's worth noting that QEMU also grew an option to change lowmem based on the
> > +  // user's preferences. Because of all of this, it's near impossible to hardcode
> > +  // a range, so we grab TOLUD (not in a literal way, since QEMU does not implement
> > +  // that register ;)) and calculate our PCI MMIO based on that. This also makes it so
> > +  // the DSDT built by QEMU will have correct _CRS ranges.
> > +  // hw/pci-host/q35.c explicitly says our PCI hole ranges from [TOLUD, IO APIC].
> > +  // As far as I can tell, we seem to be allowed to add a 64-bit resource range anywhere.
>
> There is a etc/reserved-memory-end fw_cfg FwCfg file.  If present it
> the 64-bit resource range should be placed above the address specified
> there (qemu uses that to reserve address space if needed, happens for
> example when you enable memory hotplug).

ACK, will change on v2 (together with Isaac's comments).

> The patch should be splitted up into smaller pieces to make it easier
> to review the changes.

Agreed, will try to devise a strategy to break it up in smaller, logical chunks.
Again, thank you so much for your time looking at something you're not
even a maintainer for :))

-- 
Pedro

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

* Re: [edk2-devel] [PATCH edk2-platforms 1/2] QemuOpenBoardPkg: Redo PCI bus initialization
  2023-01-30 11:27     ` Pedro Falcato
@ 2023-03-09  3:48       ` Isaac Oram
  2023-05-08 23:48         ` Pedro Falcato
  0 siblings, 1 reply; 10+ messages in thread
From: Isaac Oram @ 2023-03-09  3:48 UTC (permalink / raw)
  To: devel@edk2.groups.io, pedro.falcato@gmail.com, Gerd Hoffmann; +Cc: Theo Jehl

We are doing some work to enable CI on edk2-platforms and would like to add QemuOpenBoardPkg to the CI list.  I do think that this will become more active over time.

Is there a V2 coming?  If so, I found a compiler issue with VS2019 and VS2015.
It doesn't like the local variable PlatformData having the same name as the global variable PlatformData.
I would suggest changing the global to mPlatformData.

Regards,
Isaac

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Pedro Falcato
Sent: Monday, January 30, 2023 3:28 AM
To: Gerd Hoffmann <kraxel@redhat.com>
Cc: devel@edk2.groups.io; Oram, Isaac W <isaac.w.oram@intel.com>; Theo Jehl <theojehl76@gmail.com>
Subject: Re: [edk2-devel] [PATCH edk2-platforms 1/2] QemuOpenBoardPkg: Redo PCI bus initialization

On Fri, Jan 13, 2023 at 6:34 AM Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> > diff --git a/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Memory.c 
> > b/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Memory.c
> > index 21705256191b..4f312c36016e 100644
> > --- a/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Memory.c
> > +++ b/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Memory.c
>
> There is OvmfPkg/Library/PlatformInitLib which you could use instead 
> of reinventing the wheel ...
>
> If there are changes needed to make PlatformInitLib work for you feel 
> free to propose patches (same goes for eventually moving code from 
> OvmfPkg/PlatformPei to the Library).

Gerd,

Thank you for the comments.

That is an interesting idea, however I believe OvmfPkg/Library/PlatformInitLib is ATM a bit too tightly coupled with the rest of OVMF itself.
For instance, this work was partially inspired by your recent-ish work in similar OvmfPkg code and also drove me to remove the dependency on PciHostBridgeLib, et al (replacing it with the "standard"
MinPlatformPkg PCI libs). My solution is based on dynamic PCD war-crimes but it Just Works(tm).
Changing it in OvmfPkg would be too invasive, I feel; it's not like my solution is good or anything.

And because of the way the repos are split, there is no way to currently dedup OVMF's PciHostBridgeLib by also making OVMF depend on MinPlatformPkg's, because why would there be, right?

Anyway, doing deep changes to OVMF for QemuOpenBoardPkg is uncomfortable for me because its place in Tiano is not defined. OVMF and Intel folks have said very few things about QOBP.
At the moment, it's the typical "GSoC project stagnating in edk2-platforms/-staging" that Tianocore seems to love so much. I don't see much point in it being this way.
QemuOBP has been a nice fun exercise in reducing code duplication, which shows a lot of promise, but it also doesn't support some of the things OvmfPkg does, like PEI-less booting (and TDX/SEV but those could certainly be worked in).

If this kicks off discussion on the future of QemuOpenBoardPkg, I'll be happy.

Until that happens, I am reluctant to change OVMF internals for an obscure QEMU PlatformPkg hidden in the best worst repo, edk2-platforms
:)

> > +  // It's worth noting that QEMU also grew an option to change 
> > + lowmem based on the  // user's preferences. Because of all of 
> > + this, it's near impossible to hardcode  // a range, so we grab 
> > + TOLUD (not in a literal way, since QEMU does not implement  // 
> > + that register ;)) and calculate our PCI MMIO based on that. This also makes it so  // the DSDT built by QEMU will have correct _CRS ranges.
> > +  // hw/pci-host/q35.c explicitly says our PCI hole ranges from [TOLUD, IO APIC].
> > +  // As far as I can tell, we seem to be allowed to add a 64-bit resource range anywhere.
>
> There is a etc/reserved-memory-end fw_cfg FwCfg file.  If present it 
> the 64-bit resource range should be placed above the address specified 
> there (qemu uses that to reserve address space if needed, happens for 
> example when you enable memory hotplug).

ACK, will change on v2 (together with Isaac's comments).

> The patch should be splitted up into smaller pieces to make it easier 
> to review the changes.

Agreed, will try to devise a strategy to break it up in smaller, logical chunks.
Again, thank you so much for your time looking at something you're not even a maintainer for :))

--
Pedro






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

* Re: [edk2-devel] [PATCH edk2-platforms 1/2] QemuOpenBoardPkg: Redo PCI bus initialization
  2023-03-09  3:48       ` [edk2-devel] " Isaac Oram
@ 2023-05-08 23:48         ` Pedro Falcato
  0 siblings, 0 replies; 10+ messages in thread
From: Pedro Falcato @ 2023-05-08 23:48 UTC (permalink / raw)
  To: devel, isaac.w.oram; +Cc: Gerd Hoffmann, Theo Jehl

On Thu, Mar 9, 2023 at 3:48 AM Isaac Oram <isaac.w.oram@intel.com> wrote:
>
> We are doing some work to enable CI on edk2-platforms and would like to add QemuOpenBoardPkg to the CI list.  I do think that this will become more active over time.
>
> Is there a V2 coming?  If so, I found a compiler issue with VS2019 and VS2015.
> It doesn't like the local variable PlatformData having the same name as the global variable PlatformData.
> I would suggest changing the global to mPlatformData.

Hi!

Really sorry for ghosting you here, I've been busy lately and this
email slipped through my inbox's cracks :)

Yes, I'll try and spin up a V2 in the coming weeks if I can. Thank you
for spotting the issue with VS, I'll take a look.
Also, any update on CI?

-- 
Pedro

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

end of thread, other threads:[~2023-05-08 23:48 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-01-12 23:13 [PATCH edk2-platforms 0/2] QemuOpenBoardPkg: PCI rework and small fixes Pedro Falcato
2023-01-12 23:13 ` [PATCH edk2-platforms 1/2] QemuOpenBoardPkg: Redo PCI bus initialization Pedro Falcato
2023-01-13  1:46   ` Isaac Oram
2023-01-13  6:33   ` Gerd Hoffmann
2023-01-30 11:27     ` Pedro Falcato
2023-03-09  3:48       ` [edk2-devel] " Isaac Oram
2023-05-08 23:48         ` Pedro Falcato
2023-01-12 23:13 ` [PATCH edk2-platforms 2/2] QemuOpenBoardPkg: Trivial code cleanup Pedro Falcato
2023-01-13  1:48   ` Isaac Oram
2023-01-13 12:58   ` Théo Jehl

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