[Qemu-devel] [PATCH v2 0/4] virtio-1/virtio-ccw related patches

[Qemu-devel] [PATCH v2 0/4] virtio-1/virtio-ccw related patches

[Cornelia Huck]

v2 of my recent virtio patches.

Patch 1 is the stateless version I sent out before. Patch 4 replaces
the broken version I sent out previously. R-bs have been added.

I can take patches 2-4 through the s390x tree.

Cornelia Huck (4):
  virtio: ring sizes vs. reset
  virtio-ccw: support ring size changes
  virtio-ccw: feature bits > 31 handling
  virtio-ccw: enable virtio-1

 hw/s390x/s390-virtio-ccw.c | 20 +++++++++++++++
 hw/s390x/virtio-ccw.c      | 64 +++++++++++++++++++++++++++++++++-------------
 hw/s390x/virtio-ccw.h      |  6 +++--
 hw/virtio/virtio.c         | 64 ++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 134 insertions(+), 20 deletions(-)

-- 
2.3.8

[Qemu-devel] [PATCH v2 1/4] virtio: ring sizes vs. reset

[Cornelia Huck]

We allow guests to change the size of the virtqueue rings by supplying
a number of buffers that is different from the number of buffers the
device was initialized with. Current code has some problems, however,
since reset does not reset the ringsizes to the default values (as this
is not saved anywhere).

Let's extend the core code to keep track of the default ringsizes and
migrate them once the guest changed them for any of the virtqueues
for a device.

Reviewed-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 hw/virtio/virtio.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 64 insertions(+)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 0832db9..787a319 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -60,6 +60,7 @@ typedef struct VRingUsed
 typedef struct VRing
 {
     unsigned int num;
+    unsigned int num_default;
     unsigned int align;
     hwaddr desc;
     hwaddr avail;
@@ -633,6 +634,7 @@ void virtio_reset(void *opaque)
         vdev->vq[i].signalled_used = 0;
         vdev->vq[i].signalled_used_valid = false;
         vdev->vq[i].notification = true;
+        vdev->vq[i].vring.num = vdev->vq[i].vring.num_default;
     }
 }
 
@@ -964,6 +966,7 @@ VirtQueue *virtio_add_queue(VirtIODevice *vdev, int queue_size,
         abort();
 
     vdev->vq[i].vring.num = queue_size;
+    vdev->vq[i].vring.num_default = queue_size;
     vdev->vq[i].vring.align = VIRTIO_PCI_VRING_ALIGN;
     vdev->vq[i].handle_output = handle_output;
 
@@ -977,6 +980,7 @@ void virtio_del_queue(VirtIODevice *vdev, int n)
     }
 
     vdev->vq[n].vring.num = 0;
+    vdev->vq[n].vring.num_default = 0;
 }
 
 void virtio_irq(VirtQueue *vq)
@@ -1056,6 +1060,19 @@ static bool virtio_virtqueue_needed(void *opaque)
     return virtio_host_has_feature(vdev, VIRTIO_F_VERSION_1);
 }
 
+static bool virtio_ringsize_needed(void *opaque)
+{
+    VirtIODevice *vdev = opaque;
+    int i;
+
+    for (i = 0; i < VIRTIO_QUEUE_MAX; i++) {
+        if (vdev->vq[i].vring.num != vdev->vq[i].vring.num_default) {
+            return true;
+        }
+    }
+    return false;
+}
+
 static void put_virtqueue_state(QEMUFile *f, void *pv, size_t size)
 {
     VirtIODevice *vdev = pv;
@@ -1104,6 +1121,52 @@ static const VMStateDescription vmstate_virtio_virtqueues = {
     }
 };
 
+static void put_ringsize_state(QEMUFile *f, void *pv, size_t size)
+{
+    VirtIODevice *vdev = pv;
+    int i;
+
+    for (i = 0; i < VIRTIO_QUEUE_MAX; i++) {
+        qemu_put_be32(f, vdev->vq[i].vring.num_default);
+    }
+}
+
+static int get_ringsize_state(QEMUFile *f, void *pv, size_t size)
+{
+    VirtIODevice *vdev = pv;
+    int i;
+
+    for (i = 0; i < VIRTIO_QUEUE_MAX; i++) {
+        vdev->vq[i].vring.num_default = qemu_get_be32(f);
+    }
+    return 0;
+}
+
+static VMStateInfo vmstate_info_ringsize = {
+    .name = "ringsize_state",
+    .get = get_ringsize_state,
+    .put = put_ringsize_state,
+};
+
+static const VMStateDescription vmstate_virtio_ringsize = {
+    .name = "virtio/ringsize",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = &virtio_ringsize_needed,
+    .fields = (VMStateField[]) {
+        {
+            .name         = "ringsize",
+            .version_id   = 0,
+            .field_exists = NULL,
+            .size         = 0,
+            .info         = &vmstate_info_ringsize,
+            .flags        = VMS_SINGLE,
+            .offset       = 0,
+        },
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static const VMStateDescription vmstate_virtio_device_endian = {
     .name = "virtio/device_endian",
     .version_id = 1,
@@ -1138,6 +1201,7 @@ static const VMStateDescription vmstate_virtio = {
         &vmstate_virtio_device_endian,
         &vmstate_virtio_64bit_features,
         &vmstate_virtio_virtqueues,
+        &vmstate_virtio_ringsize,
         NULL
     }
 };
-- 
2.3.8

[Qemu-devel] [PATCH v2 2/4] virtio-ccw: support ring size changes

[Cornelia Huck]

Wire up changing the ring size for virtio-1 devices.

Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 hw/s390x/virtio-ccw.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index d36373e..85e2a5d 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -307,11 +307,18 @@ static int virtio_ccw_set_vqs(SubchDev *sch, VqInfoBlock *info,
     if (!desc) {
         virtio_queue_set_vector(vdev, index, VIRTIO_NO_VECTOR);
     } else {
-        /* Fail if we don't have a big enough queue. */
-        /* TODO: Add interface to handle vring.num changing */
-        if (virtio_queue_get_num(vdev, index) > num) {
+        if (info) {
+            /* virtio-1 allows changing the ring size. */
+            if (virtio_queue_get_num(vdev, index) < num) {
+                /* Fail if we exceed the maximum number. */
+                return -EINVAL;
+            }
+            virtio_queue_set_num(vdev, index, num);
+        } else if (virtio_queue_get_num(vdev, index) > num) {
+            /* Fail if we don't have a big enough queue. */
             return -EINVAL;
         }
+        /* We ignore possible increased num for legacy for compatibility. */
         virtio_queue_set_vector(vdev, index, index);
     }
     /* tell notify handler in case of config change */
-- 
2.3.8

[Qemu-devel] [PATCH v2 3/4] virtio-ccw: feature bits > 31 handling

[Cornelia Huck]

We currently switch off the VERSION_1 feature bit if the guest has
not negotiated at least revision 1. As no feature bits beyond 31 are
valid however unless VERSION_1 has been negotiated, make sure that
legacy guests never see a feature bit beyond 31.

Reviewed-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 hw/s390x/virtio-ccw.c | 21 ++++++++-------------
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index 85e2a5d..eed7b3e 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -468,15 +468,12 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
                                                 NULL);
             if (features.index == 0) {
                 features.features = (uint32_t)vdev->host_features;
-            } else if (features.index == 1) {
-                features.features = (uint32_t)(vdev->host_features >> 32);
+            } else if ((features.index == 1) && (dev->revision >= 1)) {
                 /*
-                 * Don't offer version 1 to the guest if it did not
-                 * negotiate at least revision 1.
+                 * Only offer feature bits beyond 31 if the guest has
+                 * negotiated at least revision 1.
                  */
-                if (dev->revision <= 0) {
-                    features.features &= ~(1 << (VIRTIO_F_VERSION_1 - 32));
-                }
+                features.features = (uint32_t)(vdev->host_features >> 32);
             } else {
                 /* Return zeroes if the guest supports more feature bits. */
                 features.features = 0;
@@ -515,14 +512,12 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
                 virtio_set_features(vdev,
                                     (vdev->guest_features & 0xffffffff00000000ULL) |
                                     features.features);
-            } else if (features.index == 1) {
+            } else if ((features.index == 1) && (dev->revision >= 1)) {
                 /*
-                 * The guest should not set version 1 if it didn't
-                 * negotiate a revision >= 1.
+                 * If the guest did not negotiate at least revision 1,
+                 * we did not offer it any feature bits beyond 31. Such a
+                 * guest passing us any bit here is therefore buggy.
                  */
-                if (dev->revision <= 0) {
-                    features.features &= ~(1 << (VIRTIO_F_VERSION_1 - 32));
-                }
                 virtio_set_features(vdev,
                                     (vdev->guest_features & 0x00000000ffffffffULL) |
                                     ((uint64_t)features.features << 32));
-- 
2.3.8

[Qemu-devel] [PATCH v2 4/4] virtio-ccw: enable virtio-1

[Cornelia Huck]

Let's enable revision 1 for virtio-ccw devices. We can always offer
VERSION_1 as drivers in legacy mode won't be able to see it anyway.

We have to introduce a way to set a lower maximum revision for a device
to accommodate the following cases:
- compat machines (to enforce legacy only)
- virtio-blk with scsi support (version 1 + scsi is fenced by common
  code, with a user-configured max revision of 0 we can allow scsi
  via not offering VERSION_1)

Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 hw/s390x/s390-virtio-ccw.c | 20 ++++++++++++++++++++
 hw/s390x/virtio-ccw.c      | 30 ++++++++++++++++++++++++++++--
 hw/s390x/virtio-ccw.h      |  6 ++++--
 3 files changed, 52 insertions(+), 4 deletions(-)

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 27a8360..0062eb5 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -242,6 +242,26 @@ static const TypeInfo ccw_machine_info = {
             .driver   = TYPE_S390_SKEYS,\
             .property = "migration-enabled",\
             .value    = "off",\
+        },{\
+            .driver   = "virtio-blk-ccw",\
+            .property = "max_revision",\
+            .value    = "0",\
+        },{\
+            .driver   = "virtio-balloon-ccw",\
+            .property = "max_revision",\
+            .value    = "0",\
+        },{\
+            .driver   = "virtio-serial-ccw",\
+            .property = "max_revision",\
+            .value    = "0",\
+        },{\
+            .driver   = "virtio-9p-ccw",\
+            .property = "max_revision",\
+            .value    = "0",\
+        },{\
+            .driver   = "virtio-rng-ccw",\
+            .property = "max_revision",\
+            .value    = "0",\
         },
 
 static void ccw_machine_2_4_class_init(ObjectClass *oc, void *data)
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index eed7b3e..fb103b7 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -467,7 +467,13 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
                                                 MEMTXATTRS_UNSPECIFIED,
                                                 NULL);
             if (features.index == 0) {
-                features.features = (uint32_t)vdev->host_features;
+                if (dev->revision >= 1) {
+                    /* Don't offer legacy features for modern devices. */
+                    features.features = (uint32_t)
+                        (vdev->host_features & ~VIRTIO_LEGACY_FEATURES);
+                } else {
+                    features.features = (uint32_t)vdev->host_features;
+                }
             } else if ((features.index == 1) && (dev->revision >= 1)) {
                 /*
                  * Only offer feature bits beyond 31 if the guest has
@@ -768,7 +774,7 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
          * need to fetch it here. Nothing to do for now, though.
          */
         if (dev->revision >= 0 ||
-            revinfo.revision > virtio_ccw_rev_max(vdev)) {
+            revinfo.revision > virtio_ccw_rev_max(dev)) {
             ret = -ENOSYS;
             break;
         }
@@ -1541,6 +1547,10 @@ static void virtio_ccw_device_plugged(DeviceState *d, Error **errp)
 
     sch->id.cu_model = virtio_bus_get_vdev_id(&dev->bus);
 
+    if (dev->max_rev >= 1) {
+        virtio_add_feature(&vdev->host_features, VIRTIO_F_VERSION_1);
+    }
+
     css_generate_sch_crws(sch->cssid, sch->ssid, sch->schid,
                           d->hotplugged, 1);
 }
@@ -1557,6 +1567,8 @@ static Property virtio_ccw_net_properties[] = {
     DEFINE_PROP_STRING("devno", VirtioCcwDevice, bus_id),
     DEFINE_PROP_BIT("ioeventfd", VirtioCcwDevice, flags,
                     VIRTIO_CCW_FLAG_USE_IOEVENTFD_BIT, true),
+    DEFINE_PROP_UINT32("max_revision", VirtioCcwDevice, max_rev,
+                       VIRTIO_CCW_MAX_REV),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -1584,6 +1596,8 @@ static Property virtio_ccw_blk_properties[] = {
     DEFINE_PROP_STRING("devno", VirtioCcwDevice, bus_id),
     DEFINE_PROP_BIT("ioeventfd", VirtioCcwDevice, flags,
                     VIRTIO_CCW_FLAG_USE_IOEVENTFD_BIT, true),
+    DEFINE_PROP_UINT32("max_revision", VirtioCcwDevice, max_rev,
+                       VIRTIO_CCW_MAX_REV),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -1611,6 +1625,8 @@ static Property virtio_ccw_serial_properties[] = {
     DEFINE_PROP_STRING("devno", VirtioCcwDevice, bus_id),
     DEFINE_PROP_BIT("ioeventfd", VirtioCcwDevice, flags,
                     VIRTIO_CCW_FLAG_USE_IOEVENTFD_BIT, true),
+    DEFINE_PROP_UINT32("max_revision", VirtioCcwDevice, max_rev,
+                       VIRTIO_CCW_MAX_REV),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -1638,6 +1654,8 @@ static Property virtio_ccw_balloon_properties[] = {
     DEFINE_PROP_STRING("devno", VirtioCcwDevice, bus_id),
     DEFINE_PROP_BIT("ioeventfd", VirtioCcwDevice, flags,
                     VIRTIO_CCW_FLAG_USE_IOEVENTFD_BIT, true),
+    DEFINE_PROP_UINT32("max_revision", VirtioCcwDevice, max_rev,
+                       VIRTIO_CCW_MAX_REV),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -1665,6 +1683,8 @@ static Property virtio_ccw_scsi_properties[] = {
     DEFINE_PROP_STRING("devno", VirtioCcwDevice, bus_id),
     DEFINE_PROP_BIT("ioeventfd", VirtioCcwDevice, flags,
                     VIRTIO_CCW_FLAG_USE_IOEVENTFD_BIT, true),
+    DEFINE_PROP_UINT32("max_revision", VirtioCcwDevice, max_rev,
+                       VIRTIO_CCW_MAX_REV),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -1691,6 +1711,8 @@ static const TypeInfo virtio_ccw_scsi = {
 #ifdef CONFIG_VHOST_SCSI
 static Property vhost_ccw_scsi_properties[] = {
     DEFINE_PROP_STRING("devno", VirtioCcwDevice, bus_id),
+    DEFINE_PROP_UINT32("max_revision", VirtioCcwDevice, max_rev,
+                       VIRTIO_CCW_MAX_REV),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -1729,6 +1751,8 @@ static Property virtio_ccw_rng_properties[] = {
     DEFINE_PROP_STRING("devno", VirtioCcwDevice, bus_id),
     DEFINE_PROP_BIT("ioeventfd", VirtioCcwDevice, flags,
                     VIRTIO_CCW_FLAG_USE_IOEVENTFD_BIT, true),
+    DEFINE_PROP_UINT32("max_revision", VirtioCcwDevice, max_rev,
+                       VIRTIO_CCW_MAX_REV),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -1882,6 +1906,8 @@ static Property virtio_ccw_9p_properties[] = {
     DEFINE_PROP_STRING("devno", VirtioCcwDevice, bus_id),
     DEFINE_PROP_BIT("ioeventfd", VirtioCcwDevice, flags,
             VIRTIO_CCW_FLAG_USE_IOEVENTFD_BIT, true),
+    DEFINE_PROP_UINT32("max_revision", VirtioCcwDevice, max_rev,
+                       VIRTIO_CCW_MAX_REV),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/s390x/virtio-ccw.h b/hw/s390x/virtio-ccw.h
index 692ddd7..7ab8367 100644
--- a/hw/s390x/virtio-ccw.h
+++ b/hw/s390x/virtio-ccw.h
@@ -88,6 +88,7 @@ struct VirtioCcwDevice {
     SubchDev *sch;
     char *bus_id;
     int revision;
+    uint32_t max_rev;
     VirtioBusState bus;
     bool ioeventfd_started;
     bool ioeventfd_disabled;
@@ -102,9 +103,10 @@ struct VirtioCcwDevice {
 };
 
 /* The maximum virtio revision we support. */
-static inline int virtio_ccw_rev_max(VirtIODevice *vdev)
+#define VIRTIO_CCW_MAX_REV 1
+static inline int virtio_ccw_rev_max(VirtioCcwDevice *dev)
 {
-    return 0;
+    return dev->max_rev;
 }
 
 /* virtual css bus type */
-- 
2.3.8