Re: [PATCH v12 4/4] drm/xe/ras: Use fault-inject to trigger punit error handler

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Real hardware errors (PUNIT/CSC) detected during probe trigger a broken KOBJ_CHANGE uevent with an unresolvable DEVPATH, dropping the event silently and breaking cold-reset recovery. The patch works around this exclusively for injected errors, leaving the real bug unhandled.
--

--- Patch [4]: [PATCH v12 4/4] drm/xe/ras: Use fault-inject to trigger punit error handler ---
commit 7bec129b8ce98871b6643acdf5450a3fe7c276cd
Author: Mallesh Koujalagi <mallesh.koujalagi@intel.com>

drm/xe/ras: Use fault-inject to trigger punit error handler

Use fault-inject framework to trigger punit_error_handler()
for testing.

> diff --git a/drivers/gpu/drm/xe/xe_ras.c b/drivers/gpu/drm/xe/xe_ras.c
> index 92b4181026cbe..da06c16cf7ed9 100644
> --- a/drivers/gpu/drm/xe/xe_ras.c
> +++ b/drivers/gpu/drm/xe/xe_ras.c
> @@ -391,6 +392,22 @@ enum xe_ras_recovery_action xe_ras_process_errors(struct xe_device *xe)
>  	size_t rlen;
>  	int ret;
>  
> +	/*
> +	 * Only allow the injected PUNIT error once the DRM device is registered.
> +	 * xe_ras_process_errors() also runs during probe (via xe_ras_init()), before
> +	 * drm_dev_register() calls device_add() on the DRM minor's kdev. At that
> +	 * point kdev->kobj.parent is not yet linked into the sysfs hierarchy, so
> +	 * kobject_get_path() returns "/card0" instead of the real sysfs path.
> +	 * drm_dev_wedged_event() would then emit a KOBJ_CHANGE uevent with a wrong
> +	 * DEVPATH that udev cannot resolve, silently dropping the event and leaving
> +	 * the cold-reset recovery broken.
> +	 */
> +	if (xe->drm.registered && xe_fault_punit_error()) {
> +		xe_err(xe, "[RAS]: PUNIT error injected\n");
> +		punit_error_handler(xe);
> +		return XE_RAS_RECOVERY_ACTION_DISCONNECT;
> +	}

[Severity: High]
Does this same broken uevent issue also apply to real hardware errors detected
during probe?

The comment correctly identifies that calling punit_error_handler() before 
the DRM device is registered emits a KOBJ_CHANGE uevent with a wrong DEVPATH.
However, if a real PUNIT error is detected during probe, the code bypasses
this guard and proceeds to handle_soc_internal_errors():

drivers/gpu/drm/xe/xe_ras.c:xe_ras_process_errors() {
	...
	case XE_RAS_COMP_SOC_INTERNAL:
		action = handle_soc_internal_errors(xe, arr);
		break;
	...
}

Which unconditionally calls punit_error_handler() without the
xe->drm.registered check:

drivers/gpu/drm/xe/xe_ras.c:handle_soc_internal_errors() {
	...
	if (ieh_error->global_error_status & XE_RAS_SOC_IEH_PUNIT) {
		xe_err(xe, "[RAS]: PUNIT %s detected: 0x%x\n",
		       sev_to_str(counter->common.severity),
		       ieh_error->global_error_status);
		punit_error_handler(xe);
		return XE_RAS_RECOVERY_ACTION_DISCONNECT;
	}
	...
}

Should the underlying uevent emission in punit_error_handler() be delayed
or guarded until after registration, rather than only working around it for
injected errors?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260724100302.706685-6-mallesh.koujalagi@intel.com?part=4