kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH kvmtool v2 00/17] Fix vhost-net, scsi and vsock
@ 2023-06-06 13:04 Jean-Philippe Brucker
  2023-06-06 13:04 ` [PATCH kvmtool v2 01/17] virtio: Factor vhost initialization Jean-Philippe Brucker
                   ` (17 more replies)
  0 siblings, 18 replies; 19+ messages in thread
From: Jean-Philippe Brucker @ 2023-06-06 13:04 UTC (permalink / raw)
  To: kvm, will; +Cc: andre.przywara, Jean-Philippe Brucker

This is version 2 of the vhost fixes for kvmtool posted here:
https://lore.kernel.org/all/20230419132119.124457-1-jean-philippe@linaro.org/

Since v1:
* Added review tags from Andre
* Fixed issues reported by Andre, and the max_target size found while
  rebasing patch 8
* Added patch 14 (warn and disable an unsupported configuration)

Jean-Philippe Brucker (17):
  virtio: Factor vhost initialization
  virtio/vhost: Factor vring operation
  virtio/vhost: Factor notify_vq_eventfd()
  virtio/vhost: Factor notify_vq_gsi()
  virtio/scsi: Move VHOST_SCSI_SET_ENDPOINT to device start
  virtio/scsi: Fix and simplify command-line
  disk/core: Fix segfault on exit with SCSI
  virtio/scsi: Initialize max_target
  virtio/scsi: Fix feature selection
  virtio/vsock: Fix feature selection
  virtio/net: Fix feature selection
  virtio: Document how to test the devices
  virtio: Fix messages about missing Linux config
  virtio/net: Warn about enabling multiqueue with vhost
  Factor epoll thread
  virtio/vhost: Support line interrupt signaling
  virtio/vhost: Clear VIRTIO_F_ACCESS_PLATFORM

 Makefile                     |   2 +
 Documentation/io-testing.txt | 141 +++++++++++++++++++++++
 include/kvm/disk-image.h     |   7 +-
 include/kvm/epoll.h          |  17 +++
 include/kvm/virtio.h         |  16 +++
 disk/core.c                  |  15 +--
 epoll.c                      |  91 +++++++++++++++
 ioeventfd.c                  |  94 +++-------------
 kvm-ipc.c                    | 103 +++++------------
 virtio/core.c                |   1 +
 virtio/net.c                 | 129 +++++----------------
 virtio/scsi.c                | 118 ++++++--------------
 virtio/vhost.c               | 209 +++++++++++++++++++++++++++++++++++
 virtio/vsock.c               | 134 +++++-----------------
 14 files changed, 611 insertions(+), 466 deletions(-)
 create mode 100644 Documentation/io-testing.txt
 create mode 100644 include/kvm/epoll.h
 create mode 100644 epoll.c
 create mode 100644 virtio/vhost.c

-- 
2.40.1


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH kvmtool v2 01/17] virtio: Factor vhost initialization
  2023-06-06 13:04 [PATCH kvmtool v2 00/17] Fix vhost-net, scsi and vsock Jean-Philippe Brucker
@ 2023-06-06 13:04 ` Jean-Philippe Brucker
  2023-06-06 13:04 ` [PATCH kvmtool v2 02/17] virtio/vhost: Factor vring operation Jean-Philippe Brucker
                   ` (16 subsequent siblings)
  17 siblings, 0 replies; 19+ messages in thread
From: Jean-Philippe Brucker @ 2023-06-06 13:04 UTC (permalink / raw)
  To: kvm, will; +Cc: andre.przywara, Jean-Philippe Brucker

Move vhost owner and memory table setup to virtio/vhost.c.

This also fixes vsock and SCSI which did not support multiple memory
regions until now (vsock didn't allocate the right region size and would
trigger a buffer overflow).

Reviewed-by: Andre Przywara <andre.przywara@arm.com>
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 Makefile             |  1 +
 include/kvm/virtio.h |  1 +
 virtio/net.c         | 29 +----------------------------
 virtio/scsi.c        | 21 +--------------------
 virtio/vhost.c       | 36 ++++++++++++++++++++++++++++++++++++
 virtio/vsock.c       | 29 ++---------------------------
 6 files changed, 42 insertions(+), 75 deletions(-)
 create mode 100644 virtio/vhost.c

diff --git a/Makefile b/Makefile
index ed2414bd..86e19339 100644
--- a/Makefile
+++ b/Makefile
@@ -76,6 +76,7 @@ OBJS	+= virtio/pci.o
 OBJS	+= virtio/vsock.o
 OBJS	+= virtio/pci-legacy.o
 OBJS	+= virtio/pci-modern.o
+OBJS	+= virtio/vhost.o
 OBJS	+= disk/blk.o
 OBJS	+= disk/qcow.o
 OBJS	+= disk/raw.o
diff --git a/include/kvm/virtio.h b/include/kvm/virtio.h
index a8bbaf21..4cc2e3d2 100644
--- a/include/kvm/virtio.h
+++ b/include/kvm/virtio.h
@@ -258,6 +258,7 @@ void virtio_set_guest_features(struct kvm *kvm, struct virtio_device *vdev,
 			       void *dev, u64 features);
 void virtio_notify_status(struct kvm *kvm, struct virtio_device *vdev,
 			  void *dev, u8 status);
+void virtio_vhost_init(struct kvm *kvm, int vhost_fd);
 
 int virtio_transport_parser(const struct option *opt, const char *arg, int unset);
 
diff --git a/virtio/net.c b/virtio/net.c
index bc20ce09..65fdbd17 100644
--- a/virtio/net.c
+++ b/virtio/net.c
@@ -791,40 +791,13 @@ static struct virtio_ops net_dev_virtio_ops = {
 
 static void virtio_net__vhost_init(struct kvm *kvm, struct net_dev *ndev)
 {
-	struct kvm_mem_bank *bank;
-	struct vhost_memory *mem;
-	int r, i;
-
 	ndev->vhost_fd = open("/dev/vhost-net", O_RDWR);
 	if (ndev->vhost_fd < 0)
 		die_perror("Failed openning vhost-net device");
 
-	mem = calloc(1, sizeof(*mem) + kvm->mem_slots * sizeof(struct vhost_memory_region));
-	if (mem == NULL)
-		die("Failed allocating memory for vhost memory map");
-
-	i = 0;
-	list_for_each_entry(bank, &kvm->mem_banks, list) {
-		mem->regions[i] = (struct vhost_memory_region) {
-			.guest_phys_addr = bank->guest_phys_addr,
-			.memory_size	 = bank->size,
-			.userspace_addr	 = (unsigned long)bank->host_addr,
-		};
-		i++;
-	}
-	mem->nregions = i;
-
-	r = ioctl(ndev->vhost_fd, VHOST_SET_OWNER);
-	if (r != 0)
-		die_perror("VHOST_SET_OWNER failed");
-
-	r = ioctl(ndev->vhost_fd, VHOST_SET_MEM_TABLE, mem);
-	if (r != 0)
-		die_perror("VHOST_SET_MEM_TABLE failed");
+	virtio_vhost_init(kvm, ndev->vhost_fd);
 
 	ndev->vdev.use_vhost = true;
-
-	free(mem);
 }
 
 static inline void str_to_mac(const char *str, char *mac)
diff --git a/virtio/scsi.c b/virtio/scsi.c
index 9af8a65c..621a8334 100644
--- a/virtio/scsi.c
+++ b/virtio/scsi.c
@@ -201,7 +201,6 @@ static struct virtio_ops scsi_dev_virtio_ops = {
 
 static void virtio_scsi_vhost_init(struct kvm *kvm, struct scsi_dev *sdev)
 {
-	struct vhost_memory *mem;
 	u64 features;
 	int r;
 
@@ -209,20 +208,7 @@ static void virtio_scsi_vhost_init(struct kvm *kvm, struct scsi_dev *sdev)
 	if (sdev->vhost_fd < 0)
 		die_perror("Failed openning vhost-scsi device");
 
-	mem = calloc(1, sizeof(*mem) + sizeof(struct vhost_memory_region));
-	if (mem == NULL)
-		die("Failed allocating memory for vhost memory map");
-
-	mem->nregions = 1;
-	mem->regions[0] = (struct vhost_memory_region) {
-		.guest_phys_addr	= 0,
-		.memory_size		= kvm->ram_size,
-		.userspace_addr		= (unsigned long)kvm->ram_start,
-	};
-
-	r = ioctl(sdev->vhost_fd, VHOST_SET_OWNER);
-	if (r != 0)
-		die_perror("VHOST_SET_OWNER failed");
+	virtio_vhost_init(kvm, sdev->vhost_fd);
 
 	r = ioctl(sdev->vhost_fd, VHOST_GET_FEATURES, &features);
 	if (r != 0)
@@ -231,13 +217,8 @@ static void virtio_scsi_vhost_init(struct kvm *kvm, struct scsi_dev *sdev)
 	r = ioctl(sdev->vhost_fd, VHOST_SET_FEATURES, &features);
 	if (r != 0)
 		die_perror("VHOST_SET_FEATURES failed");
-	r = ioctl(sdev->vhost_fd, VHOST_SET_MEM_TABLE, mem);
-	if (r != 0)
-		die_perror("VHOST_SET_MEM_TABLE failed");
 
 	sdev->vdev.use_vhost = true;
-
-	free(mem);
 }
 
 
diff --git a/virtio/vhost.c b/virtio/vhost.c
new file mode 100644
index 00000000..f9f72f51
--- /dev/null
+++ b/virtio/vhost.c
@@ -0,0 +1,36 @@
+#include <linux/kvm.h>
+#include <linux/vhost.h>
+#include <linux/list.h>
+#include "kvm/virtio.h"
+
+void virtio_vhost_init(struct kvm *kvm, int vhost_fd)
+{
+	struct kvm_mem_bank *bank;
+	struct vhost_memory *mem;
+	int i = 0, r;
+
+	mem = calloc(1, sizeof(*mem) +
+		     kvm->mem_slots * sizeof(struct vhost_memory_region));
+	if (mem == NULL)
+		die("Failed allocating memory for vhost memory map");
+
+	list_for_each_entry(bank, &kvm->mem_banks, list) {
+		mem->regions[i] = (struct vhost_memory_region) {
+			.guest_phys_addr = bank->guest_phys_addr,
+			.memory_size	 = bank->size,
+			.userspace_addr	 = (unsigned long)bank->host_addr,
+		};
+		i++;
+	}
+	mem->nregions = i;
+
+	r = ioctl(vhost_fd, VHOST_SET_OWNER);
+	if (r != 0)
+		die_perror("VHOST_SET_OWNER failed");
+
+	r = ioctl(vhost_fd, VHOST_SET_MEM_TABLE, mem);
+	if (r != 0)
+		die_perror("VHOST_SET_MEM_TABLE failed");
+
+	free(mem);
+}
diff --git a/virtio/vsock.c b/virtio/vsock.c
index a108e637..4b8be8d7 100644
--- a/virtio/vsock.c
+++ b/virtio/vsock.c
@@ -218,37 +218,14 @@ static struct virtio_ops vsock_dev_virtio_ops = {
 
 static void virtio_vhost_vsock_init(struct kvm *kvm, struct vsock_dev *vdev)
 {
-	struct kvm_mem_bank *bank;
-	struct vhost_memory *mem;
 	u64 features;
-	int r, i;
+	int r;
 
 	vdev->vhost_fd = open("/dev/vhost-vsock", O_RDWR);
 	if (vdev->vhost_fd < 0)
 		die_perror("Failed opening vhost-vsock device");
 
-	mem = calloc(1, sizeof(*mem) + sizeof(struct vhost_memory_region));
-	if (mem == NULL)
-		die("Failed allocating memory for vhost memory map");
-
-	i = 0;
-	list_for_each_entry(bank, &kvm->mem_banks, list) {
-		mem->regions[i] = (struct vhost_memory_region) {
-			.guest_phys_addr = bank->guest_phys_addr,
-			.memory_size	 = bank->size,
-			.userspace_addr	 = (unsigned long)bank->host_addr,
-		};
-		i++;
-	}
-	mem->nregions = i;
-
-	r = ioctl(vdev->vhost_fd, VHOST_SET_OWNER);
-	if (r != 0)
-		die_perror("VHOST_SET_OWNER failed");
-
-	r = ioctl(vdev->vhost_fd, VHOST_SET_MEM_TABLE, mem);
-	if (r != 0)
-		die_perror("VHOST_SET_MEM_TABLE failed");
+	virtio_vhost_init(kvm, vdev->vhost_fd);
 
 	r = ioctl(vdev->vhost_fd, VHOST_GET_FEATURES, &features);
 	if (r != 0)
@@ -263,8 +240,6 @@ static void virtio_vhost_vsock_init(struct kvm *kvm, struct vsock_dev *vdev)
 		die_perror("VHOST_VSOCK_SET_GUEST_CID failed");
 
 	vdev->vdev.use_vhost = true;
-
-	free(mem);
 }
 
 static int virtio_vsock_init_one(struct kvm *kvm, u64 guest_cid)
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH kvmtool v2 02/17] virtio/vhost: Factor vring operation
  2023-06-06 13:04 [PATCH kvmtool v2 00/17] Fix vhost-net, scsi and vsock Jean-Philippe Brucker
  2023-06-06 13:04 ` [PATCH kvmtool v2 01/17] virtio: Factor vhost initialization Jean-Philippe Brucker
@ 2023-06-06 13:04 ` Jean-Philippe Brucker
  2023-06-06 13:04 ` [PATCH kvmtool v2 03/17] virtio/vhost: Factor notify_vq_eventfd() Jean-Philippe Brucker
                   ` (15 subsequent siblings)
  17 siblings, 0 replies; 19+ messages in thread
From: Jean-Philippe Brucker @ 2023-06-06 13:04 UTC (permalink / raw)
  To: kvm, will; +Cc: andre.przywara, Jean-Philippe Brucker

The VHOST_VRING* ioctls are common to all device types, move them to
virtio/vhost.c

Reviewed-by: Andre Przywara <andre.przywara@arm.com>
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 include/kvm/virtio.h |  2 ++
 virtio/net.c         | 25 +------------------------
 virtio/scsi.c        | 24 +-----------------------
 virtio/vhost.c       | 33 ++++++++++++++++++++++++++++++++-
 virtio/vsock.c       | 30 ++----------------------------
 5 files changed, 38 insertions(+), 76 deletions(-)

diff --git a/include/kvm/virtio.h b/include/kvm/virtio.h
index 4cc2e3d2..daea8554 100644
--- a/include/kvm/virtio.h
+++ b/include/kvm/virtio.h
@@ -259,6 +259,8 @@ void virtio_set_guest_features(struct kvm *kvm, struct virtio_device *vdev,
 void virtio_notify_status(struct kvm *kvm, struct virtio_device *vdev,
 			  void *dev, u8 status);
 void virtio_vhost_init(struct kvm *kvm, int vhost_fd);
+void virtio_vhost_set_vring(struct kvm *kvm, int vhost_fd, u32 index,
+			    struct virt_queue *queue);
 
 int virtio_transport_parser(const struct option *opt, const char *arg, int unset);
 
diff --git a/virtio/net.c b/virtio/net.c
index 65fdbd17..b7c64a08 100644
--- a/virtio/net.c
+++ b/virtio/net.c
@@ -600,10 +600,8 @@ static bool is_ctrl_vq(struct net_dev *ndev, u32 vq)
 
 static int init_vq(struct kvm *kvm, void *dev, u32 vq)
 {
-	struct vhost_vring_state state = { .index = vq };
 	struct vhost_vring_file file = { .index = vq };
 	struct net_dev_queue *net_queue;
-	struct vhost_vring_addr addr;
 	struct net_dev *ndev = dev;
 	struct virt_queue *queue;
 	int r;
@@ -634,28 +632,7 @@ static int init_vq(struct kvm *kvm, void *dev, u32 vq)
 		return 0;
 	}
 
-	if (queue->endian != VIRTIO_ENDIAN_HOST)
-		die_perror("VHOST requires the same endianness in guest and host");
-
-	state.num = queue->vring.num;
-	r = ioctl(ndev->vhost_fd, VHOST_SET_VRING_NUM, &state);
-	if (r < 0)
-		die_perror("VHOST_SET_VRING_NUM failed");
-	state.num = 0;
-	r = ioctl(ndev->vhost_fd, VHOST_SET_VRING_BASE, &state);
-	if (r < 0)
-		die_perror("VHOST_SET_VRING_BASE failed");
-
-	addr = (struct vhost_vring_addr) {
-		.index = vq,
-		.desc_user_addr = (u64)(unsigned long)queue->vring.desc,
-		.avail_user_addr = (u64)(unsigned long)queue->vring.avail,
-		.used_user_addr = (u64)(unsigned long)queue->vring.used,
-	};
-
-	r = ioctl(ndev->vhost_fd, VHOST_SET_VRING_ADDR, &addr);
-	if (r < 0)
-		die_perror("VHOST_SET_VRING_ADDR failed");
+	virtio_vhost_set_vring(kvm, ndev->vhost_fd, vq, queue);
 
 	file.fd = ndev->tap_fd;
 	r = ioctl(ndev->vhost_fd, VHOST_NET_SET_BACKEND, &file);
diff --git a/virtio/scsi.c b/virtio/scsi.c
index 621a8334..6aa0909f 100644
--- a/virtio/scsi.c
+++ b/virtio/scsi.c
@@ -72,11 +72,8 @@ static void notify_status(struct kvm *kvm, void *dev, u32 status)
 
 static int init_vq(struct kvm *kvm, void *dev, u32 vq)
 {
-	struct vhost_vring_state state = { .index = vq };
-	struct vhost_vring_addr addr;
 	struct scsi_dev *sdev = dev;
 	struct virt_queue *queue;
-	int r;
 
 	compat__remove_message(compat_id);
 
@@ -87,26 +84,7 @@ static int init_vq(struct kvm *kvm, void *dev, u32 vq)
 	if (sdev->vhost_fd == 0)
 		return 0;
 
-	state.num = queue->vring.num;
-	r = ioctl(sdev->vhost_fd, VHOST_SET_VRING_NUM, &state);
-	if (r < 0)
-		die_perror("VHOST_SET_VRING_NUM failed");
-	state.num = 0;
-	r = ioctl(sdev->vhost_fd, VHOST_SET_VRING_BASE, &state);
-	if (r < 0)
-		die_perror("VHOST_SET_VRING_BASE failed");
-
-	addr = (struct vhost_vring_addr) {
-		.index = vq,
-		.desc_user_addr = (u64)(unsigned long)queue->vring.desc,
-		.avail_user_addr = (u64)(unsigned long)queue->vring.avail,
-		.used_user_addr = (u64)(unsigned long)queue->vring.used,
-	};
-
-	r = ioctl(sdev->vhost_fd, VHOST_SET_VRING_ADDR, &addr);
-	if (r < 0)
-		die_perror("VHOST_SET_VRING_ADDR failed");
-
+	virtio_vhost_set_vring(kvm, sdev->vhost_fd, vq, queue);
 	return 0;
 }
 
diff --git a/virtio/vhost.c b/virtio/vhost.c
index f9f72f51..afe37465 100644
--- a/virtio/vhost.c
+++ b/virtio/vhost.c
@@ -1,7 +1,8 @@
+#include "kvm/virtio.h"
+
 #include <linux/kvm.h>
 #include <linux/vhost.h>
 #include <linux/list.h>
-#include "kvm/virtio.h"
 
 void virtio_vhost_init(struct kvm *kvm, int vhost_fd)
 {
@@ -34,3 +35,33 @@ void virtio_vhost_init(struct kvm *kvm, int vhost_fd)
 
 	free(mem);
 }
+
+void virtio_vhost_set_vring(struct kvm *kvm, int vhost_fd, u32 index,
+			    struct virt_queue *queue)
+{
+	int r;
+	struct vhost_vring_addr addr = {
+		.index = index,
+		.desc_user_addr = (u64)(unsigned long)queue->vring.desc,
+		.avail_user_addr = (u64)(unsigned long)queue->vring.avail,
+		.used_user_addr = (u64)(unsigned long)queue->vring.used,
+	};
+	struct vhost_vring_state state = { .index = index };
+
+	if (queue->endian != VIRTIO_ENDIAN_HOST)
+		die("VHOST requires the same endianness in guest and host");
+
+	state.num = queue->vring.num;
+	r = ioctl(vhost_fd, VHOST_SET_VRING_NUM, &state);
+	if (r < 0)
+		die_perror("VHOST_SET_VRING_NUM failed");
+
+	state.num = 0;
+	r = ioctl(vhost_fd, VHOST_SET_VRING_BASE, &state);
+	if (r < 0)
+		die_perror("VHOST_SET_VRING_BASE failed");
+
+	r = ioctl(vhost_fd, VHOST_SET_VRING_ADDR, &addr);
+	if (r < 0)
+		die_perror("VHOST_SET_VRING_ADDR failed");
+}
diff --git a/virtio/vsock.c b/virtio/vsock.c
index 4b8be8d7..2f7906f2 100644
--- a/virtio/vsock.c
+++ b/virtio/vsock.c
@@ -62,44 +62,18 @@ static bool is_event_vq(u32 vq)
 
 static int init_vq(struct kvm *kvm, void *dev, u32 vq)
 {
-	struct vhost_vring_state state = { .index = vq };
-	struct vhost_vring_addr addr;
 	struct vsock_dev *vdev = dev;
 	struct virt_queue *queue;
-	int r;
 
 	compat__remove_message(compat_id);
 
 	queue		= &vdev->vqs[vq];
 	virtio_init_device_vq(kvm, &vdev->vdev, queue, VIRTIO_VSOCK_QUEUE_SIZE);
 
-	if (vdev->vhost_fd == -1)
+	if (vdev->vhost_fd == -1 || is_event_vq(vq))
 		return 0;
 
-	if (is_event_vq(vq))
-		return 0;
-
-	state.num = queue->vring.num;
-	r = ioctl(vdev->vhost_fd, VHOST_SET_VRING_NUM, &state);
-	if (r < 0)
-		die_perror("VHOST_SET_VRING_NUM failed");
-
-	state.num = 0;
-	r = ioctl(vdev->vhost_fd, VHOST_SET_VRING_BASE, &state);
-	if (r < 0)
-		die_perror("VHOST_SET_VRING_BASE failed");
-
-	addr = (struct vhost_vring_addr) {
-		.index = vq,
-		.desc_user_addr = (u64)(unsigned long)queue->vring.desc,
-		.avail_user_addr = (u64)(unsigned long)queue->vring.avail,
-		.used_user_addr = (u64)(unsigned long)queue->vring.used,
-	};
-
-	r = ioctl(vdev->vhost_fd, VHOST_SET_VRING_ADDR, &addr);
-	if (r < 0)
-		die_perror("VHOST_SET_VRING_ADDR failed");
-
+	virtio_vhost_set_vring(kvm, vdev->vhost_fd, vq, queue);
 	return 0;
 }
 
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH kvmtool v2 03/17] virtio/vhost: Factor notify_vq_eventfd()
  2023-06-06 13:04 [PATCH kvmtool v2 00/17] Fix vhost-net, scsi and vsock Jean-Philippe Brucker
  2023-06-06 13:04 ` [PATCH kvmtool v2 01/17] virtio: Factor vhost initialization Jean-Philippe Brucker
  2023-06-06 13:04 ` [PATCH kvmtool v2 02/17] virtio/vhost: Factor vring operation Jean-Philippe Brucker
@ 2023-06-06 13:04 ` Jean-Philippe Brucker
  2023-06-06 13:04 ` [PATCH kvmtool v2 04/17] virtio/vhost: Factor notify_vq_gsi() Jean-Philippe Brucker
                   ` (14 subsequent siblings)
  17 siblings, 0 replies; 19+ messages in thread
From: Jean-Philippe Brucker @ 2023-06-06 13:04 UTC (permalink / raw)
  To: kvm, will; +Cc: andre.przywara, Jean-Philippe Brucker

All vhost devices perform the same operation when setting up the
ioeventfd. Move it to virtio/vhost.c

Reviewed-by: Andre Przywara <andre.przywara@arm.com>
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 include/kvm/virtio.h |  2 ++
 virtio/net.c         |  9 +--------
 virtio/scsi.c        |  9 +--------
 virtio/vhost.c       | 14 ++++++++++++++
 virtio/vsock.c       | 14 ++------------
 5 files changed, 20 insertions(+), 28 deletions(-)

diff --git a/include/kvm/virtio.h b/include/kvm/virtio.h
index daea8554..6912875e 100644
--- a/include/kvm/virtio.h
+++ b/include/kvm/virtio.h
@@ -261,6 +261,8 @@ void virtio_notify_status(struct kvm *kvm, struct virtio_device *vdev,
 void virtio_vhost_init(struct kvm *kvm, int vhost_fd);
 void virtio_vhost_set_vring(struct kvm *kvm, int vhost_fd, u32 index,
 			    struct virt_queue *queue);
+void virtio_vhost_set_vring_kick(struct kvm *kvm, int vhost_fd,
+				 u32 index, int event_fd);
 
 int virtio_transport_parser(const struct option *opt, const char *arg, int unset);
 
diff --git a/virtio/net.c b/virtio/net.c
index b7c64a08..c0871163 100644
--- a/virtio/net.c
+++ b/virtio/net.c
@@ -701,18 +701,11 @@ static void notify_vq_gsi(struct kvm *kvm, void *dev, u32 vq, u32 gsi)
 static void notify_vq_eventfd(struct kvm *kvm, void *dev, u32 vq, u32 efd)
 {
 	struct net_dev *ndev = dev;
-	struct vhost_vring_file file = {
-		.index	= vq,
-		.fd	= efd,
-	};
-	int r;
 
 	if (ndev->vhost_fd == 0 || is_ctrl_vq(ndev, vq))
 		return;
 
-	r = ioctl(ndev->vhost_fd, VHOST_SET_VRING_KICK, &file);
-	if (r < 0)
-		die_perror("VHOST_SET_VRING_KICK failed");
+	virtio_vhost_set_vring_kick(kvm, ndev->vhost_fd, vq, efd);
 }
 
 static int notify_vq(struct kvm *kvm, void *dev, u32 vq)
diff --git a/virtio/scsi.c b/virtio/scsi.c
index 6aa0909f..ebddec36 100644
--- a/virtio/scsi.c
+++ b/virtio/scsi.c
@@ -121,18 +121,11 @@ static void notify_vq_gsi(struct kvm *kvm, void *dev, u32 vq, u32 gsi)
 static void notify_vq_eventfd(struct kvm *kvm, void *dev, u32 vq, u32 efd)
 {
 	struct scsi_dev *sdev = dev;
-	struct vhost_vring_file file = {
-		.index	= vq,
-		.fd	= efd,
-	};
-	int r;
 
 	if (sdev->vhost_fd == 0)
 		return;
 
-	r = ioctl(sdev->vhost_fd, VHOST_SET_VRING_KICK, &file);
-	if (r < 0)
-		die_perror("VHOST_SET_VRING_KICK failed");
+	virtio_vhost_set_vring_kick(kvm, sdev->vhost_fd, vq, efd);
 }
 
 static int notify_vq(struct kvm *kvm, void *dev, u32 vq)
diff --git a/virtio/vhost.c b/virtio/vhost.c
index afe37465..3acfd30a 100644
--- a/virtio/vhost.c
+++ b/virtio/vhost.c
@@ -65,3 +65,17 @@ void virtio_vhost_set_vring(struct kvm *kvm, int vhost_fd, u32 index,
 	if (r < 0)
 		die_perror("VHOST_SET_VRING_ADDR failed");
 }
+
+void virtio_vhost_set_vring_kick(struct kvm *kvm, int vhost_fd,
+				 u32 index, int event_fd)
+{
+	int r;
+	struct vhost_vring_file file = {
+		.index	= index,
+		.fd	= event_fd,
+	};
+
+	r = ioctl(vhost_fd, VHOST_SET_VRING_KICK, &file);
+	if (r < 0)
+		die_perror("VHOST_SET_VRING_KICK failed");
+}
diff --git a/virtio/vsock.c b/virtio/vsock.c
index 2f7906f2..0ada9e09 100644
--- a/virtio/vsock.c
+++ b/virtio/vsock.c
@@ -80,21 +80,11 @@ static int init_vq(struct kvm *kvm, void *dev, u32 vq)
 static void notify_vq_eventfd(struct kvm *kvm, void *dev, u32 vq, u32 efd)
 {
 	struct vsock_dev *vdev = dev;
-	struct vhost_vring_file file = {
-		.index	= vq,
-		.fd	= efd,
-	};
-	int r;
 
-	if (is_event_vq(vq))
+	if (vdev->vhost_fd == -1 || is_event_vq(vq))
 		return;
 
-	if (vdev->vhost_fd == -1)
-		return;
-
-	r = ioctl(vdev->vhost_fd, VHOST_SET_VRING_KICK, &file);
-	if (r < 0)
-		die_perror("VHOST_SET_VRING_KICK failed");
+	virtio_vhost_set_vring_kick(kvm, vdev->vhost_fd, vq, efd);
 }
 
 static void notify_status(struct kvm *kvm, void *dev, u32 status)
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH kvmtool v2 04/17] virtio/vhost: Factor notify_vq_gsi()
  2023-06-06 13:04 [PATCH kvmtool v2 00/17] Fix vhost-net, scsi and vsock Jean-Philippe Brucker
                   ` (2 preceding siblings ...)
  2023-06-06 13:04 ` [PATCH kvmtool v2 03/17] virtio/vhost: Factor notify_vq_eventfd() Jean-Philippe Brucker
@ 2023-06-06 13:04 ` Jean-Philippe Brucker
  2023-06-06 13:04 ` [PATCH kvmtool v2 05/17] virtio/scsi: Move VHOST_SCSI_SET_ENDPOINT to device start Jean-Philippe Brucker
                   ` (13 subsequent siblings)
  17 siblings, 0 replies; 19+ messages in thread
From: Jean-Philippe Brucker @ 2023-06-06 13:04 UTC (permalink / raw)
  To: kvm, will; +Cc: andre.przywara, Jean-Philippe Brucker

All vhost devices should perform the same operations when initializing
the IRQFD. Move it to virtio/vhost.c

This fixes vsock, which didn't go through the irq__add_irqfd() helper
and couldn't be used on systems that require GSI translation (GICv2m).

Also correct notify_vq_gsi() in net.c, to check which virtqueue is being
configured. Since vhost only manages the data queues, we shouldn't try
to setup GSI routing for the control queue. This hasn't been a problem
so far because the Linux guest doesn't use IRQs for the control queue.

Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 include/kvm/virtio.h |  8 ++++++++
 virtio/net.c         | 32 +++++---------------------------
 virtio/scsi.c        | 15 ++-------------
 virtio/vhost.c       | 35 +++++++++++++++++++++++++++++++++++
 virtio/vsock.c       | 26 +++-----------------------
 5 files changed, 53 insertions(+), 63 deletions(-)

diff --git a/include/kvm/virtio.h b/include/kvm/virtio.h
index 6912875e..a2f8355a 100644
--- a/include/kvm/virtio.h
+++ b/include/kvm/virtio.h
@@ -77,6 +77,10 @@ struct virt_queue {
 	u16		endian;
 	bool		use_event_idx;
 	bool		enabled;
+
+	/* vhost IRQ handling */
+	int		gsi;
+	int		irqfd;
 };
 
 /*
@@ -263,6 +267,10 @@ void virtio_vhost_set_vring(struct kvm *kvm, int vhost_fd, u32 index,
 			    struct virt_queue *queue);
 void virtio_vhost_set_vring_kick(struct kvm *kvm, int vhost_fd,
 				 u32 index, int event_fd);
+void virtio_vhost_set_vring_call(struct kvm *kvm, int vhost_fd, u32 index,
+				 u32 gsi, struct virt_queue *queue);
+void virtio_vhost_reset_vring(struct kvm *kvm, int vhost_fd, u32 index,
+			      struct virt_queue *queue);
 
 int virtio_transport_parser(const struct option *opt, const char *arg, int unset);
 
diff --git a/virtio/net.c b/virtio/net.c
index c0871163..3e1aedf7 100644
--- a/virtio/net.c
+++ b/virtio/net.c
@@ -4,12 +4,12 @@
 #include "kvm/mutex.h"
 #include "kvm/util.h"
 #include "kvm/kvm.h"
-#include "kvm/irq.h"
 #include "kvm/uip.h"
 #include "kvm/guest_compat.h"
 #include "kvm/iovec.h"
 #include "kvm/strbuf.h"
 
+#include <linux/list.h>
 #include <linux/vhost.h>
 #include <linux/virtio_net.h>
 #include <linux/if_tun.h>
@@ -25,7 +25,6 @@
 #include <sys/ioctl.h>
 #include <sys/types.h>
 #include <sys/wait.h>
-#include <sys/eventfd.h>
 
 #define VIRTIO_NET_QUEUE_SIZE		256
 #define VIRTIO_NET_NUM_QUEUES		8
@@ -44,8 +43,6 @@ struct net_dev_queue {
 	pthread_t			thread;
 	struct mutex			lock;
 	pthread_cond_t			cond;
-	int				gsi;
-	int				irqfd;
 };
 
 struct net_dev {
@@ -647,11 +644,7 @@ static void exit_vq(struct kvm *kvm, void *dev, u32 vq)
 	struct net_dev *ndev = dev;
 	struct net_dev_queue *queue = &ndev->queues[vq];
 
-	if (!is_ctrl_vq(ndev, vq) && queue->gsi) {
-		irq__del_irqfd(kvm, queue->gsi, queue->irqfd);
-		close(queue->irqfd);
-		queue->gsi = queue->irqfd = 0;
-	}
+	virtio_vhost_reset_vring(kvm, ndev->vhost_fd, vq, &queue->vq);
 
 	/*
 	 * TODO: vhost reset owner. It's the only way to cleanly stop vhost, but
@@ -675,27 +668,12 @@ static void notify_vq_gsi(struct kvm *kvm, void *dev, u32 vq, u32 gsi)
 {
 	struct net_dev *ndev = dev;
 	struct net_dev_queue *queue = &ndev->queues[vq];
-	struct vhost_vring_file file;
-	int r;
 
-	if (ndev->vhost_fd == 0)
+	if (ndev->vhost_fd == 0 || is_ctrl_vq(ndev, vq))
 		return;
 
-	file = (struct vhost_vring_file) {
-		.index	= vq,
-		.fd	= eventfd(0, 0),
-	};
-
-	r = irq__add_irqfd(kvm, gsi, file.fd, -1);
-	if (r < 0)
-		die_perror("KVM_IRQFD failed");
-
-	queue->irqfd = file.fd;
-	queue->gsi = gsi;
-
-	r = ioctl(ndev->vhost_fd, VHOST_SET_VRING_CALL, &file);
-	if (r < 0)
-		die_perror("VHOST_SET_VRING_CALL failed");
+	virtio_vhost_set_vring_call(kvm, ndev->vhost_fd, vq, gsi,
+				    &queue->vq);
 }
 
 static void notify_vq_eventfd(struct kvm *kvm, void *dev, u32 vq, u32 efd)
diff --git a/virtio/scsi.c b/virtio/scsi.c
index ebddec36..708fb23a 100644
--- a/virtio/scsi.c
+++ b/virtio/scsi.c
@@ -90,25 +90,14 @@ static int init_vq(struct kvm *kvm, void *dev, u32 vq)
 
 static void notify_vq_gsi(struct kvm *kvm, void *dev, u32 vq, u32 gsi)
 {
-	struct vhost_vring_file file;
 	struct scsi_dev *sdev = dev;
 	int r;
 
 	if (sdev->vhost_fd == 0)
 		return;
 
-	file = (struct vhost_vring_file) {
-		.index	= vq,
-		.fd	= eventfd(0, 0),
-	};
-
-	r = irq__add_irqfd(kvm, gsi, file.fd, -1);
-	if (r < 0)
-		die_perror("KVM_IRQFD failed");
-
-	r = ioctl(sdev->vhost_fd, VHOST_SET_VRING_CALL, &file);
-	if (r < 0)
-		die_perror("VHOST_SET_VRING_CALL failed");
+	virtio_vhost_set_vring_call(kvm, sdev->vhost_fd, vq, gsi,
+				    &sdev->vqs[vq]);
 
 	if (vq > 0)
 		return;
diff --git a/virtio/vhost.c b/virtio/vhost.c
index 3acfd30a..cd83645c 100644
--- a/virtio/vhost.c
+++ b/virtio/vhost.c
@@ -1,9 +1,12 @@
+#include "kvm/irq.h"
 #include "kvm/virtio.h"
 
 #include <linux/kvm.h>
 #include <linux/vhost.h>
 #include <linux/list.h>
 
+#include <sys/eventfd.h>
+
 void virtio_vhost_init(struct kvm *kvm, int vhost_fd)
 {
 	struct kvm_mem_bank *bank;
@@ -79,3 +82,35 @@ void virtio_vhost_set_vring_kick(struct kvm *kvm, int vhost_fd,
 	if (r < 0)
 		die_perror("VHOST_SET_VRING_KICK failed");
 }
+
+void virtio_vhost_set_vring_call(struct kvm *kvm, int vhost_fd, u32 index,
+				 u32 gsi, struct virt_queue *queue)
+{
+	int r;
+	struct vhost_vring_file file = {
+		.index	= index,
+		.fd	= eventfd(0, 0),
+	};
+
+	r = irq__add_irqfd(kvm, gsi, file.fd, -1);
+	if (r < 0)
+		die_perror("KVM_IRQFD failed");
+
+	r = ioctl(vhost_fd, VHOST_SET_VRING_CALL, &file);
+	if (r < 0)
+		die_perror("VHOST_SET_VRING_CALL failed");
+
+	queue->irqfd = file.fd;
+	queue->gsi = gsi;
+}
+
+void virtio_vhost_reset_vring(struct kvm *kvm, int vhost_fd, u32 index,
+			      struct virt_queue *queue)
+
+{
+	if (queue->gsi) {
+		irq__del_irqfd(kvm, queue->gsi, queue->irqfd);
+		close(queue->irqfd);
+		queue->gsi = queue->irqfd = 0;
+	}
+}
diff --git a/virtio/vsock.c b/virtio/vsock.c
index 0ada9e09..559fbaba 100644
--- a/virtio/vsock.c
+++ b/virtio/vsock.c
@@ -131,33 +131,13 @@ static int set_size_vq(struct kvm *kvm, void *dev, u32 vq, int size)
 
 static void notify_vq_gsi(struct kvm *kvm, void *dev, u32 vq, u32 gsi)
 {
-	struct vhost_vring_file file;
 	struct vsock_dev *vdev = dev;
-	struct kvm_irqfd irq;
-	int r;
 
-	if (vdev->vhost_fd == -1)
+	if (vdev->vhost_fd == -1 || is_event_vq(vq))
 		return;
 
-	if (is_event_vq(vq))
-		return;
-
-	irq = (struct kvm_irqfd) {
-		.gsi	= gsi,
-		.fd	= eventfd(0, 0),
-	};
-	file = (struct vhost_vring_file) {
-		.index	= vq,
-		.fd	= irq.fd,
-	};
-
-	r = ioctl(kvm->vm_fd, KVM_IRQFD, &irq);
-	if (r < 0)
-		die_perror("KVM_IRQFD failed");
-
-	r = ioctl(vdev->vhost_fd, VHOST_SET_VRING_CALL, &file);
-	if (r < 0)
-		die_perror("VHOST_SET_VRING_CALL failed");
+	virtio_vhost_set_vring_call(kvm, vdev->vhost_fd, vq, gsi,
+				    &vdev->vqs[vq]);
 }
 
 static unsigned int get_vq_count(struct kvm *kvm, void *dev)
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH kvmtool v2 05/17] virtio/scsi: Move VHOST_SCSI_SET_ENDPOINT to device start
  2023-06-06 13:04 [PATCH kvmtool v2 00/17] Fix vhost-net, scsi and vsock Jean-Philippe Brucker
                   ` (3 preceding siblings ...)
  2023-06-06 13:04 ` [PATCH kvmtool v2 04/17] virtio/vhost: Factor notify_vq_gsi() Jean-Philippe Brucker
@ 2023-06-06 13:04 ` Jean-Philippe Brucker
  2023-06-06 13:04 ` [PATCH kvmtool v2 06/17] virtio/scsi: Fix and simplify command-line Jean-Philippe Brucker
                   ` (12 subsequent siblings)
  17 siblings, 0 replies; 19+ messages in thread
From: Jean-Philippe Brucker @ 2023-06-06 13:04 UTC (permalink / raw)
  To: kvm, will; +Cc: andre.przywara, Jean-Philippe Brucker

The vhost driver expects virtqueues to be operational by the time we
call SET_ENDPOINT. We currently do it too early. Device start, which
happens when the driver writes the DRIVER_OK status, is a good time to
do this.

Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 virtio/scsi.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/virtio/scsi.c b/virtio/scsi.c
index 708fb23a..8249a9cd 100644
--- a/virtio/scsi.c
+++ b/virtio/scsi.c
@@ -57,6 +57,13 @@ static void notify_status(struct kvm *kvm, void *dev, u32 status)
 	struct virtio_scsi_config *conf = &sdev->config;
 	u16 endian = vdev->endian;
 
+	if (status & VIRTIO__STATUS_START) {
+		int r = ioctl(sdev->vhost_fd, VHOST_SCSI_SET_ENDPOINT,
+			      &sdev->target);
+		if (r != 0)
+			die("VHOST_SCSI_SET_ENDPOINT failed %d", errno);
+	}
+
 	if (!(status & VIRTIO__STATUS_CONFIG))
 		return;
 
@@ -91,20 +98,12 @@ static int init_vq(struct kvm *kvm, void *dev, u32 vq)
 static void notify_vq_gsi(struct kvm *kvm, void *dev, u32 vq, u32 gsi)
 {
 	struct scsi_dev *sdev = dev;
-	int r;
 
 	if (sdev->vhost_fd == 0)
 		return;
 
 	virtio_vhost_set_vring_call(kvm, sdev->vhost_fd, vq, gsi,
 				    &sdev->vqs[vq]);
-
-	if (vq > 0)
-		return;
-
-	r = ioctl(sdev->vhost_fd, VHOST_SCSI_SET_ENDPOINT, &sdev->target);
-	if (r != 0)
-		die("VHOST_SCSI_SET_ENDPOINT failed %d", errno);
 }
 
 static void notify_vq_eventfd(struct kvm *kvm, void *dev, u32 vq, u32 efd)
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH kvmtool v2 06/17] virtio/scsi: Fix and simplify command-line
  2023-06-06 13:04 [PATCH kvmtool v2 00/17] Fix vhost-net, scsi and vsock Jean-Philippe Brucker
                   ` (4 preceding siblings ...)
  2023-06-06 13:04 ` [PATCH kvmtool v2 05/17] virtio/scsi: Move VHOST_SCSI_SET_ENDPOINT to device start Jean-Philippe Brucker
@ 2023-06-06 13:04 ` Jean-Philippe Brucker
  2023-06-06 13:04 ` [PATCH kvmtool v2 07/17] disk/core: Fix segfault on exit with SCSI Jean-Philippe Brucker
                   ` (11 subsequent siblings)
  17 siblings, 0 replies; 19+ messages in thread
From: Jean-Philippe Brucker @ 2023-06-06 13:04 UTC (permalink / raw)
  To: kvm, will; +Cc: andre.przywara, Jean-Philippe Brucker

Fix and simplify the command-line parameter for virtio-scsi. Currently
passing a "scsi:xxxx" parameter without the second "tpgt" argument
causes kvmtool to segfault. But only the "wwpn" parameter is necessary.

The tpgt parameter is ignored and was never used upstream. See
linux/vhost_types.h:

 * ABI Rev 0: July 2012 version starting point for v3.6-rc merge candidate +
 *            RFC-v2 vhost-scsi userspace.  Add GET_ABI_VERSION ioctl usage
 * ABI Rev 1: January 2013. Ignore vhost_tpgt field in struct vhost_scsi_target.
 *            All the targets under vhost_wwpn can be seen and used by guset.

Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 include/kvm/disk-image.h |  7 +------
 disk/core.c              | 11 ++++-------
 virtio/scsi.c            |  2 +-
 3 files changed, 6 insertions(+), 14 deletions(-)

diff --git a/include/kvm/disk-image.h b/include/kvm/disk-image.h
index b2123838..bf602b58 100644
--- a/include/kvm/disk-image.h
+++ b/include/kvm/disk-image.h
@@ -48,12 +48,8 @@ struct disk_image_operations {
 
 struct disk_image_params {
 	const char *filename;
-	/*
-	 * wwpn == World Wide Port Number
-	 * tpgt == Target Portal Group Tag
-	 */
+	/* wwpn == World Wide Port Number */
 	const char *wwpn;
-	const char *tpgt;
 	bool readonly;
 	bool direct;
 };
@@ -74,7 +70,6 @@ struct disk_image {
 	u64				aio_inflight;
 #endif /* CONFIG_HAS_AIO */
 	const char			*wwpn;
-	const char			*tpgt;
 	int				debug_iodelay;
 };
 
diff --git a/disk/core.c b/disk/core.c
index f69095d9..652fcd1a 100644
--- a/disk/core.c
+++ b/disk/core.c
@@ -25,14 +25,14 @@ int disk_img_name_parser(const struct option *opt, const char *arg, int unset)
 
 	if (strncmp(arg, "scsi:", 5) == 0) {
 		sep = strstr(arg, ":");
-		if (sep)
-			kvm->cfg.disk_image[kvm->nr_disks].wwpn = sep + 1;
+		kvm->cfg.disk_image[kvm->nr_disks].wwpn = sep + 1;
+
+		/* Old invocation had two parameters. Ignore the second one. */
 		sep = strstr(sep + 1, ":");
 		if (sep) {
 			*sep = 0;
-			kvm->cfg.disk_image[kvm->nr_disks].tpgt = sep + 1;
+			cur = sep + 1;
 		}
-		cur = sep + 1;
 	}
 
 	do {
@@ -147,7 +147,6 @@ static struct disk_image **disk_image__open_all(struct kvm *kvm)
 	struct disk_image **disks;
 	const char *filename;
 	const char *wwpn;
-	const char *tpgt;
 	bool readonly;
 	bool direct;
 	void *err;
@@ -169,14 +168,12 @@ static struct disk_image **disk_image__open_all(struct kvm *kvm)
 		readonly = params[i].readonly;
 		direct = params[i].direct;
 		wwpn = params[i].wwpn;
-		tpgt = params[i].tpgt;
 
 		if (wwpn) {
 			disks[i] = malloc(sizeof(struct disk_image));
 			if (!disks[i])
 				return ERR_PTR(-ENOMEM);
 			disks[i]->wwpn = wwpn;
-			disks[i]->tpgt = tpgt;
 			continue;
 		}
 
diff --git a/virtio/scsi.c b/virtio/scsi.c
index 8249a9cd..db4adc75 100644
--- a/virtio/scsi.c
+++ b/virtio/scsi.c
@@ -197,7 +197,7 @@ static int virtio_scsi_init_one(struct kvm *kvm, struct disk_image *disk)
 		.kvm			= kvm,
 	};
 	strlcpy((char *)&sdev->target.vhost_wwpn, disk->wwpn, sizeof(sdev->target.vhost_wwpn));
-	sdev->target.vhost_tpgt = strtol(disk->tpgt, NULL, 0);
+	sdev->target.abi_version = VHOST_SCSI_ABI_VERSION;
 
 	list_add_tail(&sdev->list, &sdevs);
 
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH kvmtool v2 07/17] disk/core: Fix segfault on exit with SCSI
  2023-06-06 13:04 [PATCH kvmtool v2 00/17] Fix vhost-net, scsi and vsock Jean-Philippe Brucker
                   ` (5 preceding siblings ...)
  2023-06-06 13:04 ` [PATCH kvmtool v2 06/17] virtio/scsi: Fix and simplify command-line Jean-Philippe Brucker
@ 2023-06-06 13:04 ` Jean-Philippe Brucker
  2023-06-06 13:04 ` [PATCH kvmtool v2 08/17] virtio/scsi: Initialize max_target Jean-Philippe Brucker
                   ` (10 subsequent siblings)
  17 siblings, 0 replies; 19+ messages in thread
From: Jean-Philippe Brucker @ 2023-06-06 13:04 UTC (permalink / raw)
  To: kvm, will; +Cc: andre.przywara, Jean-Philippe Brucker

The SCSI backend doesn't call disk_image__new() so the disk ops are
NULL. Check for this case on exit.

Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 disk/core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/disk/core.c b/disk/core.c
index 652fcd1a..b00b0c0a 100644
--- a/disk/core.c
+++ b/disk/core.c
@@ -223,10 +223,10 @@ static int disk_image__close(struct disk_image *disk)
 
 	disk_aio_destroy(disk);
 
-	if (disk->ops->close)
+	if (disk->ops && disk->ops->close)
 		return disk->ops->close(disk);
 
-	if (close(disk->fd) < 0)
+	if (disk->fd && close(disk->fd) < 0)
 		pr_warning("close() failed");
 
 	free(disk);
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH kvmtool v2 08/17] virtio/scsi: Initialize max_target
  2023-06-06 13:04 [PATCH kvmtool v2 00/17] Fix vhost-net, scsi and vsock Jean-Philippe Brucker
                   ` (6 preceding siblings ...)
  2023-06-06 13:04 ` [PATCH kvmtool v2 07/17] disk/core: Fix segfault on exit with SCSI Jean-Philippe Brucker
@ 2023-06-06 13:04 ` Jean-Philippe Brucker
  2023-06-06 13:04 ` [PATCH kvmtool v2 09/17] virtio/scsi: Fix feature selection Jean-Philippe Brucker
                   ` (9 subsequent siblings)
  17 siblings, 0 replies; 19+ messages in thread
From: Jean-Philippe Brucker @ 2023-06-06 13:04 UTC (permalink / raw)
  To: kvm, will; +Cc: andre.przywara, Jean-Philippe Brucker

The Linux guest does not find any target when 'max_target' is 0.
Initialize it to the maximum defined by virtio, "5.6.4 Device
configuration layout":

	max_target SHOULD be less than or equal to 255.

Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 virtio/scsi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/virtio/scsi.c b/virtio/scsi.c
index db4adc75..4d1ed9b8 100644
--- a/virtio/scsi.c
+++ b/virtio/scsi.c
@@ -73,6 +73,7 @@ static void notify_status(struct kvm *kvm, void *dev, u32 status)
 	conf->cmd_per_lun = virtio_host_to_guest_u32(endian, 128);
 	conf->sense_size = virtio_host_to_guest_u32(endian, VIRTIO_SCSI_SENSE_SIZE);
 	conf->cdb_size = virtio_host_to_guest_u32(endian, VIRTIO_SCSI_CDB_SIZE);
+	conf->max_target = virtio_host_to_guest_u16(endian, 255);
 	conf->max_lun = virtio_host_to_guest_u32(endian, 16383);
 	conf->event_info_size = virtio_host_to_guest_u32(endian, sizeof(struct virtio_scsi_event));
 }
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH kvmtool v2 09/17] virtio/scsi: Fix feature selection
  2023-06-06 13:04 [PATCH kvmtool v2 00/17] Fix vhost-net, scsi and vsock Jean-Philippe Brucker
                   ` (7 preceding siblings ...)
  2023-06-06 13:04 ` [PATCH kvmtool v2 08/17] virtio/scsi: Initialize max_target Jean-Philippe Brucker
@ 2023-06-06 13:04 ` Jean-Philippe Brucker
  2023-06-06 13:04 ` [PATCH kvmtool v2 10/17] virtio/vsock: " Jean-Philippe Brucker
                   ` (8 subsequent siblings)
  17 siblings, 0 replies; 19+ messages in thread
From: Jean-Philippe Brucker @ 2023-06-06 13:04 UTC (permalink / raw)
  To: kvm, will; +Cc: andre.przywara, Jean-Philippe Brucker

We should advertise to the guest only the features supported by vhost
and kvmtool. Then we should set in vhost only the features acked by the
guest. Move vhost feature query to get_host_features(), and vhost
feature setting to device start (after the guest has acked features).

This fixes scsi because we used to enable all vhost features including
VIRTIO_SCSI_F_T10_PI which changes the request layout and caused
inconsistency between guest and vhost.

Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 virtio/scsi.c | 33 +++++++++++++++++++--------------
 1 file changed, 19 insertions(+), 14 deletions(-)

diff --git a/virtio/scsi.c b/virtio/scsi.c
index 4d1ed9b8..50f184c7 100644
--- a/virtio/scsi.c
+++ b/virtio/scsi.c
@@ -46,19 +46,35 @@ static size_t get_config_size(struct kvm *kvm, void *dev)
 
 static u64 get_host_features(struct kvm *kvm, void *dev)
 {
-	return	1UL << VIRTIO_RING_F_EVENT_IDX |
-		1UL << VIRTIO_RING_F_INDIRECT_DESC;
+	int r;
+	u64 features;
+	struct scsi_dev *sdev = dev;
+
+	r = ioctl(sdev->vhost_fd, VHOST_GET_FEATURES, &features);
+	if (r != 0)
+		die_perror("VHOST_GET_FEATURES failed");
+
+	return features &
+		(1ULL << VIRTIO_RING_F_EVENT_IDX |	\
+		 1ULL << VIRTIO_RING_F_INDIRECT_DESC |	\
+		 1ULL << VIRTIO_F_ANY_LAYOUT);
 }
 
 static void notify_status(struct kvm *kvm, void *dev, u32 status)
 {
+	int r;
 	struct scsi_dev *sdev = dev;
 	struct virtio_device *vdev = &sdev->vdev;
 	struct virtio_scsi_config *conf = &sdev->config;
 	u16 endian = vdev->endian;
 
 	if (status & VIRTIO__STATUS_START) {
-		int r = ioctl(sdev->vhost_fd, VHOST_SCSI_SET_ENDPOINT,
+		r = ioctl(sdev->vhost_fd, VHOST_SET_FEATURES,
+			  &sdev->vdev.features);
+		if (r != 0)
+			die_perror("VHOST_SET_FEATURES failed");
+
+		r = ioctl(sdev->vhost_fd, VHOST_SCSI_SET_ENDPOINT,
 			      &sdev->target);
 		if (r != 0)
 			die("VHOST_SCSI_SET_ENDPOINT failed %d", errno);
@@ -161,23 +177,12 @@ static struct virtio_ops scsi_dev_virtio_ops = {
 
 static void virtio_scsi_vhost_init(struct kvm *kvm, struct scsi_dev *sdev)
 {
-	u64 features;
-	int r;
-
 	sdev->vhost_fd = open("/dev/vhost-scsi", O_RDWR);
 	if (sdev->vhost_fd < 0)
 		die_perror("Failed openning vhost-scsi device");
 
 	virtio_vhost_init(kvm, sdev->vhost_fd);
 
-	r = ioctl(sdev->vhost_fd, VHOST_GET_FEATURES, &features);
-	if (r != 0)
-		die_perror("VHOST_GET_FEATURES failed");
-
-	r = ioctl(sdev->vhost_fd, VHOST_SET_FEATURES, &features);
-	if (r != 0)
-		die_perror("VHOST_SET_FEATURES failed");
-
 	sdev->vdev.use_vhost = true;
 }
 
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH kvmtool v2 10/17] virtio/vsock: Fix feature selection
  2023-06-06 13:04 [PATCH kvmtool v2 00/17] Fix vhost-net, scsi and vsock Jean-Philippe Brucker
                   ` (8 preceding siblings ...)
  2023-06-06 13:04 ` [PATCH kvmtool v2 09/17] virtio/scsi: Fix feature selection Jean-Philippe Brucker
@ 2023-06-06 13:04 ` Jean-Philippe Brucker
  2023-06-06 13:04 ` [PATCH kvmtool v2 11/17] virtio/net: " Jean-Philippe Brucker
                   ` (7 subsequent siblings)
  17 siblings, 0 replies; 19+ messages in thread
From: Jean-Philippe Brucker @ 2023-06-06 13:04 UTC (permalink / raw)
  To: kvm, will; +Cc: andre.przywara, Jean-Philippe Brucker

We should advertise to the guest only the features supported by vhost
and kvmtool. Then we should set in vhost only the features acked by the
guest. Move vhost feature query to get_host_features(), and vhost
feature setting to device start (after the guest has acked features).

This fixes vsock because we used to enable all vhost features including
VIRTIO_F_ACCESS_PLATFORM, which forces vhost to use vhost-iotlb and
isn't supported by kvmtool.

Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 virtio/vsock.c | 34 ++++++++++++++++++++--------------
 1 file changed, 20 insertions(+), 14 deletions(-)

diff --git a/virtio/vsock.c b/virtio/vsock.c
index 559fbaba..64512713 100644
--- a/virtio/vsock.c
+++ b/virtio/vsock.c
@@ -51,8 +51,17 @@ static size_t get_config_size(struct kvm *kvm, void *dev)
 
 static u64 get_host_features(struct kvm *kvm, void *dev)
 {
-	return 1UL << VIRTIO_RING_F_EVENT_IDX
-		| 1UL << VIRTIO_RING_F_INDIRECT_DESC;
+	int r;
+	u64 features;
+	struct vsock_dev *vdev = dev;
+
+	r = ioctl(vdev->vhost_fd, VHOST_GET_FEATURES, &features);
+	if (r != 0)
+		die_perror("VHOST_GET_FEATURES failed");
+
+	return features &
+		(1ULL << VIRTIO_RING_F_EVENT_IDX |
+		 1ULL << VIRTIO_RING_F_INDIRECT_DESC);
 }
 
 static bool is_event_vq(u32 vq)
@@ -95,12 +104,18 @@ static void notify_status(struct kvm *kvm, void *dev, u32 status)
 	if (status & VIRTIO__STATUS_CONFIG)
 		vdev->config.guest_cid = cpu_to_le64(vdev->guest_cid);
 
-	if (status & VIRTIO__STATUS_START)
+	if (status & VIRTIO__STATUS_START) {
 		start = 1;
-	else if (status & VIRTIO__STATUS_STOP)
+
+		r = ioctl(vdev->vhost_fd, VHOST_SET_FEATURES,
+			  &vdev->vdev.features);
+		if (r != 0)
+			die_perror("VHOST_SET_FEATURES failed");
+	} else if (status & VIRTIO__STATUS_STOP) {
 		start = 0;
-	else
+	} else {
 		return;
+	}
 
 	r = ioctl(vdev->vhost_fd, VHOST_VSOCK_SET_RUNNING, &start);
 	if (r != 0)
@@ -162,7 +177,6 @@ static struct virtio_ops vsock_dev_virtio_ops = {
 
 static void virtio_vhost_vsock_init(struct kvm *kvm, struct vsock_dev *vdev)
 {
-	u64 features;
 	int r;
 
 	vdev->vhost_fd = open("/dev/vhost-vsock", O_RDWR);
@@ -171,14 +185,6 @@ static void virtio_vhost_vsock_init(struct kvm *kvm, struct vsock_dev *vdev)
 
 	virtio_vhost_init(kvm, vdev->vhost_fd);
 
-	r = ioctl(vdev->vhost_fd, VHOST_GET_FEATURES, &features);
-	if (r != 0)
-		die_perror("VHOST_GET_FEATURES failed");
-
-	r = ioctl(vdev->vhost_fd, VHOST_SET_FEATURES, &features);
-	if (r != 0)
-		die_perror("VHOST_SET_FEATURES failed");
-
 	r = ioctl(vdev->vhost_fd, VHOST_VSOCK_SET_GUEST_CID, &vdev->guest_cid);
 	if (r != 0)
 		die_perror("VHOST_VSOCK_SET_GUEST_CID failed");
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH kvmtool v2 11/17] virtio/net: Fix feature selection
  2023-06-06 13:04 [PATCH kvmtool v2 00/17] Fix vhost-net, scsi and vsock Jean-Philippe Brucker
                   ` (9 preceding siblings ...)
  2023-06-06 13:04 ` [PATCH kvmtool v2 10/17] virtio/vsock: " Jean-Philippe Brucker
@ 2023-06-06 13:04 ` Jean-Philippe Brucker
  2023-06-06 13:04 ` [PATCH kvmtool v2 12/17] virtio: Document how to test the devices Jean-Philippe Brucker
                   ` (6 subsequent siblings)
  17 siblings, 0 replies; 19+ messages in thread
From: Jean-Philippe Brucker @ 2023-06-06 13:04 UTC (permalink / raw)
  To: kvm, will; +Cc: andre.przywara, Jean-Philippe Brucker

Move VHOST_GET_FEATURES to get_host_features() so the guest is aware of
what will actually be supported. This removes the invalid guess about
VIRTIO_NET_F_MRG_RXBUF (if vhost didn't support it, we shouldn't let the
guest negotiate it).

Note the masking of VHOST_NET_F_VIRTIO_NET_HDR when handing features to
vhost. Unfortunately the vhost-net driver interprets VIRTIO_F_ANY_LAYOUT
as VHOST_NET_F_VIRTIO_NET_HDR, which is specific to vhost and forces
vhost-net to supply the vnet header. Since this is done by tap, we don't
want to set the bit.

Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 virtio/net.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/virtio/net.c b/virtio/net.c
index 3e1aedf7..c4d20f22 100644
--- a/virtio/net.c
+++ b/virtio/net.c
@@ -505,21 +505,23 @@ static u64 get_host_features(struct kvm *kvm, void *dev)
 		features |= (1UL << VIRTIO_NET_F_HOST_UFO
 				| 1UL << VIRTIO_NET_F_GUEST_UFO);
 
+	if (ndev->vhost_fd) {
+		u64 vhost_features;
+
+		if (ioctl(ndev->vhost_fd, VHOST_GET_FEATURES, &vhost_features) != 0)
+			die_perror("VHOST_GET_FEATURES failed");
+
+		features &= vhost_features;
+	}
+
 	return features;
 }
 
 static int virtio_net__vhost_set_features(struct net_dev *ndev)
 {
-	u64 features = 1UL << VIRTIO_RING_F_EVENT_IDX;
-	u64 vhost_features;
-
-	if (ioctl(ndev->vhost_fd, VHOST_GET_FEATURES, &vhost_features) != 0)
-		die_perror("VHOST_GET_FEATURES failed");
-
-	/* make sure both side support mergable rx buffers */
-	if (vhost_features & 1UL << VIRTIO_NET_F_MRG_RXBUF &&
-			has_virtio_feature(ndev, VIRTIO_NET_F_MRG_RXBUF))
-		features |= 1UL << VIRTIO_NET_F_MRG_RXBUF;
+	/* VHOST_NET_F_VIRTIO_NET_HDR clashes with VIRTIO_F_ANY_LAYOUT! */
+	u64 features = ndev->vdev.features &
+			~(1UL << VHOST_NET_F_VIRTIO_NET_HDR);
 
 	return ioctl(ndev->vhost_fd, VHOST_SET_FEATURES, &features);
 }
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH kvmtool v2 12/17] virtio: Document how to test the devices
  2023-06-06 13:04 [PATCH kvmtool v2 00/17] Fix vhost-net, scsi and vsock Jean-Philippe Brucker
                   ` (10 preceding siblings ...)
  2023-06-06 13:04 ` [PATCH kvmtool v2 11/17] virtio/net: " Jean-Philippe Brucker
@ 2023-06-06 13:04 ` Jean-Philippe Brucker
  2023-06-06 13:04 ` [PATCH kvmtool v2 13/17] virtio: Fix messages about missing Linux config Jean-Philippe Brucker
                   ` (5 subsequent siblings)
  17 siblings, 0 replies; 19+ messages in thread
From: Jean-Philippe Brucker @ 2023-06-06 13:04 UTC (permalink / raw)
  To: kvm, will; +Cc: andre.przywara, Jean-Philippe Brucker

Add a few instructions for testing the devices. Testing devices like
vhost-scsi or vsock may seem daunting but is relatively easy.

Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 Documentation/io-testing.txt | 141 +++++++++++++++++++++++++++++++++++
 1 file changed, 141 insertions(+)
 create mode 100644 Documentation/io-testing.txt

diff --git a/Documentation/io-testing.txt b/Documentation/io-testing.txt
new file mode 100644
index 00000000..c2e41902
--- /dev/null
+++ b/Documentation/io-testing.txt
@@ -0,0 +1,141 @@
+This document describes how to test each device, which is required when
+modifying the common I/O infrastructure.
+
+
+9P
+--
+
+  CONFIG_NET_9P_VIRTIO
+
+Without a --disk parameter, kvmtool shares part of the host filesystem
+with the guest using 9p. Otherwise, use the `--9p <directory>,<tag>`
+parameter to share a directory with the guest, and mount it in the guest
+with:
+
+	$ mount -t 9p <tag> <mountpoint>
+
+
+BALLOON
+-------
+
+  CONFIG_VIRTIO_BALLOON
+
+	$ lkvm run ... --balloon
+
+Display memory statistics:
+
+	$ lkvm stat -a -m
+		*** Guest memory statistics ***
+		...
+
+Remove 20MB of memory from the guest:
+
+	$ lkvm balloon -n guest-$(pidof lkvm) -i 20
+
+
+BLOCK
+-----
+
+  CONFIG_VIRTIO_BLK
+
+	$ lkvm run ... --disk <raw or qcow2 image>
+
+
+CONSOLE
+-------
+
+	$ lkvm run ... --console virtio
+
+See also virtio-console.txt
+
+
+NET
+---
+
+  CONFIG_VIRTIO_NET	(guest)
+  CONFIG_VHOST_NET	(host)
+
+By default kvmtool instantiates a user network device. In order to test
+both tap and vhost, setup a tap interface on a local network.
+
+In the host:
+
+	# ip tuntap add tap0 mode tap user $USER
+	# ip link set tap0 up
+	# ip link add br0 type bridge
+	# ip link set tap0 master br0
+	# ip link set br0 up
+	# ip addr add 192.168.3.1/24 dev br0
+
+	$ lkvm run ... -n mode=tap,tapif=tap0,vhost=1
+
+In the guest:
+
+	# ip link set eth0 up
+	# ip addr add 192.168.3.12/24 dev eth0
+	$ ping -c 1 192.168.3.1
+	64 bytes from 192.168.3.1: seq=0 ttl=64 time=0.303 ms
+
+
+RNG
+---
+
+  CONFIG_HW_RANDOM_VIRTIO
+
+	$ lkvm run ... --rng
+
+In the guest:
+
+	$ cat /sys/devices/virtual/misc/hw_random/rng_available
+	virtio_rng.0
+
+
+SCSI
+----
+
+  CONFIG_SCSI_VIRTIO	(guest)
+  CONFIG_TCM_FILEIO	(host)
+  CONFIG_VHOST_SCSI	(host)
+
+In the host, create a fileio backstore and a target:
+
+	# targetcli (https://github.com/open-iscsi/targetcli-fb)
+	/> cd backstores/fileio
+	/backstores/fileio> create kvmtool_1 /srv/kvmtool_1 2M
+	Created fileio kvmtool_1 with size 2097152
+	/backstores/fileio> cd /vhost
+	/vhost> create
+	Created target naa.500140571c9308aa.
+	Created TPG 1.
+	/vhost> cd naa.500140571c9308aa/tpg1/luns
+	/vhost/naa.50...8aa/tpg1/luns> create /backstores/fileio/kvmtool_1
+	Created LUN 0.
+
+	$ lkvm run ... --disk scsi:naa.500140571c9308aa
+	[    0.479644] scsi host0: Virtio SCSI HBA
+	[    0.483009] scsi 0:0:1:0: Direct-Access     LIO-ORG  kvmtool_1        4.0  PQ: 0 ANSI: 6
+
+	[    1.242833] sd 0:0:1:0: [sda] 4096 512-byte logical blocks: (2.10 MB/2.00 MiB)
+
+
+VSOCK
+-----
+
+  CONFIG_VSOCKETS
+  CONFIG_VIRTIO_VSOCKETS	(guest)
+  CONFIG_VHOST_VSOCK		(host)
+
+In the host, start a vsock server:
+
+	$ socat - VSOCK-LISTEN:1234
+
+We pick 12 as the guest ID. 0 and 1 are reserved, and the host has default
+ID 2.
+
+	$ lkvm run ... --vsock 12
+
+In the guest, send a message to the host:
+
+	$ echo Hello | socat - VSOCK-CONNECT:2:1234
+
+The host server should display "Hello".
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH kvmtool v2 13/17] virtio: Fix messages about missing Linux config
  2023-06-06 13:04 [PATCH kvmtool v2 00/17] Fix vhost-net, scsi and vsock Jean-Philippe Brucker
                   ` (11 preceding siblings ...)
  2023-06-06 13:04 ` [PATCH kvmtool v2 12/17] virtio: Document how to test the devices Jean-Philippe Brucker
@ 2023-06-06 13:04 ` Jean-Philippe Brucker
  2023-06-06 13:04 ` [PATCH kvmtool v2 14/17] virtio/net: Warn about enabling multiqueue with vhost Jean-Philippe Brucker
                   ` (4 subsequent siblings)
  17 siblings, 0 replies; 19+ messages in thread
From: Jean-Philippe Brucker @ 2023-06-06 13:04 UTC (permalink / raw)
  To: kvm, will; +Cc: andre.przywara, Jean-Philippe Brucker

The suggested CONFIG options do not exist.

Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 virtio/scsi.c  | 2 +-
 virtio/vsock.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/virtio/scsi.c b/virtio/scsi.c
index 50f184c7..52bc4936 100644
--- a/virtio/scsi.c
+++ b/virtio/scsi.c
@@ -216,7 +216,7 @@ static int virtio_scsi_init_one(struct kvm *kvm, struct disk_image *disk)
 	virtio_scsi_vhost_init(kvm, sdev);
 
 	if (compat_id == -1)
-		compat_id = virtio_compat_add_message("virtio-scsi", "CONFIG_VIRTIO_SCSI");
+		compat_id = virtio_compat_add_message("virtio-scsi", "CONFIG_SCSI_VIRTIO");
 
 	return 0;
 }
diff --git a/virtio/vsock.c b/virtio/vsock.c
index 64512713..070cfbb6 100644
--- a/virtio/vsock.c
+++ b/virtio/vsock.c
@@ -218,7 +218,7 @@ static int virtio_vsock_init_one(struct kvm *kvm, u64 guest_cid)
 	virtio_vhost_vsock_init(kvm, vdev);
 
 	if (compat_id == -1)
-		compat_id = virtio_compat_add_message("virtio-vsock", "CONFIG_VIRTIO_VSOCK");
+		compat_id = virtio_compat_add_message("virtio-vsock", "CONFIG_VIRTIO_VSOCKETS");
 
 	return 0;
 }
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH kvmtool v2 14/17] virtio/net: Warn about enabling multiqueue with vhost
  2023-06-06 13:04 [PATCH kvmtool v2 00/17] Fix vhost-net, scsi and vsock Jean-Philippe Brucker
                   ` (12 preceding siblings ...)
  2023-06-06 13:04 ` [PATCH kvmtool v2 13/17] virtio: Fix messages about missing Linux config Jean-Philippe Brucker
@ 2023-06-06 13:04 ` Jean-Philippe Brucker
  2023-06-06 13:04 ` [PATCH kvmtool v2 15/17] Factor epoll thread Jean-Philippe Brucker
                   ` (3 subsequent siblings)
  17 siblings, 0 replies; 19+ messages in thread
From: Jean-Philippe Brucker @ 2023-06-06 13:04 UTC (permalink / raw)
  To: kvm, will; +Cc: andre.przywara, Jean-Philippe Brucker

vhost-net requires to open one file descriptor for each TX/RX queue
pair. At the moment kvmtool does not support multi-queue vhost: it
issues all vhost ioctls on the first pair, and the other pairs are
broken. Refuse the enable vhost when the user asks for multi-queue.

Using multi-queue vhost-net also requires creating the tap interface
with the 'multi_queue' parameter.

Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 virtio/net.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/virtio/net.c b/virtio/net.c
index c4d20f22..02667176 100644
--- a/virtio/net.c
+++ b/virtio/net.c
@@ -741,6 +741,11 @@ static struct virtio_ops net_dev_virtio_ops = {
 
 static void virtio_net__vhost_init(struct kvm *kvm, struct net_dev *ndev)
 {
+	if (ndev->queue_pairs > 1) {
+		pr_warning("multiqueue is not supported with vhost yet");
+		return;
+	}
+
 	ndev->vhost_fd = open("/dev/vhost-net", O_RDWR);
 	if (ndev->vhost_fd < 0)
 		die_perror("Failed openning vhost-net device");
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH kvmtool v2 15/17] Factor epoll thread
  2023-06-06 13:04 [PATCH kvmtool v2 00/17] Fix vhost-net, scsi and vsock Jean-Philippe Brucker
                   ` (13 preceding siblings ...)
  2023-06-06 13:04 ` [PATCH kvmtool v2 14/17] virtio/net: Warn about enabling multiqueue with vhost Jean-Philippe Brucker
@ 2023-06-06 13:04 ` Jean-Philippe Brucker
  2023-06-06 13:04 ` [PATCH kvmtool v2 16/17] virtio/vhost: Support line interrupt signaling Jean-Philippe Brucker
                   ` (2 subsequent siblings)
  17 siblings, 0 replies; 19+ messages in thread
From: Jean-Philippe Brucker @ 2023-06-06 13:04 UTC (permalink / raw)
  To: kvm, will; +Cc: andre.przywara, Jean-Philippe Brucker

Both ioeventfd and ipc use an epoll thread roughly the same way. In
order to add a new epoll user, factor the common bits into epoll.c

Slight implementation changes which shouldn't affect behavior:

* At the moment ioeventfd mixes file descriptor (for the stop event) and
  pointers in the epoll_event.data union, which could in theory cause
  aliasing. Use a pointer for the stop event instead. kvm-ipc uses only
  file descriptors. It could be changed but since epoll.c compares the
  stop event pointer first, the risk of aliasing with an fd is much
  lower there.

* kvm-ipc uses EPOLLET, edge-triggered events, but having the stop event
  level-triggered shouldn't make a difference.

Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 Makefile            |   1 +
 include/kvm/epoll.h |  17 ++++++++
 epoll.c             |  91 ++++++++++++++++++++++++++++++++++++++
 ioeventfd.c         |  94 ++++++----------------------------------
 kvm-ipc.c           | 103 +++++++++++++-------------------------------
 5 files changed, 151 insertions(+), 155 deletions(-)
 create mode 100644 include/kvm/epoll.h
 create mode 100644 epoll.c

diff --git a/Makefile b/Makefile
index 86e19339..6b742369 100644
--- a/Makefile
+++ b/Makefile
@@ -80,6 +80,7 @@ OBJS	+= virtio/vhost.o
 OBJS	+= disk/blk.o
 OBJS	+= disk/qcow.o
 OBJS	+= disk/raw.o
+OBJS	+= epoll.o
 OBJS	+= ioeventfd.o
 OBJS	+= net/uip/core.o
 OBJS	+= net/uip/arp.o
diff --git a/include/kvm/epoll.h b/include/kvm/epoll.h
new file mode 100644
index 00000000..dbb5a8d9
--- /dev/null
+++ b/include/kvm/epoll.h
@@ -0,0 +1,17 @@
+#include <sys/epoll.h>
+#include "kvm/kvm.h"
+
+typedef void (*epoll__event_handler_t)(struct kvm *kvm, struct epoll_event *ev);
+
+struct kvm__epoll {
+	int fd;
+	int stop_fd;
+	struct kvm *kvm;
+	const char *name;
+	pthread_t thread;
+	epoll__event_handler_t handle_event;
+};
+
+int epoll__init(struct kvm *kvm, struct kvm__epoll *epoll,
+		const char *name, epoll__event_handler_t handle_event);
+int epoll__exit(struct kvm__epoll *epoll);
diff --git a/epoll.c b/epoll.c
new file mode 100644
index 00000000..8cb0cee5
--- /dev/null
+++ b/epoll.c
@@ -0,0 +1,91 @@
+#include <sys/eventfd.h>
+
+#include "kvm/epoll.h"
+
+#define EPOLLFD_MAX_EVENTS	20
+
+static void *epoll__thread(void *param)
+{
+	u64 stop;
+	int nfds, i;
+	struct kvm__epoll *epoll = param;
+	struct kvm *kvm = epoll->kvm;
+	struct epoll_event events[EPOLLFD_MAX_EVENTS];
+
+	kvm__set_thread_name(epoll->name);
+
+	for (;;) {
+		nfds = epoll_wait(epoll->fd, events, EPOLLFD_MAX_EVENTS, -1);
+		for (i = 0; i < nfds; i++) {
+			if (events[i].data.ptr == &epoll->stop_fd)
+				goto done;
+
+			epoll->handle_event(kvm, &events[i]);
+		}
+	}
+done:
+	if (read(epoll->stop_fd, &stop, sizeof(stop)) < 0)
+		pr_warning("%s: read(stop) failed with %d", __func__, errno);
+	if (write(epoll->stop_fd, &stop, sizeof(stop)) < 0)
+		pr_warning("%s: write(stop) failed with %d", __func__, errno);
+	return NULL;
+}
+
+int epoll__init(struct kvm *kvm, struct kvm__epoll *epoll,
+		const char *name, epoll__event_handler_t handle_event)
+{
+	int r;
+	struct epoll_event stop_event = {
+		.events = EPOLLIN,
+		.data.ptr = &epoll->stop_fd,
+	};
+
+	epoll->kvm = kvm;
+	epoll->name = name;
+	epoll->handle_event = handle_event;
+
+	epoll->fd = epoll_create(EPOLLFD_MAX_EVENTS);
+	if (epoll->fd < 0)
+		return -errno;
+
+	epoll->stop_fd = eventfd(0, 0);
+	if (epoll->stop_fd < 0) {
+		r = -errno;
+		goto err_close_fd;
+	}
+
+	r = epoll_ctl(epoll->fd, EPOLL_CTL_ADD, epoll->stop_fd, &stop_event);
+	if (r < 0)
+		goto err_close_all;
+
+	r = pthread_create(&epoll->thread, NULL, epoll__thread, epoll);
+	if (r < 0)
+		goto err_close_all;
+
+	return 0;
+
+err_close_all:
+	close(epoll->stop_fd);
+err_close_fd:
+	close(epoll->fd);
+
+	return r;
+}
+
+int epoll__exit(struct kvm__epoll *epoll)
+{
+	int r;
+	u64 stop = 1;
+
+	r = write(epoll->stop_fd, &stop, sizeof(stop));
+	if (r < 0)
+		return r;
+
+	r = read(epoll->stop_fd, &stop, sizeof(stop));
+	if (r < 0)
+		return r;
+
+	close(epoll->stop_fd);
+	close(epoll->fd);
+	return 0;
+}
diff --git a/ioeventfd.c b/ioeventfd.c
index 3ae82672..a0fc2578 100644
--- a/ioeventfd.c
+++ b/ioeventfd.c
@@ -9,113 +9,45 @@
 #include <linux/kvm.h>
 #include <linux/types.h>
 
+#include "kvm/epoll.h"
 #include "kvm/ioeventfd.h"
 #include "kvm/kvm.h"
 #include "kvm/util.h"
 
 #define IOEVENTFD_MAX_EVENTS	20
 
-static struct	epoll_event events[IOEVENTFD_MAX_EVENTS];
-static int	epoll_fd, epoll_stop_fd;
 static LIST_HEAD(used_ioevents);
 static bool	ioeventfd_avail;
+static struct kvm__epoll epoll;
 
-static void *ioeventfd__thread(void *param)
+static void ioeventfd__handle_event(struct kvm *kvm, struct epoll_event *ev)
 {
-	u64 tmp = 1;
+	u64 tmp;
+	struct ioevent *ioevent = ev->data.ptr;
 
-	kvm__set_thread_name("ioeventfd-worker");
+	if (read(ioevent->fd, &tmp, sizeof(tmp)) < 0)
+		die("Failed reading event");
 
-	for (;;) {
-		int nfds, i;
-
-		nfds = epoll_wait(epoll_fd, events, IOEVENTFD_MAX_EVENTS, -1);
-		for (i = 0; i < nfds; i++) {
-			struct ioevent *ioevent;
-
-			if (events[i].data.fd == epoll_stop_fd)
-				goto done;
-
-			ioevent = events[i].data.ptr;
-
-			if (read(ioevent->fd, &tmp, sizeof(tmp)) < 0)
-				die("Failed reading event");
-
-			ioevent->fn(ioevent->fn_kvm, ioevent->fn_ptr);
-		}
-	}
-
-done:
-	tmp = write(epoll_stop_fd, &tmp, sizeof(tmp));
-
-	return NULL;
-}
-
-static int ioeventfd__start(void)
-{
-	pthread_t thread;
-
-	if (!ioeventfd_avail)
-		return -ENOSYS;
-
-	return pthread_create(&thread, NULL, ioeventfd__thread, NULL);
+	ioevent->fn(ioevent->fn_kvm, ioevent->fn_ptr);
 }
 
 int ioeventfd__init(struct kvm *kvm)
 {
-	struct epoll_event epoll_event = {.events = EPOLLIN};
-	int r;
-
 	ioeventfd_avail = kvm__supports_extension(kvm, KVM_CAP_IOEVENTFD);
 	if (!ioeventfd_avail)
 		return 1; /* Not fatal, but let caller determine no-go. */
 
-	epoll_fd = epoll_create(IOEVENTFD_MAX_EVENTS);
-	if (epoll_fd < 0)
-		return -errno;
-
-	epoll_stop_fd = eventfd(0, 0);
-	epoll_event.data.fd = epoll_stop_fd;
-
-	r = epoll_ctl(epoll_fd, EPOLL_CTL_ADD, epoll_stop_fd, &epoll_event);
-	if (r < 0)
-		goto cleanup;
-
-	r = ioeventfd__start();
-	if (r < 0)
-		goto cleanup;
-
-	r = 0;
-
-	return r;
-
-cleanup:
-	close(epoll_stop_fd);
-	close(epoll_fd);
-
-	return r;
+	return epoll__init(kvm, &epoll, "ioeventfd-worker",
+			   ioeventfd__handle_event);
 }
 base_init(ioeventfd__init);
 
 int ioeventfd__exit(struct kvm *kvm)
 {
-	u64 tmp = 1;
-	int r;
-
 	if (!ioeventfd_avail)
 		return 0;
 
-	r = write(epoll_stop_fd, &tmp, sizeof(tmp));
-	if (r < 0)
-		return r;
-
-	r = read(epoll_stop_fd, &tmp, sizeof(tmp));
-	if (r < 0)
-		return r;
-
-	close(epoll_fd);
-	close(epoll_stop_fd);
-
+	epoll__exit(&epoll);
 	return 0;
 }
 base_exit(ioeventfd__exit);
@@ -165,7 +97,7 @@ int ioeventfd__add_event(struct ioevent *ioevent, int flags)
 			.data.ptr	= new_ioevent,
 		};
 
-		r = epoll_ctl(epoll_fd, EPOLL_CTL_ADD, event, &epoll_event);
+		r = epoll_ctl(epoll.fd, EPOLL_CTL_ADD, event, &epoll_event);
 		if (r) {
 			r = -errno;
 			goto cleanup;
@@ -213,7 +145,7 @@ int ioeventfd__del_event(u64 addr, u64 datamatch)
 
 	ioctl(ioevent->fn_kvm->vm_fd, KVM_IOEVENTFD, &kvm_ioevent);
 
-	epoll_ctl(epoll_fd, EPOLL_CTL_DEL, ioevent->fd, NULL);
+	epoll_ctl(epoll.fd, EPOLL_CTL_DEL, ioevent->fd, NULL);
 
 	list_del(&ioevent->list);
 
diff --git a/kvm-ipc.c b/kvm-ipc.c
index 23f7b12e..265d80c5 100644
--- a/kvm-ipc.c
+++ b/kvm-ipc.c
@@ -2,9 +2,9 @@
 #include <sys/un.h>
 #include <sys/types.h>
 #include <sys/socket.h>
-#include <sys/eventfd.h>
 #include <dirent.h>
 
+#include "kvm/epoll.h"
 #include "kvm/kvm-ipc.h"
 #include "kvm/rwsem.h"
 #include "kvm/read-write.h"
@@ -28,8 +28,8 @@ struct kvm_ipc_head {
 extern __thread struct kvm_cpu *current_kvm_cpu;
 static void (*msgs[KVM_IPC_MAX_MSGS])(struct kvm *kvm, int fd, u32 type, u32 len, u8 *msg);
 static DECLARE_RWSEM(msgs_rwlock);
-static int epoll_fd, server_fd, stop_fd;
-static pthread_t thread;
+static int server_fd;
+static struct kvm__epoll epoll;
 
 static int kvm__create_socket(struct kvm *kvm)
 {
@@ -268,7 +268,7 @@ static int kvm_ipc__new_conn(int fd)
 
 	ev.events = EPOLLIN | EPOLLRDHUP;
 	ev.data.fd = client;
-	if (epoll_ctl(epoll_fd, EPOLL_CTL_ADD, client, &ev) < 0) {
+	if (epoll_ctl(epoll.fd, EPOLL_CTL_ADD, client, &ev) < 0) {
 		close(client);
 		return -1;
 	}
@@ -278,7 +278,7 @@ static int kvm_ipc__new_conn(int fd)
 
 static void kvm_ipc__close_conn(int fd)
 {
-	epoll_ctl(epoll_fd, EPOLL_CTL_DEL, fd, NULL);
+	epoll_ctl(epoll.fd, EPOLL_CTL_DEL, fd, NULL);
 	close(fd);
 }
 
@@ -309,42 +309,26 @@ done:
 	return -1;
 }
 
-static void *kvm_ipc__thread(void *param)
+static void kvm_ipc__handle_event(struct kvm *kvm, struct epoll_event *ev)
 {
-	struct epoll_event event;
-	struct kvm *kvm = param;
+	int fd = ev->data.fd;
 
-	kvm__set_thread_name("kvm-ipc");
+	if (fd == server_fd) {
+		int client, r;
 
-	for (;;) {
-		int nfds;
+		client = kvm_ipc__new_conn(fd);
+		/*
+		 * Handle multiple IPC cmd at a time
+		 */
+		do {
+			r = kvm_ipc__receive(kvm, client);
+		} while	(r == 0);
 
-		nfds = epoll_wait(epoll_fd, &event, 1, -1);
-		if (nfds > 0) {
-			int fd = event.data.fd;
-
-			if (fd == stop_fd && event.events & EPOLLIN) {
-				break;
-			} else if (fd == server_fd) {
-				int client, r;
-
-				client = kvm_ipc__new_conn(fd);
-				/*
-				 * Handle multiple IPC cmd at a time
-				 */
-				do {
-					r = kvm_ipc__receive(kvm, client);
-				} while	(r == 0);
-
-			} else if (event.events & (EPOLLERR | EPOLLRDHUP | EPOLLHUP)) {
-				kvm_ipc__close_conn(fd);
-			} else {
-				kvm_ipc__receive(kvm, fd);
-			}
-		}
+	} else if (ev->events & (EPOLLERR | EPOLLRDHUP | EPOLLHUP)) {
+		kvm_ipc__close_conn(fd);
+	} else {
+		kvm_ipc__receive(kvm, fd);
 	}
-
-	return NULL;
 }
 
 static void kvm__pid(struct kvm *kvm, int fd, u32 type, u32 len, u8 *msg)
@@ -482,42 +466,21 @@ int kvm_ipc__init(struct kvm *kvm)
 
 	server_fd = sock;
 
-	epoll_fd = epoll_create(KVM_IPC_MAX_MSGS);
-	if (epoll_fd < 0) {
-		perror("epoll_create");
-		ret = epoll_fd;
+	ret = epoll__init(kvm, &epoll, "kvm-ipc",
+			  kvm_ipc__handle_event);
+	if (ret) {
+		pr_err("Failed starting IPC thread");
 		goto err;
 	}
 
 	ev.events = EPOLLIN | EPOLLET;
 	ev.data.fd = sock;
-	if (epoll_ctl(epoll_fd, EPOLL_CTL_ADD, sock, &ev) < 0) {
+	if (epoll_ctl(epoll.fd, EPOLL_CTL_ADD, sock, &ev) < 0) {
 		pr_err("Failed adding socket to epoll");
 		ret = -EFAULT;
 		goto err_epoll;
 	}
 
-	stop_fd = eventfd(0, 0);
-	if (stop_fd < 0) {
-		perror("eventfd");
-		ret = stop_fd;
-		goto err_epoll;
-	}
-
-	ev.events = EPOLLIN | EPOLLET;
-	ev.data.fd = stop_fd;
-	if (epoll_ctl(epoll_fd, EPOLL_CTL_ADD, stop_fd, &ev) < 0) {
-		pr_err("Failed adding stop event to epoll");
-		ret = -EFAULT;
-		goto err_stop;
-	}
-
-	if (pthread_create(&thread, NULL, kvm_ipc__thread, kvm) != 0) {
-		pr_err("Failed starting IPC thread");
-		ret = -EFAULT;
-		goto err_stop;
-	}
-
 	kvm_ipc__register_handler(KVM_IPC_PID, kvm__pid);
 	kvm_ipc__register_handler(KVM_IPC_DEBUG, handle_debug);
 	kvm_ipc__register_handler(KVM_IPC_PAUSE, handle_pause);
@@ -528,10 +491,9 @@ int kvm_ipc__init(struct kvm *kvm)
 
 	return 0;
 
-err_stop:
-	close(stop_fd);
 err_epoll:
-	close(epoll_fd);
+	epoll__exit(&epoll);
+	close(server_fd);
 err:
 	return ret;
 }
@@ -539,18 +501,11 @@ base_init(kvm_ipc__init);
 
 int kvm_ipc__exit(struct kvm *kvm)
 {
-	u64 val = 1;
-	int ret;
-
-	ret = write(stop_fd, &val, sizeof(val));
-	if (ret < 0)
-		return ret;
-
+	epoll__exit(&epoll);
 	close(server_fd);
-	close(epoll_fd);
 
 	kvm__remove_socket(kvm->cfg.guest_name);
 
-	return ret;
+	return 0;
 }
 base_exit(kvm_ipc__exit);
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH kvmtool v2 16/17] virtio/vhost: Support line interrupt signaling
  2023-06-06 13:04 [PATCH kvmtool v2 00/17] Fix vhost-net, scsi and vsock Jean-Philippe Brucker
                   ` (14 preceding siblings ...)
  2023-06-06 13:04 ` [PATCH kvmtool v2 15/17] Factor epoll thread Jean-Philippe Brucker
@ 2023-06-06 13:04 ` Jean-Philippe Brucker
  2023-06-06 13:04 ` [PATCH kvmtool v2 17/17] virtio/vhost: Clear VIRTIO_F_ACCESS_PLATFORM Jean-Philippe Brucker
  2023-06-08 21:45 ` [PATCH kvmtool v2 00/17] Fix vhost-net, scsi and vsock Will Deacon
  17 siblings, 0 replies; 19+ messages in thread
From: Jean-Philippe Brucker @ 2023-06-06 13:04 UTC (permalink / raw)
  To: kvm, will; +Cc: andre.przywara, Jean-Philippe Brucker

To signal a virtqueue, a kernel vhost worker writes an eventfd
registered by kvmtool with VHOST_SET_VRING_CALL. When MSIs are
supported, this eventfd is connected directly to KVM IRQFD to inject the
interrupt into the guest. However direct injection does not work when
MSIs are not supported. The virtio-mmio transport does not support MSIs
at all, and even with virtio-pci, the guest may use INTx if the irqchip
does not support MSIs (e.g. irqchip=gicv3 on arm64).

In this case, injecting the interrupt requires writing an ISR register
in virtio to signal that it is a virtqueue notification rather than a
config change. Add a thread that polls the vhost eventfd for interrupts,
and notifies the guest. When the guest configures MSIs, disable polling
on the eventfd and enable direct injection.

Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 include/kvm/virtio.h |   6 ++-
 virtio/core.c        |   1 +
 virtio/net.c         |   3 +-
 virtio/scsi.c        |   3 +-
 virtio/vhost.c       | 110 +++++++++++++++++++++++++++++++++++++------
 virtio/vsock.c       |   3 +-
 6 files changed, 104 insertions(+), 22 deletions(-)

diff --git a/include/kvm/virtio.h b/include/kvm/virtio.h
index a2f8355a..1fa33e5b 100644
--- a/include/kvm/virtio.h
+++ b/include/kvm/virtio.h
@@ -77,10 +77,12 @@ struct virt_queue {
 	u16		endian;
 	bool		use_event_idx;
 	bool		enabled;
+	struct virtio_device *vdev;
 
 	/* vhost IRQ handling */
 	int		gsi;
 	int		irqfd;
+	int		index;
 };
 
 /*
@@ -267,8 +269,8 @@ void virtio_vhost_set_vring(struct kvm *kvm, int vhost_fd, u32 index,
 			    struct virt_queue *queue);
 void virtio_vhost_set_vring_kick(struct kvm *kvm, int vhost_fd,
 				 u32 index, int event_fd);
-void virtio_vhost_set_vring_call(struct kvm *kvm, int vhost_fd, u32 index,
-				 u32 gsi, struct virt_queue *queue);
+void virtio_vhost_set_vring_irqfd(struct kvm *kvm, u32 gsi,
+				  struct virt_queue *queue);
 void virtio_vhost_reset_vring(struct kvm *kvm, int vhost_fd, u32 index,
 			      struct virt_queue *queue);
 
diff --git a/virtio/core.c b/virtio/core.c
index 63735fae..a77e23bc 100644
--- a/virtio/core.c
+++ b/virtio/core.c
@@ -198,6 +198,7 @@ void virtio_init_device_vq(struct kvm *kvm, struct virtio_device *vdev,
 	vq->endian		= vdev->endian;
 	vq->use_event_idx	= (vdev->features & (1UL << VIRTIO_RING_F_EVENT_IDX));
 	vq->enabled		= true;
+	vq->vdev		= vdev;
 
 	if (addr->legacy) {
 		unsigned long base = (u64)addr->pfn * addr->pgsize;
diff --git a/virtio/net.c b/virtio/net.c
index 02667176..2b4b3661 100644
--- a/virtio/net.c
+++ b/virtio/net.c
@@ -674,8 +674,7 @@ static void notify_vq_gsi(struct kvm *kvm, void *dev, u32 vq, u32 gsi)
 	if (ndev->vhost_fd == 0 || is_ctrl_vq(ndev, vq))
 		return;
 
-	virtio_vhost_set_vring_call(kvm, ndev->vhost_fd, vq, gsi,
-				    &queue->vq);
+	virtio_vhost_set_vring_irqfd(kvm, gsi, &queue->vq);
 }
 
 static void notify_vq_eventfd(struct kvm *kvm, void *dev, u32 vq, u32 efd)
diff --git a/virtio/scsi.c b/virtio/scsi.c
index 52bc4936..27cb3798 100644
--- a/virtio/scsi.c
+++ b/virtio/scsi.c
@@ -119,8 +119,7 @@ static void notify_vq_gsi(struct kvm *kvm, void *dev, u32 vq, u32 gsi)
 	if (sdev->vhost_fd == 0)
 		return;
 
-	virtio_vhost_set_vring_call(kvm, sdev->vhost_fd, vq, gsi,
-				    &sdev->vqs[vq]);
+	virtio_vhost_set_vring_irqfd(kvm, gsi, &sdev->vqs[vq]);
 }
 
 static void notify_vq_eventfd(struct kvm *kvm, void *dev, u32 vq, u32 efd)
diff --git a/virtio/vhost.c b/virtio/vhost.c
index cd83645c..0049003b 100644
--- a/virtio/vhost.c
+++ b/virtio/vhost.c
@@ -1,5 +1,6 @@
 #include "kvm/irq.h"
 #include "kvm/virtio.h"
+#include "kvm/epoll.h"
 
 #include <linux/kvm.h>
 #include <linux/vhost.h>
@@ -7,12 +8,52 @@
 
 #include <sys/eventfd.h>
 
+static struct kvm__epoll epoll;
+
+static void virtio_vhost_signal_vq(struct kvm *kvm, struct epoll_event *ev)
+{
+	int r;
+	u64 tmp;
+	struct virt_queue *queue = ev->data.ptr;
+
+	if (read(queue->irqfd, &tmp, sizeof(tmp)) < 0)
+		pr_warning("%s: failed to read eventfd", __func__);
+
+	r = queue->vdev->ops->signal_vq(kvm, queue->vdev, queue->index);
+	if (r)
+		pr_warning("%s failed to signal virtqueue", __func__);
+}
+
+static int virtio_vhost_start_poll(struct kvm *kvm)
+{
+	if (epoll.fd)
+		return 0;
+
+	if (epoll__init(kvm, &epoll, "vhost-irq-worker",
+			virtio_vhost_signal_vq))
+		return -1;
+
+	return 0;
+}
+
+static int virtio_vhost_stop_poll(struct kvm *kvm)
+{
+	if (epoll.fd)
+		epoll__exit(&epoll);
+	return 0;
+}
+base_exit(virtio_vhost_stop_poll);
+
 void virtio_vhost_init(struct kvm *kvm, int vhost_fd)
 {
 	struct kvm_mem_bank *bank;
 	struct vhost_memory *mem;
 	int i = 0, r;
 
+	r = virtio_vhost_start_poll(kvm);
+	if (r)
+		die("Unable to start vhost polling thread\n");
+
 	mem = calloc(1, sizeof(*mem) +
 		     kvm->mem_slots * sizeof(struct vhost_memory_region));
 	if (mem == NULL)
@@ -39,6 +80,16 @@ void virtio_vhost_init(struct kvm *kvm, int vhost_fd)
 	free(mem);
 }
 
+static int virtio_vhost_get_irqfd(struct virt_queue *queue)
+{
+	if (!queue->irqfd) {
+		queue->irqfd = eventfd(0, 0);
+		if (queue->irqfd < 0)
+			die_perror("eventfd()");
+	}
+	return queue->irqfd;
+}
+
 void virtio_vhost_set_vring(struct kvm *kvm, int vhost_fd, u32 index,
 			    struct virt_queue *queue)
 {
@@ -50,6 +101,16 @@ void virtio_vhost_set_vring(struct kvm *kvm, int vhost_fd, u32 index,
 		.used_user_addr = (u64)(unsigned long)queue->vring.used,
 	};
 	struct vhost_vring_state state = { .index = index };
+	struct vhost_vring_file file = {
+		.index	= index,
+		.fd	= virtio_vhost_get_irqfd(queue),
+	};
+	struct epoll_event event = {
+		.events = EPOLLIN,
+		.data.ptr = queue,
+	};
+
+	queue->index = index;
 
 	if (queue->endian != VIRTIO_ENDIAN_HOST)
 		die("VHOST requires the same endianness in guest and host");
@@ -67,6 +128,14 @@ void virtio_vhost_set_vring(struct kvm *kvm, int vhost_fd, u32 index,
 	r = ioctl(vhost_fd, VHOST_SET_VRING_ADDR, &addr);
 	if (r < 0)
 		die_perror("VHOST_SET_VRING_ADDR failed");
+
+	r = ioctl(vhost_fd, VHOST_SET_VRING_CALL, &file);
+	if (r < 0)
+		die_perror("VHOST_SET_VRING_CALL failed");
+
+	r = epoll_ctl(epoll.fd, EPOLL_CTL_ADD, file.fd, &event);
+	if (r < 0)
+		die_perror("EPOLL_CTL_ADD vhost call fd");
 }
 
 void virtio_vhost_set_vring_kick(struct kvm *kvm, int vhost_fd,
@@ -83,24 +152,23 @@ void virtio_vhost_set_vring_kick(struct kvm *kvm, int vhost_fd,
 		die_perror("VHOST_SET_VRING_KICK failed");
 }
 
-void virtio_vhost_set_vring_call(struct kvm *kvm, int vhost_fd, u32 index,
-				 u32 gsi, struct virt_queue *queue)
+void virtio_vhost_set_vring_irqfd(struct kvm *kvm, u32 gsi,
+				  struct virt_queue *queue)
 {
 	int r;
-	struct vhost_vring_file file = {
-		.index	= index,
-		.fd	= eventfd(0, 0),
-	};
+	int fd = virtio_vhost_get_irqfd(queue);
 
-	r = irq__add_irqfd(kvm, gsi, file.fd, -1);
+	if (queue->gsi)
+		irq__del_irqfd(kvm, queue->gsi, fd);
+	else
+		/* Disconnect user polling thread */
+		epoll_ctl(epoll.fd, EPOLL_CTL_DEL, fd, NULL);
+
+	/* Connect the direct IRQFD route */
+	r = irq__add_irqfd(kvm, gsi, fd, -1);
 	if (r < 0)
 		die_perror("KVM_IRQFD failed");
 
-	r = ioctl(vhost_fd, VHOST_SET_VRING_CALL, &file);
-	if (r < 0)
-		die_perror("VHOST_SET_VRING_CALL failed");
-
-	queue->irqfd = file.fd;
 	queue->gsi = gsi;
 }
 
@@ -108,9 +176,23 @@ void virtio_vhost_reset_vring(struct kvm *kvm, int vhost_fd, u32 index,
 			      struct virt_queue *queue)
 
 {
+	struct vhost_vring_file file = {
+		.index	= index,
+		.fd	= -1,
+	};
+
+	if (!queue->irqfd)
+		return;
+
 	if (queue->gsi) {
 		irq__del_irqfd(kvm, queue->gsi, queue->irqfd);
-		close(queue->irqfd);
-		queue->gsi = queue->irqfd = 0;
+		queue->gsi = 0;
 	}
+
+	epoll_ctl(epoll.fd, EPOLL_CTL_DEL, queue->irqfd, NULL);
+
+	if (ioctl(vhost_fd, VHOST_SET_VRING_CALL, &file))
+		perror("SET_VRING_CALL");
+	close(queue->irqfd);
+	queue->irqfd = 0;
 }
diff --git a/virtio/vsock.c b/virtio/vsock.c
index 070cfbb6..5d22bd24 100644
--- a/virtio/vsock.c
+++ b/virtio/vsock.c
@@ -151,8 +151,7 @@ static void notify_vq_gsi(struct kvm *kvm, void *dev, u32 vq, u32 gsi)
 	if (vdev->vhost_fd == -1 || is_event_vq(vq))
 		return;
 
-	virtio_vhost_set_vring_call(kvm, vdev->vhost_fd, vq, gsi,
-				    &vdev->vqs[vq]);
+	virtio_vhost_set_vring_irqfd(kvm, gsi, &vdev->vqs[vq]);
 }
 
 static unsigned int get_vq_count(struct kvm *kvm, void *dev)
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH kvmtool v2 17/17] virtio/vhost: Clear VIRTIO_F_ACCESS_PLATFORM
  2023-06-06 13:04 [PATCH kvmtool v2 00/17] Fix vhost-net, scsi and vsock Jean-Philippe Brucker
                   ` (15 preceding siblings ...)
  2023-06-06 13:04 ` [PATCH kvmtool v2 16/17] virtio/vhost: Support line interrupt signaling Jean-Philippe Brucker
@ 2023-06-06 13:04 ` Jean-Philippe Brucker
  2023-06-08 21:45 ` [PATCH kvmtool v2 00/17] Fix vhost-net, scsi and vsock Will Deacon
  17 siblings, 0 replies; 19+ messages in thread
From: Jean-Philippe Brucker @ 2023-06-06 13:04 UTC (permalink / raw)
  To: kvm, will; +Cc: andre.przywara, Jean-Philippe Brucker

Vhost interprets the VIRTIO_F_ACCESS_PLATFORM flag as if accesses need
to use vhost-iotlb, and since kvmtool does not implement vhost-iotlb,
vhost will fail to access the virtqueue.

This fix is preventive. Kvmtool does not set VIRTIO_F_ACCESS_PLATFORM at
the moment but the Arm CCA and pKVM changes will likely hit the issue
(as experienced with the CCA development tree), so we might as well fix
it now.

Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 include/kvm/virtio.h |  1 +
 virtio/net.c         | 16 +++++-----------
 virtio/scsi.c        |  3 +--
 virtio/vhost.c       | 11 +++++++++++
 virtio/vsock.c       |  4 ++--
 5 files changed, 20 insertions(+), 15 deletions(-)

diff --git a/include/kvm/virtio.h b/include/kvm/virtio.h
index 1fa33e5b..95b5142b 100644
--- a/include/kvm/virtio.h
+++ b/include/kvm/virtio.h
@@ -273,6 +273,7 @@ void virtio_vhost_set_vring_irqfd(struct kvm *kvm, u32 gsi,
 				  struct virt_queue *queue);
 void virtio_vhost_reset_vring(struct kvm *kvm, int vhost_fd, u32 index,
 			      struct virt_queue *queue);
+int virtio_vhost_set_features(int vhost_fd, u64 features);
 
 int virtio_transport_parser(const struct option *opt, const char *arg, int unset);
 
diff --git a/virtio/net.c b/virtio/net.c
index 2b4b3661..f09dd0a4 100644
--- a/virtio/net.c
+++ b/virtio/net.c
@@ -517,23 +517,17 @@ static u64 get_host_features(struct kvm *kvm, void *dev)
 	return features;
 }
 
-static int virtio_net__vhost_set_features(struct net_dev *ndev)
-{
-	/* VHOST_NET_F_VIRTIO_NET_HDR clashes with VIRTIO_F_ANY_LAYOUT! */
-	u64 features = ndev->vdev.features &
-			~(1UL << VHOST_NET_F_VIRTIO_NET_HDR);
-
-	return ioctl(ndev->vhost_fd, VHOST_SET_FEATURES, &features);
-}
-
 static void virtio_net_start(struct net_dev *ndev)
 {
+	/* VHOST_NET_F_VIRTIO_NET_HDR clashes with VIRTIO_F_ANY_LAYOUT! */
+	u64 features = ndev->vdev.features & ~(1UL << VHOST_NET_F_VIRTIO_NET_HDR);
+
 	if (ndev->mode == NET_MODE_TAP) {
 		if (!virtio_net__tap_init(ndev))
 			die_perror("TAP device initialized failed because");
 
-		if (ndev->vhost_fd &&
-				virtio_net__vhost_set_features(ndev) != 0)
+		if (ndev->vhost_fd && virtio_vhost_set_features(ndev->vhost_fd,
+								features))
 			die_perror("VHOST_SET_FEATURES failed");
 	} else {
 		ndev->info.vnet_hdr_len = virtio_net_hdr_len(ndev);
diff --git a/virtio/scsi.c b/virtio/scsi.c
index 27cb3798..a842290b 100644
--- a/virtio/scsi.c
+++ b/virtio/scsi.c
@@ -69,8 +69,7 @@ static void notify_status(struct kvm *kvm, void *dev, u32 status)
 	u16 endian = vdev->endian;
 
 	if (status & VIRTIO__STATUS_START) {
-		r = ioctl(sdev->vhost_fd, VHOST_SET_FEATURES,
-			  &sdev->vdev.features);
+		r = virtio_vhost_set_features(sdev->vhost_fd, sdev->vdev.features);
 		if (r != 0)
 			die_perror("VHOST_SET_FEATURES failed");
 
diff --git a/virtio/vhost.c b/virtio/vhost.c
index 0049003b..ea640fa6 100644
--- a/virtio/vhost.c
+++ b/virtio/vhost.c
@@ -196,3 +196,14 @@ void virtio_vhost_reset_vring(struct kvm *kvm, int vhost_fd, u32 index,
 	close(queue->irqfd);
 	queue->irqfd = 0;
 }
+
+int virtio_vhost_set_features(int vhost_fd, u64 features)
+{
+	/*
+	 * vhost interprets VIRTIO_F_ACCESS_PLATFORM as meaning there is an
+	 * iotlb. Since this is not the case for kvmtool, mask it.
+	 */
+	u64 masked_feat = features & ~(1ULL << VIRTIO_F_ACCESS_PLATFORM);
+
+	return ioctl(vhost_fd, VHOST_SET_FEATURES, &masked_feat);
+}
diff --git a/virtio/vsock.c b/virtio/vsock.c
index 5d22bd24..7d4053a1 100644
--- a/virtio/vsock.c
+++ b/virtio/vsock.c
@@ -107,8 +107,8 @@ static void notify_status(struct kvm *kvm, void *dev, u32 status)
 	if (status & VIRTIO__STATUS_START) {
 		start = 1;
 
-		r = ioctl(vdev->vhost_fd, VHOST_SET_FEATURES,
-			  &vdev->vdev.features);
+		r = virtio_vhost_set_features(vdev->vhost_fd,
+					      vdev->vdev.features);
 		if (r != 0)
 			die_perror("VHOST_SET_FEATURES failed");
 	} else if (status & VIRTIO__STATUS_STOP) {
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH kvmtool v2 00/17] Fix vhost-net, scsi and vsock
  2023-06-06 13:04 [PATCH kvmtool v2 00/17] Fix vhost-net, scsi and vsock Jean-Philippe Brucker
                   ` (16 preceding siblings ...)
  2023-06-06 13:04 ` [PATCH kvmtool v2 17/17] virtio/vhost: Clear VIRTIO_F_ACCESS_PLATFORM Jean-Philippe Brucker
@ 2023-06-08 21:45 ` Will Deacon
  17 siblings, 0 replies; 19+ messages in thread
From: Will Deacon @ 2023-06-08 21:45 UTC (permalink / raw)
  To: kvm, Jean-Philippe Brucker
  Cc: catalin.marinas, kernel-team, Will Deacon, andre.przywara

On Tue, 6 Jun 2023 14:04:09 +0100, Jean-Philippe Brucker wrote:
> This is version 2 of the vhost fixes for kvmtool posted here:
> https://lore.kernel.org/all/20230419132119.124457-1-jean-philippe@linaro.org/
> 
> Since v1:
> * Added review tags from Andre
> * Fixed issues reported by Andre, and the max_target size found while
>   rebasing patch 8
> * Added patch 14 (warn and disable an unsupported configuration)
> 
> [...]

Applied to kvmtool (master), thanks!

[01/17] virtio: Factor vhost initialization
        https://git.kernel.org/will/kvmtool/c/f84ab9eb74fc
[02/17] virtio/vhost: Factor vring operation
        https://git.kernel.org/will/kvmtool/c/745221e582f7
[03/17] virtio/vhost: Factor notify_vq_eventfd()
        https://git.kernel.org/will/kvmtool/c/676c0c8ad95b
[04/17] virtio/vhost: Factor notify_vq_gsi()
        https://git.kernel.org/will/kvmtool/c/029cd2bb7898
[05/17] virtio/scsi: Move VHOST_SCSI_SET_ENDPOINT to device start
        https://git.kernel.org/will/kvmtool/c/13e7d626e00f
[06/17] virtio/scsi: Fix and simplify command-line
        https://git.kernel.org/will/kvmtool/c/145a86fedfe8
[07/17] disk/core: Fix segfault on exit with SCSI
        https://git.kernel.org/will/kvmtool/c/7bc3b5d7ef80
[08/17] virtio/scsi: Initialize max_target
        https://git.kernel.org/will/kvmtool/c/b8420e8d5d8d
[09/17] virtio/scsi: Fix feature selection
        https://git.kernel.org/will/kvmtool/c/13ea439a1f48
[10/17] virtio/vsock: Fix feature selection
        https://git.kernel.org/will/kvmtool/c/cf8358d34f31
[11/17] virtio/net: Fix feature selection
        https://git.kernel.org/will/kvmtool/c/53171d59b081
[12/17] virtio: Document how to test the devices
        https://git.kernel.org/will/kvmtool/c/13534ee80ce3
[13/17] virtio: Fix messages about missing Linux config
        https://git.kernel.org/will/kvmtool/c/33e026a78c9f
[14/17] virtio/net: Warn about enabling multiqueue with vhost
        https://git.kernel.org/will/kvmtool/c/3a70ab1e7bb3
[15/17] Factor epoll thread
        https://git.kernel.org/will/kvmtool/c/d30d94872e7f
[16/17] virtio/vhost: Support line interrupt signaling
        https://git.kernel.org/will/kvmtool/c/46aaf3b87d7e
[17/17] virtio/vhost: Clear VIRTIO_F_ACCESS_PLATFORM
        https://git.kernel.org/will/kvmtool/c/3b1cdcf9e78f

Cheers,
-- 
Will

https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2023-06-08 21:46 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-06 13:04 [PATCH kvmtool v2 00/17] Fix vhost-net, scsi and vsock Jean-Philippe Brucker
2023-06-06 13:04 ` [PATCH kvmtool v2 01/17] virtio: Factor vhost initialization Jean-Philippe Brucker
2023-06-06 13:04 ` [PATCH kvmtool v2 02/17] virtio/vhost: Factor vring operation Jean-Philippe Brucker
2023-06-06 13:04 ` [PATCH kvmtool v2 03/17] virtio/vhost: Factor notify_vq_eventfd() Jean-Philippe Brucker
2023-06-06 13:04 ` [PATCH kvmtool v2 04/17] virtio/vhost: Factor notify_vq_gsi() Jean-Philippe Brucker
2023-06-06 13:04 ` [PATCH kvmtool v2 05/17] virtio/scsi: Move VHOST_SCSI_SET_ENDPOINT to device start Jean-Philippe Brucker
2023-06-06 13:04 ` [PATCH kvmtool v2 06/17] virtio/scsi: Fix and simplify command-line Jean-Philippe Brucker
2023-06-06 13:04 ` [PATCH kvmtool v2 07/17] disk/core: Fix segfault on exit with SCSI Jean-Philippe Brucker
2023-06-06 13:04 ` [PATCH kvmtool v2 08/17] virtio/scsi: Initialize max_target Jean-Philippe Brucker
2023-06-06 13:04 ` [PATCH kvmtool v2 09/17] virtio/scsi: Fix feature selection Jean-Philippe Brucker
2023-06-06 13:04 ` [PATCH kvmtool v2 10/17] virtio/vsock: " Jean-Philippe Brucker
2023-06-06 13:04 ` [PATCH kvmtool v2 11/17] virtio/net: " Jean-Philippe Brucker
2023-06-06 13:04 ` [PATCH kvmtool v2 12/17] virtio: Document how to test the devices Jean-Philippe Brucker
2023-06-06 13:04 ` [PATCH kvmtool v2 13/17] virtio: Fix messages about missing Linux config Jean-Philippe Brucker
2023-06-06 13:04 ` [PATCH kvmtool v2 14/17] virtio/net: Warn about enabling multiqueue with vhost Jean-Philippe Brucker
2023-06-06 13:04 ` [PATCH kvmtool v2 15/17] Factor epoll thread Jean-Philippe Brucker
2023-06-06 13:04 ` [PATCH kvmtool v2 16/17] virtio/vhost: Support line interrupt signaling Jean-Philippe Brucker
2023-06-06 13:04 ` [PATCH kvmtool v2 17/17] virtio/vhost: Clear VIRTIO_F_ACCESS_PLATFORM Jean-Philippe Brucker
2023-06-08 21:45 ` [PATCH kvmtool v2 00/17] Fix vhost-net, scsi and vsock Will Deacon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).