| Message ID | 20220224212314.1326-10-gdawar@xilinx.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC,v2,01/19] vhost: move the backend feature bits to vhost_types.h | expand |
On Thu, Feb 24, 2022 at 10:26 PM Gautam Dawar <gautam.dawar@xilinx.com> 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> > Signed-off-by: Gautam Dawar <gdawar@xilinx.com> > --- > drivers/vhost/vdpa.c | 5 ++++- > drivers/vhost/vhost.c | 23 ++++++++++++++++++----- > drivers/vhost/vhost.h | 4 ++-- > include/uapi/linux/vhost_types.h | 6 +++++- > 4 files changed, 29 insertions(+), 9 deletions(-) > > diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c > index 6bf755f84d26..d0aacc0cc79a 100644 > --- a/drivers/vhost/vdpa.c > +++ b/drivers/vhost/vdpa.c > @@ -836,7 +836,7 @@ static int vhost_vdpa_process_iotlb_update(struct vhost_vdpa *v, > msg->perm); > } > > -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); > @@ -847,6 +847,9 @@ static int vhost_vdpa_process_iotlb_msg(struct vhost_dev *dev, > > mutex_lock(&dev->mutex); > > + if (asid != 0) > + return -EINVAL; > + Should we check for asid != 0 before acquiring the mutex? Otherwise the code never releases it. > r = vhost_dev_check_owner(dev); > if (r) > goto unlock; > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c > index 59edb5a1ffe2..1f514d98f0de 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; > @@ -1090,11 +1090,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, u32 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) { > @@ -1141,6 +1144,7 @@ ssize_t vhost_chr_write_iter(struct vhost_dev *dev, > struct vhost_iotlb_msg msg; > size_t offset; > int type, ret; > + u32 asid = 0; > > ret = copy_from_iter(&type, sizeof(type), from); > if (ret != sizeof(type)) { > @@ -1156,7 +1160,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; > @@ -1171,9 +1184,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 638bb640d6b4..9f238d6c7b58 100644 > --- a/drivers/vhost/vhost.h > +++ b/drivers/vhost/vhost.h > @@ -161,7 +161,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); > }; > > @@ -169,7 +169,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..634cee485abb 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,9 @@ 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.0 >
-----Original Message----- From: Eugenio Perez Martin <eperezma@redhat.com> Sent: Friday, March 4, 2022 3:55 PM To: Gautam Dawar <gdawar@xilinx.com> Cc: Gautam Dawar <gdawar@xilinx.com>; Martin Petrus Hubertus Habets <martinh@xilinx.com>; Harpreet Singh Anand <hanand@xilinx.com>; Tanuj Murlidhar Kamde <tanujk@xilinx.com>; Jason Wang <jasowang@redhat.com>; Michael S. Tsirkin <mst@redhat.com>; Zhu Lingshan <lingshan.zhu@intel.com>; Stefano Garzarella <sgarzare@redhat.com>; Xie Yongji <xieyongji@bytedance.com>; Eli Cohen <elic@nvidia.com>; Si-Wei Liu <si-wei.liu@oracle.com>; Parav Pandit <parav@nvidia.com>; Longpeng <longpeng2@huawei.com>; virtualization <virtualization@lists.linux-foundation.org>; linux-kernel@vger.kernel.org; kvm list <kvm@vger.kernel.org>; netdev@vger.kernel.org Subject: Re: [RFC PATCH v2 09/19] vhost: support ASID in IOTLB API On Thu, Feb 24, 2022 at 10:26 PM Gautam Dawar <gautam.dawar@xilinx.com> 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> > Signed-off-by: Gautam Dawar <gdawar@xilinx.com> > --- > drivers/vhost/vdpa.c | 5 ++++- > drivers/vhost/vhost.c | 23 ++++++++++++++++++----- > drivers/vhost/vhost.h | 4 ++-- > include/uapi/linux/vhost_types.h | 6 +++++- > 4 files changed, 29 insertions(+), 9 deletions(-) > > diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c index > 6bf755f84d26..d0aacc0cc79a 100644 > --- a/drivers/vhost/vdpa.c > +++ b/drivers/vhost/vdpa.c > @@ -836,7 +836,7 @@ static int vhost_vdpa_process_iotlb_update(struct vhost_vdpa *v, > msg->perm); } > > -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); @@ -847,6 +847,9 @@ static int > vhost_vdpa_process_iotlb_msg(struct vhost_dev *dev, > > mutex_lock(&dev->mutex); > > + if (asid != 0) > + return -EINVAL; > + Should we check for asid != 0 before acquiring the mutex? Otherwise the code never releases it. [GD>>] Yes, you are right. Will fix this in next revision. > r = vhost_dev_check_owner(dev); > if (r) > goto unlock; > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c index > 59edb5a1ffe2..1f514d98f0de 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; > @@ -1090,11 +1090,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, u32 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) { > @@ -1141,6 +1144,7 @@ ssize_t vhost_chr_write_iter(struct vhost_dev *dev, > struct vhost_iotlb_msg msg; > size_t offset; > int type, ret; > + u32 asid = 0; > > ret = copy_from_iter(&type, sizeof(type), from); > if (ret != sizeof(type)) { > @@ -1156,7 +1160,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; > @@ -1171,9 +1184,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 > 638bb640d6b4..9f238d6c7b58 100644 > --- a/drivers/vhost/vhost.h > +++ b/drivers/vhost/vhost.h > @@ -161,7 +161,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); }; > > @@ -169,7 +169,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..634cee485abb 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,9 @@ 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.0 >
diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c index 6bf755f84d26..d0aacc0cc79a 100644 --- a/drivers/vhost/vdpa.c +++ b/drivers/vhost/vdpa.c @@ -836,7 +836,7 @@ static int vhost_vdpa_process_iotlb_update(struct vhost_vdpa *v, msg->perm); } -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); @@ -847,6 +847,9 @@ static int vhost_vdpa_process_iotlb_msg(struct vhost_dev *dev, mutex_lock(&dev->mutex); + if (asid != 0) + return -EINVAL; + r = vhost_dev_check_owner(dev); if (r) goto unlock; diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c index 59edb5a1ffe2..1f514d98f0de 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; @@ -1090,11 +1090,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, u32 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) { @@ -1141,6 +1144,7 @@ ssize_t vhost_chr_write_iter(struct vhost_dev *dev, struct vhost_iotlb_msg msg; size_t offset; int type, ret; + u32 asid = 0; ret = copy_from_iter(&type, sizeof(type), from); if (ret != sizeof(type)) { @@ -1156,7 +1160,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; @@ -1171,9 +1184,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 638bb640d6b4..9f238d6c7b58 100644 --- a/drivers/vhost/vhost.h +++ b/drivers/vhost/vhost.h @@ -161,7 +161,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); }; @@ -169,7 +169,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..634cee485abb 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,9 @@ 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