On Wed, Jun 17, 2026 at 4:53 AM Zhang, Jesse(Jie) <[email protected]> wrote:
>
> AMD General
>
> > -----Original Message-----
> > From: amd-gfx <[email protected]> On Behalf Of Zhang,
> > Jesse(Jie)
> > Sent: Wednesday, June 17, 2026 1:52 PM
> > To: Alex Deucher <[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
> >
> > 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.
> >
> Think more about it. Maybe we can drop the per-queue guilty flag and always
> pass the queue into the reset path,
> regardless of whether the reset is triggered by a userq fence timeout (the
> hang-detect work start when the fence is initialized)
> or by a hung queue reported through an error interrupt.
RIght. We know which queue is guilty from the interrupt vector. The
work item is hung off the queue so we can just pass that queue in
directly in amdgpu_userq_hang_detect_work().
Alex
> > 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
> > > >
