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

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 <[email protected]> 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? CI results are already full of noise, so it's a
bit unclear how this makes developers' life any easier. I now have to
figure out all the macros before reaching to the point of failure :)

Raag