Re: [PATCH v3 11/12] drm/xe/xe_ras: Report correctable errors using SIGID

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] Raw hardware component ID passed to xe_log_from() instead of a structured log component identifier.

Pre-existing issues:
- [High] The function xe_emit_hardware_error_cper() contains an inverted condition that unconditionally skips hardware CPER emission.
--

commit 477ea5ac3298dd8da692c324d242eff3e7e49b28
Author: Badal Nilawar <badal.nilawar@intel.com>

drm/xe/xe_ras: Report correctable errors using SIGID
    
Use xe_log_* helper to report correctable errors.

> diff --git a/drivers/gpu/drm/xe/xe_ras.c b/drivers/gpu/drm/xe/xe_ras.c
> index e93a989fc8f66..8cd6544b5336c 100644
> --- a/drivers/gpu/drm/xe/xe_ras.c
> +++ b/drivers/gpu/drm/xe/xe_ras.c
> @@ -464,8 +464,11 @@ void xe_ras_counter_threshold_crossed(struct xe_device *xe,
>  		if (!xe_ras_counter_is_valid(xe, &errors[id]))
>  			continue;
>  
> -		xe_warn(xe, "[RAS]: %s %s detected\n",
> -			comp_to_str(component), sev_to_str(severity));
> +		xe_log_from(xe, ras_sev_to_cper_sev(severity),
> +			    ras_comp_to_hw_sigid(component),
> +			    component,

[Severity: Medium]
In drivers/gpu/drm/xe/xe_ras.c:xe_ras_counter_threshold_crossed(), does this
code pass the raw hardware component ID to xe_log_from() instead of a
structured log component identifier?

Passing the raw 8-bit component enum value instead of a properly formatted
value could trigger a runtime WARN trace when debug is enabled, and produce a
confusing "DRIVER? " prefix in the dmesg log instead of the correct hardware
component name.

> +			    &errors[id], sizeof(errors[id]),
> +			    "error detected\n");
>  
>  		/* Send event once per component */
>  		if (sent & BIT(component))

[Severity: High]
This is a pre-existing issue, but does xe_emit_hardware_error_cper() in
drivers/gpu/drm/xe/xe_cper.c skip CPER emission for valid hardware error
signatures?

drivers/gpu/drm/xe/xe_cper.c:xe_emit_hardware_error_cper() {
    ...
	if (!xe)
		return;

	if ((int)sigid >= INTEL_SIGID_GPU_XE_HARDWARE_START)
		return;

	if (!counter || !xe_ras_counter_is_valid(xe, counter))
		return;
    ...
}

It looks like the condition if ((int)sigid >= INTEL_SIGID_GPU_XE_HARDWARE_START)
might be inverted.  This appears to cause hardware RAS errors to silently fail
to generate CPER records when sigid is a valid hardware signature ID.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260906172604.2215987-14-badal.nilawar@intel.com?part=11