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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Synchronous mailbox commands injected during an active pagination loop disrupt stateful firmware communication.
- [Medium] Unvalidated firmware-provided `component` ID is used in the `BIT()` macro, leading to out-of-bounds shifts (Undefined Behavior) and truncation that breaks deduplication.
- [Medium] Errors of different severities for the same component are incorrectly collapsed, masking critical hardware errors.
--

--- Patch [3]: [PATCH v5 3/3] drm/xe/xe_ras: Report uncorrectable error events to userspace ---
commit 51a0403024c186a2a443b450caaf0da16723e651
Author: Riana Tauro <riana.tauro@intel.com>

drm/xe/xe_ras: Report uncorrectable error events to userspace

When the firmware reports uncorrectable errors, this patch delivers a
drm-ras error event to userspace for each affected component. Multiple
errors for the same component are collapsed to avoid duplicate events.

> diff --git a/drivers/gpu/drm/xe/xe_ras.c b/drivers/gpu/drm/xe/xe_ras.c
> index cf85075d62667..462cbb548f07b 100644
> --- a/drivers/gpu/drm/xe/xe_ras.c
> +++ b/drivers/gpu/drm/xe/xe_ras.c
> @@ -378,6 +378,7 @@ enum xe_ras_recovery_action xe_ras_process_errors(struct xe_device *xe)
>  	enum xe_ras_recovery_action final_action;
>  	u32 remaining = XE_SYSCTRL_FLOOD_LIMIT;
>  	struct xe_ras_get_soc_error response;
> +	u8 sent = 0;

[ ... ]

> @@ -421,6 +422,12 @@ enum xe_ras_recovery_action xe_ras_process_errors(struct xe_device *xe)
>  			xe_info(xe, "[RAS]: %s %s detected\n", comp_to_str(component),
>  				sev_to_str(severity));
>  
> +			/* Send event once per component */
> +			if (!(sent & BIT(component))) {
> +				sent |= BIT(component);

[Severity: Medium]
Is there a risk of undefined behavior or truncation here if the firmware
returns an unexpected component ID?

Since sent is a u8, any component value of 8 or greater will cause
BIT(component) to be truncated to 0 when bitwise OR'ed with sent. The
condition !(sent & BIT(component)) would then constantly evaluate to
true, bypassing the duplicate suppression.

Also, if component is greater than or equal to BITS_PER_LONG, BIT()
will cause an out-of-bounds shift. Should component be validated against a
maximum value before using it in the bit shift?

[Severity: Medium]
Does this deduplication logic unintentionally mask uncorrectable errors if
multiple errors for the same component arrive with different severities?

If the firmware responds with a correctable error followed by an
uncorrectable error for the exact same component, the first error sets
the bit in the sent mask. The subsequent uncorrectable error would then
be skipped, and userspace would only be notified of the correctable
severity.

> +				ras_send_error_event(xe, severity, component);

[Severity: High]
Can calling ras_send_error_event() here disrupt the firmware's pagination
state?

The xe_ras_process_errors() function retrieves errors using a stateful
XE_SYSCTRL_CMD_GET_SOC_ERROR command inside a do-while loop based on
response.additional_errors.

However, ras_send_error_event() synchronously sends a different mailbox
command (XE_SYSCTRL_CMD_GET_COUNTER):

xe_ras.c:ras_send_error_event() {
    ...
    ret = get_counter(xe, &counter, &value);
    ...
}

If xe_sysctrl_send_command() releases its mutex between calls, injecting
this GET_COUNTER command into the GET_SOC_ERROR pagination stream on the
wire might reset or corrupt the firmware's internal cursor. This could lead
to dropped errors or an infinite loop reading the same initial chunk.

> +			}
> +
>  			switch (component) {
>  			case XE_RAS_COMP_CORE_COMPUTE:
>  				action = handle_core_compute_errors(arr);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720082208.2648279-5-riana.tauro@intel.com?part=3