Re: [PATCH v4 3/3] drm/xe/xe_ras: Add error-event support for CRI

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

On 09-07-2026 15:34, Raag Jadav wrote:
> On Wed, Jul 01, 2026 at 03:14:13PM +0530, Riana Tauro wrote:
>> Add error-event support for Correctable errors in CRI. Report an error
>> event to userspace for every component that has crossed the threshold on
>> receiving an interrupt.
> ...
>
>> +static void ras_send_error_event(struct xe_device *xe, u8 severity, u8 component)
>> +{
>> +	u8 drm_severity, drm_component;
>> +	u32 value;
>> +	int ret;
>> +
>> +	drm_severity = xe_to_drm_ras_severity(severity);
>> +	if (drm_severity == DRM_XE_RAS_ERR_SEV_MAX) {
>> +		xe_warn(xe, "sysctrl: unexpected severity %u\n", severity);
> This is uapi and not coming from sysctrl, so the message is a bit
> misleading. But if at all it needs validation, it should be done in
> drm_ras layer.

You mean in the ras_event function?  The parameters to this function are 
coming from sysctrl.
So added sysctrl flag


>
>> +		return;
>> +	}
>> +
>> +	drm_component = xe_to_drm_ras_component(component);
>> +	if (drm_component == DRM_XE_RAS_ERR_COMP_MAX) {
>> +		xe_warn(xe, "sysctrl: unexpected component %u\n", component);
> Ditto.
>
>> +		return;
>> +	}
>> +
>> +	ret = xe_ras_get_counter(xe, drm_severity, drm_component, &value);
> No, instead of converting back and forth just do get_counter() using
> sysctrl values and send_event() afterwards.

The reason for using this is to avoid unnecessary churn of moving the 
get counter above or use forward declaration.
Yeah i can use that too directly.

>> +	if (ret)
>> +		return;
>> +
>> +	xe_drm_ras_event(xe, drm_component, drm_severity, value, GFP_KERNEL);
>> +}
>> +
>>   void xe_ras_counter_threshold_crossed(struct xe_device *xe,
>>   				      struct xe_sysctrl_event_response *response)
>>   {
>>   	struct xe_ras_threshold_crossed *pending = (void *)&response->data;
>>   	struct xe_ras_error_class *errors = pending->counters;
>>   	u32 id, ncounters = pending->ncounters;
>> +	u8 sent = 0;
>>   
>>   	BUILD_BUG_ON(sizeof(response->data) < sizeof(*pending));
>> +	BUILD_BUG_ON(XE_RAS_COMP_MAX > (BITS_PER_BYTE * sizeof(sent)));
> I prefer it the other way around. Also, have you tried using BITS_PER_TYPE()?
>
> 	BUILD_BUG_ON(BITS_PER_TYPE(sent) < XE_RAS_COMP_MAX));

Will try this

>
>>   	xe_device_assert_mem_access(xe);
>>   
>>   	if (!ncounters || ncounters > XE_RAS_NUM_COUNTERS)
>> @@ -154,6 +211,24 @@ void xe_ras_counter_threshold_crossed(struct xe_device *xe,
>>   
>>   		xe_warn(xe, "[RAS]: %s %s detected\n",
>>   			comp_to_str(component), sev_to_str(severity));
>> +
>> +		if (severity != XE_RAS_SEV_CORRECTABLE) {
>> +			xe_warn(xe, "sysctrl: unexpected severity %s (%u)\n", sev_to_str(severity),
>> +				severity);
> This should be before "detected" log above.

okay

>
>> +			continue;
>> +		}
>> +
>> +		if (component >= XE_RAS_COMP_MAX) {
>> +			xe_warn(xe, "sysctrl: unexpected component %u\n", component);
> Ditto. Also, use xx_to_str() in both cases or don't but be consistent.
>

It will only display not supported. Will remove the string in both.

Thanks
Riana

>
> Raag
>
>> +			continue;
>> +		}
>> +
>> +		/* Send event once per component */
>> +		if (sent & BIT(component))
>> +			continue;
>> +		sent |= BIT(component);
>> +
>> +		ras_send_error_event(xe, severity, component);
>>   	}
>>   }
>>   
>> -- 
>> 2.47.1
>>