Re: [PATCH v2 03/11] drm/xe/cper: Add CPER structures and trace event

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

On 25-08-2026 23:21, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Medium] CPER structures use native integer types and C bitfields for a standardized external binary format, causing ABI corruption on non-little-endian architectures.
> - [Medium] Tracepoint uses `%pU` to print `guid_t` fields, incorrectly formatting them as big-endian.
> --
>
> commit ad510a6c4b7008ac7d33a168936259e5f8400da7
> Author: Badal Nilawar <badal.nilawar@intel.com>
>
> drm/xe/cper: Add CPER structures and trace event
>
> Define packed data structures and Intel-specific GUID macros needed
> to build Intel GPU CPER (Common Platform Error Record) non-standard
> records.
>
>> diff --git a/drivers/gpu/drm/xe/xe_cper_types.h b/drivers/gpu/drm/xe/xe_cper_types.h
>> new file mode 100644
>> index 0000000000000..82167ea4eb161
>> --- /dev/null
>> +++ b/drivers/gpu/drm/xe/xe_cper_types.h
> [ ... ]
>> +struct xe_cper_sec_intel_err_hdr {
>> +	/** @error_class: Error classification (type, component, location, cause) */
>> +	union {
>> +		struct {
>> +			/** @error_class.error_type: RAS error severity */
>> +			u8  error_type;
>> +			/** @error_class.error_component: IP block that raised the error */
>> +			u8  error_component;
>> +			/** @error_class.tile: Tile number */
>> +			u8  tile;
>> +			/** @error_class.instance: Instance within the tile */
>> +			u32 instance;
>> +			/** @error_class.cause: Error cause code */
>> +			u32 cause;
>> +			/** @error_class.reserved: Reserved, must be zero */
>> +			u8  reserved;
>> +		} error_class;
>> +		/** @class: Raw byte view of the error class */
>> +		u8 class[12];
>> +	};
>> +	/** @first_timestamp: Timestamp of the first occurrence of this error class */
>> +	u64 first_timestamp;
> [Severity: Medium]
> Can this native u64 type break the CPER binary layout on big-endian
> architectures?
This timestamp is reported by firmware, represents ticks since boot.
>
> The CPER standard dictates a strict binary layout and requires multi-byte fields
> to be little-endian. Using architecture-dependent native integer types like u64
> causes the struct's binary representation to vary based on host endianness,
> which can produce corrupted CPER records on big-endian architectures.
>
> Should this use explicitly sized and endian-specific types like __le64?
>
>> +	/** @sig_id: Aggregated error class SIG ID; set to U32_MAX if unknown */
>> +	u32 sig_id;
>> +	/** @error_count: Number of times this error has been observed */
>> +	u32 error_count;
>> +	/** @valid_bits: Bitmask indicating which header fields are populated */
>> +	union {
>> +		struct {
>> +			/** @valid_bits.location: @error_class field is valid */
>> +			u16 location		: 1;
> [Severity: Medium]
> Will this compiler-dependent bitfield cause ABI corruption?
>
> Since bitfield layout is compiler- and architecture-dependent, it breaks the
> exact bit-level binary layout required for the CPER standard.
>
> Should this be handled with standard bitwise operations instead?
This will be taken care in next rev
>
> [ ... ]
>> diff --git a/drivers/gpu/drm/xe/xe_trace_cper.h b/drivers/gpu/drm/xe/xe_trace_cper.h
>> new file mode 100644
>> index 0000000000000..6d2dbf5048884
>> --- /dev/null
>> +++ b/drivers/gpu/drm/xe/xe_trace_cper.h
> [ ... ]
>> +	TP_printk("dev=%s severity=%d platform_id=%pU fru_id=%pU "
> [Severity: Medium]
> Does this tracepoint format guid_t fields incorrectly?
>
> The printk format specifier %pU defaults to %pUb, which formats UUIDs in
> big-endian byte order (RFC 4122). However, guid_t represents little-endian
> UUIDs.
>
> Using %pU instead of %pUl causes the first three components of the GUID to be
> incorrectly byte-swapped in the trace output, presenting a corrupted GUID to
> userspace consumers.

This is expected, followed the non_standard_event from 
include/ras/ras_event.h

Thanks,
Badal

>
>> +		"intel_err_hdr_raw=%s cper_len=%u cper_raw=%s",
>> +		__get_str(dev), __entry->sev,
>> +		__entry->platform_id, __entry->fru_id,
>> +		__print_hex(__entry->ihdr_raw,
>> +		    sizeof(struct xe_cper_sec_intel_err_hdr)),
>> +		__entry->cper_len,
>> +		__print_hex(__get_dynamic_array(cper),
>> +		    __entry->cper_len))
>> +);