Re: [PATCH v3 25/33] drm/xe: Preallocate system BO backing outside the dma-resv lock

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] `xe_device_ccs_bytes()` truncates the calculated CCS size to `u32`, causing massive under-allocation of the `ttm_tt` pages array for buffers >= 1TB.
--

--- Patch [25]: [PATCH v3 25/33] drm/xe: Preallocate system BO backing outside the dma-resv lock ---
commit 4ef9a2744ced48ab96c8a05eeb4fe0fa0b3bafac
Author: Matthew Brost <matthew.brost@intel.com>

drm/xe: Preallocate system BO backing outside the dma-resv lock
    
This commit uses the TTM full-preallocation mechanism to allocate the entire
tt backing up front before entering the validation guard for system-only,
non-deferred user BOs. It sizes the preallocation by querying the CCS-pages
decision directly from the ioctl.

> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
> index 25e5b72f870c0..a646a72122d40 100644
> --- a/drivers/gpu/drm/xe/xe_bo.c
> +++ b/drivers/gpu/drm/xe/xe_bo.c

[ ... ]

> @@ -4018,6 +4033,64 @@ int xe_gem_create_ioctl(struct drm_device *dev, void *data,
>  			return -ENOENT;
>  	}
>  
> +	/*
> +	 * For system-only BOs, preallocate the whole backing outside the
> +	 * dma-resv lock so the populate under the validation guard does not
> +	 * stall in reclaim/compaction. The caching mirrors the system-BO path
> +	 * in xe_ttm_tt_create(); the pool falls back to in-line allocation for
> +	 * any shortfall, so a failed fill is harmless.
> +	 */
> +	if ((bo_flags & XE_BO_FLAG_SYSTEM) &&
> +	    !(bo_flags & (XE_BO_FLAG_DEFER_BACKING | XE_BO_FLAG_VRAM_MASK |
> +			  XE_BO_FLAG_STOLEN))) {
> +		struct ttm_resource_manager *tt_man =
> +			ttm_manager_type(&xe->ttm, XE_PL_TT);
> +		enum ttm_caching caching = ttm_cached;
> +		unsigned long num_pages = args->size >> PAGE_SHIFT;
> +
> +		if (!IS_DGFX(xe) &&
> +		    args->cpu_caching == DRM_XE_GEM_CPU_CACHING_WC)
> +			caching = ttm_write_combined;
> +
> +		/*
> +		 * Cover the CCS backup pages appended to the tt (see
> +		 * xe_ttm_tt_create()) so the whole tt is preallocated.
> +		 */
> +		if (__xe_bo_needs_ccs_pages(xe, ttm_bo_type_device, bo_flags,
> +					    args->cpu_caching))
> +			num_pages += DIV_ROUND_UP(xe_device_ccs_bytes(xe,
> +								      args->size),
> +						  PAGE_SIZE);

[Severity: Critical]
This is a pre-existing issue, but does xe_device_ccs_bytes() truncate the
calculated size for very large buffers?

Looking at drivers/gpu/drm/xe/xe_device.c:

u32 xe_device_ccs_bytes(struct xe_device *xe, u64 size)
{
	return xe_device_has_flat_ccs(xe) ?
		DIV_ROUND_UP_ULL(size, NUM_BYTES_PER_CCS_BYTE(xe)) : 0;
}

Because the return type is u32, if a user requests a buffer of 1TB or larger,
the calculated CCS size truncates. For a 1TB buffer with a 256:1 compression
ratio, size / 256 exactly equals 0x100000000, which truncates to 0.

This means num_pages will not include the necessary CCS backup pages (and
similarly extra_pages in xe_ttm_tt_create() will be 0), which can lead to a
massive heap buffer overflow when the driver or GPU writes past the end of
the under-sized ttm_tt pages array.

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