Re: [PATCH v2] drm/xe/guc: Check GuC running state before deregistering exec queue

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

On Sat, Oct 11, 2025 at 03:35:34PM -0600, Lin, Shuicheng wrote:
> On Sat, Oct 11, 2025 8:13 AM Matthew Brost wrote:
> > On Fri, Oct 10, 2025 at 05:25:29PM +0000, Shuicheng Lin wrote:
> > > In normal operation, a registered exec queue is disabled and
> > > deregistered through the GuC, and freed only after the GuC confirms
> > > completion. However, if the driver is forced to unbind while the exec
> > > queue is still running, the user may call exec_destroy() after the GuC
> > > has already been stopped and CT communication disabled.
> > >
> > > In this case, the driver cannot receive a response from the GuC,
> > > preventing proper cleanup of exec queue resources. Fix this by
> > > directly releasing the resources when GuC is not running.
> > >
> > > Here is the failure dmesg log:
> > > "
> > > [  468.089581] ---[ end trace 0000000000000000 ]--- [  468.089608] pci
> > > 0000:03:00.0: [drm] *ERROR* GT0: GUC ID manager unclean (1/65535)
> > > [  468.090558] pci 0000:03:00.0: [drm] GT0:     total 65535
> > > [  468.090562] pci 0000:03:00.0: [drm] GT0:     used 1
> > > [  468.090564] pci 0000:03:00.0: [drm] GT0:     range 1..1 (1)
> > > [  468.092716] ------------[ cut here ]------------ [  468.092719]
> > > WARNING: CPU: 14 PID: 4775 at drivers/gpu/drm/xe/xe_ttm_vram_mgr.c:298
> > > ttm_vram_mgr_fini+0xf8/0x130 [xe] "
> > 
> > Does public bug for this exist, if so we need a Close + link in the commit message.
> > 
> > Also I believe this warrents a fixes tag - I can add one when merging this for you.
> > 
> 
> No. It was found during internal validation. I will share the bug number with you offline.
> 
> For the fix tag, the logic is implemented in the initial version of xe, then the function is renamed later. 
> So this patch cannot be applied to the initial code directly and makes me not sure about the fix tag.
> I will leave it to you. Thanks in advance for it.

Just so you know - the flow is always apply a fixes tag even if it may
not cleanly backport. We hope the stable maintainers can figure it out,
if not it on us to provide patches to stable kernels which the
maintainers of kernels can the apply.

Matt 

> 
> Shuicheng
> 
> > I'll wait on answer to my first question before merging but this LGTM.
> > Reviewed-by: Matthew Brost <matthew.brost@intel.com>
> > 
> > >
> > > v2: use xe_uc_fw_is_running() instead of xe_guc_ct_enabled().
> > >     As CT may go down and come back during VF migration.
> > >
> > > Cc: Matthew Brost <matthew.brost@intel.com>
> > > Signed-off-by: Shuicheng Lin <shuicheng.lin@intel.com>
> > > ---
> > >  drivers/gpu/drm/xe/xe_guc_submit.c | 13 ++++++++++++-
> > >  1 file changed, 12 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c
> > > b/drivers/gpu/drm/xe/xe_guc_submit.c
> > > index e9aa0625ce60..0ef67d3523a7 100644
> > > --- a/drivers/gpu/drm/xe/xe_guc_submit.c
> > > +++ b/drivers/gpu/drm/xe/xe_guc_submit.c
> > > @@ -44,6 +44,7 @@
> > >  #include "xe_ring_ops_types.h"
> > >  #include "xe_sched_job.h"
> > >  #include "xe_trace.h"
> > > +#include "xe_uc_fw.h"
> > >  #include "xe_vm.h"
> > >
> > >  static struct xe_guc *
> > > @@ -1501,7 +1502,17 @@ static void
> > __guc_exec_queue_process_msg_cleanup(struct xe_sched_msg *msg)
> > >  	xe_gt_assert(guc_to_gt(guc), !(q->flags &
> > EXEC_QUEUE_FLAG_PERMANENT));
> > >  	trace_xe_exec_queue_cleanup_entity(q);
> > >
> > > -	if (exec_queue_registered(q))
> > > +	/*
> > > +	 * Expected state transitions for cleanup:
> > > +	 * - If the exec queue is registered and GuC firmware is running, we must
> > first
> > > +	 *   disable scheduling and deregister the queue to ensure proper
> > teardown and
> > > +	 *   resource release in the GuC, then destroy the exec queue on driver
> > side.
> > > +	 * - If the GuC is already stopped (e.g., during driver unload or GPU reset),
> > > +	 *   we cannot expect a response for the deregister request. In this case,
> > > +	 *   it is safe to directly destroy the exec queue on driver side, as the GuC
> > > +	 *   will not process further requests and all resources must be cleaned up
> > locally.
> > > +	 */
> > > +	if (exec_queue_registered(q) && xe_uc_fw_is_running(&guc->fw))
> > >  		disable_scheduling_deregister(guc, q);
> > >  	else
> > >  		__guc_exec_queue_destroy(guc, q);
> > > --
> > > 2.49.0
> > >