AMD General
> -----Original Message-----
> From: Alex Deucher <[email protected]>
> Sent: Wednesday, June 17, 2026 5:47 AM
> To: Zhang, Jesse(Jie) <[email protected]>
> Cc: [email protected]; Deucher, Alexander
> <[email protected]>; Koenig, Christian <[email protected]>
> Subject: Re: [PATCH v2 1/7] drm/amdgpu/userq: add reset helper and identify
> guilty
> user queue
>
> On Thu, Jun 11, 2026 at 12:34 AM Jesse Zhang <[email protected]> wrote:
> >
> > From: Alex Deucher <[email protected]>
>
> Feel free to take over the authorship of this patch. You've done most of the
> work.
>
> >
> > If we get an interrupt for a bad user queue (bad opcode, etc.), add a
> > helper to handle the reset for user queues.
> >
> > v2: squash in fixes
> >
> > Signed-off-by: Alex Deucher <[email protected]>
> > Co-developed-by: Jesse Zhang <[email protected]>
> > Signed-off-by: Jesse Zhang <[email protected]>
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 40
> > +++++++++++++++++++++-- drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h |
> > 12 +++++++
> > 2 files changed, 49 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> > index de6848b4fd67..231ffb29fe5e 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> > @@ -140,9 +140,14 @@ static void amdgpu_userq_hang_detect_work(struct
> work_struct *work)
> >
> > AMDGPU_RESET_TYPE_PER_QUEUE)) {
> > int r;
> >
> > - if (queue->queue_type == AMDGPU_HW_IP_COMPUTE)
> > - r = amdgpu_gfx_reset_mes_compute(adev, NULL, NULL,
> > NULL,
> NULL, NULL);
> > - else
> > + if (queue->queue_type == AMDGPU_HW_IP_COMPUTE) {
> > + struct amdgpu_usermode_queue *guilty_uq;
> > +
> > + /* IRQ-side WRITE_ONCE(guilty,true) hand-off via
> > xchg */
> > + guilty_uq = xchg(&queue->guilty, false) ? queue :
> > NULL;
> > + r = amdgpu_gfx_reset_mes_compute(adev, NULL, NULL,
> > + guilty_uq, NULL,
> > NULL);
> > + } else
> > r = userq_funcs->reset(queue);
> > if (r)
> > gpu_reset = true; @@ -671,6 +676,7 @@
> > amdgpu_userq_create(struct drm_file *filp, union drm_amdgpu_userq *args)
> > }
> >
> > queue->doorbell_index = index;
> > + queue->doorbell_offset = (u32)args->in.doorbell_offset;
> > r = uq_funcs->mqd_create(queue, &args->in);
> > if (r) {
> > drm_file_err(uq_mgr->file, "Failed to create
> > Queue\n"); @@ -1111,6 +1117,34 @@ static void
> amdgpu_userq_restore_worker(struct work_struct *work)
> > dma_fence_put(ev_fence);
> > }
> >
> > +void amdgpu_userq_process_reset_irq(struct amdgpu_device *adev,
> > + u32 pasid, u32 doorbell_offset) {
> > + struct xarray *xa = &adev->userq_doorbell_xa;
> > + struct amdgpu_usermode_queue *queue;
> > + unsigned long flags, idx;
> > +
> > + /*
> > + * CP priv-fault payload is (pasid, src_data[0] & 0x3ff) — the same
> > + * per-process doorbell encoding KFD extracts via
> > + * KFD_CTXID0_DOORBELL_ID_MASK. Find the offending queue by the
> > + * (vm->pasid, doorbell_offset) pair, mark it guilty and fire
> > + * hang_detect immediately (queue_delayed_work() would no-op if the
> > + * work is already armed at submit time).
> > + */
> > + xa_lock_irqsave(xa, flags);
> > + xa_for_each(xa, idx, queue) {
> > + if (queue->vm && queue->vm->pasid == pasid &&
> > + queue->doorbell_offset == doorbell_offset) {
> > + WRITE_ONCE(queue->guilty, true);
> > + mod_delayed_work(adev->reset_domain->wq,
> > + &queue->hang_detect_work, 0);
>
> Why wouldn't we use queue_delayed_work()? If we get multiple interrupts we
> want
> to queue resets for all of them. Additionally why do we need the extra guilty
> tracking?
Thanks Alex, will fix it , and replace mod_delayed_work with
amdgpu_userq_start_hang_detect_work which will call queue_delayed_work.
About the guilty tracking:
the reason we pass the queue to mes_userq_reset() directly (rather than relying
on MES detect) is that for the bad-opcode case we've seen,
the queue is clearly broken but amdgpu_mes_detect_and_reset_hung_queues() does
not report it as hung.
So the HW priv-fault IRQ is the only reliable signal for which queue to reset.
Jesse
>
> Alex
>
> > + break;
> > + }
> > + }
> > + xa_unlock_irqrestore(xa, flags); }
> > +
> > static int
> > amdgpu_userq_evict_all(struct amdgpu_userq_mgr *uq_mgr) { diff --git
> > a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h
> > index 9df1b78407f5..a55d57e2c169 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h
> > @@ -53,6 +53,7 @@ struct amdgpu_usermode_queue {
> > enum amdgpu_userq_state state;
> > uint64_t doorbell_handle;
> > uint64_t doorbell_index;
> > + u32 doorbell_offset;
> > uint64_t flags;
> > struct amdgpu_mqd_prop *userq_prop;
> > struct amdgpu_userq_mgr *userq_mgr; @@ -86,6 +87,7 @@ struct
> > amdgpu_usermode_queue {
> > * Delayed work which runs when userq_fences time out.
> > */
> > struct delayed_work hang_detect_work;
> > + bool guilty;
> > struct kref refcount;
> >
> > union {
> > @@ -176,6 +178,16 @@ int amdgpu_userq_post_reset(struct amdgpu_device
> > *adev, bool vram_lost); void
> > amdgpu_userq_start_hang_detect_work(struct amdgpu_usermode_queue
> > *queue); void amdgpu_userq_process_fence_irq(struct amdgpu_device
> > *adev, u32 doorbell);
> >
> > +/*
> > + * CP packs the per-process doorbell_id of the queue in
> > + * CTXID0[9:0] on priv-fault (same encoding KFD uses via
> > + * KFD_CTXID0_DOORBELL_ID_MASK)
> > + */
> > +#define AMDGPU_CTXID0_DOORBELL_ID_MASK 0x3ff
> > +
> > +void amdgpu_userq_process_reset_irq(struct amdgpu_device *adev,
> > + u32 pasid, u32 doorbell_offset);
> > +
> > int amdgpu_userq_input_va_validate(struct amdgpu_device *adev,
> > struct amdgpu_usermode_queue *queue,
> > u64 addr, u64 expected_size, u64
> > *va_out);
> > --
> > 2.49.0
> >
