Re: [PATCH v5 03/13] drm/xe/cper: Initialize Intel-specific section header

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

On Fri, Oct 23, 2026 at 10:04:25AM +0530, Badal Nilawar wrote:
> Define the Intel-specific CPER header and initialize it

Please also use the same terminology in subject.

> from the error counter record for Intel GPU hardware errors.
> 
> Assisted-by: LLM
> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> ---
> v5:
>  - Move xe_cper_emit_counter() from patch 7 of rev4 to this patch to
>    avoid code churn (Raag)
>  - Use __packed for individual structures instead of #pragma pack (Raag)
>  - Drop redundent 'err'/'error' references, shorten structure name (Raag)
> v4:
>  - Split from "drm/xe/cper: Prepare CPER record" (Raag)
>  - Add dedicated struct xe_cper_error_class (Michal)
>  - Drop empty fill_fw_id() stub, leave a TODO instead
> v2:
>  - Guard against NULL THIS_MODULE->srcversion (sashiko)
> ---
>  drivers/gpu/drm/xe/xe_cper.c       | 52 ++++++++++++++++++++++++++++++
>  drivers/gpu/drm/xe/xe_cper_types.h | 51 +++++++++++++++++++++++++++++
>  2 files changed, 103 insertions(+)
>  create mode 100644 drivers/gpu/drm/xe/xe_cper_types.h
> 
> diff --git a/drivers/gpu/drm/xe/xe_cper.c b/drivers/gpu/drm/xe/xe_cper.c
> index a9044e708cac..3d7657517223 100644
> --- a/drivers/gpu/drm/xe/xe_cper.c
> +++ b/drivers/gpu/drm/xe/xe_cper.c
> @@ -6,18 +6,68 @@
>  #include <linux/err.h>
>  #include <linux/kernel.h>
>  #include <linux/minmax.h>
> +#include <linux/pci.h>
>  
>  #include "xe_cper.h"
> +#include "xe_cper_types.h"
>  #include "xe_device.h"
>  #include "xe_printk.h"
>  #include "xe_ras.h"
>  #include "xe_ras_types.h"
>  
> +static bool is_valid_sigid(u32 sigid)

Can simply be is_sigid().

> +{
> +	return sigid != U32_MAX;
> +}
> +
>  static bool is_hw_sigid(enum xe_sigid sigid)
>  {
>  	return in_range(sigid, INTEL_SIGID_GPU_XE_HARDWARE_START, INTEL_SIGID_BATCH);
>  }
>  
> +static void cper_init_intel_hdr(struct xe_device *xe,
> +				const struct xe_ras_error_class *counter,

This looks like can be less lines.

> +				u64 first_timestamp, u32 sig_id, u32 error_count,
> +				struct xe_cper_intel_hdr *ihdr)
> +{
> +	ihdr->counter = *counter;
> +	ihdr->validation_bits |= XE_CPER_VALID_LOCATION;
> +
> +	if (first_timestamp) {
> +		ihdr->first_timestamp = first_timestamp;
> +		ihdr->validation_bits |= XE_CPER_VALID_FIRST_TIMESTAMP;
> +	}
> +
> +	if (is_valid_sigid(sig_id)) {
> +		ihdr->sig_id = sig_id;
> +		ihdr->validation_bits |= XE_CPER_VALID_SIG_ID;
> +	}
> +
> +	ihdr->error_count = error_count;
> +
> +	strscpy(ihdr->pci_bdf, pci_name(to_pci_dev(xe->drm.dev)), sizeof(ihdr->pci_bdf));
> +	ihdr->validation_bits |= XE_CPER_VALID_PCI_BDF;
> +
> +#ifdef MODULE
> +	if (THIS_MODULE->srcversion) {
> +		strscpy(ihdr->drv_version, THIS_MODULE->srcversion, sizeof(ihdr->drv_version));
> +		ihdr->validation_bits |= XE_CPER_VALID_DRV_VERSION;
> +	}
> +#endif
> +
> +	/* TODO: fill fw_id and set XE_CPER_VALID_FW_ID */

Hm, see below.

> +}
> +
> +static void cper_emit_counter(struct xe_device *xe, int cper_sev, enum xe_sigid sigid,
> +			      const struct xe_ras_error_class *counter,
> +			      const struct xe_ras_get_counter_response *response)
> +{
> +	struct xe_cper_intel_hdr ihdr = {};
> +
> +	cper_init_intel_hdr(xe, counter, response->timestamp,
> +			    sigid, response->value, &ihdr);
> +}
> +
>  static void cper_emit_hw_err(struct xe_device *xe, int cper_sev, enum xe_sigid sigid,
>  			     const struct xe_ras_error_class *counter)
>  {
> @@ -31,6 +81,8 @@ static void cper_emit_hw_err(struct xe_device *xe, int cper_sev, enum xe_sigid s
>  		xe_err(xe, "CPER: failed to get counter, skipping record\n");
>  		return;
>  	}
> +
> +	cper_emit_counter(xe, cper_sev, sigid, counter, &response);
>  }
>  
>  /**
> 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 000000000000..a8730ff62b69
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/xe_cper_types.h
> @@ -0,0 +1,51 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2026 Intel Corporation
> + */
> +
> +#ifndef _XE_CPER_TYPES_H_
> +#define _XE_CPER_TYPES_H_
> +
> +#include <linux/types.h>
> +
> +#include "xe_ras_types.h"
> +
> +/*
> + * Validation bits definition for validation_bits in struct
> + * xe_cper_intel_hdr. When set, corresponding fields in header
> + * contains valid data
> + */
> +#define XE_CPER_VALID_LOCATION		0x0001
> +#define XE_CPER_VALID_FIRST_TIMESTAMP	0x0002
> +#define XE_CPER_VALID_SIG_ID		0x0004
> +#define XE_CPER_VALID_PCI_BDF		0x0008
> +#define XE_CPER_VALID_DRV_VERSION	0x0010
> +
> +/**
> + * struct xe_cper_intel_hdr - Intel-specific CPER header
> + *
> + * Fixed-size header for the Intel GPU error section of a CPER record.
> + * 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;
> +	/** @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 */
> +	u32 sig_id;
> +	/** @error_count: Number of times this error has been observed */
> +	u32 error_count;
> +	/** @validation_bits: Raw u16 view of all valid bits */
> +	u16 validation_bits;
> +	/** @pci_bdf: PCI location string, format "DDDD:bb:dd.f" */
> +	char pci_bdf[16];
> +	/** @drv_version: Driver source version string (THIS_MODULE->srcversion) */
> +	char drv_version[25];
> +	/** @fw_id: Firmware version string (GFSP+PCODE+CSC+GUC or MNG+NUC+RAS+GUC) */
> +	char fw_id[256];

Please introduce this member in the patch you actually use it.

Raag

> +	/** @reserved: Reserved for future use, must be zero */
> +	u8 reserved[5];
> +} __packed;
> +
> +#endif
> -- 
> 2.54.0
>