[PATCH v4 0/4] drm/xe/xe_migrate: Fix memory corruption with unaligned dst in MEM_COPY

[PATCH v4 0/4] drm/xe/xe_migrate: Fix memory corruption with unaligned dst in MEM_COPY

[Jan Maslak]

MEM_COPY_CMD in MATRIX_COPY mode produces incorrect results when the
destination address is not aligned to the pitch. This manifests as
memory corruption during xe_migrate_access_memory() reads from VRAM.

The probable cause appears to be a hardware requirement that the
destination address must be aligned to the pitch when using MATRIX_COPY
mode.

This was observed on BMG during xe_migrate_access_memory() calls where
the destination offset within a page was not aligned to the computed
pitch value.

Patch 1 fixes an independent page tracking bug in
xe_migrate_access_memory().

Patch 2 adds the minimal workaround by selecting a pitch that is aligned
to both the copy length and the destination address.

Patch 3 adds a follow-up optimization to reduce the number of linear
copy jobs for unaligned destinations.

Patch 4 adds a KUnit live test that exercises xe_migrate_access_memory()
with various unaligned destination offsets to prevent regressions.

Tested on BMG (Battlemage).

v2:
 - Split the original xe_migrate change into a minimal workaround,
   a standalone page tracking fix, and a follow-up optimization
   per Matthew Auld's review feedback
v3:
 - Used a helper function and added an additional comment in patch 3
   per Matthew Auld's review feedback
v4:
 - Rebased onto current drm-xe-next
 - Moved the page tracking fix before the pitch workaround, so the
   series stays correct at every commit
 - Patch 2: free the batch buffer when returning -EOPNOTSUPP instead
   of leaking it
 - Patch 3: use a single max_bytes limit to shorten an over-long line
 - Patch 4: validate the BO into VRAM and wait for it to be idle
   after locking it, before accessing it

Christoph Manszewski (2):
  drm/xe/xe_migrate: Fix page tracking in access_memory
  drm/xe/xe_migrate: Align MEM_COPY pitch with destination address

Jan Maslak (2):
  drm/xe/xe_migrate: Optimize unaligned access_memory copies
  drm/xe/tests: Add xe_migrate_access_memory subtest

 drivers/gpu/drm/xe/tests/xe_migrate.c | 302 ++++++++++++++++++++++++++
 drivers/gpu/drm/xe/xe_migrate.c       | 176 +++++++++++++--
 2 files changed, 459 insertions(+), 19 deletions(-)


base-commit: f7f8e453bef8ea29ae3615e5308eb2d4c250545d
-- 
2.43.0

[PATCH v4 1/4] drm/xe/xe_migrate: Fix page tracking in access_memory

[Jan Maslak]

From: Christoph Manszewski <christoph.manszewski@intel.com>

xe_migrate_access_memory() advances the pagemap cursor based on the
current page in the CPU buffer.

Include the initial page offset when recalculating current_page so
subsequent iterations continue to index the mapped pages correctly.

Signed-off-by: Christoph Manszewski <christoph.manszewski@intel.com>
Signed-off-by: Jan Maslak <jan.maslak@intel.com>
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
---
 drivers/gpu/drm/xe/xe_migrate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/xe/xe_migrate.c b/drivers/gpu/drm/xe/xe_migrate.c
index fffb3eb6c76e..3711d4542ad3 100644
--- a/drivers/gpu/drm/xe/xe_migrate.c
+++ b/drivers/gpu/drm/xe/xe_migrate.c
@@ -2563,7 +2563,7 @@ int xe_migrate_access_memory(struct xe_migrate *m, struct xe_bo *bo,
 
 		buf += current_bytes;
 		offset += current_bytes;
-		current_page = (int)(buf - orig_buf) / PAGE_SIZE;
+		current_page = (int)(buf - orig_buf + page_offset) / PAGE_SIZE;
 		bytes_left -= current_bytes;
 		if (bytes_left)
 			xe_res_next(&cursor, current_bytes);
-- 
2.43.0

[PATCH v4 2/4] drm/xe/xe_migrate: Align MEM_COPY pitch with destination address

[Jan Maslak]

From: Christoph Manszewski <christoph.manszewski@intel.com>

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.

Signed-off-by: Christoph Manszewski <christoph.manszewski@intel.com>
Signed-off-by: Jan Maslak <jan.maslak@intel.com>
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
---
 drivers/gpu/drm/xe/xe_migrate.c | 31 +++++++++++++++++++------------
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_migrate.c b/drivers/gpu/drm/xe/xe_migrate.c
index 3711d4542ad3..4b995c93fcea 100644
--- a/drivers/gpu/drm/xe/xe_migrate.c
+++ b/drivers/gpu/drm/xe/xe_migrate.c
@@ -2187,17 +2187,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;
@@ -2224,16 +2225,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);
@@ -2285,6 +2281,15 @@ 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)) {
+		err = -EOPNOTSUPP;
+		goto err;
+	}
+
 	bb->cs[bb->len++] = MI_BATCH_BUFFER_END;
 	update_idx = bb->len;
 
@@ -2535,7 +2540,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
-- 
2.43.0

[PATCH v4 3/4] drm/xe/xe_migrate: Optimize unaligned access_memory copies

[Jan Maslak]

When xe_migrate_access_memory() falls back to a smaller pitch for an
unaligned destination, the copy can end up running linearly for the
entire transfer.

Reduce the number of copy jobs by first using a short linear copy to
reach a better destination alignment, then using the largest matrix copy
pitch that alignment supports, and finally using a linear copy for any
remaining tail bytes.

Signed-off-by: Christoph Manszewski <christoph.manszewski@intel.com>
Signed-off-by: Jan Maslak <jan.maslak@intel.com>
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
---
 drivers/gpu/drm/xe/xe_migrate.c | 149 ++++++++++++++++++++++++++++++--
 1 file changed, 140 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_migrate.c b/drivers/gpu/drm/xe/xe_migrate.c
index 4b995c93fcea..5609f29b28fe 100644
--- a/drivers/gpu/drm/xe/xe_migrate.c
+++ b/drivers/gpu/drm/xe/xe_migrate.c
@@ -2207,6 +2207,86 @@ static u32 xe_migrate_copy_pitch(struct xe_device *xe, u32 len, u64 dst_addr)
 	return pitch;
 }
 
+/**
+ * xe_migrate_dst_pitch() - Determine max pitch supported by dst alignment
+ * @dst_addr: Destination address (page offset)
+ *
+ * Returns the largest pitch (PAGE_SIZE, SZ_4K, SZ_256, or 4) that dst_addr
+ * supports based solely on its alignment. Returns 1 if not even 4-byte aligned.
+ */
+static u32 xe_migrate_dst_pitch(u64 dst_addr)
+{
+	if (IS_ALIGNED(dst_addr, PAGE_SIZE))
+		return PAGE_SIZE;
+	if (IS_ALIGNED(dst_addr, SZ_4K))
+		return SZ_4K;
+	if (IS_ALIGNED(dst_addr, SZ_256))
+		return SZ_256;
+	if (IS_ALIGNED(dst_addr, 4))
+		return 4;
+	return 1;
+}
+
+/**
+ * xe_migrate_bytes_to_align() - Calculate bytes needed to reach target alignment
+ * @dst_addr: Current destination address
+ * @alignment: Target alignment (PAGE_SIZE, SZ_4K, SZ_256, or 4)
+ *
+ * Returns: Number of bytes to copy to reach the target alignment.
+ */
+static u32 xe_migrate_bytes_to_align(u64 dst_addr, u32 alignment)
+{
+	return round_up(dst_addr, alignment) - dst_addr;
+}
+
+/**
+ * xe_migrate_best_alignment() - Find the best alignment reachable within max_bytes
+ * @dst_addr: Current destination address
+ * @max_bytes: Maximum bytes we can copy to reach alignment (typically U16_MAX)
+ *
+ * Returns the largest alignment (PAGE_SIZE, SZ_4K, SZ_256, or 4) that dst_addr
+ * can reach by copying at most max_bytes. Returns 0 if already at best alignment
+ * or if no useful alignment can be reached.
+ */
+static u32 xe_migrate_best_alignment(u64 dst_addr, u32 max_bytes)
+{
+	u32 bytes_to_page = xe_migrate_bytes_to_align(dst_addr, PAGE_SIZE);
+	u32 bytes_to_4k = xe_migrate_bytes_to_align(dst_addr, SZ_4K);
+	u32 bytes_to_256 = xe_migrate_bytes_to_align(dst_addr, SZ_256);
+	u32 bytes_to_4 = xe_migrate_bytes_to_align(dst_addr, 4);
+
+	if (bytes_to_page == 0)
+		return 0;
+
+	if (bytes_to_page <= max_bytes)
+		return PAGE_SIZE;
+
+	if (bytes_to_4k > 0 && bytes_to_4k <= max_bytes)
+		return SZ_4K;
+
+	if (bytes_to_256 > 0 && bytes_to_256 <= max_bytes)
+		return SZ_256;
+
+	if (bytes_to_4 > 0 && bytes_to_4 <= max_bytes)
+		return 4;
+
+	return 0;
+}
+
+/**
+ * xe_migrate_aligned_copy_len() - Calculate the aligned copy length for matrix copy
+ * @len: Remaining bytes
+ * @pitch: The pitch to use for matrix copy
+ *
+ * Returns: Length rounded down to pitch, or 0 if less than pitch.
+ */
+static u32 xe_migrate_aligned_copy_len(u32 len, u32 pitch)
+{
+	if (len < pitch)
+		return 0;
+	return round_down(len, pitch);
+}
+
 static struct dma_fence *xe_migrate_vram(struct xe_migrate *m,
 					 unsigned long len,
 					 unsigned long sram_offset,
@@ -2540,15 +2620,66 @@ 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,
-					      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
-			current_bytes = min_t(int, current_bytes,
-					      round_down(S16_MAX * pitch,
-							 XE_CACHELINE_BYTES));
+		{
+			/*
+			 * Optimized copy strategy to minimize number of jobs:
+			 * 1. Linear copy to align dst to best reachable alignment
+			 * 2. Matrix copy for bulk data copy
+			 * 3. Single linear copy for remaining bytes
+			 *
+			 * The phase logic controls transfer chunking; emit_copy() selects
+			 * MEM_COPY_CMD or XY_FAST_COPY_BLT based on hardware support.
+			 */
+			u64 dst_addr = write ? vram_addr :
+					       (unsigned long)buf & ~PAGE_MASK;
+			u32 max_linear = xe->info.has_mem_copy_instr ? U16_MAX :
+					 round_down(S16_MAX, XE_CACHELINE_BYTES);
+			u32 best_align = xe_migrate_best_alignment(dst_addr, max_linear);
+			u32 bytes_to_align = best_align ?
+					     xe_migrate_bytes_to_align(dst_addr, best_align) : 0;
+
+			if (bytes_to_align > 0 && bytes_to_align <= (u32)current_bytes) {
+				/*
+				 * Phase 1: Linear copy to reach best alignment.
+				 * Copy just enough bytes to align dst_addr.
+				 */
+				pitch = 1;
+				current_bytes = min_t(int, bytes_to_align, max_linear);
+			} else if (IS_ALIGNED(dst_addr, 4)) {
+				/*
+				 * Phase 2: Matrix copy for aligned bulk data.
+				 * Use the best pitch that dst alignment supports,
+				 * then round down len to that pitch.
+				 */
+				u32 dst_pitch = xe_migrate_dst_pitch(dst_addr);
+				u32 aligned_len = xe_migrate_aligned_copy_len(current_bytes,
+								      dst_pitch);
+
+				if (aligned_len >= dst_pitch) {
+					u32 max_bytes = xe->info.has_mem_copy_instr ?
+							U16_MAX * dst_pitch :
+							round_down(S16_MAX * dst_pitch,
+								   XE_CACHELINE_BYTES);
+
+					pitch = dst_pitch;
+					current_bytes = min_t(int, aligned_len, max_bytes);
+				} else {
+					/*
+					 * Phase 3: Linear copy for remainder.
+					 * Not enough for matrix copy - do single linear copy.
+					 */
+					pitch = 1;
+					current_bytes = min_t(int, current_bytes, max_linear);
+				}
+			} else {
+				/*
+				 * Phase 3: Linear copy for remainder.
+				 * dst not 4-aligned and can't reach alignment - linear copy.
+				 */
+				pitch = 1;
+				current_bytes = min_t(int, current_bytes, max_linear);
+			}
+		}
 
 		__fence = xe_migrate_vram(m, current_bytes,
 					  (unsigned long)buf & ~PAGE_MASK,
-- 
2.43.0

[PATCH v4 4/4] drm/xe/tests: Add xe_migrate_access_memory subtest

[Jan Maslak]

Add a new xe_migrate subtest for vram to sram migration works:
 - when the dst address is not 4-byte aligned
 - when the dst address in not 4-byte aligned but the migration size is
 - when src BO offset is not 4-byte aligned

Signed-off-by: Christoph Manszewski <christoph.manszewski@intel.com>
Signed-off-by: Jan Maslak <jan.maslak@intel.com>
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
---
 drivers/gpu/drm/xe/tests/xe_migrate.c | 302 ++++++++++++++++++++++++++
 1 file changed, 302 insertions(+)

diff --git a/drivers/gpu/drm/xe/tests/xe_migrate.c b/drivers/gpu/drm/xe/tests/xe_migrate.c
index f10d9513747b..d4454dc70441 100644
--- a/drivers/gpu/drm/xe/tests/xe_migrate.c
+++ b/drivers/gpu/drm/xe/tests/xe_migrate.c
@@ -6,9 +6,12 @@
 #include <kunit/test.h>
 #include <kunit/visibility.h>
 
+#include <linux/prandom.h>
+
 #include "tests/xe_kunit_helpers.h"
 #include "tests/xe_pci_test.h"
 
+#include "xe_migrate.h"
 #include "xe_pat.h"
 #include "xe_pci.h"
 #include "xe_pm.h"
@@ -770,9 +773,308 @@ static void xe_validate_ccs_kunit(struct kunit *test)
 	validate_ccs_test_run_device(xe);
 }
 
+#define ACCESS_MEM_BO_SIZE		SZ_512K
+#define ACCESS_MEM_NUM_CELLS		(ACCESS_MEM_BO_SIZE / sizeof(u32))
+#define ACCESS_MEM_UNALIGNED_OFFSET	0x215ee
+
+/*
+ * Verify byte-by-byte that data read from the BO matches expected cell pattern.
+ * The BO is primed with u32 cells where cell[i] = i, so BO byte at offset B
+ * has value: (B / 4 >> (B%4 * 8)) & 0xFF.
+ *
+ * @buf:       the target buffer
+ * @buf_start: index into buf where read data begins
+ * @bo_start:  the BO byte offset corresponding to buf[buf_start]
+ * @size:      number of bytes to verify
+ * @scenario:  label for error messages
+ *
+ * Returns true if all bytes match, false on first mismatch.
+ */
+static bool verify_access_memory_read(struct kunit *test, u8 *buf,
+				      u32 buf_start, u32 bo_start,
+				      u32 size, const char *scenario)
+{
+	u32 j;
+
+	for (j = 0; j < size; j++) {
+		u32 bo_byte_offset = bo_start + j;
+		u32 byte_in_cell = bo_byte_offset % sizeof(u32);
+		u32 cell_idx = bo_byte_offset / sizeof(u32);
+		u32 cell_value = cell_idx;
+		u8 expected_byte = (cell_value >> (byte_in_cell * 8)) & 0xFF;
+		u8 got_byte = buf[buf_start + j];
+
+		if (got_byte != expected_byte) {
+			u32 context_end = min(j + 8, size - 1);
+			u32 context_start = (j >= 8) ? j - 8 : 0;
+			u32 k;
+
+			KUNIT_FAIL(test,
+				   "%s MISMATCH\n"
+				   "  Buffer index:    %u (0x%x)\n"
+				   "  BO offset:       0x%x\n"
+				   "  Cell index:      %u (BO offset 0x%x..0x%x)\n"
+				   "  Byte in cell:    %u\n"
+				   "  Expected value:  0x%02x (from cell_value 0x%08x)\n"
+				   "  Got value:       0x%02x\n",
+				   scenario,
+				   buf_start + j, buf_start + j,
+				   bo_byte_offset,
+				   cell_idx, cell_idx * 4, cell_idx * 4 + 3,
+				   byte_in_cell,
+				   expected_byte, cell_value,
+				   got_byte);
+
+			kunit_info(test, "Buffer context around mismatch (buf[%u..%u]):\n",
+				   buf_start + context_start,
+				   buf_start + context_end);
+
+			for (k = context_start; k <= context_end; k++) {
+				u32 k_bo_offset = bo_start + k;
+				u32 k_cell = k_bo_offset / sizeof(u32);
+				u32 k_byte = k_bo_offset % sizeof(u32);
+				u32 k_cell_val = k_cell;
+				u8 k_expected = (k_cell_val >> (k_byte * 8)) & 0xFF;
+
+				kunit_info(test,
+					   "  buf[%4u] @ BO 0x%05x: got 0x%02x, expected 0x%02x %s\n",
+					   buf_start + k, k_bo_offset,
+					   buf[buf_start + k], k_expected,
+					   (k == j) ? "<-- MISMATCH" : "");
+			}
+			return false;
+		}
+	}
+	return true;
+}
+
+static void access_memory_test_unaligned_bo_offset(struct kunit *test,
+						    struct xe_migrate *m,
+						    struct xe_bo *bo, u8 *buf,
+						    u32 unaligned_offset)
+{
+	u32 partial_size = ACCESS_MEM_BO_SIZE - unaligned_offset;
+	int ret;
+
+	memset(buf, 0, ACCESS_MEM_BO_SIZE);
+
+	kunit_info(test, "Partial read: bo_offset=0x%x bytes=0x%x\n",
+		   unaligned_offset, partial_size);
+
+	ret = xe_migrate_access_memory(m, bo, unaligned_offset, buf,
+				       partial_size, false);
+	if (ret) {
+		KUNIT_FAIL(test, "Partial read failed: %d\n", ret);
+		return;
+	}
+
+	if (verify_access_memory_read(test, buf, 0, unaligned_offset,
+				      partial_size,
+				      "Partial read from unaligned BO offset"))
+		kunit_info(test, "Partial read from unaligned BO offset 0x%x verified\n",
+			   unaligned_offset);
+}
+
+static void access_memory_test_unaligned_dst(struct kunit *test,
+					     struct xe_migrate *m,
+					     struct xe_bo *bo, u8 *buf,
+					     u32 unaligned_offset)
+{
+	u32 partial_size = ACCESS_MEM_BO_SIZE - unaligned_offset;
+	int ret;
+
+	memset(buf, 0, ACCESS_MEM_BO_SIZE);
+
+	kunit_info(test, "Read to unaligned dst: bo_offset=0x0 dst_offset=0x%x bytes=0x%x\n",
+		   unaligned_offset, partial_size);
+
+	ret = xe_migrate_access_memory(m, bo, 0, buf + unaligned_offset,
+				       partial_size, false);
+	if (ret) {
+		KUNIT_FAIL(test, "Read to unaligned dst failed: %d\n", ret);
+		return;
+	}
+
+	if (verify_access_memory_read(test, buf, unaligned_offset, 0,
+				      partial_size,
+				      "Read to unaligned dst"))
+		kunit_info(test, "Read to unaligned dst offset 0x%x verified\n",
+			   unaligned_offset);
+}
+
+static void access_memory_test_unaligned_dst_aligned_size(struct kunit *test,
+							  struct xe_migrate *m,
+							  struct xe_bo *bo,
+							  u8 *buf,
+							  u32 unaligned_offset)
+{
+	u32 partial_size = round_down(ACCESS_MEM_BO_SIZE - unaligned_offset, 4);
+	int ret;
+
+	memset(buf, 0, ACCESS_MEM_BO_SIZE);
+
+	kunit_info(test, "Read to unaligned dst (aligned size): bo_offset=0x0 dst_offset=0x%x bytes=0x%x\n",
+		   unaligned_offset, partial_size);
+
+	ret = xe_migrate_access_memory(m, bo, 0, buf + unaligned_offset,
+				       partial_size, false);
+	if (ret) {
+		KUNIT_FAIL(test, "Read to unaligned dst (aligned size) failed: %d\n", ret);
+		return;
+	}
+
+	if (verify_access_memory_read(test, buf, unaligned_offset, 0,
+				      partial_size,
+				      "Read to unaligned dst (aligned size)"))
+		kunit_info(test, "Read to unaligned dst (aligned size) offset 0x%x verified\n",
+			   unaligned_offset);
+}
+
+static void access_memory_test_run_tile(struct xe_device *xe,
+					struct xe_tile *tile,
+					struct kunit *test,
+					bool use_random_offset)
+{
+	struct drm_exec *exec = XE_VALIDATION_OPT_OUT;
+	unsigned int bo_flags = XE_BO_FLAG_VRAM_IF_DGFX(tile);
+	struct xe_migrate *m = tile->migrate;
+	u32 j, unaligned_offset;
+	struct xe_bo *bo;
+	long timeout;
+	u8 *buf;
+	int ret;
+
+	if (use_random_offset) {
+		unaligned_offset = get_random_u32() % (ACCESS_MEM_BO_SIZE / 2);
+		/* Ensure the offset is not 4-byte aligned */
+		if (IS_ALIGNED(unaligned_offset, 4))
+			unaligned_offset |= 2;
+		if (!unaligned_offset)
+			unaligned_offset = 3;
+	} else {
+		unaligned_offset = ACCESS_MEM_UNALIGNED_OFFSET;
+	}
+
+	kunit_info(test, "Using unaligned offset: 0x%x (%s)\n",
+		   unaligned_offset, use_random_offset ? "random" : "predefined");
+
+	buf = kvmalloc(ACCESS_MEM_BO_SIZE, GFP_KERNEL);
+	if (!buf) {
+		KUNIT_FAIL(test, "Failed to allocate buffer\n");
+		return;
+	}
+
+	bo = xe_bo_create_user(xe, NULL, ACCESS_MEM_BO_SIZE,
+			       DRM_XE_GEM_CPU_CACHING_WC,
+			       bo_flags | XE_BO_FLAG_NEEDS_CPU_ACCESS, NULL);
+	if (IS_ERR(bo)) {
+		KUNIT_FAIL(test, "Failed to create bo: %pe\n", bo);
+		goto free_buf;
+	}
+
+	/* Prime the BO */
+	for (j = 0; j < ACCESS_MEM_NUM_CELLS; j++)
+		((u32 *)buf)[j] = j;
+
+	kunit_info(test, "Writing %lu KiB to VRAM bo\n",
+		   (unsigned long)ACCESS_MEM_BO_SIZE / SZ_1K);
+
+	xe_bo_lock(bo, false);
+
+	/* The BO is unpinned, so make sure it is in VRAM once locked */
+	ret = xe_bo_validate(bo, NULL, false, exec);
+	if (ret) {
+		KUNIT_FAIL(test, "Failed to validate bo: %d\n", ret);
+		goto cleanup_bo;
+	}
+
+	if (!xe_bo_is_vram(bo)) {
+		KUNIT_FAIL(test, "BO is not in VRAM after validation\n");
+		goto cleanup_bo;
+	}
+
+	timeout = dma_resv_wait_timeout(bo->ttm.base.resv, DMA_RESV_USAGE_KERNEL,
+					true, 5 * HZ);
+	if (timeout <= 0) {
+		KUNIT_FAIL(test, "Failed to wait for bo to become idle: %ld\n",
+			   timeout);
+		goto cleanup_bo;
+	}
+
+	ret = xe_migrate_access_memory(m, bo, 0, buf, ACCESS_MEM_BO_SIZE, true);
+	if (ret) {
+		KUNIT_FAIL(test, "Failed to write bo: %d\n", ret);
+		goto cleanup_bo;
+	}
+
+	/* Full read */
+	memset(buf, 0, ACCESS_MEM_BO_SIZE);
+	kunit_info(test, "Full read: bo_offset=0x0 bytes=0x%x\n", ACCESS_MEM_BO_SIZE);
+
+	ret = xe_migrate_access_memory(m, bo, 0, buf, ACCESS_MEM_BO_SIZE, false);
+	if (ret) {
+		KUNIT_FAIL(test, "Full read failed: %d\n", ret);
+		goto cleanup_bo;
+	}
+
+	for (j = 0; j < ACCESS_MEM_NUM_CELLS; j++) {
+		u32 expected = j;
+		u32 got = ((u32 *)buf)[j];
+
+		if (got != expected) {
+			KUNIT_FAIL(test,
+				   "Full read mismatch at cell %u: expected 0x%08x, got 0x%08x\n",
+				   j, expected, got);
+			goto cleanup_bo;
+		}
+	}
+	kunit_info(test, "Full read verified\n");
+
+	access_memory_test_unaligned_bo_offset(test, m, bo, buf, unaligned_offset);
+	access_memory_test_unaligned_dst(test, m, bo, buf, unaligned_offset);
+	access_memory_test_unaligned_dst_aligned_size(test, m, bo, buf, unaligned_offset);
+
+cleanup_bo:
+	xe_bo_unlock(bo);
+	xe_bo_put(bo);
+free_buf:
+	kvfree(buf);
+}
+
+static int access_memory_test_run_device(struct xe_device *xe)
+{
+	struct kunit *test = kunit_get_current_test();
+	struct xe_tile *tile;
+	int id;
+
+	if (!IS_DGFX(xe)) {
+		kunit_skip(test, "non-discrete device\n");
+		return 0;
+	}
+
+	guard(xe_pm_runtime)(xe);
+	for_each_tile(tile, xe, id) {
+		kunit_info(test, "Testing tile id %d (predefined offset)\n", id);
+		access_memory_test_run_tile(xe, tile, test, false);
+
+		kunit_info(test, "Testing tile id %d (random offset)\n", id);
+		access_memory_test_run_tile(xe, tile, test, true);
+	}
+
+	return 0;
+}
+
+static void xe_migrate_access_memory_kunit(struct kunit *test)
+{
+	struct xe_device *xe = test->priv;
+
+	access_memory_test_run_device(xe);
+}
+
 static struct kunit_case xe_migrate_tests[] = {
 	KUNIT_CASE_PARAM(xe_migrate_sanity_kunit, xe_pci_live_device_gen_param),
 	KUNIT_CASE_PARAM(xe_validate_ccs_kunit, xe_pci_live_device_gen_param),
+	KUNIT_CASE_PARAM(xe_migrate_access_memory_kunit, xe_pci_live_device_gen_param),
 	{}
 };
 
-- 
2.43.0

Re: [PATCH v4 3/4] drm/xe/xe_migrate: Optimize unaligned access_memory copies

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] xe_migrate: Integer overflow in max_bytes calculation with 64K pages
- [Low] drm/xe: unused variable 'pitch' in xe_migrate_access_memory()
--

commit 1cfd0248d15da5743e2e644789c02a8f2508bbb8
Author: Jan Maslak <jan.maslak@intel.com>
Date:   Wed Oct 14 13:46:33 2026 +0000

    drm/xe/xe_migrate: Optimize unaligned access_memory copies

This commit modifies xe_migrate_access_memory() to optimize memory copies
by splitting transfers into an alignment phase, a matrix copy phase, and a
tail phase. It calculates the largest supported alignment pitch and
restricts copy lengths accordingly.

> diff --git a/drivers/gpu/drm/xe/xe_migrate.c b/drivers/gpu/drm/xe/xe_migrate.c
> index 4b995c93fcea9..5609f29b28fea 100644
> --- a/drivers/gpu/drm/xe/xe_migrate.c
> +++ b/drivers/gpu/drm/xe/xe_migrate.c
[ ... ]
> @@ -2540,15 +2620,66 @@ 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,
> -					      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
> -			current_bytes = min_t(int, current_bytes,
> -					      round_down(S16_MAX * pitch,
> -							 XE_CACHELINE_BYTES));
> +		{
> +			/*
> +			 * Optimized copy strategy to minimize number of jobs:
> +			 * 1. Linear copy to align dst to best reachable alignment
> +			 * 2. Matrix copy for bulk data copy
> +			 * 3. Single linear copy for remaining bytes
> +			 *
> +			 * The phase logic controls transfer chunking; emit_copy() selects
> +			 * MEM_COPY_CMD or XY_FAST_COPY_BLT based on hardware support.
> +			 */
> +			u64 dst_addr = write ? vram_addr :
> +					       (unsigned long)buf & ~PAGE_MASK;
> +			u32 max_linear = xe->info.has_mem_copy_instr ? U16_MAX :
> +					 round_down(S16_MAX, XE_CACHELINE_BYTES);
> +			u32 best_align = xe_migrate_best_alignment(dst_addr, max_linear);
> +			u32 bytes_to_align = best_align ?
> +					     xe_migrate_bytes_to_align(dst_addr, best_align) : 0;
> +
> +			if (bytes_to_align > 0 && bytes_to_align <= (u32)current_bytes) {
> +				/*
> +				 * Phase 1: Linear copy to reach best alignment.
> +				 * Copy just enough bytes to align dst_addr.
> +				 */
> +				pitch = 1;

[Severity: Low]
Is the pitch variable used anywhere after being assigned?

It looks like it is assigned multiple times across these branches but is never
read or passed to xe_migrate_vram() later in the loop. Could this unused
variable trigger compiler warnings with CONFIG_WERROR?

> +				current_bytes = min_t(int, bytes_to_align, max_linear);
> +			} else if (IS_ALIGNED(dst_addr, 4)) {
> +				/*
> +				 * Phase 2: Matrix copy for aligned bulk data.
> +				 * Use the best pitch that dst alignment supports,
> +				 * then round down len to that pitch.
> +				 */
> +				u32 dst_pitch = xe_migrate_dst_pitch(dst_addr);
> +				u32 aligned_len = xe_migrate_aligned_copy_len(current_bytes,
> +								      dst_pitch);
> +
> +				if (aligned_len >= dst_pitch) {
> +					u32 max_bytes = xe->info.has_mem_copy_instr ?
> +							U16_MAX * dst_pitch :
> +							round_down(S16_MAX * dst_pitch,
> +								   XE_CACHELINE_BYTES);
> +
> +					pitch = dst_pitch;
> +					current_bytes = min_t(int, aligned_len, max_bytes);

[Severity: High]
Can max_bytes overflow and become negative here when cast by min_t()?

On architectures supporting 64K pages (where PAGE_SIZE is 65536), max_bytes
would be evaluated as U16_MAX * dst_pitch (65535 * 65536 = 4294901760, or
0xFFFE0000).

Because min_t(int, ...) forces a cast to a signed 32-bit int, 0xFFFE0000
evaluates to -131072. Since -131072 is less than aligned_len, current_bytes
would become negative. This negative value would then be passed to
xe_migrate_vram() where it is implicitly cast to an unsigned long, wrapping
around to an enormous positive value and triggering out-of-bounds page table
indexing.

> +				} else {
> +					/*
> +					 * Phase 3: Linear copy for remainder.
> +					 * Not enough for matrix copy - do single linear copy.
> +					 */
> +					pitch = 1;
> +					current_bytes = min_t(int, current_bytes, max_linear);
> +				}
> +			} else {
> +				/*
> +				 * Phase 3: Linear copy for remainder.
> +				 * dst not 4-aligned and can't reach alignment - linear copy.
> +				 */
> +				pitch = 1;
> +				current_bytes = min_t(int, current_bytes, max_linear);
> +			}
> +		}
>  
>  		__fence = xe_migrate_vram(m, current_bytes,
>  					  (unsigned long)buf & ~PAGE_MASK,

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014134634.582314-1-jan.maslak@intel.com?part=3

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm/xe: integer overflow and assertion failure with 64K pages
--

commit c393bea53ab90ffca1212c4a87ea68cccbab7104
Author: Christoph Manszewski <c.manszewski@gmail.com>

drm/xe/xe_migrate: Align MEM_COPY pitch with destination address

MEM_COPY_CMD can corrupt memory when MATRIX_COPY mode uses a pitch that
is not aligned to the destination address. This commit fixes it by
incorporating the destination address into the pitch selection so the
chosen pitch is aligned to both the copy length and the destination address.

> diff --git a/drivers/gpu/drm/xe/xe_migrate.c b/drivers/gpu/drm/xe/xe_migrate.c
> index 3711d4542ad3d..4b995c93fcea9 100644
> --- a/drivers/gpu/drm/xe/xe_migrate.c
> +++ b/drivers/gpu/drm/xe/xe_migrate.c
> @@ -2187,17 +2187,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;

[Severity: Medium]
Could returning PAGE_SIZE here exceed the hardware's U16_MAX limit when 
the kernel is built with 64K pages, such as on ARM64 under COMPILE_TEST?

If PAGE_SIZE evaluates to 65536, this pitch will be greater than 65535, 
which would trigger the xe_gt_assert(gt, pitch <= U16_MAX) inside 
emit_mem_copy().

[ ... ]

> @@ -2535,7 +2540,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);

[Severity: Medium]
When pitch is 65536 due to 64K pages, does U16_MAX * pitch overflow here?

U16_MAX * 65536 evaluates to 4294901760. The min_t(int, ...) macro casts
this value to a signed 32-bit int, resulting in -65536. 

Since current_bytes becomes negative, and is later passed to xe_migrate_vram() 
as an unsigned long len, could this result in a massive length that triggers 
memory corruption or OOM panics during KUnit tests or CI systems?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014134634.582314-1-jan.maslak@intel.com?part=2

Re: [PATCH v4 4/4] drm/xe/tests: Add xe_migrate_access_memory subtest

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] xe: infinite recursion in xe_migrate_access_memory() due to unaligned kmalloc
- [Medium] drm/xe: test failure on big-endian architectures due to little-endian assumption
- [Low] drm/xe/tests: dead code in random offset generation
--

commit 123f5e095a34197f112fb5cb560c6b8a8cf51802
Author: Jan Maslak <jan.maslak@intel.com>

drm/xe/tests: Add xe_migrate_access_memory subtest

This commit adds a new xe_migrate subtest to verify VRAM to SRAM migration.
It explicitly tests migration when the destination address, migration size,
or source BO offset is not 4-byte aligned.

> diff --git a/drivers/gpu/drm/xe/tests/xe_migrate.c b/drivers/gpu/drm/xe/tests/xe_migrate.c
> --- a/drivers/gpu/drm/xe/tests/xe_migrate.c
> +++ b/drivers/gpu/drm/xe/tests/xe_migrate.c

[ ... ]

> +static bool verify_access_memory_read(struct kunit *test, u8 *buf,
> +				      u32 buf_start, u32 bo_start,
> +				      u32 size, const char *scenario)
> +{
> +	u32 j;
> +
> +	for (j = 0; j < size; j++) {
> +		u32 bo_byte_offset = bo_start + j;
> +		u32 byte_in_cell = bo_byte_offset % sizeof(u32);
> +		u32 cell_idx = bo_byte_offset / sizeof(u32);
> +		u32 cell_value = cell_idx;
> +		u8 expected_byte = (cell_value >> (byte_in_cell * 8)) & 0xFF;

[Severity: Medium]
Will this test falsely fail on big-endian architectures? 

Buffer generation in access_memory_test_run_tile() primes the test buffer
using native host endianness:

    ((u32 *)buf)[j] = j;

However, the bit shift here fundamentally assumes a little-endian memory
layout where the least significant byte is stored at the lowest address. On
a big-endian CPU, the most significant byte sits at the lowest address,
resulting in a mismatch. Should the code use explicit cpu_to_le32 and
le32_to_cpu macros to remain architecture-agnostic?

[ ... ]

> +static void access_memory_test_run_tile(struct xe_device *xe,
> +					struct xe_tile *tile,
> +					struct kunit *test,
> +					bool use_random_offset)
> +{
> +	struct drm_exec *exec = XE_VALIDATION_OPT_OUT;
> +	unsigned int bo_flags = XE_BO_FLAG_VRAM_IF_DGFX(tile);
> +	struct xe_migrate *m = tile->migrate;
> +	u32 j, unaligned_offset;
> +	struct xe_bo *bo;
> +	long timeout;
> +	u8 *buf;
> +	int ret;
> +
> +	if (use_random_offset) {
> +		unaligned_offset = get_random_u32() % (ACCESS_MEM_BO_SIZE / 2);
> +		/* Ensure the offset is not 4-byte aligned */
> +		if (IS_ALIGNED(unaligned_offset, 4))
> +			unaligned_offset |= 2;
> +		if (!unaligned_offset)
> +			unaligned_offset = 3;

[Severity: Low]
Is the zero-check fallback logic unreachable? 

Any initial zero value from get_random_u32() is transformed to 2 by the
preceding alignment enforcement, so unaligned_offset cannot be 0 at this
point. This appears to be unnecessary dead code.

[ ... ]

> +	access_memory_test_unaligned_bo_offset(test, m, bo, buf, unaligned_offset);

[Severity: High]
Could this unaligned testing trigger an infinite recursion and stack overflow
in xe_migrate_access_memory() when memory debugging is enabled? 

When xe->info.has_mem_copy_instr is false, xe_migrate_access_memory()
handles unaligned offsets by allocating a bounce buffer and then recursively
calling itself:

drivers/gpu/drm/xe/xe_migrate.c:xe_migrate_access_memory() {
    ...
    bounce = kmalloc(XE_CACHELINE_BYTES, GFP_KERNEL);
    ...
    err = xe_migrate_access_memory(m, bo,
                                   offset & ~XE_CACHELINE_MASK,
                                   bounce,
                                   XE_CACHELINE_BYTES, 0);
    ...
}

Under configurations like CONFIG_SLUB_DEBUG or CONFIG_KASAN, kmalloc()
for 64 bytes does not guarantee 64-byte alignment due to redzoning and
padding. If the returned bounce buffer is not cacheline aligned, the
recursive call's page_offset will be unaligned, causing the alignment check
to evaluate to true again and leading to unbounded recursion.

> +	access_memory_test_unaligned_dst(test, m, bo, buf, unaligned_offset);
> +	access_memory_test_unaligned_dst_aligned_size(test, m, bo, buf, unaligned_offset);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014134634.582314-1-jan.maslak@intel.com?part=4