On Tue, Jan 21, 2014 at 03:40:05PM +0100, Igor Mammedov wrote: > when running with machine types older than 1.7 (i.e. without ACPI > builtin tables), PCI bus won't have ACPI_PCIHP_PROP_BSEL property > set. > Taking in account that acpi hotplug handler in 1.7 and older > machines is called only for root PCI bus, to make pcihp code > compatible with legacy machine types assume that bus without > ACPI_PCIHP_PROP_BSEL property has it equal to 0 and bail out > if it's not root bus. > > Signed-off-by: Igor Mammedov <imamm...@redhat.com>
I think that's not the best way to do this. If bsel 0 *is* set on some bus, it should select it. Fallback to bus 0 only if bsel value 0 is not set anywhere. See e.g. how acpi_pcihp_find_hotplug_bus does it: if (!bsel && !find.bus) { find.bus = s->root; } otherwise we introduce dependency on the logic that sets bsel, this makes code fragile. > --- > hw/acpi/pcihp.c | 10 +++------- > 1 files changed, 3 insertions(+), 7 deletions(-) > > diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c > index 6d34fe9..76dce8d 100644 > --- a/hw/acpi/pcihp.c > +++ b/hw/acpi/pcihp.c > @@ -63,14 +63,10 @@ static int acpi_pcihp_get_bsel(PCIBus *bus) > { > QObject *o = object_property_get_qobject(OBJECT(bus), > ACPI_PCIHP_PROP_BSEL, NULL); > - int64_t bsel = -1; > if (o) { > - bsel = qint_get_int(qobject_to_qint(o)); > + return qint_get_int(qobject_to_qint(o)); > } > - if (bsel < 0) { > - return -1; > - } > - return bsel; > + return 0; > } > > static void acpi_pcihp_test_hotplug_bus(PCIBus *bus, void *opaque) > @@ -190,7 +186,7 @@ int acpi_pcihp_device_hotplug(AcpiPciHpState *s, > PCIDevice *dev, > { > int slot = PCI_SLOT(dev->devfn); > int bsel = acpi_pcihp_get_bsel(dev->bus); > - if (bsel < 0) { > + if ((bsel == 0) && (dev->bus != s->root)) { > return -1; > } > > -- > 1.7.1