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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] xe/tests: Endianness assumption in verify_access_memory_read()
- [Low] xe/tests: Unreachable dead code in unaligned_offset generation
--

commit 9484a3b8a65a7889a267c6cc520be578ff0d0b1f
Author: Jan Maslak <jan.maslak@intel.com>

drm/xe/tests: Add xe_migrate_access_memory subtest

This commit adds a new xe_migrate KUnit subtest to verify VRAM to SRAM
migration behavior. It specifically tests scenarios where the destination
address or the source buffer object 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
[ ... ]
> @@ -770,9 +773,308 @@ static void xe_validate_ccs_kunit(struct kunit *test)
> +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: Low]
Will this byte extraction logic fail on big-endian architectures?

The test buffer is primed using a host-endian assignment in
access_memory_test_run_tile():

	for (j = 0; j < ACCESS_MEM_NUM_CELLS; j++)
		((u32 *)buf)[j] = j;

However, extracting the expected byte using a bitwise right shift
implicitly assumes a little-endian memory layout (where byte 0 is the
least significant byte). On big-endian systems, the physical byte
sequence in memory won't match this extraction, causing spurious
test failures.

Should this either use cpu_to_le32(j) when priming the buffer, or use
an endian-aware extraction here?

[ ... ]
> +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 this condition reachable?

If unaligned_offset is initially 0 (which is 4-byte aligned), the
previous check will execute unaligned_offset |= 2, making it 2. Since
unaligned_offset is mathematically guaranteed to be strictly greater
than 0 after the alignment check, the zero-check here appears to be
unreachable dead code.

> +	} else {
> +		unaligned_offset = ACCESS_MEM_UNALIGNED_OFFSET;
> +	}

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