Re: [PATCH v2 1/4] drm/xe/xe_migrate: Align MEM_COPY pitch with destination address

On 22/06/2026 11:34, Jan Maslak wrote:
> From: Christoph Manszewski <[email protected]>
> 
> MEM_COPY_CMD can corrupt memory when MATRIX_COPY mode uses a pitch that
> is not aligned to the destination address.
> 
> Fix this by incorporating the destination address into the pitch
> selection so the chosen pitch is aligned to both the copy length and the
> destination address.

Should we play it safe and apply this to the src addr also, even if it 
appears to work without? The spec doesn't explicitly state that the dst 
needed this special treatment, but if it does then I would think src 
would be the same?

> 
> Signed-off-by: Christoph Manszewski <[email protected]>
> Signed-off-by: Jan Maslak <[email protected]>
> ---
>   drivers/gpu/drm/xe/xe_migrate.c | 29 +++++++++++++++++------------
>   1 file changed, 17 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_migrate.c b/drivers/gpu/drm/xe/xe_migrate.c
> index 9428dd5e7760..9218eba31d45 100644
> --- a/drivers/gpu/drm/xe/xe_migrate.c
> +++ b/drivers/gpu/drm/xe/xe_migrate.c
> @@ -2186,17 +2186,18 @@ static bool xe_migrate_vram_use_pde(struct drm_pagemap_addr *sram_addr,
>   #define XE_CACHELINE_BYTES	64ull
>   #define XE_CACHELINE_MASK	(XE_CACHELINE_BYTES - 1)
>   
> -static u32 xe_migrate_copy_pitch(struct xe_device *xe, u32 len)
> +static u32 xe_migrate_copy_pitch(struct xe_device *xe, u32 len, u64 dst_addr)
>   {
> +	u64 align_check = len | dst_addr;
>   	u32 pitch;
>   
> -	if (IS_ALIGNED(len, PAGE_SIZE))
> +	if (IS_ALIGNED(align_check, PAGE_SIZE))
>   		pitch = PAGE_SIZE;
> -	else if (IS_ALIGNED(len, SZ_4K))
> +	else if (IS_ALIGNED(align_check, SZ_4K))
>   		pitch = SZ_4K;
> -	else if (IS_ALIGNED(len, SZ_256))
> +	else if (IS_ALIGNED(align_check, SZ_256))
>   		pitch = SZ_256;
> -	else if (IS_ALIGNED(len, 4))
> +	else if (IS_ALIGNED(align_check, 4))
>   		pitch = 4;
>   	else
>   		pitch = 1;
> @@ -2223,16 +2224,11 @@ static struct dma_fence *xe_migrate_vram(struct xe_migrate *m,
>   	struct xe_bb *bb;
>   	u32 update_idx, pt_slot = 0;
>   	unsigned long npages = DIV_ROUND_UP(len + sram_offset, PAGE_SIZE);
> -	unsigned int pitch = xe_migrate_copy_pitch(xe, len);
> +	unsigned int pitch;
>   	int err;
>   	unsigned long i, j;
>   	bool use_pde = xe_migrate_vram_use_pde(sram_addr, len + sram_offset);
>   
> -	if (!xe->info.has_mem_copy_instr &&
> -	    drm_WARN_ON(&xe->drm,
> -			(!IS_ALIGNED(len, pitch)) || (sram_offset | vram_addr) & XE_CACHELINE_MASK))
> -		return ERR_PTR(-EOPNOTSUPP);
> -
>   	xe_assert(xe, npages * PAGE_SIZE <= MAX_PREEMPTDISABLE_TRANSFER);
>   
>   	batch_size += pte_update_cmd_size(npages << PAGE_SHIFT);
> @@ -2284,6 +2280,13 @@ static struct dma_fence *xe_migrate_vram(struct xe_migrate *m,
>   			dst_L0_ofs = xe_migrate_vm_addr(pt_slot, 0) + sram_offset;
>   	}
>   
> +	pitch = xe_migrate_copy_pitch(xe, len, dst_L0_ofs);
> +	if (!xe->info.has_mem_copy_instr &&
> +	    drm_WARN_ON(&xe->drm,
> +			(!IS_ALIGNED(len, pitch)) ||
> +			(sram_offset | vram_addr) & XE_CACHELINE_MASK))
> +		return ERR_PTR(-EOPNOTSUPP);
> +
>   	bb->cs[bb->len++] = MI_BATCH_BUFFER_END;
>   	update_idx = bb->len;
>   
> @@ -2536,7 +2539,9 @@ int xe_migrate_access_memory(struct xe_migrate *m, struct xe_bo *bo,
>   		else
>   			current_bytes = min_t(int, bytes_left, cursor.size);
>   
> -		pitch = xe_migrate_copy_pitch(xe, current_bytes);
> +		pitch = xe_migrate_copy_pitch(xe, current_bytes,
> +					      write ? vram_addr :
> +					      (unsigned long)buf & ~PAGE_MASK);
>   		if (xe->info.has_mem_copy_instr)
>   			current_bytes = min_t(int, current_bytes, U16_MAX * pitch);
>   		else