[PATCH 13/22] hw/i386/acpi-build: Introduce and use acpi_get_pci_host

Eric Auger posted 22 patches 6 months ago
There is a newer version of this series
[PATCH 13/22] hw/i386/acpi-build: Introduce and use acpi_get_pci_host
Posted by Eric Auger 6 months ago
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.auger@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);
-}
-
 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;
     }
-- 
2.49.0
Re: [PATCH 13/22] hw/i386/acpi-build: Introduce and use acpi_get_pci_host
Posted by Gustavo Romero 5 months, 4 weeks ago
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.auger@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.romero@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;
>       }
Re: [PATCH 13/22] hw/i386/acpi-build: Introduce and use acpi_get_pci_host
Posted by Eric Auger 5 months, 3 weeks ago

On 5/20/25 10:00 PM, Gustavo Romero wrote:
> 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.auger@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))
indeed
>
> but I like better your solution of using the final class types, so:
>
> Reviewed-by: Gustavo Romero <gustavo.romero@linaro.org>

OK thanks! I described that change in the commit msg because it was
effectively not properly described.

Cheers

Eric
>
>
> 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;
>>       }
>