Re: [PATCH v3 2/6] drm/xe/cper: Prepare CPER headers for software errors

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

[-- Attachment #1: Type: text/plain, Size: 5251 bytes --]


On 05-11-2026 14:23, Mallesh Koujalagi wrote:
> Add software error classification support and reuse the
> common CPER header initialization path.
>
> Populate software error metadata without emitting a CPER
> record yet.
No need of this statement. First statement already covers the change.
>
> Cc: Badal Nilawar<badal.nilawar@intel.com>
> Signed-off-by: Mallesh Koujalagi<mallesh.koujalagi@intel.com>
> ---
> v2:
> - Move union inside xe_cper_intel_hdr structure. (Badal)
>
> v3:
> - Introduce structure where it's used. (Badal)
> - Move error-class union inside cper_init_intel_hdr. (Badal)
> ---
>   drivers/gpu/drm/xe/xe_cper.c       | 21 +++++++++++++++++---
>   drivers/gpu/drm/xe/xe_cper_types.h | 32 ++++++++++++++++++++++++++++--
>   2 files changed, 48 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_cper.c b/drivers/gpu/drm/xe/xe_cper.c
> index db3513c59834..10aa10bbf282 100644
> --- a/drivers/gpu/drm/xe/xe_cper.c
> +++ b/drivers/gpu/drm/xe/xe_cper.c
> @@ -14,6 +14,8 @@
>   #include <linux/time.h>
>   #include <linux/timekeeping.h>
>   
> +#include "abi/xe_log_abi.h"
> +
>   #include "xe_cper.h"
>   #include "xe_cper_types.h"
>   #include "xe_device.h"
> @@ -92,11 +94,11 @@ static u64 cper_timestamp_now(void)
>   	return ts;
>   }
>   
> -static void cper_init_intel_hdr(struct xe_device *xe, const struct xe_ras_error_class *counter,
> +static void cper_init_intel_hdr(struct xe_device *xe, const union xe_cper_error_class *error_class,
>   				u64 first_timestamp, u32 sig_id, u32 error_count,
>   				struct xe_cper_intel_hdr *ihdr)
>   {
> -	ihdr->counter = *counter;
> +	ihdr->error_class = *error_class;
>   	ihdr->validation_bits |= XE_CPER_VALID_LOCATION;
>   	ihdr->sig_id = U32_MAX;
>   
> @@ -219,11 +221,12 @@ void xe_cper_emit_counter(struct xe_device *xe, int cper_sev, enum xe_sigid sigi
>   			  const struct xe_ras_error_class *counter,
>   			  const struct xe_ras_get_counter_response *response)
>   {
> +	union xe_cper_error_class error_class = { .counter = *counter };
>   	struct xe_cper_intel_hdr ihdr = {};
>   
>   	xe_assert(xe, is_hw_sigid(sigid));
>   
> -	cper_init_intel_hdr(xe, counter, response->timestamp,
> +	cper_init_intel_hdr(xe, &error_class, response->timestamp,
>   			    sigid, response->value, &ihdr);
>   	cper_record_emit(xe, cper_sev, &INTEL_CPER_NOTIFY_GPU_ERROR, &ihdr, NULL, 0);
>   }
> @@ -249,6 +252,18 @@ static void cper_emit_sw_err(struct xe_device *xe, int cper_sev, enum xe_sigid s
>   			     u32 component, u32 location, const void *data,
>   			     struct va_format *vaf)
>   {
> +	union xe_cper_error_class error_class = {};
> +	struct xe_cper_intel_hdr ihdr = {};
> +	struct xe_cper_driver_error_class *driver = &error_class.driver;
> +	u64 timestamp = ktime_get_boottime_ns();

Why not ktime_get_real_ns? Also, for readability, initialize it later.

> +
> +	driver->error_type = cper_sev;
> +	driver->error_component = component;
> +	driver->tile = location;
> +	if (IS_ERR(data))
> +		driver->cause = -PTR_ERR(data);
> +
> +	cper_init_intel_hdr(xe, &error_class, timestamp, sigid, 1, &ihdr);
>   }
>   
>   /**
> diff --git a/drivers/gpu/drm/xe/xe_cper_types.h b/drivers/gpu/drm/xe/xe_cper_types.h
> index 68e30df0440e..d8581992dc45 100644
> --- a/drivers/gpu/drm/xe/xe_cper_types.h
> +++ b/drivers/gpu/drm/xe/xe_cper_types.h
> @@ -45,6 +45,29 @@
>   	GUID_INIT(0xea9d8f84, 0x4258, 0x5227, \
>   		  0x80, 0x28, 0xb9, 0xb1, 0x3e, 0x6d, 0x58, 0xb0)
>   
> +/**
> + * struct xe_cper_driver_error_class - Intel GPU CPER driver-detected error classification
> + */
> +struct xe_cper_driver_error_class {
> +	/** @error_type: CPER severity: CPER_SEV_* */
> +	u8  error_type;
> +	/** @reserved0: Reserved, must be zero */
> +	u8  reserved0;
> +	/** @error_component: Driver component id: CLASS[7:0].TYPE[15:8] */
> +	u16 error_component;
> +	/** @tile: Driver location id: TYPE[7:0].ID[15:8] ('tile' and 'GT' field region) */
> +	u16 tile;
> +	/** @reserved1: Reserved, must be zero */
> +	u16 reserved1;
> +	/** @cause: errno of the failing operation, or 0 if none */
> +	u32 cause;
> +};
> +
> +/* Same size as the counter-sourced error_class */
> +#define XE_CPER_ERROR_CLASS_SIZE	12
> +
> +static_assert(sizeof(struct xe_cper_driver_error_class) == XE_CPER_ERROR_CLASS_SIZE);
> +
>   /**
>    * struct xe_cper_intel_hdr - Intel-specific CPER header
>    *
> @@ -52,8 +75,13 @@
>    * All multi-byte fields are little-endian.
>    */
>   struct xe_cper_intel_hdr {
> -	/** @counter: Error class (severity, component, location, cause) */
> -	struct xe_ras_error_class counter;
> +	/** @error_class: Counter-sourced or driver-detected error classification */
This seems redundant. Otherwise  /** @error_class: Error classification 
*/ is sufficient.
> +	union xe_cper_error_class {
> +		/** @error_class.counter: Counter-sourced error classification */

Keep the original doc.

Thanks,
Badal

> +		struct xe_ras_error_class counter;
> +		/** @error_class.driver: Driver-detected error classification */
> +		struct xe_cper_driver_error_class driver;
> +	} error_class;
>   	/** @first_timestamp: First occurrence of this error class, in ticks */
>   	u64 first_timestamp;
>   	/** @sig_id: Aggregated error class SIG ID; set to U32_MAX if unknown */

[-- Attachment #2: Type: text/html, Size: 6487 bytes --]