Re: [PATCH 10/21] vhost: support ASID in IOTLB API

[Eli Cohen <elic@nvidia.com>]

On Wed, Dec 16, 2020 at 02:48:07PM +0800, Jason Wang wrote:
> This patches allows userspace to send ASID based IOTLB message to
> vhost. This idea is to use the reserved u32 field in the existing V2
> IOTLB message. Vhost device should advertise this capability via
> VHOST_BACKEND_F_IOTLB_ASID backend feature.
> 
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
>  drivers/vhost/vdpa.c             |  5 ++++-
>  drivers/vhost/vhost.c            | 23 ++++++++++++++++++-----
>  drivers/vhost/vhost.h            |  4 ++--
>  include/uapi/linux/vhost_types.h |  5 ++++-
>  4 files changed, 28 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
> index 03a9b3311c6c..feb6a58df22d 100644
> --- a/drivers/vhost/vdpa.c
> +++ b/drivers/vhost/vdpa.c
> @@ -739,7 +739,7 @@ static int vhost_vdpa_process_iotlb_update(struct vhost_vdpa *v,
>  	return ret;
>  }
>  
> -static int vhost_vdpa_process_iotlb_msg(struct vhost_dev *dev,
> +static int vhost_vdpa_process_iotlb_msg(struct vhost_dev *dev, u32 asid,
>  					struct vhost_iotlb_msg *msg)
>  {
>  	struct vhost_vdpa *v = container_of(dev, struct vhost_vdpa, vdev);
> @@ -748,6 +748,9 @@ static int vhost_vdpa_process_iotlb_msg(struct vhost_dev *dev,
>  	struct vhost_iotlb *iotlb = v->iotlb;
>  	int r = 0;
>  
> +	if (asid != 0)
> +		return -EINVAL;
> +
>  	r = vhost_dev_check_owner(dev);
>  	if (r)
>  		return r;
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index a262e12c6dc2..7477b724c29b 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -468,7 +468,7 @@ void vhost_dev_init(struct vhost_dev *dev,
>  		    struct vhost_virtqueue **vqs, int nvqs,
>  		    int iov_limit, int weight, int byte_weight,
>  		    bool use_worker,
> -		    int (*msg_handler)(struct vhost_dev *dev,
> +		    int (*msg_handler)(struct vhost_dev *dev, u32 asid,
>  				       struct vhost_iotlb_msg *msg))
>  {
>  	struct vhost_virtqueue *vq;
> @@ -1084,11 +1084,14 @@ static bool umem_access_ok(u64 uaddr, u64 size, int access)
>  	return true;
>  }
>  
> -static int vhost_process_iotlb_msg(struct vhost_dev *dev,
> +static int vhost_process_iotlb_msg(struct vhost_dev *dev, u16 asid,
>  				   struct vhost_iotlb_msg *msg)
>  {
>  	int ret = 0;
>  
> +	if (asid != 0)
> +		return -EINVAL;
> +
>  	mutex_lock(&dev->mutex);
>  	vhost_dev_lock_vqs(dev);
>  	switch (msg->type) {
> @@ -1135,6 +1138,7 @@ ssize_t vhost_chr_write_iter(struct vhost_dev *dev,
>  	struct vhost_iotlb_msg msg;
>  	size_t offset;
>  	int type, ret;
> +	u16 asid = 0;

You assume asid occupies just 16 bits. So maybe you should reserve the
other 16 bits for future extension:

struct vhost_msg_v2 {
        __u32 type;
-       __u32 reserved;
+       __u16 asid;
+       __u16 reserved;
        union {

Moreover, maybe this should be reflected in previous patches that use
the asid:

-static int mlx5_vdpa_set_map(struct vdpa_device *vdev, struct vhost_iotlb *iotlb)
+static int mlx5_vdpa_set_map(struct vdpa_device *vdev, u16 asid,
+                            struct vhost_iotlb *iotlb)

-static int vhost_vdpa_process_iotlb_msg(struct vhost_dev *dev,
+static int vhost_vdpa_process_iotlb_msg(struct vhost_dev *dev, u16 asid,
                                        struct vhost_iotlb_msg *msg)

etc.

>  
>  	ret = copy_from_iter(&type, sizeof(type), from);
>  	if (ret != sizeof(type)) {
> @@ -1150,7 +1154,16 @@ ssize_t vhost_chr_write_iter(struct vhost_dev *dev,
>  		offset = offsetof(struct vhost_msg, iotlb) - sizeof(int);
>  		break;
>  	case VHOST_IOTLB_MSG_V2:
> -		offset = sizeof(__u32);
> +		if (vhost_backend_has_feature(dev->vqs[0],
> +					      VHOST_BACKEND_F_IOTLB_ASID)) {
> +			ret = copy_from_iter(&asid, sizeof(asid), from);
> +			if (ret != sizeof(asid)) {
> +				ret = -EINVAL;
> +				goto done;
> +			}
> +			offset = sizeof(__u16);
> +		} else
> +			offset = sizeof(__u32);
>  		break;
>  	default:
>  		ret = -EINVAL;
> @@ -1165,9 +1178,9 @@ ssize_t vhost_chr_write_iter(struct vhost_dev *dev,
>  	}
>  
>  	if (dev->msg_handler)
> -		ret = dev->msg_handler(dev, &msg);
> +		ret = dev->msg_handler(dev, asid, &msg);
>  	else
> -		ret = vhost_process_iotlb_msg(dev, &msg);
> +		ret = vhost_process_iotlb_msg(dev, asid, &msg);
>  	if (ret) {
>  		ret = -EFAULT;
>  		goto done;
> diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> index b063324c7669..19753a90875c 100644
> --- a/drivers/vhost/vhost.h
> +++ b/drivers/vhost/vhost.h
> @@ -162,7 +162,7 @@ struct vhost_dev {
>  	int byte_weight;
>  	u64 kcov_handle;
>  	bool use_worker;
> -	int (*msg_handler)(struct vhost_dev *dev,
> +	int (*msg_handler)(struct vhost_dev *dev, u32 asid,
>  			   struct vhost_iotlb_msg *msg);
>  };
>  
> @@ -170,7 +170,7 @@ bool vhost_exceeds_weight(struct vhost_virtqueue *vq, int pkts, int total_len);
>  void vhost_dev_init(struct vhost_dev *, struct vhost_virtqueue **vqs,
>  		    int nvqs, int iov_limit, int weight, int byte_weight,
>  		    bool use_worker,
> -		    int (*msg_handler)(struct vhost_dev *dev,
> +		    int (*msg_handler)(struct vhost_dev *dev, u32 asid,
>  				       struct vhost_iotlb_msg *msg));
>  long vhost_dev_set_owner(struct vhost_dev *dev);
>  bool vhost_dev_has_owner(struct vhost_dev *dev);
> diff --git a/include/uapi/linux/vhost_types.h b/include/uapi/linux/vhost_types.h
> index 76ee7016c501..222fc66ce2ac 100644
> --- a/include/uapi/linux/vhost_types.h
> +++ b/include/uapi/linux/vhost_types.h
> @@ -87,7 +87,7 @@ struct vhost_msg {
>  
>  struct vhost_msg_v2 {
>  	__u32 type;
> -	__u32 reserved;
> +	__u32 asid;
>  	union {
>  		struct vhost_iotlb_msg iotlb;
>  		__u8 padding[64];
> @@ -157,5 +157,8 @@ struct vhost_vdpa_iova_range {
>  #define VHOST_BACKEND_F_IOTLB_MSG_V2 0x1
>  /* IOTLB can accept batching hints */
>  #define VHOST_BACKEND_F_IOTLB_BATCH  0x2
> +/* IOTLB can accept address space identifier through V2 type of IOTLB
> +   message */
> +#define VHOST_BACKEND_F_IOTLB_ASID  0x3
>  
>  #endif
> -- 
> 2.25.1
>