On 07/11/2016 18:23, Felipe Franciosi wrote: > Following the recent refactor of virtio notfiers [1], more specifically > the patch that uses virtio_bus_set_host_notifier [2] by default, core > virtio code requires 'ioeventfd_started' to be set to true/false when > the host notifiers are configured. Since vhost-scsi uses the legacy > interface, this value is not updated. > > When booting a guest with a vhost-scsi backend controller, SeaBIOS will > initially configure the device which sets all notifiers. The guest will > continue to boot fine until the kernel virtio-scsi module reinitialises > the device causing a stop followed by another start. Since > ioeventfd_started was never set to true, the 'stop' operation triggered > by virtio_bus_set_host_notifier() will not result in a call to > virtio_pci_ioeventfd_assign(assign=false). This leaves the memory > regions with stale notifiers and results on the next start triggering > the following assertion: > > kvm_mem_ioeventfd_add: error adding ioeventfd: File exists > Aborted > > This patch updates ioeventfd_started whenever the notifiers are set or > cleared, fixing this issue. > > Signed-off-by: Felipe Franciosi <[email protected]> > > [1] http://lists.nongnu.org/archive/html/qemu-devel/2016-10/msg07748.html > [2] http://lists.nongnu.org/archive/html/qemu-devel/2016-10/msg07760.html > --- > hw/scsi/vhost-scsi.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c > index 5b26946..1c6e6d4 100644 > --- a/hw/scsi/vhost-scsi.c > +++ b/hw/scsi/vhost-scsi.c > @@ -95,6 +95,7 @@ static int vhost_scsi_start(VHostSCSI *s) > if (ret < 0) { > return ret; > } > + VIRTIO_BUS(qbus)->ioeventfd_started = true; > > s->dev.acked_features = vdev->guest_features; > ret = vhost_dev_start(&s->dev, vdev); > @@ -152,6 +153,7 @@ static void vhost_scsi_stop(VHostSCSI *s) > vhost_scsi_clear_endpoint(s); > vhost_dev_stop(&s->dev, vdev); > vhost_dev_disable_notifiers(&s->dev, vdev); > + VIRTIO_BUS(qbus)->ioeventfd_started = false; > } > > static uint64_t vhost_scsi_get_features(VirtIODevice *vdev, >
While a bit hacky, at least for 2.8 the idea should be fine. Only it has to be done in vhost_dev_enable_notifiers and vhost_dev_disable_notifiers, close to the calls to virtio_device_stop_ioeventfd (i.e., set bus->ioeventfd_started after the call) and virtio_device_start_ioeventfd (clear it before the call). I've now worked through a fix that uses start_ioeventfd/stop_ioeventfd, and it does comes out nice (29 lines added, 65 removed :)), but it isn't bisectable (so it has to be squashed into a single patch) and does not cover vhost-net yet. So let's go with your patch for now. I'll send the vm_running fix separately, since that one is a real bugfix. Paolo
