Re: [PATCH v3 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:
- [Critical] Overloading `dst_packed_pages == 0` to mean 'not a packed destination' causes out-of-bounds cursor access and memory corruption when a packed defrag move has exactly zero changed pages.

Pre-existing issues:
- [Critical] During a TT-to-TT CCS defrag pass on DGFX systems, an uninitialized stack variable (`ccs_ofs`) is passed to the GPU blitter, causing memory corruption.
--

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

drm/xe: Blit src-natural to dst-packed for defrag-IOVA copies

This commit introduces a packed-destination mode in __xe_migrate_copy() for
defrag moves with IOVA enabled. It adds a dst_packed_pages argument to handle
asymmetric geometries where the destination only exposes changed pages. The
destination PTE update size is calculated based on the changed-page count.

> diff --git a/drivers/gpu/drm/xe/xe_migrate.c b/drivers/gpu/drm/xe/xe_migrate.c
> index e9f050eab8871..8eb5e331d5156 100644
> --- a/drivers/gpu/drm/xe/xe_migrate.c
> +++ b/drivers/gpu/drm/xe/xe_migrate.c

[ ... ]

> @@ -1033,7 +1179,9 @@ static struct dma_fence *__xe_migrate_copy(struct xe_migrate *m,
>  	else
>  		xe_res_first(src, 0, size, &src_it);
>  	if (!dst_is_vram)
> -		xe_res_first_tt(dst_bo->ttm.ttm, 0, size, &dst_it);
> +		xe_res_first_tt(dst_bo->ttm.ttm, 0,
> +				pack_dst ? (u64)dst_packed_pages << PAGE_SHIFT :
> +				size, &dst_it);
>  	else
>  		xe_res_first(dst, 0, size, &dst_it);

[Severity: Critical]
Does overloading dst_packed_pages == 0 to mean 'not a packed destination'
cause issues when a packed defrag move has exactly zero changed pages?

If a defragmentation move executes under memory pressure and fails to allocate
any new pages, it results in 100% borrowed pages. The IOVA mapping function
legitimately maps 0 pages, so xe_migrate_copy_defrag_iova() is called with
dst_packed_pages = 0.

Because dst_packed_pages == 0, pack_dst becomes false (set earlier in
__xe_migrate_copy). This forces the fallback to a full-size non-packed copy,
and initializes dst_it requesting the full BO size against a destination IOVA
that was packed to size 0.

Can this walk the broken cursor and fetch garbage DMA addresses for the GPU
blitter, corrupting memory?

[ ... ]

> @@ -1145,16 +1355,22 @@ static struct dma_fence *__xe_migrate_copy(struct xe_migrate *m,
>  
>  		if (skip_borrowed)
>  			emit_copy_chunks(gt, bb, src_L0_ofs, dst_L0_ofs, src_L0,
> -					 src_tt, dst_tt, start_page);
> +					 src_tt, dst_tt, start_page, pack_dst);
>  		else if (!flags.copy_only_ccs && !flags.defrag_ccs)
>  			emit_copy(gt, bb, src_L0_ofs, dst_L0_ofs, src_L0, XE_PAGE_SIZE);
>  
> -		if (needs_ccs_emit)
> +		if (ccs_pack) {
> +			if (emit_ccs_copy_chunks(gt, bb, src_L0_ofs, dst_L0_ofs,
> +						 src_L0, src_tt, dst_tt,
> +						 start_page))
> +				flush_flags = MI_FLUSH_DW_CCS;
> +		} else if (needs_ccs_emit) {
>  			flush_flags = xe_migrate_ccs_copy(m, bb, src_L0_ofs,
>  							  IS_DGFX(xe) ? src_is_vram : src_is_pltt,
>  							  dst_L0_ofs,
>  							  IS_DGFX(xe) ? dst_is_vram : dst_is_pltt,
>  							  src_L0, ccs_ofs, copy_ccs);
> +		}

[Severity: Critical]
This is a pre-existing issue, but can this pass an uninitialized ccs_ofs stack
variable to the GPU blitter during a TT-to-TT CCS defrag pass on DGFX systems?

When __xe_migrate_copy() is called with flags.defrag_ccs = true,
copy_system_ccs evaluates to false, leaving ccs_ofs uninitialized.

Because !flags.defrag_copy is true, needs_ccs_emit evaluates to true and calls
xe_migrate_ccs_copy(). For a TT source on DGFX, IS_DGFX(xe) ? src_is_vram :
src_is_pltt evaluates to false (meaning src_is_indirect is false).

xe_migrate_ccs_copy() then consumes the uninitialized ccs_ofs:

drivers/gpu/drm/xe/xe_migrate.c:xe_migrate_ccs_copy() {
    ...
    } else if (copy_ccs) {
        if (!src_is_indirect)
            src_ofs = ccs_ofs;
    ...
}

Does this cause the uninitialized src_ofs to be emitted into the GPU batch
buffer, resulting in memory corruption?

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