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

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

On Fri, Oct 09, 2026 at 04:16:04PM +0530, Badal Nilawar wrote:
> Introduce xe_cper_emit() as the single entry point for emitting
> CPER records from xe_log. xe_log now forwards every SIGID log entry
> with its full context (component, location, data/len and formatted
> message), and xe_cper decides how to report it. Hardware SIGIDs are
> routed to a hardware error helper, software SIGIDs are left as TODO.
> 
> No functional change is intended yet, as the CPER emission logic
> is added in follow-up patches.

This line isn't telling me anything and redundant for this patch.

> Assisted-by: Copilot:Claude-Opus-5.5
> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> ---
> v4:
>   - s/CONFIG_UEFI_CPER_X86/CONFIG_UEFI_CPER/ (Raag)
>   - Use in_range() for hardware SIGID check in xe_cper (Raag)
>   - Drop unused response param and constify error_class
>   - Check data len against struct xe_ras_error_class before use (Michal)
> ---
>  drivers/gpu/drm/xe/Makefile  |  2 ++
>  drivers/gpu/drm/xe/xe_cper.c | 51 ++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/xe/xe_cper.h | 26 ++++++++++++++++++
>  drivers/gpu/drm/xe/xe_log.c  |  5 +++-
>  4 files changed, 83 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/gpu/drm/xe/xe_cper.c
>  create mode 100644 drivers/gpu/drm/xe/xe_cper.h
> 
> diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
> index 10930c924070..da78026b0bb4 100644
> --- a/drivers/gpu/drm/xe/Makefile
> +++ b/drivers/gpu/drm/xe/Makefile
> @@ -168,6 +168,8 @@ xe-$(CONFIG_HWMON) += xe_hwmon.o
>  xe-$(CONFIG_PERF_EVENTS) += xe_pmu.o
>  xe-$(CONFIG_CONFIGFS_FS) += xe_configfs.o
>  
> +xe-$(CONFIG_UEFI_CPER) += xe_cper.o
> +
>  # graphics virtualization (SR-IOV) support
>  xe-y += \
>  	xe_gt_sriov_vf.o \
> diff --git a/drivers/gpu/drm/xe/xe_cper.c b/drivers/gpu/drm/xe/xe_cper.c
> new file mode 100644
> index 000000000000..c5caa0e8f496
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/xe_cper.c
> @@ -0,0 +1,51 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2026 Intel Corporation
> + */
> +
> +#include <linux/err.h>
> +#include <linux/minmax.h>
> +
> +#include "xe_cper.h"
> +#include "xe_device.h"
> +#include "xe_ras_types.h"
> +
> +static bool is_hw_sigid(enum xe_sigid sigid)
> +{
> +	return in_range(sigid, INTEL_SIGID_GPU_XE_HARDWARE_START, INTEL_SIGID_BATCH);
> +}
> +
> +static void xe_cper_emit_hw_err(struct xe_device *xe, int cper_sev, enum xe_sigid sigid,
> +				const struct xe_ras_error_class *error_class)
> +{
> +	/* TODO */
> +}
> +
> +/**
> + * xe_cper_emit() - Emit a CPER record for a SIGID log entry
> + * @xe: The &xe_device
> + * @cper_sev: CPER severity
> + * @sigid: Error signature identifier
> + * @component: Component identifier
> + * @location: Location details of the @component
> + * @data: Pointer to the additional details, or ERR_PTR, or NULL if not applicable
> + * @len: Length of the @data in bytes, or 0 if not applicable
> + * @vaf: Formatted log message
> + *
> + * Emit a CPER record for a SIGID log entry
> + */
> +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)

Why? Do we plan to shoot in out foot?

> +		return;
> +
> +	if (is_hw_sigid(sigid)) {
> +		if (!IS_ERR_OR_NULL(data) && len == sizeof(struct xe_ras_error_class))

Why the 'len' check?

Note: Any caller with incorrect length is abusing xe_log and will fail
silently here, so let's not hide bugs.

Raag

> +			xe_cper_emit_hw_err(xe, cper_sev, sigid, data);
> +		return;
> +	}
> +
> +	/* TODO software CPER */
> +}
> diff --git a/drivers/gpu/drm/xe/xe_cper.h b/drivers/gpu/drm/xe/xe_cper.h
> new file mode 100644
> index 000000000000..84676c7645a5
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/xe_cper.h
> @@ -0,0 +1,26 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2026 Intel Corporation
> + */
> +
> +#ifndef _XE_CPER_H_
> +#define _XE_CPER_H_
> +
> +#include <linux/types.h>
> +
> +#include "abi/xe_sigid_abi.h"
> +
> +struct va_format;
> +struct xe_device;
> +
> +#if IS_ENABLED(CONFIG_UEFI_CPER)
> +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);
> +#else
> +static inline 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) {}
> +#endif
> +
> +#endif /* _XE_CPER_H_ */
> diff --git a/drivers/gpu/drm/xe/xe_log.c b/drivers/gpu/drm/xe/xe_log.c
> index 29eb16db3320..80e2b1724ceb 100644
> --- a/drivers/gpu/drm/xe/xe_log.c
> +++ b/drivers/gpu/drm/xe/xe_log.c
> @@ -8,6 +8,7 @@
>  
>  #include "abi/xe_log_abi.h"
>  
> +#include "xe_cper.h"
>  #include "xe_device.h"
>  #include "xe_log.h"
>  #include "xe_pci_types.h"
> @@ -19,7 +20,9 @@ static void log_emit_cper(struct pci_dev *pdev, int cper_sev, enum xe_sigid sigi
>  {
>  	KUNIT_STATIC_STUB_REDIRECT(log_emit_cper, pdev, cper_sev, sigid,
>  				   component, location, data, len, vaf);
> -	/* TODO */
> +
> +	xe_cper_emit(xe_any_to_xe(pdev), cper_sev, sigid,
> +		     component, location, data, len, vaf);
>  }
>  
>  static const char *log_unknown_component_prefix(u32 component)
> -- 
> 2.54.0
>