Re: [PATCH v3 02/10] drm/xe/xe_pci_error: Implement PCI error recovery callbacks

Matthew Brost
2026-04-07T04:51:06Z

On Thu, Apr 02, 2026 at 12:31:33PM +0530, Riana Tauro wrote:
> Add error_detected, mmio_enabled, slot_reset and resume
> recovery callbacks to handle PCIe Advanced Error Reporting
> (AER) errors.
>
> For fatal errors, the device is wedged and becomes
> inaccessible. Return PCI_ERS_RESULT_SLOT_RESET from
> error_detected to request a Secondary Bus Reset (SBR).
>
> For non-fatal errors, return PCI_ERS_RESULT_CAN_RECOVER from
> error_detected to trigger the mmio_enabled callback. In this callback,
> the device is queried to determine the error cause and attempt
> recovery based on the error type.
>
> Once the secondary bus reset(SBR) is completed the slot_reset callback
> cleanly removes and reprobe the device to restore functionality.
>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Matthew Brost <matthew.brost@intel.com>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Signed-off-by: Riana Tauro <riana.tauro@intel.com>
> ---
> v2: re-order linux headers
> reword error messages
> do not clear in_recovery after remove
> return PCI_ERS_RESULT_DISCONNECT if probe fails (Michal)
> only wedge device do not send uevent (Raag)
> set recovery flag in error_detected and clear on resume
> add default switch case (Mallesh)
>
> v3: do not set in_recovery for disconnect (Mallesh)
> return if already wedged or in survivability mode
> ---
> drivers/gpu/drm/xe/Makefile | 1 +
> drivers/gpu/drm/xe/xe_device.h | 15 ++++
> drivers/gpu/drm/xe/xe_device_types.h | 3 +
> drivers/gpu/drm/xe/xe_pci.c | 3 +
> drivers/gpu/drm/xe/xe_pci_error.c | 104 +++++++++++++++++++++++++++
> 5 files changed, 126 insertions(+)
> create mode 100644 drivers/gpu/drm/xe/xe_pci_error.c
>
> diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
> index 9dacb0579a7d..7f03f06df186 100644
> --- a/drivers/gpu/drm/xe/Makefile
> +++ b/drivers/gpu/drm/xe/Makefile
> @@ -100,6 +100,7 @@ xe-y += xe_bb.o \
> xe_page_reclaim.o \
> xe_pat.o \
> xe_pci.o \
> + xe_pci_error.o \
> xe_pci_rebar.o \
> xe_pcode.o \
> xe_pm.o \
> diff --git a/drivers/gpu/drm/xe/xe_device.h b/drivers/gpu/drm/xe/xe_device.h
> index e4b9de8d8e95..60db2492cb92 100644
> --- a/drivers/gpu/drm/xe/xe_device.h
> +++ b/drivers/gpu/drm/xe/xe_device.h
> @@ -43,6 +43,21 @@ static inline struct xe_device *ttm_to_xe_device(struct ttm_device *ttm)
> return container_of(ttm, struct xe_device, ttm);
> }
>
> +static inline bool xe_device_is_in_recovery(struct xe_device *xe)
> +{
> + return atomic_read(&xe->in_recovery);
> +}
> +
> +static inline void xe_device_set_in_recovery(struct xe_device *xe)
> +{
> + atomic_set(&xe->in_recovery, 1);
> +}
> +
> +static inline void xe_device_clear_in_recovery(struct xe_device *xe)
> +{
> + atomic_set(&xe->in_recovery, 0);
> +}
> +
> struct xe_device *xe_device_create(struct pci_dev *pdev,
> const struct pci_device_id *ent);
> int xe_device_probe_early(struct xe_device *xe);
> diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
> index 150c76b2acaf..c9fe86b670bd 100644
> --- a/drivers/gpu/drm/xe/xe_device_types.h
> +++ b/drivers/gpu/drm/xe/xe_device_types.h
> @@ -494,6 +494,9 @@ struct xe_device {
> bool inconsistent_reset;
> } wedged;
>
> + /** @in_recovery: Indicates if device is in recovery */
> + atomic_t in_recovery;
> +
> /** @bo_device: Struct to control async free of BOs */
> struct xe_bo_dev {
> /** @bo_device.async_free: Free worker */
> diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
> index 1df3f08e2e1c..30d71795dd2e 100644
> --- a/drivers/gpu/drm/xe/xe_pci.c
> +++ b/drivers/gpu/drm/xe/xe_pci.c
> @@ -1323,6 +1323,8 @@ static const struct dev_pm_ops xe_pm_ops = {
> };
> #endif
>
> +extern const struct pci_error_handlers xe_pci_error_handlers;
> +
> static struct pci_driver xe_pci_driver = {
> .name = DRIVER_NAME,
> .id_table = pciidlist,
> @@ -1330,6 +1332,7 @@ static struct pci_driver xe_pci_driver = {
> .remove = xe_pci_remove,
> .shutdown = xe_pci_shutdown,
> .sriov_configure = xe_pci_sriov_configure,
> + .err_handler = &xe_pci_error_handlers,
> #ifdef CONFIG_PM_SLEEP
> .driver.pm = &xe_pm_ops,
> #endif
> diff --git a/drivers/gpu/drm/xe/xe_pci_error.c b/drivers/gpu/drm/xe/xe_pci_error.c
> new file mode 100644
> index 000000000000..cd9f39010278
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/xe_pci_error.c
> @@ -0,0 +1,104 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2026 Intel Corporation
> + */
> +#include <linux/pci.h>
> +
> +#include <drm/drm_drv.h>
> +
> +#include "xe_device.h"
> +#include "xe_gt.h"
> +#include "xe_pci.h"
> +#include "xe_survivability_mode.h"
> +#include "xe_uc.h"
> +
> +static void xe_pci_error_handling(struct pci_dev *pdev)
> +{
> + struct xe_device *xe = pdev_to_xe_device(pdev);
> + struct xe_gt *gt;
> + u8 id;
> +
> + /* Return if device is wedged or in survivability mode */
> + if (xe_survivability_mode_is_boot_enabled(xe) || xe_device_wedged(xe))
> + return;
> +
> + /* Wedge the device to prevent userspace access but don't send the event yet */
> + atomic_set(&xe->wedged.flag, 1);

We can't blindly set '&xe->wedged.flag, 1' as this is tied to a PM ref
[1], [2]. The existing sematic might be wrong but we to normalize
adjustmets to the '&xe->wedged.flag' field with uniform rules, or the
cases when we wedge we also take a PM ref.

Matt

[1] https://patchwork.freedesktop.org/patch/714622/?series=163948&rev=1
[2] https://patchwork.freedesktop.org/patch/715028/?series=162055&rev=4#comment_1315905

> +
> + for_each_gt(gt, xe, id)
> + xe_gt_declare_wedged(gt);
> +
> + pci_disable_device(pdev);
> +}
> +
> +static pci_ers_result_t xe_pci_error_detected(struct pci_dev *pdev, pci_channel_state_t state)
> +{
> + struct xe_device *xe = pdev_to_xe_device(pdev);
> +
> + dev_err(&pdev->dev, "Xe Pci error recovery: error detected state %d\n", state);
> +
> + if (state == pci_channel_io_perm_failure)
> + return PCI_ERS_RESULT_DISCONNECT;
> +
> + xe_device_set_in_recovery(xe);
> +
> + switch (state) {
> + case pci_channel_io_normal:
> + return PCI_ERS_RESULT_CAN_RECOVER;
> + case pci_channel_io_frozen:
> + xe_pci_error_handling(pdev);
> + return PCI_ERS_RESULT_NEED_RESET;
> + default:
> + dev_err(&pdev->dev, "Unknown state %d\n", state);
> + return PCI_ERS_RESULT_NEED_RESET;
> + }
> +}
> +
> +static pci_ers_result_t xe_pci_error_mmio_enabled(struct pci_dev *pdev)
> +{
> + dev_err(&pdev->dev, "Xe Pci error recovery: MMIO enabled\n");
> +
> + return PCI_ERS_RESULT_NEED_RESET;
> +}
> +
> +static pci_ers_result_t xe_pci_error_slot_reset(struct pci_dev *pdev)
> +{
> + const struct pci_device_id *ent = pci_match_id(pdev->driver->id_table, pdev);
> +
> + dev_err(&pdev->dev, "Xe Pci error recovery: Slot reset\n");
> +
> + pci_restore_state(pdev);
> +
> + if (pci_enable_device(pdev)) {
> + dev_err(&pdev->dev,
> + "Cannot re-enable PCI device after reset\n");
> + return PCI_ERS_RESULT_DISCONNECT;
> + }
> +
> + /*
> + * Secondary Bus Reset wipes out all device memory
> + * requiring XE KMD to perform a device removal and reprobe.
> + */
> + pdev->driver->remove(pdev);
> +
> + if (!pdev->driver->probe(pdev, ent))
> + return PCI_ERS_RESULT_RECOVERED;
> +
> + return PCI_ERS_RESULT_DISCONNECT;
> +}
> +
> +static void xe_pci_error_resume(struct pci_dev *pdev)
> +{
> + struct xe_device *xe = pdev_to_xe_device(pdev);
> +
> + dev_info(&pdev->dev, "Xe Pci error recovery: Recovered\n");
> +
> + xe_device_clear_in_recovery(xe);
> +}
> +
> +const struct pci_error_handlers xe_pci_error_handlers = {
> + .error_detected = xe_pci_error_detected,
> + .mmio_enabled = xe_pci_error_mmio_enabled,
> + .slot_reset = xe_pci_error_slot_reset,
> + .resume = xe_pci_error_resume,
> +};
> --
> 2.47.1
>