Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Unprotected concurrent access to `xe->wedged.method` can cause the `COLD_RESET` recovery method to be overwritten by the `DEFAULT` fallback method during simultaneous wedge declarations. -- --- Patch [3]: [PATCH v13 3/4] drm/xe: Handle PUNIT errors by requesting cold-reset recovery --- commit 5edfcf29ea367e4df826b556199c8154b0704d81 Author: Mallesh Koujalagi <[email protected]> drm/xe: Handle PUNIT errors by requesting cold-reset recovery This commit handles PUNIT errors by marking the device as wedged with DRM_WEDGE_RECOVERY_COLD_RESET, indicating that a full device power cycle is required to restore normal operation. > diff --git a/drivers/gpu/drm/xe/xe_ras.c b/drivers/gpu/drm/xe/xe_ras.c > index 352f056fd9ef4..3033a377bace9 100644 > --- a/drivers/gpu/drm/xe/xe_ras.c > +++ b/drivers/gpu/drm/xe/xe_ras.c [ ... ] > @@ -254,6 +254,12 @@ static u8 handle_core_compute_errors(struct > xe_ras_error_array *arr) > return XE_RAS_RECOVERY_ACTION_RECOVERED; > } > > +static void punit_error_handler(struct xe_device *xe) > +{ > + xe_device_set_wedged_method(xe, DRM_WEDGE_RECOVERY_COLD_RESET); > + xe_device_declare_wedged(xe); > +} ^^^^ [Severity: High] This is a pre-existing issue, but can the COLD_RESET recovery method be lost here due to unprotected concurrent access to xe->wedged.method? If a GT hang occurs concurrently with a PUNIT error, a race condition can overwrite the COLD_RESET escalation with the fallback method: CPU1 (GT hang) xe_device_declare_wedged() // Evaluates as true (sees 0) if (!xe->wedged.method) CPU2 (PUNIT error) punit_error_handler() xe_device_set_wedged_method(xe, DRM_WEDGE_RECOVERY_COLD_RESET); CPU1 (GT hang) // Overwrites COLD_RESET xe_device_set_wedged_method(xe, DRM_WEDGE_RECOVERY_REBIND | DRM_WEDGE_RECOVERY_BUS_RESET); Because xe->wedged.method is assigned directly and read in a check-then-act pattern without locking, both threads might pass the incorrect fallback method to drm_dev_wedged_event(). Could this cause userspace to perform a generic bus reset instead of the required full power cycle? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=3
