public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Isaac Oram" <isaac.w.oram@intel.com>
To: "devel@edk2.groups.io" <devel@edk2.groups.io>
Cc: Theo Jehl <theojehl76@gmail.com>, Gerd Hoffmann <kraxel@redhat.com>
Subject: Re: [PATCH edk2-platforms 1/2] QemuOpenBoardPkg: Redo PCI bus initialization
Date: Fri, 13 Jan 2023 01:46:37 +0000	[thread overview]
Message-ID: <SA1PR11MB5801AC539FD5BAB510FB071CD0C29@SA1PR11MB5801.namprd11.prod.outlook.com> (raw)
In-Reply-To: <20230112231359.452800-2-pedro.falcato@gmail.com>

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


  reply	other threads:[~2023-01-13  1:47 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=SA1PR11MB5801AC539FD5BAB510FB071CD0C29@SA1PR11MB5801.namprd11.prod.outlook.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox