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

[Matthew Brost <matthew.brost@intel.com>]

On Fri, Sep 25, 2026 at 06:25:10AM +0000, sashiko-bot@kernel.org wrote:
> 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].
> 

Yes, we over allocate 4k. I'm ok with this.

> [ ... ]
> > @@ -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?
> 

!info.has_pt_mirror means VRAM access across tile is uniform perf or we'd mirror them.

Matt

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