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. > 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 > > >
