Re: [PATCH 02/13] drm/xe: Separate AER reset state from device wedging

["Thomas Hellström" <thomas.hellstrom@linux.intel.com>]

On Thu, 2026-08-27 at 15:47 +0530, Arvind Yadav wrote:
> PCI error recovery currently uses xe->wedged.flag to block driver
> access. This mixes a temporary AER reset with a permanent device
> wedge.
> 
> If the device wedges during AER recovery, the wedge is not seen as
> the
> first transition. The AER resume callback may then clear the flag and
> make the permanently wedged device appear usable again.
> 
> Keep the old device blocked while slot reset removes it, and block
> the
> new device until the AER resume callback.
> 
> The old AER path took a runtime PM reference to balance
> xe_device_wedged_fini(), which drops one when wedged.flag is set. AER
> no
> longer sets that flag, so keeping the Xe-owned reference would leak
> it.
> pcie_do_recovery() holds a PCI-core runtime PM reference across the
> error_detected, slot_reset and resume callbacks.
> 
> Cc: Matthew Brost <matthew.brost@intel.com>
> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> Cc: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Arvind Yadav <arvind.yadav@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_bo.c            |  2 +-
>  drivers/gpu/drm/xe/xe_device.c        |  4 ++--
>  drivers/gpu/drm/xe/xe_device.h        | 12 ++++++++++++
>  drivers/gpu/drm/xe/xe_guc_ct.c        |  4 ++--
>  drivers/gpu/drm/xe/xe_guc_pc.c        | 10 +++++-----
>  drivers/gpu/drm/xe/xe_guc_rc.c        |  4 ++--
>  drivers/gpu/drm/xe/xe_guc_submit.c    |  8 ++++++--
>  drivers/gpu/drm/xe/xe_guc_tlb_inval.c |  8 +++++++-
>  drivers/gpu/drm/xe/xe_pci_error.c     | 22 +++++++++++-----------
>  drivers/gpu/drm/xe/xe_sriov_pf.c      |  2 +-
>  10 files changed, 49 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
> index dde309821237..b86cd6030ed6 100644
> --- a/drivers/gpu/drm/xe/xe_bo.c
> +++ b/drivers/gpu/drm/xe/xe_bo.c
> @@ -2094,7 +2094,7 @@ static vm_fault_t xe_bo_cpu_fault(struct
> vm_fault *vmf)
>  	int err = 0;
>  	int idx;
>  
> -	if (xe_device_wedged(xe) || !drm_dev_enter(&xe->drm, &idx))
> +	if (xe_device_io_blocked(xe) || !drm_dev_enter(&xe->drm,
> &idx))
>  		return ttm_bo_vm_dummy_page(vmf, vmf->vma-
> >vm_page_prot);
>  
>  	ret = xe_bo_cpu_fault_fastpath(vmf, xe, bo, needs_rpm);
> diff --git a/drivers/gpu/drm/xe/xe_device.c
> b/drivers/gpu/drm/xe/xe_device.c
> index 74d566693dfd..a92e90acdf0d 100644
> --- a/drivers/gpu/drm/xe/xe_device.c
> +++ b/drivers/gpu/drm/xe/xe_device.c
> @@ -225,7 +225,7 @@ static long xe_drm_ioctl(struct file *file,
> unsigned int cmd, unsigned long arg)
>  	struct xe_device *xe = to_xe_device(file_priv->minor->dev);
>  	long ret;
>  
> -	if (xe_device_wedged(xe))
> +	if (xe_device_io_blocked(xe))
>  		return -ECANCELED;
>  
>  	ACQUIRE(xe_pm_runtime_ioctl, pm)(xe);
> @@ -243,7 +243,7 @@ static long xe_drm_compat_ioctl(struct file
> *file, unsigned int cmd, unsigned lo
>  	struct xe_device *xe = to_xe_device(file_priv->minor->dev);
>  	long ret;
>  
> -	if (xe_device_wedged(xe))
> +	if (xe_device_io_blocked(xe))
>  		return -ECANCELED;
>  
>  	ACQUIRE(xe_pm_runtime_ioctl, pm)(xe);
> diff --git a/drivers/gpu/drm/xe/xe_device.h
> b/drivers/gpu/drm/xe/xe_device.h
> index 6c4cfaebc44a..a3f876c60d76 100644
> --- a/drivers/gpu/drm/xe/xe_device.h
> +++ b/drivers/gpu/drm/xe/xe_device.h
> @@ -212,6 +212,18 @@ static inline bool xe_device_wedged(struct
> xe_device *xe)
>  	return atomic_read(&xe->wedged.flag);
>  }
>  
> +/*
> + * Return true when device access must be blocked either permanently
> because
> + * the device is wedged or temporarily while PCI error recovery is
> running.
> + *
> + * Do not use this helper for one-way wedged-device decisions such
> as DMA
> + * isolation, IRQ resume suppression or recovery-method reporting.
> + */
> +static inline bool xe_device_io_blocked(struct xe_device *xe)
> +{
> +	return xe_device_wedged(xe) || xe_device_is_in_reset(xe);
> +}
> +
>  #ifdef CONFIG_DRM_XE_DEBUG_PAGE_SIZE
>  static inline bool xe_debug_page_size_supported(struct xe_device
> *xe)
>  {
> diff --git a/drivers/gpu/drm/xe/xe_guc_ct.c
> b/drivers/gpu/drm/xe/xe_guc_ct.c
> index 5c4733da385c..3c3fe4928fa2 100644
> --- a/drivers/gpu/drm/xe/xe_guc_ct.c
> +++ b/drivers/gpu/drm/xe/xe_guc_ct.c
> @@ -1062,7 +1062,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_device_wedged(ct_to_xe(ct))) {
> +	if (xe_device_io_blocked(ct_to_xe(ct))) {
>  		ret = -ENOTRECOVERABLE;
>  		goto out;
>  	}
> @@ -1813,7 +1813,7 @@ static int g2h_read(struct xe_guc_ct *ct, u32
> *msg, bool fast_path)
>  	xe_gt_assert(gt, xe_guc_ct_initialized(ct));
>  	lockdep_assert_held(&ct->fast_lock);
>  
> -	if (xe_device_wedged(xe))
> +	if (xe_device_io_blocked(xe))
>  		return -ENOTRECOVERABLE;
>  
>  	if (ct->state == XE_GUC_CT_STATE_DISABLED)
> diff --git a/drivers/gpu/drm/xe/xe_guc_pc.c
> b/drivers/gpu/drm/xe/xe_guc_pc.c
> index 097b075bd89a..9fe397296dc4 100644
> --- a/drivers/gpu/drm/xe/xe_guc_pc.c
> +++ b/drivers/gpu/drm/xe/xe_guc_pc.c
> @@ -188,7 +188,7 @@ static int pc_action_reset(struct xe_guc_pc *pc)
>  	int ret;
>  
>  	ret = xe_guc_ct_send(ct, action, ARRAY_SIZE(action), 0, 0);
> -	if (ret && !(xe_device_wedged(pc_to_xe(pc)) && ret == -
> ECANCELED))
> +	if (ret && !(xe_device_io_blocked(pc_to_xe(pc)) && ret == -
> ECANCELED))
>  		xe_gt_err(pc_to_gt(pc), "GuC PC reset failed:
> %pe\n",
>  			  ERR_PTR(ret));
>  
> @@ -212,7 +212,7 @@ static int pc_action_query_task_state(struct
> xe_guc_pc *pc)
>  
>  	/* Blocking here to ensure the results are ready before
> reading them */
>  	ret = xe_guc_ct_send_block(ct, action, ARRAY_SIZE(action));
> -	if (ret && !(xe_device_wedged(pc_to_xe(pc)) && ret == -
> ECANCELED))
> +	if (ret && !(xe_device_io_blocked(pc_to_xe(pc)) && ret == -
> ECANCELED))
>  		xe_gt_err(pc_to_gt(pc), "GuC PC query task state
> failed: %pe\n",
>  			  ERR_PTR(ret));
>  
> @@ -235,7 +235,7 @@ static int pc_action_set_param(struct xe_guc_pc
> *pc, u8 id, u32 value)
>  		return -EAGAIN;
>  
>  	ret = xe_guc_ct_send(ct, action, ARRAY_SIZE(action), 0, 0);
> -	if (ret && !(xe_device_wedged(pc_to_xe(pc)) && ret == -
> ECANCELED))
> +	if (ret && !(xe_device_io_blocked(pc_to_xe(pc)) && ret == -
> ECANCELED))
>  		xe_gt_err(pc_to_gt(pc), "GuC PC set param[%u]=%u
> failed: %pe\n",
>  			  id, value, ERR_PTR(ret));
>  
> @@ -257,7 +257,7 @@ static int pc_action_unset_param(struct xe_guc_pc
> *pc, u8 id)
>  		return -EAGAIN;
>  
>  	ret = xe_guc_ct_send(ct, action, ARRAY_SIZE(action), 0, 0);
> -	if (ret && !(xe_device_wedged(pc_to_xe(pc)) && ret == -
> ECANCELED))
> +	if (ret && !(xe_device_io_blocked(pc_to_xe(pc)) && ret == -
> ECANCELED))
>  		xe_gt_err(pc_to_gt(pc), "GuC PC unset param failed:
> %pe",
>  			  ERR_PTR(ret));
>  
> @@ -1357,7 +1357,7 @@ static void xe_guc_pc_fini_hw(void *arg)
>  	struct xe_guc_pc *pc = arg;
>  	struct xe_device *xe = pc_to_xe(pc);
>  
> -	if (xe_device_wedged(xe))
> +	if (xe_device_io_blocked(xe))
>  		return;
>  
>  	xe_guc_pc_stop(pc);
> diff --git a/drivers/gpu/drm/xe/xe_guc_rc.c
> b/drivers/gpu/drm/xe/xe_guc_rc.c
> index 99fa127b261f..eb5ec443f7ee 100644
> --- a/drivers/gpu/drm/xe/xe_guc_rc.c
> +++ b/drivers/gpu/drm/xe/xe_guc_rc.c
> @@ -40,7 +40,7 @@ static int guc_action_setup_gucrc(struct xe_guc
> *guc, u32 control)
>  	int ret;
>  
>  	ret = xe_guc_ct_send(&guc->ct, action, ARRAY_SIZE(action),
> 0, 0);
> -	if (ret && !(xe_device_wedged(guc_to_xe(guc)) && ret == -
> ECANCELED))
> +	if (ret && !(xe_device_io_blocked(guc_to_xe(guc)) && ret ==
> -ECANCELED))
>  		xe_gt_err(guc_to_gt(guc),
>  			  "GuC RC setup %s(%u) failed (%pe)\n",
>  			   control == GUCRC_HOST_CONTROL ?
> "HOST_CONTROL" :
> @@ -73,7 +73,7 @@ static void xe_guc_rc_fini_hw(void *arg)
>  	struct xe_device *xe = guc_to_xe(guc);
>  	struct xe_gt *gt = guc_to_gt(guc);
>  
> -	if (xe_device_wedged(xe))
> +	if (xe_device_io_blocked(xe))
>  		return;
>  
>  	CLASS(xe_force_wake, fw_ref)(gt_to_fw(gt), XE_FW_GT);
> diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c
> b/drivers/gpu/drm/xe/xe_guc_submit.c
> index 99d8c807ff05..a307af458cf8 100644
> --- a/drivers/gpu/drm/xe/xe_guc_submit.c
> +++ b/drivers/gpu/drm/xe/xe_guc_submit.c
> @@ -2452,7 +2452,7 @@ static int
> guc_exec_queue_wait_suspend_done(struct xe_exec_queue *q, bool blocki
>  						       WAIT_COND, HZ
> * 5);
>  	}
>  
> -	if (!blocking && vf_recovery(guc) && !xe_device_wedged(xe))
> +	if (!blocking && vf_recovery(guc) &&
> !xe_device_io_blocked(xe))
>  		return -EAGAIN;
>  
>  	if (!ret)
> @@ -2694,7 +2694,11 @@ int xe_guc_submit_reset_prepare(struct xe_guc
> *guc)
>  
>  void xe_guc_submit_reset_wait(struct xe_guc *guc)
>  {
> -	wait_event(guc->ct.wq, xe_device_wedged(guc_to_xe(guc)) ||
> +	/*
> +	 * AER sets in_reset before declaring the GT wedged, which
> wakes this
> +	 * waitqueue.
> +	 */
> +	wait_event(guc->ct.wq, xe_device_io_blocked(guc_to_xe(guc))
> ||
>  		   !xe_guc_read_stopped(guc));
>  }
>  
> diff --git a/drivers/gpu/drm/xe/xe_guc_tlb_inval.c
> b/drivers/gpu/drm/xe/xe_guc_tlb_inval.c
> index 046d0655122f..646e13671cd9 100644
> --- a/drivers/gpu/drm/xe/xe_guc_tlb_inval.c
> +++ b/drivers/gpu/drm/xe/xe_guc_tlb_inval.c
> @@ -34,6 +34,9 @@ static int send_tlb_inval(struct xe_guc *guc, const
> u32 *action, int len)
>  
>  	xe_gt_assert(gt, action[1]);	/* Seqno */
>  
> +	if (xe_device_io_blocked(guc_to_xe(guc)))
> +		return -ECANCELED;
> +
>  	xe_gt_stats_incr(gt, XE_GT_STATS_ID_TLB_INVAL, 1);
>  	return xe_guc_ct_send(&guc->ct, action, len,
>  			      G2H_LEN_DW_TLB_INVALIDATE, 1);
> @@ -69,6 +72,9 @@ static int send_tlb_inval_ggtt(struct xe_tlb_inval
> *tlb_inval, u32 seqno)
>  	 * signals waiters.
>  	 */
>  
> +	if (xe_device_io_blocked(xe))
> +		return -ECANCELED;
> +
>  	if (xe_guc_ct_enabled(&guc->ct) && guc-
> >submission_state.enabled) {
>  		u32 action[] = {
>  			XE_GUC_ACTION_TLB_INVALIDATION,
> @@ -77,7 +83,7 @@ static int send_tlb_inval_ggtt(struct xe_tlb_inval
> *tlb_inval, u32 seqno)
>  		};
>  
>  		return send_tlb_inval(guc, action,
> ARRAY_SIZE(action));
> -	} else if (xe_device_uc_enabled(xe) &&
> !xe_device_wedged(xe)) {
> +	} else if (xe_device_uc_enabled(xe)) {
>  		struct xe_mmio *mmio = &gt->mmio;
>  
>  		if (IS_SRIOV_VF(xe))
> diff --git a/drivers/gpu/drm/xe/xe_pci_error.c
> b/drivers/gpu/drm/xe/xe_pci_error.c
> index 79ce0c671549..d82256d8721f 100644
> --- a/drivers/gpu/drm/xe/xe_pci_error.c
> +++ b/drivers/gpu/drm/xe/xe_pci_error.c
> @@ -9,7 +9,6 @@
>  #include "xe_gt.h"
>  #include "xe_log.h"
>  #include "xe_pci.h"
> -#include "xe_pm.h"
>  #include "xe_printk.h"
>  #include "xe_ras.h"
>  #include "xe_survivability_mode.h"
> @@ -20,14 +19,15 @@ static void prepare_device_for_reset(struct
> pci_dev *pdev)
>  	struct xe_gt *gt;
>  	u8 id;
>  
> +
>  	/*
> -	 * Wedge the device to prevent userspace access but do not
> send the uevent.
> -	 * xe_device_wedged_fini() releases runtime pm if wedged
> flag is set, so acquire a runtime
> -	 * pm reference to avoid underflow.
> +	 * Block device access while PCI error recovery is in
> progress.
> +	 *
> +	 * The old runtime PM reference balanced
> xe_device_wedged_fini() while
> +	 * AER set wedged.flag. AER no longer sets that flag, and
> +	 * pcie_do_recovery() holds its own runtime PM reference
> across the
> +	 * recovery callbacks.

This is an in-code comment describing what this patch is doing. A
future code reader has no idea what "The old runtime PM reference" is.
Please keep comments involving the old pre-patch code in the commit
message.


>  	 */
> -	if (!atomic_xchg(&xe->wedged.flag, 1))
> -		xe_pm_runtime_get_noresume(xe);
> -
>  	xe_device_set_in_reset(xe);
>  
>  	for_each_gt(gt, xe, id)
> @@ -116,7 +116,6 @@ static pci_ers_result_t
> xe_pci_error_slot_reset(struct pci_dev *pdev)
>  	 * TODO: optimize by re-initializing only the hardware state
> and re-creating
>  	 * kernel BOs.
>  	 */
> -	xe_device_clear_in_reset(xe);
>  	pdev->driver->remove(pdev);
>  	devres_release_group(&pdev->dev, xe->devres_group);
>  
> @@ -125,8 +124,8 @@ 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);
> +	/* Block the new instance until the resume callback. */
> +	xe_device_set_in_reset(xe);
>  
>  	return PCI_ERS_RESULT_RECOVERED;
>  }
> @@ -137,7 +136,8 @@ static void xe_pci_error_resume(struct pci_dev
> *pdev)
>  
>  	xe_info(xe, "PCI error: resume\n");
>  
> -	atomic_set(&xe->wedged.flag, 0);
> +	/* Resume I/O operations. */
> +	xe_device_clear_in_reset(xe);
>  }
>  
>  const struct pci_error_handlers xe_pci_error_handlers = {
> diff --git a/drivers/gpu/drm/xe/xe_sriov_pf.c
> b/drivers/gpu/drm/xe/xe_sriov_pf.c
> index 33bd754d138f..568b7ed7c380 100644
> --- a/drivers/gpu/drm/xe/xe_sriov_pf.c
> +++ b/drivers/gpu/drm/xe/xe_sriov_pf.c
> @@ -157,7 +157,7 @@ int xe_sriov_pf_wait_ready(struct xe_device *xe)
>  	unsigned int id;
>  	int err;
>  
> -	if (xe_device_wedged(xe))
> +	if (xe_device_io_blocked(xe))
>  		return -ECANCELED;
>  
>  	for_each_gt(gt, xe, id) {


/Thomas