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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





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




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux