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

["Tauro, Riana" <riana.tauro@intel.com>]

On 21-07-2026 22:38, Raag Jadav wrote:
> 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.

But here we are searching for already registered nodes with drm_ras.
So registered nodes are already range checked. The only reason of adding 
this here was to avoid
array out of bounds.

Agreed will remove the check since patch [1] is taking care of the check 
before
it reaches this function.

>
>>>> +		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.

confused. CPER requires a get counter on a detailed error_class. Unlike 
here which is a aggregated class
and is called once per component.

So would retain the function and remove the severity and component checks.

Thanks
Riana

> Raag