Re: [PATCH v4 06/12] drm/xe/cper: Prepare Intel CPER error info records

[Raag Jadav <raag.jadav@intel.com>]

On Fri, Oct 09, 2026 at 04:16:09PM +0530, Badal Nilawar wrote:
> Prepare xe_cper_sec_intel_error_info records from RAS info queue data.
> Depending on the error class type, detail or aggregate, one or more error
> info records may be generated and embedded in the CPER record.
> 
> Assisted-by: Copilot:Claude-Opus-5.5
> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> ---
> v4:
>  - Move struct xe_cper_einfo_entry to xe_cper_types.h (Raag)
>  - Use kzalloc_flex()/kzalloc_objs() (Raag)
>  - Allocate info queue buffer from reported total size (Raag)
> ---
>  drivers/gpu/drm/xe/xe_cper.c       | 247 +++++++++++++++++++++++++++--
>  drivers/gpu/drm/xe/xe_cper_types.h |  54 ++++++-
>  2 files changed, 290 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_cper.c b/drivers/gpu/drm/xe/xe_cper.c
> index 1000c78b41e9..7acb503efd11 100644
> --- a/drivers/gpu/drm/xe/xe_cper.c
> +++ b/drivers/gpu/drm/xe/xe_cper.c
> @@ -78,16 +78,22 @@ static u64 cper_timestamp_now(void)
>  	return ts;
>  }
>  
> +static void fill_cper_error_class(struct xe_cper_error_class *dst,
> +				  const struct xe_ras_error_class *src)
> +{
> +	dst->error_type = src->common.severity;
> +	dst->error_component = src->common.component;
> +	dst->tile = src->product.unit.tile;
> +	dst->instance = src->product.unit.instance;
> +	dst->cause = src->product.cause.cause;
> +}

No needless refactoring, please introduce this function in the original
patch. Assuming you'll reuse struct xe_ras_error_class here, this can
simply be a memcpy().

...

> +/**
> + * struct xe_cper_sec_intel_error_info - Variable-length Intel GPU error payload

Do we really need to add 'error' everywhere?

> + * Appended after &xe_cper_sec_intel_err_hdr when detailed per-event data
> + * is available. The @event_queue flexible array holds @event_queue_count
> + * packed &xe_intel_priv_event_entry records.
> + */
> +struct xe_cper_sec_intel_error_info {
> +	/** @error_class: Error classification */
> +	struct xe_cper_error_class error_class;

'counter'

> +	/** @error_count: Total number of errors recorded */
> +	u32 error_count;

'count'

> +	/** @event_queue_length: Total byte size of the @event_queue array */
> +	u32 event_queue_length;

'queue_length'

> +	/** @event_queue_count: Number of entries in @event_queue */
> +	u32 event_queue_count;

'queue_count'

> +	/** @event_queue: Packed array of &xe_intel_priv_event_entry records */
> +	u8 event_queue[];

'queue'

> +};
> +
> +/**
> + * struct xe_intel_priv_event_entry - Single error event in the event queue

All cper struct must contain 'cper' naming. Also, redundant 'event'.

> + * Each entry is variable-length; @entry_length gives the byte size of
> + * @metadata only (not including @entry_length or @timestamp).
> + */
> +struct xe_intel_priv_event_entry {
> +	/** @entry_length: Byte length of the @metadata payload */
> +	u32 entry_length;

'length'

> +	/** @timestamp: Timestamp of this event, in firmware ticks */
> +	u64 timestamp;
> +	/** @metadata: Event-specific payload bytes */
> +	u8  metadata[];

Nit: Fix the spacing.

> +};
> +
>  /**
>   * struct xe_cper_nonstd_record - Intel GPU CPER record
>   *
>   * Contains the standard CPER record header, section descriptor, and the
> - * Intel error section header.
> + * Intel error section header. A &xe_cper_sec_intel_error_info payload
> + * may be appended dynamically.
>   */
>  struct xe_cper_nonstd_record {
>  	/** @record_hdr: Standard CPER record header (UEFI Appendix N.2.1) */
> @@ -122,4 +160,18 @@ struct xe_platform_id_entry {
>  	guid_t    platform_id;
>  };
>  
> +/**
> + * struct xe_cper_einfo_entry - One CPER error info payload and its source counter
> + */
> +struct xe_cper_einfo_entry {
> +	/** @hdr: Error class and counter value used for the CPER section header */
> +	struct xe_ras_info_queue_dynamic_counter_hdr hdr;
> +	/** @einfo: Allocated error info payload, NULL if none */
> +	struct xe_cper_sec_intel_error_info *einfo;
> +	/** @einfo_size: Byte size of @einfo including the event queue */
> +	u32 einfo_size;

You're using length and size interchangeably. Whatever your preference,
please use consistent naming for the terminology (in all places where
applicable).

Raag

> +	/** @timestamp: Timestamp of the first occurrence of this error class */
> +	u64 timestamp;
> +};
> +
>  #endif
> -- 
> 2.54.0
>