[PATCH] drm/amdgpu/userq: lock and validate wptr BOs before reading their GPU offset on restore
Jesse Zhang <[email protected]>
| Newsgroups | org.freedesktop.lists.amd-gfx |
|---|---|
| Message-ID | <[email protected]> |
amdgpu_userq_vm_validate_and_restore_queue() reads each queue's wptr BO GPU offset via amdgpu_bo_gpu_offset() after only calling amdgpu_ttm_alloc_gart() on it. But the wptr BOs are not part of this VM (their reservation object is their own, not vm->root), so neither amdgpu_vm_validate() nor amdgpu_userq_bo_validate() (which only handles the VM's evicted list) covers them. As a result a wptr BO can still be in TTM_PL_SYSTEM and unreserved when its offset is read, tripping the amdgpu_bo_gpu_offset() sanity checks from the restore worker: ------------[ cut here ]------------ WARNING: amdgpu_object.c:1486 at amdgpu_bo_gpu_offset+0x75/0xa0 [amdgpu], CPU#3: kworker/3:1/116 Workqueue: events amdgpu_userq_restore_worker [amdgpu] RIP: 0010:amdgpu_bo_gpu_offset+0x75/0xa0 [amdgpu] Call Trace: <TASK> amdgpu_userq_vm_validate_and_restore_queue+0x629/0x960 [amdgpu] amdgpu_userq_restore_worker+0xa6/0x180 [amdgpu] process_scheduled_works+0xa6/0x460 worker_thread+0x13c/0x290 kthread+0xfb/0x140 ret_from_fork+0x1b6/0x2b0 ret_from_fork_asm+0x1a/0x30 </TASK> ---[ end trace 0000000000000000 ]--- ------------[ cut here ]------------ WARNING: amdgpu_object.c:1485 at amdgpu_bo_gpu_offset+0x9a/0xa0 [amdgpu], CPU#2: kworker/2:1/127 Workqueue: events amdgpu_userq_restore_worker [amdgpu] RIP: 0010:amdgpu_bo_gpu_offset+0x9a/0xa0 [amdgpu] amdgpu_ttm_alloc_gart() only creates the GART mapping; it does not migrate the BO out of system memory, so the offset read is bogus (the queue would resume with a wrong wptr address). Lock each wptr BO into the drm_exec context and validate it into GTT inside the drm_exec_until_all_locked() block, mirroring what the create path (mes_userq_create_wptr_mapping) already does, so that the offset read later is safe and correct. Signed-off-by: Jesse Zhang <[email protected]> --- drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 26 +++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c index 17cc48d87c4d..59aa4802c111 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c @@ -1054,6 +1054,32 @@ amdgpu_userq_vm_validate_and_restore_queue(struct amdgpu_userq_mgr *uq_mgr) drm_exec_retry_on_contention(&exec); if (unlikely(ret)) goto unlock_all; + + /* + * The per-queue wptr BOs are not part of this VM (their resv is + * their own, not vm->root), so the validation above does not + * cover them. Lock and validate each into GTT here so that + * reading its GPU offset below is safe - matching what the + * create path (mes_userq_create_wptr_mapping) does. + */ + xa_for_each(&uq_mgr->userq_xa, tmp_key, queue) { + struct ttm_operation_ctx wptr_ctx = { false, false }; + + bo = queue->wptr_obj.obj; + if (!bo) + continue; + + ret = drm_exec_prepare_obj(&exec, &bo->tbo.base, + TTM_NUM_MOVE_FENCES + 1); + drm_exec_retry_on_contention(&exec); + if (unlikely(ret)) + goto unlock_all; + + amdgpu_bo_placement_from_domain(bo, bo->allowed_domains); + ret = ttm_bo_validate(&bo->tbo, &bo->placement, &wptr_ctx); + if (unlikely(ret)) + goto unlock_all; + } } if (invalidated) { -- 2.49.0