On Thu, 27 Apr 2017, Bruce Rogers wrote: > >>> On 4/27/2017 at 03:09 PM, Stefano Stabellini <sstabell...@kernel.org> > >>> wrote: > > On Thu, 27 Apr 2017, Bruce Rogers wrote: > >> Commit f0c9d64a exposed the issue that with a xenfv machine using > >> pci passthrough, acpi cpi hotplug code was being executed by mistake. > >> Guard calls to acpi_pcihp_device_plug_cb (and corresponding > >> acpi_pcihp_device_unplug_cb) with a check for xen_enabled(). Without > >> this check I am seeing an error that the bus doesn't have the > >> acpi-pcihp-bsel property set. > >> > >> Signed-off-by: Bruce Rogers <brog...@suse.com> > >> --- > >> hw/acpi/piix4.c | 11 ++++++++--- > >> 1 file changed, 8 insertions(+), 3 deletions(-) > >> > >> diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c > >> index a553a7e..c409374 100644 > >> --- a/hw/acpi/piix4.c > >> +++ b/hw/acpi/piix4.c > >> @@ -385,7 +385,10 @@ static void piix4_device_plug_cb(HotplugHandler > > *hotplug_dev, > >> dev, errp); > >> } > >> } else if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) { > >> - acpi_pcihp_device_plug_cb(hotplug_dev, &s->acpi_pci_hotplug, dev, > > errp); > >> + if (!xen_enabled()) { > >> + acpi_pcihp_device_plug_cb(hotplug_dev, &s->acpi_pci_hotplug, > >> dev, > >> + errp); > >> + } > >> } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) { > >> if (s->cpu_hotplug_legacy) { > >> legacy_acpi_cpu_plug_cb(hotplug_dev, &s->gpe_cpu, dev, errp); > >> @@ -408,8 +411,10 @@ static void > > piix4_device_unplug_request_cb(HotplugHandler *hotplug_dev, > >> acpi_memory_unplug_request_cb(hotplug_dev, > >> &s->acpi_memory_hotplug, > >> dev, errp); > >> } else if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) { > >> - acpi_pcihp_device_unplug_cb(hotplug_dev, &s->acpi_pci_hotplug, > >> dev, > >> - errp); > >> + if (!xen_enabled()) { > >> + acpi_pcihp_device_unplug_cb(hotplug_dev, > >> &s->acpi_pci_hotplug, > > dev, > >> + errp); > >> + } > >> } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU) && > >> !s->cpu_hotplug_legacy) { > >> acpi_cpu_unplug_request_cb(hotplug_dev, &s->cpuhp_state, dev, > >> errp); > > > > Wouldn't it be better to simply turn the error_setg("Unsupported bus...) > > in acpi_pcihp_device_plug_cb and acpi_pcihp_device_unplug_cb into debug > > printfs? After all, from this description it is clear that they are not > > an error conditions. PCI Passthough works correctly, right? > > > > I realize that I should have suggested this earlier, sorry about that. > > I don't think we want to be running through qemu acpi code any more than is > necessary when using xen hvm.
Fair enough, but at the same time it is not good to disseminate if (!xen_enabled()) around. I am happy either way, I'll let Michael and Igor decide.