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 <felipe@nutanix.com>
Reported-by: Christian Borntraeger <borntraeger@de.ibm.com>
Reported-by: Alex Williamson <alex.williamson@redhat.com>
Fixes: ed08a2a0b ("virtio: use virtio_bus_set_host_notifier to start/stop ioeventfd")
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Tested-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Tested-by: Farhan Ali <alifm@linux.vnet.ibm.com>
Tested-by: Alex Williamson <alex.williamson@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
master
Paolo Bonzini 2016-11-18 16:07:00 +01:00 committed by Michael S. Tsirkin
parent 600f5ce356
commit 310837de6c
5 changed files with 86 additions and 18 deletions

View File

@ -1214,17 +1214,17 @@ 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) {
/* We will pass the notifiers to the kernel, make sure that QEMU
* doesn't interfere.
*/
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);
@ -1244,7 +1244,7 @@ fail_vq:
}
assert (e >= 0);
}
virtio_device_start_ioeventfd(vdev);
virtio_device_release_ioeventfd(vdev);
fail:
return r;
}
@ -1267,7 +1267,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.

View File

@ -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);
}

View File

@ -2191,6 +2191,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. */

View File

@ -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);

View File

@ -272,6 +272,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);