Re: [PATCH v7 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 1 potential issue(s) to consider:
- [Medium] xe: wasted VRAM and forced cross-tile PT access for migration VMs when PT mirroring is disabled
--

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

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

This commit introduces a device flag to enable or disable page-table
mirroring across tiles. It defaults to enabled for PVC, aiming to allow
flexibility based on memory bandwidth needs.

> 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,

[Severity: Medium]
When has_pt_mirror is false, does this logic in xe_vm_create() allocate
page tables that will never be used?

For normal VMs (where flags & XE_VM_FLAG_MIGRATION is false), the condition
fails and we allocate pt_root[id] for all tiles. However, xe_vm_pt_root()
will unconditionally return pt_root[0] when has_pt_mirror is false, leaving
pt_root[1..N] unused and wasting VRAM.

Additionally, for a migration VM targeting a secondary tile (e.g., tile 1),
the condition tile->id != 1 && (has_pt_mirror || 0) evaluates to false when
id == 0, causing it to allocate both pt_root[0] and pt_root[1].

[ ... ]
> @@ -5074,3 +5085,21 @@ void xe_vm_remove_exec_queue(struct xe_vm *vm, struct xe_exec_queue *q)
>  	}
>  	up_write(&vm->exec_queues.lock);
>  }
> +
> +/**
> + * xe_vm_pt_root() - Retrieve VM page-table root
> + * @vm: The VM.
> + * @tile_id: Tile ID
> + *
> + * Retrieve VM page-table root for a tile ID, used to abstract if PT mirroring is
> + * enabled across tiles.
> + *
> + * Return: VM page-table root for a tile ID
> + */
> +struct xe_pt *xe_vm_pt_root(struct xe_vm *vm, u8 tile_id)
> +{
> +	if (vm->xe->info.has_pt_mirror)
> +		return vm->pt_root[tile_id];
> +
> +	return vm->pt_root[0];
> +}

[Severity: Medium]
Does returning vm->pt_root[0] here in xe_vm_pt_root() when has_pt_mirror
is false force cross-tile VRAM access for migration VMs on secondary tiles?

As noted above, a migration VM targeting a secondary tile will have its page
tables allocated in pt_root[1]. But since xe_vm_pt_root(vm, 1) will return
pt_root[0], the migration engine on tile 1 will be forced to walk page
tables physically located in tile 0's VRAM.

Can this cause a performance regression for migration operations on secondary
tiles?

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