Re: [PATCH v4 01/12] drm/xe/cper: Add CPER reporting entry point for xe_log

["Nilawar, Badal" <badal.nilawar@intel.com>]

On 09-10-2026 16:13, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] drm/xe: Silent telemetry loss when hardware SIGIDs lack exact RAS data payload
> --
>
> commit a8ad6417f3a91a193b5e9bf972e9a0e60ff46e5a
> Author: Badal Nilawar <badal.nilawar@intel.com>
>
> drm/xe/cper: Add CPER reporting entry point for xe_log
>
> Introduce xe_cper_emit() as the single entry point for emitting
> CPER records from xe_log. Hardware SIGIDs are routed to a hardware
> error helper, and software SIGIDs are left as TODO.
>
>> diff --git a/drivers/gpu/drm/xe/xe_cper.c b/drivers/gpu/drm/xe/xe_cper.c
>> new file mode 100644
>> index 0000000000000..c5caa0e8f496a
>> --- /dev/null
>> +++ b/drivers/gpu/drm/xe/xe_cper.c
> [ ... ]
>> +void xe_cper_emit(struct xe_device *xe, int cper_sev, enum xe_sigid sigid,
>> +		  u32 component, u32 location, const void *data, size_t len,
>> +		  struct va_format *vaf)
>> +{
>> +	if (!xe)
>> +		return;
>> +
>> +	if (is_hw_sigid(sigid)) {
>> +		if (!IS_ERR_OR_NULL(data) && len == sizeof(struct xe_ras_error_class))
>> +			xe_cper_emit_hw_err(xe, cper_sev, sigid, data);
>> +		return;
> [Severity: Medium]
> If a hardware SIGID log entry is missing the expected RAS data payload (e.g.,
> the data pointer is ERR_PTR, NULL, or the length is unexpected), does this
> unconditionally return and drop the telemetry?
>
> Since there is no fallback to emit a software CPER log or print a warning,
> it looks like this could result in a silent loss of hardware error data.

The log entry is not lost: __xe_log_emit() emits the dmesg entry before 
the CPER path, so the hardware error is always reported in dmesg.

A hardware CPER record is built from the firmware error counter, which 
requires the RAS error class; without it there is no meaningful record 
to emit.

All hardware SIGID users (xe_ras.c) pass the error class, so this path 
is only a guard against misuse.

Thanks,
Badal

>
>> +	}
>> +
>> +	/* TODO software CPER */
>> +}