Re: [PATCH v4 5/5] drm/xe/ras: Move xe_ras_process_errors() to xe_ras

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] xe: early wedging causes probe failure and dropped uevents
--

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

drm/xe/ras: Move xe_ras_process_errors() to xe_ras
    
This commit moves xe_ras_process_errors() from xe_device_probe() to
xe_ras_init() to process and log boot-time RAS errors earlier in the
probe sequence.

> diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
> --- a/drivers/gpu/drm/xe/xe_device.c
> +++ b/drivers/gpu/drm/xe/xe_device.c
> @@ -1087,13 +1087,6 @@ int xe_device_probe(struct xe_device *xe)
>  
>  	detect_preproduction_hw(xe);
>  
> -	/*
> -	 * Process and log any errors detected by hardware. Possible results can
> -	 * include declaring the device as wedged, which must be done only after
> -	 * xe_wedge_fini() is registered.
> -	 */
> -	xe_ras_process_errors(xe);
> -
>  	err = devm_add_action_or_reset(xe->drm.dev, xe_device_sanitize, xe);
>  	if (err)
>  		goto err_unregister_display;

[ ... ]

> diff --git a/drivers/gpu/drm/xe/xe_ras.c b/drivers/gpu/drm/xe/xe_ras.c
> --- a/drivers/gpu/drm/xe/xe_ras.c
> +++ b/drivers/gpu/drm/xe/xe_ras.c
> @@ -933,6 +933,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 the driver to fail probe instead
of remaining loaded in a wedged state?

If xe_ras_process_errors() encounters a fatal error, it sets the wedged flag.
Because xe_ras_init() now runs before xe_gt_init(), subsequent GuC CT
operations during GT initialization will fail due to the wedged state:

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

This would cause xe_device_probe() to abort entirely.

Additionally, when the wedge worker calls drm_dev_wedged_event(), won't the
uevent be silently dropped?

drivers/gpu/drm/drm_drv.c:drm_dev_wedged_event() {
	...
	return kobject_uevent_env(&dev->primary->kdev->kobj, KOBJ_CHANGE, envp);
}

At this point in xe_ras_init(), drm_dev_register() hasn't been called yet, so
the DRM device kobject hasn't been added to sysfs. The original location in
xe_device_probe() was placed safely after GT initialization and DRM device
registration.

> +
>  	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/20261010090718.68308-1-raag.jadav@intel.com?part=5