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.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))
indeed
>
> but I like better your solution of using the final class types, so:
>
> Reviewed-by: Gustavo Romero <gustavo.rom...@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;
>> }
>
