intel-xe.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
From: Raag Jadav <raag.jadav@intel.com>
To: "Tauro, Riana" <riana.tauro@intel.com>
Cc: "Laguna, Lukasz" <lukasz.laguna@intel.com>,
	intel-xe@lists.freedesktop.org, matthew.brost@intel.com,
	rodrigo.vivi@intel.com, thomas.hellstrom@linux.intel.com,
	michal.wajdeczko@intel.com, matthew.d.roper@intel.com,
	michal.winiarski@intel.com, matthew.auld@intel.com,
	dev@lankhorst.se, jani.nikula@intel.com, lukas@wunner.de,
	daniele.ceraolospurio@intel.com, badal.nilawar@intel.com
Subject: Re: [PATCH v10 09/10] drm/xe/pci: Introduce PCIe Function Level Reset
Date: Wed, 26 Aug 2026 13:38:45 +0200	[thread overview]
Message-ID: <ao7QRcbFlJxVxHHP@black.igk.intel.com> (raw)
In-Reply-To: <a2bb115f-80aa-480a-b8d4-c6f38c590e85@intel.com>

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.

> > > > +		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?

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,
> > > >    };

  reply	other threads:[~2026-08-26 11:38 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-21 11:23 [PATCH v10 00/10] Introduce Xe PCIe FLR Raag Jadav
2026-08-21 11:23 ` [PATCH v10 01/10] drm/xe/uc_fw: Allow re-initializing firmware Raag Jadav
2026-08-21 11:41   ` sashiko-bot
2026-08-21 11:23 ` [PATCH v10 02/10] drm/xe/guc_submit: Introduce guc_exec_queue_reinit_kernel() Raag Jadav
2026-08-21 11:52   ` sashiko-bot
2026-08-21 11:23 ` [PATCH v10 03/10] drm/xe/gt: Introduce FLR helpers Raag Jadav
2026-08-26  9:39   ` Tauro, Riana
2026-08-27 15:40     ` Raag Jadav
2026-08-21 11:23 ` [PATCH v10 04/10] drm/xe/bo_evict: Introduce xe_bo_restore_map() Raag Jadav
2026-08-21 11:23 ` [PATCH v10 05/10] drm/xe/exec_queue: Introduce xe_exec_queue_reinit() Raag Jadav
2026-08-21 11:43   ` sashiko-bot
2026-08-21 11:23 ` [PATCH v10 06/10] drm/xe/migrate: Introduce xe_migrate_reinit() Raag Jadav
2026-08-21 11:39   ` sashiko-bot
2026-08-21 11:23 ` [PATCH v10 07/10] drm/xe/pm: Introduce xe_device_suspend/resume() Raag Jadav
2026-08-21 11:43   ` sashiko-bot
2026-08-24 18:22     ` Rodrigo Vivi
2026-08-26  7:01       ` Raag Jadav
2026-08-21 11:23 ` [PATCH v10 08/10] drm/xe: Introduce temporary device wedging Raag Jadav
2026-08-21 11:37   ` sashiko-bot
2026-08-24 14:30     ` Laguna, Lukasz
2026-08-24 16:08       ` Raag Jadav
2026-08-24 18:29       ` Rodrigo Vivi
2026-08-25  7:41         ` Raag Jadav
2026-08-21 11:23 ` [PATCH v10 09/10] drm/xe/pci: Introduce PCIe Function Level Reset Raag Jadav
2026-08-21 11:39   ` sashiko-bot
2026-08-25 10:06   ` Laguna, Lukasz
2026-08-26  9:01     ` Raag Jadav
2026-08-26  9:52       ` Tauro, Riana
2026-08-26 11:38         ` Raag Jadav [this message]
2026-08-28  6:12           ` Tauro, Riana
2026-08-28  7:54             ` Raag Jadav
2026-08-21 11:23 ` [PATCH v10 10/10] drm/xe/doc: Wire up PCI Error Handling Raag Jadav
2026-08-21 11:31 ` ✗ CI.checkpatch: warning for Introduce Xe PCIe FLR (rev10) Patchwork
2026-08-21 11:33 ` ✓ CI.KUnit: success " Patchwork
2026-08-21 12:37 ` ✓ Xe.CI.BAT: " Patchwork
2026-08-21 14:38 ` ✓ Xe.CI.FULL: " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ao7QRcbFlJxVxHHP@black.igk.intel.com \
    --to=raag.jadav@intel.com \
    --cc=badal.nilawar@intel.com \
    --cc=daniele.ceraolospurio@intel.com \
    --cc=dev@lankhorst.se \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=jani.nikula@intel.com \
    --cc=lukas@wunner.de \
    --cc=lukasz.laguna@intel.com \
    --cc=matthew.auld@intel.com \
    --cc=matthew.brost@intel.com \
    --cc=matthew.d.roper@intel.com \
    --cc=michal.wajdeczko@intel.com \
    --cc=michal.winiarski@intel.com \
    --cc=riana.tauro@intel.com \
    --cc=rodrigo.vivi@intel.com \
    --cc=thomas.hellstrom@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).