Re: [PATCH v9 08/10] drm/xe: Improve wedged state management

[Raag Jadav <raag.jadav@intel.com>]

On Thu, Jul 09, 2026 at 02:27:40PM -0700, Matthew Brost wrote:
> On Wed, Jul 01, 2026 at 01:59:32PM +0530, Raag Jadav wrote:
> > Currently, wedged state is serving a single usecase where the device is
> > permanently declared wedged, but this doesn't allow any wedged state
> > management for runtime usecases. In preparation of usecases which require
> > to facilitate temporary device wedging, convert wedged.flag to wedged.ref
> > which serves as a driver internal refcount for wedged state and blocks
> > critical path execution during device lifetime. While at it, introduce
> > wedged.perm which signifies permanent device wedging and operates
> > independent of the refcount allowing relevant cleanup action on unwind
> > path.
> > 
> > Signed-off-by: Raag Jadav <raag.jadav@intel.com>
> > ---
> > v9: Redesign to prevent races
> > ---
> >  drivers/gpu/drm/xe/xe_device.c       | 102 ++++++++++++++++++++-------
> >  drivers/gpu/drm/xe/xe_device.h       |   8 +--
> >  drivers/gpu/drm/xe/xe_device_types.h |   8 ++-
> >  3 files changed, 84 insertions(+), 34 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
> > index c5e3e2d1f7c3..e8b6afb29ab6 100644
> > --- a/drivers/gpu/drm/xe/xe_device.c
> > +++ b/drivers/gpu/drm/xe/xe_device.c
> > @@ -598,6 +598,10 @@ int xe_device_init_early(struct xe_device *xe)
> >  	if (err)
> >  		return err;
> >  
> > +	err = drmm_mutex_init(&xe->drm, &xe->wedged.lock);
> > +	if (err)
> > +		return err;
> > +
> >  	err = xe_pm_init_early(xe);
> >  	if (err)
> >  		return err;
> > @@ -920,8 +924,10 @@ static void xe_device_wedged_fini(struct drm_device *drm, void *arg)
> >  {
> >  	struct xe_device *xe = arg;
> >  
> > -	if (atomic_read(&xe->wedged.flag))
> > -		xe_pm_runtime_put(xe);
> > +	if (xe->wedged.perm)
> > +		xe_device_wedged_put(xe);
> > +
> > +	xe_assert(xe, !xe_device_wedged(xe));
> >  }
> >  
> >  int xe_device_probe(struct xe_device *xe)
> > @@ -1387,6 +1393,44 @@ u64 xe_device_uncanonicalize_addr(struct xe_device *xe, u64 address)
> >   *   firmware and restore device to normal operation.
> >   */
> >  
> > +/**
> > + * xe_device_wedged_get() - Acquire wedged reference
> > + * @xe: xe device instance
> > + *
> > + * Get runtime PM reference and block critical path execution.
> > + */
> > +void xe_device_wedged_get(struct xe_device *xe)
> > +{
> > +	int ref;
> > +
> > +	/* We should not be runtime suspending as long as critical paths are blocked */
> > +	xe_pm_runtime_get_noresume(xe);
> > +
> > +	ref = atomic_inc_return(&xe->wedged.ref);
> > +	xe_assert(xe, ref > 0);
> > +}
> > +
> > +/**
> > + * xe_device_wedged_put() - Relinquish wedged reference
> > + * @xe: xe device instance
> > + *
> > + * Restore critical path execution and put runtime PM reference.
> > + */
> > +void xe_device_wedged_put(struct xe_device *xe)
> > +{
> > +	int ref;
> > +
> > +	ref = atomic_dec_return(&xe->wedged.ref);
> > +	xe_assert(xe, ref >= 0);
> > +
> > +	xe_pm_runtime_put(xe);
> > +}
> > +
> > +bool xe_device_wedged(struct xe_device *xe)
> > +{
> > +	return atomic_read(&xe->wedged.ref);
> > +}
> > +
> >  /**
> >   * xe_device_set_wedged_method - Set wedged recovery method
> >   * @xe: xe device instance
> > @@ -1427,36 +1471,40 @@ void xe_device_declare_wedged(struct xe_device *xe)
> >  		return;
> >  	}
> >  
> > -	if (!atomic_xchg(&xe->wedged.flag, 1)) {
> > -		xe->needs_flr_on_fini = true;
> > -		xe_pm_runtime_get_noresume(xe);
> > -		drm_err(&xe->drm,
> > -			"CRITICAL: Xe has declared device %s as wedged.\n"
> > -			"IOCTLs and executions are blocked.\n"
> > -			"For recovery procedure, refer to https://docs.kernel.org/gpu/drm-uapi.html#device-wedging\n"
> > -			"Please file a _new_ bug report at https://gitlab.freedesktop.org/drm/xe/kernel/issues/new\n",
> > -			dev_name(xe->drm.dev));
> > -	}
> > +	mutex_lock(&xe->wedged.lock);
> 
> Sashiko pointed out what is likely a pre existing issue:
> 
> 
> Can this mutex_lock() trigger a scheduling while atomic panic?
> Looking at the call chain, xe_device_declare_wedged() can be invoked from
> a hardirq context when a hardware error occurs:
> xe_irq_handler()
>   xe_mert_irq_handler()
>     mert_handle_cat_error()
>       xe_device_declare_wedged()
> Since the interrupt handler runs in atomic context, acquiring a sleeping
> lock here causes a crash.
> Could this synchronization be handled without introducing a mutex in the
> interrupt path?

Yes, the event call performs memory allocation for environment buffer
so mert was broken to begin with. But I'm not sure if that in scope of
series? Perhaps fix it separately? Michal?

> This is an issue and currently unsafe too as we take mutexes deeper in
> xe_device_declare_wedged call stack too. Maybe declare wedged in an
> async work item? We have other issues in xe_device_declare_wedged too
> which stop / start scheduling which also isn't safe unless this is on
> the GT ordered work queue too.

Yes, I have planned this as part of my wedging improvement series once we
land this. It'll be reusing some parts of patch 3 where we use GT ordered
wq for it.

Raag

> > +
> > +	if (xe->wedged.perm)
> > +		goto out;
> > +
> > +	xe_device_wedged_get(xe);
> > +	xe->wedged.perm = true;
> > +	xe->needs_flr_on_fini = true;
> > +	drm_err(&xe->drm,
> > +		"CRITICAL: Xe has declared device %s as wedged.\n"
> > +		"IOCTLs and executions are blocked.\n"
> > +		"For recovery procedure, refer to https://docs.kernel.org/gpu/drm-uapi.html#device-wedging\n"
> > +		"Please file a _new_ bug report at https://gitlab.freedesktop.org/drm/xe/kernel/issues/new\n",
> > +		dev_name(xe->drm.dev));
> >  
> >  	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);
> > +	/*
> > +	 * 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);
> > -	}
> > +	/* Notify userspace of wedged device */
> > +	drm_dev_wedged_event(&xe->drm, xe->wedged.method, NULL);
> > +out:
> > +	mutex_unlock(&xe->wedged.lock);
> >  }
> >  
> >  /**
> > diff --git a/drivers/gpu/drm/xe/xe_device.h b/drivers/gpu/drm/xe/xe_device.h
> > index 11b67820eb67..89bf909c96e0 100644
> > --- a/drivers/gpu/drm/xe/xe_device.h
> > +++ b/drivers/gpu/drm/xe/xe_device.h
> > @@ -194,11 +194,9 @@ bool xe_device_is_l2_flush_optimized(struct xe_device *xe);
> >  void xe_device_td_flush(struct xe_device *xe);
> >  void xe_device_l2_flush(struct xe_device *xe);
> >  
> > -static inline bool xe_device_wedged(struct xe_device *xe)
> > -{
> > -	return atomic_read(&xe->wedged.flag);
> > -}
> > -
> > +void xe_device_wedged_get(struct xe_device *xe);
> > +void xe_device_wedged_put(struct xe_device *xe);
> > +bool xe_device_wedged(struct xe_device *xe);
> >  void xe_device_set_wedged_method(struct xe_device *xe, unsigned long method);
> >  void xe_device_declare_wedged(struct xe_device *xe);
> >  int xe_device_validate_wedged_mode(struct xe_device *xe, unsigned int mode);
> > diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
> > index 46a9e9fad7a9..80fa60821a47 100644
> > --- a/drivers/gpu/drm/xe/xe_device_types.h
> > +++ b/drivers/gpu/drm/xe/xe_device_types.h
> > @@ -485,14 +485,18 @@ struct xe_device {
> >  
> >  	/** @wedged: Struct to control Wedged States and mode */
> >  	struct {
> > -		/** @wedged.flag: Xe device faced a critical error and is now blocked. */
> > -		atomic_t flag;
> > +		/** @wedged.ref: Refcount for wedged device, blocks critical path execution */
> > +		atomic_t ref;
> >  		/** @wedged.mode: Mode controlled by kernel parameter and debugfs */
> >  		enum xe_wedged_mode mode;
> >  		/** @wedged.method: Recovery method to be sent in the drm device wedged uevent */
> >  		unsigned long method;
> >  		/** @wedged.inconsistent_reset: Inconsistent reset policy state between GTs */
> >  		bool inconsistent_reset;
> > +		/** @wedged.perm: Permanently wedged, needs cleanup on fini */
> > +		bool perm;
> > +		/** @wedged.lock: Lock protecting wedged state */
> > +		struct mutex lock;
> >  	} wedged;
> >  
> >  	/** @bo_device: Struct to control async free of BOs */
> > -- 
> > 2.43.0
> >