[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 3/4] hw:acpi:pcihp: assume root PCI bus if bus h
From: |
Michael S. Tsirkin |
Subject: |
Re: [Qemu-devel] [PATCH 3/4] hw:acpi:pcihp: assume root PCI bus if bus has no ACPI_PCIHP_PROP_BSEL property |
Date: |
Sun, 26 Jan 2014 12:02:23 +0200 |
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 <address@hidden>
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