[PATCH 0/4] vfio: Fix cdev second-open and harden selftests

[PATCH 0/4] vfio: Fix cdev second-open and harden selftests

[Alex Williamson]

In porting some testing infrastructure to a different system I found
the igb selftest failing in mix_and_match and generating a cascade
failure through the remaining tests.  The difference in the new system
is the firmware error handling.  When the igb device gets wedged due
to bad DMA in mix_and_match, we need to FLR the device.  The igb holds
transaction pending asserted for the full duration of
pci_wait_for_pending().  Meanwhile, firmware based error handling is
triggering SMIs and stealing time, such that the 700ms total delay
in pci_wait_for_pending() turns into several seconds.  With 10 cases
in mix_and_match generating bad DMAs, the transaction pending delays
alone push us close to the 30s per-test timeout.  The worst case I
observed for the total test was ~45s.  Increasing the mix_and_match
timeout to 90s provides plenty of headroom to get a passing test and
avoid the cascade failure.

When the process is killed via timeout, the release occurs through a
scheduled fput(), which is also delayed by the SMI storm.  Each
subsequent test then sees a non-zero open_count (for the group open
in legacy mode or on the iommufd bind in the cdev mode), resulting in
the cascade failure.  Group mode already uses -EBUSY when the group
fd open count is elevated, which allows selftests to interpret the
failure as potentially transient and implement a bounded retry.  The
cdev path instead returns -EINVAL for this case.  -EBUSY seems
justified here and allows userspace to have compatible retry flows
for group open and cdev bind operations.

A local sashiko review then found two existing issues.  First, in
analyzing the exit flow from the iommufd bind, we can see that the
vf_token and kvm pointers are clobbered by the second process before
the open count test.  The open_count test in vfio_df_open() is only
for the cdev path (!df->group) and is called under the dev_set lock,
so we really only need to relocate the test to
vfio_df_ioctl_bind_iommufd() prior to vf_token/kvm manipulation.

The second existing issue is that when executed via the kselftest
runner, all tests have a 45s timeout, which is the cumulative time
across each sub-test of the execution.  The pci_driver test already
fails this with ioatdma, nv_falcon, and obviously with physical igb.
Running in parallel across both ports of an igb on the system prone
to SMI overhead, the worst case I saw was 450s.  Therefore, we not
only need to extend the mix_and_match timeout to 90s to handle the
extra transaction pending delay, we need to extend the default
timeout to allow the full pci-driver test to complete when executed
via the runner.  600s is picked here as a "sufficient" margin.

Please review and comment.  Thanks,

Alex

Alex Williamson (4):
  vfio: Reject a second cdev open before mutating shared device state
  selftests/vfio: Wait out transient -EBUSY on open/bind
  selftests/vfio: Extend mix_and_match timeout to 90s
  selftests/vfio: Extend timeout for runner executions

 drivers/vfio/device_cdev.c                    | 12 ++++++++
 drivers/vfio/vfio_main.c                      |  7 -----
 tools/testing/selftests/vfio/.gitignore       |  1 +
 .../selftests/vfio/lib/vfio_pci_device.c      | 29 +++++++++++++++++--
 tools/testing/selftests/vfio/settings         |  5 ++++
 .../selftests/vfio/vfio_pci_driver_test.c     |  2 +-
 6 files changed, 46 insertions(+), 10 deletions(-)
 create mode 100644 tools/testing/selftests/vfio/settings


base-commit: cee9395acd8043be0644b25c34bfa86623f2b935
-- 
2.53.0

[PATCH 1/4] vfio: Reject a second cdev open before mutating shared device state

[Alex Williamson]

The cdev single-open check lives in vfio_df_open(), which runs at the
end of the bind ioctl, after vfio_df_ioctl_bind_iommufd() has already
updated state shared across all opens: vfio_df_check_token() can set
the PF vf_token and vfio_df_get_kvm_safe() records the caller's KVM
pointer in device->kvm and takes a reference.

A second cdev bind of an already-open device runs both, only to be
rejected in vfio_df_open().  The error path clears device->kvm and
drops the reference, tearing down the current opener's KVM association
and potentially resulting in an unbalanced reference on close or
premature release, while the vf_token remains clobbered.

Move the single-open check into vfio_df_ioctl_bind_iommufd() ahead of
both mutations, so a bind that cannot complete leaves the current
opener's state untouched.  df->group is NULL on this path, so a
non-zero open_count is exactly what vfio_df_open() rejected.  The test
in vfio_df_open() becomes redundant and is removed.

Return -EBUSY rather than -EINVAL here.  The arguments are not invalid,
the device is in use, which could be a transient condition due to a
delayed fput if the prior user is terminated.  This provides
compatibility with the group path, where a group open returns -EBUSY,
and users may choose bounded polling to detect such a transient
condition.

Fixes: 839e692fa4eb ("vfio: Make vfio_df_open() single open for device cdev path")
Fixes: 5fcc26969a16 ("vfio: Add VFIO_DEVICE_BIND_IOMMUFD")
Fixes: 86624ba3b522 ("vfio/pci: Do vf_token checks for VFIO_DEVICE_BIND_IOMMUFD")
Assisted-by: claude-opus-4-8
Signed-off-by: Alex Williamson <alex.williamson@nvidia.com>
---
 drivers/vfio/device_cdev.c | 12 ++++++++++++
 drivers/vfio/vfio_main.c   |  7 -------
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/vfio/device_cdev.c b/drivers/vfio/device_cdev.c
index 1d9515c967b0..30362936c7b5 100644
--- a/drivers/vfio/device_cdev.c
+++ b/drivers/vfio/device_cdev.c
@@ -130,6 +130,18 @@ long vfio_df_ioctl_bind_iommufd(struct vfio_device_file *df,
 		goto out_unlock;
 	}
 
+	/*
+	 * The cdev path allows only a single open.  Reject a second open here,
+	 * before the VF token and device->kvm updates below would clobber the
+	 * current opener's state on a bind that cannot complete.  Return -EBUSY
+	 * rather than -EINVAL since a delayed release of the prior opener can
+	 * make this transient.
+	 */
+	if (device->open_count) {
+		ret = -EBUSY;
+		goto out_unlock;
+	}
+
 	ret = vfio_df_check_token(device, &bind);
 	if (ret)
 		goto out_unlock;
diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c
index 423ead48aafe..cb3deb5a4857 100644
--- a/drivers/vfio/vfio_main.c
+++ b/drivers/vfio/vfio_main.c
@@ -603,13 +603,6 @@ int vfio_df_open(struct vfio_device_file *df)
 
 	lockdep_assert_held(&device->dev_set->lock);
 
-	/*
-	 * Only the group path allows the device to be opened multiple
-	 * times.  The device cdev path doesn't have a secure way for it.
-	 */
-	if (device->open_count != 0 && !df->group)
-		return -EINVAL;
-
 	device->open_count++;
 	if (device->open_count == 1) {
 		ret = vfio_df_device_first_open(df);
-- 
2.53.0

[PATCH 2/4] selftests/vfio: Wait out transient -EBUSY on open/bind

[Alex Williamson]

If a test is killed, for example due to timeout, fput can be delayed,
allowing the subsequent test to be started while the failing test still
holds the device open count elevated.  This results in a cascade of
failures as each subsequent test fails on open, blocked by the single
user requirement at the group or device cdev file.

We can make the test framework more robust, and allow better
identification of specific failing scenarios, by waiting-out transient
-EBUSY failures on group open and cdev bind.

The 20s retry window is heuristically determined in testing on a system
where scheduling can be significantly delayed due to SMI handling of
platform errors generated from the mix-and-match test.

The SR-IOV uAPI and IOMMUFD setup tests retain their non-retry bind
paths as these are not expected to encounter process kills due to
underlying platform error handling variability.

Assisted-by: Qwen3.8-27B
Signed-off-by: Alex Williamson <alex.williamson@nvidia.com>
---
 .../selftests/vfio/lib/vfio_pci_device.c      | 29 +++++++++++++++++--
 1 file changed, 27 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/vfio/lib/vfio_pci_device.c b/tools/testing/selftests/vfio/lib/vfio_pci_device.c
index 4063a0e2b3df..8a3139b7c6dd 100644
--- a/tools/testing/selftests/vfio/lib/vfio_pci_device.c
+++ b/tools/testing/selftests/vfio/lib/vfio_pci_device.c
@@ -282,18 +282,34 @@ void vfio_pci_device_reset(struct vfio_pci_device *device)
 	VFIO_ASSERT_EQ(r, 0, "ioctl(device->fd, VFIO_DEVICE_RESET) failed\n");
 }
 
+/*
+ * A prior test's delayed fput can briefly leave the open count elevated, so
+ * group open and cdev bind can see a transient -EBUSY.  Retry to wait out the
+ * fput scheduling latency.
+ */
+#define VFIO_DEVICE_BUSY_RETRIES	200
+#define VFIO_DEVICE_BUSY_INTERVAL_US	100000
+
 void vfio_pci_group_setup(struct vfio_pci_device *device, const char *bdf)
 {
 	struct vfio_group_status group_status = {
 		.argsz = sizeof(group_status),
 	};
 	char group_path[32];
+	int retries = VFIO_DEVICE_BUSY_RETRIES;
 	int group;
 
 	group = sysfs_iommu_group_get(bdf);
 	snprintf_assert(group_path, sizeof(group_path), "/dev/vfio/%d", group);
 
-	device->group_fd = open(group_path, O_RDWR);
+	for (;;) {
+		device->group_fd = open(group_path, O_RDWR);
+		if (device->group_fd >= 0 || errno != EBUSY || retries-- <= 0)
+			break;
+
+		usleep(VFIO_DEVICE_BUSY_INTERVAL_US);
+	}
+
 	VFIO_ASSERT_GE(device->group_fd, 0, "open(%s) failed\n", group_path);
 
 	ioctl_assert(device->group_fd, VFIO_GROUP_GET_STATUS, &group_status);
@@ -432,7 +448,16 @@ int __vfio_device_bind_iommufd(int device_fd, int iommufd, const char *vf_token)
 static void vfio_device_bind_iommufd(int device_fd, int iommufd,
 				     const char *vf_token)
 {
-	int ret = __vfio_device_bind_iommufd(device_fd, iommufd, vf_token);
+	int retries = VFIO_DEVICE_BUSY_RETRIES;
+	int ret;
+
+	for (;;) {
+		ret = __vfio_device_bind_iommufd(device_fd, iommufd, vf_token);
+		if (ret != -EBUSY || retries-- <= 0)
+			break;
+
+		usleep(VFIO_DEVICE_BUSY_INTERVAL_US);
+	}
 
 	VFIO_ASSERT_EQ(ret, 0, "Failed VFIO_DEVICE_BIND_IOMMUFD ioctl\n");
 }
-- 
2.53.0

[PATCH 3/4] selftests/vfio: Extend mix_and_match timeout to 90s

[Alex Williamson]

Systems with firmware first error handling can see heavy SMM load
as a result of the mix_and_match induced IOMMU errors.  For devices
like igb that get wedged on these errors, the resulting FLR is delayed
waiting for the transaction pending register to clear, which never
occurs.  However, due to the background SMM, the backoff delays in
PCI code wildly exceed their timeouts.  As a result, the whole test
can report failure.

Extend the mix_and_match timeout to a value shown to complete reliably
on such systems, with ~2x margin.

Assisted-by: Qwen3.8-27B
Signed-off-by: Alex Williamson <alex.williamson@nvidia.com>
---
 tools/testing/selftests/vfio/vfio_pci_driver_test.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/vfio/vfio_pci_driver_test.c b/tools/testing/selftests/vfio/vfio_pci_driver_test.c
index 761bf117d624..1d532f944cee 100644
--- a/tools/testing/selftests/vfio/vfio_pci_driver_test.c
+++ b/tools/testing/selftests/vfio/vfio_pci_driver_test.c
@@ -190,7 +190,7 @@ TEST_F(vfio_pci_driver_test, send_msi)
 	ASSERT_EQ(1, value);
 }
 
-TEST_F(vfio_pci_driver_test, mix_and_match)
+TEST_F_TIMEOUT(vfio_pci_driver_test, mix_and_match, 90)
 {
 	u64 value;
 	int i;
-- 
2.53.0

[PATCH 4/4] selftests/vfio: Extend timeout for runner executions

[Alex Williamson]

When executed via the runner, ie. run_kselftest.sh, tests are subject
to a default 45s timeout.  None of the igb, ioatdma, or nv_falcon
drivers can run the vfio_pci_driver_test within this window.

The igb driver is the worst due to the device getting wedged from bad
DMAs in mix_and_match and requiring the full transaction pending
delay prior to FLR.

This is even further exacerbated when the system implements
firmware-first error handling and the transaction pending scheduled
delays are preempted beyond expectations by SMIs.

Worst case observed runtime for the whole vfio_pci_driver_test on
such a setup is 450s.  Round up to 600s.

Assisted-by: GPT-5.5
Signed-off-by: Alex Williamson <alex.williamson@nvidia.com>
---
 tools/testing/selftests/vfio/.gitignore | 1 +
 tools/testing/selftests/vfio/settings   | 5 +++++
 2 files changed, 6 insertions(+)
 create mode 100644 tools/testing/selftests/vfio/settings

diff --git a/tools/testing/selftests/vfio/.gitignore b/tools/testing/selftests/vfio/.gitignore
index 7fadc19d3bca..fc14bc18bcd8 100644
--- a/tools/testing/selftests/vfio/.gitignore
+++ b/tools/testing/selftests/vfio/.gitignore
@@ -8,3 +8,4 @@
 !*.mk
 !.gitignore
 !Makefile
+!settings
diff --git a/tools/testing/selftests/vfio/settings b/tools/testing/selftests/vfio/settings
new file mode 100644
index 000000000000..c9abf1c19f19
--- /dev/null
+++ b/tools/testing/selftests/vfio/settings
@@ -0,0 +1,5 @@
+# Extend kselftest default 45s timeout based on worst case test runtime,
+# currently vfio:vfio_pci_driver_test running on physical NIC, battling
+# SMIs due to invoked firmware-first error handling from bad DMAs and
+# device wedging.  Observed worst case ~450s, extend to 600s.
+timeout=600
-- 
2.53.0