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

[Matt Roper <matthew.d.roper@intel.com>]

On Wed, Aug 26, 2026 at 04:55:31PM +0530, tilak.tirumalesh.tangudu@intel.com wrote:
> From: Tangudu Tilak Tirumalesh <tilak.tirumalesh.tangudu@intel.com>
> 
> On multi-queue platforms an engine can retain a stale GGTT TLB entry
> for a range that has since been unmapped and recycled. When unmapped
> GGTT entries resolve to the scratch page, such a stale access lands on
> scratch silently instead of faulting, masking the error.

Drive-by question; I haven't reviewed the changes in depth.  How does
multi-queue relate to GGTT here?  Multi-queue submission is something
userspace can do, but those userspace submissions can't access the GGTT,
only the PPGTT.  GGTT accesses would only come from privileged
instructions that the KMD inserts into the ring and I don't think those
are ever multi-queue.  Am I overlooking something?  It seems like GGTT
and multi-queue should be orthogonal.


Matt

> 
> On multi-queue platforms, skip allocating the GGTT scratch page so
> cleared and unmapped ranges hold PTE=0. The whole GGTT then
> consistently faults on unmapped access for the entire boot; other
> platforms keep the scratch page 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)
> 
> 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 | 39 ++++++++++++++++++++++--------------
>  drivers/gpu/drm/xe/xe_gt.h   | 13 ++++++++++++
>  2 files changed, 37 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_ggtt.c b/drivers/gpu/drm/xe/xe_ggtt.c
> index 8ec23862477f..bd65100283e3 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,35 @@ 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 platforms leave unmapped GGTT faulting (no scratch) so a
> +	 * stale engine TLB entry can't silently resolve to the scratch page.
>  	 */
> -	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
> 

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation