Re: [PATCH v2 31/33] drm/xe: Blit src-natural to dst-packed for defrag-IOVA copies

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Overloading `dst_packed_pages` as a boolean flag causes GPU IOMMU faults when defragmentation yields 0 changed pages.

Pre-existing issues:
- [High] Mismatch between XE_PTE_SHIFT and PAGE_SHIFT causes out-of-bounds array indexing and assertion failures on architectures with 64K pages.
--

--- Patch [31]: [PATCH v2 31/33] drm/xe: Blit src-natural to dst-packed for defrag-IOVA copies ---
commit b1d39bbd2548d9ad82a4ac12f4936dc748d93736
Author: Matthew Brost <matthew.brost@intel.com>
Subject: drm/xe: Blit src-natural to dst-packed for defrag-IOVA copies

This commit adds a packed-destination mode to __xe_migrate_copy for defrag
moves with IOVA enabled. It allows reading from the old tt at natural offsets
and writing to the new tt at packed contiguous offsets. A new
xe_migrate_copy_defrag_iova entry point is exposed for this functionality.

> diff --git a/drivers/gpu/drm/xe/xe_migrate.c b/drivers/gpu/drm/xe/xe_migrate.c
> index 0a3a1c5ca1938..35fd0253fef30 100644
> --- a/drivers/gpu/drm/xe/xe_migrate.c
> +++ b/drivers/gpu/drm/xe/xe_migrate.c
[ ... ]
> @@ -883,9 +949,71 @@ static u32 emit_copy_chunks(struct xe_gt *gt, struct xe_bb *bb,
>  
>  		if (bb)
>  			emit_copy(gt, bb, src_L0_ofs + (u64)i * PAGE_SIZE,
> -				  dst_L0_ofs + (u64)i * PAGE_SIZE,
> +				  dst_L0_ofs +
> +				  (u64)(pack_dst ? pack : i) * PAGE_SIZE,
>  				  run * PAGE_SIZE, PAGE_SIZE);

[Severity: High]
This isn't a bug introduced by this patch, but does passing PAGE_SIZE as the
pitch to emit_copy overflow the 16-bit pitch limit on systems with 64KB CPU
pages? The xe_gt_assert condition in emit_xy_fast_copy expects the pitch to be
<= U16_MAX, which could cause a crash on architectures like ARM64 when
defragmenting BOs.

>  		nblits++;
> +		pack += run;
> +		i += run;
> +	}
[ ... ]
> @@ -1005,6 +1134,23 @@ static struct dma_fence *__xe_migrate_copy(struct xe_migrate *m,
>  	 */
>  	struct ttm_tt *dst_tt = dst_bo->ttm.ttm;
>  	bool skip_borrowed = flags.defrag_copy && src_tt && dst_tt;
> +	/*
> +	 * Defrag-IOVA copy: the destination tt only maps the changed
> +	 * (non-borrowed) pages, packed contiguously into the first
> +	 * @dst_packed_pages pages of its IOVA. The source is read from the old
> +	 * tt at natural offsets and each changed run is blitted into the packed
> +	 * destination prefix.
> +	 */
> +	bool pack_dst = dst_packed_pages != 0;

[Severity: High]
Can evaluating pack_dst this way lead to GPU IOMMU faults when a
defragmentation move yields 0 changed pages?

If a BO defragmentation successfully borrows all pages, dst_packed_pages will
be 0. This makes pack_dst false, which disables the packed CCS logic. During
the CCS pass, the driver might then fall back to a full BO CCS copy via
xe_migrate_ccs_copy(), assuming the destination is fully mapped. However,
because the destination IOVA mapping only covers the 0 changed pages, the GPU
would write CCS data to unmapped IOVA addresses.

> +	/*
> +	 * Packed defrag CCS pass: migrate the flat-CCS aux state of the changed
> +	 * pages only, from the source at its natural offset to the destination
> +	 * packed prefix. Borrowed pages share the physical page (and thus its
> +	 * aux) on both sides, so they are skipped exactly like the data pass.
> +	 */
> +	bool ccs_pack = flags.defrag_ccs && pack_dst && src_tt && dst_tt;
[ ... ]
> @@ -1054,29 +1202,86 @@ static struct dma_fence *__xe_migrate_copy(struct xe_migrate *m,
>  		u32 pte_flags;
>  		pgoff_t start_page = (xe_bo_size(src_bo) - size) >> XE_PTE_SHIFT;

[Severity: High]
This isn't a bug introduced by this patch, but could the start_page
calculation cause out-of-bounds array indexing on architectures with
64KB pages? 

The index start_page is computed using XE_PTE_SHIFT (which is 4KB), but
it is later used to index ttm_tt->pages which is sized based on
PAGE_SHIFT. On a 64KB page system, the calculated index would be 16x
larger than the array bounds, potentially leading to memory corruption
during defragmentation.

>  		u32 ndata_blits = 1;
> +		u32 nchg = 0;
> +		u64 dst_size;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260710215442.2444235-1-matthew.brost@intel.com?part=31