Re: [PATCH v2 1/8] vdpa: add bind_mm/unbind_mm callbacks

[Stefano Garzarella]

On Tue, Mar 14, 2023 at 11:39:42AM +0800, Jason Wang wrote:
> On Thu, Mar 2, 2023 at 7:34 PM Stefano Garzarella <sgarzare@xxxxxxxxxx> wrote:
> > These new optional callbacks is used to bind/unbind the device to
> > a specific address space so the vDPA framework can use VA when
> > these callbacks are implemented.
> >
> > Suggested-by: Jason Wang <jasowang@xxxxxxxxxx>
> > Signed-off-by: Stefano Garzarella <sgarzare@xxxxxxxxxx>
> > ---
>
> One thing that came into my mind is that after this commit:
>
> commit 5ce995f313ce56c0c62425c3ddc37c5c50fc33db
> Author: Jason Wang <jasowang@xxxxxxxxxx>
> Date:   Fri May 29 16:02:59 2020 +0800
>
>    vhost: use mmgrab() instead of mmget() for non worker device
>
>    For the device that doesn't use vhost worker and use_mm(), mmget() is
>    too heavy weight and it may brings troubles for implementing mmap()
>    support for vDPA device.
>
> We don't hold the address space after this commit, so the userspace
> mapping could be invalid if the owner exits?

Thanks for mentioning it, I'll take a look at it!

In case maybe I should do a mmget (or get_task_mm) in vhost-vdpa before
calling the callback, or in the parent driver inside the callback, but
it seems duplicating code.

Thanks,
Stefano

> Thanks
>
> > Notes:
> >     v2:
> >     - removed `struct task_struct *owner` param (unused for now, maybe
> >       useful to support cgroups) [Jason]
> >     - add unbind_mm callback [Jason]
> >
> >  include/linux/vdpa.h | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> >
> > diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
> > index 43f59ef10cc9..369c21394284 100644
> > --- a/include/linux/vdpa.h
> > +++ b/include/linux/vdpa.h
> > @@ -290,6 +290,14 @@ struct vdpa_map_file {
> >   *                             @vdev: vdpa device
> >   *                             @idx: virtqueue index
> >   *                             Returns pointer to structure device or error (NULL)
> > + * @bind_mm:                   Bind the device to a specific address space
> > + *                             so the vDPA framework can use VA when this
> > + *                             callback is implemented. (optional)
> > + *                             @vdev: vdpa device
> > + *                             @mm: address space to bind
> > + * @unbind_mm:                 Unbind the device from the address space
> > + *                             bound using the bind_mm callback. (optional)
> > + *                             @vdev: vdpa device
> >   * @free:                      Free resources that belongs to vDPA (optional)
> >   *                             @vdev: vdpa device
> >   */
> > @@ -351,6 +359,8 @@ struct vdpa_config_ops {
> >         int (*set_group_asid)(struct vdpa_device *vdev, unsigned int group,
> >                               unsigned int asid);
> >         struct device *(*get_vq_dma_dev)(struct vdpa_device *vdev, u16 idx);
> > +       int (*bind_mm)(struct vdpa_device *vdev, struct mm_struct *mm);
> > +       void (*unbind_mm)(struct vdpa_device *vdev);
> >
> >         /* Free device resources */
> >         void (*free)(struct vdpa_device *vdev);
> > --
> > 2.39.2