Re: [PATCH v2 1/2] drm/xe: Skip over non leaf pte for PRL generation

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

On Fri, Feb 27, 2026 at 01:42:14AM +0000, Brian Nguyen wrote:
> The check using xe_child->base.children was insufficient in determining
> if a pte was a leaf node. So explicitly skip over every non-leaf pt and
> conditionally abort if there is a scenario where a non-leaf pt is
> interleaved between leaf pt, which results in the page walker skipping
> over some leaf pt.
> 
> Note that the behavior being targeted for abort is
> PD[0] = 2M PTE
> PD[1] = PT -> 512 4K PTEs
> PD[2] = 2M PTE
> 
> results in abort, page walker won't descend PD[1].
> 
> With new abort, ensuring valid PRL before handling a second abort.
> 
> v2:
>  - Revert to previous assert.
>  - Revised non-leaf handling for interleaf child pt and leaf pte.
>  - Update comments to specifications. (Stuart)
>  - Remove unnecessary XE_PTE_PS64. (Matthew B)
> 
> Fixes: b912138df299 ("drm/xe: Create page reclaim list on unbind")
> Signed-off-by: Brian Nguyen <brian3.nguyen@intel.com>
> Cc: Matthew Brost <matthew.brost@intel.com>
> Cc: Stuart Summers <stuart.summers@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_pt.c | 34 +++++++++++++++++++++++++++-------
>  1 file changed, 27 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_pt.c b/drivers/gpu/drm/xe/xe_pt.c
> index 13b355fadd58..6fd5f0c29d44 100644
> --- a/drivers/gpu/drm/xe/xe_pt.c
> +++ b/drivers/gpu/drm/xe/xe_pt.c
> @@ -1655,14 +1655,35 @@ static int xe_pt_stage_unbind_entry(struct xe_ptw *parent, pgoff_t offset,
>  	XE_WARN_ON(!level);
>  	/* Check for leaf node */
>  	if (xe_walk->prl && xe_page_reclaim_list_valid(xe_walk->prl) &&
> -	    (!xe_child->base.children || !xe_child->base.children[first])) {
> +	    xe_child->level <= MAX_HUGEPTE_LEVEL) {
>  		struct iosys_map *leaf_map = &xe_child->bo->vmap;
>  		pgoff_t count = xe_pt_num_entries(addr, next, xe_child->level, walk);
>  
>  		for (pgoff_t i = 0; i < count; i++) {
> -			u64 pte = xe_map_rd(xe, leaf_map, (first + i) * sizeof(u64), u64);
> +			u64 pte;
>  			int ret;
>  
> +			/*
> +			 * If not a leaf pt, skip unless non-leaf pt is interleaved between
> +			 * leaf ptes which causes the page walk to skip over the child leaves
> +			 */
> +			if (xe_child->base.children && xe_child->base.children[first + i]) {
> +				u64 pt_size = 1ULL << walk->shifts[xe_child->level];
> +				bool edge_pt = (i == 0 && !IS_ALIGNED(addr, pt_size)) ||
> +					       (i == count - 1 && !IS_ALIGNED(next, pt_size));
> +
> +				if (!edge_pt) {
> +					xe_page_reclaim_list_abort(xe_walk->tile->primary_gt,
> +								   xe_walk->prl,
> +								   "PT is skipped by walk at level=%u offset=%lu",
> +								   xe_child->level, first + i);
> +					break;
> +				}
> +				continue;
> +			}
> +
> +			pte = xe_map_rd(xe, leaf_map, (first + i) * sizeof(u64), u64);
> +
>  			/*
>  			 * In rare scenarios, pte may not be written yet due to racy conditions.
>  			 * In such cases, invalidate the PRL and fallback to full PPC invalidation.
> @@ -1674,9 +1695,8 @@ static int xe_pt_stage_unbind_entry(struct xe_ptw *parent, pgoff_t offset,
>  			}
>  
>  			/* Ensure it is a defined page */
> -			xe_tile_assert(xe_walk->tile,
> -				       xe_child->level == 0 ||
> -				       (pte & (XE_PTE_PS64 | XE_PDE_PS_2M | XE_PDPE_PS_1G)));
> +			xe_tile_assert(xe_walk->tile, xe_child->level == 0 ||
> +				       (pte & (XE_PDE_PS_2M | XE_PDPE_PS_1G)));
>  
>  			/* An entry should be added for 64KB but contigious 4K have XE_PTE_PS64 */
>  			if (pte & XE_PTE_PS64)
> @@ -1704,8 +1724,8 @@ static int xe_pt_stage_unbind_entry(struct xe_ptw *parent, pgoff_t offset,
>  	 * Verify PRL is active and if entry is not a leaf pte (base.children conditions),
>  	 * there is a potential need to invalidate the PRL if any PTE (num_live) are dropped.
>  	 */

Everything above here looks correct.

> -	if (xe_walk->prl && level > 1 && xe_child->num_live &&
> -	    xe_child->base.children && xe_child->base.children[first]) {
> +	if (xe_walk->prl && xe_page_reclaim_list_valid(xe_walk->prl) && level > 1 &&
> +	    xe_child->num_live && xe_child->base.children && xe_child->base.children[first]) {

Is this part correct? In particular the 'xe_child->base.children &&
xe_child->base.children[first]' part? Wouldn't the above loop catch this
now and invalidate the PRL?

Matt

>  		bool covered = xe_pt_covers(addr, next, xe_child->level, &xe_walk->base);
>  
>  		/*
> -- 
> 2.43.0
>