On Thu, 22 Jul 2021 13:56:18 -0400 "Michael S. Tsirkin" <m...@redhat.com> wrote:
> 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. this was my mistake when splitting it into separate patch, I shouldn't have touched += PCI_FUNC_MAX here, point of the patch was to s/i/devfn/m so it won't distract from what the next patch is doing. > Or just squash this part into next patch. up to you. ok I'll squash it into next. > > > @@ -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 >