On 8/17/26 09:05, Jesse Zhang wrote:
> On resume, amdgpu_userq_vm_validate_and_restore_queue() updates each queue's
> wptr GPU address via amdgpu_bo_gpu_offset().
>
> WPTR BOs are VM-mapped, but each BO has its own reservation object and is not
> implicitly covered by the VM validation path here. This can leave offset reads
> without proper BO locking/placement state and trigger WARN_ONs.
> ------------[ 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]
>
> Add each queue's WPTR BO to the drm_exec ww context and validate it to its
> allowed placement before the later offset update.
>
> Signed-off-by: Jesse Zhang <[email protected]>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 24 +++++++++++++++++++++++
> 1 file changed, 24 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> index 17cc48d87c4d..ab8fc14a235b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> @@ -1054,6 +1054,30 @@ 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;
> +
> + /*
> + * WPTR BOs are VM-mapped, but each BO has its own reservation
> + * object. Lock them into this drm_exec ww context so the later
> + * amdgpu_bo_gpu_offset() reads are done with the BO resv
> locked.
> + */
> + 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;
> + }
Mhm, something is very wrong here and that is clearly not the right solution.
At this point all BOs which are part of the VM should already be locked and
validated and that include the WPTR BOs.
So you are just working around a bug somewhere else.
Regards,
Christian.
> }
>
> if (invalidated) {
