Re: [PATCH v2 3/4] drm/xe/multi_queue: track and recover lost CGP updates across VF migration

On Fri, Jul 31, 2026 at 04:26:26PM -0700, Niranjana Vishwanathapura wrote:
> When a CGP_SYNC (or REGISTER_CONTEXT_MULTI_QUEUE with CGP) is in flight
> during VF migration, GuC loses the message and never sends CGP_SYNC_DONE.
> Two failure modes exist:
> 
> 1. The send was already issued (CGP_SYNC_DONE not received):
>    group->sync_pending stays set and cgp_update_q points to the queue
>    that owns the outstanding sync.
> 
> 2. The wait woke early (send not issued):
>    The queue returned from xe_guc_exec_queue_group_cgp_sync() without
>    sending after vf_recovery() became true.
> 
> Track which kind of sync is outstanding (registering_cgp / updating_cgp)
> and the owning queue (cgp_update_q) so
> guc_exec_queue_revert_pending_state_change()
> can recover both cases:
> - A registration-time CGP bails or is lost  → clear registered flag so
>   run_job re-registers after unpause (re_register / registering_cgp paths).
> - A dynamic CGP update bails or is lost → set needs_cgp_sync so replay
>   re-issues the update after unpause (re_update / updating_cgp paths).
> 
> Tag all registration call sites with CGP_SYNC_REGISTRATION so the bail
> path distinguishes them from dynamic updates.
> 
> Assisted-by: Github-Copilot:Claude-opus-4.8
> Signed-off-by: Niranjana Vishwanathapura <[email protected]>

Reviewed-by: Matthew Brost <[email protected]>

> ---
>  drivers/gpu/drm/xe/xe_exec_queue_types.h     |   7 ++
>  drivers/gpu/drm/xe/xe_guc_exec_queue_types.h |  30 +++++
>  drivers/gpu/drm/xe/xe_guc_submit.c           | 112 +++++++++++++++++--
>  3 files changed, 141 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_exec_queue_types.h b/drivers/gpu/drm/xe/xe_exec_queue_types.h
> index 53b6c0bf4849..b2276559c2f6 100644
> --- a/drivers/gpu/drm/xe/xe_exec_queue_types.h
> +++ b/drivers/gpu/drm/xe/xe_exec_queue_types.h
> @@ -70,6 +70,13 @@ struct xe_exec_queue_group {
>  	spinlock_t suspend_lock;
>  	/** @sync_pending: CGP_SYNC_DONE g2h response pending */
>  	bool sync_pending;
> +	/**
> +	 * @cgp_update_q: Queue that issued the currently outstanding (sent)
> +	 * CGP_SYNC or REGISTER_CONTEXT_MULTI_QUEUE; NULL when none is
> +	 * outstanding. Used during VF recovery to identify and replay the
> +	 * message whose CGP_SYNC_DONE was not received.
> +	 */
> +	struct xe_exec_queue *cgp_update_q;
>  	/** @banned: Group banned */
>  	bool banned;
>  	/** @stopped: Group is stopped, protected by list_lock */
> diff --git a/drivers/gpu/drm/xe/xe_guc_exec_queue_types.h b/drivers/gpu/drm/xe/xe_guc_exec_queue_types.h
> index acdc24d1a6bd..573b920edb41 100644
> --- a/drivers/gpu/drm/xe/xe_guc_exec_queue_types.h
> +++ b/drivers/gpu/drm/xe/xe_guc_exec_queue_types.h
> @@ -76,6 +76,36 @@ struct xe_guc_exec_queue {
>  	 * recovery.
>  	 */
>  	bool needs_resume;
> +	/** @multi_queue: multi-queue group CGP state for VF post migration recovery */
> +	struct {
> +		/**
> +		 * @multi_queue.needs_cgp_sync: Needs a CGP_SYNC (dynamic CGP
> +		 * update) message replayed during recovery.
> +		 */
> +		u8 needs_cgp_sync:1;
> +		/**
> +		 * @multi_queue.re_register: A registration-time CGP update was
> +		 * interrupted by recovery; the queue must be re-registered.
> +		 */
> +		u8 re_register:1;
> +		/**
> +		 * @multi_queue.re_update: A dynamic CGP update was interrupted
> +		 * by recovery; the CGP update must be replayed.
> +		 */
> +		u8 re_update:1;
> +		/**
> +		 * @multi_queue.registering_cgp: This queue's currently
> +		 * outstanding CGP_SYNC is a registration (matched against
> +		 * group->cgp_update_q in revert).
> +		 */
> +		u8 registering_cgp:1;
> +		/**
> +		 * @multi_queue.updating_cgp: This queue's currently outstanding
> +		 * CGP_SYNC is a dynamic update (matched against
> +		 * group->cgp_update_q in revert).
> +		 */
> +		u8 updating_cgp:1;
> +	} multi_queue;
>  };
>  
>  #endif
> diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c b/drivers/gpu/drm/xe/xe_guc_submit.c
> index 13d0ab8052e5..c018bc0d8d6f 100644
> --- a/drivers/gpu/drm/xe/xe_guc_submit.c
> +++ b/drivers/gpu/drm/xe/xe_guc_submit.c
> @@ -800,9 +800,12 @@ static void xe_guc_exec_queue_group_cgp_update(struct xe_device *xe,
>  	}
>  }
>  
> +#define CGP_SYNC_REGISTRATION			BIT(0)
> +
>  static void xe_guc_exec_queue_group_cgp_sync(struct xe_guc *guc,
>  					     struct xe_exec_queue *q,
> -					     const u32 *action, u32 len)
> +					     const u32 *action, u32 len,
> +					     unsigned int flags)
>  {
>  	struct xe_exec_queue_group *group = q->multi_queue.group;
>  	struct xe_device *xe = guc_to_xe(guc);
> @@ -829,17 +832,45 @@ static void xe_guc_exec_queue_group_cgp_sync(struct xe_guc *guc,
>  		return;
>  	}
>  
> +	/*
> +	 * If woken by VF migration recovery, do not touch the CGP or send: the
> +	 * message would be lost and, for a registration, GuC must (re-)register
> +	 * the context before its CGP entry may be read. Flag the queue so revert
> +	 * replays it - a registration by re-registration, a dynamic update by a
> +	 * replayed CGP_SYNC - and bail.
> +	 */
> +	if (vf_recovery(guc)) {
> +		if (flags & CGP_SYNC_REGISTRATION)
> +			q->guc->multi_queue.re_register = true;
> +		else
> +			q->guc->multi_queue.re_update = true;
> +		return;
> +	}
> +
>  	scoped_guard(spinlock, &q->multi_queue.lock)
>  		priority = q->multi_queue.priority;
>  
>  	xe_lrc_set_multi_queue_priority(q->lrc[0], priority);
>  	xe_guc_exec_queue_group_cgp_update(xe, q);
>  
> +	/*
> +	 * Record the nature of this outstanding sync so revert can replay it if
> +	 * its CGP_SYNC_DONE is lost across a migration: a registration is
> +	 * recovered by re-registration, a dynamic update by a replayed CGP_SYNC.
> +	 */
> +	if (flags & CGP_SYNC_REGISTRATION) {
> +		q->guc->multi_queue.registering_cgp = true;
> +		q->guc->multi_queue.updating_cgp = false;
> +	} else {
> +		q->guc->multi_queue.updating_cgp = true;
> +		q->guc->multi_queue.registering_cgp = false;
> +	}
> +	WRITE_ONCE(group->cgp_update_q, q);
>  	WRITE_ONCE(group->sync_pending, true);
>  	xe_guc_ct_send(&guc->ct, action, len, G2H_LEN_DW_MULTI_QUEUE_CONTEXT, 1);
>  }
>  
> -static void guc_exec_queue_send_cgp_sync(struct xe_exec_queue *q)
> +static void guc_exec_queue_send_cgp_sync(struct xe_exec_queue *q, unsigned int flags)
>  {
>  #define MAX_MULTI_QUEUE_CGP_SYNC_SIZE	(2)
>  	struct xe_guc *guc = exec_queue_to_guc(q);
> @@ -853,7 +884,7 @@ static void guc_exec_queue_send_cgp_sync(struct xe_exec_queue *q)
>  	xe_gt_assert(guc_to_gt(guc), len <= MAX_MULTI_QUEUE_CGP_SYNC_SIZE);
>  #undef MAX_MULTI_QUEUE_CGP_SYNC_SIZE
>  
> -	xe_guc_exec_queue_group_cgp_sync(guc, q, action, len);
> +	xe_guc_exec_queue_group_cgp_sync(guc, q, action, len, flags);
>  }
>  
>  static void __register_exec_queue_group(struct xe_exec_queue *q,
> @@ -881,7 +912,8 @@ static void __register_exec_queue_group(struct xe_exec_queue *q,
>  	 * XE_GUC_ACTION_NOTIFY_MULTI_QUEUE_CONTEXT_CGP_SYNC_DONE response
>  	 * from guc.
>  	 */
> -	xe_guc_exec_queue_group_cgp_sync(guc, q, action, len);
> +	xe_guc_exec_queue_group_cgp_sync(guc, q, action, len,
> +					 CGP_SYNC_REGISTRATION);
>  }
>  
>  static void __register_mlrc_exec_queue(struct xe_guc *guc,
> @@ -1041,7 +1073,7 @@ static void register_exec_queue(struct xe_exec_queue *q, int ctx_type)
>  		init_policies(guc, q);
>  
>  	if (xe_exec_queue_is_multi_queue_secondary(q))
> -		guc_exec_queue_send_cgp_sync(q);
> +		guc_exec_queue_send_cgp_sync(q, CGP_SYNC_REGISTRATION);
>  }
>  
>  static u32 wq_space_until_wrap(struct xe_exec_queue *q)
> @@ -1923,7 +1955,7 @@ static void __guc_exec_queue_process_msg_set_multi_queue_priority(struct xe_sche
>  	struct xe_exec_queue *q = msg->private_data;
>  
>  	if (guc_exec_queue_allowed_to_change_state(q))
> -		guc_exec_queue_send_cgp_sync(q);
> +		guc_exec_queue_send_cgp_sync(q, 0);
>  
>  	kfree(msg);
>  }
> @@ -2716,6 +2748,57 @@ static void guc_exec_queue_revert_pending_state_change(struct xe_guc *guc,
>  			  q->guc->id);
>  	}
>  
> +	/*
> +	 * A registration time CGP update that bailed when woken by VF recovery.
> +	 * Re-register the queue.
> +	 */
> +	if (q->guc->multi_queue.re_register) {
> +		clear_exec_queue_registered(q);
> +		q->guc->multi_queue.re_register = false;
> +		xe_gt_dbg(guc_to_gt(guc), "Replay REGISTER (cgp) - guc_id=%d",
> +			  q->guc->id);
> +	}
> +
> +	/*
> +	 * If a CGP update gets dropped during migration, CGP_SYNC_DONE will not
> +	 * be received (sync_pending still set and this queue owns it). Recover
> +	 * it the same way and clear the stuck sync_pending.
> +	 */
> +	if (xe_exec_queue_is_multi_queue(q)) {
> +		struct xe_exec_queue_group *group = q->multi_queue.group;
> +
> +		if (q == READ_ONCE(group->cgp_update_q) &&
> +		    READ_ONCE(group->sync_pending)) {
> +			if (q->guc->multi_queue.registering_cgp) {
> +				clear_exec_queue_registered(q);
> +				xe_gt_dbg(guc_to_gt(guc), "Replay REGISTER (cgp sync) - guc_id=%d",
> +					  q->guc->id);
> +			} else if (q->guc->multi_queue.updating_cgp) {
> +				q->guc->multi_queue.needs_cgp_sync = true;
> +				xe_gt_dbg(guc_to_gt(guc), "Replay CGP_SYNC - guc_id=%d",
> +					  q->guc->id);
> +			}
> +			q->guc->multi_queue.registering_cgp = false;
> +			q->guc->multi_queue.updating_cgp = false;
> +			WRITE_ONCE(group->cgp_update_q, NULL);
> +			WRITE_ONCE(group->sync_pending, false);
> +		}
> +	}
> +
> +	/*
> +	 * A dynamic-time CGP update that bailed when woken by VF recovery.
> +	 * Replay the dynamic CGP update unless the queue is registered or being
> +	 * re-registered, which re-does the CGP anyway.
> +	 */
> +	if (q->guc->multi_queue.re_update) {
> +		q->guc->multi_queue.re_update = false;
> +		if (exec_queue_registered(q)) {
> +			q->guc->multi_queue.needs_cgp_sync = true;
> +			xe_gt_dbg(guc_to_gt(guc), "Replay CGP_SYNC (re-update) - guc_id=%d",
> +				  q->guc->id);
> +		}
> +	}
> +
>  	q->guc->resume_time = 0;
>  }
>  
> @@ -3424,7 +3507,8 @@ int xe_guc_exec_queue_cgp_context_error_handler(struct xe_guc *guc, u32 *msg,
>  int xe_guc_exec_queue_cgp_sync_done_handler(struct xe_guc *guc, u32 *msg, u32 len)
>  {
>  	struct xe_device *xe = guc_to_xe(guc);
> -	struct xe_exec_queue *q;
> +	struct xe_exec_queue_group *group;
> +	struct xe_exec_queue *q, *upd_q;
>  	u32 guc_id = msg[0];
>  
>  	if (unlikely(len < 1)) {
> @@ -3441,8 +3525,20 @@ int xe_guc_exec_queue_cgp_sync_done_handler(struct xe_guc *guc, u32 *msg, u32 le
>  		return -EPROTO;
>  	}
>  
> +	/*
> +	 * The outstanding CGP update is now confirmed; clear the owning queue's
> +	 * tracking so a later migration does not needlessly replay it.
> +	 */
> +	group = q->multi_queue.group;
> +	upd_q = READ_ONCE(group->cgp_update_q);
> +	if (upd_q) {
> +		upd_q->guc->multi_queue.registering_cgp = false;
> +		upd_q->guc->multi_queue.updating_cgp = false;
> +		WRITE_ONCE(group->cgp_update_q, NULL);
> +	}
> +
>  	/* Wakeup the serialized cgp update wait */
> -	WRITE_ONCE(q->multi_queue.group->sync_pending, false);
> +	WRITE_ONCE(group->sync_pending, false);
>  	xe_guc_ct_wake_waiters(&guc->ct);
>  
>  	return 0;
> -- 
> 2.43.0
>