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 <[email protected]> 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/[email protected]?part=4
