Re: [PATCH v4 23/25] drm/xe: Enter ULLS for migration jobs upon page fault or SVM prefetch

[Matthew Brost <matthew.brost@intel.com>]

On Fri, Sep 04, 2026 at 12:28:52AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Incorrect ordering of xe_pm_runtime_put() and xe_force_wake_put() leading to hardware MMIO access on a potentially suspended device.
> --
> 
> commit 43d7d496e886ff3c7ea66fac7a62b351eae1e2b2
> Author: Matthew Brost <matthew.brost@intel.com>
> 
> drm/xe: Enter ULLS for migration jobs upon page fault or SVM prefetch
> 
> This commit calls xe_migrate_ulls_enter upon page fault or SVM prefetch in an
> effort to speed up these critical paths. It also adjusts the teardown and exit
> sequences for ULLS migration jobs.
> 
> Link: https://patch.msgid.link/20260228013501.106680-25-matthew.brost@intel.com
> 
> > diff --git a/drivers/gpu/drm/xe/xe_migrate.c b/drivers/gpu/drm/xe/xe_migrate.c
> > index 1fa236eb1a260..80efa67fc2e15 100644
> > --- a/drivers/gpu/drm/xe/xe_migrate.c
> > +++ b/drivers/gpu/drm/xe/xe_migrate.c
> > @@ -114,8 +114,8 @@ static void xe_migrate_fini(void *arg)
> >  	disable_delayed_work_sync(&m->ulls.exit_work);
> >  	mutex_lock(&m->job_mutex);
> >  	if (m->ulls.enabled) {
> > -		xe_force_wake_put(gt_to_fw(m->q->hwe->gt), m->q->hwe->domain);
> >  		xe_pm_runtime_put(xe);
> > +		xe_force_wake_put(gt_to_fw(m->q->hwe->gt), m->q->hwe->domain);
> 
> [Severity: High]
> Does swapping the order here risk accessing hardware MMIO while the device
> is suspended?
> 
> Because xe_pm_runtime_put() drops the runtime PM usage counter, it could
> trigger a synchronous or asynchronous runtime suspend. If the device suspends
> before xe_force_wake_put() completes its MMIO writes to release the forcewake
> domain, it could result in PCI Unsupported Request (UR) errors or device hangs.
> 
> >  		m->ulls.enabled = false;
> >  	}
> >  	mutex_unlock(&m->job_mutex);
> > @@ -589,8 +589,8 @@ static void xe_migrate_ulls_exit(struct work_struct *work)
> >  		xe_sched_job_put(job);
> >  	if (!m->ulls.enabled) {
> >  		/* Pairs with PM gets on enter */
> > -		xe_force_wake_put(gt_to_fw(m->q->hwe->gt), m->q->hwe->domain);
> >  		xe_pm_runtime_put(xe);
> > +		xe_force_wake_put(gt_to_fw(m->q->hwe->gt), m->q->hwe->domain);
> 
> [Severity: High]
> Is this order change safe from a runtime PM perspective? 
> 
> Similar to the sequence in xe_migrate_fini(), dropping the PM reference in
> xe_migrate_ulls_exit() before releasing the forcewake domain might allow the
> device to suspend while xe_force_wake_put() is still accessing hardware MMIO
> registers.
> 

These are correct, will fix.

Matt

> >  
> >  		cancel_delayed_work(&m->ulls.exit_work);
> >  		xe_dbg(xe, "Migrate ULLS mode exit");
> 
> [ ... ]
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260903235842.3401722-1-matthew.brost@intel.com?part=23