Re: [RFC PATCH v4 1/5] drm/xe: Add SIG_IDs for RAS error logging

[Michal Wajdeczko <michal.wajdeczko@intel.com>]

On 6/30/2026 1:55 PM, Mallesh Koujalagi wrote:
> Add xe_sig_ids.h 

nit: the file itself does not define any labels, there is an enum there
maybe just say: "Add stable numeric labels for ...

> which defines a set of stable numeric labels
> for Xe error categories, called SIG_IDs.

hmm, quite unusual transformation from "categories" to "SIG"
maybe: "referred as XE_ERR_ID"
> 
> Each SIG_ID identifies which subsystem reported an error (e.g. probe,
> wedged, GT TDR, firmware). 

from the above list only firmware seems to be real 'subsystem'
maybe stick with the 'category' naming?


> It does not encode the full failure details,
> those come from errno and the free-form message:
> 
>   SIG_ID  -> which area failed
>   errno   -> what failed

hmm, errno is also more like a category of the failure, not a 'what'

>   message -> extra human-readable context
> 
> Also add a DOC: kernel-doc section covering design rationale, driver
> severity labels (FATAL/RECOVERABLE), usage guidelines, and the
> distinction between driver and hardware error paths.
> 
> Signed-off-by: Mallesh Koujalagi <mallesh.koujalagi@intel.com>
> ---
> v3:
> - Add HW SIG IDS details. (Riana)
> 
> v4:
> - Change commit message.
> - Update the document. (Riana)
> - Move documents to xe_sig_ids.h
> - Update Hardware error.
> - Remove example in Hardware error SIG_ID.
> - Make Fabric errors.
> - Add kernel doc for SIG_IDS. (Michal Wajdeczko)
> - Provide severity details.
> ---
>  Documentation/gpu/xe/index.rst      |   1 +
>  Documentation/gpu/xe/xe_sig_ids.rst |   8 ++
>  drivers/gpu/drm/xe/xe_sig_ids.h     | 124 ++++++++++++++++++++++++++++
>  3 files changed, 133 insertions(+)
>  create mode 100644 Documentation/gpu/xe/xe_sig_ids.rst
>  create mode 100644 drivers/gpu/drm/xe/xe_sig_ids.h
> 
> diff --git a/Documentation/gpu/xe/index.rst b/Documentation/gpu/xe/index.rst
> index 665c0e93601c..557877837348 100644
> --- a/Documentation/gpu/xe/index.rst
> +++ b/Documentation/gpu/xe/index.rst
> @@ -35,3 +35,4 @@ The display, or :ref:`drm-kms`, support for drm/xe is provided by
>     xe-drm-usage-stats.rst
>     xe_configfs
>     xe_gt_stats
> +   xe_sig_ids
> diff --git a/Documentation/gpu/xe/xe_sig_ids.rst b/Documentation/gpu/xe/xe_sig_ids.rst
> new file mode 100644
> index 000000000000..0e65d577927b
> --- /dev/null
> +++ b/Documentation/gpu/xe/xe_sig_ids.rst
> @@ -0,0 +1,8 @@
> +.. SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +
> +===============
> +SIG_ID Overview
> +===============
> +
> +.. kernel-doc:: drivers/gpu/drm/xe/xe_sig_ids.h
> +   :doc: SIG_ID Overview
> diff --git a/drivers/gpu/drm/xe/xe_sig_ids.h b/drivers/gpu/drm/xe/xe_sig_ids.h
> new file mode 100644
> index 000000000000..57e4037fd7d4
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/xe_sig_ids.h
> @@ -0,0 +1,124 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2026 Intel Corporation
> + */
> +
> +#ifndef _XE_SIG_IDS_H_
> +#define _XE_SIG_IDS_H_
> +
> +/**
> + * DOC: SIG_ID Overview

The Xe Error Signature Overview ?

> + *
> + * Signature ID (SIG_ID) is a stable numeric identifier (u16) for a defined
> + * Xe error category - it answers: "which subsystem reported the error?"
> + *
> + * SIG_ID format

just above you said that SIG_ID is u16
maybe this section should be: "Xe Structured Error Message Format" ?

> + * =============
> + *
> + * Xe error events are emitted as a single structured line using stable fields.

I guess we should first explain our plans how/where we want to use this new
error mechanism, how we will capture/expose those and define the mandatory
fields that we expect in each new 'structured-error'

then we could show how those fields will be exposed in dmesg

> + * Driver events use:
> + *
> + *   [xe-err] SIG_ID = <u16> Severity = <CPER_SEV_*>
> + *            Location = <device|tile/gt> Errno = <neg_errno>
> + *            Message = "<free-form text>"

this heavily deviates from what we currently have in xe via xe_err() or xe_gt_err()
without any benefit, just making the output less readable for human admins

> + *
> + * Hardware-originated RAS events use the same overall format, but typically
> + * omit Errno and Message and instead report the hardware-derived location /
> + * error-class information.
> + *
> + * Important
> + * =========
> + *
> + * SIG_ID identifies the reporting subsystem/category only. It does not encode
> + * the detailed failure reason. The detailed reason is carried separately by::
> + *
> + *   SIG_ID   -> which subsystem/category failed
> + *   Severity -> how serious the event is
> + *   Errno    -> what failed (driver events)
> + *   Message  -> human-readable context
> + *
> + * Example (driver event)
> + * ======================
> + *
> + *   [xe-err] SIG_ID = 6 Severity = CPER_SEV_RECOVERABLE

"[xe-err]" seems redundant, as the dmesg line will already have "xe" and "ERROR" tags

"SIG_ID = 6" will provide zero clue for the human admin what subsystem/category it is

"Severity = " seems redundant as it is followed by the "CPER_SEV_"

"CPER_SEV_" is also redundant as "RECOVERABLE" tag alone is fully recognizable


> + *   Location = tile0/gt0 Errno = -5

"Location = " do we really need such prefix ?

"device" - redundant as it doesn't bring any value
"tile0/gt0" - differs from "Tile0:" or "GT0:" used in regular xe_tile_err/xe_gt_err

"Errno = -5" this is a step back, as in regular errors we use friendly error names
printed using %pe format

> + *   Message = "Engine 'rcs0' hung; TDR triggered, engine reset succeeded"

"Message =" do we need such prefix ? everything after a errno can be treated as message


so this message could be rendered as:

xe 0000:00.0 [drm] *ERROR* \
	TDR RECOVERABLE Tile0: GT0: Engine 'rcs0' hung; TDR triggered, engine reset succeeded (-EIO)

without loosing any info and being close to our regular error messages that we have today

xe 0000:00.0 [drm] *ERROR* \
	Tile0: GT0: Engine 'rcs0' hung; TDR triggered, engine reset succeeded

> + *
> + * In the example
> + * ==============
> + *
> + *   SIG_ID 6   = XE GT/TDR category
> + *   RECOVERABLE = workload impacted, device still operational
> + *   -5         = Linux errno (-EIO)
> + *   Message    = extra context for triage
> + *
> + * Why SIG_ID exists?
> + * ==================

I guess this should be earlier in the DOC, as we should start with the
rationale, and have examples at the end

> + *
> + * The goal is to replace inconsistent ad-hoc error strings with a small
> + * set of stable, structured error events that are easier for operators
> + * and tools to understand. Each driver event carries a fixed SIG_ID with
> + * a severity determined by its error category, and structured output that
> + * can be consumed consistently across driver or firmware versions. It
> + * reduces guesswork during triage and allows machine parsing of important
> + * fault events.
> + *
> + * Driver severity labels
> + * ======================
> + *
> + * FATAL means the device cannot continue operation (e.g. probe failure,
> + * device wedged). RECOVERABLE means the driver encountered an error but
> + * may continue with degraded functionality.
> + *
> + * When to use the xe_ras_log helpers (see xe_ras_log.h)
> + * =====================================================
> + *
> + * Use them only for defined Xe error events that belong to the published
> + * error categories. These helpers are intended for important fault paths
> + * such as probe failure, wedged device, survivability mode, firmware
> + * failures, GT hang/TDR/reset, memory faults, and runtime IO/bus faults.
> + * The selected macro fixes the SIG_ID and severity for that category.

fixed ID/severity mapping seems wrong, as the same subsystem may have different
errors with different severity and different recovery actions

or our ID definition coverage/usage is wrong

> + *
> + * Do not use xe_ras_log helpers for all logs
> + * ==========================================
> + *
> + * These helpers are not a replacement for normal drm_info(), drm_dbg(),

what about xe_err() ?

nit: we have and use xe_info/xe_dbg
> + * tracing, or one-off diagnostics. They are for stable, structured error
> + * reporting only. Using them for ordinary logs would dilute the error
> + * stream and make operator-facing fault reporting noisy and less useful.

who makes the final call whether error should be reported using xe_err or xe_ras_log ?

or, actually all xe_err() shall be converted to xe_ras_log as 99% of them are
about the HW/FW/DRV errors anyway?

> + *
> + * Hardware errors
> + * ===============
> + *
> + * Hardware errors are hardware-reported RAS events and map to the
> + * XE_SIG_HW_* identifiers. They are reported through the hardware error
> + * path (e.g. CPER records), not through the driver xe_ras_log helpers.
> + *
> + * Unlike driver xe_ras_log helpers, hardware events do not have one fixed

while this is inconsistent, it actually shows that we shouldn't have a fixed
mapping between ID and SEV

> + * severity per SIG_ID. For example, a fabric event (XE_SIG_HW_FABRIC) may
> + * be reported as CPER_SEV_CORRECTED, CPER_SEV_FATAL, CPER_SEV_RECOVERABLE,
> + * or CPER_SEV_INFORMATIONAL, depending on what the hardware reported.
> + */
> +
> +/*
> + * Driver errors: SIG_IDs
> + */

please use correct kernel-doc formatting for all categories

/**
 * @XE_ERR_SIG_PROBE: failed probe error signature */
 */

> +#define XE_SIG_PROBE			1  /* FATAL: probe failed */
> +#define XE_SIG_WEDGED			2  /* FATAL: device wedged */
> +#define XE_SIG_SURVIVABILITY		3  /* FATAL: survivability mode */
> +#define XE_SIG_RUNTIME_FW		4  /* RECOVERABLE: GuC/HuC/UC/GSC */
> +#define XE_SIG_DEVICE_FW		5  /* RECOVERABLE: PCODE/CSC/System controller */
> +#define XE_SIG_GT_TDR			6  /* RECOVERABLE: engine hang / reset */
> +#define XE_SIG_MEM_FAULT		7  /* RECOVERABLE: VM bind, page fault, GTT */
> +#define XE_SIG_IO_BUS			8  /* RECOVERABLE: runtime PCIe/IOMMU/MMIO */
> +
> +/*
> + * Hardware errors: SIG_IDs

any reasons to have them grouped (driver vs hardware)
any reasons to have them contiguous (driver followed by hardware)

what's the plan for future additions of new categories?

having an option to define 64K unique IDs, or even more if u16 is just our limitation
why not pre-define each ID for known component? and make it really 'structured' ?
	
#define XE_SIG_CLASS_SOFTWARE		0x0	/* SIG = 0.*     */
#define XE_SIG_SUBCLASS_SOFTWARE_OOM	0x1	/* SIG = 0.1.0.0 */
#define XE_SIG_SUBCLASS_SOFTWARE_PROBE	0x2	/* SIG = 0.2.0.0 */
#define XE_SIG_SUBCLASS_SOFTWARE_WEDGE	0x3	/* SIG = 0.3.0.0 */
#define XE_SIG_SUBCLASS_SOFTWARE_OTHER	0xF	/* SIG = 0.F.0.0 */

#define XE_SIG_CLASS_HARDWARE		0x1	/* SIG = 1.*     */
#define XE_SIG_SUBCLASS_HARDWARE_BUS	0x1	/* SIG = 1.1.0.0 */
#define XE_SIG_SUBCLASS_HARDWARE_DEVICE	0x2	/* SIG = 1.2.0.0 */
#define XE_SIG_SUBCLASS_HARDWARE_TILE	0x3	/* SIG = 1.3.0.* */
#define XE_SIG_INSTANCE_HARDWARE_TILE(x) ..	/* SIG = 1.3.0.x */
#define XE_SIG_SUBCLASS_HARDWARE_GT	0x4	/* SIG = 1.4.*   */
#define XE_SIG_INSTANCE_HARDWARE_GT(x)	..	/* SIG = 1.4.y.x */
#define XE_SIG_SUBCLASS_HARDWARE_CORE	0x5	/* SIG = 1.5.*   */
#define XE_SIG_INSTANCE_HARDWARE_CORE(x) ..	/* SIG = 1.5.y.x */
#define XE_SIG_SUBCLASS_HARDWARE_FABRIC	0x6	/* SIG = 1.6.*   */
#define XE_SIG_SUBCLASS_HARDWARE_OTHER	0xF	/* SIG = 1.F.0.0 */

#define XE_SIG_CLASS_FIRMWARE		0x2	/* SIG = 2.*     */
#define XE_SIG_SUBCLASS_FIRMWARE_GUC	0x1	/* SIG = 2.1.*   */
#define XE_SIG_INSTANCE_FIRMWARE_GUC(x)	..	/* SIG = 2.1.y.x */
#define XE_SIG_SUBCLASS_FIRMWARE_HUC	0x2	/* SIG = 2.2.*   */
#define XE_SIG_INSTANCE_FIRMWARE_HUC(x)	..	/* SIG = 2.2.y.x */
#define XE_SIG_SUBCLASS_FIRMWARE_GSC	0x3	/* SIG = 2.3.*   */
#define XE_SIG_SUBCLASS_FIRMWARE_OTHER	0xF	/* SIG = 2.F.0.0 */

#define XE_SIG_CLASS_DEVICE_FIRMWARE	0x3	/* SIG = 3.*     */
..

the number of IDs will still be manageable IMO

to build final SIG_ID we can have some macros like:

#define MAKE_XE_SIG_ID(_CLASS, _SUB, _INST) /* SIG = class.sub.instance  */ \
	FIELD_PREP(0xF000, XE_SIG_CLASS_##_CLASS) |\
	FIELD_PREP(0x0F00, XE_SIG_SUBCLASS_##_CLASS##_##_SUB) | \
	FIELD_PREP(0x00FF, _INST)

and use it as:

	xe_err_fatal(gt, SOFTWARE, PROBE, "message...");
	xe_gt_err_recoverable(gt, FIRMWARE, GUC, "message...");


> + */
> +#define XE_SIG_HW_DEVICE_MEMORY		9  /* Device memory errors */
> +#define XE_SIG_HW_CORE_COMPUTE		10 /* Compute/shader core errors */
> +#define XE_SIG_HW_PCIE			11 /* PCIe interface errors */
> +#define XE_SIG_HW_FABRIC		12 /* Fabric errors */
> +#define XE_SIG_HW_SOC_INTERNAL		13 /* SoC-internal errors */
> +
> +#endif /* _XE_SIG_IDS_H_ */