Add a selftest, vfio_pci_sriov_uapi_test.c, to validate the
SR-IOV UAPI, including the following cases, iterating over
all the IOMMU modes currently supported:
 - Setting correct/incorrect/NULL tokens during device init.
 - Close the PF device immediately after setting the token.
 - Change/override the PF's token after device init.

Signed-off-by: Raghavendra Rao Ananta <rananta@google.com>
Reviewed-by: David Matlack <dmatlack@google.com>
---
 tools/testing/selftests/vfio/Makefile         |   1 +
 .../selftests/vfio/vfio_pci_sriov_uapi_test.c | 200 ++++++++++++++++++
 2 files changed, 201 insertions(+)
 create mode 100644 tools/testing/selftests/vfio/vfio_pci_sriov_uapi_test.c

diff --git a/tools/testing/selftests/vfio/Makefile b/tools/testing/selftests/vfio/Makefile
index f27ed18070f14..d0c8cea53eb54 100644
--- a/tools/testing/selftests/vfio/Makefile
+++ b/tools/testing/selftests/vfio/Makefile
@@ -12,6 +12,7 @@ TEST_GEN_PROGS += vfio_iommufd_setup_test
 TEST_GEN_PROGS += vfio_pci_device_test
 TEST_GEN_PROGS += vfio_pci_device_init_perf_test
 TEST_GEN_PROGS += vfio_pci_driver_test
+TEST_GEN_PROGS += vfio_pci_sriov_uapi_test
 
 TEST_FILES += scripts/cleanup.sh
 TEST_FILES += scripts/lib.sh
diff --git a/tools/testing/selftests/vfio/vfio_pci_sriov_uapi_test.c b/tools/testing/selftests/vfio/vfio_pci_sriov_uapi_test.c
new file mode 100644
index 0000000000000..9cfbecccb759f
--- /dev/null
+++ b/tools/testing/selftests/vfio/vfio_pci_sriov_uapi_test.c
@@ -0,0 +1,200 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <fcntl.h>
+#include <unistd.h>
+#include <stdlib.h>
+#include <sys/ioctl.h>
+#include <linux/limits.h>
+
+#include <libvfio.h>
+
+#include "../kselftest_harness.h"
+
+#define UUID_1 "52ac9bff-3a88-4fbd-901a-0d767c3b6c97"
+#define UUID_2 "88594674-90a0-47a9-aea8-9d9b352ac08a"
+
+static const char *pf_bdf;
+
+static int container_setup(struct vfio_pci_device *device, const char *bdf,
+			   const char *vf_token)
+{
+	vfio_pci_group_setup(device, bdf);
+	vfio_container_set_iommu(device);
+	__vfio_pci_group_get_device_fd(device, bdf, vf_token);
+
+	/* The device fd will be -1 in case of mismatched tokens */
+	return (device->fd < 0);
+}
+
+static int iommufd_setup(struct vfio_pci_device *device, const char *bdf,
+			 const char *vf_token)
+{
+	vfio_pci_cdev_open(device, bdf);
+	return __vfio_device_bind_iommufd(device->fd,
+					  device->iommu->iommufd, vf_token);
+}
+
+static struct vfio_pci_device *device_init(const char *bdf, struct iommu *iommu,
+					   const char *vf_token, int *out_ret)
+{
+	struct vfio_pci_device *device = vfio_pci_device_alloc(bdf, iommu);
+
+	if (iommu->mode->container_path)
+		*out_ret = container_setup(device, bdf, vf_token);
+	else
+		*out_ret = iommufd_setup(device, bdf, vf_token);
+
+	return device;
+}
+
+static void device_cleanup(struct vfio_pci_device *device)
+{
+	if (device->fd > 0)
+		VFIO_ASSERT_EQ(close(device->fd), 0);
+
+	if (device->group_fd)
+		VFIO_ASSERT_EQ(close(device->group_fd), 0);
+
+	vfio_pci_device_free(device);
+}
+
+FIXTURE(vfio_pci_sriov_uapi_test) {
+	char *vf_bdf;
+};
+
+FIXTURE_SETUP(vfio_pci_sriov_uapi_test)
+{
+	char *vf_driver;
+	int nr_vfs;
+
+	nr_vfs = sysfs_sriov_totalvfs_get(pf_bdf);
+	if (nr_vfs <= 0)
+		SKIP(return, "SR-IOV may not be supported by the PF: %s\n", pf_bdf);
+
+	nr_vfs = sysfs_sriov_numvfs_get(pf_bdf);
+	if (nr_vfs != 0)
+		SKIP(return, "SR-IOV already configured for the PF: %s\n", pf_bdf);
+
+	/* Create only one VF for testing */
+	sysfs_sriov_numvfs_set(pf_bdf, 1);
+	self->vf_bdf = sysfs_sriov_vf_bdf_get(pf_bdf, 0);
+
+	/*
+	 * The VF inherits the driver from the PF.
+	 * Ensure this is 'vfio-pci' before proceeding.
+	 */
+	vf_driver = sysfs_driver_get(self->vf_bdf);
+	ASSERT_NE(vf_driver, NULL);
+	ASSERT_EQ(strcmp(vf_driver, "vfio-pci"), 0);
+	free(vf_driver);
+
+	printf("Created 1 VF (%s) under the PF: %s\n", self->vf_bdf, pf_bdf);
+}
+
+FIXTURE_TEARDOWN(vfio_pci_sriov_uapi_test)
+{
+	free(self->vf_bdf);
+	sysfs_sriov_numvfs_set(pf_bdf, 0);
+}
+
+FIXTURE_VARIANT(vfio_pci_sriov_uapi_test) {
+	const char *iommu_mode;
+	char *vf_token;
+};
+
+#define FIXTURE_VARIANT_ADD_IOMMU_MODE(_iommu_mode, _name, _vf_token)		\
+FIXTURE_VARIANT_ADD(vfio_pci_sriov_uapi_test, _iommu_mode ## _ ## _name) {	\
+	.iommu_mode = #_iommu_mode,						\
+	.vf_token = (_vf_token),						\
+}
+
+FIXTURE_VARIANT_ADD_ALL_IOMMU_MODES(same_uuid, UUID_1);
+FIXTURE_VARIANT_ADD_ALL_IOMMU_MODES(diff_uuid, UUID_2);
+FIXTURE_VARIANT_ADD_ALL_IOMMU_MODES(null_uuid, NULL);
+
+/*
+ * PF's token is always set with UUID_1 and VF's token is rotated with
+ * various tokens (including UUID_1 and NULL).
+ * This asserts if the VF device is successfully created for a match
+ * in the token or actually fails during a mismatch.
+ */
+#define ASSERT_VF_CREATION(_ret) do {					\
+	if (!variant->vf_token || strcmp(UUID_1, variant->vf_token)) {	\
+		ASSERT_NE((_ret), 0);					\
+	} else {							\
+		ASSERT_EQ((_ret), 0);					\
+	}								\
+} while (0)
+
+/*
+ * Validate if the UAPI handles correctly and incorrectly set token on the VF.
+ */
+TEST_F(vfio_pci_sriov_uapi_test, init_token_match)
+{
+	struct vfio_pci_device *pf;
+	struct vfio_pci_device *vf;
+	struct iommu *iommu;
+	int ret;
+
+	iommu = iommu_init(variant->iommu_mode);
+	pf = device_init(pf_bdf, iommu, UUID_1, &ret);
+	vf = device_init(self->vf_bdf, iommu, variant->vf_token, &ret);
+
+	ASSERT_VF_CREATION(ret);
+
+	device_cleanup(vf);
+	device_cleanup(pf);
+	iommu_cleanup(iommu);
+}
+
+/*
+ * After setting a token on the PF, validate if the VF can still set the
+ * expected token.
+ */
+TEST_F(vfio_pci_sriov_uapi_test, pf_early_close)
+{
+	struct vfio_pci_device *pf;
+	struct vfio_pci_device *vf;
+	struct iommu *iommu;
+	int ret;
+
+	iommu = iommu_init(variant->iommu_mode);
+	pf = device_init(pf_bdf, iommu, UUID_1, &ret);
+	device_cleanup(pf);
+
+	vf = device_init(self->vf_bdf, iommu, variant->vf_token, &ret);
+
+	ASSERT_VF_CREATION(ret);
+
+	device_cleanup(vf);
+	iommu_cleanup(iommu);
+}
+
+/*
+ * After PF device init, override the existing token and validate if the newly
+ * set token is the one that's active.
+ */
+TEST_F(vfio_pci_sriov_uapi_test, override_token)
+{
+	struct vfio_pci_device *pf;
+	struct vfio_pci_device *vf;
+	struct iommu *iommu;
+	int ret;
+
+	iommu = iommu_init(variant->iommu_mode);
+	pf = device_init(pf_bdf, iommu, UUID_2, &ret);
+	vfio_device_set_vf_token(pf->fd, UUID_1);
+
+	vf = device_init(self->vf_bdf, iommu, variant->vf_token, &ret);
+
+	ASSERT_VF_CREATION(ret);
+
+	device_cleanup(vf);
+	device_cleanup(pf);
+	iommu_cleanup(iommu);
+}
+
+int main(int argc, char *argv[])
+{
+	pf_bdf = vfio_selftests_get_bdf(&argc, argv);
+	return test_harness_run(argc, argv);
+}
-- 
2.53.0.473.g4a7958ca14-goog

On 2026-03-03 19:38:22, Raghavendra Rao Ananta wrote:
> +static int container_setup(struct vfio_pci_device *device, const char *bdf,
> +			   const char *vf_token)
> +{
> +	vfio_pci_group_setup(device, bdf);
> +	vfio_container_set_iommu(device);
> +	__vfio_pci_group_get_device_fd(device, bdf, vf_token);
> +
> +	/* The device fd will be -1 in case of mismatched tokens */
> +	return (device->fd < 0);
> +}
> +

This is one is exactly similar to vfio_pci_container_setup() except you
are calling __vfio_pci_group_get_device_fd() which doesn't do assertion.

Any reason to not create __vfio_pci_container_setup() in the library and
just write assertion in vfio_pci_container_setup()? 

> +static int iommufd_setup(struct vfio_pci_device *device, const char *bdf,
> +			 const char *vf_token)
> +{
> +	vfio_pci_cdev_open(device, bdf);
> +	return __vfio_device_bind_iommufd(device->fd,
> +					  device->iommu->iommufd, vf_token);
> +}

I see these are also similar to the code in library with some
differences. May be we can refactor library code and reuse it here.

One option can be to split vfio_pci_iommufd_setup() in two parts where
first part is what your are doing above and second part is attaching PT.

> +
> +static struct vfio_pci_device *device_init(const char *bdf, struct iommu *iommu,
> +					   const char *vf_token, int *out_ret)
> +{
> +	struct vfio_pci_device *device = vfio_pci_device_alloc(bdf, iommu);
> +
> +	if (iommu->mode->container_path)
> +		*out_ret = container_setup(device, bdf, vf_token);
> +	else
> +		*out_ret = iommufd_setup(device, bdf, vf_token);
> +
> +	return device;
> +}

Same for this one, vfio_pci_device_init() can be composed of two parts,
first one being alloc and container/iommufd setup and second device
setup and driver probe.

My reasoning for this is to avoid having separate code for these common
flows and avoid them diverging down the line.

> +
> +static void device_cleanup(struct vfio_pci_device *device)
> +{
> +	if (device->fd > 0)
> +		VFIO_ASSERT_EQ(close(device->fd), 0);
> +
> +	if (device->group_fd)
> +		VFIO_ASSERT_EQ(close(device->group_fd), 0);
> +
> +	vfio_pci_device_free(device);
> +}
> +
> +FIXTURE(vfio_pci_sriov_uapi_test) {
> +	char *vf_bdf;

Nit: Can we just use an array like char vf_bdf[16]? 
Just not sure why we need to do alloc and free for each run.

> +};
> +
> +FIXTURE_SETUP(vfio_pci_sriov_uapi_test)
> +{
> +	char *vf_driver;
> +	int nr_vfs;
> +
> +	nr_vfs = sysfs_sriov_totalvfs_get(pf_bdf);
> +	if (nr_vfs <= 0)
> +		SKIP(return, "SR-IOV may not be supported by the PF: %s\n", pf_bdf);
> +
> +	nr_vfs = sysfs_sriov_numvfs_get(pf_bdf);
> +	if (nr_vfs != 0)
> +		SKIP(return, "SR-IOV already configured for the PF: %s\n", pf_bdf);
> +

If there is a test failure (ASSERT) then fixture cleanup will not run
leaving SR-IOV enabled and all subsequent tests will skip. 

Since this test is specific to SR-IOV and user is intentionally passing
a device to test, I think we can just reset the VFs to 0 before
proceeding instead of skipping.

> +	/* Create only one VF for testing */
> +	sysfs_sriov_numvfs_set(pf_bdf, 1);
> +	self->vf_bdf = sysfs_sriov_vf_bdf_get(pf_bdf, 0);
> +
> +	/*
> +	 * The VF inherits the driver from the PF.
> +	 * Ensure this is 'vfio-pci' before proceeding.
> +	 */
> +	vf_driver = sysfs_driver_get(self->vf_bdf);
> +	ASSERT_NE(vf_driver, NULL);
> +	ASSERT_EQ(strcmp(vf_driver, "vfio-pci"), 0);
> +	free(vf_driver);
> +
> +	printf("Created 1 VF (%s) under the PF: %s\n", self->vf_bdf, pf_bdf);
> +}
> +
> +FIXTURE_TEARDOWN(vfio_pci_sriov_uapi_test)
> +{
> +	free(self->vf_bdf);
> +	sysfs_sriov_numvfs_set(pf_bdf, 0);
> +}
> +
> +FIXTURE_VARIANT(vfio_pci_sriov_uapi_test) {
> +	const char *iommu_mode;
> +	char *vf_token;
> +};
> +
> +#define FIXTURE_VARIANT_ADD_IOMMU_MODE(_iommu_mode, _name, _vf_token)		\
> +FIXTURE_VARIANT_ADD(vfio_pci_sriov_uapi_test, _iommu_mode ## _ ## _name) {	\
> +	.iommu_mode = #_iommu_mode,						\
> +	.vf_token = (_vf_token),						\
> +}
> +
> +FIXTURE_VARIANT_ADD_ALL_IOMMU_MODES(same_uuid, UUID_1);
> +FIXTURE_VARIANT_ADD_ALL_IOMMU_MODES(diff_uuid, UUID_2);
> +FIXTURE_VARIANT_ADD_ALL_IOMMU_MODES(null_uuid, NULL);
> +
> +/*
> + * PF's token is always set with UUID_1 and VF's token is rotated with
> + * various tokens (including UUID_1 and NULL).
> + * This asserts if the VF device is successfully created for a match
> + * in the token or actually fails during a mismatch.
> + */
> +#define ASSERT_VF_CREATION(_ret) do {					\
> +	if (!variant->vf_token || strcmp(UUID_1, variant->vf_token)) {	\
> +		ASSERT_NE((_ret), 0);					\
> +	} else {							\
> +		ASSERT_EQ((_ret), 0);					\
> +	}								\
> +} while (0)
> +
> +/*
> + * Validate if the UAPI handles correctly and incorrectly set token on the VF.
> + */
> +TEST_F(vfio_pci_sriov_uapi_test, init_token_match)
> +{
> +	struct vfio_pci_device *pf;
> +	struct vfio_pci_device *vf;
> +	struct iommu *iommu;
> +	int ret;
> +
> +	iommu = iommu_init(variant->iommu_mode);

Can this and cleanup go in fixture setup?

> +	pf = device_init(pf_bdf, iommu, UUID_1, &ret);

We should assert here as well and in other functions.

> +	vf = device_init(self->vf_bdf, iommu, variant->vf_token, &ret);
> +
> +	ASSERT_VF_CREATION(ret);
> +
> +	device_cleanup(vf);
> +	device_cleanup(pf);
> +	iommu_cleanup(iommu);
> +}
> +
> +/*
> + * After setting a token on the PF, validate if the VF can still set the
> + * expected token.
> + */
> +TEST_F(vfio_pci_sriov_uapi_test, pf_early_close)
> +{
> +	struct vfio_pci_device *pf;
> +	struct vfio_pci_device *vf;
> +	struct iommu *iommu;
> +	int ret;
> +
> +	iommu = iommu_init(variant->iommu_mode);
> +	pf = device_init(pf_bdf, iommu, UUID_1, &ret);
> +	device_cleanup(pf);
> +
> +	vf = device_init(self->vf_bdf, iommu, variant->vf_token, &ret);
> +
> +	ASSERT_VF_CREATION(ret);
> +
> +	device_cleanup(vf);
> +	iommu_cleanup(iommu);
> +}
> +
> +/*
> + * After PF device init, override the existing token and validate if the newly
> + * set token is the one that's active.
> + */
> +TEST_F(vfio_pci_sriov_uapi_test, override_token)
> +{
> +	struct vfio_pci_device *pf;
> +	struct vfio_pci_device *vf;
> +	struct iommu *iommu;
> +	int ret;
> +
> +	iommu = iommu_init(variant->iommu_mode);
> +	pf = device_init(pf_bdf, iommu, UUID_2, &ret);
> +	vfio_device_set_vf_token(pf->fd, UUID_1);
> +
> +	vf = device_init(self->vf_bdf, iommu, variant->vf_token, &ret);
> +
> +	ASSERT_VF_CREATION(ret);
> +
> +	device_cleanup(vf);
> +	device_cleanup(pf);
> +	iommu_cleanup(iommu);
> +}
> +
> +int main(int argc, char *argv[])
> +{
> +	pf_bdf = vfio_selftests_get_bdf(&argc, argv);
> +	return test_harness_run(argc, argv);
> +}
> -- 
> 2.53.0.473.g4a7958ca14-goog
>

On Wed, Mar 11, 2026 at 4:57 PM Vipin Sharma <vipinsh@google.com> wrote:
>
> On 2026-03-03 19:38:22, Raghavendra Rao Ananta wrote:
> > +static int container_setup(struct vfio_pci_device *device, const char *bdf,
> > +                        const char *vf_token)
> > +{
> > +     vfio_pci_group_setup(device, bdf);
> > +     vfio_container_set_iommu(device);
> > +     __vfio_pci_group_get_device_fd(device, bdf, vf_token);
> > +
> > +     /* The device fd will be -1 in case of mismatched tokens */
> > +     return (device->fd < 0);
> > +}
> > +
>
> This is one is exactly similar to vfio_pci_container_setup() except you
> are calling __vfio_pci_group_get_device_fd() which doesn't do assertion.
>
> Any reason to not create __vfio_pci_container_setup() in the library and
> just write assertion in vfio_pci_container_setup()?
>
Yes, the test is specifically interested in skipping the 'device->fd'
assertion out of the many assertions that vfio_pci_container_setup()
performs. Hence, there's no point in creating
__vfio_pci_container_setup() that skips only this part, as it could be
confusing. If there are more usecases of similar assertions being
skipped, we can consider implementing the functions as needed in the
lib.

> > +static int iommufd_setup(struct vfio_pci_device *device, const char *bdf,
> > +                      const char *vf_token)
> > +{
> > +     vfio_pci_cdev_open(device, bdf);
> > +     return __vfio_device_bind_iommufd(device->fd,
> > +                                       device->iommu->iommufd, vf_token);
> > +}
>
> I see these are also similar to the code in library with some
> differences. May be we can refactor library code and reuse it here.
>
> One option can be to split vfio_pci_iommufd_setup() in two parts where
> first part is what your are doing above and second part is attaching PT.
>
I think my above statement applies here too. Moreover,
vfio_pci_iommufd_setup() seems to integrate the three calls really
well, even in terms of readability. Splitting out only
vfio_device_attach_iommufd_pt() (or an other function) just for the
sake of this test didn't make much sense. Hence, I retained the flow.
But again, if we have usecases in the future, we can split it.

> > +
> > +static struct vfio_pci_device *device_init(const char *bdf, struct iommu *iommu,
> > +                                        const char *vf_token, int *out_ret)
> > +{
> > +     struct vfio_pci_device *device = vfio_pci_device_alloc(bdf, iommu);
> > +
> > +     if (iommu->mode->container_path)
> > +             *out_ret = container_setup(device, bdf, vf_token);
> > +     else
> > +             *out_ret = iommufd_setup(device, bdf, vf_token);
> > +
> > +     return device;
> > +}
>
> Same for this one, vfio_pci_device_init() can be composed of two parts,
> first one being alloc and container/iommufd setup and second device
> setup and driver probe.
>
> My reasoning for this is to avoid having separate code for these common
> flows and avoid them diverging down the line.
>
Similar comment to the one above. I avoided refactoring the library
code just to make one-off assertions relevant only to this test.

> > +
> > +static void device_cleanup(struct vfio_pci_device *device)
> > +{
> > +     if (device->fd > 0)
> > +             VFIO_ASSERT_EQ(close(device->fd), 0);
> > +
> > +     if (device->group_fd)
> > +             VFIO_ASSERT_EQ(close(device->group_fd), 0);
> > +
> > +     vfio_pci_device_free(device);
> > +}
> > +
> > +FIXTURE(vfio_pci_sriov_uapi_test) {
> > +     char *vf_bdf;
>
> Nit: Can we just use an array like char vf_bdf[16]?
> Just not sure why we need to do alloc and free for each run.
>
We can. In fact, I initially had that (for getting the driver path
though in v2), but this felt a bit cleaner (suggested by David) and
similar to how we handle most kernel allocations. This way the caller
doesn't have to guess a size, while the library knows an appropriate
size.

> > +};
> > +
> > +FIXTURE_SETUP(vfio_pci_sriov_uapi_test)
> > +{
> > +     char *vf_driver;
> > +     int nr_vfs;
> > +
> > +     nr_vfs = sysfs_sriov_totalvfs_get(pf_bdf);
> > +     if (nr_vfs <= 0)
> > +             SKIP(return, "SR-IOV may not be supported by the PF: %s\n", pf_bdf);
> > +
> > +     nr_vfs = sysfs_sriov_numvfs_get(pf_bdf);
> > +     if (nr_vfs != 0)
> > +             SKIP(return, "SR-IOV already configured for the PF: %s\n", pf_bdf);
> > +
>
> If there is a test failure (ASSERT) then fixture cleanup will not run
> leaving SR-IOV enabled and all subsequent tests will skip.
>
> Since this test is specific to SR-IOV and user is intentionally passing
> a device to test, I think we can just reset the VFs to 0 before
> proceeding instead of skipping.
>
The idea was to eliminate the assumption that if 'sriov_numvfs'
returns > 0, someone else might be using it. This is based on David's
suggestion: https://lore.kernel.org/all/aQzcQ0fJd-aCRThS@google.com/ .
Moreover, the device might be in a partial state. So, even if the next
test proceeds, it could fail due to other reasons.

But I see your point about cleanup on failure. I think this is a
common issue across all tests. Was the expectation that we rely on
cleanup.sh to take care of all the cleanups, regardless of test
pass/failure?

> > +     /* Create only one VF for testing */
> > +     sysfs_sriov_numvfs_set(pf_bdf, 1);
> > +     self->vf_bdf = sysfs_sriov_vf_bdf_get(pf_bdf, 0);
> > +
> > +     /*
> > +      * The VF inherits the driver from the PF.
> > +      * Ensure this is 'vfio-pci' before proceeding.
> > +      */
> > +     vf_driver = sysfs_driver_get(self->vf_bdf);
> > +     ASSERT_NE(vf_driver, NULL);
> > +     ASSERT_EQ(strcmp(vf_driver, "vfio-pci"), 0);
> > +     free(vf_driver);
> > +
> > +     printf("Created 1 VF (%s) under the PF: %s\n", self->vf_bdf, pf_bdf);
> > +}
> > +
> > +FIXTURE_TEARDOWN(vfio_pci_sriov_uapi_test)
> > +{
> > +     free(self->vf_bdf);
> > +     sysfs_sriov_numvfs_set(pf_bdf, 0);
> > +}
> > +
> > +FIXTURE_VARIANT(vfio_pci_sriov_uapi_test) {
> > +     const char *iommu_mode;
> > +     char *vf_token;
> > +};
> > +
> > +#define FIXTURE_VARIANT_ADD_IOMMU_MODE(_iommu_mode, _name, _vf_token)                \
> > +FIXTURE_VARIANT_ADD(vfio_pci_sriov_uapi_test, _iommu_mode ## _ ## _name) {   \
> > +     .iommu_mode = #_iommu_mode,                                             \
> > +     .vf_token = (_vf_token),                                                \
> > +}
> > +
> > +FIXTURE_VARIANT_ADD_ALL_IOMMU_MODES(same_uuid, UUID_1);
> > +FIXTURE_VARIANT_ADD_ALL_IOMMU_MODES(diff_uuid, UUID_2);
> > +FIXTURE_VARIANT_ADD_ALL_IOMMU_MODES(null_uuid, NULL);
> > +
> > +/*
> > + * PF's token is always set with UUID_1 and VF's token is rotated with
> > + * various tokens (including UUID_1 and NULL).
> > + * This asserts if the VF device is successfully created for a match
> > + * in the token or actually fails during a mismatch.
> > + */
> > +#define ASSERT_VF_CREATION(_ret) do {                                        \
> > +     if (!variant->vf_token || strcmp(UUID_1, variant->vf_token)) {  \
> > +             ASSERT_NE((_ret), 0);                                   \
> > +     } else {                                                        \
> > +             ASSERT_EQ((_ret), 0);                                   \
> > +     }                                                               \
> > +} while (0)
> > +
> > +/*
> > + * Validate if the UAPI handles correctly and incorrectly set token on the VF.
> > + */
> > +TEST_F(vfio_pci_sriov_uapi_test, init_token_match)
> > +{
> > +     struct vfio_pci_device *pf;
> > +     struct vfio_pci_device *vf;
> > +     struct iommu *iommu;
> > +     int ret;
> > +
> > +     iommu = iommu_init(variant->iommu_mode);
>
> Can this and cleanup go in fixture setup?
>
Isn't 'variant' inaccessible from FIXUTURE_SETUP()?

> > +     pf = device_init(pf_bdf, iommu, UUID_1, &ret);
>
> We should assert here as well and in other functions.
>
Good point. I'll add it in v7.

Thank you.
Raghavendra

On Tue, Mar 31, 2026 at 4:53 PM Raghavendra Rao Ananta
<rananta@google.com> wrote:

> > > +FIXTURE_SETUP(vfio_pci_sriov_uapi_test)
> > > +{
> > > +     char *vf_driver;
> > > +     int nr_vfs;
> > > +
> > > +     nr_vfs = sysfs_sriov_totalvfs_get(pf_bdf);
> > > +     if (nr_vfs <= 0)
> > > +             SKIP(return, "SR-IOV may not be supported by the PF: %s\n", pf_bdf);
> > > +
> > > +     nr_vfs = sysfs_sriov_numvfs_get(pf_bdf);
> > > +     if (nr_vfs != 0)
> > > +             SKIP(return, "SR-IOV already configured for the PF: %s\n", pf_bdf);
> > > +
> >
> > If there is a test failure (ASSERT) then fixture cleanup will not run
> > leaving SR-IOV enabled and all subsequent tests will skip.
> >
> > Since this test is specific to SR-IOV and user is intentionally passing
> > a device to test, I think we can just reset the VFs to 0 before
> > proceeding instead of skipping.
> >
> The idea was to eliminate the assumption that if 'sriov_numvfs'
> returns > 0, someone else might be using it. This is based on David's
> suggestion: https://lore.kernel.org/all/aQzcQ0fJd-aCRThS@google.com/ .
> Moreover, the device might be in a partial state. So, even if the next
> test proceeds, it could fail due to other reasons.
>
> But I see your point about cleanup on failure. I think this is a
> common issue across all tests. Was the expectation that we rely on
> cleanup.sh to take care of all the cleanups, regardless of test
> pass/failure?

I think we rely on the fact that test cases run in sub-processes and
so if there is a failure in fixture setup then the child process exits
and all vfio and iommufd fds are cleaned up. So the device is still
"clean" for the next test.

However if we are modifying system state like the number of VFs
enabled on a device, I think Vipin is right we need to clean up after
ourselves if the error occurs in fixture setup.

On Tue, Mar 31, 2026 at 5:11 PM David Matlack <dmatlack@google.com> wrote:
>
> On Tue, Mar 31, 2026 at 4:53 PM Raghavendra Rao Ananta
> <rananta@google.com> wrote:
>
> > > > +FIXTURE_SETUP(vfio_pci_sriov_uapi_test)
> > > > +{
> > > > +     char *vf_driver;
> > > > +     int nr_vfs;
> > > > +
> > > > +     nr_vfs = sysfs_sriov_totalvfs_get(pf_bdf);
> > > > +     if (nr_vfs <= 0)
> > > > +             SKIP(return, "SR-IOV may not be supported by the PF: %s\n", pf_bdf);
> > > > +
> > > > +     nr_vfs = sysfs_sriov_numvfs_get(pf_bdf);
> > > > +     if (nr_vfs != 0)
> > > > +             SKIP(return, "SR-IOV already configured for the PF: %s\n", pf_bdf);
> > > > +
> > >
> > > If there is a test failure (ASSERT) then fixture cleanup will not run
> > > leaving SR-IOV enabled and all subsequent tests will skip.
> > >
> > > Since this test is specific to SR-IOV and user is intentionally passing
> > > a device to test, I think we can just reset the VFs to 0 before
> > > proceeding instead of skipping.
> > >
> > The idea was to eliminate the assumption that if 'sriov_numvfs'
> > returns > 0, someone else might be using it. This is based on David's
> > suggestion: https://lore.kernel.org/all/aQzcQ0fJd-aCRThS@google.com/ .
> > Moreover, the device might be in a partial state. So, even if the next
> > test proceeds, it could fail due to other reasons.
> >
> > But I see your point about cleanup on failure. I think this is a
> > common issue across all tests. Was the expectation that we rely on
> > cleanup.sh to take care of all the cleanups, regardless of test
> > pass/failure?
>
> I think we rely on the fact that test cases run in sub-processes and
> so if there is a failure in fixture setup then the child process exits
> and all vfio and iommufd fds are cleaned up. So the device is still
> "clean" for the next test.
>
> However if we are modifying system state like the number of VFs
> enabled on a device, I think Vipin is right we need to clean up after
> ourselves if the error occurs in fixture setup.

In that case, since the FIXTURE_TEARDOWN() is not invoked for a
failure in FIXTURE_SETUP(), the test must install its own cleaning
handler. Using conventional error handling with goto labels might also
be tricky because the sysfs lib contains ASSERTs. Even if we do reset
sriov_nrvfs and exit, since the failure is in FIXTURE_SETUP(), it's
highly likely that it's going to fail again.

Since we need the VF creation only once, shall we simply do this setup
in main()?

int main()
{
  pf_bdf = vfio_selftests_get_bdf(&argc, argv);
  vf_bdf = setup_vf(pf_bdf); // Do all the sysfs stuff here
  ret = test_harness_run(argc, argv);
  destroy_vf();

  return ret;
}

Another possible way is to expose the VF from setup.sh itself and pass
it as an arg to the test.

WDYT?

Thank you.
Raghavendra

On Wed, Apr 1, 2026 at 4:46 PM Raghavendra Rao Ananta
<rananta@google.com> wrote:
>
> On Tue, Mar 31, 2026 at 5:11 PM David Matlack <dmatlack@google.com> wrote:
> >
> > On Tue, Mar 31, 2026 at 4:53 PM Raghavendra Rao Ananta
> > <rananta@google.com> wrote:
> >
> > > > > +FIXTURE_SETUP(vfio_pci_sriov_uapi_test)
> > > > > +{
> > > > > +     char *vf_driver;
> > > > > +     int nr_vfs;
> > > > > +
> > > > > +     nr_vfs = sysfs_sriov_totalvfs_get(pf_bdf);
> > > > > +     if (nr_vfs <= 0)
> > > > > +             SKIP(return, "SR-IOV may not be supported by the PF: %s\n", pf_bdf);
> > > > > +
> > > > > +     nr_vfs = sysfs_sriov_numvfs_get(pf_bdf);
> > > > > +     if (nr_vfs != 0)
> > > > > +             SKIP(return, "SR-IOV already configured for the PF: %s\n", pf_bdf);
> > > > > +
> > > >
> > > > If there is a test failure (ASSERT) then fixture cleanup will not run
> > > > leaving SR-IOV enabled and all subsequent tests will skip.
> > > >
> > > > Since this test is specific to SR-IOV and user is intentionally passing
> > > > a device to test, I think we can just reset the VFs to 0 before
> > > > proceeding instead of skipping.
> > > >
> > > The idea was to eliminate the assumption that if 'sriov_numvfs'
> > > returns > 0, someone else might be using it. This is based on David's
> > > suggestion: https://lore.kernel.org/all/aQzcQ0fJd-aCRThS@google.com/ .
> > > Moreover, the device might be in a partial state. So, even if the next
> > > test proceeds, it could fail due to other reasons.
> > >
> > > But I see your point about cleanup on failure. I think this is a
> > > common issue across all tests. Was the expectation that we rely on
> > > cleanup.sh to take care of all the cleanups, regardless of test
> > > pass/failure?
> >
> > I think we rely on the fact that test cases run in sub-processes and
> > so if there is a failure in fixture setup then the child process exits
> > and all vfio and iommufd fds are cleaned up. So the device is still
> > "clean" for the next test.
> >
> > However if we are modifying system state like the number of VFs
> > enabled on a device, I think Vipin is right we need to clean up after
> > ourselves if the error occurs in fixture setup.
>
> In that case, since the FIXTURE_TEARDOWN() is not invoked for a
> failure in FIXTURE_SETUP(), the test must install its own cleaning
> handler. Using conventional error handling with goto labels might also
> be tricky because the sysfs lib contains ASSERTs. Even if we do reset
> sriov_nrvfs and exit, since the failure is in FIXTURE_SETUP(), it's
> highly likely that it's going to fail again.
>
> Since we need the VF creation only once, shall we simply do this setup
> in main()?
>
> int main()
> {
>   pf_bdf = vfio_selftests_get_bdf(&argc, argv);
>   vf_bdf = setup_vf(pf_bdf); // Do all the sysfs stuff here
>   ret = test_harness_run(argc, argv);
>   destroy_vf();
>
>   return ret;
> }
>
> Another possible way is to expose the VF from setup.sh itself and pass
> it as an arg to the test.
>
> WDYT?

Creating and destroying VFs in main() outside of the test harness
sounds like a good solution to me.

On Wed, Apr 1, 2026 at 4:51 PM David Matlack <dmatlack@google.com> wrote:
>
> On Wed, Apr 1, 2026 at 4:46 PM Raghavendra Rao Ananta
> <rananta@google.com> wrote:
> >
> > On Tue, Mar 31, 2026 at 5:11 PM David Matlack <dmatlack@google.com> wrote:
> > >
> > > On Tue, Mar 31, 2026 at 4:53 PM Raghavendra Rao Ananta
> > > <rananta@google.com> wrote:
> > >
> > > > > > +FIXTURE_SETUP(vfio_pci_sriov_uapi_test)
> > > > > > +{
> > > > > > +     char *vf_driver;
> > > > > > +     int nr_vfs;
> > > > > > +
> > > > > > +     nr_vfs = sysfs_sriov_totalvfs_get(pf_bdf);
> > > > > > +     if (nr_vfs <= 0)
> > > > > > +             SKIP(return, "SR-IOV may not be supported by the PF: %s\n", pf_bdf);
> > > > > > +
> > > > > > +     nr_vfs = sysfs_sriov_numvfs_get(pf_bdf);
> > > > > > +     if (nr_vfs != 0)
> > > > > > +             SKIP(return, "SR-IOV already configured for the PF: %s\n", pf_bdf);
> > > > > > +
> > > > >
> > > > > If there is a test failure (ASSERT) then fixture cleanup will not run
> > > > > leaving SR-IOV enabled and all subsequent tests will skip.
> > > > >
> > > > > Since this test is specific to SR-IOV and user is intentionally passing
> > > > > a device to test, I think we can just reset the VFs to 0 before
> > > > > proceeding instead of skipping.
> > > > >
> > > > The idea was to eliminate the assumption that if 'sriov_numvfs'
> > > > returns > 0, someone else might be using it. This is based on David's
> > > > suggestion: https://lore.kernel.org/all/aQzcQ0fJd-aCRThS@google.com/ .
> > > > Moreover, the device might be in a partial state. So, even if the next
> > > > test proceeds, it could fail due to other reasons.
> > > >
> > > > But I see your point about cleanup on failure. I think this is a
> > > > common issue across all tests. Was the expectation that we rely on
> > > > cleanup.sh to take care of all the cleanups, regardless of test
> > > > pass/failure?
> > >
> > > I think we rely on the fact that test cases run in sub-processes and
> > > so if there is a failure in fixture setup then the child process exits
> > > and all vfio and iommufd fds are cleaned up. So the device is still
> > > "clean" for the next test.
> > >
> > > However if we are modifying system state like the number of VFs
> > > enabled on a device, I think Vipin is right we need to clean up after
> > > ourselves if the error occurs in fixture setup.
> >
> > In that case, since the FIXTURE_TEARDOWN() is not invoked for a
> > failure in FIXTURE_SETUP(), the test must install its own cleaning
> > handler. Using conventional error handling with goto labels might also
> > be tricky because the sysfs lib contains ASSERTs. Even if we do reset
> > sriov_nrvfs and exit, since the failure is in FIXTURE_SETUP(), it's
> > highly likely that it's going to fail again.
> >
> > Since we need the VF creation only once, shall we simply do this setup
> > in main()?
> >
> > int main()
> > {
> >   pf_bdf = vfio_selftests_get_bdf(&argc, argv);
> >   vf_bdf = setup_vf(pf_bdf); // Do all the sysfs stuff here
> >   ret = test_harness_run(argc, argv);
> >   destroy_vf();
> >
> >   return ret;
> > }
> >
> > Another possible way is to expose the VF from setup.sh itself and pass
> > it as an arg to the test.
> >
> > WDYT?
>
> Creating and destroying VFs in main() outside of the test harness
> sounds like a good solution to me.

Gah.. thinking this through, since we use ASSERTs in sysfs setup, we'd
still have problems with 'goto label' like error handling. If it's not
getting too complicated, I can set up an exit handler (say via
exitat()) and destroy VFs there.