Hi Eric,

On 5/14/25 14:01, Eric Auger wrote:
pcihp acpi_set_pci_info() generic code currently uses
acpi_get_i386_pci_host() to retrieve the pci host bridge.

Let's rename acpi_get_i386_pci_host into acpi_get_pci_host and
move it in pci generic code.

The helper is augmented with the support of ARM GPEX.

Signed-off-by: Eric Auger <eric.au...@redhat.com>
---
  include/hw/acpi/pci.h |  2 ++
  hw/acpi/pci.c         | 20 ++++++++++++++++++++
  hw/acpi/pcihp.c       |  3 ++-
  hw/i386/acpi-build.c  | 24 ++++--------------------
  4 files changed, 28 insertions(+), 21 deletions(-)

diff --git a/include/hw/acpi/pci.h b/include/hw/acpi/pci.h
index 4dca22c0e2..310cbd02db 100644
--- a/include/hw/acpi/pci.h
+++ b/include/hw/acpi/pci.h
@@ -41,4 +41,6 @@ void build_pci_bridge_aml(AcpiDevAmlIf *adev, Aml *scope);
void build_srat_generic_affinity_structures(GArray *table_data); +Object *acpi_get_pci_host(void);
+
  #endif
diff --git a/hw/acpi/pci.c b/hw/acpi/pci.c
index d511a85029..4191886ebe 100644
--- a/hw/acpi/pci.c
+++ b/hw/acpi/pci.c
@@ -26,6 +26,7 @@
  #include "qemu/osdep.h"
  #include "qemu/error-report.h"
  #include "qom/object_interfaces.h"
+#include "qom/object.h"
  #include "qapi/error.h"
  #include "hw/boards.h"
  #include "hw/acpi/aml-build.h"
@@ -33,6 +34,9 @@
  #include "hw/pci/pci_bridge.h"
  #include "hw/pci/pci_device.h"
  #include "hw/pci/pcie_host.h"
+#include "hw/pci-host/i440fx.h"
+#include "hw/pci-host/q35.h"
+#include "hw/pci-host/gpex.h"
/*
   * PCI Firmware Specification, Revision 3.0
@@ -301,3 +305,19 @@ void build_srat_generic_affinity_structures(GArray 
*table_data)
      object_child_foreach_recursive(object_get_root(), build_acpi_generic_port,
                                     table_data);
  }
+
+Object *acpi_get_pci_host(void)
+{
+    Object *host;
+
+    host = object_resolve_type_unambiguous(TYPE_I440FX_PCI_HOST_BRIDGE, NULL);
+    if (host) {
+        return host;
+    }
+    host = object_resolve_type_unambiguous(TYPE_Q35_HOST_DEVICE, NULL);
+    if (host) {
+        return host;
+    }
+    host = object_resolve_type_unambiguous(TYPE_GPEX_HOST, NULL);
+    return host;
+}
diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
index 942669ea89..d800371ddc 100644
--- a/hw/acpi/pcihp.c
+++ b/hw/acpi/pcihp.c
@@ -36,6 +36,7 @@
  #include "hw/pci-bridge/xio3130_downstream.h"
  #include "hw/i386/acpi-build.h"
  #include "hw/acpi/acpi.h"
+#include "hw/acpi/pci.h"
  #include "hw/pci/pci_bus.h"
  #include "migration/vmstate.h"
  #include "qapi/error.h"
@@ -102,7 +103,7 @@ static void *acpi_set_bsel(PCIBus *bus, void *opaque)
  static void acpi_set_pci_info(bool has_bridge_hotplug)
  {
      static bool bsel_is_set;
-    Object *host = acpi_get_i386_pci_host();
+    Object *host = acpi_get_pci_host();
      PCIBus *bus;
      BSELInfo info = { .bsel_alloc = ACPI_PCIHP_BSEL_DEFAULT,
                        .has_bridge_hotplug = has_bridge_hotplug };
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 06b4b8eed4..bcfba2ccb3 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -269,27 +269,11 @@ static void acpi_get_misc_info(AcpiMiscInfo *info)
  #endif
  }
-/*
- * Because of the PXB hosts we cannot simply query TYPE_PCI_HOST_BRIDGE.
- * On i386 arch we only have two pci hosts, so we can look only for them.
- */
-Object *acpi_get_i386_pci_host(void)
-{
-    PCIHostState *host;
-
-    host = PCI_HOST_BRIDGE(object_resolve_path("/machine/i440fx", NULL));
-    if (!host) {
-        host = PCI_HOST_BRIDGE(object_resolve_path("/machine/q35", NULL));
-    }
-
-    return OBJECT(host);
-}
-

It's also possible to add a child prop to the TYPE_GPEX_HOST in create_pcie, 
like:

@@ -1510,6 +1510,7 @@ static void create_pcie(VirtMachineState *vms)
     MachineClass *mc = MACHINE_GET_CLASS(ms);
dev = qdev_new(TYPE_GPEX_HOST);
+    object_property_add_child(OBJECT(vms), "gpex", OBJECT(dev));

then follow to logic in acpi_get_i386_pci_host via:

PCI_HOST_BRIDGE(object_resolve_path("/machine/gpex", NULL))

but I like better your solution of using the final class types, so:

Reviewed-by: Gustavo Romero <gustavo.rom...@linaro.org>


Cheers,
Gustavo

  static void acpi_get_pci_holes(Range *hole, Range *hole64)
  {
      Object *pci_host;
- pci_host = acpi_get_i386_pci_host();
+    pci_host = acpi_get_pci_host();
if (!pci_host) {
          return;
@@ -1245,7 +1229,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
sb_scope = aml_scope("\\_SB");
      {
-        Object *pci_host = acpi_get_i386_pci_host();
+        Object *pci_host = acpi_get_pci_host();
if (pci_host) {
              PCIBus *pbus = PCI_HOST_BRIDGE(pci_host)->bus;
@@ -1306,7 +1290,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
      if (pm->pcihp_bridge_en || pm->pcihp_root_en) {
          bool has_pcnt;
- Object *pci_host = acpi_get_i386_pci_host();
+        Object *pci_host = acpi_get_pci_host();
          PCIBus *b = PCI_HOST_BRIDGE(pci_host)->bus;
scope = aml_scope("\\_SB.PCI0");
@@ -1946,7 +1930,7 @@ static bool acpi_get_mcfg(AcpiMcfgInfo *mcfg)
      Object *pci_host;
      QObject *o;
- pci_host = acpi_get_i386_pci_host();
+    pci_host = acpi_get_pci_host();
      if (!pci_host) {
          return false;
      }


Reply via email to