Re: [PATCH v9 4/7] drm/xe: Make xe_device_declare_wedged() IRQ safe

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/xe: PM runtime ref leak and state corruption due to unconditional wedge clear in PCI resume
--

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

drm/xe: Make xe_device_declare_wedged() IRQ safe

This commit makes xe_device_declare_wedged() safe to call in IRQ context
by deferring the uevent emission and other blocking calls to a dedicated
workqueue. This allows the device to be declared wedged on catastrophic
errors received via IRQ.

> diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
> index 8abe42aa7cb5d..4205cfee68a76 100644
> --- a/drivers/gpu/drm/xe/xe_device.c
> +++ b/drivers/gpu/drm/xe/xe_device.c

[ ... ]

> @@ -1423,22 +1439,21 @@ void xe_device_declare_wedged(struct xe_device *xe, unsigned long method)
>  			    "For recovery procedure, refer to %s\n"
>  			    "Please file a _new_ bug report at %s\n",
>  			    WEDGED_URL, XE_BUG_URL);
> -	}
>  
> -	for_each_initialized_gt(gt, xe, id)
> -		xe_gt_declare_wedged(gt);
> +		for_each_initialized_gt(gt, xe, id)
> +			xe_gt_declare_wedged(gt);
>  
> -	if (xe_device_wedged(xe)) {
>  		/*
>  		 * XE_WEDGED_MODE_UPON_ANY_HANG_NO_RESET is intended for debugging
>  		 * hangs, so wedge the device with 'none' recovery method and have
>  		 * it available to the user for debugging.
>  		 */
>  		if (xe->wedged.mode == XE_WEDGED_MODE_UPON_ANY_HANG_NO_RESET)
> -			method = DRM_WEDGE_RECOVERY_NONE;
> +			xe->wedged.method = DRM_WEDGE_RECOVERY_NONE;
> +		else
> +			xe->wedged.method = method;
>  
> -		/* Notify userspace of wedged device */
> -		drm_dev_wedged_event(&xe->drm, method, NULL);
> +		schedule_work(&xe->wedged.work);

[Severity: High]
Does this code leak the PM runtime reference and silently un-wedge the device
if a concurrent PCI error recovery happens?

Because this patch makes xe_device_declare_wedged() safe to call in IRQ
context, the race window with PCI error recovery is widened. When the device
is wedged, a PM reference is acquired at the start of this function:

drivers/gpu/drm/xe/xe_device.c:xe_device_declare_wedged() {
	if (!atomic_xchg(&xe->wedged.flag, 1)) {
		xe->needs_flr_on_fini = true;
		xe_pm_runtime_get_noresume(xe);
    ...

If xe_device_declare_wedged() is triggered by a fatal hardware error between
the PCI error recovery subsystem calling xe_pci_error_detected() and
xe_pci_error_resume(), the flag can be cleared unconditionally:

drivers/gpu/drm/xe/xe_pci_error.c:xe_pci_error_resume() {
    ...
    atomic_set(&xe->wedged.flag, 0);
    ...
}

This wipes out the concurrent wedging event without releasing the acquired
PM reference. Later, during device teardown, the cleanup function fails to
drop the leaked PM reference because the flag was incorrectly cleared:

drivers/gpu/drm/xe/xe_device.c:xe_device_wedged_fini() {
    ...
    if (atomic_read(&xe->wedged.flag))
        xe_pm_runtime_put(xe);
    ...
}

Can this lead to a permanent PM runtime reference leak and state corruption?

>  	}
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016031559.427941-1-raag.jadav@intel.com?part=4