Re: [PATCH v10 09/10] drm/xe/pci: Introduce PCIe Function Level Reset

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

On 26-08-2026 17:08, Raag Jadav wrote:
> On Wed, Aug 26, 2026 at 03:22:19PM +0530, Tauro, Riana wrote:
>> On 26-08-2026 14:31, Raag Jadav wrote:
>>> On Tue, Aug 25, 2026 at 12:06:32PM +0200, Laguna, Lukasz wrote:
>>>> On 8/21/2026 13:23, Raag Jadav wrote:
>>>>> With bare minimum pieces in place, we can finally introduce PCIe Function
>>>>> Level Reset (FLR) support 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>
>>>>> Tested-by: Lukasz Laguna <lukasz.laguna@intel.com>
>>>>> Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>>>>> ---
>>>>>     v2: Spell out Function Level Reset (Jani)
>>>>>     v5: Prevent PM ref leak for wedged device (Matthew Brost)
>>>>>     v6: Add PCIe FLR documentation (Daniele)
>>>>>     v7: Refine PCIe FLR documentation (Daniele)
>>>>>         Introduce xe_pci_reset_skip() helper (Lukasz)
>>>>>     v9: Add 'Xe' prefix to document title (Rodrigo)
>>>>> v10: Update documentation to include PCI Error Handling (Lukasz)
>>>>>         Maintain wedged reference on FLR failure (Lukasz)
>>>>> ---
>>>>>     drivers/gpu/drm/xe/xe_device_types.h |   3 +
>>>>>     drivers/gpu/drm/xe/xe_pci_error.c    | 128 +++++++++++++++++++++++++++
>>>>>     2 files changed, 131 insertions(+)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
>>>>> index 7be3f15bf7a0..8cbb5b747710 100644
>>>>> --- a/drivers/gpu/drm/xe/xe_device_types.h
>>>>> +++ b/drivers/gpu/drm/xe/xe_device_types.h
>>>>> @@ -518,6 +518,9 @@ struct xe_device {
>>>>>     	/** @pxp: Encapsulate Protected Xe Path support */
>>>>>     	struct xe_pxp *pxp;
>>>>> +	/** @flr_prepared: Prepared for function-reset */
>>>>> +	bool flr_prepared;
>>>>> +
>>>>>     	/** @needs_flr_on_fini: requests function-reset on fini */
>>>>>     	bool needs_flr_on_fini;
>>>>> diff --git a/drivers/gpu/drm/xe/xe_pci_error.c b/drivers/gpu/drm/xe/xe_pci_error.c
>>>>> index 48466d726eae..81c4e55fae8f 100644
>>>>> --- a/drivers/gpu/drm/xe/xe_pci_error.c
>>>>> +++ b/drivers/gpu/drm/xe/xe_pci_error.c
>>>>> @@ -12,8 +12,55 @@
>>>>>     #include "xe_pm.h"
>>>>>     #include "xe_printk.h"
>>>>>     #include "xe_ras.h"
>>>>> +#include "xe_sriov_pf_helpers.h"
>>>>>     #include "xe_survivability_mode.h"
>>>>> +/**
>>>>> + * DOC: Xe PCI Error Handling
>>>>> + *
>>>>> + * Xe driver registers PCI callbacks which are called by PCI core in case of
>>>>> + * bus errors or resets.
>>>>> + *
>>>>> + * Currently both Function Level Reset (FLR) and bus error handling callbacks are
>>>>> + * supported. Both wipe the VRAM and resets the state of all the hardware units.
>>>>> + * Therefore, the contents of all exec queues and BOs in VRAM are lost, and the
>>>>> + * hardware needs a full re-initialization. The way Xe driver handles it, is
>>>>> + * pretty much similar to system suspend/resume flow with a few notable exceptions.
>>>>> + *
>>>>> + * Prepare phase:
>>>>> + *
>>>>> + * - Temporarily wedge the device to prevent userspace access
>>>>> + * - Kill exec queues which signals all fences and frees in-flight jobs
>>>>> + * - Stop the scheduler and all submissions to GuC
>>>>> + * - The fact that FLR is needed is because hardware could be in corrupted state
>>>>> + *   and access unreliable, so skip memory eviction due to untrustworthy VRAM
>>>>> + *   contents
>>>>> + * - Remove all memory mappings since VRAM contents will be lost
>>>>> + *
>>>>> + * Re-initialization phase:
>>>>> + *
>>>>> + * - Recreate kernel BOs due to skipped memory eviction in prepare phase
>>>>> + * - Restore kernel queues which were killed in prepare phase
>>>>> + * - Reload all uC firmwares
>>>>> + * - Bring up all hardware units
>>>>> + * - Unwedge the device to allow userspace access
>>>>> + *
>>>>> + * Since VRAM contents are lost, the user is expected to recreate user memory
>>>>> + * and reload context.
>>>>> + *
>>>>> + * TODO: Reuse FLR callbacks for bus error handling.
>>>>> + *
>>>>> + * Current implementation is only limited to re-initializing GT. This needs to
>>>>> + * be extended for a lot of components listed below.
>>>>> + *
>>>>> + * - Proper re-initialization of GSC and PXP for integrated platforms
>>>>> + * - SR-IOV cases which need PF and VF synchronization
>>>>> + * - Re-initialization of all child devices registered by Xe
>>>>> + * - Prepare existing xe_device_wedged() users for temporary wedging
>>>>> + * - MM corner cases
>>>>> + * - Display
>>>>> + */
>>>>> +
>>>>>     static void prepare_device_for_reset(struct pci_dev *pdev)
>>>>>     {
>>>>>     	struct xe_device *xe = pdev_to_xe_device(pdev);
>>>>> @@ -142,9 +189,90 @@ static void xe_pci_error_resume(struct pci_dev *pdev)
>>>>>     	xe_device_wedged_put(xe);
>>>>>     }
>>>>> +static inline bool xe_pci_reset_skip(struct xe_device *xe)
>>>>> +{
>> Add a TODO
> Sure.
>
>>>>> +	return !IS_DGFX(xe) || IS_SRIOV_VF(xe) || xe_sriov_pf_num_vfs(xe) || xe->info.probe_display;
>>>>> +}
>>>>> +
>>>>> +static void xe_pci_reset_prepare(struct pci_dev *pdev)
>>>>> +{
>>>>> +	struct xe_device *xe = pdev_to_xe_device(pdev);
>>>>> +	int err;
>>>>> +
>>>>> +	err = xe_pci_reset_skip(xe);
>>>>> +	if (err) {
>>>>> +		xe_err(xe, "PCIe FLR not supported\n");
>> Nit: Can the log be just PCIe reset
>  From PCI standpoint there are different types of resets, so better to be
> explicit.

I see bus-reset also using the reset_prepare/done callbacks.
I was thinking use the same name as function calls
It's a nit so upto you.

>
>>>>> +		goto wedge;
>>>> Looks like we always take the reference, so can't we do it at the top of
>>>> this function and only return here?
>>> Because we check for pre-existing ref below, in that case something else
>>> is already wrong and it's pointless to move forward. We take a local ref
>>> anyway because PCI core doesn't distinguish failure cases and calls
>>> ->reset_done() regardless, so we have to compensate for it by maintaining
>>> the ref across calls (which we later drop in ->reset_done()).
>>   If it is not supported why not just return in both function calls? Why
>> proceed?
> You mean allow userspace access while FLR is in progress?

If we do not quiesce the device and trigger a flr would it not cause 
other crashes anyway?

Thanks
Riana

>
> Raag
>
>>>>> +	}
>>>>> +
>>>>> +	err = xe_device_wedged(xe);
>>>>> +	if (err)
>>>>> +		xe_err(xe, "PCIe FLR failed, device in unexpected state\n");
>>>>> +
>>>>> +wedge:
>>>>> +	/* Wedge the device to prevent userspace access but don't send the event yet */
>>>>> +	xe_device_wedged_get(xe);
>>>>> +	if (err)
>>>>> +		return;
>>>>> +
>>>>> +	/*
>>>>> +	 * 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 system suspend flow but without eviction.
>>>>> +	 */
>>>>> +	err = xe_device_suspend(xe, true);
>>>>> +	if (err) {
>>>>> +		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);
>>>>> +	int err;
>>>>> +
>>>>> +	err = xe_pci_reset_skip(xe);
>>>>> +	if (err)
>>>>> +		goto out;
>>>>> +
>>>>> +	if (!xe->flr_prepared)
>>>>> +		goto out;
>>>>> +
>>>>> +	/* 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 system resume flow, except we'll also need to recreate
>>>>> +	 * kernel bos and restore kernel queues.
>>>>> +	 */
>>>>> +	err = xe_device_resume(xe, true);
>>>>> +	if (err) {
>>>>> +		xe_err(xe, "Re-initialization failed\n");
>>>>> +		goto out;
>>>>> +	}
>>>>> +
>>>>> +	/* Unwedge to allow userspace access */
>>>>> +	xe_device_wedged_put(xe);
>>>>> +	xe_info(xe, "Re-initialization success\n");
>>>>> +
>>>>> +	return;
>>>>> +out:
>>>>> +	/* Most likely the device is unusable and there's nothing we can do about it */
>>>>> +	xe_device_declare_wedged(xe);
>>>>> +	/* Drop local reference */
>>>>> +	xe_device_wedged_put(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,
>>>>> +	.reset_prepare	= xe_pci_reset_prepare,
>>>>> +	.reset_done	= xe_pci_reset_done,
>>>>>     };