Re: [Qemu-devel] [PATCH 3/9] dataplane: remove EventPoll in favor of AioContext

[Christian Borntraeger <borntraeger@de.ibm.com>]

[-- Attachment #1: Type: text/plain, Size: 1584 bytes --]

On 04/03/13 10:15, Stefan Hajnoczi wrote:
> From: Paolo Bonzini <pbonzini@redhat.com>
> 
> During the review of the dataplane code, the EventPoll API morphed itself
> (not concidentially) into something very very similar to an AioContext.
> Thus, it is trivial to convert virtio-blk-dataplane to use AioContext,
> and a first baby step towards letting dataplane talk directly to the
> QEMU block layer.
> 
> The only interesting note is the value-copy of EventNotifiers.  At least
> in my opinion this is part of the EventNotifier API and is even portable
> to Windows.  Of course, in this case you should not close the notifier's
> underlying file descriptors or handle with event_notifier_cleanup.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

Hmm, this broke data plane on our internal notifier prototype code on virtio-ccw
(attached for reference)
[...]


> +    /* Note that these EventNotifiers are assigned by value.  This is
> +     * fine as long as you do not call event_notifier_cleanup on them
> +     * (because you don't own the file descriptor or handle; you just
> +     * use it).
> +     */

And this might be the reason. Currently we only have eventfd to wire up 
guest_to_host notifiers. The host_to_guest notification is not done
via vectors/irqfd, instead we let our qemu transport listen to the irq
eventfd. Worked fine so far with vhost and dataplane without this patch.

Any ideas how to properly enable a transport that has full host notifiers
but only poor mans guest notifiers?

Christian


[-- Attachment #2: guest-host-notifiers.patch --]
[-- Type: text/x-patch, Size: 5251 bytes --]

>From 76ceaec73c44f71b2e703accb157c09fef94ccd1 Mon Sep 17 00:00:00 2001
From: Cornelia Huck <cornelia.huck@de.ibm.com>
Date: Tue, 19 Feb 2013 13:48:17 +0100
Subject: [PATCH 23/28] Re-add guest/host notifiers.

Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 hw/s390x/virtio-ccw.c | 90 +++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/s390x/virtio-ccw.h |  1 +
 2 files changed, 91 insertions(+)

diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index 63c851f..d4fa42a 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -95,6 +95,7 @@ static void virtio_ccw_start_ioeventfd(VirtioCcwDevice *dev)
     int n, r;
 
     if (!(dev->flags & VIRTIO_CCW_FLAG_USE_IOEVENTFD) ||
+        dev->ioeventfd_disabled ||
         dev->ioeventfd_started) {
         return;
     }
@@ -793,6 +794,89 @@ static void virtio_ccw_vmstate_change(DeviceState *d, bool running)
     }
 }
 
+static bool virtio_ccw_query_guest_notifiers(DeviceState *d)
+{
+    VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d);
+
+    return !!(dev->sch->curr_status.pmcw.flags & PMCW_FLAGS_MASK_ENA);
+}
+
+static int virtio_ccw_set_host_notifier(DeviceState *d, int n, bool assign)
+{
+    VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d);
+
+    /* Stop using the generic ioeventfd, we are doing eventfd handling
+     * ourselves below */
+    dev->ioeventfd_disabled = assign;
+    if (assign) {
+        virtio_ccw_stop_ioeventfd(dev);
+    }
+    return virtio_ccw_set_guest2host_notifier(dev, n, assign, false);
+}
+
+static int virtio_ccw_set_guest_notifier(VirtioCcwDevice *dev, int n,
+                                         bool assign, bool with_irqfd)
+{
+    VirtQueue *vq = virtio_get_queue(dev->vdev, n);
+    EventNotifier *notifier = virtio_queue_get_guest_notifier(vq);
+
+    if (assign) {
+        int r = event_notifier_init(notifier, 0);
+
+        if (r < 0) {
+            return r;
+        }
+        virtio_queue_set_guest_notifier_fd_handler(vq, true, with_irqfd);
+        /* We do not support irqfd for classic I/O interrupts, because the
+         * classic interrupts are intermixed with the subchannel status, that
+         * is queried with test subchannel. We want to use vhost, though.
+         * Lets make sure to have vhost running and wire up the irq fd to 
+         * land in qemu (and only the irq fd) in this code.
+         */
+        if (dev->vdev->guest_notifier_mask) {
+            dev->vdev->guest_notifier_mask(dev->vdev, n, false);
+        }
+        /* get lost events and re-inject */
+        if (dev->vdev->guest_notifier_pending &&
+            dev->vdev->guest_notifier_pending(dev->vdev, n)) {
+            event_notifier_set(notifier);
+        }
+    } else {
+        if (dev->vdev->guest_notifier_mask) {
+            dev->vdev->guest_notifier_mask(dev->vdev, n, true);
+        }
+        virtio_queue_set_guest_notifier_fd_handler(vq, false, with_irqfd);
+        event_notifier_cleanup(notifier);
+    }
+    return 0;
+}
+
+static int virtio_ccw_set_guest_notifiers(DeviceState *d, int nvqs,
+                                          bool assigned)
+{
+    VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d);
+    VirtIODevice *vdev = dev->vdev;
+    int r, n;
+
+    for (n = 0; n < nvqs; n++) {
+        if (!virtio_queue_get_num(vdev, n)) {
+            break;
+        }
+        /* false -> true, as soon as irqfd works */
+        r = virtio_ccw_set_guest_notifier(dev, n, assigned, false);
+        if (r < 0) {
+            goto assign_error;
+        }
+    }
+    return 0;
+
+assign_error:
+    while (--n >= 0) {
+        virtio_ccw_set_guest_notifier(dev, n, !assigned, false);
+    }
+    return r;
+}
+
 static void virtio_ccw_save_queue(DeviceState *d, int n, QEMUFile *f)
 {
     VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d);
@@ -840,6 +924,9 @@ static const VirtIOBindings virtio_ccw_bindings = {
     .notify = virtio_ccw_notify,
     .get_features = virtio_ccw_get_features,
     .vmstate_change = virtio_ccw_vmstate_change,
+    .query_guest_notifiers = virtio_ccw_query_guest_notifiers,
+    .set_host_notifier = virtio_ccw_set_host_notifier,
+    .set_guest_notifiers = virtio_ccw_set_guest_notifiers,
     .save_queue = virtio_ccw_save_queue,
     .load_queue = virtio_ccw_load_queue,
     .save_config = virtio_ccw_save_config,
@@ -1093,6 +1180,9 @@ static void virtio_ccw_bus_class_init(ObjectClass *klass, void *data)
     k->notify = virtio_ccw_notify;
     k->get_features = virtio_ccw_get_features;
     k->vmstate_change = virtio_ccw_vmstate_change;
+    k->query_guest_notifiers = virtio_ccw_query_guest_notifiers;
+    k->set_host_notifier = virtio_ccw_set_host_notifier;
+    k->set_guest_notifiers = virtio_ccw_set_guest_notifiers;
     k->save_queue = virtio_ccw_save_queue;
     k->load_queue = virtio_ccw_load_queue;
     k->save_config = virtio_ccw_save_config;
diff --git a/hw/s390x/virtio-ccw.h b/hw/s390x/virtio-ccw.h
index db83e0f..a2e066f 100644
--- a/hw/s390x/virtio-ccw.h
+++ b/hw/s390x/virtio-ccw.h
@@ -84,6 +84,7 @@ struct VirtioCcwDevice {
     VirtIOSCSIConf scsi;
     VirtioBusState bus;
     bool ioeventfd_started;
+    bool ioeventfd_disabled;
     uint32_t flags;
     /* Guest provided values: */
     hwaddr indicators;
-- 
1.8.0.1