[PATCH 2/2] drm/amdgpu: separate out vm pasid assignment
Felix Kuehling
felix.kuehling at amd.com
Wed Jun 30 21:57:40 UTC 2021
On 2021-06-29 11:19 a.m., Nirmoy Das wrote:
> Use new helper function amdgpu_vm_set_pasid() to
> assign vm pasid value. This also ensures that we don't free
> a pasid from vm code as pasids are allocated somewhere else.
>
> Signed-off-by: Nirmoy Das <nirmoy.das at amd.com>
Christian's comments notwithstanding, the series is
Acked-by: Felix Kuehling <Felix.Kuehling at amd.com>
> ---
> .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 13 ++++++++-
> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 10 +++++--
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 28 +++----------------
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 4 +--
> 4 files changed, 26 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> index f96598279593..3a2ac7f66bbd 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> @@ -1287,11 +1287,22 @@ int amdgpu_amdkfd_gpuvm_acquire_process_vm(struct kgd_dev *kgd,
> if (avm->process_info)
> return -EINVAL;
>
> + /* Free the original amdgpu allocated pasid,
> + * will be replaced with kfd allocated pasid.
> + */
> + if (avm->pasid) {
> + amdgpu_pasid_free(avm->pasid);
> + amdgpu_vm_set_pasid(adev, avm, 0);
> + }
> +
> /* Convert VM into a compute VM */
> - ret = amdgpu_vm_make_compute(adev, avm, pasid);
> + ret = amdgpu_vm_make_compute(adev, avm);
> if (ret)
> return ret;
>
> + ret = amdgpu_vm_set_pasid(adev, avm, pasid);
> + if (ret)
> + return ret;
> /* Initialize KFD part of the VM and process info */
> ret = init_kfd_vm(avm, process_info, ef);
> if (ret)
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> index cbb932f97355..66bf8b0c56bb 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> @@ -1178,10 +1178,14 @@ int amdgpu_driver_open_kms(struct drm_device *dev, struct drm_file *file_priv)
> pasid = 0;
> }
>
> - r = amdgpu_vm_init(adev, &fpriv->vm, pasid);
> + r = amdgpu_vm_init(adev, &fpriv->vm);
> if (r)
> goto error_pasid;
>
> + r = amdgpu_vm_set_pasid(adev, &fpriv->vm, pasid);
> + if (r)
> + goto error_vm;
> +
> fpriv->prt_va = amdgpu_vm_bo_add(adev, &fpriv->vm, NULL);
> if (!fpriv->prt_va) {
> r = -ENOMEM;
> @@ -1209,8 +1213,10 @@ int amdgpu_driver_open_kms(struct drm_device *dev, struct drm_file *file_priv)
> amdgpu_vm_fini(adev, &fpriv->vm);
>
> error_pasid:
> - if (pasid)
> + if (pasid) {
> amdgpu_pasid_free(pasid);
> + amdgpu_vm_set_pasid(adev, &fpriv->vm, 0);
> + }
>
> kfree(fpriv);
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index fd92ff27931a..1dccede6387e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -2899,14 +2899,13 @@ long amdgpu_vm_wait_idle(struct amdgpu_vm *vm, long timeout)
> *
> * @adev: amdgpu_device pointer
> * @vm: requested vm
> - * @pasid: Process address space identifier
> *
> * Init @vm fields.
> *
> * Returns:
> * 0 for success, error for failure.
> */
> -int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm, u32 pasid)
> +int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm)
> {
> struct amdgpu_bo *root_bo;
> struct amdgpu_bo_vm *root;
> @@ -2980,10 +2979,6 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm, u32 pasid)
>
> amdgpu_bo_unreserve(vm->root.bo);
>
> - r = amdgpu_vm_set_pasid(adev, vm, pasid);
> - if (r)
> - goto error_free_root;
> -
> INIT_KFIFO(vm->faults);
>
> return 0;
> @@ -3039,7 +3034,6 @@ static int amdgpu_vm_check_clean_reserved(struct amdgpu_device *adev,
> *
> * @adev: amdgpu_device pointer
> * @vm: requested vm
> - * @pasid: pasid to use
> *
> * This only works on GFX VMs that don't have any BOs added and no
> * page tables allocated yet.
> @@ -3047,7 +3041,6 @@ static int amdgpu_vm_check_clean_reserved(struct amdgpu_device *adev,
> * Changes the following VM parameters:
> * - use_cpu_for_update
> * - pte_supports_ats
> - * - pasid (old PASID is released, because compute manages its own PASIDs)
> *
> * Reinitializes the page directory to reflect the changed ATS
> * setting.
> @@ -3055,8 +3048,7 @@ static int amdgpu_vm_check_clean_reserved(struct amdgpu_device *adev,
> * Returns:
> * 0 for success, -errno for errors.
> */
> -int amdgpu_vm_make_compute(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> - u32 pasid)
> +int amdgpu_vm_make_compute(struct amdgpu_device *adev, struct amdgpu_vm *vm)
> {
> bool pte_support_ats = (adev->asic_type == CHIP_RAVEN);
> int r;
> @@ -3070,16 +3062,6 @@ int amdgpu_vm_make_compute(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> if (r)
> goto unreserve_bo;
>
> - /* Free the original amdgpu allocated pasid,
> - * will be replaced with kfd allocated pasid.
> - */
> - if (vm->pasid)
> - amdgpu_pasid_free(vm->pasid);
> -
> - r = amdgpu_vm_set_pasid(adev, vm, pasid);
> - if (r)
> - goto unreserve_bo;
> -
> /* Check if PD needs to be reinitialized and do it before
> * changing any other state, in case it fails.
> */
> @@ -3089,7 +3071,7 @@ int amdgpu_vm_make_compute(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> to_amdgpu_bo_vm(vm->root.bo),
> false);
> if (r)
> - goto free_pasid_entry;
> + goto unreserve_bo;
> }
>
> /* Update VM state */
> @@ -3106,7 +3088,7 @@ int amdgpu_vm_make_compute(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> r = amdgpu_bo_sync_wait(vm->root.bo,
> AMDGPU_FENCE_OWNER_UNDEFINED, true);
> if (r)
> - goto free_pasid_entry;
> + goto unreserve_bo;
>
> vm->update_funcs = &amdgpu_vm_cpu_funcs;
> } else {
> @@ -3121,8 +3103,6 @@ int amdgpu_vm_make_compute(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>
> goto unreserve_bo;
>
> -free_pasid_entry:
> - amdgpu_vm_set_pasid(adev, vm, 0);
> unreserve_bo:
> amdgpu_bo_unreserve(vm->root.bo);
> return r;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> index 8e8bc132e590..a0bceccaa6b5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> @@ -378,8 +378,8 @@ int amdgpu_vm_set_pasid(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> unsigned long pasid);
>
> long amdgpu_vm_wait_idle(struct amdgpu_vm *vm, long timeout);
> -int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm, u32 pasid);
> -int amdgpu_vm_make_compute(struct amdgpu_device *adev, struct amdgpu_vm *vm, u32 pasid);
> +int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm);
> +int amdgpu_vm_make_compute(struct amdgpu_device *adev, struct amdgpu_vm *vm);
> void amdgpu_vm_release_compute(struct amdgpu_device *adev, struct amdgpu_vm *vm);
> void amdgpu_vm_fini(struct amdgpu_device *adev, struct amdgpu_vm *vm);
> void amdgpu_vm_get_pd_bo(struct amdgpu_vm *vm,
More information about the amd-gfx
mailing list