Hi, v5 of the TLB invalidation ack stall fix for ARL. 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. Patches 1 and 2 picked up Matthew Brost's Reviewed-by - thank you. On patch 2 we agreed the SIGID conversion of the TLB logging is a follow-up on top of this series, to be done by the people working on SIGID; there is no TLB component in DEFINE_XE_LOG_COMPONENTS() yet. Happy to rebase on top of that work once it lands. Patch 3 is unchanged since v4 and is still the one without review. It is also the one that actually stops the stalls: 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 suggested by Daniele. Daniele, Stuart, would one of you be able to take a look? 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. Two notes carried over from v4, still open to either answer: 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. Fixes:/Cc: stable are left out, since MTL/ARL is require_force_probe in xe. Also happy to add them. checkpatch is clean, except for one --strict CHECK about macro argument reuse in the xe_devcoredump() wrapper in patch 1, which is intentional: the macro only exists to forward (_q)->gt alongside _q. v4 -> v5: - Rebased on today's drm-tip; builds clean, no conflicts. - Patch 1: fix a data race reported by Sashiko - seqno_recv was read outside pending_lock in the timeout path. It is now sampled while the lock is still held and passed to xe_devcoredump_gt() by value, so the capture itself still runs outside the lock. - Patches 1-2: collected Reviewed-by from Matthew Brost. - Patch 3: unchanged. 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 | 41 ++++++++++++++++++ drivers/gpu/drm/xe/xe_tlb_inval_types.h | 17 ++++++++ drivers/gpu/drm/xe/xe_wa_oob.rules | 1 + 12 files changed, 130 insertions(+), 33 deletions(-) -- 2.55.0
