[PATCH 1/2] drm/amdgpu: Set pasid for compute vm

Felix Kuehling felix.kuehling at amd.com
Tue Aug 28 19:13:11 UTC 2018


See one more comment inline. With that fixed, the series is Reviewed-by:
Felix Kuehling <Felix.Kuehling at amd.com>

Regards,
  Felix

On 2018-08-28 01:34 PM, Oak Zeng wrote:
> To make a amdgpu vm to a compute vm, the old pasid will be freed and
> replaced with a pasid managed by kfd. Kfd can't reuse original pasid
> allocated by amdgpu because kfd uses different pasid policy with amdgpu.
> For example, all graphic devices share one same pasid in a process.
>
> Signed-off-by: Oak Zeng <Oak.Zeng at amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h       |  6 ++---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 10 +++----
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c           | 33 +++++++++++++++++++++---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h           |  2 +-
>  drivers/gpu/drm/amd/amdkfd/kfd_process.c         |  4 +--
>  drivers/gpu/drm/amd/include/kgd_kfd_interface.h  |  5 ++--
>  6 files changed, 43 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> index a8418a3..e1f6454 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> @@ -155,11 +155,11 @@ uint64_t amdgpu_amdkfd_get_vram_usage(struct kgd_dev *kgd);
>  	})
>  
>  /* GPUVM API */
> -int amdgpu_amdkfd_gpuvm_create_process_vm(struct kgd_dev *kgd, void **vm,
> -					void **process_info,
> +int amdgpu_amdkfd_gpuvm_create_process_vm(struct kgd_dev *kgd, unsigned int pasid,
> +					void **vm, void **process_info,
>  					struct dma_fence **ef);
>  int amdgpu_amdkfd_gpuvm_acquire_process_vm(struct kgd_dev *kgd,
> -					struct file *filp,
> +					struct file *filp, unsigned int pasid,
>  					void **vm, void **process_info,
>  					struct dma_fence **ef);
>  void amdgpu_amdkfd_gpuvm_destroy_cb(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 7eadc58..0980a1f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> @@ -1003,8 +1003,8 @@ static int init_kfd_vm(struct amdgpu_vm *vm, void **process_info,
>  	return ret;
>  }
>  
> -int amdgpu_amdkfd_gpuvm_create_process_vm(struct kgd_dev *kgd, void **vm,
> -					  void **process_info,
> +int amdgpu_amdkfd_gpuvm_create_process_vm(struct kgd_dev *kgd, unsigned int pasid,
> +					  void **vm, void **process_info,
>  					  struct dma_fence **ef)
>  {
>  	struct amdgpu_device *adev = get_amdgpu_device(kgd);
> @@ -1016,7 +1016,7 @@ int amdgpu_amdkfd_gpuvm_create_process_vm(struct kgd_dev *kgd, void **vm,
>  		return -ENOMEM;
>  
>  	/* Initialize AMDGPU part of the VM */
> -	ret = amdgpu_vm_init(adev, new_vm, AMDGPU_VM_CONTEXT_COMPUTE, 0);
> +	ret = amdgpu_vm_init(adev, new_vm, AMDGPU_VM_CONTEXT_COMPUTE, pasid);
>  	if (ret) {
>  		pr_err("Failed init vm ret %d\n", ret);
>  		goto amdgpu_vm_init_fail;
> @@ -1039,7 +1039,7 @@ int amdgpu_amdkfd_gpuvm_create_process_vm(struct kgd_dev *kgd, void **vm,
>  }
>  
>  int amdgpu_amdkfd_gpuvm_acquire_process_vm(struct kgd_dev *kgd,
> -					   struct file *filp,
> +					   struct file *filp, unsigned int pasid,
>  					   void **vm, void **process_info,
>  					   struct dma_fence **ef)
>  {
> @@ -1054,7 +1054,7 @@ int amdgpu_amdkfd_gpuvm_acquire_process_vm(struct kgd_dev *kgd,
>  		return -EINVAL;
>  
>  	/* Convert VM into a compute VM */
> -	ret = amdgpu_vm_make_compute(adev, avm);
> +	ret = amdgpu_vm_make_compute(adev, avm, pasid);
>  	if (ret)
>  		return ret;
>  
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 662aec5..d8a99f4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -2690,7 +2690,7 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>   * Returns:
>   * 0 for success, -errno for errors.
>   */
> -int amdgpu_vm_make_compute(struct amdgpu_device *adev, struct amdgpu_vm *vm)
> +int amdgpu_vm_make_compute(struct amdgpu_device *adev, struct amdgpu_vm *vm, unsigned int pasid)
>  {
>  	bool pte_support_ats = (adev->asic_type == CHIP_RAVEN);
>  	int r;
> @@ -2702,7 +2702,20 @@ int amdgpu_vm_make_compute(struct amdgpu_device *adev, struct amdgpu_vm *vm)
>  	/* Sanity checks */
>  	if (!RB_EMPTY_ROOT(&vm->va.rb_root) || vm->root.entries) {
>  		r = -EINVAL;
> -		goto error;
> +		goto unreserve_bo;
> +	}
> +
> +	if (pasid) {
> +		unsigned long flags;
> +
> +		spin_lock_irqsave(&adev->vm_manager.pasid_lock, flags);
> +		r = idr_alloc(&adev->vm_manager.pasid_idr, vm, pasid, pasid + 1,
> +			      GFP_ATOMIC);
> +		spin_unlock_irqrestore(&adev->vm_manager.pasid_lock, flags);
> +
> +		if (r == -ENOSPC)
> +			goto unreserve_bo;
> +		r = 0;
>  	}
>  
>  	/* Check if PD needs to be reinitialized and do it before
> @@ -2713,7 +2726,7 @@ int amdgpu_vm_make_compute(struct amdgpu_device *adev, struct amdgpu_vm *vm)
>  			       adev->vm_manager.root_level,
>  			       pte_support_ats);
>  		if (r)
> -			goto error;
> +			goto free_idr;
>  	}
>  
>  	/* Update VM state */
> @@ -2732,10 +2745,22 @@ int amdgpu_vm_make_compute(struct amdgpu_device *adev, struct amdgpu_vm *vm)
>  		idr_remove(&adev->vm_manager.pasid_idr, vm->pasid);
>  		spin_unlock_irqrestore(&adev->vm_manager.pasid_lock, flags);
>  
> +		/* Free the original amdgpu allocated pasid
> +		 * Will be replaced with kfd allocated pasid
> +		 */
> +		amdgpu_pasid_free(vm->pasid);
>  		vm->pasid = 0;
>  	}
>  
> -error:
> +	if (pasid)
> +		vm->pasid = pasid;
> +
> +	goto unreserve_bo;
> +
> +free_idr:
> +	if (pasid)
> +		idr_remove(&adev->vm_manager.pasid_idr, pasid);

This needs to be protected by adev->vm_manager.pasid_lock.

> +unreserve_bo:
>  	amdgpu_bo_unreserve(vm->root.base.bo);
>  	return r;
>  }
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> index 1162c2b..c68945d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> @@ -294,7 +294,7 @@ void amdgpu_vm_manager_init(struct amdgpu_device *adev);
>  void amdgpu_vm_manager_fini(struct amdgpu_device *adev);
>  int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>  		   int vm_context, unsigned int pasid);
> -int amdgpu_vm_make_compute(struct amdgpu_device *adev, struct amdgpu_vm *vm);
> +int amdgpu_vm_make_compute(struct amdgpu_device *adev, struct amdgpu_vm *vm, unsigned int pasid);
>  void amdgpu_vm_fini(struct amdgpu_device *adev, struct amdgpu_vm *vm);
>  bool amdgpu_vm_pasid_fault_credit(struct amdgpu_device *adev,
>  				  unsigned int pasid);
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> index 1d80b4f..5f559c5 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> @@ -685,11 +685,11 @@ int kfd_process_device_init_vm(struct kfd_process_device *pdd,
>  
>  	if (drm_file)
>  		ret = dev->kfd2kgd->acquire_process_vm(
> -			dev->kgd, drm_file,
> +			dev->kgd, drm_file, p->pasid,
>  			&pdd->vm, &p->kgd_process_info, &p->ef);
>  	else
>  		ret = dev->kfd2kgd->create_process_vm(
> -			dev->kgd, &pdd->vm, &p->kgd_process_info, &p->ef);
> +			dev->kgd, p->pasid, &pdd->vm, &p->kgd_process_info, &p->ef);
>  	if (ret) {
>  		pr_err("Failed to create process VM object\n");
>  		return ret;
> diff --git a/drivers/gpu/drm/amd/include/kgd_kfd_interface.h b/drivers/gpu/drm/amd/include/kgd_kfd_interface.h
> index 5733fbe..1632869 100644
> --- a/drivers/gpu/drm/amd/include/kgd_kfd_interface.h
> +++ b/drivers/gpu/drm/amd/include/kgd_kfd_interface.h
> @@ -346,10 +346,11 @@ struct kfd2kgd_calls {
>  			struct kfd_cu_info *cu_info);
>  	uint64_t (*get_vram_usage)(struct kgd_dev *kgd);
>  
> -	int (*create_process_vm)(struct kgd_dev *kgd, void **vm,
> +	int (*create_process_vm)(struct kgd_dev *kgd, unsigned int pasid, void **vm,
>  			void **process_info, struct dma_fence **ef);
>  	int (*acquire_process_vm)(struct kgd_dev *kgd, struct file *filp,
> -			void **vm, void **process_info, struct dma_fence **ef);
> +			unsigned int pasid, void **vm, void **process_info,
> +			struct dma_fence **ef);
>  	void (*destroy_process_vm)(struct kgd_dev *kgd, void *vm);
>  	uint32_t (*get_process_page_dir)(void *vm);
>  	void (*set_vm_context_page_table_base)(struct kgd_dev *kgd,



More information about the amd-gfx mailing list