Re: [PATCH 3/3] drm/xe/ggtt: invalidate engine GGTT TLBs for multi-queue GTs

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

On Mon, Aug 24, 2026 at 10:04:58PM +0530, tilak.tirumalesh.tangudu@intel.com wrote:
> From: Tangudu Tilak Tirumalesh <tilak.tirumalesh.tangudu@intel.com>
> 
> Engines cache GGTT translations in their own TLBs, which the
> xe_tlb_inval_ggtt() does not touch.
> After a GGTT unmap the engines can still hit the stale translation and
> read the old page.
> 
> Issue an INVAL_FULL (intra vf) invalidation on GGTT unmap, gated on the
> GT having any multi-queue engine class.
> 
> v2: Use xe_gt_has_multi_queue() helper and other knits. - Niranjana
> 
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Tangudu Tilak Tirumalesh <tilak.tirumalesh.tangudu@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_ggtt.c | 23 ++++++++++++++++++++++-
>  1 file changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_ggtt.c b/drivers/gpu/drm/xe/xe_ggtt.c
> index 4cddd69f0e99..0f8946fddbaa 100644
> --- a/drivers/gpu/drm/xe/xe_ggtt.c
> +++ b/drivers/gpu/drm/xe/xe_ggtt.c
> @@ -464,6 +464,7 @@ int xe_ggtt_init_early(struct xe_ggtt *ggtt)
>  ALLOW_ERROR_INJECTION(xe_ggtt_init_early, ERRNO); /* See xe_pci_probe() */
>  
>  static void xe_ggtt_invalidate(struct xe_ggtt *ggtt);
> +static void xe_ggtt_invalidate_engine(struct xe_ggtt *ggtt);
>  
>  static void xe_ggtt_initial_clear(struct xe_ggtt *ggtt)
>  {
> @@ -502,8 +503,11 @@ static void ggtt_node_remove(struct xe_ggtt_node *node)
>  			      xe_ggtt_node_size(node), use_scratch);
>  	drm_mm_remove_node(&node->base);
>  	node->base.size = 0;
> -	if (bound && node->invalidate_on_remove)
> +	if (bound && node->invalidate_on_remove) {
>  		xe_ggtt_invalidate(ggtt);
> +		/* Drain engine TLBs so a recycled range can't hit a stale entry. */
> +		xe_ggtt_invalidate_engine(ggtt);
> +	}
>  	mutex_unlock(&ggtt->lock);
>  
>  	ggtt_node_fini(node);
> @@ -612,6 +616,23 @@ static void xe_ggtt_invalidate(struct xe_ggtt *ggtt)
>  	ggtt_invalidate_gt_tlb(ggtt->tile->media_gt);
>  }
>  
> +/*
> + * Drain engine-side GGTT TLBs on teardown so a recycled range's next
> + * occupant can't hit a predecessor's cached translation.
> + */
> +static void xe_ggtt_invalidate_engine(struct xe_ggtt *ggtt)
> +{
> +	struct xe_gt *gt = ggtt->tile->primary_gt;
> +	int err;
> +
> +	if (!xe_gt_has_multi_queue(gt))
> +		return;

For future-proofing, look similar too xe_ggtt_invalidate():

It should attempt to invalidate both the primary and media GTs. The
media GT will either be NULL or have xe_gt_has_multi_queue() == false,
in which case xe_tlb_inval_ggtt_full() will be skipped for that GT.

Sashiko also flagged this as a potential issue, not functional thing but
good feedback.

Matt

> +
> +	err = xe_tlb_inval_ggtt_full(&gt->tlb_inval);
> +	xe_gt_WARN(gt, err, "Failed to invalidate engine GGTT TLBs (%pe)",
> +		   ERR_PTR(err));
> +}
> +
>  /**
>   * xe_ggtt_shift_nodes() - Shift GGTT nodes to adjust for a change in usable address range.
>   * @ggtt: the &xe_ggtt struct instance
> -- 
> 2.46.0
>