I would add a description: we want to scan all functions not just function 0 to describe hotplug into bridges at function != 0. in preparation for this, refactor code to not skip functions != 0.
On Thu, Jul 22, 2021 at 06:59:44AM -0400, Igor Mammedov wrote: > Signed-off-by: Igor Mammedov <imamm...@redhat.com> > --- > hw/i386/acpi-build.c | 13 ++++++------- > 1 file changed, 6 insertions(+), 7 deletions(-) > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > index 17836149fe..b40e284b72 100644 > --- a/hw/i386/acpi-build.c > +++ b/hw/i386/acpi-build.c > @@ -374,7 +374,7 @@ static void build_append_pci_bus_devices(Aml > *parent_scope, PCIBus *bus, > Aml *dev, *notify_method = NULL, *method; > QObject *bsel; > PCIBus *sec; > - int i; > + int devfn; > > bsel = object_property_get_qobject(OBJECT(bus), ACPI_PCIHP_PROP_BSEL, > NULL); > if (bsel) { > @@ -384,11 +384,11 @@ static void build_append_pci_bus_devices(Aml > *parent_scope, PCIBus *bus, > notify_method = aml_method("DVNT", 2, AML_NOTSERIALIZED); > } > > - for (i = 0; i < ARRAY_SIZE(bus->devices); i += PCI_FUNC_MAX) { > + for (devfn = 0; devfn < ARRAY_SIZE(bus->devices); devfn++) { > DeviceClass *dc; > PCIDeviceClass *pc; > - PCIDevice *pdev = bus->devices[i]; > - int slot = PCI_SLOT(i); > + PCIDevice *pdev = bus->devices[devfn]; > + int slot = PCI_SLOT(devfn); > bool hotplug_enabled_dev; > bool bridge_in_acpi; > bool cold_plugged_bridge; I am a bit puzzled about why this is equivalent. so we used to scan just function 0 on each slot. now we are scanning them all. won't this generate a different AML code? in fact duplicate descriptions? I suspect you need to move the check for slot == 0 from the next patch to this one otherwise bisect will be broken. Or just squash this part into next patch. up to you. > @@ -525,13 +525,12 @@ static void build_append_pci_bus_devices(Aml > *parent_scope, PCIBus *bus, > /* Notify about child bus events in any case */ > if (pcihp_bridge_en) { > QLIST_FOREACH(sec, &bus->child, sibling) { > - int32_t devfn = sec->parent_dev->devfn; > - > if (pci_bus_is_root(sec)) { > continue; > } > > - aml_append(method, aml_name("^S%.02X.PCNT", devfn)); > + aml_append(method, aml_name("^S%.02X.PCNT", > + sec->parent_dev->devfn)); > } > } > this is a refactor, sure. > -- > 2.27.0