Re: [Qemu-stable] [PATCH] ACPI: don't call acpi_pcihp_device_plug_cb on xen

[Bruce Rogers]

>>> On 4/27/2017 at 03:09 PM, Stefano Stabellini <address@hidden> 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 <address@hidden>
>> ---
>>  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.