Re: [PATCH v6 7/7] drm/xe/ras: Move xe_ras_process_errors() to xe_ras

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] drm/xe: early wedging causes probe failure and breaks survivability mode
- [Medium] drm/xe/ras: fault injection bypassed on devices without sysctrl
--

commit f19cf587dc3dbb4d3eeb0716be575b1dc0369223
Author: Raag Jadav <raag.jadav@intel.com>

drm/xe/ras: Move xe_ras_process_errors() to xe_ras
    
This patch moves xe_ras_process_errors() from xe_device_probe() to
xe_ras_init(), reverting a previous commit that delayed ras error processing.

> diff --git a/drivers/gpu/drm/xe/xe_ras.c b/drivers/gpu/drm/xe/xe_ras.c
> index 922e5e793b135..8e399ffec9856 100644
> --- a/drivers/gpu/drm/xe/xe_ras.c
> +++ b/drivers/gpu/drm/xe/xe_ras.c
> @@ -932,6 +932,13 @@ void xe_ras_init(struct xe_device *xe)
>  	if (IS_ENABLED(CONFIG_PCIEAER))
>  		ras_usp_aer_init(xe);
>  
> +	/*
> +	 * During probe, process and log any errors detected by firmware while the driver was not
> +	 * loaded. Critical errors such as Punit and CSC are reported through Pcode init failure,
> +	 * causing the driver to enter survivability mode.
> +	 */
> +	xe_ras_process_errors(xe);
> +

[Severity: High]
Does moving xe_ras_process_errors() here cause probe failure on boot-time
errors?

xe_ras_init() is called before xe_gt_init() during probe. If
xe_ras_process_errors() detects a hardware error and wedges the device,
subsequent calls in xe_device_probe() such as xe_gt_init() will fail.

For example, when xe_gt_init() calls into GuC CT:

drivers/gpu/drm/xe/xe_guc_ct.c:__guc_ct_send_locked() {
	if (xe_wedged(ct_to_xe(ct))) {
		ret = -ENOTRECOVERABLE;
		goto out;
	}
	...
}

This failure propagates up, causing the driver probe sequence to abort
entirely instead of binding the device in a wedged state. This prevents
access to the sysfs files required for userspace firmware recovery (Runtime
Survivability Mode) and drops the WEDGED uevent.

[Severity: Medium]
Will this bypass the fault injection check on devices without a system
controller?

Earlier in xe_ras_init(), there is an early return:

drivers/gpu/drm/xe/xe_ras.c:xe_ras_init() {
	...
	if (!xe->info.has_sysctrl)
		return;
	...
}

Because of this early return, xe_ras_process_errors() will never be called
on platforms where has_sysctrl is false.

However, xe_ras_process_errors() checks for cold-reset fault injection
before checking for has_sysctrl:

drivers/gpu/drm/xe/xe_ras.c:xe_ras_process_errors() {
	if (xe_fault_wedge_cold_reset()) {
		xe_err(xe, "[RAS]: cold-reset wedge injected\n");
		punit_error_handler(xe);
		return XE_RAS_RECOVERY_ACTION_DISCONNECT;
	}

	if (!xe->info.has_sysctrl)
		return XE_RAS_RECOVERY_ACTION_RESET;
	...
}

Placing xe_ras_process_errors() after the has_sysctrl check in xe_ras_init()
appears to silently break cold-reset fault injection during probe on these
platforms.

>  	ret = devm_device_add_group(xe->drm.dev, &gpu_health_group);
>  	if (ret)
>  		xe_err(xe, "Failed to create GPU health sysfs, err=%d\n", ret);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014055215.330408-1-raag.jadav@intel.com?part=7