Re: [RFC PATCH 00/19] vfio/pci: Handle PCI error recovery and report state to userspace

[Alex Williamson <alex@shazbot.org>]

On Tue, 8 Sep 2026 10:58:49 +0000
Shameer Kolothum Thodi <skolothumtho@nvidia.com> wrote:

> > -----Original Message-----
> > From: Shameer Kolothum Thodi <skolothumtho@nvidia.com>
> > Sent: 07 September 2026 10:39
> > To: Alex Williamson <alex@shazbot.org>
> > Cc: kvm@vger.kernel.org; linux-pci@vger.kernel.org; linux-
> > kernel@vger.kernel.org; jgg@ziepe.ca; kevin.tian@intel.com;
> > kbusch@meta.com; michal.winiarski@intel.com;
> > satyanarayana.k.v.p@intel.com; Sonang Patel <sonangp@nvidia.com>;
> > Nathan Chen <nathanc@nvidia.com>; Matt Ochs <mochs@nvidia.com>
> > Subject: RE: [RFC PATCH 00/19] vfio/pci: Handle PCI error recovery and report
> > state to userspace
> > 
> > 
> >   
> > > -----Original Message-----
> > > From: Alex Williamson <alex@shazbot.org>
> > > Sent: 04 September 2026 20:09
> > > To: Shameer Kolothum Thodi <skolothumtho@nvidia.com>
> > > Cc: kvm@vger.kernel.org; linux-pci@vger.kernel.org; linux-
> > > kernel@vger.kernel.org; jgg@ziepe.ca; kevin.tian@intel.com;
> > > kbusch@meta.com; michal.winiarski@intel.com;
> > > satyanarayana.k.v.p@intel.com; Sonang Patel <sonangp@nvidia.com>;
> > > Nathan Chen <nathanc@nvidia.com>; Matt Ochs <mochs@nvidia.com>;
> > > alex@shazbot.org
> > > Subject: Re: [RFC PATCH 00/19] vfio/pci: Handle PCI error recovery and  
> > report  
> > > state to userspace
> > >
> > > External email: Use caution opening links or attachments
> > >
> > >
> > > On Tue, 1 Sep 2026 10:31:58 +0100
> > > Shameer Kolothum <skolothumtho@nvidia.com> wrote:
> > >  
> > > > Hi,
> > > >
> > > > Currently, vfio-pci takes almost no part in PCI error recovery. It
> > > > implements error_detected() and neither of the other two callbacks. That
> > > > one callback ignores the pci_channel_state_t it is given, signals the
> > > > error eventfd, and returns PCI_ERS_RESULT_CAN_RECOVER for every error,  
> > a  
> > > > permanent failure included. Nothing implements slot_reset() or resume(),
> > > > so vfio-pci never learns that the host reset the device, or that
> > > > recovery finished.
> > > >
> > > > Userspace gets one eventfd signal with nothing attached to it. It cannot
> > > > tell a non-fatal error the host recovered from apart from a permanent
> > > > failure, and it is never told when recovery is over. With nothing to go
> > > > on, QEMU assumes the worst and calls  
> > > vm_stop(RUN_STATE_INTERNAL_ERROR),  
> > > > which the VM cannot come back from.
> > > >
> > > > Any device assigned through vfio-pci can hit this. A non-fatal
> > > > uncorrectable error is reported, the host AER path recovers the device
> > > > fine, and the VM is killed anyway.
> > > >
> > > > This series lets userspace observe host recovery state, and keeps it off
> > > > the device while recovery is running. With that state visible, userspace
> > > > can decide what to do with the guest rather than assuming the worst.
> > > >
> > > > The approach here comes from an earlier discussion with Alex.
> > > >
> > > > https://lore.kernel.org/qemu-  
> > > devel/20260707161234.23ed28db@nvidia.com/  
> > > > https://lore.kernel.org/all/20260818083754.7ccf76d9@shazbot.org/
> > > >
> > > > Design
> > > > ------
> > > >
> > > > The VMM watches recovery. It does not take part in it. The kernel runs
> > > > the recovery sequence and tells userspace what happened and when it is
> > > > done.
> > > >
> > > > vfio-pci already has error_detected(). This series extends it and adds
> > > > the other two callbacks:
> > > >
> > > >   - error_detected() now records the channel state, blocks new device
> > > >     access, revokes BAR mappings and exported DMA-BUFs, and quiesces
> > > >     INTx. It still signals err_trigger as it does today. It votes on
> > > >     severity rather than always claiming it can recover: CAN_RECOVER for
> > > >     a non-fatal error, NEED_RESET for a frozen channel, DISCONNECT for a
> > > >     permanent failure, and NONE if our own quiesce failed, which leaves
> > > >     the rest of the domain alone.
> > > >   - slot_reset() is new. It restores config state after the host has
> > > >     reset the device. Nothing does that today, which is why a device
> > > >     comes back from an AER reset with its config lost.
> > > >   - resume() is new. It restores PCI_COMMAND, unblocks access and wakes
> > > >     waiters.
> > > >   - A new device feature reports the state and carries an eventfd.
> > > >
> > > > A non-fatal error gets the same quiesce as a frozen one. The host has not
> > > > finished deciding what the error was, and can still escalate to a reset,
> > > > so the device is not the user's again until resume() says so.
> > > >
> > > > The support is opt-in. Until userspace installs the recovery eventfd,
> > > > generic vfio-pci behaves as it does today. error_detected() takes its
> > > > existing path and signals the same eventfd. VFIO variant driver support
> > > > is not added for now.
> > > >
> > > > The uAPI is VFIO_DEVICE_FEATURE_PCI_ERROR_RECOVERY. It carries the
> > > > eventfd and reports a status word plus a sequence number, so userspace
> > > > can tell coalesced notifications apart. IN_PROGRESS is set while a
> > > > recovery is running. CHANNEL_FROZEN says the link went down.
> > > > DEVICE_RESET says the host reset the device. FAILED says the device
> > > > cannot be used again until close and reopen. ENABLED says userspace has
> > > > opted in.
> > > >
> > > > A non-fatal recovery can complete before userspace reacts to the eventfd,
> > > > so IN_PROGRESS may already be clear by the time the feature is read. Work
> > > > from the sequence number and the status bits rather than expecting to
> > > > catch the event while it runs.
> > > >
> > > > Patches
> > > > -------
> > > >
> > > >   1-3    the groundwork: the recovery state fields, the open and close
> > > >          lifecycle so a callback never sees a half built or half torn
> > > >          down device, and the access guards the rest of the series uses
> > > >   4-13   close the access paths one at a time: function reset, config
> > > >          space, ioeventfd, BAR faults, BAR and ROM, interrupts, hot
> > > >          reset, runtime PM, info queries, DMA-BUF
> > > >   14-18  the error handler callbacks: slot reset, the INTx helpers and
> > > >          the quiesce that uses them, then resume and error_detected
> > > >   19     the uAPI a user opts in through
> > > >
> > > > Locking
> > > > -------
> > > >
> > > > Blocking access is the hard part of this series, and it comes down to
> > > > one rule.
> > > >
> > > >   recovery_lock can be held while publishing state, and while draining
> > > >   operations that are already under way. It cannot be held across a reset,
> > > >   or across anything else that reaches pci_bus_sem.
> > > >
> > > > The reason is the order AER arrives in. It enters the driver already
> > > > holding device_lock, and pci_bus_sem too when the device sits under a
> > > > bridge with a subordinate bus, and only then takes recovery_lock. A
> > > > secondary bus reset reaches pci_bus_sem. So a vfio path which holds
> > > > recovery_lock across a reset ends up taking those two the other way
> > > > round.
> > > >
> > > > Seven places needed reshaping for this rule: device close, slot_reset(),
> > > > open, VFIO_DEVICE_RESET, the guest triggered config space FLR,
> > > > VFIO_DEVICE_SET_IRQS, and a guest write putting the device back in D0,
> > > > which reaches pci_bus_sem through pcie_aspm_pm_state_change().
> > > >
> > > > Most access takes recovery_lock for reading and checks whether a recovery
> > > > or a reset is blocking the device. A few places cannot take the lock and
> > > > read that state directly instead. All of them fail safe. A stale read
> > > > costs an extra refusal or retry, never an unguarded access.
> > > >
> > > > Interrupt teardown is the one deliberate exception. It flushes the global
> > > > virqfd workqueue with recovery_lock held, which can make the hold last as
> > > > long as a reset on another vfio device. It costs latency, not
> > > > correctness.
> > > >
> > > > I am not sure this is the best way to handle it, and would welcome
> > > > suggestions.  
> > >
> > >
> > > Thanks for tackling this, Shameer.  The recovery_lock wrapping all
> > > these accesses does make me nervous, both in lock complexity and
> > > overhead.  Wouldn't it be a better solution to decouple the user
> > > interface from the device by replacing the access path via SRCU then
> > > doing zap/move/interrupt teardown?
> > >
> > > Such a solution would have utility beyond the error path.  We could use
> > > it for surprise removal/DPC, we could allow a policy to remove the
> > > device from the user on unbind, in place of or in addition to the
> > > request eventfd we use currently.  In the error case, the intention
> > > would be to temporarily suspend access to the device, but if it falls
> > > off the bus after recovery, it may turn into a permanent removal.
> > >
> > > What do you think?  Thanks,  
> > 
> > Agree. As it stands, it looks not that maintainable due to the lock
> > complexity and dependencies.  Let me look at replacing the
> > recovery_lock with SRCU and see how that evolves.
> > 
> > The generalisation makes sense too. I will keep this series to the error
> > path but make sure the mechanism is not tied to it.  
> 
> One more thing I want to highlight.
> 
> This series mostly does fail access if recovery is in progress. Config
> space, trapped BAR read and write, the ioctls and DMA-BUF export all
> return -EIO while access is blocked.
> 
> The one exception is a guest fault on an mmap'd BAR. See patch 7, where
> it returns VM_FAULT_RETRY and waits for recovery to finish rather than
> failing, and the reason is that failing is not currently useful to the
> VMM. On arm64 a failed BAR fault returns a bare -EFAULT from KVM_RUN
> with no KVM_EXIT_MEMORY_FAULT, so the VMM gets no address, cannot tell
> which device faulted, and cannot map the failure to a device under
> recovery.
> 
> (+Mike)
> 
> However I think it is fixable, as discussed here [1]. That proposes a
> KVM exit to userspace with KVM_EXIT_MEMORY_FAULT, filling
> run->memory_fault via kvm_mmu_prepare_memory_fault_exit().
> 
> I will take a look at the arm64 part, since that is what makes failing
> the access actually useful. Once the VMM can resolve the address to a
> device and query the recovery state, switching the fault path from wait
> to fail is a minimal change, I think.

Yes, agreed, and the VMM should choose the policy for a given memory
fault anyway.  The intention at the vfio-pci kernel level is that a VMM
can actually have better error containment than a host platform by
following the fault address to a device and deciding whether to expose
soft or hard errors to the guest.  Thanks,

Alex