Re: [PATCH v7 10/10] drm/xe/xe_ras: Report correctable errors using SIGID

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm/xe/ras: unrecognized log component for XE_RAS_COMP_RESERVED in ras_comp_to_log_comp()
--

commit 523a0553a8f4f634b0b558643739a2f201d23a02
Author: Badal Nilawar <badal.nilawar@intel.com>

drm/xe/xe_ras: Report correctable errors using SIGID

This commit replaces xe_warn() in xe_ras_counter_threshold_crossed() with
xe_log_from() to report RAS counter threshold crossings using hardware
SIGIDs and log components.

> diff --git a/drivers/gpu/drm/xe/xe_ras.c b/drivers/gpu/drm/xe/xe_ras.c
> index 58d3834345bc1..09b3c96042d5b 100644
> --- a/drivers/gpu/drm/xe/xe_ras.c
> +++ b/drivers/gpu/drm/xe/xe_ras.c
[ ... ]
> @@ -226,6 +228,12 @@ static int ras_status_to_errno(u32 status)
>  	}
>  }
>  
> +static u16 ras_comp_to_log_comp(u8 component)
> +{
> +	return FIELD_PREP(XE_LOG_COMPONENT_CLASS_MASK, XE_LOG_COMPONENT_CLASS_HARDWARE) |
> +	       FIELD_PREP(XE_LOG_COMPONENT_TYPE_MASK, component);
> +}
> +
>  static inline const char *sev_to_str(u8 severity)
>  {
>  	if (severity >= XE_RAS_SEV_MAX)
> @@ -714,8 +722,11 @@ void xe_ras_counter_threshold_crossed(struct xe_device *xe,
>  		if (!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),
> +			    ras_comp_to_log_comp(component),
> +			    &errors[id], sizeof(errors[id]),
> +			    "error detected\n");

[Severity: Medium]
What happens if the system controller reports a counter with component set
to XE_RAS_COMP_RESERVED (3)?

In drivers/gpu/drm/xe/xe_ras.c, ras_counter_is_valid() only checks the outer
bounds of component:

	if (!in_range(component, XE_RAS_COMP_NOT_SUPPORTED + 1, XE_RAS_COMP_MAX - 1)) {
		xe_err(xe, "sysctrl: unexpected component %u\n", component);
		return false;
	}

This allows XE_RAS_COMP_RESERVED (3) to pass validation. While
ras_comp_to_hw_sigid() uses a switch statement that falls back to
XE_SIGID_HW for unhandled values, ras_comp_to_log_comp() unconditionally
encodes component into XE_LOG_COMPONENT_TYPE_MASK under
XE_LOG_COMPONENT_CLASS_HARDWARE.

Because hardware component 3 is reserved and omitted from
DEFINE_XE_LOG_HARDWARE_COMPONENTS in drivers/gpu/drm/xe/abi/xe_log_abi.h:

#define DEFINE_XE_LOG_HARDWARE_COMPONENTS(define) \
	define(HARDWARE, 1, DEVICE_MEMORY, DEVICE_MEMORY, "Device Memory")	\
	define(HARDWARE, 2, CORE_COMPUTE, CORE_COMPUTE, "Core Compute")		\
	/*     HARDWARE, 3, RESERVED */						\
	define(HARDWARE, 4, PCIE, PCIE, "PCIe Interface")			\
	define(HARDWARE, 5, FABRIC, FABRIC, "Fabric")				\
	define(HARDWARE, 6, SOC_INTERNAL, SOC_INTERNAL, "SoC Internal")		\

passing that encoded value to xe_log_from() will miss all cases in
log_component_prefix() and hit log_unknown_component_prefix() in
drivers/gpu/drm/xe/xe_log.c:

	WARN(IS_ENABLED(CONFIG_DRM_XE_DEBUG), "LOG: unrecognized component %u.%u\n", class, type);

Should ras_counter_is_valid() reject XE_RAS_COMP_RESERVED, or should
ras_comp_to_log_comp() handle unmapped components like
ras_comp_to_hw_sigid() does?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261029082752.1739281-12-badal.nilawar@intel.com?part=10