On Fri, Sep 18, 2026 at 03:02:00PM -0700, Matthew Brost wrote: > With your v5 fix Sashiko pointed out: > > Reviewed-by: Matthew Brost <[email protected]>
Thanks. That fix is in: seqno_recv is now sampled inside pending_lock in xe_tlb_inval_fence_timeout() and passed to xe_devcoredump_gt() by value, so the capture itself - which takes sleeping locks - still runs outside the lock. v5 goes out this week carrying your tag on this patch. Tales Em sex., 18 de set. de 2026 às 19:02, Matthew Brost <[email protected]> escreveu: > > On Thu, Sep 17, 2026 at 01:35:51PM -0300, Tales A. Mendonça wrote: > > TLB invalidation timeouts currently leave no record of the firmware > > state behind: there is no exec queue or job to blame, so nothing calls > > xe_devcoredump() and the GuC log content at the time of the hang is > > lost. > > > > Add xe_devcoredump_gt(), a variant of xe_devcoredump() for hangs that > > are not tied to an exec queue or job. It captures the GuC log and CT > > state of the affected GT, reusing the existing snapshot machinery and > > the "only first snapshot" policy, and hook it up to the TLB invalidation > > timeout path. > > > > This was instrumental in diagnosing GuC TLB invalidation ack stalls on > > ARL (see Link), where the invalidation request is consumed from the H2G > > CTB immediately but the ack G2H only arrives ~2.3s later, after the > > timeout has already fired. > > > > Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/work_items/8678 > > Signed-off-by: Tales A. Mendonça <[email protected]> > > With your v5 fix Sashiko pointed out: > > Reviewed-by: Matthew Brost <[email protected]> > > > --- > > drivers/gpu/drm/xe/xe_devcoredump.c | 46 ++++++++++++++++------------- > > drivers/gpu/drm/xe/xe_devcoredump.h | 15 +++++++--- > > drivers/gpu/drm/xe/xe_tlb_inval.c | 20 +++++++++++++ > > 3 files changed, 56 insertions(+), 25 deletions(-) > > > > diff --git a/drivers/gpu/drm/xe/xe_devcoredump.c > > b/drivers/gpu/drm/xe/xe_devcoredump.c > > index 5f2b90b18f9..6bee8933f95 100644 > > --- a/drivers/gpu/drm/xe/xe_devcoredump.c > > +++ b/drivers/gpu/drm/xe/xe_devcoredump.c > > @@ -74,11 +74,6 @@ static struct xe_device *coredump_to_xe(const struct > > xe_devcoredump *coredump) > > return container_of(coredump, struct xe_device, devcoredump); > > } > > > > -static struct xe_guc *exec_queue_to_guc(struct xe_exec_queue *q) > > -{ > > - return &q->gt->uc.guc; > > -} > > - > > static ssize_t __xe_devcoredump_read(char *buffer, ssize_t count, > > ssize_t start, > > struct xe_devcoredump *coredump) > > @@ -323,40 +318,44 @@ static void xe_devcoredump_deferred_snap_work(struct > > work_struct *work) > > } > > > > static void devcoredump_snapshot(struct xe_devcoredump *coredump, > > + struct xe_gt *gt, > > struct xe_exec_queue *q, > > struct xe_sched_job *job) > > { > > struct xe_devcoredump_snapshot *ss = &coredump->snapshot; > > - struct xe_guc *guc = exec_queue_to_guc(q); > > + struct xe_guc *guc = >->uc.guc; > > const char *process_name = "no process"; > > bool cookie; > > > > ss->snapshot_time = ktime_get_real(); > > ss->boot_time = ktime_get_boottime(); > > > > - if (q->vm && q->vm->xef) { > > + if (q && q->vm && q->vm->xef) { > > process_name = q->vm->xef->process_name; > > ss->pid = q->vm->xef->pid; > > } > > > > strscpy(ss->process_name, process_name); > > > > - ss->gt = q->gt; > > + ss->gt = gt; > > INIT_WORK(&ss->work, xe_devcoredump_deferred_snap_work); > > > > /* keep going if fw fails as we still want to save the memory and SW > > data */ > > - CLASS(xe_force_wake, fw_ref)(gt_to_fw(q->gt), XE_FORCEWAKE_ALL); > > + CLASS(xe_force_wake, fw_ref)(gt_to_fw(gt), XE_FORCEWAKE_ALL); > > > > cookie = dma_fence_begin_signalling(); > > > > ss->guc.log = xe_guc_log_snapshot_capture(&guc->log, true); > > ss->guc.ct = xe_guc_ct_snapshot_capture(&guc->ct); > > - ss->ge = xe_guc_exec_queue_snapshot_capture(q); > > - if (job) > > - ss->job = xe_sched_job_snapshot_capture(job); > > - ss->vm = xe_vm_snapshot_capture(q->vm); > > > > - xe_engine_snapshot_capture_for_queue(q); > > + if (q) { > > + ss->ge = xe_guc_exec_queue_snapshot_capture(q); > > + if (job) > > + ss->job = xe_sched_job_snapshot_capture(job); > > + ss->vm = xe_vm_snapshot_capture(q->vm); > > + > > + xe_engine_snapshot_capture_for_queue(q); > > + } > > > > queue_work(system_dfl_wq, &ss->work); > > > > @@ -364,19 +363,24 @@ static void devcoredump_snapshot(struct > > xe_devcoredump *coredump, > > } > > > > /** > > - * xe_devcoredump - Take the required snapshots and initialize coredump > > device. > > - * @q: The faulty xe_exec_queue, where the issue was detected. > > - * @job: The faulty xe_sched_job, where the issue was detected. > > + * __xe_devcoredump - Take the required snapshots and initialize coredump > > device. > > + * @gt: The GT where the issue was detected. > > + * @q: The faulty xe_exec_queue, where the issue was detected, may be NULL > > for > > + * hangs that are not tied to an exec queue (e.g. TLB invalidation > > + * timeouts); in that case only the GT-level state (GuC log and CT > > state) > > + * is captured. > > + * @job: The faulty xe_sched_job, where the issue was detected, may be > > NULL. > > * @fmt: Printf format + args to describe the reason for the core dump > > * > > * This function should be called at the crash time within the serialized > > * gt_reset. It is skipped if we still have the core dump device available > > * with the information of the 'first' snapshot. > > */ > > -__printf(3, 4) > > -void xe_devcoredump(struct xe_exec_queue *q, struct xe_sched_job *job, > > const char *fmt, ...) > > +__printf(4, 5) > > +void __xe_devcoredump(struct xe_gt *gt, struct xe_exec_queue *q, > > + struct xe_sched_job *job, const char *fmt, ...) > > { > > - struct xe_device *xe = gt_to_xe(q->gt); > > + struct xe_device *xe = gt_to_xe(gt); > > struct xe_devcoredump *coredump = &xe->devcoredump; > > va_list varg; > > > > @@ -394,7 +398,7 @@ void xe_devcoredump(struct xe_exec_queue *q, struct > > xe_sched_job *job, const cha > > coredump->snapshot.reason = kvasprintf(GFP_ATOMIC, fmt, varg); > > va_end(varg); > > > > - devcoredump_snapshot(coredump, q, job); > > + devcoredump_snapshot(coredump, gt, q, job); > > > > drm_info(&xe->drm, "Xe device coredump has been created\n"); > > drm_info(&xe->drm, "Check your > > /sys/class/drm/card%d/device/devcoredump/data\n", > > diff --git a/drivers/gpu/drm/xe/xe_devcoredump.h > > b/drivers/gpu/drm/xe/xe_devcoredump.h > > index 5391a80a4d1..bc4800b3a3f 100644 > > --- a/drivers/gpu/drm/xe/xe_devcoredump.h > > +++ b/drivers/gpu/drm/xe/xe_devcoredump.h > > @@ -11,15 +11,17 @@ > > struct drm_printer; > > struct xe_device; > > struct xe_exec_queue; > > +struct xe_gt; > > struct xe_sched_job; > > > > #ifdef CONFIG_DEV_COREDUMP > > -void xe_devcoredump(struct xe_exec_queue *q, struct xe_sched_job *job, > > const char *fmt, ...); > > +void __xe_devcoredump(struct xe_gt *gt, struct xe_exec_queue *q, > > + struct xe_sched_job *job, const char *fmt, ...); > > int xe_devcoredump_init(struct xe_device *xe); > > #else > > -static inline void xe_devcoredump(struct xe_exec_queue *q, > > - struct xe_sched_job *job, > > - const char *fmt, ...) > > +static inline void __xe_devcoredump(struct xe_gt *gt, struct xe_exec_queue > > *q, > > + struct xe_sched_job *job, > > + const char *fmt, ...) > > { > > } > > > > @@ -29,6 +31,11 @@ static inline int xe_devcoredump_init(struct xe_device > > *xe) > > } > > #endif > > > > +#define xe_devcoredump(_q, _job, _fmt, ...) \ > > + __xe_devcoredump((_q)->gt, _q, _job, _fmt, ##__VA_ARGS__) > > +#define xe_devcoredump_gt(_gt, _fmt, ...) \ > > + __xe_devcoredump(_gt, NULL, NULL, _fmt, ##__VA_ARGS__) > > + > > void xe_print_blob_ascii85(struct drm_printer *p, const char *prefix, char > > suffix, > > const void *blob, size_t offset, size_t size); > > > > diff --git a/drivers/gpu/drm/xe/xe_tlb_inval.c > > b/drivers/gpu/drm/xe/xe_tlb_inval.c > > index 7c3bf32d925..7a0c04fac60 100644 > > --- a/drivers/gpu/drm/xe/xe_tlb_inval.c > > +++ b/drivers/gpu/drm/xe/xe_tlb_inval.c > > @@ -5,6 +5,7 @@ > > > > #include <drm/drm_managed.h> > > > > +#include "xe_devcoredump.h" > > #include "xe_device_types.h" > > #include "xe_force_wake.h" > > #include "xe_gt_stats.h" > > @@ -29,6 +30,12 @@ > > > > #define FENCE_STACK_BIT DMA_FENCE_FLAG_USER_BITS > > > > +/* The frontend is only ever embedded in a GT */ > > +static struct xe_gt *tlb_inval_to_gt(struct xe_tlb_inval *tlb_inval) > > +{ > > + return container_of(tlb_inval, struct xe_gt, tlb_inval); > > +} > > + > > static void xe_tlb_inval_fence_fini(struct xe_tlb_inval_fence *fence) > > { > > if (WARN_ON_ONCE(!fence->tlb_inval)) > > @@ -73,6 +80,7 @@ static void xe_tlb_inval_fence_timeout(struct work_struct > > *work) > > struct xe_device *xe = tlb_inval->xe; > > struct xe_tlb_inval_fence *fence, *next; > > long timeout_delay = tlb_inval->ops->timeout_delay(tlb_inval); > > + int timedout_seqno = 0; > > > > tlb_inval->ops->flush(tlb_inval); > > > > @@ -90,6 +98,8 @@ static void xe_tlb_inval_fence_timeout(struct work_struct > > *work) > > "TLB invalidation fence timeout, seqno=%d recv=%d", > > fence->seqno, tlb_inval->seqno_recv); > > > > + timedout_seqno = fence->seqno; > > + > > fence->base.error = -ETIME; > > xe_tlb_inval_fence_signal(fence); > > } > > @@ -97,6 +107,16 @@ static void xe_tlb_inval_fence_timeout(struct > > work_struct *work) > > queue_delayed_work(tlb_inval->timeout_wq, > > &tlb_inval->fence_tdr, > > timeout_delay); > > spin_unlock_irq(&tlb_inval->pending_lock); > > + > > + /* > > + * Capture the GuC log and CT state so the firmware side of the hang > > + * can be inspected; there is no queue or job to blame here. Must be > > + * outside pending_lock as the capture takes sleeping locks. > > + */ > > + if (timedout_seqno) > > + xe_devcoredump_gt(tlb_inval_to_gt(tlb_inval), > > + "TLB invalidation fence timeout, seqno=%d > > recv=%d", > > + timedout_seqno, tlb_inval->seqno_recv); > > } > > > > /** > > -- > > 2.55.0 > > -- Com os cumprimentos, Tales A. Mendonça talesam.org communitybig.org
