[PATCH 2/2] drm/amdgpu: separate out vm pasid assignment

2021-07-02 Thread Nirmoy Das
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 
Acked-by: Felix Kuehling 
---
 .../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, >vm, pasid);
+   r = amdgpu_vm_init(adev, >vm);
if (r)
goto error_pasid;
 
+   r = amdgpu_vm_set_pasid(adev, >vm, pasid);
+   if (r)
+   goto error_vm;
+
fpriv->prt_va = amdgpu_vm_bo_add(adev, >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, >vm);
 
 error_pasid:
-   if (pasid)
+   if (pasid) {
amdgpu_pasid_free(pasid);
+   amdgpu_vm_set_pasid(adev, >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 0f82df5bfa7a..565c8c59c995 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 

Re: [PATCH 2/2] drm/amdgpu: separate out vm pasid assignment

2021-06-30 Thread Felix Kuehling

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 


Christian's comments notwithstanding, the series is

Acked-by: Felix Kuehling 



---
  .../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, >vm, pasid);

+   r = amdgpu_vm_init(adev, >vm);
if (r)
goto error_pasid;
  
+	r = amdgpu_vm_set_pasid(adev, >vm, pasid);

+   if (r)
+   goto error_vm;
+
fpriv->prt_va = amdgpu_vm_bo_add(adev, >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, >vm);
  
  error_pasid:

-   if (pasid)
+   if (pasid) {
amdgpu_pasid_free(pasid);
+   amdgpu_vm_set_pasid(adev, >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 

[PATCH 2/2] drm/amdgpu: separate out vm pasid assignment

2021-06-29 Thread Nirmoy Das
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 
---
 .../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, >vm, pasid);
+   r = amdgpu_vm_init(adev, >vm);
if (r)
goto error_pasid;
 
+   r = amdgpu_vm_set_pasid(adev, >vm, pasid);
+   if (r)
+   goto error_vm;
+
fpriv->prt_va = amdgpu_vm_bo_add(adev, >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, >vm);
 
 error_pasid:
-   if (pasid)
+   if (pasid) {
amdgpu_pasid_free(pasid);
+   amdgpu_vm_set_pasid(adev, >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.
-*/
-