Re: [RFC PATCH v4 2/5] drm/xe: Add RAS logging helpers

["Mallesh, Koujalagi" <mallesh.koujalagi@intel.com>]

On 03-07-2026 04:45 pm, Tauro, Riana wrote:
>
> On 30-06-2026 17:25, Mallesh Koujalagi wrote:
>> Add xe_ras_log_(*) macros to report Xe driver errors in a
>> consistent, structured format.
>>
>> Exposes one macro per error category (probe, wedged,
>> survivability, firmware, GT TDR, memory fault, IO bus). Each macro
>> hard-codes the correct SIG_ID and severity for that category, so
>> callers only need to pass the device or GT handle, errno, and message.
>>
>> Signed-off-by: Mallesh Koujalagi <mallesh.koujalagi@intel.com>
>> ---
>> v3:
>> - Refer "Tile%u" and "GT%u" strings. (Michal Wajdeczko)
>> - Remov xe_cper_severity_str(). (Michal Wajdeczko/Riana)
>> - Declare __xe_ras_log() function to xe_ras_log.h. (Michal Wajdeczko)
>> - Make macro function properly.
>> - Remove *_FIRST and *_LAST macro. (Michal Wajdeczko/Riana)
>> - Add sig id documents. (Riana)
>> - Change macro function same prefix as the file.
>> - Handle __xe_ras_log() function with variable format.
>>
>> v4:
>> - Handle CONFIG_UEFI_CPER properly.
>> - Add CPER_SEV_RECOVERABLE in __xe_ras_log().
>> - Add xe_ras_log_runtime_fw() and xe_ras_log_device_fw() macros.
>> - Update commit message.
>> - Remove document. (Riana)
>> ---
>>   drivers/gpu/drm/xe/Makefile     |  1 +
>>   drivers/gpu/drm/xe/xe_ras_log.c | 65 +++++++++++++++++++++++++++++++++
>>   drivers/gpu/drm/xe/xe_ras_log.h | 62 +++++++++++++++++++++++++++++++
>>   3 files changed, 128 insertions(+)
>>   create mode 100644 drivers/gpu/drm/xe/xe_ras_log.c
>>   create mode 100644 drivers/gpu/drm/xe/xe_ras_log.h
>>
>> diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
>> index e5a04253e73b..cac19c21be08 100644
>> --- a/drivers/gpu/drm/xe/Makefile
>> +++ b/drivers/gpu/drm/xe/Makefile
>> @@ -114,6 +114,7 @@ xe-y += xe_bb.o \
>>       xe_query.o \
>>       xe_range_fence.o \
>>       xe_ras.o \
>> +    xe_ras_log.o \
>>       xe_reg_sr.o \
>>       xe_reg_whitelist.o \
>>       xe_ring_ops.o \
>> diff --git a/drivers/gpu/drm/xe/xe_ras_log.c 
>> b/drivers/gpu/drm/xe/xe_ras_log.c
>> new file mode 100644
>> index 000000000000..928fa4f45cdc
>> --- /dev/null
>> +++ b/drivers/gpu/drm/xe/xe_ras_log.c
>> @@ -0,0 +1,65 @@
>> +// SPDX-License-Identifier: MIT
>> +/*
>> + * Copyright © 2026 Intel Corporation
>> + */
>> +
>> +#include <drm/drm_print.h>
>> +
>> +#include "xe_device.h"
>> +#include "xe_gt.h"
>> +#include "xe_ras_log.h"
>> +
>> +/**
>> + * __xe_ras_log - Emit a structured RAS log entry
>> + * @xe: xe device instance
>> + * @gt: GT instance where the error occurred, or NULL if device-wide
>> + * @sig_id: signature ID from xe_sig_ids.h identifying the error class
>> + * @cper_sev: CPER severity (one of CPER_SEV_FATAL, 
>> CPER_SEV_RECOVERABLE, etc.)
>> + * @errno_val: negative errno describing the error condition
>> + * @fmt: printf-style format string
>> + * @...: format arguments
>> + *
>> + * Formats the message and emits a kernel log line via drm_err() for 
>> fatal
>> + * events or drm_warn() for all others. CPER record generation and 
>> hex dump
>> + * are planned as follow-ups.
>> + *
>> + * Format:
>> + *   [xe-err] SIG_ID = <id> Severity = <sev> Location = <loc> Errno 
>> = <n> Message = "<msg>"
>> + */
>> +__printf(6, 7)
>> +void __xe_ras_log(struct xe_device *xe, struct xe_gt *gt,
>> +          u16 sig_id, u32 cper_sev, int errno_val,
>> +          const char *fmt, ...)
>> +{
>> +    char loc[32];
>> +    struct va_format vaf;
>> +    va_list ap;
>> +
>> +    if (gt)
>> +        snprintf(loc, sizeof(loc), "tile%u/gt%u",
>> +             gt->tile->id, gt->info.id);
>> +    else
>> +        snprintf(loc, sizeof(loc), "device");
>> +
>> +    va_start(ap, fmt);
>> +    vaf.fmt = fmt;
>> +    vaf.va = &ap;
>> +
>> +    if (cper_sev == CPER_SEV_FATAL || cper_sev == CPER_SEV_RECOVERABLE)
>> +        drm_err(&xe->drm,
>> +            "[xe-err] SIG_ID = %u Severity = %s Location = %s Errno 
>> = %d Message = \"%pV\"",
>> +            sig_id,
>> +            IS_ENABLED(CONFIG_UEFI_CPER) ? 
>> cper_severity_str(cper_sev) : "unknown",
>> +            loc, errno_val, &vaf);
>> +    else
>> +        drm_warn(&xe->drm,
>> +             "[xe-err] SIG_ID = %u Severity = %s Location = %s Errno 
>> = %d Message = \"%pV\"",
>> +             sig_id,
>> +             IS_ENABLED(CONFIG_UEFI_CPER) ? 
>> cper_severity_str(cper_sev) : "unknown",
>> +             loc, errno_val, &vaf);
>
>
> There were few comments regarding this from both me and Michal from 
> previous rev that were agreed by you but not fixed.
> Also let's close comments from previous rev before sending new revisions.
>
I checked with the Arch team. Even though this is logged using drm_warn(),
we still want the output to be prefixed with xe_err because it is used
for error handling paths. These messages are generated when correctable
or other hardware errors occur, so having an explicit xe_err tag helps
identify them as error-related events in the logs.

Thanks,
-/Mallesh

> Thanks
> Riana
>
>> +
>> +    va_end(ap);
>> +
>> +    /* TODO: Add CPER record driver handler */
>> +    /* TODO: Add RAS dump cper hex handler */
>> +}
>> diff --git a/drivers/gpu/drm/xe/xe_ras_log.h 
>> b/drivers/gpu/drm/xe/xe_ras_log.h
>> new file mode 100644
>> index 000000000000..3f94e6747e86
>> --- /dev/null
>> +++ b/drivers/gpu/drm/xe/xe_ras_log.h
>> @@ -0,0 +1,62 @@
>> +/* SPDX-License-Identifier: MIT */
>> +/*
>> + * Copyright © 2026 Intel Corporation
>> + */
>> +
>> +#ifndef _XE_RAS_LOG_H_
>> +#define _XE_RAS_LOG_H_
>> +
>> +#include <linux/cper.h>
>> +
>> +#include "xe_sig_ids.h"
>> +
>> +struct xe_device;
>> +struct xe_gt;
>> +
>> +/*
>> + * Common backend helper
>> + */
>> +__printf(6, 7)
>> +void __xe_ras_log(struct xe_device *xe, struct xe_gt *gt,
>> +          u16 sig_id, u32 cper_sev, int errno_val,
>> +          const char *fmt, ...);
>> +
>> +/*
>> + * Driver error reporting macros
>> + */
>> +
>> +/* FATAL */
>> +#define xe_ras_log_probe(xe, errno, fmt, ...) \
>> +    __xe_ras_log((xe), NULL, XE_SIG_PROBE, CPER_SEV_FATAL, \
>> +             (errno), fmt, ##__VA_ARGS__)
>> +
>> +#define xe_ras_log_wedged(xe, errno, fmt, ...) \
>> +    __xe_ras_log((xe), NULL, XE_SIG_WEDGED, CPER_SEV_FATAL, \
>> +             (errno), fmt, ##__VA_ARGS__)
>> +
>> +#define xe_ras_log_survivability(xe, errno, fmt, ...) \
>> +    __xe_ras_log((xe), NULL, XE_SIG_SURVIVABILITY, CPER_SEV_FATAL, \
>> +             (errno), fmt, ##__VA_ARGS__)
>> +
>> +/* RECOVERABLE */
>> +#define xe_ras_log_runtime_fw(xe, gt, errno, fmt, ...) \
>> +    __xe_ras_log((xe), (gt), XE_SIG_RUNTIME_FW, CPER_SEV_RECOVERABLE, \
>> +             (errno), fmt, ##__VA_ARGS__)
>> +
>> +#define xe_ras_log_device_fw(xe, gt, errno, fmt, ...) \
>> +    __xe_ras_log((xe), (gt), XE_SIG_DEVICE_FW, CPER_SEV_RECOVERABLE, \
>> +             (errno), fmt, ##__VA_ARGS__)
>> +
>> +#define xe_ras_log_gt_tdr(xe, gt, errno, fmt, ...) \
>> +    __xe_ras_log((xe), (gt), XE_SIG_GT_TDR, CPER_SEV_RECOVERABLE, \
>> +             (errno), fmt, ##__VA_ARGS__)
>> +
>> +#define xe_ras_log_mem_fault(xe, gt, errno, fmt, ...) \
>> +    __xe_ras_log((xe), (gt), XE_SIG_MEM_FAULT, CPER_SEV_RECOVERABLE, \
>> +             (errno), fmt, ##__VA_ARGS__)
>> +
>> +#define xe_ras_log_io_bus(xe, errno, fmt, ...) \
>> +    __xe_ras_log((xe), NULL, XE_SIG_IO_BUS, CPER_SEV_RECOVERABLE, \
>> +             (errno), fmt, ##__VA_ARGS__)
>> +
>> +#endif /* _XE_RAS_LOG_H_ */