Re: [PATCH v5 2/3] drm/xe/xe_ras: Report correctable error events to userspace

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

On Tue, Jul 21, 2026 at 06:52:51PM +0530, Tauro, Riana wrote:
> On 21-07-2026 14:07, Raag Jadav wrote:
> > On Mon, Jul 20, 2026 at 01:52:11PM +0530, Riana Tauro wrote:
> > > When an interrupt is received indicating that error counter has crossed
> > > its threshold, read the current counter value and deliver a drm_ras error
> > > event to userspace for each affected component.
> > > 
> > > To avoid sending duplicate events when the same component appears multiple
> > > times in the response. Send the error-event once per component.
> > ...
> > 
> > > +void xe_drm_ras_event(struct xe_device *xe, u8 component, u8 severity, u32 value)
> > > +{
> > > +	struct xe_drm_ras *ras = &xe->ras;
> > > +	struct xe_drm_ras_counter *info;
> > > +	struct drm_ras_node *node;
> > > +	int ret;
> > > +
> > > +	/* Event is supported only if drm_ras is enabled */
> > > +	if (!xe->info.has_drm_ras)
> > > +		return;
> > > +
> > > +	if (component >= DRM_XE_RAS_ERR_COMP_MAX) {
> > IIUC this is error_id and should be validated against first/last counter
> > range in drm_ras layer (similar to registration code).
> 
> This should be done before because we are accessing the nodes here.
> The nodes anyway won't be available in xe_drm_ras if not registered with
> drm_ras.

Yes. We get the node from severity which we already validate here, but
error_counter_range is the property of the node and unrelated to xe.
Even if you prefer it here, these checks must be in core functions to
make sure they are not abused.

> > > +		drm_warn(&xe->drm, "unsupported component %u\n", component);
> > > +		return;
> > > +	}
> > > +
> > > +	if (severity >= DRM_XE_RAS_ERR_SEV_MAX) {
> > > +		drm_warn(&xe->drm, "unsupported severity %u\n", severity);
> > > +		return;
> > > +	}
> > > +
> > > +	node = &ras->node[severity];
> > > +	info = ras->info[severity];
> > > +
> > > +	if (!info || !info[component].name)
> > > +		return;
> > > +
> > > +	ret = drm_ras_nl_error_event(node, component, info[component].name, value);
> > > +	if (ret)
> > > +		drm_err_ratelimited(&xe->drm, "drm_ras error-event failed: %d for %s %s\n", ret,
> > > +				    info[component].name, error_severity[severity]);
> > > +}
> > ...
> > 
> > > +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;
> > > +
> > > +	counter.common.severity = severity;
> > > +	counter.common.component = component;
> > > +
> > > +	ret = get_counter(xe, &counter, &value);
> > > +	if (ret)
> > > +		return;
> > > +
> > > +	drm_severity = xe_to_drm_ras_severity(severity);
> > > +	drm_component = xe_to_drm_ras_component(component);
> > > +
> > > +	xe_drm_ras_event(xe, drm_component, drm_severity, value);
> > > +}
> > This entire function can be dropped. See below.
> 
> We don't need to drop function. It's cleaner to have it in the function
> than repeating it twice.

I thought we'd lay the groundwork for cper which'll also need get_counter()
in the same path, but upto you.

Raag