[PATCH 0/7] Add VIRTIO_F_IN_ORDER support to vhost shadow virtqueue

[PATCH 0/7] Add VIRTIO_F_IN_ORDER support to vhost shadow virtqueue

[Eugenio Pérez]

This series adds support for the VIRTIO_F_IN_ORDER feature flag to vhost shadow
virtqueue (SVQ), enabling migration of vDPA devices that offer this feature.
As SVQ acts as a virtio driver, the series follows the Linux kernel
implementation for the feature by Jason Wang.  This enables the live migration
of vhost-vdpa devices that supports IN_ORDER.

The IN_ORDER feature allows virtio devices to use many descriptors in batch,
just by marking the last one of the set as used.

The series is structured in three parts.  First, max_steps calculation in
virtqueue_ordered_fill is fixed to allow filling the entire virtqueue at once.
Otherwise, a full queue cannot be used with just one used entry.

Afterwards, the series extracts helper functions for descriptor processing to
prepare for in order changes:

Finally, the series adds IN_ORDER support, first adding conditional logic for
in-order vs regular processing and whitelisting the feature flag in SVQ.

Eugenio Pérez (7):
  virtio: Allow to fill a whole virtqueue in order
  vhost: move svq next desc array to descs state struct
  vhost: factor out the descriptor next fetching
  vhost: factor out the get of last used desc in SVQ
  vhost: factor out the detach buf logic in SVQ
  vhost: add in_order feature to shadow virtqueue
  vhost: accept in order feature flag

 hw/virtio/vhost-shadow-virtqueue.c | 215 +++++++++++++++++++++++++----
 hw/virtio/vhost-shadow-virtqueue.h |  38 ++++-
 hw/virtio/virtio.c                 |   2 +-
 3 files changed, 219 insertions(+), 36 deletions(-)

-- 
2.53.0

[PATCH 1/7] virtio: Allow to fill a whole virtqueue in order

[Eugenio Pérez]

As the while steps < max_steps is already one less than the vq size, the
right maximum max_steps variable is queue length, not the maximum
possible remainder of % vq->vring.num.

Fixes: b44135daa37 ("virtio: virtqueue_ordered_fill - VIRTIO_F_IN_ORDER support")
Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
---
Personally I'd just remove max_steps and let it be vq->vring.num, but
let's make the minimal changes for now.
---
 hw/virtio/virtio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index e9d553295257..17f171551892 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -980,7 +980,7 @@ static void virtqueue_ordered_fill(VirtQueue *vq, const VirtQueueElement *elem,
      * We shouldn't need to increase 'i' by more than or equal to
      * the distance between used_idx and last_avail_idx (max_steps).
      */
-    max_steps = (vq->last_avail_idx - vq->used_idx) % vq->vring.num;
+    max_steps = MIN(vq->last_avail_idx - vq->used_idx, vq->vring.num);
 
     /* Search for element in vq->used_elems */
     while (steps < max_steps) {
-- 
2.53.0

[PATCH 2/7] vhost: move svq next desc array to descs state struct

[Eugenio Pérez]

It's the right place for it as it is part of the descriptor state.  We
save the memory management of the array, and make the code changes of
the next patches easier.

Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
---
 hw/virtio/vhost-shadow-virtqueue.c | 14 ++++++--------
 hw/virtio/vhost-shadow-virtqueue.h | 12 ++++++------
 2 files changed, 12 insertions(+), 14 deletions(-)

diff --git a/hw/virtio/vhost-shadow-virtqueue.c b/hw/virtio/vhost-shadow-virtqueue.c
index 6242aeb69c1a..901253d06e50 100644
--- a/hw/virtio/vhost-shadow-virtqueue.c
+++ b/hw/virtio/vhost-shadow-virtqueue.c
@@ -175,7 +175,7 @@ static bool vhost_svq_vring_write_descs(VhostShadowVirtqueue *svq, hwaddr *sg,
     for (n = 0; n < num; n++) {
         if (more_descs || (n + 1 < num)) {
             descs[i].flags = flags | cpu_to_le16(VRING_DESC_F_NEXT);
-            descs[i].next = cpu_to_le16(svq->desc_next[i]);
+            descs[i].next = cpu_to_le16(svq->desc_state[i].next);
         } else {
             descs[i].flags = flags;
         }
@@ -183,10 +183,10 @@ static bool vhost_svq_vring_write_descs(VhostShadowVirtqueue *svq, hwaddr *sg,
         descs[i].len = cpu_to_le32(iovec[n].iov_len);
 
         last = i;
-        i = svq->desc_next[i];
+        i = svq->desc_state[i].next;
     }
 
-    svq->free_head = svq->desc_next[last];
+    svq->free_head = svq->desc_state[last].next;
     return true;
 }
 
@@ -432,7 +432,7 @@ static uint16_t vhost_svq_last_desc_of_chain(const VhostShadowVirtqueue *svq,
                                              uint16_t num, uint16_t i)
 {
     for (uint16_t j = 0; j < (num - 1); ++j) {
-        i = svq->desc_next[i];
+        i = svq->desc_state[i].next;
     }
 
     return i;
@@ -473,7 +473,7 @@ static VirtQueueElement *vhost_svq_get_buf(VhostShadowVirtqueue *svq,
     num = svq->desc_state[used_elem.id].ndescs;
     svq->desc_state[used_elem.id].ndescs = 0;
     last_used_chain = vhost_svq_last_desc_of_chain(svq, num, used_elem.id);
-    svq->desc_next[last_used_chain] = svq->free_head;
+    svq->desc_state[last_used_chain].next = svq->free_head;
     svq->free_head = used_elem.id;
     svq->num_free += num;
 
@@ -705,9 +705,8 @@ void vhost_svq_start(VhostShadowVirtqueue *svq, VirtIODevice *vdev,
                            PROT_READ | PROT_WRITE, MAP_SHARED | MAP_ANONYMOUS,
                            -1, 0);
     svq->desc_state = g_new0(SVQDescState, svq->vring.num);
-    svq->desc_next = g_new0(uint16_t, svq->vring.num);
     for (unsigned i = 0; i < svq->vring.num - 1; i++) {
-        svq->desc_next[i] = i + 1;
+        svq->desc_state[i].next = i + 1;
     }
 }
 
@@ -744,7 +743,6 @@ void vhost_svq_stop(VhostShadowVirtqueue *svq)
         virtqueue_unpop(svq->vq, next_avail_elem, 0);
     }
     svq->vq = NULL;
-    g_free(svq->desc_next);
     g_free(svq->desc_state);
     munmap(svq->vring.desc, vhost_svq_driver_area_size(svq));
     munmap(svq->vring.used, vhost_svq_device_area_size(svq));
diff --git a/hw/virtio/vhost-shadow-virtqueue.h b/hw/virtio/vhost-shadow-virtqueue.h
index 9c273739d6df..f52c33e65046 100644
--- a/hw/virtio/vhost-shadow-virtqueue.h
+++ b/hw/virtio/vhost-shadow-virtqueue.h
@@ -23,6 +23,12 @@ typedef struct SVQDescState {
      * guest's
      */
     unsigned int ndescs;
+
+    /*
+     * Backup next field for each descriptor so we can recover securely, not
+     * needing to trust the device access.
+     */
+    uint16_t next;
 } SVQDescState;
 
 typedef struct VhostShadowVirtqueue VhostShadowVirtqueue;
@@ -84,12 +90,6 @@ typedef struct VhostShadowVirtqueue {
     /* Next VirtQueue element that guest made available */
     VirtQueueElement *next_guest_avail_elem;
 
-    /*
-     * Backup next field for each descriptor so we can recover securely, not
-     * needing to trust the device access.
-     */
-    uint16_t *desc_next;
-
     /* Caller callbacks */
     const VhostShadowVirtqueueOps *ops;
 
-- 
2.53.0

[PATCH 3/7] vhost: factor out the descriptor next fetching

[Eugenio Pérez]

The next field will not be used if IN_ORDER is enabled.

Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
---
 hw/virtio/vhost-shadow-virtqueue.c | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/hw/virtio/vhost-shadow-virtqueue.c b/hw/virtio/vhost-shadow-virtqueue.c
index 901253d06e50..7adacd4f542a 100644
--- a/hw/virtio/vhost-shadow-virtqueue.c
+++ b/hw/virtio/vhost-shadow-virtqueue.c
@@ -139,6 +139,20 @@ static bool vhost_svq_translate_addr(const VhostShadowVirtqueue *svq,
     return true;
 }
 
+/**
+ * Get the next descriptor in the chain in SVQ vring from a descriptor id
+ *
+ * @svq Shadow Virtqueue
+ * @id ID of the descriptor
+ *
+ * Return the id of the next descriptor.
+ */
+static uint16_t vhost_svq_next_desc(const VhostShadowVirtqueue *svq,
+                                    uint16_t id)
+{
+    return svq->desc_state[id].next;
+}
+
 /**
  * Write descriptors to SVQ vring
  *
@@ -173,9 +187,11 @@ static bool vhost_svq_vring_write_descs(VhostShadowVirtqueue *svq, hwaddr *sg,
     }
 
     for (n = 0; n < num; n++) {
+        uint16_t next = vhost_svq_next_desc(svq, i);
+
         if (more_descs || (n + 1 < num)) {
             descs[i].flags = flags | cpu_to_le16(VRING_DESC_F_NEXT);
-            descs[i].next = cpu_to_le16(svq->desc_state[i].next);
+            descs[i].next = cpu_to_le16(next);
         } else {
             descs[i].flags = flags;
         }
@@ -183,10 +199,10 @@ static bool vhost_svq_vring_write_descs(VhostShadowVirtqueue *svq, hwaddr *sg,
         descs[i].len = cpu_to_le32(iovec[n].iov_len);
 
         last = i;
-        i = svq->desc_state[i].next;
+        i = next;
     }
 
-    svq->free_head = svq->desc_state[last].next;
+    svq->free_head = vhost_svq_next_desc(svq, last);
     return true;
 }
 
@@ -432,7 +448,7 @@ static uint16_t vhost_svq_last_desc_of_chain(const VhostShadowVirtqueue *svq,
                                              uint16_t num, uint16_t i)
 {
     for (uint16_t j = 0; j < (num - 1); ++j) {
-        i = svq->desc_state[i].next;
+        i = vhost_svq_next_desc(svq, i);
     }
 
     return i;
-- 
2.53.0

[PATCH 4/7] vhost: factor out the get of last used desc in SVQ

[Eugenio Pérez]

This code path is modified to handle in order devices.  Abstract here so
we can generalize on the caller.

Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
---
 hw/virtio/vhost-shadow-virtqueue.c | 45 +++++++++++++++++++-----------
 1 file changed, 29 insertions(+), 16 deletions(-)

diff --git a/hw/virtio/vhost-shadow-virtqueue.c b/hw/virtio/vhost-shadow-virtqueue.c
index 7adacd4f542a..e7e3c9155cd0 100644
--- a/hw/virtio/vhost-shadow-virtqueue.c
+++ b/hw/virtio/vhost-shadow-virtqueue.c
@@ -444,6 +444,25 @@ static void vhost_svq_disable_notification(VhostShadowVirtqueue *svq)
     }
 }
 
+/*
+ * Gets the next buffer id and moves forward the used idx, so the next time
+ * SVQ calls this function will get the next one.
+ *
+ * @svq: Shadow VirtQueue
+ * @len: Consumed length by the device.
+ *
+ * Return the next descriptor consumed by the device.
+ */
+static uint16_t vhost_svq_get_last_used_split(VhostShadowVirtqueue *svq,
+                                              uint32_t *len)
+{
+    const vring_used_t *used = svq->vring.used;
+    uint16_t last_used = svq->last_used_idx++ & (svq->vring.num - 1);
+
+    *len = le32_to_cpu(used->ring[last_used].len);
+    return le32_to_cpu(used->ring[last_used].id);
+}
+
 static uint16_t vhost_svq_last_desc_of_chain(const VhostShadowVirtqueue *svq,
                                              uint16_t num, uint16_t i)
 {
@@ -458,8 +477,6 @@ G_GNUC_WARN_UNUSED_RESULT
 static VirtQueueElement *vhost_svq_get_buf(VhostShadowVirtqueue *svq,
                                            uint32_t *len)
 {
-    const vring_used_t *used = svq->vring.used;
-    vring_used_elem_t used_elem;
     uint16_t last_used, last_used_chain, num;
 
     if (!vhost_svq_more_used(svq)) {
@@ -468,33 +485,29 @@ static VirtQueueElement *vhost_svq_get_buf(VhostShadowVirtqueue *svq,
 
     /* Only get used array entries after they have been exposed by dev */
     smp_rmb();
-    last_used = svq->last_used_idx & (svq->vring.num - 1);
-    used_elem.id = le32_to_cpu(used->ring[last_used].id);
-    used_elem.len = le32_to_cpu(used->ring[last_used].len);
+    last_used = vhost_svq_get_last_used_split(svq, len);
 
-    svq->last_used_idx++;
-    if (unlikely(used_elem.id >= svq->vring.num)) {
+    if (unlikely(last_used >= svq->vring.num)) {
         qemu_log_mask(LOG_GUEST_ERROR, "Device %s says index %u is used",
-                      svq->vdev->name, used_elem.id);
+                      svq->vdev->name, last_used);
         return NULL;
     }
 
-    if (unlikely(!svq->desc_state[used_elem.id].ndescs)) {
+    if (unlikely(!svq->desc_state[last_used].ndescs)) {
         qemu_log_mask(LOG_GUEST_ERROR,
             "Device %s says index %u is used, but it was not available",
-            svq->vdev->name, used_elem.id);
+            svq->vdev->name, last_used);
         return NULL;
     }
 
-    num = svq->desc_state[used_elem.id].ndescs;
-    svq->desc_state[used_elem.id].ndescs = 0;
-    last_used_chain = vhost_svq_last_desc_of_chain(svq, num, used_elem.id);
+    num = svq->desc_state[last_used].ndescs;
+    svq->desc_state[last_used].ndescs = 0;
+    last_used_chain = vhost_svq_last_desc_of_chain(svq, num, last_used);
     svq->desc_state[last_used_chain].next = svq->free_head;
-    svq->free_head = used_elem.id;
+    svq->free_head = last_used;
     svq->num_free += num;
 
-    *len = used_elem.len;
-    return g_steal_pointer(&svq->desc_state[used_elem.id].elem);
+    return g_steal_pointer(&svq->desc_state[last_used].elem);
 }
 
 /**
-- 
2.53.0

[PATCH 5/7] vhost: factor out the detach buf logic in SVQ

[Eugenio Pérez]

This code path is modified to handle in order devices.  Abstract here so
we can generalize on the caller.

Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
---
 hw/virtio/vhost-shadow-virtqueue.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/hw/virtio/vhost-shadow-virtqueue.c b/hw/virtio/vhost-shadow-virtqueue.c
index e7e3c9155cd0..2d8fc82cc06f 100644
--- a/hw/virtio/vhost-shadow-virtqueue.c
+++ b/hw/virtio/vhost-shadow-virtqueue.c
@@ -473,11 +473,24 @@ static uint16_t vhost_svq_last_desc_of_chain(const VhostShadowVirtqueue *svq,
     return i;
 }
 
+G_GNUC_WARN_UNUSED_RESULT
+static VirtQueueElement *vhost_svq_detach_buf(VhostShadowVirtqueue *svq,
+                                              uint16_t id)
+{
+    uint16_t num = svq->desc_state[id].ndescs;
+    uint16_t last_used_chain = vhost_svq_last_desc_of_chain(svq, num, id);
+
+    svq->desc_state[last_used_chain].next = svq->free_head;
+    svq->free_head = id;
+
+    return g_steal_pointer(&svq->desc_state[id].elem);
+}
+
 G_GNUC_WARN_UNUSED_RESULT
 static VirtQueueElement *vhost_svq_get_buf(VhostShadowVirtqueue *svq,
                                            uint32_t *len)
 {
-    uint16_t last_used, last_used_chain, num;
+    uint16_t last_used;
 
     if (!vhost_svq_more_used(svq)) {
         return NULL;
@@ -500,14 +513,9 @@ static VirtQueueElement *vhost_svq_get_buf(VhostShadowVirtqueue *svq,
         return NULL;
     }
 
-    num = svq->desc_state[last_used].ndescs;
+    svq->num_free += svq->desc_state[last_used].ndescs;
     svq->desc_state[last_used].ndescs = 0;
-    last_used_chain = vhost_svq_last_desc_of_chain(svq, num, last_used);
-    svq->desc_state[last_used_chain].next = svq->free_head;
-    svq->free_head = last_used;
-    svq->num_free += num;
-
-    return g_steal_pointer(&svq->desc_state[last_used].elem);
+    return vhost_svq_detach_buf(svq, last_used);
 }
 
 /**
-- 
2.53.0

[PATCH 6/7] vhost: add in_order feature to shadow virtqueue

[Eugenio Pérez]

Some vdpa devices benefit from the in order feature.  Add support to SVQ
so QEMU can migrate these.

Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
---
 hw/virtio/vhost-shadow-virtqueue.c | 137 +++++++++++++++++++++++++++--
 hw/virtio/vhost-shadow-virtqueue.h |  36 ++++++--
 2 files changed, 160 insertions(+), 13 deletions(-)

diff --git a/hw/virtio/vhost-shadow-virtqueue.c b/hw/virtio/vhost-shadow-virtqueue.c
index 2d8fc82cc06f..60212fcd7bf3 100644
--- a/hw/virtio/vhost-shadow-virtqueue.c
+++ b/hw/virtio/vhost-shadow-virtqueue.c
@@ -12,11 +12,14 @@
 
 #include "qemu/error-report.h"
 #include "qapi/error.h"
+#include "qemu/iov.h"
 #include "qemu/main-loop.h"
 #include "qemu/log.h"
 #include "qemu/memalign.h"
 #include "linux-headers/linux/vhost.h"
 
+#define VIRTIO_RING_NOT_IN_BATCH UINT16_MAX
+
 /**
  * Validate the transport device features that both guests can use with the SVQ
  * and SVQs can use with the device.
@@ -150,7 +153,33 @@ static bool vhost_svq_translate_addr(const VhostShadowVirtqueue *svq,
 static uint16_t vhost_svq_next_desc(const VhostShadowVirtqueue *svq,
                                     uint16_t id)
 {
-    return svq->desc_state[id].next;
+    if (virtio_vdev_has_feature(svq->vdev, VIRTIO_F_IN_ORDER)) {
+        return (id == svq->vring.num) ? 0 : ++id;
+    } else {
+        return svq->desc_state[id].next;
+    }
+}
+
+/**
+ * Updates the SVQ free_head member after adding them to the SVQ avail ring.
+ * The new free_head is the next descriptor that SVQ will make available by
+ * forwarding a new guest descriptor.
+ *
+ * @svq Shadow Virtqueue
+ * @num Number of descriptors added
+ * @id ID of the last descriptor added to the SVQ avail ring.
+ */
+static void vhost_svq_update_free_head(VhostShadowVirtqueue *svq,
+                                       size_t num, uint16_t id)
+{
+    if (virtio_vdev_has_feature(svq->vdev, VIRTIO_F_IN_ORDER)) {
+        svq->free_head += num;
+        if (svq->free_head >= svq->vring.num) {
+            svq->free_head -= svq->vring.num;
+        }
+    } else {
+        svq->free_head = vhost_svq_next_desc(svq, id);
+    }
 }
 
 /**
@@ -202,7 +231,7 @@ static bool vhost_svq_vring_write_descs(VhostShadowVirtqueue *svq, hwaddr *sg,
         i = next;
     }
 
-    svq->free_head = vhost_svq_next_desc(svq, last);
+    vhost_svq_update_free_head(svq, num, last);
     return true;
 }
 
@@ -306,6 +335,9 @@ int vhost_svq_add(VhostShadowVirtqueue *svq, const struct iovec *out_sg,
     svq->num_free -= ndescs;
     svq->desc_state[qemu_head].elem = elem;
     svq->desc_state[qemu_head].ndescs = ndescs;
+    if (virtio_vdev_has_feature(svq->vdev, VIRTIO_F_IN_ORDER)) {
+        svq->desc_state[qemu_head].in_bytes = iov_size(in_sg, in_num);
+    }
     vhost_svq_kick(svq);
     return 0;
 }
@@ -401,6 +433,12 @@ static void vhost_handle_guest_kick_notifier(EventNotifier *n)
 static bool vhost_svq_more_used(VhostShadowVirtqueue *svq)
 {
     uint16_t *used_idx = &svq->vring.used->idx;
+
+    if (virtio_vdev_has_feature(svq->vdev, VIRTIO_F_IN_ORDER) &&
+        svq->batch_last.id != VIRTIO_RING_NOT_IN_BATCH) {
+        return true;
+    }
+
     if (svq->last_used_idx != svq->shadow_used_idx) {
         return true;
     }
@@ -463,6 +501,47 @@ static uint16_t vhost_svq_get_last_used_split(VhostShadowVirtqueue *svq,
     return le32_to_cpu(used->ring[last_used].id);
 }
 
+/*
+ * Gets the next buffer id and moves forward the used idx, so the next time
+ * SVQ calls this function will get the next one.  IN_ORDER version
+ *
+ * @svq: Shadow VirtQueue
+ * @len: Consumed length by the device.
+ *
+ * Return the next descriptor consumed by the device.
+ */
+static int32_t vhost_svq_get_last_used_split_in_order(
+        VhostShadowVirtqueue *svq,
+        uint32_t *len)
+{
+    unsigned num = svq->vring.num;
+    const vring_used_t *used = svq->vring.used;
+    uint16_t last_used = svq->last_used & (num - 1);
+    uint16_t last_used_idx = svq->last_used_idx & (num - 1);
+
+    if (svq->batch_last.id == VIRTIO_RING_NOT_IN_BATCH) {
+        svq->batch_last.id = le32_to_cpu(used->ring[last_used_idx].id);
+        svq->batch_last.len = le32_to_cpu(used->ring[last_used_idx].len);
+    }
+
+    if (unlikely(last_used >= num)) {
+        qemu_log_mask(LOG_GUEST_ERROR, "Device %s says index %u is used",
+                      svq->vdev->name, last_used);
+        return -1;
+    }
+
+    if (svq->batch_last.id == last_used) {
+        svq->batch_last.id = VIRTIO_RING_NOT_IN_BATCH;
+        *len = svq->batch_last.len;
+    } else {
+        *len = svq->desc_state[last_used].in_bytes;
+    }
+
+    svq->last_used += svq->desc_state[last_used].ndescs;
+    svq->last_used_idx++;
+    return last_used;
+}
+
 static uint16_t vhost_svq_last_desc_of_chain(const VhostShadowVirtqueue *svq,
                                              uint16_t num, uint16_t i)
 {
@@ -474,8 +553,8 @@ static uint16_t vhost_svq_last_desc_of_chain(const VhostShadowVirtqueue *svq,
 }
 
 G_GNUC_WARN_UNUSED_RESULT
-static VirtQueueElement *vhost_svq_detach_buf(VhostShadowVirtqueue *svq,
-                                              uint16_t id)
+static VirtQueueElement *vhost_svq_detach_buf_split(VhostShadowVirtqueue *svq,
+                                                    uint16_t id)
 {
     uint16_t num = svq->desc_state[id].ndescs;
     uint16_t last_used_chain = vhost_svq_last_desc_of_chain(svq, num, id);
@@ -486,6 +565,33 @@ static VirtQueueElement *vhost_svq_detach_buf(VhostShadowVirtqueue *svq,
     return g_steal_pointer(&svq->desc_state[id].elem);
 }
 
+G_GNUC_WARN_UNUSED_RESULT
+static VirtQueueElement *vhost_svq_detach_buf_split_in_order(
+        VhostShadowVirtqueue *svq,
+        uint16_t id)
+{
+    return g_steal_pointer(&svq->desc_state[id].elem);
+}
+
+/*
+ * Return the descriptor id (and the chain of ids) to the free list
+ *
+ * @svq: Shadow Virtqueue
+ * @id: Id of the buffer to return.
+ *
+ * Return the element associated to the buffer if any.
+ */
+G_GNUC_WARN_UNUSED_RESULT
+static VirtQueueElement *vhost_svq_detach_buf(VhostShadowVirtqueue *svq,
+                                              uint16_t id)
+{
+    if (virtio_vdev_has_feature(svq->vdev, VIRTIO_F_IN_ORDER)) {
+        return vhost_svq_detach_buf_split_in_order(svq, id);
+    } else {
+        return vhost_svq_detach_buf_split(svq, id);
+    }
+}
+
 G_GNUC_WARN_UNUSED_RESULT
 static VirtQueueElement *vhost_svq_get_buf(VhostShadowVirtqueue *svq,
                                            uint32_t *len)
@@ -498,7 +604,18 @@ static VirtQueueElement *vhost_svq_get_buf(VhostShadowVirtqueue *svq,
 
     /* Only get used array entries after they have been exposed by dev */
     smp_rmb();
-    last_used = vhost_svq_get_last_used_split(svq, len);
+
+    if (virtio_vdev_has_feature(svq->vdev, VIRTIO_F_IN_ORDER)) {
+        int32_t r;
+        r = vhost_svq_get_last_used_split_in_order(svq, len);
+        if (r < 0) {
+            return NULL;
+        }
+
+        last_used = r;
+    } else {
+        last_used = vhost_svq_get_last_used_split(svq, len);
+    }
 
     if (unlikely(last_used >= svq->vring.num)) {
         qemu_log_mask(LOG_GUEST_ERROR, "Device %s says index %u is used",
@@ -726,6 +843,8 @@ void vhost_svq_start(VhostShadowVirtqueue *svq, VirtIODevice *vdev,
     svq->next_guest_avail_elem = NULL;
     svq->shadow_avail_idx = 0;
     svq->shadow_used_idx = 0;
+    memset(&svq->batch_last, 0, sizeof(svq->batch_last));
+    svq->last_used = 0;
     svq->last_used_idx = 0;
     svq->vdev = vdev;
     svq->vq = vq;
@@ -742,8 +861,12 @@ void vhost_svq_start(VhostShadowVirtqueue *svq, VirtIODevice *vdev,
                            PROT_READ | PROT_WRITE, MAP_SHARED | MAP_ANONYMOUS,
                            -1, 0);
     svq->desc_state = g_new0(SVQDescState, svq->vring.num);
-    for (unsigned i = 0; i < svq->vring.num - 1; i++) {
-        svq->desc_state[i].next = i + 1;
+    if (virtio_vdev_has_feature(svq->vdev, VIRTIO_F_IN_ORDER)) {
+        svq->batch_last.id = VIRTIO_RING_NOT_IN_BATCH;
+    } else {
+        for (unsigned i = 0; i < svq->vring.num - 1; i++) {
+            svq->desc_state[i].next = i + 1;
+        }
     }
 }
 
diff --git a/hw/virtio/vhost-shadow-virtqueue.h b/hw/virtio/vhost-shadow-virtqueue.h
index f52c33e65046..ec16a1e83858 100644
--- a/hw/virtio/vhost-shadow-virtqueue.h
+++ b/hw/virtio/vhost-shadow-virtqueue.h
@@ -24,11 +24,19 @@ typedef struct SVQDescState {
      */
     unsigned int ndescs;
 
-    /*
-     * Backup next field for each descriptor so we can recover securely, not
-     * needing to trust the device access.
-     */
-    uint16_t next;
+    union {
+        /*
+         * Total length of the available buffer that is writable by the device.
+         * Only used in packed vq.
+         */
+        uint32_t in_bytes;
+
+        /*
+         * Backup next field for each descriptor so we can recover securely, not
+         * needing to trust the device access.  Only used in split vq.
+         */
+        uint16_t next;
+    };
 } SVQDescState;
 
 typedef struct VhostShadowVirtqueue VhostShadowVirtqueue;
@@ -99,9 +107,25 @@ typedef struct VhostShadowVirtqueue {
     /* Next head to expose to the device */
     uint16_t shadow_avail_idx;
 
-    /* Next free descriptor */
+    /*
+     * Next free descriptor.
+     *
+     * Without IN_ORDER free_head is used as a linked list head, and
+     * desc_next[id] is the next element.
+     * With IN_ORDER free_head is the next available buffer index.
+     */
     uint16_t free_head;
 
+    /*
+     * Last used element of the processing batch of used descriptors if
+     * IN_ORDER.
+     * If SVQ is not processing a batch of descriptors id is set to UINT_MAX.
+     */
+    vring_used_elem_t batch_last;
+
+    /* Last used id if IN_ORDER and split vq */
+    uint16_t last_used;
+
     /* Last seen used idx */
     uint16_t shadow_used_idx;
 
-- 
2.53.0

[PATCH 7/7] vhost: accept in order feature flag

[Eugenio Pérez]

Let's accept the feature flag with all the infrastructure to process it
in place.

Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
---
 hw/virtio/vhost-shadow-virtqueue.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/virtio/vhost-shadow-virtqueue.c b/hw/virtio/vhost-shadow-virtqueue.c
index 60212fcd7bf3..bcb7f2ffc79f 100644
--- a/hw/virtio/vhost-shadow-virtqueue.c
+++ b/hw/virtio/vhost-shadow-virtqueue.c
@@ -38,6 +38,7 @@ bool vhost_svq_valid_features(uint64_t features, Error **errp)
         case VIRTIO_F_ANY_LAYOUT:
         case VIRTIO_RING_F_EVENT_IDX:
         case VIRTIO_RING_F_INDIRECT_DESC:
+        case VIRTIO_F_IN_ORDER:
             continue;
 
         case VIRTIO_F_ACCESS_PLATFORM:
-- 
2.53.0

Re: [PATCH 1/7] virtio: Allow to fill a whole virtqueue in order

[Jason Wang]

On Thu, Mar 5, 2026 at 1:35 AM Eugenio Pérez <eperezma@redhat.com> wrote:
>
> As the while steps < max_steps is already one less than the vq size, the
> right maximum max_steps variable is queue length, not the maximum
> possible remainder of % vq->vring.num.
>
> Fixes: b44135daa37 ("virtio: virtqueue_ordered_fill - VIRTIO_F_IN_ORDER support")
> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> ---
> Personally I'd just remove max_steps and let it be vq->vring.num, but
> let's make the minimal changes for now.
> ---
>  hw/virtio/virtio.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index e9d553295257..17f171551892 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -980,7 +980,7 @@ static void virtqueue_ordered_fill(VirtQueue *vq, const VirtQueueElement *elem,
>       * We shouldn't need to increase 'i' by more than or equal to
>       * the distance between used_idx and last_avail_idx (max_steps).
>       */
> -    max_steps = (vq->last_avail_idx - vq->used_idx) % vq->vring.num;
> +    max_steps = MIN(vq->last_avail_idx - vq->used_idx, vq->vring.num);
>
>      /* Search for element in vq->used_elems */
>      while (steps < max_steps) {

Not directly related but I found that the virtqueue_ordered_fill() and
virtqueue_ordered_flush() can "emulate" an in-order device even if it
isn't one. I'm not sure this is good for the device that can benefit
from out of order completion like block devices.

It seems the correct approach is to let the device decide whether it
can operate in order, not the virtio core (as vhost-kernel did).

Thanks

> --
> 2.53.0
>

Re: [PATCH 1/7] virtio: Allow to fill a whole virtqueue in order

[Eugenio Perez Martin]

On Fri, Mar 6, 2026 at 4:26 AM Jason Wang <jasowang@redhat.com> wrote:
>
> On Thu, Mar 5, 2026 at 1:35 AM Eugenio Pérez <eperezma@redhat.com> wrote:
> >
> > As the while steps < max_steps is already one less than the vq size, the
> > right maximum max_steps variable is queue length, not the maximum
> > possible remainder of % vq->vring.num.
> >
> > Fixes: b44135daa37 ("virtio: virtqueue_ordered_fill - VIRTIO_F_IN_ORDER support")
> > Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> > ---
> > Personally I'd just remove max_steps and let it be vq->vring.num, but
> > let's make the minimal changes for now.
> > ---
> >  hw/virtio/virtio.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> > index e9d553295257..17f171551892 100644
> > --- a/hw/virtio/virtio.c
> > +++ b/hw/virtio/virtio.c
> > @@ -980,7 +980,7 @@ static void virtqueue_ordered_fill(VirtQueue *vq, const VirtQueueElement *elem,
> >       * We shouldn't need to increase 'i' by more than or equal to
> >       * the distance between used_idx and last_avail_idx (max_steps).
> >       */
> > -    max_steps = (vq->last_avail_idx - vq->used_idx) % vq->vring.num;
> > +    max_steps = MIN(vq->last_avail_idx - vq->used_idx, vq->vring.num);
> >
> >      /* Search for element in vq->used_elems */
> >      while (steps < max_steps) {
>
> Not directly related but I found that the virtqueue_ordered_fill() and
> virtqueue_ordered_flush() can "emulate" an in-order device even if it
> isn't one. I'm not sure this is good for the device that can benefit
> from out of order completion like block devices.
>

I guess the benefit is significantly reduced if the device cannot use
more than one descriptor in the same used entry.

> It seems the correct approach is to let the device decide whether it
> can operate in order, not the virtio core (as vhost-kernel did).
>

I agree with this.

Currently, `in_order` is off by default but we could enable it by
default per-device.

Re: [PATCH 1/7] virtio: Allow to fill a whole virtqueue in order

[Jason Wang]

On Fri, Mar 6, 2026 at 2:23 PM Eugenio Perez Martin <eperezma@redhat.com> wrote:
>
> On Fri, Mar 6, 2026 at 4:26 AM Jason Wang <jasowang@redhat.com> wrote:
> >
> > On Thu, Mar 5, 2026 at 1:35 AM Eugenio Pérez <eperezma@redhat.com> wrote:
> > >
> > > As the while steps < max_steps is already one less than the vq size, the
> > > right maximum max_steps variable is queue length, not the maximum
> > > possible remainder of % vq->vring.num.
> > >
> > > Fixes: b44135daa37 ("virtio: virtqueue_ordered_fill - VIRTIO_F_IN_ORDER support")
> > > Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> > > ---
> > > Personally I'd just remove max_steps and let it be vq->vring.num, but
> > > let's make the minimal changes for now.
> > > ---
> > >  hw/virtio/virtio.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> > > index e9d553295257..17f171551892 100644
> > > --- a/hw/virtio/virtio.c
> > > +++ b/hw/virtio/virtio.c
> > > @@ -980,7 +980,7 @@ static void virtqueue_ordered_fill(VirtQueue *vq, const VirtQueueElement *elem,
> > >       * We shouldn't need to increase 'i' by more than or equal to
> > >       * the distance between used_idx and last_avail_idx (max_steps).
> > >       */
> > > -    max_steps = (vq->last_avail_idx - vq->used_idx) % vq->vring.num;
> > > +    max_steps = MIN(vq->last_avail_idx - vq->used_idx, vq->vring.num);
> > >
> > >      /* Search for element in vq->used_elems */
> > >      while (steps < max_steps) {
> >
> > Not directly related but I found that the virtqueue_ordered_fill() and
> > virtqueue_ordered_flush() can "emulate" an in-order device even if it
> > isn't one. I'm not sure this is good for the device that can benefit
> > from out of order completion like block devices.
> >
>
> I guess the benefit is significantly reduced if the device cannot use
> more than one descriptor in the same used entry.
>
> > It seems the correct approach is to let the device decide whether it
> > can operate in order, not the virtio core (as vhost-kernel did).
> >
>
> I agree with this.
>
> Currently, `in_order` is off by default but we could enable it by
> default per-device.

After further thought, there's another reason. For example, for a
networking device, virtio-net should know nothing about the networking
backend (though some tricks exist in the code). So it can't know if
the backend can handle OOO or not. Starting with this seems to be
fine.

Thanks

>

Re: [PATCH 1/7] virtio: Allow to fill a whole virtqueue in order

[Eugenio Perez Martin]

On Mon, Mar 9, 2026 at 4:17 AM Jason Wang <jasowang@redhat.com> wrote:
>
> On Fri, Mar 6, 2026 at 2:23 PM Eugenio Perez Martin <eperezma@redhat.com> wrote:
> >
> > On Fri, Mar 6, 2026 at 4:26 AM Jason Wang <jasowang@redhat.com> wrote:
> > >
> > > On Thu, Mar 5, 2026 at 1:35 AM Eugenio Pérez <eperezma@redhat.com> wrote:
> > > >
> > > > As the while steps < max_steps is already one less than the vq size, the
> > > > right maximum max_steps variable is queue length, not the maximum
> > > > possible remainder of % vq->vring.num.
> > > >
> > > > Fixes: b44135daa37 ("virtio: virtqueue_ordered_fill - VIRTIO_F_IN_ORDER support")
> > > > Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> > > > ---
> > > > Personally I'd just remove max_steps and let it be vq->vring.num, but
> > > > let's make the minimal changes for now.
> > > > ---
> > > >  hw/virtio/virtio.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> > > > index e9d553295257..17f171551892 100644
> > > > --- a/hw/virtio/virtio.c
> > > > +++ b/hw/virtio/virtio.c
> > > > @@ -980,7 +980,7 @@ static void virtqueue_ordered_fill(VirtQueue *vq, const VirtQueueElement *elem,
> > > >       * We shouldn't need to increase 'i' by more than or equal to
> > > >       * the distance between used_idx and last_avail_idx (max_steps).
> > > >       */
> > > > -    max_steps = (vq->last_avail_idx - vq->used_idx) % vq->vring.num;
> > > > +    max_steps = MIN(vq->last_avail_idx - vq->used_idx, vq->vring.num);
> > > >
> > > >      /* Search for element in vq->used_elems */
> > > >      while (steps < max_steps) {
> > >
> > > Not directly related but I found that the virtqueue_ordered_fill() and
> > > virtqueue_ordered_flush() can "emulate" an in-order device even if it
> > > isn't one. I'm not sure this is good for the device that can benefit
> > > from out of order completion like block devices.
> > >
> >
> > I guess the benefit is significantly reduced if the device cannot use
> > more than one descriptor in the same used entry.
> >
> > > It seems the correct approach is to let the device decide whether it
> > > can operate in order, not the virtio core (as vhost-kernel did).
> > >
> >
> > I agree with this.
> >
> > Currently, `in_order` is off by default but we could enable it by
> > default per-device.
>
> After further thought, there's another reason. For example, for a
> networking device, virtio-net should know nothing about the networking
> backend (though some tricks exist in the code). So it can't know if
> the backend can handle OOO or not. Starting with this seems to be
> fine.
>

Well, if the backend exposes in_order, I'd assume it works better if
the feature is acknowledged. I was talking just about the emulated
device.

I kind of agree with what you say for virtio-net+kernel tap or
vhost-kernel case, but we're already assuming that the virtio-net
backend works in order in Live Migration anyway. With worse
consecuences that less performance. To be coherent with your mention
here, we should support out of order descriptors in live migration.
Not saying that it is urgent, just pointing it out.

Re: [PATCH 2/7] vhost: move svq next desc array to descs state struct

[Jason Wang]

On Thu, Mar 5, 2026 at 1:35 AM Eugenio Pérez <eperezma@redhat.com> wrote:
>
> It's the right place for it as it is part of the descriptor state.  We
> save the memory management of the array, and make the code changes of
> the next patches easier.
>
> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> ---

Acked-by: Jason Wang <jasowang@redhat.com>

Thanks

Re: [PATCH 3/7] vhost: factor out the descriptor next fetching

[Jason Wang]

On Thu, Mar 5, 2026 at 1:35 AM Eugenio Pérez <eperezma@redhat.com> wrote:
>
> The next field will not be used if IN_ORDER is enabled.
>
> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> ---

Acked-by: Jason Wang <jasowang@redhat.com>

Thanks

Re: [PATCH 5/7] vhost: factor out the detach buf logic in SVQ

[Jason Wang]

On Thu, Mar 5, 2026 at 1:36 AM Eugenio Pérez <eperezma@redhat.com> wrote:
>
> This code path is modified to handle in order devices.  Abstract here so
> we can generalize on the caller.
>
> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> ---
>  hw/virtio/vhost-shadow-virtqueue.c | 24 ++++++++++++++++--------
>  1 file changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/hw/virtio/vhost-shadow-virtqueue.c b/hw/virtio/vhost-shadow-virtqueue.c
> index e7e3c9155cd0..2d8fc82cc06f 100644
> --- a/hw/virtio/vhost-shadow-virtqueue.c
> +++ b/hw/virtio/vhost-shadow-virtqueue.c
> @@ -473,11 +473,24 @@ static uint16_t vhost_svq_last_desc_of_chain(const VhostShadowVirtqueue *svq,
>      return i;
>  }
>
> +G_GNUC_WARN_UNUSED_RESULT
> +static VirtQueueElement *vhost_svq_detach_buf(VhostShadowVirtqueue *svq,
> +                                              uint16_t id)
> +{
> +    uint16_t num = svq->desc_state[id].ndescs;
> +    uint16_t last_used_chain = vhost_svq_last_desc_of_chain(svq, num, id);
> +
> +    svq->desc_state[last_used_chain].next = svq->free_head;
> +    svq->free_head = id;
> +
> +    return g_steal_pointer(&svq->desc_state[id].elem);
> +}
> +
>  G_GNUC_WARN_UNUSED_RESULT
>  static VirtQueueElement *vhost_svq_get_buf(VhostShadowVirtqueue *svq,
>                                             uint32_t *len)
>  {
> -    uint16_t last_used, last_used_chain, num;
> +    uint16_t last_used;
>
>      if (!vhost_svq_more_used(svq)) {
>          return NULL;
> @@ -500,14 +513,9 @@ static VirtQueueElement *vhost_svq_get_buf(VhostShadowVirtqueue *svq,
>          return NULL;
>      }
>
> -    num = svq->desc_state[last_used].ndescs;
> +    svq->num_free += svq->desc_state[last_used].ndescs;
>      svq->desc_state[last_used].ndescs = 0;

Any reason we don't factor those two lines?

> -    last_used_chain = vhost_svq_last_desc_of_chain(svq, num, last_used);
> -    svq->desc_state[last_used_chain].next = svq->free_head;
> -    svq->free_head = last_used;
> -    svq->num_free += num;
> -
> -    return g_steal_pointer(&svq->desc_state[last_used].elem);
> +    return vhost_svq_detach_buf(svq, last_used);
>  }
>
>  /**
> --
> 2.53.0
>

Thanks

Re: [PATCH 6/7] vhost: add in_order feature to shadow virtqueue

[Jason Wang]

On Thu, Mar 5, 2026 at 1:36 AM Eugenio Pérez <eperezma@redhat.com> wrote:
>
> Some vdpa devices benefit from the in order feature.  Add support to SVQ
> so QEMU can migrate these.
>
> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> ---

Acked-by: Jason Wang <jasowang@redhat.com>

Thanks

Re: [PATCH 7/7] vhost: accept in order feature flag

[Jason Wang]

On Thu, Mar 5, 2026 at 1:36 AM Eugenio Pérez <eperezma@redhat.com> wrote:
>
> Let's accept the feature flag with all the infrastructure to process it
> in place.
>
> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> ---

Acked-by: Jason Wang <jasowang@redhat.com>

Thanks

Re: [PATCH 5/7] vhost: factor out the detach buf logic in SVQ

[Eugenio Perez Martin]

On Mon, Mar 9, 2026 at 9:57 AM Jason Wang <jasowang@redhat.com> wrote:
>
> On Thu, Mar 5, 2026 at 1:36 AM Eugenio Pérez <eperezma@redhat.com> wrote:
> >
> > This code path is modified to handle in order devices.  Abstract here so
> > we can generalize on the caller.
> >
> > Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> > ---
> >  hw/virtio/vhost-shadow-virtqueue.c | 24 ++++++++++++++++--------
> >  1 file changed, 16 insertions(+), 8 deletions(-)
> >
> > diff --git a/hw/virtio/vhost-shadow-virtqueue.c b/hw/virtio/vhost-shadow-virtqueue.c
> > index e7e3c9155cd0..2d8fc82cc06f 100644
> > --- a/hw/virtio/vhost-shadow-virtqueue.c
> > +++ b/hw/virtio/vhost-shadow-virtqueue.c
> > @@ -473,11 +473,24 @@ static uint16_t vhost_svq_last_desc_of_chain(const VhostShadowVirtqueue *svq,
> >      return i;
> >  }
> >
> > +G_GNUC_WARN_UNUSED_RESULT
> > +static VirtQueueElement *vhost_svq_detach_buf(VhostShadowVirtqueue *svq,
> > +                                              uint16_t id)
> > +{
> > +    uint16_t num = svq->desc_state[id].ndescs;
> > +    uint16_t last_used_chain = vhost_svq_last_desc_of_chain(svq, num, id);
> > +
> > +    svq->desc_state[last_used_chain].next = svq->free_head;
> > +    svq->free_head = id;
> > +
> > +    return g_steal_pointer(&svq->desc_state[id].elem);
> > +}
> > +
> >  G_GNUC_WARN_UNUSED_RESULT
> >  static VirtQueueElement *vhost_svq_get_buf(VhostShadowVirtqueue *svq,
> >                                             uint32_t *len)
> >  {
> > -    uint16_t last_used, last_used_chain, num;
> > +    uint16_t last_used;
> >
> >      if (!vhost_svq_more_used(svq)) {
> >          return NULL;
> > @@ -500,14 +513,9 @@ static VirtQueueElement *vhost_svq_get_buf(VhostShadowVirtqueue *svq,
> >          return NULL;
> >      }
> >
> > -    num = svq->desc_state[last_used].ndescs;
> > +    svq->num_free += svq->desc_state[last_used].ndescs;
> >      svq->desc_state[last_used].ndescs = 0;
>
> Any reason we don't factor those two lines?
>

They are common for split and split_in_order case.

> > -    last_used_chain = vhost_svq_last_desc_of_chain(svq, num, last_used);
> > -    svq->desc_state[last_used_chain].next = svq->free_head;
> > -    svq->free_head = last_used;
> > -    svq->num_free += num;
> > -
> > -    return g_steal_pointer(&svq->desc_state[last_used].elem);
> > +    return vhost_svq_detach_buf(svq, last_used);
> >  }
> >
> >  /**
> > --
> > 2.53.0
> >
>
> Thanks
>

Re: [PATCH 5/7] vhost: factor out the detach buf logic in SVQ

[Jason Wang]

On Mon, Mar 9, 2026 at 5:43 PM Eugenio Perez Martin <eperezma@redhat.com> wrote:
>
> On Mon, Mar 9, 2026 at 9:57 AM Jason Wang <jasowang@redhat.com> wrote:
> >
> > On Thu, Mar 5, 2026 at 1:36 AM Eugenio Pérez <eperezma@redhat.com> wrote:
> > >
> > > This code path is modified to handle in order devices.  Abstract here so
> > > we can generalize on the caller.
> > >
> > > Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> > > ---
> > >  hw/virtio/vhost-shadow-virtqueue.c | 24 ++++++++++++++++--------
> > >  1 file changed, 16 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/hw/virtio/vhost-shadow-virtqueue.c b/hw/virtio/vhost-shadow-virtqueue.c
> > > index e7e3c9155cd0..2d8fc82cc06f 100644
> > > --- a/hw/virtio/vhost-shadow-virtqueue.c
> > > +++ b/hw/virtio/vhost-shadow-virtqueue.c
> > > @@ -473,11 +473,24 @@ static uint16_t vhost_svq_last_desc_of_chain(const VhostShadowVirtqueue *svq,
> > >      return i;
> > >  }
> > >
> > > +G_GNUC_WARN_UNUSED_RESULT
> > > +static VirtQueueElement *vhost_svq_detach_buf(VhostShadowVirtqueue *svq,
> > > +                                              uint16_t id)
> > > +{
> > > +    uint16_t num = svq->desc_state[id].ndescs;
> > > +    uint16_t last_used_chain = vhost_svq_last_desc_of_chain(svq, num, id);
> > > +
> > > +    svq->desc_state[last_used_chain].next = svq->free_head;
> > > +    svq->free_head = id;
> > > +
> > > +    return g_steal_pointer(&svq->desc_state[id].elem);
> > > +}
> > > +
> > >  G_GNUC_WARN_UNUSED_RESULT
> > >  static VirtQueueElement *vhost_svq_get_buf(VhostShadowVirtqueue *svq,
> > >                                             uint32_t *len)
> > >  {
> > > -    uint16_t last_used, last_used_chain, num;
> > > +    uint16_t last_used;
> > >
> > >      if (!vhost_svq_more_used(svq)) {
> > >          return NULL;
> > > @@ -500,14 +513,9 @@ static VirtQueueElement *vhost_svq_get_buf(VhostShadowVirtqueue *svq,
> > >          return NULL;
> > >      }
> > >
> > > -    num = svq->desc_state[last_used].ndescs;
> > > +    svq->num_free += svq->desc_state[last_used].ndescs;
> > >      svq->desc_state[last_used].ndescs = 0;
> >
> > Any reason we don't factor those two lines?
> >
>
> They are common for split and split_in_order case.

Exactly, so why not factoring out them as well? Or you mean
vhost_svq_detach_buf is only for in-order (then we probably need to
rename).

Thanks

>
> > > -    last_used_chain = vhost_svq_last_desc_of_chain(svq, num, last_used);
> > > -    svq->desc_state[last_used_chain].next = svq->free_head;
> > > -    svq->free_head = last_used;
> > > -    svq->num_free += num;
> > > -
> > > -    return g_steal_pointer(&svq->desc_state[last_used].elem);
> > > +    return vhost_svq_detach_buf(svq, last_used);
> > >  }
> > >
> > >  /**
> > > --
> > > 2.53.0
> > >
> >
> > Thanks
> >
>

Re: [PATCH 4/7] vhost: factor out the get of last used desc in SVQ

[Jason Wang]

On Mon, Mar 9, 2026 at 5:42 PM Eugenio Perez Martin <eperezma@redhat.com> wrote:
>
> On Mon, Mar 9, 2026 at 9:57 AM Jason Wang <jasowang@redhat.com> wrote:
> >
> > On Thu, Mar 5, 2026 at 1:36 AM Eugenio Pérez <eperezma@redhat.com> wrote:
> > >
> > > This code path is modified to handle in order devices.  Abstract here so
> > > we can generalize on the caller.
> > >
> > > Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> > > ---
> > >  hw/virtio/vhost-shadow-virtqueue.c | 45 +++++++++++++++++++-----------
> > >  1 file changed, 29 insertions(+), 16 deletions(-)
> > >
> > > diff --git a/hw/virtio/vhost-shadow-virtqueue.c b/hw/virtio/vhost-shadow-virtqueue.c
> > > index 7adacd4f542a..e7e3c9155cd0 100644
> > > --- a/hw/virtio/vhost-shadow-virtqueue.c
> > > +++ b/hw/virtio/vhost-shadow-virtqueue.c
> > > @@ -444,6 +444,25 @@ static void vhost_svq_disable_notification(VhostShadowVirtqueue *svq)
> > >      }
> > >  }
> > >
> > > +/*
> > > + * Gets the next buffer id and moves forward the used idx, so the next time
> > > + * SVQ calls this function will get the next one.
> > > + *
> > > + * @svq: Shadow VirtQueue
> > > + * @len: Consumed length by the device.
> > > + *
> > > + * Return the next descriptor consumed by the device.
> > > + */
> > > +static uint16_t vhost_svq_get_last_used_split(VhostShadowVirtqueue *svq,
> > > +                                              uint32_t *len)
> > > +{
> > > +    const vring_used_t *used = svq->vring.used;
> > > +    uint16_t last_used = svq->last_used_idx++ & (svq->vring.num - 1);
> >
> > I think it's better
> >
> > either rename the function name for advanting last_used_idx or let the
> > caller to advance the last_used_idx.
> >
>
> You dropped the list, is there any reason for that?

Acciedentailly, I'm sorry for that. Adding them back.

>
> Sure it makes sense. What about virtqueue_update_last_used_idx? Or
> virtqueue_bump_last_used_idx?

I prefer to allow the caller to advance used idx, or we can listen to others.

>
> > > +
> > > +    *len = le32_to_cpu(used->ring[last_used].len);
> > > +    return le32_to_cpu(used->ring[last_used].id);
> > > +}
> > > +
> > >  static uint16_t vhost_svq_last_desc_of_chain(const VhostShadowVirtqueue *svq,
> > >                                               uint16_t num, uint16_t i)
> > >  {
> > > @@ -458,8 +477,6 @@ G_GNUC_WARN_UNUSED_RESULT
> > >  static VirtQueueElement *vhost_svq_get_buf(VhostShadowVirtqueue *svq,
> > >                                             uint32_t *len)
> > >  {
> > > -    const vring_used_t *used = svq->vring.used;
> > > -    vring_used_elem_t used_elem;
> > >      uint16_t last_used, last_used_chain, num;
> > >
> > >      if (!vhost_svq_more_used(svq)) {
> > > @@ -468,33 +485,29 @@ static VirtQueueElement *vhost_svq_get_buf(VhostShadowVirtqueue *svq,
> > >
> > >      /* Only get used array entries after they have been exposed by dev */
> > >      smp_rmb();
> > > -    last_used = svq->last_used_idx & (svq->vring.num - 1);
> > > -    used_elem.id = le32_to_cpu(used->ring[last_used].id);
> > > -    used_elem.len = le32_to_cpu(used->ring[last_used].len);
> > > +    last_used = vhost_svq_get_last_used_split(svq, len);
> > >
> > > -    svq->last_used_idx++;
> > > -    if (unlikely(used_elem.id >= svq->vring.num)) {
> > > +    if (unlikely(last_used >= svq->vring.num)) {
> > >          qemu_log_mask(LOG_GUEST_ERROR, "Device %s says index %u is used",
> > > -                      svq->vdev->name, used_elem.id);
> > > +                      svq->vdev->name, last_used);
> > >          return NULL;
> > >      }
> > >
> > > -    if (unlikely(!svq->desc_state[used_elem.id].ndescs)) {
> > > +    if (unlikely(!svq->desc_state[last_used].ndescs)) {
> > >          qemu_log_mask(LOG_GUEST_ERROR,
> > >              "Device %s says index %u is used, but it was not available",
> > > -            svq->vdev->name, used_elem.id);
> > > +            svq->vdev->name, last_used);
> > >          return NULL;
> > >      }
> > >
> > > -    num = svq->desc_state[used_elem.id].ndescs;
> > > -    svq->desc_state[used_elem.id].ndescs = 0;
> > > -    last_used_chain = vhost_svq_last_desc_of_chain(svq, num, used_elem.id);
> > > +    num = svq->desc_state[last_used].ndescs;
> > > +    svq->desc_state[last_used].ndescs = 0;
> > > +    last_used_chain = vhost_svq_last_desc_of_chain(svq, num, last_used);
> > >      svq->desc_state[last_used_chain].next = svq->free_head;
> > > -    svq->free_head = used_elem.id;
> > > +    svq->free_head = last_used;
> > >      svq->num_free += num;
> > >
> > > -    *len = used_elem.len;
> > > -    return g_steal_pointer(&svq->desc_state[used_elem.id].elem);
> > > +    return g_steal_pointer(&svq->desc_state[last_used].elem);
> > >  }
> > >
> > >  /**
> > > --
> > > 2.53.0
> > >
> >
> > Thanks
> >
>

Re: [PATCH 1/7] virtio: Allow to fill a whole virtqueue in order

[Jason Wang]

On Mon, Mar 9, 2026 at 2:19 PM Eugenio Perez Martin <eperezma@redhat.com> wrote:
>
> On Mon, Mar 9, 2026 at 4:17 AM Jason Wang <jasowang@redhat.com> wrote:
> >
> > On Fri, Mar 6, 2026 at 2:23 PM Eugenio Perez Martin <eperezma@redhat.com> wrote:
> > >
> > > On Fri, Mar 6, 2026 at 4:26 AM Jason Wang <jasowang@redhat.com> wrote:
> > > >
> > > > On Thu, Mar 5, 2026 at 1:35 AM Eugenio Pérez <eperezma@redhat.com> wrote:
> > > > >
> > > > > As the while steps < max_steps is already one less than the vq size, the
> > > > > right maximum max_steps variable is queue length, not the maximum
> > > > > possible remainder of % vq->vring.num.
> > > > >
> > > > > Fixes: b44135daa37 ("virtio: virtqueue_ordered_fill - VIRTIO_F_IN_ORDER support")
> > > > > Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> > > > > ---
> > > > > Personally I'd just remove max_steps and let it be vq->vring.num, but
> > > > > let's make the minimal changes for now.
> > > > > ---
> > > > >  hw/virtio/virtio.c | 2 +-
> > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> > > > > index e9d553295257..17f171551892 100644
> > > > > --- a/hw/virtio/virtio.c
> > > > > +++ b/hw/virtio/virtio.c
> > > > > @@ -980,7 +980,7 @@ static void virtqueue_ordered_fill(VirtQueue *vq, const VirtQueueElement *elem,
> > > > >       * We shouldn't need to increase 'i' by more than or equal to
> > > > >       * the distance between used_idx and last_avail_idx (max_steps).
> > > > >       */
> > > > > -    max_steps = (vq->last_avail_idx - vq->used_idx) % vq->vring.num;
> > > > > +    max_steps = MIN(vq->last_avail_idx - vq->used_idx, vq->vring.num);
> > > > >
> > > > >      /* Search for element in vq->used_elems */
> > > > >      while (steps < max_steps) {
> > > >
> > > > Not directly related but I found that the virtqueue_ordered_fill() and
> > > > virtqueue_ordered_flush() can "emulate" an in-order device even if it
> > > > isn't one. I'm not sure this is good for the device that can benefit
> > > > from out of order completion like block devices.
> > > >
> > >
> > > I guess the benefit is significantly reduced if the device cannot use
> > > more than one descriptor in the same used entry.
> > >
> > > > It seems the correct approach is to let the device decide whether it
> > > > can operate in order, not the virtio core (as vhost-kernel did).
> > > >
> > >
> > > I agree with this.
> > >
> > > Currently, `in_order` is off by default but we could enable it by
> > > default per-device.
> >
> > After further thought, there's another reason. For example, for a
> > networking device, virtio-net should know nothing about the networking
> > backend (though some tricks exist in the code). So it can't know if
> > the backend can handle OOO or not. Starting with this seems to be
> > fine.
> >
>
> Well, if the backend exposes in_order, I'd assume it works better if
> the feature is acknowledged. I was talking just about the emulated
> device.
>
> I kind of agree with what you say for virtio-net+kernel tap or
> vhost-kernel case, but we're already assuming that the virtio-net
> backend works in order in Live Migration anyway.

Could you point the code that has this asumption?

> With worse
> consecuences that less performance. To be coherent with your mention
> here, we should support out of order descriptors in live migration.
> Not saying that it is urgent, just pointing it out.
>

Thanks

Re: [PATCH 1/7] virtio: Allow to fill a whole virtqueue in order

[Eugenio Perez Martin]

On Tue, Mar 10, 2026 at 4:09 AM Jason Wang <jasowang@redhat.com> wrote:
>
> On Mon, Mar 9, 2026 at 2:19 PM Eugenio Perez Martin <eperezma@redhat.com> wrote:
> >
> > On Mon, Mar 9, 2026 at 4:17 AM Jason Wang <jasowang@redhat.com> wrote:
> > >
> > > On Fri, Mar 6, 2026 at 2:23 PM Eugenio Perez Martin <eperezma@redhat.com> wrote:
> > > >
> > > > On Fri, Mar 6, 2026 at 4:26 AM Jason Wang <jasowang@redhat.com> wrote:
> > > > >
> > > > > On Thu, Mar 5, 2026 at 1:35 AM Eugenio Pérez <eperezma@redhat.com> wrote:
> > > > > >
> > > > > > As the while steps < max_steps is already one less than the vq size, the
> > > > > > right maximum max_steps variable is queue length, not the maximum
> > > > > > possible remainder of % vq->vring.num.
> > > > > >
> > > > > > Fixes: b44135daa37 ("virtio: virtqueue_ordered_fill - VIRTIO_F_IN_ORDER support")
> > > > > > Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> > > > > > ---
> > > > > > Personally I'd just remove max_steps and let it be vq->vring.num, but
> > > > > > let's make the minimal changes for now.
> > > > > > ---
> > > > > >  hw/virtio/virtio.c | 2 +-
> > > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> > > > > > index e9d553295257..17f171551892 100644
> > > > > > --- a/hw/virtio/virtio.c
> > > > > > +++ b/hw/virtio/virtio.c
> > > > > > @@ -980,7 +980,7 @@ static void virtqueue_ordered_fill(VirtQueue *vq, const VirtQueueElement *elem,
> > > > > >       * We shouldn't need to increase 'i' by more than or equal to
> > > > > >       * the distance between used_idx and last_avail_idx (max_steps).
> > > > > >       */
> > > > > > -    max_steps = (vq->last_avail_idx - vq->used_idx) % vq->vring.num;
> > > > > > +    max_steps = MIN(vq->last_avail_idx - vq->used_idx, vq->vring.num);
> > > > > >
> > > > > >      /* Search for element in vq->used_elems */
> > > > > >      while (steps < max_steps) {
> > > > >
> > > > > Not directly related but I found that the virtqueue_ordered_fill() and
> > > > > virtqueue_ordered_flush() can "emulate" an in-order device even if it
> > > > > isn't one. I'm not sure this is good for the device that can benefit
> > > > > from out of order completion like block devices.
> > > > >
> > > >
> > > > I guess the benefit is significantly reduced if the device cannot use
> > > > more than one descriptor in the same used entry.
> > > >
> > > > > It seems the correct approach is to let the device decide whether it
> > > > > can operate in order, not the virtio core (as vhost-kernel did).
> > > > >
> > > >
> > > > I agree with this.
> > > >
> > > > Currently, `in_order` is off by default but we could enable it by
> > > > default per-device.
> > >
> > > After further thought, there's another reason. For example, for a
> > > networking device, virtio-net should know nothing about the networking
> > > backend (though some tricks exist in the code). So it can't know if
> > > the backend can handle OOO or not. Starting with this seems to be
> > > fine.
> > >
> >
> > Well, if the backend exposes in_order, I'd assume it works better if
> > the feature is acknowledged. I was talking just about the emulated
> > device.
> >
> > I kind of agree with what you say for virtio-net+kernel tap or
> > vhost-kernel case, but we're already assuming that the virtio-net
> > backend works in order in Live Migration anyway.
>
> Could you point the code that has this asumption?
>

Actually, the lack of code to migrate the inflight descriptors is the
fact that the migration code of virtio-net assumes the buffers arrive
in order :).

It is easier to reproduce in the packed vq: When used entries override
the avail descriptors, all the overridden entries for available
buffers are lost. But it's easy to reproduce in the split vq too, as
we only migrate indexes.

It does not happen in virtio-blk, where in-flight virtqueue elements
are migrated.

> > With worse
> > consecuences that less performance. To be coherent with your mention
> > here, we should support out of order descriptors in live migration.
> > Not saying that it is urgent, just pointing it out.
> >
>
> Thanks
>

Re: [PATCH 5/7] vhost: factor out the detach buf logic in SVQ

[Eugenio Perez Martin]

On Tue, Mar 10, 2026 at 4:04 AM Jason Wang <jasowang@redhat.com> wrote:
>
> On Mon, Mar 9, 2026 at 5:43 PM Eugenio Perez Martin <eperezma@redhat.com> wrote:
> >
> > On Mon, Mar 9, 2026 at 9:57 AM Jason Wang <jasowang@redhat.com> wrote:
> > >
> > > On Thu, Mar 5, 2026 at 1:36 AM Eugenio Pérez <eperezma@redhat.com> wrote:
> > > >
> > > > This code path is modified to handle in order devices.  Abstract here so
> > > > we can generalize on the caller.
> > > >
> > > > Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> > > > ---
> > > >  hw/virtio/vhost-shadow-virtqueue.c | 24 ++++++++++++++++--------
> > > >  1 file changed, 16 insertions(+), 8 deletions(-)
> > > >
> > > > diff --git a/hw/virtio/vhost-shadow-virtqueue.c b/hw/virtio/vhost-shadow-virtqueue.c
> > > > index e7e3c9155cd0..2d8fc82cc06f 100644
> > > > --- a/hw/virtio/vhost-shadow-virtqueue.c
> > > > +++ b/hw/virtio/vhost-shadow-virtqueue.c
> > > > @@ -473,11 +473,24 @@ static uint16_t vhost_svq_last_desc_of_chain(const VhostShadowVirtqueue *svq,
> > > >      return i;
> > > >  }
> > > >
> > > > +G_GNUC_WARN_UNUSED_RESULT
> > > > +static VirtQueueElement *vhost_svq_detach_buf(VhostShadowVirtqueue *svq,
> > > > +                                              uint16_t id)
> > > > +{
> > > > +    uint16_t num = svq->desc_state[id].ndescs;
> > > > +    uint16_t last_used_chain = vhost_svq_last_desc_of_chain(svq, num, id);
> > > > +
> > > > +    svq->desc_state[last_used_chain].next = svq->free_head;
> > > > +    svq->free_head = id;
> > > > +
> > > > +    return g_steal_pointer(&svq->desc_state[id].elem);
> > > > +}
> > > > +
> > > >  G_GNUC_WARN_UNUSED_RESULT
> > > >  static VirtQueueElement *vhost_svq_get_buf(VhostShadowVirtqueue *svq,
> > > >                                             uint32_t *len)
> > > >  {
> > > > -    uint16_t last_used, last_used_chain, num;
> > > > +    uint16_t last_used;
> > > >
> > > >      if (!vhost_svq_more_used(svq)) {
> > > >          return NULL;
> > > > @@ -500,14 +513,9 @@ static VirtQueueElement *vhost_svq_get_buf(VhostShadowVirtqueue *svq,
> > > >          return NULL;
> > > >      }
> > > >
> > > > -    num = svq->desc_state[last_used].ndescs;
> > > > +    svq->num_free += svq->desc_state[last_used].ndescs;
> > > >      svq->desc_state[last_used].ndescs = 0;
> > >
> > > Any reason we don't factor those two lines?
> > >
> >
> > They are common for split and split_in_order case.
>
> Exactly, so why not factoring out them as well? Or you mean
> vhost_svq_detach_buf is only for in-order (then we probably need to
> rename).
>

This is the relevant code after applying the series:
static VirtQueueElement *vhost_svq_detach_buf(VhostShadowVirtqueue *svq,
                                              uint16_t id)
{
    if (virtio_vdev_has_feature(svq->vdev, VIRTIO_F_IN_ORDER)) {
        return vhost_svq_detach_buf_split_in_order(svq, id);
    } else {
        return vhost_svq_detach_buf_split(svq, id);
    }
}

G_GNUC_WARN_UNUSED_RESULT
static VirtQueueElement *vhost_svq_get_buf(VhostShadowVirtqueue *svq,
                                           uint32_t *len)
{
    uint16_t last_used;

    if (!vhost_svq_more_used(svq)) {
        return NULL;
    }

    /* Only get used array entries after they have been exposed by dev */
    smp_rmb();

    if (virtio_vdev_has_feature(svq->vdev, VIRTIO_F_IN_ORDER)) {
        int32_t r;
        r = vhost_svq_get_last_used_split_in_order(svq, len);
        if (r < 0) {
            return NULL;
        }

        last_used = r;
    } else {
        last_used = vhost_svq_get_last_used_split(svq, len);
    }

    if (unlikely(last_used >= svq->vring.num)) {
        qemu_log_mask(LOG_GUEST_ERROR, "Device %s says index %u is used",
                      svq->vdev->name, last_used);
        return NULL;
    }

    if (unlikely(!svq->desc_state[last_used].ndescs)) {
        qemu_log_mask(LOG_GUEST_ERROR,
            "Device %s says index %u is used, but it was not available",
            svq->vdev->name, last_used);
        return NULL;
    }

    svq->num_free += svq->desc_state[last_used].ndescs;
    svq->desc_state[last_used].ndescs = 0;
    return vhost_svq_detach_buf(svq, last_used);
}
---

Whether the code is in vhost_svq_detach_buf or vhost_svq_get_buf it's
abstracted from the in order or the out of order version.

The code is in vhost_svq_get_buf because the long term plan is to make
vhost_svq_detach_buf similar to your VIRTQUEUE_CALL macro in the Linux
kernel—a dispatcher based on the acked features. But I'm willing to
move it to vhost_svq_detach_buf in this series if you prefer.

Do you want a V2 with the two lines of code moved to vhost_svq_detach_buf?

Thanks!

Re: [PATCH 0/7] Add VIRTIO_F_IN_ORDER support to vhost shadow virtqueue

[Michael Tokarev]

On 04.03.2026 20:35, Eugenio Pérez wrote:
> This series adds support for the VIRTIO_F_IN_ORDER feature flag to vhost shadow
> virtqueue (SVQ), enabling migration of vDPA devices that offer this feature.
> As SVQ acts as a virtio driver, the series follows the Linux kernel
> implementation for the feature by Jason Wang.  This enables the live migration
> of vhost-vdpa devices that supports IN_ORDER.
> 
> The IN_ORDER feature allows virtio devices to use many descriptors in batch,
> just by marking the last one of the set as used.
> 
> The series is structured in three parts.  First, max_steps calculation in
> virtqueue_ordered_fill is fixed to allow filling the entire virtqueue at once.
> Otherwise, a full queue cannot be used with just one used entry.
> 
> Afterwards, the series extracts helper functions for descriptor processing to
> prepare for in order changes:
> 
> Finally, the series adds IN_ORDER support, first adding conditional logic for
> in-order vs regular processing and whitelisting the feature flag in SVQ.
> 
> Eugenio Pérez (7):
>    virtio: Allow to fill a whole virtqueue in order
>    vhost: move svq next desc array to descs state struct
>    vhost: factor out the descriptor next fetching
>    vhost: factor out the get of last used desc in SVQ
>    vhost: factor out the detach buf logic in SVQ
>    vhost: add in_order feature to shadow virtqueue
>    vhost: accept in order feature flag
> 
>   hw/virtio/vhost-shadow-virtqueue.c | 215 +++++++++++++++++++++++++----
>   hw/virtio/vhost-shadow-virtqueue.h |  38 ++++-
>   hw/virtio/virtio.c                 |   2 +-

Hi!

This whole patchset is being Cc'ed to qemu-stable@.  Does this mean
that whole patch set should be picked up for the qemu stable series?
It's a rather unusual thing to do - to pick up new feature into a
stable series.  Is it what you have in mind?

Thanks,

/mjt

Re: [PATCH 0/7] Add VIRTIO_F_IN_ORDER support to vhost shadow virtqueue

[Eugenio Perez Martin]

On Wed, Mar 11, 2026 at 12:50 PM Michael Tokarev <mjt@tls.msk.ru> wrote:
>
> On 04.03.2026 20:35, Eugenio Pérez wrote:
> > This series adds support for the VIRTIO_F_IN_ORDER feature flag to vhost shadow
> > virtqueue (SVQ), enabling migration of vDPA devices that offer this feature.
> > As SVQ acts as a virtio driver, the series follows the Linux kernel
> > implementation for the feature by Jason Wang.  This enables the live migration
> > of vhost-vdpa devices that supports IN_ORDER.
> >
> > The IN_ORDER feature allows virtio devices to use many descriptors in batch,
> > just by marking the last one of the set as used.
> >
> > The series is structured in three parts.  First, max_steps calculation in
> > virtqueue_ordered_fill is fixed to allow filling the entire virtqueue at once.
> > Otherwise, a full queue cannot be used with just one used entry.
> >
> > Afterwards, the series extracts helper functions for descriptor processing to
> > prepare for in order changes:
> >
> > Finally, the series adds IN_ORDER support, first adding conditional logic for
> > in-order vs regular processing and whitelisting the feature flag in SVQ.
> >
> > Eugenio Pérez (7):
> >    virtio: Allow to fill a whole virtqueue in order
> >    vhost: move svq next desc array to descs state struct
> >    vhost: factor out the descriptor next fetching
> >    vhost: factor out the get of last used desc in SVQ
> >    vhost: factor out the detach buf logic in SVQ
> >    vhost: add in_order feature to shadow virtqueue
> >    vhost: accept in order feature flag
> >
> >   hw/virtio/vhost-shadow-virtqueue.c | 215 +++++++++++++++++++++++++----
> >   hw/virtio/vhost-shadow-virtqueue.h |  38 ++++-
> >   hw/virtio/virtio.c                 |   2 +-
>
> Hi!
>
> This whole patchset is being Cc'ed to qemu-stable@.  Does this mean
> that whole patch set should be picked up for the qemu stable series?
> It's a rather unusual thing to do - to pick up new feature into a
> stable series.  Is it what you have in mind?
>

Hi,

No, only the first patch. Sorry for not being clear, I thought it was
enough with the Fixes: tag, but thinking it twice I see how it might
not be clear enough. I'll make it clearer in the cover letter next
time, or is there a better way to handle this?

Thanks!

Re: [PATCH 0/7] Add VIRTIO_F_IN_ORDER support to vhost shadow virtqueue

[Michael Tokarev]

On 11.03.2026 15:24, Eugenio Perez Martin wrote:
> On Wed, Mar 11, 2026 at 12:50 PM Michael Tokarev <mjt@tls.msk.ru> wrote:

>> This whole patchset is being Cc'ed to qemu-stable@.  Does this mean
>> that whole patch set should be picked up for the qemu stable series?
>> It's a rather unusual thing to do - to pick up new feature into a
>> stable series.  Is it what you have in mind?

> No, only the first patch. Sorry for not being clear, I thought it was
> enough with the Fixes: tag, but thinking it twice I see how it might
> not be clear enough. I'll make it clearer in the cover letter next
> time, or is there a better way to handle this?

If not whole series should be picked up for stable, you can add
Cc: qemu-stable@.. to individual patches which should be.

The presence of Fixes: doesn't make it clear :)

But it's very good that you do care about stable series!

Thank you.  I've noted for this particular series, there's nothing
extra to do here.

Thanks,

/mjt

Re: [PATCH 1/7] virtio: Allow to fill a whole virtqueue in order

[Jonah Palmer]

On 3/10/26 2:21 AM, Eugenio Perez Martin wrote:
> On Tue, Mar 10, 2026 at 4:09 AM Jason Wang <jasowang@redhat.com> wrote:
>>
>> On Mon, Mar 9, 2026 at 2:19 PM Eugenio Perez Martin <eperezma@redhat.com> wrote:
>>>
>>> On Mon, Mar 9, 2026 at 4:17 AM Jason Wang <jasowang@redhat.com> wrote:
>>>>
>>>> On Fri, Mar 6, 2026 at 2:23 PM Eugenio Perez Martin <eperezma@redhat.com> wrote:
>>>>>
>>>>> On Fri, Mar 6, 2026 at 4:26 AM Jason Wang <jasowang@redhat.com> wrote:
>>>>>>
>>>>>> On Thu, Mar 5, 2026 at 1:35 AM Eugenio Pérez <eperezma@redhat.com> wrote:
>>>>>>>
>>>>>>> As the while steps < max_steps is already one less than the vq size, the
>>>>>>> right maximum max_steps variable is queue length, not the maximum
>>>>>>> possible remainder of % vq->vring.num.
>>>>>>>
>>>>>>> Fixes: b44135daa37 ("virtio: virtqueue_ordered_fill - VIRTIO_F_IN_ORDER support")
>>>>>>> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
>>>>>>> ---
>>>>>>> Personally I'd just remove max_steps and let it be vq->vring.num, but
>>>>>>> let's make the minimal changes for now.
>>>>>>> ---
>>>>>>>   hw/virtio/virtio.c | 2 +-
>>>>>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
>>>>>>> index e9d553295257..17f171551892 100644
>>>>>>> --- a/hw/virtio/virtio.c
>>>>>>> +++ b/hw/virtio/virtio.c
>>>>>>> @@ -980,7 +980,7 @@ static void virtqueue_ordered_fill(VirtQueue *vq, const VirtQueueElement *elem,
>>>>>>>        * We shouldn't need to increase 'i' by more than or equal to
>>>>>>>        * the distance between used_idx and last_avail_idx (max_steps).
>>>>>>>        */
>>>>>>> -    max_steps = (vq->last_avail_idx - vq->used_idx) % vq->vring.num;
>>>>>>> +    max_steps = MIN(vq->last_avail_idx - vq->used_idx, vq->vring.num);
>>>>>>>
>>>>>>>       /* Search for element in vq->used_elems */
>>>>>>>       while (steps < max_steps) {

After looking at this again, I realize my original max_steps calculation 
is wrong when the ring is full (outstanding descriptors == vring.num).

virtqueue_ordered_fill should only walk the current in-flight window 
from used_idx, not the entire ring vring.num (of course unless the ring 
is full).

Speaking of which, IIUC, last_avail_idx == used_idx if the ring is full 
and would cause max_steps = 0 when descriptors are in flight. What if we 
did this instead?

	max_steps = MIN(vq->inuse, vq->vring.num);

>>>>>>
>>>>>> Not directly related but I found that the virtqueue_ordered_fill() and
>>>>>> virtqueue_ordered_flush() can "emulate" an in-order device even if it
>>>>>> isn't one. I'm not sure this is good for the device that can benefit
>>>>>> from out of order completion like block devices.
>>>>>>
>>>>>
>>>>> I guess the benefit is significantly reduced if the device cannot use
>>>>> more than one descriptor in the same used entry.
>>>>>
>>>>>> It seems the correct approach is to let the device decide whether it
>>>>>> can operate in order, not the virtio core (as vhost-kernel did).
>>>>>>
>>>>>
>>>>> I agree with this.
>>>>>
>>>>> Currently, `in_order` is off by default but we could enable it by
>>>>> default per-device.
>>>>
>>>> After further thought, there's another reason. For example, for a
>>>> networking device, virtio-net should know nothing about the networking
>>>> backend (though some tricks exist in the code). So it can't know if
>>>> the backend can handle OOO or not. Starting with this seems to be
>>>> fine.
>>>>
>>>
>>> Well, if the backend exposes in_order, I'd assume it works better if
>>> the feature is acknowledged. I was talking just about the emulated
>>> device.
>>>
>>> I kind of agree with what you say for virtio-net+kernel tap or
>>> vhost-kernel case, but we're already assuming that the virtio-net
>>> backend works in order in Live Migration anyway.
>>
>> Could you point the code that has this asumption?
>>
> 
> Actually, the lack of code to migrate the inflight descriptors is the
> fact that the migration code of virtio-net assumes the buffers arrive
> in order :).
> 
> It is easier to reproduce in the packed vq: When used entries override
> the avail descriptors, all the overridden entries for available
> buffers are lost. But it's easy to reproduce in the split vq too, as
> we only migrate indexes.
> 
> It does not happen in virtio-blk, where in-flight virtqueue elements
> are migrated.
> 
>>> With worse
>>> consecuences that less performance. To be coherent with your mention
>>> here, we should support out of order descriptors in live migration.
>>> Not saying that it is urgent, just pointing it out.
>>>
>>
>> Thanks
>>
>