Re: [PATCH v2 00/22] drm/xe: Add structured SIGID error logging infrastructure

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Thu, Jul 30, 2026 at 03:50:41PM +0200, Raag Jadav wrote:
> On Wed, Jul 29, 2026 at 08:58:08PM -0400, Rodrigo Vivi wrote:
> > On Wed, Jul 29, 2026 at 11:31:46AM +0300, Jani Nikula wrote:
> > > On Tue, 28 Jul 2026, Michal Wajdeczko <michal.wajdeczko@intel.com> wrote:
> > > > Today the driver reports faults with ad-hoc drm_err()/xe_gt_err()
> > > > strings that have no stable shape. That is readable for a human, but it
> > > > gives fleet tooling nothing durable to match on: the wording changes
> > > > between releases, lines can be rate-limited or dropped under an error
> > > > storm, and there is no consistent way to ask "which recognised fault
> > > > just happened?".
> > > >
> > > > Introduce a signature identifier (SIGID): a small, stable integer that
> > > > names one recognised Xe fault situation and serves as the primary handle
> > > > for triage. A SIGID maps, through published end-user documentation, to a
> > > > description and a recommended action; the driver only has to emit the
> > > > right SIGID next to the usual human-readable text.
> > > >
> > > > Design decisions:
> > > >
> > > >  - Software-emitted signatures only. This header enumerates just the
> > > >    situations the driver detects and reports itself. Signatures that
> > > >    originate in firmware or hardware are identified by those layers (via
> > > >    their own records/counters) and are logged as received -- minting a
> > > >    driver-side id for them would duplicate an id the reporting layer
> > > >    already owns.
> > > >
> > > >  - Flat catalogue, chosen per report site. Each site emits the single
> > > >    most specific situation for that site, so a multi-layer failure
> > > >    produces a chain of reports rather than one ambiguous classification
> > > >    (e.g. a failed GT reset reports GT_TDR and then WEDGED). A site that
> > > >    matches no defined situation keeps using ordinary xe_err() /
> > > >    xe_gt_err() rather than forcing a wrong id.
> > > >
> > > >  - Stable numbering. A single flat list numbered sequentially from 1, in
> > > >    introduction order. Values are only ever appended, never renumbered
> > > >    or reused.
> > > >
> > > >  - First-order action. Each SIGID carries a coarse, in-tree resolution
> > > >    bucket (COLLECT / RETRY / UPDATE / RECOVER) so it is actionable
> > > >    without an external reference, and so every new id must declare what
> > > >    to do about it.
> > > >
> > > >  - Severity is decoupled from the SIGID and chosen at the call site via
> > > >    xe_ras_log_fatal() / _recoverable() / _info(); the same situation can
> > > >    be reported at different severities depending on the instance.
> > > >
> > > >  - dmesg stays close to a normal xe error line by reusing xe_err() /
> > > >    xe_gt_err() (and their Tile/GT decoration); the only stable,
> > > >    machine-matchable token added is SIGID=<n>. dmesg is not an ABI --
> > > >    the durable machine record is the CPER carrying the same SIGID (a
> > > >    planned follow-up, left as a TODO).
> > > >
> > > > Wire up a representative site for each software signature so the set is
> > > > exercised rather than merely declared:
> > > >
> > > >  - PROBE: xe workqueue allocation failure during early init
> > > >  - WEDGED: xe_device_declare_wedged() (drop redundant "CRITICAL" + BDF)
> > > >  - SURVIVABILITY: entering boot survivability mode
> > > >  - RUNTIME_FW: GuC mmio request failure
> > > >  - DEVICE_FW: PCODE mailbox failure
> > > >  - GT_TDR: GT reset failure (which then chains into a WEDGED report)
> > > >  - MEM_FAULT: page-fault queue overflow
> > > >  - IO_BUS: PCI re-enable failure after a bus reset
> > > 
> > > The idea is not new, see for example [1] and [2]. I'm not sure if that
> > > was ever merged, though. Would be good to know what came of it, and why.
> > > 
> > > There's also the printk index support already in the kernel [3]. Did you
> > > look into that? If not, please do. If yes, please iterate why that can't
> > > be used, or extended, and a local mechanism is required.
> > 
> > Well, I do agree with your feeling here. When this request first came to us,
> > it was framed similarly to the two older proposals you referenced.
> > I pushed back making it very clear that our kernel log is not an ABI and
> > it will never be... that our debug messages are for our own developers to
> > consume and edit as the code evolves. So, I refused and blocked any
> > attempt to modify all the messages and force them to follow this format.
> > 
> > So, this evolved a bit since. I hope. We have agreed that the ABI itself
> > is not the dmesg, but the CPER that is emitted on tracefs... that's the
> > ultimate goal. The message here is just an extra helper and it brings
> > both our CPER/tracefs and the dmesg helper. So, the printk index cannot
> > be used here. It has no mechanism to influence or annotate what goes
> > into CPER records. And even if we modified it, its volatile number
> > doesn't work with the CPER goals.
> > 
> > The goal is to have this SIGID mostly identifying FW/HW stuff and a few
> > of the key points that could help providing a guidance/map to a recovery
> > path. With an intentionally narrow scope.
> 
> So why not stick to a specific interface instead kernel logs having to
> bare the burden here?

Because dmesg is the first thing people look to understand the next step.
Just a fast first stop shop.

> CI results are already full of noise,

It is not extra noise. This was one of the things that I blocked on
the first design. There shouldn't be duplications nor multi-line
debugs.

In a matter of fact, if you look the examples that Michal already has
you will see that it looks cleaner than before.

> so it's a
> bit unclear how this makes developers' life any easier.

The goal is not to make developer life easier, but I explicit blocked
attempts to make developers life harder.

> I now have to
> figure out all the macros before reaching to the point of failure :)

You don't.

Developers can continue using all the other existing error functions for any case.
If you are not sure you need this sigid it is likely because you don't need.
So no trouble in wasting time trying to map or invent another id for a new case.

> 
> Raag