Skip to contentSkip to conversation

virtio-net: 64 bit features, event index

PATCH Superseded Michael S. Tsirkin 1 participant 9 messages May 4, 2011

Subsystems (3)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Michael S. Tsirkin

author maintainer
#0 lore raw

cover virtio-net: 64 bit features, event index

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(-)
Signature · 1 line
1.7.5.53.gc233e
author maintainer
#1 lore raw

PATCH 1/3 virtio/vhost: support 64 bit features

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

---
 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(-)
hw/qdev-properties.c +31 -8
index 1088a26..a933c9e 100644
@@ -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)
hw/qdev.h +10 -0
index 8a13ec9..e65cab0 100644
@@ -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)                      \
hw/s390-virtio-bus.c +3 -2
index 175e5cb..89e8c8e 100644
@@ -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 *******************/
hw/s390-virtio-bus.h +1 -1
index edf6d04..5c851e3 100644
@@ -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;
hw/syborg_virtio.c +4 -3
index ee08c49..b64c357 100644
@@ -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 = {
hw/vhost_net.c +4 -4
index 420e05f..7e94f61 100644
@@ -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
hw/vhost_net.h +2 -2
index 91e40b1..4069258 100644
@@ -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
hw/virtio-9p.c +1 -1
index 7e29535..184691a 100644
@@ -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;
hw/virtio-balloon.c +1 -1
index f9add1c..b34adc1 100644
@@ -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;
hw/virtio-blk.c +1 -1
index 91e0394..442ce11 100644
@@ -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);
 
hw/virtio-blk.h +1 -1
index fff46da..61104ea 100644
@@ -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)
hw/virtio-net.c +6 -5
index 6997e02..c242fd1 100644
@@ -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);
hw/virtio-net.h +17 -17
index 8af9a1c..8f8ac7f 100644
@@ -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
hw/virtio-pci.c +67 -24

196 lines not on this page · raw message

hw/virtio-serial-bus.c +1 -1
index 6227379..76acd02 100644
@@ -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;
 
hw/virtio.c +19 -5
index 31bd9e3..b9acbd4 100644
@@ -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;
     }
hw/virtio.h +10 -7
index bc72289..9517b97 100644
@@ -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);
Signature · 1 line
1.7.5.53.gc233e
author maintainer
#2 lore raw

PATCH 1/3 virtio/vhost: support 64 bit features

In reply to Michael S. Tsirkin

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

---
 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(-)
hw/qdev-properties.c +31 -8
index 1088a26..a933c9e 100644
@@ -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)
hw/qdev.h +10 -0
index 8a13ec9..e65cab0 100644
@@ -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)                      \
hw/s390-virtio-bus.c +3 -2
index 175e5cb..89e8c8e 100644
@@ -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 *******************/
hw/s390-virtio-bus.h +1 -1
index edf6d04..5c851e3 100644
@@ -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;
hw/syborg_virtio.c +4 -3
index ee08c49..b64c357 100644
@@ -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 = {
hw/vhost_net.c +4 -4
index 420e05f..7e94f61 100644
@@ -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
hw/vhost_net.h +2 -2
index 91e40b1..4069258 100644
@@ -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
hw/virtio-9p.c +1 -1
index 7e29535..184691a 100644
@@ -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;
hw/virtio-balloon.c +1 -1
index f9add1c..b34adc1 100644
@@ -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;
hw/virtio-blk.c +1 -1
index 91e0394..442ce11 100644
@@ -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);
 
hw/virtio-blk.h +1 -1
index fff46da..61104ea 100644
@@ -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)
hw/virtio-net.c +6 -5
index 6997e02..c242fd1 100644
@@ -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);
hw/virtio-net.h +17 -17
index 8af9a1c..8f8ac7f 100644
@@ -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
hw/virtio-pci.c +67 -24

196 lines not on this page · raw message

hw/virtio-serial-bus.c +1 -1
index 6227379..76acd02 100644
@@ -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;
 
hw/virtio.c +19 -5
index 31bd9e3..b9acbd4 100644
@@ -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;
     }
hw/virtio.h +10 -7
index bc72289..9517b97 100644
@@ -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);
Signature · 1 line
1.7.5.53.gc233e
author maintainer
#3 lore raw

PATCH 2/3 virtio+vhost: used_event feature

In reply to Michael S. Tsirkin

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

---
 hw/vhost_net.c |    6 +++++
 hw/virtio.c    |   66 ++++++++++++++++++++++++++++++++++++++++++++++++++------
 hw/virtio.h    |    7 +++++-
 3 files changed, 71 insertions(+), 8 deletions(-)
hw/vhost_net.c +6 -0
index 7e94f61..18e4653 100644
@@ -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);
     }
hw/virtio.c +59 -7

119 lines not on this page · raw message

hw/virtio.h +6 -1
index 9517b97..d765946 100644
@@ -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);
Signature · 1 line
1.7.5.53.gc233e
author maintainer
#4 lore raw

PATCH 2/3 virtio+vhost: used_event feature

In reply to Michael S. Tsirkin

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

---
 hw/vhost_net.c |    6 +++++
 hw/virtio.c    |   66 ++++++++++++++++++++++++++++++++++++++++++++++++++------
 hw/virtio.h    |    7 +++++-
 3 files changed, 71 insertions(+), 8 deletions(-)
hw/vhost_net.c +6 -0
index 7e94f61..18e4653 100644
@@ -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);
     }
hw/virtio.c +59 -7

119 lines not on this page · raw message

hw/virtio.h +6 -1
index 9517b97..d765946 100644
@@ -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);
Signature · 1 line
1.7.5.53.gc233e
author maintainer
#5 lore raw

PATCH 3/3 virtio: avail_event index support

In reply to Michael S. Tsirkin

Reduce the number of exits utilizing the
avail_event feature.

---
 hw/vhost_net.c |    6 ++++++
 hw/virtio.c    |   26 ++++++++++++++++++++++++--
 hw/virtio.h    |    7 ++++++-
 3 files changed, 36 insertions(+), 3 deletions(-)
hw/vhost_net.c +6 -0
index 18e4653..aeb5a84 100644
@@ -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);
     }
hw/virtio.c +24 -2
index e459093..6ae5542 100644
@@ -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;
hw/virtio.h +6 -1
index d765946..8c0923a 100644
@@ -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);
Signature · 1 line
1.7.5.53.gc233e
author maintainer
#6 lore raw

PATCH 3/3 virtio: avail_event index support

In reply to Michael S. Tsirkin

Reduce the number of exits utilizing the
avail_event feature.

---
 hw/vhost_net.c |    6 ++++++
 hw/virtio.c    |   26 ++++++++++++++++++++++++--
 hw/virtio.h    |    7 ++++++-
 3 files changed, 36 insertions(+), 3 deletions(-)
hw/vhost_net.c +6 -0
index 18e4653..aeb5a84 100644
@@ -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);
     }
hw/virtio.c +24 -2
index e459093..6ae5542 100644
@@ -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;
hw/virtio.h +6 -1
index d765946..8c0923a 100644
@@ -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);
Signature · 1 line
1.7.5.53.gc233e
author maintainer
#7 lore raw
In reply to Michael S. Tsirkin
Quoted 3 lines · Michael S. Tsirkin

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.

Sorry, forgot to mention this is on top of qemu-kvm d16e0f0 .

Copy is here:
git://git.kernel.org/pub/scm/linux/kernel/git/mst/qemu-kvm.git virtio-net-event-idx-v1

Relevant kernel bits are here:
git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git vhost-net-next-event-idx-v1

Signature · 1 line
MST
author maintainer
#8 lore raw
In reply to Michael S. Tsirkin
Quoted 3 lines · Michael S. Tsirkin

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.

Sorry, forgot to mention this is on top of qemu-kvm d16e0f0 .

Copy is here:
git://git.kernel.org/pub/scm/linux/kernel/git/mst/qemu-kvm.git virtio-net-event-idx-v1

Relevant kernel bits are here:
git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git vhost-net-next-event-idx-v1

Signature · 1 line
MST