RE: [PATCH V15 09/14] drm/xe/vram: Add VRAM page offline fault handler

> -----Original Message-----
> From: Ghimiray, Himal Prasad <[email protected]>
> Sent: 13 August 2026 18:15
> To: Upadhyay, Tejas <[email protected]>; intel-
> [email protected]
> Subject: Re: [PATCH V15 09/14] drm/xe/vram: Add VRAM page offline fault
> handler
> 
> 
> 
> On 11-08-2026 18:10, Tejas Upadhyay wrote:
> > Add the core VRAM page offlining logic to handle HW-reported faulty
> > physical addresses:
> >
> > - xe_ttm_vram_purge_page(): Purges the BO containing the faulty
> >    address. Bans the associated VM (if page table BO) and exec queue
> >    (if LRC BO). Moves xe_exec_queue_kill() outside xe_bo_lock() to
> >    avoid AB-BA deadlock with vm->lock. Uses READ_ONCE(bo->q) to
> >    safely access the exec queue pointer.
> >
> > - xe_ttm_vram_page_already_processed(): Checks if an address is
> >    already tracked in offlined_pages or queued_pages lists to avoid
> >    double-processing.
> >
> > - xe_ttm_vram_reserve_page_at_addr(): Two-phase reservation that
> >    first queues the page, purges the BO outside the lock, then
> >    reserves the buddy block. Handles both allocated (BO present)
> >    and free page cases. Returns -EIO for critical kernel BOs to
> >    trigger system reset.
> >
> > - xe_ttm_vram_addr_to_region(): Maps a DPA to its VRAM region.
> >    Uses GSMBASE MMIO register to detect GSM addresses (returns NULL
> >    for reset path). Returns ERR_PTR(-ENOENT) for addresses outside
> >    any known region.
> >
> > - xe_ttm_vram_handle_addr_fault(): Entry point called by RAS.
> >    Returns -EEXIST if already processed, -EIO for GSM/critical BO,
> >    -EOPNOTSUPP if out of bounds.
> >
> > Signed-off-by: Tejas Upadhyay <[email protected]>
> > ---
> >   drivers/gpu/drm/xe/xe_ttm_vram_mgr.c | 297
> +++++++++++++++++++++++++++
> >   drivers/gpu/drm/xe/xe_ttm_vram_mgr.h |   1 +
> >   2 files changed, 298 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/xe/xe_ttm_vram_mgr.c
> > b/drivers/gpu/drm/xe/xe_ttm_vram_mgr.c
> > index 2813ae68325e..370bcf50c7f7 100644
> > --- a/drivers/gpu/drm/xe/xe_ttm_vram_mgr.c
> > +++ b/drivers/gpu/drm/xe/xe_ttm_vram_mgr.c
> > @@ -11,9 +11,14 @@
> >   #include <drm/ttm/ttm_placement.h>
> >   #include <drm/ttm/ttm_range_manager.h>
> >
> > +#include "regs/xe_regs.h"
> >   #include "xe_bo.h"
> >   #include "xe_device.h"
> > +#include "xe_exec_queue.h"
> > +#include "xe_lrc.h"
> > +#include "xe_mmio.h"
> >   #include "xe_res_cursor.h"
> > +#include "xe_ttm_stolen_mgr.h"
> >   #include "xe_ttm_vram_mgr.h"
> >   #include "xe_vram_types.h"
> >
> > @@ -518,3 +523,295 @@ u64 xe_ttm_vram_get_avail(struct
> > ttm_resource_manager *man)
> >
> >   	return avail;
> >   }
> > +
> > +static int xe_ttm_vram_purge_page(struct xe_device *xe, struct xe_bo
> > +*bo) {
> > +	struct ttm_operation_ctx ctx = {};
> > +	struct xe_exec_queue *q_to_put = NULL;
> > +	struct xe_exec_queue *q = NULL;
> > +	struct xe_vm *vm = NULL;
> > +	u32	flags;
> > +	int ret = 0;
> > +
> > +	xe_bo_lock(bo, false);
> > +	if (bo->vm)
> > +		vm = xe_vm_get(bo->vm);
> > +	flags = bo->flags;
> > +	xe_bo_unlock(bo);
> > +	/*  Ban VM if BO is PPGTT */
> > +	if (vm && (flags & XE_BO_FLAG_PAGETABLE)) {
> > +		down_write(&vm->lock);
> > +		xe_vm_kill(vm, true);
> > +		up_write(&vm->lock);
> > +	}
> > +	if (vm)
> > +		xe_vm_put(vm);
> > +
> > +	xe_bo_lock(bo, false);
> > +	q = READ_ONCE(bo->q);
> > +	/*  Ban exec queue if BO is lrc */
> > +	if (q && xe_exec_queue_get_unless_zero(q)) {
> > +		/* ban queue */
> > +		q_to_put = q;
> > +	}
> > +
> > +	if (bo->purgeable.state == XE_MADV_PURGEABLE_PURGED) {
> > +		/* Already purged by shrinker during unlocked window —
> nothing to do */
> > +		xe_bo_unlock(bo);
> > +		goto out;
> > +	}
> > +
> > +	xe_bo_set_purgeable_state(bo, XE_MADV_PURGEABLE_DONTNEED);
> > +	ttm_bo_unmap_virtual(&bo->ttm);   /* nuke CPU mmap + VRAM IO
> mappings */
> > +	if (xe_bo_is_pinned(bo))
> > +		xe_bo_unpin(bo);
> > +	ret = xe_ttm_bo_purge(&bo->ttm, &ctx);
> 
> shouldn't we pin it back incase purging fails ?

Yes, will do it.

> 
> > +	xe_bo_unlock(bo);
> > +
> > +out:
> > +	if (q_to_put) {
> > +		xe_exec_queue_kill(q_to_put);
> > +		xe_exec_queue_put(q_to_put);
> > +	}
> > +
> > +	return ret;
> > +}
> > +
> > +static bool xe_ttm_vram_page_already_processed(struct
> xe_ttm_vram_mgr *mgr,
> > +					       u64 addr)
> > +{
> > +	struct xe_ttm_vram_offline_resource *pos;
> > +
> > +	lockdep_assert_held(&mgr->lock);
> > +
> > +	list_for_each_entry(pos, &mgr->offlined_pages, offlined_link) {
> > +		if (pos->addr == addr)
> > +			return true;
> > +	}
> > +
> > +	list_for_each_entry(pos, &mgr->queued_pages, queued_link) {
> > +		if (pos->addr == addr)
> > +			return true;
> > +	}
> > +
> > +	return false;
> > +}
> > +
> > +static int xe_ttm_vram_reserve_page_at_addr(struct xe_device *xe, u64
> addr,
> > +					    struct xe_ttm_vram_mgr
> *vram_mgr, struct gpu_buddy *mm) {
> > +	struct xe_ttm_vram_offline_resource *nentry;
> > +	struct ttm_buffer_object *tbo = NULL;
> > +	struct xe_bo *pbo_to_put = NULL;
> > +	struct gpu_buddy_block *block;
> 
> nentry->status is bool why assign enum for it ?

True, will change it.

> 
> > +	enum reserve_status {
> > +		pending = 0,
> > +		fail
> > +	};
> > +	u64 size = SZ_4K;
> > +	int ret = 0;
> > +
> > +	scoped_guard(mutex, &vram_mgr->lock) {
> > +		if (xe_ttm_vram_page_already_processed(vram_mgr, addr))
> > +			return -EEXIST;
> > +		block = gpu_buddy_allocated_addr_to_block(mm, addr);
> > +		if (WARN_ON(IS_ERR(block)))
> > +			return PTR_ERR(block);
> > +
> > +		nentry = kzalloc_obj(*nentry);
> > +		if (!nentry)
> > +			return -ENOMEM;
> > +		INIT_LIST_HEAD(&nentry->blocks);
> > +		nentry->status = pending;
> > +		nentry->addr = addr;
> > +
> > +		if (block) {
> > +			struct xe_bo *pbo;
> > +
> > +			if (!block->private) {
> > +				/* Race: another thread just reserved this
> block */
> > +				kfree(nentry);
> > +				return -EEXIST;
> > +			}
> > +			tbo = block->private;
> > +			pbo = ttm_to_xe_bo(tbo);
> > +
> > +			/* Get reference safely - BO may have zero refcount */
> > +			if (!xe_bo_get_unless_zero(pbo)) {
> > +				kfree(nentry);
> > +				return -ENOENT;
> > +			}
> > +			/*
> > +			 * Critical kernel BO? Best-effort check without resv
> lock;
> > +			 * worst case a concurrent pin causes reset path
> unnecessarily.
> > +			 */
> > +			if ((pbo->ttm.type == ttm_bo_type_kernel &&
> > +			     !(pbo->flags &
> XE_BO_FLAG_PINNED_LATE_RESTORE)) ||
> > +			    (xe_bo_is_user(pbo) && xe_bo_is_pinned(pbo))) {
> > +				kfree(nentry);
> > +				pbo_to_put = pbo;
> > +				drm_err(&xe->drm,
> > +					"%s: addr: 0x%llx is critical kernel bo,
> requesting SBR\n",
> > +					__func__, addr);
> > +				break;
> > +			}
> > +			++vram_mgr->n_queued_pages;
> > +			list_add(&nentry->queued_link, &vram_mgr-
> >queued_pages);
> > +		}
> > +	}
> > +
> > +	/* Deferred put outside lock to avoid recursive deadlock */
> > +	if (pbo_to_put) {
> > +		xe_bo_put(pbo_to_put);
> > +		/* Hint System controller driver for reset with -EIO  */
> > +		return -EIO;
> > +	}
> > +
> > +	if (block) {
> > +		struct xe_ttm_vram_offline_resource *pos, *n;
> > +		struct xe_bo *pbo = ttm_to_xe_bo(tbo);
> > +
> > +		/*
> > +		 * Purge BO containing address - reference held from above.
> > +		 * Note: brief window between purge (freeing blocks) and re-
> reserve
> > +		 * below. If another allocation claims the block, buddy_alloc
> fails
> > +		 * and the next HW fault at this address will retry.
> > +		 */
> 
> I dont think we retry here. Fix comment.

Ya its typo. Will change it.

> > +		ret = xe_ttm_vram_purge_page(xe, pbo);
> > +		xe_bo_put(pbo);
> > +		if (ret) {
> > +			nentry->status = fail;
> > +			return ret;
> > +		}
> 
> Nit: I believe better to try xe_ttm_vram_buddy_alloc here. even if purge fails.

It looks like next allocation will anyhow fail and we will end up with same thing returning. But as discussed there could be chance of allocating incase someone else freed by then. Will add drm_warn when purge fails though.

> 
> > +
> > +		/* Reserve page at address addr*/
> > +		scoped_guard(mutex, &vram_mgr->lock) {
> > +			ret = xe_ttm_vram_buddy_alloc(vram_mgr, addr, addr
> + size,
> > +						      size, size, &nentry->blocks,
> > +
> GPU_BUDDY_RANGE_ALLOCATION,
> > +						      NULL, &nentry-
> >used_visible_size);
> > +			if (ret) {
> > +				drm_warn(&xe->drm,
> > +					 "Could not reserve page at
> addr:0x%llx, ret:%d\n",
> > +					 addr, ret);
> > +				nentry->status = fail;
> > +				return ret;
> > +			}
> > +
> > +			list_for_each_entry_safe(pos, n, &vram_mgr-
> >queued_pages, queued_link) {
> > +				if (pos->addr == nentry->addr) {
> > +					--vram_mgr->n_queued_pages;
> > +					list_del(&pos->queued_link);
> > +					break;
> > +				}
> > +			}
> > +			list_add(&nentry->offlined_link, &vram_mgr-
> >offlined_pages);
> > +			/* RAS will send command to FW for offlining page
> based on ret value */
> > +			++vram_mgr->n_offlined_pages;
> > +			return ret;
> > +		}
> > +	} else {
> > +		struct xe_ttm_vram_offline_resource *pos, *n;
> > +
> > +		scoped_guard(mutex, &vram_mgr->lock) {
> > +			++vram_mgr->n_queued_pages;
> > +			list_add(&nentry->queued_link, &vram_mgr-
> >queued_pages);
> > +			ret = xe_ttm_vram_buddy_alloc(vram_mgr, addr, addr
> + size,
> > +						      size, size, &nentry->blocks,
> > +
> GPU_BUDDY_RANGE_ALLOCATION,
> > +						      NULL, &nentry-
> >used_visible_size);
> > +			if (ret) {
> > +				drm_warn(&xe->drm,
> > +					 "Could not reserve page at
> addr:0x%llx, ret:%d\n",
> > +					 addr, ret);
> > +				nentry->status = fail;
> > +				return ret;
> > +			}
> > +
> > +			list_for_each_entry_safe(pos, n, &vram_mgr-
> >queued_pages, queued_link) {
> > +				if (pos->addr == nentry->addr) {
> > +					--vram_mgr->n_queued_pages;
> > +					list_del(&pos->queued_link);
> > +					break;
> > +				}
> > +			}
> > +			++vram_mgr->n_offlined_pages;
> > +			list_add(&nentry->offlined_link, &vram_mgr-
> >offlined_pages);
> > +			/* RAS will send command to FW for offlining page
> based on ret value */
> > +		}
> > +	}
> > +	/* Success */
> > +	return ret;
> > +}
> > +
> > +static struct xe_vram_region *xe_ttm_vram_addr_to_region(struct
> > +xe_device *xe, u64 addr) {
> > +	u64 raw_offset =
> xe_mmio_read64_2x32(&xe_device_get_root_tile(xe)->mmio, GSMBASE);
> > +	/* force a 4K (4096 bytes) page alignment */
> > +	u64 gsmbase_dpa = raw_offset & ~(u64)(PAGE_SIZE - 1);
> > +	struct xe_vram_region *vr;
> > +	struct xe_tile *tile;
> > +	int id;
> > +
> > +	/* Addr from GSM? */
> > +	if (addr >= gsmbase_dpa)
> > +		/* Return NULL so the caller can request reset (SBR) */
> > +		return NULL;
> > +
> > +	for_each_tile(tile, xe, id) {
> > +		vr = tile->mem.vram;
> > +		if (addr >= vr->dpa_base &&
> > +		    addr < vr->dpa_base + vr->usable_size)
> > +			return vr;
> > +	}
> > +
> > +	/*
> > +	 * Return an explicit error pointer so the caller knows the addr
> > +	 * is invalid and should be ignored, NOT SBR.
> > +	 */
> 
> we are changing err to -EOPNOTSUPP at caller, why not pass same from here.

Ok. Will do it.

> 
> > +	return ERR_PTR(-ENOENT);
> > +}
> > +
> > +/**
> > + * xe_ttm_vram_handle_addr_fault - Handle vram physical address error
> > +flaged
> > + * @xe: pointer to parent device
> > + * @addr: physical faulty address
> > + *
> > + * Handle the physcial faulty address error on specific tile.
> > + *
> > + * Returns 0 for success, negative error code otherwise as follow:
> > + * * %-EIO - critical BO or address outside any VRAM region; next action is
> reset.
> > + * * %-EOPNOTSUPP - log-only policy; no further action.
> > + * * %-ENOMEM - allocation failure; next action is reset.
> > + * * %-ENXIO - address not found in buddy; next action is reset.
> > + * * %-EEXIST - address already processed; no further action.
> > + * * % Any other negative error - next action is reset.
> > + */
> > +int xe_ttm_vram_handle_addr_fault(struct xe_device *xe, u64 addr) {
> 
> assert addr is page aligned

RAS already has that assertion, I will double check, if not will add  here.

>   > +	struct xe_ttm_vram_mgr *vram_mgr;
> > +	struct xe_vram_region *vr;
> > +	struct gpu_buddy *mm;
> > +
> > +	vr = xe_ttm_vram_addr_to_region(xe, addr);
> > +	if (IS_ERR(vr)) {
> > +		/*
> > +		 * The addr is outside VRAM and GSM.
> > +		 * Log a debug message if needed, and safely exit/ignore.
> > +		 */
> > +		drm_dbg(&xe->drm, "Address %llx is out of bounds, ignoring
> fault.\n", addr);
> > +		return -EOPNOTSUPP;
> > +	}
> > +	if (!vr) {
> > +		drm_err(&xe->drm, "%s:%d GSM addr:%llx error requesting
> SBR\n",
> > +			__func__, __LINE__, addr);
> > +		/* Hint System controller driver for reset with -EIO  */
> > +		return -EIO;
> > +	}
> > +	vram_mgr = &vr->ttm;
> > +	mm = &vram_mgr->mm;
> > +
> > +	/* Reserve page at address */
> 
> s/addr/addr - vr->dpa_base ?

This is thought/running for CRI only, where  we have single tile and it perfectly works there. I will add this anyhow it guards multitile case as well for future cases.

Tejas
> 
> > +	return xe_ttm_vram_reserve_page_at_addr(xe, addr, vram_mgr, mm);
> }
> > +EXPORT_SYMBOL(xe_ttm_vram_handle_addr_fault);
> > diff --git a/drivers/gpu/drm/xe/xe_ttm_vram_mgr.h
> > b/drivers/gpu/drm/xe/xe_ttm_vram_mgr.h
> > index 87b7fae5edba..d5392beff30c 100644
> > --- a/drivers/gpu/drm/xe/xe_ttm_vram_mgr.h
> > +++ b/drivers/gpu/drm/xe/xe_ttm_vram_mgr.h
> > @@ -31,6 +31,7 @@ u64 xe_ttm_vram_get_cpu_visible_size(struct
> ttm_resource_manager *man);
> >   void xe_ttm_vram_get_used(struct ttm_resource_manager *man,
> >   			  u64 *used, u64 *used_visible);
> >
> > +int xe_ttm_vram_handle_addr_fault(struct xe_device *xe, u64 addr);
> >   static inline struct xe_ttm_vram_mgr_resource *
> >   to_xe_ttm_vram_mgr_resource(struct ttm_resource *res)
> >   {