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 Wed, Jul 22, 2026 at 02:10:18PM -0700, Matthew Brost wrote:
> 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.

This is roughly what I had in mind:

https://patchwork.freedesktop.org/series/170939/

Compile tested only, so if you could give it try and see what pops up, I
think that would be helpful. Fell free to use the code in anyway and
give me feedback.

Matt

> 
> 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
> >