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

[Alex Williamson]

On Tue, 29 Sep 2026 18:32:49 +0100
Shameer Kolothum <skolothumtho@xxxxxxxxxx> wrote:

> Hi,
> 
> Currently, vfio-pci signals the error eventfd from error_detected() and
> returns PCI_ERS_RESULT_CAN_RECOVER, including for permanent failure. It
> has no slot_reset() or resume() callback, so userspace is not notified
> when host recovery completes or whether the device was reset.
> 
> This series adds opt-in host PCI error recovery for vfio-pci. It blocks
> device access during recovery, restores device state after a host reset,
> and reports recovery status to userspace. The kernel runs host recovery
> and the VMM decides how to recover the guest.
> 
> Changes from v1
> ---------------
>  v1: https://lore.kernel.org/kvm/20260901093217.8539-1-skolothumtho@xxxxxxxxxx/
>   
>  - Thanks to Satya and Alex for reviews/feedback.
>  - Use SRCU to drain ongoing device accesses and reject new accesses during
>    recovery. A mutex protects recovery-state updates. 
>  - Return SIGBUS for blocked BAR faults instead of waiting in the fault
>    handler. VMM handling still needs target validation.
>  - Reject userspace SR-IOV changes while access is blocked.
>  - Exclude s390 and all CXL memory devices for now as s390 and CXL RCH
>    recovery can omit completion callbacks.
>   
> Design
> ------
> Userspace opts in through VFIO_DEVICE_FEATURE_PCI_ERROR_RECOVERY. It
> registers an eventfd and reads status flags and a sequence number. The
> existing error eventfd is still signaled.
> 
> The callbacks handle the host recovery sequence:
> 
>  - error_detected() blocks new accesses and drains admitted ones,
>    quiesces INTx, and revokes BAR mappings and exported DMA-BUFs. On a
>    normal channel it also clears bus mastering. It votes CAN_RECOVER
>    for a normal channel, NEED_RESET for a frozen channel, and DISCONNECT
>    for permanent failure. Failure to save PCI_COMMAND or disable bus
>    mastering leaves access blocked and returns NONE so other devices
>    can continue recovery.
>  - slot_reset() restores the configuration saved at open after a host
>    reset, then tears down interrupt configuration. Userspace must set
>    up interrupts again.
>  - resume() completes restoration, unblocks access and completes INTx
>    handling. A restoration failure keeps the device blocked.
> 
> Non-fatal errors also block device access because host recovery may
> require a reset. Userspace reads the recovery status to determine
> whether the device was reset or recovery failed.
>   
> Without opt-in, the existing error notification behavior is retained.
> The open check also rejects a previously recorded host transaction when
> recovery is disabled, so closing the device does not discard that state.
> 
> Locking and open questions
> --------------------------
> Several locks are involved in recovery. PCI core holds device_lock
> while calling the driver, preventing concurrent driver binding or
> unbinding. During the bus walk, it also holds pci_bus_sem for reading
> to keep the device list stable. Adding or removing PCI devices requires
> the write lock. This semaphore is shared across all PCI buses.
>  
> VFIO recovery blocks new accesses, waits for existing SRCU readers to
> finish, and takes memory_lock. Power transitions also take memory_lock,
> but entering D0 can then acquire pci_bus_sem to update PCIe link power
> management (ASPM).
> 
> Local Sashiko/Claude review reports a lock-order conflict as below:
> 
>  1. A guest D0 request holds memory_lock.
>  2. PCI core holds pci_bus_sem for reading while VFIO's recovery
>     callback waits for memory_lock.
>  3. Removal of an unrelated PCI device waits for the bus write lock.
>  4. D0's ASPM update requests the bus read lock and waits behind that
>     writer.
> 
> Each task waits for another, so none can proceed.
> 
> One possible solution could be(not implemented):
> 
>  - PCI core guarantees that pci_bus_sem is held for reading before
>    invoking supported recovery callbacks. VFIO uses
>    pci_set_power_state_locked() in those callbacks, avoiding another
>    acquisition of the bus lock.
>  - A new pci_try_set_power_state() helper tries to acquire pci_bus_sem
>    and returns -EAGAIN on contention. Ordinary VFIO D0 paths use this
>    helper and handle failure instead of waiting while holding VFIO locks
> 	  
> Not sure there are better ways to handle this or not. Feedbacks appreciated
> on this.

I've been churning tokens with an LLM on this.  My original intent was
that a disconnect via srcu between a device object and user interface
object, or at least ops pointers that act on the device, at the
vfio-core level could help to avoid this.  While that may be
worthwhile for more general use cases of removing user access to the
device (surprise unplug, force unbind, etc.), it doesn't seem to
resolve the scenario here.  In fact I haven't come up with better
solutions than you've already suggested above.

> 
> Another issue flagged was(I think this is a pre-existing one):
>  - Open/close coordination with the whole host recovery transaction
>    was already missing. Recovery can start after the open check, and
>    close can overlap the physical reset. Restoration of closed devices
>    is also not implemented.

Yes, it's pre-existing.  A more structured concept of a disconnected
device would help make these gaps easier to close as well.  Thanks,

Alex