Re: [CI 2/2] drm/xe/pm: skip BO eviction on iGPU for non-hibernation sleep

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/xe/pm: unblocked LR VM rebind worker during S2idle/S3 in xe_pm_notifier_callback()
--

commit 02558a47cf40f27ef297c0a97e9f30441e04b641
Author: Matthew Auld <matthew.auld@intel.com>

drm/xe/pm: skip BO eviction on iGPU for non-hibernation sleep

This commit distinguishes S4 hibernation from S2idle and S3 suspend and
skips BO eviction and pinned BO preparation on iGPU platforms when not
creating a hibernation image.

> diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c
> index f483b565d323f..a24424b0d1dd2 100644
> --- a/drivers/gpu/drm/xe/xe_pm.c
> +++ b/drivers/gpu/drm/xe/xe_pm.c
[ ... ]
> @@ -452,8 +460,13 @@ static int xe_pm_notifier_callback(struct notifier_block *nb,
>  				   unsigned long action, void *data)
>  {
>  	struct xe_device *xe = container_of(nb, struct xe_device, pm_notifier);
> +	bool hibernate = (action == PM_HIBERNATION_PREPARE ||
> +			  action == PM_POST_HIBERNATION);
>  	int err = 0;
>  
> +	if (!xe_pm_needs_bo_evict(xe, hibernate))
> +		return NOTIFY_DONE;

[Severity: High]
Does returning early from xe_pm_notifier_callback() on iGPU during
PM_SUSPEND_PREPARE and PM_POST_SUSPEND leave long-running VM rebind workers
unblocked across S2idle and S3 suspend?

By returning before the switch statement when xe_pm_needs_bo_evict() is
false, xe_pm_notifier_callback() skips reinit_completion(&xe->pm_block) on
PM_SUSPEND_PREPARE as well as complete_all(&xe->pm_block) and
xe_pm_wake_rebind_workers(xe) on PM_POST_SUSPEND.

Long-running VMs in preempt-fence mode run preempt_fence_work_func() on
xe->preempt_fence_wq and preempt_rebind_work_func() on xe->ordered_wq,
neither of which is allocated with WQ_FREEZABLE. Instead,
preempt_rebind_work_func() relies on xe->pm_block to park itself onto
xe->rebind_resume_list during system sleep:

drivers/gpu/drm/xe/xe_vm.c:preempt_rebind_work_func() {
	...
	if (!try_wait_for_completion(&vm->xe->pm_block) && vm_suspend_rebind_worker(vm)) {
		up_write(&vm->lock);
		/* We don't actually block but don't make progress. */
		xe_pm_might_block_on_suspend();
		return;
	}
	...
}

When a preempt fence signals during suspend preparation or while
xe_gt_suspend() -> xe_uc_suspend() stops GuC submission:

preempt_fence_work_func()
  -> q->ops->suspend_wait(q)
  -> xe_vm_queue_rebind_worker(q->vm)

guc_exec_queue_wait_suspend_done() can also return early once
xe_guc_read_stopped(guc) becomes true while ge->suspend_pending is still set.

Because xe->pm_block remains completed, try_wait_for_completion() in
preempt_rebind_work_func() and xe_gpuvm_validate() returns true instead of
parking the worker. Can preempt_rebind_work_func() then run concurrently with
or after xe_gt_suspend(), pushing TLB invalidations while the GT is
suspended and calling resume_and_reinstall_preempt_fences() ->
q->ops->resume(q) -> __guc_exec_queue_resume(), hitting:

	xe_gt_assert(guc_to_gt(guc), !ge->suspend_pending ||
		     exec_queue_killed_or_banned_or_wedged(q));

Would it make sense to still manage xe->pm_block and
xe_pm_wake_rebind_workers(xe) in xe_pm_notifier_callback() even when BO
eviction is skipped?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261105143547.1089207-4-matthew.auld@intel.com?part=1