Re: [PATCH v11 08/10] drm/xe: Introduce temporary device wedging

["Tauro, Riana" <riana.tauro@intel.com>]

On 26-10-2026 10:36, 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 (Sashiko)
> v10: Spell out temporary device wedging in subject (Lukasz)
> v11: Update GuC CT path to check against permanent wedging (Lukasz, Riana)
>       Put wedge reference only if device is wedged during resume() (Riana)
> ---
>   drivers/gpu/drm/xe/xe_guc_ct.c      |  6 +--
>   drivers/gpu/drm/xe/xe_pci.c         |  2 +
>   drivers/gpu/drm/xe/xe_pci_error.c   | 18 +++++++--
>   drivers/gpu/drm/xe/xe_wedge.c       | 58 +++++++++++++++++++++++++++--
>   drivers/gpu/drm/xe/xe_wedge.h       |  5 ++-
>   drivers/gpu/drm/xe/xe_wedge_types.h | 13 ++++++-
>   6 files changed, 88 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_guc_ct.c b/drivers/gpu/drm/xe/xe_guc_ct.c
> index d42f0133c1eb..152fccf8f6f1 100644
> --- a/drivers/gpu/drm/xe/xe_guc_ct.c
> +++ b/drivers/gpu/drm/xe/xe_guc_ct.c
> @@ -1162,7 +1162,7 @@ static int __guc_ct_send_locked(struct xe_guc_ct *ct, const u32 *action,
>   	xe_gt_assert(gt, g2h_len || !num_g2h);
>   	lockdep_assert_held(&ct->lock);
>   
> -	if (xe_wedged(ct_to_xe(ct))) {
> +	if (xe_wedge_not_recoverable(ct_to_xe(ct))) {
>   		ret = -ENOTRECOVERABLE;
>   		goto out;
>   	}
> @@ -1591,7 +1591,7 @@ static int guc_ct_send_recv(struct xe_guc_ct *ct, const u32 *action, u32 len,
>   	if (g2h_fence.fail) {
>   		if (g2h_fence.cancel) {
>   			xe_gt_dbg(gt, "H2G request %#x canceled!\n", action[0]);
> -			ret = xe_wedged(ct_to_xe(ct)) ? -ENOTRECOVERABLE : -ECANCELED;
> +			ret = xe_wedge_not_recoverable(ct_to_xe(ct)) ? -ENOTRECOVERABLE : -ECANCELED;
>   			goto unlock;
>   		}
>   		xe_gt_err(gt, "H2G request %#x failed: error %#x hint %#x\n",
> @@ -1936,7 +1936,7 @@ static int g2h_read(struct xe_guc_ct *ct, u32 *msg, bool fast_path)
>   	lockdep_assert_held(&ct->fast_lock);
>   
>   	/* Keep in sync with g2h_err_is_fatal() */
> -	if (xe_wedged(xe))
> +	if (xe_wedge_not_recoverable(xe))
>   		return -ENOTRECOVERABLE;
>   
>   	if (ct->state == XE_GUC_CT_STATE_DISABLED)
> diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
> index 605c452a9c33..531d38bcfd1c 100644
> --- a/drivers/gpu/drm/xe/xe_pci.c
> +++ b/drivers/gpu/drm/xe/xe_pci.c
> @@ -39,6 +39,7 @@
>   #include "xe_survivability_mode.h"
>   #include "xe_tile.h"
>   #include "xe_tile_printk.h"
> +#include "xe_wedge.h"
>   
>   enum toggle_d3cold {
>   	D3COLD_DISABLE,
> @@ -1400,6 +1401,7 @@ static int xe_pci_runtime_suspend(struct device *dev)
>   	 */
>   	xe_assert(xe, !IS_SRIOV_VF(xe));
>   	xe_assert(xe, !pci_num_vf(pdev));
> +	xe_assert(xe, !xe_wedged(xe));
>   
>   	err = xe_pm_runtime_suspend(xe);
>   	if (err)
> diff --git a/drivers/gpu/drm/xe/xe_pci_error.c b/drivers/gpu/drm/xe/xe_pci_error.c
> index 9bf0da3a3d15..cdd0af4ec139 100644
> --- a/drivers/gpu/drm/xe/xe_pci_error.c
> +++ b/drivers/gpu/drm/xe/xe_pci_error.c
> @@ -26,8 +26,7 @@ static void prepare_device_for_reset(struct pci_dev *pdev)
>   	 * xe_wedge_fini() releases runtime pm if wedged flag is set, so acquire a runtime
>   	 * pm reference to avoid underflow.
>   	 */
> -	if (!atomic_xchg(&xe->wedged.flag, 1))
> -		xe_pm_runtime_get_noresume(xe);
> +	xe_wedge_get(xe);
>   
>   	xe_device_set_in_reset(xe);
>   
> @@ -41,10 +40,16 @@ static void prepare_device_for_reset(struct pci_dev *pdev)
>   
>   static pci_ers_result_t ras_action_to_pci_result(struct pci_dev *pdev, u8 action)
>   {
> +	struct xe_device *xe = xe_any_to_xe(pdev);
> +
>   	switch (action) {
>   	case XE_RAS_RECOVERY_ACTION_RECOVERED:
>   		return PCI_ERS_RESULT_RECOVERED;
>   	case XE_RAS_RECOVERY_ACTION_RESET:
> +		/* We should not be doing such ugly hacks, reuse FLR callbacks and drop this. */
> +		if (atomic_xchg(&xe->wedged.perm, 1))
> +			return PCI_ERS_RESULT_DISCONNECT;
> +
>   		prepare_device_for_reset(pdev);
>   		return PCI_ERS_RESULT_NEED_RESET;
>   	case XE_RAS_RECOVERY_ACTION_DISCONNECT:
> @@ -71,6 +76,10 @@ static pci_ers_result_t xe_pci_error_detected(struct pci_dev *pdev, pci_channel_
>   	case pci_channel_io_normal:
>   		return PCI_ERS_RESULT_CAN_RECOVER;
>   	case pci_channel_io_frozen:
> +		/* We should not be doing such ugly hacks, reuse FLR callbacks and drop this. */
> +		if (atomic_xchg(&xe->wedged.perm, 1))
> +			return PCI_ERS_RESULT_DISCONNECT;
> +
>   		prepare_device_for_reset(pdev);
>   		return PCI_ERS_RESULT_NEED_RESET;
>   	default:
> @@ -129,7 +138,7 @@ static pci_ers_result_t xe_pci_error_slot_reset(struct pci_dev *pdev)
>   	xe = pdev_to_xe_device(pdev);
>   
>   	/* Wedge the device to prevent I/O operations till the resume callback */
> -	atomic_set(&xe->wedged.flag, 1);
> +	xe_wedge_get(xe);
>   
>   	return PCI_ERS_RESULT_RECOVERED;
>   }
> @@ -140,7 +149,8 @@ static void xe_pci_error_resume(struct pci_dev *pdev)
>   
>   	xe_info(xe, "PCI error: resume\n");
>   
> -	atomic_set(&xe->wedged.flag, 0);
> +	if (xe_wedged(xe))
> +		xe_wedge_put(xe);
>   }
>   
>   const struct pci_error_handlers xe_pci_error_handlers = {
> diff --git a/drivers/gpu/drm/xe/xe_wedge.c b/drivers/gpu/drm/xe/xe_wedge.c
> index 2f35937c5e66..49fd9251448b 100644
> --- a/drivers/gpu/drm/xe/xe_wedge.c
> +++ b/drivers/gpu/drm/xe/xe_wedge.c
> @@ -6,6 +6,7 @@
>   #include <drm/drm_drv.h>
>   #include <drm/drm_managed.h>
>   
> +#include "xe_assert.h"
>   #include "xe_defaults.h"
>   #include "xe_device.h"
>   #include "xe_gt.h"
> @@ -55,6 +56,53 @@
>    *   firmware and restore device to normal operation.
>    */
>   
> +/**
> + * xe_wedge_get() - Acquire wedge reference
> + * @xe: xe device instance
> + *
> + * Get runtime PM reference and block critical path execution.

Add a documentation that this is temporary should not be used unless 
required to temporarily block
userspace access like resets.

Shouldn't runtime pm be associated with permanent wedging instead of 
temporary?

> + */
> +void xe_wedge_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_wedge_put() - Relinquish wedge reference
> + * @xe: xe device instance
> + *
> + * Restore critical path execution and put runtime PM reference.
> + */
> +void xe_wedge_put(struct xe_device *xe)
> +{
> +	int ref;
> +
> +	ref = atomic_dec_return(&xe->wedged.ref);
> +	xe_assert(xe, ref >= 0);
> +
> +	xe_pm_runtime_put(xe);
> +}
> +
> +/**
> + * xe_wedge_not_recoverable() - Check for unrecoverable device
> + * @xe: xe device instance
> + *
> + * Do not abuse this. Use xe_wedged() instead where possible. Only
> + * use this as a last resort where xe_wedged() is problematic.
> + *
> + * Returns: %true if device is unrecoverable, %false otherwise.
> + */
> +bool xe_wedge_not_recoverable(struct xe_device *xe)

xe_wedged_permanent?

Thanks
Riana

> +{
> +	return atomic_read(&xe->wedged.perm);
> +}
> +
>   static void wedge_work(struct work_struct *work)
>   {
>   	struct xe_device *xe = container_of(work, struct xe_device, wedged.work);
> @@ -97,11 +145,11 @@ void xe_wedge_declare(struct xe_device *xe, unsigned long method)
>   		return;
>   	}
>   
> -	if (atomic_xchg(&xe->wedged.flag, 1))
> +	if (atomic_xchg(&xe->wedged.perm, 1))
>   		return;
>   
> +	xe_wedge_get(xe);
>   	xe->needs_flr_on_fini = true;
> -	xe_pm_runtime_get_noresume(xe);
>   
>   	xe_log_err_fatal(xe, WEDGED, -EIO, "Device declared wedged!\n");
>   	xe_err_once(xe, "IOCTLs and executions are now blocked!\n"
> @@ -239,8 +287,10 @@ static void xe_wedge_fini(struct drm_device *drm, void *arg)
>   
>   	disable_work_sync(&xe->wedged.work);
>   
> -	if (atomic_read(&xe->wedged.flag))
> -		xe_pm_runtime_put(xe);
> +	if (atomic_read(&xe->wedged.perm))
> +		xe_wedge_put(xe);
> +
> +	xe_assert(xe, !xe_wedged(xe));
>   }
>   
>   /**
> diff --git a/drivers/gpu/drm/xe/xe_wedge.h b/drivers/gpu/drm/xe/xe_wedge.h
> index 3b5499dfc30d..28f43295948e 100644
> --- a/drivers/gpu/drm/xe/xe_wedge.h
> +++ b/drivers/gpu/drm/xe/xe_wedge.h
> @@ -14,11 +14,14 @@
>   
>   static inline bool xe_wedged(struct xe_device *xe)
>   {
> -	return atomic_read(&xe->wedged.flag);
> +	return atomic_read(&xe->wedged.ref);
>   }
>   
>   void xe_wedge_declare(struct xe_device *xe, unsigned long method);
>   int xe_wedge_init(struct xe_device *xe);
>   int xe_wedge_set_mode(struct xe_device *xe, enum xe_wedged_mode mode);
> +void xe_wedge_get(struct xe_device *xe);
> +void xe_wedge_put(struct xe_device *xe);
> +bool xe_wedge_not_recoverable(struct xe_device *xe);
>   
>   #endif
> diff --git a/drivers/gpu/drm/xe/xe_wedge_types.h b/drivers/gpu/drm/xe/xe_wedge_types.h
> index e472c5fcbd26..8e87290444dd 100644
> --- a/drivers/gpu/drm/xe/xe_wedge_types.h
> +++ b/drivers/gpu/drm/xe/xe_wedge_types.h
> @@ -29,8 +29,17 @@ enum xe_wedged_mode {
>    * struct xe_wedge - Struct to control Wedged States and mode
>    */
>   struct xe_wedge {
> -	/** @flag: Xe device faced a critical error and is now blocked. */
> -	atomic_t flag;
> +	/**
> +	 * @ref: Refcount for wedged device, blocks critical path execution. Used for
> +	 * driver internal usecases which require temporarily blocking user access of
> +	 * the device during autonomous recovery, like PCIe FLR and bus reset.
> +	 */
> +	atomic_t ref;
> +	/**
> +	 * @perm: Permanently wedged device, not recoverable. Only to be set when
> +	 * notifying userspace of wedged device through drm wedged uevent.
> +	 */
> +	atomic_t perm;
>   	/** @mode: Mode controlled by kernel parameter and debugfs */
>   	enum xe_wedged_mode mode;
>   	/** @method: Recovery method to be sent in the drm device wedged uevent */