On 10/16/26 05:03, Zhang, Jesse(Jie) wrote:
> AMD General
>
>> -----Original Message-----
>> From: Koenig, Christian <[email protected]>
>> Sent: Thursday, October 15, 2026 6:38 PM
>> To: Zhang, Jesse(Jie) <[email protected]>; amd-
>> [email protected]
>> Cc: Deucher, Alexander <[email protected]>; Liang, Prike
>> <[email protected]>; Khatri, Sunil <[email protected]>; Soltani,
>> Shahyan <[email protected]>
>> Subject: Re: [PATCH] drm/amdgpu/userq: map the seq64 memory when
>> restoring user queues
>>
>> On 10/15/26 09:19, Jesse Zhang wrote:
>>> Since the seq64 mapping of a VM only gets its page tables updated by
>>> the CS ioctl, a process which only uses user queues never has its
>>> seq64 memory mapped. USERQ_WAIT hands out the seq64 address of user
>>> queue fences for the GPU to wait on, so a queue waiting for an earlier
>>> job faults on it and hangs.
>>>
>>> A GFX user queue waiting with FENCE_WAIT_MULTI for the already
>>> signaled fence of its previous job (va 0xffffffffffc00000 from
>>> USERQ_WAIT) never gets past the wait:
>>>
>>> amdgpu 0000:03:00.0: [gfxhub] page fault (src_id:0 ring:24 vmid:8
>> pasid:13)
>>> amdgpu 0000:03:00.0: Faulty UTCL2 client ID: CPG (0x6)
>>>
>>> Update the seq64 mapping where the user queues get their VM restored,
>>> which also happens before the first queue of a file is mapped, once
>>> the page tables are validated again, and wait for it like for the
>>> other page table updates. Only do it when the PTEs aren't there yet or
>>> were cleared, so that restoring after every eviction doesn't write them
>>> again.
>>>
>>> Fixes: ae05e3f3d02b ("drm/amdgpu: move amdgpu_vm_bo_update() call
>> into
>>> correct location")
>>> Signed-off-by: Jesse Zhang <[email protected]>
>>
>> The seq64 mapping should just be a normal individual BO mapping, so the
>> extra handling here should be superflous.
>>
>> My educated guess is that this is already fixed fix the patch
>> "drm/amdgpu/userq: actually validate userq BOs" from Benjamin Cheng from
>> a few days ago.
>>
>> Please make sure that you have this patch and re-test if that issue still
>> happens
>> or if we are missing something else.
> Hi Christian,
> Benjamin’s patch, ea9be90a5558 (“drm/amdgpu/userq: actually validate userq
> BOs”), is already included in the source base for this patch.
>
> The gap I see is that the seq64 BO is pinned in GTT and its bo_va remains on
> individual.idle.
> amdgpu_seq64_map() adds the mapping to bo_va->invalids, but
> amdgpu_vm_bo_insert_map() only moves always-valid BOs to needs_update.
Oh, that is completely broken!
With user queues amdgpu_vm_bo_insert_map() needs to do this for all BOs, not
just for the always-valid ones.
That probably also explains issues we have seen after VRAM eviction and/or
suspend/resume.
Good catch we need to fix that instead.
Regards,
Christian.
> Benjamin’s patch only looks at the individual evicted list. so it does not
> pick up this case.
>
> Thanks
> Jesse
>>
>> Thanks,
>> Christian.
>>
>>> ---
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 19
>> +++++++++++++++++++
>>> 1 file changed, 19 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
>>> index 4dca1ab48410..94b9e6890f91 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
>>> @@ -1070,6 +1070,12 @@
>> amdgpu_userq_vm_validate_and_restore_queue(struct amdgpu_userq_mgr
>> *uq_mgr)
>>> if (unlikely(ret))
>>> goto unlock_all;
>>>
>>> + /* The pinned seq64 BO only needs a slot for the rearm */
>>> + ret = drm_exec_prepare_obj(&exec, &adev->seq64.sbo-
>>> tbo.base, 1);
>>> + drm_exec_retry_on_contention(&exec);
>>> + if (unlikely(ret))
>>> + goto unlock_all;
>>> +
>>> /* This validates PDs, PTs and per VM BOs */
>>> ret = amdgpu_vm_validate(adev, vm, NULL,
>>> amdgpu_userq_validate_vm,
>>> @@ -1140,6 +1146,18 @@
>> amdgpu_userq_vm_validate_and_restore_queue(struct amdgpu_userq_mgr
>> *uq_mgr)
>>> if (ret)
>>> goto unlock_all;
>>>
>>> + /*
>>> + * The seq64 mapping is kept up to date by the CS path only, but the
>>> + * queues wait for user queue fences through their seq64 address. Its
>>> + * PTEs only need writing when they aren't there yet or were cleared,
>>> + * e.g. by amdgpu_vm_handle_moved() after a VRAM loss.
>>> + */
>>> + if (!list_empty(&fpriv->seq64_va->invalids) || fpriv->seq64_va-
>>> cleared) {
>>> + ret = amdgpu_vm_bo_update(adev, fpriv->seq64_va, false);
>>> + if (ret)
>>> + goto unlock_all;
>>> + }
>>> +
>>> key = 0;
>>> /* Validate User Ptr BOs */
>>> list_for_each_entry(bo_va, &vm->always_valid.idle, base.vm_status) {
>>> @@ -1203,6 +1221,7 @@
>> amdgpu_userq_vm_validate_and_restore_queue(struct amdgpu_userq_mgr
>> *uq_mgr)
>>> * it explicitly (as the CS path syncs it) before restarting queues.
>>> */
>>> dma_fence_wait(fpriv->prt_va->last_pt_update, false);
>>> + dma_fence_wait(fpriv->seq64_va->last_pt_update, false);
>>> dma_fence_wait(vm->last_update, false);
>>>
>>> xa_for_each(&uq_mgr->userq_xa, tmp_key, queue) {
>
