On 2019-01-22 16:11, David Hildenbrand wrote: > On 22.01.19 16:03, Christian Borntraeger wrote: >> >> >> On 22.01.2019 13:52, David Hildenbrand wrote: >>> On 22.01.19 13:44, Cornelia Huck wrote: >>>> On Tue, 22 Jan 2019 10:41:43 +0100 >>>> David Hildenbrand <da...@redhat.com> wrote: >>>> >>>>> We decided to always create the PCI host bridge, even if 'zpci' is not >>>>> enabled (due to migration compatibility). This however right now allows >>>>> to add zPCI/PCI devices to a VM although the guest will never actually see >>>>> them, confusing people that are using a simple CPU model that has no >>>>> 'zpci' enabled - "Why isn't this working" (David Hildenbrand) >>>>> >>>>> Let's check for 'zpci' and at least print a warning that this will not >>>>> work as expected. We could also bail out, however that might break >>>>> existing QEMU commandlines. >>>>> >>>>> Signed-off-by: David Hildenbrand <da...@redhat.com> >>>>> --- >>>>> hw/s390x/s390-pci-bus.c | 5 +++++ >>>>> 1 file changed, 5 insertions(+) >>>>> >>>>> diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c >>>>> index b86a8bdcd4..e7d4f49611 100644 >>>>> --- a/hw/s390x/s390-pci-bus.c >>>>> +++ b/hw/s390x/s390-pci-bus.c >>>>> @@ -863,6 +863,11 @@ static void s390_pcihost_pre_plug(HotplugHandler >>>>> *hotplug_dev, DeviceState *dev, >>>>> { >>>>> S390pciState *s = S390_PCI_HOST_BRIDGE(hotplug_dev); >>>>> >>>>> + if (!s390_has_feat(S390_FEAT_ZPCI)) { >>>>> + warn_report("Adding PCI or zPCI devices without the 'zpci' CPU >>>>> feature." >>>>> + " The guest will not be able to see/use these >>>>> devices."); >>>>> + } >>>>> + >>>>> if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) { >>>>> PCIDevice *pdev = PCI_DEVICE(dev); >>>>> >>>> >>>> That's hotplug only, isn't it? IIRC coldplugging already fails? >>>> >>> >>> No, applies also to coldplugging. >> >> Back then we made this a conscious decision, because removing the bridge >> triggered a >> lot of issues regarding migration. And the current behaviour actually is a >> good >> match to the real hardware, there are PCI devices in the system that can not >> be used >> by guests. I understand that this is kind of surprising, so I am fine with >> the warn_report >> but I do not want to have a hard error right now. > > So you agree to this patch, unmodified, correct? Thanks!
FWIW, it's fine for me: Reviewed-by: Thomas Huth <th...@redhat.com>