Hi,
On Tue, 10 Nov 2015 13:41:29 +0200, mar...@redhat.com wrote:
> The virtio devices are converted to PCI-Express
> if they are plugged into a PCI-Express bus and
> the 'modern' protocol is enabled.
>
> @@ -1592,6 +1592,26 @@ static void virtio_pci_realize(PCIDevice *pci_dev,
> Error **errp)
>
> + if (!(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_PCIE)
> + && !(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_MODERN)
> + && pci_bus_is_express(pci_dev->bus)
> + && !pci_bus_is_root(pci_dev->bus)) {
> + int pos;
> +
> + pci_dev->cap_present |= QEMU_PCI_CAP_EXPRESS;
Setting QEMU_PCI_CAP_EXPRESS here in 'virtio_pci_realize' is too late.
This is since 'pci_qdev_realize' (DeviceClass.realize of TYPE_PCI_DEVICE)
is invoked prior the PCIDeviceClass's specific 'realize' method
(virtio_pci_realize in this case).
During 'pci_qdev_realize' (specifically, within do_pci_register_device),
the QEMU_PCI_CAP_EXPRESS gets tested indirectly, when pci_is_express
and pci_config_size helpers are called.
For example: 'pci_config_alloc' uses 'pci_config_size' which relies on
QEMU_PCI_CAP_EXPRESS property.
Since virtio_pci sets QEMU_PCI_CAP_EXPRESS *after* pci_qdev_realize
has finished, we end up having an insufficient pci config space
allocated for the virtio "pcie" device.
May I suggest the following:
- Expose 'pci_qdev_realize'
- Have 'virtio_pci_class_init' arm it's own dc->realize,
which will first set 'QEMU_PCI_CAP_EXPRESS' flag as needed,
and then call 'pci_qdev_realize'
- Now, in 'virtio_pci_realize' we may use 'pci_is_express' instead of
directly checking the proxy->flags
If this sounds ok, I'll submit a fix.
Regards,
Shmulik
