Re: [PATCH v3 18/36] drm/xe/vf: Don't allow GT reset to be queued during VF post migration recovery

[Michal Wajdeczko <michal.wajdeczko@intel.com>]

On 9/29/2025 4:55 AM, Matthew Brost wrote:
> With well-behaved software, a GT reset should never occur, nor should it
> happen during VF post-migration recovery. If it does, trigger a warning

hmm, I'm not sure that GT-resets depends only on the SW side, nor that
reasons for it couldn't happen just before VF was migrated

> but suppress the GT reset, as VF post-migration recovery is expected to
> bring the VF back to a working state.

can't we just say this last sentence, that "there is no need to run
explicit VF-reset sequence during recovery as VF-recovery is equivalent 
and it is also expected to bring VF back to working state?

also, since the patch is a refactor, it should mention that "instead of
blocking resets, just rely on the recovery" 
> 
> v3:
>  - Better commit message (Tomasz)
> 
> Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_gt.c          |  9 -------
>  drivers/gpu/drm/xe/xe_gt_sriov_vf.c |  7 -----
>  drivers/gpu/drm/xe/xe_guc_submit.c  | 41 +++--------------------------
>  drivers/gpu/drm/xe/xe_guc_submit.h  |  3 ---
>  4 files changed, 4 insertions(+), 56 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c
> index 82be38c99205..5f04d562604b 100644
> --- a/drivers/gpu/drm/xe/xe_gt.c
> +++ b/drivers/gpu/drm/xe/xe_gt.c
> @@ -815,11 +815,6 @@ static int do_gt_restart(struct xe_gt *gt)
>  	return 0;
>  }
>  
> -static int gt_wait_reset_unblock(struct xe_gt *gt)
> -{
> -	return xe_guc_wait_reset_unblock(&gt->uc.guc);
> -}
> -
>  static int gt_reset(struct xe_gt *gt)
>  {
>  	unsigned int fw_ref;
> @@ -834,10 +829,6 @@ static int gt_reset(struct xe_gt *gt)
>  
>  	xe_gt_info(gt, "reset started\n");
>  
> -	err = gt_wait_reset_unblock(gt);
> -	if (!err)
> -		xe_gt_warn(gt, "reset block failed to get lifted");
> -
>  	xe_pm_runtime_get(gt_to_xe(gt));
>  
>  	if (xe_fault_inject_gt_reset()) {
> diff --git a/drivers/gpu/drm/xe/xe_gt_sriov_vf.c b/drivers/gpu/drm/xe/xe_gt_sriov_vf.c
> index cc5af19c1911..b16e8fd271f8 100644
> --- a/drivers/gpu/drm/xe/xe_gt_sriov_vf.c
> +++ b/drivers/gpu/drm/xe/xe_gt_sriov_vf.c
> @@ -1175,17 +1175,11 @@ void xe_gt_sriov_vf_print_version(struct xe_gt *gt, struct drm_printer *p)
>  
>  static void vf_post_migration_shutdown(struct xe_gt *gt)
>  {
> -	int ret = 0;
> -
>  	spin_lock_irq(&gt->sriov.vf.migration.lock);
>  	gt->sriov.vf.migration.recovery_queued = false;
>  	spin_unlock_irq(&gt->sriov.vf.migration.lock);
>  
>  	xe_guc_submit_pause(&gt->uc.guc);
> -	ret |= xe_guc_submit_reset_block(&gt->uc.guc);
> -
> -	if (ret)
> -		xe_gt_sriov_info(gt, "migration recovery encountered ongoing reset\n");
>  }
>  
>  static size_t post_migration_scratch_size(struct xe_device *xe)
> @@ -1219,7 +1213,6 @@ static void vf_post_migration_kickstart(struct xe_gt *gt)
>  	 */
>  	xe_irq_resume(gt_to_xe(gt));
>  
> -	xe_guc_submit_reset_unblock(&gt->uc.guc);
>  	xe_guc_submit_unpause(&gt->uc.guc);
>  }
>  
> diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c b/drivers/gpu/drm/xe/xe_guc_submit.c
> index cd5e506527fe..b82976f031e5 100644
> --- a/drivers/gpu/drm/xe/xe_guc_submit.c
> +++ b/drivers/gpu/drm/xe/xe_guc_submit.c
> @@ -27,6 +27,7 @@
>  #include "xe_gt.h"
>  #include "xe_gt_clock.h"
>  #include "xe_gt_printk.h"
> +#include "xe_gt_sriov_vf.h"
>  #include "xe_guc.h"
>  #include "xe_guc_capture.h"
>  #include "xe_guc_ct.h"
> @@ -2182,47 +2183,13 @@ static void guc_exec_queue_stop(struct xe_guc *guc, struct xe_exec_queue *q)
>  	}
>  }
>  
> -/**
> - * xe_guc_submit_reset_block - Disallow reset calls on given GuC.
> - * @guc: the &xe_guc struct instance
> - */
> -int xe_guc_submit_reset_block(struct xe_guc *guc)
> -{
> -	return atomic_fetch_or(1, &guc->submission_state.reset_blocked);
> -}
> -
> -/**
> - * xe_guc_submit_reset_unblock - Allow back reset calls on given GuC.
> - * @guc: the &xe_guc struct instance
> - */
> -void xe_guc_submit_reset_unblock(struct xe_guc *guc)
> -{
> -	atomic_set_release(&guc->submission_state.reset_blocked, 0);
> -	wake_up_all(&guc->ct.wq);
> -}
> -
> -static int guc_submit_reset_is_blocked(struct xe_guc *guc)
> -{
> -	return atomic_read_acquire(&guc->submission_state.reset_blocked);
> -}
> -
> -/* Maximum time of blocking reset */
> -#define RESET_BLOCK_PERIOD_MAX (HZ * 5)
> -
> -/**
> - * xe_guc_wait_reset_unblock - Wait until reset blocking flag is lifted, or timeout.
> - * @guc: the &xe_guc struct instance
> - */
> -int xe_guc_wait_reset_unblock(struct xe_guc *guc)
> -{
> -	return wait_event_timeout(guc->ct.wq,
> -				  !guc_submit_reset_is_blocked(guc), RESET_BLOCK_PERIOD_MAX);
> -}
> -
>  int xe_guc_submit_reset_prepare(struct xe_guc *guc)
>  {
>  	int ret;
>  
> +	if (WARN_ON_ONCE(xe_gt_sriov_vf_recovery_inprogress(guc_to_gt(guc))))

we have a gt-oriented variant:

	xe_gt_WARN_ON

and likely we should not use the _ONCE variant as resets could happen few
times in the VF lifetime

and since we are sure that "recovery" has the same power as "reset"
does it really need to be full WARN ? maybe just "info or notice" ?

also if want to skip real GT resets, shouldn't we have this check
upper in the reset stack, at GT level, not just under gt.uc.guc.submit ?

> +		return 0;
> +
>  	if (!guc->submission_state.initialized)
>  		return 0;
>  
> diff --git a/drivers/gpu/drm/xe/xe_guc_submit.h b/drivers/gpu/drm/xe/xe_guc_submit.h
> index 5b4a0a6fd818..f535fe3895e5 100644
> --- a/drivers/gpu/drm/xe/xe_guc_submit.h
> +++ b/drivers/gpu/drm/xe/xe_guc_submit.h
> @@ -22,9 +22,6 @@ 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);
> -int xe_guc_submit_reset_block(struct xe_guc *guc);
> -void xe_guc_submit_reset_unblock(struct xe_guc *guc);
> -int xe_guc_wait_reset_unblock(struct xe_guc *guc);
>  void xe_guc_submit_wedge(struct xe_guc *guc);
>  
>  int xe_guc_read_stopped(struct xe_guc *guc);