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

The current title seems to be too general. Please consider something 
like "drm/xe: Introduce temporary wedged state".


On 7/10/2026 07:01, Raag Jadav wrote:
> 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 <[email protected]>
>>> ---
>>> 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);
>>> +}
>>> +

missing doc

>>> +bool xe_device_wedged(struct xe_device *xe)
>>> +{
>>> +	return atomic_read(&xe->wedged.ref);
>>> +}

__guc_ct_send_locked() and g2h_read() return -ENOTRECOVERABLE when 
xe_device_wedged() is true, but with the new refcount approach device 
may only be temporarily wedged.


>>> +
>>>   /**
>>>    * 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.


xe_device_declare_wedged() is already unsafe from atomic context. I 
agree that it can be fixed in the follow-up series.

Lukasz


> 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
>>>