Re: [PATCH v1 3/4] drm/xe/guc/ct: Queue G2H worker before flushing it in timeout paths

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

On Tue, Jul 21, 2026 at 09:46:53PM -0300, Tales A. Mendonça wrote:
> Timeout-recovery paths flush the G2H worker to pick up a response that
> may have been posted by the GuC but not yet processed:
> 
>  - guc_ct_send_recv() after the 1s wait for a G2H response fails
>  - the TLB invalidation backend's .flush() hook, called by
>    xe_tlb_inval_fence_timeout() before declaring a fence timed out
> 
> However flush_work() on a work item that is neither queued nor running
> is a no-op. If the GUC2HOST interrupt for the response was lost or

GUC2HOST messages getting lost are a different issue and would be
catastrophic for a variety of reasons. I'd like to dig into that if it
occurs on a POR platform (i.e., LNL+).

> coalesced, the G2H worker was never queued: the flush does not read the
> G2H CTB and the response sits there unprocessed until an unrelated G2H
> interrupt arrives. For TLB invalidations this results in
> 
>   TLB invalidation fence timeout, seqno=N recv=N-1
> 
> with the fence force-signalled with -ETIME even though the ack may
> already be present in the CTB. Observed sporadically on ARL-H under CPU
> load, always with recv == seqno - 1 and self-recovering on the next G2H
> interrupt, which is consistent with a lost wakeup rather than a
> GuC-side failure.
> 
> Add xe_guc_ct_flush_g2h(), which queues the worker before flushing it,
> guaranteeing the flush always drains the CTB, and use it in both
> timeout paths. A spurious worker run is safe: g2h_read() returns no
> data under fast_lock, and receive_g2h() copes with runtime-PM state and
> disabled CT communication.
> 
> After this change the fence-timeout error only fires when the ack is
> genuinely absent from the CTB, making the message a reliable indicator
> of a GuC-side stall.
> 
> Signed-off-by: Tales A. Mendonça <talesam@gmail.com>
> ---
>  drivers/gpu/drm/xe/xe_guc_ct.c        | 23 ++++++++++++++++++++++-
>  drivers/gpu/drm/xe/xe_guc_ct.h        |  1 +
>  drivers/gpu/drm/xe/xe_guc_tlb_inval.c |  2 +-
>  3 files changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_guc_ct.c b/drivers/gpu/drm/xe/xe_guc_ct.c
> index fe70c0fd85c..11a05c2b8c7 100644
> --- a/drivers/gpu/drm/xe/xe_guc_ct.c
> +++ b/drivers/gpu/drm/xe/xe_guc_ct.c
> @@ -1377,7 +1377,7 @@ static int guc_ct_send_recv(struct xe_guc_ct *ct, const u32 *action, u32 len,
>  wait_again:
>  	ret = wait_event_timeout(ct->g2h_fence_wq, READ_ONCE(g2h_fence.done), HZ);
>  	if (!ret) {
> -		LNL_FLUSH_WORK(&ct->g2h_worker);
> +		xe_guc_ct_flush_g2h(ct);
>  		if (READ_ONCE(g2h_fence.done)) {
>  			xe_gt_warn(gt, "G2H fence %u, action %04x, done\n",
>  				   g2h_fence.seqno, action[0]);
> @@ -2046,6 +2046,27 @@ static void g2h_worker_func(struct work_struct *w)
>  	receive_g2h(ct);
>  }
>  
> +/**
> + * xe_guc_ct_flush_g2h() - Force processing of pending G2H messages
> + * @ct: GuC CT object
> + *
> + * The GUC2HOST interrupt for a G2H message may be lost or coalesced. When
> + * that happens the G2H worker is never queued and flushing it is a no-op
> + * that does not read the G2H CTB, leaving messages the GuC has already
> + * posted unprocessed until the next interrupt arrives. Queue the worker
> + * before flushing it so the flush always drains the G2H CTB. A spurious
> + * worker run is safe: it returns without side effects if the CTB is empty
> + * or CT communication is disabled.
> + */
> +void xe_guc_ct_flush_g2h(struct xe_guc_ct *ct)
> +{
> +	if (!xe_guc_ct_enabled(ct))
> +		return;
> +
> +	queue_work(ct->g2h_wq, &ct->g2h_worker);

If the original queued work gets lost or is never submitted, then there
is a bug somewhere else in the kernel, as mentioned above.

As for the flush part, yes, it's possible that this could take a while
if the scheduler goes out to lunch, but it shouldn't happen with a
sufficiently large timeout. Have you tried making the GuC CT worker high
priority? We could also consider making the G2H handlers RT once those
land upstream.

In general, I'm quite unhappy with the existing LNL_FLUSH_WORK, and we
never determined the root cause of the original issue. In retrospect, we
probably should never have merged that code. Looking at it now, the LNL
flush code isn't even scoped to LNL, which makes this even worse than I
initially thought, as it can paper over bugs. I'm going to post a patch
to scope the existing workaround to LNL and earlier platforms (or maybe
just removing it completely) and see what happens in our CI over the
next several months before it makes its way into a kernel release.

With that said, the patch as proposed is a no from me. If you want to
pursue my suggestion of scoping this change to LNL and earlier
platforms, we can discuss that separately.

Matt

> +	flush_work(&ct->g2h_worker);
> +}
> +
>  static struct xe_guc_ct_snapshot *guc_ct_snapshot_alloc(struct xe_guc_ct *ct, bool atomic,
>  							bool want_ctb)
>  {
> diff --git a/drivers/gpu/drm/xe/xe_guc_ct.h b/drivers/gpu/drm/xe/xe_guc_ct.h
> index 767365a33de..4e0338953f1 100644
> --- a/drivers/gpu/drm/xe/xe_guc_ct.h
> +++ b/drivers/gpu/drm/xe/xe_guc_ct.h
> @@ -22,6 +22,7 @@ void xe_guc_ct_runtime_suspend(struct xe_guc_ct *ct);
>  void xe_guc_ct_stop(struct xe_guc_ct *ct);
>  void xe_guc_ct_flush_and_stop(struct xe_guc_ct *ct);
>  void xe_guc_ct_fast_path(struct xe_guc_ct *ct);
> +void xe_guc_ct_flush_g2h(struct xe_guc_ct *ct);
>  
>  struct xe_guc_ct_snapshot *xe_guc_ct_snapshot_capture(struct xe_guc_ct *ct);
>  void xe_guc_ct_snapshot_print(struct xe_guc_ct_snapshot *snapshot, struct drm_printer *p);
> diff --git a/drivers/gpu/drm/xe/xe_guc_tlb_inval.c b/drivers/gpu/drm/xe/xe_guc_tlb_inval.c
> index 046d0655122..91960dc4ba9 100644
> --- a/drivers/gpu/drm/xe/xe_guc_tlb_inval.c
> +++ b/drivers/gpu/drm/xe/xe_guc_tlb_inval.c
> @@ -327,7 +327,7 @@ static void tlb_inval_flush(struct xe_tlb_inval *tlb_inval)
>  {
>  	struct xe_guc *guc = tlb_inval->private;
>  
> -	LNL_FLUSH_WORK(&guc->ct.g2h_worker);
> +	xe_guc_ct_flush_g2h(&guc->ct);
>  }
>  
>  static long tlb_inval_timeout_delay(struct xe_tlb_inval *tlb_inval)
> -- 
> 2.55.0
>