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

["Ghimiray, Himal Prasad" <himal.prasad.ghimiray@intel.com>]

On 05-09-2026 03:10, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] Unnecessary allocation of page table roots and scratch structures for secondary tiles when PT mirroring is disabled.

Looks valid.

> --
> 
> commit 34b539e899a7643dc15dedad6a3cd229e0c0f4d5
> 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 toggle page-table mirroring across
> tiles for memory bandwidth considerations. It updates the VM creation,
> rebind, and page table update paths to check this flag and fall back to the
> primary tile's root page table when mirroring is disabled.
> 
> Link: https://patch.msgid.link/20260228013501.106680-18-matthew.brost@intel.com
> 
>> diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
>> --- a/drivers/gpu/drm/xe/xe_vm.c
>> +++ b/drivers/gpu/drm/xe/xe_vm.c
> 
> [ ... ]
> 
>> @@ -1738,7 +1748,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]
> Does this code unnecessarily allocate page table roots and scratch structures
> for secondary tiles when PT mirroring is disabled?
> 
> For standard user VMs, XE_VM_FLAG_MIGRATION is unset, meaning this if
> condition evaluates to false and we proceed to allocate pt_root[id] for all
> tiles.
> 
> However, when vm->xe->info.has_pt_mirror is false, xe_vm_pt_root()
> unconditionally returns pt_root[0]. This means the allocated pt_root[id]
> (and associated scratch page tables up to vm_max_level) for id > 0 will never
> be used, permanently wasting VRAM for the lifetime of every VM.
> 
> Should there be an explicit check to skip secondary tile allocations when
> has_pt_mirror is false, regardless of whether it is a migration VM?
>