Re: [PATCH v5 2/6] drm/xe/xe_ras: Add support to get error counter in CRI

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

On Tue, May 12, 2026 at 10:57:50AM +0530, Tauro, Riana wrote:
> On 5/11/2026 8:57 PM, Raag Jadav wrote:
> > On Mon, May 04, 2026 at 12:26:17PM +0530, Riana Tauro wrote:
> > > Add request/response structures and helper functions to query system
> > > controller to get error counter value.
> > > 
> > > Signed-off-by: Riana Tauro <riana.tauro@intel.com>
> > > ---
> > > v2: add structures for clear counter
> > >      move commands to sysctrl file
> > >      split functions
> > >      fix commit message (Raag)
> > > 
> > > v3: fix log message
> > >      squash patches
> > >      change error code for sysctrl error (Raag)
> > > 
> > > v4: rename function
> > >      remove unecessary macro (Raag)
> > >      add documentation for enum
> > > 
> > > v5: rebase
> > > ---
> > >   drivers/gpu/drm/xe/xe_ras.c                   | 91 +++++++++++++++++++
> > >   drivers/gpu/drm/xe/xe_ras.h                   |  4 +
> > >   drivers/gpu/drm/xe/xe_ras_types.h             | 30 ++++++
> > >   drivers/gpu/drm/xe/xe_sysctrl_mailbox_types.h |  2 +
> > >   4 files changed, 127 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/xe/xe_ras.c b/drivers/gpu/drm/xe/xe_ras.c
> > > index 4cb16b419b0c..47a58ce3b3ca 100644
> > > --- a/drivers/gpu/drm/xe/xe_ras.c
> > > +++ b/drivers/gpu/drm/xe/xe_ras.c
> > > @@ -4,11 +4,14 @@
> > >    */
> > >   #include "xe_device.h"
> > > +#include "xe_pm.h"
> > >   #include "xe_printk.h"
> > >   #include "xe_ras.h"
> > >   #include "xe_ras_types.h"
> > >   #include "xe_sysctrl.h"
> > >   #include "xe_sysctrl_event_types.h"
> > > +#include "xe_sysctrl_mailbox.h"
> > > +#include "xe_sysctrl_mailbox_types.h"
> > >   /* Severity of detected errors  */
> > >   enum xe_ras_severity {
> > > @@ -50,6 +53,23 @@ static const char *const xe_ras_components[] = {
> > >   };
> > >   static_assert(ARRAY_SIZE(xe_ras_components) == XE_RAS_COMP_MAX);
> > > +/* Mapping from drm_xe_ras_error_component to xe_ras_component */
> > > +static const int drm_to_xe_ras_component[] = {
> > > +	[DRM_XE_RAS_ERR_COMP_CORE_COMPUTE]	= XE_RAS_COMP_CORE_COMPUTE,
> > > +	[DRM_XE_RAS_ERR_COMP_SOC_INTERNAL]	= XE_RAS_COMP_SOC_INTERNAL,
> > > +	[DRM_XE_RAS_ERR_COMP_DEVICE_MEMORY]	= XE_RAS_COMP_DEVICE_MEMORY,
> > > +	[DRM_XE_RAS_ERR_COMP_PCIE]		= XE_RAS_COMP_PCIE,
> > > +	[DRM_XE_RAS_ERR_COMP_FABRIC]		= XE_RAS_COMP_FABRIC
> > > +};
> > > +static_assert(ARRAY_SIZE(drm_to_xe_ras_component) == DRM_XE_RAS_ERR_COMP_MAX);
> > > +
> > > +/* Mapping from drm_xe_ras_error_severity to xe_ras_severity */
> > > +static const int drm_to_xe_ras_severity[] = {
> > > +	[DRM_XE_RAS_ERR_SEV_CORRECTABLE]	= XE_RAS_SEV_CORRECTABLE,
> > > +	[DRM_XE_RAS_ERR_SEV_UNCORRECTABLE]	= XE_RAS_SEV_UNCORRECTABLE
> > > +};
> > > +static_assert(ARRAY_SIZE(drm_to_xe_ras_severity) == DRM_XE_RAS_ERR_SEV_MAX);
> > So we don't accept new entries unless also added in uapi and vice versa
> > which is good, but if you feel the need to have bounds checking just
> > switch() instead.
> 
> If i move this to switch, there will be inconsistency with the component
> array.

With switch() I think you won't need an array ...

> The component array has multiple entries so array is preferable.
> Also bounds check is already done in upper layers for this so not needed.

... but fair.

> > >   static inline const char *sev_to_str(u8 severity)
> > >   {
> > >   	if (severity >= XE_RAS_SEV_MAX)
> > > @@ -66,6 +86,22 @@ static inline const char *comp_to_str(u8 component)
> > >   	return xe_ras_components[component];
> > >   }
> > > +static void prepare_ras_command(struct xe_sysctrl_mailbox_command *command,
> > This looks like it should be a sysctrl helper (for non-RAS mailbox users).
> 
> We do not have any non-ras users. We can move if we need it

I thought we did[1], but I might be missing something.

[1] https://lore.kernel.org/intel-xe/20260320072528.1780651-12-anoop.c.vijay@intel.com/

> > > +				u32 cmd, void *request, size_t request_len,
> > > +				void *response, size_t response_len)
> > > +{
> > > +	struct xe_sysctrl_app_msg_hdr header = {0};
> > > +
> > > +	header.data = FIELD_PREP(APP_HDR_GROUP_ID_MASK, XE_SYSCTRL_GROUP_GFSP) |
> > > +		      FIELD_PREP(APP_HDR_COMMAND_MASK, cmd);
> > > +
> > > +	command->header = header;
> > > +	command->data_in = request;
> > > +	command->data_in_len = request_len;
> > > +	command->data_out = response;
> > > +	command->data_out_len = response_len;
> > > +}
> > > +
> > >   void xe_ras_counter_threshold_crossed(struct xe_device *xe,
> > >   				      struct xe_sysctrl_event_response *response)
> > >   {
> > > @@ -91,3 +127,58 @@ void xe_ras_counter_threshold_crossed(struct xe_device *xe,
> > >   			comp_to_str(component), sev_to_str(severity));
> > >   	}
> > >   }
> > > +
> > > +static int get_counter(struct xe_device *xe, struct xe_ras_error_class *error_class,
> > s/error_class/counter
> > 
> > Single word parameters usually help with wrapping but ofcourse it's a
> > personal preference.
> okay
> > 
> > > +		       u32 *value)
> > > +{
> > > +	struct xe_ras_get_counter_response response = {0};
> > > +	struct xe_ras_get_counter_request request = {0};
> > > +	struct xe_sysctrl_mailbox_command command = {0};
> > > +	size_t rlen;
> > > +	int ret;
> > > +
> > > +	request.error_class = *error_class;
> > > +
> > > +	prepare_ras_command(&command, XE_SYSCTRL_CMD_GET_COUNTER, &request, sizeof(request),
> > > +			    &response, sizeof(response));
> > > +
> > > +	ret = xe_sysctrl_send_command(&xe->sc, &command, &rlen);
> > > +	if (ret) {
> > > +		xe_err(xe, "sysctrl: failed to get counter %d\n", ret);
> > > +		return ret;
> > > +	}
> > > +
> > > +	if (rlen != sizeof(response)) {
> > > +		xe_err(xe, "sysctrl: unexpected get counter response length %zu (expected %zu)\n",
> > > +		       rlen, sizeof(response));
> > > +		return -EIO;
> > > +	}
> > > +
> > > +	*value = response.counter_value;
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +/**
> > > + * xe_ras_get_counter() - Get error counter value
> > > + * @xe: xe device instance
> > > + * @severity: Error severity level to be queried
> > > + * @error_id: Error component to be queried
> > > + * @value: Counter value
> > > + *
> > > + * This function retrieves the value of a specific error counter based on
> > > + * the error severity and component.
> > > + *
> > > + * Return: 0 on success, negative error code on failure.
> > > + */
> > > +int xe_ras_get_counter(struct xe_device *xe, enum drm_xe_ras_error_severity severity,
> > const for consistency.
> 
> We do not need const since it's passed by value
> consistency ? the upper layer does not have a const

Then perhaps u32? Anyway, I'll leave it to you.

Raag

> > > +		       u32 error_id, u32 *value)
> > > +{
> > > +	struct xe_ras_error_class error_class = {0};
> > > +
> > > +	error_class.common.severity = drm_to_xe_ras_severity[severity];
> > > +	error_class.common.component = drm_to_xe_ras_component[error_id];
> > > +
> > > +	guard(xe_pm_runtime)(xe);
> > > +	return get_counter(xe, &error_class, value);
> > > +}
> > > diff --git a/drivers/gpu/drm/xe/xe_ras.h b/drivers/gpu/drm/xe/xe_ras.h
> > > index ea90593b62dc..74582c911b02 100644
> > > --- a/drivers/gpu/drm/xe/xe_ras.h
> > > +++ b/drivers/gpu/drm/xe/xe_ras.h
> > > @@ -6,10 +6,14 @@
> > >   #ifndef _XE_RAS_H_
> > >   #define _XE_RAS_H_
> > > +#include <uapi/drm/xe_drm.h>
> > > +
> > >   struct xe_device;
> > >   struct xe_sysctrl_event_response;
> > >   void xe_ras_counter_threshold_crossed(struct xe_device *xe,
> > >   				      struct xe_sysctrl_event_response *response);
> > > +int xe_ras_get_counter(struct xe_device *xe, enum drm_xe_ras_error_severity severity,
> > > +		       u32 error_id, u32 *value);
> > >   #endif
> > > diff --git a/drivers/gpu/drm/xe/xe_ras_types.h b/drivers/gpu/drm/xe/xe_ras_types.h
> > > index 4e63c67f806a..74d85875cd63 100644
> > > --- a/drivers/gpu/drm/xe/xe_ras_types.h
> > > +++ b/drivers/gpu/drm/xe/xe_ras_types.h
> > > @@ -70,4 +70,34 @@ struct xe_ras_threshold_crossed {
> > >   	struct xe_ras_error_class counters[XE_RAS_NUM_COUNTERS];
> > >   } __packed;
> > > +/**
> > > + * struct xe_ras_get_counter_request - Request for get error counter
> > > + */
> > > +struct xe_ras_get_counter_request {
> > > +	/** @error_class: Error class counter to be queried */
> > > +	struct xe_ras_error_class error_class;
> > > +	/** @reserved: Reserved for future use */
> > > +	u32 reserved;
> > > +} __packed;
> > > +
> > > +/**
> > > + * struct xe_ras_get_counter_response - Response for get error counter
> > > + */
> > > +struct xe_ras_get_counter_response {
> > > +	/** @error_class: Error class counter that was queried */
> > > +	struct xe_ras_error_class error_class;
> > > +	/** @counter_value: Current counter value */
> > > +	u32 counter_value;
> > > +	/** @timestamp: Timestamp when counter was last updated */
> > > +	u64 timestamp;
> > > +	/** @threshold_value: Threshold value for the counter */
> > > +	u32 threshold_value;
> > > +	/** @counter_status: Status of the counter */
> > > +	u32 counter_status:8;
> > > +	/** @reserved: Reserved for future use */
> > > +	u32 reserved:24;
> > > +	/** @reserved1: Reserved for future use */
> > > +	u32 reserved1[56];
> > > +} __packed;
> > > +
> > >   #endif
> > > diff --git a/drivers/gpu/drm/xe/xe_sysctrl_mailbox_types.h b/drivers/gpu/drm/xe/xe_sysctrl_mailbox_types.h
> > > index 84d7c647e743..b315847cbf64 100644
> > > --- a/drivers/gpu/drm/xe/xe_sysctrl_mailbox_types.h
> > > +++ b/drivers/gpu/drm/xe/xe_sysctrl_mailbox_types.h
> > > @@ -22,9 +22,11 @@ enum xe_sysctrl_group {
> > >   /**
> > >    * enum xe_sysctrl_gfsp_cmd - Commands supported by GFSP group
> > >    *
> > > + * @XE_SYSCTRL_CMD_GET_COUNTER: Get error counter value
> > >    * @XE_SYSCTRL_CMD_GET_PENDING_EVENT: Retrieve pending event
> > >    */
> > >   enum xe_sysctrl_gfsp_cmd {
> > > +	XE_SYSCTRL_CMD_GET_COUNTER		= 0x03,
> > >   	XE_SYSCTRL_CMD_GET_PENDING_EVENT	= 0x07,
> > >   };
> > > -- 
> > > 2.47.1
> > >