Intel-XE Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: "Ghimiray, Himal Prasad" <himal.prasad.ghimiray@intel.com>
To: Tejas Upadhyay <tejas.upadhyay@intel.com>,
	<intel-xe@lists.freedesktop.org>
Subject: Re: [PATCH V15 09/14] drm/xe/vram: Add VRAM page offline fault handler
Date: Thu, 13 Aug 2026 18:14:59 +0530	[thread overview]
Message-ID: <8b6a3d8e-1d0c-426a-841a-66961d855471@intel.com> (raw)
In-Reply-To: <20260811124016.3614699-25-tejas.upadhyay@intel.com>



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 <tejas.upadhyay@intel.com>
> ---
>   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 ?

> +	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 ?

> +	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.
> +		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.

> +
> +		/* 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.

> +	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
  > +	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 ?

> +	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)
>   {


  reply	other threads:[~2026-08-13 12:45 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-11 12:40 [PATCH V15 00/14] Add memory page offlining support Tejas Upadhyay
2026-08-11 12:40 ` [PATCH V15 01/14] drm/xe: Link VRAM object with gpu buddy Tejas Upadhyay
2026-08-11 12:40 ` [PATCH V15 02/14] [DO_NOT_MERGE]drm/gpu: Add gpu_buddy_allocated_addr_to_block helper Tejas Upadhyay
2026-08-11 12:40 ` [PATCH V15 03/14] drm/xe: Link LRC BO and its execution Queue Tejas Upadhyay
2026-08-11 12:40 ` [PATCH V15 04/14] drm/xe: Extend BO purge to handle vram pages as well Tejas Upadhyay
2026-08-11 12:40 ` [PATCH V15 05/14] drm/xe/bo: Make xe_bo_is_user() public Tejas Upadhyay
2026-08-11 15:38   ` Ghimiray, Himal Prasad
2026-08-11 12:40 ` [PATCH V15 06/14] drm/xe: Guard teardown paths against purged BOs Tejas Upadhyay
2026-08-12  3:24   ` Ghimiray, Himal Prasad
2026-08-12  9:40     ` Upadhyay, Tejas
2026-08-11 12:40 ` [PATCH V15 07/14] drm/xe/vram: Extract buddy alloc and free helpers Tejas Upadhyay
2026-08-12  3:25   ` Ghimiray, Himal Prasad
2026-08-11 12:40 ` [PATCH V15 08/14] drm/xe/vram: Add page offline data structures and lifecycle Tejas Upadhyay
2026-08-12 12:01   ` Ghimiray, Himal Prasad
2026-08-14  6:38     ` Upadhyay, Tejas
2026-08-11 12:40 ` [PATCH V15 09/14] drm/xe/vram: Add VRAM page offline fault handler Tejas Upadhyay
2026-08-13 12:44   ` Ghimiray, Himal Prasad [this message]
2026-08-14  5:19     ` Upadhyay, Tejas
2026-08-14 10:16       ` Upadhyay, Tejas
2026-08-11 12:40 ` [PATCH V15 10/14] drm/xe/configfs: Add vram bad page reservation policy Tejas Upadhyay
2026-08-12 12:21   ` Ghimiray, Himal Prasad
2026-08-11 12:40 ` [PATCH V15 11/14] drm/xe/vram: Use RCU for lock-free sysfs reads of bad page lists Tejas Upadhyay
2026-08-12 12:14   ` Ghimiray, Himal Prasad
2026-08-11 12:40 ` [PATCH V15 12/14] drm/xe: Add sysfs interface for bad gpu vram pages Tejas Upadhyay
2026-08-12 12:27   ` Ghimiray, Himal Prasad
2026-08-11 12:40 ` [PATCH V15 13/14] drm/xe/uapi: Expose ban reason in EXEC_QUEUE_GET_PROPERTY_BAN Tejas Upadhyay
2026-08-11 20:08   ` Rodrigo Vivi
2026-08-12 12:24   ` Ghimiray, Himal Prasad
2026-08-11 12:40 ` [PATCH V15 14/14] drm/xe: Add fault-inject based VRAM page offline injection Tejas Upadhyay
2026-08-16 14:35   ` Ghimiray, Himal Prasad

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8b6a3d8e-1d0c-426a-841a-66961d855471@intel.com \
    --to=himal.prasad.ghimiray@intel.com \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=tejas.upadhyay@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox