The ->set_host_notifier() callback is invoked whenever we want to
switch from or to the generic ioeventfd handler. Currently, all
transports deregister the ioeventfd backing and then re-register
it. This opens a race window where we are without ioeventfd
backing for a time period: In the virtio-blk dataplane case, we
observed notifications coming in from both the vcpu thread and
the iothread.
Let's change pci, mmio and ccw to keep the ioeventfd during
->set_host_notifier() and only switch the ioeventfd handler.
Signed-off-by: Cornelia Huck <cornelia.h...@de.ibm.com>
---
hw/s390x/virtio-ccw.c | 22 +++++++++++++++++-----
hw/virtio/virtio-mmio.c | 27 +++++++++++++++++----------
hw/virtio/virtio-pci.c | 28 ++++++++++++++++++----------
include/hw/virtio/virtio-bus.h | 4 ++++
4 files changed, 56 insertions(+), 25 deletions(-)
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index cb887ba..7b1088e 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -1196,14 +1196,26 @@ static bool
virtio_ccw_query_guest_notifiers(DeviceState *d)
static int virtio_ccw_set_host_notifier(DeviceState *d, int n, bool assign)
{
VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d);
+ VirtIODevice *vdev = virtio_bus_get_device(&dev->bus);
+ VirtQueue *vq = virtio_get_queue(vdev, n);
- /* Stop using the generic ioeventfd, we are doing eventfd handling
- * ourselves below */
- dev->ioeventfd_disabled = assign;
if (assign) {
- virtio_ccw_stop_ioeventfd(dev);
+ /* Stop using the generic ioeventfd, we are doing eventfd handling
+ * ourselves below */
+ dev->ioeventfd_disabled = true;
+ };
+ /*
+ * Just switch the handler, don't deassign the ioeventfd.
+ * Otherwise, there's a window where we don't have an
+ * ioeventfd and we may end up with a notification where
+ * we don't expect one.
+ */
+ virtio_queue_set_host_notifier_fd_handler(vq, assign, !assign);
+ if (!assign) {
+ /* Use generic ioeventfd handler again. */
+ dev->ioeventfd_disabled = false;
}
- return virtio_ccw_set_guest2host_notifier(dev, n, assign, false);
+ return 0;
}
static int virtio_ccw_get_mappings(VirtioCcwDevice *dev)
diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c
index d4cd91f..aafebdf 100644
--- a/hw/virtio/virtio-mmio.c
+++ b/hw/virtio/virtio-mmio.c
@@ -502,19 +502,26 @@ static int virtio_mmio_set_host_notifier(DeviceState
*opaque, int n,
bool assign)
{
VirtIOMMIOProxy *proxy = VIRTIO_MMIO(opaque);
+ VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
+ VirtQueue *vq = virtio_get_queue(vdev, n);
- /* Stop using ioeventfd for virtqueue kick if the device starts using host
- * notifiers. This makes it easy to avoid stepping on each others' toes.
- */
- proxy->ioeventfd_disabled = assign;
if (assign) {
- virtio_mmio_stop_ioeventfd(proxy);
+ /* Stop using the generic ioeventfd, we are doing eventfd handling
+ * ourselves below */
+ proxy->ioeventfd_disabled = true;
+ };
+ /*
+ * Just switch the handler, don't deassign the ioeventfd.
+ * Otherwise, there's a window where we don't have an
+ * ioeventfd and we may end up with a notification where
+ * we don't expect one.
+ */
+ virtio_queue_set_host_notifier_fd_handler(vq, assign, !assign);
+ if (!assign) {
+ /* Use generic ioeventfd handler again. */
+ proxy->ioeventfd_disabled = false;
}
- /* We don't need to start here: it's not needed because backend
- * currently only stops on status change away from ok,
- * reset, vmstop and such. If we do add code to start here,
- * need to check vmstate, device state etc. */
- return virtio_mmio_set_host_notifier_internal(proxy, n, assign, false);
+ return 0;
}
/* virtio-mmio device */
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 0dadb66..a91c1e8 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1115,18 +1115,26 @@ static int virtio_pci_set_host_notifier(DeviceState *d,
int n, bool assign)
{
VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
- /* Stop using ioeventfd for virtqueue kick if the device starts using host
- * notifiers. This makes it easy to avoid stepping on each others' toes.
- */
- proxy->ioeventfd_disabled = assign;
+ VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
+ VirtQueue *vq = virtio_get_queue(vdev, n);
+
if (assign) {
- virtio_pci_stop_ioeventfd(proxy);
+ /* Stop using the generic ioeventfd, we are doing eventfd handling
+ * ourselves below */
+ proxy->ioeventfd_disabled = true;
+ };
+ /*
+ * Just switch the handler, don't deassign the ioeventfd.
+ * Otherwise, there's a window where we don't have an
+ * ioeventfd and we may end up with a notification where
+ * we don't expect one.
+ */
+ virtio_queue_set_host_notifier_fd_handler(vq, assign, !assign);
+ if (!assign) {
+ /* Use generic ioeventfd handler again. */
+ proxy->ioeventfd_disabled = false;
}
- /* We don't need to start here: it's not needed because backend
- * currently only stops on status change away from ok,
- * reset, vmstop and such. If we do add code to start here,
- * need to check vmstate, device state etc. */
- return virtio_pci_set_host_notifier_internal(proxy, n, assign, false);
+ return 0;
}
static void virtio_pci_vmstate_change(DeviceState *d, bool running)
diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h
index 3f2c136..98c660a 100644
--- a/include/hw/virtio/virtio-bus.h
+++ b/include/hw/virtio/virtio-bus.h
@@ -52,6 +52,10 @@ typedef struct VirtioBusClass {
bool (*has_extra_state)(DeviceState *d);
bool (*query_guest_notifiers)(DeviceState *d);
int (*set_guest_notifiers)(DeviceState *d, int nvqs, bool assign);
+ /*
+ * Switch from/to the generic ioeventfd handler.
+ * assigned==false means 'use generic ioeventfd handler'.
+ */
int (*set_host_notifier)(DeviceState *d, int n, bool assigned);
void (*vmstate_change)(DeviceState *d, bool running);
/*
--
2.8.0
