Re: [PATCH] drm/xe/guc: Silently discard stale G2H reply during overlap

On Fri, Aug 07, 2026 at 07:04:21AM +0000, Nishit Sharma wrote:
> When xe_force_gt_reset_async() fires a second reset before the first
> reset's post-recovery drain has completed, GuC may still be processing
> H2G requests (SCHED_CONTEXT_MODE_SET / DEREGISTER_CONTEXT) that were
> issued by the first reset. The G2H replies SCHED_DONE and DEREGISTER_DONE

We don't issue H2G as part of a GT reset. We stop the CTs, fixup
anything which is missing, and blow away anything in CTs when we
restart.

The relavent functions here are:

xe_guc_ct_stop()
guc_exec_queue_stop()
xe_guc_ct_start() 

> can then arrive after the KMD has already marked those exec queues as
> banned, killed, or wedged in response to the second reset.

> At that point the state flags that the reply handlers assert on
> have already been cleared, and the handlers treat this as a protocol
> violation, emitting an *ERROR* and returning -EPROTO.

We should not be recieving G2H after xe_guc_ct_stop() and then after we
internally mess the with the queue state in guc_exec_queue_stop().

> 
> This is not a real error. The reset path tears down exec queues
> independently of any in-flight G2H traffic, so a stale reply for a dead

No. G2H traffic is stopped in xe_guc_ct_stop() and any remaining
commands are squashed in xe_guc_ct_start().

Can you give an example of the failure you're trying to fix? This seems
to be a misdiagnosis.

> queue is harmless. The proper response is to discard it silently and let
> the reset-driven teardown proceed undisturbed.
> 
> Detect this condition by checking whether the queue is already in a
> terminal state (banned, killed, or wedged) before treating an unexpected
> G2H as a protocol error. If so, log at debug level and return success.
> 
> Signed-off-by: Nishit Sharma <[email protected]>
> ---
>  drivers/gpu/drm/xe/xe_guc_submit.c | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c b/drivers/gpu/drm/xe/xe_guc_submit.c
> index 8aaed4fd13ea..988c236c6a03 100644
> --- a/drivers/gpu/drm/xe/xe_guc_submit.c
> +++ b/drivers/gpu/drm/xe/xe_guc_submit.c
> @@ -3149,6 +3149,19 @@ int xe_guc_sched_done_handler(struct xe_guc *guc, u32 *msg, u32 len)
>  
>  	if (unlikely(!exec_queue_pending_enable(q) &&
>  		     !exec_queue_pending_disable(q))) {
> +		/*
> +		 * Stale SCHED_DONE from a prior reset cycle: the reset marked
> +		 * the queue banned/killed (clearing the pending flags) before
> +		 * GuC's reply landed. Discard silently
> +		 */
> +		if (exec_queue_banned(q) || exec_queue_killed(q) ||

"Killed" is 100% the wrong term here. We kill queues during a user queue
close or FD close without triggering a GT reset, and the normal firmware
protocol flows are expected to continue working with 100% accuracy or a
bunch of things leak.

wedged, banned also look wrong too - see above.

Matt

> +		    exec_queue_wedged(q)) {
> +			xe_gt_dbg(guc_to_gt(guc),
> +				  "SCHED_DONE: Stale G2H guc_id=%d, state=0x%04x\n",
> +				  q->guc->id, atomic_read(&q->guc->state));
> +			return 0;
> +		}
> +
>  		xe_gt_err(guc_to_gt(guc),
>  			  "SCHED_DONE: Unexpected engine state 0x%04x, guc_id=%d, runnable_state=%u",
>  			  atomic_read(&q->guc->state), q->guc->id,
> @@ -3185,6 +3198,18 @@ int xe_guc_deregister_done_handler(struct xe_guc *guc, u32 *msg, u32 len)
>  
>  	if (!exec_queue_destroyed(q) || exec_queue_pending_disable(q) ||
>  	    exec_queue_pending_enable(q) || exec_queue_enabled(q)) {
> +		 /*
> +		  * DEREGISTER_DONE from a prior reset cycle
> +		  * Discard silently
> +		  */
> +		if (exec_queue_banned(q) || exec_queue_killed(q) ||
> +		    exec_queue_wedged(q)) {
> +			xe_gt_dbg(guc_to_gt(guc),
> +				  "DEREGISTER_DONE: Stale G2H guc_id=%d, state=0x%04x\n",
> +				  q->guc->id, atomic_read(&q->guc->state));
> +			return 0;
> +		}
> +
>  		xe_gt_err(guc_to_gt(guc),
>  			  "DEREGISTER_DONE: Unexpected engine state 0x%04x, guc_id=%d",
>  			  atomic_read(&q->guc->state), q->guc->id);
> -- 
> 2.43.0
>