Re: [PATCH v2 2/5] 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] Uninitialized work_struct leads to list corruption and kernel panic if an IRQ triggers during device probe.
--

commit 7cfc61b6d67f6291a4e51db20047caa91bfb3397
Author: Raag Jadav <raag.jadav@intel.com>

drm/xe: Make xe_device_declare_wedged() IRQ safe

This commit defers wedge handling into a dedicated worker so that
xe_device_declare_wedged() can be called in IRQ context. This is required
for declaring the device as wedged on catastrophic errors received via IRQs.

> diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
> index 396d02eb2af83..150629ee4ad13 100644
> --- a/drivers/gpu/drm/xe/xe_device.c
> +++ b/drivers/gpu/drm/xe/xe_device.c
[ ... ]
> @@ -1148,7 +1159,7 @@ int xe_device_probe(struct xe_device *xe)
>  
>  	detect_preproduction_hw(xe);
>  
> -	err = drmm_add_action_or_reset(&xe->drm, xe_device_wedged_fini, xe);
> +	err = xe_device_wedged_init(xe);

[Severity: High]
Does this initialize the work_struct too late in the probe sequence?

If a hardware error interrupt (like a MERT CAT error) fires during the window
after interrupts are enabled (via xe_irq_install() called earlier in
xe_device_probe()) but before this initialization completes, the interrupt
handler will call xe_device_declare_wedged().

This would attempt to schedule the uninitialized xe->wedged.work structure,
potentially leading to list corruption and a kernel panic in the workqueue
core.

>  	if (err)
>  		goto err_unregister_display;
[ ... ]
> @@ -1483,26 +1514,11 @@ void xe_device_declare_wedged(struct xe_device *xe)
>  			    "For recovery procedure, refer to %s\n"
>  			    "Please file a _new_ bug report at %s\n",
>  			    WEDGED_URL, XE_BUG_URL);
>  
> -		for_each_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)
> -			xe_device_set_wedged_method(xe, DRM_WEDGE_RECOVERY_NONE);
> -		/* If no wedge recovery method is set, use default */
> -		else if (!xe->wedged.method)
> -			xe_device_set_wedged_method(xe, DRM_WEDGE_RECOVERY_REBIND |
> -						    DRM_WEDGE_RECOVERY_BUS_RESET);
> -
> -		/* Notify userspace of wedged device */
> -		drm_dev_wedged_event(&xe->drm, xe->wedged.method, NULL);
> +		schedule_work(&xe->wedged.work);

[Severity: High]
Could this schedule an uninitialized work_struct?

If xe_device_declare_wedged() is invoked from an early interrupt handler
before xe_device_wedged_init() completes, this will attempt to queue
xe->wedged.work. 

Additionally, the loop right before this calls xe_gt_declare_wedged(gt),
which could queue an uninitialized gt->wedge.worker if xe_gt_init() hasn't
completed for that GT yet.

>  	}
>  }

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