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

[Felix Kuehling]

On 2024-03-18 16:12, Felix Kuehling wrote:
>
> 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.

Never mind. I double checked: On single-partition GPUs, bo->xcp_id 
always seems to be 0. So your code won't change the behaviour here. The 
patch is

Reviewed-by: Felix Kuehling <felix.kuehling at amd.com>



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