Re: [PATCH v5 02/13] drm/xe/cper: Retrieve the error counter record for CPER reporting

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

On Fri, Oct 23, 2026 at 10:04:24AM +0530, Badal Nilawar wrote:
> Retrieve error counter record required to build a CPER record.
> 
> Assisted-by: LLM
> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> ---
> v5:
>   - Retain get_counter() (Raag)
> v4:
>   - Drop redundant xe_ras_counter_is_valid() check (Raag)
>   - Add might_sleep() to assert that the caller is in a sleepable
>     context (sashiko)
>   - Add xe_device_assert_mem_access() to assert the outer runtime PM
>     wake reference requirement (sashiko)
> ---
>  drivers/gpu/drm/xe/xe_cper.c | 14 ++++++++++-
>  drivers/gpu/drm/xe/xe_ras.c  | 47 +++++++++++++++++++++++++++---------
>  drivers/gpu/drm/xe/xe_ras.h  |  2 ++
>  3 files changed, 51 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_cper.c b/drivers/gpu/drm/xe/xe_cper.c
> index 8b52752c067f..a9044e708cac 100644
> --- a/drivers/gpu/drm/xe/xe_cper.c
> +++ b/drivers/gpu/drm/xe/xe_cper.c
> @@ -4,10 +4,13 @@
>   */
>  
>  #include <linux/err.h>
> +#include <linux/kernel.h>
>  #include <linux/minmax.h>
>  
>  #include "xe_cper.h"
>  #include "xe_device.h"
> +#include "xe_printk.h"
> +#include "xe_ras.h"
>  #include "xe_ras_types.h"
>  
>  static bool is_hw_sigid(enum xe_sigid sigid)
> @@ -18,7 +21,16 @@ static bool is_hw_sigid(enum xe_sigid sigid)
>  static void cper_emit_hw_err(struct xe_device *xe, int cper_sev, enum xe_sigid sigid,
>  			     const struct xe_ras_error_class *counter)
>  {
> -	/* TODO */
> +	struct xe_ras_get_counter_response response = {};
> +
> +	/* Fetching the counter sends a sysctrl command */
> +	might_sleep();
> +	xe_device_assert_mem_access(xe);
> +
> +	if (xe_ras_get_counter_response(xe, counter, &response)) {
> +		xe_err(xe, "CPER: failed to get counter, skipping record\n");
> +		return;
> +	}
>  }
>  
>  /**
> diff --git a/drivers/gpu/drm/xe/xe_ras.c b/drivers/gpu/drm/xe/xe_ras.c
> index db3cbc186a51..1da523c6e84c 100644
> --- a/drivers/gpu/drm/xe/xe_ras.c
> +++ b/drivers/gpu/drm/xe/xe_ras.c
> @@ -528,8 +528,7 @@ static void ras_send_error_event(struct xe_device *xe, u8 severity, u8 component
>  {
>  	struct xe_ras_error_class counter = {0};
>  	u8 drm_severity, drm_component;
> -	u32 value;
> -	int ret;
> +	u32 value; int ret;

Redundant churn, shouldn't be in this patch.

>  	counter.common.severity = severity;
>  	counter.common.component = component;
> @@ -676,19 +675,46 @@ void xe_ras_counter_threshold_crossed(struct xe_device *xe,
>  	}
>  }
>  
> -static int get_counter(struct xe_device *xe, struct xe_ras_error_class *counter, u32 *value)
> +static int get_counter(struct xe_device *xe,
> +		       struct xe_ras_error_class *counter,
> +		       u32 *value)

Why the wrapping? What changed?

> +{
> +	struct xe_ras_get_counter_response response = {};
> +	int ret;
> +
> +	ret = xe_ras_get_counter_response(xe, counter, &response);
> +	if (ret)
> +		return ret;
> +
> +	*value = response.value;
> +
> +	return 0;
> +}
> +
> +/**
> + * xe_ras_get_counter_response() - Get error counter record
> + * @xe: Xe device instance
> + * @counter: RAS error class
> + * @out: Counter record retrieved
> + *
> + * Retrieve the counter record of a specific error class from firmware.
> + *
> + * Return: 0 on success, negative error code on failure.
> + */
> +int xe_ras_get_counter_response(struct xe_device *xe, const struct xe_ras_error_class *counter,
> +				struct xe_ras_get_counter_response *out)

'response'

>  {
> -	struct xe_ras_get_counter_response response = {0};
>  	struct xe_ras_get_counter_request request = {0};
>  	struct xe_sysctrl_mailbox_command command = {0};
>  	struct xe_ras_error_common *common;
>  	size_t rlen;
>  	int ret;
>  
> +	memset(out, 0, sizeof(*out));

I think I've already mentioned to drop this in the previous rev[1].

[1] https://lore.kernel.org/intel-xe/asyjSMUDo0XMX-U4@black.igk.intel.com/

Raag

>  	request.counter = *counter;
>  
>  	xe_sysctrl_create_command(&command, XE_SYSCTRL_GROUP_GFSP, XE_SYSCTRL_CMD_GET_COUNTER,
> -				  &request, sizeof(request), &response, sizeof(response));
> +				  &request, sizeof(request), out, sizeof(*out));
>  
>  	ret = xe_sysctrl_send_command(&xe->sc, &command, &rlen);
>  	if (ret) {
> @@ -696,19 +722,18 @@ static int get_counter(struct xe_device *xe, struct xe_ras_error_class *counter,
>  		return ret;
>  	}
>  
> -	if (rlen != sizeof(response)) {
> +	if (rlen != sizeof(*out)) {
>  		xe_err(xe, "sysctrl: unexpected get counter response length %zu (expected %zu)\n",
> -		       rlen, sizeof(response));
> +		       rlen, sizeof(*out));
>  		return -EIO;
>  	}
>  
> -	if (!ras_counter_is_valid(xe, &response.counter))
> +	if (!ras_counter_is_valid(xe, &out->counter))
>  		return -EBADMSG;
>  
> -	common = &response.counter.common;
> -	*value = response.value;
> +	common = &out->counter.common;
>  
> -	xe_dbg(xe, "[RAS]: get counter %u for %s %s\n", *value, comp_to_str(common->component),
> +	xe_dbg(xe, "[RAS]: get counter %u for %s %s\n", out->value, comp_to_str(common->component),
>  	       sev_to_str(common->severity));
>  
>  	return 0;
> diff --git a/drivers/gpu/drm/xe/xe_ras.h b/drivers/gpu/drm/xe/xe_ras.h
> index 1b358c7d040f..c736d5a01c47 100644
> --- a/drivers/gpu/drm/xe/xe_ras.h
> +++ b/drivers/gpu/drm/xe/xe_ras.h
> @@ -23,5 +23,7 @@ int xe_ras_init(struct xe_device *xe);
>  enum xe_ras_recovery_action xe_ras_process_errors(struct xe_device *xe);
>  bool xe_ras_get_disable_vram_page_offline(struct xe_device *xe);
>  u32 xe_ras_get_max_pages(struct xe_device *xe);
> +int xe_ras_get_counter_response(struct xe_device *xe, const struct xe_ras_error_class *counter,
> +				struct xe_ras_get_counter_response *out);
>  
>  #endif
> -- 
> 2.54.0
>