All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/4] vfio: Fix cdev second-open and harden selftests
@ 2026-09-11 17:04 Alex Williamson
  2026-09-11 17:04 ` [PATCH v2 1/4] vfio: Reject a second cdev open before mutating shared device state Alex Williamson
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Alex Williamson @ 2026-09-11 17:04 UTC (permalink / raw)
  To: Alex Williamson, kvm
  Cc: Alex Williamson, linux-kernel, Jason Gunthorpe, Kevin Tian,
	Yi Liu, David Matlack

v2:
 - Re-try on -EBUSY reworked to harness issue and posted separately[1],
   dropped here.
 - vfio selftests subjects updated for consistency, Reviewed-bys
   incorporated, Assisted-bys updated to current standards.
 - Added the selftest suggested by David, but split it into two tests,
   one that validates vf_token is not clobbered regardless of the 2nd
   bind errno, and another that enforces the -EBUSY expected errno.
   This allows us to observe that an old kernel fails both and avoids
   conflating the uAPI expectation vs the underlying data clobber.

[1]https://lore.kernel.org/all/20260910230254.1198094-1-alex.williamson@nvidia.com/

v1:

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
  vfio: selftests: Verify a failed second open preserves the vf_token
  vfio: selftests: Extend mix_and_match timeout to 90s
  vfio: selftests: Extend timeout for runner executions

 drivers/vfio/device_cdev.c                    | 12 ++++
 drivers/vfio/vfio_main.c                      |  7 ---
 tools/testing/selftests/vfio/.gitignore       |  1 +
 tools/testing/selftests/vfio/settings         |  5 ++
 .../selftests/vfio/vfio_pci_driver_test.c     |  2 +-
 .../selftests/vfio/vfio_pci_sriov_uapi_test.c | 57 +++++++++++++++++++
 6 files changed, 76 insertions(+), 8 deletions(-)
 create mode 100644 tools/testing/selftests/vfio/settings


base-commit: cee9395acd8043be0644b25c34bfa86623f2b935
-- 
2.53.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2026-09-25 20:51 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-11 17:04 [PATCH v2 0/4] vfio: Fix cdev second-open and harden selftests Alex Williamson
2026-09-11 17:04 ` [PATCH v2 1/4] vfio: Reject a second cdev open before mutating shared device state Alex Williamson
2026-09-16  6:58   ` Tian, Kevin
2026-09-11 17:04 ` [PATCH v2 2/4] vfio: selftests: Verify a failed second open preserves the vf_token Alex Williamson
2026-09-24 18:05   ` Alex Williamson
2026-09-24 18:56   ` David Matlack
2026-09-11 17:04 ` [PATCH v2 3/4] vfio: selftests: Extend mix_and_match timeout to 90s Alex Williamson
2026-09-11 17:04 ` [PATCH v2 4/4] vfio: selftests: Extend timeout for runner executions Alex Williamson
2026-09-25 20:49 ` [PATCH v2 0/4] vfio: Fix cdev second-open and harden selftests Alex Williamson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.