Following the recent refactoring of virtio notifiers [1], more specifically
the patch ed08a2a0b ("virtio: use virtio_bus_set_host_notifier to
start/stop ioeventfd") 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.When vhost is stopped and started, however, there is 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 reintroduces (hopefully in a cleaner way) the concept that was present with ioeventfd_disabled before the refactoring. When ioeventfd_grabbed>0, ioeventfd_started tracks whether ioeventfd should be enabled or not, but ioeventfd is actually not started at all until vhost releases the host notifiers. [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 Reported-by: Felipe Franciosi <fel...@nutanix.com> Reported-by: Christian Borntraeger <borntrae...@de.ibm.com> Reported-by: Alex Williamson <alex.william...@redhat.com> Fixes: ed08a2a0b ("virtio: use virtio_bus_set_host_notifier to start/stop ioeventfd") Signed-off-by: Paolo Bonzini <pbonz...@redhat.com> Message-Id: <20161111192855.26350-1-pbonz...@redhat.com> Signed-off-by: Paolo Bonzini <pbonz...@redhat.com> --- hw/virtio/vhost.c | 11 ++++----- hw/virtio/virtio-bus.c | 54 +++++++++++++++++++++++++++++++++--------- hw/virtio/virtio.c | 16 +++++++++++++ include/hw/virtio/virtio-bus.h | 14 +++++++++++ include/hw/virtio/virtio.h | 2 ++ 5 files changed, 79 insertions(+), 18 deletions(-) diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c index 131f164..a8b5ab8 100644 --- a/hw/virtio/vhost.c +++ b/hw/virtio/vhost.c @@ -1186,17 +1186,14 @@ void vhost_dev_cleanup(struct vhost_dev *hdev) int vhost_dev_enable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev) { BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev))); - VirtioBusState *vbus = VIRTIO_BUS(qbus); - VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(vbus); int i, r, e; - if (!k->ioeventfd_assign) { + r = virtio_device_grab_ioeventfd(vdev); + if (r < 0) { error_report("binding does not support host notifiers"); - r = -ENOSYS; goto fail; } - virtio_device_stop_ioeventfd(vdev); for (i = 0; i < hdev->nvqs; ++i) { r = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), hdev->vq_index + i, true); @@ -1216,7 +1213,7 @@ fail_vq: } assert (e >= 0); } - virtio_device_start_ioeventfd(vdev); + virtio_device_release_ioeventfd(vdev); fail: return r; } @@ -1239,7 +1236,7 @@ void vhost_dev_disable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev) } assert (r >= 0); } - virtio_device_start_ioeventfd(vdev); + virtio_device_release_ioeventfd(vdev); } /* Test and clear event pending status. diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c index bf61f66..c8a446e 100644 --- a/hw/virtio/virtio-bus.c +++ b/hw/virtio/virtio-bus.c @@ -147,6 +147,38 @@ void virtio_bus_set_vdev_config(VirtioBusState *bus, uint8_t *config) } } +int virtio_bus_grab_ioeventfd(VirtioBusState *bus) +{ + VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(bus); + + /* vhost can be used even if ioeventfd=off in the proxy device, + * so do not check k->ioeventfd_enabled. + */ + if (!k->ioeventfd_assign) { + return -ENOSYS; + } + + if (bus->ioeventfd_grabbed == 0 && bus->ioeventfd_started) { + virtio_bus_stop_ioeventfd(bus); + /* Remember that we need to restart ioeventfd + * when ioeventfd_grabbed becomes zero. + */ + bus->ioeventfd_started = true; + } + bus->ioeventfd_grabbed++; + return 0; +} + +void virtio_bus_release_ioeventfd(VirtioBusState *bus) +{ + assert(bus->ioeventfd_grabbed != 0); + if (--bus->ioeventfd_grabbed == 0 && bus->ioeventfd_started) { + /* Force virtio_bus_start_ioeventfd to act. */ + bus->ioeventfd_started = false; + virtio_bus_start_ioeventfd(bus); + } +} + int virtio_bus_start_ioeventfd(VirtioBusState *bus) { VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(bus); @@ -161,10 +193,12 @@ int virtio_bus_start_ioeventfd(VirtioBusState *bus) if (bus->ioeventfd_started) { return 0; } - r = vdc->start_ioeventfd(vdev); - if (r < 0) { - error_report("%s: failed. Fallback to userspace (slower).", __func__); - return r; + if (!bus->ioeventfd_grabbed) { + r = vdc->start_ioeventfd(vdev); + if (r < 0) { + error_report("%s: failed. Fallback to userspace (slower).", __func__); + return r; + } } bus->ioeventfd_started = true; return 0; @@ -179,9 +213,11 @@ void virtio_bus_stop_ioeventfd(VirtioBusState *bus) return; } - vdev = virtio_bus_get_device(bus); - vdc = VIRTIO_DEVICE_GET_CLASS(vdev); - vdc->stop_ioeventfd(vdev); + if (!bus->ioeventfd_grabbed) { + vdev = virtio_bus_get_device(bus); + vdc = VIRTIO_DEVICE_GET_CLASS(vdev); + vdc->stop_ioeventfd(vdev); + } bus->ioeventfd_started = false; } @@ -211,7 +247,6 @@ int virtio_bus_set_host_notifier(VirtioBusState *bus, int n, bool assign) } if (assign) { - assert(!bus->ioeventfd_started); r = event_notifier_init(notifier, 1); if (r < 0) { error_report("%s: unable to init event notifier: %s (%d)", @@ -225,9 +260,6 @@ int virtio_bus_set_host_notifier(VirtioBusState *bus, int n, bool assign) } return 0; } else { - if (!bus->ioeventfd_started) { - return 0; - } k->ioeventfd_assign(proxy, notifier, n, false); } diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index bcbcfe0..89b0b80 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -2202,6 +2202,22 @@ void virtio_device_stop_ioeventfd(VirtIODevice *vdev) virtio_bus_stop_ioeventfd(vbus); } +int virtio_device_grab_ioeventfd(VirtIODevice *vdev) +{ + BusState *qbus = qdev_get_parent_bus(DEVICE(vdev)); + VirtioBusState *vbus = VIRTIO_BUS(qbus); + + return virtio_bus_grab_ioeventfd(vbus); +} + +void virtio_device_release_ioeventfd(VirtIODevice *vdev) +{ + BusState *qbus = qdev_get_parent_bus(DEVICE(vdev)); + VirtioBusState *vbus = VIRTIO_BUS(qbus); + + virtio_bus_release_ioeventfd(vbus); +} + static void virtio_device_class_init(ObjectClass *klass, void *data) { /* Set the default value here. */ diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h index fdf7fda..8a51e2c 100644 --- a/include/hw/virtio/virtio-bus.h +++ b/include/hw/virtio/virtio-bus.h @@ -97,6 +97,16 @@ struct VirtioBusState { * Set if ioeventfd has been started. */ bool ioeventfd_started; + + /* + * Set if ioeventfd has been grabbed by vhost. When ioeventfd + * is grabbed by vhost, we track its started/stopped state (which + * depends in turn on the virtio status register), but do not + * register a handler for the ioeventfd. When ioeventfd is + * released, if ioeventfd_started is true we finally register + * the handler so that QEMU's device model can use ioeventfd. + */ + int ioeventfd_grabbed; }; void virtio_bus_device_plugged(VirtIODevice *vdev, Error **errp); @@ -131,6 +141,10 @@ bool virtio_bus_ioeventfd_enabled(VirtioBusState *bus); int virtio_bus_start_ioeventfd(VirtioBusState *bus); /* Stop the ioeventfd. */ void virtio_bus_stop_ioeventfd(VirtioBusState *bus); +/* Tell the bus that vhost is grabbing the ioeventfd. */ +int virtio_bus_grab_ioeventfd(VirtioBusState *bus); +/* bus that vhost is not using the ioeventfd anymore. */ +void virtio_bus_release_ioeventfd(VirtioBusState *bus); /* Switch from/to the generic ioeventfd handler */ int virtio_bus_set_host_notifier(VirtioBusState *bus, int n, bool assign); diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h index ac65d6a..35ede30 100644 --- a/include/hw/virtio/virtio.h +++ b/include/hw/virtio/virtio.h @@ -270,6 +270,8 @@ void virtio_queue_set_guest_notifier_fd_handler(VirtQueue *vq, bool assign, bool with_irqfd); int virtio_device_start_ioeventfd(VirtIODevice *vdev); void virtio_device_stop_ioeventfd(VirtIODevice *vdev); +int virtio_device_grab_ioeventfd(VirtIODevice *vdev); +void virtio_device_release_ioeventfd(VirtIODevice *vdev); bool virtio_device_ioeventfd_enabled(VirtIODevice *vdev); EventNotifier *virtio_queue_get_host_notifier(VirtQueue *vq); void virtio_queue_host_notifier_read(EventNotifier *n); -- 2.9.3
