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

[Joshi, Mukul]

[AMD Official Use Only - General]

> -----Original Message-----
> From: Kuehling, Felix <Felix.Kuehling at amd.com>
> Sent: Monday, March 18, 2024 4:13 PM
> To: Joshi, Mukul <Mukul.Joshi at amd.com>; amd-gfx at lists.freedesktop.org
> Subject: Re: [PATCH] drm/amdkfd: Check cgroup when returning DMABuf info
>
>
> 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:
>
Can you please explain why this could be a issue on a single partition GPU?

Regards,
Mukul

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