[PATCH v4 0/3] drm/xe: fix GuC TLB invalidation ack stalls on ARL (Wa_22016122933)

[PATCH v4 0/3] drm/xe: fix GuC TLB invalidation ack stalls on ARL (Wa_22016122933)

[Tales A. Mendonça]

Hi,

v4 of the TLB invalidation ack stall fix for ARL, rebased on today's
drm-tip. Tracked in:

  https://gitlab.freedesktop.org/drm/xe/kernel/-/work_items/8678

Recap: on the standalone media GT of MTL/ARL the CPU reads stale cache
lines for data the GuC has already written. The visible symptom is TLB
invalidation acks appearing to stall for a near-constant ~2.3s. i915
works around this as Wa_22016122933; xe never inherited it. Patch 3
implements it, scoped like i915.

In v3 I asked two open questions in this cover letter. Since there was
no preference expressed, v4 makes the conservative choice in both cases
and documents the reasoning in the commit message, so the series is not
blocked on a decision:

 1. CPU mapping: keeping XE_BO_FLAG_NEEDS_UC (uncached on both sides).
    It is the tested configuration and no throughput difference against
    the CPU-WC variant was measurable. Matching i915's exact CPU-WC +
    GGTT-UC combination needs either a new BO flag or decoupling the
    GGTT cache-mode selection from XE_BO_FLAG_NEEDS_UC; happy to add
    that plumbing if parity is preferred.

 2. Scope: covering the GuC-shared allocations (CTBs, log, ADS, SLPC,
    engine activity), which is where the failures were observed. i915
    additionally covers media-GT LRC/ring state; that can be a
    follow-up if wanted.

Fixes:/Cc: stable are still left out, since MTL/ARL is require_force_probe
in xe. Also happy to add them.

Validation of patch 3 is now six weeks on two ARL machines (7d51 and
7dd1), across kernels 7.1.6, 7.1.8 and 7.2, with over 10M TLB
invalidations processed and zero ack stalls. Before the fix both
machines reproduced 20-60 stalls/day, every day, on two GuC firmware
versions. The 7dd1 machine, which could not survive a day of media
workloads on xe without a platform freeze, has been running xe full
time since 11 August with zero incidents.

Patches 1-2 are the diagnostics that made the investigation possible
and are unchanged since v3. checkpatch is clean on the series.

v3 -> v4:
- Rebased on drm-tip.
- Patch 3: resolved both open questions in the commit message instead
  of leaving them for discussion; refreshed validation data.

Thanks,
Tales

Tales A. Mendonça (3):
  drm/xe: Capture devcoredump on TLB invalidation timeout
  drm/xe: Log when a timed out TLB invalidation ack finally arrives
  drm/xe: Implement Wa_22016122933

 drivers/gpu/drm/xe/xe_devcoredump.c         | 46 +++++++++++----------
 drivers/gpu/drm/xe/xe_devcoredump.h         | 15 +++++--
 drivers/gpu/drm/xe/xe_guc.c                 | 16 +++++++
 drivers/gpu/drm/xe/xe_guc.h                 |  2 +
 drivers/gpu/drm/xe/xe_guc_ads.c             |  3 +-
 drivers/gpu/drm/xe/xe_guc_ct.c              |  6 ++-
 drivers/gpu/drm/xe/xe_guc_engine_activity.c |  6 ++-
 drivers/gpu/drm/xe/xe_guc_log.c             |  7 +++-
 drivers/gpu/drm/xe/xe_guc_pc.c              |  3 +-
 drivers/gpu/drm/xe/xe_tlb_inval.c           | 39 +++++++++++++++++
 drivers/gpu/drm/xe/xe_tlb_inval_types.h     | 17 ++++++++
 drivers/gpu/drm/xe/xe_wa_oob.rules          |  1 +
 12 files changed, 128 insertions(+), 33 deletions(-)

-- 
2.55.0

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

[Tales A. Mendonça]

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 <talesam@gmail.com>
---
 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 = &gt->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

[PATCH v4 2/3] drm/xe: Log when a timed out TLB invalidation ack finally arrives

[Tales A. Mendonça]

When a TLB invalidation fence times out we log the timeout, but if the
ack for that seqno later shows up there is no record of it, making it
impossible to tell from logs whether the ack was lost forever or merely
(very) late.

Track the most recent timed out seqno and log how late its ack arrives,
relative to both the original request and the moment the fence was
signaled with -ETIME.

On ARL with GuC 70.53.0 this shows the acks are never lost: they
consistently arrive ~2.3s after the request, tens of milliseconds after
the TDR has already signaled the fence:

  TLB invalidation fence timeout, seqno=10992 recv=10991
  TLB invalidation late ack: seqno=10992 recv=10992,
    request-to-ack=2314ms, timeout-to-ack=45ms

Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/work_items/8678
Signed-off-by: Tales A. Mendonça <talesam@gmail.com>
---
 drivers/gpu/drm/xe/xe_tlb_inval.c       | 19 +++++++++++++++++++
 drivers/gpu/drm/xe/xe_tlb_inval_types.h | 17 +++++++++++++++++
 2 files changed, 36 insertions(+)

diff --git a/drivers/gpu/drm/xe/xe_tlb_inval.c b/drivers/gpu/drm/xe/xe_tlb_inval.c
index 7a0c04fac60..7047a347551 100644
--- a/drivers/gpu/drm/xe/xe_tlb_inval.c
+++ b/drivers/gpu/drm/xe/xe_tlb_inval.c
@@ -14,6 +14,7 @@
 #include "xe_guc_tlb_inval.h"
 #include "xe_mmio.h"
 #include "xe_pm.h"
+#include "xe_printk.h"
 #include "xe_tlb_inval.h"
 #include "xe_trace.h"
 
@@ -99,6 +100,11 @@ static void xe_tlb_inval_fence_timeout(struct work_struct *work)
 			fence->seqno, tlb_inval->seqno_recv);
 
 		timedout_seqno = fence->seqno;
+		if (!tlb_inval->timedout_seqno) {
+			tlb_inval->timedout_seqno = fence->seqno;
+			tlb_inval->timedout_inval_time = fence->inval_time;
+			tlb_inval->timedout_time = ktime_get();
+		}
 
 		fence->base.error = -ETIME;
 		xe_tlb_inval_fence_signal(fence);
@@ -227,6 +233,7 @@ void xe_tlb_inval_reset(struct xe_tlb_inval *tlb_inval)
 	else
 		pending_seqno = tlb_inval->seqno - 1;
 	WRITE_ONCE(tlb_inval->seqno_recv, pending_seqno);
+	tlb_inval->timedout_seqno = 0;
 
 	list_for_each_entry_safe(fence, next,
 				 &tlb_inval->pending_fences, link)
@@ -454,6 +461,18 @@ void xe_tlb_inval_done_handler(struct xe_tlb_inval *tlb_inval, int seqno)
 
 	WRITE_ONCE(tlb_inval->seqno_recv, seqno);
 
+	if (tlb_inval->timedout_seqno &&
+	    xe_tlb_inval_seqno_past(tlb_inval, tlb_inval->timedout_seqno)) {
+		ktime_t now = ktime_get();
+
+		xe_warn(xe,
+			"TLB invalidation late ack: seqno=%d recv=%d, request-to-ack=%lldms, timeout-to-ack=%lldms",
+			tlb_inval->timedout_seqno, seqno,
+			ktime_ms_delta(now, tlb_inval->timedout_inval_time),
+			ktime_ms_delta(now, tlb_inval->timedout_time));
+		tlb_inval->timedout_seqno = 0;
+	}
+
 	list_for_each_entry_safe(fence, next,
 				 &tlb_inval->pending_fences, link) {
 		trace_xe_tlb_inval_fence_recv(xe, fence);
diff --git a/drivers/gpu/drm/xe/xe_tlb_inval_types.h b/drivers/gpu/drm/xe/xe_tlb_inval_types.h
index d77be1aedc9..80d2019fa20 100644
--- a/drivers/gpu/drm/xe/xe_tlb_inval_types.h
+++ b/drivers/gpu/drm/xe/xe_tlb_inval_types.h
@@ -112,6 +112,23 @@ struct xe_tlb_inval {
 	 * @pending_lock: protects @pending_fences and updating @seqno_recv.
 	 */
 	spinlock_t pending_lock;
+	/**
+	 * @timedout_seqno: seqno of the most recent timed out TLB
+	 * invalidation, 0 if none. Used to measure how late the ack for a
+	 * timed out invalidation actually arrives. Protected by
+	 * @pending_lock.
+	 */
+	int timedout_seqno;
+	/**
+	 * @timedout_inval_time: request time of @timedout_seqno. Protected by
+	 * @pending_lock.
+	 */
+	ktime_t timedout_inval_time;
+	/**
+	 * @timedout_time: time @timedout_seqno was signaled with -ETIME.
+	 * Protected by @pending_lock.
+	 */
+	ktime_t timedout_time;
 	/**
 	 * @fence_tdr: schedules a delayed call to xe_tlb_fence_timeout after
 	 * the timeout interval is over.
-- 
2.55.0

[PATCH v4 3/3] drm/xe: Implement Wa_22016122933

[Tales A. Mendonça]

On platforms with a standalone media GT and media version 13.00
(MTL/ARL), memory shared between the CPU and the media GT's GuC must
not be mapped cached on the CPU side: the CPU can otherwise read stale
cache lines for data the GuC has already written.

i915 implements this as Wa_22016122933 (see
intel_gt_needs_wa_22016122933(), used by intel_guc_allocate_vma() and
intel_gt_coherent_map_type()); xe never inherited it.

The visible symptom on ARL is TLB invalidation acks stalling for a
near-constant ~2.3s: the GuC writes the G2H ack in time, but the CPU
keeps reading a stale (empty) view of the G2H CTB until the line is
naturally evicted, so the fence timeout at 2.25s fires first. GuC log
decode confirmed all invalidations were handled promptly by the
firmware, and only the media GT was affected. See Link for the full
investigation (three machines affected: 7d51, 7dd1, Arc Pro 130T).

Making the mapping coherent instead of uncached does not help: with a
GGTT PAT entry repurposed to WB|COH_2WAY the driver comes up and the
media GT GuC runs, but the stalls remain (request-to-ack 2290ms).
Uncached really is required here. Note that 2-way coherency is not
normally reachable from a GGTT PTE (only 2 PAT bits), so that
experiment needed a modified PAT table and may not reflect a supported
configuration.

Add the OOB workaround scoped like i915 (media version 13.00, media GT
only - MEDIA_VERSION() OOB rules only match the media GT on standalone
media platforms) and apply XE_BO_FLAG_NEEDS_UC to the GuC-shared
allocations the CPU reads from: the CTBs, the GuC log, ADS, the SLPC
shared data and the engine activity buffers. hwconfig and the G2G
buffer are allocated on the primary GT only, where the workaround does
not apply.

Note that XE_BO_FLAG_NEEDS_UC drives both the CPU mapping (uncached)
and the GGTT cache mode (XE_CACHE_NONE instead of XE_CACHE_WB), which is
stricter than i915: i915 documents the workaround as WC on the CPU side
and UC on the GPU side. A CPU-WC variant with the GGTT side kept at
XE_CACHE_NONE was measured to be equally effective, but expressing it
would require either a new BO flag or decoupling the GGTT cache-mode
selection from XE_BO_FLAG_NEEDS_UC - simply swapping in
XE_BO_FLAG_FORCE_WC silently relaxes the GPU side back to WB and the
stalls return. The stricter mapping is kept here since it is the tested
configuration and no throughput difference between the two was
measurable; the extra plumbing can be added later if parity with i915 is
preferred.

Scope is limited to the GuC-shared allocations, which is where the
failures were observed. i915 additionally covers media-GT LRC/ring
state; extending xe to match can be done as a follow-up if wanted.

Validation on two ARL machines (7d51 and 7dd1): before, 20-60 TLB
invalidation ack stalls per day, every day, for weeks, on every kernel
and on two GuC firmware versions (70.53.0 and 70.72.1). After: zero
stalls in six weeks of combined runtime, over 10M TLB invalidations
processed under the same workloads, across kernels 7.1.6, 7.1.8 and
7.2. The 7dd1 machine, which could not survive a day of media
workloads on xe without a platform freeze, has been running xe full
time since 11 August, including days with heavy video transcoding,
with zero stalls and zero freezes.

The coherency experiment, the CPU-WC measurements and the engine
activity coverage gap were found by Navon John Lukose while A/B
testing v2 on an ARL 7d51.

Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/work_items/8678
Suggested-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Signed-off-by: Tales A. Mendonça <talesam@gmail.com>
Tested-by: Navon John Lukose <navonjohnlukose@gmail.com>
---
 drivers/gpu/drm/xe/xe_guc.c                 | 16 ++++++++++++++++
 drivers/gpu/drm/xe/xe_guc.h                 |  2 ++
 drivers/gpu/drm/xe/xe_guc_ads.c             |  3 ++-
 drivers/gpu/drm/xe/xe_guc_ct.c              |  6 ++++--
 drivers/gpu/drm/xe/xe_guc_engine_activity.c |  6 ++++--
 drivers/gpu/drm/xe/xe_guc_log.c             |  7 +++++--
 drivers/gpu/drm/xe/xe_guc_pc.c              |  3 ++-
 drivers/gpu/drm/xe/xe_wa_oob.rules          |  1 +
 8 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_guc.c b/drivers/gpu/drm/xe/xe_guc.c
index c7f8bbd4cb9..3ab4cb9e496 100644
--- a/drivers/gpu/drm/xe/xe_guc.c
+++ b/drivers/gpu/drm/xe/xe_guc.c
@@ -1469,6 +1469,22 @@ int xe_guc_suspend(struct xe_guc *guc)
 	return 0;
 }
 
+/**
+ * xe_guc_bo_wa_flags - Extra BO flags for memory shared with the GuC
+ * @gt: the &xe_gt whose GuC the buffer will be shared with
+ *
+ * Wa_22016122933: on the standalone media GT, memory shared between the
+ * CPU and the GuC must not be mapped cached on the CPU side, otherwise
+ * the CPU can read stale data written by the GuC (e.g. G2H CTB writes)
+ * for multiple seconds.
+ *
+ * Return: additional XE_BO_FLAG_* to use when allocating GuC-shared memory
+ */
+u32 xe_guc_bo_wa_flags(struct xe_gt *gt)
+{
+	return XE_GT_WA(gt, 22016122933) ? XE_BO_FLAG_NEEDS_UC : 0;
+}
+
 void xe_guc_notify(struct xe_guc *guc)
 {
 	struct xe_gt *gt = guc_to_gt(guc);
diff --git a/drivers/gpu/drm/xe/xe_guc.h b/drivers/gpu/drm/xe/xe_guc.h
index 61e3ee19a59..c4eca40d69c 100644
--- a/drivers/gpu/drm/xe/xe_guc.h
+++ b/drivers/gpu/drm/xe/xe_guc.h
@@ -30,6 +30,7 @@
 	xe_guc_fw_version_at_least((guc), MAKE_GUC_VER_ARGS(ver))
 
 struct drm_printer;
+struct xe_gt;
 
 void xe_guc_comm_init_early(struct xe_guc *guc);
 int xe_guc_init_noalloc(struct xe_guc *guc);
@@ -45,6 +46,7 @@ void xe_guc_runtime_suspend(struct xe_guc *guc);
 void xe_guc_runtime_resume(struct xe_guc *guc);
 int xe_guc_suspend(struct xe_guc *guc);
 int xe_guc_softreset(struct xe_guc *guc);
+u32 xe_guc_bo_wa_flags(struct xe_gt *gt);
 void xe_guc_notify(struct xe_guc *guc);
 int xe_guc_auth_huc(struct xe_guc *guc, u32 rsa_addr);
 int xe_guc_mmio_send(struct xe_guc *guc, const u32 *request, u32 len);
diff --git a/drivers/gpu/drm/xe/xe_guc_ads.c b/drivers/gpu/drm/xe/xe_guc_ads.c
index ff8eee3831a..abc7266fc6f 100644
--- a/drivers/gpu/drm/xe/xe_guc_ads.c
+++ b/drivers/gpu/drm/xe/xe_guc_ads.c
@@ -435,7 +435,8 @@ int xe_guc_ads_init(struct xe_guc_ads *ads)
 					  XE_BO_FLAG_SYSTEM |
 					  XE_BO_FLAG_GGTT |
 					  XE_BO_FLAG_GGTT_INVALIDATE |
-					  XE_BO_FLAG_PINNED_NORESTORE);
+					  XE_BO_FLAG_PINNED_NORESTORE |
+					  xe_guc_bo_wa_flags(gt));
 	if (IS_ERR(bo))
 		return PTR_ERR(bo);
 
diff --git a/drivers/gpu/drm/xe/xe_guc_ct.c b/drivers/gpu/drm/xe/xe_guc_ct.c
index 5c4733da385..5c393aa29de 100644
--- a/drivers/gpu/drm/xe/xe_guc_ct.c
+++ b/drivers/gpu/drm/xe/xe_guc_ct.c
@@ -376,7 +376,8 @@ int xe_guc_ct_init(struct xe_guc_ct *ct)
 					  XE_BO_FLAG_SYSTEM |
 					  XE_BO_FLAG_GGTT |
 					  XE_BO_FLAG_GGTT_INVALIDATE |
-					  XE_BO_FLAG_PINNED_NORESTORE);
+					  XE_BO_FLAG_PINNED_NORESTORE |
+					  xe_guc_bo_wa_flags(gt));
 	if (IS_ERR(bo))
 		return PTR_ERR(bo);
 
@@ -386,7 +387,8 @@ int xe_guc_ct_init(struct xe_guc_ct *ct)
 					  XE_BO_FLAG_SYSTEM |
 					  XE_BO_FLAG_GGTT |
 					  XE_BO_FLAG_GGTT_INVALIDATE |
-					  XE_BO_FLAG_PINNED_NORESTORE);
+					  XE_BO_FLAG_PINNED_NORESTORE |
+					  xe_guc_bo_wa_flags(gt));
 	if (IS_ERR(bo))
 		return PTR_ERR(bo);
 
diff --git a/drivers/gpu/drm/xe/xe_guc_engine_activity.c b/drivers/gpu/drm/xe/xe_guc_engine_activity.c
index a782be57caa..729ce8ac140 100644
--- a/drivers/gpu/drm/xe/xe_guc_engine_activity.c
+++ b/drivers/gpu/drm/xe/xe_guc_engine_activity.c
@@ -97,7 +97,8 @@ static int allocate_engine_activity_buffers(struct xe_guc *guc,
 
 	metadata_bo = xe_bo_create_pin_map_novm(gt_to_xe(gt), tile, PAGE_ALIGN(metadata_size),
 						ttm_bo_type_kernel, XE_BO_FLAG_SYSTEM |
-						XE_BO_FLAG_GGTT | XE_BO_FLAG_GGTT_INVALIDATE,
+						XE_BO_FLAG_GGTT | XE_BO_FLAG_GGTT_INVALIDATE |
+						xe_guc_bo_wa_flags(gt),
 						false);
 
 	if (IS_ERR(metadata_bo))
@@ -105,7 +106,8 @@ static int allocate_engine_activity_buffers(struct xe_guc *guc,
 
 	bo = xe_bo_create_pin_map_novm(gt_to_xe(gt), tile, PAGE_ALIGN(size),
 				       ttm_bo_type_kernel, XE_BO_FLAG_VRAM_IF_DGFX(tile) |
-				       XE_BO_FLAG_GGTT | XE_BO_FLAG_GGTT_INVALIDATE, false);
+				       XE_BO_FLAG_GGTT | XE_BO_FLAG_GGTT_INVALIDATE |
+				       xe_guc_bo_wa_flags(gt), false);
 
 	if (IS_ERR(bo)) {
 		xe_bo_unpin_map_no_vm(metadata_bo);
diff --git a/drivers/gpu/drm/xe/xe_guc_log.c b/drivers/gpu/drm/xe/xe_guc_log.c
index 538d4df0f7a..7d006268ce9 100644
--- a/drivers/gpu/drm/xe/xe_guc_log.c
+++ b/drivers/gpu/drm/xe/xe_guc_log.c
@@ -17,6 +17,7 @@
 #include "xe_force_wake.h"
 #include "xe_gt_printk.h"
 #include "xe_gt_types.h"
+#include "xe_guc.h"
 #include "xe_map.h"
 #include "xe_mmio.h"
 #include "xe_module.h"
@@ -624,14 +625,16 @@ void xe_guc_log_print_lfd(struct xe_guc_log *log, struct drm_printer *p)
 int xe_guc_log_init(struct xe_guc_log *log)
 {
 	struct xe_device *xe = log_to_xe(log);
-	struct xe_tile *tile = gt_to_tile(log_to_gt(log));
+	struct xe_gt *gt = log_to_gt(log);
+	struct xe_tile *tile = gt_to_tile(gt);
 	struct xe_bo *bo;
 
 	bo = xe_managed_bo_create_pin_map(xe, tile, GUC_LOG_SIZE,
 					  XE_BO_FLAG_SYSTEM |
 					  XE_BO_FLAG_GGTT |
 					  XE_BO_FLAG_GGTT_INVALIDATE |
-					  XE_BO_FLAG_PINNED_NORESTORE);
+					  XE_BO_FLAG_PINNED_NORESTORE |
+					  xe_guc_bo_wa_flags(gt));
 	if (IS_ERR(bo))
 		return PTR_ERR(bo);
 
diff --git a/drivers/gpu/drm/xe/xe_guc_pc.c b/drivers/gpu/drm/xe/xe_guc_pc.c
index 097b075bd89..e0105222a2c 100644
--- a/drivers/gpu/drm/xe/xe_guc_pc.c
+++ b/drivers/gpu/drm/xe/xe_guc_pc.c
@@ -1391,7 +1391,8 @@ int xe_guc_pc_init(struct xe_guc_pc *pc)
 					  XE_BO_FLAG_VRAM_IF_DGFX(tile) |
 					  XE_BO_FLAG_GGTT |
 					  XE_BO_FLAG_GGTT_INVALIDATE |
-					  XE_BO_FLAG_PINNED_NORESTORE);
+					  XE_BO_FLAG_PINNED_NORESTORE |
+					  xe_guc_bo_wa_flags(gt));
 	if (IS_ERR(bo))
 		return PTR_ERR(bo);
 
diff --git a/drivers/gpu/drm/xe/xe_wa_oob.rules b/drivers/gpu/drm/xe/xe_wa_oob.rules
index dd69ad07f7a..30958b26a1d 100644
--- a/drivers/gpu/drm/xe/xe_wa_oob.rules
+++ b/drivers/gpu/drm/xe/xe_wa_oob.rules
@@ -14,6 +14,7 @@
 16017236439	PLATFORM(PVC)
 14019821291	MEDIA_VERSION_RANGE(1300, 2000)
 14015076503	MEDIA_VERSION(1300)
+22016122933	MEDIA_VERSION(1300)
 14018913170	GRAPHICS_VERSION_RANGE(1270, 1274)
 		MEDIA_VERSION(1300)
 		PLATFORM(DG2)
-- 
2.55.0

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] drm/xe: data race in xe_tlb_inval_fence_timeout() reading seqno_recv
--

commit ef794f6bbaafa679257b9fec9405245af4942e68
Author: Tales A. Mendonça <talesam@gmail.com>

drm/xe: Capture devcoredump on TLB invalidation timeout

This commit adds xe_devcoredump_gt(), a variant of xe_devcoredump() for hangs
that are not tied to an exec queue or job. It hooks this new capture mechanism
into the TLB invalidation timeout path to capture the GuC log and CT state.

Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/work_items/8678

> diff --git a/drivers/gpu/drm/xe/xe_tlb_inval.c b/drivers/gpu/drm/xe/xe_tlb_inval.c
> index 7c3bf32d9258b..7a0c04fac60aa 100644
> --- a/drivers/gpu/drm/xe/xe_tlb_inval.c
> +++ b/drivers/gpu/drm/xe/xe_tlb_inval.c
[ ... ]
> @@ -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);

[Severity: Low]
Does this code introduce a data race when reading tlb_inval->seqno_recv?

In xe_tlb_inval_fence_timeout() in drivers/gpu/drm/xe/xe_tlb_inval.c, the
pending_lock spinlock is dropped just before this block. Because seqno_recv
is read locklessly here, and concurrent updates (such as those in
xe_tlb_inval_done_handler()) use WRITE_ONCE(), reading it directly could
result in a data race.

Could this lead to KCSAN warnings or logging a corrupted value in the
devcoredump reason string? Would it be safer to read this using READ_ONCE()
or cache the value while the lock is held?

>  }
>  
>  /**

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260917163553.1742580-1-talesam@gmail.com?part=1

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

[Tales A. Mendonça]

Good catch - that read is genuinely racy, and it is my patch that
introduced it. seqno_recv is written with WRITE_ONCE() from
xe_tlb_inval_done_handler(), and I deliberately moved the devcoredump
call outside pending_lock (the capture takes sleeping locks), which
left the read unsynchronised.

  Fixed locally by sampling it while the lock is still held:

        seqno_recv = tlb_inval->seqno_recv;
        spin_unlock_irq(&tlb_inval->pending_lock);
        ...
        if (timedout_seqno)
                xe_devcoredump_gt(tlb_inval_to_gt(tlb_inval),
                                  "TLB invalidation fence timeout,
seqno=%d recv=%d",
                                  timedout_seqno, seqno_recv);

  This will be in v5, together with any other review feedback.

Em qui., 17 de set. de 2026 às 13:50, <sashiko-bot@kernel.org> escreveu:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] drm/xe: data race in xe_tlb_inval_fence_timeout() reading seqno_recv
> --
>
> commit ef794f6bbaafa679257b9fec9405245af4942e68
> Author: Tales A. Mendonça <talesam@gmail.com>
>
> drm/xe: Capture devcoredump on TLB invalidation timeout
>
> This commit adds xe_devcoredump_gt(), a variant of xe_devcoredump() for hangs
> that are not tied to an exec queue or job. It hooks this new capture mechanism
> into the TLB invalidation timeout path to capture the GuC log and CT state.
>
> Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/work_items/8678
>
> > diff --git a/drivers/gpu/drm/xe/xe_tlb_inval.c b/drivers/gpu/drm/xe/xe_tlb_inval.c
> > index 7c3bf32d9258b..7a0c04fac60aa 100644
> > --- a/drivers/gpu/drm/xe/xe_tlb_inval.c
> > +++ b/drivers/gpu/drm/xe/xe_tlb_inval.c
> [ ... ]
> > @@ -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);
>
> [Severity: Low]
> Does this code introduce a data race when reading tlb_inval->seqno_recv?
>
> In xe_tlb_inval_fence_timeout() in drivers/gpu/drm/xe/xe_tlb_inval.c, the
> pending_lock spinlock is dropped just before this block. Because seqno_recv
> is read locklessly here, and concurrent updates (such as those in
> xe_tlb_inval_done_handler()) use WRITE_ONCE(), reading it directly could
> result in a data race.
>
> Could this lead to KCSAN warnings or logging a corrupted value in the
> devcoredump reason string? Would it be safer to read this using READ_ONCE()
> or cache the value while the lock is held?
>
> >  }
> >
> >  /**
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260917163553.1742580-1-talesam@gmail.com?part=1



-- 
Com os cumprimentos,

Tales A. Mendonça
talesam.org
communitybig.org

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

[Matthew Brost]

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 <talesam@gmail.com>

With your v5 fix Sashiko pointed out:

Reviewed-by: Matthew Brost <matthew.brost@intel.com>

> ---
>  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 = &gt->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
> 

Re: [PATCH v4 2/3] drm/xe: Log when a timed out TLB invalidation ack finally arrives

[Matthew Brost]

On Thu, Sep 17, 2026 at 01:35:52PM -0300, Tales A. Mendonça wrote:
> When a TLB invalidation fence times out we log the timeout, but if the
> ack for that seqno later shows up there is no record of it, making it
> impossible to tell from logs whether the ack was lost forever or merely
> (very) late.
> 
> Track the most recent timed out seqno and log how late its ack arrives,
> relative to both the original request and the moment the fence was
> signaled with -ETIME.
> 
> On ARL with GuC 70.53.0 this shows the acks are never lost: they
> consistently arrive ~2.3s after the request, tens of milliseconds after
> the TDR has already signaled the fence:
> 
>   TLB invalidation fence timeout, seqno=10992 recv=10991
>   TLB invalidation late ack: seqno=10992 recv=10992,
>     request-to-ack=2314ms, timeout-to-ack=45ms
> 
> Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/work_items/8678
> Signed-off-by: Tales A. Mendonça <talesam@gmail.com>

I would give this an RB but my colleagues have done a bunch of work on
SIGID and I haven't been involved at all, so I need some help here.

> ---
>  drivers/gpu/drm/xe/xe_tlb_inval.c       | 19 +++++++++++++++++++
>  drivers/gpu/drm/xe/xe_tlb_inval_types.h | 17 +++++++++++++++++
>  2 files changed, 36 insertions(+)
> 
> diff --git a/drivers/gpu/drm/xe/xe_tlb_inval.c b/drivers/gpu/drm/xe/xe_tlb_inval.c
> index 7a0c04fac60..7047a347551 100644
> --- a/drivers/gpu/drm/xe/xe_tlb_inval.c
> +++ b/drivers/gpu/drm/xe/xe_tlb_inval.c
> @@ -14,6 +14,7 @@
>  #include "xe_guc_tlb_inval.h"
>  #include "xe_mmio.h"
>  #include "xe_pm.h"
> +#include "xe_printk.h"
>  #include "xe_tlb_inval.h"
>  #include "xe_trace.h"
>  
> @@ -99,6 +100,11 @@ static void xe_tlb_inval_fence_timeout(struct work_struct *work)
>  			fence->seqno, tlb_inval->seqno_recv);
>  
>  		timedout_seqno = fence->seqno;
> +		if (!tlb_inval->timedout_seqno) {
> +			tlb_inval->timedout_seqno = fence->seqno;
> +			tlb_inval->timedout_inval_time = fence->inval_time;
> +			tlb_inval->timedout_time = ktime_get();
> +		}
>  
>  		fence->base.error = -ETIME;
>  		xe_tlb_inval_fence_signal(fence);
> @@ -227,6 +233,7 @@ void xe_tlb_inval_reset(struct xe_tlb_inval *tlb_inval)
>  	else
>  		pending_seqno = tlb_inval->seqno - 1;
>  	WRITE_ONCE(tlb_inval->seqno_recv, pending_seqno);
> +	tlb_inval->timedout_seqno = 0;
>  
>  	list_for_each_entry_safe(fence, next,
>  				 &tlb_inval->pending_fences, link)
> @@ -454,6 +461,18 @@ void xe_tlb_inval_done_handler(struct xe_tlb_inval *tlb_inval, int seqno)
>  
>  	WRITE_ONCE(tlb_inval->seqno_recv, seqno);
>  
> +	if (tlb_inval->timedout_seqno &&
> +	    xe_tlb_inval_seqno_past(tlb_inval, tlb_inval->timedout_seqno)) {
> +		ktime_t now = ktime_get();
> +
> +		xe_warn(xe,
> +			"TLB invalidation late ack: seqno=%d recv=%d, request-to-ack=%lldms, timeout-to-ack=%lldms",
> +			tlb_inval->timedout_seqno, seqno,
> +			ktime_ms_delta(now, tlb_inval->timedout_inval_time),
> +			ktime_ms_delta(now, tlb_inval->timedout_time));

Should this be some type SIGID message?

Matt

> +		tlb_inval->timedout_seqno = 0;
> +	}
> +
>  	list_for_each_entry_safe(fence, next,
>  				 &tlb_inval->pending_fences, link) {
>  		trace_xe_tlb_inval_fence_recv(xe, fence);
> diff --git a/drivers/gpu/drm/xe/xe_tlb_inval_types.h b/drivers/gpu/drm/xe/xe_tlb_inval_types.h
> index d77be1aedc9..80d2019fa20 100644
> --- a/drivers/gpu/drm/xe/xe_tlb_inval_types.h
> +++ b/drivers/gpu/drm/xe/xe_tlb_inval_types.h
> @@ -112,6 +112,23 @@ struct xe_tlb_inval {
>  	 * @pending_lock: protects @pending_fences and updating @seqno_recv.
>  	 */
>  	spinlock_t pending_lock;
> +	/**
> +	 * @timedout_seqno: seqno of the most recent timed out TLB
> +	 * invalidation, 0 if none. Used to measure how late the ack for a
> +	 * timed out invalidation actually arrives. Protected by
> +	 * @pending_lock.
> +	 */
> +	int timedout_seqno;
> +	/**
> +	 * @timedout_inval_time: request time of @timedout_seqno. Protected by
> +	 * @pending_lock.
> +	 */
> +	ktime_t timedout_inval_time;
> +	/**
> +	 * @timedout_time: time @timedout_seqno was signaled with -ETIME.
> +	 * Protected by @pending_lock.
> +	 */
> +	ktime_t timedout_time;
>  	/**
>  	 * @fence_tdr: schedules a delayed call to xe_tlb_fence_timeout after
>  	 * the timeout interval is over.
> -- 
> 2.55.0
> 

Re: [PATCH v4 2/3] drm/xe: Log when a timed out TLB invalidation ack finally arrives

[Matthew Brost]

On Fri, Sep 18, 2026 at 03:06:25PM -0700, Matthew Brost wrote:
> On Thu, Sep 17, 2026 at 01:35:52PM -0300, Tales A. Mendonça wrote:
> > When a TLB invalidation fence times out we log the timeout, but if the
> > ack for that seqno later shows up there is no record of it, making it
> > impossible to tell from logs whether the ack was lost forever or merely
> > (very) late.
> > 
> > Track the most recent timed out seqno and log how late its ack arrives,
> > relative to both the original request and the moment the fence was
> > signaled with -ETIME.
> > 
> > On ARL with GuC 70.53.0 this shows the acks are never lost: they
> > consistently arrive ~2.3s after the request, tens of milliseconds after
> > the TDR has already signaled the fence:
> > 
> >   TLB invalidation fence timeout, seqno=10992 recv=10991
> >   TLB invalidation late ack: seqno=10992 recv=10992,
> >     request-to-ack=2314ms, timeout-to-ack=45ms
> > 
> > Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/work_items/8678
> > Signed-off-by: Tales A. Mendonça <talesam@gmail.com>
> 
> I would give this an RB but my colleagues have done a bunch of work on
> SIGID and I haven't been involved at all, so I need some help here.
> 

I discussed with my colleagues - the we agree the TLB component should
hook into SIGID for logging but we will do this as a follow up on top of
this change.

So this is:
Reviewed-by: Matthew Brost <matthew.brost@intel.com> 

> > ---
> >  drivers/gpu/drm/xe/xe_tlb_inval.c       | 19 +++++++++++++++++++
> >  drivers/gpu/drm/xe/xe_tlb_inval_types.h | 17 +++++++++++++++++
> >  2 files changed, 36 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_tlb_inval.c b/drivers/gpu/drm/xe/xe_tlb_inval.c
> > index 7a0c04fac60..7047a347551 100644
> > --- a/drivers/gpu/drm/xe/xe_tlb_inval.c
> > +++ b/drivers/gpu/drm/xe/xe_tlb_inval.c
> > @@ -14,6 +14,7 @@
> >  #include "xe_guc_tlb_inval.h"
> >  #include "xe_mmio.h"
> >  #include "xe_pm.h"
> > +#include "xe_printk.h"
> >  #include "xe_tlb_inval.h"
> >  #include "xe_trace.h"
> >  
> > @@ -99,6 +100,11 @@ static void xe_tlb_inval_fence_timeout(struct work_struct *work)
> >  			fence->seqno, tlb_inval->seqno_recv);
> >  
> >  		timedout_seqno = fence->seqno;
> > +		if (!tlb_inval->timedout_seqno) {
> > +			tlb_inval->timedout_seqno = fence->seqno;
> > +			tlb_inval->timedout_inval_time = fence->inval_time;
> > +			tlb_inval->timedout_time = ktime_get();
> > +		}
> >  
> >  		fence->base.error = -ETIME;
> >  		xe_tlb_inval_fence_signal(fence);
> > @@ -227,6 +233,7 @@ void xe_tlb_inval_reset(struct xe_tlb_inval *tlb_inval)
> >  	else
> >  		pending_seqno = tlb_inval->seqno - 1;
> >  	WRITE_ONCE(tlb_inval->seqno_recv, pending_seqno);
> > +	tlb_inval->timedout_seqno = 0;
> >  
> >  	list_for_each_entry_safe(fence, next,
> >  				 &tlb_inval->pending_fences, link)
> > @@ -454,6 +461,18 @@ void xe_tlb_inval_done_handler(struct xe_tlb_inval *tlb_inval, int seqno)
> >  
> >  	WRITE_ONCE(tlb_inval->seqno_recv, seqno);
> >  
> > +	if (tlb_inval->timedout_seqno &&
> > +	    xe_tlb_inval_seqno_past(tlb_inval, tlb_inval->timedout_seqno)) {
> > +		ktime_t now = ktime_get();
> > +
> > +		xe_warn(xe,
> > +			"TLB invalidation late ack: seqno=%d recv=%d, request-to-ack=%lldms, timeout-to-ack=%lldms",
> > +			tlb_inval->timedout_seqno, seqno,
> > +			ktime_ms_delta(now, tlb_inval->timedout_inval_time),
> > +			ktime_ms_delta(now, tlb_inval->timedout_time));
> 
> Should this be some type SIGID message?
> 
> Matt
> 
> > +		tlb_inval->timedout_seqno = 0;
> > +	}
> > +
> >  	list_for_each_entry_safe(fence, next,
> >  				 &tlb_inval->pending_fences, link) {
> >  		trace_xe_tlb_inval_fence_recv(xe, fence);
> > diff --git a/drivers/gpu/drm/xe/xe_tlb_inval_types.h b/drivers/gpu/drm/xe/xe_tlb_inval_types.h
> > index d77be1aedc9..80d2019fa20 100644
> > --- a/drivers/gpu/drm/xe/xe_tlb_inval_types.h
> > +++ b/drivers/gpu/drm/xe/xe_tlb_inval_types.h
> > @@ -112,6 +112,23 @@ struct xe_tlb_inval {
> >  	 * @pending_lock: protects @pending_fences and updating @seqno_recv.
> >  	 */
> >  	spinlock_t pending_lock;
> > +	/**
> > +	 * @timedout_seqno: seqno of the most recent timed out TLB
> > +	 * invalidation, 0 if none. Used to measure how late the ack for a
> > +	 * timed out invalidation actually arrives. Protected by
> > +	 * @pending_lock.
> > +	 */
> > +	int timedout_seqno;
> > +	/**
> > +	 * @timedout_inval_time: request time of @timedout_seqno. Protected by
> > +	 * @pending_lock.
> > +	 */
> > +	ktime_t timedout_inval_time;
> > +	/**
> > +	 * @timedout_time: time @timedout_seqno was signaled with -ETIME.
> > +	 * Protected by @pending_lock.
> > +	 */
> > +	ktime_t timedout_time;
> >  	/**
> >  	 * @fence_tdr: schedules a delayed call to xe_tlb_fence_timeout after
> >  	 * the timeout interval is over.
> > -- 
> > 2.55.0
> > 

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

[Tales A. Mendonça]

On Fri, Sep 18, 2026 at 03:02:00PM -0700, Matthew Brost wrote:
> With your v5 fix Sashiko pointed out:
>
> Reviewed-by: Matthew Brost <matthew.brost@intel.com>

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
<matthew.brost@intel.com> 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 <talesam@gmail.com>
>
> With your v5 fix Sashiko pointed out:
>
> Reviewed-by: Matthew Brost <matthew.brost@intel.com>
>
> > ---
> >  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 = &gt->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

Re: [PATCH v4 2/3] drm/xe: Log when a timed out TLB invalidation ack finally arrives

[Tales A. Mendonça]

On Sun, Sep 20, 2026 at 10:30:00PM -0700, Matthew Brost wrote:
> I discussed with my colleagues - the we agree the TLB component should
> hook into SIGID for logging but we will do this as a follow up on top of
> this change.
>
> So this is:
> Reviewed-by: Matthew Brost <matthew.brost@intel.com>

Thanks Matt, and thanks for taking the question to the people who own
SIGID rather than letting it sit.

I will carry your tag on this patch in v5. Agreed on leaving the SIGID
conversion as a follow-up: there is no TLB component in
DEFINE_XE_LOG_COMPONENTS() yet, so adding one touches the log ABI and
belongs with the people doing that work rather than bolted onto a bug
fix. Happy to rebase on top of it once it lands.

v5 is otherwise ready and goes out this week. The only code delta is in
patch 1, from the data race Sashiko spotted after v4.

While you are here - patch 3 is the one that actually stops the stalls,
and it is still the only one without review. It applies
XE_BO_FLAG_NEEDS_UC to the GuC-shared allocations (CTBs, log, ADS, SLPC,
engine activity) on the standalone media GT, scoped by a new OOB rule
(22016122933 MEDIA_VERSION(1300)). The approach was Daniele's
suggestion. It has been running on two ARL machines for about six weeks
across several kernel versions, with over 10 million invalidations and
zero stalls; before it, both machines hit the ~2.3s ack delay 20-60
times a day.

Daniele, Stuart, would either of you be able to take a look at that one?

Tales

Em seg., 21 de set. de 2026 às 02:30, Matthew Brost
<matthew.brost@intel.com> escreveu:
>
> On Fri, Sep 18, 2026 at 03:06:25PM -0700, Matthew Brost wrote:
> > On Thu, Sep 17, 2026 at 01:35:52PM -0300, Tales A. Mendonça wrote:
> > > When a TLB invalidation fence times out we log the timeout, but if the
> > > ack for that seqno later shows up there is no record of it, making it
> > > impossible to tell from logs whether the ack was lost forever or merely
> > > (very) late.
> > >
> > > Track the most recent timed out seqno and log how late its ack arrives,
> > > relative to both the original request and the moment the fence was
> > > signaled with -ETIME.
> > >
> > > On ARL with GuC 70.53.0 this shows the acks are never lost: they
> > > consistently arrive ~2.3s after the request, tens of milliseconds after
> > > the TDR has already signaled the fence:
> > >
> > >   TLB invalidation fence timeout, seqno=10992 recv=10991
> > >   TLB invalidation late ack: seqno=10992 recv=10992,
> > >     request-to-ack=2314ms, timeout-to-ack=45ms
> > >
> > > Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/work_items/8678
> > > Signed-off-by: Tales A. Mendonça <talesam@gmail.com>
> >
> > I would give this an RB but my colleagues have done a bunch of work on
> > SIGID and I haven't been involved at all, so I need some help here.
> >
>
> I discussed with my colleagues - the we agree the TLB component should
> hook into SIGID for logging but we will do this as a follow up on top of
> this change.
>
> So this is:
> Reviewed-by: Matthew Brost <matthew.brost@intel.com>
>
> > > ---
> > >  drivers/gpu/drm/xe/xe_tlb_inval.c       | 19 +++++++++++++++++++
> > >  drivers/gpu/drm/xe/xe_tlb_inval_types.h | 17 +++++++++++++++++
> > >  2 files changed, 36 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/xe/xe_tlb_inval.c b/drivers/gpu/drm/xe/xe_tlb_inval.c
> > > index 7a0c04fac60..7047a347551 100644
> > > --- a/drivers/gpu/drm/xe/xe_tlb_inval.c
> > > +++ b/drivers/gpu/drm/xe/xe_tlb_inval.c
> > > @@ -14,6 +14,7 @@
> > >  #include "xe_guc_tlb_inval.h"
> > >  #include "xe_mmio.h"
> > >  #include "xe_pm.h"
> > > +#include "xe_printk.h"
> > >  #include "xe_tlb_inval.h"
> > >  #include "xe_trace.h"
> > >
> > > @@ -99,6 +100,11 @@ static void xe_tlb_inval_fence_timeout(struct work_struct *work)
> > >                     fence->seqno, tlb_inval->seqno_recv);
> > >
> > >             timedout_seqno = fence->seqno;
> > > +           if (!tlb_inval->timedout_seqno) {
> > > +                   tlb_inval->timedout_seqno = fence->seqno;
> > > +                   tlb_inval->timedout_inval_time = fence->inval_time;
> > > +                   tlb_inval->timedout_time = ktime_get();
> > > +           }
> > >
> > >             fence->base.error = -ETIME;
> > >             xe_tlb_inval_fence_signal(fence);
> > > @@ -227,6 +233,7 @@ void xe_tlb_inval_reset(struct xe_tlb_inval *tlb_inval)
> > >     else
> > >             pending_seqno = tlb_inval->seqno - 1;
> > >     WRITE_ONCE(tlb_inval->seqno_recv, pending_seqno);
> > > +   tlb_inval->timedout_seqno = 0;
> > >
> > >     list_for_each_entry_safe(fence, next,
> > >                              &tlb_inval->pending_fences, link)
> > > @@ -454,6 +461,18 @@ void xe_tlb_inval_done_handler(struct xe_tlb_inval *tlb_inval, int seqno)
> > >
> > >     WRITE_ONCE(tlb_inval->seqno_recv, seqno);
> > >
> > > +   if (tlb_inval->timedout_seqno &&
> > > +       xe_tlb_inval_seqno_past(tlb_inval, tlb_inval->timedout_seqno)) {
> > > +           ktime_t now = ktime_get();
> > > +
> > > +           xe_warn(xe,
> > > +                   "TLB invalidation late ack: seqno=%d recv=%d, request-to-ack=%lldms, timeout-to-ack=%lldms",
> > > +                   tlb_inval->timedout_seqno, seqno,
> > > +                   ktime_ms_delta(now, tlb_inval->timedout_inval_time),
> > > +                   ktime_ms_delta(now, tlb_inval->timedout_time));
> >
> > Should this be some type SIGID message?
> >
> > Matt
> >
> > > +           tlb_inval->timedout_seqno = 0;
> > > +   }
> > > +
> > >     list_for_each_entry_safe(fence, next,
> > >                              &tlb_inval->pending_fences, link) {
> > >             trace_xe_tlb_inval_fence_recv(xe, fence);
> > > diff --git a/drivers/gpu/drm/xe/xe_tlb_inval_types.h b/drivers/gpu/drm/xe/xe_tlb_inval_types.h
> > > index d77be1aedc9..80d2019fa20 100644
> > > --- a/drivers/gpu/drm/xe/xe_tlb_inval_types.h
> > > +++ b/drivers/gpu/drm/xe/xe_tlb_inval_types.h
> > > @@ -112,6 +112,23 @@ struct xe_tlb_inval {
> > >      * @pending_lock: protects @pending_fences and updating @seqno_recv.
> > >      */
> > >     spinlock_t pending_lock;
> > > +   /**
> > > +    * @timedout_seqno: seqno of the most recent timed out TLB
> > > +    * invalidation, 0 if none. Used to measure how late the ack for a
> > > +    * timed out invalidation actually arrives. Protected by
> > > +    * @pending_lock.
> > > +    */
> > > +   int timedout_seqno;
> > > +   /**
> > > +    * @timedout_inval_time: request time of @timedout_seqno. Protected by
> > > +    * @pending_lock.
> > > +    */
> > > +   ktime_t timedout_inval_time;
> > > +   /**
> > > +    * @timedout_time: time @timedout_seqno was signaled with -ETIME.
> > > +    * Protected by @pending_lock.
> > > +    */
> > > +   ktime_t timedout_time;
> > >     /**
> > >      * @fence_tdr: schedules a delayed call to xe_tlb_fence_timeout after
> > >      * the timeout interval is over.
> > > --
> > > 2.55.0
> > >



-- 
Com os cumprimentos,

Tales A. Mendonça
talesam.org
communitybig.org

Re: [PATCH v4 2/3] drm/xe: Log when a timed out TLB invalidation ack finally arrives

[Matthew Brost]

On Mon, Sep 21, 2026 at 03:10:06PM -0300, Tales A. Mendonça wrote:
> On Sun, Sep 20, 2026 at 10:30:00PM -0700, Matthew Brost wrote:
> > I discussed with my colleagues - the we agree the TLB component should
> > hook into SIGID for logging but we will do this as a follow up on top of
> > this change.
> >
> > So this is:
> > Reviewed-by: Matthew Brost <matthew.brost@intel.com>
> 
> Thanks Matt, and thanks for taking the question to the people who own
> SIGID rather than letting it sit.
> 
> I will carry your tag on this patch in v5. Agreed on leaving the SIGID
> conversion as a follow-up: there is no TLB component in
> DEFINE_XE_LOG_COMPONENTS() yet, so adding one touches the log ABI and
> belongs with the people doing that work rather than bolted onto a bug
> fix. Happy to rebase on top of it once it lands.
> 

I think we can stage the SIGID behind this, a colleagues volunteered to
pick this up after.

> v5 is otherwise ready and goes out this week. The only code delta is in
> patch 1, from the data race Sashiko spotted after v4.
> 
> While you are here - patch 3 is the one that actually stops the stalls,
> and it is still the only one without review. It applies
> XE_BO_FLAG_NEEDS_UC to the GuC-shared allocations (CTBs, log, ADS, SLPC,
> engine activity) on the standalone media GT, scoped by a new OOB rule
> (22016122933 MEDIA_VERSION(1300)). The approach was Daniele's
> suggestion. It has been running on two ARL machines for about six weeks
> across several kernel versions, with over 10 million invalidations and
> zero stalls; before it, both machines hit the ~2.3s ack delay 20-60
> times a day.
> 
> Daniele, Stuart, would either of you be able to take a look at that one?
> 

Daniele is out on sabatical, I'll look now but this is typically not my
department but should be able give feedback if I cross code all the
code.

Matt

> Tales
> 
> Em seg., 21 de set. de 2026 às 02:30, Matthew Brost
> <matthew.brost@intel.com> escreveu:
> >
> > On Fri, Sep 18, 2026 at 03:06:25PM -0700, Matthew Brost wrote:
> > > On Thu, Sep 17, 2026 at 01:35:52PM -0300, Tales A. Mendonça wrote:
> > > > When a TLB invalidation fence times out we log the timeout, but if the
> > > > ack for that seqno later shows up there is no record of it, making it
> > > > impossible to tell from logs whether the ack was lost forever or merely
> > > > (very) late.
> > > >
> > > > Track the most recent timed out seqno and log how late its ack arrives,
> > > > relative to both the original request and the moment the fence was
> > > > signaled with -ETIME.
> > > >
> > > > On ARL with GuC 70.53.0 this shows the acks are never lost: they
> > > > consistently arrive ~2.3s after the request, tens of milliseconds after
> > > > the TDR has already signaled the fence:
> > > >
> > > >   TLB invalidation fence timeout, seqno=10992 recv=10991
> > > >   TLB invalidation late ack: seqno=10992 recv=10992,
> > > >     request-to-ack=2314ms, timeout-to-ack=45ms
> > > >
> > > > Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/work_items/8678
> > > > Signed-off-by: Tales A. Mendonça <talesam@gmail.com>
> > >
> > > I would give this an RB but my colleagues have done a bunch of work on
> > > SIGID and I haven't been involved at all, so I need some help here.
> > >
> >
> > I discussed with my colleagues - the we agree the TLB component should
> > hook into SIGID for logging but we will do this as a follow up on top of
> > this change.
> >
> > So this is:
> > Reviewed-by: Matthew Brost <matthew.brost@intel.com>
> >
> > > > ---
> > > >  drivers/gpu/drm/xe/xe_tlb_inval.c       | 19 +++++++++++++++++++
> > > >  drivers/gpu/drm/xe/xe_tlb_inval_types.h | 17 +++++++++++++++++
> > > >  2 files changed, 36 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/xe/xe_tlb_inval.c b/drivers/gpu/drm/xe/xe_tlb_inval.c
> > > > index 7a0c04fac60..7047a347551 100644
> > > > --- a/drivers/gpu/drm/xe/xe_tlb_inval.c
> > > > +++ b/drivers/gpu/drm/xe/xe_tlb_inval.c
> > > > @@ -14,6 +14,7 @@
> > > >  #include "xe_guc_tlb_inval.h"
> > > >  #include "xe_mmio.h"
> > > >  #include "xe_pm.h"
> > > > +#include "xe_printk.h"
> > > >  #include "xe_tlb_inval.h"
> > > >  #include "xe_trace.h"
> > > >
> > > > @@ -99,6 +100,11 @@ static void xe_tlb_inval_fence_timeout(struct work_struct *work)
> > > >                     fence->seqno, tlb_inval->seqno_recv);
> > > >
> > > >             timedout_seqno = fence->seqno;
> > > > +           if (!tlb_inval->timedout_seqno) {
> > > > +                   tlb_inval->timedout_seqno = fence->seqno;
> > > > +                   tlb_inval->timedout_inval_time = fence->inval_time;
> > > > +                   tlb_inval->timedout_time = ktime_get();
> > > > +           }
> > > >
> > > >             fence->base.error = -ETIME;
> > > >             xe_tlb_inval_fence_signal(fence);
> > > > @@ -227,6 +233,7 @@ void xe_tlb_inval_reset(struct xe_tlb_inval *tlb_inval)
> > > >     else
> > > >             pending_seqno = tlb_inval->seqno - 1;
> > > >     WRITE_ONCE(tlb_inval->seqno_recv, pending_seqno);
> > > > +   tlb_inval->timedout_seqno = 0;
> > > >
> > > >     list_for_each_entry_safe(fence, next,
> > > >                              &tlb_inval->pending_fences, link)
> > > > @@ -454,6 +461,18 @@ void xe_tlb_inval_done_handler(struct xe_tlb_inval *tlb_inval, int seqno)
> > > >
> > > >     WRITE_ONCE(tlb_inval->seqno_recv, seqno);
> > > >
> > > > +   if (tlb_inval->timedout_seqno &&
> > > > +       xe_tlb_inval_seqno_past(tlb_inval, tlb_inval->timedout_seqno)) {
> > > > +           ktime_t now = ktime_get();
> > > > +
> > > > +           xe_warn(xe,
> > > > +                   "TLB invalidation late ack: seqno=%d recv=%d, request-to-ack=%lldms, timeout-to-ack=%lldms",
> > > > +                   tlb_inval->timedout_seqno, seqno,
> > > > +                   ktime_ms_delta(now, tlb_inval->timedout_inval_time),
> > > > +                   ktime_ms_delta(now, tlb_inval->timedout_time));
> > >
> > > Should this be some type SIGID message?
> > >
> > > Matt
> > >
> > > > +           tlb_inval->timedout_seqno = 0;
> > > > +   }
> > > > +
> > > >     list_for_each_entry_safe(fence, next,
> > > >                              &tlb_inval->pending_fences, link) {
> > > >             trace_xe_tlb_inval_fence_recv(xe, fence);
> > > > diff --git a/drivers/gpu/drm/xe/xe_tlb_inval_types.h b/drivers/gpu/drm/xe/xe_tlb_inval_types.h
> > > > index d77be1aedc9..80d2019fa20 100644
> > > > --- a/drivers/gpu/drm/xe/xe_tlb_inval_types.h
> > > > +++ b/drivers/gpu/drm/xe/xe_tlb_inval_types.h
> > > > @@ -112,6 +112,23 @@ struct xe_tlb_inval {
> > > >      * @pending_lock: protects @pending_fences and updating @seqno_recv.
> > > >      */
> > > >     spinlock_t pending_lock;
> > > > +   /**
> > > > +    * @timedout_seqno: seqno of the most recent timed out TLB
> > > > +    * invalidation, 0 if none. Used to measure how late the ack for a
> > > > +    * timed out invalidation actually arrives. Protected by
> > > > +    * @pending_lock.
> > > > +    */
> > > > +   int timedout_seqno;
> > > > +   /**
> > > > +    * @timedout_inval_time: request time of @timedout_seqno. Protected by
> > > > +    * @pending_lock.
> > > > +    */
> > > > +   ktime_t timedout_inval_time;
> > > > +   /**
> > > > +    * @timedout_time: time @timedout_seqno was signaled with -ETIME.
> > > > +    * Protected by @pending_lock.
> > > > +    */
> > > > +   ktime_t timedout_time;
> > > >     /**
> > > >      * @fence_tdr: schedules a delayed call to xe_tlb_fence_timeout after
> > > >      * the timeout interval is over.
> > > > --
> > > > 2.55.0
> > > >
> 
> 
> 
> -- 
> Com os cumprimentos,
> 
> Tales A. Mendonça
> talesam.org
> communitybig.org