Re: [PATCH 3/3] drm/xe: Do not clear SVM device memory allocations up front

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] xe/svm: oversized chunk lengths in migration cause kernel panic
--

commit ed6428a65926a64520b6fe556921289286c9c71f
Author: Matthew Brost <matthew.brost@intel.com>

    drm/xe: Do not clear SVM device memory allocations up front

The patch updates SVM device memory allocations to skip initial clearing
by using XE_BO_FLAG_SKIP_CLEAR. Instead, only sparse areas without CPU
source data are cleared explicitly during migration using the new
xe_svm_clear_vram() function.

> diff --git a/drivers/gpu/drm/xe/xe_svm.c b/drivers/gpu/drm/xe/xe_svm.c
> index f39e647512ad9..1b4d1222fbb7e 100644
> --- a/drivers/gpu/drm/xe/xe_svm.c
> +++ b/drivers/gpu/drm/xe/xe_svm.c
[ ... ]
> @@ -586,6 +586,124 @@ static void xe_svm_copy_us_stats_incr(struct xe_gt *gt,
[ ... ]
> +static int xe_svm_clear_vram(struct page **pages, unsigned long npages,
> +			     struct dma_fence **fence,
> +			     struct dma_fence **deps)
> +{
[ ... ]
> +		/* Not contiguous with the pending clear, or chunk is full */
> +		if (count && (addr != vram_addr + count * PAGE_SIZE ||
> +			      count + nr > XE_MIGRATE_CHUNK_SIZE / PAGE_SIZE)) {
> +			err = xe_svm_clear_vram_chunk(vr, vram_addr, count,
> +						      fence, deps);
> +			if (err)
> +				return err;
> +			count = 0;
> +		}
> +
> +		if (!count)
> +			vram_addr = addr;
> +		count += nr;
> +		i += nr;

[Severity: High]
Does this logic allow oversized chunks to bypass the chunk size limit?

If a single device folio exceeds the 8MB limit (e.g., THP or 1GB huge pages)
and is the first page processed in a chunk (where count == 0), the check
is skipped.

This would cause the oversized nr to be added to count directly, which later
gets passed to xe_svm_clear_vram_chunk() and xe_migrate_clear_vram().

Could this trigger a kernel panic via an xe_assert() or cause hardware
corruption by emitting oversized memory copy commands that exceed max
parameters?

[ ... ]
> @@ -617,7 +746,6 @@ static int xe_svm_copy(struct page **pages,
>  		u64 __vram_addr;
>  		bool match = false, chunk, last;
>  
> -#define XE_MIGRATE_CHUNK_SIZE	SZ_8M
>  		chunk = (i - pos) == (XE_MIGRATE_CHUNK_SIZE / PAGE_SIZE);
>  		last = (i + 1) == npages;

[Severity: High]
Can i - pos entirely overshoot the exact equality check here and later
in the function?

Further down in xe_svm_copy(), the chunk flag is updated for large orders:

			if (pagemap_addr[i].order) {
				i += NR_PAGES(pagemap_addr[i].order) - 1;
				chunk = (i - pos) == (XE_MIGRATE_CHUNK_SIZE / PAGE_SIZE);
				last = (i + 1) == npages;
			}

If a large system folio's NR_PAGES (such as a 1GB huge page) is added to i,
could i - pos jump completely over the exact XE_MIGRATE_CHUNK_SIZE boundary,
causing chunk to evaluate to false?

This would bypass the chunk splitting, resulting in an oversized transfer length
passed to xe_migrate_to_vram() or xe_migrate_from_vram(), which could lead to
hardware command corruption and a GPU hang.

[ ... ]

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