drm/amdgpu: convert amdgpu_vm_lock_by_pasid() to drm_exec

amdgpu_vm_lock_by_pasid() looks up a VM by PASID and reserves its root
PD with a bare amdgpu_bo_reserve(), returning the still-reserved root to
the caller. A caller that then needs to reserve further BOs (for example
the devcoredump IB dump) ends up nesting reservation_ww_class_mutex
acquires without a ww_acquire_ctx, which lockdep flags as recursive
locking.

Convert the helper to take a drm_exec context and lock the root PD with
drm_exec_lock_obj(). Callers now run it inside a
drm_exec_until_all_locked() loop and can lock additional BOs in the same
ww ticket, so there is no nested ww_mutex acquire.

The drm_exec context holds its own reference on the locked root BO, so
the helper no longer hands a root reference back to the caller: the
root output parameter is dropped, and the transient reference taken
across the PASID lookup is released before returning.

The only existing caller, amdgpu_vm_handle_fault(), is updated
accordingly. Its is_compute_context path, which previously dropped the
root reservation around svm_range_restore_pages() and re-took it, now
finalises the drm_exec context and re-initialises a fresh one; behaviour
is otherwise unchanged.

No functional change intended for the page-fault path.

Reviewed-by: Christian König <christian.koenig@amd.com>
Signed-off-by: Mikhail Gavrilov <mikhail.v.gavrilov@gmail.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
(cherry picked from commit 14682de8ad377bf13ea66e47c26dcfea0b19a21d)
This commit is contained in:
Mikhail Gavrilov
2026-05-29 11:47:38 +05:00
committed by Alex Deucher
parent 75b3e4d049
commit 9920249a52
2 changed files with 58 additions and 35 deletions

View File

@@ -2920,47 +2920,56 @@ int amdgpu_vm_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
} }
/** /**
* amdgpu_vm_lock_by_pasid - return an amdgpu_vm and its root bo from a pasid, if possible. * amdgpu_vm_lock_by_pasid - look up a VM by PASID and lock its root PD
* @adev: amdgpu device pointer * @adev: amdgpu device pointer
* @root: root BO of the VM
* @pasid: PASID of the VM * @pasid: PASID of the VM
* The caller needs to unreserve and unref the root bo on success. * @exec: drm_exec context to lock the root PD in
*
* Must be called from within a drm_exec_until_all_locked() loop; the caller
* runs drm_exec_retry_on_contention() afterwards. The drm_exec context holds
* a reference on the root BO until it is finalised.
*
* Return: the VM on success, or NULL if the PASID has no VM, the VM is being
* torn down, or locking the root PD failed.
*/ */
struct amdgpu_vm *amdgpu_vm_lock_by_pasid(struct amdgpu_device *adev, struct amdgpu_vm *amdgpu_vm_lock_by_pasid(struct amdgpu_device *adev,
struct amdgpu_bo **root, u32 pasid) u32 pasid, struct drm_exec *exec)
{ {
unsigned long irqflags; unsigned long irqflags;
struct amdgpu_bo *root;
struct amdgpu_vm *vm; struct amdgpu_vm *vm;
int r; int r;
xa_lock_irqsave(&adev->vm_manager.pasids, irqflags); xa_lock_irqsave(&adev->vm_manager.pasids, irqflags);
vm = xa_load(&adev->vm_manager.pasids, pasid); vm = xa_load(&adev->vm_manager.pasids, pasid);
*root = vm ? amdgpu_bo_ref(vm->root.bo) : NULL; root = vm ? amdgpu_bo_ref(vm->root.bo) : NULL;
xa_unlock_irqrestore(&adev->vm_manager.pasids, irqflags); xa_unlock_irqrestore(&adev->vm_manager.pasids, irqflags);
if (!*root) if (!root)
return NULL; return NULL;
r = amdgpu_bo_reserve(*root, true); r = drm_exec_lock_obj(exec, &root->tbo.base);
if (r) if (r) {
goto error_unref; amdgpu_bo_unref(&root);
return NULL;
}
/* Double check that the VM still exists */ /* Double check that the VM still exists */
xa_lock_irqsave(&adev->vm_manager.pasids, irqflags); xa_lock_irqsave(&adev->vm_manager.pasids, irqflags);
vm = xa_load(&adev->vm_manager.pasids, pasid); vm = xa_load(&adev->vm_manager.pasids, pasid);
if (vm && vm->root.bo != *root) if (vm && vm->root.bo != root)
vm = NULL; vm = NULL;
xa_unlock_irqrestore(&adev->vm_manager.pasids, irqflags); xa_unlock_irqrestore(&adev->vm_manager.pasids, irqflags);
if (!vm) if (!vm) {
goto error_unlock; drm_exec_unlock_obj(exec, &root->tbo.base);
amdgpu_bo_unref(&root);
return NULL;
}
/* The drm_exec context holds its own reference on the root BO. */
amdgpu_bo_unref(&root);
return vm; return vm;
error_unlock:
amdgpu_bo_unreserve(*root);
error_unref:
amdgpu_bo_unref(root);
return NULL;
} }
/** /**
@@ -2982,33 +2991,49 @@ bool amdgpu_vm_handle_fault(struct amdgpu_device *adev, u32 pasid,
uint64_t ts, bool write_fault) uint64_t ts, bool write_fault)
{ {
bool is_compute_context = false; bool is_compute_context = false;
struct amdgpu_bo *root; struct drm_exec exec;
uint64_t value, flags; uint64_t value, flags;
struct amdgpu_vm *vm; struct amdgpu_vm *vm;
int r; int r;
vm = amdgpu_vm_lock_by_pasid(adev, &root, pasid); drm_exec_init(&exec, 0, 1);
if (!vm) drm_exec_until_all_locked(&exec) {
vm = amdgpu_vm_lock_by_pasid(adev, pasid, &exec);
drm_exec_retry_on_contention(&exec);
if (!vm)
break;
}
if (!vm) {
drm_exec_fini(&exec);
return false; return false;
}
is_compute_context = vm->is_compute_context; is_compute_context = vm->is_compute_context;
if (is_compute_context) { if (is_compute_context) {
/* Unreserve root since svm_range_restore_pages might try to reserve it. */ /* Release the root PD lock since svm_range_restore_pages
/* TODO: rework svm_range_restore_pages so that this isn't necessary. */ * might try to take it.
amdgpu_bo_unreserve(root); * TODO: rework svm_range_restore_pages so that this isn't
* necessary.
*/
drm_exec_fini(&exec);
if (!svm_range_restore_pages(adev, pasid, vmid, if (!svm_range_restore_pages(adev, pasid, vmid,
node_id, addr >> PAGE_SHIFT, ts, write_fault)) { node_id, addr >> PAGE_SHIFT, ts, write_fault))
amdgpu_bo_unref(&root);
return true; return true;
}
amdgpu_bo_unref(&root);
/* Re-acquire the VM lock, could be that the VM was freed in between. */ /* Re-acquire the VM lock, could be that the VM was freed in between. */
vm = amdgpu_vm_lock_by_pasid(adev, &root, pasid); drm_exec_init(&exec, 0, 1);
if (!vm) drm_exec_until_all_locked(&exec) {
vm = amdgpu_vm_lock_by_pasid(adev, pasid, &exec);
drm_exec_retry_on_contention(&exec);
if (!vm)
break;
}
if (!vm) {
drm_exec_fini(&exec);
return false; return false;
}
} }
addr /= AMDGPU_GPU_PAGE_SIZE; addr /= AMDGPU_GPU_PAGE_SIZE;
@@ -3032,7 +3057,7 @@ bool amdgpu_vm_handle_fault(struct amdgpu_device *adev, u32 pasid,
value = 0; value = 0;
} }
r = dma_resv_reserve_fences(root->tbo.base.resv, 1); r = dma_resv_reserve_fences(vm->root.bo->tbo.base.resv, 1);
if (r) { if (r) {
pr_debug("failed %d to reserve fence slot\n", r); pr_debug("failed %d to reserve fence slot\n", r);
goto error_unlock; goto error_unlock;
@@ -3046,12 +3071,10 @@ bool amdgpu_vm_handle_fault(struct amdgpu_device *adev, u32 pasid,
r = amdgpu_vm_update_pdes(adev, vm, true); r = amdgpu_vm_update_pdes(adev, vm, true);
error_unlock: error_unlock:
amdgpu_bo_unreserve(root); drm_exec_fini(&exec);
if (r < 0) if (r < 0)
dev_err(adev->dev, "Can't handle page fault (%d)\n", r); dev_err(adev->dev, "Can't handle page fault (%d)\n", r);
amdgpu_bo_unref(&root);
return false; return false;
} }

View File

@@ -592,7 +592,7 @@ bool amdgpu_vm_handle_fault(struct amdgpu_device *adev, u32 pasid,
bool write_fault); bool write_fault);
struct amdgpu_vm *amdgpu_vm_lock_by_pasid(struct amdgpu_device *adev, struct amdgpu_vm *amdgpu_vm_lock_by_pasid(struct amdgpu_device *adev,
struct amdgpu_bo **root, u32 pasid); u32 pasid, struct drm_exec *exec);
void amdgpu_vm_set_task_info(struct amdgpu_vm *vm); void amdgpu_vm_set_task_info(struct amdgpu_vm *vm);