[PATCH] drm/amdkfd: Check cgroup when returning DMABuf info

[Felix Kuehling]

On 2024-03-15 14:17, Mukul Joshi wrote:
> Check cgroup permissions when returning DMA-buf info and
> based on cgroup check return the id of the GPU that has
> access to the BO.
>
> Signed-off-by: Mukul Joshi <mukul.joshi at amd.com>
> ---
>   drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> index dfa8c69532d4..f9631f4b1a02 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> @@ -1523,7 +1523,7 @@ static int kfd_ioctl_get_dmabuf_info(struct file *filep,
>   
>   	/* Find a KFD GPU device that supports the get_dmabuf_info query */
>   	for (i = 0; kfd_topology_enum_kfd_devices(i, &dev) == 0; i++)
> -		if (dev)
> +		if (dev && !kfd_devcgroup_check_permission(dev))
>   			break;
>   	if (!dev)
>   		return -EINVAL;
> @@ -1545,7 +1545,7 @@ static int kfd_ioctl_get_dmabuf_info(struct file *filep,
>   	if (xcp_id >= 0)
>   		args->gpu_id = dmabuf_adev->kfd.dev->nodes[xcp_id]->id;
>   	else
> -		args->gpu_id = dmabuf_adev->kfd.dev->nodes[0]->id;
> +		args->gpu_id = dev->id;

If I remember correctly, this was meant as a fallback in case for GTT 
BOs where the exporting partition wasn't known and the application 
didn't have access to the first partition. I think the way you wrote 
this, it could also change the behaviour (report the wrong GPU ID) on 
single-partition GPUs, which is probably not intended. Maybe this would 
preserve the behaviour for that case:

	...
-	else
+	else if (!kfd_devcgroup_check_permission(dmabuf_adev->kfd.dev->nodes[0]))
  		args->gpu_id = dmabuf_adev->kfd.dev->nodes[0]->id;
+	else
+		args->gpu_id = dev->id;

Or maybe a more general solution would make DMABuf import work when the 
exporter is really unknown or not even a GPU. This came up not so long 
ago in the context of interop with 3rd-party devices. This may require 
user mode changes as well.

Regards,
   Felix


>   	args->flags = flags;
>   
>   	/* Copy metadata buffer to user mode */