On Thu, 2025-09-04 at 16:22 +0100, Matthew Auld wrote:

On 04/09/2025 14:06, Thomas Hellström wrote:

...

When the xe pm_notifier evicts for suspend / hibernate, there might be racing tasks trying to re-validate again. This can lead to suspend taking excessive time or get stuck in a live-lock. This behaviour becomes much worse with the fix that actually makes re-validation bring back bos to VRAM rather than letting them remain in TT.

Prevent that by having exec and the rebind worker waiting for a completion that is set to block by the pm_notifier before suspend and is signaled by the pm_notifier after resume / wakeup.

It's probably still possible to craft malicious applications that block suspending. More work is pending to fix that.

v3:

• Avoid wait_for_completion() in the kernel worker since it could

potentially cause work item flushes from freezable processes to    wait forever. Instead terminate the rebind workers if needed and    re-launch at resume. (Matt Auld)

Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/4288 Fixes: c6a4d46ec1d7 ("drm/xe: evict user memory in PM notifier") Cc: Matthew Auld matthew.auld@intel.com Cc: Rodrigo Vivi rodrigo.vivi@intel.com Cc: stable@vger.kernel.org # v6.16+ Signed-off-by: Thomas Hellström thomas.hellstrom@linux.intel.com

---

drivers/gpu/drm/xe/xe_device_types.h |  6 ++++   drivers/gpu/drm/xe/xe_exec.c         |  9 ++++++   drivers/gpu/drm/xe/xe_pm.c           | 20 ++++++++++++   drivers/gpu/drm/xe/xe_vm.c           | 46 +++++++++++++++++++++++++++-   drivers/gpu/drm/xe/xe_vm.h           |  2 ++   drivers/gpu/drm/xe/xe_vm_types.h     |  5 +++   6 files changed, 87 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h index 092004d14db2..1e780f8a2a8c 100644 --- a/drivers/gpu/drm/xe/xe_device_types.h +++ b/drivers/gpu/drm/xe/xe_device_types.h @@ -507,6 +507,12 @@ struct xe_device {      /** @pm_notifier: Our PM notifier to perform actions in response to various PM events. */    struct notifier_block pm_notifier;

• /** @pm_block: Completion to block validating tasks on

suspend / hibernate prepare */

• struct completion pm_block;
• /** @rebind_resume_list: List of wq items to kick on

resume. */

• struct list_head rebind_resume_list;
• /** @rebind_resume_lock: Lock to protect the

rebind_resume_list */

• struct mutex rebind_resume_lock;

/** @pmt: Support the PMT driver callback interface */    struct { diff --git a/drivers/gpu/drm/xe/xe_exec.c b/drivers/gpu/drm/xe/xe_exec.c index 44364c042ad7..374c831e691b 100644 --- a/drivers/gpu/drm/xe/xe_exec.c +++ b/drivers/gpu/drm/xe/xe_exec.c @@ -237,6 +237,15 @@ int xe_exec_ioctl(struct drm_device *dev, void *data, struct drm_file *file)    goto err_unlock_list;    }

• /*

• * It's OK to block interruptible here with the vm lock
  

held, since

• * on task freezing during suspend / hibernate, the call
  

will

• * return -ERESTARTSYS and the IOCTL will be rerun.
  

• */
  

• err = wait_for_completion_interruptible(&xe->pm_block);
• if (err)

• goto err_unlock_list;
  

vm_exec.vm = &vm->gpuvm;    vm_exec.flags = DRM_EXEC_INTERRUPTIBLE_WAIT;    if (xe_vm_in_lr_mode(vm)) { diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c index bee9aacd82e7..6d59990ff6ba 100644 --- a/drivers/gpu/drm/xe/xe_pm.c +++ b/drivers/gpu/drm/xe/xe_pm.c @@ -297,6 +297,18 @@ static u32 vram_threshold_value(struct xe_device *xe)    return DEFAULT_VRAM_THRESHOLD;   }   +static void xe_pm_wake_preempt_workers(struct xe_device *xe) +{

• struct list_head *link, *next;
• mutex_lock(&xe->rebind_resume_lock);
• list_for_each_safe(link, next, &xe->rebind_resume_list) {

• list_del_init(link);
  

• xe_vm_resume_preempt_worker(link);
  

• }
• mutex_unlock(&xe->rebind_resume_lock);

+}

static int xe_pm_notifier_callback(struct notifier_block *nb,       unsigned long action, void *data)   { @@ -306,6 +318,7 @@ static int xe_pm_notifier_callback(struct notifier_block *nb,    switch (action) {    case PM_HIBERNATION_PREPARE:    case PM_SUSPEND_PREPARE:

• reinit_completion(&xe->pm_block);
  

xe_pm_runtime_get(xe);    err = xe_bo_evict_all_user(xe);    if (err) @@ -322,6 +335,8 @@ static int xe_pm_notifier_callback(struct notifier_block *nb,    break;    case PM_POST_HIBERNATION:    case PM_POST_SUSPEND:

• complete_all(&xe->pm_block);
  

• xe_pm_wake_preempt_workers(xe);
  

xe_bo_notifier_unprepare_all_pinned(xe);    xe_pm_runtime_put(xe);    break; @@ -348,6 +363,11 @@ int xe_pm_init(struct xe_device *xe)    if (err)    return err;

• init_completion(&xe->pm_block);
• complete_all(&xe->pm_block);
• mutex_init(&xe->rebind_resume_lock);

err = drmm_mutex_init(&xe->rebind_resume_lock);

Sure.

?

...

• INIT_LIST_HEAD(&xe->rebind_resume_list);

/* For now suspend/resume is only allowed with GuC */    if (!xe_device_uc_enabled(xe))    return 0; diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c index f55f96bb240a..97aad1d53a8c 100644 --- a/drivers/gpu/drm/xe/xe_vm.c +++ b/drivers/gpu/drm/xe/xe_vm.c @@ -394,6 +394,9 @@ static int xe_gpuvm_validate(struct drm_gpuvm_bo *vm_bo, struct drm_exec *exec)    list_move_tail(&gpuva_to_vma(gpuva)-

...

combined_links.rebind,

&vm->rebind_list);

• if (!try_wait_for_completion(&vm->xe->pm_block))

• return -EAGAIN;
  

ret = xe_bo_validate(gem_to_xe_bo(vm_bo->obj), vm, false);    if (ret)    return ret; @@ -480,6 +483,37 @@ static int xe_preempt_work_begin(struct drm_exec *exec, struct xe_vm *vm,    return xe_vm_validate_rebind(vm, exec, vm-

...

preempt.num_exec_queues);

}   +static bool vm_suspend_preempt_worker(struct xe_vm *vm) +{

• struct xe_device *xe = vm->xe;
• bool ret = false;
• mutex_lock(&xe->rebind_resume_lock);
• if (!try_wait_for_completion(&vm->xe->pm_block)) {

• ret = true;
  

• list_move_tail(&vm->preempt.pm_activate_link, &xe-
  

...

rebind_resume_list);

• }
• pr_info("Suspending %p\n", vm);
• mutex_unlock(&xe->rebind_resume_lock);
• return ret;

+}

+/**

• • xe_vm_resume_preempt_worker() - Resume the preempt worker.
• • @vm: The vm whose preempt worker to resume.
• • Resume a preempt worker that was previously suspended by
• • vm_suspend_preempt_worker().
• */

+void xe_vm_resume_preempt_worker(struct list_head *link)

I guess should use vm arg here?

Yes I tried to avoid downcasting from within xe_pm.c but since we need to include xe_vm.h anyway, I might as well do that.

Thanks for reviewing!

/Thomas

...

+{

• struct xe_vm *vm = container_of(link, typeof(*vm),

preempt.pm_activate_link);

• pr_info("Resuming %p\n", vm);
• queue_work(vm->xe->ordered_wq, &vm->preempt.rebind_work);

+}

static void preempt_rebind_work_func(struct work_struct *w)   {    struct xe_vm *vm = container_of(w, struct xe_vm, preempt.rebind_work); @@ -503,6 +537,11 @@ static void preempt_rebind_work_func(struct work_struct *w)    }     retry:

• if (!try_wait_for_completion(&vm->xe->)) &&

vm_suspend_preempt_worker(vm)) {

• up_write(&vm->lock);
  

• return;
  

• }

if (xe_vm_userptr_check_repin(vm)) {    err = xe_vm_userptr_pin(vm);    if (err) @@ -1741,6 +1780,7 @@ struct xe_vm *xe_vm_create(struct xe_device *xe, u32 flags, struct xe_file *xef)    if (flags & XE_VM_FLAG_LR_MODE) {    INIT_WORK(&vm->preempt.rebind_work, preempt_rebind_work_func);    xe_pm_runtime_get_noresume(xe);

• INIT_LIST_HEAD(&vm->preempt.pm_activate_link);
  

}      if (flags & XE_VM_FLAG_FAULT_MODE) { @@ -1922,8 +1962,12 @@ void xe_vm_close_and_put(struct xe_vm *vm)    xe_assert(xe, !vm->preempt.num_exec_queues);      xe_vm_close(vm);

• if (xe_vm_in_preempt_fence_mode(vm))

• if (xe_vm_in_preempt_fence_mode(vm)) {

• mutex_lock(&xe->rebind_resume_lock);
  

• list_del_init(&vm->preempt.pm_activate_link);
  

• mutex_unlock(&xe->rebind_resume_lock);
  

flush_work(&vm->preempt.rebind_work);

• }

if (xe_vm_in_fault_mode(vm))    xe_svm_close(vm);   diff --git a/drivers/gpu/drm/xe/xe_vm.h b/drivers/gpu/drm/xe/xe_vm.h index b3e5bec0fa58..f2639794278b 100644 --- a/drivers/gpu/drm/xe/xe_vm.h +++ b/drivers/gpu/drm/xe/xe_vm.h @@ -281,6 +281,8 @@ struct dma_fence *xe_vm_bind_kernel_bo(struct xe_vm *vm, struct xe_bo *bo,           struct xe_exec_queue *q, u64 addr,           enum xe_cache_level cache_lvl);   +void xe_vm_resume_preempt_worker(struct list_head *link);

/**    * xe_vm_resv() - Return's the vm's reservation object    * @vm: The vm diff --git a/drivers/gpu/drm/xe/xe_vm_types.h b/drivers/gpu/drm/xe/xe_vm_types.h index b5108d010786..e1a786db5f89 100644 --- a/drivers/gpu/drm/xe/xe_vm_types.h +++ b/drivers/gpu/drm/xe/xe_vm_types.h @@ -338,6 +338,11 @@ struct xe_vm {    * BOs    */    struct work_struct rebind_work;

• /**
  

•  * @preempt.pm_activate_link: Link to list of
  

rebind workers to be

•  * kicked on resume.
  

•  */
  

• struct list_head pm_activate_link;
  

} preempt;      /** @um: unified memory state */