From 310837de6c1e0badfd736b1b316b1698c53120a7 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Fri, 18 Nov 2016 16:07:00 +0100 Subject: virtio: introduce grab/release_ioeventfd to fix vhost 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 Reported-by: Christian Borntraeger Reported-by: Alex Williamson Fixes: ed08a2a0b ("virtio: use virtio_bus_set_host_notifier to start/stop ioeventfd") Reviewed-by: Cornelia Huck Reviewed-by: Stefan Hajnoczi Tested-by: Alexey Kardashevskiy Tested-by: Farhan Ali Tested-by: Alex Williamson Signed-off-by: Paolo Bonzini Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/virtio/virtio-bus.c | 58 ++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 47 insertions(+), 11 deletions(-) (limited to 'hw/virtio/virtio-bus.c') diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c index bf61f66a04..d6c0c72bd2 100644 --- a/hw/virtio/virtio-bus.c +++ b/hw/virtio/virtio-bus.c @@ -147,6 +147,39 @@ void virtio_bus_set_vdev_config(VirtioBusState *bus, uint8_t *config) } } +/* On success, ioeventfd ownership belongs to the caller. */ +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 +194,14 @@ 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; + + /* Only set our notifier if we have ownership. */ + 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 +216,12 @@ void virtio_bus_stop_ioeventfd(VirtioBusState *bus) return; } - vdev = virtio_bus_get_device(bus); - vdc = VIRTIO_DEVICE_GET_CLASS(vdev); - vdc->stop_ioeventfd(vdev); + /* Only remove our notifier if we have ownership. */ + 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 +251,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 +264,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); } -- cgit 1.4.1