thr3ads.net - Virtualization - [PATCH net-next] vhost: switch to use new message format [Aug 2018]

If this information is useful, please help other people find it:
Share via:

Jason Wang

2018-Aug-03 07:04 UTC

[PATCH net-next] vhost: switch to use new message format

We use to have message like:

struct vhost_msg {
	int type;
	union {
		struct vhost_iotlb_msg iotlb;
		__u8 padding[64];
	};
};

Unfortunately, there will be a hole of 32bit in 64bit machine because
of the alignment. This leads a different formats between 32bit API and
64bit API. What's more it will break 32bit program running on 64bit
machine.

So fixing this by introducing a new message type with an explicit
32bit reserved field after type like:

struct vhost_msg_v2 {
	int type;
	__u32 reserved;
	union {
		struct vhost_iotlb_msg iotlb;
		__u8 padding[64];
	};
};

We will have a consistent ABI after switching to use this. To enable
this capability, introduce a new ioctl (VHOST_SET_BAKCEND_FEATURE) for
userspace to enable this feature (VHOST_BACKEND_F_IOTLB_V2).

Fixes: 6b1e6cc7855b ("vhost: new device IOTLB API")
Signed-off-by: Jason Wang <jasowang at redhat.com>
---
 drivers/vhost/net.c        | 30 ++++++++++++++++++++
 drivers/vhost/vhost.c      | 71 ++++++++++++++++++++++++++++++++++------------
 drivers/vhost/vhost.h      | 11 ++++++-
 include/uapi/linux/vhost.h | 18 ++++++++++++
 4 files changed, 111 insertions(+), 19 deletions(-)

diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 367d802..4e656f8 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -78,6 +78,10 @@ enum {
 };
 
 enum {
+	VHOST_NET_BACKEND_FEATURES = (1ULL << VHOST_BACKEND_F_IOTLB_MSG_V2)
+};
+
+enum {
 	VHOST_NET_VQ_RX = 0,
 	VHOST_NET_VQ_TX = 1,
 	VHOST_NET_VQ_MAX = 2,
@@ -1399,6 +1403,21 @@ static long vhost_net_reset_owner(struct vhost_net *n)
 	return err;
 }
 
+static int vhost_net_set_backend_features(struct vhost_net *n, u64 features)
+{
+	int i;
+
+	mutex_lock(&n->dev.mutex);
+	for (i = 0; i < VHOST_NET_VQ_MAX; ++i) {
+		mutex_lock(&n->vqs[i].vq.mutex);
+		n->vqs[i].vq.acked_backend_features = features;
+		mutex_unlock(&n->vqs[i].vq.mutex);
+	}
+	mutex_unlock(&n->dev.mutex);
+
+	return 0;
+}
+
 static int vhost_net_set_features(struct vhost_net *n, u64 features)
 {
 	size_t vhost_hlen, sock_hlen, hdr_len;
@@ -1489,6 +1508,17 @@ static long vhost_net_ioctl(struct file *f, unsigned int
ioctl,
 		if (features & ~VHOST_NET_FEATURES)
 			return -EOPNOTSUPP;
 		return vhost_net_set_features(n, features);
+	case VHOST_GET_BACKEND_FEATURES:
+		features = VHOST_NET_BACKEND_FEATURES;
+		if (copy_to_user(featurep, &features, sizeof(features)))
+			return -EFAULT;
+		return 0;
+	case VHOST_SET_BACKEND_FEATURES:
+		if (copy_from_user(&features, featurep, sizeof(features)))
+			return -EFAULT;
+		if (features & ~VHOST_NET_BACKEND_FEATURES)
+			return -EOPNOTSUPP;
+		return vhost_net_set_backend_features(n, features);
 	case VHOST_RESET_OWNER:
 		return vhost_net_reset_owner(n);
 	case VHOST_SET_OWNER:
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index a502f1a..6f6c42d 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -315,6 +315,7 @@ static void vhost_vq_reset(struct vhost_dev *dev,
 	vq->log_addr = -1ull;
 	vq->private_data = NULL;
 	vq->acked_features = 0;
+	vq->acked_backend_features = 0;
 	vq->log_base = NULL;
 	vq->error_ctx = NULL;
 	vq->kick = NULL;
@@ -1027,28 +1028,40 @@ static int vhost_process_iotlb_msg(struct vhost_dev
*dev,
 ssize_t vhost_chr_write_iter(struct vhost_dev *dev,
 			     struct iov_iter *from)
 {
-	struct vhost_msg_node node;
-	unsigned size = sizeof(struct vhost_msg);
-	size_t ret;
-	int err;
+	struct vhost_iotlb_msg msg;
+	size_t offset;
+	int type, ret;
 
-	if (iov_iter_count(from) < size)
-		return 0;
-	ret = copy_from_iter(&node.msg, size, from);
-	if (ret != size)
+	ret = copy_from_iter(&type, sizeof(type), from);
+	if (ret != sizeof(type))
 		goto done;
 
-	switch (node.msg.type) {
+	switch (type) {
 	case VHOST_IOTLB_MSG:
-		err = vhost_process_iotlb_msg(dev, &node.msg.iotlb);
-		if (err)
-			ret = err;
+		/* There maybe a hole after type for V1 message type,
+		 * so skip it here.
+		 */
+		offset = offsetof(struct vhost_msg, iotlb) - sizeof(int);
+		break;
+	case VHOST_IOTLB_MSG_V2:
+		offset = sizeof(__u32);
 		break;
 	default:
 		ret = -EINVAL;
-		break;
+		goto done;
+	}
+
+	iov_iter_advance(from, offset);
+	ret = copy_from_iter(&msg, sizeof(msg), from);
+	if (ret != sizeof(msg))
+		goto done;
+	if (vhost_process_iotlb_msg(dev, &msg)) {
+		ret = -EFAULT;
+		goto done;
 	}
 
+	ret = (type == VHOST_IOTLB_MSG) ? sizeof(struct vhost_msg) :
+	      sizeof(struct vhost_msg_v2);
 done:
 	return ret;
 }
@@ -1107,13 +1120,28 @@ ssize_t vhost_chr_read_iter(struct vhost_dev *dev,
struct iov_iter *to,
 		finish_wait(&dev->wait, &wait);
 
 	if (node) {
-		ret = copy_to_iter(&node->msg, size, to);
+		struct vhost_iotlb_msg *msg;
+		void *start = &node->msg;
+
+		switch (node->msg.type) {
+		case VHOST_IOTLB_MSG:
+			size = sizeof(node->msg);
+			msg = &node->msg.iotlb;
+			break;
+		case VHOST_IOTLB_MSG_V2:
+			size = sizeof(node->msg_v2);
+			msg = &node->msg_v2.iotlb;
+			break;
+		default:
+			BUG();
+			break;
+		}
 
-		if (ret != size || node->msg.type != VHOST_IOTLB_MISS) {
+		ret = copy_to_iter(start, size, to);
+		if (ret != size || msg->type != VHOST_IOTLB_MISS) {
 			kfree(node);
 			return ret;
 		}
-
 		vhost_enqueue_msg(dev, &dev->pending_list, node);
 	}
 
@@ -1126,12 +1154,19 @@ static int vhost_iotlb_miss(struct vhost_virtqueue *vq,
u64 iova, int access)
 	struct vhost_dev *dev = vq->dev;
 	struct vhost_msg_node *node;
 	struct vhost_iotlb_msg *msg;
+	bool v2 = vhost_backend_has_feature(vq, VHOST_BACKEND_F_IOTLB_MSG_V2);
 
-	node = vhost_new_msg(vq, VHOST_IOTLB_MISS);
+	node = vhost_new_msg(vq, v2 ? VHOST_IOTLB_MSG_V2 : VHOST_IOTLB_MSG);
 	if (!node)
 		return -ENOMEM;
 
-	msg = &node->msg.iotlb;
+	if (v2) {
+		node->msg_v2.type = VHOST_IOTLB_MSG_V2;
+		msg = &node->msg_v2.iotlb;
+	} else {
+		msg = &node->msg.iotlb;
+	}
+
 	msg->type = VHOST_IOTLB_MISS;
 	msg->iova = iova;
 	msg->perm = access;
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index 6c844b9..466ef75 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -132,6 +132,7 @@ struct vhost_virtqueue {
 	struct vhost_umem *iotlb;
 	void *private_data;
 	u64 acked_features;
+	u64 acked_backend_features;
 	/* Log write descriptors */
 	void __user *log_base;
 	struct vhost_log *log;
@@ -147,7 +148,10 @@ struct vhost_virtqueue {
 };
 
 struct vhost_msg_node {
-  struct vhost_msg msg;
+  union {
+	  struct vhost_msg msg;
+	  struct vhost_msg_v2 msg_v2;
+  };
   struct vhost_virtqueue *vq;
   struct list_head node;
 };
@@ -238,6 +242,11 @@ static inline bool vhost_has_feature(struct vhost_virtqueue
*vq, int bit)
 	return vq->acked_features & (1ULL << bit);
 }
 
+static inline bool vhost_backend_has_feature(struct vhost_virtqueue *vq, int
bit)
+{
+	return vq->acked_backend_features & (1ULL << bit);
+}
+
 #ifdef CONFIG_VHOST_CROSS_ENDIAN_LEGACY
 static inline bool vhost_is_little_endian(struct vhost_virtqueue *vq)
 {
diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h
index c51f8e5..c176e9d 100644
--- a/include/uapi/linux/vhost.h
+++ b/include/uapi/linux/vhost.h
@@ -65,6 +65,7 @@ struct vhost_iotlb_msg {
 };
 
 #define VHOST_IOTLB_MSG 0x1
+#define VHOST_IOTLB_MSG_V2 0x2
 
 struct vhost_msg {
 	int type;
@@ -74,6 +75,15 @@ struct vhost_msg {
 	};
 };
 
+struct vhost_msg_v2 {
+	int type;
+	__u32 reserved;
+	union {
+		struct vhost_iotlb_msg iotlb;
+		__u8 padding[64];
+	};
+};
+
 struct vhost_memory_region {
 	__u64 guest_phys_addr;
 	__u64 memory_size; /* bytes */
@@ -160,6 +170,14 @@ struct vhost_memory {
 #define VHOST_GET_VRING_BUSYLOOP_TIMEOUT _IOW(VHOST_VIRTIO, 0x24,	\
 					 struct vhost_vring_state)
 
+/* Set or get vhost backend capability */
+
+/* Use message type V2 */
+#define VHOST_BACKEND_F_IOTLB_MSG_V2 0x1
+
+#define VHOST_SET_BACKEND_FEATURES _IOW(VHOST_VIRTIO, 0x25, __u64)
+#define VHOST_GET_BACKEND_FEATURES _IOW(VHOST_VIRTIO, 0x26, __u64)
+
 /* VHOST_NET specific defines */
 
 /* Attach virtio net ring to a raw socket, or tap device.
-- 
2.7.4

Michael S. Tsirkin

2018-Aug-03 07:59 UTC

head link

[PATCH net-next] vhost: switch to use new message format

On Fri, Aug 03, 2018 at 03:04:51PM +0800, Jason Wang
wrote:> We use to have message like:
> 
> struct vhost_msg {
> 	int type;
> 	union {
> 		struct vhost_iotlb_msg iotlb;
> 		__u8 padding[64];
> 	};
> };
> 
> Unfortunately, there will be a hole of 32bit in 64bit machine because
> of the alignment. This leads a different formats between 32bit API and
> 64bit API. What's more it will break 32bit program running on 64bit
> machine.
> 
> So fixing this by introducing a new message type with an explicit
> 32bit reserved field after type like:
> 
> struct vhost_msg_v2 {
> 	int type;
> 	__u32 reserved;
> 	union {
> 		struct vhost_iotlb_msg iotlb;
> 		__u8 padding[64];
> 	};
> };
> 
> We will have a consistent ABI after switching to use this. To enable
> this capability, introduce a new ioctl (VHOST_SET_BAKCEND_FEATURE) for
> userspace to enable this feature (VHOST_BACKEND_F_IOTLB_V2).
> 
> Fixes: 6b1e6cc7855b ("vhost: new device IOTLB API")
> Signed-off-by: Jason Wang <jasowang at redhat.com>
> ---
>  drivers/vhost/net.c        | 30 ++++++++++++++++++++
>  drivers/vhost/vhost.c      | 71
++++++++++++++++++++++++++++++++++------------
>  drivers/vhost/vhost.h      | 11 ++++++-
>  include/uapi/linux/vhost.h | 18 ++++++++++++
>  4 files changed, 111 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index 367d802..4e656f8 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -78,6 +78,10 @@ enum {
>  };
>  
>  enum {
> +	VHOST_NET_BACKEND_FEATURES = (1ULL << VHOST_BACKEND_F_IOTLB_MSG_V2)
> +};
> +
> +enum {
>  	VHOST_NET_VQ_RX = 0,
>  	VHOST_NET_VQ_TX = 1,
>  	VHOST_NET_VQ_MAX = 2,
> @@ -1399,6 +1403,21 @@ static long vhost_net_reset_owner(struct vhost_net
*n)
>  	return err;
>  }
>  
> +static int vhost_net_set_backend_features(struct vhost_net *n, u64
features)
> +{
> +	int i;
> +
> +	mutex_lock(&n->dev.mutex);
> +	for (i = 0; i < VHOST_NET_VQ_MAX; ++i) {
> +		mutex_lock(&n->vqs[i].vq.mutex);
> +		n->vqs[i].vq.acked_backend_features = features;
> +		mutex_unlock(&n->vqs[i].vq.mutex);
> +	}
> +	mutex_unlock(&n->dev.mutex);
> +
> +	return 0;
> +}
> +
>  static int vhost_net_set_features(struct vhost_net *n, u64 features)
>  {
>  	size_t vhost_hlen, sock_hlen, hdr_len;
> @@ -1489,6 +1508,17 @@ static long vhost_net_ioctl(struct file *f, unsigned
int ioctl,
>  		if (features & ~VHOST_NET_FEATURES)
>  			return -EOPNOTSUPP;
>  		return vhost_net_set_features(n, features);
> +	case VHOST_GET_BACKEND_FEATURES:
> +		features = VHOST_NET_BACKEND_FEATURES;
> +		if (copy_to_user(featurep, &features, sizeof(features)))
> +			return -EFAULT;
> +		return 0;
> +	case VHOST_SET_BACKEND_FEATURES:
> +		if (copy_from_user(&features, featurep, sizeof(features)))
> +			return -EFAULT;
> +		if (features & ~VHOST_NET_BACKEND_FEATURES)
> +			return -EOPNOTSUPP;
> +		return vhost_net_set_backend_features(n, features);
>  	case VHOST_RESET_OWNER:
>  		return vhost_net_reset_owner(n);
>  	case VHOST_SET_OWNER:
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index a502f1a..6f6c42d 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -315,6 +315,7 @@ static void vhost_vq_reset(struct vhost_dev *dev,
>  	vq->log_addr = -1ull;
>  	vq->private_data = NULL;
>  	vq->acked_features = 0;
> +	vq->acked_backend_features = 0;
>  	vq->log_base = NULL;
>  	vq->error_ctx = NULL;
>  	vq->kick = NULL;
> @@ -1027,28 +1028,40 @@ static int vhost_process_iotlb_msg(struct vhost_dev
*dev,
>  ssize_t vhost_chr_write_iter(struct vhost_dev *dev,
>  			     struct iov_iter *from)
>  {
> -	struct vhost_msg_node node;
> -	unsigned size = sizeof(struct vhost_msg);
> -	size_t ret;
> -	int err;
> +	struct vhost_iotlb_msg msg;
> +	size_t offset;
> +	int type, ret;
>  
> -	if (iov_iter_count(from) < size)
> -		return 0;
> -	ret = copy_from_iter(&node.msg, size, from);
> -	if (ret != size)
> +	ret = copy_from_iter(&type, sizeof(type), from);
> +	if (ret != sizeof(type))
>  		goto done;
>  
> -	switch (node.msg.type) {
> +	switch (type) {
>  	case VHOST_IOTLB_MSG:
> -		err = vhost_process_iotlb_msg(dev, &node.msg.iotlb);
> -		if (err)
> -			ret = err;
> +		/* There maybe a hole after type for V1 message type,
> +		 * so skip it here.
> +		 */
> +		offset = offsetof(struct vhost_msg, iotlb) - sizeof(int);
> +		break;
> +	case VHOST_IOTLB_MSG_V2:
> +		offset = sizeof(__u32);
>  		break;
>  	default:
>  		ret = -EINVAL;
> -		break;
> +		goto done;
> +	}
> +
> +	iov_iter_advance(from, offset);
> +	ret = copy_from_iter(&msg, sizeof(msg), from);
> +	if (ret != sizeof(msg))
> +		goto done;
> +	if (vhost_process_iotlb_msg(dev, &msg)) {
> +		ret = -EFAULT;
> +		goto done;
>  	}
>  
> +	ret = (type == VHOST_IOTLB_MSG) ? sizeof(struct vhost_msg) :
> +	      sizeof(struct vhost_msg_v2);
>  done:
>  	return ret;
>  }
We can actually fix 32 bit apps too, checking the mode for v1.
But that can wait for another patch.
> @@ -1107,13 +1120,28 @@ ssize_t vhost_chr_read_iter(struct vhost_dev *dev,
struct iov_iter *to,
>  		finish_wait(&dev->wait, &wait);
>  
>  	if (node) {
> -		ret = copy_to_iter(&node->msg, size, to);
> +		struct vhost_iotlb_msg *msg;
> +		void *start = &node->msg;
> +
> +		switch (node->msg.type) {
> +		case VHOST_IOTLB_MSG:
> +			size = sizeof(node->msg);
> +			msg = &node->msg.iotlb;
> +			break;
> +		case VHOST_IOTLB_MSG_V2:
> +			size = sizeof(node->msg_v2);
> +			msg = &node->msg_v2.iotlb;
> +			break;
> +		default:
> +			BUG();
> +			break;
> +		}
>  
> -		if (ret != size || node->msg.type != VHOST_IOTLB_MISS) {
> +		ret = copy_to_iter(start, size, to);
> +		if (ret != size || msg->type != VHOST_IOTLB_MISS) {
>  			kfree(node);
>  			return ret;
>  		}
> -
>  		vhost_enqueue_msg(dev, &dev->pending_list, node);
>  	}
>  
> @@ -1126,12 +1154,19 @@ static int vhost_iotlb_miss(struct vhost_virtqueue
*vq, u64 iova, int access)
>  	struct vhost_dev *dev = vq->dev;
>  	struct vhost_msg_node *node;
>  	struct vhost_iotlb_msg *msg;
> +	bool v2 = vhost_backend_has_feature(vq, VHOST_BACKEND_F_IOTLB_MSG_V2);
>  
> -	node = vhost_new_msg(vq, VHOST_IOTLB_MISS);
> +	node = vhost_new_msg(vq, v2 ? VHOST_IOTLB_MSG_V2 : VHOST_IOTLB_MSG);
>  	if (!node)
>  		return -ENOMEM;
>  
> -	msg = &node->msg.iotlb;
> +	if (v2) {
> +		node->msg_v2.type = VHOST_IOTLB_MSG_V2;
> +		msg = &node->msg_v2.iotlb;
> +	} else {
> +		msg = &node->msg.iotlb;
> +	}
> +
>  	msg->type = VHOST_IOTLB_MISS;
>  	msg->iova = iova;
>  	msg->perm = access;
> diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> index 6c844b9..466ef75 100644
> --- a/drivers/vhost/vhost.h
> +++ b/drivers/vhost/vhost.h
> @@ -132,6 +132,7 @@ struct vhost_virtqueue {
>  	struct vhost_umem *iotlb;
>  	void *private_data;
>  	u64 acked_features;
> +	u64 acked_backend_features;
>  	/* Log write descriptors */
>  	void __user *log_base;
>  	struct vhost_log *log;
> @@ -147,7 +148,10 @@ struct vhost_virtqueue {
>  };
>  
>  struct vhost_msg_node {
> -  struct vhost_msg msg;
> +  union {
> +	  struct vhost_msg msg;
> +	  struct vhost_msg_v2 msg_v2;
> +  };
>    struct vhost_virtqueue *vq;
>    struct list_head node;
>  };
> @@ -238,6 +242,11 @@ static inline bool vhost_has_feature(struct
vhost_virtqueue *vq, int bit)
>  	return vq->acked_features & (1ULL << bit);
>  }
>  
> +static inline bool vhost_backend_has_feature(struct vhost_virtqueue *vq,
int bit)
> +{
> +	return vq->acked_backend_features & (1ULL << bit);
> +}
> +
>  #ifdef CONFIG_VHOST_CROSS_ENDIAN_LEGACY
>  static inline bool vhost_is_little_endian(struct vhost_virtqueue *vq)
>  {
> diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h
> index c51f8e5..c176e9d 100644
> --- a/include/uapi/linux/vhost.h
> +++ b/include/uapi/linux/vhost.h
> @@ -65,6 +65,7 @@ struct vhost_iotlb_msg {
>  };
>  
>  #define VHOST_IOTLB_MSG 0x1
> +#define VHOST_IOTLB_MSG_V2 0x2
>  
>  struct vhost_msg {
>  	int type;
> @@ -74,6 +75,15 @@ struct vhost_msg {
>  	};
>  };
>  
> +struct vhost_msg_v2 {
> +	int type;
> +	__u32 reserved;
> +	union {
> +		struct vhost_iotlb_msg iotlb;
> +		__u8 padding[64];
> +	};
> +};
> +
>  struct vhost_memory_region {
>  	__u64 guest_phys_addr;
>  	__u64 memory_size; /* bytes */
> @@ -160,6 +170,14 @@ struct vhost_memory {
>  #define VHOST_GET_VRING_BUSYLOOP_TIMEOUT _IOW(VHOST_VIRTIO, 0x24,	\
>  					 struct vhost_vring_state)
>  
> +/* Set or get vhost backend capability */
> +
> +/* Use message type V2 */
> +#define VHOST_BACKEND_F_IOTLB_MSG_V2 0x1
> +
> +#define VHOST_SET_BACKEND_FEATURES _IOW(VHOST_VIRTIO, 0x25, __u64)
> +#define VHOST_GET_BACKEND_FEATURES _IOW(VHOST_VIRTIO, 0x26, __u64)
> +
>  /* VHOST_NET specific defines */
>  
>  /* Attach virtio net ring to a raw socket, or tap device.
Acked-by: Michael S. Tsirkin <mst at redhat.com>
> -- 
> 2.7.4

David Miller

2018-Aug-04 20:21 UTC

head link

[PATCH net-next] vhost: switch to use new message format

From: Jason Wang <jasowang at redhat.com>
Date: Fri,  3 Aug 2018 15:04:51 +0800
> So fixing this by introducing a new message type with an explicit
> 32bit reserved field after type like:
> 
> struct vhost_msg_v2 {
> 	int type;
> 	__u32 reserved;
Please use fixed sized types consistently.  Use 's32' instead of
'int'
here.

Thanks!

Jason Wang

2018-Aug-06 03:08 UTC

head link

[PATCH net-next] vhost: switch to use new message format

On 2018?08?05? 04:21, David Miller wrote:> From: Jason Wang <jasowang at redhat.com>
> Date: Fri,  3 Aug 2018 15:04:51 +0800
>
>> So fixing this by introducing a new message type with an explicit
>> 32bit reserved field after type like:
>>
>> struct vhost_msg_v2 {
>> 	int type;
>> 	__u32 reserved;
> Please use fixed sized types consistently.  Use 's32' instead of
'int'
> here.
>
> Thanks!
Ok, V2 will be posted soon.

And it looks to me u32 is sufficient.

Thanks

Jason Wang

2018-Aug-06 03:15 UTC

head link

[PATCH net-next] vhost: switch to use new message format

On 2018?08?03? 15:59, Michael S. Tsirkin wrote:>> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
>> index a502f1a..6f6c42d 100644
>> --- a/drivers/vhost/vhost.c
>> +++ b/drivers/vhost/vhost.c
>> @@ -315,6 +315,7 @@ static void vhost_vq_reset(struct vhost_dev *dev,
>>   	vq->log_addr = -1ull;
>>   	vq->private_data = NULL;
>>   	vq->acked_features = 0;
>> +	vq->acked_backend_features = 0;
>>   	vq->log_base = NULL;
>>   	vq->error_ctx = NULL;
>>   	vq->kick = NULL;
>> @@ -1027,28 +1028,40 @@ static int vhost_process_iotlb_msg(struct
vhost_dev *dev,
>>   ssize_t vhost_chr_write_iter(struct vhost_dev *dev,
>>   			     struct iov_iter *from)
>>   {
>> -	struct vhost_msg_node node;
>> -	unsigned size = sizeof(struct vhost_msg);
>> -	size_t ret;
>> -	int err;
>> +	struct vhost_iotlb_msg msg;
>> +	size_t offset;
>> +	int type, ret;
>>   
>> -	if (iov_iter_count(from) < size)
>> -		return 0;
>> -	ret = copy_from_iter(&node.msg, size, from);
>> -	if (ret != size)
>> +	ret = copy_from_iter(&type, sizeof(type), from);
>> +	if (ret != sizeof(type))
>>   		goto done;
>>   
>> -	switch (node.msg.type) {
>> +	switch (type) {
>>   	case VHOST_IOTLB_MSG:
>> -		err = vhost_process_iotlb_msg(dev, &node.msg.iotlb);
>> -		if (err)
>> -			ret = err;
>> +		/* There maybe a hole after type for V1 message type,
>> +		 * so skip it here.
>> +		 */
>> +		offset = offsetof(struct vhost_msg, iotlb) - sizeof(int);
>> +		break;
>> +	case VHOST_IOTLB_MSG_V2:
>> +		offset = sizeof(__u32);
>>   		break;
>>   	default:
>>   		ret = -EINVAL;
>> -		break;
>> +		goto done;
>> +	}
>> +
>> +	iov_iter_advance(from, offset);
>> +	ret = copy_from_iter(&msg, sizeof(msg), from);
>> +	if (ret != sizeof(msg))
>> +		goto done;
>> +	if (vhost_process_iotlb_msg(dev, &msg)) {
>> +		ret = -EFAULT;
>> +		goto done;
>>   	}
>>   
>> +	ret = (type == VHOST_IOTLB_MSG) ? sizeof(struct vhost_msg) :
>> +	      sizeof(struct vhost_msg_v2);
>>   done:
>>   	return ret;
>>   }
> We can actually fix 32 bit apps too, checking the mode for v1.
> But that can wait for another patch.
>
Yes, let me do it on top.

Thanks

Maybe Matching Threads

Search for more apparently analagous threads

Virtualization - Aug 2018 - [PATCH net-next] vhost: switch to use new message format

[PATCH net-next] vhost: switch to use new message format

[PATCH net-next] vhost: switch to use new message format

[PATCH net-next] vhost: switch to use new message format

[PATCH net-next] vhost: switch to use new message format

[PATCH net-next] vhost: switch to use new message format

Maybe Matching Threads