[kvmtool,01/16] virtio: Factor vhost initialization

Message ID	20230419132119.124457-2-jean-philippe@linaro.org (mailing list archive)
State	New, archived
Headers	show Return-Path: <kvm-owner@vger.kernel.org> From: Jean-Philippe Brucker <jean-philippe@linaro.org> To: kvm@vger.kernel.org, will@kernel.org Cc: suzuki.poulose@arm.com, Jean-Philippe Brucker <jean-philippe@linaro.org> Subject: [PATCH kvmtool 01/16] virtio: Factor vhost initialization Date: Wed, 19 Apr 2023 14:21:05 +0100 Message-Id: <20230419132119.124457-2-jean-philippe@linaro.org> In-Reply-To: <20230419132119.124457-1-jean-philippe@linaro.org> References: <20230419132119.124457-1-jean-philippe@linaro.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Precedence: bulk
Series	Fix vhost-net, scsi and vsock \| expand [kvmtool,00/16] Fix vhost-net, scsi and vsock [kvmtool,01/16] virtio: Factor vhost initialization [kvmtool,02/16] virtio/vhost: Factor vring operation [kvmtool,03/16] virtio/vhost: Factor notify_vq_eventfd() [kvmtool,04/16] virtio/vhost: Factor notify_vq_gsi() [kvmtool,05/16] virtio/scsi: Move VHOST_SCSI_SET_ENDPOINT to device start [kvmtool,06/16] virtio/scsi: Fix and simplify command-line [kvmtool,07/16] disk/core: Fix segfault on exit with SCSI [kvmtool,08/16] virtio/scsi: Initialize max_target [kvmtool,09/16] virtio/scsi: Fix feature selection [kvmtool,10/16] virtio/vsock: Fix feature selection [kvmtool,11/16] virtio/net: Fix feature selection [kvmtool,12/16] virtio: Document how to test the devices [kvmtool,13/16] virtio: Fix messages about missing Linux config [kvmtool,14/16] Factor epoll thread [kvmtool,15/16] virtio/vhost: Support line interrupt signaling [kvmtool,16/16] virtio/vhost: Clear VIRTIO_F_ACCESS_PLATFORM

Message ID

20230419132119.124457-2-jean-philippe@linaro.org (mailing list archive)

State

New, archived

Headers

From: Jean-Philippe Brucker <jean-philippe@linaro.org>
To: kvm@vger.kernel.org, will@kernel.org
Cc: suzuki.poulose@arm.com,
        Jean-Philippe Brucker <jean-philippe@linaro.org>
Subject: [PATCH kvmtool 01/16] virtio: Factor vhost initialization
Date: Wed, 19 Apr 2023 14:21:05 +0100
Message-Id: <20230419132119.124457-2-jean-philippe@linaro.org>
In-Reply-To: <20230419132119.124457-1-jean-philippe@linaro.org>
References: <20230419132119.124457-1-jean-philippe@linaro.org>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

Series

Fix vhost-net, scsi and vsock | expand

Commit Message

Jean-Philippe Brucker April 19, 2023, 1:21 p.m. UTC

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).

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

Comments

Andre Przywara April 20, 2023, 11:23 a.m. UTC | #1

On Wed, 19 Apr 2023 14:21:05 +0100
Jean-Philippe Brucker <jean-philippe@linaro.org> wrote:

> 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).

Compared the code in all the versions, and it matches what the commit
message claims, also still compiles:

Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Cheers,
Andre

> 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 0e8c7a67..cd72bf11 100644
> --- a/include/kvm/virtio.h
> +++ b/include/kvm/virtio.h
> @@ -247,6 +247,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 8749ebfe..6b44754f 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 893dfe60..4dee24a0 100644
> --- a/virtio/scsi.c
> +++ b/virtio/scsi.c
> @@ -203,7 +203,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;
>  
> @@ -211,20 +210,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)
> @@ -233,13 +219,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)

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 0e8c7a67..cd72bf11 100644
--- a/include/kvm/virtio.h
+++ b/include/kvm/virtio.h
@@ -247,6 +247,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 8749ebfe..6b44754f 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 893dfe60..4dee24a0 100644
--- a/virtio/scsi.c
+++ b/virtio/scsi.c
@@ -203,7 +203,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;
 
@@ -211,20 +210,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)
@@ -233,13 +219,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)

[kvmtool,01/16] virtio: Factor vhost initialization

Commit Message

Comments

Patch