Re: [RFC 4/4] drm/xe/multi_queue: Add needs_cgp_sync mechanism for VF post-migration recovery

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

On Wed, Apr 22, 2026 at 07:49:56PM -0700, Niranjana Vishwanathapura wrote:
> Multi-queue groups use CGP_SYNC H2G messages to notify GuC of queue
> additions and priority changes. If a CGP_SYNC was in-flight when a VF
> migration was detected, the response will never arrive and sync_pending
> will be stuck set, causing a timeout on the next attempt.
> 
> Introduce a needs_cgp_sync flag in xe_guc_exec_queue (mirroring the
> existing needs_resume / needs_suspend / needs_cleanup pattern) to track
> this case:
> 
> - In guc_exec_queue_revert_pending_state_change(), if sync_pending is
>   set on a multi-queue exec_queue, clear it (preventing a timeout on
>   recovery) and set needs_cgp_sync, unless the queue is about to be
>   re-registered (which will issue its own CGP_SYNC).
> 
> - In guc_exec_queue_replay_pending_state_change(), if needs_cgp_sync is
>   set, push a STATIC_MSG_CGP_SYNC to the scheduler head so the CGP_SYNC
>   is replayed after unpause.
> 
> Wire up the new CGP_SYNC_MSG opcode and STATIC_MSG_CGP_SYNC static
> message slot, along with __guc_exec_queue_process_msg_cgp_sync() to
> handle the message in the scheduler workqueue.
> 
> Assisted-by: GitHub Copilot:claude-sonnet-4.6
> Signed-off-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>

This looks right to me. Ideally in your testing we'd like to see...

'Replay REGISTER - guc_id' for both primary and secondary queues...

So maybe update the 'Replay REGISTER - guc_id' debug with information if
the queue is normal, primary, or secondary. This logging is how I
initially checked out VF migration before turning it over to the SRIOV
team extensive testing.

We'd also ideally like to see 'Replay CGP_SYNC - guc_id=%d' in some test
case too.

This is all quite racey so you make have run test cases and move a VF a
bunch of times to get these things to trigger.

One more nit below.

> ---
>  drivers/gpu/drm/xe/xe_guc_exec_queue_types.h |  7 +++-
>  drivers/gpu/drm/xe/xe_guc_submit.c           | 39 ++++++++++++++++++++
>  2 files changed, 45 insertions(+), 1 deletion(-)
> 
> 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 e5e53b421f29..f12843b80ca2 100644
> --- a/drivers/gpu/drm/xe/xe_guc_exec_queue_types.h
> +++ b/drivers/gpu/drm/xe/xe_guc_exec_queue_types.h
> @@ -31,7 +31,7 @@ struct xe_guc_exec_queue {
>  	 * a message needs to sent through the GPU scheduler but memory
>  	 * allocations are not allowed.
>  	 */
> -#define MAX_STATIC_MSG_TYPE	3
> +#define MAX_STATIC_MSG_TYPE  4
>  	struct xe_sched_msg static_msgs[MAX_STATIC_MSG_TYPE];
>  	/** @destroy_async: do final destroy async from this worker */
>  	struct work_struct destroy_async;
> @@ -64,6 +64,11 @@ struct xe_guc_exec_queue {
>  	 * recovery.
>  	 */
>  	bool needs_resume;
> +	/**
> +	 * @needs_cgp_sync: Needs a CGP_SYNC message during VF post migration
> +	 * recovery.
> +	 */
> +	bool needs_cgp_sync;
>  };
>  
>  #endif
> diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c b/drivers/gpu/drm/xe/xe_guc_submit.c
> index 0e857ef06611..9cb72754215d 100644
> --- a/drivers/gpu/drm/xe/xe_guc_submit.c
> +++ b/drivers/gpu/drm/xe/xe_guc_submit.c
> @@ -1886,11 +1886,20 @@ static void __guc_exec_queue_process_msg_set_multi_queue_priority(struct xe_sche
>  	kfree(msg);
>  }
>  
> +static void __guc_exec_queue_process_msg_cgp_sync(struct xe_sched_msg *msg)
> +{
> +	struct xe_exec_queue *q = msg->private_data;
> +
> +	if (guc_exec_queue_allowed_to_change_state(q))
> +		guc_exec_queue_send_cgp_sync(q);
> +}
> +
>  #define CLEANUP				1	/* Non-zero values to catch uninitialized msg */
>  #define SET_SCHED_PROPS			2
>  #define SUSPEND				3
>  #define RESUME				4
>  #define SET_MULTI_QUEUE_PRIORITY	5
> +#define CGP_SYNC_MSG			6
>  #define OPCODE_MASK	0xf
>  #define MSG_LOCKED	BIT(8)
>  #define MSG_HEAD	BIT(9)
> @@ -1917,6 +1926,9 @@ static void guc_exec_queue_process_msg(struct xe_sched_msg *msg)
>  	case SET_MULTI_QUEUE_PRIORITY:
>  		__guc_exec_queue_process_msg_set_multi_queue_priority(msg);
>  		break;
> +	case CGP_SYNC_MSG:
> +		__guc_exec_queue_process_msg_cgp_sync(msg);
> +		break;
>  	default:
>  		XE_WARN_ON("Unknown message type");
>  	}
> @@ -2082,6 +2094,7 @@ static bool guc_exec_queue_try_add_msg(struct xe_exec_queue *q,
>  #define STATIC_MSG_CLEANUP	0
>  #define STATIC_MSG_SUSPEND	1
>  #define STATIC_MSG_RESUME	2
> +#define STATIC_MSG_CGP_SYNC	3
>  static void guc_exec_queue_destroy(struct xe_exec_queue *q)
>  {
>  	struct xe_sched_msg *msg = q->guc->static_msgs + STATIC_MSG_CLEANUP;
> @@ -2452,6 +2465,22 @@ static void guc_exec_queue_revert_pending_state_change(struct xe_guc *guc,
>  			  q->guc->id);
>  	}
>  
> +	/*
> +	 * If a CGP_SYNC was in flight when the VF was paused, clear the pending
> +	 * flag to prevent a timeout during recovery. Only schedule a replay if
> +	 * the queue won't be re-registered (which would include its own
> +	 * CGP_SYNC as part of the registration process).
> +	 */
> +	if (xe_exec_queue_is_multi_queue(q) &&
> +	    READ_ONCE(q->multi_queue.group->sync_pending)) {
> +		WRITE_ONCE(q->multi_queue.group->sync_pending, false);
> +		if (!(pending_enable && !pending_resume)) {

Maybe a local variable...

bool register_inflight = pending_enable && !pending_resume;

Then use this in the unseen re-register logic above and then here.

Matt

> +			q->guc->needs_cgp_sync = true;
> +			xe_gt_dbg(guc_to_gt(guc), "Replay CGP_SYNC - guc_id=%d",
> +				  q->guc->id);
> +		}
> +	}
> +
>  	q->guc->resume_time = 0;
>  }
>  
> @@ -2688,6 +2717,16 @@ static void guc_exec_queue_replay_pending_state_change(struct xe_exec_queue *q)
>  
>  		q->guc->needs_resume = false;
>  	}
> +
> +	if (q->guc->needs_cgp_sync) {
> +		msg = q->guc->static_msgs + STATIC_MSG_CGP_SYNC;
> +
> +		xe_sched_msg_lock(sched);
> +		guc_exec_queue_try_add_msg_head(q, msg, CGP_SYNC_MSG);
> +		xe_sched_msg_unlock(sched);
> +
> +		q->guc->needs_cgp_sync = false;
> +	}
>  }
>  
>  static void guc_exec_queue_unpause(struct xe_guc *guc, struct xe_exec_queue *q)
> -- 
> 2.43.0
>