* [RFC PATCH v3 01/20] vfio/pci: Add SRCU-protected access ops
2026-11-12 14:16 [RFC PATCH v3 00/20] vfio/pci: Handle PCI error recovery and report state to userspace Shameer Kolothum
@ 2026-11-12 14:16 ` Shameer Kolothum
2026-11-12 14:16 ` [RFC PATCH v3 02/20] PCI/PM: Add a power-state helper that tries the bus lock Shameer Kolothum
` (18 subsequent siblings)
19 siblings, 0 replies; 28+ messages in thread
From: Shameer Kolothum @ 2026-11-12 14:16 UTC (permalink / raw)
To: kvm, linux-pci, linux-kernel
Cc: alex, jgg, kevin.tian, kbusch, michal.winiarski,
satyanarayana.k.v.p, sonangp, ankita, nathanc, mochs,
skolothumtho
Before recovering a device, VFIO needs to stop new hardware accesses and
wait for operations already in progress to finish.
Give each open device an SRCU instance to track hardware accesses.
Enable SRCU access tracking only for devices with
pci_recovery_supported set.
Select and call the access operation within the same SRCU read-side
section. Keep copies to and from userspace outside that section so a
page fault cannot hold up recovery.
Introduce live and blocked tables. Initialize the ops pointer to blocked
when the device is created, and select live ops when open finishes.
Close uses a blocking transition that installs blocked ops and drains
SRCU readers before teardown. Open publishes live ops after setup.
Serialize these transitions with access_lock. Internal setup and cleanup
use their own helpers.
Provide an accessor for operations that use their existing synchronization
instead of SRCU. The accessor only returns the selected table. It does
not acquire a lock or keep the device's per-open resources alive.
Subsequent patches convert hardware access paths to these ops and add
support for host PCI error recovery using the same blocked table.
Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
---
drivers/vfio/pci/vfio_pci_priv.h | 15 ++++++
include/linux/vfio_pci_core.h | 10 ++++
drivers/vfio/pci/vfio_pci_core.c | 86 ++++++++++++++++++++++++++++++++
3 files changed, 111 insertions(+)
diff --git a/drivers/vfio/pci/vfio_pci_priv.h b/drivers/vfio/pci/vfio_pci_priv.h
index 4e7162234a2e..97b69e4bc8be 100644
--- a/drivers/vfio/pci/vfio_pci_priv.h
+++ b/drivers/vfio/pci/vfio_pci_priv.h
@@ -73,6 +73,21 @@ u16 vfio_pci_memory_lock_and_enable(struct vfio_pci_core_device *vdev);
void vfio_pci_memory_unlock_and_restore(struct vfio_pci_core_device *vdev,
u16 cmd);
+/* Operations receive kernel buffers only. User copies stay outside SRCU. */
+typedef long (*vfio_pci_access_fn)(struct vfio_pci_core_device *vdev,
+ void *data);
+long vfio_pci_core_access(struct vfio_pci_core_device *vdev,
+ vfio_pci_access_fn fn, void *data);
+
+struct vfio_pci_access_ops {
+ /* Select and call within SRCU. The callback receives kernel buffers. */
+ long (*execute)(struct vfio_pci_core_device *vdev,
+ vfio_pci_access_fn fn, void *data);
+};
+
+const struct vfio_pci_access_ops *
+vfio_pci_access_ops(struct vfio_pci_core_device *vdev);
+
#ifdef CONFIG_VFIO_PCI_IGD
bool vfio_pci_is_intel_display(struct pci_dev *pdev);
int vfio_pci_igd_init(struct vfio_pci_core_device *vdev);
diff --git a/include/linux/vfio_pci_core.h b/include/linux/vfio_pci_core.h
index 2f3fa2787737..88aa7b752231 100644
--- a/include/linux/vfio_pci_core.h
+++ b/include/linux/vfio_pci_core.h
@@ -14,6 +14,7 @@
#include <linux/vfio.h>
#include <linux/irqbypass.h>
#include <linux/rcupdate.h>
+#include <linux/srcu.h>
#include <linux/types.h>
#include <linux/uuid.h>
#include <linux/notifier.h>
@@ -97,6 +98,8 @@ static inline int vfio_pci_core_get_dmabuf_phys(
}
#endif
+struct vfio_pci_access_ops;
+
struct vfio_pci_core_device {
struct vfio_device vdev;
struct pci_dev *pdev;
@@ -131,6 +134,7 @@ struct vfio_pci_core_device {
bool disable_idle_d3:1;
bool nointxmask:1;
bool disable_vga:1;
+ bool pci_recovery_supported:1;
/* Flags modified at runtime - dedicated storage unit */
bool needs_reset;
bool pm_intx_masked;
@@ -150,6 +154,12 @@ struct vfio_pci_core_device {
struct vfio_pci_core_device *sriov_pf_core_dev;
struct notifier_block nb;
struct rw_semaphore memory_lock;
+ struct srcu_struct access_srcu;
+ /* Updated under access_lock. */
+ const struct vfio_pci_access_ops __rcu *access_ops;
+ /* Set after open completes, cleared before close tears down state. */
+ bool device_open;
+ struct mutex access_lock; /* protects changes to access ops and flags */
struct list_head dmabufs;
};
diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
index fcbb2da860e2..b7d81233b99b 100644
--- a/drivers/vfio/pci/vfio_pci_core.c
+++ b/drivers/vfio/pci/vfio_pci_core.c
@@ -312,6 +312,65 @@ static void vfio_pci_probe_power_state(struct vfio_pci_core_device *vdev)
vdev->needs_pm_restore = !(pmcsr & PCI_PM_CTRL_NO_SOFT_RESET);
}
+static long vfio_pci_access_live(struct vfio_pci_core_device *vdev,
+ vfio_pci_access_fn fn, void *data)
+{
+ return fn(vdev, data);
+}
+
+static long vfio_pci_access_blocked(struct vfio_pci_core_device *vdev,
+ vfio_pci_access_fn fn, void *data)
+{
+ return -EIO;
+}
+
+static const struct vfio_pci_access_ops vfio_pci_live_ops = {
+ .execute = vfio_pci_access_live,
+};
+
+static const struct vfio_pci_access_ops vfio_pci_blocked_ops = {
+ .execute = vfio_pci_access_blocked,
+};
+
+/*
+ * The ops tables are static and remain valid across open and close.
+ * Ordinary access callbacks run through vfio_pci_core_access(), which
+ * holds SRCU across table selection and callback execution.
+ */
+const struct vfio_pci_access_ops *
+vfio_pci_access_ops(struct vfio_pci_core_device *vdev)
+{
+ if (!vdev->pci_recovery_supported)
+ return &vfio_pci_live_ops;
+ return rcu_dereference_raw(vdev->access_ops);
+}
+
+/* Block new operations and drain ordinary accesses before teardown or recovery. */
+static void vfio_pci_core_block_access(struct vfio_pci_core_device *vdev)
+{
+ lockdep_assert_held(&vdev->access_lock);
+ rcu_assign_pointer(vdev->access_ops, &vfio_pci_blocked_ops);
+
+ synchronize_srcu(&vdev->access_srcu);
+}
+
+long vfio_pci_core_access(struct vfio_pci_core_device *vdev,
+ vfio_pci_access_fn fn, void *data)
+{
+ const struct vfio_pci_access_ops *ops;
+ long ret;
+ int idx;
+
+ if (!vdev->pci_recovery_supported)
+ return vfio_pci_live_ops.execute(vdev, fn, data);
+
+ idx = srcu_read_lock(&vdev->access_srcu);
+ ops = srcu_dereference(vdev->access_ops, &vdev->access_srcu);
+ ret = ops->execute(vdev, fn, data);
+ srcu_read_unlock(&vdev->access_srcu, idx);
+ return ret;
+}
+
/*
* pci_set_power_state() wrapper handling devices which perform a soft reset on
* D3->D0 transition. Save state prior to D0/1/2->D3, stash it on the vdev,
@@ -649,10 +708,18 @@ int vfio_pci_core_enable(struct vfio_pci_core_device *vdev)
if (!vfio_vga_disabled(vdev) && vfio_pci_is_vga(pdev))
vdev->has_vga = true;
+ if (vdev->pci_recovery_supported) {
+ ret = init_srcu_struct(&vdev->access_srcu);
+ if (ret)
+ goto out_free_config;
+ }
+
vfio_pci_core_map_bars(vdev);
return 0;
+out_free_config:
+ vfio_config_free(vdev);
out_free_zdev:
vfio_pci_zdev_close_device(vdev);
out_free_state:
@@ -799,6 +866,9 @@ void vfio_pci_core_disable(struct vfio_pci_core_device *vdev)
/* Put the pm-runtime usage counter acquired during enable */
if (!vdev->disable_idle_d3)
pm_runtime_put(&pdev->dev);
+
+ if (vdev->pci_recovery_supported)
+ cleanup_srcu_struct(&vdev->access_srcu);
}
EXPORT_SYMBOL_GPL(vfio_pci_core_disable);
@@ -807,6 +877,13 @@ void vfio_pci_core_close_device(struct vfio_device *core_vdev)
struct vfio_pci_core_device *vdev =
container_of(core_vdev, struct vfio_pci_core_device, vdev);
+ if (vdev->pci_recovery_supported) {
+ scoped_guard(mutex, &vdev->access_lock) {
+ WRITE_ONCE(vdev->device_open, false);
+ vfio_pci_core_block_access(vdev);
+ }
+ }
+
if (vdev->sriov_pf_core_dev) {
mutex_lock(&vdev->sriov_pf_core_dev->vf_token->lock);
WARN_ON(!vdev->sriov_pf_core_dev->vf_token->users);
@@ -839,6 +916,12 @@ void vfio_pci_core_finish_enable(struct vfio_pci_core_device *vdev)
vdev->sriov_pf_core_dev->vf_token->users++;
mutex_unlock(&vdev->sriov_pf_core_dev->vf_token->lock);
}
+
+ if (vdev->pci_recovery_supported) {
+ guard(mutex)(&vdev->access_lock);
+ WRITE_ONCE(vdev->device_open, true);
+ rcu_assign_pointer(vdev->access_ops, &vfio_pci_live_ops);
+ }
}
EXPORT_SYMBOL_GPL(vfio_pci_core_finish_enable);
@@ -2183,6 +2266,8 @@ int vfio_pci_core_init_dev(struct vfio_device *core_vdev)
if (ret && ret != -EOPNOTSUPP)
return ret;
INIT_LIST_HEAD(&vdev->dmabufs);
+ mutex_init(&vdev->access_lock);
+ RCU_INIT_POINTER(vdev->access_ops, &vfio_pci_blocked_ops);
init_rwsem(&vdev->memory_lock);
xa_init(&vdev->ctx);
@@ -2197,6 +2282,7 @@ void vfio_pci_core_release_dev(struct vfio_device *core_vdev)
mutex_destroy(&vdev->igate);
mutex_destroy(&vdev->ioeventfds_lock);
+ mutex_destroy(&vdev->access_lock);
kfree(vdev->region);
kfree(vdev->pm_save);
}
--
2.43.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [RFC PATCH v3 02/20] PCI/PM: Add a power-state helper that tries the bus lock
2026-11-12 14:16 [RFC PATCH v3 00/20] vfio/pci: Handle PCI error recovery and report state to userspace Shameer Kolothum
2026-11-12 14:16 ` [RFC PATCH v3 01/20] vfio/pci: Add SRCU-protected access ops Shameer Kolothum
@ 2026-11-12 14:16 ` Shameer Kolothum
2026-11-12 14:16 ` [RFC PATCH v3 03/20] vfio/pci: Avoid waiting for the bus lock when preparing D0 Shameer Kolothum
` (17 subsequent siblings)
19 siblings, 0 replies; 28+ messages in thread
From: Shameer Kolothum @ 2026-11-12 14:16 UTC (permalink / raw)
To: kvm, linux-pci, linux-kernel
Cc: alex, jgg, kevin.tian, kbusch, michal.winiarski,
satyanarayana.k.v.p, sonangp, ankita, nathanc, mochs,
skolothumtho
PCI error recovery can hold pci_bus_sem for reading while waiting for a
lock held by a driver. If that driver also needs the bus read lock to
bring a device to D0, a queued writer can block it. Recovery, the driver
and the writer then all wait for each other.
Add pci_try_set_power_state(), which returns -EAGAIN if it cannot take
the bus lock immediately. The caller can then release its locks. Once
the bus lock is held, use pci_set_power_state_locked() for the transition.
Keep the existing cases that need no power-state change. The transition
itself can still sleep.
Callers that already hold the bus read lock must use the locked helper
instead.
Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
---
include/linux/pci.h | 3 +++
drivers/pci/pci.c | 37 +++++++++++++++++++++++++++++++++++++
2 files changed, 40 insertions(+)
diff --git a/include/linux/pci.h b/include/linux/pci.h
index aa3765a20a96..542fa4af0a43 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1535,6 +1535,7 @@ int pci_load_and_free_saved_state(struct pci_dev *dev,
int pci_platform_power_transition(struct pci_dev *dev, pci_power_t state);
int pci_set_power_state(struct pci_dev *dev, pci_power_t state);
int pci_set_power_state_locked(struct pci_dev *dev, pci_power_t state);
+int pci_try_set_power_state(struct pci_dev *dev, pci_power_t state);
pci_power_t pci_choose_state(struct pci_dev *dev, pm_message_t state);
bool pci_pme_capable(struct pci_dev *dev, pci_power_t state);
void pci_pme_active(struct pci_dev *dev, bool enable);
@@ -2216,6 +2217,8 @@ static inline int pci_set_power_state(struct pci_dev *dev, pci_power_t state)
{ return 0; }
static inline int pci_set_power_state_locked(struct pci_dev *dev, pci_power_t state)
{ return 0; }
+static inline int pci_try_set_power_state(struct pci_dev *dev, pci_power_t state)
+{ return 0; }
static inline int pci_wake_from_d3(struct pci_dev *dev, bool enable)
{ return 0; }
static inline pci_power_t pci_choose_state(struct pci_dev *dev,
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 7c6f2aba8144..f2ebd19204aa 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -1655,6 +1655,43 @@ int pci_set_power_state_locked(struct pci_dev *dev, pci_power_t state)
}
EXPORT_SYMBOL(pci_set_power_state_locked);
+/**
+ * pci_try_set_power_state - Set power state without waiting for the bus lock
+ * @dev: PCI device to handle
+ * @state: Target PCI power state
+ *
+ * Like pci_set_power_state(), but return -EAGAIN without changing the power
+ * state if the bus lock cannot be acquired immediately. The transition
+ * itself can sleep.
+ *
+ * Callers already holding the bus read lock must use
+ * pci_set_power_state_locked().
+ */
+int pci_try_set_power_state(struct pci_dev *dev, pci_power_t state)
+{
+ int ret;
+
+ if (state > PCI_D3cold)
+ state = PCI_D3cold;
+ else if (state < PCI_D0)
+ state = PCI_D0;
+ else if ((state == PCI_D1 || state == PCI_D2) && pci_no_d1d2(dev))
+ return 0;
+
+ if (dev->current_state == state)
+ return 0;
+ if (state >= PCI_D3hot && (dev->dev_flags & PCI_DEV_FLAGS_NO_D3))
+ return 0;
+
+ if (!down_read_trylock(&pci_bus_sem))
+ return -EAGAIN;
+ ret = pci_set_power_state_locked(dev, state);
+ up_read(&pci_bus_sem);
+
+ return ret;
+}
+EXPORT_SYMBOL(pci_try_set_power_state);
+
#define PCI_EXP_SAVE_REGS 7
static struct pci_cap_saved_state *_pci_find_saved_cap(struct pci_dev *pci_dev,
--
2.43.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [RFC PATCH v3 03/20] vfio/pci: Avoid waiting for the bus lock when preparing D0
2026-11-12 14:16 [RFC PATCH v3 00/20] vfio/pci: Handle PCI error recovery and report state to userspace Shameer Kolothum
2026-11-12 14:16 ` [RFC PATCH v3 01/20] vfio/pci: Add SRCU-protected access ops Shameer Kolothum
2026-11-12 14:16 ` [RFC PATCH v3 02/20] PCI/PM: Add a power-state helper that tries the bus lock Shameer Kolothum
@ 2026-11-12 14:16 ` Shameer Kolothum
2026-11-12 14:16 ` [RFC PATCH v3 04/20] vfio/pci: Use access ops for config-space accesses Shameer Kolothum
` (16 subsequent siblings)
19 siblings, 0 replies; 28+ messages in thread
From: Shameer Kolothum @ 2026-11-12 14:16 UTC (permalink / raw)
To: kvm, linux-pci, linux-kernel
Cc: alex, jgg, kevin.tian, kbusch, michal.winiarski,
satyanarayana.k.v.p, sonangp, ankita, nathanc, mochs,
skolothumtho
A D0 transition can wait behind a pci_bus_sem writer while holding locks
needed by PCI error recovery. Recovery holds the bus read lock, leaving
all three tasks waiting for each other.
Use pci_try_set_power_state() for D0 preparation during function reset,
hot reset, runtime suspend and SR-IOV enable. Preserve VFIO's saved PM
state handling and unwind reset and SR-IOV requests if D0 preparation
fails.
Runtime suspend returns -EAGAIN on bus-lock contention. Keep its existing
handling of other D0 errors to avoid leaving runtime PM in an error state.
Apply this even when recovery is disabled, since global IRQ cleanup can
wait for another device's D0 transition. Low-power transitions keep the
existing helper. Subsequent patch handles config-space power requests.
Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
---
drivers/vfio/pci/vfio_pci_priv.h | 2 ++
drivers/vfio/pci/vfio_pci_core.c | 49 +++++++++++++++++++++++++++-----
2 files changed, 44 insertions(+), 7 deletions(-)
diff --git a/drivers/vfio/pci/vfio_pci_priv.h b/drivers/vfio/pci/vfio_pci_priv.h
index 97b69e4bc8be..2a15a8956205 100644
--- a/drivers/vfio/pci/vfio_pci_priv.h
+++ b/drivers/vfio/pci/vfio_pci_priv.h
@@ -67,6 +67,8 @@ void vfio_config_free(struct vfio_pci_core_device *vdev);
int vfio_pci_set_power_state(struct vfio_pci_core_device *vdev,
pci_power_t state);
+int vfio_pci_try_set_power_state(struct vfio_pci_core_device *vdev,
+ pci_power_t state);
void vfio_pci_zap_and_down_write_memory_lock(struct vfio_pci_core_device *vdev);
u16 vfio_pci_memory_lock_and_enable(struct vfio_pci_core_device *vdev);
diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
index b7d81233b99b..6529eab7865b 100644
--- a/drivers/vfio/pci/vfio_pci_core.c
+++ b/drivers/vfio/pci/vfio_pci_core.c
@@ -378,7 +378,10 @@ long vfio_pci_core_access(struct vfio_pci_core_device *vdev,
* by PM capability emulation and separately from pci_dev internal saved state
* to avoid it being overwritten and consumed around other resets.
*/
-int vfio_pci_set_power_state(struct vfio_pci_core_device *vdev, pci_power_t state)
+static int __vfio_pci_set_power_state(struct vfio_pci_core_device *vdev,
+ pci_power_t state,
+ int (*set_power_state)(struct pci_dev *,
+ pci_power_t))
{
struct pci_dev *pdev = vdev->pdev;
bool needs_restore = false, needs_save = false;
@@ -401,7 +404,7 @@ int vfio_pci_set_power_state(struct vfio_pci_core_device *vdev, pci_power_t stat
needs_restore = true;
}
- ret = pci_set_power_state(pdev, state);
+ ret = set_power_state(pdev, state);
if (!ret) {
/* D3 might be unsupported via quirk, skip unless in D3 */
@@ -429,6 +432,20 @@ int vfio_pci_set_power_state(struct vfio_pci_core_device *vdev, pci_power_t stat
return ret;
}
+int vfio_pci_set_power_state(struct vfio_pci_core_device *vdev, pci_power_t state)
+{
+ return __vfio_pci_set_power_state(vdev, state, pci_set_power_state);
+}
+
+/* Avoid waiting for pci_bus_sem under locks needed by PCI recovery. */
+int vfio_pci_try_set_power_state(struct vfio_pci_core_device *vdev,
+ pci_power_t state)
+{
+ return __vfio_pci_set_power_state(vdev, state,
+ state == PCI_D0 ? pci_try_set_power_state :
+ pci_set_power_state);
+}
+
static int vfio_pci_runtime_pm_entry(struct vfio_pci_core_device *vdev,
struct eventfd_ctx *efdctx)
{
@@ -550,6 +567,8 @@ static int vfio_pci_core_pm_exit(struct vfio_pci_core_device *vdev, u32 flags,
#ifdef CONFIG_PM
int vfio_pci_core_runtime_suspend(struct vfio_pci_core_device *vdev)
{
+ int ret;
+
down_write(&vdev->memory_lock);
/*
* The user can move the device into D3hot state before invoking
@@ -559,8 +578,11 @@ int vfio_pci_core_runtime_suspend(struct vfio_pci_core_device *vdev)
* NoSoftRst-, it will help in restoring the original state
* (saved locally in 'vdev->pm_save').
*/
- vfio_pci_set_power_state(vdev, PCI_D0);
+ ret = vfio_pci_try_set_power_state(vdev, PCI_D0);
up_write(&vdev->memory_lock);
+ /* Preserve the existing handling of other D0 transition errors. */
+ if (ret == -EAGAIN)
+ return ret;
/*
* If INTx is enabled, then mask INTx before going into the runtime
@@ -1401,7 +1423,11 @@ static int vfio_pci_ioctl_reset(struct vfio_pci_core_device *vdev,
* reset without restoring the original state (saved locally in
* 'vdev->pm_save').
*/
- vfio_pci_set_power_state(vdev, PCI_D0);
+ ret = vfio_pci_try_set_power_state(vdev, PCI_D0);
+ if (ret) {
+ up_write(&vdev->memory_lock);
+ return ret;
+ }
vfio_pci_dma_buf_move(vdev, true);
ret = pci_try_reset_function(vdev->pdev);
@@ -2453,7 +2479,12 @@ int vfio_pci_core_sriov_configure(struct vfio_pci_core_device *vdev,
goto out_del;
down_write(&vdev->memory_lock);
- vfio_pci_set_power_state(vdev, PCI_D0);
+ ret = vfio_pci_try_set_power_state(vdev, PCI_D0);
+ if (ret) {
+ up_write(&vdev->memory_lock);
+ pm_runtime_put(&pdev->dev);
+ goto out_del;
+ }
vdev->sriov_active = true;
up_write(&vdev->memory_lock);
ret = pci_enable_sriov(pdev, nr_virtfn);
@@ -2656,11 +2687,15 @@ static int vfio_pci_dev_set_hot_reset(struct vfio_device_set *dev_set,
* cause the PCI config space reset without restoring the original
* state (saved locally in 'vdev->pm_save').
*/
- list_for_each_entry(vdev, &dev_set->device_list, vdev.dev_set_list)
- vfio_pci_set_power_state(vdev, PCI_D0);
+ list_for_each_entry(vdev, &dev_set->device_list, vdev.dev_set_list) {
+ ret = vfio_pci_try_set_power_state(vdev, PCI_D0);
+ if (ret)
+ goto out_unlock_memory;
+ }
ret = pci_reset_bus(pdev);
+out_unlock_memory:
vdev = list_last_entry(&dev_set->device_list,
struct vfio_pci_core_device, vdev.dev_set_list);
--
2.43.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [RFC PATCH v3 04/20] vfio/pci: Use access ops for config-space accesses
2026-11-12 14:16 [RFC PATCH v3 00/20] vfio/pci: Handle PCI error recovery and report state to userspace Shameer Kolothum
` (2 preceding siblings ...)
2026-11-12 14:16 ` [RFC PATCH v3 03/20] vfio/pci: Avoid waiting for the bus lock when preparing D0 Shameer Kolothum
@ 2026-11-12 14:16 ` Shameer Kolothum
2026-11-12 14:41 ` sashiko-bot
2026-11-12 14:16 ` [RFC PATCH v3 05/20] vfio/pci: Buffer ROM reads before copying to userspace Shameer Kolothum
` (15 subsequent siblings)
19 siblings, 1 reply; 28+ messages in thread
From: Shameer Kolothum @ 2026-11-12 14:16 UTC (permalink / raw)
To: kvm, linux-pci, linux-kernel
Cc: alex, jgg, kevin.tian, kbusch, michal.winiarski,
satyanarayana.k.v.p, sonangp, ankita, nathanc, mochs,
skolothumtho
Config-space handlers can touch hardware during recovery. Run them
through vfio_pci_core_access() and keep userspace copies outside SRCU
so page faults cannot delay recovery.
Defer D0 requests until the config handler has left SRCU. Select the
power-state operation under memory_lock and use the bus try-lock helper
for D0. Return transition errors to userspace.
Blocked power operations reject requests. Open and close continue to
use the internal power helper.
Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
---
drivers/vfio/pci/vfio_pci_priv.h | 5 ++
drivers/vfio/pci/vfio_pci_config.c | 122 +++++++++++++++++++++++------
drivers/vfio/pci/vfio_pci_core.c | 8 ++
3 files changed, 109 insertions(+), 26 deletions(-)
diff --git a/drivers/vfio/pci/vfio_pci_priv.h b/drivers/vfio/pci/vfio_pci_priv.h
index 2a15a8956205..0ea7c710dc38 100644
--- a/drivers/vfio/pci/vfio_pci_priv.h
+++ b/drivers/vfio/pci/vfio_pci_priv.h
@@ -41,6 +41,9 @@ ssize_t vfio_pci_config_rw_single(struct vfio_pci_core_device *vdev,
char __user *buf, size_t count, loff_t *ppos,
bool iswrite);
+int vfio_lock_and_set_power_state(struct vfio_pci_core_device *vdev,
+ pci_power_t state);
+
ssize_t vfio_pci_bar_rw(struct vfio_pci_core_device *vdev, char __user *buf,
size_t count, loff_t *ppos, bool iswrite);
@@ -85,6 +88,8 @@ struct vfio_pci_access_ops {
/* Select and call within SRCU. The callback receives kernel buffers. */
long (*execute)(struct vfio_pci_core_device *vdev,
vfio_pci_access_fn fn, void *data);
+ /* Caller holds memory_lock. */
+ int (*set_power_state)(struct vfio_pci_core_device *vdev, pci_power_t state);
};
const struct vfio_pci_access_ops *
diff --git a/drivers/vfio/pci/vfio_pci_config.c b/drivers/vfio/pci/vfio_pci_config.c
index 9914f3ac69ae..1e54e2e8b2ef 100644
--- a/drivers/vfio/pci/vfio_pci_config.c
+++ b/drivers/vfio/pci/vfio_pci_config.c
@@ -111,8 +111,13 @@ struct perm_bits {
u8 *write; /* writeable bits */
int (*readfn)(struct vfio_pci_core_device *vdev, int pos, int count,
struct perm_bits *perm, int offset, __le32 *val);
+ /*
+ * Set power_up for D0 requests. The caller performs the transition
+ * after leaving SRCU.
+ */
int (*writefn)(struct vfio_pci_core_device *vdev, int pos, int count,
- struct perm_bits *perm, int offset, __le32 val);
+ struct perm_bits *perm, int offset, __le32 val,
+ bool *power_up);
};
#define NO_VIRT 0
@@ -200,7 +205,8 @@ static int vfio_default_config_read(struct vfio_pci_core_device *vdev, int pos,
static int vfio_default_config_write(struct vfio_pci_core_device *vdev, int pos,
int count, struct perm_bits *perm,
- int offset, __le32 val)
+ int offset, __le32 val,
+ bool *power_up)
{
__le32 virt = 0, write = 0;
@@ -272,7 +278,8 @@ static int vfio_direct_config_read(struct vfio_pci_core_device *vdev, int pos,
/* Raw access skips any kind of virtualization */
static int vfio_raw_config_write(struct vfio_pci_core_device *vdev, int pos,
int count, struct perm_bits *perm,
- int offset, __le32 val)
+ int offset, __le32 val,
+ bool *power_up)
{
int ret;
@@ -299,7 +306,8 @@ static int vfio_raw_config_read(struct vfio_pci_core_device *vdev, int pos,
/* Virt access uses only virtualization */
static int vfio_virt_config_write(struct vfio_pci_core_device *vdev, int pos,
int count, struct perm_bits *perm,
- int offset, __le32 val)
+ int offset, __le32 val,
+ bool *power_up)
{
memcpy(vdev->vconfig + pos, &val, count);
return count;
@@ -563,7 +571,8 @@ static bool vfio_need_bar_restore(struct vfio_pci_core_device *vdev)
static int vfio_basic_config_write(struct vfio_pci_core_device *vdev, int pos,
int count, struct perm_bits *perm,
- int offset, __le32 val)
+ int offset, __le32 val,
+ bool *power_up)
{
struct pci_dev *pdev = vdev->pdev;
__le16 *virt_cmd;
@@ -613,7 +622,8 @@ static int vfio_basic_config_write(struct vfio_pci_core_device *vdev, int pos,
vfio_bar_restore(vdev);
}
- count = vfio_default_config_write(vdev, pos, count, perm, offset, val);
+ count = vfio_default_config_write(vdev, pos, count, perm, offset, val,
+ power_up);
if (count < 0) {
if (offset == PCI_COMMAND)
up_write(&vdev->memory_lock);
@@ -707,11 +717,13 @@ static int __init init_pci_cap_basic_perm(struct perm_bits *perm)
/*
* It takes all the required locks to protect the access of power related
- * variables and then invokes vfio_pci_set_power_state().
+ * variables and then invokes the selected power-state operation.
*/
-static void vfio_lock_and_set_power_state(struct vfio_pci_core_device *vdev,
- pci_power_t state)
+int vfio_lock_and_set_power_state(struct vfio_pci_core_device *vdev,
+ pci_power_t state)
{
+ int ret;
+
if (state >= PCI_D3hot) {
vfio_pci_zap_and_down_write_memory_lock(vdev);
vfio_pci_dma_buf_move(vdev, true);
@@ -719,27 +731,36 @@ static void vfio_lock_and_set_power_state(struct vfio_pci_core_device *vdev,
down_write(&vdev->memory_lock);
}
- vfio_pci_set_power_state(vdev, state);
+ ret = vfio_pci_access_ops(vdev)->set_power_state(vdev, state);
if (__vfio_pci_memory_enabled(vdev))
vfio_pci_dma_buf_move(vdev, false);
up_write(&vdev->memory_lock);
+
+ return ret;
}
static int vfio_pm_config_write(struct vfio_pci_core_device *vdev, int pos,
int count, struct perm_bits *perm,
- int offset, __le32 val)
+ int offset, __le32 val,
+ bool *power_up)
{
- count = vfio_default_config_write(vdev, pos, count, perm, offset, val);
+ count = vfio_default_config_write(vdev, pos, count, perm, offset, val,
+ power_up);
if (count < 0)
return count;
if (offset == PCI_PM_CTRL) {
pci_power_t state;
+ int ret;
switch (le32_to_cpu(val) & PCI_PM_CTRL_STATE_MASK) {
case 0:
- state = PCI_D0;
- break;
+ /*
+ * Request D0 after leaving access_srcu. Otherwise, the ASPM update can
+ * wait for the bus lock while AER waits for this SRCU reader.
+ */
+ *power_up = true;
+ return count;
case 1:
state = PCI_D1;
break;
@@ -751,7 +772,9 @@ static int vfio_pm_config_write(struct vfio_pci_core_device *vdev, int pos,
break;
}
- vfio_lock_and_set_power_state(vdev, state);
+ ret = vfio_lock_and_set_power_state(vdev, state);
+ if (ret)
+ return ret;
}
return count;
@@ -799,7 +822,8 @@ static int __init init_pci_cap_pm_perm(struct perm_bits *perm)
static int vfio_vpd_config_write(struct vfio_pci_core_device *vdev, int pos,
int count, struct perm_bits *perm,
- int offset, __le32 val)
+ int offset, __le32 val,
+ bool *power_up)
{
struct pci_dev *pdev = vdev->pdev;
__le16 *paddr = (__le16 *)(vdev->vconfig + pos - offset + PCI_VPD_ADDR);
@@ -812,7 +836,8 @@ static int vfio_vpd_config_write(struct vfio_pci_core_device *vdev, int pos,
* of PCI_VPD_ADDR, then the PCI_VPD_ADDR_F bit is written and we
* have work to do.
*/
- count = vfio_default_config_write(vdev, pos, count, perm, offset, val);
+ count = vfio_default_config_write(vdev, pos, count, perm, offset, val,
+ power_up);
if (count < 0 || offset > PCI_VPD_ADDR + 1 ||
offset + count <= PCI_VPD_ADDR + 1)
return count;
@@ -881,13 +906,15 @@ static int __init init_pci_cap_pcix_perm(struct perm_bits *perm)
static int vfio_exp_config_write(struct vfio_pci_core_device *vdev, int pos,
int count, struct perm_bits *perm,
- int offset, __le32 val)
+ int offset, __le32 val,
+ bool *power_up)
{
__le16 *ctrl = (__le16 *)(vdev->vconfig + pos -
offset + PCI_EXP_DEVCTL);
int readrq = le16_to_cpu(*ctrl) & PCI_EXP_DEVCTL_READRQ;
- count = vfio_default_config_write(vdev, pos, count, perm, offset, val);
+ count = vfio_default_config_write(vdev, pos, count, perm, offset, val,
+ power_up);
if (count < 0)
return count;
@@ -968,11 +995,13 @@ static int __init init_pci_cap_exp_perm(struct perm_bits *perm)
static int vfio_af_config_write(struct vfio_pci_core_device *vdev, int pos,
int count, struct perm_bits *perm,
- int offset, __le32 val)
+ int offset, __le32 val,
+ bool *power_up)
{
u8 *ctrl = vdev->vconfig + pos - offset + PCI_AF_CTRL;
- count = vfio_default_config_write(vdev, pos, count, perm, offset, val);
+ count = vfio_default_config_write(vdev, pos, count, perm, offset, val,
+ power_up);
if (count < 0)
return count;
@@ -1168,9 +1197,11 @@ static int vfio_msi_config_read(struct vfio_pci_core_device *vdev, int pos,
static int vfio_msi_config_write(struct vfio_pci_core_device *vdev, int pos,
int count, struct perm_bits *perm,
- int offset, __le32 val)
+ int offset, __le32 val,
+ bool *power_up)
{
- count = vfio_default_config_write(vdev, pos, count, perm, offset, val);
+ count = vfio_default_config_write(vdev, pos, count, perm, offset, val,
+ power_up);
if (count < 0)
return count;
@@ -1881,6 +1912,28 @@ static size_t vfio_pci_cap_remaining_dword(struct vfio_pci_core_device *vdev,
return i;
}
+struct vfio_pci_config_access {
+ struct perm_bits *perm;
+ loff_t pos;
+ int count;
+ int offset;
+ __le32 val;
+ bool power_up;
+ bool iswrite;
+};
+
+static long vfio_pci_access_config(struct vfio_pci_core_device *vdev, void *data)
+{
+ struct vfio_pci_config_access *access = data;
+ struct perm_bits *perm = access->perm;
+
+ if (access->iswrite)
+ return perm->writefn(vdev, access->pos, access->count, perm,
+ access->offset, access->val, &access->power_up);
+ return perm->readfn(vdev, access->pos, access->count, perm,
+ access->offset, &access->val);
+}
+
ssize_t vfio_pci_config_rw_single(struct vfio_pci_core_device *vdev,
char __user *buf, size_t count, loff_t *ppos,
bool iswrite)
@@ -1889,6 +1942,7 @@ ssize_t vfio_pci_config_rw_single(struct vfio_pci_core_device *vdev,
struct perm_bits *perm;
__le32 val = 0;
int cap_start = 0, offset;
+ struct vfio_pci_config_access access = {};
u8 cap_id;
ssize_t ret;
@@ -1949,6 +2003,12 @@ ssize_t vfio_pci_config_rw_single(struct vfio_pci_core_device *vdev,
WARN_ON(cap_start > *ppos);
offset = *ppos - cap_start;
+ access.perm = perm;
+ access.pos = *ppos;
+ access.count = count;
+ access.offset = offset;
+ access.iswrite = iswrite;
+
if (iswrite) {
if (!perm->writefn)
@@ -1957,11 +2017,21 @@ ssize_t vfio_pci_config_rw_single(struct vfio_pci_core_device *vdev,
if (copy_from_user(&val, buf, count))
return -EFAULT;
- ret = perm->writefn(vdev, *ppos, count, perm, offset, val);
+ access.val = val;
+ ret = vfio_pci_core_access(vdev, vfio_pci_access_config, &access);
+ if (ret < 0)
+ return ret;
+ if (access.power_up) {
+ int err = vfio_lock_and_set_power_state(vdev, PCI_D0);
+
+ if (err)
+ return err;
+ }
} else {
if (perm->readfn) {
- ret = perm->readfn(vdev, *ppos, count,
- perm, offset, &val);
+ access.val = val;
+ ret = vfio_pci_core_access(vdev, vfio_pci_access_config, &access);
+ val = access.val;
if (ret < 0)
return ret;
}
diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
index 6529eab7865b..21c0dda37dd1 100644
--- a/drivers/vfio/pci/vfio_pci_core.c
+++ b/drivers/vfio/pci/vfio_pci_core.c
@@ -324,12 +324,20 @@ static long vfio_pci_access_blocked(struct vfio_pci_core_device *vdev,
return -EIO;
}
+static int vfio_pci_power_blocked(struct vfio_pci_core_device *vdev,
+ pci_power_t state)
+{
+ return -EIO;
+}
+
static const struct vfio_pci_access_ops vfio_pci_live_ops = {
.execute = vfio_pci_access_live,
+ .set_power_state = vfio_pci_try_set_power_state,
};
static const struct vfio_pci_access_ops vfio_pci_blocked_ops = {
.execute = vfio_pci_access_blocked,
+ .set_power_state = vfio_pci_power_blocked,
};
/*
--
2.43.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [RFC PATCH v3 04/20] vfio/pci: Use access ops for config-space accesses
2026-11-12 14:16 ` [RFC PATCH v3 04/20] vfio/pci: Use access ops for config-space accesses Shameer Kolothum
@ 2026-11-12 14:41 ` sashiko-bot
0 siblings, 0 replies; 28+ messages in thread
From: sashiko-bot @ 2026-11-12 14:41 UTC (permalink / raw)
To: Shameer Kolothum, kvm, linux-pci
Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [High] vfio/pci: pci_bus_sem lock inversion in config-space FLR under
access_srcu
--
commit 07dee5e58e5bbc760b7e991d4fc5ab36537fd5e0
Author: Shameer Kolothum <skolothumtho@nvidia.com>
vfio/pci: Use access ops for config-space accesses
This commit runs PCI config-space accesses through vfio_pci_core_access(),
routes power-state transitions through vfio_pci_access_ops, and defers
PCI_PM_CTRL D0 transitions until after leaving access_srcu.
> diff --git a/drivers/vfio/pci/vfio_pci_config.c b/drivers/vfio/pci/vfio_pci_config.c
> index 9914f3ac69aef..1e54e2e8b2ef8 100644
> --- a/drivers/vfio/pci/vfio_pci_config.c
> +++ b/drivers/vfio/pci/vfio_pci_config.c
[ ... ]
> @@ -1957,11 +2017,21 @@ ssize_t vfio_pci_config_rw_single(struct vfio_pci_core_device *vdev,
> if (copy_from_user(&val, buf, count))
> return -EFAULT;
>
> - ret = perm->writefn(vdev, *ppos, count, perm, offset, val);
> + access.val = val;
> + ret = vfio_pci_core_access(vdev, vfio_pci_access_config, &access);
> + if (ret < 0)
> + return ret;
> + if (access.power_up) {
> + int err = vfio_lock_and_set_power_state(vdev, PCI_D0);
> +
> + if (err)
> + return err;
> + }
[Severity: High]
Can a config-space FLR write in vfio_exp_config_write() or
vfio_af_config_write() invert the lock order between access_srcu and
pci_bus_sem when the device is in a low-power state?
vfio_pm_config_write() defers D0 transitions outside access_srcu so that
pcie_aspm_pm_state_change() does not block on pci_bus_sem while AER waits
for access_srcu readers. However, if userspace puts the device into
D1-D3hot via PCI_PM_CTRL and then writes PCI_EXP_DEVCTL_BCR_FLR or
PCI_AF_CTRL_FLR, vfio_pci_core_access() runs the handler inside
access_srcu, where it takes vdev->memory_lock and calls
pci_try_reset_function():
drivers/pci/pci.c:pci_dev_save_and_disable() {
...
pci_set_power_state(dev, PCI_D0);
...
}
pci_set_power_state() calls pci_set_full_power_state(dev, false) ->
pcie_aspm_pm_state_change() -> down_read(&pci_bus_sem) while holding both
access_srcu and vdev->memory_lock:
CPU0 CPU1 (AER recovery)
---- -------------------
down_read(&pci_bus_sem)
vfio_pci_config_rw_single()
vfio_pci_core_access()
srcu_read_lock(&access_srcu)
...
vfio_exp_config_write()
down_write(&memory_lock)
pci_try_reset_function()
...
down_read(&pci_bus_sem)
synchronize_srcu(&access_srcu)
*** DEADLOCK ***
(if a writer is queued on pci_bus_sem, or against any path nesting
pci_bus_sem -> access_srcu / memory_lock)
In addition, letting pci_dev_save_and_disable() perform the D3hot -> D0
transition directly bypasses vfio_pci_try_set_power_state() and skips
restoring vdev->pm_save on NoSoftRst- devices.
Calling vfio_pci_try_set_power_state(vdev, PCI_D0) before
pci_try_reset_function() in vfio_exp_config_write() and
vfio_af_config_write() (as commit 358432f8bbe9 ("vfio/pci: Avoid waiting
for the bus lock when preparing D0") did for vfio_pci_ioctl_reset()), or
deferring the FLR outside access_srcu, would avoid this.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261112141632.918954-1-skolothumtho@nvidia.com?part=4
^ permalink raw reply [flat|nested] 28+ messages in thread
* [RFC PATCH v3 05/20] vfio/pci: Buffer ROM reads before copying to userspace
2026-11-12 14:16 [RFC PATCH v3 00/20] vfio/pci: Handle PCI error recovery and report state to userspace Shameer Kolothum
` (3 preceding siblings ...)
2026-11-12 14:16 ` [RFC PATCH v3 04/20] vfio/pci: Use access ops for config-space accesses Shameer Kolothum
@ 2026-11-12 14:16 ` Shameer Kolothum
2026-11-12 14:40 ` sashiko-bot
2026-11-12 14:16 ` [RFC PATCH v3 06/20] vfio/pci: Use access ops for BAR and ROM accesses Shameer Kolothum
` (14 subsequent siblings)
19 siblings, 1 reply; 28+ messages in thread
From: Shameer Kolothum @ 2026-11-12 14:16 UTC (permalink / raw)
To: kvm, linux-pci, linux-kernel
Cc: alex, jgg, kevin.tian, kbusch, michal.winiarski,
satyanarayana.k.v.p, sonangp, ankita, nathanc, mochs,
skolothumtho
ROM reads copy directly to userspace while ROM decoding is enabled.
Putting the whole operation inside SRCU would let a userspace page
fault delay recovery.
Read the requested ROM data into a kernel buffer, then unmap the ROM
before copying to userspace. Hold memory_lock across mapping and reading
to serialize ROM decoding with power changes and other ROM accesses.
Preserve byte, word and dword reads and 0xff padding beyond the ROM.
Subsequent patch adds SRCU protection around the hardware access.
Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
---
drivers/vfio/pci/vfio_pci_rdwr.c | 147 ++++++++++++++++++++++---------
1 file changed, 104 insertions(+), 43 deletions(-)
diff --git a/drivers/vfio/pci/vfio_pci_rdwr.c b/drivers/vfio/pci/vfio_pci_rdwr.c
index df8848087681..10cd091031fe 100644
--- a/drivers/vfio/pci/vfio_pci_rdwr.c
+++ b/drivers/vfio/pci/vfio_pci_rdwr.c
@@ -10,8 +10,10 @@
* Author: Tom Lyon, pugs@cisco.com
*/
+#include <linux/align.h>
#include <linux/fs.h>
#include <linux/pci.h>
+#include <linux/slab.h>
#include <linux/uaccess.h>
#include <linux/io.h>
#include <linux/vfio.h>
@@ -198,6 +200,96 @@ ssize_t vfio_pci_core_do_io_rw(struct vfio_pci_core_device *vdev, bool test_mem,
}
EXPORT_SYMBOL_GPL(vfio_pci_core_do_io_rw);
+static ssize_t vfio_pci_rom_read(struct vfio_pci_core_device *vdev,
+ char __user *buf, size_t count, loff_t pos)
+{
+ struct pci_dev *pdev = vdev->pdev;
+ bool rom_bar = pci_resource_start(pdev, PCI_ROM_RESOURCE);
+ size_t size, length = 0, done = 0;
+ void *data = NULL;
+ void __iomem *io;
+ ssize_t ret;
+
+ /* Protect ROM decoding against concurrent power changes and ROM accesses. */
+ down_write(&vdev->memory_lock);
+ if (rom_bar) {
+ io = pci_map_rom(pdev, &size);
+ } else {
+ io = ioremap(pdev->rom, pdev->romlen);
+ size = pdev->romlen;
+ }
+ if (!io) {
+ ret = -ENOMEM;
+ goto out_unlock;
+ }
+
+ /* Buffer only ROM data, not unused space in a large ROM BAR. */
+ if (pos < size)
+ length = min(count, size - (size_t)pos);
+ if (length) {
+ if ((pci_resource_flags(pdev, PCI_ROM_RESOURCE) & IORESOURCE_MEM) &&
+ !__vfio_pci_memory_enabled(vdev)) {
+ ret = -EIO;
+ goto out_unmap;
+ }
+ data = kvmalloc(length, GFP_KERNEL_ACCOUNT);
+ if (!data) {
+ ret = -ENOMEM;
+ goto out_unmap;
+ }
+ }
+
+ /*
+ * Certain devices (e.g. Intel X710) don't support qword
+ * access to the ROM bar. Otherwise PCI AER errors might be
+ * triggered.
+ *
+ * Disable qword access to the ROM bar universally, which
+ * worked reliably for years before qword access is enabled.
+ */
+ while (done < length) {
+ if (length - done >= 4 && IS_ALIGNED(pos + done, 4)) {
+ u32 val = vfio_ioread32(io + pos + done);
+
+ memcpy(data + done, &val, sizeof(val));
+ done += sizeof(val);
+ } else if (length - done >= 2 && IS_ALIGNED(pos + done, 2)) {
+ u16 val = vfio_ioread16(io + pos + done);
+
+ memcpy(data + done, &val, sizeof(val));
+ done += sizeof(val);
+ } else {
+ ((u8 *)data)[done] = vfio_ioread8(io + pos + done);
+ done++;
+ }
+ }
+ ret = count;
+
+out_unmap:
+ if (rom_bar)
+ pci_unmap_rom(pdev, io);
+ else
+ iounmap(io);
+out_unlock:
+ up_write(&vdev->memory_lock);
+ if (ret < 0)
+ goto out_free;
+
+ if (length && copy_to_user(buf, data, length)) {
+ ret = -EFAULT;
+ goto out_free;
+ }
+ for (done = length; done < count; done++) {
+ if (put_user((u8)0xff, buf + done)) {
+ ret = -EFAULT;
+ break;
+ }
+ }
+out_free:
+ kvfree(data);
+ return ret;
+}
+
ssize_t vfio_pci_bar_rw(struct vfio_pci_core_device *vdev, char __user *buf,
size_t count, loff_t *ppos, bool iswrite)
{
@@ -209,7 +301,6 @@ ssize_t vfio_pci_bar_rw(struct vfio_pci_core_device *vdev, char __user *buf,
void __iomem *io;
struct resource *res = &vdev->pdev->resource[bar];
ssize_t done;
- enum vfio_pci_io_width max_width = VFIO_PCI_IO_WIDTH_8;
if (pci_resource_start(pdev, bar))
end = pci_resource_len(pdev, bar);
@@ -224,57 +315,27 @@ ssize_t vfio_pci_bar_rw(struct vfio_pci_core_device *vdev, char __user *buf,
count = min(count, (size_t)(end - pos));
if (bar == PCI_ROM_RESOURCE) {
- /*
- * The ROM can fill less space than the BAR, so we start the
- * excluded range at the end of the actual ROM. This makes
- * filling large ROM BARs much faster.
- */
- if (pci_resource_start(pdev, bar)) {
- io = pci_map_rom(pdev, &x_start);
- } else {
- io = ioremap(pdev->rom, pdev->romlen);
- x_start = pdev->romlen;
- }
- if (!io)
- return -ENOMEM;
- x_end = end;
-
- /*
- * Certain devices (e.g. Intel X710) don't support qword
- * access to the ROM bar. Otherwise PCI AER errors might be
- * triggered.
- *
- * Disable qword access to the ROM bar universally, which
- * worked reliably for years before qword access is enabled.
- */
- max_width = VFIO_PCI_IO_WIDTH_4;
+ if (iswrite)
+ return -EINVAL;
+ done = vfio_pci_rom_read(vdev, buf, count, pos);
} else {
io = vfio_pci_core_get_iomap(vdev, bar);
- if (IS_ERR(io)) {
- done = PTR_ERR(io);
- goto out;
+ if (IS_ERR(io))
+ return PTR_ERR(io);
+
+ if (bar == vdev->msix_bar) {
+ x_start = vdev->msix_offset;
+ x_end = vdev->msix_offset + vdev->msix_size;
}
- }
- if (bar == vdev->msix_bar) {
- x_start = vdev->msix_offset;
- x_end = vdev->msix_offset + vdev->msix_size;
+ done = vfio_pci_core_do_io_rw(vdev, res->flags & IORESOURCE_MEM,
+ io, buf, pos, count, x_start, x_end,
+ iswrite, VFIO_PCI_IO_WIDTH_8);
}
- done = vfio_pci_core_do_io_rw(vdev, res->flags & IORESOURCE_MEM, io, buf, pos,
- count, x_start, x_end, iswrite, max_width);
-
if (done >= 0)
*ppos += done;
- if (bar == PCI_ROM_RESOURCE) {
- if (pci_resource_start(pdev, bar))
- pci_unmap_rom(pdev, io);
- else
- iounmap(io);
- }
-
-out:
return done;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [RFC PATCH v3 05/20] vfio/pci: Buffer ROM reads before copying to userspace
2026-11-12 14:16 ` [RFC PATCH v3 05/20] vfio/pci: Buffer ROM reads before copying to userspace Shameer Kolothum
@ 2026-11-12 14:40 ` sashiko-bot
0 siblings, 0 replies; 28+ messages in thread
From: sashiko-bot @ 2026-11-12 14:40 UTC (permalink / raw)
To: Shameer Kolothum, kvm, linux-pci
Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] vfio/pci: pci_map_rom() called before
__vfio_pci_memory_enabled() in vfio_pci_rom_read()
--
commit 95981493fc3ccf127f13d3a0117382988fa550cd
Author: Shameer Kolothum <skolothumtho@nvidia.com>
vfio/pci: Buffer ROM reads before copying to userspace
This commit buffers PCI ROM reads into a kernel buffer under memory_lock
and unmaps the ROM before copying the data to userspace.
> diff --git a/drivers/vfio/pci/vfio_pci_rdwr.c b/drivers/vfio/pci/vfio_pci_rdwr.c
> index df88480876815..10cd091031fe4 100644
> --- a/drivers/vfio/pci/vfio_pci_rdwr.c
> +++ b/drivers/vfio/pci/vfio_pci_rdwr.c
[ ... ]
> @@ -198,6 +200,96 @@ ssize_t vfio_pci_core_do_io_rw(struct vfio_pci_core_device *vdev, bool test_mem,
> }
> EXPORT_SYMBOL_GPL(vfio_pci_core_do_io_rw);
>
> +static ssize_t vfio_pci_rom_read(struct vfio_pci_core_device *vdev,
> + char __user *buf, size_t count, loff_t pos)
> +{
> + struct pci_dev *pdev = vdev->pdev;
> + bool rom_bar = pci_resource_start(pdev, PCI_ROM_RESOURCE);
> + size_t size, length = 0, done = 0;
> + void *data = NULL;
> + void __iomem *io;
> + ssize_t ret;
> +
> + /* Protect ROM decoding against concurrent power changes and ROM accesses. */
> + down_write(&vdev->memory_lock);
> + if (rom_bar) {
> + io = pci_map_rom(pdev, &size);
> + } else {
> + io = ioremap(pdev->rom, pdev->romlen);
> + size = pdev->romlen;
> + }
> + if (!io) {
> + ret = -ENOMEM;
> + goto out_unlock;
> + }
> +
> + /* Buffer only ROM data, not unused space in a large ROM BAR. */
> + if (pos < size)
> + length = min(count, size - (size_t)pos);
> + if (length) {
> + if ((pci_resource_flags(pdev, PCI_ROM_RESOURCE) & IORESOURCE_MEM) &&
> + !__vfio_pci_memory_enabled(vdev)) {
> + ret = -EIO;
> + goto out_unmap;
> + }
[Severity: Medium]
Should __vfio_pci_memory_enabled(vdev) be checked before calling
pci_map_rom()?
pci_map_rom() enables ROM decoding and calls pci_get_rom_size(), which
performs MMIO reads on the ROM BAR:
drivers/pci/rom.c:pci_map_rom() {
...
if (pci_enable_rom(pdev))
return NULL;
rom = ioremap(start, *size);
if (!rom)
goto err_ioremap;
*size = pci_get_rom_size(pdev, rom, *size);
if (!*size)
goto invalid_rom;
...
}
If userspace reads the ROM region while PCI_COMMAND_MEMORY is cleared in
vconfig or the device is in PCI_D3hot, pci_get_rom_size() still issues MMIO
reads to the disabled or D3hot device, which can trigger Unsupported Request
AER errors or platform faults. When those reads return 0xffff,
pci_map_rom() also logs an invalid ROM signature error and returns NULL, so
vfio_pci_rom_read() fails with -ENOMEM before reaching the
__vfio_pci_memory_enabled() check.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261112141632.918954-1-skolothumtho@nvidia.com?part=5
^ permalink raw reply [flat|nested] 28+ messages in thread
* [RFC PATCH v3 06/20] vfio/pci: Use access ops for BAR and ROM accesses
2026-11-12 14:16 [RFC PATCH v3 00/20] vfio/pci: Handle PCI error recovery and report state to userspace Shameer Kolothum
` (4 preceding siblings ...)
2026-11-12 14:16 ` [RFC PATCH v3 05/20] vfio/pci: Buffer ROM reads before copying to userspace Shameer Kolothum
@ 2026-11-12 14:16 ` Shameer Kolothum
2026-11-12 14:16 ` [RFC PATCH v3 07/20] vfio/pci: Use access ops when mapping BAR pages Shameer Kolothum
` (13 subsequent siblings)
19 siblings, 0 replies; 28+ messages in thread
From: Shameer Kolothum @ 2026-11-12 14:16 UTC (permalink / raw)
To: kvm, linux-pci, linux-kernel
Cc: alex, jgg, kevin.tian, kbusch, michal.winiarski,
satyanarayana.k.v.p, sonangp, ankita, nathanc, mochs,
skolothumtho
Recovery needs to wait for hardware reads and writes to finish before
changing device state. Use access ops for individual MMIO and PIO
reads and writes, covering BAR, VGA, port-I/O and ioeventfd accesses.
Keep copies to and from userspace outside SRCU.
For ROM reads, keep mapping, reading into the buffer and unmapping within
one SRCU read-side section. Copy the buffered data to userspace afterward.
Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
---
drivers/vfio/pci/vfio_pci_rdwr.c | 156 ++++++++++++++++++++-----------
1 file changed, 102 insertions(+), 54 deletions(-)
diff --git a/drivers/vfio/pci/vfio_pci_rdwr.c b/drivers/vfio/pci/vfio_pci_rdwr.c
index 10cd091031fe..8494e93c18fb 100644
--- a/drivers/vfio/pci/vfio_pci_rdwr.c
+++ b/drivers/vfio/pci/vfio_pci_rdwr.c
@@ -40,57 +40,81 @@
#define vfio_ioread8 ioread8
#define vfio_iowrite8 iowrite8
-#define VFIO_IOWRITE(size) \
-int vfio_pci_core_iowrite##size(struct vfio_pci_core_device *vdev, \
- bool test_mem, u##size val, void __iomem *io) \
-{ \
- if (test_mem) { \
- down_read(&vdev->memory_lock); \
- if (!__vfio_pci_memory_enabled(vdev)) { \
- up_read(&vdev->memory_lock); \
- return -EIO; \
- } \
- } \
- \
- vfio_iowrite##size(val, io); \
- \
- if (test_mem) \
- up_read(&vdev->memory_lock); \
- \
- return 0; \
-} \
-EXPORT_SYMBOL_GPL(vfio_pci_core_iowrite##size);
-
-VFIO_IOWRITE(8)
-VFIO_IOWRITE(16)
-VFIO_IOWRITE(32)
-VFIO_IOWRITE(64)
-
-#define VFIO_IOREAD(size) \
-int vfio_pci_core_ioread##size(struct vfio_pci_core_device *vdev, \
- bool test_mem, u##size *val, void __iomem *io) \
-{ \
- if (test_mem) { \
- down_read(&vdev->memory_lock); \
- if (!__vfio_pci_memory_enabled(vdev)) { \
- up_read(&vdev->memory_lock); \
- return -EIO; \
- } \
- } \
- \
- *val = vfio_ioread##size(io); \
- \
- if (test_mem) \
- up_read(&vdev->memory_lock); \
- \
- return 0; \
-} \
-EXPORT_SYMBOL_GPL(vfio_pci_core_ioread##size);
-
-VFIO_IOREAD(8)
-VFIO_IOREAD(16)
-VFIO_IOREAD(32)
-VFIO_IOREAD(64)
+struct vfio_pci_io_access {
+ bool test_mem;
+ void __iomem *io;
+ void *val;
+};
+
+#define VFIO_IOWRITE(size, type) \
+static long vfio_pci_access_iowrite##size(struct vfio_pci_core_device *vdev, \
+ void *data) \
+{ \
+ struct vfio_pci_io_access *access = data; \
+ type *val = access->val; \
+ \
+ if (access->test_mem) { \
+ down_read(&vdev->memory_lock); \
+ if (!__vfio_pci_memory_enabled(vdev)) { \
+ up_read(&vdev->memory_lock); \
+ return -EIO; \
+ } \
+ } \
+ \
+ vfio_iowrite##size(*val, access->io); \
+ if (access->test_mem) \
+ up_read(&vdev->memory_lock); \
+ return 0; \
+} \
+ \
+int vfio_pci_core_iowrite##size(struct vfio_pci_core_device *vdev, \
+ bool test_mem, type val, void __iomem *io) \
+{ \
+ struct vfio_pci_io_access access = { test_mem, io, &val }; \
+ \
+ return vfio_pci_core_access(vdev, vfio_pci_access_iowrite##size, &access); \
+} \
+EXPORT_SYMBOL_GPL(vfio_pci_core_iowrite##size)
+
+VFIO_IOWRITE(8, u8);
+VFIO_IOWRITE(16, u16);
+VFIO_IOWRITE(32, u32);
+VFIO_IOWRITE(64, u64);
+
+#define VFIO_IOREAD(size, type) \
+static long vfio_pci_access_ioread##size(struct vfio_pci_core_device *vdev, \
+ void *data) \
+{ \
+ struct vfio_pci_io_access *access = data; \
+ type *val = access->val; \
+ \
+ if (access->test_mem) { \
+ down_read(&vdev->memory_lock); \
+ if (!__vfio_pci_memory_enabled(vdev)) { \
+ up_read(&vdev->memory_lock); \
+ return -EIO; \
+ } \
+ } \
+ \
+ *val = vfio_ioread##size(access->io); \
+ if (access->test_mem) \
+ up_read(&vdev->memory_lock); \
+ return 0; \
+} \
+ \
+int vfio_pci_core_ioread##size(struct vfio_pci_core_device *vdev, \
+ bool test_mem, type *val, void __iomem *io) \
+{ \
+ struct vfio_pci_io_access access = { test_mem, io, val }; \
+ \
+ return vfio_pci_core_access(vdev, vfio_pci_access_ioread##size, &access); \
+} \
+EXPORT_SYMBOL_GPL(vfio_pci_core_ioread##size)
+
+VFIO_IOREAD(8, u8);
+VFIO_IOREAD(16, u16);
+VFIO_IOREAD(32, u32);
+VFIO_IOREAD(64, u64);
#define VFIO_IORDWR(size) \
static int vfio_pci_iordwr##size(struct vfio_pci_core_device *vdev,\
@@ -200,12 +224,20 @@ ssize_t vfio_pci_core_do_io_rw(struct vfio_pci_core_device *vdev, bool test_mem,
}
EXPORT_SYMBOL_GPL(vfio_pci_core_do_io_rw);
-static ssize_t vfio_pci_rom_read(struct vfio_pci_core_device *vdev,
- char __user *buf, size_t count, loff_t pos)
+struct vfio_pci_rom_access {
+ size_t count;
+ loff_t pos;
+ void *data;
+ size_t length;
+};
+
+static long vfio_pci_access_rom(struct vfio_pci_core_device *vdev, void *arg)
{
+ struct vfio_pci_rom_access *access = arg;
struct pci_dev *pdev = vdev->pdev;
bool rom_bar = pci_resource_start(pdev, PCI_ROM_RESOURCE);
- size_t size, length = 0, done = 0;
+ size_t count = access->count, size, length = 0, done = 0;
+ loff_t pos = access->pos;
void *data = NULL;
void __iomem *io;
ssize_t ret;
@@ -272,6 +304,22 @@ static ssize_t vfio_pci_rom_read(struct vfio_pci_core_device *vdev,
iounmap(io);
out_unlock:
up_write(&vdev->memory_lock);
+ access->data = data;
+ access->length = length;
+ return ret;
+}
+
+static ssize_t vfio_pci_rom_read(struct vfio_pci_core_device *vdev,
+ char __user *buf, size_t count, loff_t pos)
+{
+ struct vfio_pci_rom_access access = { .count = count, .pos = pos };
+ size_t length, done;
+ void *data;
+ ssize_t ret;
+
+ ret = vfio_pci_core_access(vdev, vfio_pci_access_rom, &access);
+ data = access.data;
+ length = access.length;
if (ret < 0)
goto out_free;
--
2.43.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [RFC PATCH v3 07/20] vfio/pci: Use access ops when mapping BAR pages
2026-11-12 14:16 [RFC PATCH v3 00/20] vfio/pci: Handle PCI error recovery and report state to userspace Shameer Kolothum
` (5 preceding siblings ...)
2026-11-12 14:16 ` [RFC PATCH v3 06/20] vfio/pci: Use access ops for BAR and ROM accesses Shameer Kolothum
@ 2026-11-12 14:16 ` Shameer Kolothum
2026-11-12 14:16 ` [RFC PATCH v3 08/20] vfio/pci: Use access ops for hardware IRQ operations Shameer Kolothum
` (12 subsequent siblings)
19 siblings, 0 replies; 28+ messages in thread
From: Shameer Kolothum @ 2026-11-12 14:16 UTC (permalink / raw)
To: kvm, linux-pci, linux-kernel
Cc: alex, jgg, kevin.tian, kbusch, michal.winiarski,
satyanarayana.k.v.p, sonangp, ankita, nathanc, mochs,
skolothumtho
A BAR page fault can race with recovery and install a mapping after
recovery has removed the existing mappings.
Insert the mapping through the SRCU-protected access ops while holding
memory_lock. Recovery can then wait for pending insertions before
removing mappings. If access is blocked, return VM_FAULT_SIGBUS instead
of waiting for recovery in the fault handler.
Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
---
drivers/vfio/pci/vfio_pci_core.c | 26 ++++++++++++++++++++++++--
1 file changed, 24 insertions(+), 2 deletions(-)
diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
index 21c0dda37dd1..b19721617e5f 100644
--- a/drivers/vfio/pci/vfio_pci_core.c
+++ b/drivers/vfio/pci/vfio_pci_core.c
@@ -1880,6 +1880,24 @@ vm_fault_t vfio_pci_vmf_insert_pfn(struct vfio_pci_core_device *vdev,
}
EXPORT_SYMBOL_GPL(vfio_pci_vmf_insert_pfn);
+struct vfio_pci_fault_access {
+ struct vm_fault *vmf;
+ unsigned long pfn;
+ unsigned int order;
+};
+
+static long vfio_pci_access_fault(struct vfio_pci_core_device *vdev, void *data)
+{
+ struct vfio_pci_fault_access *access = data;
+ vm_fault_t ret;
+
+ down_read(&vdev->memory_lock);
+ ret = vfio_pci_vmf_insert_pfn(vdev, access->vmf, access->pfn,
+ access->order);
+ up_read(&vdev->memory_lock);
+ return (__force unsigned int)ret;
+}
+
static vm_fault_t vfio_pci_mmap_huge_fault(struct vm_fault *vmf,
unsigned int order)
{
@@ -1891,8 +1909,12 @@ static vm_fault_t vfio_pci_mmap_huge_fault(struct vm_fault *vmf,
vm_fault_t ret = VM_FAULT_FALLBACK;
if (is_aligned_for_order(vma, addr, pfn, order)) {
- scoped_guard(rwsem_read, &vdev->memory_lock)
- ret = vfio_pci_vmf_insert_pfn(vdev, vmf, pfn, order);
+ struct vfio_pci_fault_access access = { vmf, pfn, order };
+ long result;
+
+ /* Recovery must wait for this insertion before removing mappings. */
+ result = vfio_pci_core_access(vdev, vfio_pci_access_fault, &access);
+ ret = result < 0 ? VM_FAULT_SIGBUS : (__force vm_fault_t)result;
}
dev_dbg_ratelimited(&vdev->pdev->dev,
--
2.43.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [RFC PATCH v3 08/20] vfio/pci: Use access ops for hardware IRQ operations
2026-11-12 14:16 [RFC PATCH v3 00/20] vfio/pci: Handle PCI error recovery and report state to userspace Shameer Kolothum
` (6 preceding siblings ...)
2026-11-12 14:16 ` [RFC PATCH v3 07/20] vfio/pci: Use access ops when mapping BAR pages Shameer Kolothum
@ 2026-11-12 14:16 ` Shameer Kolothum
2026-11-12 14:16 ` [RFC PATCH v3 09/20] vfio/pci: Use access ops for function reset Shameer Kolothum
` (11 subsequent siblings)
19 siblings, 0 replies; 28+ messages in thread
From: Shameer Kolothum @ 2026-11-12 14:16 UTC (permalink / raw)
To: kvm, linux-pci, linux-kernel
Cc: alex, jgg, kevin.tian, kbusch, michal.winiarski,
satyanarayana.k.v.p, sonangp, ankita, nathanc, mochs,
skolothumtho
IRQ queries and configuration can access PCI registers while recovery
resets the device or restores its configuration.
Use access ops when reading hardware IRQ counts and applying IRQ
configuration. Continue to hold igate for configuration changes.
Use separate SRCU read-side sections for the count query and the
configuration change so copies to and from userspace stay outside SRCU.
ERR and REQ operations do not touch hardware and remain available while
access is blocked.
IRQ teardown flushes the global virqfd workqueue. Waiting for an IRQ
operation to finish therefore also waits for that flush, including work
for other devices.
Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
---
drivers/vfio/pci/vfio_pci_core.c | 78 ++++++++++++++++++++++++++++----
1 file changed, 70 insertions(+), 8 deletions(-)
diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
index b19721617e5f..de3a1c8b6d9e 100644
--- a/drivers/vfio/pci/vfio_pci_core.c
+++ b/drivers/vfio/pci/vfio_pci_core.c
@@ -1342,11 +1342,33 @@ int vfio_pci_ioctl_get_region_info(struct vfio_device *core_vdev,
}
EXPORT_SYMBOL_GPL(vfio_pci_ioctl_get_region_info);
+/* ERR and REQ do not access hardware. */
+static bool vfio_pci_irq_index_is_device(u32 index)
+{
+ switch (index) {
+ case VFIO_PCI_INTX_IRQ_INDEX:
+ case VFIO_PCI_MSI_IRQ_INDEX:
+ case VFIO_PCI_MSIX_IRQ_INDEX:
+ return true;
+ default:
+ return false;
+ }
+}
+
+static long vfio_pci_access_irq_count(struct vfio_pci_core_device *vdev,
+ void *data)
+{
+ u32 *index = data;
+
+ return vfio_pci_get_irq_count(vdev, *index);
+}
+
static int vfio_pci_ioctl_get_irq_info(struct vfio_pci_core_device *vdev,
struct vfio_irq_info __user *arg)
{
unsigned long minsz = offsetofend(struct vfio_irq_info, count);
struct vfio_irq_info info;
+ int count;
if (copy_from_user(&info, arg, minsz))
return -EFAULT;
@@ -1365,7 +1387,15 @@ static int vfio_pci_ioctl_get_irq_info(struct vfio_pci_core_device *vdev,
info.flags = VFIO_IRQ_INFO_EVENTFD;
- info.count = vfio_pci_get_irq_count(vdev, info.index);
+ if (vfio_pci_irq_index_is_device(info.index)) {
+ count = vfio_pci_core_access(vdev, vfio_pci_access_irq_count,
+ &info.index);
+ if (count < 0)
+ return count;
+ info.count = count;
+ } else {
+ info.count = vfio_pci_get_irq_count(vdev, info.index);
+ }
if (info.index == VFIO_PCI_INTX_IRQ_INDEX)
info.flags |=
@@ -1376,19 +1406,48 @@ static int vfio_pci_ioctl_get_irq_info(struct vfio_pci_core_device *vdev,
return copy_to_user(arg, &info, minsz) ? -EFAULT : 0;
}
+struct vfio_pci_irq_access {
+ struct vfio_irq_set *hdr;
+ u8 *data;
+};
+
+static long vfio_pci_access_irqs(struct vfio_pci_core_device *vdev, void *data)
+{
+ struct vfio_pci_irq_access *access = data;
+ struct vfio_irq_set *hdr = access->hdr;
+ int ret;
+
+ mutex_lock(&vdev->igate);
+ ret = vfio_pci_set_irqs_ioctl(vdev, hdr->flags, hdr->index, hdr->start,
+ hdr->count, access->data);
+ mutex_unlock(&vdev->igate);
+ return ret;
+}
+
static int vfio_pci_ioctl_set_irqs(struct vfio_pci_core_device *vdev,
struct vfio_irq_set __user *arg)
{
unsigned long minsz = offsetofend(struct vfio_irq_set, count);
struct vfio_irq_set hdr;
+ struct vfio_pci_irq_access access = { .hdr = &hdr };
u8 *data = NULL;
+ bool device_irq;
int max, ret = 0;
size_t data_size = 0;
if (copy_from_user(&hdr, arg, minsz))
return -EFAULT;
- max = vfio_pci_get_irq_count(vdev, hdr.index);
+ /* Keep the userspace copy between the two SRCU sections. */
+ device_irq = vfio_pci_irq_index_is_device(hdr.index);
+ if (device_irq) {
+ max = vfio_pci_core_access(vdev, vfio_pci_access_irq_count,
+ &hdr.index);
+ if (max < 0)
+ return max;
+ } else {
+ max = vfio_pci_get_irq_count(vdev, hdr.index);
+ }
ret = vfio_set_irqs_validate_and_prepare(&hdr, max, VFIO_PCI_NUM_IRQS,
&data_size);
@@ -1401,12 +1460,15 @@ static int vfio_pci_ioctl_set_irqs(struct vfio_pci_core_device *vdev,
return PTR_ERR(data);
}
- mutex_lock(&vdev->igate);
-
- ret = vfio_pci_set_irqs_ioctl(vdev, hdr.flags, hdr.index, hdr.start,
- hdr.count, data);
-
- mutex_unlock(&vdev->igate);
+ /*
+ * IRQ teardown flushes the global virqfd workqueue, so draining SRCU
+ * can also wait for work on other devices.
+ */
+ access.data = data;
+ if (device_irq)
+ ret = vfio_pci_core_access(vdev, vfio_pci_access_irqs, &access);
+ else
+ ret = vfio_pci_access_irqs(vdev, &access);
kfree(data);
return ret;
--
2.43.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [RFC PATCH v3 09/20] vfio/pci: Use access ops for function reset
2026-11-12 14:16 [RFC PATCH v3 00/20] vfio/pci: Handle PCI error recovery and report state to userspace Shameer Kolothum
` (7 preceding siblings ...)
2026-11-12 14:16 ` [RFC PATCH v3 08/20] vfio/pci: Use access ops for hardware IRQ operations Shameer Kolothum
@ 2026-11-12 14:16 ` Shameer Kolothum
2026-11-12 14:16 ` [RFC PATCH v3 10/20] vfio/pci: Use access ops for runtime-PM requests Shameer Kolothum
` (10 subsequent siblings)
19 siblings, 0 replies; 28+ messages in thread
From: Shameer Kolothum @ 2026-11-12 14:16 UTC (permalink / raw)
To: kvm, linux-pci, linux-kernel
Cc: alex, jgg, kevin.tian, kbusch, michal.winiarski,
satyanarayana.k.v.p, sonangp, ankita, nathanc, mochs,
skolothumtho
Userspace function reset and its D0 preparation must finish before host
recovery changes device state.
Run both through the SRCU-protected access ops. Reject blocked requests
and let recovery drain admitted operations. The D0 helper returns
-EAGAIN on bus-lock contention.
Hold memory_lock across preparation and reset so concurrent PM writes
cannot put the device back into D3. Keep the existing DMA-BUF mapping
removal and restoration around reset.
Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
---
drivers/vfio/pci/vfio_pci_core.c | 29 +++++++++++++++++------------
1 file changed, 17 insertions(+), 12 deletions(-)
diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
index de3a1c8b6d9e..fb7f46c2ca36 100644
--- a/drivers/vfio/pci/vfio_pci_core.c
+++ b/drivers/vfio/pci/vfio_pci_core.c
@@ -1474,16 +1474,11 @@ static int vfio_pci_ioctl_set_irqs(struct vfio_pci_core_device *vdev,
return ret;
}
-static int vfio_pci_ioctl_reset(struct vfio_pci_core_device *vdev,
- void __user *arg)
+static long vfio_pci_access_reset(struct vfio_pci_core_device *vdev, void *data)
{
int ret;
- if (!vdev->reset_works)
- return -EINVAL;
-
- vfio_pci_zap_and_down_write_memory_lock(vdev);
-
+ lockdep_assert_held_write(&vdev->memory_lock);
/*
* This function can be invoked while the power state is non-D0. If
* pci_try_reset_function() has been called while the power state is
@@ -1494,17 +1489,27 @@ static int vfio_pci_ioctl_reset(struct vfio_pci_core_device *vdev,
* 'vdev->pm_save').
*/
ret = vfio_pci_try_set_power_state(vdev, PCI_D0);
- if (ret) {
- up_write(&vdev->memory_lock);
+ if (ret)
return ret;
- }
-
vfio_pci_dma_buf_move(vdev, true);
ret = pci_try_reset_function(vdev->pdev);
if (__vfio_pci_memory_enabled(vdev))
vfio_pci_dma_buf_move(vdev, false);
- up_write(&vdev->memory_lock);
+ return ret;
+}
+
+static int vfio_pci_ioctl_reset(struct vfio_pci_core_device *vdev,
+ void __user *arg)
+{
+ int ret;
+
+ if (!vdev->reset_works)
+ return -EINVAL;
+ vfio_pci_zap_and_down_write_memory_lock(vdev);
+ /* Keep D0 preparation and reset together under memory_lock and SRCU. */
+ ret = vfio_pci_core_access(vdev, vfio_pci_access_reset, NULL);
+ up_write(&vdev->memory_lock);
return ret;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [RFC PATCH v3 10/20] vfio/pci: Use access ops for runtime-PM requests
2026-11-12 14:16 [RFC PATCH v3 00/20] vfio/pci: Handle PCI error recovery and report state to userspace Shameer Kolothum
` (8 preceding siblings ...)
2026-11-12 14:16 ` [RFC PATCH v3 09/20] vfio/pci: Use access ops for function reset Shameer Kolothum
@ 2026-11-12 14:16 ` Shameer Kolothum
2026-11-12 14:16 ` [RFC PATCH v3 11/20] vfio/pci: Use access ops for hardware information queries Shameer Kolothum
` (9 subsequent siblings)
19 siblings, 0 replies; 28+ messages in thread
From: Shameer Kolothum @ 2026-11-12 14:16 UTC (permalink / raw)
To: kvm, linux-pci, linux-kernel
Cc: alex, jgg, kevin.tian, kbusch, michal.winiarski,
satyanarayana.k.v.p, sonangp, ankita, nathanc, mochs,
skolothumtho
Userspace low-power requests change PM references and device mappings.
Run these requests through access ops so recovery can reject new
requests and drain admitted operations.
Validate userspace arguments and obtain the eventfd reference before
entering SRCU. Release the reference if entry fails. Keep the existing
memory_lock protection and PM reference handling.
Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
---
drivers/vfio/pci/vfio_pci_core.c | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
index fb7f46c2ca36..fc6b2673be88 100644
--- a/drivers/vfio/pci/vfio_pci_core.c
+++ b/drivers/vfio/pci/vfio_pci_core.c
@@ -454,9 +454,11 @@ int vfio_pci_try_set_power_state(struct vfio_pci_core_device *vdev,
pci_set_power_state);
}
-static int vfio_pci_runtime_pm_entry(struct vfio_pci_core_device *vdev,
- struct eventfd_ctx *efdctx)
+static long vfio_pci_runtime_pm_entry(struct vfio_pci_core_device *vdev,
+ void *data)
{
+ struct eventfd_ctx *efdctx = data;
+
/*
* The vdev power related flags are protected with 'memory_lock'
* semaphore.
@@ -492,7 +494,7 @@ static int vfio_pci_core_pm_entry(struct vfio_pci_core_device *vdev, u32 flags,
* while returning from the ioctl and then the device can go into
* runtime suspended state.
*/
- return vfio_pci_runtime_pm_entry(vdev, NULL);
+ return vfio_pci_core_access(vdev, vfio_pci_runtime_pm_entry, NULL);
}
static int vfio_pci_core_pm_entry_with_wakeup(
@@ -519,7 +521,7 @@ static int vfio_pci_core_pm_entry_with_wakeup(
if (IS_ERR(efdctx))
return PTR_ERR(efdctx);
- ret = vfio_pci_runtime_pm_entry(vdev, efdctx);
+ ret = vfio_pci_core_access(vdev, vfio_pci_runtime_pm_entry, efdctx);
if (ret)
eventfd_ctx_put(efdctx);
@@ -552,6 +554,13 @@ static void vfio_pci_runtime_pm_exit(struct vfio_pci_core_device *vdev)
up_write(&vdev->memory_lock);
}
+static long vfio_pci_access_pm_exit(struct vfio_pci_core_device *vdev,
+ void *data)
+{
+ vfio_pci_runtime_pm_exit(vdev);
+ return 0;
+}
+
static int vfio_pci_core_pm_exit(struct vfio_pci_core_device *vdev, u32 flags,
void __user *arg, size_t argsz)
{
@@ -568,8 +577,7 @@ static int vfio_pci_core_pm_exit(struct vfio_pci_core_device *vdev, u32 flags,
* already signaled the eventfd and exited low power mode itself.
* pm_runtime_engaged protects the redundant call here.
*/
- vfio_pci_runtime_pm_exit(vdev);
- return 0;
+ return vfio_pci_core_access(vdev, vfio_pci_access_pm_exit, NULL);
}
#ifdef CONFIG_PM
--
2.43.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [RFC PATCH v3 11/20] vfio/pci: Use access ops for hardware information queries
2026-11-12 14:16 [RFC PATCH v3 00/20] vfio/pci: Handle PCI error recovery and report state to userspace Shameer Kolothum
` (9 preceding siblings ...)
2026-11-12 14:16 ` [RFC PATCH v3 10/20] vfio/pci: Use access ops for runtime-PM requests Shameer Kolothum
@ 2026-11-12 14:16 ` Shameer Kolothum
2026-11-12 14:47 ` sashiko-bot
2026-11-12 14:16 ` [RFC PATCH v3 12/20] vfio/pci: Use access ops for DMA-BUF export and restoration Shameer Kolothum
` (8 subsequent siblings)
19 siblings, 1 reply; 28+ messages in thread
From: Shameer Kolothum @ 2026-11-12 14:16 UTC (permalink / raw)
To: kvm, linux-pci, linux-kernel
Cc: alex, jgg, kevin.tian, kbusch, michal.winiarski,
satyanarayana.k.v.p, sonangp, ankita, nathanc, mochs,
skolothumtho
AtomicOps and ROM information queries access PCI configuration and ROM
contents. Run these accesses through access ops and keep userspace
copies outside SRCU. Free the capability buffer if the query fails.
A VF's AtomicOps query accesses its PF. If the PF is managed by VFIO and
supports recovery, check its access ops too. Hold the PF's access_lock
and reject queries when it is closed, since its SRCU instance exists
only while open. Other PF drivers retain their existing behavior.
Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
---
drivers/vfio/pci/vfio_pci_core.c | 82 ++++++++++++++++++++++++--------
1 file changed, 61 insertions(+), 21 deletions(-)
diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
index fc6b2673be88..da7166669dc7 100644
--- a/drivers/vfio/pci/vfio_pci_core.c
+++ b/drivers/vfio/pci/vfio_pci_core.c
@@ -1178,6 +1178,33 @@ static int vfio_pci_info_atomic_cap(struct vfio_pci_core_device *vdev,
return vfio_info_add_capability(caps, &cap.header, sizeof(cap));
}
+static long vfio_pci_access_atomic_cap_target(struct vfio_pci_core_device *vdev,
+ void *data)
+{
+ return vfio_pci_info_atomic_cap(vdev, data);
+}
+
+static long vfio_pci_access_atomic_cap(struct vfio_pci_core_device *vdev,
+ void *data)
+{
+ struct vfio_pci_core_device *pf_vdev = vdev->sriov_pf_core_dev;
+
+ if (!pf_vdev || !pf_vdev->pci_recovery_supported)
+ return vfio_pci_info_atomic_cap(vdev, data);
+
+ /*
+ * VF AtomicOps queries read the PF's configuration. The PF association
+ * lasts until VF removal, but its SRCU instance exists only while
+ * the PF is open.
+ */
+ guard(mutex)(&pf_vdev->access_lock);
+ if (!pf_vdev->device_open)
+ return -EIO;
+
+ return vfio_pci_core_access(pf_vdev, vfio_pci_access_atomic_cap_target,
+ data);
+}
+
static int vfio_pci_ioctl_get_info(struct vfio_pci_core_device *vdev,
struct vfio_device_info __user *arg)
{
@@ -1209,10 +1236,12 @@ static int vfio_pci_ioctl_get_info(struct vfio_pci_core_device *vdev,
return ret;
}
- ret = vfio_pci_info_atomic_cap(vdev, &caps);
+ ret = vfio_pci_core_access(vdev, vfio_pci_access_atomic_cap, &caps);
if (ret && ret != -ENODEV) {
- pci_warn(vdev->pdev,
- "Failed to setup AtomicOps info capability\n");
+ if (ret != -EIO)
+ pci_warn(vdev->pdev,
+ "Failed to setup AtomicOps info capability\n");
+ kfree(caps.buf);
return ret;
}
@@ -1235,6 +1264,31 @@ static int vfio_pci_ioctl_get_info(struct vfio_pci_core_device *vdev,
return copy_to_user(arg, &info, minsz) ? -EFAULT : 0;
}
+static long vfio_pci_access_rom_info(struct vfio_pci_core_device *vdev,
+ void *data)
+{
+ struct vfio_region_info *info = data;
+ struct pci_dev *pdev = vdev->pdev;
+ void __iomem *io;
+ size_t size;
+ u16 cmd;
+
+ /*
+ * Check ROM content is valid. Need to enable memory
+ * decode for ROM access in pci_map_rom().
+ */
+ cmd = vfio_pci_memory_lock_and_enable(vdev);
+ io = pci_map_rom(pdev, &size);
+ if (io) {
+ info->flags = VFIO_REGION_INFO_FLAG_READ;
+ /* Report the BAR size, not the ROM size. */
+ info->size = pci_resource_len(pdev, PCI_ROM_RESOURCE);
+ pci_unmap_rom(pdev, io);
+ }
+ vfio_pci_memory_unlock_and_restore(vdev, cmd);
+ return 0;
+}
+
int vfio_pci_ioctl_get_region_info(struct vfio_device *core_vdev,
struct vfio_region_info *info,
struct vfio_info_cap *caps)
@@ -1272,29 +1326,15 @@ int vfio_pci_ioctl_get_region_info(struct vfio_device *core_vdev,
break;
case VFIO_PCI_ROM_REGION_INDEX: {
- void __iomem *io;
- size_t size;
- u16 cmd;
-
info->offset = VFIO_PCI_INDEX_TO_OFFSET(info->index);
info->flags = 0;
info->size = 0;
if (pci_resource_start(pdev, PCI_ROM_RESOURCE)) {
- /*
- * Check ROM content is valid. Need to enable memory
- * decode for ROM access in pci_map_rom().
- */
- cmd = vfio_pci_memory_lock_and_enable(vdev);
- io = pci_map_rom(pdev, &size);
- if (io) {
- info->flags = VFIO_REGION_INFO_FLAG_READ;
- /* Report the BAR size, not the ROM size. */
- info->size = pci_resource_len(pdev,
- PCI_ROM_RESOURCE);
- pci_unmap_rom(pdev, io);
- }
- vfio_pci_memory_unlock_and_restore(vdev, cmd);
+ ret = vfio_pci_core_access(vdev, vfio_pci_access_rom_info,
+ info);
+ if (ret)
+ return ret;
} else if (pdev->rom && pdev->romlen) {
info->flags = VFIO_REGION_INFO_FLAG_READ;
/* Report BAR size as power of two. */
--
2.43.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [RFC PATCH v3 11/20] vfio/pci: Use access ops for hardware information queries
2026-11-12 14:16 ` [RFC PATCH v3 11/20] vfio/pci: Use access ops for hardware information queries Shameer Kolothum
@ 2026-11-12 14:47 ` sashiko-bot
0 siblings, 0 replies; 28+ messages in thread
From: sashiko-bot @ 2026-11-12 14:47 UTC (permalink / raw)
To: Shameer Kolothum, kvm, linux-pci
Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [High] vfio/pci: VFIO_DEVICE_GET_INFO fails with -EIO on VF when PF is
closed
- [Medium] vfio/pci: lockdep circular locking dependency in
vfio_pci_access_atomic_cap()
--
commit 8a20ca42b1e7365162ac6cecdd060d990d426a4f
Author: Shameer Kolothum <skolothumtho@nvidia.com>
vfio/pci: Use access ops for hardware information queries
This commit routes AtomicOps and ROM region hardware information queries
through VFIO PCI access ops so they participate in device error recovery.
> diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
> index fc6b2673be88c..da7166669dc75 100644
> --- a/drivers/vfio/pci/vfio_pci_core.c
> +++ b/drivers/vfio/pci/vfio_pci_core.c
> @@ -1178,6 +1178,33 @@ static int vfio_pci_info_atomic_cap(struct vfio_pci_core_device *vdev,
> return vfio_info_add_capability(caps, &cap.header, sizeof(cap));
> }
>
> +static long vfio_pci_access_atomic_cap_target(struct vfio_pci_core_device *vdev,
> + void *data)
> +{
> + return vfio_pci_info_atomic_cap(vdev, data);
> +}
> +
> +static long vfio_pci_access_atomic_cap(struct vfio_pci_core_device *vdev,
> + void *data)
> +{
> + struct vfio_pci_core_device *pf_vdev = vdev->sriov_pf_core_dev;
> +
> + if (!pf_vdev || !pf_vdev->pci_recovery_supported)
> + return vfio_pci_info_atomic_cap(vdev, data);
> +
> + /*
> + * VF AtomicOps queries read the PF's configuration. The PF association
> + * lasts until VF removal, but its SRCU instance exists only while
> + * the PF is open.
> + */
> + guard(mutex)(&pf_vdev->access_lock);
[Severity: Medium]
Can locking pf_vdev->access_lock here trigger a lockdep circular locking
warning?
vfio_pci_ioctl_get_info() calls vfio_pci_core_access() on the VF, which
takes srcu_read_lock(&vdev->access_srcu) before entering
vfio_pci_access_atomic_cap() and acquiring pf_vdev->access_lock. Because
every struct vfio_pci_core_device initializes access_lock and access_srcu
at the same mutex_init() and init_srcu_struct() call sites, the VF and PF
share lockdep classes.
Meanwhile, vfio_pci_core_close_device() holds vdev->access_lock while
vfio_pci_core_block_access() calls synchronize_srcu(&vdev->access_srcu),
establishing the opposite access_lock -> access_srcu class order:
CPU0 CPU1
---- ----
lock(&vdev->access_lock)
srcu_read_lock(&vdev->access_srcu)
lock(&pf_vdev->access_lock)
synchronize_srcu(&vdev->access_srcu)
*** DEADLOCK ***
CPU0: vfio_pci_core_close_device() -> vfio_pci_core_block_access()
CPU1: vfio_pci_ioctl_get_info() -> vfio_pci_core_access() ->
vfio_pci_access_atomic_cap()
> + if (!pf_vdev->device_open)
> + return -EIO;
> +
> + return vfio_pci_core_access(pf_vdev, vfio_pci_access_atomic_cap_target,
> + data);
> +}
> +
> static int vfio_pci_ioctl_get_info(struct vfio_pci_core_device *vdev,
> struct vfio_device_info __user *arg)
> {
> @@ -1209,10 +1236,12 @@ static int vfio_pci_ioctl_get_info(struct vfio_pci_core_device *vdev,
> return ret;
> }
>
> - ret = vfio_pci_info_atomic_cap(vdev, &caps);
> + ret = vfio_pci_core_access(vdev, vfio_pci_access_atomic_cap, &caps);
> if (ret && ret != -ENODEV) {
> - pci_warn(vdev->pdev,
> - "Failed to setup AtomicOps info capability\n");
> + if (ret != -EIO)
> + pci_warn(vdev->pdev,
> + "Failed to setup AtomicOps info capability\n");
> + kfree(caps.buf);
> return ret;
> }
[Severity: High]
Does this cause VFIO_DEVICE_GET_INFO on a VF to fail with -EIO whenever its
recovery-capable vfio-pci PF is not currently open in userspace?
A PF bound to vfio-pci does not need to stay open while its VFs are in use;
for example, userspace can open the PF only long enough to configure
vf_token and then close the PF fd before opening VFs, while
vfio_pci_core_sriov_configure() keeps the PF in PCI_D0. When
pf_vdev->device_open is false, vfio_pci_access_atomic_cap() returns -EIO
and vfio_pci_ioctl_get_info() aborts the entire ioctl, breaking
VFIO_DEVICE_GET_INFO on healthy VFs even if AtomicOps are not supported.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261112141632.918954-1-skolothumtho@nvidia.com?part=11
^ permalink raw reply [flat|nested] 28+ messages in thread
* [RFC PATCH v3 12/20] vfio/pci: Use access ops for DMA-BUF export and restoration
2026-11-12 14:16 [RFC PATCH v3 00/20] vfio/pci: Handle PCI error recovery and report state to userspace Shameer Kolothum
` (10 preceding siblings ...)
2026-11-12 14:16 ` [RFC PATCH v3 11/20] vfio/pci: Use access ops for hardware information queries Shameer Kolothum
@ 2026-11-12 14:16 ` Shameer Kolothum
2026-11-12 14:16 ` [RFC PATCH v3 13/20] vfio/pci: Track PCI recovery state Shameer Kolothum
` (7 subsequent siblings)
19 siblings, 0 replies; 28+ messages in thread
From: Shameer Kolothum @ 2026-11-12 14:16 UTC (permalink / raw)
To: kvm, linux-pci, linux-kernel
Cc: alex, jgg, kevin.tian, kbusch, michal.winiarski,
satyanarayana.k.v.p, sonangp, ankita, nathanc, mochs,
skolothumtho
DMA-BUF export can race with recovery revoking device mappings.
Keep the range lookup, export and list insertion inside SRCU so recovery
waits for admitted exports before revoking mappings. Keep userspace
copies and file descriptor installation outside SRCU.
Select ordinary mapping restoration through access ops under
memory_lock. Skip restoration while access is blocked. Provide an
internal helper so recovery can restore mappings before allowing access
again. Revocation remains available while access is blocked.
Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
---
drivers/vfio/pci/vfio_pci_priv.h | 8 ++
drivers/vfio/pci/vfio_pci_core.c | 6 ++
drivers/vfio/pci/vfio_pci_dmabuf.c | 113 +++++++++++++++++++----------
3 files changed, 89 insertions(+), 38 deletions(-)
diff --git a/drivers/vfio/pci/vfio_pci_priv.h b/drivers/vfio/pci/vfio_pci_priv.h
index 0ea7c710dc38..b02fd1fbcd31 100644
--- a/drivers/vfio/pci/vfio_pci_priv.h
+++ b/drivers/vfio/pci/vfio_pci_priv.h
@@ -90,6 +90,8 @@ struct vfio_pci_access_ops {
vfio_pci_access_fn fn, void *data);
/* Caller holds memory_lock. */
int (*set_power_state)(struct vfio_pci_core_device *vdev, pci_power_t state);
+ /* Caller holds memory_lock. */
+ void (*restore_dmabufs)(struct vfio_pci_core_device *vdev);
};
const struct vfio_pci_access_ops *
@@ -151,6 +153,7 @@ int vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags,
size_t argsz);
void vfio_pci_dma_buf_cleanup(struct vfio_pci_core_device *vdev);
void vfio_pci_dma_buf_move(struct vfio_pci_core_device *vdev, bool revoked);
+void vfio_pci_dma_buf_restore(struct vfio_pci_core_device *vdev);
#else
static inline int
vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags,
@@ -162,6 +165,11 @@ vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags,
static inline void vfio_pci_dma_buf_cleanup(struct vfio_pci_core_device *vdev)
{
}
+
+static inline void vfio_pci_dma_buf_restore(struct vfio_pci_core_device *vdev)
+{
+}
+
static inline void vfio_pci_dma_buf_move(struct vfio_pci_core_device *vdev,
bool revoked)
{
diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
index da7166669dc7..0299f5afca91 100644
--- a/drivers/vfio/pci/vfio_pci_core.c
+++ b/drivers/vfio/pci/vfio_pci_core.c
@@ -330,14 +330,20 @@ static int vfio_pci_power_blocked(struct vfio_pci_core_device *vdev,
return -EIO;
}
+static void vfio_pci_restore_dmabufs_blocked(struct vfio_pci_core_device *vdev)
+{
+}
+
static const struct vfio_pci_access_ops vfio_pci_live_ops = {
.execute = vfio_pci_access_live,
.set_power_state = vfio_pci_try_set_power_state,
+ .restore_dmabufs = vfio_pci_dma_buf_restore,
};
static const struct vfio_pci_access_ops vfio_pci_blocked_ops = {
.execute = vfio_pci_access_blocked,
.set_power_state = vfio_pci_power_blocked,
+ .restore_dmabufs = vfio_pci_restore_dmabufs_blocked,
};
/*
diff --git a/drivers/vfio/pci/vfio_pci_dmabuf.c b/drivers/vfio/pci/vfio_pci_dmabuf.c
index c16f460c01d6..a9db011f0c06 100644
--- a/drivers/vfio/pci/vfio_pci_dmabuf.c
+++ b/drivers/vfio/pci/vfio_pci_dmabuf.c
@@ -218,14 +218,67 @@ static int validate_dmabuf_input(struct vfio_device_feature_dma_buf *dma_buf,
return 0;
}
+struct vfio_pci_dmabuf_access {
+ struct vfio_pci_dma_buf *priv;
+ struct vfio_device_feature_dma_buf *get_dma_buf;
+ struct vfio_region_dma_range *dma_ranges;
+};
+
+static long vfio_pci_access_dmabuf(struct vfio_pci_core_device *vdev, void *data)
+{
+ struct vfio_pci_dmabuf_access *access = data;
+ struct vfio_pci_dma_buf *priv = access->priv;
+ struct vfio_region_dma_range *dma_ranges = access->dma_ranges;
+ DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
+ int ret;
+
+ ret = vdev->pci_ops->get_dmabuf_phys(vdev, &priv->provider,
+ access->get_dma_buf->region_index,
+ priv->phys_vec, dma_ranges,
+ priv->nr_ranges);
+ if (ret)
+ return ret;
+
+ if (!vfio_device_try_get_registration(&vdev->vdev))
+ return -ENODEV;
+
+ exp_info.ops = &vfio_pci_dmabuf_ops;
+ exp_info.size = priv->size;
+ exp_info.flags = access->get_dma_buf->open_flags;
+ exp_info.priv = priv;
+
+ priv->dmabuf = dma_buf_export(&exp_info);
+ if (IS_ERR(priv->dmabuf)) {
+ ret = PTR_ERR(priv->dmabuf);
+ goto err_dev_put;
+ }
+
+ kref_init(&priv->kref);
+ init_completion(&priv->comp);
+
+ /* dma_buf_put() now frees priv */
+ INIT_LIST_HEAD(&priv->dmabufs_elm);
+ down_write(&vdev->memory_lock);
+ dma_resv_lock(priv->dmabuf->resv, NULL);
+ priv->revoked = !__vfio_pci_memory_enabled(vdev);
+ list_add_tail(&priv->dmabufs_elm, &vdev->dmabufs);
+ dma_resv_unlock(priv->dmabuf->resv);
+ up_write(&vdev->memory_lock);
+ return 0;
+
+err_dev_put:
+ vfio_device_put_registration(&vdev->vdev);
+ return ret;
+}
+
int vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags,
struct vfio_device_feature_dma_buf __user *arg,
size_t argsz)
{
struct vfio_device_feature_dma_buf get_dma_buf = {};
struct vfio_region_dma_range *dma_ranges;
- DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
struct vfio_pci_dma_buf *priv;
+ struct vfio_pci_dmabuf_access access;
size_t length;
int ret;
@@ -274,43 +327,14 @@ int vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags,
priv->vdev = vdev;
priv->nr_ranges = get_dma_buf.nr_ranges;
priv->size = length;
- ret = vdev->pci_ops->get_dmabuf_phys(vdev, &priv->provider,
- get_dma_buf.region_index,
- priv->phys_vec, dma_ranges,
- priv->nr_ranges);
+
+ access.priv = priv;
+ access.get_dma_buf = &get_dma_buf;
+ access.dma_ranges = dma_ranges;
+ ret = vfio_pci_core_access(vdev, vfio_pci_access_dmabuf, &access);
if (ret)
goto err_free_phys;
-
kfree(dma_ranges);
- dma_ranges = NULL;
-
- if (!vfio_device_try_get_registration(&vdev->vdev)) {
- ret = -ENODEV;
- goto err_free_phys;
- }
-
- exp_info.ops = &vfio_pci_dmabuf_ops;
- exp_info.size = priv->size;
- exp_info.flags = get_dma_buf.open_flags;
- exp_info.priv = priv;
-
- priv->dmabuf = dma_buf_export(&exp_info);
- if (IS_ERR(priv->dmabuf)) {
- ret = PTR_ERR(priv->dmabuf);
- goto err_dev_put;
- }
-
- kref_init(&priv->kref);
- init_completion(&priv->comp);
-
- /* dma_buf_put() now frees priv */
- INIT_LIST_HEAD(&priv->dmabufs_elm);
- down_write(&vdev->memory_lock);
- dma_resv_lock(priv->dmabuf->resv, NULL);
- priv->revoked = !__vfio_pci_memory_enabled(vdev);
- list_add_tail(&priv->dmabufs_elm, &vdev->dmabufs);
- dma_resv_unlock(priv->dmabuf->resv);
- up_write(&vdev->memory_lock);
/*
* dma_buf_fd() consumes the reference, when the file closes the dmabuf
@@ -322,8 +346,6 @@ int vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags,
return ret;
-err_dev_put:
- vfio_device_put_registration(&vdev->vdev);
err_free_phys:
kfree(priv->phys_vec);
err_free_priv:
@@ -333,7 +355,7 @@ int vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags,
return ret;
}
-void vfio_pci_dma_buf_move(struct vfio_pci_core_device *vdev, bool revoked)
+static void __vfio_pci_dma_buf_move(struct vfio_pci_core_device *vdev, bool revoked)
{
struct vfio_pci_dma_buf *priv;
struct vfio_pci_dma_buf *tmp;
@@ -404,3 +426,18 @@ void vfio_pci_dma_buf_cleanup(struct vfio_pci_core_device *vdev)
}
up_write(&vdev->memory_lock);
}
+
+/* Recovery may restore mappings while ordinary access is blocked. */
+void vfio_pci_dma_buf_restore(struct vfio_pci_core_device *vdev)
+{
+ __vfio_pci_dma_buf_move(vdev, false);
+}
+
+void vfio_pci_dma_buf_move(struct vfio_pci_core_device *vdev, bool revoked)
+{
+ lockdep_assert_held_write(&vdev->memory_lock);
+ if (revoked)
+ __vfio_pci_dma_buf_move(vdev, true);
+ else
+ vfio_pci_access_ops(vdev)->restore_dmabufs(vdev);
+}
--
2.43.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [RFC PATCH v3 13/20] vfio/pci: Track PCI recovery state
2026-11-12 14:16 [RFC PATCH v3 00/20] vfio/pci: Handle PCI error recovery and report state to userspace Shameer Kolothum
` (11 preceding siblings ...)
2026-11-12 14:16 ` [RFC PATCH v3 12/20] vfio/pci: Use access ops for DMA-BUF export and restoration Shameer Kolothum
@ 2026-11-12 14:16 ` Shameer Kolothum
2026-11-12 14:16 ` [RFC PATCH v3 14/20] vfio/pci: Add INTx recovery operations Shameer Kolothum
` (6 subsequent siblings)
19 siblings, 0 replies; 28+ messages in thread
From: Shameer Kolothum @ 2026-11-12 14:16 UTC (permalink / raw)
To: kvm, linux-pci, linux-kernel
Cc: alex, jgg, kevin.tian, kbusch, michal.winiarski,
satyanarayana.k.v.p, sonangp, ankita, nathanc, mochs,
skolothumtho
PCI error recovery spans several callbacks. Add state to track its
progress so later patches can keep access blocked until recovery
succeeds and leave it blocked if recovery fails.
Add recovery flags, a sequence counter and saved PCI_COMMAND state.
Initialize per-open state during open and clear it on close. Keep host
recovery state separate so close does not lose an ongoing recovery that
must prevent reopening.
Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
---
include/linux/vfio_pci_core.h | 18 +++++++++++++++++-
drivers/vfio/pci/vfio_pci_core.c | 7 +++++++
2 files changed, 24 insertions(+), 1 deletion(-)
diff --git a/include/linux/vfio_pci_core.h b/include/linux/vfio_pci_core.h
index 88aa7b752231..f2b64420cb07 100644
--- a/include/linux/vfio_pci_core.h
+++ b/include/linux/vfio_pci_core.h
@@ -98,6 +98,11 @@ static inline int vfio_pci_core_get_dmabuf_phys(
}
#endif
+#define VFIO_PCI_RECOVERY_IN_PROGRESS BIT(0)
+#define VFIO_PCI_RECOVERY_FROZEN BIT(1)
+#define VFIO_PCI_RECOVERY_RESET BIT(2)
+#define VFIO_PCI_RECOVERY_FAILED BIT(3)
+
struct vfio_pci_access_ops;
struct vfio_pci_core_device {
@@ -159,7 +164,18 @@ struct vfio_pci_core_device {
const struct vfio_pci_access_ops __rcu *access_ops;
/* Set after open completes, cleared before close tears down state. */
bool device_open;
- struct mutex access_lock; /* protects changes to access ops and flags */
+ struct mutex access_lock; /* protects access and recovery state */
+ u32 pci_recovery_flags;
+ u64 pci_recovery_sequence;
+ /* Saved PCI_COMMAND, valid when pci_recovery_command_valid is set. */
+ u16 pci_recovery_command;
+ bool pci_recovery_enabled;
+ bool pci_recovery_command_valid;
+ /*
+ * Host recovery is still running. Protected by access_lock and kept across
+ * close, unlike recovery status for the current open.
+ */
+ bool pci_recovery_host_active;
struct list_head dmabufs;
};
diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
index 0299f5afca91..cdc6bafe9861 100644
--- a/drivers/vfio/pci/vfio_pci_core.c
+++ b/drivers/vfio/pci/vfio_pci_core.c
@@ -923,8 +923,11 @@ void vfio_pci_core_close_device(struct vfio_device *core_vdev)
if (vdev->pci_recovery_supported) {
scoped_guard(mutex, &vdev->access_lock) {
+ WRITE_ONCE(vdev->pci_recovery_enabled, false);
+ vdev->pci_recovery_command_valid = false;
WRITE_ONCE(vdev->device_open, false);
vfio_pci_core_block_access(vdev);
+ WRITE_ONCE(vdev->pci_recovery_flags, 0);
}
}
@@ -963,6 +966,10 @@ void vfio_pci_core_finish_enable(struct vfio_pci_core_device *vdev)
if (vdev->pci_recovery_supported) {
guard(mutex)(&vdev->access_lock);
+ WRITE_ONCE(vdev->pci_recovery_flags, 0);
+ vdev->pci_recovery_sequence = 0;
+ vdev->pci_recovery_command_valid = false;
+ WRITE_ONCE(vdev->pci_recovery_enabled, false);
WRITE_ONCE(vdev->device_open, true);
rcu_assign_pointer(vdev->access_ops, &vfio_pci_live_ops);
}
--
2.43.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [RFC PATCH v3 14/20] vfio/pci: Add INTx recovery operations
2026-11-12 14:16 [RFC PATCH v3 00/20] vfio/pci: Handle PCI error recovery and report state to userspace Shameer Kolothum
` (12 preceding siblings ...)
2026-11-12 14:16 ` [RFC PATCH v3 13/20] vfio/pci: Track PCI recovery state Shameer Kolothum
@ 2026-11-12 14:16 ` Shameer Kolothum
2026-11-12 14:52 ` sashiko-bot
2026-11-12 14:16 ` [RFC PATCH v3 15/20] vfio/pci: Use access ops for SR-IOV configuration and shared reset Shameer Kolothum
` (5 subsequent siblings)
19 siblings, 1 reply; 28+ messages in thread
From: Shameer Kolothum @ 2026-11-12 14:16 UTC (permalink / raw)
To: kvm, linux-pci, linux-kernel
Cc: alex, jgg, kevin.tian, kbusch, michal.winiarski,
satyanarayana.k.v.p, sonangp, ankita, nathanc, mochs,
skolothumtho
Physical INTx interrupts and eventfd unmask requests can arrive while
device access is blocked for recovery. Add blocked operations that
remember unmask requests and keep masking pending interrupts on shared
lines. An explicit mask cancels a pending unmask request.
Select and call INTx operations under irqlock, and hold the same lock
when switching access ops. Process pending unmask requests and publish
live ops under irqlock so requests cannot be lost between these steps.
Keep closed and failed devices blocked.
Provide helpers to mask INTx at recovery entry and preserve its mask
when restoring PCI_COMMAND. The completion caller holds igate and
memory_lock through restoration, then sends any resulting INTx
notification after releasing memory_lock and irqlock.
Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
---
drivers/vfio/pci/vfio_pci_priv.h | 22 ++++
include/linux/vfio_pci_core.h | 2 +-
drivers/vfio/pci/vfio_pci_core.c | 23 ++++
drivers/vfio/pci/vfio_pci_intrs.c | 183 ++++++++++++++++++++++++++----
4 files changed, 209 insertions(+), 21 deletions(-)
diff --git a/drivers/vfio/pci/vfio_pci_priv.h b/drivers/vfio/pci/vfio_pci_priv.h
index b02fd1fbcd31..c195ed3e4d9e 100644
--- a/drivers/vfio/pci/vfio_pci_priv.h
+++ b/drivers/vfio/pci/vfio_pci_priv.h
@@ -3,6 +3,7 @@
#define VFIO_PCI_PRIV_H
#include <linux/vfio_pci_core.h>
+#include <linux/irqreturn.h>
/* Special capability IDs predefined access */
#define PCI_CAP_ID_INVALID 0xFF /* default raw access */
@@ -25,6 +26,11 @@ struct vfio_pci_ioeventfd {
bool vfio_pci_intx_mask(struct vfio_pci_core_device *vdev);
void vfio_pci_intx_unmask(struct vfio_pci_core_device *vdev);
+void vfio_pci_intx_recovery_start(struct vfio_pci_core_device *vdev);
+bool vfio_pci_intx_recovery_finish(struct vfio_pci_core_device *vdev);
+void vfio_pci_intx_recovery_signal(struct vfio_pci_core_device *vdev);
+u16 vfio_pci_intx_recovery_update_command(struct vfio_pci_core_device *vdev,
+ u16 command);
int vfio_pci_eventfd_replace_locked(struct vfio_pci_core_device *vdev,
struct vfio_pci_eventfd __rcu **peventfd,
@@ -84,6 +90,8 @@ typedef long (*vfio_pci_access_fn)(struct vfio_pci_core_device *vdev,
long vfio_pci_core_access(struct vfio_pci_core_device *vdev,
vfio_pci_access_fn fn, void *data);
+struct vfio_pci_irq_ctx;
+
struct vfio_pci_access_ops {
/* Select and call within SRCU. The callback receives kernel buffers. */
long (*execute)(struct vfio_pci_core_device *vdev,
@@ -92,10 +100,24 @@ struct vfio_pci_access_ops {
int (*set_power_state)(struct vfio_pci_core_device *vdev, pci_power_t state);
/* Caller holds memory_lock. */
void (*restore_dmabufs)(struct vfio_pci_core_device *vdev);
+ /* Select and call both INTx methods while holding irqlock. */
+ irqreturn_t (*intx_handler)(struct vfio_pci_core_device *vdev,
+ struct vfio_pci_irq_ctx *ctx);
+ int (*intx_unmask)(struct vfio_pci_core_device *vdev,
+ struct vfio_pci_irq_ctx *ctx);
};
const struct vfio_pci_access_ops *
vfio_pci_access_ops(struct vfio_pci_core_device *vdev);
+void vfio_pci_restore_access(struct vfio_pci_core_device *vdev);
+irqreturn_t vfio_pci_intx_handle(struct vfio_pci_core_device *vdev,
+ struct vfio_pci_irq_ctx *ctx);
+irqreturn_t vfio_pci_intx_handle_blocked(struct vfio_pci_core_device *vdev,
+ struct vfio_pci_irq_ctx *ctx);
+int vfio_pci_intx_unmask_live(struct vfio_pci_core_device *vdev,
+ struct vfio_pci_irq_ctx *ctx);
+int vfio_pci_intx_unmask_blocked(struct vfio_pci_core_device *vdev,
+ struct vfio_pci_irq_ctx *ctx);
#ifdef CONFIG_VFIO_PCI_IGD
bool vfio_pci_is_intel_display(struct pci_dev *pdev);
diff --git a/include/linux/vfio_pci_core.h b/include/linux/vfio_pci_core.h
index f2b64420cb07..fadbbaa58770 100644
--- a/include/linux/vfio_pci_core.h
+++ b/include/linux/vfio_pci_core.h
@@ -160,7 +160,7 @@ struct vfio_pci_core_device {
struct notifier_block nb;
struct rw_semaphore memory_lock;
struct srcu_struct access_srcu;
- /* Updated under access_lock. */
+ /* Updated under access_lock and irqlock. */
const struct vfio_pci_access_ops __rcu *access_ops;
/* Set after open completes, cleared before close tears down state. */
bool device_open;
diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
index cdc6bafe9861..89cebe2522ba 100644
--- a/drivers/vfio/pci/vfio_pci_core.c
+++ b/drivers/vfio/pci/vfio_pci_core.c
@@ -338,12 +338,16 @@ static const struct vfio_pci_access_ops vfio_pci_live_ops = {
.execute = vfio_pci_access_live,
.set_power_state = vfio_pci_try_set_power_state,
.restore_dmabufs = vfio_pci_dma_buf_restore,
+ .intx_handler = vfio_pci_intx_handle,
+ .intx_unmask = vfio_pci_intx_unmask_live,
};
static const struct vfio_pci_access_ops vfio_pci_blocked_ops = {
.execute = vfio_pci_access_blocked,
.set_power_state = vfio_pci_power_blocked,
.restore_dmabufs = vfio_pci_restore_dmabufs_blocked,
+ .intx_handler = vfio_pci_intx_handle_blocked,
+ .intx_unmask = vfio_pci_intx_unmask_blocked,
};
/*
@@ -362,12 +366,27 @@ vfio_pci_access_ops(struct vfio_pci_core_device *vdev)
/* Block new operations and drain ordinary accesses before teardown or recovery. */
static void vfio_pci_core_block_access(struct vfio_pci_core_device *vdev)
{
+ unsigned long flags;
+
lockdep_assert_held(&vdev->access_lock);
+ spin_lock_irqsave(&vdev->irqlock, flags);
rcu_assign_pointer(vdev->access_ops, &vfio_pci_blocked_ops);
+ spin_unlock_irqrestore(&vdev->irqlock, flags);
synchronize_srcu(&vdev->access_srcu);
}
+/* Caller finishes internal restoration before allowing ordinary access. */
+void vfio_pci_restore_access(struct vfio_pci_core_device *vdev)
+{
+ lockdep_assert_held(&vdev->access_lock);
+ lockdep_assert_held(&vdev->irqlock);
+ if (vdev->device_open &&
+ !(vdev->pci_recovery_flags &
+ (VFIO_PCI_RECOVERY_IN_PROGRESS | VFIO_PCI_RECOVERY_FAILED)))
+ rcu_assign_pointer(vdev->access_ops, &vfio_pci_live_ops);
+}
+
long vfio_pci_core_access(struct vfio_pci_core_device *vdev,
vfio_pci_access_fn fn, void *data)
{
@@ -965,13 +984,17 @@ void vfio_pci_core_finish_enable(struct vfio_pci_core_device *vdev)
}
if (vdev->pci_recovery_supported) {
+ unsigned long flags;
+
guard(mutex)(&vdev->access_lock);
WRITE_ONCE(vdev->pci_recovery_flags, 0);
vdev->pci_recovery_sequence = 0;
vdev->pci_recovery_command_valid = false;
WRITE_ONCE(vdev->pci_recovery_enabled, false);
WRITE_ONCE(vdev->device_open, true);
+ spin_lock_irqsave(&vdev->irqlock, flags);
rcu_assign_pointer(vdev->access_ops, &vfio_pci_live_ops);
+ spin_unlock_irqrestore(&vdev->irqlock, flags);
}
}
EXPORT_SYMBOL_GPL(vfio_pci_core_finish_enable);
diff --git a/drivers/vfio/pci/vfio_pci_intrs.c b/drivers/vfio/pci/vfio_pci_intrs.c
index 64f80f64ff57..f78c7217017d 100644
--- a/drivers/vfio/pci/vfio_pci_intrs.c
+++ b/drivers/vfio/pci/vfio_pci_intrs.c
@@ -29,6 +29,8 @@ struct vfio_pci_irq_ctx {
struct virqfd *mask;
char *name;
bool masked;
+ bool recovery_masked;
+ bool unmask_pending;
struct irq_bypass_producer producer;
};
@@ -126,6 +128,10 @@ static bool __vfio_pci_intx_mask(struct vfio_pci_core_device *vdev)
if (WARN_ON_ONCE(!ctx))
goto out_unlock;
+ /* An explicit mask cancels any unmask request saved during recovery. */
+ ctx->recovery_masked = false;
+ ctx->unmask_pending = false;
+
if (!ctx->masked) {
/*
* Can't use check_and_mask here because we always want to
@@ -162,16 +168,13 @@ bool vfio_pci_intx_mask(struct vfio_pci_core_device *vdev)
* a signal is necessary, which can then be handled via a work queue
* or directly depending on the caller.
*/
-static int vfio_pci_intx_unmask_handler(void *opaque, void *data)
+int vfio_pci_intx_unmask_live(struct vfio_pci_core_device *vdev,
+ struct vfio_pci_irq_ctx *ctx)
{
- struct vfio_pci_core_device *vdev = opaque;
struct pci_dev *pdev = vdev->pdev;
- struct vfio_pci_irq_ctx *ctx = data;
- unsigned long flags;
int ret = 0;
- spin_lock_irqsave(&vdev->irqlock, flags);
-
+ lockdep_assert_held(&vdev->irqlock);
/*
* Unmasking comes from ioctl or config, so again, have the
* physical bit follow the virtual even when not using INTx.
@@ -179,9 +182,12 @@ static int vfio_pci_intx_unmask_handler(void *opaque, void *data)
if (unlikely(!is_intx(vdev))) {
if (vdev->pci_2_3)
pci_intx(pdev, 1);
- goto out_unlock;
+ goto out;
}
+ ctx->unmask_pending = false;
+ ctx->recovery_masked = false;
+
if (ctx->masked && !vdev->virq_disabled) {
/*
* A pending interrupt here would immediately trigger,
@@ -197,9 +203,28 @@ static int vfio_pci_intx_unmask_handler(void *opaque, void *data)
ctx->masked = (ret > 0);
}
-out_unlock:
- spin_unlock_irqrestore(&vdev->irqlock, flags);
+out:
+ return ret;
+}
+int vfio_pci_intx_unmask_blocked(struct vfio_pci_core_device *vdev,
+ struct vfio_pci_irq_ctx *ctx)
+{
+ lockdep_assert_held(&vdev->irqlock);
+ if (is_intx(vdev))
+ ctx->unmask_pending = true;
+ return 0;
+}
+
+static int vfio_pci_intx_unmask_handler(void *opaque, void *data)
+{
+ struct vfio_pci_core_device *vdev = opaque;
+ unsigned long flags;
+ int ret;
+
+ spin_lock_irqsave(&vdev->irqlock, flags);
+ ret = vfio_pci_access_ops(vdev)->intx_unmask(vdev, data);
+ spin_unlock_irqrestore(&vdev->irqlock, flags);
return ret;
}
@@ -220,33 +245,151 @@ void vfio_pci_intx_unmask(struct vfio_pci_core_device *vdev)
mutex_unlock(&vdev->igate);
}
-static irqreturn_t vfio_intx_handler(int irq, void *dev_id)
+/*
+ * Recovery always masks INTx and remembers whether to unmask it afterward.
+ * The interrupt handler masks INTx only if this device has a pending
+ * interrupt, since another device may share the line.
+ */
+static bool vfio_pci_intx_mask_for_recovery(struct vfio_pci_core_device *vdev,
+ struct vfio_pci_irq_ctx *ctx,
+ bool quiesce)
{
- struct vfio_pci_irq_ctx *ctx = dev_id;
- struct vfio_pci_core_device *vdev = ctx->vdev;
- unsigned long flags;
- int ret = IRQ_NONE;
+ lockdep_assert_held(&vdev->irqlock);
- spin_lock_irqsave(&vdev->irqlock, flags);
+ if (ctx->masked)
+ return false;
+ if (!vdev->pci_2_3)
+ disable_irq_nosync(vdev->pdev->irq);
+ else if (quiesce)
+ pci_intx(vdev->pdev, 0);
+ else if (!pci_check_and_mask_intx(vdev->pdev))
+ return false;
+
+ ctx->masked = true;
+ if (quiesce)
+ ctx->recovery_masked = true;
+ return true;
+}
+
+irqreturn_t vfio_pci_intx_handle(struct vfio_pci_core_device *vdev,
+ struct vfio_pci_irq_ctx *ctx)
+{
+ lockdep_assert_held(&vdev->irqlock);
if (!vdev->pci_2_3) {
disable_irq_nosync(vdev->pdev->irq);
ctx->masked = true;
- ret = IRQ_HANDLED;
- } else if (!ctx->masked && /* may be shared */
- pci_check_and_mask_intx(vdev->pdev)) {
+ return IRQ_HANDLED;
+ }
+ if (!ctx->masked && pci_check_and_mask_intx(vdev->pdev)) {
ctx->masked = true;
- ret = IRQ_HANDLED;
+ return IRQ_HANDLED;
}
+ return IRQ_NONE;
+}
+
+irqreturn_t vfio_pci_intx_handle_blocked(struct vfio_pci_core_device *vdev,
+ struct vfio_pci_irq_ctx *ctx)
+{
+ lockdep_assert_held(&vdev->irqlock);
+ /* A shared interrupt line still needs pending INTx to be masked. */
+ if (vfio_pci_intx_mask_for_recovery(vdev, ctx, false) ||
+ (ctx->masked && !vdev->pci_2_3))
+ return IRQ_HANDLED;
+ return IRQ_NONE;
+}
+
+static irqreturn_t vfio_intx_handler(int irq, void *dev_id)
+{
+ struct vfio_pci_irq_ctx *ctx = dev_id;
+ struct vfio_pci_core_device *vdev = ctx->vdev;
+ unsigned long flags;
+ irqreturn_t ret;
+ spin_lock_irqsave(&vdev->irqlock, flags);
+ ret = vfio_pci_access_ops(vdev)->intx_handler(vdev, ctx);
spin_unlock_irqrestore(&vdev->irqlock, flags);
if (ret == IRQ_HANDLED)
vfio_send_intx_eventfd(vdev, ctx);
-
return ret;
}
+void vfio_pci_intx_recovery_start(struct vfio_pci_core_device *vdev)
+{
+ struct vfio_pci_irq_ctx *ctx;
+ unsigned long flags;
+
+ lockdep_assert_held(&vdev->access_lock);
+
+ spin_lock_irqsave(&vdev->irqlock, flags);
+ if (!is_intx(vdev))
+ goto out_unlock;
+
+ ctx = vfio_irq_ctx_get(vdev, 0);
+ if (WARN_ON_ONCE(!ctx))
+ goto out_unlock;
+
+ vfio_pci_intx_mask_for_recovery(vdev, ctx, true);
+
+out_unlock:
+ spin_unlock_irqrestore(&vdev->irqlock, flags);
+}
+
+/* Preserve the current INTx mask when restoring PCI_COMMAND. */
+u16 vfio_pci_intx_recovery_update_command(struct vfio_pci_core_device *vdev,
+ u16 command)
+{
+ struct vfio_pci_irq_ctx *ctx;
+
+ lockdep_assert_held(&vdev->irqlock);
+
+ if (!vdev->pci_2_3 || !is_intx(vdev))
+ return command;
+
+ ctx = vfio_irq_ctx_get(vdev, 0);
+ if (ctx && ctx->masked)
+ command |= PCI_COMMAND_INTX_DISABLE;
+
+ return command;
+}
+
+/*
+ * Restore INTx and allow ordinary access under the same lock. An unmask
+ * request must either be replayed here or reach the live op afterward.
+ */
+bool vfio_pci_intx_recovery_finish(struct vfio_pci_core_device *vdev)
+{
+ struct vfio_pci_irq_ctx *ctx = NULL;
+ unsigned long flags;
+ int ret = 0;
+
+ lockdep_assert_held(&vdev->access_lock);
+ lockdep_assert_held(&vdev->igate);
+ lockdep_assert_held_write(&vdev->memory_lock);
+ spin_lock_irqsave(&vdev->irqlock, flags);
+ if (!is_intx(vdev))
+ goto out_restore;
+
+ ctx = vfio_irq_ctx_get(vdev, 0);
+ if (WARN_ON_ONCE(!ctx))
+ goto out_restore;
+
+ if (ctx->recovery_masked || ctx->unmask_pending)
+ ret = vfio_pci_intx_unmask_live(vdev, ctx);
+
+out_restore:
+ vfio_pci_restore_access(vdev);
+ spin_unlock_irqrestore(&vdev->irqlock, flags);
+ return ret > 0;
+}
+
+void vfio_pci_intx_recovery_signal(struct vfio_pci_core_device *vdev)
+{
+ lockdep_assert_held(&vdev->igate);
+ vfio_send_intx_eventfd(vdev, vfio_irq_ctx_get(vdev, 0));
+}
+
static int vfio_intx_enable(struct vfio_pci_core_device *vdev,
struct eventfd_ctx *trigger)
{
--
2.43.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [RFC PATCH v3 14/20] vfio/pci: Add INTx recovery operations
2026-11-12 14:16 ` [RFC PATCH v3 14/20] vfio/pci: Add INTx recovery operations Shameer Kolothum
@ 2026-11-12 14:52 ` sashiko-bot
0 siblings, 0 replies; 28+ messages in thread
From: sashiko-bot @ 2026-11-12 14:52 UTC (permalink / raw)
To: Shameer Kolothum, kvm, linux-pci
Thank you for your contribution! Sashiko AI review found 3 potential
issues to consider:
- [High] vfio/pci: permanent INTx masking in __vfio_pci_intx_mask() on
runtime suspend
- [High] vfio/pci: unmasked shared INTx in
vfio_pci_intx_mask_for_recovery()
- [Medium] vfio/pci: premature INTx unmask after blocked irq in
vfio_intx_handler()
--
commit 832141c440dca237115d9dc7d2fca6d1c8bdf80a
Author: Shameer Kolothum <skolothumtho@nvidia.com>
vfio/pci: Add INTx recovery operations
This commit adds blocked INTx operations and recovery helpers to mask INTx,
record pending unmask requests while access is blocked, and restore INTx
state when recovery finishes.
> diff --git a/drivers/vfio/pci/vfio_pci_intrs.c b/drivers/vfio/pci/vfio_pci_intrs.c
> index 64f80f64ff57a..f78c7217017d2 100644
> --- a/drivers/vfio/pci/vfio_pci_intrs.c
> +++ b/drivers/vfio/pci/vfio_pci_intrs.c
> @@ -29,6 +29,8 @@ struct vfio_pci_irq_ctx {
> struct virqfd *mask;
> char *name;
> bool masked;
> + bool recovery_masked;
> + bool unmask_pending;
> struct irq_bypass_producer producer;
> };
>
> @@ -126,6 +128,10 @@ static bool __vfio_pci_intx_mask(struct vfio_pci_core_device *vdev)
> if (WARN_ON_ONCE(!ctx))
> goto out_unlock;
>
> + /* An explicit mask cancels any unmask request saved during recovery. */
> + ctx->recovery_masked = false;
> + ctx->unmask_pending = false;
> +
> if (!ctx->masked) {
> /*
> * Can't use check_and_mask here because we always want to
[Severity: High]
Can __vfio_pci_intx_mask() leave INTx permanently masked if
vfio_pci_core_runtime_suspend() runs during recovery?
Unlike userspace mask paths, vfio_pci_core_runtime_suspend() is not gated
by vfio_pci_core_access() and calls:
drivers/vfio/pci/vfio_pci_core.c:vfio_pci_core_runtime_suspend() {
...
vdev->pm_intx_masked = ((vdev->irq_type == VFIO_PCI_INTX_IRQ_INDEX) &&
vfio_pci_intx_mask(vdev));
...
}
When INTx is masked for recovery (ctx->masked is true and
ctx->recovery_masked or ctx->unmask_pending is true), a runtime suspend
(for example when pm_runtime_put() runs after an ioctl on a device in
VFIO_DEVICE_FEATURE_LOW_POWER_ENTRY) clears both ctx->recovery_masked and
ctx->unmask_pending here, skips the !ctx->masked block, and returns false.
vfio_pci_core_runtime_suspend() then records vdev->pm_intx_masked = false,
so neither vfio_pci_core_runtime_resume() nor
vfio_pci_intx_recovery_finish() will unmask INTx again.
> @@ -220,33 +245,151 @@ void vfio_pci_intx_unmask(struct vfio_pci_core_device *vdev)
> mutex_unlock(&vdev->igate);
> }
>
> -static irqreturn_t vfio_intx_handler(int irq, void *dev_id)
> +/*
> + * Recovery always masks INTx and remembers whether to unmask it afterward.
> + * The interrupt handler masks INTx only if this device has a pending
> + * interrupt, since another device may share the line.
> + */
> +static bool vfio_pci_intx_mask_for_recovery(struct vfio_pci_core_device *vdev,
> + struct vfio_pci_irq_ctx *ctx,
> + bool quiesce)
> {
> - struct vfio_pci_irq_ctx *ctx = dev_id;
> - struct vfio_pci_core_device *vdev = ctx->vdev;
> - unsigned long flags;
> - int ret = IRQ_NONE;
> + lockdep_assert_held(&vdev->irqlock);
>
> - spin_lock_irqsave(&vdev->irqlock, flags);
> + if (ctx->masked)
> + return false;
>
> + if (!vdev->pci_2_3)
> + disable_irq_nosync(vdev->pdev->irq);
> + else if (quiesce)
> + pci_intx(vdev->pdev, 0);
> + else if (!pci_check_and_mask_intx(vdev->pdev))
> + return false;
>
> + ctx->masked = true;
> + if (quiesce)
> + ctx->recovery_masked = true;
> + return true;
> +}
[Severity: High]
Does the early return when ctx->masked is true prevent
vfio_pci_intx_handle_blocked() from masking shared PCI 2.3 interrupts
during recovery?
vfio_pci_intx_recovery_start() calls vfio_pci_intx_mask_for_recovery()
with quiesce = true, setting ctx->masked = true. However, on a PCI 2.3
device, PCI_COMMAND_INTX_DISABLE can still be clear in hardware if config
space read 0xffff when error_detected ran, or after a host slot/bus reset
clears PCI_COMMAND before state restoration. If the device then asserts a
shared INTx line while access is blocked, vfio_pci_intx_handle_blocked()
calls vfio_pci_intx_mask_for_recovery(vdev, ctx, false), which returns
false immediately because ctx->masked is true instead of calling
pci_check_and_mask_intx(), returning IRQ_NONE and leaving the shared level
interrupt asserted.
> +
> +irqreturn_t vfio_pci_intx_handle(struct vfio_pci_core_device *vdev,
> + struct vfio_pci_irq_ctx *ctx)
> +{
> + lockdep_assert_held(&vdev->irqlock);
> if (!vdev->pci_2_3) {
> disable_irq_nosync(vdev->pdev->irq);
> ctx->masked = true;
> - ret = IRQ_HANDLED;
> - } else if (!ctx->masked && /* may be shared */
> - pci_check_and_mask_intx(vdev->pdev)) {
> + return IRQ_HANDLED;
> + }
> + if (!ctx->masked && pci_check_and_mask_intx(vdev->pdev)) {
> ctx->masked = true;
> - ret = IRQ_HANDLED;
> + return IRQ_HANDLED;
> }
> + return IRQ_NONE;
> +}
> +
> +irqreturn_t vfio_pci_intx_handle_blocked(struct vfio_pci_core_device *vdev,
> + struct vfio_pci_irq_ctx *ctx)
> +{
> + lockdep_assert_held(&vdev->irqlock);
> + /* A shared interrupt line still needs pending INTx to be masked. */
> + if (vfio_pci_intx_mask_for_recovery(vdev, ctx, false) ||
> + (ctx->masked && !vdev->pci_2_3))
> + return IRQ_HANDLED;
> + return IRQ_NONE;
> +}
> +
> +static irqreturn_t vfio_intx_handler(int irq, void *dev_id)
> +{
> + struct vfio_pci_irq_ctx *ctx = dev_id;
> + struct vfio_pci_core_device *vdev = ctx->vdev;
> + unsigned long flags;
> + irqreturn_t ret;
>
> + spin_lock_irqsave(&vdev->irqlock, flags);
> + ret = vfio_pci_access_ops(vdev)->intx_handler(vdev, ctx);
> spin_unlock_irqrestore(&vdev->irqlock, flags);
>
> if (ret == IRQ_HANDLED)
> vfio_send_intx_eventfd(vdev, ctx);
> -
> return ret;
> }
[Severity: Medium]
When vfio_pci_intx_handle_blocked() returns IRQ_HANDLED,
vfio_intx_handler() delivers the automasked interrupt to userspace via
vfio_send_intx_eventfd(), but neither vfio_pci_intx_mask_for_recovery()
nor vfio_pci_intx_handle_blocked() clears ctx->recovery_masked or
ctx->unmask_pending. Can this cause vfio_pci_intx_recovery_finish() to
unmask INTx prematurely before userspace has processed and unmasked the
interrupt?
CPU0 CPU1 (<hardirq>)
vfio_pci_intx_recovery_start()
spin_lock_irqsave(&vdev->irqlock)
vfio_pci_intx_mask_for_recovery(..., true)
disable_irq_nosync(pdev->irq)
ctx->masked = true
ctx->recovery_masked = true
spin_unlock_irqrestore(&vdev->irqlock)
vfio_intx_handler()
spin_lock_irqsave(&vdev->irqlock)
vfio_pci_intx_handle_blocked()
ctx->masked && !vdev->pci_2_3
--> returns IRQ_HANDLED
spin_unlock_irqrestore(...)
vfio_send_intx_eventfd()
...
vfio_pci_intx_recovery_finish()
if (ctx->recovery_masked || ...)
vfio_pci_intx_unmask_live() <-- unmasks before userspace
On !vdev->pci_2_3 devices, if ctx->masked was already true before recovery
started, the (ctx->masked && !vdev->pci_2_3) check also returns
IRQ_HANDLED and triggers a spurious eventfd signal while masked. The same
premature unmask happens if vfio_pci_intx_unmask_blocked() sets
ctx->unmask_pending = true while ctx->masked is still false (before
vfio_pci_intx_recovery_start() runs) and an interrupt is then masked and
signaled by vfio_pci_intx_handle_blocked() without clearing
ctx->unmask_pending.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261112141632.918954-1-skolothumtho@nvidia.com?part=14
^ permalink raw reply [flat|nested] 28+ messages in thread
* [RFC PATCH v3 15/20] vfio/pci: Use access ops for SR-IOV configuration and shared reset
2026-11-12 14:16 [RFC PATCH v3 00/20] vfio/pci: Handle PCI error recovery and report state to userspace Shameer Kolothum
` (13 preceding siblings ...)
2026-11-12 14:16 ` [RFC PATCH v3 14/20] vfio/pci: Add INTx recovery operations Shameer Kolothum
@ 2026-11-12 14:16 ` Shameer Kolothum
2026-11-12 14:49 ` sashiko-bot
2026-11-12 14:16 ` [RFC PATCH v3 16/20] vfio/pci: Restore device state from slot_reset() Shameer Kolothum
` (4 subsequent siblings)
19 siblings, 1 reply; 28+ messages in thread
From: Shameer Kolothum @ 2026-11-12 14:16 UTC (permalink / raw)
To: kvm, linux-pci, linux-kernel
Cc: alex, jgg, kevin.tian, kbusch, michal.winiarski,
satyanarayana.k.v.p, sonangp, ankita, nathanc, mochs,
skolothumtho
SR-IOV configuration and shared hot reset can change device state during
recovery. Both can involve unopened devices, whose per-open SRCU
instance is unavailable.
Select SR-IOV configuration through access ops under the PCI device
lock. Reject requests for open devices while access is blocked, but
preserve configuration of unopened devices. Unregister uses the internal
helper to remove VFs regardless of the selected ops.
Select hot-reset preparation under each device's memory_lock. Reject
open recovering devices, but allow unopened and failed devices so they
do not prevent resetting healthy siblings. Unwind the acquired locks if
preparation fails.
Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
---
drivers/vfio/pci/vfio_pci_priv.h | 4 +++
drivers/vfio/pci/vfio_pci_core.c | 55 +++++++++++++++++++++++++++++---
2 files changed, 54 insertions(+), 5 deletions(-)
diff --git a/drivers/vfio/pci/vfio_pci_priv.h b/drivers/vfio/pci/vfio_pci_priv.h
index c195ed3e4d9e..e4f3e4f51180 100644
--- a/drivers/vfio/pci/vfio_pci_priv.h
+++ b/drivers/vfio/pci/vfio_pci_priv.h
@@ -100,6 +100,10 @@ struct vfio_pci_access_ops {
int (*set_power_state)(struct vfio_pci_core_device *vdev, pci_power_t state);
/* Caller holds memory_lock. */
void (*restore_dmabufs)(struct vfio_pci_core_device *vdev);
+ /* Caller holds the PCI device lock. The device need not be open. */
+ int (*sriov_configure)(struct vfio_pci_core_device *vdev, int nr_virtfn);
+ /* Caller holds memory_lock for this member of the reset set. */
+ int (*prepare_hot_reset)(struct vfio_pci_core_device *vdev);
/* Select and call both INTx methods while holding irqlock. */
irqreturn_t (*intx_handler)(struct vfio_pci_core_device *vdev,
struct vfio_pci_irq_ctx *ctx);
diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
index 89cebe2522ba..0ef51810254e 100644
--- a/drivers/vfio/pci/vfio_pci_core.c
+++ b/drivers/vfio/pci/vfio_pci_core.c
@@ -312,6 +312,11 @@ static void vfio_pci_probe_power_state(struct vfio_pci_core_device *vdev)
vdev->needs_pm_restore = !(pmcsr & PCI_PM_CTRL_NO_SOFT_RESET);
}
+static int vfio_pci_sriov_configure_live(struct vfio_pci_core_device *vdev,
+ int nr_virtfn);
+static int vfio_pci_prepare_hot_reset(struct vfio_pci_core_device *vdev);
+static int vfio_pci_prepare_hot_reset_blocked(struct vfio_pci_core_device *vdev);
+
static long vfio_pci_access_live(struct vfio_pci_core_device *vdev,
vfio_pci_access_fn fn, void *data)
{
@@ -334,10 +339,21 @@ static void vfio_pci_restore_dmabufs_blocked(struct vfio_pci_core_device *vdev)
{
}
+static int vfio_pci_sriov_configure_blocked(struct vfio_pci_core_device *vdev,
+ int nr_virtfn)
+{
+ /* SR-IOV can be configured without an open VFIO device. */
+ if (!READ_ONCE(vdev->device_open))
+ return vfio_pci_sriov_configure_live(vdev, nr_virtfn);
+ return -EIO;
+}
+
static const struct vfio_pci_access_ops vfio_pci_live_ops = {
.execute = vfio_pci_access_live,
.set_power_state = vfio_pci_try_set_power_state,
.restore_dmabufs = vfio_pci_dma_buf_restore,
+ .sriov_configure = vfio_pci_sriov_configure_live,
+ .prepare_hot_reset = vfio_pci_prepare_hot_reset,
.intx_handler = vfio_pci_intx_handle,
.intx_unmask = vfio_pci_intx_unmask_live,
};
@@ -346,6 +362,8 @@ static const struct vfio_pci_access_ops vfio_pci_blocked_ops = {
.execute = vfio_pci_access_blocked,
.set_power_state = vfio_pci_power_blocked,
.restore_dmabufs = vfio_pci_restore_dmabufs_blocked,
+ .sriov_configure = vfio_pci_sriov_configure_blocked,
+ .prepare_hot_reset = vfio_pci_prepare_hot_reset_blocked,
.intx_handler = vfio_pci_intx_handle_blocked,
.intx_unmask = vfio_pci_intx_unmask_blocked,
};
@@ -2594,7 +2612,7 @@ EXPORT_SYMBOL_GPL(vfio_pci_core_register_device);
void vfio_pci_core_unregister_device(struct vfio_pci_core_device *vdev)
{
- vfio_pci_core_sriov_configure(vdev, 0);
+ vfio_pci_sriov_configure_live(vdev, 0);
vfio_unregister_group_dev(&vdev->vdev);
@@ -2622,8 +2640,8 @@ pci_ers_result_t vfio_pci_core_aer_err_detected(struct vfio_pci_core_device *vde
}
EXPORT_SYMBOL_GPL(vfio_pci_core_aer_err_detected);
-int vfio_pci_core_sriov_configure(struct vfio_pci_core_device *vdev,
- int nr_virtfn)
+static int vfio_pci_sriov_configure_live(struct vfio_pci_core_device *vdev,
+ int nr_virtfn)
{
struct pci_dev *pdev = vdev->pdev;
int ret = 0;
@@ -2695,6 +2713,13 @@ int vfio_pci_core_sriov_configure(struct vfio_pci_core_device *vdev,
mutex_unlock(&vfio_pci_sriov_pfs_mutex);
return ret;
}
+
+int vfio_pci_core_sriov_configure(struct vfio_pci_core_device *vdev,
+ int nr_virtfn)
+{
+ device_lock_assert(&vdev->pdev->dev);
+ return vfio_pci_access_ops(vdev)->sriov_configure(vdev, nr_virtfn);
+}
EXPORT_SYMBOL_GPL(vfio_pci_core_sriov_configure);
static bool vfio_dev_in_groups(struct vfio_device *vdev,
@@ -2776,6 +2801,23 @@ static int vfio_pci_dev_set_pm_runtime_get(struct vfio_device_set *dev_set)
return ret;
}
+static int vfio_pci_prepare_hot_reset(struct vfio_pci_core_device *vdev)
+{
+ lockdep_assert_held_write(&vdev->memory_lock);
+ vfio_pci_dma_buf_move(vdev, true);
+ vfio_pci_zap_bars(vdev);
+ return 0;
+}
+
+static int vfio_pci_prepare_hot_reset_blocked(struct vfio_pci_core_device *vdev)
+{
+ /* Unopened and failed devices must not prevent a shared reset. */
+ if (READ_ONCE(vdev->device_open) &&
+ !(READ_ONCE(vdev->pci_recovery_flags) & VFIO_PCI_RECOVERY_FAILED))
+ return -EBUSY;
+ return vfio_pci_prepare_hot_reset(vdev);
+}
+
static int vfio_pci_dev_set_hot_reset(struct vfio_device_set *dev_set,
struct vfio_pci_group_info *groups,
struct iommufd_ctx *iommufd_ctx)
@@ -2849,8 +2891,11 @@ static int vfio_pci_dev_set_hot_reset(struct vfio_device_set *dev_set,
break;
}
- vfio_pci_dma_buf_move(vdev, true);
- vfio_pci_zap_bars(vdev);
+ ret = vfio_pci_access_ops(vdev)->prepare_hot_reset(vdev);
+ if (ret) {
+ up_write(&vdev->memory_lock);
+ break;
+ }
}
if (!list_entry_is_head(vdev,
--
2.43.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [RFC PATCH v3 15/20] vfio/pci: Use access ops for SR-IOV configuration and shared reset
2026-11-12 14:16 ` [RFC PATCH v3 15/20] vfio/pci: Use access ops for SR-IOV configuration and shared reset Shameer Kolothum
@ 2026-11-12 14:49 ` sashiko-bot
0 siblings, 0 replies; 28+ messages in thread
From: sashiko-bot @ 2026-11-12 14:49 UTC (permalink / raw)
To: Shameer Kolothum, kvm, linux-pci
Thank you for your contribution! Sashiko AI review found 3 potential
issues to consider:
- [Medium] vfio/pci: vfio_pci_dev_set_hot_reset() calls D0 helper on
failed devices
- [Medium] vfio/pci: vfio_pci_prepare_hot_reset_blocked() misses host
recovery and caller checks
- [Medium] vfio/pci: unsynchronized open and recovery state in
vfio_pci_core_sriov_configure()
--
commit abe74651d949fa43ae45614261ad8c3282ae7603
Author: Shameer Kolothum <skolothumtho@nvidia.com>
vfio/pci: Use access ops for SR-IOV configuration and shared reset
This commit routes SR-IOV configuration and shared hot-reset preparation
through vfio_pci_access_ops so that recovering open devices are rejected
while unopened or failed devices remain allowed.
> diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
> index 89cebe2522ba0..0ef51810254ea 100644
> --- a/drivers/vfio/pci/vfio_pci_core.c
> +++ b/drivers/vfio/pci/vfio_pci_core.c
> @@ -334,10 +339,21 @@ static void vfio_pci_restore_dmabufs_blocked(struct vfio_pci_core_device *vdev)
> {
> }
>
> +static int vfio_pci_sriov_configure_blocked(struct vfio_pci_core_device *vdev,
> + int nr_virtfn)
> +{
> + /* SR-IOV can be configured without an open VFIO device. */
> + if (!READ_ONCE(vdev->device_open))
> + return vfio_pci_sriov_configure_live(vdev, nr_virtfn);
> + return -EIO;
> +}
[Severity: Medium]
Can vfio_pci_core_sriov_configure() race with device open, close, or host
PCI recovery because neither vdev->access_ops nor vdev->device_open is
serialized by device_lock(&vdev->pdev->dev)?
vfio_pci_core_finish_enable() and vfio_pci_core_close_device() update
vdev->device_open and vdev->access_ops under vdev->access_lock without
holding device_lock(&vdev->pdev->dev):
CPU0 CPU1
vfio_pci_core_finish_enable()
WRITE_ONCE(vdev->device_open, true)
vfio_pci_core_sriov_configure()
vfio_pci_access_ops(vdev)
/* sees blocked_ops */
vfio_pci_sriov_configure_blocked()
READ_ONCE(vdev->device_open)
--> returns -EIO
rcu_assign_pointer(vdev->access_ops,
&vfio_pci_live_ops)
In the reverse direction, vfio_pci_core_close_device() sets
vdev->device_open = false before vfio_pci_core_disable() finishes tearing
down the PF, and pcie_do_recovery() drops device_lock(&pdev->dev) between
recovery callbacks while vdev->pci_recovery_host_active is true. Because
vfio_pci_sriov_configure_blocked() only checks !READ_ONCE(vdev->device_open)
without taking vdev->access_lock or checking vdev->pci_recovery_host_active,
sysfs sriov_numvfs can run vfio_pci_sriov_configure_live() while close
teardown or host recovery is in progress.
[ ... ]
> @@ -2776,6 +2801,23 @@ static int vfio_pci_dev_set_pm_runtime_get(struct vfio_device_set *dev_set)
> return ret;
> }
>
> +static int vfio_pci_prepare_hot_reset(struct vfio_pci_core_device *vdev)
> +{
> + lockdep_assert_held_write(&vdev->memory_lock);
> + vfio_pci_dma_buf_move(vdev, true);
> + vfio_pci_zap_bars(vdev);
> + return 0;
> +}
> +
> +static int vfio_pci_prepare_hot_reset_blocked(struct vfio_pci_core_device *vdev)
> +{
> + /* Unopened and failed devices must not prevent a shared reset. */
> + if (READ_ONCE(vdev->device_open) &&
> + !(READ_ONCE(vdev->pci_recovery_flags) & VFIO_PCI_RECOVERY_FAILED))
> + return -EBUSY;
> + return vfio_pci_prepare_hot_reset(vdev);
> +}
[Severity: Medium]
Can vfio_pci_prepare_hot_reset_blocked() allow pci_reset_bus() to run
while host PCI recovery is active or when VFIO_DEVICE_PCI_HOT_RESET is
called directly on a failed device FD?
When host recovery is running on an unopened sibling, or on a sibling that
was closed during recovery (where vfio_pci_core_close_device() clears
device_open and pci_recovery_flags while keeping pci_recovery_host_active
set), vfio_pci_prepare_hot_reset_blocked() sees !vdev->device_open and
allows userspace to trigger pci_reset_bus() concurrently with host
recovery. In addition, unlike vfio_pci_ioctl_reset(),
vfio_pci_ioctl_pci_hot_reset() does not check whether the calling device FD
itself has blocked access, so invoking VFIO_DEVICE_PCI_HOT_RESET on a
device with VFIO_PCI_RECOVERY_FAILED succeeds and resets the bus.
> static int vfio_pci_dev_set_hot_reset(struct vfio_device_set *dev_set,
> struct vfio_pci_group_info *groups,
> struct iommufd_ctx *iommufd_ctx)
> @@ -2849,8 +2891,11 @@ static int vfio_pci_dev_set_hot_reset(struct vfio_device_set *dev_set,
> break;
> }
>
> - vfio_pci_dma_buf_move(vdev, true);
> - vfio_pci_zap_bars(vdev);
> + ret = vfio_pci_access_ops(vdev)->prepare_hot_reset(vdev);
> + if (ret) {
> + up_write(&vdev->memory_lock);
> + break;
> + }
> }
[Severity: Medium]
When a failed sibling passes vfio_pci_prepare_hot_reset_blocked(), does the
subsequent loop in vfio_pci_dev_set_hot_reset() still call
vfio_pci_try_set_power_state(vdev, PCI_D0) directly on that failed device?
drivers/vfio/pci/vfio_pci_core.c:vfio_pci_dev_set_hot_reset() {
...
list_for_each_entry(vdev, &dev_set->device_list, vdev.dev_set_list) {
ret = vfio_pci_try_set_power_state(vdev, PCI_D0);
if (ret)
goto out_unlock_memory;
}
If an open sibling in VFIO_PCI_RECOVERY_FAILED is in a non-D0 state and
inaccessible, vfio_pci_try_set_power_state() fails with -EIO and aborts
the shared reset for the healthy siblings. If the failed sibling was in
PCI_D3hot with vdev->needs_pm_restore set and responds to the transition,
__vfio_pci_set_power_state() loads vdev->pm_save and calls
pci_restore_state(), re-enabling the pre-failure PCI_COMMAND (including
bus mastering and memory decoding) on the failed device before
pci_reset_bus() saves and restores config space.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261112141632.918954-1-skolothumtho@nvidia.com?part=15
^ permalink raw reply [flat|nested] 28+ messages in thread
* [RFC PATCH v3 16/20] vfio/pci: Restore device state from slot_reset()
2026-11-12 14:16 [RFC PATCH v3 00/20] vfio/pci: Handle PCI error recovery and report state to userspace Shameer Kolothum
` (14 preceding siblings ...)
2026-11-12 14:16 ` [RFC PATCH v3 15/20] vfio/pci: Use access ops for SR-IOV configuration and shared reset Shameer Kolothum
@ 2026-11-12 14:16 ` Shameer Kolothum
2026-11-12 14:16 ` [RFC PATCH v3 17/20] vfio/pci: Complete recovery in resume() Shameer Kolothum
` (3 subsequent siblings)
19 siblings, 0 replies; 28+ messages in thread
From: Shameer Kolothum @ 2026-11-12 14:16 UTC (permalink / raw)
To: kvm, linux-pci, linux-kernel
Cc: alex, jgg, kevin.tian, kbusch, michal.winiarski,
satyanarayana.k.v.p, sonangp, ankita, nathanc, mochs,
skolothumtho
A host reset loses device configuration and IRQ state. Add slot_reset()
to restore the configuration saved at open, discard the old PM snapshot
and tear down IRQ configuration for userspace to rebuild. Record RESET
before restoration so failures also report that a reset occurred. Leave
failed devices blocked.
Use the locked PCI power helper because native PCI recovery already
holds pci_bus_sem for reading. Taking it again could deadlock behind a
queued writer.
Preserve NoINTx and recovery masking when restoring saved configuration.
The state saved at open predates the NoINTx correction. Route PM
restoration through access ops while the device is open. Open and close
continue to restore state directly.
Keep the original error_detected-only callback table for Xen PV and EEH-
enabled builds. Xen treats a missing slot_reset callback as recovered
but treats NONE from a callback as failure. EEH also uses callback
presence to decide whether to keep a device. Disabling the new feature
alone would not preserve these decisions.
Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
---
drivers/vfio/pci/vfio_pci_priv.h | 2 +
include/linux/vfio_pci_core.h | 1 +
drivers/vfio/pci/vfio_pci.c | 17 +++++
drivers/vfio/pci/vfio_pci_core.c | 112 ++++++++++++++++++++++++++++++-
4 files changed, 131 insertions(+), 1 deletion(-)
diff --git a/drivers/vfio/pci/vfio_pci_priv.h b/drivers/vfio/pci/vfio_pci_priv.h
index e4f3e4f51180..bb22f0a15416 100644
--- a/drivers/vfio/pci/vfio_pci_priv.h
+++ b/drivers/vfio/pci/vfio_pci_priv.h
@@ -98,6 +98,8 @@ struct vfio_pci_access_ops {
vfio_pci_access_fn fn, void *data);
/* Caller holds memory_lock. */
int (*set_power_state)(struct vfio_pci_core_device *vdev, pci_power_t state);
+ /* Used by PM restoration while the device is open. */
+ void (*restore_config)(struct vfio_pci_core_device *vdev);
/* Caller holds memory_lock. */
void (*restore_dmabufs)(struct vfio_pci_core_device *vdev);
/* Caller holds the PCI device lock. The device need not be open. */
diff --git a/include/linux/vfio_pci_core.h b/include/linux/vfio_pci_core.h
index fadbbaa58770..d36bca65d180 100644
--- a/include/linux/vfio_pci_core.h
+++ b/include/linux/vfio_pci_core.h
@@ -288,6 +288,7 @@ int vfio_pci_dma_buf_iommufd_map(struct dma_buf_attachment *attachment,
int vfio_pci_core_runtime_suspend(struct vfio_pci_core_device *vdev);
int vfio_pci_core_runtime_resume(struct vfio_pci_core_device *vdev);
pci_ers_result_t vfio_pci_core_aer_err_detected(struct vfio_pci_core_device *vdev);
+pci_ers_result_t vfio_pci_core_aer_slot_reset(struct vfio_pci_core_device *vdev);
/*
* Per-driver PM/AER trampolines. driver_data points to the driver's private
diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
index e7c012febe91..b4b3351cbd85 100644
--- a/drivers/vfio/pci/vfio_pci.c
+++ b/drivers/vfio/pci/vfio_pci.c
@@ -24,6 +24,7 @@
#include <linux/slab.h>
#include <linux/types.h>
#include <linux/uaccess.h>
+#include <xen/xen.h>
#include "vfio_pci_priv.h"
@@ -249,8 +250,20 @@ static pci_ers_result_t vfio_pci_aer_err_detected(struct pci_dev *pdev,
return vfio_pci_core_aer_err_detected(vdev);
}
+static pci_ers_result_t vfio_pci_aer_slot_reset(struct pci_dev *pdev)
+{
+ struct vfio_pci_core_device *vdev = pci_get_drvdata(pdev);
+
+ return vfio_pci_core_aer_slot_reset(vdev);
+}
+
+static const struct pci_error_handlers vfio_pci_legacy_err_handlers = {
+ .error_detected = vfio_pci_aer_err_detected,
+};
+
static const struct pci_error_handlers vfio_pci_err_handlers = {
.error_detected = vfio_pci_aer_err_detected,
+ .slot_reset = vfio_pci_aer_slot_reset,
};
static struct pci_driver vfio_pci_driver = {
@@ -309,6 +322,10 @@ static int __init vfio_pci_init(void)
{
int ret;
+ /* Xen PV and EEH check for callbacks. Keep the original table. */
+ if (xen_pv_domain() || IS_ENABLED(CONFIG_EEH))
+ vfio_pci_driver.err_handler = &vfio_pci_legacy_err_handlers;
+
/* Register and scan for devices */
ret = pci_register_driver(&vfio_pci_driver);
if (ret)
diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
index 0ef51810254e..039fd864119b 100644
--- a/drivers/vfio/pci/vfio_pci_core.c
+++ b/drivers/vfio/pci/vfio_pci_core.c
@@ -312,6 +312,16 @@ static void vfio_pci_probe_power_state(struct vfio_pci_core_device *vdev)
vdev->needs_pm_restore = !(pmcsr & PCI_PM_CTRL_NO_SOFT_RESET);
}
+/* Keep NoINTx and recovery INTx masking when loading saved state. */
+static void vfio_pci_recovery_mask_saved_intx(struct vfio_pci_core_device *vdev)
+{
+ if (vdev->nointx ||
+ (vdev->pci_2_3 &&
+ READ_ONCE(vdev->irq_type) == VFIO_PCI_INTX_IRQ_INDEX))
+ vdev->pdev->saved_config_space[PCI_COMMAND / 4] |=
+ PCI_COMMAND_INTX_DISABLE;
+}
+
static int vfio_pci_sriov_configure_live(struct vfio_pci_core_device *vdev,
int nr_virtfn);
static int vfio_pci_prepare_hot_reset(struct vfio_pci_core_device *vdev);
@@ -329,6 +339,11 @@ static long vfio_pci_access_blocked(struct vfio_pci_core_device *vdev,
return -EIO;
}
+static void vfio_pci_restore_config(struct vfio_pci_core_device *vdev)
+{
+ pci_restore_state(vdev->pdev);
+}
+
static int vfio_pci_power_blocked(struct vfio_pci_core_device *vdev,
pci_power_t state)
{
@@ -339,6 +354,12 @@ static void vfio_pci_restore_dmabufs_blocked(struct vfio_pci_core_device *vdev)
{
}
+static void vfio_pci_restore_config_blocked(struct vfio_pci_core_device *vdev)
+{
+ vfio_pci_recovery_mask_saved_intx(vdev);
+ pci_restore_state(vdev->pdev);
+}
+
static int vfio_pci_sriov_configure_blocked(struct vfio_pci_core_device *vdev,
int nr_virtfn)
{
@@ -351,6 +372,7 @@ static int vfio_pci_sriov_configure_blocked(struct vfio_pci_core_device *vdev,
static const struct vfio_pci_access_ops vfio_pci_live_ops = {
.execute = vfio_pci_access_live,
.set_power_state = vfio_pci_try_set_power_state,
+ .restore_config = vfio_pci_restore_config,
.restore_dmabufs = vfio_pci_dma_buf_restore,
.sriov_configure = vfio_pci_sriov_configure_live,
.prepare_hot_reset = vfio_pci_prepare_hot_reset,
@@ -361,6 +383,7 @@ static const struct vfio_pci_access_ops vfio_pci_live_ops = {
static const struct vfio_pci_access_ops vfio_pci_blocked_ops = {
.execute = vfio_pci_access_blocked,
.set_power_state = vfio_pci_power_blocked,
+ .restore_config = vfio_pci_restore_config_blocked,
.restore_dmabufs = vfio_pci_restore_dmabufs_blocked,
.sriov_configure = vfio_pci_sriov_configure_blocked,
.prepare_hot_reset = vfio_pci_prepare_hot_reset_blocked,
@@ -476,7 +499,11 @@ static int __vfio_pci_set_power_state(struct vfio_pci_core_device *vdev,
vdev->pm_save = pci_store_saved_state(pdev);
} else if (needs_restore) {
pci_load_and_free_saved_state(pdev, &vdev->pm_save);
- pci_restore_state(pdev);
+ /* Open and close restore state without reopening access. */
+ if (!READ_ONCE(vdev->device_open))
+ pci_restore_state(pdev);
+ else
+ vfio_pci_access_ops(vdev)->restore_config(vdev);
}
}
@@ -2640,6 +2667,89 @@ pci_ers_result_t vfio_pci_core_aer_err_detected(struct vfio_pci_core_device *vde
}
EXPORT_SYMBOL_GPL(vfio_pci_core_aer_err_detected);
+/*
+ * The caller holds memory_lock. Discard the PM state saved before reset.
+ * Supported native PCI recovery paths already hold pci_bus_sem for reading.
+ * Taking it again could deadlock behind a queued writer.
+ */
+static int vfio_pci_recovery_restore_state(struct vfio_pci_core_device *vdev)
+{
+ struct pci_dev *pdev = vdev->pdev;
+ int ret;
+
+ if (!vdev->pci_saved_state)
+ return -ENODATA;
+ ret = pci_load_saved_state(pdev, vdev->pci_saved_state);
+ if (ret)
+ return ret;
+
+ kfree(vdev->pm_save);
+ vdev->pm_save = NULL;
+ ret = pci_set_power_state_locked(pdev, PCI_D0);
+ if (ret)
+ return ret;
+
+ vfio_pci_recovery_mask_saved_intx(vdev);
+ pci_restore_state(pdev);
+ return 0;
+}
+
+pci_ers_result_t vfio_pci_core_aer_slot_reset(struct vfio_pci_core_device *vdev)
+{
+ int ret = 0;
+
+ mutex_lock(&vdev->access_lock);
+ if (!(vdev->pci_recovery_flags & VFIO_PCI_RECOVERY_IN_PROGRESS) ||
+ !vdev->device_open) {
+ mutex_unlock(&vdev->access_lock);
+ return PCI_ERS_RESULT_NONE;
+ }
+
+ /* The host reset completed even if restoring the device fails. */
+ WRITE_ONCE(vdev->pci_recovery_flags,
+ vdev->pci_recovery_flags | VFIO_PCI_RECOVERY_RESET);
+
+ /*
+ * Load the configuration saved at open before entering D0, which may
+ * restore BARs from that state. IRQ teardown needs the BARs restored so
+ * MSI-X shutdown can access its table after the host reset.
+ */
+ down_write(&vdev->memory_lock);
+ ret = vfio_pci_recovery_restore_state(vdev);
+ up_write(&vdev->memory_lock);
+ if (ret)
+ goto out_failed;
+
+ /*
+ * Close clears device_open under access_lock before tearing down IRQs,
+ * so this callback cannot race with that teardown.
+ */
+ mutex_lock(&vdev->igate);
+ if (vdev->irq_type < VFIO_PCI_NUM_IRQS)
+ ret = vfio_pci_set_irqs_ioctl(vdev,
+ VFIO_IRQ_SET_DATA_NONE |
+ VFIO_IRQ_SET_ACTION_TRIGGER,
+ vdev->irq_type, 0, 0, NULL);
+ mutex_unlock(&vdev->igate);
+ if (ret)
+ goto out_failed;
+
+ mutex_unlock(&vdev->access_lock);
+
+ return PCI_ERS_RESULT_RECOVERED;
+
+out_failed:
+ WRITE_ONCE(vdev->pci_recovery_flags,
+ (vdev->pci_recovery_flags | VFIO_PCI_RECOVERY_FAILED) &
+ ~VFIO_PCI_RECOVERY_IN_PROGRESS);
+ vdev->pci_recovery_command_valid = false;
+ mutex_unlock(&vdev->access_lock);
+
+ /* Allow recovery of other devices under the bridge to continue. */
+ return PCI_ERS_RESULT_NONE;
+}
+EXPORT_SYMBOL_GPL(vfio_pci_core_aer_slot_reset);
+
static int vfio_pci_sriov_configure_live(struct vfio_pci_core_device *vdev,
int nr_virtfn)
{
--
2.43.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [RFC PATCH v3 17/20] vfio/pci: Complete recovery in resume()
2026-11-12 14:16 [RFC PATCH v3 00/20] vfio/pci: Handle PCI error recovery and report state to userspace Shameer Kolothum
` (15 preceding siblings ...)
2026-11-12 14:16 ` [RFC PATCH v3 16/20] vfio/pci: Restore device state from slot_reset() Shameer Kolothum
@ 2026-11-12 14:16 ` Shameer Kolothum
2026-11-12 14:16 ` [RFC PATCH v3 18/20] vfio/pci: Block device access during host recovery Shameer Kolothum
` (2 subsequent siblings)
19 siblings, 0 replies; 28+ messages in thread
From: Shameer Kolothum @ 2026-11-12 14:16 UTC (permalink / raw)
To: kvm, linux-pci, linux-kernel
Cc: alex, jgg, kevin.tian, kbusch, michal.winiarski,
satyanarayana.k.v.p, sonangp, ankita, nathanc, mochs,
skolothumtho
Add resume() to finish recovery and allow device access again. This also
handles recovery that did not require a reset.
Complete deferred D0 requests and restore PCI_COMMAND, DMA-BUF mappings
and INTx state before installing live ops. Take igate before
memory_lock, matching IRQ configuration, and hold both through
restoration and live-ops installation so no deferred D0 request is
missed.
Use the locked PCI power helper because native PCI recovery already
holds pci_bus_sem for reading. Leave failed devices blocked.
Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
---
include/linux/vfio_pci_core.h | 3 ++
drivers/vfio/pci/vfio_pci.c | 8 +++
drivers/vfio/pci/vfio_pci_core.c | 87 ++++++++++++++++++++++++++++++++
3 files changed, 98 insertions(+)
diff --git a/include/linux/vfio_pci_core.h b/include/linux/vfio_pci_core.h
index d36bca65d180..a7a87c04ee60 100644
--- a/include/linux/vfio_pci_core.h
+++ b/include/linux/vfio_pci_core.h
@@ -147,6 +147,8 @@ struct vfio_pci_core_device {
bool sriov_active;
struct pci_saved_state *pci_saved_state;
struct pci_saved_state *pm_save;
+ /* Deferred D0 request, protected by memory_lock. */
+ bool power_up_pending;
int ioeventfds_nr;
struct vfio_pci_eventfd __rcu *err_trigger;
struct vfio_pci_eventfd __rcu *req_trigger;
@@ -289,6 +291,7 @@ int vfio_pci_core_runtime_suspend(struct vfio_pci_core_device *vdev);
int vfio_pci_core_runtime_resume(struct vfio_pci_core_device *vdev);
pci_ers_result_t vfio_pci_core_aer_err_detected(struct vfio_pci_core_device *vdev);
pci_ers_result_t vfio_pci_core_aer_slot_reset(struct vfio_pci_core_device *vdev);
+void vfio_pci_core_aer_resume(struct vfio_pci_core_device *vdev);
/*
* Per-driver PM/AER trampolines. driver_data points to the driver's private
diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
index b4b3351cbd85..13f4f1faf90d 100644
--- a/drivers/vfio/pci/vfio_pci.c
+++ b/drivers/vfio/pci/vfio_pci.c
@@ -257,6 +257,13 @@ static pci_ers_result_t vfio_pci_aer_slot_reset(struct pci_dev *pdev)
return vfio_pci_core_aer_slot_reset(vdev);
}
+static void vfio_pci_aer_resume(struct pci_dev *pdev)
+{
+ struct vfio_pci_core_device *vdev = pci_get_drvdata(pdev);
+
+ vfio_pci_core_aer_resume(vdev);
+}
+
static const struct pci_error_handlers vfio_pci_legacy_err_handlers = {
.error_detected = vfio_pci_aer_err_detected,
};
@@ -264,6 +271,7 @@ static const struct pci_error_handlers vfio_pci_legacy_err_handlers = {
static const struct pci_error_handlers vfio_pci_err_handlers = {
.error_detected = vfio_pci_aer_err_detected,
.slot_reset = vfio_pci_aer_slot_reset,
+ .resume = vfio_pci_aer_resume,
};
static struct pci_driver vfio_pci_driver = {
diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
index 039fd864119b..09923a48a547 100644
--- a/drivers/vfio/pci/vfio_pci_core.c
+++ b/drivers/vfio/pci/vfio_pci_core.c
@@ -515,6 +515,13 @@ int vfio_pci_set_power_state(struct vfio_pci_core_device *vdev, pci_power_t stat
return __vfio_pci_set_power_state(vdev, state, pci_set_power_state);
}
+static int
+vfio_pci_set_power_state_locked(struct vfio_pci_core_device *vdev,
+ pci_power_t state)
+{
+ return __vfio_pci_set_power_state(vdev, state, pci_set_power_state_locked);
+}
+
/* Avoid waiting for pci_bus_sem under locks needed by PCI recovery. */
int vfio_pci_try_set_power_state(struct vfio_pci_core_device *vdev,
pci_power_t state)
@@ -2750,6 +2757,86 @@ pci_ers_result_t vfio_pci_core_aer_slot_reset(struct vfio_pci_core_device *vdev)
}
EXPORT_SYMBOL_GPL(vfio_pci_core_aer_slot_reset);
+void vfio_pci_core_aer_resume(struct vfio_pci_core_device *vdev)
+{
+ struct pci_dev *pdev = vdev->pdev;
+ unsigned long irq_flags;
+ bool notify_intx = false;
+ bool power_up;
+ u32 flags;
+ int ret = 0;
+
+ mutex_lock(&vdev->access_lock);
+ vdev->pci_recovery_host_active = false;
+ if (!(vdev->pci_recovery_flags & VFIO_PCI_RECOVERY_IN_PROGRESS))
+ goto out_unlock;
+
+ /*
+ * Match IRQ configuration's lock order and keep D0 requests out until
+ * live ops are installed, so no deferred request can be left behind.
+ */
+ mutex_lock(&vdev->igate);
+ down_write(&vdev->memory_lock);
+ /*
+ * Complete a deferred D0 request before restoring PCI_COMMAND.
+ * After a reset, slot_reset() has already put the device in D0.
+ */
+ power_up = vdev->power_up_pending;
+ vdev->power_up_pending = false;
+ if (power_up && !(vdev->pci_recovery_flags & VFIO_PCI_RECOVERY_RESET)) {
+ ret = vfio_pci_set_power_state_locked(vdev, PCI_D0);
+ if (ret)
+ goto out_memory;
+ }
+
+ /*
+ * Hold irqlock while updating PCI_COMMAND, as the INTx handler does.
+ * Keep masked INTx disabled until vfio_pci_intx_recovery_finish().
+ */
+ spin_lock_irqsave(&vdev->irqlock, irq_flags);
+ if (!(vdev->pci_recovery_flags & VFIO_PCI_RECOVERY_RESET) &&
+ vdev->pci_recovery_command_valid) {
+ u16 cmd = vdev->pci_recovery_command;
+
+ cmd = vfio_pci_intx_recovery_update_command(vdev, cmd);
+ ret = pci_write_config_word(pdev, PCI_COMMAND, cmd);
+ }
+ spin_unlock_irqrestore(&vdev->irqlock, irq_flags);
+ if (ret)
+ goto out_memory;
+
+ /*
+ * Restoring saved state may leave ROM decoding enabled. Disable it
+ * before allowing access to shared decoders.
+ */
+ if (pci_resource_start(pdev, PCI_ROM_RESOURCE) &&
+ !(pdev->resource[PCI_ROM_RESOURCE].flags & IORESOURCE_ROM_ENABLE))
+ pci_disable_rom(pdev);
+
+ /* Recovery restores mappings while ordinary access still uses stubs. */
+ if (__vfio_pci_memory_enabled(vdev))
+ vfio_pci_dma_buf_restore(vdev);
+
+out_memory:
+ vdev->pci_recovery_command_valid = false;
+ flags = vdev->pci_recovery_flags & ~VFIO_PCI_RECOVERY_IN_PROGRESS;
+ if (ret) {
+ WRITE_ONCE(vdev->pci_recovery_flags,
+ flags | VFIO_PCI_RECOVERY_FAILED);
+ } else {
+ WRITE_ONCE(vdev->pci_recovery_flags, flags);
+ notify_intx = vfio_pci_intx_recovery_finish(vdev);
+ }
+ up_write(&vdev->memory_lock);
+ if (notify_intx)
+ vfio_pci_intx_recovery_signal(vdev);
+ mutex_unlock(&vdev->igate);
+
+out_unlock:
+ mutex_unlock(&vdev->access_lock);
+}
+EXPORT_SYMBOL_GPL(vfio_pci_core_aer_resume);
+
static int vfio_pci_sriov_configure_live(struct vfio_pci_core_device *vdev,
int nr_virtfn)
{
--
2.43.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [RFC PATCH v3 18/20] vfio/pci: Block device access during host recovery
2026-11-12 14:16 [RFC PATCH v3 00/20] vfio/pci: Handle PCI error recovery and report state to userspace Shameer Kolothum
` (16 preceding siblings ...)
2026-11-12 14:16 ` [RFC PATCH v3 17/20] vfio/pci: Complete recovery in resume() Shameer Kolothum
@ 2026-11-12 14:16 ` Shameer Kolothum
2026-11-12 15:06 ` sashiko-bot
2026-11-12 14:16 ` [RFC PATCH v3 19/20] vfio/pci: Add VFIO_DEVICE_FEATURE_PCI_ERROR_RECOVERY Shameer Kolothum
2026-11-12 14:16 ` [RFC PATCH v3 20/20] vfio/pci: Enable recovery for supported native PCIe endpoints Shameer Kolothum
19 siblings, 1 reply; 28+ messages in thread
From: Shameer Kolothum @ 2026-11-12 14:16 UTC (permalink / raw)
To: kvm, linux-pci, linux-kernel
Cc: alex, jgg, kevin.tian, kbusch, michal.winiarski,
satyanarayana.k.v.p, sonangp, ankita, nathanc, mochs,
skolothumtho
An error notification does not stop userspace or asynchronous VFIO work
from accessing a device while the host is recovering it.
For an open device with recovery enabled, install blocked ops in
error_detected() and drain existing SRCU readers. Block INTx unmasking,
remove BAR mappings and revoke DMA-BUF mappings. Disable bus mastering
on a normal channel. Keep shared-line INTx masking available.
Hold memory_lock and irqlock when publishing blocked ops so an earlier
PM restore cannot undo the blocked handler's INTx mask. Release both
locks before draining SRCU, since readers may need memory_lock to
finish. Use the same transition during close.
Defer blocked D0 requests while the device is open and reject them once
close begins. Reject low-power requests while blocked. Check for ongoing
host recovery before enabling a device.
Return CAN_RECOVER for a normal channel, NEED_RESET for a frozen channel
and DISCONNECT for permanent failure. If saving PCI_COMMAND or disabling
bus mastering fails, keep access blocked and return NONE so other
devices can continue recovery. Preserve existing error eventfd
notifications.
Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
---
include/linux/vfio_pci_core.h | 5 +-
drivers/vfio/pci/vfio_pci.c | 2 +-
drivers/vfio/pci/vfio_pci_core.c | 156 ++++++++++++++++++++++++++++++-
3 files changed, 157 insertions(+), 6 deletions(-)
diff --git a/include/linux/vfio_pci_core.h b/include/linux/vfio_pci_core.h
index a7a87c04ee60..cd7d87788de0 100644
--- a/include/linux/vfio_pci_core.h
+++ b/include/linux/vfio_pci_core.h
@@ -289,7 +289,8 @@ int vfio_pci_dma_buf_iommufd_map(struct dma_buf_attachment *attachment,
int vfio_pci_core_runtime_suspend(struct vfio_pci_core_device *vdev);
int vfio_pci_core_runtime_resume(struct vfio_pci_core_device *vdev);
-pci_ers_result_t vfio_pci_core_aer_err_detected(struct vfio_pci_core_device *vdev);
+pci_ers_result_t vfio_pci_core_aer_err_detected(struct vfio_pci_core_device *vdev,
+ pci_channel_state_t state);
pci_ers_result_t vfio_pci_core_aer_slot_reset(struct vfio_pci_core_device *vdev);
void vfio_pci_core_aer_resume(struct vfio_pci_core_device *vdev);
@@ -322,7 +323,7 @@ static pci_ers_result_t name##_aer_err_detected(struct pci_dev *pdev, \
{ \
drv_type *drvdata = pci_get_drvdata(pdev); \
\
- return vfio_pci_core_aer_err_detected(&drvdata->member); \
+ return vfio_pci_core_aer_err_detected(&drvdata->member, state); \
}
#endif /* VFIO_PCI_CORE_H */
diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
index 13f4f1faf90d..a5ba4bb11638 100644
--- a/drivers/vfio/pci/vfio_pci.c
+++ b/drivers/vfio/pci/vfio_pci.c
@@ -247,7 +247,7 @@ static pci_ers_result_t vfio_pci_aer_err_detected(struct pci_dev *pdev,
{
struct vfio_pci_core_device *vdev = pci_get_drvdata(pdev);
- return vfio_pci_core_aer_err_detected(vdev);
+ return vfio_pci_core_aer_err_detected(vdev, state);
}
static pci_ers_result_t vfio_pci_aer_slot_reset(struct pci_dev *pdev)
diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
index 09923a48a547..5e9451e08263 100644
--- a/drivers/vfio/pci/vfio_pci_core.c
+++ b/drivers/vfio/pci/vfio_pci_core.c
@@ -347,7 +347,11 @@ static void vfio_pci_restore_config(struct vfio_pci_core_device *vdev)
static int vfio_pci_power_blocked(struct vfio_pci_core_device *vdev,
pci_power_t state)
{
- return -EIO;
+ lockdep_assert_held_write(&vdev->memory_lock);
+ if (state != PCI_D0 || !READ_ONCE(vdev->device_open))
+ return -EIO;
+ vdev->power_up_pending = true;
+ return 0;
}
static void vfio_pci_restore_dmabufs_blocked(struct vfio_pci_core_device *vdev)
@@ -410,10 +414,17 @@ static void vfio_pci_core_block_access(struct vfio_pci_core_device *vdev)
unsigned long flags;
lockdep_assert_held(&vdev->access_lock);
+ /*
+ * Finish live PM restoration before blocked INTx handlers can mask the
+ * device. Otherwise an old restore can undo their hardware mask.
+ */
+ down_write(&vdev->memory_lock);
spin_lock_irqsave(&vdev->irqlock, flags);
rcu_assign_pointer(vdev->access_ops, &vfio_pci_blocked_ops);
spin_unlock_irqrestore(&vdev->irqlock, flags);
+ up_write(&vdev->memory_lock);
+ /* SRCU readers may need memory_lock to finish. */
synchronize_srcu(&vdev->access_srcu);
}
@@ -756,6 +767,16 @@ int vfio_pci_core_enable(struct vfio_pci_core_device *vdev)
u16 cmd;
u8 msix_pos;
+ if (vdev->pci_recovery_supported) {
+ if (pci_dev_is_disconnected(pdev))
+ return -ENODEV;
+ mutex_lock(&vdev->access_lock);
+ ret = vdev->pci_recovery_host_active ? -EBUSY : 0;
+ mutex_unlock(&vdev->access_lock);
+ if (ret)
+ return ret;
+ }
+
if (!vdev->disable_idle_d3) {
ret = pm_runtime_resume_and_get(&pdev->dev);
if (ret < 0)
@@ -999,6 +1020,10 @@ void vfio_pci_core_close_device(struct vfio_device *core_vdev)
WRITE_ONCE(vdev->device_open, false);
vfio_pci_core_block_access(vdev);
WRITE_ONCE(vdev->pci_recovery_flags, 0);
+ /* Discard pending D0 before the next open. */
+ down_write(&vdev->memory_lock);
+ vdev->power_up_pending = false;
+ up_write(&vdev->memory_lock);
}
}
@@ -2660,17 +2685,142 @@ void vfio_pci_core_unregister_device(struct vfio_pci_core_device *vdev)
}
EXPORT_SYMBOL_GPL(vfio_pci_core_unregister_device);
-pci_ers_result_t vfio_pci_core_aer_err_detected(struct vfio_pci_core_device *vdev)
+/*
+ * Save PCI_COMMAND, clear bus mastering and set the requested bits.
+ * Reject an all-ones read from an inaccessible device.
+ */
+static int
+vfio_pci_recovery_save_and_clear_master(struct vfio_pci_core_device *vdev, u16 set)
+{
+ struct pci_dev *pdev = vdev->pdev;
+ u16 command;
+ int ret;
+
+ ret = pci_read_config_word(pdev, PCI_COMMAND, &vdev->pci_recovery_command);
+ if (ret)
+ return ret;
+ if (PCI_POSSIBLE_ERROR(vdev->pci_recovery_command))
+ return -EIO;
+
+ command = (vdev->pci_recovery_command & ~PCI_COMMAND_MASTER) | set;
+ ret = pci_write_config_word(pdev, PCI_COMMAND, command);
+ if (ret)
+ return ret;
+
+ vdev->pci_recovery_command_valid = true;
+ return 0;
+}
+
+pci_ers_result_t vfio_pci_core_aer_err_detected(struct vfio_pci_core_device *vdev,
+ pci_channel_state_t state)
{
struct vfio_pci_eventfd *eventfd;
+ pci_ers_result_t result = PCI_ERS_RESULT_CAN_RECOVER;
+ unsigned long irq_flags;
+ bool nested;
+ u32 flags;
+ int ret = 0;
+
+ if (!vdev->pci_recovery_supported)
+ goto out;
+
+ mutex_lock(&vdev->access_lock);
+ /*
+ * Track host recovery even without userspace opt-in so open can reject
+ * attempts while recovery is running.
+ */
+ vdev->pci_recovery_host_active =
+ state != pci_channel_io_perm_failure;
+ if (!vdev->pci_recovery_enabled)
+ goto out_unlock;
+
+ /* Keep failed devices blocked until close. */
+ if (vdev->pci_recovery_flags & VFIO_PCI_RECOVERY_FAILED) {
+ result = PCI_ERS_RESULT_NONE;
+ goto out_unlock;
+ }
+
+ if (!vdev->device_open) {
+ result = PCI_ERS_RESULT_NONE;
+ goto out_unlock;
+ }
+
+ vfio_pci_core_block_access(vdev);
+ /*
+ * Keep the original command register for nested events. Recovery may
+ * already have disabled bus mastering in the current value.
+ */
+ nested = vdev->pci_recovery_flags & VFIO_PCI_RECOVERY_IN_PROGRESS;
+ if (!nested)
+ vdev->pci_recovery_command_valid = false;
+ vfio_pci_intx_recovery_start(vdev);
+ vfio_pci_zap_and_down_write_memory_lock(vdev);
+ /*
+ * Hold irqlock while updating PCI_COMMAND to protect against the INTx
+ * handler, which does not use access_srcu.
+ */
+ spin_lock_irqsave(&vdev->irqlock, irq_flags);
+ if (state == pci_channel_io_normal && vdev->pci_2_3 && !nested)
+ ret = vfio_pci_recovery_save_and_clear_master(vdev,
+ PCI_COMMAND_INTX_DISABLE);
+ spin_unlock_irqrestore(&vdev->irqlock, irq_flags);
+ vfio_pci_dma_buf_move(vdev, true);
+
+ /*
+ * Advance the sequence unless recovery is already running. Write the
+ * flags in one store so readers without the lock cannot see them briefly
+ * cleared between the old and new values.
+ */
+ if (nested) {
+ flags = vdev->pci_recovery_flags;
+ } else {
+ if (++vdev->pci_recovery_sequence == 0)
+ vdev->pci_recovery_sequence++;
+ flags = 0;
+ }
+
+ if (state == pci_channel_io_perm_failure) {
+ WRITE_ONCE(vdev->pci_recovery_flags,
+ (flags | VFIO_PCI_RECOVERY_FAILED) &
+ ~VFIO_PCI_RECOVERY_IN_PROGRESS);
+ vdev->pci_recovery_command_valid = false;
+ result = PCI_ERS_RESULT_DISCONNECT;
+ goto out_memory;
+ }
+
+ if (state == pci_channel_io_frozen) {
+ WRITE_ONCE(vdev->pci_recovery_flags,
+ flags | VFIO_PCI_RECOVERY_IN_PROGRESS |
+ VFIO_PCI_RECOVERY_FROZEN);
+ result = PCI_ERS_RESULT_NEED_RESET;
+ goto out_memory;
+ }
+
+ if (!ret && !vdev->pci_2_3 && !nested)
+ ret = vfio_pci_recovery_save_and_clear_master(vdev, 0);
+ if (ret) {
+ flags |= VFIO_PCI_RECOVERY_FAILED;
+ flags &= ~VFIO_PCI_RECOVERY_IN_PROGRESS;
+ result = PCI_ERS_RESULT_NONE;
+ } else {
+ flags |= VFIO_PCI_RECOVERY_IN_PROGRESS;
+ }
+ WRITE_ONCE(vdev->pci_recovery_flags, flags);
+
+out_memory:
+ up_write(&vdev->memory_lock);
+out_unlock:
+ mutex_unlock(&vdev->access_lock);
+
+out:
rcu_read_lock();
eventfd = rcu_dereference(vdev->err_trigger);
if (eventfd)
eventfd_signal(eventfd->ctx);
rcu_read_unlock();
- return PCI_ERS_RESULT_CAN_RECOVER;
+ return result;
}
EXPORT_SYMBOL_GPL(vfio_pci_core_aer_err_detected);
--
2.43.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [RFC PATCH v3 18/20] vfio/pci: Block device access during host recovery
2026-11-12 14:16 ` [RFC PATCH v3 18/20] vfio/pci: Block device access during host recovery Shameer Kolothum
@ 2026-11-12 15:06 ` sashiko-bot
0 siblings, 0 replies; 28+ messages in thread
From: sashiko-bot @ 2026-11-12 15:06 UTC (permalink / raw)
To: Shameer Kolothum, kvm, linux-pci
Thank you for your contribution! Sashiko AI review found 5 potential
issues to consider:
- [High] vfio/pci: unrestored BARs after bus reset when recovery fails
in err_detected()
- [Medium] vfio/pci: TOCTOU race on pci_recovery_host_active during
device open
- [Medium] vfio/pci: close during host recovery races with bus reset and
loses saved state
- [Medium] vfio/pci: close clears device_open before blocking PM config
restore
- [Low] vfio/pci: dead !device_open check in
vfio_pci_core_aer_err_detected()
--
commit f9997001c74d4f1e906c8a9ec79cb059867865ef
Author: Shameer Kolothum <skolothumtho@nvidia.com>
vfio/pci: Block device access during host recovery
This commit blocks userspace and asynchronous VFIO device access during
host PCI error recovery in vfio_pci_core_aer_err_detected(), coordinates
blocked power-state and INTx handling with memory_lock and irqlock, and
rejects device open while host recovery is active.
> diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
> index 09923a48a5470..5e9451e082630 100644
> --- a/drivers/vfio/pci/vfio_pci_core.c
> +++ b/drivers/vfio/pci/vfio_pci_core.c
[ ... ]
> @@ -410,10 +414,17 @@ static void vfio_pci_core_block_access(struct vfio_pci_core_device *vdev)
> unsigned long flags;
>
> lockdep_assert_held(&vdev->access_lock);
> + /*
> + * Finish live PM restoration before blocked INTx handlers can mask the
> + * device. Otherwise an old restore can undo their hardware mask.
> + */
> + down_write(&vdev->memory_lock);
> spin_lock_irqsave(&vdev->irqlock, flags);
> rcu_assign_pointer(vdev->access_ops, &vfio_pci_blocked_ops);
> spin_unlock_irqrestore(&vdev->irqlock, flags);
> + up_write(&vdev->memory_lock);
>
> + /* SRCU readers may need memory_lock to finish. */
> synchronize_srcu(&vdev->access_srcu);
> }
[Severity: Medium]
Does clearing vdev->device_open before calling vfio_pci_core_block_access()
in vfio_pci_core_close_device() defeat this protection against PM config
restoration unmasking INTx?
In __vfio_pci_set_power_state(), D3hot-to-D0 restoration bypasses
access_ops->restore_config() as soon as vdev->device_open is false:
drivers/vfio/pci/vfio_pci_core.c:__vfio_pci_set_power_state() {
...
} else if (needs_restore) {
pci_load_and_free_saved_state(pdev, &vdev->pm_save);
/* Open and close restore state without reopening access. */
if (!READ_ONCE(vdev->device_open))
pci_restore_state(pdev);
else
vfio_pci_access_ops(vdev)->restore_config(vdev);
}
...
}
If a NoSoftRst- PCI 2.3 device entered D3hot while INTx was unmasked
(saving unmasked PCI_COMMAND in vdev->pm_save) and then had INTx masked by
recovery, both an in-flight runtime PM transition holding memory_lock when
vfio_pci_core_close_device() clears vdev->device_open and the subsequent
vfio_pci_set_power_state(vdev, PCI_D0) at the start of
vfio_pci_core_disable() take the pci_restore_state(pdev) branch. That
restores the unmasked PCI_COMMAND in hardware while ctx->masked is still
true and the IRQ handler is still registered, so a level-triggered INTx
assertion before vfio_pci_set_irqs_ioctl() runs sees ctx->masked == true in
vfio_pci_intx_mask_for_recovery() and returns IRQ_NONE without masking the
shared interrupt line.
> @@ -756,6 +767,16 @@ int vfio_pci_core_enable(struct vfio_pci_core_device *vdev)
> u16 cmd;
> u8 msix_pos;
>
> + if (vdev->pci_recovery_supported) {
> + if (pci_dev_is_disconnected(pdev))
> + return -ENODEV;
> + mutex_lock(&vdev->access_lock);
> + ret = vdev->pci_recovery_host_active ? -EBUSY : 0;
> + mutex_unlock(&vdev->access_lock);
> + if (ret)
> + return ret;
> + }
> +
> if (!vdev->disable_idle_d3) {
[Severity: Medium]
Can host recovery start after vfio_pci_core_enable() drops
vdev->access_lock and still allow vfio_pci_core_finish_enable() to publish
live access?
CPU0 CPU1
vfio_pci_core_enable()
mutex_lock(&vdev->access_lock)
/* pci_recovery_host_active is false */
mutex_unlock(&vdev->access_lock)
pcie_do_recovery()
vfio_pci_core_aer_err_detected()
mutex_lock(&vdev->access_lock)
vdev->pci_recovery_host_active = true
/* !pci_recovery_enabled */
mutex_unlock(&vdev->access_lock)
reset_subordinates()
pci_save_state(pdev)
vfio_config_init(vdev)
vfio_pci_core_finish_enable()
mutex_lock(&vdev->access_lock)
WRITE_ONCE(vdev->device_open, true)
rcu_assign_pointer(vdev->access_ops,
&vfio_pci_live_ops) <-- live access while recovery runs
Because vfio_pci_core_enable() runs without device_lock(&pdev->dev) held
across enablement and vfio_pci_core_finish_enable() does not re-check
vdev->pci_recovery_host_active or pci_dev_is_disconnected(pdev), the open
succeeds while host recovery is active, vdev->pci_saved_state and
vdev->rbar[] can capture corrupted or post-reset BAR values, and
vfio_pci_core_aer_slot_reset() skips state restoration because
VFIO_PCI_RECOVERY_IN_PROGRESS was never set.
> @@ -999,6 +1020,10 @@ void vfio_pci_core_close_device(struct vfio_device *core_vdev)
> WRITE_ONCE(vdev->device_open, false);
> vfio_pci_core_block_access(vdev);
> WRITE_ONCE(vdev->pci_recovery_flags, 0);
> + /* Discard pending D0 before the next open. */
> + down_write(&vdev->memory_lock);
> + vdev->power_up_pending = false;
> + up_write(&vdev->memory_lock);
> }
> }
[Severity: Medium]
What happens if userspace closes the device after
vfio_pci_core_aer_err_detected() signals err_trigger, while
pcie_do_recovery() is resetting the bus?
CPU0 CPU1
pcie_do_recovery()
vfio_pci_core_aer_err_detected()
/* returns NEED_RESET */
vfio_pci_core_close_device()
WRITE_ONCE(vdev->device_open, false)
WRITE_ONCE(vdev->pci_recovery_flags, 0)
vfio_pci_core_disable()
pci_load_and_free_saved_state(
pdev, &vdev->pci_saved_state)
pci_restore_state(pdev)
reset_subordinates()
/* resets bus, wiping BARs */
vfio_pci_core_aer_slot_reset()
/* !device_open -> returns NONE */ <-- BARs left zeroed in hardware
pcie_do_recovery() drops device_lock(&pdev->dev) between error_detected()
and slot_reset(). If vfio_pci_core_disable() consumes and frees
vdev->pci_saved_state before reset_subordinates() finishes resetting the
secondary bus without restoring config space, vfio_pci_core_aer_slot_reset()
sees !vdev->device_open and returns early, leaving the hardware BARs zeroed
for the next open. If close instead runs just after reset_subordinates()
wipes the BARs, vfio_pci_core_disable() tears down MSI-X in
vfio_pci_set_irqs_ioctl() before restoring vdev->pci_saved_state.
> @@ -2660,17 +2685,142 @@ void vfio_pci_core_unregister_device(struct vfio_pci_core_device *vdev)
[ ... ]
> + mutex_lock(&vdev->access_lock);
> + /*
> + * Track host recovery even without userspace opt-in so open can reject
> + * attempts while recovery is running.
> + */
> + vdev->pci_recovery_host_active =
> + state != pci_channel_io_perm_failure;
> + if (!vdev->pci_recovery_enabled)
> + goto out_unlock;
> +
> + /* Keep failed devices blocked until close. */
> + if (vdev->pci_recovery_flags & VFIO_PCI_RECOVERY_FAILED) {
> + result = PCI_ERS_RESULT_NONE;
> + goto out_unlock;
> + }
> +
> + if (!vdev->device_open) {
> + result = PCI_ERS_RESULT_NONE;
> + goto out_unlock;
> + }
[Severity: Low]
Can the !vdev->device_open check ever be reached here?
Under vdev->access_lock, vdev->pci_recovery_enabled is only set when
vdev->device_open is true and is cleared alongside vdev->device_open in
vfio_pci_core_close_device(), so !vdev->pci_recovery_enabled always
branches to out_unlock first (returning PCI_ERS_RESULT_CAN_RECOVER rather
than PCI_ERS_RESULT_NONE when the device is closed).
> +
> + vfio_pci_core_block_access(vdev);
[ ... ]
> + if (!ret && !vdev->pci_2_3 && !nested)
> + ret = vfio_pci_recovery_save_and_clear_master(vdev, 0);
>
> + if (ret) {
> + flags |= VFIO_PCI_RECOVERY_FAILED;
> + flags &= ~VFIO_PCI_RECOVERY_IN_PROGRESS;
> + result = PCI_ERS_RESULT_NONE;
> + } else {
> + flags |= VFIO_PCI_RECOVERY_IN_PROGRESS;
> + }
> + WRITE_ONCE(vdev->pci_recovery_flags, flags);
[Severity: High]
If recovery fails in vfio_pci_core_aer_err_detected() (or
VFIO_PCI_RECOVERY_FAILED is already set on a subsequent frozen-channel
error) and pcie_do_recovery() still resets the subordinate bus, will the
device's BARs be left zeroed when vfio_pci_core_disable() later tears down
MSI-X?
Clearing VFIO_PCI_RECOVERY_IN_PROGRESS and returning PCI_ERS_RESULT_NONE
allows pcie_do_recovery() to proceed with reset_subordinates() when a
sibling device requests PCI_ERS_RESULT_NEED_RESET or when state is
pci_channel_io_frozen, wiping this device's BARs in hardware. Because
VFIO_PCI_RECOVERY_IN_PROGRESS is not set, vfio_pci_core_aer_slot_reset()
returns early without calling vfio_pci_recovery_restore_state() or tearing
down IRQs:
drivers/vfio/pci/vfio_pci_core.c:vfio_pci_core_aer_slot_reset() {
mutex_lock(&vdev->access_lock);
if (!(vdev->pci_recovery_flags & VFIO_PCI_RECOVERY_IN_PROGRESS) ||
!vdev->device_open) {
mutex_unlock(&vdev->access_lock);
return PCI_ERS_RESULT_NONE;
}
...
}
When userspace later closes the device, vfio_pci_core_disable() calls
vfio_pci_set_irqs_ioctl() -> vfio_msi_disable() before loading
vdev->pci_saved_state, which enables PCI_COMMAND_MEMORY via
vfio_pci_memory_lock_and_enable() and writes to the MSI-X table while the
hardware BARs are still 0x00000000.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261112141632.918954-1-skolothumtho@nvidia.com?part=18
^ permalink raw reply [flat|nested] 28+ messages in thread
* [RFC PATCH v3 19/20] vfio/pci: Add VFIO_DEVICE_FEATURE_PCI_ERROR_RECOVERY
2026-11-12 14:16 [RFC PATCH v3 00/20] vfio/pci: Handle PCI error recovery and report state to userspace Shameer Kolothum
` (17 preceding siblings ...)
2026-11-12 14:16 ` [RFC PATCH v3 18/20] vfio/pci: Block device access during host recovery Shameer Kolothum
@ 2026-11-12 14:16 ` Shameer Kolothum
2026-11-12 14:16 ` [RFC PATCH v3 20/20] vfio/pci: Enable recovery for supported native PCIe endpoints Shameer Kolothum
19 siblings, 0 replies; 28+ messages in thread
From: Shameer Kolothum @ 2026-11-12 14:16 UTC (permalink / raw)
To: kvm, linux-pci, linux-kernel
Cc: alex, jgg, kevin.tian, kbusch, michal.winiarski,
satyanarayana.k.v.p, sonangp, ankita, nathanc, mochs,
skolothumtho
Userspace needs a way to enable host recovery and find out whether it is
still running, succeeded, reset the device or failed.
Add a device feature that registers a recovery eventfd and reports
status flags and a sequence number. SET with eventfd -1 disables
recovery. GET is available while hardware access is blocked. The
sequence number lets userspace detect recovery that finished before it
read the notification.
Reject SET if access is blocked, recovery has failed or a recorded host
recovery is still running. This also applies if userspace had not
enabled recovery when the event started, so a late subscription cannot
report part of an older event as a new one. Updating the eventfd resets
status for the current open. The existing error eventfd is unchanged.
Add the recovery eventfd and signal it when recovery starts, completes
or fails. Release it during close. Signal under access_lock so SET cannot
replace the eventfd and reset status before notification.
Userspace must probe each device and restore guest state after a reset.
Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
---
include/linux/vfio_pci_core.h | 1 +
include/uapi/linux/vfio.h | 51 ++++++++++++
drivers/vfio/pci/vfio_pci_core.c | 139 +++++++++++++++++++++++++++++++
3 files changed, 191 insertions(+)
diff --git a/include/linux/vfio_pci_core.h b/include/linux/vfio_pci_core.h
index cd7d87788de0..896c176c28a0 100644
--- a/include/linux/vfio_pci_core.h
+++ b/include/linux/vfio_pci_core.h
@@ -152,6 +152,7 @@ struct vfio_pci_core_device {
int ioeventfds_nr;
struct vfio_pci_eventfd __rcu *err_trigger;
struct vfio_pci_eventfd __rcu *req_trigger;
+ struct vfio_pci_eventfd __rcu *pci_recovery_trigger;
struct eventfd_ctx *pm_wake_eventfd_ctx;
struct list_head dummy_resources_list;
struct mutex ioeventfds_lock;
diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
index e41437fa17ad..86a3952ee302 100644
--- a/include/uapi/linux/vfio.h
+++ b/include/uapi/linux/vfio.h
@@ -1555,6 +1555,57 @@ struct vfio_device_feature_zpci_err {
#define VFIO_DEVICE_FEATURE_ZPCI_ERROR 13
+/*
+ * Report host PCI error recovery state for this device.
+ *
+ * Userspace must probe the feature on each device before enabling it.
+ *
+ * VFIO_DEVICE_FEATURE_SET with a valid eventfd enables recovery for this
+ * open and requests notifications when each event starts and ends. SET
+ * with eventfd -1 disables recovery. flags and sequence must be zero. SET
+ * returns -EBUSY during recovery, after recovery failure, or while device
+ * access is blocked, and -ENODEV after close has started.
+ * VFIO_PCI_ERR_IRQ_INDEX notifications are unchanged.
+ *
+ * VFIO_DEVICE_FEATURE_GET returns the current state and eventfd = -1. GET
+ * is allowed during recovery, but may wait for a running callback.
+ *
+ * The sequence number increments for each event and resets to zero after
+ * a successful SET. It lets userspace detect multiple events from a single
+ * notification. Recovery may complete before userspace reads the
+ * notification, so userspace must check the sequence and status rather
+ * than wait to observe IN_PROGRESS.
+ *
+ * Ordinary hardware accesses are blocked during recovery. Read and write
+ * requests fail with -EIO, and faults on BAR mappings result in SIGBUS.
+ *
+ * CHANNEL_FROZEN records a frozen channel. DEVICE_RESET records a host
+ * reset, after which userspace must reconfigure interrupts with
+ * VFIO_DEVICE_SET_IRQS.
+ * FAILED indicates that recovery failed and access remains blocked until
+ * close and reopen. Status bits remain set until the next event or SET.
+ * ENABLED indicates that recovery is enabled.
+ *
+ * SET returns -EBUSY while a host recovery event recorded by the driver is
+ * still running, even if recovery was disabled when it was reported.
+ * Enabling recovery does not report events that have already completed.
+ *
+ * Open returns -EBUSY while a recorded host recovery is still running,
+ * including events reported while recovery was disabled.
+ */
+struct vfio_device_pci_error_recovery {
+ __u32 flags;
+#define VFIO_PCI_ERROR_RECOVERY_IN_PROGRESS (1U << 0)
+#define VFIO_PCI_ERROR_RECOVERY_CHANNEL_FROZEN (1U << 1)
+#define VFIO_PCI_ERROR_RECOVERY_DEVICE_RESET (1U << 2)
+#define VFIO_PCI_ERROR_RECOVERY_FAILED (1U << 3)
+#define VFIO_PCI_ERROR_RECOVERY_ENABLED (1U << 4)
+ __s32 eventfd;
+ __aligned_u64 sequence;
+};
+
+#define VFIO_DEVICE_FEATURE_PCI_ERROR_RECOVERY 14
+
/* -------- API for Type1 VFIO IOMMU -------- */
/**
diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
index 5e9451e08263..0897d1f0e082 100644
--- a/drivers/vfio/pci/vfio_pci_core.c
+++ b/drivers/vfio/pci/vfio_pci_core.c
@@ -1043,6 +1043,10 @@ void vfio_pci_core_close_device(struct vfio_device *core_vdev)
mutex_lock(&vdev->igate);
vfio_pci_eventfd_replace_locked(vdev, &vdev->err_trigger, NULL);
vfio_pci_eventfd_replace_locked(vdev, &vdev->req_trigger, NULL);
+ if (vdev->pci_recovery_supported)
+ vfio_pci_eventfd_replace_locked(vdev,
+ &vdev->pci_recovery_trigger,
+ NULL);
mutex_unlock(&vdev->igate);
}
EXPORT_SYMBOL_GPL(vfio_pci_core_close_device);
@@ -1939,6 +1943,114 @@ static int vfio_pci_core_feature_token(struct vfio_pci_core_device *vdev,
return 0;
}
+/* Consumes the eventfd reference on both success and failure. */
+static int vfio_pci_recovery_set(struct vfio_pci_core_device *vdev,
+ struct eventfd_ctx *ctx, bool enable)
+{
+ int ret;
+
+ mutex_lock(&vdev->access_lock);
+ if (!vdev->device_open) {
+ ret = -ENODEV;
+ goto out_put;
+ }
+ if (rcu_access_pointer(vdev->access_ops) != &vfio_pci_live_ops ||
+ vdev->pci_recovery_host_active) {
+ ret = -EBUSY;
+ goto out_put;
+ }
+ if (!enable &&
+ (vdev->pci_recovery_flags &
+ (VFIO_PCI_RECOVERY_IN_PROGRESS | VFIO_PCI_RECOVERY_FAILED))) {
+ ret = -EBUSY;
+ goto out_put;
+ }
+
+ mutex_lock(&vdev->igate);
+ ret = vfio_pci_eventfd_replace_locked(vdev, &vdev->pci_recovery_trigger,
+ ctx);
+ mutex_unlock(&vdev->igate);
+ if (ret)
+ goto out_put;
+
+ WRITE_ONCE(vdev->pci_recovery_enabled, enable);
+ /*
+ * Updating the recovery eventfd resets status for the current open.
+ * It does not change the access ops or whether host recovery is running.
+ */
+ WRITE_ONCE(vdev->pci_recovery_flags, 0);
+ vdev->pci_recovery_sequence = 0;
+ vdev->pci_recovery_command_valid = false;
+
+ goto out_unlock;
+
+out_put:
+ if (ctx)
+ eventfd_ctx_put(ctx);
+out_unlock:
+ mutex_unlock(&vdev->access_lock);
+ return ret;
+}
+
+static int
+vfio_pci_core_feature_error_recovery(struct vfio_pci_core_device *vdev, u32 flags,
+ struct vfio_device_pci_error_recovery __user *arg,
+ size_t argsz)
+{
+ struct vfio_device_pci_error_recovery state = { .eventfd = -1 };
+ struct eventfd_ctx *ctx = NULL;
+ bool enable;
+ int ret;
+
+ if (!vdev->pci_recovery_supported)
+ return -ENOTTY;
+
+ ret = vfio_check_feature(flags, argsz,
+ VFIO_DEVICE_FEATURE_GET |
+ VFIO_DEVICE_FEATURE_SET, sizeof(state));
+ if (ret != 1)
+ return ret;
+
+ if (flags & VFIO_DEVICE_FEATURE_GET) {
+ scoped_guard(mutex, &vdev->access_lock) {
+ u32 rflags = vdev->pci_recovery_flags;
+
+ if (vdev->pci_recovery_enabled)
+ state.flags |= VFIO_PCI_ERROR_RECOVERY_ENABLED;
+ if (rflags & VFIO_PCI_RECOVERY_IN_PROGRESS)
+ state.flags |=
+ VFIO_PCI_ERROR_RECOVERY_IN_PROGRESS;
+ if (rflags & VFIO_PCI_RECOVERY_FROZEN)
+ state.flags |=
+ VFIO_PCI_ERROR_RECOVERY_CHANNEL_FROZEN;
+ if (rflags & VFIO_PCI_RECOVERY_RESET)
+ state.flags |=
+ VFIO_PCI_ERROR_RECOVERY_DEVICE_RESET;
+ if (rflags & VFIO_PCI_RECOVERY_FAILED)
+ state.flags |= VFIO_PCI_ERROR_RECOVERY_FAILED;
+ state.sequence = vdev->pci_recovery_sequence;
+ }
+
+ if (copy_to_user(arg, &state, sizeof(state)))
+ return -EFAULT;
+ return 0;
+ }
+
+ if (copy_from_user(&state, arg, sizeof(state)))
+ return -EFAULT;
+ if (state.flags || state.sequence || state.eventfd < -1)
+ return -EINVAL;
+
+ enable = state.eventfd >= 0;
+ if (enable) {
+ ctx = eventfd_ctx_fdget(state.eventfd);
+ if (IS_ERR(ctx))
+ return PTR_ERR(ctx);
+ }
+
+ return vfio_pci_recovery_set(vdev, ctx, enable);
+}
+
int vfio_pci_core_ioctl_feature(struct vfio_device *device, u32 flags,
void __user *arg, size_t argsz)
{
@@ -1959,6 +2071,9 @@ int vfio_pci_core_ioctl_feature(struct vfio_device *device, u32 flags,
return vfio_pci_core_feature_dma_buf(vdev, flags, arg, argsz);
case VFIO_DEVICE_FEATURE_ZPCI_ERROR:
return vfio_pci_zdev_feature_err(device, flags, arg, argsz);
+
+ case VFIO_DEVICE_FEATURE_PCI_ERROR_RECOVERY:
+ return vfio_pci_core_feature_error_recovery(vdev, flags, arg, argsz);
default:
return -ENOTTY;
}
@@ -2685,6 +2800,20 @@ void vfio_pci_core_unregister_device(struct vfio_pci_core_device *vdev)
}
EXPORT_SYMBOL_GPL(vfio_pci_core_unregister_device);
+static void
+vfio_pci_signal_recovery_event(struct vfio_pci_core_device *vdev)
+{
+ struct vfio_pci_eventfd *eventfd;
+
+ lockdep_assert_held(&vdev->access_lock);
+
+ rcu_read_lock();
+ eventfd = rcu_dereference(vdev->pci_recovery_trigger);
+ if (eventfd)
+ eventfd_signal(eventfd->ctx);
+ rcu_read_unlock();
+}
+
/*
* Save PCI_COMMAND, clear bus mastering and set the requested bits.
* Reject an all-ones read from an inaccessible device.
@@ -2717,6 +2846,7 @@ pci_ers_result_t vfio_pci_core_aer_err_detected(struct vfio_pci_core_device *vde
struct vfio_pci_eventfd *eventfd;
pci_ers_result_t result = PCI_ERS_RESULT_CAN_RECOVER;
unsigned long irq_flags;
+ bool notify_recovery = false;
bool nested;
u32 flags;
int ret = 0;
@@ -2745,6 +2875,7 @@ pci_ers_result_t vfio_pci_core_aer_err_detected(struct vfio_pci_core_device *vde
goto out_unlock;
}
+ notify_recovery = true;
vfio_pci_core_block_access(vdev);
/*
* Keep the original command register for nested events. Recovery may
@@ -2811,6 +2942,8 @@ pci_ers_result_t vfio_pci_core_aer_err_detected(struct vfio_pci_core_device *vde
out_memory:
up_write(&vdev->memory_lock);
out_unlock:
+ if (notify_recovery)
+ vfio_pci_signal_recovery_event(vdev);
mutex_unlock(&vdev->access_lock);
out:
@@ -2900,6 +3033,8 @@ pci_ers_result_t vfio_pci_core_aer_slot_reset(struct vfio_pci_core_device *vdev)
(vdev->pci_recovery_flags | VFIO_PCI_RECOVERY_FAILED) &
~VFIO_PCI_RECOVERY_IN_PROGRESS);
vdev->pci_recovery_command_valid = false;
+ /* Report failure now. resume() skips recovery that has already finished. */
+ vfio_pci_signal_recovery_event(vdev);
mutex_unlock(&vdev->access_lock);
/* Allow recovery of other devices under the bridge to continue. */
@@ -2911,6 +3046,7 @@ void vfio_pci_core_aer_resume(struct vfio_pci_core_device *vdev)
{
struct pci_dev *pdev = vdev->pdev;
unsigned long irq_flags;
+ bool notify_recovery = false;
bool notify_intx = false;
bool power_up;
u32 flags;
@@ -2921,6 +3057,7 @@ void vfio_pci_core_aer_resume(struct vfio_pci_core_device *vdev)
if (!(vdev->pci_recovery_flags & VFIO_PCI_RECOVERY_IN_PROGRESS))
goto out_unlock;
+ notify_recovery = true;
/*
* Match IRQ configuration's lock order and keep D0 requests out until
* live ops are installed, so no deferred request can be left behind.
@@ -2983,6 +3120,8 @@ void vfio_pci_core_aer_resume(struct vfio_pci_core_device *vdev)
mutex_unlock(&vdev->igate);
out_unlock:
+ if (notify_recovery)
+ vfio_pci_signal_recovery_event(vdev);
mutex_unlock(&vdev->access_lock);
}
EXPORT_SYMBOL_GPL(vfio_pci_core_aer_resume);
--
2.43.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [RFC PATCH v3 20/20] vfio/pci: Enable recovery for supported native PCIe endpoints
2026-11-12 14:16 [RFC PATCH v3 00/20] vfio/pci: Handle PCI error recovery and report state to userspace Shameer Kolothum
` (18 preceding siblings ...)
2026-11-12 14:16 ` [RFC PATCH v3 19/20] vfio/pci: Add VFIO_DEVICE_FEATURE_PCI_ERROR_RECOVERY Shameer Kolothum
@ 2026-11-12 14:16 ` Shameer Kolothum
2026-11-12 15:10 ` sashiko-bot
19 siblings, 1 reply; 28+ messages in thread
From: Shameer Kolothum @ 2026-11-12 14:16 UTC (permalink / raw)
To: kvm, linux-pci, linux-kernel
Cc: alex, jgg, kevin.tian, kbusch, michal.winiarski,
satyanarayana.k.v.p, sonangp, ankita, nathanc, mochs,
skolothumtho
Enable recovery for generic vfio-pci endpoints on the subordinate bus of
a PCIe Root Port or Downstream Port.
The recovery power helpers require pci_bus_sem held for reading. Native
PCI recovery provides this through its bus walk. Check the endpoint's
bus as well as its upstream port, since synthetic VF buses are outside
that walk. Exclude direct RCiEP and root-bus endpoints.
Exclude EEH-enabled builds and Xen PV because their callbacks do not
provide the required locking. Exclude s390 and CXL memory devices
because their recovery paths can omit completion callbacks. Variant
drivers remain unsupported until their hardware accesses are protected.
Exclude functions with VPD redirected to function zero. The lookup takes
pci_bus_sem inside SRCU and can deadlock with recovery when a bus writer
is queued.
Also exclude Intel Ethernet function zero devices with VPD. Redirected
accesses from sibling functions bypass the target's access checks.
Exclude potential targets even before siblings are probed so support
does not depend on probe order.
These restrictions affect only the new recovery feature. Ordinary
assignment and existing error notifications remain available.
Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
---
include/uapi/linux/vfio.h | 6 +++-
drivers/vfio/pci/vfio_pci.c | 61 +++++++++++++++++++++++++++++++++++++
2 files changed, 66 insertions(+), 1 deletion(-)
diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
index 86a3952ee302..f855d064c4b0 100644
--- a/include/uapi/linux/vfio.h
+++ b/include/uapi/linux/vfio.h
@@ -1558,7 +1558,11 @@ struct vfio_device_feature_zpci_err {
/*
* Report host PCI error recovery state for this device.
*
- * Userspace must probe the feature on each device before enabling it.
+ * Support is limited to PCIe endpoints on a Root Port or Downstream Port
+ * subordinate bus, using native PCI error recovery. Direct RCiEP, EEH, Xen
+ * PV, s390 and CXL memory recovery are not supported. Functions with
+ * VPD redirected to function zero, and Intel Ethernet function zero
+ * devices with VPD are excluded. Userspace must probe each device.
*
* VFIO_DEVICE_FEATURE_SET with a valid eventfd enables recovery for this
* open and requests notifications when each event starts and ends. SET
diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
index a5ba4bb11638..fe9b8b8decca 100644
--- a/drivers/vfio/pci/vfio_pci.c
+++ b/drivers/vfio/pci/vfio_pci.c
@@ -126,10 +126,70 @@ static int vfio_pci_open_device(struct vfio_device *core_vdev)
return 0;
}
+static bool vfio_pci_supports_recovery(struct pci_dev *pdev)
+{
+ struct pci_dev *bridge = pci_upstream_bridge(pdev);
+
+ /*
+ * Looking up function zero for VPD takes pci_bus_sem inside access_srcu.
+ * Recovery holds the bus read lock while waiting for these SRCU readers.
+ * A queued bus writer can leave both paths waiting for each other.
+ */
+ if (pdev->dev_flags & PCI_DEV_FLAGS_VPD_REF_F0)
+ return false;
+
+ /*
+ * quirk_f0_vpd_link() can redirect sibling VPD accesses to these devices
+ * without checking whether recovery has blocked function zero. Exclude
+ * them even if no redirecting sibling is present at probe.
+ */
+ if (!PCI_FUNC(pdev->devfn) && pdev->vpd.cap &&
+ pdev->vendor == PCI_VENDOR_ID_INTEL &&
+ (pdev->class >> 8) == PCI_CLASS_NETWORK_ETHERNET)
+ return false;
+
+ /*
+ * EEH and Xen PV can call recovery without the required bus locking.
+ * s390 recovery can omit completion callbacks.
+ */
+ if (IS_ENABLED(CONFIG_S390) || IS_ENABLED(CONFIG_EEH) ||
+ xen_pv_domain())
+ return false;
+
+ /*
+ * Recovery power helpers require pci_bus_sem held for reading.
+ * The native recovery walk holds it on a port's subordinate bus.
+ * Direct RCiEP recovery does not use that walk.
+ */
+ if (!pci_is_pcie(pdev))
+ return false;
+
+ if (pci_pcie_type(pdev) != PCI_EXP_TYPE_ENDPOINT &&
+ pci_pcie_type(pdev) != PCI_EXP_TYPE_LEG_END)
+ return false;
+
+ if (!bridge || !pci_is_pcie(bridge))
+ return false;
+
+ if (pci_pcie_type(bridge) != PCI_EXP_TYPE_ROOT_PORT &&
+ pci_pcie_type(bridge) != PCI_EXP_TYPE_DOWNSTREAM)
+ return false;
+
+ if (pdev->bus != bridge->subordinate)
+ return false;
+
+ /* CXL RCH recovery can omit completion callbacks. */
+ if ((pdev->class >> 8) == PCI_CLASS_MEMORY_CXL)
+ return false;
+
+ return true;
+}
+
static int vfio_pci_init_dev(struct vfio_device *core_vdev)
{
struct vfio_pci_core_device *vdev =
container_of(core_vdev, struct vfio_pci_core_device, vdev);
+ struct pci_dev *pdev = to_pci_dev(core_vdev->dev);
/*
* These behaviors originated in vfio-pci and moved into
@@ -140,6 +200,7 @@ static int vfio_pci_init_dev(struct vfio_device *core_vdev)
*/
vdev->nointxmask = nointxmask;
vdev->disable_idle_d3 = disable_idle_d3;
+ vdev->pci_recovery_supported = vfio_pci_supports_recovery(pdev);
#ifdef CONFIG_VFIO_PCI_VGA
vdev->disable_vga = disable_vga;
#endif
--
2.43.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [RFC PATCH v3 20/20] vfio/pci: Enable recovery for supported native PCIe endpoints
2026-11-12 14:16 ` [RFC PATCH v3 20/20] vfio/pci: Enable recovery for supported native PCIe endpoints Shameer Kolothum
@ 2026-11-12 15:10 ` sashiko-bot
0 siblings, 0 replies; 28+ messages in thread
From: sashiko-bot @ 2026-11-12 15:10 UTC (permalink / raw)
To: Shameer Kolothum, kvm, linux-pci
Thank you for your contribution! Sashiko AI review found 3 potential
issues to consider:
- [High] vfio/pci: VFIO_DEVICE_GET_INFO fails with -EIO on VFs when PF
is closed
- [Medium] vfio/pci: circular locking between access_srcu and
access_lock on VF and PF
- [Medium] vfio/pci: false recovery in vfio_pci_supports_recovery() for
SR-IOV VFs
--
commit 4182befeb4982c12ce357363cc414cc54cc9381b
Author: Shameer Kolothum <skolothumtho@nvidia.com>
vfio/pci: Enable recovery for supported native PCIe endpoints
This commit enables host PCI error recovery for supported generic vfio-pci
endpoints on the subordinate bus of a PCIe Root Port or Downstream Port and
updates the UAPI documentation.
> diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
> index a5ba4bb116382..fe9b8b8deccab 100644
> --- a/drivers/vfio/pci/vfio_pci.c
> +++ b/drivers/vfio/pci/vfio_pci.c
> @@ -126,10 +126,70 @@ static int vfio_pci_open_device(struct vfio_device *core_vdev)
> return 0;
> }
>
> +static bool vfio_pci_supports_recovery(struct pci_dev *pdev)
> +{
> + struct pci_dev *bridge = pci_upstream_bridge(pdev);
> +
> + /*
> + * Looking up function zero for VPD takes pci_bus_sem inside access_srcu.
> + * Recovery holds the bus read lock while waiting for these SRCU readers.
> + * A queued bus writer can leave both paths waiting for each other.
> + */
> + if (pdev->dev_flags & PCI_DEV_FLAGS_VPD_REF_F0)
> + return false;
> +
> + /*
> + * quirk_f0_vpd_link() can redirect sibling VPD accesses to these devices
> + * without checking whether recovery has blocked function zero. Exclude
> + * them even if no redirecting sibling is present at probe.
> + */
> + if (!PCI_FUNC(pdev->devfn) && pdev->vpd.cap &&
> + pdev->vendor == PCI_VENDOR_ID_INTEL &&
> + (pdev->class >> 8) == PCI_CLASS_NETWORK_ETHERNET)
> + return false;
> +
> + /*
> + * EEH and Xen PV can call recovery without the required bus locking.
> + * s390 recovery can omit completion callbacks.
> + */
> + if (IS_ENABLED(CONFIG_S390) || IS_ENABLED(CONFIG_EEH) ||
> + xen_pv_domain())
> + return false;
> +
> + /*
> + * Recovery power helpers require pci_bus_sem held for reading.
> + * The native recovery walk holds it on a port's subordinate bus.
> + * Direct RCiEP recovery does not use that walk.
> + */
> + if (!pci_is_pcie(pdev))
> + return false;
> +
> + if (pci_pcie_type(pdev) != PCI_EXP_TYPE_ENDPOINT &&
> + pci_pcie_type(pdev) != PCI_EXP_TYPE_LEG_END)
> + return false;
> +
> + if (!bridge || !pci_is_pcie(bridge))
> + return false;
> +
> + if (pci_pcie_type(bridge) != PCI_EXP_TYPE_ROOT_PORT &&
> + pci_pcie_type(bridge) != PCI_EXP_TYPE_DOWNSTREAM)
> + return false;
> +
> + if (pdev->bus != bridge->subordinate)
> + return false;
[Severity: Medium]
Does this allow SR-IOV VFs placed on the PF's bus to report a false
recovery when the PF itself does not recover?
When pci_iov_virtfn_bus() matches the PF's bus number, virtfn_add_bus()
places VFs directly on pf->bus (bridge->subordinate) rather than a
synthetic VF bus, and pci_upstream_bridge(pdev) resolves through the PF to
bridge. vfio_pci_supports_recovery() then returns true for the VF even if
the PF is excluded from recovery (such as an Intel Ethernet function 0 PF
with VPD), closed, or has recovery disabled.
If pcie_do_recovery() performs a secondary bus reset on bridge, the PF is
reset in hardware, clearing PCI_SRIOV_CTRL_VFE and disabling all VFs.
When the PF is not participating in vfio-pci recovery,
vfio_pci_core_aer_slot_reset() returns PCI_ERS_RESULT_NONE on the PF
without calling pci_restore_state() / pci_restore_iov_state(). The
subsequent vfio_pci_core_aer_slot_reset() on the VF still succeeds because
pci_set_power_state_locked() sees current_state == PCI_D0 and
pci_restore_state() returns void, so the VF reports
PCI_ERS_RESULT_RECOVERED and re-enables live access while VFE remains
cleared on the PF.
[Severity: Medium]
Enabling pci_recovery_supported on both a PF and an SR-IOV VF on the same
bus also triggers a lockdep circular locking splat when
VFIO_DEVICE_GET_INFO is called on the VF while the PF is open.
All vfio_pci_core_device instances share the same lockdep classes for
access_lock and access_srcu. vfio_pci_core_block_access() runs
synchronize_srcu(&vdev->access_srcu) under vdev->access_lock (establishing
access_lock -> access_srcu), whereas vfio_pci_ioctl_get_info() on the VF
holds srcu_read_lock(&vdev->access_srcu) across
vfio_pci_access_atomic_cap(), which takes pf_vdev->access_lock and
srcu_read_lock(&pf_vdev->access_srcu):
drivers/vfio/pci/vfio_pci_core.c:vfio_pci_access_atomic_cap() {
...
guard(mutex)(&pf_vdev->access_lock);
if (!pf_vdev->device_open)
return -EIO;
return vfio_pci_core_access(pf_vdev, vfio_pci_access_atomic_cap_target,
data);
}
> +
> + /* CXL RCH recovery can omit completion callbacks. */
> + if ((pdev->class >> 8) == PCI_CLASS_MEMORY_CXL)
> + return false;
> +
> + return true;
> +}
> +
> static int vfio_pci_init_dev(struct vfio_device *core_vdev)
> {
> struct vfio_pci_core_device *vdev =
> container_of(core_vdev, struct vfio_pci_core_device, vdev);
> + struct pci_dev *pdev = to_pci_dev(core_vdev->dev);
>
> /*
> * These behaviors originated in vfio-pci and moved into
> @@ -140,6 +200,7 @@ static int vfio_pci_init_dev(struct vfio_device *core_vdev)
> */
> vdev->nointxmask = nointxmask;
> vdev->disable_idle_d3 = disable_idle_d3;
> + vdev->pci_recovery_supported = vfio_pci_supports_recovery(pdev);
[Severity: High]
Does setting pci_recovery_supported on an SR-IOV PF cause
VFIO_DEVICE_GET_INFO on its VFs to fail with -EIO whenever the PF is not
open?
SR-IOV can be enabled while a vfio-pci PF is unopened, or the PF fd can be
closed after configuring a vf_token while VFs are used. Once
pf_vdev->pci_recovery_supported is true, vfio_pci_ioctl_get_info() ->
vfio_pci_access_atomic_cap() on a VF checks pf_vdev->device_open and
returns -EIO whenever the PF is closed, even when no host recovery is
active on the PF:
drivers/vfio/pci/vfio_pci_core.c:vfio_pci_access_atomic_cap() {
struct vfio_pci_core_device *pf_vdev = vdev->sriov_pf_core_dev;
if (!pf_vdev || !pf_vdev->pci_recovery_supported)
return vfio_pci_info_atomic_cap(vdev, data);
...
guard(mutex)(&pf_vdev->access_lock);
if (!pf_vdev->device_open)
return -EIO;
}
vfio_pci_ioctl_get_info() treats any return value other than 0 or -ENODEV
as fatal and fails the VF's VFIO_DEVICE_GET_INFO ioctl with -EIO.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261112141632.918954-1-skolothumtho@nvidia.com?part=20
^ permalink raw reply [flat|nested] 28+ messages in thread