Re: [PATCH 1/1] drm/xe/vf: Reset recovery_queued after issuing RESFIX_START

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

On 12/5/2025 9:26 AM, Satyanarayana K V P wrote:
> During VF_RESTORE or VF_RESUME, the GuC sends a migration interrupt and
> clears the RESFIX_START marker. If migration or resume occurs before the
> VF issues its own RESFIX_START, VF KMD may receive two back-to-back
> migration interrupts. VF then sends RESFIX_START to indicate the beginning
> of fixups and RESFIX_DONE to mark completion. However, the second
> RESFIX_START fails because the GuC is already in the RUNNING state.
> 
> To prevent VF KMD from queuing additional recovery work items when extra
> interrupts arrive, move the clearing of recovery_queued from
> vf_post_migration_shutdown() to vf_post_migration_resfix_start().

hmm, it's not about moving the code from one function to other, as this is
implementation detail clear from the diff below, but rather we want to say
"clear the flag after sending a RESFIX_START message to ignore duplicated
IRQs seen before we start actual recovery"

> This ensures the state is reset only after the fixup process begins,
> avoiding redundant work item queuing.
> 
> Fixes: b5fbb94341a2 ("drm/xe/vf: Introduce RESFIX start marker support")
> Signed-off-by: Satyanarayana K V P <satyanarayana.k.v.p@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Matthew Brost <matthew.brost@intel.com>
> Cc: Tomasz Lis <tomasz.lis@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_gt_sriov_vf.c | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_gt_sriov_vf.c b/drivers/gpu/drm/xe/xe_gt_sriov_vf.c
> index 3c806c8e5f3e..90f2ef1772f2 100644
> --- a/drivers/gpu/drm/xe/xe_gt_sriov_vf.c
> +++ b/drivers/gpu/drm/xe/xe_gt_sriov_vf.c
> @@ -781,7 +781,7 @@ static void vf_start_migration_recovery(struct xe_gt *gt)
>  
>  	spin_lock(&gt->sriov.vf.migration.lock);
>  
> -	if (!gt->sriov.vf.migration.recovery_queued ||
> +	if (!gt->sriov.vf.migration.recovery_queued &&

this is a different fix that deserves its own separate patch with
a proper commit message

>  	    !gt->sriov.vf.migration.recovery_teardown) {
>  		gt->sriov.vf.migration.recovery_queued = true;
>  		WRITE_ONCE(gt->sriov.vf.migration.recovery_inprogress, true);
> @@ -1171,10 +1171,6 @@ static bool vf_post_migration_shutdown(struct xe_gt *gt)
>  			return true;
>  	}
>  
> -	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_ct_flush_and_stop(&gt->uc.guc.ct);
>  	xe_guc_submit_pause_vf(&gt->uc.guc);
>  	xe_tlb_inval_reset(&gt->tlb_inval);
> @@ -1258,7 +1254,15 @@ static int vf_post_migration_resfix_done(struct xe_gt *gt, u16 marker)
>  
>  static int vf_post_migration_resfix_start(struct xe_gt *gt, u16 marker)
>  {
> -	return vf_resfix_start(gt, marker);
> +	int err;
> +
> +	err = vf_resfix_start(gt, marker);
> +
> +	spin_lock_irq(&gt->sriov.vf.migration.lock);
> +	gt->sriov.vf.migration.recovery_queued = false;
> +	spin_unlock_irq(&gt->sriov.vf.migration.lock);

we may want to use

	scoped_guard(spinlock_irq, &gt->sriov.vf.migration.lock)
		gt->sriov.vf.migration.recovery_queued = false;

> +
> +	return err;
>  }
>  
>  static u16 vf_post_migration_next_resfix_marker(struct xe_gt *gt)