On Mon, Jan 09, 2023 at 10:20:45AM +0000, Dr. David Alan Gilbert wrote: > * Michael S. Tsirkin (m...@redhat.com) wrote: > > From: David Daney <david.da...@fungible.com> > > > > Most other virtio-pci devices allow MSI-X, let's have it for rng too. > > > > Signed-off-by: David Daney <david.da...@fungible.com> > > Reviewed-by: Marcin Nowakowski <marcin.nowakow...@fungible.com> > > Signed-off-by: Philippe Mathieu-Daudé <phi...@fungible.com> > > Message-Id: <20221014160947.66105-1-phi...@fungible.com> > > Reviewed-by: Stefan Hajnoczi <stefa...@redhat.com> > > Reviewed-by: Michael S. Tsirkin <m...@redhat.com> > > Signed-off-by: Michael S. Tsirkin <m...@redhat.com> > > This breaks migration compatibility 7.1->7.2 : > > (qemu) qemu: get_pci_config_device: Bad config data: i=0x34 read: 84 device: > 98 cmask: ff wmask: 0 w1cmask:0 > qemu: Failed to load PCIDevice:config > qemu: Failed to load virtio-rng:virtio > qemu: error while loading state for instance 0x0 of device > '0000:00:03.0/virtio-rng' > qemu: load of migration failed: Invalid argument > > because the destination is configured with msi-x but the source isn't. > > The fix is in theory simple: > diff --git a/hw/core/machine.c b/hw/core/machine.c > index f589b92909..45459d1cef 100644 > --- a/hw/core/machine.c > +++ b/hw/core/machine.c > @@ -45,6 +45,7 @@ const size_t hw_compat_7_2_len = > G_N_ELEMENTS(hw_compat_7_2); > > GlobalProperty hw_compat_7_1[] = { > { "virtio-device", "queue_reset", "false" }, > + { "virtio-rng-pci", "vectors", "0" }, > }; > const size_t hw_compat_7_1_len = G_N_ELEMENTS(hw_compat_7_1); > > the gotcha is that will break 7.2->7.2-fixed. > > (I guess you can also work around it by explicitly passing vectors=0 to > the virtio-rng on the cli) > > Does anyone have preferences as to whether that should be fixed in the > 7.2 world or left as is? > > This is: > https://bugzilla.redhat.com/show_bug.cgi?id=2155749 > > Dave
I think that yes, it should be fixed in 7.2. > > --- > > hw/virtio/virtio-rng-pci.c | 14 ++++++++++++++ > > 1 file changed, 14 insertions(+) > > > > diff --git a/hw/virtio/virtio-rng-pci.c b/hw/virtio/virtio-rng-pci.c > > index 151ece6f94..6e76f8b57b 100644 > > --- a/hw/virtio/virtio-rng-pci.c > > +++ b/hw/virtio/virtio-rng-pci.c > > @@ -13,6 +13,7 @@ > > > > #include "hw/virtio/virtio-pci.h" > > #include "hw/virtio/virtio-rng.h" > > +#include "hw/qdev-properties.h" > > #include "qapi/error.h" > > #include "qemu/module.h" > > #include "qom/object.h" > > @@ -31,11 +32,23 @@ struct VirtIORngPCI { > > VirtIORNG vdev; > > }; > > > > +static Property virtio_rng_properties[] = { > > + DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags, > > + VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true), > > + DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, > > + DEV_NVECTORS_UNSPECIFIED), > > + DEFINE_PROP_END_OF_LIST(), > > +}; > > + > > static void virtio_rng_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp) > > { > > VirtIORngPCI *vrng = VIRTIO_RNG_PCI(vpci_dev); > > DeviceState *vdev = DEVICE(&vrng->vdev); > > > > + if (vpci_dev->nvectors == DEV_NVECTORS_UNSPECIFIED) { > > + vpci_dev->nvectors = 2; > > + } > > + > > if (!qdev_realize(vdev, BUS(&vpci_dev->bus), errp)) { > > return; > > } > > @@ -54,6 +67,7 @@ static void virtio_rng_pci_class_init(ObjectClass *klass, > > void *data) > > pcidev_k->device_id = PCI_DEVICE_ID_VIRTIO_RNG; > > pcidev_k->revision = VIRTIO_PCI_ABI_VERSION; > > pcidev_k->class_id = PCI_CLASS_OTHERS; > > + device_class_set_props(dc, virtio_rng_properties); > > } > > > > static void virtio_rng_initfn(Object *obj) > > -- > > MST > > > > > -- > Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK