[AMD Official Use Only - General] > -----Original Message----- > From: Kuehling, Felix <Felix.Kuehling@xxxxxxx> > Sent: Monday, March 18, 2024 4:13 PM > To: Joshi, Mukul <Mukul.Joshi@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx > 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@xxxxxxx> > > --- > > 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 */
