Re: [PATCH v4 07/12] drm/xe/cper: Log CPER records for aggregate counter retrieval

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

On Fri, Oct 09, 2026 at 04:16:10PM +0530, Badal Nilawar wrote:
> Log CPER records for aggregate counter retrieval from userspace when
> cper_on_query sysfs is enabled.

...

> -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)
> +/**
> + * xe_cper_emit_counter() - Emit CPER records for an already fetched error counter
> + * @xe: The &xe_device
> + * @cper_sev: CPER severity
> + * @sigid: Error signature identifier
> + * @error_class: RAS error class of the counter
> + * @resp: Counter response returned by firmware for @error_class
> + *
> + * Build and emit CPER records from @resp, including any info queue data.
> + * Must be called from a sleepable context with a runtime PM reference held.
> + */
> +void xe_cper_emit_counter(struct xe_device *xe, int cper_sev, enum xe_sigid sigid,
> +			  const struct xe_ras_error_class *error_class,
> +			  const struct xe_ras_get_counter_response *resp)
>  {
>  	struct xe_cper_einfo_entry *einfo_arr = NULL, *entries;
> -	struct xe_ras_get_counter_response local_resp;
>  	struct xe_cper_einfo_entry single = {};
>  	u32 einfo_count = 0, count, i;
>  
> -	/* Fetching the counter sends a sysctrl command */
> +	if (!is_hw_sigid(sigid))

Nack. No guardrails for abusers, they are expected to fail.

> +		return;
> +
> +	/* Draining the info queue sends sysctrl commands */
>  	might_sleep();
>  	xe_device_assert_mem_access(xe);
>  
> -	if (xe_ras_get_counter_response(xe, error_class, &local_resp)) {
> -		xe_err(xe, "CPER: failed to get counter, skipping record\n");
> -		return;
> -	}
> -
>  	/* Without info queue data, emit a single record from the counter itself */
>  	single.hdr.error_class = *error_class;
> -	single.hdr.counter = local_resp.value;
> -	single.timestamp = local_resp.timestamp;
> +	single.hdr.counter = resp->value;
> +	single.timestamp = resp->timestamp;
>  	entries = &single;
>  	count = 1;
>  
> -	if (local_resp.has_info_queue) {
> -		einfo_arr = xe_prepare_cper_error_info(xe, &local_resp, error_class, &einfo_count);
> +	if (resp->has_info_queue) {
> +		einfo_arr = xe_prepare_cper_error_info(xe, resp, error_class, &einfo_count);
>  		if (einfo_arr) {
>  			entries = einfo_arr;
>  			count = einfo_count;
> @@ -425,6 +436,23 @@ static void xe_cper_emit_hw_err(struct xe_device *xe, int cper_sev, enum xe_sigi
>  	free_einfo_arr(einfo_arr, einfo_count);
>  }
>  
> +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)
> +{
> +	struct xe_ras_get_counter_response resp;
> +
> +	/* Fetching the counter sends a sysctrl command */
> +	might_sleep();
> +	xe_device_assert_mem_access(xe);
> +
> +	if (xe_ras_get_counter_response(xe, error_class, &resp)) {
> +		xe_err(xe, "CPER: failed to get counter, skipping record\n");
> +		return;
> +	}
> +
> +	xe_cper_emit_counter(xe, cper_sev, sigid, error_class, &resp);
> +}

This is how I think it should've been introduced in the original
patch, so we don't churn here.

...

> @@ -616,6 +649,11 @@ int xe_ras_get_counter(struct xe_device *xe, u8 severity, u8 component, u32 *val
>  		return ret;
>  
>  	*value = response.value;
> +
> +	if (READ_ONCE(xe->cper.on_query))

Following the comments on patch 2, this condition will now be passed as
a bool argument to get_counter(). Based on which, it will go on to call
xe_cper_emit_counter().

The callers that aren't interesed in the entire response will pass
explicit false to get_counter(), so cper isn't emitted in those cases.

Raag

> +		xe_cper_emit_counter(xe, ras_sev_to_cper_sev(counter.common.severity),
> +				     ras_comp_to_hw_sigid(counter.common.component),
> +				     &counter, &response);
>  	return 0;
>  }
>  
> @@ -1067,4 +1105,8 @@ void xe_ras_init(struct xe_device *xe)
>  	ret = devm_device_add_group(xe->drm.dev, &gpu_health_group);
>  	if (ret)
>  		xe_err(xe, "Failed to create GPU health sysfs, err=%d\n", ret);
> +
> +	ret = xe_cper_sysfs_init(xe);
> +	if (ret)
> +		xe_err(xe, "Failed to create cper_on_query sysfs, err=%d\n", ret);
>  }
> -- 
> 2.54.0
>