Re: [PATCH v2 2/3] drm/xe: Forcefully tear down exec queues in GuC submit fini

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

On Thu, Dec 18, 2025 at 04:36:32PM -0700, Summers, Stuart wrote:
> On Thu, 2025-12-18 at 13:44 -0800, Matthew Brost wrote:
> > In GuC submit fini, forcefully tear down any exec queues by disabling
> > CTs, stopping the scheduler (which cleans up lost G2H), killing all
> > remaining queues, and resuming scheduling to allow any remaining
> > cleanup
> > actions to complete and signal any remaining fences.
> > 
> > v2:
> >  - Fix VF failure (CI)
> > 
> > Fixes: dd08ebf6c352 ("drm/xe: Introduce a new DRM driver for Intel
> > GPUs")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Zhanjun Dong <zhanjun.dong@intel.com>
> > Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> > 
> > ---
> > 
> > This fix will not apply outright to any stable kernel as it depeneds
> > on
> > functions which have added in the KMD since the original commit.
> > Likely
> > will have to manually send out patches to stable for kernel which
> > we'd
> > like to fix.
> > ---
> >  drivers/gpu/drm/xe/xe_guc_submit.c | 27 ++++++++++++++++++++-------
> >  1 file changed, 20 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c
> > b/drivers/gpu/drm/xe/xe_guc_submit.c
> > index 071cbfec2401..58ec94439df1 100644
> > --- a/drivers/gpu/drm/xe/xe_guc_submit.c
> > +++ b/drivers/gpu/drm/xe/xe_guc_submit.c
> > @@ -289,6 +289,8 @@ static bool
> > exec_queue_killed_or_banned_or_wedged(struct xe_exec_queue *q)
> >                  EXEC_QUEUE_STATE_BANNED));
> >  }
> >  
> > +static int __xe_guc_submit_reset_prepare(struct xe_guc *guc);
> > +
> >  static void guc_submit_fini(struct drm_device *drm, void *arg)
> >  {
> >         struct xe_guc *guc = arg;
> > @@ -296,6 +298,12 @@ static void guc_submit_fini(struct drm_device
> > *drm, void *arg)
> >         struct xe_gt *gt = guc_to_gt(guc);
> >         int ret;
> >  
> > +       /* Forcefully kill any remaining exec queues */
> > +       xe_guc_ct_stop(&guc->ct);
> > +       __xe_guc_submit_reset_prepare(guc);
> 
> In xe_guc_declare_wedged() we have the opposite sequence -
> reset_prepare and ct_stop after. Can we adjust to the same here? Or is
> there a reason to swap that?
> 

The ordering here doesn't matter. Can swap it here.

Matt

> Thanks,
> Stuart
> 
> > +       xe_guc_submit_stop(guc);
> > +       xe_guc_submit_pause_abort(guc);
> > +
> >         ret = wait_event_timeout(guc->submission_state.fini_wq,
> >                                  xa_empty(&guc-
> > >submission_state.exec_queue_lookup),
> >                                  HZ * 5);
> > @@ -2459,16 +2467,10 @@ static void guc_exec_queue_stop(struct xe_guc
> > *guc, struct xe_exec_queue *q)
> >         }
> >  }
> >  
> > -int xe_guc_submit_reset_prepare(struct xe_guc *guc)
> > +static int __xe_guc_submit_reset_prepare(struct xe_guc *guc)
> >  {
> >         int ret;
> >  
> > -       if (xe_gt_WARN_ON(guc_to_gt(guc), vf_recovery(guc)))
> > -               return 0;
> > -
> > -       if (!guc->submission_state.initialized)
> > -               return 0;
> > -
> >         /*
> >          * Using an atomic here rather than submission_state.lock as
> > this
> >          * function can be called while holding the CT lock (engine
> > reset
> > @@ -2483,6 +2485,17 @@ int xe_guc_submit_reset_prepare(struct xe_guc
> > *guc)
> >         return ret;
> >  }
> >  
> > +int xe_guc_submit_reset_prepare(struct xe_guc *guc)
> > +{
> > +       if (xe_gt_WARN_ON(guc_to_gt(guc), vf_recovery(guc)))
> > +               return 0;
> > +
> > +       if (!guc->submission_state.initialized)
> > +               return 0;
> > +
> > +       return __xe_guc_submit_reset_prepare(guc);
> > +}
> > +
> >  void xe_guc_submit_reset_wait(struct xe_guc *guc)
> >  {
> >         wait_event(guc->ct.wq, xe_device_wedged(guc_to_xe(guc)) ||
>