Re: [PATCH 5/6] drm/amdkfd: Import DMABufs for interop through DRM

["Christian König" <ckoenig.leichtzumerken@gmail.com>]

Am 07.11.23 um 17:58 schrieb Felix Kuehling:
> Use drm_gem_prime_fd_to_handle to import DMABufs for interop. This
> ensures that a GEM handle is created on import and that obj->dma_buf
> will be set and remain set as long as the object is imported into KFD.
>
> Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>

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

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h    |  9 ++-
>   .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  | 64 +++++++++++++------
>   drivers/gpu/drm/amd/amdkfd/kfd_chardev.c      | 15 ++---
>   3 files changed, 52 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> index 4caf8cece028..88a0e0734270 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> @@ -318,11 +318,10 @@ int amdgpu_amdkfd_gpuvm_restore_process_bos(void *process_info,
>   					    struct dma_fence **ef);
>   int amdgpu_amdkfd_gpuvm_get_vm_fault_info(struct amdgpu_device *adev,
>   					      struct kfd_vm_fault_info *info);
> -int amdgpu_amdkfd_gpuvm_import_dmabuf(struct amdgpu_device *adev,
> -				      struct dma_buf *dmabuf,
> -				      uint64_t va, void *drm_priv,
> -				      struct kgd_mem **mem, uint64_t *size,
> -				      uint64_t *mmap_offset);
> +int amdgpu_amdkfd_gpuvm_import_dmabuf_fd(struct amdgpu_device *adev, int fd,
> +					 uint64_t va, void *drm_priv,
> +					 struct kgd_mem **mem, uint64_t *size,
> +					 uint64_t *mmap_offset);
>   int amdgpu_amdkfd_gpuvm_export_dmabuf(struct kgd_mem *mem,
>   				      struct dma_buf **dmabuf);
>   void amdgpu_amdkfd_debug_mem_fence(struct amdgpu_device *adev);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> index 4bb8b5fd7598..1077de8bced2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> @@ -2006,8 +2006,7 @@ int amdgpu_amdkfd_gpuvm_free_memory_of_gpu(
>   
>   	/* Free the BO*/
>   	drm_vma_node_revoke(&mem->bo->tbo.base.vma_node, drm_priv);
> -	if (!mem->is_imported)
> -		drm_gem_handle_delete(adev->kfd.client.file, mem->gem_handle);
> +	drm_gem_handle_delete(adev->kfd.client.file, mem->gem_handle);
>   	if (mem->dmabuf) {
>   		dma_buf_put(mem->dmabuf);
>   		mem->dmabuf = NULL;
> @@ -2363,34 +2362,26 @@ int amdgpu_amdkfd_gpuvm_get_vm_fault_info(struct amdgpu_device *adev,
>   	return 0;
>   }
>   
> -int amdgpu_amdkfd_gpuvm_import_dmabuf(struct amdgpu_device *adev,
> -				      struct dma_buf *dma_buf,
> -				      uint64_t va, void *drm_priv,
> -				      struct kgd_mem **mem, uint64_t *size,
> -				      uint64_t *mmap_offset)
> +static int import_obj_create(struct amdgpu_device *adev,
> +			     struct dma_buf *dma_buf,
> +			     struct drm_gem_object *obj,
> +			     uint64_t va, void *drm_priv,
> +			     struct kgd_mem **mem, uint64_t *size,
> +			     uint64_t *mmap_offset)
>   {
>   	struct amdgpu_vm *avm = drm_priv_to_vm(drm_priv);
> -	struct drm_gem_object *obj;
>   	struct amdgpu_bo *bo;
>   	int ret;
>   
> -	obj = amdgpu_gem_prime_import(adev_to_drm(adev), dma_buf);
> -	if (IS_ERR(obj))
> -		return PTR_ERR(obj);
> -
>   	bo = gem_to_amdgpu_bo(obj);
>   	if (!(bo->preferred_domains & (AMDGPU_GEM_DOMAIN_VRAM |
> -				    AMDGPU_GEM_DOMAIN_GTT))) {
> +				    AMDGPU_GEM_DOMAIN_GTT)))
>   		/* Only VRAM and GTT BOs are supported */
> -		ret = -EINVAL;
> -		goto err_put_obj;
> -	}
> +		return -EINVAL;
>   
>   	*mem = kzalloc(sizeof(struct kgd_mem), GFP_KERNEL);
> -	if (!*mem) {
> -		ret = -ENOMEM;
> -		goto err_put_obj;
> -	}
> +	if (!*mem)
> +		return -ENOMEM;
>   
>   	ret = drm_vma_node_allow(&obj->vma_node, drm_priv);
>   	if (ret)
> @@ -2440,8 +2431,41 @@ int amdgpu_amdkfd_gpuvm_import_dmabuf(struct amdgpu_device *adev,
>   	drm_vma_node_revoke(&obj->vma_node, drm_priv);
>   err_free_mem:
>   	kfree(*mem);
> +	return ret;
> +}
> +
> +int amdgpu_amdkfd_gpuvm_import_dmabuf_fd(struct amdgpu_device *adev, int fd,
> +					 uint64_t va, void *drm_priv,
> +					 struct kgd_mem **mem, uint64_t *size,
> +					 uint64_t *mmap_offset)
> +{
> +	struct drm_gem_object *obj;
> +	uint32_t handle;
> +	int ret;
> +
> +	ret = drm_gem_prime_fd_to_handle(&adev->ddev, adev->kfd.client.file, fd,
> +					 &handle);
> +	if (ret)
> +		return ret;
> +	obj = drm_gem_object_lookup(adev->kfd.client.file, handle);
> +	if (!obj) {
> +		ret = -EINVAL;
> +		goto err_release_handle;
> +	}
> +
> +	ret = import_obj_create(adev, obj->dma_buf, obj, va, drm_priv, mem, size,
> +				mmap_offset);
> +	if (ret)
> +		goto err_put_obj;
> +
> +	(*mem)->gem_handle = handle;
> +
> +	return 0;
> +
>   err_put_obj:
>   	drm_gem_object_put(obj);
> +err_release_handle:
> +	drm_gem_handle_delete(adev->kfd.client.file, handle);
>   	return ret;
>   }
>   
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> index 4417a9863cd0..1a2e9f564b7f 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> @@ -1564,16 +1564,11 @@ static int kfd_ioctl_import_dmabuf(struct file *filep,
>   {
>   	struct kfd_ioctl_import_dmabuf_args *args = data;
>   	struct kfd_process_device *pdd;
> -	struct dma_buf *dmabuf;
>   	int idr_handle;
>   	uint64_t size;
>   	void *mem;
>   	int r;
>   
> -	dmabuf = dma_buf_get(args->dmabuf_fd);
> -	if (IS_ERR(dmabuf))
> -		return PTR_ERR(dmabuf);
> -
>   	mutex_lock(&p->mutex);
>   	pdd = kfd_process_device_data_by_id(p, args->gpu_id);
>   	if (!pdd) {
> @@ -1587,10 +1582,10 @@ static int kfd_ioctl_import_dmabuf(struct file *filep,
>   		goto err_unlock;
>   	}
>   
> -	r = amdgpu_amdkfd_gpuvm_import_dmabuf(pdd->dev->adev, dmabuf,
> -					      args->va_addr, pdd->drm_priv,
> -					      (struct kgd_mem **)&mem, &size,
> -					      NULL);
> +	r = amdgpu_amdkfd_gpuvm_import_dmabuf_fd(pdd->dev->adev, args->dmabuf_fd,
> +						 args->va_addr, pdd->drm_priv,
> +						 (struct kgd_mem **)&mem, &size,
> +						 NULL);
>   	if (r)
>   		goto err_unlock;
>   
> @@ -1601,7 +1596,6 @@ static int kfd_ioctl_import_dmabuf(struct file *filep,
>   	}
>   
>   	mutex_unlock(&p->mutex);
> -	dma_buf_put(dmabuf);
>   
>   	args->handle = MAKE_HANDLE(args->gpu_id, idr_handle);
>   
> @@ -1612,7 +1606,6 @@ static int kfd_ioctl_import_dmabuf(struct file *filep,
>   					       pdd->drm_priv, NULL);
>   err_unlock:
>   	mutex_unlock(&p->mutex);
> -	dma_buf_put(dmabuf);
>   	return r;
>   }
>