* [PATCH] vfio/pci: Restore standard PCI config space in .slot_reset()
@ 2026-08-17 19:51 Keith Busch
2026-08-17 20:04 ` sashiko-bot
2026-08-17 21:50 ` Alex Williamson
0 siblings, 2 replies; 8+ messages in thread
From: Keith Busch @ 2026-08-17 19:51 UTC (permalink / raw)
To: alex, bhelgaas, linux-pci; +Cc: mattev, matt, nekto0n, Keith Busch
From: Keith Busch <kbusch@kernel.org>
Hi Alex, Bjorn, Linux-PCI, and VFIO,
I'd like to initiate, or maybe restart, a discussion regarding the
current state of PCIe AER and DPC (Downstream Port Containment) recovery
for devices bound to vfio-pci, potentially expanding to devices not
bound to anything at all.
Currently, when a link reset or Secondary Bus Reset occurs due to an
AER/DPC event, the kernel PCI error recovery service executes reset
routines across affected downstream devices. Since vfio-pci does not
implement the .slot_reset() callback in its pci_error_handlers struct,
the standard PCI config space (BARs, Command register, MSI-X setup,
DevCtl, etc.) for downstream devices is left entirely uninitialized
after the link is brought back up.
Historically the PCI core introduced early pci_save_state()
checkpointing prior to driver probe (e.g. commit 30d52f6385d8, "PCI:
Save device state in pci_pm_init() before driver probe") precisely to
ensure that the kernel maintains a valid baseline config space
snapshot—even for driverless or stub-bound devices. However, that
infrastructure was never fully connected to the error recovery callbacks
for generic pass-through drivers.
For bare-metal userspace drivers, like custom user-level drivers, this
creates an operational deadlock:
1. The kernel PCI error handling triggers a bus/link reset, but leaves
standard PCI config space wiped back to power-on defaults.
2. The eventfd (VFIO_PCI_ERR_IRQ_INDEX) alerts userspace that an error occurred,
but gives no indication of when link recovery/reset completes.
3. Userspace attempting to re-initialize or access the device races against
the kernel's reset sequence or operates on zeroed/uninitialized config registers.
4. While QEMU originally included stubs intended to coordinate AER recovery with
guest OS drivers, that guest-host coordination infrastructure has remained
unimplemented for over a decade.
In scenarios where userspace simply needs the underlying hardware
restored to its known-good baseline state post-reset, having vfio-pci
implement .slot_reset() and leverage the existing PCI core snapshot via
pci_restore_state() bridges this gap cleanly.
We recently saw related discussions around VFIO error recovery on s390x
(https://lore.kernel.org/linux-pci/20260603182415.2324-1-alifm@linux.ibm.com/#t),
where platform-specific mechanisms had to be wired up to communicate PCI
errors to userspace. However, for standard PCIe setups relying on
generic kernel error recovery (pcie_do_recovery), vfio-pci still lacks
the fundamental .slot_reset() handling needed to restore standard PCI
config space after recoverying from a fatal error.
---
A few questions I'd like to put to the list:
1. Since the PCI core already takes responsibility for holding the early
config space checkpoint, is calling pci_restore_state() during
.slot_reset() the appropriate place for vfio-pci to apply it, or
should this be explicitly driven/triggered via a VFIO ioctl?
2. Does returning PCI_ERS_RESULT_RECOVERED here create subtle state
issues if userspace directly modified config space registers that
were not captured in pdev->saved_config_space?
3. Should we pair this with an explicit "link restored / reset complete"
eventfd notification so userspace knows exactly when it is safe to
resume access?
Appreciate any feedback or historical context on how VFIO and PCI error
recovery should interact here.
drivers/vfio/pci/vfio_pci_core.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
index 3f11a9624b9c0..5b42430337e29 100644
--- a/drivers/vfio/pci/vfio_pci_core.c
+++ b/drivers/vfio/pci/vfio_pci_core.c
@@ -2347,6 +2347,13 @@ pci_ers_result_t vfio_pci_core_aer_err_detected(struct pci_dev *pdev,
}
EXPORT_SYMBOL_GPL(vfio_pci_core_aer_err_detected);
+pci_ers_result_t vfio_pci_core_aer_slot_reset(struct pci_dev *pdev,
+ pci_channel_state_t state)
+{
+ pci_restore_state(pdev);
+ return PCI_ERS_RESULT_RECOVERED;
+}
+
int vfio_pci_core_sriov_configure(struct vfio_pci_core_device *vdev,
int nr_virtfn)
{
@@ -2419,6 +2426,7 @@ EXPORT_SYMBOL_GPL(vfio_pci_core_sriov_configure);
const struct pci_error_handlers vfio_pci_core_err_handlers = {
.error_detected = vfio_pci_core_aer_err_detected,
+ .slot_reset = vfio_pci_core_aer_slot_reset,
};
EXPORT_SYMBOL_GPL(vfio_pci_core_err_handlers);
--
2.53.0-Meta
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] vfio/pci: Restore standard PCI config space in .slot_reset()
2026-08-17 19:51 [PATCH] vfio/pci: Restore standard PCI config space in .slot_reset() Keith Busch
@ 2026-08-17 20:04 ` sashiko-bot
2026-08-17 20:18 ` Keith Busch
2026-08-18 16:02 ` Lukas Wunner
2026-08-17 21:50 ` Alex Williamson
1 sibling, 2 replies; 8+ messages in thread
From: sashiko-bot @ 2026-08-17 20:04 UTC (permalink / raw)
To: Keith Busch; +Cc: linux-pci
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [Critical] The `vfio_pci_core_aer_slot_reset` callback has an incorrect function signature that violates `struct pci_error_handlers`, causing CFI panics.
- [High] The newly added `.slot_reset` callback is unreachable dead code for isolated VFIO devices because `error_detected` never returns `PCI_ERS_RESULT_NEED_RESET`.
- [High] `pci_restore_state()` clobbers userspace runtime PCI configuration with an outdated kernel probe-time snapshot, breaking VFIO virtualization state.
- [High] `pci_restore_state()` modifies hardware state locklessly, bypassing VFIO synchronization primitives and causing severe race conditions with concurrent MMIO accesses.
- [Medium] Prematurely returning `PCI_ERS_RESULT_RECOVERED` from `.slot_reset` races with userspace recovery logic due to the lack of a reset completion notification.
--
--- Patch [1]: [PATCH] vfio/pci: Restore standard PCI config space in .slot_reset() ---
commit e72e9372390b9003aeabcf98521c52beaad6c1a4
Author: Keith Busch <kbusch@kernel.org>
vfio/pci: Restore standard PCI config space in .slot_reset()
Currently, when a link reset or Secondary Bus Reset occurs due to an
AER/DPC event, the kernel PCI error recovery service executes reset
routines across affected downstream devices. Since vfio-pci does not
implement the .slot_reset() callback in its pci_error_handlers struct,
the standard PCI config space (BARs, Command register, MSI-X setup,
DevCtl, etc.) for downstream devices is left entirely uninitialized
after the link is brought back up.
> diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
> index ea7bcd44bdfa9..9abf725446158 100644
> --- a/drivers/vfio/pci/vfio_pci_core.c
> +++ b/drivers/vfio/pci/vfio_pci_core.c
[ ... ]
> @@ -2351,6 +2351,13 @@ pci_ers_result_t vfio_pci_core_aer_err_detected(struct pci_dev *pdev,
> }
> EXPORT_SYMBOL_GPL(vfio_pci_core_aer_err_detected);
>
> +pci_ers_result_t vfio_pci_core_aer_slot_reset(struct pci_dev *pdev,
> + pci_channel_state_t state)
[Severity: Critical]
Does the extra pci_channel_state_t argument violate the slot_reset
callback signature in struct pci_error_handlers?
The slot_reset callback expects a function pointer with exactly one
argument: pci_ers_result_t (*slot_reset)(struct pci_dev *dev).
Will this extra argument cause a control flow integrity panic or compiler
type-mismatch error when the PCI AER core invokes this via
err_handler->slot_reset(dev)?
[Severity: High]
Is this callback reachable under normal single-device circumstances?
In pcie_do_recovery(), the report_slot_reset broadcast is gated behind
checking if the combined error status is PCI_ERS_RESULT_NEED_RESET.
However, vfio_pci_core_aer_err_detected() unconditionally returns
PCI_ERS_RESULT_CAN_RECOVER. The PCI recovery state machine promotes
CAN_RECOVER to RECOVERED during the mmio_enabled phase, so the
condition to call slot_reset evaluates to false unless another driver
on the same PCIe bridge votes for NEED_RESET.
> +{
> + pci_restore_state(pdev);
[Severity: High]
Can pci_restore_state() clobber userspace runtime configurations with
an outdated probe-time snapshot?
For a VFIO device, pdev->saved_config_space contains the host kernel's
baseline state captured during vfio_pci_core_enable(). Subsequent
userspace modifications (like enabling bus master or setting up MSI)
are recorded in VFIO's virtual cache (vdev->vconfig) but do not update
pdev->saved_config_space.
Will reverting the hardware to this stale baseline silently destroy
guest configurations while VFIO still thinks they are intact?
[Severity: High]
Does modifying the hardware state here locklessly bypass VFIO
synchronization primitives?
The PCI AER core invokes the slot_reset callback holding only the
baseline device lock. VFIO relies on vdev->memory_lock (using
down_write(&vdev->memory_lock)) to serialize state-mutating operations
against concurrent userspace operations like vfio_pci_rw or MMIO page
faults.
Could calling pci_restore_state() without this lock rewrite BARs and
the PCI Command register concurrently with active userspace MMIO accesses,
potentially causing host crashes or machine check exceptions?
> + return PCI_ERS_RESULT_RECOVERED;
[Severity: Medium]
Does returning PCI_ERS_RESULT_RECOVERED here create a race condition
with userspace recovery logic?
VFIO notifies userspace via the VFIO_PCI_ERR_IRQ_INDEX eventfd when
an error is initially detected. By unconditionally returning
PCI_ERS_RESULT_RECOVERED, the host kernel completes its physical PCI
recovery and brings the link fully up before userspace has a chance
to react or rebuild its state.
Since there is no explicit completion notification to coordinate with
the kernel's state machine, will userspace operate blindly and potentially
attempt to access the device concurrently during the kernel's reset
sequence?
> +}
> +
> int vfio_pci_core_sriov_configure(struct vfio_pci_core_device *vdev,
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260817195110.2077698-1-kbusch@meta.com?part=1
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] vfio/pci: Restore standard PCI config space in .slot_reset()
2026-08-17 20:04 ` sashiko-bot
@ 2026-08-17 20:18 ` Keith Busch
2026-08-18 16:02 ` Lukas Wunner
1 sibling, 0 replies; 8+ messages in thread
From: Keith Busch @ 2026-08-17 20:18 UTC (permalink / raw)
To: sashiko-reviews; +Cc: Keith Busch, linux-pci
On Mon, Aug 17, 2026 at 08:04:04PM +0000, sashiko-bot@kernel.org wrote:
> > +pci_ers_result_t vfio_pci_core_aer_slot_reset(struct pci_dev *pdev,
> > + pci_channel_state_t state)
>
> [Severity: Critical]
> Does the extra pci_channel_state_t argument violate the slot_reset
> callback signature in struct pci_error_handlers?
Yeah, it does, and I messed up with a bad snapshot of the patch. I also
should have labeled this RFC as this isn't a serious suggestion at this
point. The entire vfio kernel-user space interaction for recoverying
from these conditions isn't there.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] vfio/pci: Restore standard PCI config space in .slot_reset()
2026-08-17 19:51 [PATCH] vfio/pci: Restore standard PCI config space in .slot_reset() Keith Busch
2026-08-17 20:04 ` sashiko-bot
@ 2026-08-17 21:50 ` Alex Williamson
2026-08-17 23:21 ` Keith Busch
1 sibling, 1 reply; 8+ messages in thread
From: Alex Williamson @ 2026-08-17 21:50 UTC (permalink / raw)
To: Keith Busch; +Cc: bhelgaas, linux-pci, mattev, matt, nekto0n, Keith Busch, alex
On Mon, 17 Aug 2026 12:51:10 -0700
Keith Busch <kbusch@meta.com> wrote:
> From: Keith Busch <kbusch@kernel.org>
>
> Hi Alex, Bjorn, Linux-PCI, and VFIO,
>
> I'd like to initiate, or maybe restart, a discussion regarding the
> current state of PCIe AER and DPC (Downstream Port Containment) recovery
> for devices bound to vfio-pci, potentially expanding to devices not
> bound to anything at all.
>
> Currently, when a link reset or Secondary Bus Reset occurs due to an
> AER/DPC event, the kernel PCI error recovery service executes reset
> routines across affected downstream devices. Since vfio-pci does not
> implement the .slot_reset() callback in its pci_error_handlers struct,
> the standard PCI config space (BARs, Command register, MSI-X setup,
> DevCtl, etc.) for downstream devices is left entirely uninitialized
> after the link is brought back up.
>
> Historically the PCI core introduced early pci_save_state()
> checkpointing prior to driver probe (e.g. commit 30d52f6385d8, "PCI:
> Save device state in pci_pm_init() before driver probe") precisely to
> ensure that the kernel maintains a valid baseline config space
> snapshot—even for driverless or stub-bound devices. However, that
> infrastructure was never fully connected to the error recovery callbacks
> for generic pass-through drivers.
>
> For bare-metal userspace drivers, like custom user-level drivers, this
> creates an operational deadlock:
>
> 1. The kernel PCI error handling triggers a bus/link reset, but leaves
> standard PCI config space wiped back to power-on defaults.
> 2. The eventfd (VFIO_PCI_ERR_IRQ_INDEX) alerts userspace that an error occurred,
> but gives no indication of when link recovery/reset completes.
> 3. Userspace attempting to re-initialize or access the device races against
> the kernel's reset sequence or operates on zeroed/uninitialized config registers.
> 4. While QEMU originally included stubs intended to coordinate AER recovery with
> guest OS drivers, that guest-host coordination infrastructure has remained
> unimplemented for over a decade.
>
> In scenarios where userspace simply needs the underlying hardware
> restored to its known-good baseline state post-reset, having vfio-pci
> implement .slot_reset() and leverage the existing PCI core snapshot via
> pci_restore_state() bridges this gap cleanly.
Does it though? Even for a simple reset to initial state we need to
prevent the host and guest stepping on each other across the reset as
well as tear down user modified state, like interrupts.
> We recently saw related discussions around VFIO error recovery on s390x
> (https://lore.kernel.org/linux-pci/20260603182415.2324-1-alifm@linux.ibm.com/#t),
> where platform-specific mechanisms had to be wired up to communicate PCI
> errors to userspace. However, for standard PCIe setups relying on
> generic kernel error recovery (pcie_do_recovery), vfio-pci still lacks
> the fundamental .slot_reset() handling needed to restore standard PCI
> config space after recoverying from a fatal error.
The s390x series preempts recovery for assigned devices and only
provides an extra reporting channel through vfio-pci. I don't know if
that's a model we can (or want to) copy, but may work for them given the
underlying hypervisor virtualization of the device.
> ---
> A few questions I'd like to put to the list:
> 1. Since the PCI core already takes responsibility for holding the early
> config space checkpoint, is calling pci_restore_state() during
> .slot_reset() the appropriate place for vfio-pci to apply it, or
> should this be explicitly driven/triggered via a VFIO ioctl?
> 2. Does returning PCI_ERS_RESULT_RECOVERED here create subtle state
> issues if userspace directly modified config space registers that
> were not captured in pdev->saved_config_space?
> 3. Should we pair this with an explicit "link restored / reset complete"
> eventfd notification so userspace knows exactly when it is safe to
> resume access?
>
> Appreciate any feedback or historical context on how VFIO and PCI error
> recovery should interact here.
Certainly the host saved state doesn't take into account user
manipulation of the device since the last snapshot. But also, vfio-pci
error handling is currently limited to generating an event when a
non-recoverable error has occurred. I don't think we can nudge it
forward in any meaningful way by implementing a .slot_reset to restore
a prior host snapshot. That misses the user modified state,
coordination across error handling, and may not even match the hand-off
state of the device to the user.
Forwarding AER to the guest and continuing came up in another thread
recently where I proposed[1] an approach we might use. Effectively we
need to proactively block access to the device during the host error
handling progression while providing some observability of that
process and the resulting state of the device. GHES/APEI is
potentially a model that this "host-first" error handling hides behind
for a guest. Thanks,
Alex
[1]https://lore.kernel.org/all/20260707161234.23ed28db@nvidia.com/
>
> drivers/vfio/pci/vfio_pci_core.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
> index 3f11a9624b9c0..5b42430337e29 100644
> --- a/drivers/vfio/pci/vfio_pci_core.c
> +++ b/drivers/vfio/pci/vfio_pci_core.c
> @@ -2347,6 +2347,13 @@ pci_ers_result_t vfio_pci_core_aer_err_detected(struct pci_dev *pdev,
> }
> EXPORT_SYMBOL_GPL(vfio_pci_core_aer_err_detected);
>
> +pci_ers_result_t vfio_pci_core_aer_slot_reset(struct pci_dev *pdev,
> + pci_channel_state_t state)
> +{
> + pci_restore_state(pdev);
> + return PCI_ERS_RESULT_RECOVERED;
> +}
> +
> int vfio_pci_core_sriov_configure(struct vfio_pci_core_device *vdev,
> int nr_virtfn)
> {
> @@ -2419,6 +2426,7 @@ EXPORT_SYMBOL_GPL(vfio_pci_core_sriov_configure);
>
> const struct pci_error_handlers vfio_pci_core_err_handlers = {
> .error_detected = vfio_pci_core_aer_err_detected,
> + .slot_reset = vfio_pci_core_aer_slot_reset,
> };
> EXPORT_SYMBOL_GPL(vfio_pci_core_err_handlers);
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] vfio/pci: Restore standard PCI config space in .slot_reset()
2026-08-17 21:50 ` Alex Williamson
@ 2026-08-17 23:21 ` Keith Busch
2026-08-18 14:37 ` Alex Williamson
0 siblings, 1 reply; 8+ messages in thread
From: Keith Busch @ 2026-08-17 23:21 UTC (permalink / raw)
To: Alex Williamson; +Cc: Keith Busch, bhelgaas, linux-pci, mattev, matt, nekto0n
On Mon, Aug 17, 2026 at 03:50:06PM -0600, Alex Williamson wrote:
> > In scenarios where userspace simply needs the underlying hardware
> > restored to its known-good baseline state post-reset, having vfio-pci
> > implement .slot_reset() and leverage the existing PCI core snapshot via
> > pci_restore_state() bridges this gap cleanly.
>
> Does it though? Even for a simple reset to initial state we need to
> prevent the host and guest stepping on each other across the reset as
> well as tear down user modified state, like interrupts.
Specifically considering host-guest interactions, I don't think this
scenario is handled at all. After 14 years, this is the state of QEMU:
static void vfio_err_notifier_handler(void *opaque)
{
VFIOPCIDevice *vdev = opaque;
if (!event_notifier_test_and_clear(&vdev->err_notifier)) {
return;
}
/*
* TBD. Retrieve the error details and decide what action
* needs to be taken. One of the actions could be to pass
* the error to the guest and have the guest driver recover
* from the error. This requires that PCIe capabilities be
* exposed to the guest. For now, we just terminate the
* guest to contain the error.
*/
error_report("%s(%s) Unrecoverable error detected. Please collect any data possible and then kill the guest", __func__, vdev->vbasedev.name);
vm_stop(RUN_STATE_INTERNAL_ERROR);
}
Is there another common VMM that actually does something useful to
continue from this event?
Outside virtualization, I'm more interested in enabling user space
DPDK-like drivers. I don't want to break anyone, so starting small here:
restoring the config space to the baseline before the device was handed
to a user space driver feels right.
> > A few questions I'd like to put to the list:
> > 1. Since the PCI core already takes responsibility for holding the early
> > config space checkpoint, is calling pci_restore_state() during
> > .slot_reset() the appropriate place for vfio-pci to apply it, or
> > should this be explicitly driven/triggered via a VFIO ioctl?
> > 2. Does returning PCI_ERS_RESULT_RECOVERED here create subtle state
> > issues if userspace directly modified config space registers that
> > were not captured in pdev->saved_config_space?
> > 3. Should we pair this with an explicit "link restored / reset complete"
> > eventfd notification so userspace knows exactly when it is safe to
> > resume access?
> >
> > Appreciate any feedback or historical context on how VFIO and PCI error
> > recovery should interact here.
>
> Certainly the host saved state doesn't take into account user
> manipulation of the device since the last snapshot.
Yeah, the kernel emits an eventfd that an error occured. The user side
should have some baseline from which to proceed. It feels outside the
scope of user space to save and restore such low level and early
initialization things like the PCI BAR config space.
Also consider that the user space side may not have even been
initialized at the time a PCIe error occured. What happens then?
> But also, vfio-pci error handling is currently limited to generating
> an event when a non-recoverable error has occurred. I don't think we
> can nudge it forward in any meaningful way by implementing a
> .slot_reset to restore a prior host snapshot. That misses the user
> modified state, coordination across error handling, and may not even
> match the hand-off state of the device to the user.
I totally agree. Lacking a notification, user space can at best poll
something specific to their device, but it'd be better to generically
coordinate this sequence with the kernel's error handling.
Would it be acceptable to introduce additional eventfd's for each part
in the pcie error handling? Privately, I've proposed and tested the
user-space component to quiesce on .error_detected, then start from
scratch after the .slot_reset. But I still need something to restore the
config space, and I feel kernel is the right place to do it.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] vfio/pci: Restore standard PCI config space in .slot_reset()
2026-08-17 23:21 ` Keith Busch
@ 2026-08-18 14:37 ` Alex Williamson
2026-08-18 15:11 ` Shameer Kolothum Thodi
0 siblings, 1 reply; 8+ messages in thread
From: Alex Williamson @ 2026-08-18 14:37 UTC (permalink / raw)
To: Keith Busch
Cc: Keith Busch, bhelgaas, linux-pci, mattev, matt, nekto0n, alex,
Shameer Kolothum
[Cc +Shameer]
On Mon, 17 Aug 2026 17:21:23 -0600
Keith Busch <kbusch@kernel.org> wrote:
> On Mon, Aug 17, 2026 at 03:50:06PM -0600, Alex Williamson wrote:
> > > In scenarios where userspace simply needs the underlying hardware
> > > restored to its known-good baseline state post-reset, having vfio-pci
> > > implement .slot_reset() and leverage the existing PCI core snapshot via
> > > pci_restore_state() bridges this gap cleanly.
> >
> > Does it though? Even for a simple reset to initial state we need to
> > prevent the host and guest stepping on each other across the reset as
> > well as tear down user modified state, like interrupts.
>
> Specifically considering host-guest interactions, I don't think this
> scenario is handled at all. After 14 years, this is the state of QEMU:
>
> static void vfio_err_notifier_handler(void *opaque)
> {
> VFIOPCIDevice *vdev = opaque;
>
> if (!event_notifier_test_and_clear(&vdev->err_notifier)) {
> return;
> }
>
> /*
> * TBD. Retrieve the error details and decide what action
> * needs to be taken. One of the actions could be to pass
> * the error to the guest and have the guest driver recover
> * from the error. This requires that PCIe capabilities be
> * exposed to the guest. For now, we just terminate the
> * guest to contain the error.
> */
>
> error_report("%s(%s) Unrecoverable error detected. Please collect any data possible and then kill the guest", __func__, vdev->vbasedev.name);
>
> vm_stop(RUN_STATE_INTERNAL_ERROR);
> }
>
> Is there another common VMM that actually does something useful to
> continue from this event?
Not that I'm aware of, the kernel interface really isn't designed for
recovery, it's designed only to notify.
> Outside virtualization, I'm more interested in enabling user space
> DPDK-like drivers. I don't want to break anyone, so starting small here:
> restoring the config space to the baseline before the device was handed
> to a user space driver feels right.
But we have no hand-back-to-userspace mechanism currently. Alone, it's
certainly a step towards letting the device run again, but we really
need more uAPI defined to provide coordination.
> > > A few questions I'd like to put to the list:
> > > 1. Since the PCI core already takes responsibility for holding the early
> > > config space checkpoint, is calling pci_restore_state() during
> > > .slot_reset() the appropriate place for vfio-pci to apply it, or
> > > should this be explicitly driven/triggered via a VFIO ioctl?
> > > 2. Does returning PCI_ERS_RESULT_RECOVERED here create subtle state
> > > issues if userspace directly modified config space registers that
> > > were not captured in pdev->saved_config_space?
> > > 3. Should we pair this with an explicit "link restored / reset complete"
> > > eventfd notification so userspace knows exactly when it is safe to
> > > resume access?
> > >
> > > Appreciate any feedback or historical context on how VFIO and PCI error
> > > recovery should interact here.
> >
> > Certainly the host saved state doesn't take into account user
> > manipulation of the device since the last snapshot.
>
> Yeah, the kernel emits an eventfd that an error occured. The user side
> should have some baseline from which to proceed. It feels outside the
> scope of user space to save and restore such low level and early
> initialization things like the PCI BAR config space.
There's a fair bit of config space the user cannot write, particularly
BARs, so a VM has an obligation to restore the virtualized BARs to make
a coherent view of the device, but a userspace driver can't effect a
meaningful value change of the physical BAR register anyway.
> Also consider that the user space side may not have even been
> initialized at the time a PCIe error occured. What happens then?
I'd tend to think a userspace driver would consider aborting if the
device is triggering errors before they've even touched it. Closing
the device writes back the state saved on open.
> > But also, vfio-pci error handling is currently limited to generating
> > an event when a non-recoverable error has occurred. I don't think we
> > can nudge it forward in any meaningful way by implementing a
> > .slot_reset to restore a prior host snapshot. That misses the user
> > modified state, coordination across error handling, and may not even
> > match the hand-off state of the device to the user.
>
> I totally agree. Lacking a notification, user space can at best poll
> something specific to their device, but it'd be better to generically
> coordinate this sequence with the kernel's error handling.
>
> Would it be acceptable to introduce additional eventfd's for each part
> in the pcie error handling? Privately, I've proposed and tested the
> user-space component to quiesce on .error_detected, then start from
> scratch after the .slot_reset. But I still need something to restore the
> config space, and I feel kernel is the right place to do it.
Absolutely the kernel needs to provide some restore of the device,
especially where the user doesn't have access. We need some mechanism
for the user to observe the host recovery and know when it can access
the device again. I provided my high level vision of that in the link
I previously shared. Shameer has also been looking into this and can
share his plans.
I might caution tying the uAPI too closely with the kernel recovery
implementation, for example eventfds mapped to each part of the
internal recovery sequence. I think userspace needs to be an observer
to the kernel recovery, not a participant. You're already doing
something similar with quiesce on error, but we need some mechanism to
know when the device is reachable again. Recovery may also not be
successful, so an eventfd-only mechanism expands into multiple eventfds
to report success vs failure. The VM use case probably wants a report
of the error, so it might be easier to have userspace poll an ioctl on
error to know when to continue or abort the device. Thanks,
Alex
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH] vfio/pci: Restore standard PCI config space in .slot_reset()
2026-08-18 14:37 ` Alex Williamson
@ 2026-08-18 15:11 ` Shameer Kolothum Thodi
0 siblings, 0 replies; 8+ messages in thread
From: Shameer Kolothum Thodi @ 2026-08-18 15:11 UTC (permalink / raw)
To: Alex Williamson, Keith Busch
Cc: Keith Busch, bhelgaas@google.com, linux-pci@vger.kernel.org,
mattev@meta.com, matt@ozlabs.org, nekto0n@meta.com,
michal.winiarski@intel.com, satyanarayana.k.v.p@intel.com
> -----Original Message-----
> From: Alex Williamson <alex@shazbot.org>
> Sent: 18 August 2026 15:38
> To: Keith Busch <kbusch@kernel.org>
> Cc: Keith Busch <kbusch@meta.com>; bhelgaas@google.com; linux-
> pci@vger.kernel.org; mattev@meta.com; matt@ozlabs.org;
> nekto0n@meta.com; alex@shazbot.org; Shameer Kolothum Thodi
> <skolothumtho@nvidia.com>
> Subject: Re: [PATCH] vfio/pci: Restore standard PCI config space in .slot_reset()
>
> External email: Use caution opening links or attachments
>
>
> [Cc +Shameer]
Thanks Alex.
>
> On Mon, 17 Aug 2026 17:21:23 -0600
> Keith Busch <kbusch@kernel.org> wrote:
>
> > On Mon, Aug 17, 2026 at 03:50:06PM -0600, Alex Williamson wrote:
> > > > In scenarios where userspace simply needs the underlying hardware
> > > > restored to its known-good baseline state post-reset, having vfio-pci
> > > > implement .slot_reset() and leverage the existing PCI core snapshot via
> > > > pci_restore_state() bridges this gap cleanly.
> > >
> > > Does it though? Even for a simple reset to initial state we need to
> > > prevent the host and guest stepping on each other across the reset as
> > > well as tear down user modified state, like interrupts.
> >
> > Specifically considering host-guest interactions, I don't think this
> > scenario is handled at all. After 14 years, this is the state of QEMU:
> >
> > static void vfio_err_notifier_handler(void *opaque)
> > {
> > VFIOPCIDevice *vdev = opaque;
> >
> > if (!event_notifier_test_and_clear(&vdev->err_notifier)) {
> > return;
> > }
> >
> > /*
> > * TBD. Retrieve the error details and decide what action
> > * needs to be taken. One of the actions could be to pass
> > * the error to the guest and have the guest driver recover
> > * from the error. This requires that PCIe capabilities be
> > * exposed to the guest. For now, we just terminate the
> > * guest to contain the error.
> > */
> >
> > error_report("%s(%s) Unrecoverable error detected. Please collect any
> data possible and then kill the guest", __func__, vdev->vbasedev.name);
> >
> > vm_stop(RUN_STATE_INTERNAL_ERROR);
> > }
> >
> > Is there another common VMM that actually does something useful to
> > continue from this event?
>
> Not that I'm aware of, the kernel interface really isn't designed for
> recovery, it's designed only to notify.
>
> > Outside virtualization, I'm more interested in enabling user space
> > DPDK-like drivers. I don't want to break anyone, so starting small here:
> > restoring the config space to the baseline before the device was handed
> > to a user space driver feels right.
>
> But we have no hand-back-to-userspace mechanism currently. Alone, it's
> certainly a step towards letting the device run again, but we really
> need more uAPI defined to provide coordination.
>
> > > > A few questions I'd like to put to the list:
> > > > 1. Since the PCI core already takes responsibility for holding the early
> > > > config space checkpoint, is calling pci_restore_state() during
> > > > .slot_reset() the appropriate place for vfio-pci to apply it, or
> > > > should this be explicitly driven/triggered via a VFIO ioctl?
> > > > 2. Does returning PCI_ERS_RESULT_RECOVERED here create subtle state
> > > > issues if userspace directly modified config space registers that
> > > > were not captured in pdev->saved_config_space?
> > > > 3. Should we pair this with an explicit "link restored / reset complete"
> > > > eventfd notification so userspace knows exactly when it is safe to
> > > > resume access?
> > > >
> > > > Appreciate any feedback or historical context on how VFIO and PCI error
> > > > recovery should interact here.
> > >
> > > Certainly the host saved state doesn't take into account user
> > > manipulation of the device since the last snapshot.
> >
> > Yeah, the kernel emits an eventfd that an error occured. The user side
> > should have some baseline from which to proceed. It feels outside the
> > scope of user space to save and restore such low level and early
> > initialization things like the PCI BAR config space.
>
> There's a fair bit of config space the user cannot write, particularly
> BARs, so a VM has an obligation to restore the virtualized BARs to make
> a coherent view of the device, but a userspace driver can't effect a
> meaningful value change of the physical BAR register anyway.
>
> > Also consider that the user space side may not have even been
> > initialized at the time a PCIe error occured. What happens then?
>
> I'd tend to think a userspace driver would consider aborting if the
> device is triggering errors before they've even touched it. Closing
> the device writes back the state saved on open.
>
> > > But also, vfio-pci error handling is currently limited to generating
> > > an event when a non-recoverable error has occurred. I don't think we
> > > can nudge it forward in any meaningful way by implementing a
> > > .slot_reset to restore a prior host snapshot. That misses the user
> > > modified state, coordination across error handling, and may not even
> > > match the hand-off state of the device to the user.
> >
> > I totally agree. Lacking a notification, user space can at best poll
> > something specific to their device, but it'd be better to generically
> > coordinate this sequence with the kernel's error handling.
> >
> > Would it be acceptable to introduce additional eventfd's for each part
> > in the pcie error handling? Privately, I've proposed and tested the
> > user-space component to quiesce on .error_detected, then start from
> > scratch after the .slot_reset. But I still need something to restore the
> > config space, and I feel kernel is the right place to do it.
>
> Absolutely the kernel needs to provide some restore of the device,
> especially where the user doesn't have access. We need some mechanism
> for the user to observe the host recovery and know when it can access
> the device again. I provided my high level vision of that in the link
> I previously shared. Shameer has also been looking into this and can
> share his plans.
[+Michał, +Satyanarayana]
Right. I am working on a series trying to do much of what Alex described
in the other thread. Currently in internal review and testing.
On the eventfd question, what I have is one recovery eventfd plus a
feature reporting the state and a sequence number, rather than one
eventfd per callback. The state says whether recovery is in progress,
whether the channel was frozen, whether the device was reset, and
whether it failed. A non-fatal recovery can complete before userspace
reacts to the event, so the state and the sequence are more use than
catching each phase live, I think.
In short:
- error_detected() stops being a single "something went wrong" signal.
It votes on the actual channel state: CAN_RECOVER for a non-fatal
error, NEED_RESET for a frozen channel, DISCONNECT for a permanent
failure, and signals userspace that an event has started.
- slot_reset() restores config space with pci_restore_state(), tears
down user modified interrupt state so it is rebuilt rather than
resumed, and records that the device was reset for userspace to read.
- resume() puts PCI_COMMAND back, restores the DMA-BUF exports, unblocks
access and signals the eventfd again. Userspace reads the state from
the feature to see the event has finished.
- The device is blocked from user access for the whole sequence: config
space, BARs, faults on mapped BARs, interrupts, DMA-BUF exports,
runtime PM. That is most of the series, and getting the locking right
is the hard part here.
It is opt-in. Until userspace installs the recovery eventfd, vfio-pci
behaves exactly as it does today.
With the state visible, QEMU can virtualise the AER capability to the
guest, through the AER interrupt or GHES, so a non-fatal error the host
recovered from need not end in vm_stop().
I plan to post it as an RFC once the merge window closes.
Would appreciate any comments in the meantime.
Thanks,
Shameer
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] vfio/pci: Restore standard PCI config space in .slot_reset()
2026-08-17 20:04 ` sashiko-bot
2026-08-17 20:18 ` Keith Busch
@ 2026-08-18 16:02 ` Lukas Wunner
1 sibling, 0 replies; 8+ messages in thread
From: Lukas Wunner @ 2026-08-18 16:02 UTC (permalink / raw)
To: sashiko-reviews; +Cc: Keith Busch, linux-pci
On Mon, Aug 17, 2026 at 08:04:04PM +0000, sashiko-bot@kernel.org wrote:
> [Severity: High]
> Is this callback reachable under normal single-device circumstances?
>
> In pcie_do_recovery(), the report_slot_reset broadcast is gated behind
> checking if the combined error status is PCI_ERS_RESULT_NEED_RESET.
>
> However, vfio_pci_core_aer_err_detected() unconditionally returns
> PCI_ERS_RESULT_CAN_RECOVER. The PCI recovery state machine promotes
> CAN_RECOVER to RECOVERED during the mmio_enabled phase, so the
> condition to call slot_reset evaluates to false unless another driver
> on the same PCIe bridge votes for NEED_RESET.
I have a patch on my development branch so that ->reset_slot() is
also invoked if (state == pci_channel_io_frozen), i.e. on Fatal Errors.
I have another patch pending to allow error recovery for unbound devices.
Both patches are available for testing on this branch and I hope to
submit them in the upcoming cycle:
https://github.com/l1k/linux/commits/aer_unbound/
> [Severity: High]
> Can pci_restore_state() clobber userspace runtime configurations with
> an outdated probe-time snapshot?
>
> For a VFIO device, pdev->saved_config_space contains the host kernel's
> baseline state captured during vfio_pci_core_enable(). Subsequent
> userspace modifications (like enabling bus master or setting up MSI)
> are recorded in VFIO's virtual cache (vdev->vconfig) but do not update
> pdev->saved_config_space.
I guess the right thing to do is to write vdev->vconfig to config space
after pci_restore_state()?
Thanks,
Lukas
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-08-18 16:03 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-17 19:51 [PATCH] vfio/pci: Restore standard PCI config space in .slot_reset() Keith Busch
2026-08-17 20:04 ` sashiko-bot
2026-08-17 20:18 ` Keith Busch
2026-08-18 16:02 ` Lukas Wunner
2026-08-17 21:50 ` Alex Williamson
2026-08-17 23:21 ` Keith Busch
2026-08-18 14:37 ` Alex Williamson
2026-08-18 15:11 ` Shameer Kolothum Thodi
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.