Patchwork [RFC,v9,27/27] virtio-blk: add EVENT_IDX support to dataplane

login
register
mail settings
Submitter Stefan Hajnoczi
Date July 18, 2012, 3:07 p.m.
Message ID <1342624074-24650-28-git-send-email-stefanha@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/171735/
State New
Headers show

Comments

Stefan Hajnoczi - July 18, 2012, 3:07 p.m.
This patch adds support for the VIRTIO_RING_F_EVENT_IDX feature for
interrupt mitigation.  virtio-blk doesn't do anything fancy with it so
we may not see a performance improvement.  This patch will allow newer
guest kernels to run successfully.

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 hw/dataplane/vring.h |   65 ++++++++++++++++++++++++++++++++++++++++----------
 hw/virtio-blk.c      |   16 ++++++-------
 2 files changed, 60 insertions(+), 21 deletions(-)

Patch

diff --git a/hw/dataplane/vring.h b/hw/dataplane/vring.h
index bbf8c86..d939a22 100644
--- a/hw/dataplane/vring.h
+++ b/hw/dataplane/vring.h
@@ -14,6 +14,8 @@  typedef struct {
     struct vring vr;                /* virtqueue vring mapped to host memory */
     __u16 last_avail_idx;           /* last processed avail ring index */
     __u16 last_used_idx;            /* last processed used ring index */
+    uint16_t signalled_used;        /* EVENT_IDX state */
+    bool signalled_used_valid;
 } Vring;
 
 static inline unsigned int vring_get_num(Vring *vring)
@@ -63,6 +65,8 @@  static void vring_setup(Vring *vring, VirtIODevice *vdev, int n)
 
     vring->last_avail_idx = 0;
     vring->last_used_idx = 0;
+    vring->signalled_used = 0;
+    vring->signalled_used_valid = false;
 
     fprintf(stderr, "vring physical=%#lx desc=%p avail=%p used=%p\n",
             (unsigned long)virtio_queue_get_ring_addr(vdev, n),
@@ -75,21 +79,48 @@  static bool vring_more_avail(Vring *vring)
 	return vring->vr.avail->idx != vring->last_avail_idx;
 }
 
-/* Hint to disable guest->host notifies */
-static void vring_disable_cb(Vring *vring)
+/* Toggle guest->host notifies */
+static void vring_set_notification(VirtIODevice *vdev, Vring *vring, bool enable)
 {
-    vring->vr.used->flags |= VRING_USED_F_NO_NOTIFY;
+    if (vdev->guest_features & (1 << VIRTIO_RING_F_EVENT_IDX)) {
+        if (enable) {
+            vring_avail_event(&vring->vr) = vring->vr.avail->idx;
+        }
+    } else if (enable) {
+        vring->vr.used->flags &= ~VRING_USED_F_NO_NOTIFY;
+    } else {
+        vring->vr.used->flags |= VRING_USED_F_NO_NOTIFY;
+    }
 }
 
-/* Re-enable guest->host notifies
- *
- * Returns false if there are more descriptors in the ring.
- */
-static bool vring_enable_cb(Vring *vring)
+/* This is stolen from linux/drivers/vhost/vhost.c:vhost_notify() */
+static bool vring_should_notify(VirtIODevice *vdev, Vring *vring)
 {
-    vring->vr.used->flags &= ~VRING_USED_F_NO_NOTIFY;
-    __sync_synchronize(); /* mb() */
-    return !vring_more_avail(vring);
+    uint16_t old, new;
+    bool v;
+    /* Flush out used index updates. This is paired
+     * with the barrier that the Guest executes when enabling
+     * interrupts. */
+    __sync_synchronize(); /* smp_mb() */
+
+    if ((vdev->guest_features & VIRTIO_F_NOTIFY_ON_EMPTY) &&
+        unlikely(vring->vr.avail->idx == vring->last_avail_idx)) {
+        return true;
+    }
+
+    if (!(vdev->guest_features & VIRTIO_RING_F_EVENT_IDX)) {
+        return !(vring->vr.avail->flags & VRING_AVAIL_F_NO_INTERRUPT);
+    }
+    old = vring->signalled_used;
+    v = vring->signalled_used_valid;
+    new = vring->signalled_used = vring->last_used_idx;
+    vring->signalled_used_valid = true;
+
+    if (unlikely(!v)) {
+        return true;
+    }
+
+    return vring_need_event(vring_used_event(&vring->vr), new, old);
 }
 
 /* This is stolen from linux-2.6/drivers/vhost/vhost.c. */
@@ -178,7 +209,7 @@  static bool get_indirect(Vring *vring,
  *
  * Stolen from linux-2.6/drivers/vhost/vhost.c.
  */
-static int vring_pop(Vring *vring,
+static int vring_pop(VirtIODevice *vdev, Vring *vring,
 		      struct iovec iov[], struct iovec *iov_end,
 		      unsigned int *out_num, unsigned int *in_num)
 {
@@ -214,6 +245,10 @@  static int vring_pop(Vring *vring,
 		exit(1);
 	}
 
+	if (vdev->guest_features & (1 << VIRTIO_RING_F_EVENT_IDX)) {
+		vring_avail_event(&vring->vr) = vring->vr.avail->idx;
+	}
+
 	/* When we start there are none of either input nor output. */
 	*out_num = *in_num = 0;
 
@@ -279,6 +314,7 @@  static int vring_pop(Vring *vring,
 static void vring_push(Vring *vring, unsigned int head, int len)
 {
 	struct vring_used_elem *used;
+	uint16_t new;
 
 	/* The virtqueue contains a ring of used buffers.  Get a pointer to the
 	 * next entry in that used ring. */
@@ -289,7 +325,10 @@  static void vring_push(Vring *vring, unsigned int head, int len)
 	/* Make sure buffer is written before we update index. */
 	__sync_synchronize(); /* smp_wmb() */
 
-    vring->vr.used->idx = ++vring->last_used_idx;
+	new = vring->vr.used->idx = ++vring->last_used_idx;
+	if (unlikely((int16_t)(new - vring->signalled_used) < (uint16_t)1)) {
+		vring->signalled_used_valid = false;
+	}
 }
 
 #endif /* VRING_H */
diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index cff2298..a3e3d8c 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -96,11 +96,9 @@  static int get_raw_posix_fd_hack(VirtIOBlock *s)
 /* Raise an interrupt to signal guest, if necessary */
 static void virtio_blk_notify_guest(VirtIOBlock *s)
 {
-    /* Always notify when queue is empty (when feature acknowledge) */
-	if ((s->vring.vr.avail->flags & VRING_AVAIL_F_NO_INTERRUPT) &&
-	    (s->vring.vr.avail->idx != s->vring.last_avail_idx ||
-        !(s->vdev.guest_features & (1 << VIRTIO_F_NOTIFY_ON_EMPTY))))
-		return;
+    if (!vring_should_notify(&s->vdev, &s->vring)) {
+        return;
+    }
 
     /* Try to issue the ioctl() directly for speed */
     if (likely(virtio_queue_try_notify_from_thread(s->vq))) {
@@ -307,10 +305,10 @@  static bool handle_notify(EventHandler *handler)
 
     for (;;) {
         /* Disable guest->host notifies to avoid unnecessary vmexits */
-        vring_disable_cb(&s->vring);
+        vring_set_notification(&s->vdev, &s->vring, false);
 
         for (;;) {
-            head = vring_pop(&s->vring, iov, end, &out_num, &in_num);
+            head = vring_pop(&s->vdev, &s->vring, iov, end, &out_num, &in_num);
             if (head < 0) {
                 break; /* no more requests */
             }
@@ -327,7 +325,9 @@  static bool handle_notify(EventHandler *handler)
             /* Re-enable guest->host notifies and stop processing the vring.
              * But if the guest has snuck in more descriptors, keep processing.
              */
-            if (likely(vring_enable_cb(&s->vring))) {
+            vring_set_notification(&s->vdev, &s->vring, true);
+            __sync_synchronize(); /* smp_mb() */
+            if (!vring_more_avail(&s->vring)) {
                 break;
             }
         } else { /* head == -ENOBUFS, cannot continue since iovecs[] is depleted */