<html><head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
  </head>
  <body>
    <p><br>
    </p>
    <div class="moz-cite-prefix">On 2021-04-07 7:12 p.m., Felix Kuehling
      wrote:<br>
    </div>
    <blockquote type="cite" cite="mid:20210407231259.1787-3-Felix.Kuehling@amd.com">
      <pre class="moz-quote-pre" wrap="">DRM allows access automatically when it creates a GEM handle for a BO.
KFD BOs don't have GEM handles, so KFD needs to manage access manually.
</pre>
    </blockquote>
    <p>After reading drm vma manager, I understand it uses rbtree to
      store all GPU drm file handle when calling drm_vma_node_allow,
      drm_vma_node_is_allowed/drm_vma_node_verify_access is checked when
      creating mapping, I don't understand how application uses this,
      but seems we need call drm_vma_node_allow when
      amdgpu_amdkfd_gpuvm_map_memory_to_gpu, to add mapping GPUs drm
      file handle to vma manager.</p>
    <p>Regards,</p>
    <p>Philip<br>
    </p>
    <blockquote type="cite" cite="mid:20210407231259.1787-3-Felix.Kuehling@amd.com">
      <pre class="moz-quote-pre" wrap="">
Signed-off-by: Felix Kuehling <a class="moz-txt-link-rfc2396E" href="mailto:Felix.Kuehling@amd.com"><Felix.Kuehling@amd.com></a>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h    |  3 ++-
 .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  | 19 ++++++++++++++++++-
 drivers/gpu/drm/amd/amdkfd/kfd_chardev.c      |  8 +++++---
 drivers/gpu/drm/amd/amdkfd/kfd_process.c      |  7 ++++---
 4 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
index 0d59bebd92af..7c8c5e469707 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
@@ -245,7 +245,8 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
                void *drm_priv, struct kgd_mem **mem,
                uint64_t *offset, uint32_t flags);
 int amdgpu_amdkfd_gpuvm_free_memory_of_gpu(
-               struct kgd_dev *kgd, struct kgd_mem *mem, uint64_t *size);
+               struct kgd_dev *kgd, struct kgd_mem *mem, void *drm_priv,
+               uint64_t *size);
 int amdgpu_amdkfd_gpuvm_map_memory_to_gpu(
                struct kgd_dev *kgd, struct kgd_mem *mem, void *drm_priv);
 int amdgpu_amdkfd_gpuvm_unmap_memory_from_gpu(
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index 95442bcd60fb..e7d61ec966b6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -1232,6 +1232,12 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
                         domain_string(alloc_domain), ret);
                goto err_bo_create;
        }
+       ret = drm_vma_node_allow(&gobj->vma_node, drm_priv);
+       if (ret) {
+               pr_debug("Failed to allow vma node access. ret %d\n",
+                        ret);</pre>
    </blockquote>
    <p>pr_debug("Failed to allow vma node access. ret %d\n", ret); <br>
    </p>
    <p>It's within 80 columns.</p>
    <p>Philip<br>
    </p>
    <blockquote type="cite" cite="mid:20210407231259.1787-3-Felix.Kuehling@amd.com">
      <pre class="moz-quote-pre" wrap="">
+               goto err_node_allow;
+       }
        bo = gem_to_amdgpu_bo(gobj);
        if (bo_type == ttm_bo_type_sg) {
                bo->tbo.sg = sg;
@@ -1261,6 +1267,8 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
 
 allocate_init_user_pages_failed:
        remove_kgd_mem_from_kfd_bo_list(*mem, avm->process_info);
+       drm_vma_node_revoke(&gobj->vma_node, drm_priv);
+err_node_allow:
        amdgpu_bo_unref(&bo);
        /* Don't unreserve system mem limit twice */
        goto err_reserve_limit;
@@ -1278,7 +1286,8 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
 }
 
 int amdgpu_amdkfd_gpuvm_free_memory_of_gpu(
-               struct kgd_dev *kgd, struct kgd_mem *mem, uint64_t *size)
+               struct kgd_dev *kgd, struct kgd_mem *mem, void *drm_priv,
+               uint64_t *size)
 {
        struct amdkfd_process_info *process_info = mem->process_info;
        unsigned long bo_size = mem->bo->tbo.base.size;
@@ -1355,6 +1364,7 @@ int amdgpu_amdkfd_gpuvm_free_memory_of_gpu(
        }
 
        /* Free the BO*/
+       drm_vma_node_revoke(&mem->bo->tbo.base.vma_node, drm_priv);
        drm_gem_object_put(&mem->bo->tbo.base);
        mutex_destroy(&mem->lock);
        kfree(mem);
@@ -1666,6 +1676,7 @@ int amdgpu_amdkfd_gpuvm_import_dmabuf(struct kgd_dev *kgd,
        struct amdgpu_vm *avm = drm_priv_to_vm(drm_priv);
        struct drm_gem_object *obj;
        struct amdgpu_bo *bo;
+       int ret;
 
        if (dma_buf->ops != &amdgpu_dmabuf_ops)
                /* Can't handle non-graphics buffers */
@@ -1686,6 +1697,12 @@ int amdgpu_amdkfd_gpuvm_import_dmabuf(struct kgd_dev *kgd,
        if (!*mem)
                return -ENOMEM;
 
+       ret = drm_vma_node_allow(&obj->vma_node, drm_priv);
+       if (ret) {
+               kfree(mem);
+               return ret;
+       }
+
        if (size)
                *size = amdgpu_bo_size(bo);
 
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index 43de260b2230..8fc18de7cff4 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -1328,7 +1328,8 @@ static int kfd_ioctl_alloc_memory_of_gpu(struct file *filep,
        return 0;
 
 err_free:
-       amdgpu_amdkfd_gpuvm_free_memory_of_gpu(dev->kgd, (struct kgd_mem *)mem, NULL);
+       amdgpu_amdkfd_gpuvm_free_memory_of_gpu(dev->kgd, (struct kgd_mem *)mem,
+                                              pdd->vm, NULL);
 err_unlock:
        mutex_unlock(&p->mutex);
        return err;
@@ -1365,7 +1366,7 @@ static int kfd_ioctl_free_memory_of_gpu(struct file *filep,
        }
 
        ret = amdgpu_amdkfd_gpuvm_free_memory_of_gpu(dev->kgd,
-                                               (struct kgd_mem *)mem, &size);
+                                       (struct kgd_mem *)mem, pdd->vm, &size);
 
        /* If freeing the buffer failed, leave the handle in place for
         * clean-up during process tear-down.
@@ -1721,7 +1722,8 @@ static int kfd_ioctl_import_dmabuf(struct file *filep,
        return 0;
 
 err_free:
-       amdgpu_amdkfd_gpuvm_free_memory_of_gpu(dev->kgd, (struct kgd_mem *)mem, NULL);
+       amdgpu_amdkfd_gpuvm_free_memory_of_gpu(dev->kgd, (struct kgd_mem *)mem,
+                                              pdd->vm, NULL);
 err_unlock:
        mutex_unlock(&p->mutex);
        dma_buf_put(dmabuf);
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
index bad0ecd6ef87..da452407c4e5 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
@@ -648,7 +648,7 @@ static void kfd_process_free_gpuvm(struct kgd_mem *mem,
        struct kfd_dev *dev = pdd->dev;
 
        amdgpu_amdkfd_gpuvm_unmap_memory_from_gpu(dev->kgd, mem, pdd->vm);
-       amdgpu_amdkfd_gpuvm_free_memory_of_gpu(dev->kgd, mem, NULL);
+       amdgpu_amdkfd_gpuvm_free_memory_of_gpu(dev->kgd, mem, pdd->vm, NULL);
 }
 
 /* kfd_process_alloc_gpuvm - Allocate GPU VM for the KFD process
@@ -712,7 +712,7 @@ static int kfd_process_alloc_gpuvm(struct kfd_process_device *pdd,
        return err;
 
 err_map_mem:
-       amdgpu_amdkfd_gpuvm_free_memory_of_gpu(kdev->kgd, mem, NULL);
+       amdgpu_amdkfd_gpuvm_free_memory_of_gpu(kdev->kgd, mem, pdd->vm, NULL);
 err_alloc_mem:
        *kptr = NULL;
        return err;
@@ -907,7 +907,8 @@ static void kfd_process_device_free_bos(struct kfd_process_device *pdd)
                                peer_pdd->dev->kgd, mem, peer_pdd->vm);
                }
 
-               amdgpu_amdkfd_gpuvm_free_memory_of_gpu(pdd->dev->kgd, mem, NULL);
+               amdgpu_amdkfd_gpuvm_free_memory_of_gpu(pdd->dev->kgd, mem,
+                                                      pdd->vm, NULL);
                kfd_process_device_remove_obj_handle(pdd, id);
        }
 }
</pre>
    </blockquote>
  </body>
</html>