Re: [PATCH v8 17/24] drm/xe: Add device flag to enable PT mirroring across tiles

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] drm/xe: Silent drop of PT updates and zaps causing UAF when mirroring is disabled
- [Medium] drm/xe: Resource leak allocating unreachable PT roots when mirroring is disabled
--

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

drm/xe: Add device flag to enable PT mirroring across tiles

This commit introduces a device flag has_pt_mirror to enable or disable
page-table mirroring across tiles for multi-tile devices. It updates VM
creation, page table operations, and rebind logic to accommodate disabled
mirroring.

> diff --git a/drivers/gpu/drm/xe/xe_pt.c b/drivers/gpu/drm/xe/xe_pt.c
> index fbefcfc52dcac..dcef29b56d636 100644
> --- a/drivers/gpu/drm/xe/xe_pt.c
> +++ b/drivers/gpu/drm/xe/xe_pt.c
[ ... ]
> @@ -1046,6 +1051,9 @@ bool xe_pt_zap_ptes(struct xe_tile *tile, struct xe_vma *vma)
>  	if (!(pt_mask & BIT(tile->id)))
>  		return false;
>  
> +	if (pt_mirroring_disabled_for_tile(xe_vma_vm(vma), tile->id))
> +		return true;
> +
>  	(void)xe_pt_walk_shared(&pt->base, pt->level, xe_vma_start(vma),
>  				xe_vma_end(vma), &xe_walk.base);

[Severity: Critical]
If vma->tile_present excludes tile 0 (e.g., mask is 0x2), calls for tile 0
will return false early, and calls for tile 1 will return true here because
mirroring is disabled.

Could this prevent the PTEs from being zapped in the shared root, potentially
causing a use-after-free if pt_root[0] points to freed memory?

[ ... ]
> @@ -2539,9 +2550,21 @@ int xe_pt_update_ops_prepare(struct xe_device *xe, struct xe_vma_ops *vops)
>  	int id, err;
>  
>  	for_each_tile(tile, xe, id) {
> +		struct xe_vm_pgtable_update_ops *pt_update_ops =
> +			&vops->pt_update_ops[id];
> +
>  		if (!vops->pt_update_ops[id].num_ops)
>  			continue;
>  
> +		if (pt_mirroring_disabled_for_tile(vops->vm, id)) {
> +			struct xe_page_reclaim_list *prl = &pt_update_ops->prl;
> +
> +			/* Transfer root PT update ops PRL to current */
> +			*prl = vops->pt_update_ops[0].prl;
> +			xe_page_reclaim_entries_get(prl->entries);
> +			continue;
> +		}
> +

[Severity: Critical]
Similarly, if a PT update operation processes a VMA whose mask excludes tile 0,
tile 0 is skipped because its num_ops is 0, and tile 1 is skipped here
because mirroring is disabled.

This appears to happen in xe_vm_rebind() in drivers/gpu/drm/xe/xe_vm.c, which
was not updated to use adjust_rebind_tile_mask() and still passes
vma->tile_present directly:

drivers/gpu/drm/xe/xe_vm.c:xe_vm_rebind() {
    ...
		err = xe_vm_ops_add_rebind(&vops, vma,
					   vma->tile_present);
    ...
}

Could this cause PT updates and invalidations to be silently dropped entirely,
leaving stale page tables?

> diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
> index 0f542f47b9a84..f450e1c6f7500 100644
> --- a/drivers/gpu/drm/xe/xe_vm.c
> +++ b/drivers/gpu/drm/xe/xe_vm.c
[ ... ]
> @@ -1739,7 +1749,8 @@ struct xe_vm *xe_vm_create(struct xe_device *xe, u32 flags, struct xe_file *xef)
>  
>  		for_each_tile(tile, xe, id) {
>  			if (flags & XE_VM_FLAG_MIGRATION &&
> -			    tile->id != XE_VM_FLAG_TILE_ID(flags))
> +			    tile->id != XE_VM_FLAG_TILE_ID(flags) &&
> +			    (vm->xe->info.has_pt_mirror || id))
>  				continue;
>  
>  			vm->pt_root[id] = xe_pt_create(vm, tile, xe->info.vm_max_level,
>  						       &exec);

[Severity: Medium]
For normal (non-migration) VMs, this loop seems to allocate pt_root[id] and
associated scratch buffers for all tiles, even when PT mirroring is disabled.

Since xe_vm_pt_root(vm, id) will always return pt_root[0] when
!has_pt_mirror, does this leak VRAM and system memory by allocating orphaned
PT roots and scratch buffers for id > 0?

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