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

[Jani Nikula <jani.nikula@linux.intel.com>]

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.

FWIW, I think merging driver local mechanisms for what should really be
generic kernel stuff usually ends up being a maintenance PITA. It's easy
in the beginning, a burden in the end, and even harder to migrate to
generic mechanisms. We've seen this over and over in i915, even for
superficially small things. Part of the reason for xe existing in the
first place is to use common stuff instead of reinventing the weels.

BR,
Jani.


[1] https://lwn.net/Articles/464276/
[2] https://lwn.net/Articles/492125/
[3] https://lkml.org/lkml/2021/6/15/1053

>
> Signed-off-by: Mallesh Koujalagi <mallesh.koujalagi@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> Cc: Matthew Brost <matthew.brost@intel.com>
> Cc: Aravind Iddamsetty <aravind.iddamsetty@intel.com>
> Cc: Riana Tauro <riana.tauro@intel.com>
> Cc: Raag Jadav <raag.jadav@intel.com>
> Cc: Badal Nilawar <badal.nilawar@intel.com>
>
> v1: https://patchwork.freedesktop.org/series/171022/#rev1
> v2: component/location dmesg decorations moved away from macros
>     HW component identifiers related to RAS/FW enum values
>     CORRECTED severity rendered as an error
>     more support for const pointers
>     log 'probe blocked' as INFO
>     fix kunit device setup
>     more tests
>
> Mallesh Koujalagi (1):
>   drm/xe/log: Add structured SIGID error logging infrastructure
>
> Michal Wajdeczko (21):
>   drm/xe: Introduce xe_any helpers
>   drm/xe/log: Introduce structured component/location identifiers
>   drm/xe/log: Add component/location decorations to dmesg
>   drm/xe/log: Add SIGID log helpers for severity
>   drm/xe/log: Add SIGID log helpers for location
>   drm/xe/log: Add SIGID log helpers for location & severity
>   drm/xe/log: Add SIGID log helpers for components
>   drm/xe/log: Add SIGID log helpers for errno-only
>   drm/xe/log: Add hardware error signatures
>   drm/xe/log: Extend components list with hardware items
>   drm/xe/ras: Check RAS and LOG component definitions
>   drm/xe/kunit: Setup driver data in the test device
>   drm/xe/tests: Add Kunit tests for xe_log
>   drm/xe: Report 'probe blocked' error using SIGID
>   drm/xe: Report 'device wedged' errors using SIGID
>   drm/xe: Report 'Survivability Mode' errors using SIGID
>   drm/xe/guc: Report 'GuC mmio' errors using SIGID
>   drm/xe/pcode: Report 'Mailbox failed' error using SIGID
>   drm/xe/gt: Report 'reset failed' errors using SIGID
>   drm/xe/gt: Report 'pagefault' errors using SIGID
>   drm/xe/pci: Report 'cannot re-enable' error using SIGID
>
>  Documentation/gpu/xe/index.rst              |   1 +
>  Documentation/gpu/xe/xe_sigid.rst           |  14 +
>  drivers/gpu/drm/xe/Makefile                 |   1 +
>  drivers/gpu/drm/xe/abi/xe_log_abi.h         | 197 ++++++++
>  drivers/gpu/drm/xe/abi/xe_sigid_abi.h       | 195 ++++++++
>  drivers/gpu/drm/xe/tests/xe_kunit_helpers.c |   4 +
>  drivers/gpu/drm/xe/tests/xe_log_kunit.c     | 480 ++++++++++++++++++++
>  drivers/gpu/drm/xe/xe_any.h                 | 112 +++++
>  drivers/gpu/drm/xe/xe_device.c              |  16 +-
>  drivers/gpu/drm/xe/xe_gt.c                  |   7 +-
>  drivers/gpu/drm/xe/xe_guc.c                 |  18 +-
>  drivers/gpu/drm/xe/xe_log.c                 | 212 +++++++++
>  drivers/gpu/drm/xe/xe_log.h                 | 158 +++++++
>  drivers/gpu/drm/xe/xe_pagefault.c           |  10 +-
>  drivers/gpu/drm/xe/xe_pci.c                 |   5 +-
>  drivers/gpu/drm/xe/xe_pci_error.c           |   7 +-
>  drivers/gpu/drm/xe/xe_pcode.c               |   5 +-
>  drivers/gpu/drm/xe/xe_ras.c                 |  11 +
>  drivers/gpu/drm/xe/xe_survivability_mode.c  |  24 +-
>  19 files changed, 1438 insertions(+), 39 deletions(-)
>  create mode 100644 Documentation/gpu/xe/xe_sigid.rst
>  create mode 100644 drivers/gpu/drm/xe/abi/xe_log_abi.h
>  create mode 100644 drivers/gpu/drm/xe/abi/xe_sigid_abi.h
>  create mode 100644 drivers/gpu/drm/xe/tests/xe_log_kunit.c
>  create mode 100644 drivers/gpu/drm/xe/xe_any.h
>  create mode 100644 drivers/gpu/drm/xe/xe_log.c
>  create mode 100644 drivers/gpu/drm/xe/xe_log.h

-- 
Jani Nikula, Intel