On Mon, Apr 15, 2024 at 11:23:46AM +0200, Philippe Mathieu-Daudé wrote: > On 15/4/24 11:19, Michael S. Tsirkin wrote: > > From: Cindy Lu <l...@redhat.com> > > > > During the booting process of the non-standard image, the behavior of the > > called function in qemu is as follows: > > > > 1. vhost_net_stop() was triggered by guest image. This will call the > > function > > virtio_pci_set_guest_notifiers() with assgin= false, > > virtio_pci_set_guest_notifiers() will release the irqfd for vector 0 > > > > 2. virtio_reset() was triggered, this will set configure vector to > > VIRTIO_NO_VECTOR > > > > 3.vhost_net_start() was called (at this time, the configure vector is > > still VIRTIO_NO_VECTOR) and then call virtio_pci_set_guest_notifiers() with > > assgin=true, so the irqfd for vector 0 is still not "init" during this > > process > > > > 4. The system continues to boot and sets the vector back to 0. After that > > msix_fire_vector_notifier() was triggered to unmask the vector 0 and meet > > the crash > > > > To fix the issue, we need to support changing the vector after > > VIRTIO_CONFIG_S_DRIVER_OK is set. > > > > MST: coding style and typo fixups > > > > Fixes: f9a09ca3ea ("vhost: add support for configure interrupt") > > Cc: qemu-sta...@nongnu.org > > Signed-off-by: Cindy Lu <l...@redhat.com> > > Message-Id: <20240412062750.475180-1-l...@redhat.com> > > Reviewed-by: Michael S. Tsirkin <m...@redhat.com> > > Signed-off-by: Michael S. Tsirkin <m...@redhat.com> > > Signed-off-by: Michael S. Tsirkin <m...@redhat.com> > > --- > > > > To expedite fixed lots of style issues myself. > > Completely untested - guys can you pls test and ack? > > > > > > hw/virtio/virtio-pci.c | 41 +++++++++++++++++++++++++++++++++++++++-- > > 1 file changed, 39 insertions(+), 2 deletions(-) > > > > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c > > index cb6940fc0e..e9edd57339 100644 > > --- a/hw/virtio/virtio-pci.c > > +++ b/hw/virtio/virtio-pci.c > > @@ -1424,6 +1424,34 @@ static int virtio_pci_add_mem_cap(VirtIOPCIProxy > > *proxy, > > return offset; > > } > > +static void virtio_pci_set_and_change_vector(VirtIODevice *vdev, > > + VirtIOPCIProxy *proxy, > > + int queue_no, uint16_t > > old_vector, > > + uint16_t new_vector) > > +{ > > Alternatively: > > if (new_vector == old_vector) { > return; > }
True. And in fact callers do not need check this. Let me do v8. > > + bool kvm_irqfd = (vdev->status & VIRTIO_CONFIG_S_DRIVER_OK) && > > + msix_enabled(&proxy->pci_dev) && kvm_msi_via_irqfd_enabled(); > > + > > + /* > > + * If the device uses irqfd and the vector changes after DRIVER_OK is > > + * set, we need to release the old vector and set up the new one. > > + * Otherwise just need to set the new vector on the device. > > + */ > > + if (kvm_irqfd && old_vector != VIRTIO_NO_VECTOR) { > > + kvm_virtio_pci_vector_release_one(proxy, queue_no); > > + } > > + /* Set the new vector on the device. */ > > + if (queue_no == VIRTIO_CONFIG_IRQ_IDX) { > > + vdev->config_vector = new_vector; > > + } else { > > + virtio_queue_set_vector(vdev, queue_no, new_vector); > > + } > > + /* If the new vector changed need to set it up. */ > > + if (kvm_irqfd && new_vector != VIRTIO_NO_VECTOR) { > > + kvm_virtio_pci_vector_use_one(proxy, queue_no); > > + } > > +} > > + > > int virtio_pci_add_shm_cap(VirtIOPCIProxy *proxy, > > uint8_t bar, uint64_t offset, uint64_t length, > > uint8_t id) > > @@ -1570,7 +1598,12 @@ static void virtio_pci_common_write(void *opaque, > > hwaddr addr, > > } else { > > val = VIRTIO_NO_VECTOR; > > } > > - vdev->config_vector = val; > > + vector = vdev->config_vector; > > + /* Check if we need to change the vector. */ > > + if (val != vector) { > > + virtio_pci_set_and_change_vector(vdev, proxy, > > VIRTIO_CONFIG_IRQ_IDX, > > + vector, val); > > + } > > break;