Re: [PATCH v5 3/3] drm/xe: Implement Wa_22016122933

[Matthew Brost <matthew.brost@intel.com>]

On Mon, Sep 21, 2026 at 03:21:21PM -0300, Tales A. Mendonça wrote:
> 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 +

One thing the i915 WA does that isn't present here is if non-dGPU the
LRC is marked UC - see __lrc_alloc_state in the i915.

Is that needed? AFIAK the GuC only reads the LRC though and all LRC
write are issued from hardware, so maybe the i915 application of this
W/A isn't correct? The fact you are not seeing hangs with this suggests
this part of thw W/A isn't needed.

Matt

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