[PATCH v2] drm/amdgpu: fix root reservation in amdgpu_vm_handle_fault

[PATCH v2] drm/amdgpu: fix root reservation in amdgpu_vm_handle_fault

[Pierre-Eric Pelloux-Prayer]

svm_range_restore_pages might reserve the root bo so it must
be called after unreserving it.

---
v2:
  - don't modify amdgpu_vm_lock_by_pasid
  - add a TODO
---

Fixes: 32b486e8541c ("drm/amdgpu: extract amdgpu_vm_lock_by_pasid from amdgpu_vm_handle_fault")
Signed-off-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 63156289ae7f..799a1803d941 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -3026,11 +3026,22 @@ bool amdgpu_vm_handle_fault(struct amdgpu_device *adev, u32 pasid,
 
 	is_compute_context = vm->is_compute_context;
 
-	if (is_compute_context && !svm_range_restore_pages(adev, pasid, vmid,
-	    node_id, addr >> PAGE_SHIFT, ts, write_fault)) {
+	if (is_compute_context) {
+		/* Unreserve root since svm_range_restore_pages might try to reserve it. */
+		/* TODO: rework svm_range_restore_pages so that this isn't necessary. */
 		amdgpu_bo_unreserve(root);
+
+		if (!svm_range_restore_pages(adev, pasid, vmid,
+					     node_id, addr >> PAGE_SHIFT, ts, write_fault)) {
+			amdgpu_bo_unref(&root);
+			return true;
+		}
 		amdgpu_bo_unref(&root);
-		return true;
+
+		/* Double check that the VM still exists. */
+		vm = amdgpu_vm_lock_by_pasid(adev, &root, pasid);
+		if (!vm)
+			return false;
 	}
 
 	addr /= AMDGPU_GPU_PAGE_SIZE;
-- 
2.43.0

Re: [PATCH v2] drm/amdgpu: fix root reservation in amdgpu_vm_handle_fault

[Christian König]

On 4/20/26 14:14, Pierre-Eric Pelloux-Prayer wrote:
> svm_range_restore_pages might reserve the root bo so it must
> be called after unreserving it.
> 
> ---
> v2:
>   - don't modify amdgpu_vm_lock_by_pasid
>   - add a TODO
> ---
> 
> Fixes: 32b486e8541c ("drm/amdgpu: extract amdgpu_vm_lock_by_pasid from amdgpu_vm_handle_fault")
> Signed-off-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 17 ++++++++++++++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 63156289ae7f..799a1803d941 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -3026,11 +3026,22 @@ bool amdgpu_vm_handle_fault(struct amdgpu_device *adev, u32 pasid,
>  
>  	is_compute_context = vm->is_compute_context;
>  
> -	if (is_compute_context && !svm_range_restore_pages(adev, pasid, vmid,
> -	    node_id, addr >> PAGE_SHIFT, ts, write_fault)) {
> +	if (is_compute_context) {
> +		/* Unreserve root since svm_range_restore_pages might try to reserve it. */
> +		/* TODO: rework svm_range_restore_pages so that this isn't necessary. */
>  		amdgpu_bo_unreserve(root);
> +
> +		if (!svm_range_restore_pages(adev, pasid, vmid,
> +					     node_id, addr >> PAGE_SHIFT, ts, write_fault)) {
> +			amdgpu_bo_unref(&root);
> +			return true;
> +		}
>  		amdgpu_bo_unref(&root);
> -		return true;
> +
> +		/* Double check that the VM still exists. */

Probably better to write "Re-acquire the VM lock, could be that the VM was freed in between.".

With that done Reviewed-by: Christian König <christian.koenig@amd.com>.

> +		vm = amdgpu_vm_lock_by_pasid(adev, &root, pasid);
> +		if (!vm)
> +			return false;
>  	}
>  
>  	addr /= AMDGPU_GPU_PAGE_SIZE;