Re: [PATCH] drm/xe/pm: do not warn about missing runtime PM protection after hot-unplug

On Thu, Aug 06, 2026 at 10:30:20AM +0200, Raag Jadav wrote:
> On Thu, Aug 06, 2026 at 02:27:12PM +0530, Nitin Gote wrote:
> > Exec queues are owned by user fds and are destroyed when the fd is closed.
> > After hot-unplug this can happen from a deferred close during process
> > exit, after the device has been removed and runtime PM has been disabled.
> > The queue destroy path can call xe_pm_runtime_get_noresume(), and since
> > runtime PM is disabled pm_runtime_get_if_in_use() returns no reference,
> > so it warns about "Missing outer runtime PM protection".
> > 
> > This is a false positive for the hot-unplug teardown case. The device is
> > already unplugged, and the queue destroy path that triggered this warning
> > was checked and does not touch hardware state after unplug. GuC has already
> > been sanitized by guc_fini_hw(), so no H2G is sent and the hardware
> > teardown path is no longer reachable.
> > 
> > Skip the warning when the DRM device is already unplugged.
> > 
> > Observed with new IGT core_hotunplug subtests:
> >   igt@core_hotunplug@hotreplug-with-load
> >   igt@core_hotunplug@hotunplug-rescan-with-load
> > 
> > v2:
> >  - Drop the drm_dev_is_unplugged() bypass from guc_exec_queue_destroy()
> >    and instead exclude hot-unplug from the WARN in
> >    xe_pm_runtime_get_noresume(). (Matthew Brost)
> > 

I don't think this was suggestion but also I believe I misunderstood the
problem, but maybe this works.

> > v3:
> >  - Clarify that the queue destroy path was checked and does not touch
> >    hardware state after unplug. (Matthew Auld)
> > 
> > Link: https://patchwork.freedesktop.org/patch/725773/?series=166744&rev=4
> > Cc: Matthew Brost <[email protected]>
> > Cc: Matthew Auld <[email protected]>
> > Assisted-by: GitHub-Copilot:claude-opus-4.8
> > Signed-off-by: Nitin Gote <[email protected]>
> > ---
> >  drivers/gpu/drm/xe/xe_pm.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c
> > index a5289a9df8d2..a038687d5b2c 100644
> > --- a/drivers/gpu/drm/xe/xe_pm.c
> > +++ b/drivers/gpu/drm/xe/xe_pm.c
> > @@ -10,6 +10,7 @@
> >  #include <linux/suspend.h>
> >  #include <linux/dmi.h>
> >  
> > +#include <drm/drm_drv.h>
> >  #include <drm/drm_managed.h>
> >  #include <drm/ttm/ttm_placement.h>
> >  
> > @@ -914,7 +915,9 @@ void xe_pm_runtime_get_noresume(struct xe_device *xe)
> 
> From the documentation
> 
>  * This function should be used in inner places where it is surely already
>  * protected by outer-bound callers of `xe_pm_runtime_get`.
>

We should have those.

> >  	if (!ref) {
> >  		pm_runtime_get_noresume(xe->drm.dev);
> > -		drm_WARN(&xe->drm, !xe_pm_suspending_or_resuming(xe),
> > +		drm_WARN(&xe->drm,
> > +			 !drm_dev_is_unplugged(&xe->drm) &&
> > +			 !xe_pm_suspending_or_resuming(xe),
> 
> So IMO this is more of a band-aid.
> 

So if I'm understanding the problem correctly,
pm_runtime_get_if_in_use() (via guc_exec_queue_add_msg()) fails when the
device is unplugged? I don't see how the PM reference count could be
zero, regardless of whether the device is plugged in or unplugged.

My suggestion was (and still is) that guc_exec_queue_destroy() should
always add a message and determine whether the device/firmware is still
alive while processing that message, then either kick the destory or
issue H2G. Other messages likely are also missing proper device alive
checks too.

If, for some reason, obtaining a PM reference via
xe_pm_runtime_get_noresume() doesn't work when the device is unplugged,
then that logic should be handled in the message submission layer. For
example, use drm_dev_enter()/drm_dev_exit() for hot-unplug protection,
only take a PM reference if the device is still bound, encode that
information into the message, and then determine the appropriate next
steps when the message is processed.

Sorry a lot of this stuff in xe_guc_submit.c around corner cases /
teardwons is a mess from early Xe work I did, we should aim to clean
this up with clear semantics.

Matt

> Raag
> 
> >  			 "Missing outer runtime PM protection\n");
> >  	}
> >  }
> > -- 
> > 2.50.1
> >