Re: [PATCH v2 07/15] drm/xe: Send wedged notification from a worker

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] xe: TOCTOU race in xe_device_declare_wedged() overwrites recovery method
--

commit 8a75aea67f604c5a996849e643c64617844ddad3
Author: Arvind Yadav <arvind.yadav@intel.com>
Subject: drm/xe: Send wedged notification from a worker

This commit moves the wedged event to a sleepable device worker to allow
isolation work to complete before userspace is notified. It updates the
wedge bookkeeping to rescan the GT state on later declarations and tracks
the last reported method to avoid lost updates.

> 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

[ ... ]

> @@ -1435,14 +1482,22 @@ void xe_device_declare_wedged(struct xe_device *xe)
>  		 */
>  		if (xe->wedged.mode == XE_WEDGED_MODE_UPON_ANY_HANG_NO_RESET)
>  			xe_device_set_wedged_method(xe, DRM_WEDGE_RECOVERY_NONE);
>  	}
> +
> +	/* Re-scan GT submission state on every declaration. */
> +	for_each_gt(gt, xe, id)
> +		xe_gt_declare_wedged(gt);
> +
> +	/* If no wedge recovery method is set, use default */
> +	if (!READ_ONCE(xe->wedged.method))
> +		xe_device_set_wedged_method(xe, DRM_WEDGE_RECOVERY_REBIND |
> +					    DRM_WEDGE_RECOVERY_BUS_RESET);

[Severity: Medium]
Is there a Time-of-Check to Time-of-Use (TOCTOU) race condition here in
xe_device_declare_wedged()?

If multiple threads invoke xe_device_declare_wedged() concurrently (for
example, during simultaneous GT hangs), could Thread A evaluate the
!READ_ONCE(xe->wedged.method) condition as true, get preempted, and then
allow Thread B to successfully set a specific method like
DRM_WEDGE_RECOVERY_NONE?

If Thread A then resumes, it seems it would unconditionally call
xe_device_set_wedged_method() with the default REBIND | BUS_RESET flags,
silently overwriting Thread B's legitimately set method.

Does this check-and-set sequence need atomic protection, such as cmpxchg,
to prevent concurrent threads from overwriting a newly set recovery method
with the default fallback?

> +
> +	if (!atomic_read(&xe->wedged.stopping) &&
> +	    (first ||
> +	     READ_ONCE(xe->wedged.method) !=
> +	     READ_ONCE(xe->wedged.reported_method)))
> +		queue_work(xe->unordered_wq, &xe->wedged.work);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260922101721.1583542-1-arvind.yadav@intel.com?part=7