[PATCH 0/6] vhost: Reset batched descriptors on SET_VRING_BASE call

[PATCH 0/6] vhost: Reset batched descriptors on SET_VRING_BASE call

[Eugenio Pérez]

Vhost did not reset properly the batched descriptors on SET_VRING_BASE event. Because of that, is possible to return an invalid descriptor to the guest.

This series ammend this, and creates a test to assert correct behavior. To do that, they need to expose a new function in virtio_ring, virtqueue_reset_free_head. Not sure if this can be avoided.

Also, change from https://lkml.org/lkml/2020/3/27/108 is not included, that avoids to update a variable in a loop where it can be updated once.

This is meant to be applied on top of eccb852f1fe6bede630e2e4f1a121a81e34354ab in git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git, and some commits should be squashed with that series.

Eugenio Pérez (6):
  tools/virtio: Add --batch option
  tools/virtio: Add --batch=random option
  tools/virtio: Add --reset=random
  tools/virtio: Make --reset reset ring idx
  vhost: Delete virtqueue batch_descs member
  fixup! vhost: batching fetches

 drivers/vhost/test.c         |  57 ++++++++++++++++
 drivers/vhost/test.h         |   1 +
 drivers/vhost/vhost.c        |  12 +++-
 drivers/vhost/vhost.h        |   1 -
 drivers/virtio/virtio_ring.c |  18 +++++
 include/linux/virtio.h       |   2 +
 tools/virtio/linux/virtio.h  |   2 +
 tools/virtio/virtio_test.c   | 123 +++++++++++++++++++++++++++++++----
 8 files changed, 201 insertions(+), 15 deletions(-)

-- 
2.18.1

[PATCH 1/6] tools/virtio: Add --batch option

[Eugenio Pérez]

This allow to test vhost having >1 buffers in flight

Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
---
 tools/virtio/virtio_test.c | 47 ++++++++++++++++++++++++++++++--------
 1 file changed, 37 insertions(+), 10 deletions(-)

diff --git a/tools/virtio/virtio_test.c b/tools/virtio/virtio_test.c
index b427def67e7e..c30de9088f3c 100644
--- a/tools/virtio/virtio_test.c
+++ b/tools/virtio/virtio_test.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
 #define _GNU_SOURCE
 #include <getopt.h>
+#include <limits.h>
 #include <string.h>
 #include <poll.h>
 #include <sys/eventfd.h>
@@ -152,11 +153,11 @@ static void wait_for_interrupt(struct vdev_info *dev)
 }
 
 static void run_test(struct vdev_info *dev, struct vq_info *vq,
-		     bool delayed, int bufs)
+		     bool delayed, int batch, int bufs)
 {
 	struct scatterlist sl;
 	long started = 0, completed = 0;
-	long completed_before;
+	long completed_before, started_before;
 	int r, test = 1;
 	unsigned len;
 	long long spurious = 0;
@@ -165,28 +166,42 @@ static void run_test(struct vdev_info *dev, struct vq_info *vq,
 	for (;;) {
 		virtqueue_disable_cb(vq->vq);
 		completed_before = completed;
+		started_before = started;
 		do {
-			if (started < bufs) {
+			while (started < bufs &&
+			       (started - completed) < batch) {
 				sg_init_one(&sl, dev->buf, dev->buf_size);
 				r = virtqueue_add_outbuf(vq->vq, &sl, 1,
 							 dev->buf + started,
 							 GFP_ATOMIC);
-				if (likely(r == 0)) {
-					++started;
-					if (unlikely(!virtqueue_kick(vq->vq)))
+				if (unlikely(r != 0)) {
+					if (r == -ENOSPC &&
+					    started > started_before)
+						r = 0;
+					else
 						r = -1;
+					break;
 				}
-			} else
+
+				++started;
+
+				if (unlikely(!virtqueue_kick(vq->vq))) {
+					r = -1;
+					break;
+				}
+			}
+
+			if (started >= bufs)
 				r = -1;
 
 			/* Flush out completed bufs if any */
-			if (virtqueue_get_buf(vq->vq, &len)) {
+			while (virtqueue_get_buf(vq->vq, &len)) {
 				++completed;
 				r = 0;
 			}
 
 		} while (r == 0);
-		if (completed == completed_before)
+		if (completed == completed_before && started == started_before)
 			++spurious;
 		assert(completed <= bufs);
 		assert(started <= bufs);
@@ -244,6 +259,11 @@ const struct option longopts[] = {
 		.name = "no-delayed-interrupt",
 		.val = 'd',
 	},
+	{
+		.name = "batch",
+		.val = 'b',
+		.has_arg = required_argument,
+	},
 	{
 	}
 };
@@ -255,6 +275,7 @@ static void help(void)
 		" [--no-event-idx]"
 		" [--no-virtio-1]"
 		" [--delayed-interrupt]"
+		" [--batch=N]"
 		"\n");
 }
 
@@ -263,6 +284,7 @@ int main(int argc, char **argv)
 	struct vdev_info dev;
 	unsigned long long features = (1ULL << VIRTIO_RING_F_INDIRECT_DESC) |
 		(1ULL << VIRTIO_RING_F_EVENT_IDX) | (1ULL << VIRTIO_F_VERSION_1);
+	long batch = 1;
 	int o;
 	bool delayed = false;
 
@@ -289,6 +311,11 @@ int main(int argc, char **argv)
 		case 'D':
 			delayed = true;
 			break;
+		case 'b':
+			batch = strtol(optarg, NULL, 10);
+			assert(batch > 0);
+			assert(batch < (long)INT_MAX + 1);
+			break;
 		default:
 			assert(0);
 			break;
@@ -298,6 +325,6 @@ int main(int argc, char **argv)
 done:
 	vdev_info_init(&dev, features);
 	vq_info_add(&dev, 256);
-	run_test(&dev, &dev.vqs[0], delayed, 0x100000);
+	run_test(&dev, &dev.vqs[0], delayed, batch, 0x100000);
 	return 0;
 }
-- 
2.18.1

[PATCH 2/6] tools/virtio: Add --batch=random option

[Eugenio Pérez]

So we can test with non-deterministic batches in flight.

Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
---
 tools/virtio/virtio_test.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/tools/virtio/virtio_test.c b/tools/virtio/virtio_test.c
index c30de9088f3c..b0dd73db5cbf 100644
--- a/tools/virtio/virtio_test.c
+++ b/tools/virtio/virtio_test.c
@@ -19,6 +19,8 @@
 #include <linux/virtio_ring.h>
 #include "../../drivers/vhost/test.h"
 
+#define RANDOM_BATCH -1
+
 /* Unused */
 void *__kmalloc_fake, *__kfree_ignore_start, *__kfree_ignore_end;
 
@@ -161,6 +163,7 @@ static void run_test(struct vdev_info *dev, struct vq_info *vq,
 	int r, test = 1;
 	unsigned len;
 	long long spurious = 0;
+	const bool random_batch = batch == RANDOM_BATCH;
 	r = ioctl(dev->control, VHOST_TEST_RUN, &test);
 	assert(r >= 0);
 	for (;;) {
@@ -168,6 +171,10 @@ static void run_test(struct vdev_info *dev, struct vq_info *vq,
 		completed_before = completed;
 		started_before = started;
 		do {
+			const bool reset = reset_n && completed > next_reset;
+			if (random_batch)
+				batch = (random() % vq->vring.num) + 1;
+
 			while (started < bufs &&
 			       (started - completed) < batch) {
 				sg_init_one(&sl, dev->buf, dev->buf_size);
@@ -275,7 +282,7 @@ static void help(void)
 		" [--no-event-idx]"
 		" [--no-virtio-1]"
 		" [--delayed-interrupt]"
-		" [--batch=N]"
+		" [--batch=random/N]"
 		"\n");
 }
 
@@ -312,9 +319,13 @@ int main(int argc, char **argv)
 			delayed = true;
 			break;
 		case 'b':
-			batch = strtol(optarg, NULL, 10);
-			assert(batch > 0);
-			assert(batch < (long)INT_MAX + 1);
+			if (0 == strcmp(optarg, "random")) {
+				batch = RANDOM_BATCH;
+			} else {
+				batch = strtol(optarg, NULL, 10);
+				assert(batch > 0);
+				assert(batch < (long)INT_MAX + 1);
+			}
 			break;
 		default:
 			assert(0);
-- 
2.18.1

[PATCH 3/6] tools/virtio: Add --reset=random

[Eugenio Pérez]

Currently, it only removes and add backend, but it will reset vq
position in future commits.

Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
---
 drivers/vhost/test.c       | 57 ++++++++++++++++++++++++++++++++++++++
 drivers/vhost/test.h       |  1 +
 tools/virtio/virtio_test.c | 43 +++++++++++++++++++++++++---
 3 files changed, 97 insertions(+), 4 deletions(-)

diff --git a/drivers/vhost/test.c b/drivers/vhost/test.c
index 58da17af93e9..329a865227ff 100644
--- a/drivers/vhost/test.c
+++ b/drivers/vhost/test.c
@@ -263,9 +263,62 @@ static int vhost_test_set_features(struct vhost_test *n, u64 features)
 	return 0;
 }
 
+static long vhost_test_set_backend(struct vhost_test *n, unsigned index, int fd)
+{
+	static void *private_data;
+
+	const bool enable = fd != -1;
+	struct vhost_virtqueue *vq;
+	int r;
+
+	mutex_lock(&n->dev.mutex);
+	r = vhost_dev_check_owner(&n->dev);
+	if (r)
+		goto err;
+
+	if (index >= VHOST_TEST_VQ_MAX) {
+		r = -ENOBUFS;
+		goto err;
+	}
+	vq = &n->vqs[index];
+	mutex_lock(&vq->mutex);
+
+	/* Verify that ring has been setup correctly. */
+	if (!vhost_vq_access_ok(vq)) {
+		r = -EFAULT;
+		goto err_vq;
+	}
+	if (!enable) {
+		vhost_poll_stop(&vq->poll);
+		private_data = vq->private_data;
+		vq->private_data = NULL;
+	} else {
+		r = vhost_vq_init_access(vq);
+		vq->private_data = private_data;
+		if (r == 0)
+			r = vhost_poll_start(&vq->poll, vq->kick);
+	}
+
+	mutex_unlock(&vq->mutex);
+
+	if (enable) {
+		vhost_test_flush_vq(n, index);
+	}
+
+	mutex_unlock(&n->dev.mutex);
+	return 0;
+
+err_vq:
+	mutex_unlock(&vq->mutex);
+err:
+	mutex_unlock(&n->dev.mutex);
+	return r;
+}
+
 static long vhost_test_ioctl(struct file *f, unsigned int ioctl,
 			     unsigned long arg)
 {
+	struct vhost_vring_file backend;
 	struct vhost_test *n = f->private_data;
 	void __user *argp = (void __user *)arg;
 	u64 __user *featurep = argp;
@@ -277,6 +330,10 @@ static long vhost_test_ioctl(struct file *f, unsigned int ioctl,
 		if (copy_from_user(&test, argp, sizeof test))
 			return -EFAULT;
 		return vhost_test_run(n, test);
+	case VHOST_TEST_SET_BACKEND:
+		if (copy_from_user(&backend, argp, sizeof backend))
+			return -EFAULT;
+		return vhost_test_set_backend(n, backend.index, backend.fd);
 	case VHOST_GET_FEATURES:
 		features = VHOST_FEATURES;
 		if (copy_to_user(featurep, &features, sizeof features))
diff --git a/drivers/vhost/test.h b/drivers/vhost/test.h
index 7dd265bfdf81..822bc4bee03a 100644
--- a/drivers/vhost/test.h
+++ b/drivers/vhost/test.h
@@ -4,5 +4,6 @@
 
 /* Start a given test on the virtio null device. 0 stops all tests. */
 #define VHOST_TEST_RUN _IOW(VHOST_VIRTIO, 0x31, int)
+#define VHOST_TEST_SET_BACKEND _IOW(VHOST_VIRTIO, 0x32, int)
 
 #endif
diff --git a/tools/virtio/virtio_test.c b/tools/virtio/virtio_test.c
index b0dd73db5cbf..93d81cd64ba0 100644
--- a/tools/virtio/virtio_test.c
+++ b/tools/virtio/virtio_test.c
@@ -20,6 +20,7 @@
 #include "../../drivers/vhost/test.h"
 
 #define RANDOM_BATCH -1
+#define RANDOM_RESET -1
 
 /* Unused */
 void *__kmalloc_fake, *__kfree_ignore_start, *__kfree_ignore_end;
@@ -46,6 +47,9 @@ struct vdev_info {
 	struct vhost_memory *mem;
 };
 
+static const struct vhost_vring_file no_backend = { .fd = -1 },
+				     backend = { .fd = 1 };
+
 bool vq_notify(struct virtqueue *vq)
 {
 	struct vq_info *info = vq->priv;
@@ -155,10 +159,10 @@ static void wait_for_interrupt(struct vdev_info *dev)
 }
 
 static void run_test(struct vdev_info *dev, struct vq_info *vq,
-		     bool delayed, int batch, int bufs)
+		     bool delayed, int batch, int reset_n, int bufs)
 {
 	struct scatterlist sl;
-	long started = 0, completed = 0;
+	long started = 0, completed = 0, next_reset = reset_n;
 	long completed_before, started_before;
 	int r, test = 1;
 	unsigned len;
@@ -201,12 +205,26 @@ static void run_test(struct vdev_info *dev, struct vq_info *vq,
 			if (started >= bufs)
 				r = -1;
 
+			if (reset) {
+				r = ioctl(dev->control, VHOST_TEST_SET_BACKEND,
+					  &no_backend);
+				assert(!r);
+			}
+
 			/* Flush out completed bufs if any */
 			while (virtqueue_get_buf(vq->vq, &len)) {
 				++completed;
 				r = 0;
 			}
 
+			if (reset) {
+				r = ioctl(dev->control, VHOST_TEST_SET_BACKEND,
+					  &backend);
+				assert(!r);
+
+                                while (completed > next_reset)
+					next_reset += completed;
+			}
 		} while (r == 0);
 		if (completed == completed_before && started == started_before)
 			++spurious;
@@ -271,6 +289,11 @@ const struct option longopts[] = {
 		.val = 'b',
 		.has_arg = required_argument,
 	},
+	{
+		.name = "reset",
+		.val = 'r',
+		.has_arg = optional_argument,
+	},
 	{
 	}
 };
@@ -283,6 +306,7 @@ static void help(void)
 		" [--no-virtio-1]"
 		" [--delayed-interrupt]"
 		" [--batch=random/N]"
+		" [--reset=random/N]"
 		"\n");
 }
 
@@ -291,7 +315,7 @@ int main(int argc, char **argv)
 	struct vdev_info dev;
 	unsigned long long features = (1ULL << VIRTIO_RING_F_INDIRECT_DESC) |
 		(1ULL << VIRTIO_RING_F_EVENT_IDX) | (1ULL << VIRTIO_F_VERSION_1);
-	long batch = 1;
+	long batch = 1, reset = 0;
 	int o;
 	bool delayed = false;
 
@@ -327,6 +351,17 @@ int main(int argc, char **argv)
 				assert(batch < (long)INT_MAX + 1);
 			}
 			break;
+		case 'r':
+			if (!optarg) {
+				reset = 1;
+			} else if (0 == strcmp(optarg, "random")) {
+				reset = RANDOM_RESET;
+			} else {
+				reset = strtol(optarg, NULL, 10);
+				assert(reset >= 0);
+				assert(reset < (long)INT_MAX + 1);
+			}
+			break;
 		default:
 			assert(0);
 			break;
@@ -336,6 +371,6 @@ int main(int argc, char **argv)
 done:
 	vdev_info_init(&dev, features);
 	vq_info_add(&dev, 256);
-	run_test(&dev, &dev.vqs[0], delayed, batch, 0x100000);
+	run_test(&dev, &dev.vqs[0], delayed, batch, reset, 0x100000);
 	return 0;
 }
-- 
2.18.1

[PATCH 4/6] tools/virtio: Make --reset reset ring idx

[Eugenio Pérez]

Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
---
 drivers/virtio/virtio_ring.c | 18 ++++++++++++++++++
 include/linux/virtio.h       |  2 ++
 tools/virtio/linux/virtio.h  |  2 ++
 tools/virtio/virtio_test.c   | 28 +++++++++++++++++++++++++++-
 4 files changed, 49 insertions(+), 1 deletion(-)

diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 867c7ebd3f10..aba44ac3f0d6 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -1810,6 +1810,24 @@ int virtqueue_add_inbuf_ctx(struct virtqueue *vq,
 }
 EXPORT_SYMBOL_GPL(virtqueue_add_inbuf_ctx);
 
+void virtqueue_reset_free_head(struct virtqueue *_vq)
+{
+	struct vring_virtqueue *vq = to_vvq(_vq);
+
+	// vq->last_used_idx = 0;
+	vq->num_added = 0;
+
+	vq->split.queue_size_in_bytes = 0;
+	vq->split.avail_flags_shadow = 0;
+	vq->split.avail_idx_shadow = 0;
+
+	memset(vq->split.desc_state, 0, vq->split.vring.num *
+			sizeof(struct vring_desc_state_split));
+
+	vq->free_head = 0;
+}
+EXPORT_SYMBOL_GPL(virtqueue_reset_free_head);
+
 /**
  * virtqueue_kick_prepare - first half of split virtqueue_kick call.
  * @_vq: the struct virtqueue
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index 15f906e4a748..286a0048fbeb 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -58,6 +58,8 @@ int virtqueue_add_sgs(struct virtqueue *vq,
 		      void *data,
 		      gfp_t gfp);
 
+void virtqueue_reset_free_head(struct virtqueue *vq);
+
 bool virtqueue_kick(struct virtqueue *vq);
 
 bool virtqueue_kick_prepare(struct virtqueue *vq);
diff --git a/tools/virtio/linux/virtio.h b/tools/virtio/linux/virtio.h
index b751350d4ce8..cf2e9ccf4de2 100644
--- a/tools/virtio/linux/virtio.h
+++ b/tools/virtio/linux/virtio.h
@@ -43,6 +43,8 @@ int virtqueue_add_inbuf(struct virtqueue *vq,
 			void *data,
 			gfp_t gfp);
 
+void virtqueue_reset_free_head(struct virtqueue *vq);
+
 bool virtqueue_kick(struct virtqueue *vq);
 
 void *virtqueue_get_buf(struct virtqueue *vq, unsigned int *len);
diff --git a/tools/virtio/virtio_test.c b/tools/virtio/virtio_test.c
index 93d81cd64ba0..bf21ece30594 100644
--- a/tools/virtio/virtio_test.c
+++ b/tools/virtio/virtio_test.c
@@ -49,6 +49,7 @@ struct vdev_info {
 
 static const struct vhost_vring_file no_backend = { .fd = -1 },
 				     backend = { .fd = 1 };
+static const struct vhost_vring_state null_state = {};
 
 bool vq_notify(struct virtqueue *vq)
 {
@@ -218,10 +219,33 @@ static void run_test(struct vdev_info *dev, struct vq_info *vq,
 			}
 
 			if (reset) {
+				struct vhost_vring_state s = { .index = 0 };
+				int i;
+				vq->vring.avail->idx = 0;
+				vq->vq->num_free = vq->vring.num;
+
+				// Put everything in free lists.
+				for (i = 0; i < vq->vring.num-1; i++)
+					vq->vring.desc[i].next =
+						cpu_to_virtio16(&dev->vdev,
+								i + 1);
+				vq->vring.desc[vq->vring.num-1].next = 0;
+				virtqueue_reset_free_head(vq->vq);
+
+				r = ioctl(dev->control, VHOST_GET_VRING_BASE,
+					  &s);
+				assert(!r);
+
+				s.num = 0;
+				r = ioctl(dev->control, VHOST_SET_VRING_BASE,
+					  &null_state);
+				assert(!r);
+
 				r = ioctl(dev->control, VHOST_TEST_SET_BACKEND,
 					  &backend);
 				assert(!r);
 
+				started = completed;
                                 while (completed > next_reset)
 					next_reset += completed;
 			}
@@ -243,7 +267,9 @@ static void run_test(struct vdev_info *dev, struct vq_info *vq,
 	test = 0;
 	r = ioctl(dev->control, VHOST_TEST_RUN, &test);
 	assert(r >= 0);
-	fprintf(stderr, "spurious wakeups: 0x%llx\n", spurious);
+	fprintf(stderr,
+		"spurious wakeups: 0x%llx started=0x%lx completed=0x%lx\n",
+		spurious, started, completed);
 }
 
 const char optstring[] = "h";
-- 
2.18.1

[PATCH 5/6] vhost: Delete virtqueue batch_descs member

[Eugenio Pérez]

It can be deduced from "max_descs".

Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
---
 drivers/vhost/vhost.c | 11 +++++++++--
 drivers/vhost/vhost.h |  1 -
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index b5a51b1f2e79..5f84f29b6c47 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -372,6 +372,11 @@ static int vhost_worker(void *data)
 	return 0;
 }
 
+static int vhost_vq_num_batch_descs(struct vhost_virtqueue *vq)
+{
+	return vq->max_descs - UIO_MAXIOV;
+}
+
 static void vhost_vq_free_iovecs(struct vhost_virtqueue *vq)
 {
 	kfree(vq->descs);
@@ -394,7 +399,9 @@ static long vhost_dev_alloc_iovecs(struct vhost_dev *dev)
 	for (i = 0; i < dev->nvqs; ++i) {
 		vq = dev->vqs[i];
 		vq->max_descs = dev->iov_limit;
-		vq->batch_descs = dev->iov_limit - UIO_MAXIOV;
+		if (vhost_vq_num_batch_descs(vq) < 0) {
+			return -EINVAL;
+		}
 		vq->descs = kmalloc_array(vq->max_descs,
 					  sizeof(*vq->descs),
 					  GFP_KERNEL);
@@ -2333,7 +2340,7 @@ static int fetch_descs(struct vhost_virtqueue *vq)
 	if (vq->ndescs)
 		return 0;
 
-	while (!ret && vq->ndescs <= vq->batch_descs)
+	while (!ret && vq->ndescs <= vhost_vq_num_batch_descs(vq))
 		ret = fetch_buf(vq);
 
 	return vq->ndescs ? 0 : ret;
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index 661088ae6dc7..e648b9b997d4 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -102,7 +102,6 @@ struct vhost_virtqueue {
 	int ndescs;
 	int first_desc;
 	int max_descs;
-	int batch_descs;
 
 	const struct vhost_umem_node *meta_iotlb[VHOST_NUM_ADDRS];
 	struct file *kick;
-- 
2.18.1

[PATCH 6/6] fixup! vhost: batching fetches

[Eugenio Pérez]

Old code did not take into account the _SET_BASE ioctl.

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

diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 5f84f29b6c47..1646b1ce312a 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -1652,6 +1652,7 @@ long vhost_vring_ioctl(struct vhost_dev *d, unsigned int ioctl, void __user *arg
 		vq->last_avail_idx = s.num;
 		/* Forget the cached index value. */
 		vq->avail_idx = vq->last_avail_idx;
+		vq->ndescs = vq->first_desc = 0;
 		break;
 	case VHOST_GET_VRING_BASE:
 		s.index = idx;
-- 
2.18.1

Re: [PATCH 0/6] vhost: Reset batched descriptors on SET_VRING_BASE call

[Michael S. Tsirkin]

On Sun, Mar 29, 2020 at 01:33:53PM +0200, Eugenio Pérez wrote:
> Vhost did not reset properly the batched descriptors on SET_VRING_BASE event. Because of that, is possible to return an invalid descriptor to the guest.
> This series ammend this, and creates a test to assert correct behavior. To do that, they need to expose a new function in virtio_ring, virtqueue_reset_free_head. Not sure if this can be avoided.

Question: why not reset the batch when private_data changes?
At the moment both net and scsi poke at private data directly,
if they do this through a wrapper we can use that to
1. check that vq mutex is taken properly
2. reset batching

This seems like a slightly better API

> 
> Also, change from https://lkml.org/lkml/2020/3/27/108 is not included, that avoids to update a variable in a loop where it can be updated once.
> 
> This is meant to be applied on top of eccb852f1fe6bede630e2e4f1a121a81e34354ab in git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git, and some commits should be squashed with that series.

Thanks a lot! I'll apply this for now so Christian can start testing,
but I'd like the comment above addressed before I push this to Linus.

> Eugenio Pérez (6):
>   tools/virtio: Add --batch option
>   tools/virtio: Add --batch=random option
>   tools/virtio: Add --reset=random
>   tools/virtio: Make --reset reset ring idx
>   vhost: Delete virtqueue batch_descs member
>   fixup! vhost: batching fetches
> 
>  drivers/vhost/test.c         |  57 ++++++++++++++++
>  drivers/vhost/test.h         |   1 +
>  drivers/vhost/vhost.c        |  12 +++-
>  drivers/vhost/vhost.h        |   1 -
>  drivers/virtio/virtio_ring.c |  18 +++++
>  include/linux/virtio.h       |   2 +
>  tools/virtio/linux/virtio.h  |   2 +
>  tools/virtio/virtio_test.c   | 123 +++++++++++++++++++++++++++++++----
>  8 files changed, 201 insertions(+), 15 deletions(-)
> 
> -- 
> 2.18.1

Re: [PATCH 0/6] vhost: Reset batched descriptors on SET_VRING_BASE call

[Eugenio Perez Martin]

On Sun, Mar 29, 2020 at 1:49 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Sun, Mar 29, 2020 at 01:33:53PM +0200, Eugenio Pérez wrote:
> > Vhost did not reset properly the batched descriptors on SET_VRING_BASE event. Because of that, is possible to return an invalid descriptor to the guest.
> > This series ammend this, and creates a test to assert correct behavior. To do that, they need to expose a new function in virtio_ring, virtqueue_reset_free_head. Not sure if this can be avoided.
>
> Question: why not reset the batch when private_data changes?
> At the moment both net and scsi poke at private data directly,
> if they do this through a wrapper we can use that to
> 1. check that vq mutex is taken properly
> 2. reset batching
>
> This seems like a slightly better API
>

I didn't do that way because qemu could just SET_BACKEND to -1 and
SET_BACKEND to the same one, with no call to SET_VRING. In this case,
I think that qemu should not change the descriptors already pushed. I
do agree with the interface to modify private_data properly (regarding
the mutex).

However, I can see how your proposal is safer, so we don't even need
to check if private_data is != NULL when we have descriptors in the
batch_descs array. Also, this ioctls should not be in the hot path, so
we can change to that mode anyway.

> >
> > Also, change from https://lkml.org/lkml/2020/3/27/108 is not included, that avoids to update a variable in a loop where it can be updated once.
> >
> > This is meant to be applied on top of eccb852f1fe6bede630e2e4f1a121a81e34354ab in git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git, and some commits should be squashed with that series.
>
> Thanks a lot! I'll apply this for now so Christian can start testing,
> but I'd like the comment above addressed before I push this to Linus.
>
> > Eugenio Pérez (6):
> >   tools/virtio: Add --batch option
> >   tools/virtio: Add --batch=random option
> >   tools/virtio: Add --reset=random
> >   tools/virtio: Make --reset reset ring idx
> >   vhost: Delete virtqueue batch_descs member
> >   fixup! vhost: batching fetches
> >
> >  drivers/vhost/test.c         |  57 ++++++++++++++++
> >  drivers/vhost/test.h         |   1 +
> >  drivers/vhost/vhost.c        |  12 +++-
> >  drivers/vhost/vhost.h        |   1 -
> >  drivers/virtio/virtio_ring.c |  18 +++++
> >  include/linux/virtio.h       |   2 +
> >  tools/virtio/linux/virtio.h  |   2 +
> >  tools/virtio/virtio_test.c   | 123 +++++++++++++++++++++++++++++++----
> >  8 files changed, 201 insertions(+), 15 deletions(-)
> >
> > --
> > 2.18.1
>

Re: [PATCH 0/6] vhost: Reset batched descriptors on SET_VRING_BASE call

[Michael S. Tsirkin]

On Sun, Mar 29, 2020 at 02:19:55PM +0200, Eugenio Perez Martin wrote:
> On Sun, Mar 29, 2020 at 1:49 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > On Sun, Mar 29, 2020 at 01:33:53PM +0200, Eugenio Pérez wrote:
> > > Vhost did not reset properly the batched descriptors on SET_VRING_BASE event. Because of that, is possible to return an invalid descriptor to the guest.
> > > This series ammend this, and creates a test to assert correct behavior. To do that, they need to expose a new function in virtio_ring, virtqueue_reset_free_head. Not sure if this can be avoided.
> >
> > Question: why not reset the batch when private_data changes?
> > At the moment both net and scsi poke at private data directly,
> > if they do this through a wrapper we can use that to
> > 1. check that vq mutex is taken properly
> > 2. reset batching
> >
> > This seems like a slightly better API
> >
> 
> I didn't do that way because qemu could just SET_BACKEND to -1 and
> SET_BACKEND to the same one, with no call to SET_VRING. In this case,
> I think that qemu should not change the descriptors already pushed.

Well dropping the batch is always safe, batch is an optimization.


> I
> do agree with the interface to modify private_data properly (regarding
> the mutex).
> 
> However, I can see how your proposal is safer, so we don't even need
> to check if private_data is != NULL when we have descriptors in the
> batch_descs array. Also, this ioctls should not be in the hot path, so
> we can change to that mode anyway.
> 
> > >
> > > Also, change from https://lkml.org/lkml/2020/3/27/108 is not included, that avoids to update a variable in a loop where it can be updated once.
> > >
> > > This is meant to be applied on top of eccb852f1fe6bede630e2e4f1a121a81e34354ab in git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git, and some commits should be squashed with that series.
> >
> > Thanks a lot! I'll apply this for now so Christian can start testing,
> > but I'd like the comment above addressed before I push this to Linus.
> >
> > > Eugenio Pérez (6):
> > >   tools/virtio: Add --batch option
> > >   tools/virtio: Add --batch=random option
> > >   tools/virtio: Add --reset=random
> > >   tools/virtio: Make --reset reset ring idx
> > >   vhost: Delete virtqueue batch_descs member
> > >   fixup! vhost: batching fetches
> > >
> > >  drivers/vhost/test.c         |  57 ++++++++++++++++
> > >  drivers/vhost/test.h         |   1 +
> > >  drivers/vhost/vhost.c        |  12 +++-
> > >  drivers/vhost/vhost.h        |   1 -
> > >  drivers/virtio/virtio_ring.c |  18 +++++
> > >  include/linux/virtio.h       |   2 +
> > >  tools/virtio/linux/virtio.h  |   2 +
> > >  tools/virtio/virtio_test.c   | 123 +++++++++++++++++++++++++++++++----
> > >  8 files changed, 201 insertions(+), 15 deletions(-)
> > >
> > > --
> > > 2.18.1
> >

Re: [PATCH 4/6] tools/virtio: Make --reset reset ring idx

[Michael S. Tsirkin]

On Sun, Mar 29, 2020 at 01:33:57PM +0200, Eugenio Pérez wrote:
> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> ---
>  drivers/virtio/virtio_ring.c | 18 ++++++++++++++++++
>  include/linux/virtio.h       |  2 ++
>  tools/virtio/linux/virtio.h  |  2 ++
>  tools/virtio/virtio_test.c   | 28 +++++++++++++++++++++++++++-
>  4 files changed, 49 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index 867c7ebd3f10..aba44ac3f0d6 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -1810,6 +1810,24 @@ int virtqueue_add_inbuf_ctx(struct virtqueue *vq,
>  }
>  EXPORT_SYMBOL_GPL(virtqueue_add_inbuf_ctx);
>  
> +void virtqueue_reset_free_head(struct virtqueue *_vq)
> +{
> +	struct vring_virtqueue *vq = to_vvq(_vq);
> +
> +	// vq->last_used_idx = 0;
> +	vq->num_added = 0;
> +
> +	vq->split.queue_size_in_bytes = 0;
> +	vq->split.avail_flags_shadow = 0;
> +	vq->split.avail_idx_shadow = 0;
> +
> +	memset(vq->split.desc_state, 0, vq->split.vring.num *
> +			sizeof(struct vring_desc_state_split));
> +
> +	vq->free_head = 0;
> +}
> +EXPORT_SYMBOL_GPL(virtqueue_reset_free_head);
> +
>  /**
>   * virtqueue_kick_prepare - first half of split virtqueue_kick call.
>   * @_vq: the struct virtqueue

Add documentation please. When should this be called?
If it's just for testing, we can put this within some ifdef
that only triggers when building the test ...


> diff --git a/include/linux/virtio.h b/include/linux/virtio.h
> index 15f906e4a748..286a0048fbeb 100644
> --- a/include/linux/virtio.h
> +++ b/include/linux/virtio.h
> @@ -58,6 +58,8 @@ int virtqueue_add_sgs(struct virtqueue *vq,
>  		      void *data,
>  		      gfp_t gfp);
>  
> +void virtqueue_reset_free_head(struct virtqueue *vq);
> +
>  bool virtqueue_kick(struct virtqueue *vq);
>  
>  bool virtqueue_kick_prepare(struct virtqueue *vq);
> diff --git a/tools/virtio/linux/virtio.h b/tools/virtio/linux/virtio.h
> index b751350d4ce8..cf2e9ccf4de2 100644
> --- a/tools/virtio/linux/virtio.h
> +++ b/tools/virtio/linux/virtio.h
> @@ -43,6 +43,8 @@ int virtqueue_add_inbuf(struct virtqueue *vq,
>  			void *data,
>  			gfp_t gfp);
>  
> +void virtqueue_reset_free_head(struct virtqueue *vq);
> +
>  bool virtqueue_kick(struct virtqueue *vq);
>  
>  void *virtqueue_get_buf(struct virtqueue *vq, unsigned int *len);
> diff --git a/tools/virtio/virtio_test.c b/tools/virtio/virtio_test.c
> index 93d81cd64ba0..bf21ece30594 100644
> --- a/tools/virtio/virtio_test.c
> +++ b/tools/virtio/virtio_test.c
> @@ -49,6 +49,7 @@ struct vdev_info {
>  
>  static const struct vhost_vring_file no_backend = { .fd = -1 },
>  				     backend = { .fd = 1 };
> +static const struct vhost_vring_state null_state = {};
>  
>  bool vq_notify(struct virtqueue *vq)
>  {
> @@ -218,10 +219,33 @@ static void run_test(struct vdev_info *dev, struct vq_info *vq,
>  			}
>  
>  			if (reset) {
> +				struct vhost_vring_state s = { .index = 0 };
> +				int i;
> +				vq->vring.avail->idx = 0;
> +				vq->vq->num_free = vq->vring.num;
> +
> +				// Put everything in free lists.
> +				for (i = 0; i < vq->vring.num-1; i++)
> +					vq->vring.desc[i].next =
> +						cpu_to_virtio16(&dev->vdev,
> +								i + 1);
> +				vq->vring.desc[vq->vring.num-1].next = 0;
> +				virtqueue_reset_free_head(vq->vq);
> +


Hmm this is poking at internal vq format ...


> +				r = ioctl(dev->control, VHOST_GET_VRING_BASE,
> +					  &s);
> +				assert(!r);
> +
> +				s.num = 0;
> +				r = ioctl(dev->control, VHOST_SET_VRING_BASE,
> +					  &null_state);
> +				assert(!r);
> +
>  				r = ioctl(dev->control, VHOST_TEST_SET_BACKEND,
>  					  &backend);
>  				assert(!r);
>  
> +				started = completed;
>                                  while (completed > next_reset)
>  					next_reset += completed;
>  			}
> @@ -243,7 +267,9 @@ static void run_test(struct vdev_info *dev, struct vq_info *vq,
>  	test = 0;
>  	r = ioctl(dev->control, VHOST_TEST_RUN, &test);
>  	assert(r >= 0);
> -	fprintf(stderr, "spurious wakeups: 0x%llx\n", spurious);
> +	fprintf(stderr,
> +		"spurious wakeups: 0x%llx started=0x%lx completed=0x%lx\n",
> +		spurious, started, completed);
>  }
>  
>  const char optstring[] = "h";
> -- 
> 2.18.1

Re: [PATCH 0/6] vhost: Reset batched descriptors on SET_VRING_BASE call

[Christian Borntraeger]

On 29.03.20 13:33, Eugenio Pérez wrote:
> Vhost did not reset properly the batched descriptors on SET_VRING_BASE event. Because of that, is possible to return an invalid descriptor to the guest.

I guess this could explain my problems that I have seen during reset?

> 
> This series ammend this, and creates a test to assert correct behavior. To do that, they need to expose a new function in virtio_ring, virtqueue_reset_free_head. Not sure if this can be avoided.
> 
> Also, change from https://lkml.org/lkml/2020/3/27/108 is not included, that avoids to update a variable in a loop where it can be updated once.
> 
> This is meant to be applied on top of eccb852f1fe6bede630e2e4f1a121a81e34354ab in git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git, and some commits should be squashed with that series.
> 
> Eugenio Pérez (6):
>   tools/virtio: Add --batch option
>   tools/virtio: Add --batch=random option
>   tools/virtio: Add --reset=random
>   tools/virtio: Make --reset reset ring idx
>   vhost: Delete virtqueue batch_descs member
>   fixup! vhost: batching fetches
> 
>  drivers/vhost/test.c         |  57 ++++++++++++++++
>  drivers/vhost/test.h         |   1 +
>  drivers/vhost/vhost.c        |  12 +++-
>  drivers/vhost/vhost.h        |   1 -
>  drivers/virtio/virtio_ring.c |  18 +++++
>  include/linux/virtio.h       |   2 +
>  tools/virtio/linux/virtio.h  |   2 +
>  tools/virtio/virtio_test.c   | 123 +++++++++++++++++++++++++++++++----
>  8 files changed, 201 insertions(+), 15 deletions(-)
> 

Re: [PATCH 0/6] vhost: Reset batched descriptors on SET_VRING_BASE call

[Eugenio Perez Martin]

On Mon, Mar 30, 2020 at 9:14 AM Christian Borntraeger
<borntraeger@de.ibm.com> wrote:
>
>
> On 29.03.20 13:33, Eugenio Pérez wrote:
> > Vhost did not reset properly the batched descriptors on SET_VRING_BASE event. Because of that, is possible to return an invalid descriptor to the guest.
>
> I guess this could explain my problems that I have seen during reset?
>

Yes, I think so. The series has a test that should reproduce more or
less what you are seeing. However, it would be useful to reproduce on
your system and to know what causes qemu to send the reset :).

Thanks!

> >
> > This series ammend this, and creates a test to assert correct behavior. To do that, they need to expose a new function in virtio_ring, virtqueue_reset_free_head. Not sure if this can be avoided.
> >
> > Also, change from https://lkml.org/lkml/2020/3/27/108 is not included, that avoids to update a variable in a loop where it can be updated once.
> >
> > This is meant to be applied on top of eccb852f1fe6bede630e2e4f1a121a81e34354ab in git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git, and some commits should be squashed with that series.
> >
> > Eugenio Pérez (6):
> >   tools/virtio: Add --batch option
> >   tools/virtio: Add --batch=random option
> >   tools/virtio: Add --reset=random
> >   tools/virtio: Make --reset reset ring idx
> >   vhost: Delete virtqueue batch_descs member
> >   fixup! vhost: batching fetches
> >
> >  drivers/vhost/test.c         |  57 ++++++++++++++++
> >  drivers/vhost/test.h         |   1 +
> >  drivers/vhost/vhost.c        |  12 +++-
> >  drivers/vhost/vhost.h        |   1 -
> >  drivers/virtio/virtio_ring.c |  18 +++++
> >  include/linux/virtio.h       |   2 +
> >  tools/virtio/linux/virtio.h  |   2 +
> >  tools/virtio/virtio_test.c   | 123 +++++++++++++++++++++++++++++++----
> >  8 files changed, 201 insertions(+), 15 deletions(-)
> >
>

Re: [PATCH 0/6] vhost: Reset batched descriptors on SET_VRING_BASE call

[Christian Borntraeger]

On 30.03.20 09:18, Eugenio Perez Martin wrote:
> On Mon, Mar 30, 2020 at 9:14 AM Christian Borntraeger
> <borntraeger@de.ibm.com> wrote:
>>
>>
>> On 29.03.20 13:33, Eugenio Pérez wrote:
>>> Vhost did not reset properly the batched descriptors on SET_VRING_BASE event. Because of that, is possible to return an invalid descriptor to the guest.
>>
>> I guess this could explain my problems that I have seen during reset?
>>
> 
> Yes, I think so. The series has a test that should reproduce more or
> less what you are seeing. However, it would be useful to reproduce on
> your system and to know what causes qemu to send the reset :).

I do see SET_VRING_BASE in the debug output
[228101.438630] [2113] vhost:vhost_vring_ioctl:1668: VHOST_GET_VRING_BASE [vq=00000000618905fc][s.index=1][s.num=42424][vq->avail_idx=42424][vq->last_avail_idx=42424][vq->ndescs=0][vq->first_desc=0]
[228101.438631] CPU: 54 PID: 2113 Comm: qemu-system-s39 Not tainted 5.5.0+ #344
[228101.438632] Hardware name: IBM 3906 M04 704 (LPAR)
[228101.438633] Call Trace:
[228101.438634]  [<00000004fc71c132>] show_stack+0x8a/0xd0 
[228101.438636]  [<00000004fd10e72a>] dump_stack+0x8a/0xb8 
[228101.438639]  [<000003ff80377600>] vhost_vring_ioctl+0x668/0x848 [vhost] 
[228101.438640]  [<000003ff80395fd4>] vhost_net_ioctl+0x4f4/0x570 [vhost_net] 
[228101.438642]  [<00000004fc9ccdd8>] do_vfs_ioctl+0x430/0x6f8 
[228101.438643]  [<00000004fc9cd124>] ksys_ioctl+0x84/0xb0 
[228101.438645]  [<00000004fc9cd1ba>] __s390x_sys_ioctl+0x2a/0x38 
[228101.438646]  [<00000004fd12ff72>] system_call+0x2a6/0x2c8 
[228103.682732] [2122] vhost:vhost_vring_ioctl:1653: VHOST_SET_VRING_BASE [vq=000000009e1ac3e7][s.index=0][s.num=0][vq->avail_idx=27875][vq->last_avail_idx=27709][vq->ndescs=65][vq->first_desc=22]
[228103.682735] CPU: 44 PID: 2122 Comm: CPU 0/KVM Not tainted 5.5.0+ #344
[228103.682739] Hardware name: IBM 3906 M04 704 (LPAR)
[228103.682741] Call Trace:
[228103.682748]  [<00000004fc71c132>] show_stack+0x8a/0xd0 
[228103.682752]  [<00000004fd10e72a>] dump_stack+0x8a/0xb8 
[228103.682761]  [<000003ff80377422>] vhost_vring_ioctl+0x48a/0x848 [vhost] 
[228103.682764]  [<000003ff80395fd4>] vhost_net_ioctl+0x4f4/0x570 [vhost_net] 
[228103.682767]  [<00000004fc9ccdd8>] do_vfs_ioctl+0x430/0x6f8 
[228103.682769]  [<00000004fc9cd124>] ksys_ioctl+0x84/0xb0 
[228103.682771]  [<00000004fc9cd1ba>] __s390x_sys_ioctl+0x2a/0x38 
[228103.682773]  [<00000004fd12ff72>] system_call+0x2a6/0x2c8 
[228103.682794] [2122] vhost:vhost_vring_ioctl:1653: VHOST_SET_VRING_BASE [vq=00000000618905fc][s.index=1][s.num=0][vq->avail_idx=42424][vq->last_avail_idx=42424][vq->ndescs=0][vq->first_desc=0]
[228103.682795] CPU: 44 PID: 2122 Comm: CPU 0/KVM Not tainted 5.5.0+ #344
[228103.682797] Hardware name: IBM 3906 M04 704 (LPAR)
[228103.682797] Call Trace:
[228103.682799]  [<00000004fc71c132>] show_stack+0x8a/0xd0 
[228103.682801]  [<00000004fd10e72a>] dump_stack+0x8a/0xb8 
[228103.682804]  [<000003ff80377422>] vhost_vring_ioctl+0x48a/0x848 [vhost] 
[228103.682806]  [<000003ff80395fd4>] vhost_net_ioctl+0x4f4/0x570 [vhost_net] 
[228103.682808]  [<00000004fc9ccdd8>] do_vfs_ioctl+0x430/0x6f8 
[228103.682810]  [<00000004fc9cd124>] ksys_ioctl+0x84/0xb0 
[228103.682812]  [<00000004fc9cd1ba>] __s390x_sys_ioctl+0x2a/0x38 
[228103.682813]  [<00000004fd12ff72>] system_call+0x2a6/0x2c8 


Isnt that triggered by resetting the virtio devices during system reboot?

Re: [PATCH 0/6] vhost: Reset batched descriptors on SET_VRING_BASE call

[Eugenio Perez Martin]

On Mon, Mar 30, 2020 at 9:34 AM Christian Borntraeger
<borntraeger@de.ibm.com> wrote:
>
>
>
> On 30.03.20 09:18, Eugenio Perez Martin wrote:
> > On Mon, Mar 30, 2020 at 9:14 AM Christian Borntraeger
> > <borntraeger@de.ibm.com> wrote:
> >>
> >>
> >> On 29.03.20 13:33, Eugenio Pérez wrote:
> >>> Vhost did not reset properly the batched descriptors on SET_VRING_BASE event. Because of that, is possible to return an invalid descriptor to the guest.
> >>
> >> I guess this could explain my problems that I have seen during reset?
> >>
> >
> > Yes, I think so. The series has a test that should reproduce more or
> > less what you are seeing. However, it would be useful to reproduce on
> > your system and to know what causes qemu to send the reset :).
>
> I do see SET_VRING_BASE in the debug output
> [228101.438630] [2113] vhost:vhost_vring_ioctl:1668: VHOST_GET_VRING_BASE [vq=00000000618905fc][s.index=1][s.num=42424][vq->avail_idx=42424][vq->last_avail_idx=42424][vq->ndescs=0][vq->first_desc=0]
> [228101.438631] CPU: 54 PID: 2113 Comm: qemu-system-s39 Not tainted 5.5.0+ #344
> [228101.438632] Hardware name: IBM 3906 M04 704 (LPAR)
> [228101.438633] Call Trace:
> [228101.438634]  [<00000004fc71c132>] show_stack+0x8a/0xd0
> [228101.438636]  [<00000004fd10e72a>] dump_stack+0x8a/0xb8
> [228101.438639]  [<000003ff80377600>] vhost_vring_ioctl+0x668/0x848 [vhost]
> [228101.438640]  [<000003ff80395fd4>] vhost_net_ioctl+0x4f4/0x570 [vhost_net]
> [228101.438642]  [<00000004fc9ccdd8>] do_vfs_ioctl+0x430/0x6f8
> [228101.438643]  [<00000004fc9cd124>] ksys_ioctl+0x84/0xb0
> [228101.438645]  [<00000004fc9cd1ba>] __s390x_sys_ioctl+0x2a/0x38
> [228101.438646]  [<00000004fd12ff72>] system_call+0x2a6/0x2c8
> [228103.682732] [2122] vhost:vhost_vring_ioctl:1653: VHOST_SET_VRING_BASE [vq=000000009e1ac3e7][s.index=0][s.num=0][vq->avail_idx=27875][vq->last_avail_idx=27709][vq->ndescs=65][vq->first_desc=22]
> [228103.682735] CPU: 44 PID: 2122 Comm: CPU 0/KVM Not tainted 5.5.0+ #344
> [228103.682739] Hardware name: IBM 3906 M04 704 (LPAR)
> [228103.682741] Call Trace:
> [228103.682748]  [<00000004fc71c132>] show_stack+0x8a/0xd0
> [228103.682752]  [<00000004fd10e72a>] dump_stack+0x8a/0xb8
> [228103.682761]  [<000003ff80377422>] vhost_vring_ioctl+0x48a/0x848 [vhost]
> [228103.682764]  [<000003ff80395fd4>] vhost_net_ioctl+0x4f4/0x570 [vhost_net]
> [228103.682767]  [<00000004fc9ccdd8>] do_vfs_ioctl+0x430/0x6f8
> [228103.682769]  [<00000004fc9cd124>] ksys_ioctl+0x84/0xb0
> [228103.682771]  [<00000004fc9cd1ba>] __s390x_sys_ioctl+0x2a/0x38
> [228103.682773]  [<00000004fd12ff72>] system_call+0x2a6/0x2c8
> [228103.682794] [2122] vhost:vhost_vring_ioctl:1653: VHOST_SET_VRING_BASE [vq=00000000618905fc][s.index=1][s.num=0][vq->avail_idx=42424][vq->last_avail_idx=42424][vq->ndescs=0][vq->first_desc=0]
> [228103.682795] CPU: 44 PID: 2122 Comm: CPU 0/KVM Not tainted 5.5.0+ #344
> [228103.682797] Hardware name: IBM 3906 M04 704 (LPAR)
> [228103.682797] Call Trace:
> [228103.682799]  [<00000004fc71c132>] show_stack+0x8a/0xd0
> [228103.682801]  [<00000004fd10e72a>] dump_stack+0x8a/0xb8
> [228103.682804]  [<000003ff80377422>] vhost_vring_ioctl+0x48a/0x848 [vhost]
> [228103.682806]  [<000003ff80395fd4>] vhost_net_ioctl+0x4f4/0x570 [vhost_net]
> [228103.682808]  [<00000004fc9ccdd8>] do_vfs_ioctl+0x430/0x6f8
> [228103.682810]  [<00000004fc9cd124>] ksys_ioctl+0x84/0xb0
> [228103.682812]  [<00000004fc9cd1ba>] __s390x_sys_ioctl+0x2a/0x38
> [228103.682813]  [<00000004fd12ff72>] system_call+0x2a6/0x2c8
>
>
> Isnt that triggered by resetting the virtio devices during system reboot?
>

Yes. I don't know exactly why qemu is sending them, but vhost should
be able to "protect/continue" the same way it used to be before
batching patches.

Did you lose connectivity or experienced rebooting with this patches applied?

Thanks!