[PATCH 3/4] drm/amdkfd: Allow access for mmapping KFD BOs
Felix Kuehling
felix.kuehling at amd.com
Wed Apr 14 15:46:31 UTC 2021
Am 2021-04-14 um 11:37 a.m. schrieb philip yang:
>
>
> On 2021-04-07 7:12 p.m., Felix Kuehling wrote:
>> 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.
>
> 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.
>
The drm file handle is used for CPU mapping of BOs using mmap by the
Thunk. It uses on the file handle of the GPU where the BO was allocated.
It doesn't matter what other GPUs map the BO in their device page table.
Therefore we don't need to call drm_vma_node_allow in
amdgpu_amdkfd_gpuvm_map_memory_to_gpu.
I will add an explanation in the commit description about how user mode
depends on this access permission.
Regards,
Felix
> Regards,
>
> Philip
>
>> Signed-off-by: Felix Kuehling <Felix.Kuehling at amd.com>
>> ---
>> 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);
>
> pr_debug("Failed to allow vma node access. ret %d\n", ret);
>
> It's within 80 columns.
>
> Philip
>
>> + 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);
>> }
>> }
More information about the dri-devel
mailing list