Re: [PATCH 1/3] drm/xe/ggtt: stop mapping unmapped GGTT pages to scratch

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Writing faulting PTEs for recycled holes but scratch PTEs for virgin holes causes non-deterministic spurious hardware faults depending on memory allocation history.
--

--- Patch [1]: [PATCH 1/3] drm/xe/ggtt: stop mapping unmapped GGTT pages to scratch ---
commit 240e3aa087f4e7c9c8232bdeba69ae3fe654d294
Author: Tangudu Tilak Tirumalesh <tilak.tirumalesh.tangudu@intel.com>

drm/xe/ggtt: stop mapping unmapped GGTT pages to scratch

This commit updates xe_ggtt_clear() to take a use_scratch parameter, leaving
the init-time clear to use scratch PTEs while forcing PTE=0 during node 
removal on multi-queue platforms. This aims to expose stale hardware TLB 
entries as observable faults.

> diff --git a/drivers/gpu/drm/xe/xe_ggtt.c b/drivers/gpu/drm/xe/xe_ggtt.c
> index 8ec23862477fc..4cddd69f0e992 100644
> --- a/drivers/gpu/drm/xe/xe_ggtt.c
> +++ b/drivers/gpu/drm/xe/xe_ggtt.c

[ ... ]

> @@ -257,23 +258,36 @@ static u64 xe_ggtt_get_pte(struct xe_ggtt *ggtt, u64 addr)
>  	return readq(&ggtt->gsm[addr >> XE_PTE_SHIFT]);
>  }
>  
> -static void xe_ggtt_clear(struct xe_ggtt *ggtt, u64 start, u64 size)
> +/**
> + * xe_ggtt_clear() - Clear a range of GGTT page-table entries
> + * @ggtt: the &xe_ggtt to clear
> + * @start: start of the range (GGTT address)
> + * @size: size of the range in bytes
> + * @use_scratch: true fills the range with the scratch PTE (init-time clear so
> + *		 engines walking free holes read zeros safely); false writes
> + *		 PTE=0 so a stale HW TLB entry faults instead of silently
> + *		 landing on scratch.
> + */

[Severity: High]
Does this explanation imply that hardware prefetchers speculatively walk
into unmapped ranges?

If engines legitimately walk free holes (as stated here for the init-time
clear), the hardware prefetcher cannot distinguish between virgin holes and
recycled holes.

[ ... ]

> @@ -473,12 +487,19 @@ static void ggtt_node_fini(struct xe_ggtt_node *node)
>  static void ggtt_node_remove(struct xe_ggtt_node *node)
>  {
>  	struct xe_ggtt *ggtt = node->ggtt;
> -	bool bound;
> +	bool use_scratch, bound;
> +
> +	/*
> +	 * Addition of a secondary queue while a multi-queue context is live on
> +	 * HW requires proper GGTT TLB clearing, so use faulting PTEs here.
> +	 */
> +	use_scratch = !xe_gt_has_multi_queue(ggtt->tile->primary_gt);
>  
>  	mutex_lock(&ggtt->lock);
>  	bound = ggtt->flags & XE_GGTT_FLAGS_ONLINE;
>  	if (bound)
> -		xe_ggtt_clear(ggtt, xe_ggtt_node_addr(node), xe_ggtt_node_size(node));
> +		xe_ggtt_clear(ggtt, xe_ggtt_node_addr(node),
> +			      xe_ggtt_node_size(node), use_scratch);

[Severity: High]
Can writing PTE=0 here cause spurious GPU or IOMMU page faults on
multi-queue platforms?

Since a prefetching engine might speculatively read past its buffer boundary
into an adjacent recycled hole, stripping the scratch PTE protection for
recycled memory while keeping it for virgin memory creates a situation
where faults depend entirely on memory allocation history.

Could this lead to engine hangs or system instability when a prefetching
engine hits the PTE=0 in a recently freed node?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260824163458.3129935-1-tilak.tirumalesh.tangudu@intel.com?part=1