Re: [PATCH 1/3] drm/xe/ggtt: fault on unmapped GGTT for multi-queue platforms

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

On Thu, Aug 27, 2026 at 01:50:09PM +0530, tilak.tirumalesh.tangudu@intel.com wrote:
> From: Tangudu Tilak Tirumalesh <tilak.tirumalesh.tangudu@intel.com>
> 
> Engines cache GGTT translations in their TLBs.
> A full context restore on engine switch flushes these, so a stale
> translation for an unmapped and recycled range does not survive.
> 
> During a secondary queue append, GuC does a lite-restore and skips the
> full context restore, so engine TLB invalidations are missed. This was
> observed while running multi-queue tests on another OS.
> 
> If unmapped GGTT points to scratch, such a stale access silently reads
> scratch instead of faulting, masking the error. On multi-queue
> platforms skip allocating the scratch page so unmapped GGTT holds
> PTE=0 and faults consistently for the whole boot; other platforms keep
> scratch and are unaffected.
> 
> v2:
> - Add xe_gt_has_multi_queue() helper and address nits (Niranjana)
> v3:
> - Drop scratch entirely instead of toggling per-unmap (Matt Brost)
> v5:
> - Document the rationale (Matt Roper / Matt Brost)
> 
> Assisted-by: Claude:claude-opus-4-8
> Reviewed-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>

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

> Signed-off-by: Tangudu Tilak Tirumalesh <tilak.tirumalesh.tangudu@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_ggtt.c | 40 ++++++++++++++++++++++--------------
>  drivers/gpu/drm/xe/xe_gt.h   | 13 ++++++++++++
>  2 files changed, 38 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_ggtt.c b/drivers/gpu/drm/xe/xe_ggtt.c
> index 8ec23862477f..9b58f47986f6 100644
> --- a/drivers/gpu/drm/xe/xe_ggtt.c
> +++ b/drivers/gpu/drm/xe/xe_ggtt.c
> @@ -20,6 +20,7 @@
>  #include "regs/xe_regs.h"
>  #include "xe_assert.h"
>  #include "xe_bo.h"
> +#include "xe_gt.h"
>  #include "xe_gt_printk.h"
>  #include "xe_gt_types.h"
>  #include "xe_map.h"
> @@ -533,27 +534,36 @@ void xe_ggtt_node_remove(struct xe_ggtt_node *node, bool invalidate)
>  int xe_ggtt_init(struct xe_ggtt *ggtt)
>  {
>  	struct xe_device *xe = tile_to_xe(ggtt->tile);
> -	unsigned int flags;
>  	int err;
>  
>  	/*
> -	 * So we don't need to worry about 64K GGTT layout when dealing with
> -	 * scratch entries, rather keep the scratch page in system memory on
> -	 * platforms where 64K pages are needed for VRAM.
> +	 * Multi-queue misses engine GGTT TLB invalidations (GuC lite-restore
> +	 * skips the full context restore), so skip scratch: a stale translation
> +	 * to a freed range then faults instead of silently reading scratch.
>  	 */
> -	flags = 0;
> -	if (ggtt->flags & XE_GGTT_FLAGS_64K)
> -		flags |= XE_BO_FLAG_SYSTEM;
> -	else
> -		flags |= XE_BO_FLAG_VRAM_IF_DGFX(ggtt->tile);
> +	if (!xe_gt_has_multi_queue(ggtt->tile->primary_gt)) {
> +		unsigned int flags = 0;
> +
> +		/*
> +		 * So we don't need to worry about 64K GGTT layout when dealing
> +		 * with scratch entries, rather keep the scratch page in system
> +		 * memory on platforms where 64K pages are needed for VRAM.
> +		 */
> +		if (ggtt->flags & XE_GGTT_FLAGS_64K)
> +			flags |= XE_BO_FLAG_SYSTEM;
> +		else
> +			flags |= XE_BO_FLAG_VRAM_IF_DGFX(ggtt->tile);
>  
> -	ggtt->scratch = xe_managed_bo_create_pin_map(xe, ggtt->tile, XE_PAGE_SIZE, flags);
> -	if (IS_ERR(ggtt->scratch)) {
> -		err = PTR_ERR(ggtt->scratch);
> -		goto err;
> -	}
> +		ggtt->scratch = xe_managed_bo_create_pin_map(xe, ggtt->tile,
> +							     XE_PAGE_SIZE, flags);
> +		if (IS_ERR(ggtt->scratch)) {
> +			err = PTR_ERR(ggtt->scratch);
> +			goto err;
> +		}
>  
> -	xe_map_memset(xe, &ggtt->scratch->vmap, 0, 0, xe_bo_size(ggtt->scratch));
> +		xe_map_memset(xe, &ggtt->scratch->vmap, 0, 0,
> +			      xe_bo_size(ggtt->scratch));
> +	}
>  
>  	xe_ggtt_initial_clear(ggtt);
>  
> diff --git a/drivers/gpu/drm/xe/xe_gt.h b/drivers/gpu/drm/xe/xe_gt.h
> index 65a4655b0994..92d934d50244 100644
> --- a/drivers/gpu/drm/xe/xe_gt.h
> +++ b/drivers/gpu/drm/xe/xe_gt.h
> @@ -160,4 +160,17 @@ static inline bool xe_gt_supports_multi_queue(const struct xe_gt *gt,
>  	return gt->info.multi_queue_engine_class_mask & BIT(class);
>  }
>  
> +/**
> + * xe_gt_has_multi_queue() - Check if gt supports multi-queue on
> + * any engine class.
> + *
> + * @gt: the GT object (may be NULL)
> + *
> + * Return: true if any engine class on @gt supports multi queue, else false
> + */
> +static inline bool xe_gt_has_multi_queue(const struct xe_gt *gt)
> +{
> +	return gt && gt->info.multi_queue_engine_class_mask;
> +}
> +
>  #endif
> -- 
> 2.46.0
>