Re: [PATCH 3/4] drm/amdkfd: Allow access for mmapping KFD BOs

[Christian König <ckoenig.leichtzumerken@xxxxxxxxx>]

Am 08.04.21 um 01:12 schrieb Felix Kuehling:
> 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.

Ok, double checking the code that makes sense.

> Signed-off-by: Felix Kuehling <Felix.Kuehling@xxxxxxx>

Acked-by: Christian König <christian.koenig@xxxxxxx>

> ---
>   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);
> +		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);
>   	}
>   }

_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx