Virtualization - Jul 2022 - [PATCH v4 5/5] vduse: Support querying information of IOVA regions

On Thu, Jul 28, 2022 at 2:36 PM Yongji Xie <xieyongji at bytedance.com>
wrote:
>
> On Thu, Jul 28, 2022 at 1:58 PM Jason Wang <jasowang at redhat.com>
wrote:
> >
> > On Thu, Jul 28, 2022 at 11:20 AM Xie Yongji <xieyongji at
bytedance.com> wrote:
> > >
> > > This introduces a new ioctl: VDUSE_IOTLB_GET_INFO to
> > > support querying some information of IOVA regions.
> > >
> > > Now it can be used to query whether the IOVA region
> > > supports userspace memory registration.
> > >
> > > Signed-off-by: Xie Yongji <xieyongji at bytedance.com>
> > > ---
> > >  drivers/vdpa/vdpa_user/vduse_dev.c | 39
++++++++++++++++++++++++++++++
> > >  include/uapi/linux/vduse.h         | 24 ++++++++++++++++++
> > >  2 files changed, 63 insertions(+)
> > >
> > > diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c
b/drivers/vdpa/vdpa_user/vduse_dev.c
> > > index eedff0a3885a..e820c37dcba8 100644
> > > --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> > > +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> > > @@ -1228,6 +1228,45 @@ static long vduse_dev_ioctl(struct file
*file, unsigned int cmd,
> > >                                            umem.size);
> > >                 break;
> > >         }
> > > +       case VDUSE_IOTLB_GET_INFO: {
> > > +               struct vduse_iova_info info;
> > > +               struct vhost_iotlb_map *map;
> > > +               struct vduse_iova_domain *domain =
dev->domain;
> > > +
> > > +               ret = -EFAULT;
> > > +               if (copy_from_user(&info, argp,
sizeof(info)))
> > > +                       break;
> > > +
> > > +               ret = -EINVAL;
> > > +               if (info.start > info.last)
> > > +                       break;
> > > +
> > > +               if (!is_mem_zero((const char *)info.reserved,
> > > +                                sizeof(info.reserved)))
> > > +                       break;
> > > +
> > > +               spin_lock(&domain->iotlb_lock);
> > > +               map = vhost_iotlb_itree_first(domain->iotlb,
> > > +                                             info.start,
info.last);
> > > +               if (map) {
> > > +                       info.start = map->start;
> > > +                       info.last = map->last;
> > > +                       info.capability = 0;
> > > +                       if (domain->bounce_map &&
map->start >= 0 &&
> > > +                           map->last <
domain->bounce_size)
> > > +                               info.capability |=
VDUSE_IOVA_CAP_UMEM;
> > > +               }
> > > +               spin_unlock(&domain->iotlb_lock);
> > > +               if (!map)
> > > +                       break;
> > > +
> > > +               ret = -EFAULT;
> > > +               if (copy_to_user(argp, &info, sizeof(info)))
> > > +                       break;
> > > +
> > > +               ret = 0;
> > > +               break;
> > > +       }
> > >         default:
> > >                 ret = -ENOIOCTLCMD;
> > >                 break;
> > > diff --git a/include/uapi/linux/vduse.h
b/include/uapi/linux/vduse.h
> > > index 9885e0571f09..11bd48c72c6c 100644
> > > --- a/include/uapi/linux/vduse.h
> > > +++ b/include/uapi/linux/vduse.h
> > > @@ -233,6 +233,30 @@ struct vduse_iova_umem {
> > >  /* De-register the userspace memory. Caller should set iova and
size field. */
> > >  #define VDUSE_IOTLB_DEREG_UMEM _IOW(VDUSE_BASE, 0x19, struct
vduse_iova_umem)
> > >
> > > +/**
> > > + * struct vduse_iova_info - information of one IOVA region
> > > + * @start: start of the IOVA region
> > > + * @last: last of the IOVA region
> > > + * @capability: capability of the IOVA regsion
> > > + * @reserved: for future use, needs to be initialized to zero
> > > + *
> > > + * Structure used by VDUSE_IOTLB_GET_INFO ioctl to get
information of
> > > + * one IOVA region.
> > > + */
> > > +struct vduse_iova_info {
> > > +       __u64 start;
> > > +       __u64 last;
> > > +#define VDUSE_IOVA_CAP_UMEM (1 << 0)
> > > +       __u64 capability;
> > > +       __u64 reserved[3];
> > > +};
> > > +
> > > +/*
> > > + * Find the first IOVA region that overlaps with the range
[start, last]
> >
> > So the code is actually find the IOVA region that is the super range
> > of [start, last] instead of overlap:
> >
>
> This is achieved by vhost_iotlb_itree_first(). And can't the super
> range of [start,last] be considered overlapping?
Ok, but what I want to ask is, under which condition can we hit the
following case

map->last >= domain->bounce_size ?

Thanks
>
> >
> > > +                       if (domain->bounce_map &&
map->start >= 0 &&
> > > +                           map->last <
domain->bounce_size)
> > > +                               info.capability |=
VDUSE_IOVA_CAP_UMEM;
> >
> > Which part is wrong?
> >
>
> We will first call vhost_iotlb_itree_first() which will find the first
> IOVA region that overlaps with the range [start, last]. Then the flag
> will only be set if the IOVA region is within the bounce range.
>
> Thanks,
> Yongji
>