Re: [PATCH v2 2/4] vfio: selftests: Verify a failed second open preserves the vf_token

[David Matlack <dmatlack@google.com>]

On 2026-09-11 11:04 AM, Alex Williamson wrote:
> The cdev path enforces a single open per device and rejects a second
> bind of an already open device.  That rejection must happen before the
> bind can mutate state shared across opens, notably the PF vf_token, so
> that a bind which cannot complete leaves the current opener's state
> untouched.
> 
> Add a regression test that binds a PF with one token, attempts a
> second bind of the same PF with a different token, then initializes a
> VF with the original token.  The VF init succeeds only if the second
> bind left the PF vf_token intact; a regression that clobbered it to
> the second token would make the VF init fail.
> 
> Additionally add a second separate test that enforces the -EBUSY
> errno on second open so that the vf_token clobber and errno testing
> are independent.
> 
> These hazards are specific to the cdev/iommufd single-open path, so
> the tests run only in iommufd mode.
> 
> Suggested-by: David Matlack <dmatlack@google.com>
> Assisted-by: LLM
> Signed-off-by: Alex Williamson <alex.williamson@nvidia.com>
> ---
>  .../selftests/vfio/vfio_pci_sriov_uapi_test.c | 57 +++++++++++++++++++
>  1 file changed, 57 insertions(+)
> 
> diff --git a/tools/testing/selftests/vfio/vfio_pci_sriov_uapi_test.c b/tools/testing/selftests/vfio/vfio_pci_sriov_uapi_test.c
> index 19d657d00b75..b57e4498443f 100644
> --- a/tools/testing/selftests/vfio/vfio_pci_sriov_uapi_test.c
> +++ b/tools/testing/selftests/vfio/vfio_pci_sriov_uapi_test.c
> @@ -157,6 +157,63 @@ TEST_F(vfio_pci_sriov_uapi_test, override_token)
>  	ASSERT_COND_VF_CREATION(ret);
>  }
>  
> +TEST(failed_second_open_does_not_clobber_token)
> +{
> +	struct vfio_pci_device *pf = NULL, *pf_second_fd = NULL, *vf = NULL;
> +	struct iommu *iommu;
> +	int ret;
> +
> +	iommu = iommu_init("iommufd");
> +
> +	/* Create and bind PF using UUID_1 */
> +	ret = device_init(pf_bdf, iommu, UUID_1, &pf);
> +	ASSERT_EQ(ret, 0);
> +
> +	/*
> +	 * Attempt to open the same PF again and bind it with a *different*
> +	 * token (UUID_2).  Return value intentionally unenforced.
> +	 */
> +	device_init(pf_bdf, iommu, UUID_2, &pf_second_fd);

I originally suggested a single test here and I think that still makes
sense. There's too much duplicate code otherwise.

If you want the rest of the test to still run independent of what this
returns you can use EXPECT_EQ(ret, -EBUSY) instead of ASSERT_EQ().

> +
> +	/*
> +	 * Attempt to initialize a VF using the original PF token (UUID_1).
> +	 * If the failed open above clobbered the PF's token (i.e. updated it to
> +	 * UUID_2), this VF initialization will fail.
> +	 */
> +	ret = device_init(vf_bdf, iommu, UUID_1, &vf);
> +	ASSERT_EQ(ret, 0);
> +
> +	device_cleanup(vf);
> +	device_cleanup(pf_second_fd);
> +	device_cleanup(pf);
> +	iommu_cleanup(iommu);
> +}
> +
> +TEST(failed_second_open_returns_ebusy)
> +{
> +	struct vfio_pci_device *pf = NULL, *pf_second_fd = NULL;
> +	struct iommu *iommu;
> +	int ret;
> +
> +	iommu = iommu_init("iommufd");
> +
> +	/* Create and bind PF using UUID_1 */
> +	ret = device_init(pf_bdf, iommu, UUID_1, &pf);
> +	ASSERT_EQ(ret, 0);
> +
> +	/*
> +	 * Attempt to open the same PF again and bind it with a *different*
> +	 * token (UUID_2). This must fail with EBUSY because it's a second open.
> +	 * Previously failed with EINVAL.
> +	 */

"Previously failed with EINVAL" should probably go in the commit message
rather than the test.

> +	ret = device_init(pf_bdf, iommu, UUID_2, &pf_second_fd);
> +	ASSERT_EQ(ret, -EBUSY);
> +
> +	device_cleanup(pf_second_fd);
> +	device_cleanup(pf);
> +	iommu_cleanup(iommu);
> +}
> +
>  static void vf_teardown(void)
>  {
>  	/*
> -- 
> 2.53.0
>