lkml.org 
[lkml]   [2022]   [Mar]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    SubjectRE: [RFC PATCH v2 09/19] vhost: support ASID in IOTLB API
    Date
    -----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
    >

    \
     
     \ /
      Last update: 2022-09-17 16:15    [W:3.690 / U:0.068 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site