Re: [PATCH V15 09/14] drm/xe/vram: Add VRAM page offline fault handler
"Ghimiray, Himal Prasad" <[email protected]>
| Newsgroups | org.freedesktop.lists.intel-xe |
|---|---|
| Message-ID | <[email protected]> |
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 ? > + 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) > {