Re: [PATCH 2/2] drm/xe/xe_guc: Skip GuC reset post SBR

["Teres Alexis, Alan Previn" <alan.previn.teres.alexis@intel.com>]

On Fri, 2026-08-28 at 17:01 +0530, Riana Tauro wrote:
> Secondary Bus Reset causes all VRAM state to be lost along with hardware
> state. The goal is to keep the device in reset till the teardown is
> complete to avoid device accesses such as GuC reset in an unknown state.
> 
> Fixes: e46ee82f120f ("drm/xe: Skip device access during PCI error recovery")
> Signed-off-by: Riana Tauro <riana.tauro@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_device.h    | 5 -----
>  drivers/gpu/drm/xe/xe_guc.c       | 5 +++++
>  drivers/gpu/drm/xe/xe_pci_error.c | 1 -
>  3 files changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_device.h b/drivers/gpu/drm/xe/xe_device.h
> index 6c4cfaebc44a..a1127ab7d1a2 100644
> --- a/drivers/gpu/drm/xe/xe_device.h
> +++ b/drivers/gpu/drm/xe/xe_device.h
> @@ -191,11 +191,6 @@ static inline void xe_device_set_in_reset(struct xe_device *xe)
>  	atomic_set(&xe->in_reset, 1);
>  }
>  
> -static inline void xe_device_clear_in_reset(struct xe_device *xe)
> -{
> -	atomic_set(&xe->in_reset, 0);
> -}
> -
>  u32 xe_device_ccs_bytes(struct xe_device *xe, u64 size);
>  
>  void xe_device_snapshot_print(struct xe_device *xe, struct drm_printer *p);
> diff --git a/drivers/gpu/drm/xe/xe_guc.c b/drivers/gpu/drm/xe/xe_guc.c
> index c7f8bbd4cb92..2c7f411b89a7 100644
> --- a/drivers/gpu/drm/xe/xe_guc.c
> +++ b/drivers/gpu/drm/xe/xe_guc.c
> @@ -986,6 +986,11 @@ int xe_guc_reset(struct xe_guc *guc)
>  	u32 guc_status, gdrst;
>  	int ret;
>  
> +	if (xe_device_is_in_reset(gt_to_xe(gt))) {
> +		xe_gt_dbg(gt, "Skipping GuC reset, device is in reset\n");
> +		return 0;
> +	}
> +
alan: am i understanding correctly that in order to eliminate any race conditions,
we dont do the xe_device_clear as apret of the xe_pci_error_slot_reset... and thus
the xe_device_is_in_reset() check will always be true until the reprobe?.

Two questions:

1. Im not familiar with the SBR reset and need to understand more about the state transitions
of the GT and GuC before, during and after this reset sequence. Question: how does this
runtime SBR resolve the sudden mismatch of the driver state vs the hw state? if the old
xe-driver instance is just completely freed with no more CPU activity, and the reprobe
generates a new xe-driver instamce, why are we not just using wedged_mode before the SBR?
since "xe_device_is_in_reset" would need to be literred all over the place and we dont
need to duplicate further "points of checking" - just reuse the same places where we
check if we are wedged?

2. why this extra checking in "xe_guc_reset"? i believe most of not all runtime gt/guc reset
paths would catch the wedged-mode or in-reset mode much earlier in call stack before reaching this
point. Are we trying to focus only on eliminating that final mmio register touch? If so, why not
put this insode the mmio read/write wrappers?

...alan

>  	xe_force_wake_assert_held(gt_to_fw(gt), XE_FW_GT);
>  
>  	if (IS_SRIOV_VF(gt_to_xe(gt)))
> diff --git a/drivers/gpu/drm/xe/xe_pci_error.c b/drivers/gpu/drm/xe/xe_pci_error.c
> index 3e9c77f8483d..b7ac33965150 100644
> --- a/drivers/gpu/drm/xe/xe_pci_error.c
> +++ b/drivers/gpu/drm/xe/xe_pci_error.c
> @@ -121,7 +121,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);
>