Re: [PATCH V20 13/15] drm/xe: Expose bad VRAM pages via debugfs

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

On Wed, Sep 02, 2026 at 06:00:03PM +0000, Upadhyay, Tejas wrote:
> 
> 
> > -----Original Message-----
> > From: sashiko-bot@kernel.org <sashiko-bot@kernel.org>
> > Sent: 02 September 2026 22:27
> > To: Upadhyay, Tejas <tejas.upadhyay@intel.com>
> > Cc: intel-xe@lists.freedesktop.org
> > Subject: Re: [PATCH V20 13/15] drm/xe: Expose bad VRAM pages via debugfs
> > 
> > Thank you for your contribution! Sashiko AI review found 2 potential issue(s)
> > to consider:
> > - [Low] Debugfs output computes GPU VRAM page frame numbers and sizes
> > using host CPU macros (PAGE_SHIFT and PAGE_SIZE), causing silent truncation
> > and incorrect reporting on architectures with non-4K CPU pages.
> > - [Low] The VRAM max_pages limit is printed globally using only tile 0's
> > manager, ignoring limits of other tiles on multi-tile devices.
> > --
> > 
> > commit c12841d073a619e5edeb8c8306adf6fd03042edd
> > Author: Tejas Upadhyay <tejas.upadhyay@intel.com>
> > 
> > drm/xe: Expose bad VRAM pages via debugfs
> > 
> > This commit adds a debugfs file to show offlined and queued VRAM pages
> > across all tiles, displaying the page frame number, GPU page size, and status
> > flag.
> > 
> > > diff --git a/drivers/gpu/drm/xe/xe_ttm_vram_mgr.c
> > > b/drivers/gpu/drm/xe/xe_ttm_vram_mgr.c
> > > index 00569c5279a83..d9da2454d968e 100644
> > > --- a/drivers/gpu/drm/xe/xe_ttm_vram_mgr.c
> > > +++ b/drivers/gpu/drm/xe/xe_ttm_vram_mgr.c
> > 
> > [ ... ]
> > 
> > > @@ -917,3 +918,61 @@ int xe_ttm_vram_handle_addr_fault(struct
> > xe_device *xe, u64 addr)
> > >  	return xe_ttm_vram_reserve_page_at_addr(xe, addr - vr->dpa_base,
> > > vram_mgr, mm);  }  EXPORT_SYMBOL(xe_ttm_vram_handle_addr_fault);
> > > +
> > > +static int vram_bad_pages_show(struct seq_file *m, void *unused) {
> > > +	struct xe_device *xe = m->private;
> > > +	struct xe_ttm_vram_offline_resource *pos;
> > > +	struct ttm_resource_manager *man;
> > > +	struct xe_ttm_vram_mgr *mgr;
> > > +	struct xe_tile *tile;
> > > +	u8 id;
> > > +
> > > +	man = ttm_manager_type(&xe->ttm, XE_PL_VRAM0);
> > > +	if (man)
> > > +		/* TODO Hook with RAS to show max_pages fetched from FW
> > */
> > > +		seq_printf(m, "max_pages: %d\n",
> > > +			   to_xe_ttm_vram_mgr(man)->max_pages);
> > 
> > [Severity: Low]
> > Does this correctly represent the limit for multi-tile devices?
> > 
> > By querying only the manager for XE_PL_VRAM0, it seems the max_pages
> > value is printed as a global limit, which might ignore independent offline queue
> > limits configured on subsequent tiles.
> 
> This will come from RAS post integration with RAS and it will be same for all tile.
> 
> > 
> > > +
> > > +	for_each_tile(tile, xe, id) {
> > > +		struct xe_vram_region *vr = tile->mem.vram;
> > > +
> > > +		man = ttm_manager_type(&xe->ttm, XE_PL_VRAM0 + id);
> > > +		if (!man || !vr)
> > > +			continue;
> > > +		mgr = to_xe_ttm_vram_mgr(man);
> > > +
> > > +		rcu_read_lock();
> > > +
> > > +		list_for_each_entry_rcu(pos, &mgr->offlined_pages,
> > offlined_link) {
> > > +			u64 pfn;
> > > +
> > > +			pfn = (pos->addr + vr->dpa_base) >> PAGE_SHIFT;
> > > +			seq_printf(m, "0x%016llx : 0x%016lx : R\n", pfn,
> > PAGE_SIZE);
> > 
> > [Severity: Low]
> > Can this cause incorrect PFN calculation and page size reporting on
> > architectures with a non-4K CPU page size?
> > 
> > Using the host CPU macros PAGE_SHIFT and PAGE_SIZE to compute GPU
> > VRAM addresses couples the hardware memory topology to host CPU page
> > boundaries.
> > On a system with 64K CPU pages, shifting the 4K-aligned GPU address by 16
> > truncates the lower 4 bits of the GPU PFN, and reports 64K as the VRAM page
> > size instead of 4K.
> 
> We have 4K aligned pages everywhere so for CRI it covers alright.
> 

I think you use SZ_4K in a bunch of places in this series where you
actually want PAGE_SIZE.

xe_ttm_vram_mgr_init uses PAGE_SIZE no SZ_4K, in practice these are
value but using PAGE_SIZE is correct for future proofing or possibly
allowing non-x86 CPUs to plug into a dGPU.

Matt

> Tejas
> > 
> > > +		}
> > > +
> > > +		list_for_each_entry_rcu(pos, &mgr->queued_pages,
> > queued_link) {
> > > +			u64 pfn;
> > > +
> > > +			pfn = (pos->addr + vr->dpa_base) >> PAGE_SHIFT;
> > > +			seq_printf(m, "0x%016llx : 0x%016lx : %c\n",
> > > +				   pfn, PAGE_SIZE, pos->status ? 'F' : 'P');
> > 
> > [Severity: Low]
> > This has the same PAGE_SHIFT and PAGE_SIZE host CPU macro coupling as
> > above, which may truncate the PFN and report the wrong size on non-4K CPU
> > architectures.
> > 
> > > +		}
> > > +
> > > +		rcu_read_unlock();
> > > +	}
> > > +
> > > +	return 0;
> > > +}
> > 
> > [ ... ]
> > 
> > --
> > Sashiko AI review ·
> > https://sashiko.dev/#/patchset/20260902145343.465686-17-
> > tejas.upadhyay@intel.com?part=13