Re: [PATCH v2 17/33] drm/xe: Add xe_migrate_copy_defrag() for on-GPU defrag copies

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Uninitialized variable `ccs_ofs` and kernel assertion failure in `xe_migrate_ccs_copy()` during `defrag_ccs` pass for TT memory on DGFX.
- [High] Defragmentation bypasses mandatory CCS metadata clearing for BOs without CCS, causing a security/information leak.
--

--- Patch [17]: [PATCH v2 17/33] drm/xe: Add xe_migrate_copy_defrag() for on-GPU defrag copies ---
commit b14779b7d60fb7520bb68944732a8ae6ceb089e7
Author: Matthew Brost <matthew.brost@intel.com>

drm/xe: Add xe_migrate_copy_defrag() for on-GPU defrag copies

Add xe_migrate_copy_defrag(), a copy helper for the upcoming page
defragmentation path where a single BO has its backing reallocated at a
more beneficial page order. The contents are relocated in up to two
passes (data and CCS) because they require opposite compression PAT
settings.

> diff --git a/drivers/gpu/drm/xe/xe_migrate.c b/drivers/gpu/drm/xe/xe_migrate.c
> index 9428dd5e7760c..956fd75454fa0 100644
> --- a/drivers/gpu/drm/xe/xe_migrate.c
> +++ b/drivers/gpu/drm/xe/xe_migrate.c
[ ... ]
> @@ -905,20 +931,29 @@ static struct dma_fence *__xe_migrate_copy(struct xe_migrate *m,
>  	bool src_is_vram = mem_type_is_vram(src->mem_type);
>  	bool dst_is_vram = mem_type_is_vram(dst->mem_type);
>  	bool type_device = src_bo->ttm.type == ttm_bo_type_device;
> -	bool needs_ccs_emit = type_device && xe_migrate_needs_ccs_emit(xe);
> +	bool needs_ccs_emit = type_device && xe_migrate_needs_ccs_emit(xe) &&
> +		!flags.defrag_copy;
> +	bool copy_ccs = xe_device_has_flat_ccs(xe) &&
> -		xe_bo_needs_ccs_pages(src_bo) && xe_bo_needs_ccs_pages(dst_bo);
> -	bool copy_system_ccs = copy_ccs && (!src_is_vram || !dst_is_vram);
> +		xe_bo_needs_ccs_pages(src_bo) && xe_bo_needs_ccs_pages(dst_bo) &&
> +		!flags.defrag_copy;
> +	bool copy_system_ccs = copy_ccs && (!src_is_vram || !dst_is_vram) &&
> +		!flags.defrag_ccs && !flags.defrag_copy;

[Severity: High]
If a BO does not use CCS, does forcing needs_ccs_emit to false during the
data pass bypass the mandatory security clearing of its CCS metadata?

In xe_migrate_copy_defrag(), if need_ccs is false, it performs the data pass
and returns early:

    struct dma_fence *fence =
        __xe_migrate_copy(..., (struct xe_migrate_copy_flags) {
            .defrag_copy = true,
        });

    if (IS_ERR(fence) || !need_ccs)
        return fence;

Since needs_ccs_emit is false when flags.defrag_copy is true,
xe_migrate_ccs_copy() is never called to clear the CCS metadata. Could this
leave stale cross-process data in the newly allocated destination pages?

[ ... ]
> @@ -1014,7 +1053,7 @@ static struct dma_fence *__xe_migrate_copy(struct xe_migrate *m,
>  		bb->cs[bb->len++] = MI_BATCH_BUFFER_END;
>  		update_idx = bb->len;
>  
> -		if (!copy_only_ccs)
> +		if (!flags.copy_only_ccs && !flags.defrag_ccs)
>  			emit_copy(gt, bb, src_L0_ofs, dst_L0_ofs, src_L0, XE_PAGE_SIZE);
>  
>  		if (needs_ccs_emit)

[Severity: Critical]
Can this pass an uninitialized ccs_ofs to xe_migrate_ccs_copy() and trigger
a kernel assertion failure on DGFX?

During a defrag_ccs pass for a TT memory BO, flags.defrag_ccs is true, which
forces copy_system_ccs to false:

    bool copy_system_ccs = copy_ccs && (!src_is_vram || !dst_is_vram) &&
        !flags.defrag_ccs && !flags.defrag_copy;

Because copy_system_ccs is false, the loop skips the initialization of ccs_ofs:

    if (copy_system_ccs) {
        ...
        batch_size += pte_update_size(..., &ccs_ofs, ...);
    }

However, needs_ccs_emit remains true (since flags.defrag_copy is false). This
causes xe_migrate_ccs_copy() to be called with the uninitialized ccs_ofs.

Furthermore, for TT memory on DGFX, both src_is_vram and dst_is_vram are false.
These are passed as the indirect flags to xe_migrate_ccs_copy():

    flush_flags = xe_migrate_ccs_copy(m, bb, src_L0_ofs,
                      IS_DGFX(xe) ? src_is_vram : src_is_pltt,
                      dst_L0_ofs,
                      IS_DGFX(xe) ? dst_is_vram : dst_is_pltt,
                      src_L0, ccs_ofs, copy_ccs);

Inside xe_migrate_ccs_copy(), this will trigger the assertion because both
indirect flags are false:

    xe_gt_assert(gt, src_is_indirect || dst_is_indirect);
    if (!src_is_indirect)
        src_ofs = ccs_ofs;

And then it programs the GPU copy engine with the uninitialized ccs_ofs,
which could lead to arbitrary GPU memory access or data corruption.

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