On 3/20/2024 4:21 PM, Felix Kuehling wrote:
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@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.
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
should bo->xcp_id be >= 0 for all cases? if yes then I think all tests
can be moved. (like below)
args->gpu_id = dmabuf_adev->kfd.dev->nodes[xcp_id]->id;
David
Reviewed-by: Felix Kuehling <felix.kuehling@xxxxxxx>
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 */
