Re: [PATCH v2 25/34] drm/xe/vf: Abort VF post migration recovery on failure

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

On Sat, Sep 27, 2025 at 01:54:35PM +0200, Lis, Tomasz wrote:
> 
> On 9/24/2025 3:15 AM, Matthew Brost wrote:
> > If VF post-migration recovery fails, the device is wedged. However,
> > submission queues still need to be enabled for proper cleanup. In such
> > cases, call into the GuC submission backend to restart all queues that
> > were previously paused.
> > 
> > Signed-off-by: Matthew Brost<matthew.brost@intel.com>
> > ---
> >   drivers/gpu/drm/xe/xe_gt_sriov_vf.c | 10 ++++++++++
> >   drivers/gpu/drm/xe/xe_guc_submit.c  | 20 ++++++++++++++++++++
> >   drivers/gpu/drm/xe/xe_guc_submit.h  |  1 +
> >   3 files changed, 31 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_gt_sriov_vf.c b/drivers/gpu/drm/xe/xe_gt_sriov_vf.c
> > index 34e57f44fbf4..a987560de2c7 100644
> > --- a/drivers/gpu/drm/xe/xe_gt_sriov_vf.c
> > +++ b/drivers/gpu/drm/xe/xe_gt_sriov_vf.c
> > @@ -1224,6 +1224,15 @@ static void vf_post_migration_kickstart(struct xe_gt *gt)
> >   	xe_guc_submit_unpause(&gt->uc.guc);
> >   }
> > +static void vf_post_migration_abort(struct xe_gt *gt)
> > +{
> > +	spin_lock_irq(&gt->sriov.vf.migration.lock);
> > +	WRITE_ONCE(gt->sriov.vf.migration.recovery_inprogress, false);
> > +	spin_unlock_irq(&gt->sriov.vf.migration.lock);
> > +
> > +	xe_guc_submit_pause_abort(&gt->uc.guc);
> > +}
> > +
> >   static int vf_post_migration_notify_resfix_done(struct xe_gt *gt)
> >   {
> >   	bool skip_resfix = false;
> > @@ -1280,6 +1289,7 @@ static void vf_post_migration_recovery(struct xe_gt *gt)
> >   	xe_gt_sriov_notice(gt, "migration recovery ended\n");
> >   	return;
> >   fail:
> > +	vf_post_migration_abort(gt);
> >   	xe_pm_runtime_put(xe);
> >   	xe_gt_sriov_err(gt, "migration recovery failed (%pe)\n", ERR_PTR(err));
> >   	xe_device_declare_wedged(xe);
> 
> We could alter the order here to make sure all queues have cleanup
> triggered. Now only the ones which were previously killed/banned/wedged have
> that.
> 
> Not sure if it would bring any benefits though.
> 

In general doing things like this (doing things behind users back) has
proven to be a bad idea. Also in wedge mode == 2, we intentionally stop
all queues from being torn down until driver load - this enables SV to
come in and run tools to debug hardware issue or we can examine general
hardware state without it changing. 

> > diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c b/drivers/gpu/drm/xe/xe_guc_submit.c
> > index 52b86cab4ec5..8bee65dd9ca6 100644
> > --- a/drivers/gpu/drm/xe/xe_guc_submit.c
> > +++ b/drivers/gpu/drm/xe/xe_guc_submit.c
> > @@ -2345,6 +2345,26 @@ void xe_guc_submit_unpause(struct xe_guc *guc)
> >   	wake_up_all(&guc->ct.wq);
> >   }
> > +/**
> > + * xe_guc_submit_abort - Avort all paused submission task on given GuC.
> 
> typo - Avort
> 

Yea, will fix.

Matt

> -Tomasz
> 
> > + * @guc: the &xe_guc struct instance whose scheduler is to be aborted
> > + */
> > +void xe_guc_submit_pause_abort(struct xe_guc *guc)
> > +{
> > +	struct xe_exec_queue *q;
> > +	unsigned long index;
> > +
> > +	mutex_lock(&guc->submission_state.lock);
> > +	xa_for_each(&guc->submission_state.exec_queue_lookup, index, q) {
> > +		struct xe_gpu_scheduler *sched = &q->guc->sched;
> > +
> > +		xe_sched_submission_start(sched);
> > +		if (exec_queue_killed_or_banned_or_wedged(q))
> > +			xe_guc_exec_queue_trigger_cleanup(q);
> > +	}
> > +	mutex_unlock(&guc->submission_state.lock);
> > +}
> > +
> >   static struct xe_exec_queue *
> >   g2h_exec_queue_lookup(struct xe_guc *guc, u32 guc_id)
> >   {
> > diff --git a/drivers/gpu/drm/xe/xe_guc_submit.h b/drivers/gpu/drm/xe/xe_guc_submit.h
> > index f535fe3895e5..fe82c317048e 100644
> > --- a/drivers/gpu/drm/xe/xe_guc_submit.h
> > +++ b/drivers/gpu/drm/xe/xe_guc_submit.h
> > @@ -22,6 +22,7 @@ void xe_guc_submit_stop(struct xe_guc *guc);
> >   int xe_guc_submit_start(struct xe_guc *guc);
> >   void xe_guc_submit_pause(struct xe_guc *guc);
> >   void xe_guc_submit_unpause(struct xe_guc *guc);
> > +void xe_guc_submit_pause_abort(struct xe_guc *guc);
> >   void xe_guc_submit_wedge(struct xe_guc *guc);
> >   int xe_guc_read_stopped(struct xe_guc *guc);