lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <BY5PR02MB6980EB7C1C0935C612588D68B1059@BY5PR02MB6980.namprd02.prod.outlook.com>
Date:   Fri, 4 Mar 2022 17:52:21 +0000
From:   Gautam Dawar <gdawar@...inx.com>
To:     Eugenio Perez Martin <eperezma@...hat.com>
CC:     Martin Petrus Hubertus Habets <martinh@...inx.com>,
        Harpreet Singh Anand <hanand@...inx.com>,
        Tanuj Murlidhar Kamde <tanujk@...inx.com>,
        Jason Wang <jasowang@...hat.com>,
        "Michael S. Tsirkin" <mst@...hat.com>,
        Zhu Lingshan <lingshan.zhu@...el.com>,
        Stefano Garzarella <sgarzare@...hat.com>,
        Xie Yongji <xieyongji@...edance.com>,
        Eli Cohen <elic@...dia.com>,
        Si-Wei Liu <si-wei.liu@...cle.com>,
        Parav Pandit <parav@...dia.com>,
        Longpeng <longpeng2@...wei.com>,
        virtualization <virtualization@...ts.linux-foundation.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        kvm list <kvm@...r.kernel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: RE: [RFC PATCH v2 09/19] vhost: support ASID in IOTLB API

-----Original Message-----
From: Eugenio Perez Martin <eperezma@...hat.com> 
Sent: Friday, March 4, 2022 3:55 PM
To: Gautam Dawar <gdawar@...inx.com>
Cc: Gautam Dawar <gdawar@...inx.com>; Martin Petrus Hubertus Habets <martinh@...inx.com>; Harpreet Singh Anand <hanand@...inx.com>; Tanuj Murlidhar Kamde <tanujk@...inx.com>; Jason Wang <jasowang@...hat.com>; Michael S. Tsirkin <mst@...hat.com>; Zhu Lingshan <lingshan.zhu@...el.com>; Stefano Garzarella <sgarzare@...hat.com>; Xie Yongji <xieyongji@...edance.com>; Eli Cohen <elic@...dia.com>; Si-Wei Liu <si-wei.liu@...cle.com>; Parav Pandit <parav@...dia.com>; Longpeng <longpeng2@...wei.com>; virtualization <virtualization@...ts.linux-foundation.org>; linux-kernel@...r.kernel.org; kvm list <kvm@...r.kernel.org>; netdev@...r.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@...inx.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@...hat.com>
> Signed-off-by: Gautam Dawar <gdawar@...inx.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
>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ