Re: [PATCH v5 9/9] drm/xe/pci: Introduce PCIe FLR

[Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>]

On 4/6/2026 7:07 AM, Raag Jadav wrote:
> With bare minimum pieces in place, we can finally introduce PCIe Function
> Level Reset (FLR) handling which re-initializes hardware state without the
> need for reloading the driver from userspace. All VRAM contents are lost
> along with hardware state and driver takes care of recreating the required
> kernel bos as part of re-initialization, but user still needs to recreate
> user bos and reload context after PCIe FLR.
>
> Signed-off-by: Raag Jadav <raag.jadav@intel.com>
> ---
> v2: Spell out Function Level Reset (Jani)
> v5: Prevent PM ref leak for wedged device (Matthew Brost)
> ---
>   drivers/gpu/drm/xe/Makefile          |   1 +
>   drivers/gpu/drm/xe/xe_device_types.h |   3 +
>   drivers/gpu/drm/xe/xe_pci.c          |   1 +
>   drivers/gpu/drm/xe/xe_pci.h          |   2 +
>   drivers/gpu/drm/xe/xe_pci_err.c      | 160 +++++++++++++++++++++++++++
>   5 files changed, 167 insertions(+)
>   create mode 100644 drivers/gpu/drm/xe/xe_pci_err.c
>
> diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
> index f9abaf687d46..06b5d53e1629 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_err.o \
>   	xe_pci_rebar.o \
>   	xe_pcode.o \
>   	xe_pm.o \
> diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
> index 150c76b2acaf..b743b3986205 100644
> --- a/drivers/gpu/drm/xe/xe_device_types.h
> +++ b/drivers/gpu/drm/xe/xe_device_types.h
> @@ -482,6 +482,9 @@ struct xe_device {
>   	/** @needs_flr_on_fini: requests function-reset on fini */
>   	bool needs_flr_on_fini;
>   
> +	/** @flr_prepared: Prepared for function-reset */
> +	bool flr_prepared;
> +
>   	/** @wedged: Struct to control Wedged States and mode */
>   	struct {
>   		/** @wedged.flag: Xe device faced a critical error and is now blocked. */
> diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
> index 26eb58e11056..f3515c91e534 100644
> --- a/drivers/gpu/drm/xe/xe_pci.c
> +++ b/drivers/gpu/drm/xe/xe_pci.c
> @@ -1332,6 +1332,7 @@ static struct pci_driver xe_pci_driver = {
>   #ifdef CONFIG_PM_SLEEP
>   	.driver.pm = &xe_pm_ops,
>   #endif
> +	.err_handler = &xe_pci_err_handlers,
>   };
>   
>   /**
> diff --git a/drivers/gpu/drm/xe/xe_pci.h b/drivers/gpu/drm/xe/xe_pci.h
> index 11bcc5fe2c5b..85e85e8508c3 100644
> --- a/drivers/gpu/drm/xe/xe_pci.h
> +++ b/drivers/gpu/drm/xe/xe_pci.h
> @@ -8,6 +8,8 @@
>   
>   struct pci_dev;
>   
> +extern const struct pci_error_handlers xe_pci_err_handlers;
> +
>   int xe_register_pci_driver(void);
>   void xe_unregister_pci_driver(void);
>   struct xe_device *xe_pci_to_pf_device(struct pci_dev *pdev);
> diff --git a/drivers/gpu/drm/xe/xe_pci_err.c b/drivers/gpu/drm/xe/xe_pci_err.c
> new file mode 100644
> index 000000000000..339e8688d37f
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/xe_pci_err.c
> @@ -0,0 +1,160 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2026 Intel Corporation
> + */
> +
> +#include "xe_bo_evict.h"
> +#include "xe_device.h"
> +#include "xe_gt.h"
> +#include "xe_gt_idle.h"
> +#include "xe_i2c.h"
> +#include "xe_irq.h"
> +#include "xe_late_bind_fw.h"
> +#include "xe_pci.h"
> +#include "xe_pcode.h"
> +#include "xe_printk.h"
> +#include "xe_pxp.h"
> +#include "xe_wa.h"
> +
> +/* TODO: Extend support as a follow-up */
> +#define XE_FLR_SKIP		(!IS_DGFX(xe) || IS_SRIOV_VF(xe) || pci_num_vf(pdev) || \
> +				 xe->info.probe_display)

A comment here would be useful to explain why we're limited to those cases.
Also, I think it'd be better to define the macro as:

XE_FLR_SKIP(xe_, pdev_)

As that is more in line with the rest of the code.

> +
> +static int xe_flr_prepare(struct xe_device *xe)
> +{
> +	struct xe_gt *gt;
> +	int err;
> +	u8 id;
> +
> +	err = xe_pxp_pm_suspend(xe->pxp);
> +	if (err)
> +		return err;
> +
> +	xe_late_bind_wait_for_worker_completion(&xe->late_bind);
> +
> +	xe_irq_disable(xe);
> +
> +	for_each_gt(gt, xe, id)
> +		xe_gt_flr_prepare(gt);
> +
> +	// TODO: Drop all user bos

Nit: comment style

> +	xe_bo_pci_dev_remove_pinned(xe);
> +	unmap_mapping_range(xe->drm.anon_inode->i_mapping, 0, 0, 1);
> +
> +	return 0;
> +}
> +
> +static int xe_flr_done(struct xe_device *xe)

IMO this should be renamed to xe_reinit_post_flr or something like that.

> +{
> +	struct xe_tile *tile;
> +	struct xe_gt *gt;
> +	int err;
> +	u8 id;
> +
> +	for_each_gt(gt, xe, id)
> +		xe_gt_idle_disable_c6(gt);
> +
> +	for_each_tile(tile, xe, id)
> +		xe_wa_apply_tile_workarounds(tile);
> +
> +	err = xe_pcode_ready(xe, true);
> +	if (err)
> +		return err;
> +
> +	xe_device_assert_lmem_ready(xe);
> +
> +	err = xe_bo_restore_map(xe);
> +	if (err)
> +		return err;
> +
> +	for_each_gt(gt, xe, id) {
> +		err = xe_gt_flr_done(gt);
> +		if (err)
> +			return err;
> +	}
> +
> +	xe_i2c_pm_resume(xe, true);
> +
> +	xe_irq_resume(xe);
> +
> +	for_each_gt(gt, xe, id) {
> +		err = xe_gt_resume(gt);
> +		if (err)
> +			return err;
> +	}
> +
> +	xe_pxp_pm_resume(xe->pxp);
> +
> +	xe_late_bind_fw_load(&xe->late_bind);

A very large chunk of this code is identical to xe_pm_resume(). Can we 
move it to a common function (e.g., xe_device_resume or something)? You 
can use the flr_prepared flag (or just pass in a parameter) to know if 
it's a resume from FLR or from suspend to handle the differences.

Daniele

> +
> +	return 0;
> +}
> +
> +static void xe_pci_reset_prepare(struct pci_dev *pdev)
> +{
> +	struct xe_device *xe = pdev_to_xe_device(pdev);
> +
> +	if (XE_FLR_SKIP) {
> +		xe_err(xe, "PCIe FLR not supported\n");
> +		return;
> +	}
> +
> +	if (xe_device_wedged(xe)) {
> +		xe_err(xe, "PCIe FLR aborted, device in unexpected state\n");
> +		return;
> +	}
> +
> +	/* Wedge the device to prevent userspace access but don't send the event yet */
> +	atomic_set(&xe->wedged.flag, 1);
> +
> +	/*
> +	 * The hardware could be in corrupted state and access unreliable, but we try to
> +	 * update data structures and cleanup any pending work to avoid side effects during
> +	 * PCIe FLR. This will be similar to xe_pm_suspend() flow but without migration.
> +	 */
> +	if (xe_flr_prepare(xe)) {
> +		xe_err(xe, "Failed to prepare for PCIe FLR\n");
> +		return;
> +	}
> +
> +	xe->flr_prepared = true;
> +	xe_info(xe, "Prepared for PCIe FLR\n");
> +}
> +
> +static void xe_pci_reset_done(struct pci_dev *pdev)
> +{
> +	struct xe_device *xe = pdev_to_xe_device(pdev);
> +
> +	if (XE_FLR_SKIP)
> +		return;
> +
> +	if (!xe_device_wedged(xe) || !xe->flr_prepared)
> +		return;
> +
> +	/* Unprepare early in case we fail */
> +	xe->flr_prepared = false;
> +
> +	/*
> +	 * We already have the data structures intact, so try to re-initialize the device.
> +	 * This will be similar to xe_pm_resume() flow, except we'll also need to recreate
> +	 * all VRAM contents.
> +	 */
> +	if (xe_flr_done(xe)) {
> +		xe_err(xe, "Re-initialization failed\n");
> +		return;
> +	}
> +
> +	/* Unwedge to allow userspace access */
> +	atomic_set(&xe->wedged.flag, 0);
> +
> +	xe_info(xe, "Re-initialization success\n");
> +}
> +
> +/*
> + * PCIe Function Level Reset (FLR) support only.
> + * TODO: Add PCIe error handlers using similar flow.
> + */
> +const struct pci_error_handlers xe_pci_err_handlers = {
> +	.reset_prepare = xe_pci_reset_prepare,
> +	.reset_done = xe_pci_reset_done,
> +};