Re: [PATCH 7/7] drm/xe: Check for GuC responses on disabling scheduling

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

On Thu, Oct 02, 2025 at 11:04:44PM +0000, Stuart Summers wrote:
> In the event the GuC becomes unresponsive during a scheduling
> disable event, we still want the driver to be able to recover.
> This patch follows the same methodology we already have in place
> for TLB invalidation requests, where we send a request to GuC
> and wait for that invalidation done response. If the response
> doesn't come back in time we then at least print a message
> indicating the invalidation failed for some reason.
> 
> In this case, we send the schedule disable and the expectation
> is that GuC will respond with a schedule done response. The KMD
> then catches that response and in turn sends a context deregistration
> response. So in the event GuC becomes unresponsive after we send
> the schedule disable, we actually have two g2h responses that
> have been reserved but never received.
> 
> To handle this, make sure the pending disable event in the
> exec queue gets cleared (i.e. we received that response from
> GuC). If it doesn't in a reasonable amount of time, assume
> GuC is dead: ban the exec queue, queue up a GT reset, and
> manually call the schedule done handler. Then in the schedule
> done handler, in turn, check whether the context had been
> banned. If so, manually call the deregistration done handler
> to ensure all resources related to that exec queue get
> cleaned up properly. Without this, if the device becomes
> wedged after an exec queue has been created, the attached
> resources like the LRC will not get feed properly resulting
> in a memory leak.
> 
> Signed-off-by: Stuart Summers <stuart.summers@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_guc_submit.c | 23 ++++++++++++++++++++++-
>  1 file changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c b/drivers/gpu/drm/xe/xe_guc_submit.c
> index 45b72bebfc63..a177d87c8524 100644
> --- a/drivers/gpu/drm/xe/xe_guc_submit.c
> +++ b/drivers/gpu/drm/xe/xe_guc_submit.c
> @@ -939,6 +939,9 @@ int xe_guc_read_stopped(struct xe_guc *guc)
>  		GUC_CONTEXT_##enable_disable,				\
>  	}
>  
> +static void handle_sched_done(struct xe_guc *guc, struct xe_exec_queue *q,
> +			      u32 runnable_state);
> +
>  static void disable_scheduling_deregister(struct xe_guc *guc,
>  					  struct xe_exec_queue *q)
>  {
> @@ -974,6 +977,17 @@ static void disable_scheduling_deregister(struct xe_guc *guc,
>  	xe_guc_ct_send(&guc->ct, action, ARRAY_SIZE(action),
>  		       G2H_LEN_DW_SCHED_CONTEXT_MODE_SET +
>  		       G2H_LEN_DW_DEREGISTER_CONTEXT, 2);
> +
> +	ret = wait_event_timeout(guc->ct.wq,
> +				 !exec_queue_pending_disable(q) ||
> +				 xe_guc_read_stopped(guc),
> +				 HZ * 5);

This doesn't look right. Deregister is designed to be fully async. If
this flow stops working for whatever reason the GuC is dead and
eventually somewhere in driver will detect this and trigger a GT reset
which is cleanup all lost H2G.

> +	if (!ret || xe_guc_read_stopped(guc)) {
> +		xe_gt_warn(guc_to_gt(guc), "Schedule disable failed to respond");
> +		set_exec_queue_banned(q);
> +		handle_sched_done(guc, q, 0);
> +		xe_gt_reset_async(q->gt);
> +	}
>  }
>  
>  static void xe_guc_exec_queue_trigger_cleanup(struct xe_exec_queue *q)
> @@ -2117,6 +2131,8 @@ g2h_exec_queue_lookup(struct xe_guc *guc, u32 guc_id)
>  	return q;
>  }
>  
> +static void handle_deregister_done(struct xe_guc *guc, struct xe_exec_queue *q);
> +
>  static void deregister_exec_queue(struct xe_guc *guc, struct xe_exec_queue *q)
>  {
>  	u32 action[] = {
> @@ -2131,7 +2147,12 @@ static void deregister_exec_queue(struct xe_guc *guc, struct xe_exec_queue *q)
>  
>  	trace_xe_exec_queue_deregister(q);
>  
> -	xe_guc_ct_send_g2h_handler(&guc->ct, action, ARRAY_SIZE(action));
> +	if (exec_queue_banned(q)) {
> +		handle_deregister_done(guc, q);

This would leave the GuC with reference to guc_id and subsequent reuse
of the guc_id (i.e., next register) will fall.

Matt

> +	} else {
> +		xe_guc_ct_send_g2h_handler(&guc->ct, action,
> +					   ARRAY_SIZE(action));
> +	}
>  }
>  
>  static void handle_sched_done(struct xe_guc *guc, struct xe_exec_queue *q,
> -- 
> 2.34.1
>