| Message ID | 20230302113421.174582-2-sgarzare@redhat.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | vdpa_sim: add support for user VA | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
On Thu, Mar 2, 2023 at 7:34 PM Stefano Garzarella <sgarzare@redhat.com> 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@redhat.com> > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> > --- One thing that came into my mind is that after this commit: commit 5ce995f313ce56c0c62425c3ddc37c5c50fc33db Author: Jason Wang <jasowang@redhat.com> 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 > > 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 >
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@redhat.com> 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@redhat.com> >> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> >> --- > >One thing that came into my mind is that after this commit: > >commit 5ce995f313ce56c0c62425c3ddc37c5c50fc33db >Author: Jason Wang <jasowang@redhat.com> >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 >> >
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);
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@redhat.com> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> --- 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(+)