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 <[email protected]> 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/[email protected]?part=25
