Linux virtualization list
 help / color / mirror / Atom feed
* [PATCH 3/3] virtio: avail_event index support
From: Michael S. Tsirkin @ 2011-05-04 21:02 UTC (permalink / raw)
  To: qemu-devel
  Cc: Krishna Kumar, Carsten Otte, lguest, Shirley Ma, kvm, linux-s390,
	habanero, Heiko Carstens, virtualization, steved,
	Christian Borntraeger, Tom Lendacky, Martin Schwidefsky, linux390
In-Reply-To: <cover.1304542880.git.mst@redhat.com>

Reduce the number of exits utilizing the
avail_event feature.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/vhost_net.c |    6 ++++++
 hw/virtio.c    |   26 ++++++++++++++++++++++++--
 hw/virtio.h    |    7 ++++++-
 3 files changed, 36 insertions(+), 3 deletions(-)

diff --git a/hw/vhost_net.c b/hw/vhost_net.c
index 18e4653..aeb5a84 100644
--- a/hw/vhost_net.c
+++ b/hw/vhost_net.c
@@ -53,6 +53,9 @@ uint64_t vhost_net_get_features(struct vhost_net *net, uint64_t features)
     if (!(net->dev.features & (1ULL << VIRTIO_RING_F_USED_EVENT_IDX))) {
         features &= ~(1ULL << VIRTIO_RING_F_USED_EVENT_IDX);
     }
+    if (!(net->dev.features & (1ULL << VIRTIO_RING_F_AVAIL_EVENT_IDX))) {
+        features &= ~(1ULL << VIRTIO_RING_F_AVAIL_EVENT_IDX);
+    }
     if (!(net->dev.features & (1 << VIRTIO_NET_F_MRG_RXBUF))) {
         features &= ~(1 << VIRTIO_NET_F_MRG_RXBUF);
     }
@@ -71,6 +74,9 @@ void vhost_net_ack_features(struct vhost_net *net, uint64_t features)
     if (features & (1ULL << VIRTIO_RING_F_USED_EVENT_IDX)) {
         net->dev.acked_features |= (1ULL << VIRTIO_RING_F_USED_EVENT_IDX);
     }
+    if (features & (1ULL << VIRTIO_RING_F_AVAIL_EVENT_IDX)) {
+        net->dev.acked_features |= (1ULL << VIRTIO_RING_F_AVAIL_EVENT_IDX);
+    }
     if (features & (1 << VIRTIO_NET_F_MRG_RXBUF)) {
         net->dev.acked_features |= (1 << VIRTIO_NET_F_MRG_RXBUF);
     }
diff --git a/hw/virtio.c b/hw/virtio.c
index e459093..6ae5542 100644
--- a/hw/virtio.c
+++ b/hw/virtio.c
@@ -78,6 +78,9 @@ struct VirtQueue
     /* Last used index value we have signalled on */
     bool signalled_used_valid;
 
+    /* Notification enabled? */
+    bool notification;
+
     int inuse;
 
     uint16_t vector;
@@ -195,12 +198,26 @@ static inline void vring_used_flags_unset_bit(VirtQueue *vq, int mask)
     stw_phys(pa, lduw_phys(pa) & ~mask);
 }
 
+static inline void vring_avail_event(VirtQueue *vq, uint16_t val)
+{
+    target_phys_addr_t pa;
+    if (!vq->notification) {
+        return;
+    }
+    pa = vq->vring.used + offsetof(VRingUsed, ring[vq->vring.num]);
+    stw_phys(pa, val);
+}
+
 void virtio_queue_set_notification(VirtQueue *vq, int enable)
 {
-    if (enable)
+    vq->notification = enable;
+    if (vq->vdev->guest_features & (1ULL << VIRTIO_RING_F_AVAIL_EVENT_IDX)) {
+        vring_avail_event(vq, vq->last_avail_idx);
+    } else if (enable) {
         vring_used_flags_unset_bit(vq, VRING_USED_F_NO_NOTIFY);
-    else
+    } else {
         vring_used_flags_set_bit(vq, VRING_USED_F_NO_NOTIFY);
+    }
 }
 
 int virtio_queue_ready(VirtQueue *vq)
@@ -412,6 +429,9 @@ int virtqueue_pop(VirtQueue *vq, VirtQueueElement *elem)
     max = vq->vring.num;
 
     i = head = virtqueue_get_head(vq, vq->last_avail_idx++);
+    if (vq->vdev->guest_features & (1ULL << VIRTIO_RING_F_AVAIL_EVENT_IDX)) {
+        vring_avail_event(vq, vq->last_avail_idx);
+    }
 
     if (vring_desc_flags(desc_pa, i) & VRING_DESC_F_INDIRECT) {
         if (vring_desc_len(desc_pa, i) % sizeof(VRingDesc)) {
@@ -497,6 +517,7 @@ void virtio_reset(void *opaque)
         vdev->vq[i].vector = VIRTIO_NO_VECTOR;
         vdev->vq[i].signalled_used = 0;
         vdev->vq[i].signalled_used_valid = false;
+        vdev->vq[i].notification = true;
     }
 }
 
@@ -784,6 +805,7 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f)
         vdev->vq[i].pa = qemu_get_be64(f);
         qemu_get_be16s(f, &vdev->vq[i].last_avail_idx);
         vdev->vq[i].signalled_used_valid = false;
+        vdev->vq[i].notification = true;
 
         if (vdev->vq[i].pa) {
             uint16_t nheads;
diff --git a/hw/virtio.h b/hw/virtio.h
index d765946..8c0923a 100644
--- a/hw/virtio.h
+++ b/hw/virtio.h
@@ -54,6 +54,9 @@
 
 /* Enables feature bits 32 to 63 (only really required for virtio_pci). */
 #define VIRTIO_F_FEATURES_HI		31
+/* The Host publishes the avail index for which it expects a kick
+ * at the end of the used ring. Guest should ignore the used->flags field. */
+#define VIRTIO_RING_F_AVAIL_EVENT_IDX	32
 
 /* from Linux's linux/virtio_ring.h */
 
@@ -218,7 +221,9 @@ void virtio_serial_exit(VirtIODevice *vdev);
 	DEFINE_PROP_BIT64("indirect_desc", _state, _field, \
 			  VIRTIO_RING_F_INDIRECT_DESC, true), \
 	DEFINE_PROP_BIT64("used_event", _state, _field, \
-			  VIRTIO_RING_F_USED_EVENT_IDX, true)
+			  VIRTIO_RING_F_USED_EVENT_IDX, true), \
+	DEFINE_PROP_BIT64("avail_event", _state, _field, \
+			  VIRTIO_RING_F_AVAIL_EVENT_IDX, true)
 
 target_phys_addr_t virtio_queue_get_desc_addr(VirtIODevice *vdev, int n);
 target_phys_addr_t virtio_queue_get_avail_addr(VirtIODevice *vdev, int n);
-- 
1.7.5.53.gc233e

^ permalink raw reply related

* [PATCH 2/3] virtio+vhost: used_event feature
From: Michael S. Tsirkin @ 2011-05-04 21:02 UTC (permalink / raw)
  To: qemu-devel
  Cc: Krishna Kumar, Carsten Otte, lguest, Shirley Ma, kvm, linux-s390,
	habanero, Heiko Carstens, virtualization, steved,
	Christian Borntraeger, Tom Lendacky, Martin Schwidefsky, linux390
In-Reply-To: <cover.1304542880.git.mst@redhat.com>

Add support for used_event feature, and utilize it to
reduce the number of interrupts for the guest.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/vhost_net.c |    6 +++++
 hw/virtio.c    |   66 ++++++++++++++++++++++++++++++++++++++++++++++++++------
 hw/virtio.h    |    7 +++++-
 3 files changed, 71 insertions(+), 8 deletions(-)

diff --git a/hw/vhost_net.c b/hw/vhost_net.c
index 7e94f61..18e4653 100644
--- a/hw/vhost_net.c
+++ b/hw/vhost_net.c
@@ -50,6 +50,9 @@ uint64_t vhost_net_get_features(struct vhost_net *net, uint64_t features)
     if (!(net->dev.features & (1 << VIRTIO_RING_F_INDIRECT_DESC))) {
         features &= ~(1 << VIRTIO_RING_F_INDIRECT_DESC);
     }
+    if (!(net->dev.features & (1ULL << VIRTIO_RING_F_USED_EVENT_IDX))) {
+        features &= ~(1ULL << VIRTIO_RING_F_USED_EVENT_IDX);
+    }
     if (!(net->dev.features & (1 << VIRTIO_NET_F_MRG_RXBUF))) {
         features &= ~(1 << VIRTIO_NET_F_MRG_RXBUF);
     }
@@ -65,6 +68,9 @@ void vhost_net_ack_features(struct vhost_net *net, uint64_t features)
     if (features & (1 << VIRTIO_RING_F_INDIRECT_DESC)) {
         net->dev.acked_features |= (1 << VIRTIO_RING_F_INDIRECT_DESC);
     }
+    if (features & (1ULL << VIRTIO_RING_F_USED_EVENT_IDX)) {
+        net->dev.acked_features |= (1ULL << VIRTIO_RING_F_USED_EVENT_IDX);
+    }
     if (features & (1 << VIRTIO_NET_F_MRG_RXBUF)) {
         net->dev.acked_features |= (1 << VIRTIO_NET_F_MRG_RXBUF);
     }
diff --git a/hw/virtio.c b/hw/virtio.c
index b9acbd4..e459093 100644
--- a/hw/virtio.c
+++ b/hw/virtio.c
@@ -72,7 +72,14 @@ struct VirtQueue
     VRing vring;
     target_phys_addr_t pa;
     uint16_t last_avail_idx;
+    /* Last used index value we have signalled on */
+    uint16_t signalled_used;
+
+    /* Last used index value we have signalled on */
+    bool signalled_used_valid;
+
     int inuse;
+
     uint16_t vector;
     void (*handle_output)(VirtIODevice *vdev, VirtQueue *vq);
     VirtIODevice *vdev;
@@ -141,6 +148,11 @@ static inline uint16_t vring_avail_ring(VirtQueue *vq, int i)
     return lduw_phys(pa);
 }
 
+static inline uint16_t vring_used_event(VirtQueue *vq)
+{
+    return vring_avail_ring(vq, vq->vring.num);
+}
+
 static inline void vring_used_ring_id(VirtQueue *vq, int i, uint32_t val)
 {
     target_phys_addr_t pa;
@@ -162,11 +174,11 @@ static uint16_t vring_used_idx(VirtQueue *vq)
     return lduw_phys(pa);
 }
 
-static inline void vring_used_idx_increment(VirtQueue *vq, uint16_t val)
+static inline void vring_used_idx_set(VirtQueue *vq, uint16_t val)
 {
     target_phys_addr_t pa;
     pa = vq->vring.used + offsetof(VRingUsed, idx);
-    stw_phys(pa, vring_used_idx(vq) + val);
+    stw_phys(pa, val);
 }
 
 static inline void vring_used_flags_set_bit(VirtQueue *vq, int mask)
@@ -234,11 +246,16 @@ void virtqueue_fill(VirtQueue *vq, const VirtQueueElement *elem,
 
 void virtqueue_flush(VirtQueue *vq, unsigned int count)
 {
+    uint16_t old, new;
     /* Make sure buffer is written before we update index. */
     wmb();
     trace_virtqueue_flush(vq, count);
-    vring_used_idx_increment(vq, count);
+    old = vring_used_idx(vq);
+    new = old + count;
+    vring_used_idx_set(vq, new);
     vq->inuse -= count;
+    if (unlikely((int16_t)(new - vq->signalled_used) < (uint16_t)(new - old)))
+        vq->signalled_used_valid = false;
 }
 
 void virtqueue_push(VirtQueue *vq, const VirtQueueElement *elem,
@@ -478,6 +495,8 @@ void virtio_reset(void *opaque)
         vdev->vq[i].last_avail_idx = 0;
         vdev->vq[i].pa = 0;
         vdev->vq[i].vector = VIRTIO_NO_VECTOR;
+        vdev->vq[i].signalled_used = 0;
+        vdev->vq[i].signalled_used_valid = false;
     }
 }
 
@@ -629,13 +648,45 @@ void virtio_irq(VirtQueue *vq)
     virtio_notify_vector(vq->vdev, vq->vector);
 }
 
-void virtio_notify(VirtIODevice *vdev, VirtQueue *vq)
+/* Assuming a given event_idx value from the other size, if
+ * we have just incremented index from old to new_idx,
+ * should we trigger an event? */
+static inline int vring_need_event(uint16_t event, uint16_t new, uint16_t old)
+{
+	/* Note: Xen has similar logic for notification hold-off
+	 * in include/xen/interface/io/ring.h with req_event and req_prod
+	 * corresponding to event_idx + 1 and new respectively.
+	 * Note also that req_event and req_prod in Xen start at 1,
+	 * event indexes in virtio start at 0. */
+	return (uint16_t)(new - event - 1) < (uint16_t)(new - old);
+}
+
+static bool vring_notify(VirtIODevice *vdev, VirtQueue *vq)
 {
+    uint16_t old, new;
+    bool v;
     /* Always notify when queue is empty (when feature acknowledge) */
-    if ((vring_avail_flags(vq) & VRING_AVAIL_F_NO_INTERRUPT) &&
-        (!(vdev->guest_features & (1 << VIRTIO_F_NOTIFY_ON_EMPTY)) ||
-         (vq->inuse || vring_avail_idx(vq) != vq->last_avail_idx)))
+    if (((vdev->guest_features & (1 << VIRTIO_F_NOTIFY_ON_EMPTY)) &&
+         !vq->inuse && vring_avail_idx(vq) == vq->last_avail_idx)) {
+        return true;
+    }
+
+    if (!(vdev->guest_features & (1ULL << VIRTIO_RING_F_USED_EVENT_IDX))) {
+        return !(vring_avail_flags(vq) & VRING_AVAIL_F_NO_INTERRUPT);
+    }
+
+    v = vq->signalled_used_valid;
+    vq->signalled_used_valid = true;
+    old = vq->signalled_used;
+    new = vq->signalled_used = vring_used_idx(vq);
+    return !v || vring_need_event(vring_used_event(vq), new, old);
+}
+
+void virtio_notify(VirtIODevice *vdev, VirtQueue *vq)
+{
+    if (!vring_notify(vdev, vq)) {
         return;
+    }
 
     trace_virtio_notify(vdev, vq);
     vdev->isr |= 0x01;
@@ -732,6 +783,7 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f)
         vdev->vq[i].vring.num = qemu_get_be32(f);
         vdev->vq[i].pa = qemu_get_be64(f);
         qemu_get_be16s(f, &vdev->vq[i].last_avail_idx);
+        vdev->vq[i].signalled_used_valid = false;
 
         if (vdev->vq[i].pa) {
             uint16_t nheads;
diff --git a/hw/virtio.h b/hw/virtio.h
index 9517b97..d765946 100644
--- a/hw/virtio.h
+++ b/hw/virtio.h
@@ -46,6 +46,9 @@
 #define VIRTIO_F_NOTIFY_ON_EMPTY        24
 /* We support indirect buffer descriptors */
 #define VIRTIO_RING_F_INDIRECT_DESC     28
+/* The Guest publishes the used index for which it expects an interrupt
+ * at the end of the avail ring. Host should ignore the avail->flags field. */
+#define VIRTIO_RING_F_USED_EVENT_IDX	29
 /* A guest should never accept this.  It implies negotiation is broken. */
 #define VIRTIO_F_BAD_FEATURE		30
 
@@ -213,7 +216,9 @@ void virtio_serial_exit(VirtIODevice *vdev);
 
 #define DEFINE_VIRTIO_COMMON_FEATURES(_state, _field) \
 	DEFINE_PROP_BIT64("indirect_desc", _state, _field, \
-			  VIRTIO_RING_F_INDIRECT_DESC, true)
+			  VIRTIO_RING_F_INDIRECT_DESC, true), \
+	DEFINE_PROP_BIT64("used_event", _state, _field, \
+			  VIRTIO_RING_F_USED_EVENT_IDX, true)
 
 target_phys_addr_t virtio_queue_get_desc_addr(VirtIODevice *vdev, int n);
 target_phys_addr_t virtio_queue_get_avail_addr(VirtIODevice *vdev, int n);
-- 
1.7.5.53.gc233e

^ permalink raw reply related

* [PATCH 1/3] virtio/vhost: support 64 bit features
From: Michael S. Tsirkin @ 2011-05-04 21:01 UTC (permalink / raw)
  To: qemu-devel
  Cc: Krishna Kumar, Carsten Otte, lguest, Shirley Ma, kvm, linux-s390,
	habanero, Heiko Carstens, virtualization, steved,
	Christian Borntraeger, Tom Lendacky, Martin Schwidefsky, linux390
In-Reply-To: <cover.1304542880.git.mst@redhat.com>

Add support for extended feature bits: up to 64 bit.
Only virtio-pci is actually implemented,
s390 and syborg are stubbed out (and untested).

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/qdev-properties.c   |   39 ++++++++++++++++----
 hw/qdev.h              |   10 +++++
 hw/s390-virtio-bus.c   |    5 ++-
 hw/s390-virtio-bus.h   |    2 +-
 hw/syborg_virtio.c     |    7 ++--
 hw/vhost_net.c         |    8 ++--
 hw/vhost_net.h         |    4 +-
 hw/virtio-9p.c         |    2 +-
 hw/virtio-balloon.c    |    2 +-
 hw/virtio-blk.c        |    2 +-
 hw/virtio-blk.h        |    2 +-
 hw/virtio-net.c        |   11 +++---
 hw/virtio-net.h        |   34 +++++++++---------
 hw/virtio-pci.c        |   91 +++++++++++++++++++++++++++++++++++-------------
 hw/virtio-serial-bus.c |    2 +-
 hw/virtio.c            |   24 ++++++++++---
 hw/virtio.h            |   17 +++++----
 17 files changed, 179 insertions(+), 83 deletions(-)

diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 1088a26..a933c9e 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -10,20 +10,35 @@ void *qdev_get_prop_ptr(DeviceState *dev, Property *prop)
     return ptr;
 }
 
-static uint32_t qdev_get_prop_mask(Property *prop)
+static uint64_t qdev_get_prop_mask(Property *prop)
 {
     assert(prop->info->type == PROP_TYPE_BIT);
-    return 0x1 << prop->bitnr;
+    return 0x1ULL << prop->bitnr;
+}
+
+static uint64_t qdev_get_prop_val(DeviceState *dev, Property *prop)
+{
+    assert(prop->info->type == PROP_TYPE_BIT);
+    if (prop->info->size == sizeof(uint32_t)) {
+        return *(uint32_t *)qdev_get_prop_ptr(dev, prop);
+    } else {
+        return *(uint64_t *)qdev_get_prop_ptr(dev, prop);
+    }
 }
 
 static void bit_prop_set(DeviceState *dev, Property *props, bool val)
 {
-    uint32_t *p = qdev_get_prop_ptr(dev, props);
-    uint32_t mask = qdev_get_prop_mask(props);
+    uint64_t p = qdev_get_prop_val(dev, props);
+    uint64_t mask = qdev_get_prop_mask(props);
     if (val)
-        *p |= mask;
+        p |= mask;
     else
-        *p &= ~mask;
+        p &= ~mask;
+    if (props->info->size == sizeof(uint32_t)) {
+        *(uint32_t *)qdev_get_prop_ptr(dev, props) = p;
+    } else {
+        *(uint64_t *)qdev_get_prop_ptr(dev, props) = p;
+    }
 }
 
 static void qdev_prop_cpy(DeviceState *dev, Property *props, void *src)
@@ -51,8 +66,8 @@ static int parse_bit(DeviceState *dev, Property *prop, const char *str)
 
 static int print_bit(DeviceState *dev, Property *prop, char *dest, size_t len)
 {
-    uint32_t *p = qdev_get_prop_ptr(dev, prop);
-    return snprintf(dest, len, (*p & qdev_get_prop_mask(prop)) ? "on" : "off");
+    uint64_t val = qdev_get_prop_val(dev, prop);
+    return snprintf(dest, len, (val & qdev_get_prop_mask(prop)) ? "on" : "off");
 }
 
 PropertyInfo qdev_prop_bit = {
@@ -63,6 +78,14 @@ PropertyInfo qdev_prop_bit = {
     .print = print_bit,
 };
 
+PropertyInfo qdev_prop_bit64 = {
+    .name  = "on/off",
+    .type  = PROP_TYPE_BIT,
+    .size  = sizeof(uint64_t),
+    .parse = parse_bit,
+    .print = print_bit,
+};
+
 /* --- 8bit integer --- */
 
 static int parse_uint8(DeviceState *dev, Property *prop, const char *str)
diff --git a/hw/qdev.h b/hw/qdev.h
index 8a13ec9..e65cab0 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -219,6 +219,7 @@ int do_device_del(Monitor *mon, const QDict *qdict, QObject **ret_data);
 /*** qdev-properties.c ***/
 
 extern PropertyInfo qdev_prop_bit;
+extern PropertyInfo qdev_prop_bit64;
 extern PropertyInfo qdev_prop_uint8;
 extern PropertyInfo qdev_prop_uint16;
 extern PropertyInfo qdev_prop_uint32;
@@ -257,6 +258,15 @@ extern PropertyInfo qdev_prop_pci_devfn;
         .defval    = (bool[]) { (_defval) },                     \
         }
 
+#define DEFINE_PROP_BIT64(_name, _state, _field, _bit, _defval) {  \
+        .name      = (_name),                                    \
+        .info      = &(qdev_prop_bit64),                           \
+        .bitnr    = (_bit),                                      \
+        .offset    = offsetof(_state, _field)                    \
+            + type_check(uint64_t,typeof_field(_state, _field)), \
+        .defval    = (bool[]) { (_defval) },                     \
+        }
+
 #define DEFINE_PROP_UINT8(_n, _s, _f, _d)                       \
     DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_uint8, uint8_t)
 #define DEFINE_PROP_UINT16(_n, _s, _f, _d)                      \
diff --git a/hw/s390-virtio-bus.c b/hw/s390-virtio-bus.c
index 175e5cb..89e8c8e 100644
--- a/hw/s390-virtio-bus.c
+++ b/hw/s390-virtio-bus.c
@@ -310,10 +310,11 @@ static void virtio_s390_notify(void *opaque, uint16_t vector)
     kvm_s390_virtio_irq(s390_cpu_addr2state(0), 0, token);
 }
 
-static unsigned virtio_s390_get_features(void *opaque)
+static uint64_t virtio_s390_get_features(void *opaque)
 {
     VirtIOS390Device *dev = (VirtIOS390Device*)opaque;
-    return dev->host_features;
+    /* TODO: support high 32 bit features */
+    return dev->host_features & 0xFFFFFFFFULL;
 }
 
 /**************** S390 Virtio Bus Device Descriptions *******************/
diff --git a/hw/s390-virtio-bus.h b/hw/s390-virtio-bus.h
index edf6d04..5c851e3 100644
--- a/hw/s390-virtio-bus.h
+++ b/hw/s390-virtio-bus.h
@@ -43,7 +43,7 @@ typedef struct VirtIOS390Device {
     VirtIODevice *vdev;
     BlockConf block;
     NICConf nic;
-    uint32_t host_features;
+    uint64_t host_features;
     virtio_serial_conf serial;
     virtio_net_conf net;
 } VirtIOS390Device;
diff --git a/hw/syborg_virtio.c b/hw/syborg_virtio.c
index ee08c49..b64c357 100644
--- a/hw/syborg_virtio.c
+++ b/hw/syborg_virtio.c
@@ -67,7 +67,7 @@ typedef struct {
     uint32_t int_enable;
     uint32_t id;
     NICConf nic;
-    uint32_t host_features;
+    uint64_t host_features;
     virtio_net_conf net;
 } SyborgVirtIOProxy;
 
@@ -244,10 +244,11 @@ static void syborg_virtio_update_irq(void *opaque, uint16_t vector)
     qemu_set_irq(proxy->irq, level != 0);
 }
 
-static unsigned syborg_virtio_get_features(void *opaque)
+static uint32_t syborg_virtio_get_features(void *opaque)
 {
     SyborgVirtIOProxy *proxy = opaque;
-    return proxy->host_features;
+    /* TODO: support high 32 bit features */
+    return proxy->host_features & 0xFFFFFFFFULL;
 }
 
 static VirtIOBindings syborg_virtio_bindings = {
diff --git a/hw/vhost_net.c b/hw/vhost_net.c
index 420e05f..7e94f61 100644
--- a/hw/vhost_net.c
+++ b/hw/vhost_net.c
@@ -41,7 +41,7 @@ struct vhost_net {
     VLANClientState *vc;
 };
 
-unsigned vhost_net_get_features(struct vhost_net *net, unsigned features)
+uint64_t vhost_net_get_features(struct vhost_net *net, uint64_t features)
 {
     /* Clear features not supported by host kernel. */
     if (!(net->dev.features & (1 << VIRTIO_F_NOTIFY_ON_EMPTY))) {
@@ -56,7 +56,7 @@ unsigned vhost_net_get_features(struct vhost_net *net, unsigned features)
     return features;
 }
 
-void vhost_net_ack_features(struct vhost_net *net, unsigned features)
+void vhost_net_ack_features(struct vhost_net *net, uint64_t features)
 {
     net->dev.acked_features = net->dev.backend_features;
     if (features & (1 << VIRTIO_F_NOTIFY_ON_EMPTY)) {
@@ -219,11 +219,11 @@ void vhost_net_cleanup(struct vhost_net *net)
 {
 }
 
-unsigned vhost_net_get_features(struct vhost_net *net, unsigned features)
+uint64_t vhost_net_get_features(struct vhost_net *net, uint64_t features)
 {
     return features;
 }
-void vhost_net_ack_features(struct vhost_net *net, unsigned features)
+void vhost_net_ack_features(struct vhost_net *net, uint64_t features)
 {
 }
 #endif
diff --git a/hw/vhost_net.h b/hw/vhost_net.h
index 91e40b1..4069258 100644
--- a/hw/vhost_net.h
+++ b/hw/vhost_net.h
@@ -14,7 +14,7 @@ void vhost_net_stop(VHostNetState *net, VirtIODevice *dev);
 
 void vhost_net_cleanup(VHostNetState *net);
 
-unsigned vhost_net_get_features(VHostNetState *net, unsigned features);
-void vhost_net_ack_features(VHostNetState *net, unsigned features);
+uint64_t vhost_net_get_features(VHostNetState *net, uint64_t features);
+void vhost_net_ack_features(VHostNetState *net, uint64_t features);
 
 #endif
diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c
index 7e29535..184691a 100644
--- a/hw/virtio-9p.c
+++ b/hw/virtio-9p.c
@@ -3624,7 +3624,7 @@ static void handle_9p_output(VirtIODevice *vdev, VirtQueue *vq)
     free_pdu(s, pdu);
 }
 
-static uint32_t virtio_9p_get_features(VirtIODevice *vdev, uint32_t features)
+static uint64_t virtio_9p_get_features(VirtIODevice *vdev, uint64_t features)
 {
     features |= 1 << VIRTIO_9P_MOUNT_TAG;
     return features;
diff --git a/hw/virtio-balloon.c b/hw/virtio-balloon.c
index f9add1c..b34adc1 100644
--- a/hw/virtio-balloon.c
+++ b/hw/virtio-balloon.c
@@ -195,7 +195,7 @@ static void virtio_balloon_set_config(VirtIODevice *vdev,
     dev->actual = le32_to_cpu(config.actual);
 }
 
-static uint32_t virtio_balloon_get_features(VirtIODevice *vdev, uint32_t f)
+static uint64_t virtio_balloon_get_features(VirtIODevice *vdev, uint64_t f)
 {
     f |= (1 << VIRTIO_BALLOON_F_STATS_VQ);
     return f;
diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index 91e0394..442ce11 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -466,7 +466,7 @@ static void virtio_blk_update_config(VirtIODevice *vdev, uint8_t *config)
     memcpy(config, &blkcfg, sizeof(struct virtio_blk_config));
 }
 
-static uint32_t virtio_blk_get_features(VirtIODevice *vdev, uint32_t features)
+static uint64_t virtio_blk_get_features(VirtIODevice *vdev, uint64_t features)
 {
     VirtIOBlock *s = to_virtio_blk(vdev);
 
diff --git a/hw/virtio-blk.h b/hw/virtio-blk.h
index fff46da..61104ea 100644
--- a/hw/virtio-blk.h
+++ b/hw/virtio-blk.h
@@ -98,7 +98,7 @@ struct virtio_scsi_inhdr
 #ifdef __linux__
 #define DEFINE_VIRTIO_BLK_FEATURES(_state, _field) \
         DEFINE_VIRTIO_COMMON_FEATURES(_state, _field), \
-        DEFINE_PROP_BIT("scsi", _state, _field, VIRTIO_BLK_F_SCSI, true)
+        DEFINE_PROP_BIT64("scsi", _state, _field, VIRTIO_BLK_F_SCSI, true)
 #else
 #define DEFINE_VIRTIO_BLK_FEATURES(_state, _field) \
         DEFINE_VIRTIO_COMMON_FEATURES(_state, _field)
diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index 6997e02..c242fd1 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -223,7 +223,7 @@ static int peer_has_ufo(VirtIONet *n)
     return n->has_ufo;
 }
 
-static uint32_t virtio_net_get_features(VirtIODevice *vdev, uint32_t features)
+static uint64_t virtio_net_get_features(VirtIODevice *vdev, uint64_t features)
 {
     VirtIONet *n = to_virtio_net(vdev);
 
@@ -258,9 +258,9 @@ static uint32_t virtio_net_get_features(VirtIODevice *vdev, uint32_t features)
     return vhost_net_get_features(tap_get_vhost_net(n->nic->nc.peer), features);
 }
 
-static uint32_t virtio_net_bad_features(VirtIODevice *vdev)
+static uint64_t virtio_net_bad_features(VirtIODevice *vdev)
 {
-    uint32_t features = 0;
+    uint64_t features = 0;
 
     /* Linux kernel 2.6.25.  It understood MAC (as everyone must),
      * but also these: */
@@ -273,7 +273,7 @@ static uint32_t virtio_net_bad_features(VirtIODevice *vdev)
     return features;
 }
 
-static void virtio_net_set_features(VirtIODevice *vdev, uint32_t features)
+static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features)
 {
     VirtIONet *n = to_virtio_net(vdev);
 
@@ -628,7 +628,8 @@ static ssize_t virtio_net_receive(VLANClientState *nc, const uint8_t *buf, size_
                 return -1;
             error_report("virtio-net unexpected empty queue: "
                     "i %zd mergeable %d offset %zd, size %zd, "
-                    "guest hdr len %zd, host hdr len %zd guest features 0x%x",
+                    "guest hdr len %zd, host hdr len %zd "
+                    "guest features 0x%" PRIx64,
                     i, n->mergeable_rx_bufs, offset, size,
                     guest_hdr_len, host_hdr_len, n->vdev.guest_features);
             exit(1);
diff --git a/hw/virtio-net.h b/hw/virtio-net.h
index 8af9a1c..8f8ac7f 100644
--- a/hw/virtio-net.h
+++ b/hw/virtio-net.h
@@ -169,21 +169,21 @@ struct virtio_net_ctrl_mac {
 
 #define DEFINE_VIRTIO_NET_FEATURES(_state, _field) \
         DEFINE_VIRTIO_COMMON_FEATURES(_state, _field), \
-        DEFINE_PROP_BIT("csum", _state, _field, VIRTIO_NET_F_CSUM, true), \
-        DEFINE_PROP_BIT("guest_csum", _state, _field, VIRTIO_NET_F_GUEST_CSUM, true), \
-        DEFINE_PROP_BIT("gso", _state, _field, VIRTIO_NET_F_GSO, true), \
-        DEFINE_PROP_BIT("guest_tso4", _state, _field, VIRTIO_NET_F_GUEST_TSO4, true), \
-        DEFINE_PROP_BIT("guest_tso6", _state, _field, VIRTIO_NET_F_GUEST_TSO6, true), \
-        DEFINE_PROP_BIT("guest_ecn", _state, _field, VIRTIO_NET_F_GUEST_ECN, true), \
-        DEFINE_PROP_BIT("guest_ufo", _state, _field, VIRTIO_NET_F_GUEST_UFO, true), \
-        DEFINE_PROP_BIT("host_tso4", _state, _field, VIRTIO_NET_F_HOST_TSO4, true), \
-        DEFINE_PROP_BIT("host_tso6", _state, _field, VIRTIO_NET_F_HOST_TSO6, true), \
-        DEFINE_PROP_BIT("host_ecn", _state, _field, VIRTIO_NET_F_HOST_ECN, true), \
-        DEFINE_PROP_BIT("host_ufo", _state, _field, VIRTIO_NET_F_HOST_UFO, true), \
-        DEFINE_PROP_BIT("mrg_rxbuf", _state, _field, VIRTIO_NET_F_MRG_RXBUF, true), \
-        DEFINE_PROP_BIT("status", _state, _field, VIRTIO_NET_F_STATUS, true), \
-        DEFINE_PROP_BIT("ctrl_vq", _state, _field, VIRTIO_NET_F_CTRL_VQ, true), \
-        DEFINE_PROP_BIT("ctrl_rx", _state, _field, VIRTIO_NET_F_CTRL_RX, true), \
-        DEFINE_PROP_BIT("ctrl_vlan", _state, _field, VIRTIO_NET_F_CTRL_VLAN, true), \
-        DEFINE_PROP_BIT("ctrl_rx_extra", _state, _field, VIRTIO_NET_F_CTRL_RX_EXTRA, true)
+        DEFINE_PROP_BIT64("csum", _state, _field, VIRTIO_NET_F_CSUM, true), \
+        DEFINE_PROP_BIT64("guest_csum", _state, _field, VIRTIO_NET_F_GUEST_CSUM, true), \
+        DEFINE_PROP_BIT64("gso", _state, _field, VIRTIO_NET_F_GSO, true), \
+        DEFINE_PROP_BIT64("guest_tso4", _state, _field, VIRTIO_NET_F_GUEST_TSO4, true), \
+        DEFINE_PROP_BIT64("guest_tso6", _state, _field, VIRTIO_NET_F_GUEST_TSO6, true), \
+        DEFINE_PROP_BIT64("guest_ecn", _state, _field, VIRTIO_NET_F_GUEST_ECN, true), \
+        DEFINE_PROP_BIT64("guest_ufo", _state, _field, VIRTIO_NET_F_GUEST_UFO, true), \
+        DEFINE_PROP_BIT64("host_tso4", _state, _field, VIRTIO_NET_F_HOST_TSO4, true), \
+        DEFINE_PROP_BIT64("host_tso6", _state, _field, VIRTIO_NET_F_HOST_TSO6, true), \
+        DEFINE_PROP_BIT64("host_ecn", _state, _field, VIRTIO_NET_F_HOST_ECN, true), \
+        DEFINE_PROP_BIT64("host_ufo", _state, _field, VIRTIO_NET_F_HOST_UFO, true), \
+        DEFINE_PROP_BIT64("mrg_rxbuf", _state, _field, VIRTIO_NET_F_MRG_RXBUF, true), \
+        DEFINE_PROP_BIT64("status", _state, _field, VIRTIO_NET_F_STATUS, true), \
+        DEFINE_PROP_BIT64("ctrl_vq", _state, _field, VIRTIO_NET_F_CTRL_VQ, true), \
+        DEFINE_PROP_BIT64("ctrl_rx", _state, _field, VIRTIO_NET_F_CTRL_RX, true), \
+        DEFINE_PROP_BIT64("ctrl_vlan", _state, _field, VIRTIO_NET_F_CTRL_VLAN, true), \
+        DEFINE_PROP_BIT64("ctrl_rx_extra", _state, _field, VIRTIO_NET_F_CTRL_RX_EXTRA, true)
 #endif
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index 5236470..eb86de2 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -64,15 +64,28 @@
 /* Config space size */
 #define VIRTIO_PCI_CONFIG_NOMSI         20
 #define VIRTIO_PCI_CONFIG_MSI           24
-#define VIRTIO_PCI_REGION_SIZE(dev)     (msix_present(dev) ? \
-                                         VIRTIO_PCI_CONFIG_MSI : \
-                                         VIRTIO_PCI_CONFIG_NOMSI)
+#define VIRTIO_PCI_CONFIG_HI            32
+/* An extended 32-bit r/o bitmask of the features supported by the host */
+#define VIRTIO_PCI_HOST_FEATURES_HI     24
+
+/* An extended 32-bit r/w bitmask of features activated by the guest */
+#define VIRTIO_PCI_GUEST_FEATURES_HI    28
+
+#define VIRTIO_PCI_REGION_SIZE(proxy)   (((proxy)->host_features & \
+                                          (1ULL << VIRTIO_F_FEATURES_HI)) ? \
+                                         VIRTIO_PCI_CONFIG_HI : \
+                                         (msix_present(&(proxy)->pci_dev) ? \
+                                          VIRTIO_PCI_CONFIG_MSI : \
+                                          VIRTIO_PCI_CONFIG_NOMSI))
 
 /* The remaining space is defined by each driver as the per-driver
  * configuration space */
-#define VIRTIO_PCI_CONFIG(dev)          (msix_enabled(dev) ? \
-                                         VIRTIO_PCI_CONFIG_MSI : \
-                                         VIRTIO_PCI_CONFIG_NOMSI)
+#define VIRTIO_PCI_CONFIG(proxy)        (((proxy)->vdev->guest_features & \
+                                          (1ULL << VIRTIO_F_FEATURES_HI)) ? \
+                                         VIRTIO_PCI_CONFIG_HI : \
+                                         (msix_enabled(&(proxy)->pci_dev) ? \
+                                          VIRTIO_PCI_CONFIG_MSI : \
+                                          VIRTIO_PCI_CONFIG_NOMSI))
 
 /* Virtio ABI version, if we increment this, we break the guest driver. */
 #define VIRTIO_PCI_ABI_VERSION          0
@@ -106,7 +119,7 @@ typedef struct {
     uint32_t nvectors;
     BlockConf block;
     NICConf nic;
-    uint32_t host_features;
+    uint64_t host_features;
 #ifdef CONFIG_LINUX
     V9fsConf fsconf;
 #endif
@@ -314,11 +327,22 @@ static void virtio_pci_reset(DeviceState *d)
     proxy->flags &= ~VIRTIO_PCI_FLAG_BUS_MASTER_BUG;
 }
 
+static inline uint64_t virtio_replace_hi(uint64_t features, uint32_t hi)
+{
+	return (features & 0xffffffffull) | ((uint64_t)hi) << 32;
+}
+
+static inline uint64_t virtio_replace_lo(uint64_t features, uint32_t lo)
+{
+	return (features & 0xffffffff00000000ull) | lo;
+}
+
 static void virtio_ioport_write(void *opaque, uint32_t addr, uint32_t val)
 {
     VirtIOPCIProxy *proxy = opaque;
     VirtIODevice *vdev = proxy->vdev;
     target_phys_addr_t pa;
+    uint64_t f;
 
     switch (addr) {
     case VIRTIO_PCI_GUEST_FEATURES:
@@ -329,9 +353,15 @@ static void virtio_ioport_write(void *opaque, uint32_t addr, uint32_t val)
 	    else
 		val = 0;
 	}
+        /* Clearing VIRTIO_F_FEATURES_HI clears high 32 bit. */
+	if (val & (1ULL << VIRTIO_F_FEATURES_HI)) {
+	    f = virtio_replace_lo(vdev->guest_features, val);
+	} else {
+	    f = val;
+        }
         if (vdev->set_features)
-            vdev->set_features(vdev, val);
-        vdev->guest_features = val;
+            vdev->set_features(vdev, f);
+        vdev->guest_features = f;
         break;
     case VIRTIO_PCI_QUEUE_PFN:
         pa = (target_phys_addr_t)val << VIRTIO_PCI_QUEUE_ADDR_SHIFT;
@@ -389,6 +419,12 @@ static void virtio_ioport_write(void *opaque, uint32_t addr, uint32_t val)
             val = VIRTIO_NO_VECTOR;
         virtio_queue_set_vector(vdev, vdev->queue_sel, val);
         break;
+    case VIRTIO_PCI_GUEST_FEATURES_HI:
+	f = virtio_replace_hi(vdev->guest_features, val);
+        if (vdev->set_features)
+            vdev->set_features(vdev, f);
+        vdev->guest_features = f;
+        break;
     default:
         error_report("%s: unexpected address 0x%x value 0x%x",
                      __func__, addr, val);
@@ -433,6 +469,11 @@ static uint32_t virtio_ioport_read(VirtIOPCIProxy *proxy, uint32_t addr)
     case VIRTIO_MSI_QUEUE_VECTOR:
         ret = virtio_queue_vector(vdev, vdev->queue_sel);
         break;
+    case VIRTIO_PCI_HOST_FEATURES_HI:
+        ret = proxy->host_features >> 32;
+        break;
+    case VIRTIO_PCI_GUEST_FEATURES_HI:
+        ret = vdev->guest_features >> 32;
     default:
         break;
     }
@@ -443,7 +484,7 @@ static uint32_t virtio_ioport_read(VirtIOPCIProxy *proxy, uint32_t addr)
 static uint32_t virtio_pci_config_readb(void *opaque, uint32_t addr)
 {
     VirtIOPCIProxy *proxy = opaque;
-    uint32_t config = VIRTIO_PCI_CONFIG(&proxy->pci_dev);
+    uint32_t config = VIRTIO_PCI_CONFIG(proxy);
     addr -= proxy->addr;
     if (addr < config)
         return virtio_ioport_read(proxy, addr);
@@ -454,7 +495,7 @@ static uint32_t virtio_pci_config_readb(void *opaque, uint32_t addr)
 static uint32_t virtio_pci_config_readw(void *opaque, uint32_t addr)
 {
     VirtIOPCIProxy *proxy = opaque;
-    uint32_t config = VIRTIO_PCI_CONFIG(&proxy->pci_dev);
+    uint32_t config = VIRTIO_PCI_CONFIG(proxy);
     addr -= proxy->addr;
     if (addr < config)
         return virtio_ioport_read(proxy, addr);
@@ -465,7 +506,7 @@ static uint32_t virtio_pci_config_readw(void *opaque, uint32_t addr)
 static uint32_t virtio_pci_config_readl(void *opaque, uint32_t addr)
 {
     VirtIOPCIProxy *proxy = opaque;
-    uint32_t config = VIRTIO_PCI_CONFIG(&proxy->pci_dev);
+    uint32_t config = VIRTIO_PCI_CONFIG(proxy);
     addr -= proxy->addr;
     if (addr < config)
         return virtio_ioport_read(proxy, addr);
@@ -476,7 +517,7 @@ static uint32_t virtio_pci_config_readl(void *opaque, uint32_t addr)
 static void virtio_pci_config_writeb(void *opaque, uint32_t addr, uint32_t val)
 {
     VirtIOPCIProxy *proxy = opaque;
-    uint32_t config = VIRTIO_PCI_CONFIG(&proxy->pci_dev);
+    uint32_t config = VIRTIO_PCI_CONFIG(proxy);
     addr -= proxy->addr;
     if (addr < config) {
         virtio_ioport_write(proxy, addr, val);
@@ -489,7 +530,7 @@ static void virtio_pci_config_writeb(void *opaque, uint32_t addr, uint32_t val)
 static void virtio_pci_config_writew(void *opaque, uint32_t addr, uint32_t val)
 {
     VirtIOPCIProxy *proxy = opaque;
-    uint32_t config = VIRTIO_PCI_CONFIG(&proxy->pci_dev);
+    uint32_t config = VIRTIO_PCI_CONFIG(proxy);
     addr -= proxy->addr;
     if (addr < config) {
         virtio_ioport_write(proxy, addr, val);
@@ -502,7 +543,7 @@ static void virtio_pci_config_writew(void *opaque, uint32_t addr, uint32_t val)
 static void virtio_pci_config_writel(void *opaque, uint32_t addr, uint32_t val)
 {
     VirtIOPCIProxy *proxy = opaque;
-    uint32_t config = VIRTIO_PCI_CONFIG(&proxy->pci_dev);
+    uint32_t config = VIRTIO_PCI_CONFIG(proxy);
     addr -= proxy->addr;
     if (addr < config) {
         virtio_ioport_write(proxy, addr, val);
@@ -517,7 +558,7 @@ static void virtio_map(PCIDevice *pci_dev, int region_num,
 {
     VirtIOPCIProxy *proxy = container_of(pci_dev, VirtIOPCIProxy, pci_dev);
     VirtIODevice *vdev = proxy->vdev;
-    unsigned config_len = VIRTIO_PCI_REGION_SIZE(pci_dev) + vdev->config_len;
+    unsigned config_len = VIRTIO_PCI_REGION_SIZE(proxy) + vdev->config_len;
 
     proxy->addr = addr;
 
@@ -551,7 +592,7 @@ static void virtio_write_config(PCIDevice *pci_dev, uint32_t address,
     msix_write_config(pci_dev, address, val, len);
 }
 
-static unsigned virtio_pci_get_features(void *opaque)
+static uint64_t virtio_pci_get_features(void *opaque)
 {
     VirtIOPCIProxy *proxy = opaque;
     return proxy->host_features;
@@ -788,13 +829,6 @@ static void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev,
 
     proxy->pci_dev.config_write = virtio_write_config;
 
-    size = VIRTIO_PCI_REGION_SIZE(&proxy->pci_dev) + vdev->config_len;
-    if (size & (size-1))
-        size = 1 << qemu_fls(size);
-
-    pci_register_bar(&proxy->pci_dev, 0, size, PCI_BASE_ADDRESS_SPACE_IO,
-                           virtio_map);
-
     if (!kvm_has_many_ioeventfds()) {
         proxy->flags &= ~VIRTIO_PCI_FLAG_USE_IOEVENTFD;
     }
@@ -803,6 +837,15 @@ static void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev,
     proxy->host_features |= 0x1 << VIRTIO_F_NOTIFY_ON_EMPTY;
     proxy->host_features |= 0x1 << VIRTIO_F_BAD_FEATURE;
     proxy->host_features = vdev->get_features(vdev, proxy->host_features);
+    if (proxy->host_features & 0xffffffff00000000ull) {
+        proxy->host_features |= 0x1ULL << VIRTIO_F_FEATURES_HI;
+    }
+    size = VIRTIO_PCI_REGION_SIZE(proxy) + vdev->config_len;
+    if (size & (size-1))
+        size = 1 << qemu_fls(size);
+
+    pci_register_bar(&proxy->pci_dev, 0, size, PCI_BASE_ADDRESS_SPACE_IO,
+                           virtio_map);
 }
 
 static int virtio_blk_init_pci(PCIDevice *pci_dev)
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index 6227379..76acd02 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -461,7 +461,7 @@ static void handle_input(VirtIODevice *vdev, VirtQueue *vq)
 {
 }
 
-static uint32_t get_features(VirtIODevice *vdev, uint32_t features)
+static uint64_t get_features(VirtIODevice *vdev, uint64_t features)
 {
     VirtIOSerial *vser;
 
diff --git a/hw/virtio.c b/hw/virtio.c
index 31bd9e3..b9acbd4 100644
--- a/hw/virtio.c
+++ b/hw/virtio.c
@@ -653,6 +653,8 @@ void virtio_notify_config(VirtIODevice *vdev)
 
 void virtio_save(VirtIODevice *vdev, QEMUFile *f)
 {
+    uint32_t guest_features_hi = vdev->guest_features >> 32;
+    uint32_t guest_features_lo = vdev->guest_features;
     int i;
 
     if (vdev->binding->save_config)
@@ -661,7 +663,10 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f)
     qemu_put_8s(f, &vdev->status);
     qemu_put_8s(f, &vdev->isr);
     qemu_put_be16s(f, &vdev->queue_sel);
-    qemu_put_be32s(f, &vdev->guest_features);
+    qemu_put_be32s(f, &guest_features_lo);
+    if (guest_features_lo & (1ULL << VIRTIO_F_FEATURES_HI)) {
+        qemu_put_be32s(f, &guest_features_hi);
+    }
     qemu_put_be32(f, vdev->config_len);
     qemu_put_buffer(f, vdev->config, vdev->config_len);
 
@@ -687,8 +692,9 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f)
 int virtio_load(VirtIODevice *vdev, QEMUFile *f)
 {
     int num, i, ret;
-    uint32_t features;
-    uint32_t supported_features =
+    uint32_t features_hi, features_lo;
+    uint64_t features;
+    uint64_t supported_features =
         vdev->binding->get_features(vdev->binding_opaque);
 
     if (vdev->binding->load_config) {
@@ -700,9 +706,17 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f)
     qemu_get_8s(f, &vdev->status);
     qemu_get_8s(f, &vdev->isr);
     qemu_get_be16s(f, &vdev->queue_sel);
-    qemu_get_be32s(f, &features);
+    qemu_get_be32s(f, &features_lo);
+    if (features_lo & (1ULL << VIRTIO_F_FEATURES_HI)) {
+        qemu_get_be32s(f, &features_hi);
+    } else {
+        features_hi = 0;
+    }
+    features = ((uint64_t)features_hi) << 32 | features_lo;
+    
     if (features & ~supported_features) {
-        error_report("Features 0x%x unsupported. Allowed features: 0x%x",
+        error_report("Features 0x%" PRIx64 " unsupported. "
+                     "Allowed features: 0x%" PRIx64,
                      features, supported_features);
         return -1;
     }
diff --git a/hw/virtio.h b/hw/virtio.h
index bc72289..9517b97 100644
--- a/hw/virtio.h
+++ b/hw/virtio.h
@@ -49,6 +49,9 @@
 /* A guest should never accept this.  It implies negotiation is broken. */
 #define VIRTIO_F_BAD_FEATURE		30
 
+/* Enables feature bits 32 to 63 (only really required for virtio_pci). */
+#define VIRTIO_F_FEATURES_HI		31
+
 /* from Linux's linux/virtio_ring.h */
 
 /* This marks a buffer as continuing via the next field. */
@@ -93,7 +96,7 @@ typedef struct {
     int (*load_config)(void * opaque, QEMUFile *f);
     int (*load_queue)(void * opaque, int n, QEMUFile *f);
     int (*load_done)(void * opaque, QEMUFile *f);
-    unsigned (*get_features)(void * opaque);
+    uint64_t (*get_features)(void * opaque);
     bool (*query_guest_notifiers)(void * opaque);
     int (*set_guest_notifiers)(void * opaque, bool assigned);
     int (*set_host_notifier)(void * opaque, int n, bool assigned);
@@ -110,14 +113,14 @@ struct VirtIODevice
     uint8_t status;
     uint8_t isr;
     uint16_t queue_sel;
-    uint32_t guest_features;
+    uint64_t guest_features;
     size_t config_len;
     void *config;
     uint16_t config_vector;
     int nvectors;
-    uint32_t (*get_features)(VirtIODevice *vdev, uint32_t requested_features);
-    uint32_t (*bad_features)(VirtIODevice *vdev);
-    void (*set_features)(VirtIODevice *vdev, uint32_t val);
+    uint64_t (*get_features)(VirtIODevice *vdev, uint64_t requested_features);
+    uint64_t (*bad_features)(VirtIODevice *vdev);
+    void (*set_features)(VirtIODevice *vdev, uint64_t val);
     void (*get_config)(VirtIODevice *vdev, uint8_t *config);
     void (*set_config)(VirtIODevice *vdev, const uint8_t *config);
     void (*reset)(VirtIODevice *vdev);
@@ -209,8 +212,8 @@ void virtio_blk_exit(VirtIODevice *vdev);
 void virtio_serial_exit(VirtIODevice *vdev);
 
 #define DEFINE_VIRTIO_COMMON_FEATURES(_state, _field) \
-	DEFINE_PROP_BIT("indirect_desc", _state, _field, \
-			VIRTIO_RING_F_INDIRECT_DESC, true)
+	DEFINE_PROP_BIT64("indirect_desc", _state, _field, \
+			  VIRTIO_RING_F_INDIRECT_DESC, true)
 
 target_phys_addr_t virtio_queue_get_desc_addr(VirtIODevice *vdev, int n);
 target_phys_addr_t virtio_queue_get_avail_addr(VirtIODevice *vdev, int n);
-- 
1.7.5.53.gc233e

^ permalink raw reply related

* [PATCH 0/3] virtio-net: 64 bit features, event index
From: Michael S. Tsirkin @ 2011-05-04 21:01 UTC (permalink / raw)
  To: qemu-devel
  Cc: Krishna Kumar, Carsten Otte, lguest, Shirley Ma, kvm, linux-s390,
	habanero, Heiko Carstens, virtualization, steved,
	Christian Borntraeger, Tom Lendacky, Martin Schwidefsky, linux390

OK, here's a patch that implements the virtio spec update that I
sent earlier. It supercedes the PUBLISH_USED_IDX patches
I sent out earlier.

Support is added in both userspace and vhost-net.

I see nice performance improvements: e.g. from 12 to 18 Gbit/s host
to guest with netperf, but did not spend a lot of time testing
performance. I hope others will try this out and report.

Note: there are 2 new features, reducing the number
of interrupts (used_event) and of exits (avail_event).

If you see issues or are just curious, you can
turn them off separately. For example:

-global virtio-net-pci.avail_event=on -global
virtio-net-pci.used_event=on -global virtio-blk-pci.avail_event=off
-global virtio-blk-pci.used_event=off

Also, it's possible to try both vhost-net and virtio-net.

Michael S. Tsirkin (3):
  virtio/vhost: support 64 bit features
  virtio+vhost: used_event feature
  virtio: avail_event index support

 hw/qdev-properties.c   |   39 +++++++++++++---
 hw/qdev.h              |   10 ++++
 hw/s390-virtio-bus.c   |    5 +-
 hw/s390-virtio-bus.h   |    2 +-
 hw/syborg_virtio.c     |    7 ++-
 hw/vhost_net.c         |   20 +++++++--
 hw/vhost_net.h         |    4 +-
 hw/virtio-9p.c         |    2 +-
 hw/virtio-balloon.c    |    2 +-
 hw/virtio-blk.c        |    2 +-
 hw/virtio-blk.h        |    2 +-
 hw/virtio-net.c        |   11 +++--
 hw/virtio-net.h        |   34 +++++++-------
 hw/virtio-pci.c        |   91 +++++++++++++++++++++++++++----------
 hw/virtio-serial-bus.c |    2 +-
 hw/virtio.c            |  116 ++++++++++++++++++++++++++++++++++++++++++------
 hw/virtio.h            |   27 ++++++++---
 17 files changed, 284 insertions(+), 92 deletions(-)

-- 
1.7.5.53.gc233e

^ permalink raw reply

* [PATCH 18/18] virtio_net: limit xmit polling
From: Michael S. Tsirkin @ 2011-05-04 20:53 UTC (permalink / raw)
  Cc: Krishna Kumar, Carsten Otte, lguest, Shirley Ma, kvm, linux-s390,
	netdev, habanero, Heiko Carstens, linux-kernel, virtualization,
	steved, Christian Borntraeger, Tom Lendacky, Martin Schwidefsky,
	linux390
In-Reply-To: <cover.1304541918.git.mst@redhat.com>

Current code might introduce a lot of latency variation
if there are many pending bufs at the time we
attempt to transmit a new one. This is bad for
real-time applications and can't be good for TCP either.

Free up just enough to both clean up all buffers
eventually and to be able to xmit the next packet.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/net/virtio_net.c |   18 +++++++++++-------
 1 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index f33c92b..9982bd7 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -509,17 +509,23 @@ again:
 	return received;
 }
 
-static void free_old_xmit_skbs(struct virtnet_info *vi)
+static bool free_old_xmit_skbs(struct virtnet_info *vi, int capacity)
 {
 	struct sk_buff *skb;
 	unsigned int len;
+	bool c;
+	/* We try to free up at least 2 skbs per one sent, so that we'll get
+	 * all of the memory back if they are used fast enough. */
+	int n = 2;
 
-	while ((skb = virtqueue_get_buf(vi->svq, &len)) != NULL) {
+	while ((c = virtqueue_get_capacity(vi->svq) >= capacity) && --n > 0 &&
+	       (skb = virtqueue_get_buf(vi->svq, &len)) != NULL) {
 		pr_debug("Sent skb %p\n", skb);
 		vi->dev->stats.tx_bytes += skb->len;
 		vi->dev->stats.tx_packets++;
 		dev_kfree_skb_any(skb);
 	}
+	return c;
 }
 
 static int xmit_skb(struct virtnet_info *vi, struct sk_buff *skb)
@@ -574,8 +580,8 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
 	struct virtnet_info *vi = netdev_priv(dev);
 	int capacity;
 
-	/* Free up any pending old buffers before queueing new ones. */
-	free_old_xmit_skbs(vi);
+	/* Free enough pending old buffers to enable queueing new ones. */
+	free_old_xmit_skbs(vi, 2+MAX_SKB_FRAGS);
 
 	/* Try to transmit */
 	capacity = xmit_skb(vi, skb);
@@ -609,9 +615,7 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
 		netif_stop_queue(dev);
 		if (unlikely(!virtqueue_enable_cb_delayed(vi->svq))) {
 			/* More just got used, free them then recheck. */
-			free_old_xmit_skbs(vi);
-			capacity = virtqueue_get_capacity(vi->svq);
-			if (capacity >= 2+MAX_SKB_FRAGS) {
+			if (!likely(free_old_xmit_skbs(vi, 2+MAX_SKB_FRAGS))) {
 				netif_start_queue(dev);
 				virtqueue_disable_cb(vi->svq);
 			}
-- 
1.7.5.53.gc233e

^ permalink raw reply related

* [PATCH 17/18] virtio_net: fix TX capacity checks using new API
From: Michael S. Tsirkin @ 2011-05-04 20:53 UTC (permalink / raw)
  Cc: Krishna Kumar, Carsten Otte, lguest, Shirley Ma, kvm, linux-s390,
	netdev, habanero, Heiko Carstens, linux-kernel, virtualization,
	steved, Christian Borntraeger, Tom Lendacky, Martin Schwidefsky,
	linux390
In-Reply-To: <cover.1304541918.git.mst@redhat.com>

virtio net uses the number of sg entries to
check for TX ring capacity freed. But this
gives incorrect results when indirect buffers
are used. Use the new capacity API instead.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/net/virtio_net.c |    9 ++++-----
 1 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index f685324..f33c92b 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -509,19 +509,17 @@ again:
 	return received;
 }
 
-static unsigned int free_old_xmit_skbs(struct virtnet_info *vi)
+static void free_old_xmit_skbs(struct virtnet_info *vi)
 {
 	struct sk_buff *skb;
-	unsigned int len, tot_sgs = 0;
+	unsigned int len;
 
 	while ((skb = virtqueue_get_buf(vi->svq, &len)) != NULL) {
 		pr_debug("Sent skb %p\n", skb);
 		vi->dev->stats.tx_bytes += skb->len;
 		vi->dev->stats.tx_packets++;
-		tot_sgs += skb_vnet_hdr(skb)->num_sg;
 		dev_kfree_skb_any(skb);
 	}
-	return tot_sgs;
 }
 
 static int xmit_skb(struct virtnet_info *vi, struct sk_buff *skb)
@@ -611,7 +609,8 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
 		netif_stop_queue(dev);
 		if (unlikely(!virtqueue_enable_cb_delayed(vi->svq))) {
 			/* More just got used, free them then recheck. */
-			capacity += free_old_xmit_skbs(vi);
+			free_old_xmit_skbs(vi);
+			capacity = virtqueue_get_capacity(vi->svq);
 			if (capacity >= 2+MAX_SKB_FRAGS) {
 				netif_start_queue(dev);
 				virtqueue_disable_cb(vi->svq);
-- 
1.7.5.53.gc233e

^ permalink raw reply related

* [PATCH 16/18] virtio_ring: Add capacity check API
From: Michael S. Tsirkin @ 2011-05-04 20:52 UTC (permalink / raw)
  Cc: Krishna Kumar, Carsten Otte, lguest, Shirley Ma, kvm, linux-s390,
	netdev, habanero, Heiko Carstens, linux-kernel, virtualization,
	steved, Christian Borntraeger, Tom Lendacky, Martin Schwidefsky,
	linux390
In-Reply-To: <cover.1304541918.git.mst@redhat.com>

>From: Shirley Ma <mashirle@us.ibm.com>

Signed-off-by: Shirley Ma <xma@us.ibm.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---

I'm not sure who wrote this first anymore :)
But it's a simple patch.

 drivers/virtio/virtio_ring.c |    8 ++++++++
 include/linux/virtio.h       |    5 +++++
 2 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 3a70d70..57bf9d5 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -365,6 +365,14 @@ void *virtqueue_get_buf(struct virtqueue *_vq, unsigned int *len)
 }
 EXPORT_SYMBOL_GPL(virtqueue_get_buf);
 
+int virtqueue_get_capacity(struct virtqueue *_vq)
+{
+	struct vring_virtqueue *vq = to_vvq(_vq);
+
+	return vq->num_free;
+}
+EXPORT_SYMBOL_GPL(virtqueue_get_capacity);
+
 void virtqueue_disable_cb(struct virtqueue *_vq)
 {
 	struct vring_virtqueue *vq = to_vvq(_vq);
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index 5151fd1..944ebcd 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -42,6 +42,9 @@ struct virtqueue {
  *	vq: the struct virtqueue we're talking about.
  *	len: the length written into the buffer
  *	Returns NULL or the "data" token handed to add_buf.
+ * virtqueue_get_capacity: get the current capacity of the queue
+ *	vq: the struct virtqueue we're talking about.
+ *	Returns remaining capacity of the queue.
  * virtqueue_disable_cb: disable callbacks
  *	vq: the struct virtqueue we're talking about.
  *	Note that this is not necessarily synchronous, hence unreliable and only
@@ -89,6 +92,8 @@ void virtqueue_kick(struct virtqueue *vq);
 
 void *virtqueue_get_buf(struct virtqueue *vq, unsigned int *len);
 
+int virtqueue_get_capacity(struct virtqueue *vq);
+
 void virtqueue_disable_cb(struct virtqueue *vq);
 
 bool virtqueue_enable_cb(struct virtqueue *vq);
-- 
1.7.5.53.gc233e

^ permalink raw reply related

* [PATCH 15/18] virtio_net: delay TX callbacks
From: Michael S. Tsirkin @ 2011-05-04 20:52 UTC (permalink / raw)
  Cc: Krishna Kumar, Carsten Otte, lguest, Shirley Ma, kvm, linux-s390,
	netdev, habanero, Heiko Carstens, linux-kernel, virtualization,
	steved, Christian Borntraeger, Tom Lendacky, Martin Schwidefsky,
	linux390
In-Reply-To: <cover.1304541918.git.mst@redhat.com>

Ask for delayed callbacks on TX ring full, to give the
other side more of a chance to make progress.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/net/virtio_net.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 0cb0b06..f685324 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -609,7 +609,7 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
 	 * before it gets out of hand.  Naturally, this wastes entries. */
 	if (capacity < 2+MAX_SKB_FRAGS) {
 		netif_stop_queue(dev);
-		if (unlikely(!virtqueue_enable_cb(vi->svq))) {
+		if (unlikely(!virtqueue_enable_cb_delayed(vi->svq))) {
 			/* More just got used, free them then recheck. */
 			capacity += free_old_xmit_skbs(vi);
 			if (capacity >= 2+MAX_SKB_FRAGS) {
-- 
1.7.5.53.gc233e

^ permalink raw reply related

* [PATCH 14/18] virtio: add api for delayed callbacks
From: Michael S. Tsirkin @ 2011-05-04 20:52 UTC (permalink / raw)
  Cc: Krishna Kumar, Carsten Otte, lguest, Shirley Ma, kvm, linux-s390,
	netdev, habanero, Heiko Carstens, linux-kernel, virtualization,
	steved, Christian Borntraeger, Tom Lendacky, Martin Schwidefsky,
	linux390
In-Reply-To: <cover.1304541918.git.mst@redhat.com>

Add an API that tells the other side that callbacks
should be delayed until a lot of work has been done.
Implement using the new used_event feature.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/virtio/virtio_ring.c |   27 +++++++++++++++++++++++++++
 include/linux/virtio.h       |    9 +++++++++
 2 files changed, 36 insertions(+), 0 deletions(-)

diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 262dfe6..3a70d70 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -397,6 +397,33 @@ bool virtqueue_enable_cb(struct virtqueue *_vq)
 }
 EXPORT_SYMBOL_GPL(virtqueue_enable_cb);
 
+bool virtqueue_enable_cb_delayed(struct virtqueue *_vq)
+{
+	struct vring_virtqueue *vq = to_vvq(_vq);
+	int bufs;
+
+	START_USE(vq);
+
+	/* We optimistically turn back on interrupts, then check if there was
+	 * more to do. */
+	/* Depending on the VIRTIO_RING_F_USED_EVENT_IDX feature, we need to
+	 * either clear the flags bit or point the event index at the next
+	 * entry. Always do both to keep code simple. */
+	vq->vring.avail->flags &= ~VRING_AVAIL_F_NO_INTERRUPT;
+	/* TODO: tune this threshold */
+	bufs = (vq->vring.avail->idx - vq->last_used_idx) * 3 / 4;
+	vring_used_event(&vq->vring) = vq->last_used_idx + bufs;
+	virtio_mb();
+	if (unlikely(vq->vring.used->idx - vq->last_used_idx > bufs)) {
+		END_USE(vq);
+		return false;
+	}
+
+	END_USE(vq);
+	return true;
+}
+EXPORT_SYMBOL_GPL(virtqueue_enable_cb_delayed);
+
 void *virtqueue_detach_unused_buf(struct virtqueue *_vq)
 {
 	struct vring_virtqueue *vq = to_vvq(_vq);
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index 718336b..5151fd1 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -51,6 +51,13 @@ struct virtqueue {
  *	This re-enables callbacks; it returns "false" if there are pending
  *	buffers in the queue, to detect a possible race between the driver
  *	checking for more work, and enabling callbacks.
+ * virtqueue_enable_cb_delayed: restart callbacks after disable_cb.
+ *	vq: the struct virtqueue we're talking about.
+ *	This re-enables callbacks but hints to the other side to delay
+ *	interrupts until most of the available buffers have been processed;
+ *	it returns "false" if there are many pending buffers in the queue,
+ *	to detect a possible race between the driver checking for more work,
+ *	and enabling callbacks.
  * virtqueue_detach_unused_buf: detach first unused buffer
  * 	vq: the struct virtqueue we're talking about.
  * 	Returns NULL or the "data" token handed to add_buf
@@ -86,6 +93,8 @@ void virtqueue_disable_cb(struct virtqueue *vq);
 
 bool virtqueue_enable_cb(struct virtqueue *vq);
 
+bool virtqueue_enable_cb_delayed(struct virtqueue *vq);
+
 void *virtqueue_detach_unused_buf(struct virtqueue *vq);
 
 /**
-- 
1.7.5.53.gc233e

^ permalink raw reply related

* [PATCH 13/18] virtio_test: avail_event index support
From: Michael S. Tsirkin @ 2011-05-04 20:52 UTC (permalink / raw)
  Cc: Krishna Kumar, Carsten Otte, lguest, Shirley Ma, kvm, linux-s390,
	netdev, habanero, Heiko Carstens, linux-kernel, virtualization,
	steved, Christian Borntraeger, Tom Lendacky, Martin Schwidefsky,
	linux390
In-Reply-To: <cover.1304541918.git.mst@redhat.com>

Add ability to test the new avail_event feature,
enable by default.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 tools/virtio/virtio_test.c |   17 +++++++++++++++--
 1 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/tools/virtio/virtio_test.c b/tools/virtio/virtio_test.c
index 157ec68..8adf55d 100644
--- a/tools/virtio/virtio_test.c
+++ b/tools/virtio/virtio_test.c
@@ -202,6 +202,14 @@ const struct option longopts[] = {
 		.val = 'h',
 	},
 	{
+		.name = "avail-event-idx",
+		.val = 'A',
+	},
+	{
+		.name = "no-avail-event-idx",
+		.val = 'a',
+	},
+	{
 		.name = "indirect",
 		.val = 'I',
 	},
@@ -224,7 +232,8 @@ const struct option longopts[] = {
 static void help()
 {
 	fprintf(stderr, "Usage: virtio_test [--help]"
-		" [--no-indirect] "
+		" [--no-indirect]"
+		" [--no-avail-event-idx]"
 		" [--no-used-event-idx]\n");
 }
 
@@ -232,7 +241,8 @@ int main(int argc, char **argv)
 {
 	struct vdev_info dev;
 	unsigned long long features = (1ULL << VIRTIO_RING_F_INDIRECT_DESC) |
-		(1ULL << VIRTIO_RING_F_USED_EVENT_IDX);
+		(1ULL << VIRTIO_RING_F_USED_EVENT_IDX) |
+		(1ULL << VIRTIO_RING_F_AVAIL_EVENT_IDX);
 	int o;
 
 	for (;;) {
@@ -243,6 +253,9 @@ int main(int argc, char **argv)
 		case '?':
 			help();
 			exit(2);
+		case 'a':
+			features &= ~(1ULL << VIRTIO_RING_F_AVAIL_EVENT_IDX);
+			break;
 		case 'h':
 			help();
 			goto done;
-- 
1.7.5.53.gc233e

^ permalink raw reply related

* [PATCH 12/18] virtio_test: support used_event index
From: Michael S. Tsirkin @ 2011-05-04 20:52 UTC (permalink / raw)
  Cc: Krishna Kumar, Carsten Otte, lguest, Shirley Ma, kvm, linux-s390,
	netdev, habanero, Heiko Carstens, linux-kernel, virtualization,
	steved, Christian Borntraeger, Tom Lendacky, Martin Schwidefsky,
	linux390
In-Reply-To: <cover.1304541918.git.mst@redhat.com>

Add ability to test the new used_event feature,
enable by default.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 tools/virtio/virtio_test.c |   18 ++++++++++++++++--
 1 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/tools/virtio/virtio_test.c b/tools/virtio/virtio_test.c
index 9e65e6d..157ec68 100644
--- a/tools/virtio/virtio_test.c
+++ b/tools/virtio/virtio_test.c
@@ -210,18 +210,29 @@ const struct option longopts[] = {
 		.val = 'i',
 	},
 	{
+		.name = "used-event-idx",
+		.val = 'U',
+	},
+	{
+		.name = "no-used-event-idx",
+		.val = 'u',
+	},
+	{
 	}
 };
 
 static void help()
 {
-	fprintf(stderr, "Usage: virtio_test [--help] [--no-indirect]\n");
+	fprintf(stderr, "Usage: virtio_test [--help]"
+		" [--no-indirect] "
+		" [--no-used-event-idx]\n");
 }
 
 int main(int argc, char **argv)
 {
 	struct vdev_info dev;
-	unsigned long long features = 1ULL << VIRTIO_RING_F_INDIRECT_DESC;
+	unsigned long long features = (1ULL << VIRTIO_RING_F_INDIRECT_DESC) |
+		(1ULL << VIRTIO_RING_F_USED_EVENT_IDX);
 	int o;
 
 	for (;;) {
@@ -238,6 +249,9 @@ int main(int argc, char **argv)
 		case 'i':
 			features &= ~(1ULL << VIRTIO_RING_F_INDIRECT_DESC);
 			break;
+		case 'u':
+			features &= ~(1ULL << VIRTIO_RING_F_USED_EVENT_IDX);
+			break;
 		default:
 			assert(0);
 			break;
-- 
1.7.5.53.gc233e

^ permalink raw reply related

* [PATCH 11/18] vhost: support avail_event idx
From: Michael S. Tsirkin @ 2011-05-04 20:52 UTC (permalink / raw)
  Cc: Krishna Kumar, Carsten Otte, lguest, Shirley Ma, kvm, linux-s390,
	netdev, habanero, Heiko Carstens, linux-kernel, virtualization,
	steved, Christian Borntraeger, Tom Lendacky, Martin Schwidefsky,
	linux390
In-Reply-To: <cover.1304541918.git.mst@redhat.com>

Add support for the new avail_event feature in vhost_net
and vhost test modules.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/vhost/net.c   |   12 ++++----
 drivers/vhost/test.c  |    6 ++--
 drivers/vhost/vhost.c |   65 +++++++++++++++++++++++++++++++++++++------------
 drivers/vhost/vhost.h |   17 +++++++------
 4 files changed, 67 insertions(+), 33 deletions(-)

diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 2f7c76a..e224a92 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -144,7 +144,7 @@ static void handle_tx(struct vhost_net *net)
 	}
 
 	mutex_lock(&vq->mutex);
-	vhost_disable_notify(vq);
+	vhost_disable_notify(&net->dev, vq);
 
 	if (wmem < sock->sk->sk_sndbuf / 2)
 		tx_poll_stop(net);
@@ -166,8 +166,8 @@ static void handle_tx(struct vhost_net *net)
 				set_bit(SOCK_ASYNC_NOSPACE, &sock->flags);
 				break;
 			}
-			if (unlikely(vhost_enable_notify(vq))) {
-				vhost_disable_notify(vq);
+			if (unlikely(vhost_enable_notify(&net->dev, vq))) {
+				vhost_disable_notify(&net->dev, vq);
 				continue;
 			}
 			break;
@@ -315,7 +315,7 @@ static void handle_rx(struct vhost_net *net)
 		return;
 
 	mutex_lock(&vq->mutex);
-	vhost_disable_notify(vq);
+	vhost_disable_notify(&net->dev, vq);
 	vhost_hlen = vq->vhost_hlen;
 	sock_hlen = vq->sock_hlen;
 
@@ -334,10 +334,10 @@ static void handle_rx(struct vhost_net *net)
 			break;
 		/* OK, now we need to know about added descriptors. */
 		if (!headcount) {
-			if (unlikely(vhost_enable_notify(vq))) {
+			if (unlikely(vhost_enable_notify(&net->dev, vq))) {
 				/* They have slipped one in as we were
 				 * doing that: check again. */
-				vhost_disable_notify(vq);
+				vhost_disable_notify(&net->dev, vq);
 				continue;
 			}
 			/* Nothing new?  Wait for eventfd to tell us
diff --git a/drivers/vhost/test.c b/drivers/vhost/test.c
index 099f302..734e1d7 100644
--- a/drivers/vhost/test.c
+++ b/drivers/vhost/test.c
@@ -49,7 +49,7 @@ static void handle_vq(struct vhost_test *n)
 		return;
 
 	mutex_lock(&vq->mutex);
-	vhost_disable_notify(vq);
+	vhost_disable_notify(&n->dev, vq);
 
 	for (;;) {
 		head = vhost_get_vq_desc(&n->dev, vq, vq->iov,
@@ -61,8 +61,8 @@ static void handle_vq(struct vhost_test *n)
 			break;
 		/* Nothing new?  Wait for eventfd to tell us they refilled. */
 		if (head == vq->num) {
-			if (unlikely(vhost_enable_notify(vq))) {
-				vhost_disable_notify(vq);
+			if (unlikely(vhost_enable_notify(&n->dev, vq))) {
+				vhost_disable_notify(&n->dev, vq);
 				continue;
 			}
 			break;
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index e33d5a3..2aea4cb 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -38,6 +38,7 @@ enum {
 };
 
 #define vhost_used_event(vq) ((u16 __user *)&vq->avail->ring[vq->num])
+#define vhost_avail_event(vq) ((u16 __user *)&vq->used->ring[vq->num])
 
 static void vhost_poll_func(struct file *file, wait_queue_head_t *wqh,
 			    poll_table *pt)
@@ -499,11 +500,12 @@ static int vq_access_ok(struct vhost_dev *d, unsigned int num,
 			struct vring_used __user *used)
 {
 	size_t sa = vhost_has_feature(d, VIRTIO_RING_F_USED_EVENT_IDX) ? 2 : 0;
+	size_t su = vhost_has_feature(d, VIRTIO_RING_F_AVAIL_EVENT_IDX) ? 2 : 0;
 	return access_ok(VERIFY_READ, desc, num * sizeof *desc) &&
 	       access_ok(VERIFY_READ, avail,
 			 sizeof *avail + num * sizeof *avail->ring + sa) &&
 	       access_ok(VERIFY_WRITE, used,
-			sizeof *used + num * sizeof *used->ring);
+			sizeof *used + num * sizeof *used->ring + su);
 }
 
 /* Can we log writes? */
@@ -519,9 +521,11 @@ int vhost_log_access_ok(struct vhost_dev *dev)
 
 /* Verify access for write logging. */
 /* Caller should have vq mutex and device mutex */
-static int vq_log_access_ok(struct vhost_virtqueue *vq, void __user *log_base)
+static int vq_log_access_ok(struct vhost_dev *d, struct vhost_virtqueue *vq,
+			    void __user *log_base)
 {
 	struct vhost_memory *mp;
+	size_t s = vhost_has_feature(d, VIRTIO_RING_F_AVAIL_EVENT_IDX) ? 2 : 0;
 
 	mp = rcu_dereference_protected(vq->dev->memory,
 				       lockdep_is_held(&vq->mutex));
@@ -529,7 +533,7 @@ static int vq_log_access_ok(struct vhost_virtqueue *vq, void __user *log_base)
 			    vhost_has_feature(vq->dev, VHOST_F_LOG_ALL)) &&
 		(!vq->log_used || log_access_ok(log_base, vq->log_addr,
 					sizeof *vq->used +
-					vq->num * sizeof *vq->used->ring));
+					vq->num * sizeof *vq->used->ring + s));
 }
 
 /* Can we start vq? */
@@ -537,7 +541,7 @@ static int vq_log_access_ok(struct vhost_virtqueue *vq, void __user *log_base)
 int vhost_vq_access_ok(struct vhost_virtqueue *vq)
 {
 	return vq_access_ok(vq->dev, vq->num, vq->desc, vq->avail, vq->used) &&
-		vq_log_access_ok(vq, vq->log_base);
+		vq_log_access_ok(vq->dev, vq, vq->log_base);
 }
 
 static long vhost_set_memory(struct vhost_dev *d, struct vhost_memory __user *m)
@@ -824,7 +828,7 @@ long vhost_dev_ioctl(struct vhost_dev *d, unsigned int ioctl, unsigned long arg)
 			vq = d->vqs + i;
 			mutex_lock(&vq->mutex);
 			/* If ring is inactive, will check when it's enabled. */
-			if (vq->private_data && !vq_log_access_ok(vq, base))
+			if (vq->private_data && !vq_log_access_ok(d, vq, base))
 				r = -EFAULT;
 			else
 				vq->log_base = base;
@@ -1225,6 +1229,10 @@ int vhost_get_vq_desc(struct vhost_dev *dev, struct vhost_virtqueue *vq,
 
 	/* On success, increment avail index. */
 	vq->last_avail_idx++;
+
+	/* Assume notifications from guest are disabled at this point,
+	 * if they aren't we would need to update avail_event index. */
+	BUG_ON(!(vq->used_flags & VRING_USED_F_NO_NOTIFY));
 	return head;
 }
 
@@ -1414,7 +1422,7 @@ void vhost_add_used_and_signal_n(struct vhost_dev *dev,
 }
 
 /* OK, now we need to know about added descriptors. */
-bool vhost_enable_notify(struct vhost_virtqueue *vq)
+bool vhost_enable_notify(struct vhost_dev *dev, struct vhost_virtqueue *vq)
 {
 	u16 avail_idx;
 	int r;
@@ -1422,11 +1430,34 @@ bool vhost_enable_notify(struct vhost_virtqueue *vq)
 	if (!(vq->used_flags & VRING_USED_F_NO_NOTIFY))
 		return false;
 	vq->used_flags &= ~VRING_USED_F_NO_NOTIFY;
-	r = put_user(vq->used_flags, &vq->used->flags);
-	if (r) {
-		vq_err(vq, "Failed to enable notification at %p: %d\n",
-		       &vq->used->flags, r);
-		return false;
+	if (!vhost_has_feature(dev, VIRTIO_RING_F_AVAIL_EVENT_IDX)) {
+		r = put_user(vq->used_flags, &vq->used->flags);
+		if (r) {
+			vq_err(vq, "Failed to enable notification at %p: %d\n",
+			       &vq->used->flags, r);
+			return false;
+		}
+	} else {
+		r = put_user(vq->last_avail_idx, vhost_avail_event(vq));
+		if (r) {
+			vq_err(vq, "Failed to update avail event index at %p: %d\n",
+			       vhost_avail_event(vq), r);
+			return false;
+		}
+	}
+	if (unlikely(vq->log_used)) {
+		void __user *used;
+		/* Make sure data is seen before log. */
+		smp_wmb();
+		used = vhost_has_feature(dev, VIRTIO_RING_F_AVAIL_EVENT_IDX) ?
+			&vq->used->flags : vhost_avail_event(vq);
+		/* Log used flags or event index entry write. Both are 16 bit
+		 * fields. */
+		log_write(vq->log_base, vq->log_addr +
+			   (used - (void __user *)vq->used),
+			  sizeof(u16));
+		if (vq->log_ctx)
+			eventfd_signal(vq->log_ctx, 1);
 	}
 	/* They could have slipped one in as we were doing that: make
 	 * sure it's written, then check again. */
@@ -1442,15 +1473,17 @@ bool vhost_enable_notify(struct vhost_virtqueue *vq)
 }
 
 /* We don't need to be notified again. */
-void vhost_disable_notify(struct vhost_virtqueue *vq)
+void vhost_disable_notify(struct vhost_dev *dev, struct vhost_virtqueue *vq)
 {
 	int r;
 
 	if (vq->used_flags & VRING_USED_F_NO_NOTIFY)
 		return;
 	vq->used_flags |= VRING_USED_F_NO_NOTIFY;
-	r = put_user(vq->used_flags, &vq->used->flags);
-	if (r)
-		vq_err(vq, "Failed to enable notification at %p: %d\n",
-		       &vq->used->flags, r);
+	if (!vhost_has_feature(dev, VIRTIO_RING_F_AVAIL_EVENT_IDX)) {
+		r = put_user(vq->used_flags, &vq->used->flags);
+		if (r)
+			vq_err(vq, "Failed to enable notification at %p: %d\n",
+			       &vq->used->flags, r);
+	}
 }
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index 5825ac6..edf84be 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -155,8 +155,8 @@ void vhost_add_used_and_signal(struct vhost_dev *, struct vhost_virtqueue *,
 void vhost_add_used_and_signal_n(struct vhost_dev *, struct vhost_virtqueue *,
 			       struct vring_used_elem *heads, unsigned count);
 void vhost_signal(struct vhost_dev *, struct vhost_virtqueue *);
-void vhost_disable_notify(struct vhost_virtqueue *);
-bool vhost_enable_notify(struct vhost_virtqueue *);
+void vhost_disable_notify(struct vhost_dev *, struct vhost_virtqueue *);
+bool vhost_enable_notify(struct vhost_dev *, struct vhost_virtqueue *);
 
 int vhost_log_write(struct vhost_virtqueue *vq, struct vhost_log *log,
 		    unsigned int log_num, u64 len);
@@ -168,12 +168,13 @@ int vhost_log_write(struct vhost_virtqueue *vq, struct vhost_log *log,
 	} while (0)
 
 enum {
-	VHOST_FEATURES = (1 << VIRTIO_F_NOTIFY_ON_EMPTY) |
-			 (1 << VIRTIO_RING_F_INDIRECT_DESC) |
-			 (1 << VIRTIO_RING_F_USED_EVENT_IDX) |
-			 (1 << VHOST_F_LOG_ALL) |
-			 (1 << VHOST_NET_F_VIRTIO_NET_HDR) |
-			 (1 << VIRTIO_NET_F_MRG_RXBUF),
+	VHOST_FEATURES = (1ULL << VIRTIO_F_NOTIFY_ON_EMPTY) |
+			 (1ULL << VIRTIO_RING_F_INDIRECT_DESC) |
+			 (1ULL << VIRTIO_RING_F_USED_EVENT_IDX) |
+			 (1ULL << VIRTIO_RING_F_AVAIL_EVENT_IDX) |
+			 (1ULL << VHOST_F_LOG_ALL) |
+			 (1ULL << VHOST_NET_F_VIRTIO_NET_HDR) |
+			 (1ULL << VIRTIO_NET_F_MRG_RXBUF),
 };
 
 static inline bool vhost_has_feature(struct vhost_dev *dev, int bit)
-- 
1.7.5.53.gc233e

^ permalink raw reply related

* [PATCH 10/18] vhost: utilize used_event index
From: Michael S. Tsirkin @ 2011-05-04 20:51 UTC (permalink / raw)
  Cc: Krishna Kumar, Carsten Otte, lguest, Shirley Ma, kvm, linux-s390,
	netdev, habanero, Heiko Carstens, linux-kernel, virtualization,
	steved, Christian Borntraeger, Tom Lendacky, Martin Schwidefsky,
	linux390
In-Reply-To: <cover.1304541918.git.mst@redhat.com>

Support the new used_event index. When acked,
utilize it to reduce the # of interrupts sent to the guest.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/vhost/vhost.c |   74 +++++++++++++++++++++++++++++++++++++------------
 drivers/vhost/vhost.h |    7 ++++
 2 files changed, 63 insertions(+), 18 deletions(-)

diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 2ab2912..e33d5a3 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -37,6 +37,8 @@ enum {
 	VHOST_MEMORY_F_LOG = 0x1,
 };
 
+#define vhost_used_event(vq) ((u16 __user *)&vq->avail->ring[vq->num])
+
 static void vhost_poll_func(struct file *file, wait_queue_head_t *wqh,
 			    poll_table *pt)
 {
@@ -161,6 +163,8 @@ static void vhost_vq_reset(struct vhost_dev *dev,
 	vq->last_avail_idx = 0;
 	vq->avail_idx = 0;
 	vq->last_used_idx = 0;
+	vq->signalled_used = 0;
+	vq->signalled_used_valid = false;
 	vq->used_flags = 0;
 	vq->log_used = false;
 	vq->log_addr = -1ull;
@@ -489,14 +493,15 @@ static int memory_access_ok(struct vhost_dev *d, struct vhost_memory *mem,
 	return 1;
 }
 
-static int vq_access_ok(unsigned int num,
+static int vq_access_ok(struct vhost_dev *d, unsigned int num,
 			struct vring_desc __user *desc,
 			struct vring_avail __user *avail,
 			struct vring_used __user *used)
 {
+	size_t sa = vhost_has_feature(d, VIRTIO_RING_F_USED_EVENT_IDX) ? 2 : 0;
 	return access_ok(VERIFY_READ, desc, num * sizeof *desc) &&
 	       access_ok(VERIFY_READ, avail,
-			 sizeof *avail + num * sizeof *avail->ring) &&
+			 sizeof *avail + num * sizeof *avail->ring + sa) &&
 	       access_ok(VERIFY_WRITE, used,
 			sizeof *used + num * sizeof *used->ring);
 }
@@ -531,7 +536,7 @@ static int vq_log_access_ok(struct vhost_virtqueue *vq, void __user *log_base)
 /* Caller should have vq mutex and device mutex */
 int vhost_vq_access_ok(struct vhost_virtqueue *vq)
 {
-	return vq_access_ok(vq->num, vq->desc, vq->avail, vq->used) &&
+	return vq_access_ok(vq->dev, vq->num, vq->desc, vq->avail, vq->used) &&
 		vq_log_access_ok(vq, vq->log_base);
 }
 
@@ -577,6 +582,7 @@ static int init_used(struct vhost_virtqueue *vq,
 
 	if (r)
 		return r;
+	vq->signalled_used_valid = false;
 	return get_user(vq->last_used_idx, &used->idx);
 }
 
@@ -674,7 +680,7 @@ static long vhost_set_vring(struct vhost_dev *d, int ioctl, void __user *argp)
 		 * If it is not, we don't as size might not have been setup.
 		 * We will verify when backend is configured. */
 		if (vq->private_data) {
-			if (!vq_access_ok(vq->num,
+			if (!vq_access_ok(d, vq->num,
 				(void __user *)(unsigned long)a.desc_user_addr,
 				(void __user *)(unsigned long)a.avail_user_addr,
 				(void __user *)(unsigned long)a.used_user_addr)) {
@@ -1267,6 +1273,12 @@ int vhost_add_used(struct vhost_virtqueue *vq, unsigned int head, int len)
 			eventfd_signal(vq->log_ctx, 1);
 	}
 	vq->last_used_idx++;
+	/* If the driver never bothers to signal in a very long while,
+	 * used index might wrap around. If that happens, invalidate
+	 * signalled_used index we stored. TODO: make sure driver
+	 * signals at least once in 2^16 and remove this. */
+	if (unlikely(vq->last_used_idx == vq->signalled_used))
+		vq->signalled_used_valid = false;
 	return 0;
 }
 
@@ -1275,6 +1287,7 @@ static int __vhost_add_used_n(struct vhost_virtqueue *vq,
 			    unsigned count)
 {
 	struct vring_used_elem __user *used;
+	u16 old, new;
 	int start;
 
 	start = vq->last_used_idx % vq->num;
@@ -1292,7 +1305,14 @@ static int __vhost_add_used_n(struct vhost_virtqueue *vq,
 			   ((void __user *)used - (void __user *)vq->used),
 			  count * sizeof *used);
 	}
-	vq->last_used_idx += count;
+	old = vq->last_used_idx;
+	new = (vq->last_used_idx += count);
+	/* If the driver never bothers to signal in a very long while,
+	 * used index might wrap around. If that happens, invalidate
+	 * signalled_used index we stored. TODO: make sure driver
+	 * signals at least once in 2^16 and remove this. */
+	if (unlikely((u16)(new - vq->signalled_used) < (u16)(new - old)))
+		vq->signalled_used_valid = false;
 	return 0;
 }
 
@@ -1331,29 +1351,47 @@ int vhost_add_used_n(struct vhost_virtqueue *vq, struct vring_used_elem *heads,
 	return r;
 }
 
-/* This actually signals the guest, using eventfd. */
-void vhost_signal(struct vhost_dev *dev, struct vhost_virtqueue *vq)
+static bool vhost_notify(struct vhost_dev *dev, struct vhost_virtqueue *vq)
 {
-	__u16 flags;
-
+	__u16 old, new, event;
+	bool v;
 	/* Flush out used index updates. This is paired
 	 * with the barrier that the Guest executes when enabling
 	 * interrupts. */
 	smp_mb();
 
-	if (__get_user(flags, &vq->avail->flags)) {
-		vq_err(vq, "Failed to get flags");
-		return;
+	if (vhost_has_feature(dev, VIRTIO_F_NOTIFY_ON_EMPTY) &&
+	    unlikely(vq->avail_idx == vq->last_avail_idx))
+		return true;
+
+	if (!vhost_has_feature(dev, VIRTIO_RING_F_USED_EVENT_IDX)) {
+		__u16 flags;
+		if (__get_user(flags, &vq->avail->flags)) {
+			vq_err(vq, "Failed to get flags");
+			return true;
+		}
+		return !(flags & VRING_AVAIL_F_NO_INTERRUPT);
 	}
+	old = vq->signalled_used;
+	v = vq->signalled_used_valid;
+	new = vq->signalled_used = vq->last_used_idx;
+	vq->signalled_used_valid = true;
 
-	/* If they don't want an interrupt, don't signal, unless empty. */
-	if ((flags & VRING_AVAIL_F_NO_INTERRUPT) &&
-	    (vq->avail_idx != vq->last_avail_idx ||
-	     !vhost_has_feature(dev, VIRTIO_F_NOTIFY_ON_EMPTY)))
-		return;
+	if (unlikely(!v))
+		return true;
+
+	if (get_user(event, vhost_used_event(vq))) {
+		vq_err(vq, "Failed to get used event idx");
+		return true;
+	}
+	return vring_need_event(event, new, old);
+}
 
+/* This actually signals the guest, using eventfd. */
+void vhost_signal(struct vhost_dev *dev, struct vhost_virtqueue *vq)
+{
 	/* Signal the Guest tell them we used something up. */
-	if (vq->call_ctx)
+	if (vq->call_ctx && vhost_notify(dev, vq))
 		eventfd_signal(vq->call_ctx, 1);
 }
 
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index 0f1bf33..5825ac6 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -84,6 +84,12 @@ struct vhost_virtqueue {
 	/* Used flags */
 	u16 used_flags;
 
+	/* Last used index value we have signalled on */
+	u16 signalled_used;
+
+	/* Last used index value we have signalled on */
+	bool signalled_used_valid;
+
 	/* Log writes to used structure. */
 	bool log_used;
 	u64 log_addr;
@@ -164,6 +170,7 @@ int vhost_log_write(struct vhost_virtqueue *vq, struct vhost_log *log,
 enum {
 	VHOST_FEATURES = (1 << VIRTIO_F_NOTIFY_ON_EMPTY) |
 			 (1 << VIRTIO_RING_F_INDIRECT_DESC) |
+			 (1 << VIRTIO_RING_F_USED_EVENT_IDX) |
 			 (1 << VHOST_F_LOG_ALL) |
 			 (1 << VHOST_NET_F_VIRTIO_NET_HDR) |
 			 (1 << VIRTIO_NET_F_MRG_RXBUF),
-- 
1.7.5.53.gc233e

^ permalink raw reply related

* [PATCH 09/18] virtio: use avail_event index
From: Michael S. Tsirkin @ 2011-05-04 20:51 UTC (permalink / raw)
  Cc: Krishna Kumar, Carsten Otte, lguest, Shirley Ma, kvm, linux-s390,
	netdev, habanero, Heiko Carstens, linux-kernel, virtualization,
	steved, Christian Borntraeger, Tom Lendacky, Martin Schwidefsky,
	linux390
In-Reply-To: <cover.1304541918.git.mst@redhat.com>

Use the new avail_event feature to reduce the number
of exits from the guest.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/virtio/virtio_ring.c |   39 ++++++++++++++++++++++++++++++++++++++-
 1 files changed, 38 insertions(+), 1 deletions(-)

diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 3a3ed75..262dfe6 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -82,6 +82,15 @@ struct vring_virtqueue
 	/* Host supports indirect buffers */
 	bool indirect;
 
+	/* Host publishes avail event idx */
+	bool event;
+
+	/* Is kicked_avail below valid? */
+	bool kicked_avail_valid;
+
+	/* avail idx value we already kicked. */
+	u16 kicked_avail;
+
 	/* Number of free buffers */
 	unsigned int num_free;
 	/* Head of free buffer list. */
@@ -228,6 +237,12 @@ add_head:
 	 * new available array entries. */
 	virtio_wmb();
 	vq->vring.avail->idx++;
+	/* If the driver never bothers to kick in a very long while,
+	 * avail index might wrap around. If that happens, invalidate
+	 * kicked_avail index we stored. TODO: make sure all drivers
+	 * kick at least once in 2^16 and remove this. */
+	if (unlikely(vq->vring.avail->idx == vq->kicked_avail))
+		vq->kicked_avail_valid = true;
 
 	pr_debug("Added buffer head %i to %p\n", head, vq);
 	END_USE(vq);
@@ -236,6 +251,23 @@ add_head:
 }
 EXPORT_SYMBOL_GPL(virtqueue_add_buf_gfp);
 
+
+static bool vring_notify(struct vring_virtqueue *vq)
+{
+	u16 old, new;
+	bool v;
+	if (!vq->event)
+		return !(vq->vring.used->flags & VRING_USED_F_NO_NOTIFY);
+
+	v = vq->kicked_avail_valid;
+	old = vq->kicked_avail;
+	new = vq->kicked_avail = vq->vring.avail->idx;
+	vq->kicked_avail_valid = true;
+	if (unlikely(!v))
+		return true;
+	return vring_need_event(vring_avail_event(&vq->vring), new, old);
+}
+
 void virtqueue_kick(struct virtqueue *_vq)
 {
 	struct vring_virtqueue *vq = to_vvq(_vq);
@@ -244,7 +276,7 @@ void virtqueue_kick(struct virtqueue *_vq)
 	/* Need to update avail index before checking if we should notify */
 	virtio_mb();
 
-	if (!(vq->vring.used->flags & VRING_USED_F_NO_NOTIFY))
+	if (vring_notify(vq))
 		/* Prod other side to tell it about changes. */
 		vq->notify(&vq->vq);
 
@@ -437,6 +469,8 @@ struct virtqueue *vring_new_virtqueue(unsigned int num,
 	vq->vq.name = name;
 	vq->notify = notify;
 	vq->broken = false;
+	vq->kicked_avail_valid = false;
+	vq->kicked_avail = 0;
 	vq->last_used_idx = 0;
 	list_add_tail(&vq->vq.list, &vdev->vqs);
 #ifdef DEBUG
@@ -444,6 +478,7 @@ struct virtqueue *vring_new_virtqueue(unsigned int num,
 #endif
 
 	vq->indirect = virtio_has_feature(vdev, VIRTIO_RING_F_INDIRECT_DESC);
+	vq->event = virtio_has_feature(vdev, VIRTIO_RING_F_AVAIL_EVENT_IDX);
 
 	/* No callback?  Tell other side not to bother us. */
 	if (!callback)
@@ -482,6 +517,8 @@ void vring_transport_features(struct virtio_device *vdev)
 			break;
 		case VIRTIO_RING_F_USED_EVENT_IDX:
 			break;
+		case VIRTIO_RING_F_AVAIL_EVENT_IDX:
+			break;
 		default:
 			/* We don't understand this bit. */
 			clear_bit(i, vdev->features);
-- 
1.7.5.53.gc233e

^ permalink raw reply related

* [PATCH 08/18] virtio_ring: support for used_event idx feature
From: Michael S. Tsirkin @ 2011-05-04 20:51 UTC (permalink / raw)
  Cc: Krishna Kumar, Carsten Otte, lguest, Shirley Ma, kvm, linux-s390,
	netdev, habanero, Heiko Carstens, linux-kernel, virtualization,
	steved, Christian Borntraeger, Tom Lendacky, Martin Schwidefsky,
	linux390
In-Reply-To: <cover.1304541918.git.mst@redhat.com>

Add support for the used_event idx feature: when enabling
interrupts, publish the current avail index value to
the host so that we get interrupts on the next update.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/virtio/virtio_ring.c |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 507d6eb..3a3ed75 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -320,6 +320,14 @@ void *virtqueue_get_buf(struct virtqueue *_vq, unsigned int *len)
 	ret = vq->data[i];
 	detach_buf(vq, i);
 	vq->last_used_idx++;
+	/* If we expect an interrupt for the next entry, tell host
+	 * by writing event index and flush out the write before
+	 * the read in the next get_buf call. */
+	if (!(vq->vring.avail->flags & VRING_AVAIL_F_NO_INTERRUPT)) {
+		vring_used_event(&vq->vring) = vq->last_used_idx;
+		virtio_mb();
+	}
+
 	END_USE(vq);
 	return ret;
 }
@@ -341,7 +349,11 @@ bool virtqueue_enable_cb(struct virtqueue *_vq)
 
 	/* We optimistically turn back on interrupts, then check if there was
 	 * more to do. */
+	/* Depending on the VIRTIO_RING_F_USED_EVENT_IDX feature, we need to
+	 * either clear the flags bit or point the event index at the next
+	 * entry. Always do both to keep code simple. */
 	vq->vring.avail->flags &= ~VRING_AVAIL_F_NO_INTERRUPT;
+	vring_used_event(&vq->vring) = vq->last_used_idx;
 	virtio_mb();
 	if (unlikely(more_used(vq))) {
 		END_USE(vq);
@@ -468,6 +480,8 @@ void vring_transport_features(struct virtio_device *vdev)
 			break;
 		case VIRTIO_RING_F_INDIRECT_DESC:
 			break;
+		case VIRTIO_RING_F_USED_EVENT_IDX:
+			break;
 		default:
 			/* We don't understand this bit. */
 			clear_bit(i, vdev->features);
-- 
1.7.5.53.gc233e

^ permalink raw reply related

* [PATCH 07/18] virtio ring: inline function to check for events
From: Michael S. Tsirkin @ 2011-05-04 20:51 UTC (permalink / raw)
  Cc: Krishna Kumar, Carsten Otte, lguest, Shirley Ma, kvm, linux-s390,
	netdev, habanero, Heiko Carstens, linux-kernel, virtualization,
	steved, Christian Borntraeger, Tom Lendacky, Martin Schwidefsky,
	linux390
In-Reply-To: <cover.1304541918.git.mst@redhat.com>

With the new used_event and avail_event and features, both
host and guest need similar logic to check whether events are
enabled, so it helps to put the common code in the header.

Note that Xen has similar logic for notification hold-off
in include/xen/interface/io/ring.h with req_event and req_prod
corresponding to event_idx + 1 and new_idx respectively.
+1 comes from the fact that req_event and req_prod in Xen start at 1,
while event index in virtio starts at 0.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/linux/virtio_ring.h |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h
index f791772..2a3b0ea 100644
--- a/include/linux/virtio_ring.h
+++ b/include/linux/virtio_ring.h
@@ -124,6 +124,20 @@ static inline unsigned vring_size(unsigned int num, unsigned long align)
 		+ sizeof(__u16) * 3 + sizeof(struct vring_used_elem) * num;
 }
 
+/* The following is used with USED_EVENT_IDX and AVAIL_EVENT_IDX */
+/* Assuming a given event_idx value from the other size, if
+ * we have just incremented index from old to new_idx,
+ * should we trigger an event? */
+static inline int vring_need_event(__u16 event_idx, __u16 new_idx, __u16 old)
+{
+	/* Note: Xen has similar logic for notification hold-off
+	 * in include/xen/interface/io/ring.h with req_event and req_prod
+	 * corresponding to event_idx + 1 and new_idx respectively.
+	 * Note also that req_event and req_prod in Xen start at 1,
+	 * event indexes in virtio start at 0. */
+	return (__u16)(new_idx - event_idx - 1) < (__u16)(new_idx - old);
+}
+
 #ifdef __KERNEL__
 #include <linux/irqreturn.h>
 struct virtio_device;
-- 
1.7.5.53.gc233e

^ permalink raw reply related

* [PATCH 06/18] virtio_ring: avail event index interface
From: Michael S. Tsirkin @ 2011-05-04 20:51 UTC (permalink / raw)
  Cc: Krishna Kumar, Carsten Otte, lguest, Shirley Ma, kvm, linux-s390,
	netdev, habanero, Heiko Carstens, linux-kernel, virtualization,
	steved, Christian Borntraeger, Tom Lendacky, Martin Schwidefsky,
	linux390
In-Reply-To: <cover.1304541918.git.mst@redhat.com>

Define a new feature bit for the host to
declare that it uses an avail_event index
(like Xen) instead of a feature bit
to enable/disable interrupts.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/linux/virtio_ring.h |   11 ++++++++---
 1 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h
index f5c1b75..f791772 100644
--- a/include/linux/virtio_ring.h
+++ b/include/linux/virtio_ring.h
@@ -32,6 +32,9 @@
 /* The Guest publishes the used index for which it expects an interrupt
  * at the end of the avail ring. Host should ignore the avail->flags field. */
 #define VIRTIO_RING_F_USED_EVENT_IDX	29
+/* The Host publishes the avail index for which it expects a kick
+ * at the end of the used ring. Guest should ignore the used->flags field. */
+#define VIRTIO_RING_F_AVAIL_EVENT_IDX	32
 
 /* Virtio ring descriptors: 16 bytes.  These can chain together via "next". */
 struct vring_desc {
@@ -96,11 +99,13 @@ struct vring {
  *	__u16 used_flags;
  *	__u16 used_idx;
  *	struct vring_used_elem used[num];
+ *	__u16 avail_event_idx;
  * };
  */
-/* We publish the used event index at the end of the available ring.
- * It is at the end for backwards compatibility. */
+/* We publish the used event index at the end of the available ring, and vice
+ * versa. They are at the end for backwards compatibility. */
 #define vring_used_event(vr) ((vr)->avail->ring[(vr)->num])
+#define vring_avail_event(vr) (*(__u16 *)&(vr)->used->ring[(vr)->num])
 
 static inline void vring_init(struct vring *vr, unsigned int num, void *p,
 			      unsigned long align)
@@ -116,7 +121,7 @@ static inline unsigned vring_size(unsigned int num, unsigned long align)
 {
 	return ((sizeof(struct vring_desc) * num + sizeof(__u16) * (2 + num)
 		 + align - 1) & ~(align - 1))
-		+ sizeof(__u16) * 2 + sizeof(struct vring_used_elem) * num;
+		+ sizeof(__u16) * 3 + sizeof(struct vring_used_elem) * num;
 }
 
 #ifdef __KERNEL__
-- 
1.7.5.53.gc233e

^ permalink raw reply related

* [PATCH 05/18] virtio: used event index interface
From: Michael S. Tsirkin @ 2011-05-04 20:51 UTC (permalink / raw)
  Cc: Krishna Kumar, Carsten Otte, lguest, Shirley Ma, kvm, linux-s390,
	netdev, habanero, Heiko Carstens, linux-kernel, virtualization,
	steved, Christian Borntraeger, Tom Lendacky, Martin Schwidefsky,
	linux390
In-Reply-To: <cover.1304541918.git.mst@redhat.com>

Define a new feature bit for the guest to utilize a used_event index
(like Xen) instead if a flag bit to enable/disable interrupts.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/linux/virtio_ring.h |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h
index e4d144b..f5c1b75 100644
--- a/include/linux/virtio_ring.h
+++ b/include/linux/virtio_ring.h
@@ -29,6 +29,10 @@
 /* We support indirect buffer descriptors */
 #define VIRTIO_RING_F_INDIRECT_DESC	28
 
+/* The Guest publishes the used index for which it expects an interrupt
+ * at the end of the avail ring. Host should ignore the avail->flags field. */
+#define VIRTIO_RING_F_USED_EVENT_IDX	29
+
 /* Virtio ring descriptors: 16 bytes.  These can chain together via "next". */
 struct vring_desc {
 	/* Address (guest-physical). */
@@ -83,6 +87,7 @@ struct vring {
  *	__u16 avail_flags;
  *	__u16 avail_idx;
  *	__u16 available[num];
+ *	__u16 used_event_idx;
  *
  *	// Padding to the next align boundary.
  *	char pad[];
@@ -93,6 +98,10 @@ struct vring {
  *	struct vring_used_elem used[num];
  * };
  */
+/* We publish the used event index at the end of the available ring.
+ * It is at the end for backwards compatibility. */
+#define vring_used_event(vr) ((vr)->avail->ring[(vr)->num])
+
 static inline void vring_init(struct vring *vr, unsigned int num, void *p,
 			      unsigned long align)
 {
-- 
1.7.5.53.gc233e

^ permalink raw reply related

* [PATCH 04/18] virtio: don't delay avail index update
From: Michael S. Tsirkin @ 2011-05-04 20:51 UTC (permalink / raw)
  Cc: Krishna Kumar, Carsten Otte, lguest, Shirley Ma, kvm, linux-s390,
	netdev, habanero, Heiko Carstens, linux-kernel, virtualization,
	steved, Christian Borntraeger, Tom Lendacky, Martin Schwidefsky,
	linux390
In-Reply-To: <cover.1304541918.git.mst@redhat.com>

Update avail index immediately instead of upon kick:
for virtio-net RX this helps parallelism with the host.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/virtio/virtio_ring.c |   15 +++++----------
 1 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 059e02d..507d6eb 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -86,8 +86,6 @@ struct vring_virtqueue
 	unsigned int num_free;
 	/* Head of free buffer list. */
 	unsigned int free_head;
-	/* Number we've added since last sync. */
-	unsigned int num_added;
 
 	/* Last used index we've seen. */
 	u16 last_used_idx;
@@ -224,8 +222,12 @@ add_head:
 
 	/* Put entry in available array (but don't update avail->idx until they
 	 * do sync).  FIXME: avoid modulus here? */
-	avail = (vq->vring.avail->idx + vq->num_added++) % vq->vring.num;
+	avail = vq->vring.avail->idx % vq->vring.num;
 	vq->vring.avail->ring[avail] = head;
+	/* Descriptors and available array need to be set before we expose the
+	 * new available array entries. */
+	virtio_wmb();
+	vq->vring.avail->idx++;
 
 	pr_debug("Added buffer head %i to %p\n", head, vq);
 	END_USE(vq);
@@ -238,12 +240,6 @@ void virtqueue_kick(struct virtqueue *_vq)
 {
 	struct vring_virtqueue *vq = to_vvq(_vq);
 	START_USE(vq);
-	/* Descriptors and available array need to be set before we expose the
-	 * new available array entries. */
-	virtio_wmb();
-
-	vq->vring.avail->idx += vq->num_added;
-	vq->num_added = 0;
 
 	/* Need to update avail index before checking if we should notify */
 	virtio_mb();
@@ -430,7 +426,6 @@ struct virtqueue *vring_new_virtqueue(unsigned int num,
 	vq->notify = notify;
 	vq->broken = false;
 	vq->last_used_idx = 0;
-	vq->num_added = 0;
 	list_add_tail(&vq->vq.list, &vdev->vqs);
 #ifdef DEBUG
 	vq->in_use = false;
-- 
1.7.5.53.gc233e

^ permalink raw reply related

* [PATCH 03/18] vhost: fix 64 bit features
From: Michael S. Tsirkin @ 2011-05-04 20:50 UTC (permalink / raw)
  Cc: Krishna Kumar, Carsten Otte, lguest, Shirley Ma, kvm, linux-s390,
	netdev, habanero, Heiko Carstens, linux-kernel, virtualization,
	steved, Christian Borntraeger, Tom Lendacky, Martin Schwidefsky,
	linux390
In-Reply-To: <cover.1304541918.git.mst@redhat.com>

Update vhost_has_feature to make it work correctly for bit > 32.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/vhost/vhost.h |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index b3363ae..0f1bf33 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -117,7 +117,7 @@ struct vhost_dev {
 	struct vhost_memory __rcu *memory;
 	struct mm_struct *mm;
 	struct mutex mutex;
-	unsigned acked_features;
+	u64 acked_features;
 	struct vhost_virtqueue *vqs;
 	int nvqs;
 	struct file *log_file;
@@ -169,14 +169,14 @@ enum {
 			 (1 << VIRTIO_NET_F_MRG_RXBUF),
 };
 
-static inline int vhost_has_feature(struct vhost_dev *dev, int bit)
+static inline bool vhost_has_feature(struct vhost_dev *dev, int bit)
 {
-	unsigned acked_features;
+	u64 acked_features;
 
 	/* TODO: check that we are running from vhost_worker or dev mutex is
 	 * held? */
 	acked_features = rcu_dereference_index_check(dev->acked_features, 1);
-	return acked_features & (1 << bit);
+	return acked_features & (1ull << bit);
 }
 
 #endif
-- 
1.7.5.53.gc233e

^ permalink raw reply related

* [PATCH 02/18] virtio_test: update for 64 bit features
From: Michael S. Tsirkin @ 2011-05-04 20:50 UTC (permalink / raw)
  Cc: Krishna Kumar, Carsten Otte, lguest, Shirley Ma, kvm, linux-s390,
	netdev, habanero, Heiko Carstens, linux-kernel, virtualization,
	steved, Christian Borntraeger, Tom Lendacky, Martin Schwidefsky,
	linux390
In-Reply-To: <cover.1304541918.git.mst@redhat.com>

Extend the virtio_test tool so it can work with
64 bit features.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 tools/virtio/virtio_test.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/tools/virtio/virtio_test.c b/tools/virtio/virtio_test.c
index df0c6d2..9e65e6d 100644
--- a/tools/virtio/virtio_test.c
+++ b/tools/virtio/virtio_test.c
@@ -55,7 +55,6 @@ void vhost_vq_setup(struct vdev_info *dev, struct vq_info *info)
 {
 	struct vhost_vring_state state = { .index = info->idx };
 	struct vhost_vring_file file = { .index = info->idx };
-	unsigned long long features = dev->vdev.features[0];
 	struct vhost_vring_addr addr = {
 		.index = info->idx,
 		.desc_user_addr = (uint64_t)(unsigned long)info->vring.desc,
@@ -63,6 +62,10 @@ void vhost_vq_setup(struct vdev_info *dev, struct vq_info *info)
 		.used_user_addr = (uint64_t)(unsigned long)info->vring.used,
 	};
 	int r;
+	unsigned long long features = dev->vdev.features[0];
+	if (sizeof features > sizeof dev->vdev.features[0])
+		features |= ((unsigned long long)dev->vdev.features[1]) << 32;
+
 	r = ioctl(dev->control, VHOST_SET_FEATURES, &features);
 	assert(r >= 0);
 	state.num = info->vring.num;
@@ -107,7 +110,8 @@ static void vdev_info_init(struct vdev_info* dev, unsigned long long features)
 	int r;
 	memset(dev, 0, sizeof *dev);
 	dev->vdev.features[0] = features;
-	dev->vdev.features[1] = features >> 32;
+	if (sizeof features > sizeof dev->vdev.features[0])
+		dev->vdev.features[1] = features >> 32;
 	dev->buf_size = 1024;
 	dev->buf = malloc(dev->buf_size);
 	assert(dev->buf);
-- 
1.7.5.53.gc233e

^ permalink raw reply related

* [PATCH 01/18] virtio: 64 bit features
From: Michael S. Tsirkin @ 2011-05-04 20:50 UTC (permalink / raw)
  Cc: Krishna Kumar, Carsten Otte, lguest, Shirley Ma, kvm, linux-s390,
	netdev, habanero, Heiko Carstens, linux-kernel, virtualization,
	steved, Christian Borntraeger, Tom Lendacky, Martin Schwidefsky,
	linux390
In-Reply-To: <cover.1304541918.git.mst@redhat.com>

Extend features to 64 bit so we can use more
transport bits.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/lguest/lguest_device.c |    8 ++++----
 drivers/s390/kvm/kvm_virtio.c  |    8 ++++----
 drivers/virtio/virtio.c        |    8 ++++----
 drivers/virtio/virtio_pci.c    |   34 ++++++++++++++++++++++++++++------
 drivers/virtio/virtio_ring.c   |    2 ++
 include/linux/virtio.h         |    2 +-
 include/linux/virtio_config.h  |   15 +++++++++------
 include/linux/virtio_pci.h     |    9 ++++++++-
 8 files changed, 60 insertions(+), 26 deletions(-)

diff --git a/drivers/lguest/lguest_device.c b/drivers/lguest/lguest_device.c
index 69c84a1..d2d6953 100644
--- a/drivers/lguest/lguest_device.c
+++ b/drivers/lguest/lguest_device.c
@@ -93,17 +93,17 @@ static unsigned desc_size(const struct lguest_device_desc *desc)
 }
 
 /* This gets the device's feature bits. */
-static u32 lg_get_features(struct virtio_device *vdev)
+static u64 lg_get_features(struct virtio_device *vdev)
 {
 	unsigned int i;
-	u32 features = 0;
+	u64 features = 0;
 	struct lguest_device_desc *desc = to_lgdev(vdev)->desc;
 	u8 *in_features = lg_features(desc);
 
 	/* We do this the slow but generic way. */
-	for (i = 0; i < min(desc->feature_len * 8, 32); i++)
+	for (i = 0; i < min(desc->feature_len * 8, 64); i++)
 		if (in_features[i / 8] & (1 << (i % 8)))
-			features |= (1 << i);
+			features |= (1ull << i);
 
 	return features;
 }
diff --git a/drivers/s390/kvm/kvm_virtio.c b/drivers/s390/kvm/kvm_virtio.c
index 414427d..c56293c 100644
--- a/drivers/s390/kvm/kvm_virtio.c
+++ b/drivers/s390/kvm/kvm_virtio.c
@@ -79,16 +79,16 @@ static unsigned desc_size(const struct kvm_device_desc *desc)
 }
 
 /* This gets the device's feature bits. */
-static u32 kvm_get_features(struct virtio_device *vdev)
+static u64 kvm_get_features(struct virtio_device *vdev)
 {
 	unsigned int i;
-	u32 features = 0;
+	u64 features = 0;
 	struct kvm_device_desc *desc = to_kvmdev(vdev)->desc;
 	u8 *in_features = kvm_vq_features(desc);
 
-	for (i = 0; i < min(desc->feature_len * 8, 32); i++)
+	for (i = 0; i < min(desc->feature_len * 8, 64); i++)
 		if (in_features[i / 8] & (1 << (i % 8)))
-			features |= (1 << i);
+			features |= (1ull << i);
 	return features;
 }
 
diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index efb35aa..52b24d7 100644
--- a/drivers/virtio/virtio.c
+++ b/drivers/virtio/virtio.c
@@ -112,7 +112,7 @@ static int virtio_dev_probe(struct device *_d)
 	struct virtio_device *dev = container_of(_d,struct virtio_device,dev);
 	struct virtio_driver *drv = container_of(dev->dev.driver,
 						 struct virtio_driver, driver);
-	u32 device_features;
+	u64 device_features;
 
 	/* We have a driver! */
 	add_status(dev, VIRTIO_CONFIG_S_DRIVER);
@@ -124,14 +124,14 @@ static int virtio_dev_probe(struct device *_d)
 	memset(dev->features, 0, sizeof(dev->features));
 	for (i = 0; i < drv->feature_table_size; i++) {
 		unsigned int f = drv->feature_table[i];
-		BUG_ON(f >= 32);
-		if (device_features & (1 << f))
+		BUG_ON(f >= 64);
+		if (device_features & (1ull << f))
 			set_bit(f, dev->features);
 	}
 
 	/* Transport features always preserved to pass to finalize_features. */
 	for (i = VIRTIO_TRANSPORT_F_START; i < VIRTIO_TRANSPORT_F_END; i++)
-		if (device_features & (1 << i))
+		if (device_features & (1ull << i))
 			set_bit(i, dev->features);
 
 	dev->config->finalize_features(dev);
diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c
index 4fb5b2b..04b216f 100644
--- a/drivers/virtio/virtio_pci.c
+++ b/drivers/virtio/virtio_pci.c
@@ -44,6 +44,8 @@ struct virtio_pci_device
 	spinlock_t lock;
 	struct list_head virtqueues;
 
+	/* 64 bit features */
+	int features_hi;
 	/* MSI-X support */
 	int msix_enabled;
 	int intx_enabled;
@@ -103,26 +105,46 @@ static struct virtio_pci_device *to_vp_device(struct virtio_device *vdev)
 }
 
 /* virtio config->get_features() implementation */
-static u32 vp_get_features(struct virtio_device *vdev)
+static u64 vp_get_features(struct virtio_device *vdev)
 {
 	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
+	u32 flo, fhi;
 
-	/* When someone needs more than 32 feature bits, we'll need to
+	/* When someone needs more than 32 feature bits, we need to
 	 * steal a bit to indicate that the rest are somewhere else. */
-	return ioread32(vp_dev->ioaddr + VIRTIO_PCI_HOST_FEATURES);
+	flo = ioread32(vp_dev->ioaddr + VIRTIO_PCI_HOST_FEATURES);
+	if (flo & (0x1 << VIRTIO_F_FEATURES_HI)) {
+		vp_dev->features_hi = 1;
+		iowrite32(0x1 << VIRTIO_F_FEATURES_HI,
+			  vp_dev->ioaddr + VIRTIO_PCI_GUEST_FEATURES);
+		fhi = ioread32(vp_dev->ioaddr + VIRTIO_PCI_HOST_FEATURES_HI);
+	} else {
+		vp_dev->features_hi = 0;
+		fhi = 0;
+	}
+	return (((u64)fhi) << 32) | flo;
 }
 
 /* virtio config->finalize_features() implementation */
 static void vp_finalize_features(struct virtio_device *vdev)
 {
 	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
+	u32 flo, fhi;
 
 	/* Give virtio_ring a chance to accept features. */
 	vring_transport_features(vdev);
 
-	/* We only support 32 feature bits. */
-	BUILD_BUG_ON(ARRAY_SIZE(vdev->features) != 1);
-	iowrite32(vdev->features[0], vp_dev->ioaddr+VIRTIO_PCI_GUEST_FEATURES);
+	/* We only support 64 feature bits. */
+	BUILD_BUG_ON(ARRAY_SIZE(vdev->features) != 64 / BITS_PER_LONG);
+	flo = vdev->features[0];
+	fhi = vdev->features[64 / BITS_PER_LONG - 1] >> (BITS_PER_LONG - 32);
+	iowrite32(flo, vp_dev->ioaddr + VIRTIO_PCI_GUEST_FEATURES);
+	if (flo & (0x1 << VIRTIO_F_FEATURES_HI)) {
+		vp_dev->features_hi = 1;
+		iowrite32(fhi, vp_dev->ioaddr + VIRTIO_PCI_GUEST_FEATURES_HI);
+	} else {
+		vp_dev->features_hi = 0;
+	}
 }
 
 /* virtio config->get() implementation */
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index cc2f73e..059e02d 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -469,6 +469,8 @@ void vring_transport_features(struct virtio_device *vdev)
 
 	for (i = VIRTIO_TRANSPORT_F_START; i < VIRTIO_TRANSPORT_F_END; i++) {
 		switch (i) {
+		case VIRTIO_F_FEATURES_HI:
+			break;
 		case VIRTIO_RING_F_INDIRECT_DESC:
 			break;
 		default:
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index aff5b4f..718336b 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -105,7 +105,7 @@ struct virtio_device {
 	struct virtio_config_ops *config;
 	struct list_head vqs;
 	/* Note that this is a Linux set_bit-style bitmap. */
-	unsigned long features[1];
+	unsigned long features[64 / BITS_PER_LONG];
 	void *priv;
 };
 
diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index 800617b..b1a1981 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -18,16 +18,19 @@
 /* We've given up on this device. */
 #define VIRTIO_CONFIG_S_FAILED		0x80
 
-/* Some virtio feature bits (currently bits 28 through 31) are reserved for the
+/* Some virtio feature bits (currently bits 28 through 39) are reserved for the
  * transport being used (eg. virtio_ring), the rest are per-device feature
  * bits. */
 #define VIRTIO_TRANSPORT_F_START	28
-#define VIRTIO_TRANSPORT_F_END		32
+#define VIRTIO_TRANSPORT_F_END		40
 
 /* Do we get callbacks when the ring is completely used, even if we've
  * suppressed them? */
 #define VIRTIO_F_NOTIFY_ON_EMPTY	24
 
+/* Enables feature bits 32 to 63 (only really required for virtio_pci). */
+#define VIRTIO_F_FEATURES_HI		31
+
 #ifdef __KERNEL__
 #include <linux/err.h>
 #include <linux/virtio.h>
@@ -72,7 +75,7 @@
  * @del_vqs: free virtqueues found by find_vqs().
  * @get_features: get the array of feature bits for this device.
  *	vdev: the virtio_device
- *	Returns the first 32 feature bits (all we currently need).
+ *	Returns the first 64 feature bits (all we currently need).
  * @finalize_features: confirm what device features we'll be using.
  *	vdev: the virtio_device
  *	This gives the final feature bits for the device: it can change
@@ -92,7 +95,7 @@ struct virtio_config_ops {
 			vq_callback_t *callbacks[],
 			const char *names[]);
 	void (*del_vqs)(struct virtio_device *);
-	u32 (*get_features)(struct virtio_device *vdev);
+	u64 (*get_features)(struct virtio_device *vdev);
 	void (*finalize_features)(struct virtio_device *vdev);
 };
 
@@ -110,9 +113,9 @@ static inline bool virtio_has_feature(const struct virtio_device *vdev,
 {
 	/* Did you forget to fix assumptions on max features? */
 	if (__builtin_constant_p(fbit))
-		BUILD_BUG_ON(fbit >= 32);
+		BUILD_BUG_ON(fbit >= 64);
 	else
-		BUG_ON(fbit >= 32);
+		BUG_ON(fbit >= 64);
 
 	if (fbit < VIRTIO_TRANSPORT_F_START)
 		virtio_check_driver_offered_feature(vdev, fbit);
diff --git a/include/linux/virtio_pci.h b/include/linux/virtio_pci.h
index 9a3d7c4..90f9725 100644
--- a/include/linux/virtio_pci.h
+++ b/include/linux/virtio_pci.h
@@ -55,9 +55,16 @@
 /* Vector value used to disable MSI for queue */
 #define VIRTIO_MSI_NO_VECTOR            0xffff
 
+/* An extended 32-bit r/o bitmask of the features supported by the host */
+#define VIRTIO_PCI_HOST_FEATURES_HI	24
+
+/* An extended 32-bit r/w bitmask of features activated by the guest */
+#define VIRTIO_PCI_GUEST_FEATURES_HI	28
+
 /* The remaining space is defined by each driver as the per-driver
  * configuration space */
-#define VIRTIO_PCI_CONFIG(dev)		((dev)->msix_enabled ? 24 : 20)
+#define VIRTIO_PCI_CONFIG(dev)		((dev)->features_hi ? 32 : \
+						(dev)->msix_enabled ? 24 : 20)
 
 /* Virtio ABI version, this must match exactly */
 #define VIRTIO_PCI_ABI_VERSION		0
-- 
1.7.5.53.gc233e

^ permalink raw reply related

* [PATCH 00/18] virtio and vhost-net performance enhancements
From: Michael S. Tsirkin @ 2011-05-04 20:50 UTC (permalink / raw)
  Cc: Krishna Kumar, Carsten Otte, lguest, Shirley Ma, kvm, linux-s390,
	netdev, habanero, Heiko Carstens, linux-kernel, virtualization,
	steved, Christian Borntraeger, Tom Lendacky, Martin Schwidefsky,
	linux390

OK, here's a large patchset that implements the virtio spec update that I
sent earlier. It supercedes the PUBLISH_USED_IDX patches
I sent out earlier.

I know it's a lot to ask but please test, and please consider for 2.6.40 :)

I see nice performance improvements: one run showed going from 12
to 18 Gbit/s host to guest with netperf, but I did not spend a lot
of time testing performance, so no guarantees it's not a fluke,
I hope others will try this out and report.
Pls note I will be away from keyboard for the next week.

Essentially we change virtio ring notification
hand-off to work like the one in Xen -
each side publishes an event index, the other one
notifies when it reaches that value -
With the one difference that event index starts at 0,
same as request index (in xen event index starts at 1).

Each side of the handoff has a feature bit independent
of the other one, so we can have e.g. interrupts
handled in the new way and exits in the old one.

This is actually what made the patchset larger:
we run out of feature bits so I had to add some more.
I tested various combinations of hosts and guests and
this code seems to be solid.

With the indexes in place it becomes possbile to request an event after
many requests (and not just on the next one as done now). This shall fix
the TX queue overrun which currently triggers a storm of interrupts.

The patches are mostly independent and can also be cherry-picked,
hopefully there won't be much need of that.

One dependency I'd like to note is on two cleanup patches:
the patch removing batching of available index updates
and the patch fixing ring capability checks in virtio-net.
This simplified code a bit and made the following patch simpler.

I could unwrap the dependency but prefer as is.

The patchset is on top of net-next which at the time
I last rebased was 15ecd03 - so roughly 2.6.39-rc2.

qemu patch will follow shortly.

Rusty, I think (in the hope it will come to that) it will be easier to
merge vhost and virtio bits in one go. Can all go in through your tree
(Dave in the past acked a very similar patch so should not be a problem)
or from me to Dave Miller.

I see nice performance improvements: e.g. from 12 to 18 Gbit/s host
to guest with netperf, but did not spend a lot of time testing
performance, and I will be away from keyboard for the next week.
I hope others will try this out and report.

Michael S. Tsirkin (17):
  virtio: 64 bit features
  virtio_test: update for 64 bit features
  vhost: fix 64 bit features
  virtio: don't delay avail index update
  virtio: used event index interface
  virtio_ring: avail event index interface
  virtio ring: inline function to check for events
  virtio_ring: support for used_event idx feature
  virtio: use avail_event index
  vhost: utilize used_event index
  vhost: support avail_event idx
  virtio_test: support used_event index
  virtio_test: avail_event index support
  virtio: add api for delayed callbacks
  virtio_net: delay TX callbacks
  virtio_net: fix TX capacity checks using new API
  virtio_net: limit xmit polling

Shirley Ma (1):
  virtio_ring: Add capacity check API

 drivers/lguest/lguest_device.c |    8 +-
 drivers/net/virtio_net.c       |   25 ++++---
 drivers/s390/kvm/kvm_virtio.c  |    8 +-
 drivers/vhost/net.c            |   12 ++--
 drivers/vhost/test.c           |    6 +-
 drivers/vhost/vhost.c          |  139 ++++++++++++++++++++++++++++++----------
 drivers/vhost/vhost.h          |   30 ++++++---
 drivers/virtio/virtio.c        |    8 +-
 drivers/virtio/virtio_pci.c    |   34 ++++++++--
 drivers/virtio/virtio_ring.c   |  105 +++++++++++++++++++++++++++---
 include/linux/virtio.h         |   16 ++++-
 include/linux/virtio_config.h  |   15 +++--
 include/linux/virtio_pci.h     |    9 ++-
 include/linux/virtio_ring.h    |   30 ++++++++-
 tools/virtio/virtio_test.c     |   39 ++++++++++-
 15 files changed, 377 insertions(+), 107 deletions(-)

-- 
1.7.5.53.gc233e

^ permalink raw reply

* [PATCHv2] virtio-spec: 64 bit features, used/avail event
From: Michael S. Tsirkin @ 2011-05-04 20:32 UTC (permalink / raw)
  To: linux-kernel
  Cc: Krishna Kumar, Carsten Otte, lguest, Shirley Ma, kvm, linux-s390,
	netdev, habanero, Heiko Carstens, virtualization, steved,
	Christian Borntraeger, Tom Lendacky, Martin Schwidefsky, linux390

I'm working on a patchset (to follow shortly)
that modified the notificatin hand-off in virtio to be basically
like Xen: each side published an index, the other side only triggers
an event when it crosses that index value
(Xen event indexes start at 1, ours start at 0 for
backward-compatiblity, but that's minor).

Especially for testing, it is very convenient to have
separate feature bits for this change in used and available
ring; since we've run out of bits in the 32 bit field,
I added another 32 bit and bit 31 enables that.

I started with using both flags and indexes in parallel,
but switched to doing either-or: this means we do
not need to tweak memory access ordering as index access just
replaces flags access.

A note on naming: the index replacing avail->flags is named
used_event, the index replacing used->flags is named
avail_event to stress the fact that these actually
point into the other side of the ring:
event is triggered when avail->idx == used->avail_event + 1
and when used->idx == avail->used_event + 1, respectively.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---

Changes from v1:
	- minor wording changes to address comments
	- fill a couple of places I missed
	- add text about access ordering

 virtio-spec.lyx |  719 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 700 insertions(+), 19 deletions(-)

diff --git a/virtio-spec.lyx b/virtio-spec.lyx
index f7c9c38..95fd926 100644
--- a/virtio-spec.lyx
+++ b/virtio-spec.lyx
@@ -1,4 +1,4 @@
-#LyX 1.6.7 created this file. For more info see http://www.lyx.org/
+#LyX 1.6.8 created this file. For more info see http://www.lyx.org/
 \lyxformat 345
 \begin_document
 \begin_header
@@ -36,7 +36,7 @@
 \paperpagestyle default
 \tracking_changes true
 \output_changes true
-\author "" 
+\author "Michael S. Tsirkin" 
 \author "" 
 \end_header
 
@@ -953,6 +953,10 @@ ISR
 
 \size footnotesize
 Features
+\change_inserted 0 1304329091
+ bits 0:31
+\change_unchanged
+
 \end_layout
 
 \end_inset
@@ -964,6 +968,10 @@ Features
 
 \size footnotesize
 Features
+\change_inserted 0 1304329086
+ bits 0:31
+\change_unchanged
+
 \end_layout
 
 \end_inset
@@ -1186,6 +1194,177 @@ Vector
 \end_layout
 
 \begin_layout Standard
+
+\change_inserted 0 1304328924
+Finally, if feature bits (VIRTIO_F_FEATURES_HI) this is immediately followed
+ by two additional fields:
+\end_layout
+
+\begin_layout Standard
+
+\change_inserted 0 1304328925
+\begin_inset Tabular
+<lyxtabular version="3" rows="4" columns="3">
+<features>
+<column alignment="left" valignment="top" width="0">
+<column alignment="left" valignment="top" width="0">
+<column alignment="left" valignment="top" width="0">
+<row>
+<cell alignment="center" valignment="top" topline="true" leftline="true" rightline="true" usebox="none">
+\begin_inset Text
+
+\begin_layout Plain Layout
+
+\change_inserted 0 1304328925
+Bits
+\end_layout
+
+\end_inset
+</cell>
+<cell alignment="center" valignment="top" topline="true" leftline="true" usebox="none">
+\begin_inset Text
+
+\begin_layout Plain Layout
+
+\change_inserted 0 1304328925
+32
+\end_layout
+
+\end_inset
+</cell>
+<cell alignment="center" valignment="top" topline="true" leftline="true" usebox="none">
+\begin_inset Text
+
+\begin_layout Plain Layout
+
+\change_inserted 0 1304328925
+32
+\end_layout
+
+\end_inset
+</cell>
+</row>
+<row>
+<cell alignment="center" valignment="top" topline="true" leftline="true" rightline="true" usebox="none">
+\begin_inset Text
+
+\begin_layout Plain Layout
+
+\change_inserted 0 1304328925
+Read/Write
+\end_layout
+
+\end_inset
+</cell>
+<cell alignment="center" valignment="top" topline="true" leftline="true" usebox="none">
+\begin_inset Text
+
+\begin_layout Plain Layout
+
+\change_inserted 0 1304328925
+R
+\end_layout
+
+\end_inset
+</cell>
+<cell alignment="center" valignment="top" topline="true" leftline="true" usebox="none">
+\begin_inset Text
+
+\begin_layout Plain Layout
+
+\change_inserted 0 1304328925
+R+W
+\end_layout
+
+\end_inset
+</cell>
+</row>
+<row>
+<cell alignment="center" valignment="top" topline="true" leftline="true" rightline="true" usebox="none">
+\begin_inset Text
+
+\begin_layout Plain Layout
+
+\change_inserted 0 1304328925
+Purpose
+\end_layout
+
+\end_inset
+</cell>
+<cell alignment="center" valignment="top" topline="true" leftline="true" usebox="none">
+\begin_inset Text
+
+\begin_layout Plain Layout
+
+\change_inserted 0 1304328925
+
+\size footnotesize
+Device
+\end_layout
+
+\end_inset
+</cell>
+<cell alignment="center" valignment="top" topline="true" leftline="true" usebox="none">
+\begin_inset Text
+
+\begin_layout Plain Layout
+
+\change_inserted 0 1304328925
+
+\size footnotesize
+Guest
+\end_layout
+
+\end_inset
+</cell>
+</row>
+<row>
+<cell alignment="center" valignment="top" bottomline="true" leftline="true" rightline="true" usebox="none">
+\begin_inset Text
+
+\begin_layout Plain Layout
+
+\change_inserted 0 1304328925
+
+\end_layout
+
+\end_inset
+</cell>
+<cell alignment="center" valignment="top" bottomline="true" leftline="true" usebox="none">
+\begin_inset Text
+
+\begin_layout Plain Layout
+
+\change_inserted 0 1304329099
+
+\size footnotesize
+Features bits 32:63
+\end_layout
+
+\end_inset
+</cell>
+<cell alignment="center" valignment="top" bottomline="true" leftline="true" usebox="none">
+\begin_inset Text
+
+\begin_layout Plain Layout
+
+\change_inserted 0 1304329102
+
+\size footnotesize
+Features bits 32:63
+\end_layout
+
+\end_inset
+</cell>
+</row>
+</lyxtabular>
+
+\end_inset
+
+
+\end_layout
+
+\begin_layout Standard
 Immediately following these general headers, there may be device-specific
  headers:
 \end_layout
@@ -1348,7 +1527,20 @@ Feature Bits
 The least significant 31 bits of the first configuration field indicates
  the features that the device supports (the high bit is reserved, and will
  be used to indicate the presence of future feature bits elsewhere).
- The bits are allocated as follows:
+ 
+\change_inserted 0 1304331636
+If more than 31 feature bits are supported, the device indicates so by setting
+ feature bit 31 (see 
+\begin_inset CommandInset ref
+LatexCommand ref
+reference "cha:Reserved-Feature-Bits"
+
+\end_inset
+
+).
+ 
+\change_unchanged
+The bits are allocated as follows:
 \end_layout
 
 \begin_layout Description
@@ -1372,7 +1564,33 @@ to
 \begin_inset space ~
 \end_inset
 
-30 Feature bits reserved for extensions to the queue mechanism
+
+\change_inserted 0 1304329326
+4
+\change_deleted 0 1304329325
+3
+\change_unchanged
+0 Feature bits reserved for extensions to the queue 
+\change_inserted 0 1304540448
+and feature negotiation 
+\change_unchanged
+mechanism
+\change_inserted 0 1304540449
+s
+\end_layout
+
+\begin_layout Description
+
+\change_inserted 0 1304329398
+41
+\begin_inset space ~
+\end_inset
+
+to
+\begin_inset space ~
+\end_inset
+
+63 Feature bits reserved for future extensions
 \end_layout
 
 \begin_layout Standard
@@ -1407,6 +1625,19 @@ This allows for forwards and backwards compatibility: if the device is enhanced
  support, it will not see that feature bit in the Device Features field
  and can go into backwards compatibility mode (or, for poor implementations,
  set the FAILED Device Status bit).
+\change_inserted 0 1304329423
+
+\end_layout
+
+\begin_layout Standard
+
+\change_inserted 0 1304331742
+Access to feature bits 32 to 63 is enabled by Guest by setting feature bit
+ 31.
+ If this bit is unset, Device must assume that all feature bits > 31 are
+ unset.
+\change_unchanged
+
 \end_layout
 
 \begin_layout Subsubsection
@@ -1891,7 +2122,38 @@ flags
 
  field is currently 0 or 1: 1 indicating that we do not need an interrupt
  when the device consumes a descriptor from the available ring.
- This interrupt suppression is merely an optimization; it may not suppress
+ 
+\change_inserted 0 1304540481
+Alternatively, the guest ask the device to delay interrupts until an entry
+ with an index specified by the 
+\begin_inset Quotes eld
+\end_inset
+
+used_event
+\begin_inset Quotes erd
+\end_inset
+
+ field is written in the used ring (equivalently, until the 
+\emph on
+idx
+\emph default
+ field in the used ring will reach the value 
+\emph on
+used_event + 1
+\emph default
+).
+ The method employed by the device is controlled by the VIRTIO_RING_F_USED_EVENT
+_IDX feature bit (see 
+\begin_inset CommandInset ref
+LatexCommand ref
+reference "cha:Reserved-Feature-Bits"
+
+\end_inset
+
+).
+ 
+\change_unchanged
+This interrupt suppression is merely an optimization; it may not suppress
  interrupts entirely.
 \end_layout
 
@@ -1940,6 +2202,17 @@ struct vring_avail {
 \begin_layout Plain Layout
 
    u16 ring[qsz]; /* qsz is the Queue Size field read from device */
+\change_inserted 0 1304329945
+
+\end_layout
+
+\begin_layout Plain Layout
+
+\change_inserted 0 1304329957
+
+   u16 used_event;
+\change_unchanged
+
 \end_layout
 
 \begin_layout Plain Layout
@@ -1963,8 +2236,71 @@ The used ring is where the device returns buffers once it is done with them.
 \emph on
 available
 \emph default
- ring (the flag is kept here because this is the only part of the virtqueue
- written by the device).
+ ring
+\change_inserted 0 1304540575
+.
+ Alternatively, the 
+\begin_inset Quotes eld
+\end_inset
+
+avail_event
+\begin_inset Quotes erd
+\end_inset
+
+ field can be used by the device to hint that no notification is necessary
+ until an entry with an index specified by the 
+\begin_inset Quotes eld
+\end_inset
+
+avail_event
+\begin_inset Quotes erd
+\end_inset
+
+ is written in the available ring (equivalently, until the 
+\emph on
+idx
+\emph default
+ field in the available ring will reach the value 
+\emph on
+avail_event + 1
+\emph default
+).
+
+\change_unchanged
+ 
+\change_inserted 0 1304540614
+The method employed by the device is controlled by the guest through the
+ VIRTIO_RING_F_AVAIL_EVENT_IDX feature bit (see 
+\begin_inset CommandInset ref
+LatexCommand ref
+reference "cha:Reserved-Feature-Bits"
+
+\end_inset
+
+).
+ 
+\change_deleted 0 1304331235
+(the flag is kept here because this is the only part of the virtqueue written
+ by the device)
+\change_inserted 0 1304540560
+
+\begin_inset Foot
+status open
+
+\begin_layout Plain Layout
+
+\change_inserted 0 1304331235
+These fields are kept here because this is the only part of the virtqueue
+ written by the device
+\change_unchanged
+
+\end_layout
+
+\end_inset
+
+
+\change_unchanged
+.
 \end_layout
 
 \begin_layout Standard
@@ -2046,6 +2382,17 @@ struct vring_used {
 \begin_layout Plain Layout
 
     struct vring_used_elem ring[qsz];
+\change_inserted 0 1304330369
+
+\end_layout
+
+\begin_layout Plain Layout
+
+\change_inserted 0 1304330380
+
+    u16 avail_event;
+\change_unchanged
+
 \end_layout
 
 \begin_layout Plain Layout
@@ -2065,9 +2412,13 @@ Helpers for Managing Virtqueues
 \begin_layout Standard
 The Linux Kernel Source code contains the definitions above and helper routines
  in a more usable form, in include/linux/virtio_ring.h.
- This was explicitly licensed by IBM under the (3-clause) BSD license so
- that it can be freely used by all other projects, and is reproduced (with
- slight variation to remove Linux assumptions) in Appendix A.
+ This was explicitly licensed by IBM 
+\change_inserted 0 1304342159
+and Red Hat 
+\change_unchanged
+under the (3-clause) BSD license so that it can be freely used by all other
+ projects, and is reproduced (with slight variation to remove Linux assumptions)
+ in Appendix A.
 \end_layout
 
 \begin_layout Section
@@ -2374,12 +2725,61 @@ before
 \emph default
  checking the suppression flag: it's OK to notify gratuitously, but not
  to omit a required notification.
- So again, we use a memory barrier here before reading the flags.
+ So again, we use a memory barrier here before reading the flags
+\change_inserted 0 1304336099
+ or the avail_event field
+\change_unchanged
+.
+\end_layout
+
+\begin_layout Standard
+If 
+\change_inserted 0 1304336234
+the VIRTIO_ F_RING_AVAIL_EVENT_IDX feature is not negotiated, and if 
+\change_unchanged
+the VRING_USED_F_NOTIFY flag is not set, we go ahead and write to the PCI
+ configuration space.
+\change_inserted 0 1304336255
+
+\end_layout
+
+\begin_layout Standard
+
+\change_inserted 0 1304336617
+If the VIRTIO_ F_RING_AVAIL_EVENT_IDX feature is negotiated, we read the
+ avail_event field in the available ring structure.
+ If the available index crossed_the 
+\emph on
+avail_event
+\emph default
+ field value since the last notification, we go ahead and write to the PCI
+ configuration space.
+ The 
+\emph on
+avail_event
+\emph default
+ field wraps naturally at 65536 as well:
 \end_layout
 
 \begin_layout Standard
-If the VRING_USED_F_NOTIFY flag is not set, we go ahead and write to the
- PCI configuration space.
+
+\change_inserted 0 1304336524
+\begin_inset listings
+inline false
+status open
+
+\begin_layout Plain Layout
+
+\change_inserted 0 1304336569
+
+(u16)(new_idx - avail_event - 1) < (u16)(new_idx - old_idx)
+\end_layout
+
+\end_inset
+
+
+\change_unchanged
+
 \end_layout
 
 \begin_layout Subsection
@@ -2408,8 +2808,66 @@ Update the used ring idx.
 \end_layout
 
 \begin_layout Enumerate
-If the VRING_AVAIL_F_NO_INTERRUPT flag is not set in avail\SpecialChar \nobreakdash-
->flags:
+
+\change_inserted 0 1304336736
+Determine whether an interrupt is necessary:
+\end_layout
+
+\begin_deeper
+\begin_layout Enumerate
+
+\change_inserted 0 1304336780
+If the VIRTIO_F_RING_USED_IDX is not negotiated: check if 
+\change_deleted 0 1304336781
+I
+\change_unchanged
+f the VRING_AVAIL_F_NO_INTERRUPT flag is not set in avail\SpecialChar \nobreakdash-
+>flags
+\change_inserted 0 1304336788
+
+\end_layout
+
+\begin_layout Enumerate
+
+\change_deleted 0 1304336785
+:
+\change_inserted 0 1304336896
+If the VIRTIO_F_RING_USED_IDX is negotiated: check whether the used index
+ crossed the 
+\emph on
+used_event
+\emph default
+ field value since the last update.
+ The 
+\emph on
+used_event
+\emph default
+ field wraps naturally at 65536 as well:
+\begin_inset listings
+inline false
+status open
+
+\begin_layout Plain Layout
+
+\change_inserted 0 1304336902
+
+(u16)(new_idx - used_event - 1) < (u16)(new_idx - old_idx)
+\end_layout
+
+\end_inset
+
+
+\change_unchanged
+
+\end_layout
+
+\end_deeper
+\begin_layout Enumerate
+
+\change_inserted 0 1304336714
+If an interrupt is necessary:
+\change_unchanged
+
 \end_layout
 
 \begin_deeper
@@ -2464,13 +2922,87 @@ If MSI-X capability is enabled: look through the used rings of each virtqueue
 \end_layout
 
 \begin_layout Standard
+
+\change_inserted 0 1304341856
+For each ring, guest should then disable interrupts by writing VRING_AVAIL_F_NO_
+INTERRUPT flag in avail structure, if required.
+ It can then process used ring entries finally enabling interrupts by clearing
+ the VRING_AVAIL_F_NO_INTERRUPT flag or updating the used_idx field in the
+ available structure, Guest should then execute a memory barrier, and then
+ recheck the ring empty condition.
+ This is necessary to handle the case where, after the last check and before
+ enabling interrupts, an interrupt has been suppressed by the device:
+\end_layout
+
+\begin_layout Standard
 \begin_inset listings
 inline false
 status open
 
 \begin_layout Plain Layout
 
-while (vq->last_seen_used != vring->used.idx) {
+\change_inserted 0 1304342051
+
+vring_disable_interrupts(vq);
+\end_layout
+
+\begin_layout Plain Layout
+
+\change_inserted 0 1304341878
+
+for (;;) {
+\end_layout
+
+\begin_layout Plain Layout
+
+\change_inserted 0 1304341880
+
+    if 
+\change_deleted 0 1304341882
+while 
+\change_unchanged
+(vq->last_seen_used != vring->used.idx) {
+\change_inserted 0 1304341888
+
+\end_layout
+
+\begin_layout Plain Layout
+
+\change_inserted 0 1304342047
+
+		vring_enable_interrupts(vq);
+\end_layout
+
+\begin_layout Plain Layout
+
+\change_inserted 0 1304341986
+
+		mb();
+\end_layout
+
+\begin_layout Plain Layout
+
+\change_inserted 0 1304341964
+
+		if (vq->last_seen_used != vring->used.idx)
+\end_layout
+
+\begin_layout Plain Layout
+
+\change_inserted 0 1304341974
+
+			break;
+\change_unchanged
+
+\end_layout
+
+\begin_layout Plain Layout
+
+\change_inserted 0 1304341887
+
+    }
+\change_unchanged
+
 \end_layout
 
 \begin_layout Plain Layout
@@ -2721,6 +3253,15 @@ status open
 \begin_layout Plain Layout
 
  * Copyright 2007, 2009, IBM Corporation
+\change_inserted 0 1304341032
+
+\end_layout
+
+\begin_layout Plain Layout
+
+\change_inserted 0 1304341075
+
+ * Copyright 2011, Red Hat, Inc
 \end_layout
 
 \begin_layout Plain Layout
@@ -3019,6 +3560,17 @@ struct vring_avail {
 \begin_layout Plain Layout
 
         uint16_t ring[];
+\change_inserted 0 1304340808
+
+\end_layout
+
+\begin_layout Plain Layout
+
+\change_inserted 0 1304340816
+
+        uint16_t used_event;
+\change_unchanged
+
 \end_layout
 
 \begin_layout Plain Layout
@@ -3090,6 +3642,17 @@ struct vring_used {
 \begin_layout Plain Layout
 
         struct vring_used_elem ring[];
+\change_inserted 0 1304340824
+
+\end_layout
+
+\begin_layout Plain Layout
+
+\change_inserted 0 1304340831
+
+        uint16_t avail_event;
+\change_unchanged
+
 \end_layout
 
 \begin_layout Plain Layout
@@ -3326,12 +3889,58 @@ static inline unsigned vring_size(unsigned int num, unsigned long align)
 
 \begin_layout Plain Layout
 
-                + sizeof(uint16_t)*2 + sizeof(struct vring_used_elem)*num;
+                + sizeof(uint16_t)*
+\change_deleted 0 1304340844
+2
+\change_inserted 0 1304340844
+3
+\change_unchanged
+ + sizeof(struct vring_used_elem)*num;
+\end_layout
+
+\begin_layout Plain Layout
+
+}
+\change_inserted 0 1304340918
+
+\end_layout
+
+\begin_layout Plain Layout
+
+\change_inserted 0 1304340918
+
+\end_layout
+
+\begin_layout Plain Layout
+
+\change_inserted 0 1304340987
+
+static inline int vring_need_event(uint16_t event_idx, uint16_t new_idx,
+ uint16_t old_idx)
+\end_layout
+
+\begin_layout Plain Layout
+
+\change_inserted 0 1304340944
+
+{
 \end_layout
 
 \begin_layout Plain Layout
 
+\change_inserted 0 1304341001
+
+         return (uint16_t)(new_idx - event_idx - 1) < (uint16_t)(new_idx
+ - old_idx); 
+\end_layout
+
+\begin_layout Plain Layout
+
+\change_inserted 0 1304340938
+
 }
+\change_unchanged
+
 \end_layout
 
 \begin_layout Plain Layout
@@ -3355,7 +3964,13 @@ Appendix B: Reserved Feature Bits
 \end_layout
 
 \begin_layout Standard
-Currently there are three device-independent feature bits defined:
+Currently there are 
+\change_inserted 0 1304540655
+six
+\change_deleted 0 1304330657
+three
+\change_unchanged
+ device-independent feature bits defined:
 \end_layout
 
 \begin_layout Description
@@ -3365,7 +3980,11 @@ VIRTIO_F_NOTIFY_ON_EMPTY
 
 (24) Negotiating this feature indicates that the driver wants an interrupt
  if the device runs out of available descriptors on a virtqueue, even though
- interrupts are suppressed using the VRING_AVAIL_F_NO_INTERRUPT flag.
+ interrupts are suppressed using the VRING_AVAIL_F_NO_INTERRUPT flag
+\change_inserted 0 1304341161
+ or the used_event field
+\change_unchanged
+.
  An example of this is the networking driver: it doesn't need to know every
  time a packet is transmitted, but it does need to free the transmitted
  packets a finite time after they are transmitted.
@@ -3390,6 +4009,31 @@ reference "sub:Indirect-Descriptors"
 \end_layout
 
 \begin_layout Description
+
+\change_inserted 0 1304331394
+VIRTIO_F_RING_USED_EVENT_IDX(29) This feature indicates that the device
+ should ignore the 
+\emph on
+flags
+\emph default
+ field in the available ring structure.
+ Instead, the
+\emph on
+ used_event
+\emph default
+ field in this structure is used by guest to suppress device interrupts.
+ If unset, the device should ignore the 
+\emph on
+used_event
+\emph default
+ field; the 
+\emph on
+flags
+\emph default
+ field is used
+\end_layout
+
+\begin_layout Description
 VIRTIO_F_BAD_FEATURE(30) This feature should never be negotiated by the
  guest; doing so is an indication that the guest is faulty
 \begin_inset Foot
@@ -3403,6 +4047,43 @@ An experimental virtio PCI driver contained in Linux version 2.6.25 had this
 \end_inset
 
 
+\change_inserted 0 1304330854
+
+\end_layout
+
+\begin_layout Description
+
+\change_inserted 0 1304330961
+VIRTIO_F_FEATURES_HIGH(31) This feature indicates that the device supports
+ feature bits 32:63.
+ If unset, feature bits 32:63 are unset.
+\end_layout
+
+\begin_layout Description
+
+\change_inserted 0 1304331390
+VIRTIO_F_RING_AVAIL_EVENT_IDX(32) This feature indicates that the driver
+ should ignore the 
+\emph on
+flags
+\emph default
+ field in the used ring structure.
+ Instead, the 
+\emph on
+avail_event
+\emph default
+ field in this structure is used by the device to suppress notifications.
+ If unset, the device should ignore the 
+\emph on
+avail_event
+\emph default
+ field; the 
+\emph on
+flags
+\emph default
+ field is used
+\change_unchanged
+
 \end_layout
 
 \begin_layout Chapter*
-- 
1.7.5.53.gc233e

^ permalink raw reply related

* Re: [PATCH 1/1] Staging: hv: Add the necessary dependencies to hyperv Kconfig
From: Greg KH @ 2011-05-04 19:58 UTC (permalink / raw)
  To: KY Srinivasan
  Cc: Randy Dunlap, virtualization@lists.osdl.org,
	devel@linuxdriverproject.org, H Hartley Sweeten, Haiyang Zhang,
	linux-kernel@vger.kernel.org
In-Reply-To: <6E21E5352C11B742B20C142EB499E0481DFE61@TK5EX14MBXC124.redmond.corp.microsoft.com>

On Wed, May 04, 2011 at 07:54:09PM +0000, KY Srinivasan wrote:
> 
> 
> > -----Original Message-----
> > From: devel-bounces@linuxdriverproject.org [mailto:devel-
> > bounces@linuxdriverproject.org] On Behalf Of Greg KH
> > Sent: Wednesday, May 04, 2011 3:40 PM
> > To: Randy Dunlap
> > Cc: virtualization@lists.osdl.org; devel@linuxdriverproject.org; H Hartley
> > Sweeten; Haiyang Zhang; linux-kernel@vger.kernel.org
> > Subject: Re: [PATCH 1/1] Staging: hv: Add the necessary dependencies to hyperv
> > Kconfig
> > 
> > On Wed, May 04, 2011 at 12:33:02PM -0700, Randy Dunlap wrote:
> > > On Wed, 4 May 2011 11:53:38 -0700 Greg KH wrote:
> > >
> > > > On Wed, May 04, 2011 at 01:44:27PM -0500, H Hartley Sweeten wrote:
> > > > > On Wednesday, May 04, 2011 11:52 AM, K. Y. Srinivasan wrote:
> > > > > >
> > > > > > The vmbus driver dependes on ACPI and PCI subsystems. Change
> > > > > > Kconfig to reflect this.
> > > > > >
> > > > > > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > > > > > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> > > > > > ---
> > > > > >  drivers/staging/hv/Kconfig |    2 +-
> > > > > >  1 files changed, 1 insertions(+), 1 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/staging/hv/Kconfig b/drivers/staging/hv/Kconfig
> > > > > > index 76f0756..5e0c9f6 100644
> > > > > > --- a/drivers/staging/hv/Kconfig
> > > > > > +++ b/drivers/staging/hv/Kconfig
> > > > > > @@ -1,6 +1,6 @@
> > > > > >  config HYPERV
> > > > > >  	tristate "Microsoft Hyper-V client drivers"
> > > > > > -	depends on X86 && m
> > > > > > +	depends on X86 && ACPI && PCI && m
> > > > > >  	default n
> > > > > >  	help
> > > > > > 	  Select this option to run Linux as a Hyper-V client operating
> > > > >
> > > > > ACPI itself depends on PCI.
> > > >
> > > > Still?  I thought that got removed a while ago.
> > >
> > > Still.  Len has written in the past that technically there is no such
> > > dependency, but in reality there is, for now at least.  IIRC.
> > 
> > Ok, but then we should still say we depend on PCI to handle if/when ACPI
> > removes that dependancy in the future.  It doesn't hurt here at all.
> 
> Greg,
> I have sent you  both versions of this patch; one with explicit dependency on PCI and
> one with implicit dependency on PCI; you decide.

Thanks, I'll pick one :)

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox