Re: [RFC PATCH 1/3] drm/xe: Capture devcoredump on TLB invalidation timeout

On Mon, Aug 03, 2026 at 11:14:39PM -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.
> 

Thanks for doing this. A couple suggestions.

> Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/work_items/8678
> Signed-off-by: Tales A. Mendonça <[email protected]>
> ---
>  drivers/gpu/drm/xe/xe_devcoredump.c | 68 +++++++++++++++++++++++++++++
>  drivers/gpu/drm/xe/xe_devcoredump.h |  6 +++
>  drivers/gpu/drm/xe/xe_tlb_inval.c   | 20 +++++++++
>  3 files changed, 94 insertions(+)
> 
> diff --git a/drivers/gpu/drm/xe/xe_devcoredump.c b/drivers/gpu/drm/xe/xe_devcoredump.c
> index 5f2b90b18f9..0ccaed176a4 100644
> --- a/drivers/gpu/drm/xe/xe_devcoredump.c
> +++ b/drivers/gpu/drm/xe/xe_devcoredump.c
> @@ -403,6 +403,74 @@ void xe_devcoredump(struct xe_exec_queue *q, struct xe_sched_job *job, const cha
>  	mutex_unlock(&coredump->lock);
>  }
>  
> +static void devcoredump_snapshot_gt(struct xe_devcoredump *coredump,
> +				    struct xe_gt *gt)
> +{
> +	struct xe_devcoredump_snapshot *ss = &coredump->snapshot;
> +	struct xe_guc *guc = &gt->uc.guc;
> +	bool cookie;
> +
> +	ss->snapshot_time = ktime_get_real();
> +	ss->boot_time = ktime_get_boottime();
> +
> +	strscpy(ss->process_name, "no process");
> +
> +	ss->gt = gt;
> +	INIT_WORK(&ss->work, xe_devcoredump_deferred_snap_work);
> +
> +	/* keep going if fw fails as we still want to save the SW data */
> +	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);
> +
> +	queue_work(system_dfl_wq, &ss->work);
> +
> +	dma_fence_end_signalling(cookie);
> +}
> +
> +/**
> + * xe_devcoredump_gt - Take GT-level snapshots and initialize coredump device.
> + * @gt: The GT where the issue was detected.
> + * @fmt: Printf format + args to describe the reason for the core dump
> + *
> + * Variant of xe_devcoredump() for hangs that are not tied to an exec queue
> + * or job, e.g. TLB invalidation timeouts. Captures the GuC log and CT state
> + * of @gt so the firmware side of the hang can be inspected. Skipped if a
> + * coredump is already captured, same as xe_devcoredump().
> + */
> +__printf(2, 3)
> +void xe_devcoredump_gt(struct xe_gt *gt, const char *fmt, ...)

I'd unify these functions with the existing
xe_devcoredump/devcoredump_snapshot by adding a GT argument to both and
teaching those functions that 'q' can be NULL. Then replace
s/xe_devcoredump/__xe_devcoredump/ and add wrapper macros
xe_devcoredump and xe_devcoredump_gt in the header file.

More below.

> +{
> +	struct xe_device *xe = gt_to_xe(gt);
> +	struct xe_devcoredump *coredump = &xe->devcoredump;
> +	va_list varg;
> +
> +	mutex_lock(&coredump->lock);
> +
> +	if (coredump->captured) {
> +		drm_dbg(&xe->drm, "Multiple hangs are occurring, but only the first snapshot was taken\n");
> +		mutex_unlock(&coredump->lock);
> +		return;
> +	}
> +
> +	coredump->captured = true;
> +
> +	va_start(varg, fmt);
> +	coredump->snapshot.reason = kvasprintf(GFP_ATOMIC, fmt, varg);
> +	va_end(varg);
> +
> +	devcoredump_snapshot_gt(coredump, gt);
> +
> +	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",
> +		 xe->drm.primary->index);
> +
> +	mutex_unlock(&coredump->lock);
> +}
> +
>  static void xe_driver_devcoredump_fini(void *arg)
>  {
>  	struct drm_device *drm = arg;
> diff --git a/drivers/gpu/drm/xe/xe_devcoredump.h b/drivers/gpu/drm/xe/xe_devcoredump.h
> index 5391a80a4d1..f071bd11f24 100644
> --- a/drivers/gpu/drm/xe/xe_devcoredump.h
> +++ b/drivers/gpu/drm/xe/xe_devcoredump.h
> @@ -11,10 +11,12 @@
>  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_gt(struct xe_gt *gt, const char *fmt, ...);

This what I'm suggesting for a header...

void __xe_devcoredump(struct xe_gt *gt, struct xe_exec_queue *q,
                     struct xe_sched_job *job, const char *fmt, ...);
#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__)

I think we need wrapper macros rather than inline wrappers because of
how .../##__VA_ARGS__ work.

Matt

>  int xe_devcoredump_init(struct xe_device *xe);
>  #else
>  static inline void xe_devcoredump(struct xe_exec_queue *q,
> @@ -23,6 +25,10 @@ static inline void xe_devcoredump(struct xe_exec_queue *q,
>  {
>  }
>  
> +static inline void xe_devcoredump_gt(struct xe_gt *gt, const char *fmt, ...)
> +{
> +}
> +
>  static inline int xe_devcoredump_init(struct xe_device *xe)
>  {
>  	return 0;
> diff --git a/drivers/gpu/drm/xe/xe_tlb_inval.c b/drivers/gpu/drm/xe/xe_tlb_inval.c
> index bbd21d39306..833fb92cd3e 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
>