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

Stefano Garzarella
2023-03-16T08:19:19Z

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
>>
>