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 08:28:54PM -0300, Tales A. Mendonça wrote:
> Hi Matt,
> 
> Thanks for the detailed look - and you're right about the framing.
> Later instrumentation on this system (sampling G2H CTB head/tail in
> the TDR *before* the flush) refuted the lost-wakeup theory from my
> commit message: in every observed timeout (30+ events across two ARL
> machines, 7d51 and 7dd1) the CTB is empty at timeout time (head ==

What about the H2G CT?

> tail, 0 pending dwords), and the ack lands 5-475ms (median ~22ms)
> later via the normal path. So the flush recovers nothing - the ack
> simply hasn't been posted yet by the GuC at that point. The stall
> appears to be firmware-side (GuC 70.53.0); I'm preparing a report with
> the full data.
> 
> That also means I agree with dropping this patch, and the data is
> consistent with your suspicion that LNL_FLUSH_WORK papers over nothing
> here.
> 
> I'll test your series from
> https://patchwork.freedesktop.org/series/170939/ on both ARL machines
> (daily-driver workloads that reproduce the timeouts at ~1.5/h) and
> report back.
> 

Another thing to look at is getting a devcoredump on a TLB invalidation
timeout. I thought we supported xe_devcoredump() without a queue or job,
but I guess we never got around to doing that. If we had the CT state
and GuC log, that would at least give us some hints about what is going
on. It would probably be worthwhile to add something like
xe_devcoredump_gt(gt) to Xe to help debug hangs that are unrelated to
queues or jobs.

That said, as Matt R. mentioned in an earlier patch, we don't officially
support ARL on Xe, so our bandwidth to help here is limited. In other
words, if you're hitting a platform-unsupported-specific bug, we may or
may not be able to investigate it.

Matt

>  Thanks,
>  Tales
> 
> Em qua., 22 de jul. de 2026 às 19:03, Matthew Brost
> <matthew.brost@intel.com> escreveu:
> >
> > 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
> > > >
> 
> 
> 
> -- 
> Com os cumprimentos,
> 
> Tales A. Mendonça
> talesam.org
> communitybig.org