public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/6] vfio: selftest: Add SR-IOV UAPI test
@ 2025-12-10 18:14 Raghavendra Rao Ananta
  2025-12-10 18:14 ` [PATCH v2 1/6] vfio: selftests: Introduce snprintf_assert() Raghavendra Rao Ananta
                   ` (5 more replies)
  0 siblings, 6 replies; 28+ messages in thread
From: Raghavendra Rao Ananta @ 2025-12-10 18:14 UTC (permalink / raw)
  To: David Matlack, Alex Williamson, Alex Williamson
  Cc: Josh Hilke, kvm, linux-kernel, Raghavendra Rao Ananta

Hello,

This series adds a vfio selftest, vfio_pci_sriov_uapi_test.c, to get some
coverage on SR-IOV UAPI handling. Specifically, it includes the
following cases that iterates over all the iommu modes:
 - 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.

The test takes care of creating/setting up the VF device, and hence, it
can be executed like any other test, simply by passing the PF's BDF to
run.sh. For example,

$ ./scripts/setup.sh 0000:16:00.1
$ ./scripts/run.sh ./vfio_pci_sriov_uapi_test

TAP version 13
1..45
Starting 45 tests from 15 test cases.
  RUN  vfio_pci_sriov_uapi_test.vfio_type1_iommu_same_uuid.init_token_match
    OK vfio_pci_sriov_uapi_test.vfio_type1_iommu_same_uuid.init_token_match
ok 1 vfio_pci_sriov_uapi_test.vfio_type1_iommu_same_uuid.init_token_match
  RUN vfio_pci_sriov_uapi_test.vfio_type1_iommu_same_uuid.pf_early_close
   OK vfio_pci_sriov_uapi_test.vfio_type1_iommu_same_uuid.pf_early_close
ok 2 vfio_pci_sriov_uapi_test.vfio_type1_iommu_same_uuid.pf_early_close
  RUN vfio_pci_sriov_uapi_test.vfio_type1_iommu_same_uuid.override_token
   OK vfio_pci_sriov_uapi_test.vfio_type1_iommu_same_uuid.override_token
[...]
  RUN vfio_pci_sriov_uapi_test.iommufd_null_uuid.override_token ...
   OK vfio_pci_sriov_uapi_test.iommufd_null_uuid.override_token
ok 45 vfio_pci_sriov_uapi_test.iommufd_null_uuid.override_token
PASSED: 45 / 45 tests passed.

Thank you.
Raghavendra

v2: Suggestions by David Matlack (thank you)
 - Introduce snprintf_assert() to check against content trucation.
 - Introduce a new sysfs library to handle all the common vfio/pci sysfs
   operations.
 - Rename vfio_pci_container_get_device_fd() to
   vfio_pci_group_get_device_fd().
 - Use a fixed size 'arg' array instead of dynamic allocation in
   __vfio_pci_group_get_device_fd().
 - Exclude vfio_pci_device_init() to accept the 'vf_token' arg.
 - Move the vfio_pci_sriov_uapi_test.c global variable to the FIXTURE()
   struct or as TEST_F() local variables.
 - test_vfio_pci_container_setup() returns 'int' to indicate status.
 - Skip the test if nr_vfs != 0.
 - Explicitly set "sriov_drivers_autoprobe" for the PF.
 - Make sure to bind the VF device to the "vfio-pci" driver.
 - Cleanup the things done by FIXTURE_SETUP() in FIXTURE_TEARDOWN().

v1: https://lore.kernel.org/all/20251104003536.3601931-1-rananta@google.com/

Raghavendra Rao Ananta (6):
  vfio: selftests: Introduce snprintf_assert()
  vfio: selftests: Introduce a sysfs lib
  vfio: selftests: Extend container/iommufd setup for passing vf_token
  vfio: selftests: Export more vfio_pci functions
  vfio: selftests: Add helper to set/override a vf_token
  vfio: selftests: Add tests to validate SR-IOV UAPI

 tools/testing/selftests/vfio/Makefile         |   1 +
 .../selftests/vfio/lib/include/libvfio.h      |   1 +
 .../vfio/lib/include/libvfio/assert.h         |   5 +
 .../vfio/lib/include/libvfio/sysfs.h          |  16 ++
 .../lib/include/libvfio/vfio_pci_device.h     |   9 +
 tools/testing/selftests/vfio/lib/libvfio.mk   |   5 +-
 tools/testing/selftests/vfio/lib/sysfs.c      | 151 ++++++++++++
 .../selftests/vfio/lib/vfio_pci_device.c      | 135 ++++++++---
 .../selftests/vfio/vfio_dma_mapping_test.c    |   6 +-
 .../selftests/vfio/vfio_pci_device_test.c     |  21 +-
 .../selftests/vfio/vfio_pci_sriov_uapi_test.c | 215 ++++++++++++++++++
 11 files changed, 515 insertions(+), 50 deletions(-)
 create mode 100644 tools/testing/selftests/vfio/lib/include/libvfio/sysfs.h
 create mode 100644 tools/testing/selftests/vfio/lib/sysfs.c
 create mode 100644 tools/testing/selftests/vfio/vfio_pci_sriov_uapi_test.c


base-commit: d721f52e31553a848e0e9947ca15a49c5674aef3
--
2.52.0.239.gd5f0c6e74e-goog


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

* [PATCH v2 1/6] vfio: selftests: Introduce snprintf_assert()
  2025-12-10 18:14 [PATCH v2 0/6] vfio: selftest: Add SR-IOV UAPI test Raghavendra Rao Ananta
@ 2025-12-10 18:14 ` Raghavendra Rao Ananta
  2026-01-07 22:21   ` David Matlack
  2025-12-10 18:14 ` [PATCH v2 2/6] vfio: selftests: Introduce a sysfs lib Raghavendra Rao Ananta
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 28+ messages in thread
From: Raghavendra Rao Ananta @ 2025-12-10 18:14 UTC (permalink / raw)
  To: David Matlack, Alex Williamson, Alex Williamson
  Cc: Josh Hilke, kvm, linux-kernel, Raghavendra Rao Ananta

Introduce snprintf_assert() to protect the users of snprintf() to fail
if the requested operation was truncated due to buffer limits. VFIO
tests and libraries, including a new sysfs library that will be introduced
by an upcoming patch, rely quite heavily on snprintf()s to build PCI
sysfs paths. Having a protection against this will be helpful to prevent
false test failures.

Signed-off-by: Raghavendra Rao Ananta <rananta@google.com>
---
 .../vfio/lib/include/libvfio/assert.h         |  5 +++++
 .../selftests/vfio/lib/vfio_pci_device.c      |  8 +++----
 .../selftests/vfio/vfio_dma_mapping_test.c    |  6 +++---
 .../selftests/vfio/vfio_pci_device_test.c     | 21 ++++++++++---------
 4 files changed, 23 insertions(+), 17 deletions(-)

diff --git a/tools/testing/selftests/vfio/lib/include/libvfio/assert.h b/tools/testing/selftests/vfio/lib/include/libvfio/assert.h
index f4ebd122d9b6f..77b68c7129a64 100644
--- a/tools/testing/selftests/vfio/lib/include/libvfio/assert.h
+++ b/tools/testing/selftests/vfio/lib/include/libvfio/assert.h
@@ -51,4 +51,9 @@
 	VFIO_ASSERT_EQ(__ret, 0, "ioctl(%s, %s, %s) returned %d\n", #_fd, #_op, #_arg, __ret); \
 } while (0)
 
+#define snprintf_assert(_s, _size, _fmt, ...) do {                      \
+	int __ret = snprintf(_s, _size, _fmt, ##__VA_ARGS__);           \
+	VFIO_ASSERT_LT(__ret, _size);                                   \
+} while (0)
+
 #endif /* SELFTESTS_VFIO_LIB_INCLUDE_LIBVFIO_ASSERT_H */
diff --git a/tools/testing/selftests/vfio/lib/vfio_pci_device.c b/tools/testing/selftests/vfio/lib/vfio_pci_device.c
index 13fdb4b0b10f3..64a19481b734f 100644
--- a/tools/testing/selftests/vfio/lib/vfio_pci_device.c
+++ b/tools/testing/selftests/vfio/lib/vfio_pci_device.c
@@ -188,7 +188,7 @@ static unsigned int vfio_pci_get_group_from_dev(const char *bdf)
 	unsigned int group;
 	int ret;
 
-	snprintf(sysfs_path, PATH_MAX, "%s/%s/iommu_group", PCI_SYSFS_PATH, bdf);
+	snprintf_assert(sysfs_path, PATH_MAX, "%s/%s/iommu_group", PCI_SYSFS_PATH, bdf);
 
 	ret = readlink(sysfs_path, dev_iommu_group_path, sizeof(dev_iommu_group_path));
 	VFIO_ASSERT_NE(ret, -1, "Failed to get the IOMMU group for device: %s\n", bdf);
@@ -208,7 +208,7 @@ static void vfio_pci_group_setup(struct vfio_pci_device *device, const char *bdf
 	int group;
 
 	group = vfio_pci_get_group_from_dev(bdf);
-	snprintf(group_path, sizeof(group_path), "/dev/vfio/%d", group);
+	snprintf_assert(group_path, sizeof(group_path), "/dev/vfio/%d", group);
 
 	device->group_fd = open(group_path, O_RDWR);
 	VFIO_ASSERT_GE(device->group_fd, 0, "open(%s) failed\n", group_path);
@@ -279,7 +279,7 @@ const char *vfio_pci_get_cdev_path(const char *bdf)
 	cdev_path = calloc(PATH_MAX, 1);
 	VFIO_ASSERT_NOT_NULL(cdev_path);
 
-	snprintf(dir_path, sizeof(dir_path), "/sys/bus/pci/devices/%s/vfio-dev/", bdf);
+	snprintf_assert(dir_path, sizeof(dir_path), "/sys/bus/pci/devices/%s/vfio-dev/", bdf);
 
 	dir = opendir(dir_path);
 	VFIO_ASSERT_NOT_NULL(dir, "Failed to open directory %s\n", dir_path);
@@ -289,7 +289,7 @@ const char *vfio_pci_get_cdev_path(const char *bdf)
 		if (strncmp("vfio", entry->d_name, 4))
 			continue;
 
-		snprintf(cdev_path, PATH_MAX, "/dev/vfio/devices/%s", entry->d_name);
+		snprintf_assert(cdev_path, PATH_MAX, "/dev/vfio/devices/%s", entry->d_name);
 		break;
 	}
 
diff --git a/tools/testing/selftests/vfio/vfio_dma_mapping_test.c b/tools/testing/selftests/vfio/vfio_dma_mapping_test.c
index 5397822c3dd4b..3a0bea5e26480 100644
--- a/tools/testing/selftests/vfio/vfio_dma_mapping_test.c
+++ b/tools/testing/selftests/vfio/vfio_dma_mapping_test.c
@@ -45,9 +45,9 @@ static int intel_iommu_mapping_get(const char *bdf, u64 iova,
 	FILE *file;
 	char *rest;
 
-	snprintf(iommu_mapping_path, sizeof(iommu_mapping_path),
-		 "/sys/kernel/debug/iommu/intel/%s/domain_translation_struct",
-		 bdf);
+	snprintf_assert(iommu_mapping_path, sizeof(iommu_mapping_path),
+			"/sys/kernel/debug/iommu/intel/%s/domain_translation_struct",
+			bdf);
 
 	printf("Searching for IOVA 0x%lx in %s\n", iova, iommu_mapping_path);
 
diff --git a/tools/testing/selftests/vfio/vfio_pci_device_test.c b/tools/testing/selftests/vfio/vfio_pci_device_test.c
index ecbb669b37651..723b56b485f67 100644
--- a/tools/testing/selftests/vfio/vfio_pci_device_test.c
+++ b/tools/testing/selftests/vfio/vfio_pci_device_test.c
@@ -39,16 +39,17 @@ FIXTURE_TEARDOWN(vfio_pci_device_test)
 	iommu_cleanup(self->iommu);
 }
 
-#define read_pci_id_from_sysfs(_file) ({							\
-	char __sysfs_path[PATH_MAX];								\
-	char __buf[32];										\
-	int __fd;										\
-												\
-	snprintf(__sysfs_path, PATH_MAX, "/sys/bus/pci/devices/%s/%s", device_bdf, _file);	\
-	ASSERT_GT((__fd = open(__sysfs_path, O_RDONLY)), 0);					\
-	ASSERT_GT(read(__fd, __buf, ARRAY_SIZE(__buf)), 0);					\
-	ASSERT_EQ(0, close(__fd));								\
-	(u16)strtoul(__buf, NULL, 0);								\
+#define read_pci_id_from_sysfs(_file) ({					\
+	char __sysfs_path[PATH_MAX];						\
+	char __buf[32];								\
+	int __fd;								\
+										\
+	snprintf_assert(__sysfs_path, PATH_MAX, "/sys/bus/pci/devices/%s/%s",	\
+			device_bdf, _file);					\
+	ASSERT_GT((__fd = open(__sysfs_path, O_RDONLY)), 0);			\
+	ASSERT_GT(read(__fd, __buf, ARRAY_SIZE(__buf)), 0);			\
+	ASSERT_EQ(0, close(__fd));						\
+	(u16)strtoul(__buf, NULL, 0);						\
 })
 
 TEST_F(vfio_pci_device_test, config_space_read_write)
-- 
2.52.0.239.gd5f0c6e74e-goog


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

* [PATCH v2 2/6] vfio: selftests: Introduce a sysfs lib
  2025-12-10 18:14 [PATCH v2 0/6] vfio: selftest: Add SR-IOV UAPI test Raghavendra Rao Ananta
  2025-12-10 18:14 ` [PATCH v2 1/6] vfio: selftests: Introduce snprintf_assert() Raghavendra Rao Ananta
@ 2025-12-10 18:14 ` Raghavendra Rao Ananta
  2025-12-12 18:27   ` Raghavendra Rao Ananta
  2026-01-07 22:41   ` David Matlack
  2025-12-10 18:14 ` [PATCH v2 3/6] vfio: selftests: Extend container/iommufd setup for passing vf_token Raghavendra Rao Ananta
                   ` (3 subsequent siblings)
  5 siblings, 2 replies; 28+ messages in thread
From: Raghavendra Rao Ananta @ 2025-12-10 18:14 UTC (permalink / raw)
  To: David Matlack, Alex Williamson, Alex Williamson
  Cc: Josh Hilke, kvm, linux-kernel, Raghavendra Rao Ananta

Introduce a sysfs liibrary to handle the common reads/writes to the
PCI sysfs files, for example, getting the total number of VFs supported
by the device via /sys/bus/pci/devices/$BDF/sriov_totalvfs. The library
will be used in the upcoming test patch to configure the VFs for a given
PF device.

Opportunistically, move vfio_pci_get_group_from_dev() to this library as
it falls under the same bucket. Rename it to sysfs_get_device_group() to
align with other function names.

Signed-off-by: Raghavendra Rao Ananta <rananta@google.com>
---
 .../selftests/vfio/lib/include/libvfio.h      |   1 +
 .../vfio/lib/include/libvfio/sysfs.h          |  16 ++
 tools/testing/selftests/vfio/lib/libvfio.mk   |   1 +
 tools/testing/selftests/vfio/lib/sysfs.c      | 151 ++++++++++++++++++
 .../selftests/vfio/lib/vfio_pci_device.c      |  22 +--
 5 files changed, 170 insertions(+), 21 deletions(-)
 create mode 100644 tools/testing/selftests/vfio/lib/include/libvfio/sysfs.h
 create mode 100644 tools/testing/selftests/vfio/lib/sysfs.c

diff --git a/tools/testing/selftests/vfio/lib/include/libvfio.h b/tools/testing/selftests/vfio/lib/include/libvfio.h
index 279ddcd701944..bbe1d7616a648 100644
--- a/tools/testing/selftests/vfio/lib/include/libvfio.h
+++ b/tools/testing/selftests/vfio/lib/include/libvfio.h
@@ -5,6 +5,7 @@
 #include <libvfio/assert.h>
 #include <libvfio/iommu.h>
 #include <libvfio/iova_allocator.h>
+#include <libvfio/sysfs.h>
 #include <libvfio/vfio_pci_device.h>
 #include <libvfio/vfio_pci_driver.h>
 
diff --git a/tools/testing/selftests/vfio/lib/include/libvfio/sysfs.h b/tools/testing/selftests/vfio/lib/include/libvfio/sysfs.h
new file mode 100644
index 0000000000000..1eca6b5cbcfcc
--- /dev/null
+++ b/tools/testing/selftests/vfio/lib/include/libvfio/sysfs.h
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef SELFTESTS_VFIO_LIB_INCLUDE_LIBVFIO_SYSFS_H
+#define SELFTESTS_VFIO_LIB_INCLUDE_LIBVFIO_SYSFS_H
+
+int sysfs_get_sriov_totalvfs(const char *bdf);
+int sysfs_get_sriov_numvfs(const char *bdf);
+void sysfs_set_sriov_numvfs(const char *bdfs, int numvfs);
+void sysfs_get_sriov_vf_bdf(const char *pf_bdf, int i, char *out_vf_bdf);
+bool sysfs_get_sriov_drivers_autoprobe(const char *bdf);
+void sysfs_set_sriov_drivers_autoprobe(const char *bdf, bool val);
+void sysfs_bind_driver(const char *bdf, const char *driver);
+void sysfs_unbind_driver(const char *bdf, const char *driver);
+int sysfs_get_driver(const char *bdf, char *out_driver);
+unsigned int sysfs_get_device_group(const char *bdf);
+
+#endif /* SELFTESTS_VFIO_LIB_INCLUDE_LIBVFIO_SYSFS_H */
diff --git a/tools/testing/selftests/vfio/lib/libvfio.mk b/tools/testing/selftests/vfio/lib/libvfio.mk
index 9f47bceed16f4..b7857319c3f1f 100644
--- a/tools/testing/selftests/vfio/lib/libvfio.mk
+++ b/tools/testing/selftests/vfio/lib/libvfio.mk
@@ -6,6 +6,7 @@ LIBVFIO_SRCDIR := $(selfdir)/vfio/lib
 LIBVFIO_C := iommu.c
 LIBVFIO_C += iova_allocator.c
 LIBVFIO_C += libvfio.c
+LIBVFIO_C += sysfs.c
 LIBVFIO_C += vfio_pci_device.c
 LIBVFIO_C += vfio_pci_driver.c
 
diff --git a/tools/testing/selftests/vfio/lib/sysfs.c b/tools/testing/selftests/vfio/lib/sysfs.c
new file mode 100644
index 0000000000000..5551e8b981075
--- /dev/null
+++ b/tools/testing/selftests/vfio/lib/sysfs.c
@@ -0,0 +1,151 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <fcntl.h>
+#include <unistd.h>
+#include <stdlib.h>
+#include <string.h>
+#include <linux/limits.h>
+
+#include <libvfio.h>
+
+static int sysfs_get_val(const char *component, const char *name,
+			 const char *file)
+{
+	char path[PATH_MAX] = {0};
+	char buf[32] = {0};
+	int fd;
+
+	snprintf(path, PATH_MAX, "/sys/bus/pci/%s/%s/%s", component, name, file);
+	fd = open(path, O_RDONLY);
+	if (fd < 0)
+		return fd;
+
+	VFIO_ASSERT_GT(read(fd, buf, ARRAY_SIZE(buf)), 0);
+	VFIO_ASSERT_EQ(close(fd), 0);
+
+	return strtol(buf, NULL, 0);
+}
+
+static void sysfs_set_val(const char *component, const char *name,
+			  const char *file, const char *val)
+{
+	char path[PATH_MAX] = {0};
+	int fd;
+
+	snprintf(path, PATH_MAX, "/sys/bus/pci/%s/%s/%s", component, name, file);
+	VFIO_ASSERT_GT(fd = open(path, O_WRONLY), 0);
+
+	VFIO_ASSERT_EQ(write(fd, val, strlen(val)), strlen(val));
+	VFIO_ASSERT_EQ(close(fd), 0);
+}
+
+static int sysfs_get_device_val(const char *bdf, const char *file)
+{
+	sysfs_get_val("devices", bdf, file);
+}
+
+static void sysfs_set_device_val(const char *bdf, const char *file, const char *val)
+{
+	sysfs_set_val("devices", bdf, file, val);
+}
+
+static void sysfs_set_driver_val(const char *driver, const char *file, const char *val)
+{
+	sysfs_set_val("drivers", driver, file, val);
+}
+
+static void sysfs_set_device_val_int(const char *bdf, const char *file, int val)
+{
+	char val_str[32] = {0};
+
+	snprintf(val_str, sizeof(val_str), "%d", val);
+	sysfs_set_device_val(bdf, file, val_str);
+}
+
+int sysfs_get_sriov_totalvfs(const char *bdf)
+{
+	return sysfs_get_device_val(bdf, "sriov_totalvfs");
+}
+
+int sysfs_get_sriov_numvfs(const char *bdf)
+{
+	return sysfs_get_device_val(bdf, "sriov_numvfs");
+}
+
+void sysfs_set_sriov_numvfs(const char *bdf, int numvfs)
+{
+	sysfs_set_device_val_int(bdf, "sriov_numvfs", numvfs);
+}
+
+bool sysfs_get_sriov_drivers_autoprobe(const char *bdf)
+{
+	return (bool)sysfs_get_device_val(bdf, "sriov_drivers_autoprobe");
+}
+
+void sysfs_set_sriov_drivers_autoprobe(const char *bdf, bool val)
+{
+	sysfs_set_device_val_int(bdf, "sriov_drivers_autoprobe", val);
+}
+
+void sysfs_bind_driver(const char *bdf, const char *driver)
+{
+	sysfs_set_driver_val(driver, "bind", bdf);
+}
+
+void sysfs_unbind_driver(const char *bdf, const char *driver)
+{
+	sysfs_set_driver_val(driver, "unbind", bdf);
+}
+
+void sysfs_get_sriov_vf_bdf(const char *pf_bdf, int i, char *out_vf_bdf)
+{
+	char vf_path[PATH_MAX] = {0};
+	char path[PATH_MAX] = {0};
+	int ret;
+
+	snprintf(path, PATH_MAX, "/sys/bus/pci/devices/%s/virtfn%d", pf_bdf, i);
+
+	ret = readlink(path, vf_path, PATH_MAX);
+	VFIO_ASSERT_NE(ret, -1);
+
+	ret = sscanf(basename(vf_path), "%s", out_vf_bdf);
+	VFIO_ASSERT_EQ(ret, 1);
+}
+
+unsigned int sysfs_get_device_group(const char *bdf)
+{
+	char dev_iommu_group_path[PATH_MAX] = {0};
+	char path[PATH_MAX] = {0};
+	unsigned int group;
+	int ret;
+
+	snprintf(path, PATH_MAX, "/sys/bus/pci/devices/%s/iommu_group", bdf);
+
+	ret = readlink(path, dev_iommu_group_path, sizeof(dev_iommu_group_path));
+	VFIO_ASSERT_NE(ret, -1, "Failed to get the IOMMU group for device: %s\n", bdf);
+
+	ret = sscanf(basename(dev_iommu_group_path), "%u", &group);
+	VFIO_ASSERT_EQ(ret, 1, "Failed to get the IOMMU group for device: %s\n", bdf);
+
+	return group;
+}
+
+int sysfs_get_driver(const char *bdf, char *out_driver)
+{
+	char driver_path[PATH_MAX] = {0};
+	char path[PATH_MAX] = {0};
+	int ret;
+
+	snprintf(path, PATH_MAX, "/sys/bus/pci/devices/%s/driver", bdf);
+	ret = readlink(path, driver_path, PATH_MAX);
+	if (ret == -1) {
+		if (errno == ENOENT)
+			return -1;
+
+		VFIO_FAIL("Failed to read %s\n", path);
+	}
+
+	ret = sscanf(basename(driver_path), "%s", out_driver);
+	VFIO_ASSERT_EQ(ret, 1);
+
+	return 0;
+}
diff --git a/tools/testing/selftests/vfio/lib/vfio_pci_device.c b/tools/testing/selftests/vfio/lib/vfio_pci_device.c
index 64a19481b734f..9b2a123cee5fc 100644
--- a/tools/testing/selftests/vfio/lib/vfio_pci_device.c
+++ b/tools/testing/selftests/vfio/lib/vfio_pci_device.c
@@ -22,8 +22,6 @@
 #include "../../../kselftest.h"
 #include <libvfio.h>
 
-#define PCI_SYSFS_PATH	"/sys/bus/pci/devices"
-
 static void vfio_pci_irq_set(struct vfio_pci_device *device,
 			     u32 index, u32 vector, u32 count, int *fds)
 {
@@ -181,24 +179,6 @@ void vfio_pci_device_reset(struct vfio_pci_device *device)
 	ioctl_assert(device->fd, VFIO_DEVICE_RESET, NULL);
 }
 
-static unsigned int vfio_pci_get_group_from_dev(const char *bdf)
-{
-	char dev_iommu_group_path[PATH_MAX] = {0};
-	char sysfs_path[PATH_MAX] = {0};
-	unsigned int group;
-	int ret;
-
-	snprintf_assert(sysfs_path, PATH_MAX, "%s/%s/iommu_group", PCI_SYSFS_PATH, bdf);
-
-	ret = readlink(sysfs_path, dev_iommu_group_path, sizeof(dev_iommu_group_path));
-	VFIO_ASSERT_NE(ret, -1, "Failed to get the IOMMU group for device: %s\n", bdf);
-
-	ret = sscanf(basename(dev_iommu_group_path), "%u", &group);
-	VFIO_ASSERT_EQ(ret, 1, "Failed to get the IOMMU group for device: %s\n", bdf);
-
-	return group;
-}
-
 static void vfio_pci_group_setup(struct vfio_pci_device *device, const char *bdf)
 {
 	struct vfio_group_status group_status = {
@@ -207,7 +187,7 @@ static void vfio_pci_group_setup(struct vfio_pci_device *device, const char *bdf
 	char group_path[32];
 	int group;
 
-	group = vfio_pci_get_group_from_dev(bdf);
+	group = sysfs_get_device_group(bdf);
 	snprintf_assert(group_path, sizeof(group_path), "/dev/vfio/%d", group);
 
 	device->group_fd = open(group_path, O_RDWR);
-- 
2.52.0.239.gd5f0c6e74e-goog


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

* [PATCH v2 3/6] vfio: selftests: Extend container/iommufd setup for passing vf_token
  2025-12-10 18:14 [PATCH v2 0/6] vfio: selftest: Add SR-IOV UAPI test Raghavendra Rao Ananta
  2025-12-10 18:14 ` [PATCH v2 1/6] vfio: selftests: Introduce snprintf_assert() Raghavendra Rao Ananta
  2025-12-10 18:14 ` [PATCH v2 2/6] vfio: selftests: Introduce a sysfs lib Raghavendra Rao Ananta
@ 2025-12-10 18:14 ` Raghavendra Rao Ananta
  2026-01-07 22:49   ` David Matlack
  2025-12-10 18:14 ` [PATCH v2 4/6] vfio: selftests: Export more vfio_pci functions Raghavendra Rao Ananta
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 28+ messages in thread
From: Raghavendra Rao Ananta @ 2025-12-10 18:14 UTC (permalink / raw)
  To: David Matlack, Alex Williamson, Alex Williamson
  Cc: Josh Hilke, kvm, linux-kernel, Raghavendra Rao Ananta

A UUID is normally set as a vf_token to correspond the VFs with the
PFs, if they are both bound by the vfio-pci driver. This is true for
iommufd-based approach and container-based approach. The token can be
set either during device creation (VFIO_GROUP_GET_DEVICE_FD) in
container-based approach or during iommu bind (VFIO_DEVICE_BIND_IOMMUFD)
in the iommu-fd case. Hence extend the functions,
vfio_pci_iommufd_setup() and vfio_pci_container_setup(), to accept
vf_token as an (optional) argument and handle the necessary setup.

No functional changes are expected.

Signed-off-by: Raghavendra Rao Ananta <rananta@google.com>
---
 tools/testing/selftests/vfio/lib/libvfio.mk   |  4 +-
 .../selftests/vfio/lib/vfio_pci_device.c      | 45 +++++++++++++++----
 2 files changed, 40 insertions(+), 9 deletions(-)

diff --git a/tools/testing/selftests/vfio/lib/libvfio.mk b/tools/testing/selftests/vfio/lib/libvfio.mk
index b7857319c3f1f..459b14c6885a8 100644
--- a/tools/testing/selftests/vfio/lib/libvfio.mk
+++ b/tools/testing/selftests/vfio/lib/libvfio.mk
@@ -15,6 +15,8 @@ LIBVFIO_C += drivers/ioat/ioat.c
 LIBVFIO_C += drivers/dsa/dsa.c
 endif
 
+LDLIBS += -luuid
+
 LIBVFIO_OUTPUT := $(OUTPUT)/libvfio
 
 LIBVFIO_O := $(patsubst %.c, $(LIBVFIO_OUTPUT)/%.o, $(LIBVFIO_C))
@@ -25,6 +27,6 @@ $(shell mkdir -p $(LIBVFIO_O_DIRS))
 CFLAGS += -I$(LIBVFIO_SRCDIR)/include
 
 $(LIBVFIO_O): $(LIBVFIO_OUTPUT)/%.o : $(LIBVFIO_SRCDIR)/%.c
-	$(CC) $(CFLAGS) $(CPPFLAGS) $(TARGET_ARCH) -c $< -o $@
+	$(CC) $(CFLAGS) $(CPPFLAGS) $(TARGET_ARCH) -c $< $(LDLIBS) -o $@
 
 EXTRA_CLEAN += $(LIBVFIO_OUTPUT)
diff --git a/tools/testing/selftests/vfio/lib/vfio_pci_device.c b/tools/testing/selftests/vfio/lib/vfio_pci_device.c
index 9b2a123cee5fc..ac9a5244ddc46 100644
--- a/tools/testing/selftests/vfio/lib/vfio_pci_device.c
+++ b/tools/testing/selftests/vfio/lib/vfio_pci_device.c
@@ -12,6 +12,7 @@
 #include <sys/mman.h>
 
 #include <uapi/linux/types.h>
+#include <uuid/uuid.h>
 #include <linux/iommufd.h>
 #include <linux/limits.h>
 #include <linux/mman.h>
@@ -199,7 +200,27 @@ static void vfio_pci_group_setup(struct vfio_pci_device *device, const char *bdf
 	ioctl_assert(device->group_fd, VFIO_GROUP_SET_CONTAINER, &device->iommu->container_fd);
 }
 
-static void vfio_pci_container_setup(struct vfio_pci_device *device, const char *bdf)
+static void vfio_pci_group_get_device_fd(struct vfio_pci_device *device,
+					 const char *bdf, const char *vf_token)
+{
+	char arg[64] = {0};
+
+	/*
+	 * If a vf_token exists, argument to VFIO_GROUP_GET_DEVICE_FD
+	 * will be in the form of the following example:
+	 * "0000:04:10.0 vf_token=bd8d9d2b-5a5f-4f5a-a211-f591514ba1f3"
+	 */
+	if (vf_token)
+		snprintf(arg, ARRAY_SIZE(arg), "%s vf_token=%s", bdf, vf_token);
+	else
+		snprintf(arg, ARRAY_SIZE(arg), "%s", bdf);
+
+	device->fd = ioctl(device->group_fd, VFIO_GROUP_GET_DEVICE_FD, arg);
+	VFIO_ASSERT_GE(device->fd, 0);
+}
+
+static void vfio_pci_container_setup(struct vfio_pci_device *device,
+				     const char *bdf, const char *vf_token)
 {
 	struct iommu *iommu = device->iommu;
 	unsigned long iommu_type = iommu->mode->iommu_type;
@@ -217,8 +238,7 @@ static void vfio_pci_container_setup(struct vfio_pci_device *device, const char
 	 */
 	(void)ioctl(iommu->container_fd, VFIO_SET_IOMMU, (void *)iommu_type);
 
-	device->fd = ioctl(device->group_fd, VFIO_GROUP_GET_DEVICE_FD, bdf);
-	VFIO_ASSERT_GE(device->fd, 0);
+	vfio_pci_group_get_device_fd(device, bdf, vf_token);
 }
 
 static void vfio_pci_device_setup(struct vfio_pci_device *device)
@@ -279,12 +299,20 @@ const char *vfio_pci_get_cdev_path(const char *bdf)
 	return cdev_path;
 }
 
-static void vfio_device_bind_iommufd(int device_fd, int iommufd)
+static void vfio_device_bind_iommufd(int device_fd, int iommufd,
+				     const char *vf_token)
 {
 	struct vfio_device_bind_iommufd args = {
 		.argsz = sizeof(args),
 		.iommufd = iommufd,
 	};
+	uuid_t token_uuid = {0};
+
+	if (vf_token) {
+		VFIO_ASSERT_EQ(uuid_parse(vf_token, token_uuid), 0);
+		args.flags |= VFIO_DEVICE_BIND_FLAG_TOKEN;
+		args.token_uuid_ptr = (u64)token_uuid;
+	}
 
 	ioctl_assert(device_fd, VFIO_DEVICE_BIND_IOMMUFD, &args);
 }
@@ -299,7 +327,8 @@ static void vfio_device_attach_iommufd_pt(int device_fd, u32 pt_id)
 	ioctl_assert(device_fd, VFIO_DEVICE_ATTACH_IOMMUFD_PT, &args);
 }
 
-static void vfio_pci_iommufd_setup(struct vfio_pci_device *device, const char *bdf)
+static void vfio_pci_iommufd_setup(struct vfio_pci_device *device,
+				   const char *bdf, const char *vf_token)
 {
 	const char *cdev_path = vfio_pci_get_cdev_path(bdf);
 
@@ -307,7 +336,7 @@ static void vfio_pci_iommufd_setup(struct vfio_pci_device *device, const char *b
 	VFIO_ASSERT_GE(device->fd, 0);
 	free((void *)cdev_path);
 
-	vfio_device_bind_iommufd(device->fd, device->iommu->iommufd);
+	vfio_device_bind_iommufd(device->fd, device->iommu->iommufd, vf_token);
 	vfio_device_attach_iommufd_pt(device->fd, device->iommu->ioas_id);
 }
 
@@ -323,9 +352,9 @@ struct vfio_pci_device *vfio_pci_device_init(const char *bdf, struct iommu *iomm
 	device->bdf = bdf;
 
 	if (iommu->mode->container_path)
-		vfio_pci_container_setup(device, bdf);
+		vfio_pci_container_setup(device, bdf, NULL);
 	else
-		vfio_pci_iommufd_setup(device, bdf);
+		vfio_pci_iommufd_setup(device, bdf, NULL);
 
 	vfio_pci_device_setup(device);
 	vfio_pci_driver_probe(device);
-- 
2.52.0.239.gd5f0c6e74e-goog


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

* [PATCH v2 4/6] vfio: selftests: Export more vfio_pci functions
  2025-12-10 18:14 [PATCH v2 0/6] vfio: selftest: Add SR-IOV UAPI test Raghavendra Rao Ananta
                   ` (2 preceding siblings ...)
  2025-12-10 18:14 ` [PATCH v2 3/6] vfio: selftests: Extend container/iommufd setup for passing vf_token Raghavendra Rao Ananta
@ 2025-12-10 18:14 ` Raghavendra Rao Ananta
  2026-01-07 22:55   ` David Matlack
  2026-01-07 23:05   ` David Matlack
  2025-12-10 18:14 ` [PATCH v2 5/6] vfio: selftests: Add helper to set/override a vf_token Raghavendra Rao Ananta
  2025-12-10 18:14 ` [PATCH v2 6/6] vfio: selftests: Add tests to validate SR-IOV UAPI Raghavendra Rao Ananta
  5 siblings, 2 replies; 28+ messages in thread
From: Raghavendra Rao Ananta @ 2025-12-10 18:14 UTC (permalink / raw)
  To: David Matlack, Alex Williamson, Alex Williamson
  Cc: Josh Hilke, kvm, linux-kernel, Raghavendra Rao Ananta

Refactor and make the functions called under device initialization
public. A later patch adds a test that calls these functions to validate
the UAPI of SR-IOV devices. Opportunistically, to test the success
and failure cases of the UAPI, split the functions dealing with
VFIO_GROUP_GET_DEVICE_FD and VFIO_DEVICE_BIND_IOMMUFD into a core
function and another one that asserts the ioctl. The former will be
used for testing the SR-IOV UAPI, hence only export these.

Signed-off-by: Raghavendra Rao Ananta <rananta@google.com>
---
 .../lib/include/libvfio/vfio_pci_device.h     |  7 +++
 .../selftests/vfio/lib/vfio_pci_device.c      | 44 ++++++++++++++-----
 2 files changed, 39 insertions(+), 12 deletions(-)

diff --git a/tools/testing/selftests/vfio/lib/include/libvfio/vfio_pci_device.h b/tools/testing/selftests/vfio/lib/include/libvfio/vfio_pci_device.h
index 2858885a89bbb..6186ca463ca6e 100644
--- a/tools/testing/selftests/vfio/lib/include/libvfio/vfio_pci_device.h
+++ b/tools/testing/selftests/vfio/lib/include/libvfio/vfio_pci_device.h
@@ -122,4 +122,11 @@ static inline bool vfio_pci_device_match(struct vfio_pci_device *device,
 
 const char *vfio_pci_get_cdev_path(const char *bdf);
 
+void vfio_pci_group_setup(struct vfio_pci_device *device, const char *bdf);
+void __vfio_pci_group_get_device_fd(struct vfio_pci_device *device,
+				    const char *bdf, const char *vf_token);
+void vfio_container_set_iommu(struct vfio_pci_device *device);
+void vfio_pci_iommufd_cdev_open(struct vfio_pci_device *device, const char *bdf);
+int __vfio_device_bind_iommufd(int device_fd, int iommufd, const char *vf_token);
+
 #endif /* SELFTESTS_VFIO_LIB_INCLUDE_LIBVFIO_VFIO_PCI_DEVICE_H */
diff --git a/tools/testing/selftests/vfio/lib/vfio_pci_device.c b/tools/testing/selftests/vfio/lib/vfio_pci_device.c
index ac9a5244ddc46..208da2704d9e2 100644
--- a/tools/testing/selftests/vfio/lib/vfio_pci_device.c
+++ b/tools/testing/selftests/vfio/lib/vfio_pci_device.c
@@ -180,7 +180,7 @@ void vfio_pci_device_reset(struct vfio_pci_device *device)
 	ioctl_assert(device->fd, VFIO_DEVICE_RESET, NULL);
 }
 
-static void vfio_pci_group_setup(struct vfio_pci_device *device, const char *bdf)
+void vfio_pci_group_setup(struct vfio_pci_device *device, const char *bdf)
 {
 	struct vfio_group_status group_status = {
 		.argsz = sizeof(group_status),
@@ -200,8 +200,8 @@ static void vfio_pci_group_setup(struct vfio_pci_device *device, const char *bdf
 	ioctl_assert(device->group_fd, VFIO_GROUP_SET_CONTAINER, &device->iommu->container_fd);
 }
 
-static void vfio_pci_group_get_device_fd(struct vfio_pci_device *device,
-					 const char *bdf, const char *vf_token)
+void __vfio_pci_group_get_device_fd(struct vfio_pci_device *device,
+				    const char *bdf, const char *vf_token)
 {
 	char arg[64] = {0};
 
@@ -216,18 +216,21 @@ static void vfio_pci_group_get_device_fd(struct vfio_pci_device *device,
 		snprintf(arg, ARRAY_SIZE(arg), "%s", bdf);
 
 	device->fd = ioctl(device->group_fd, VFIO_GROUP_GET_DEVICE_FD, arg);
+}
+
+static void vfio_pci_group_get_device_fd(struct vfio_pci_device *device,
+					 const char *bdf, const char *vf_token)
+{
+	__vfio_pci_group_get_device_fd(device, bdf, vf_token);
 	VFIO_ASSERT_GE(device->fd, 0);
 }
 
-static void vfio_pci_container_setup(struct vfio_pci_device *device,
-				     const char *bdf, const char *vf_token)
+void vfio_container_set_iommu(struct vfio_pci_device *device)
 {
 	struct iommu *iommu = device->iommu;
 	unsigned long iommu_type = iommu->mode->iommu_type;
 	int ret;
 
-	vfio_pci_group_setup(device, bdf);
-
 	ret = ioctl(iommu->container_fd, VFIO_CHECK_EXTENSION, iommu_type);
 	VFIO_ASSERT_GT(ret, 0, "VFIO IOMMU type %lu not supported\n", iommu_type);
 
@@ -237,7 +240,13 @@ static void vfio_pci_container_setup(struct vfio_pci_device *device,
 	 * because the IOMMU type is already set.
 	 */
 	(void)ioctl(iommu->container_fd, VFIO_SET_IOMMU, (void *)iommu_type);
+}
 
+static void vfio_pci_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);
 }
 
@@ -299,8 +308,7 @@ const char *vfio_pci_get_cdev_path(const char *bdf)
 	return cdev_path;
 }
 
-static void vfio_device_bind_iommufd(int device_fd, int iommufd,
-				     const char *vf_token)
+int __vfio_device_bind_iommufd(int device_fd, int iommufd, const char *vf_token)
 {
 	struct vfio_device_bind_iommufd args = {
 		.argsz = sizeof(args),
@@ -314,7 +322,15 @@ static void vfio_device_bind_iommufd(int device_fd, int iommufd,
 		args.token_uuid_ptr = (u64)token_uuid;
 	}
 
-	ioctl_assert(device_fd, VFIO_DEVICE_BIND_IOMMUFD, &args);
+	return ioctl(device_fd, VFIO_DEVICE_BIND_IOMMUFD, &args);
+}
+
+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);
+
+	VFIO_ASSERT_EQ(ret, 0, "Failed VFIO_DEVICE_BIND_IOMMUFD ioctl\n");
 }
 
 static void vfio_device_attach_iommufd_pt(int device_fd, u32 pt_id)
@@ -327,15 +343,19 @@ static void vfio_device_attach_iommufd_pt(int device_fd, u32 pt_id)
 	ioctl_assert(device_fd, VFIO_DEVICE_ATTACH_IOMMUFD_PT, &args);
 }
 
-static void vfio_pci_iommufd_setup(struct vfio_pci_device *device,
-				   const char *bdf, const char *vf_token)
+void vfio_pci_iommufd_cdev_open(struct vfio_pci_device *device, const char *bdf)
 {
 	const char *cdev_path = vfio_pci_get_cdev_path(bdf);
 
 	device->fd = open(cdev_path, O_RDWR);
 	VFIO_ASSERT_GE(device->fd, 0);
 	free((void *)cdev_path);
+}
 
+static void vfio_pci_iommufd_setup(struct vfio_pci_device *device,
+				   const char *bdf, const char *vf_token)
+{
+	vfio_pci_iommufd_cdev_open(device, bdf);
 	vfio_device_bind_iommufd(device->fd, device->iommu->iommufd, vf_token);
 	vfio_device_attach_iommufd_pt(device->fd, device->iommu->ioas_id);
 }
-- 
2.52.0.239.gd5f0c6e74e-goog


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

* [PATCH v2 5/6] vfio: selftests: Add helper to set/override a vf_token
  2025-12-10 18:14 [PATCH v2 0/6] vfio: selftest: Add SR-IOV UAPI test Raghavendra Rao Ananta
                   ` (3 preceding siblings ...)
  2025-12-10 18:14 ` [PATCH v2 4/6] vfio: selftests: Export more vfio_pci functions Raghavendra Rao Ananta
@ 2025-12-10 18:14 ` Raghavendra Rao Ananta
  2026-01-07 22:56   ` David Matlack
  2025-12-10 18:14 ` [PATCH v2 6/6] vfio: selftests: Add tests to validate SR-IOV UAPI Raghavendra Rao Ananta
  5 siblings, 1 reply; 28+ messages in thread
From: Raghavendra Rao Ananta @ 2025-12-10 18:14 UTC (permalink / raw)
  To: David Matlack, Alex Williamson, Alex Williamson
  Cc: Josh Hilke, kvm, linux-kernel, Raghavendra Rao Ananta

Add a helper function, vfio_device_set_vf_token(), to set or override a
vf_token. Not only at init, but a vf_token can also be set via the
VFIO_DEVICE_FEATURE ioctl, by setting the
VFIO_DEVICE_FEATURE_PCI_VF_TOKEN flag. Hence, add an API to utilize this
functionality from the test code. The subsequent commit will use this to
test the functionality of this method to set the vf_token.

Signed-off-by: Raghavendra Rao Ananta <rananta@google.com>
---
 .../lib/include/libvfio/vfio_pci_device.h     |  2 ++
 .../selftests/vfio/lib/vfio_pci_device.c      | 34 +++++++++++++++++++
 2 files changed, 36 insertions(+)

diff --git a/tools/testing/selftests/vfio/lib/include/libvfio/vfio_pci_device.h b/tools/testing/selftests/vfio/lib/include/libvfio/vfio_pci_device.h
index 6186ca463ca6e..b370aa6a74d0b 100644
--- a/tools/testing/selftests/vfio/lib/include/libvfio/vfio_pci_device.h
+++ b/tools/testing/selftests/vfio/lib/include/libvfio/vfio_pci_device.h
@@ -129,4 +129,6 @@ void vfio_container_set_iommu(struct vfio_pci_device *device);
 void vfio_pci_iommufd_cdev_open(struct vfio_pci_device *device, const char *bdf);
 int __vfio_device_bind_iommufd(int device_fd, int iommufd, const char *vf_token);
 
+void vfio_device_set_vf_token(int fd, const char *vf_token);
+
 #endif /* SELFTESTS_VFIO_LIB_INCLUDE_LIBVFIO_VFIO_PCI_DEVICE_H */
diff --git a/tools/testing/selftests/vfio/lib/vfio_pci_device.c b/tools/testing/selftests/vfio/lib/vfio_pci_device.c
index 208da2704d9e2..7725ecc62b024 100644
--- a/tools/testing/selftests/vfio/lib/vfio_pci_device.c
+++ b/tools/testing/selftests/vfio/lib/vfio_pci_device.c
@@ -109,6 +109,40 @@ static void vfio_pci_irq_get(struct vfio_pci_device *device, u32 index,
 	ioctl_assert(device->fd, VFIO_DEVICE_GET_IRQ_INFO, irq_info);
 }
 
+static int vfio_device_feature_ioctl(int fd, u32 flags, void *data,
+				     size_t data_size)
+{
+	u8 buffer[sizeof(struct vfio_device_feature) + data_size] = {};
+	struct vfio_device_feature *feature = (void *)buffer;
+
+	memcpy(feature->data, data, data_size);
+
+	feature->argsz = sizeof(buffer);
+	feature->flags = flags;
+
+	return ioctl(fd, VFIO_DEVICE_FEATURE, feature);
+}
+
+static void vfio_device_feature_set(int fd, u16 feature, void *data, size_t data_size)
+{
+	u32 flags = VFIO_DEVICE_FEATURE_SET | feature;
+	int ret;
+
+	ret = vfio_device_feature_ioctl(fd, flags, data, data_size);
+	VFIO_ASSERT_EQ(ret, 0, "Failed to set feature %u\n", feature);
+}
+
+void vfio_device_set_vf_token(int fd, const char *vf_token)
+{
+	uuid_t token_uuid = {0};
+
+	VFIO_ASSERT_NOT_NULL(vf_token, "vf_token is NULL");
+	VFIO_ASSERT_EQ(uuid_parse(vf_token, token_uuid), 0);
+
+	vfio_device_feature_set(fd, VFIO_DEVICE_FEATURE_PCI_VF_TOKEN,
+				token_uuid, sizeof(uuid_t));
+}
+
 static void vfio_pci_region_get(struct vfio_pci_device *device, int index,
 				struct vfio_region_info *info)
 {
-- 
2.52.0.239.gd5f0c6e74e-goog


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

* [PATCH v2 6/6] vfio: selftests: Add tests to validate SR-IOV UAPI
  2025-12-10 18:14 [PATCH v2 0/6] vfio: selftest: Add SR-IOV UAPI test Raghavendra Rao Ananta
                   ` (4 preceding siblings ...)
  2025-12-10 18:14 ` [PATCH v2 5/6] vfio: selftests: Add helper to set/override a vf_token Raghavendra Rao Ananta
@ 2025-12-10 18:14 ` Raghavendra Rao Ananta
  2025-12-12 18:21   ` Raghavendra Rao Ananta
                     ` (2 more replies)
  5 siblings, 3 replies; 28+ messages in thread
From: Raghavendra Rao Ananta @ 2025-12-10 18:14 UTC (permalink / raw)
  To: David Matlack, Alex Williamson, Alex Williamson
  Cc: Josh Hilke, kvm, linux-kernel, Raghavendra Rao Ananta

Add a selfttest, 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>
---
 tools/testing/selftests/vfio/Makefile         |   1 +
 .../selftests/vfio/vfio_pci_sriov_uapi_test.c | 215 ++++++++++++++++++
 2 files changed, 216 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 3c796ca99a509..f00a63902fbfb 100644
--- a/tools/testing/selftests/vfio/Makefile
+++ b/tools/testing/selftests/vfio/Makefile
@@ -4,6 +4,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..4c2951d6e049c
--- /dev/null
+++ b/tools/testing/selftests/vfio/vfio_pci_sriov_uapi_test.c
@@ -0,0 +1,215 @@
+// 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_dev_bdf;
+
+static int test_vfio_pci_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 test_vfio_pci_iommufd_setup(struct vfio_pci_device *device,
+				       const char *bdf, const char *vf_token)
+{
+	vfio_pci_iommufd_cdev_open(device, bdf);
+	return __vfio_device_bind_iommufd(device->fd,
+					  device->iommu->iommufd, vf_token);
+}
+
+static struct vfio_pci_device *test_vfio_pci_device_init(const char *bdf,
+							 struct iommu *iommu,
+							 const char *vf_token,
+							 int *out_ret)
+{
+	struct vfio_pci_device *device;
+
+	device = calloc(1, sizeof(*device));
+	VFIO_ASSERT_NOT_NULL(device);
+
+	device->iommu = iommu;
+	device->bdf = bdf;
+
+	if (iommu->mode->container_path)
+		*out_ret = test_vfio_pci_container_setup(device, bdf, vf_token);
+	else
+		*out_ret = test_vfio_pci_iommufd_setup(device, bdf, vf_token);
+
+	return device;
+}
+
+static void test_vfio_pci_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);
+
+	free(device);
+}
+
+FIXTURE(vfio_pci_sriov_uapi_test) {
+	char vf_dev_bdf[16];
+	char vf_driver[32];
+	bool sriov_drivers_autoprobe;
+};
+
+FIXTURE_SETUP(vfio_pci_sriov_uapi_test)
+{
+	int nr_vfs;
+	int ret;
+
+	nr_vfs = sysfs_get_sriov_totalvfs(pf_dev_bdf);
+	if (nr_vfs < 0)
+		SKIP(return, "SR-IOV may not be supported by the device\n");
+
+	nr_vfs = sysfs_get_sriov_numvfs(pf_dev_bdf);
+	if (nr_vfs != 0)
+		SKIP(return, "SR-IOV already configured for the PF\n");
+
+	self->sriov_drivers_autoprobe =
+		sysfs_get_sriov_drivers_autoprobe(pf_dev_bdf);
+	if (self->sriov_drivers_autoprobe)
+		sysfs_set_sriov_drivers_autoprobe(pf_dev_bdf, 0);
+
+	/* Export only one VF for testing */
+	sysfs_set_sriov_numvfs(pf_dev_bdf, 1);
+
+	sysfs_get_sriov_vf_bdf(pf_dev_bdf, 0, self->vf_dev_bdf);
+	if (sysfs_get_driver(self->vf_dev_bdf, self->vf_driver) == 0)
+		sysfs_unbind_driver(self->vf_dev_bdf, self->vf_driver);
+	sysfs_bind_driver(self->vf_dev_bdf, "vfio-pci");
+}
+
+FIXTURE_TEARDOWN(vfio_pci_sriov_uapi_test)
+{
+	sysfs_unbind_driver(self->vf_dev_bdf, "vfio-pci");
+	sysfs_bind_driver(self->vf_dev_bdf, self->vf_driver);
+	sysfs_set_sriov_numvfs(pf_dev_bdf, 0);
+	sysfs_set_sriov_drivers_autoprobe(pf_dev_bdf,
+					  self->sriov_drivers_autoprobe);
+}
+
+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_device;
+	struct vfio_pci_device *vf_device;
+	struct iommu *iommu;
+	int ret;
+
+	iommu = iommu_init(variant->iommu_mode);
+	pf_device = test_vfio_pci_device_init(pf_dev_bdf, iommu, UUID_1, &ret);
+	vf_device = test_vfio_pci_device_init(self->vf_dev_bdf, iommu,
+					      variant->vf_token, &ret);
+
+	ASSERT_VF_CREATION(ret);
+
+	test_vfio_pci_device_cleanup(vf_device);
+	test_vfio_pci_device_cleanup(pf_device);
+	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_device;
+	struct vfio_pci_device *vf_device;
+	struct iommu *iommu;
+	int ret;
+
+	iommu = iommu_init(variant->iommu_mode);
+	pf_device = test_vfio_pci_device_init(pf_dev_bdf, iommu, UUID_1, &ret);
+	test_vfio_pci_device_cleanup(pf_device);
+
+	vf_device = test_vfio_pci_device_init(self->vf_dev_bdf, iommu,
+					      variant->vf_token, &ret);
+
+	ASSERT_VF_CREATION(ret);
+
+	test_vfio_pci_device_cleanup(vf_device);
+	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_device;
+	struct vfio_pci_device *vf_device;
+	struct iommu *iommu;
+	int ret;
+
+	iommu = iommu_init(variant->iommu_mode);
+	pf_device = test_vfio_pci_device_init(pf_dev_bdf, iommu, UUID_2, &ret);
+	vfio_device_set_vf_token(pf_device->fd, UUID_1);
+
+	vf_device = test_vfio_pci_device_init(self->vf_dev_bdf, iommu,
+					      variant->vf_token, &ret);
+
+	ASSERT_VF_CREATION(ret);
+
+	test_vfio_pci_device_cleanup(vf_device);
+	test_vfio_pci_device_cleanup(pf_device);
+	iommu_cleanup(iommu);
+}
+
+int main(int argc, char *argv[])
+{
+	pf_dev_bdf = vfio_selftests_get_bdf(&argc, argv);
+	return test_harness_run(argc, argv);
+}
-- 
2.52.0.239.gd5f0c6e74e-goog


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

* Re: [PATCH v2 6/6] vfio: selftests: Add tests to validate SR-IOV UAPI
  2025-12-10 18:14 ` [PATCH v2 6/6] vfio: selftests: Add tests to validate SR-IOV UAPI Raghavendra Rao Ananta
@ 2025-12-12 18:21   ` Raghavendra Rao Ananta
  2025-12-18 23:26   ` David Matlack
  2026-01-07 23:22   ` David Matlack
  2 siblings, 0 replies; 28+ messages in thread
From: Raghavendra Rao Ananta @ 2025-12-12 18:21 UTC (permalink / raw)
  To: David Matlack, Alex Williamson, Alex Williamson
  Cc: Josh Hilke, kvm, linux-kernel

On Wed, Dec 10, 2025 at 10:14 AM Raghavendra Rao Ananta
<rananta@google.com> wrote:
>
> +FIXTURE_SETUP(vfio_pci_sriov_uapi_test)
> +{
> +       int nr_vfs;
> +       int ret;
> +
'ret' is unused in the function. I'll remove it in v3.

Thank you.
Raghavendra

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

* Re: [PATCH v2 2/6] vfio: selftests: Introduce a sysfs lib
  2025-12-10 18:14 ` [PATCH v2 2/6] vfio: selftests: Introduce a sysfs lib Raghavendra Rao Ananta
@ 2025-12-12 18:27   ` Raghavendra Rao Ananta
  2025-12-18 21:52     ` David Matlack
  2026-01-07 22:41   ` David Matlack
  1 sibling, 1 reply; 28+ messages in thread
From: Raghavendra Rao Ananta @ 2025-12-12 18:27 UTC (permalink / raw)
  To: David Matlack, Alex Williamson, Alex Williamson
  Cc: Josh Hilke, kvm, linux-kernel

On Wed, Dec 10, 2025 at 10:14 AM Raghavendra Rao Ananta
<rananta@google.com> wrote:
> +
> +static int sysfs_get_device_val(const char *bdf, const char *file)
> +{
> +       sysfs_get_val("devices", bdf, file);
This should be 'return sysfs_get_val("devices", bdf, file);' . I'll
fix it in v3.
> +}

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

* Re: [PATCH v2 2/6] vfio: selftests: Introduce a sysfs lib
  2025-12-12 18:27   ` Raghavendra Rao Ananta
@ 2025-12-18 21:52     ` David Matlack
  0 siblings, 0 replies; 28+ messages in thread
From: David Matlack @ 2025-12-18 21:52 UTC (permalink / raw)
  To: Raghavendra Rao Ananta
  Cc: Alex Williamson, Alex Williamson, Josh Hilke, kvm, linux-kernel

On 2025-12-12 10:27 AM, Raghavendra Rao Ananta wrote:
> On Wed, Dec 10, 2025 at 10:14 AM Raghavendra Rao Ananta
> <rananta@google.com> wrote:
> > +
> > +static int sysfs_get_device_val(const char *bdf, const char *file)
> > +{
> > +       sysfs_get_val("devices", bdf, file);
> This should be 'return sysfs_get_val("devices", bdf, file);' . I'll
> fix it in v3.

Can you add a patch to add -Wall and -Werror to
tools/testing/selftests/vfio/Makefile in the next version? That would
have caught both this bug and the unused ret variable in the other
patch.

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

* Re: [PATCH v2 6/6] vfio: selftests: Add tests to validate SR-IOV UAPI
  2025-12-10 18:14 ` [PATCH v2 6/6] vfio: selftests: Add tests to validate SR-IOV UAPI Raghavendra Rao Ananta
  2025-12-12 18:21   ` Raghavendra Rao Ananta
@ 2025-12-18 23:26   ` David Matlack
  2026-01-06 19:47     ` Raghavendra Rao Ananta
  2026-01-07 23:22   ` David Matlack
  2 siblings, 1 reply; 28+ messages in thread
From: David Matlack @ 2025-12-18 23:26 UTC (permalink / raw)
  To: Raghavendra Rao Ananta
  Cc: Alex Williamson, Alex Williamson, Josh Hilke, kvm, linux-kernel

On 2025-12-10 06:14 PM, Raghavendra Rao Ananta wrote:
> Add a selfttest, 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>

I hit the following kernel NULL pointer dereference after running the
new test a few times (nice!).

Repro:

  $ tools/testing/selftests/vfio/scripts/setup.sh 0000:16:00.1
  $ tools/testing/selftests/vfio/vfio_pci_sriov_uapi_test 0000:16:00.1
  $ tools/testing/selftests/vfio/scripts/cleanup.sh
  ... repeat ...

The panic:

[  553.245784][T27601] vfio-pci 0000:1a:00.0: probe with driver vfio-pci failed with error -22
[  553.256622][T27601] vfio-pci 0000:1a:00.0: probe with driver vfio-pci failed with error -22
[  574.857650][T27935] BUG: kernel NULL pointer dereference, address: 0000000000000008
[  574.865322][T27935] #PF: supervisor read access in kernel mode
[  574.871175][T27935] #PF: error_code(0x0000) - not-present page
[  574.877021][T27935] PGD 4116e63067 P4D 40fb0a3067 PUD 409597f067 PMD 0
[  574.883654][T27935] Oops: Oops: 0000 [#1] SMP NOPTI
[  574.888551][T27935] CPU: 100 UID: 0 PID: 27935 Comm: vfio_pci_sriov_ Tainted: G S      W           6.18.0-smp-DEV #1 NONE
[  574.899600][T27935] Tainted: [S]=CPU_OUT_OF_SPEC, [W]=WARN
[  574.905104][T27935] Hardware name: Google Izumi-EMR/izumi, BIOS 0.20250801.2-0 08/25/2025
[  574.913289][T27935] RIP: 0010:rb_insert_color+0x44/0x110
[  574.918623][T27935] Code: cc cc 48 89 cf 48 83 cf 01 48 89 3a 48 89 38 48 8b 01 48 89 cf 48 83 e0 fc 48 89 01 74 d7 48 8b 08 f6 c1 01 0f 85 c1 00 00 00 <48> 8b 51 08 48 39 c2 74 0c 48 85 d2 74 4f f6 02 01 74 c5 eb 48 48
[  574.938080][T27935] RSP: 0018:ff85113dcdd6bb08 EFLAGS: 00010046
[  574.944013][T27935] RAX: ff3f257594a99e80 RBX: ff3f25758af490c0 RCX: 0000000000000000
[  574.951857][T27935] RDX: 0000000000001a00 RSI: ff3f25360038eb70 RDI: ff3f2536658bbee0
[  574.959702][T27935] RBP: ff3f25360038ea00 R08: 0000000000000002 R09: ff85113dcdd6badc
[  574.967544][T27935] R10: ff3f257590ab8000 R11: ffffffffa78210a0 R12: ff3f2536658bbea0
[  574.975387][T27935] R13: 0000000000000286 R14: ff3f25758af49000 R15: ff3f25360038eb78
[  574.983230][T27935] FS:  00000000223403c0(0000) GS:ff3f25b4d4d83000(0000) knlGS:0000000000000000
[  574.992032][T27935] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  574.998488][T27935] CR2: 0000000000000008 CR3: 00000040fa254005 CR4: 0000000000f71ef0
[  575.006332][T27935] PKRU: 55555554
[  575.009753][T27935] Call Trace:
[  575.012919][T27935]  <TASK>
[  575.015730][T27935]  intel_iommu_probe_device+0x4c9/0x7b0
[  575.021153][T27935]  __iommu_probe_device+0x101/0x4c0
[  575.026231][T27935]  iommu_bus_notifier+0x37/0x100
[  575.031046][T27935]  blocking_notifier_call_chain+0x53/0xd0
[  575.036634][T27935]  bus_notify+0x99/0xc0
[  575.040666][T27935]  device_add+0x252/0x470
[  575.044872][T27935]  pci_device_add+0x414/0x5c0
[  575.049429][T27935]  pci_iov_add_virtfn+0x2f2/0x3e0
[  575.054326][T27935]  sriov_add_vfs+0x33/0x70
[  575.058613][T27935]  sriov_enable+0x2fc/0x490
[  575.062992][T27935]  vfio_pci_core_sriov_configure+0x16c/0x210
[  575.068843][T27935]  sriov_numvfs_store+0xc4/0x190
[  575.073652][T27935]  kernfs_fop_write_iter+0xfe/0x180
[  575.078724][T27935]  vfs_write+0x2d0/0x430
[  575.082846][T27935]  ksys_write+0x7f/0x100
[  575.086965][T27935]  do_syscall_64+0x6f/0x940
[  575.091339][T27935]  ? arch_exit_to_user_mode_prepare+0x9/0xb0
[  575.097193][T27935]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
[  575.102952][T27935] RIP: 0033:0x46fcf7
[  575.106721][T27935] Code: 48 89 fa 4c 89 df e8 88 16 00 00 8b 93 08 03 00 00 59 5e 48 83 f8 fc 74 1a 5b c3 0f 1f 84 00 00 00 00 00 48 8b 44 24 10 0f 05 <5b> c3 0f 1f 80 00 00 00 00 83 e2 39 83 fa 08 75 de e8 23 ff ff ff
[  575.126178][T27935] RSP: 002b:00007ffe991aff40 EFLAGS: 00000202 ORIG_RAX: 0000000000000001
[  575.134457][T27935] RAX: ffffffffffffffda RBX: 00000000223403c0 RCX: 000000000046fcf7
[  575.142301][T27935] RDX: 0000000000000001 RSI: 00007ffe991b1050 RDI: 0000000000000003
[  575.150143][T27935] RBP: 00007ffe991b0ff0 R08: 0000000000000000 R09: 0000000000000000
[  575.157985][T27935] R10: 0000000000000000 R11: 0000000000000202 R12: 00007ffe991b1768
[  575.165829][T27935] R13: 0000000000000016 R14: 00000000004dd480 R15: 0000000000000016
[  575.173677][T27935]  </TASK>
[  575.176573][T27935] Modules linked in: vfat fat dummy bridge stp llc intel_vsec cdc_acm cdc_ncm cdc_eem cdc_ether usbnet mii xhci_pci xhci_hcd ehci_pci ehci_hcd
[  575.190930][T27935] CR2: 0000000000000008
[  575.194960][T27935] ---[ end trace 0000000000000000 ]---
[  575.204004][T27935] RIP: 0010:rb_insert_color+0x44/0x110
[  575.209336][T27935] Code: cc cc 48 89 cf 48 83 cf 01 48 89 3a 48 89 38 48 8b 01 48 89 cf 48 83 e0 fc 48 89 01 74 d7 48 8b 08 f6 c1 01 0f 85 c1 00 00 00 <48> 8b 51 08 48 39 c2 74 0c 48 85 d2 74 4f f6 02 01 74 c5 eb 48 48
[  575.228796][T27935] RSP: 0018:ff85113dcdd6bb08 EFLAGS: 00010046
[  575.234729][T27935] RAX: ff3f257594a99e80 RBX: ff3f25758af490c0 RCX: 0000000000000000
[  575.242572][T27935] RDX: 0000000000001a00 RSI: ff3f25360038eb70 RDI: ff3f2536658bbee0
[  575.250414][T27935] RBP: ff3f25360038ea00 R08: 0000000000000002 R09: ff85113dcdd6badc
[  575.258263][T27935] R10: ff3f257590ab8000 R11: ffffffffa78210a0 R12: ff3f2536658bbea0
[  575.266105][T27935] R13: 0000000000000286 R14: ff3f25758af49000 R15: ff3f25360038eb78
[  575.273948][T27935] FS:  00000000223403c0(0000) GS:ff3f25b4d4d83000(0000) knlGS:0000000000000000
[  575.282741][T27935] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  575.289197][T27935] CR2: 0000000000000008 CR3: 00000040fa254005 CR4: 0000000000f71ef0
[  575.297046][T27935] PKRU: 55555554
[  575.300466][T27935] Kernel panic - not syncing: Fatal exception
[  575.345557][T27935] Kernel Offset: 0x25800000 from 0xffffffff81000000 (relocation range: 0xffffffff80000000-0xffffffffbfffffff)
[  575.362075][T27935] mtdoops: Cannot write from panic without panic_write
[  575.368795][T27935] Rebooting in 10 seconds..

I also have the following diff on top of your series to fix the other
bug you found.

diff --git a/tools/testing/selftests/vfio/lib/sysfs.c b/tools/testing/selftests/vfio/lib/sysfs.c
index 5551e8b98107..d94616e8aff4 100644
--- a/tools/testing/selftests/vfio/lib/sysfs.c
+++ b/tools/testing/selftests/vfio/lib/sysfs.c
@@ -40,7 +40,7 @@ static void sysfs_set_val(const char *component, const char *name,

 static int sysfs_get_device_val(const char *bdf, const char *file)
 {
-       sysfs_get_val("devices", bdf, file);
+       return sysfs_get_val("devices", bdf, file);
 }

 static void sysfs_set_device_val(const char *bdf, const char *file, const char *val)

I'm not sure which exact test case triggered the panic. This is the only
test output that made it to my ssh window:

  TAP version 13
  1..45
  # Starting 45 tests from 15 test cases.
  #  RUN           vfio_pci_sriov_uapi_test.vfio_type1_iommu_same_uuid.init_token_match ...

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

* Re: [PATCH v2 6/6] vfio: selftests: Add tests to validate SR-IOV UAPI
  2025-12-18 23:26   ` David Matlack
@ 2026-01-06 19:47     ` Raghavendra Rao Ananta
  2026-02-05 21:51       ` David Matlack
  0 siblings, 1 reply; 28+ messages in thread
From: Raghavendra Rao Ananta @ 2026-01-06 19:47 UTC (permalink / raw)
  To: David Matlack, Alex Williamson, Alex Williamson
  Cc: Josh Hilke, kvm, linux-kernel, iommu

+ cc: iommu@lists.linux.dev for the crash

Thank you.
Raghavendra


On Thu, Dec 18, 2025 at 3:26 PM David Matlack <dmatlack@google.com> wrote:
>
> On 2025-12-10 06:14 PM, Raghavendra Rao Ananta wrote:
> > Add a selfttest, 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>
>
> I hit the following kernel NULL pointer dereference after running the
> new test a few times (nice!).
>
> Repro:
>
>   $ tools/testing/selftests/vfio/scripts/setup.sh 0000:16:00.1
>   $ tools/testing/selftests/vfio/vfio_pci_sriov_uapi_test 0000:16:00.1
>   $ tools/testing/selftests/vfio/scripts/cleanup.sh
>   ... repeat ...
>
> The panic:
>
> [  553.245784][T27601] vfio-pci 0000:1a:00.0: probe with driver vfio-pci failed with error -22
> [  553.256622][T27601] vfio-pci 0000:1a:00.0: probe with driver vfio-pci failed with error -22
> [  574.857650][T27935] BUG: kernel NULL pointer dereference, address: 0000000000000008
> [  574.865322][T27935] #PF: supervisor read access in kernel mode
> [  574.871175][T27935] #PF: error_code(0x0000) - not-present page
> [  574.877021][T27935] PGD 4116e63067 P4D 40fb0a3067 PUD 409597f067 PMD 0
> [  574.883654][T27935] Oops: Oops: 0000 [#1] SMP NOPTI
> [  574.888551][T27935] CPU: 100 UID: 0 PID: 27935 Comm: vfio_pci_sriov_ Tainted: G S      W           6.18.0-smp-DEV #1 NONE
> [  574.899600][T27935] Tainted: [S]=CPU_OUT_OF_SPEC, [W]=WARN
> [  574.905104][T27935] Hardware name: Google Izumi-EMR/izumi, BIOS 0.20250801.2-0 08/25/2025
> [  574.913289][T27935] RIP: 0010:rb_insert_color+0x44/0x110
> [  574.918623][T27935] Code: cc cc 48 89 cf 48 83 cf 01 48 89 3a 48 89 38 48 8b 01 48 89 cf 48 83 e0 fc 48 89 01 74 d7 48 8b 08 f6 c1 01 0f 85 c1 00 00 00 <48> 8b 51 08 48 39 c2 74 0c 48 85 d2 74 4f f6 02 01 74 c5 eb 48 48
> [  574.938080][T27935] RSP: 0018:ff85113dcdd6bb08 EFLAGS: 00010046
> [  574.944013][T27935] RAX: ff3f257594a99e80 RBX: ff3f25758af490c0 RCX: 0000000000000000
> [  574.951857][T27935] RDX: 0000000000001a00 RSI: ff3f25360038eb70 RDI: ff3f2536658bbee0
> [  574.959702][T27935] RBP: ff3f25360038ea00 R08: 0000000000000002 R09: ff85113dcdd6badc
> [  574.967544][T27935] R10: ff3f257590ab8000 R11: ffffffffa78210a0 R12: ff3f2536658bbea0
> [  574.975387][T27935] R13: 0000000000000286 R14: ff3f25758af49000 R15: ff3f25360038eb78
> [  574.983230][T27935] FS:  00000000223403c0(0000) GS:ff3f25b4d4d83000(0000) knlGS:0000000000000000
> [  574.992032][T27935] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [  574.998488][T27935] CR2: 0000000000000008 CR3: 00000040fa254005 CR4: 0000000000f71ef0
> [  575.006332][T27935] PKRU: 55555554
> [  575.009753][T27935] Call Trace:
> [  575.012919][T27935]  <TASK>
> [  575.015730][T27935]  intel_iommu_probe_device+0x4c9/0x7b0
> [  575.021153][T27935]  __iommu_probe_device+0x101/0x4c0
> [  575.026231][T27935]  iommu_bus_notifier+0x37/0x100
> [  575.031046][T27935]  blocking_notifier_call_chain+0x53/0xd0
> [  575.036634][T27935]  bus_notify+0x99/0xc0
> [  575.040666][T27935]  device_add+0x252/0x470
> [  575.044872][T27935]  pci_device_add+0x414/0x5c0
> [  575.049429][T27935]  pci_iov_add_virtfn+0x2f2/0x3e0
> [  575.054326][T27935]  sriov_add_vfs+0x33/0x70
> [  575.058613][T27935]  sriov_enable+0x2fc/0x490
> [  575.062992][T27935]  vfio_pci_core_sriov_configure+0x16c/0x210
> [  575.068843][T27935]  sriov_numvfs_store+0xc4/0x190
> [  575.073652][T27935]  kernfs_fop_write_iter+0xfe/0x180
> [  575.078724][T27935]  vfs_write+0x2d0/0x430
> [  575.082846][T27935]  ksys_write+0x7f/0x100
> [  575.086965][T27935]  do_syscall_64+0x6f/0x940
> [  575.091339][T27935]  ? arch_exit_to_user_mode_prepare+0x9/0xb0
> [  575.097193][T27935]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
> [  575.102952][T27935] RIP: 0033:0x46fcf7
> [  575.106721][T27935] Code: 48 89 fa 4c 89 df e8 88 16 00 00 8b 93 08 03 00 00 59 5e 48 83 f8 fc 74 1a 5b c3 0f 1f 84 00 00 00 00 00 48 8b 44 24 10 0f 05 <5b> c3 0f 1f 80 00 00 00 00 83 e2 39 83 fa 08 75 de e8 23 ff ff ff
> [  575.126178][T27935] RSP: 002b:00007ffe991aff40 EFLAGS: 00000202 ORIG_RAX: 0000000000000001
> [  575.134457][T27935] RAX: ffffffffffffffda RBX: 00000000223403c0 RCX: 000000000046fcf7
> [  575.142301][T27935] RDX: 0000000000000001 RSI: 00007ffe991b1050 RDI: 0000000000000003
> [  575.150143][T27935] RBP: 00007ffe991b0ff0 R08: 0000000000000000 R09: 0000000000000000
> [  575.157985][T27935] R10: 0000000000000000 R11: 0000000000000202 R12: 00007ffe991b1768
> [  575.165829][T27935] R13: 0000000000000016 R14: 00000000004dd480 R15: 0000000000000016
> [  575.173677][T27935]  </TASK>
> [  575.176573][T27935] Modules linked in: vfat fat dummy bridge stp llc intel_vsec cdc_acm cdc_ncm cdc_eem cdc_ether usbnet mii xhci_pci xhci_hcd ehci_pci ehci_hcd
> [  575.190930][T27935] CR2: 0000000000000008
> [  575.194960][T27935] ---[ end trace 0000000000000000 ]---
> [  575.204004][T27935] RIP: 0010:rb_insert_color+0x44/0x110
> [  575.209336][T27935] Code: cc cc 48 89 cf 48 83 cf 01 48 89 3a 48 89 38 48 8b 01 48 89 cf 48 83 e0 fc 48 89 01 74 d7 48 8b 08 f6 c1 01 0f 85 c1 00 00 00 <48> 8b 51 08 48 39 c2 74 0c 48 85 d2 74 4f f6 02 01 74 c5 eb 48 48
> [  575.228796][T27935] RSP: 0018:ff85113dcdd6bb08 EFLAGS: 00010046
> [  575.234729][T27935] RAX: ff3f257594a99e80 RBX: ff3f25758af490c0 RCX: 0000000000000000
> [  575.242572][T27935] RDX: 0000000000001a00 RSI: ff3f25360038eb70 RDI: ff3f2536658bbee0
> [  575.250414][T27935] RBP: ff3f25360038ea00 R08: 0000000000000002 R09: ff85113dcdd6badc
> [  575.258263][T27935] R10: ff3f257590ab8000 R11: ffffffffa78210a0 R12: ff3f2536658bbea0
> [  575.266105][T27935] R13: 0000000000000286 R14: ff3f25758af49000 R15: ff3f25360038eb78
> [  575.273948][T27935] FS:  00000000223403c0(0000) GS:ff3f25b4d4d83000(0000) knlGS:0000000000000000
> [  575.282741][T27935] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [  575.289197][T27935] CR2: 0000000000000008 CR3: 00000040fa254005 CR4: 0000000000f71ef0
> [  575.297046][T27935] PKRU: 55555554
> [  575.300466][T27935] Kernel panic - not syncing: Fatal exception
> [  575.345557][T27935] Kernel Offset: 0x25800000 from 0xffffffff81000000 (relocation range: 0xffffffff80000000-0xffffffffbfffffff)
> [  575.362075][T27935] mtdoops: Cannot write from panic without panic_write
> [  575.368795][T27935] Rebooting in 10 seconds..
>
> I also have the following diff on top of your series to fix the other
> bug you found.
>
> diff --git a/tools/testing/selftests/vfio/lib/sysfs.c b/tools/testing/selftests/vfio/lib/sysfs.c
> index 5551e8b98107..d94616e8aff4 100644
> --- a/tools/testing/selftests/vfio/lib/sysfs.c
> +++ b/tools/testing/selftests/vfio/lib/sysfs.c
> @@ -40,7 +40,7 @@ static void sysfs_set_val(const char *component, const char *name,
>
>  static int sysfs_get_device_val(const char *bdf, const char *file)
>  {
> -       sysfs_get_val("devices", bdf, file);
> +       return sysfs_get_val("devices", bdf, file);
>  }
>
>  static void sysfs_set_device_val(const char *bdf, const char *file, const char *val)
>
> I'm not sure which exact test case triggered the panic. This is the only
> test output that made it to my ssh window:
>
>   TAP version 13
>   1..45
>   # Starting 45 tests from 15 test cases.
>   #  RUN           vfio_pci_sriov_uapi_test.vfio_type1_iommu_same_uuid.init_token_match ...

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

* Re: [PATCH v2 1/6] vfio: selftests: Introduce snprintf_assert()
  2025-12-10 18:14 ` [PATCH v2 1/6] vfio: selftests: Introduce snprintf_assert() Raghavendra Rao Ananta
@ 2026-01-07 22:21   ` David Matlack
  0 siblings, 0 replies; 28+ messages in thread
From: David Matlack @ 2026-01-07 22:21 UTC (permalink / raw)
  To: Raghavendra Rao Ananta
  Cc: Alex Williamson, Alex Williamson, Josh Hilke, kvm, linux-kernel

On 2025-12-10 06:14 PM, Raghavendra Rao Ananta wrote:
> Introduce snprintf_assert() to protect the users of snprintf() to fail
> if the requested operation was truncated due to buffer limits. VFIO
> tests and libraries, including a new sysfs library that will be introduced
> by an upcoming patch, rely quite heavily on snprintf()s to build PCI
> sysfs paths. Having a protection against this will be helpful to prevent
> false test failures.
> 
> Signed-off-by: Raghavendra Rao Ananta <rananta@google.com>

Reviewed-by: David Matlack <dmatlack@google.com>

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

* Re: [PATCH v2 2/6] vfio: selftests: Introduce a sysfs lib
  2025-12-10 18:14 ` [PATCH v2 2/6] vfio: selftests: Introduce a sysfs lib Raghavendra Rao Ananta
  2025-12-12 18:27   ` Raghavendra Rao Ananta
@ 2026-01-07 22:41   ` David Matlack
  2026-01-08 21:25     ` Raghavendra Rao Ananta
  1 sibling, 1 reply; 28+ messages in thread
From: David Matlack @ 2026-01-07 22:41 UTC (permalink / raw)
  To: Raghavendra Rao Ananta
  Cc: Alex Williamson, Alex Williamson, Josh Hilke, kvm, linux-kernel

On 2025-12-10 06:14 PM, Raghavendra Rao Ananta wrote:
> Introduce a sysfs liibrary to handle the common reads/writes to the
                    library

> PCI sysfs files, for example, getting the total number of VFs supported
> by the device via /sys/bus/pci/devices/$BDF/sriov_totalvfs. The library
> will be used in the upcoming test patch to configure the VFs for a given
> PF device.
> 
> Opportunistically, move vfio_pci_get_group_from_dev() to this library as
> it falls under the same bucket. Rename it to sysfs_get_device_group() to
> align with other function names.
> 
> Signed-off-by: Raghavendra Rao Ananta <rananta@google.com>
> ---
>  .../selftests/vfio/lib/include/libvfio.h      |   1 +
>  .../vfio/lib/include/libvfio/sysfs.h          |  16 ++
>  tools/testing/selftests/vfio/lib/libvfio.mk   |   1 +
>  tools/testing/selftests/vfio/lib/sysfs.c      | 151 ++++++++++++++++++
>  .../selftests/vfio/lib/vfio_pci_device.c      |  22 +--
>  5 files changed, 170 insertions(+), 21 deletions(-)
>  create mode 100644 tools/testing/selftests/vfio/lib/include/libvfio/sysfs.h
>  create mode 100644 tools/testing/selftests/vfio/lib/sysfs.c
> 
> diff --git a/tools/testing/selftests/vfio/lib/include/libvfio.h b/tools/testing/selftests/vfio/lib/include/libvfio.h
> index 279ddcd701944..bbe1d7616a648 100644
> --- a/tools/testing/selftests/vfio/lib/include/libvfio.h
> +++ b/tools/testing/selftests/vfio/lib/include/libvfio.h
> @@ -5,6 +5,7 @@
>  #include <libvfio/assert.h>
>  #include <libvfio/iommu.h>
>  #include <libvfio/iova_allocator.h>
> +#include <libvfio/sysfs.h>
>  #include <libvfio/vfio_pci_device.h>
>  #include <libvfio/vfio_pci_driver.h>
>  
> diff --git a/tools/testing/selftests/vfio/lib/include/libvfio/sysfs.h b/tools/testing/selftests/vfio/lib/include/libvfio/sysfs.h
> new file mode 100644
> index 0000000000000..1eca6b5cbcfcc
> --- /dev/null
> +++ b/tools/testing/selftests/vfio/lib/include/libvfio/sysfs.h
> @@ -0,0 +1,16 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +#ifndef SELFTESTS_VFIO_LIB_INCLUDE_LIBVFIO_SYSFS_H
> +#define SELFTESTS_VFIO_LIB_INCLUDE_LIBVFIO_SYSFS_H
> +
> +int sysfs_get_sriov_totalvfs(const char *bdf);
> +int sysfs_get_sriov_numvfs(const char *bdf);
> +void sysfs_set_sriov_numvfs(const char *bdfs, int numvfs);
> +void sysfs_get_sriov_vf_bdf(const char *pf_bdf, int i, char *out_vf_bdf);
> +bool sysfs_get_sriov_drivers_autoprobe(const char *bdf);
> +void sysfs_set_sriov_drivers_autoprobe(const char *bdf, bool val);
> +void sysfs_bind_driver(const char *bdf, const char *driver);
> +void sysfs_unbind_driver(const char *bdf, const char *driver);
> +int sysfs_get_driver(const char *bdf, char *out_driver);
> +unsigned int sysfs_get_device_group(const char *bdf);
> +
> +#endif /* SELFTESTS_VFIO_LIB_INCLUDE_LIBVFIO_SYSFS_H */
> diff --git a/tools/testing/selftests/vfio/lib/libvfio.mk b/tools/testing/selftests/vfio/lib/libvfio.mk
> index 9f47bceed16f4..b7857319c3f1f 100644
> --- a/tools/testing/selftests/vfio/lib/libvfio.mk
> +++ b/tools/testing/selftests/vfio/lib/libvfio.mk
> @@ -6,6 +6,7 @@ LIBVFIO_SRCDIR := $(selfdir)/vfio/lib
>  LIBVFIO_C := iommu.c
>  LIBVFIO_C += iova_allocator.c
>  LIBVFIO_C += libvfio.c
> +LIBVFIO_C += sysfs.c
>  LIBVFIO_C += vfio_pci_device.c
>  LIBVFIO_C += vfio_pci_driver.c
>  
> diff --git a/tools/testing/selftests/vfio/lib/sysfs.c b/tools/testing/selftests/vfio/lib/sysfs.c
> new file mode 100644
> index 0000000000000..5551e8b981075
> --- /dev/null
> +++ b/tools/testing/selftests/vfio/lib/sysfs.c
> @@ -0,0 +1,151 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +#include <fcntl.h>
> +#include <unistd.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <linux/limits.h>
> +
> +#include <libvfio.h>
> +
> +static int sysfs_get_val(const char *component, const char *name,

nit: I'm partial to putting the verbs at the end of function names for
library calls.

e.g.

  vfio_pci_config_read()
  vfio_pci_config_write()
  vfio_pci_msi_enable()
  vfio_pci_msi_disable()

So these would be:

  sysfs_val_set()
  sysfs_val_get()
  sysfs_device_val_set()
  sysfs_device_val_get()
  sysfs_sriov_numvfs_set()
  sysfs_sriov_numvfs_get()
  ...

> +			 const char *file)
> +{
> +	char path[PATH_MAX] = {0};
> +	char buf[32] = {0};

nit: You don't need to zero-initialize these since you only use them
after they are intitialized below.

> +	int fd;
> +
> +	snprintf(path, PATH_MAX, "/sys/bus/pci/%s/%s/%s", component, name, file);

Use the new snprintf_assert() :)

> +	fd = open(path, O_RDONLY);
> +	if (fd < 0)
> +		return fd;
> +
> +	VFIO_ASSERT_GT(read(fd, buf, ARRAY_SIZE(buf)), 0);
> +	VFIO_ASSERT_EQ(close(fd), 0);
> +
> +	return strtol(buf, NULL, 0);
> +}
> +
> +static void sysfs_set_val(const char *component, const char *name,
> +			  const char *file, const char *val)
> +{
> +	char path[PATH_MAX] = {0};

Ditto here about zero-intialization being unnecessary.

> +	int fd;
> +
> +	snprintf(path, PATH_MAX, "/sys/bus/pci/%s/%s/%s", component, name, file);

Ditto here about snprintf_assert()

You get the idea... I won't comment on the ones below.

> +	VFIO_ASSERT_GT(fd = open(path, O_WRONLY), 0);
> +
> +	VFIO_ASSERT_EQ(write(fd, val, strlen(val)), strlen(val));
> +	VFIO_ASSERT_EQ(close(fd), 0);
> +}
> +
> +static int sysfs_get_device_val(const char *bdf, const char *file)
> +{
> +	sysfs_get_val("devices", bdf, file);
> +}
> +
> +static void sysfs_set_device_val(const char *bdf, const char *file, const char *val)
> +{
> +	sysfs_set_val("devices", bdf, file, val);
> +}
> +
> +static void sysfs_set_driver_val(const char *driver, const char *file, const char *val)
> +{
> +	sysfs_set_val("drivers", driver, file, val);
> +}
> +
> +static void sysfs_set_device_val_int(const char *bdf, const char *file, int val)
> +{
> +	char val_str[32] = {0};
> +
> +	snprintf(val_str, sizeof(val_str), "%d", val);
> +	sysfs_set_device_val(bdf, file, val_str);
> +}
> +
> +int sysfs_get_sriov_totalvfs(const char *bdf)
> +{
> +	return sysfs_get_device_val(bdf, "sriov_totalvfs");
> +}
> +
> +int sysfs_get_sriov_numvfs(const char *bdf)
> +{
> +	return sysfs_get_device_val(bdf, "sriov_numvfs");
> +}
> +
> +void sysfs_set_sriov_numvfs(const char *bdf, int numvfs)
> +{
> +	sysfs_set_device_val_int(bdf, "sriov_numvfs", numvfs);
> +}
> +
> +bool sysfs_get_sriov_drivers_autoprobe(const char *bdf)
> +{
> +	return (bool)sysfs_get_device_val(bdf, "sriov_drivers_autoprobe");
> +}
> +
> +void sysfs_set_sriov_drivers_autoprobe(const char *bdf, bool val)
> +{
> +	sysfs_set_device_val_int(bdf, "sriov_drivers_autoprobe", val);
> +}
> +
> +void sysfs_bind_driver(const char *bdf, const char *driver)
> +{
> +	sysfs_set_driver_val(driver, "bind", bdf);
> +}
> +
> +void sysfs_unbind_driver(const char *bdf, const char *driver)
> +{
> +	sysfs_set_driver_val(driver, "unbind", bdf);
> +}
> +
> +void sysfs_get_sriov_vf_bdf(const char *pf_bdf, int i, char *out_vf_bdf)
> +{
> +	char vf_path[PATH_MAX] = {0};
> +	char path[PATH_MAX] = {0};
> +	int ret;
> +
> +	snprintf(path, PATH_MAX, "/sys/bus/pci/devices/%s/virtfn%d", pf_bdf, i);
> +
> +	ret = readlink(path, vf_path, PATH_MAX);
> +	VFIO_ASSERT_NE(ret, -1);
> +
> +	ret = sscanf(basename(vf_path), "%s", out_vf_bdf);
> +	VFIO_ASSERT_EQ(ret, 1);
> +}
> +
> +unsigned int sysfs_get_device_group(const char *bdf)

nit: s/device_group/iommu_group/

> +{
> +	char dev_iommu_group_path[PATH_MAX] = {0};
> +	char path[PATH_MAX] = {0};
> +	unsigned int group;
> +	int ret;
> +
> +	snprintf(path, PATH_MAX, "/sys/bus/pci/devices/%s/iommu_group", bdf);
> +
> +	ret = readlink(path, dev_iommu_group_path, sizeof(dev_iommu_group_path));
> +	VFIO_ASSERT_NE(ret, -1, "Failed to get the IOMMU group for device: %s\n", bdf);
> +
> +	ret = sscanf(basename(dev_iommu_group_path), "%u", &group);
> +	VFIO_ASSERT_EQ(ret, 1, "Failed to get the IOMMU group for device: %s\n", bdf);
> +
> +	return group;
> +}
> +
> +int sysfs_get_driver(const char *bdf, char *out_driver)
> +{
> +	char driver_path[PATH_MAX] = {0};
> +	char path[PATH_MAX] = {0};
> +	int ret;
> +
> +	snprintf(path, PATH_MAX, "/sys/bus/pci/devices/%s/driver", bdf);
> +	ret = readlink(path, driver_path, PATH_MAX);
> +	if (ret == -1) {
> +		if (errno == ENOENT)
> +			return -1;
> +
> +		VFIO_FAIL("Failed to read %s\n", path);
> +	}
> +
> +	ret = sscanf(basename(driver_path), "%s", out_driver);

I think this is equivalent to:

  out_driver = basename(driver_path);

... which means out_driver to point within driver_path, which is stack
allocated? I think you want to do strcpy() after basename() to copy the
driver name to out_driver.

Also how do you prevent overflowing out_driver? Maybe it would be
cleaner for sysfs_get_driver() to allocate out_driver and return it to
the caller?

We can return an empty string for the ENOENT case.

> +	VFIO_ASSERT_EQ(ret, 1);
> +
> +	return 0;
> +}
> diff --git a/tools/testing/selftests/vfio/lib/vfio_pci_device.c b/tools/testing/selftests/vfio/lib/vfio_pci_device.c
> index 64a19481b734f..9b2a123cee5fc 100644
> --- a/tools/testing/selftests/vfio/lib/vfio_pci_device.c
> +++ b/tools/testing/selftests/vfio/lib/vfio_pci_device.c
> @@ -22,8 +22,6 @@
>  #include "../../../kselftest.h"
>  #include <libvfio.h>
>  
> -#define PCI_SYSFS_PATH	"/sys/bus/pci/devices"
> -
>  static void vfio_pci_irq_set(struct vfio_pci_device *device,
>  			     u32 index, u32 vector, u32 count, int *fds)
>  {
> @@ -181,24 +179,6 @@ void vfio_pci_device_reset(struct vfio_pci_device *device)
>  	ioctl_assert(device->fd, VFIO_DEVICE_RESET, NULL);
>  }
>  
> -static unsigned int vfio_pci_get_group_from_dev(const char *bdf)
> -{
> -	char dev_iommu_group_path[PATH_MAX] = {0};
> -	char sysfs_path[PATH_MAX] = {0};
> -	unsigned int group;
> -	int ret;
> -
> -	snprintf_assert(sysfs_path, PATH_MAX, "%s/%s/iommu_group", PCI_SYSFS_PATH, bdf);
> -
> -	ret = readlink(sysfs_path, dev_iommu_group_path, sizeof(dev_iommu_group_path));
> -	VFIO_ASSERT_NE(ret, -1, "Failed to get the IOMMU group for device: %s\n", bdf);
> -
> -	ret = sscanf(basename(dev_iommu_group_path), "%u", &group);
> -	VFIO_ASSERT_EQ(ret, 1, "Failed to get the IOMMU group for device: %s\n", bdf);
> -
> -	return group;
> -}
> -
>  static void vfio_pci_group_setup(struct vfio_pci_device *device, const char *bdf)
>  {
>  	struct vfio_group_status group_status = {
> @@ -207,7 +187,7 @@ static void vfio_pci_group_setup(struct vfio_pci_device *device, const char *bdf
>  	char group_path[32];
>  	int group;
>  
> -	group = vfio_pci_get_group_from_dev(bdf);
> +	group = sysfs_get_device_group(bdf);
>  	snprintf_assert(group_path, sizeof(group_path), "/dev/vfio/%d", group);
>  
>  	device->group_fd = open(group_path, O_RDWR);
> -- 
> 2.52.0.239.gd5f0c6e74e-goog
> 

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

* Re: [PATCH v2 3/6] vfio: selftests: Extend container/iommufd setup for passing vf_token
  2025-12-10 18:14 ` [PATCH v2 3/6] vfio: selftests: Extend container/iommufd setup for passing vf_token Raghavendra Rao Ananta
@ 2026-01-07 22:49   ` David Matlack
  2026-01-08 21:34     ` Raghavendra Rao Ananta
  0 siblings, 1 reply; 28+ messages in thread
From: David Matlack @ 2026-01-07 22:49 UTC (permalink / raw)
  To: Raghavendra Rao Ananta
  Cc: Alex Williamson, Alex Williamson, Josh Hilke, kvm, linux-kernel

On 2025-12-10 06:14 PM, Raghavendra Rao Ananta wrote:
> A UUID is normally set as a vf_token to correspond the VFs with the
> PFs, if they are both bound by the vfio-pci driver. This is true for
> iommufd-based approach and container-based approach. The token can be
> set either during device creation (VFIO_GROUP_GET_DEVICE_FD) in
> container-based approach or during iommu bind (VFIO_DEVICE_BIND_IOMMUFD)
> in the iommu-fd case. Hence extend the functions,
         iommufd

> vfio_pci_iommufd_setup() and vfio_pci_container_setup(), to accept
> vf_token as an (optional) argument and handle the necessary setup.
> 
> No functional changes are expected.
> 
> Signed-off-by: Raghavendra Rao Ananta <rananta@google.com>
> ---
>  tools/testing/selftests/vfio/lib/libvfio.mk   |  4 +-
>  .../selftests/vfio/lib/vfio_pci_device.c      | 45 +++++++++++++++----
>  2 files changed, 40 insertions(+), 9 deletions(-)
> 
> diff --git a/tools/testing/selftests/vfio/lib/libvfio.mk b/tools/testing/selftests/vfio/lib/libvfio.mk
> index b7857319c3f1f..459b14c6885a8 100644
> --- a/tools/testing/selftests/vfio/lib/libvfio.mk
> +++ b/tools/testing/selftests/vfio/lib/libvfio.mk
> @@ -15,6 +15,8 @@ LIBVFIO_C += drivers/ioat/ioat.c
>  LIBVFIO_C += drivers/dsa/dsa.c
>  endif
>  
> +LDLIBS += -luuid
> +
>  LIBVFIO_OUTPUT := $(OUTPUT)/libvfio
>  
>  LIBVFIO_O := $(patsubst %.c, $(LIBVFIO_OUTPUT)/%.o, $(LIBVFIO_C))
> @@ -25,6 +27,6 @@ $(shell mkdir -p $(LIBVFIO_O_DIRS))
>  CFLAGS += -I$(LIBVFIO_SRCDIR)/include
>  
>  $(LIBVFIO_O): $(LIBVFIO_OUTPUT)/%.o : $(LIBVFIO_SRCDIR)/%.c
> -	$(CC) $(CFLAGS) $(CPPFLAGS) $(TARGET_ARCH) -c $< -o $@
> +	$(CC) $(CFLAGS) $(CPPFLAGS) $(TARGET_ARCH) -c $< $(LDLIBS) -o $@

Do we need $(LDLIBS) when compiling the intermediate .o files? I thought
we would only need it when linking the selftests binaries.

>  
>  EXTRA_CLEAN += $(LIBVFIO_OUTPUT)
> diff --git a/tools/testing/selftests/vfio/lib/vfio_pci_device.c b/tools/testing/selftests/vfio/lib/vfio_pci_device.c
> index 9b2a123cee5fc..ac9a5244ddc46 100644
> --- a/tools/testing/selftests/vfio/lib/vfio_pci_device.c
> +++ b/tools/testing/selftests/vfio/lib/vfio_pci_device.c
> @@ -12,6 +12,7 @@
>  #include <sys/mman.h>
>  
>  #include <uapi/linux/types.h>
> +#include <uuid/uuid.h>
>  #include <linux/iommufd.h>
>  #include <linux/limits.h>
>  #include <linux/mman.h>
> @@ -199,7 +200,27 @@ static void vfio_pci_group_setup(struct vfio_pci_device *device, const char *bdf
>  	ioctl_assert(device->group_fd, VFIO_GROUP_SET_CONTAINER, &device->iommu->container_fd);
>  }
>  
> -static void vfio_pci_container_setup(struct vfio_pci_device *device, const char *bdf)
> +static void vfio_pci_group_get_device_fd(struct vfio_pci_device *device,
> +					 const char *bdf, const char *vf_token)
> +{
> +	char arg[64] = {0};

unnecessary intitialization

> +
> +	/*
> +	 * If a vf_token exists, argument to VFIO_GROUP_GET_DEVICE_FD
> +	 * will be in the form of the following example:
> +	 * "0000:04:10.0 vf_token=bd8d9d2b-5a5f-4f5a-a211-f591514ba1f3"
> +	 */
> +	if (vf_token)
> +		snprintf(arg, ARRAY_SIZE(arg), "%s vf_token=%s", bdf, vf_token);

snprintf_assert() :)

> +	else
> +		snprintf(arg, ARRAY_SIZE(arg), "%s", bdf);
> +
> +	device->fd = ioctl(device->group_fd, VFIO_GROUP_GET_DEVICE_FD, arg);
> +	VFIO_ASSERT_GE(device->fd, 0);
> +}
> +
> +static void vfio_pci_container_setup(struct vfio_pci_device *device,
> +				     const char *bdf, const char *vf_token)
>  {
>  	struct iommu *iommu = device->iommu;
>  	unsigned long iommu_type = iommu->mode->iommu_type;
> @@ -217,8 +238,7 @@ static void vfio_pci_container_setup(struct vfio_pci_device *device, const char
>  	 */
>  	(void)ioctl(iommu->container_fd, VFIO_SET_IOMMU, (void *)iommu_type);
>  
> -	device->fd = ioctl(device->group_fd, VFIO_GROUP_GET_DEVICE_FD, bdf);
> -	VFIO_ASSERT_GE(device->fd, 0);
> +	vfio_pci_group_get_device_fd(device, bdf, vf_token);
>  }
>  
>  static void vfio_pci_device_setup(struct vfio_pci_device *device)
> @@ -279,12 +299,20 @@ const char *vfio_pci_get_cdev_path(const char *bdf)
>  	return cdev_path;
>  }
>  
> -static void vfio_device_bind_iommufd(int device_fd, int iommufd)
> +static void vfio_device_bind_iommufd(int device_fd, int iommufd,
> +				     const char *vf_token)
>  {
>  	struct vfio_device_bind_iommufd args = {
>  		.argsz = sizeof(args),
>  		.iommufd = iommufd,
>  	};
> +	uuid_t token_uuid = {0};

unnecessary intitialization

> +
> +	if (vf_token) {
> +		VFIO_ASSERT_EQ(uuid_parse(vf_token, token_uuid), 0);
> +		args.flags |= VFIO_DEVICE_BIND_FLAG_TOKEN;
> +		args.token_uuid_ptr = (u64)token_uuid;
> +	}
>  
>  	ioctl_assert(device_fd, VFIO_DEVICE_BIND_IOMMUFD, &args);
>  }
> @@ -299,7 +327,8 @@ static void vfio_device_attach_iommufd_pt(int device_fd, u32 pt_id)
>  	ioctl_assert(device_fd, VFIO_DEVICE_ATTACH_IOMMUFD_PT, &args);
>  }
>  
> -static void vfio_pci_iommufd_setup(struct vfio_pci_device *device, const char *bdf)
> +static void vfio_pci_iommufd_setup(struct vfio_pci_device *device,
> +				   const char *bdf, const char *vf_token)
>  {
>  	const char *cdev_path = vfio_pci_get_cdev_path(bdf);
>  
> @@ -307,7 +336,7 @@ static void vfio_pci_iommufd_setup(struct vfio_pci_device *device, const char *b
>  	VFIO_ASSERT_GE(device->fd, 0);
>  	free((void *)cdev_path);
>  
> -	vfio_device_bind_iommufd(device->fd, device->iommu->iommufd);
> +	vfio_device_bind_iommufd(device->fd, device->iommu->iommufd, vf_token);
>  	vfio_device_attach_iommufd_pt(device->fd, device->iommu->ioas_id);
>  }
>  
> @@ -323,9 +352,9 @@ struct vfio_pci_device *vfio_pci_device_init(const char *bdf, struct iommu *iomm
>  	device->bdf = bdf;
>  
>  	if (iommu->mode->container_path)
> -		vfio_pci_container_setup(device, bdf);
> +		vfio_pci_container_setup(device, bdf, NULL);
>  	else
> -		vfio_pci_iommufd_setup(device, bdf);
> +		vfio_pci_iommufd_setup(device, bdf, NULL);
>  
>  	vfio_pci_device_setup(device);
>  	vfio_pci_driver_probe(device);
> -- 
> 2.52.0.239.gd5f0c6e74e-goog
> 

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

* Re: [PATCH v2 4/6] vfio: selftests: Export more vfio_pci functions
  2025-12-10 18:14 ` [PATCH v2 4/6] vfio: selftests: Export more vfio_pci functions Raghavendra Rao Ananta
@ 2026-01-07 22:55   ` David Matlack
  2026-01-07 23:05   ` David Matlack
  1 sibling, 0 replies; 28+ messages in thread
From: David Matlack @ 2026-01-07 22:55 UTC (permalink / raw)
  To: Raghavendra Rao Ananta
  Cc: Alex Williamson, Alex Williamson, Josh Hilke, kvm, linux-kernel

On 2025-12-10 06:14 PM, Raghavendra Rao Ananta wrote:

shotlog nit: "Expose more vfio_pci_device functions"

> Refactor and make the functions called under device initialization
> public. A later patch adds a test that calls these functions to validate
> the UAPI of SR-IOV devices. Opportunistically, to test the success
> and failure cases of the UAPI, split the functions dealing with
> VFIO_GROUP_GET_DEVICE_FD and VFIO_DEVICE_BIND_IOMMUFD into a core
> function and another one that asserts the ioctl. The former will be
> used for testing the SR-IOV UAPI, hence only export these.
> 
> Signed-off-by: Raghavendra Rao Ananta <rananta@google.com>
> ---
>  .../lib/include/libvfio/vfio_pci_device.h     |  7 +++
>  .../selftests/vfio/lib/vfio_pci_device.c      | 44 ++++++++++++++-----
>  2 files changed, 39 insertions(+), 12 deletions(-)
> 
> diff --git a/tools/testing/selftests/vfio/lib/include/libvfio/vfio_pci_device.h b/tools/testing/selftests/vfio/lib/include/libvfio/vfio_pci_device.h
> index 2858885a89bbb..6186ca463ca6e 100644
> --- a/tools/testing/selftests/vfio/lib/include/libvfio/vfio_pci_device.h
> +++ b/tools/testing/selftests/vfio/lib/include/libvfio/vfio_pci_device.h
> @@ -122,4 +122,11 @@ static inline bool vfio_pci_device_match(struct vfio_pci_device *device,
>  
>  const char *vfio_pci_get_cdev_path(const char *bdf);
>  
> +void vfio_pci_group_setup(struct vfio_pci_device *device, const char *bdf);
> +void __vfio_pci_group_get_device_fd(struct vfio_pci_device *device,
> +				    const char *bdf, const char *vf_token);
> +void vfio_container_set_iommu(struct vfio_pci_device *device);
> +void vfio_pci_iommufd_cdev_open(struct vfio_pci_device *device, const char *bdf);
> +int __vfio_device_bind_iommufd(int device_fd, int iommufd, const char *vf_token);
> +
>  #endif /* SELFTESTS_VFIO_LIB_INCLUDE_LIBVFIO_VFIO_PCI_DEVICE_H */
> diff --git a/tools/testing/selftests/vfio/lib/vfio_pci_device.c b/tools/testing/selftests/vfio/lib/vfio_pci_device.c
> index ac9a5244ddc46..208da2704d9e2 100644
> --- a/tools/testing/selftests/vfio/lib/vfio_pci_device.c
> +++ b/tools/testing/selftests/vfio/lib/vfio_pci_device.c
> @@ -180,7 +180,7 @@ void vfio_pci_device_reset(struct vfio_pci_device *device)
>  	ioctl_assert(device->fd, VFIO_DEVICE_RESET, NULL);
>  }
>  
> -static void vfio_pci_group_setup(struct vfio_pci_device *device, const char *bdf)
> +void vfio_pci_group_setup(struct vfio_pci_device *device, const char *bdf)
>  {
>  	struct vfio_group_status group_status = {
>  		.argsz = sizeof(group_status),
> @@ -200,8 +200,8 @@ static void vfio_pci_group_setup(struct vfio_pci_device *device, const char *bdf
>  	ioctl_assert(device->group_fd, VFIO_GROUP_SET_CONTAINER, &device->iommu->container_fd);
>  }
>  
> -static void vfio_pci_group_get_device_fd(struct vfio_pci_device *device,
> -					 const char *bdf, const char *vf_token)
> +void __vfio_pci_group_get_device_fd(struct vfio_pci_device *device,
> +				    const char *bdf, const char *vf_token)
>  {
>  	char arg[64] = {0};
>  
> @@ -216,18 +216,21 @@ static void vfio_pci_group_get_device_fd(struct vfio_pci_device *device,
>  		snprintf(arg, ARRAY_SIZE(arg), "%s", bdf);
>  
>  	device->fd = ioctl(device->group_fd, VFIO_GROUP_GET_DEVICE_FD, arg);
> +}
> +
> +static void vfio_pci_group_get_device_fd(struct vfio_pci_device *device,
> +					 const char *bdf, const char *vf_token)
> +{
> +	__vfio_pci_group_get_device_fd(device, bdf, vf_token);
>  	VFIO_ASSERT_GE(device->fd, 0);
>  }
>  
> -static void vfio_pci_container_setup(struct vfio_pci_device *device,
> -				     const char *bdf, const char *vf_token)
> +void vfio_container_set_iommu(struct vfio_pci_device *device)
>  {
>  	struct iommu *iommu = device->iommu;
>  	unsigned long iommu_type = iommu->mode->iommu_type;
>  	int ret;
>  
> -	vfio_pci_group_setup(device, bdf);
> -
>  	ret = ioctl(iommu->container_fd, VFIO_CHECK_EXTENSION, iommu_type);
>  	VFIO_ASSERT_GT(ret, 0, "VFIO IOMMU type %lu not supported\n", iommu_type);
>  
> @@ -237,7 +240,13 @@ static void vfio_pci_container_setup(struct vfio_pci_device *device,
>  	 * because the IOMMU type is already set.
>  	 */
>  	(void)ioctl(iommu->container_fd, VFIO_SET_IOMMU, (void *)iommu_type);
> +}
>  
> +static void vfio_pci_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);
>  }
>  
> @@ -299,8 +308,7 @@ const char *vfio_pci_get_cdev_path(const char *bdf)
>  	return cdev_path;
>  }
>  
> -static void vfio_device_bind_iommufd(int device_fd, int iommufd,
> -				     const char *vf_token)
> +int __vfio_device_bind_iommufd(int device_fd, int iommufd, const char *vf_token)
>  {
>  	struct vfio_device_bind_iommufd args = {
>  		.argsz = sizeof(args),
> @@ -314,7 +322,15 @@ static void vfio_device_bind_iommufd(int device_fd, int iommufd,
>  		args.token_uuid_ptr = (u64)token_uuid;
>  	}
>  
> -	ioctl_assert(device_fd, VFIO_DEVICE_BIND_IOMMUFD, &args);
> +	return ioctl(device_fd, VFIO_DEVICE_BIND_IOMMUFD, &args);
> +}
> +
> +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);
> +
> +	VFIO_ASSERT_EQ(ret, 0, "Failed VFIO_DEVICE_BIND_IOMMUFD ioctl\n");
>  }
>  
>  static void vfio_device_attach_iommufd_pt(int device_fd, u32 pt_id)
> @@ -327,15 +343,19 @@ static void vfio_device_attach_iommufd_pt(int device_fd, u32 pt_id)
>  	ioctl_assert(device_fd, VFIO_DEVICE_ATTACH_IOMMUFD_PT, &args);
>  }
>  
> -static void vfio_pci_iommufd_setup(struct vfio_pci_device *device,
> -				   const char *bdf, const char *vf_token)
> +void vfio_pci_iommufd_cdev_open(struct vfio_pci_device *device, const char *bdf)

nit: vfio_pci_cdev_open()

Opening the cdev doesn't interact with iommufd in the kernel in any way.
It's a pure VFIO operation. So having iommufd in the name here might be
confusing.

>  {
>  	const char *cdev_path = vfio_pci_get_cdev_path(bdf);
>  
>  	device->fd = open(cdev_path, O_RDWR);
>  	VFIO_ASSERT_GE(device->fd, 0);
>  	free((void *)cdev_path);
> +}
>  
> +static void vfio_pci_iommufd_setup(struct vfio_pci_device *device,
> +				   const char *bdf, const char *vf_token)
> +{
> +	vfio_pci_iommufd_cdev_open(device, bdf);
>  	vfio_device_bind_iommufd(device->fd, device->iommu->iommufd, vf_token);
>  	vfio_device_attach_iommufd_pt(device->fd, device->iommu->ioas_id);
>  }
> -- 
> 2.52.0.239.gd5f0c6e74e-goog
> 

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

* Re: [PATCH v2 5/6] vfio: selftests: Add helper to set/override a vf_token
  2025-12-10 18:14 ` [PATCH v2 5/6] vfio: selftests: Add helper to set/override a vf_token Raghavendra Rao Ananta
@ 2026-01-07 22:56   ` David Matlack
  2026-01-08 21:45     ` Raghavendra Rao Ananta
  0 siblings, 1 reply; 28+ messages in thread
From: David Matlack @ 2026-01-07 22:56 UTC (permalink / raw)
  To: Raghavendra Rao Ananta
  Cc: Alex Williamson, Alex Williamson, Josh Hilke, kvm, linux-kernel

On 2025-12-10 06:14 PM, Raghavendra Rao Ananta wrote:
> Add a helper function, vfio_device_set_vf_token(), to set or override a
> vf_token. Not only at init, but a vf_token can also be set via the
> VFIO_DEVICE_FEATURE ioctl, by setting the
> VFIO_DEVICE_FEATURE_PCI_VF_TOKEN flag. Hence, add an API to utilize this
> functionality from the test code. The subsequent commit will use this to
> test the functionality of this method to set the vf_token.
> 
> Signed-off-by: Raghavendra Rao Ananta <rananta@google.com>
> ---
>  .../lib/include/libvfio/vfio_pci_device.h     |  2 ++
>  .../selftests/vfio/lib/vfio_pci_device.c      | 34 +++++++++++++++++++
>  2 files changed, 36 insertions(+)
> 
> diff --git a/tools/testing/selftests/vfio/lib/include/libvfio/vfio_pci_device.h b/tools/testing/selftests/vfio/lib/include/libvfio/vfio_pci_device.h
> index 6186ca463ca6e..b370aa6a74d0b 100644
> --- a/tools/testing/selftests/vfio/lib/include/libvfio/vfio_pci_device.h
> +++ b/tools/testing/selftests/vfio/lib/include/libvfio/vfio_pci_device.h
> @@ -129,4 +129,6 @@ void vfio_container_set_iommu(struct vfio_pci_device *device);
>  void vfio_pci_iommufd_cdev_open(struct vfio_pci_device *device, const char *bdf);
>  int __vfio_device_bind_iommufd(int device_fd, int iommufd, const char *vf_token);
>  
> +void vfio_device_set_vf_token(int fd, const char *vf_token);
> +
>  #endif /* SELFTESTS_VFIO_LIB_INCLUDE_LIBVFIO_VFIO_PCI_DEVICE_H */
> diff --git a/tools/testing/selftests/vfio/lib/vfio_pci_device.c b/tools/testing/selftests/vfio/lib/vfio_pci_device.c
> index 208da2704d9e2..7725ecc62b024 100644
> --- a/tools/testing/selftests/vfio/lib/vfio_pci_device.c
> +++ b/tools/testing/selftests/vfio/lib/vfio_pci_device.c
> @@ -109,6 +109,40 @@ static void vfio_pci_irq_get(struct vfio_pci_device *device, u32 index,
>  	ioctl_assert(device->fd, VFIO_DEVICE_GET_IRQ_INFO, irq_info);
>  }
>  
> +static int vfio_device_feature_ioctl(int fd, u32 flags, void *data,
> +				     size_t data_size)
> +{
> +	u8 buffer[sizeof(struct vfio_device_feature) + data_size] = {};
> +	struct vfio_device_feature *feature = (void *)buffer;
> +
> +	memcpy(feature->data, data, data_size);
> +
> +	feature->argsz = sizeof(buffer);
> +	feature->flags = flags;
> +
> +	return ioctl(fd, VFIO_DEVICE_FEATURE, feature);
> +}
> +
> +static void vfio_device_feature_set(int fd, u16 feature, void *data, size_t data_size)
> +{
> +	u32 flags = VFIO_DEVICE_FEATURE_SET | feature;
> +	int ret;
> +
> +	ret = vfio_device_feature_ioctl(fd, flags, data, data_size);
> +	VFIO_ASSERT_EQ(ret, 0, "Failed to set feature %u\n", feature);
> +}
> +
> +void vfio_device_set_vf_token(int fd, const char *vf_token)
> +{
> +	uuid_t token_uuid = {0};
> +
> +	VFIO_ASSERT_NOT_NULL(vf_token, "vf_token is NULL");
> +	VFIO_ASSERT_EQ(uuid_parse(vf_token, token_uuid), 0);
> +
> +	vfio_device_feature_set(fd, VFIO_DEVICE_FEATURE_PCI_VF_TOKEN,
> +				token_uuid, sizeof(uuid_t));
> +}

Would it be useful to have a variant that returns an int for negative
testing?

> +
>  static void vfio_pci_region_get(struct vfio_pci_device *device, int index,
>  				struct vfio_region_info *info)
>  {
> -- 
> 2.52.0.239.gd5f0c6e74e-goog
> 

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

* Re: [PATCH v2 4/6] vfio: selftests: Export more vfio_pci functions
  2025-12-10 18:14 ` [PATCH v2 4/6] vfio: selftests: Export more vfio_pci functions Raghavendra Rao Ananta
  2026-01-07 22:55   ` David Matlack
@ 2026-01-07 23:05   ` David Matlack
  2026-01-08 21:47     ` Raghavendra Rao Ananta
  1 sibling, 1 reply; 28+ messages in thread
From: David Matlack @ 2026-01-07 23:05 UTC (permalink / raw)
  To: Raghavendra Rao Ananta
  Cc: Alex Williamson, Alex Williamson, Josh Hilke, kvm, linux-kernel

On 2025-12-10 06:14 PM, Raghavendra Rao Ananta wrote:

> -static void vfio_device_bind_iommufd(int device_fd, int iommufd,
> -				     const char *vf_token)
> +int __vfio_device_bind_iommufd(int device_fd, int iommufd, const char *vf_token)
>  {
>  	struct vfio_device_bind_iommufd args = {
>  		.argsz = sizeof(args),
> @@ -314,7 +322,15 @@ static void vfio_device_bind_iommufd(int device_fd, int iommufd,
>  		args.token_uuid_ptr = (u64)token_uuid;
>  	}
>  
> -	ioctl_assert(device_fd, VFIO_DEVICE_BIND_IOMMUFD, &args);
> +	return ioctl(device_fd, VFIO_DEVICE_BIND_IOMMUFD, &args);

For ioctls that return 0 on success and -1 on error, let's follow the
precedent set in iommu.c and return -errno on error from our library
functions. i.e.

       if (ioctl(device_fd, VFIO_DEVICE_BIND_IOMMUFD, &args))
               return -errno;

       return 0;

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

* Re: [PATCH v2 6/6] vfio: selftests: Add tests to validate SR-IOV UAPI
  2025-12-10 18:14 ` [PATCH v2 6/6] vfio: selftests: Add tests to validate SR-IOV UAPI Raghavendra Rao Ananta
  2025-12-12 18:21   ` Raghavendra Rao Ananta
  2025-12-18 23:26   ` David Matlack
@ 2026-01-07 23:22   ` David Matlack
  2026-01-09 19:05     ` Raghavendra Rao Ananta
  2 siblings, 1 reply; 28+ messages in thread
From: David Matlack @ 2026-01-07 23:22 UTC (permalink / raw)
  To: Raghavendra Rao Ananta
  Cc: Alex Williamson, Alex Williamson, Josh Hilke, kvm, linux-kernel

On 2025-12-10 06:14 PM, Raghavendra Rao Ananta wrote:
> Add a selfttest, 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>
> ---
>  tools/testing/selftests/vfio/Makefile         |   1 +
>  .../selftests/vfio/vfio_pci_sriov_uapi_test.c | 215 ++++++++++++++++++
>  2 files changed, 216 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 3c796ca99a509..f00a63902fbfb 100644
> --- a/tools/testing/selftests/vfio/Makefile
> +++ b/tools/testing/selftests/vfio/Makefile
> @@ -4,6 +4,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..4c2951d6e049c
> --- /dev/null
> +++ b/tools/testing/selftests/vfio/vfio_pci_sriov_uapi_test.c
> @@ -0,0 +1,215 @@
> +// 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_dev_bdf;

nit: I think you could simplify some of the names in this file. This
code isn't in a library so the names dont' have to be globally unique
and quite so long.

  s/pf_dev_bdf/pf_bdf/
  s/vf_dev_bdf/vf_bdf/
  s/pf_device/pf/
  s/vf_device/vf/
  s/test_vfio_pci_container_setup/container_setup/
  s/test_vfio_pci_iommufd_setup/iommufd_setup/
  s/test_vfio_pci_device_init/device_init/
  s/test_vfio_pci_device_cleanup/device_cleanup/

Feel free to ignore this though if you think it makes the names too
terse.

> +
> +static int test_vfio_pci_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 test_vfio_pci_iommufd_setup(struct vfio_pci_device *device,
> +				       const char *bdf, const char *vf_token)
> +{
> +	vfio_pci_iommufd_cdev_open(device, bdf);
> +	return __vfio_device_bind_iommufd(device->fd,
> +					  device->iommu->iommufd, vf_token);
> +}
> +
> +static struct vfio_pci_device *test_vfio_pci_device_init(const char *bdf,
> +							 struct iommu *iommu,
> +							 const char *vf_token,
> +							 int *out_ret)
> +{
> +	struct vfio_pci_device *device;
> +
> +	device = calloc(1, sizeof(*device));
> +	VFIO_ASSERT_NOT_NULL(device);
> +
> +	device->iommu = iommu;
> +	device->bdf = bdf;

Can you put this in a helper exposed by vfio_pci_device.h? e.g.
vfio_pci_device_alloc()

> +
> +	if (iommu->mode->container_path)
> +		*out_ret = test_vfio_pci_container_setup(device, bdf, vf_token);
> +	else
> +		*out_ret = test_vfio_pci_iommufd_setup(device, bdf, vf_token);
> +
> +	return device;
> +}
> +
> +static void test_vfio_pci_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);
> +
> +	free(device);
> +}
> +
> +FIXTURE(vfio_pci_sriov_uapi_test) {
> +	char vf_dev_bdf[16];
> +	char vf_driver[32];
> +	bool sriov_drivers_autoprobe;
> +};
> +
> +FIXTURE_SETUP(vfio_pci_sriov_uapi_test)
> +{
> +	int nr_vfs;
> +	int ret;
> +
> +	nr_vfs = sysfs_get_sriov_totalvfs(pf_dev_bdf);
> +	if (nr_vfs < 0)
> +		SKIP(return, "SR-IOV may not be supported by the device\n");

Should this be <= 0?

And replace "the device" with the BDF.

> +
> +	nr_vfs = sysfs_get_sriov_numvfs(pf_dev_bdf);
> +	if (nr_vfs != 0)
> +		SKIP(return, "SR-IOV already configured for the PF\n");

Let's print the BDF and nr_vfs for the user.

> +
> +	self->sriov_drivers_autoprobe =
> +		sysfs_get_sriov_drivers_autoprobe(pf_dev_bdf);
> +	if (self->sriov_drivers_autoprobe)
> +		sysfs_set_sriov_drivers_autoprobe(pf_dev_bdf, 0);
> +
> +	/* Export only one VF for testing */

s/Export/Create/

> +	sysfs_set_sriov_numvfs(pf_dev_bdf, 1);
> +
> +	sysfs_get_sriov_vf_bdf(pf_dev_bdf, 0, self->vf_dev_bdf);
> +	if (sysfs_get_driver(self->vf_dev_bdf, self->vf_driver) == 0)
> +		sysfs_unbind_driver(self->vf_dev_bdf, self->vf_driver);

This should be impossible since we disabled autoprobing.

> +	sysfs_bind_driver(self->vf_dev_bdf, "vfio-pci");

Some devices also require setting driver_override to "vfio-pci" as well
so the device can be bound to vfio-pci. Let's just do that
unconditionally.

> +}
> +
> +FIXTURE_TEARDOWN(vfio_pci_sriov_uapi_test)
> +{
> +	sysfs_unbind_driver(self->vf_dev_bdf, "vfio-pci");
> +	sysfs_bind_driver(self->vf_dev_bdf, self->vf_driver);
> +	sysfs_set_sriov_numvfs(pf_dev_bdf, 0);
> +	sysfs_set_sriov_drivers_autoprobe(pf_dev_bdf,
> +					  self->sriov_drivers_autoprobe);
> +}
> +
> +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_device;
> +	struct vfio_pci_device *vf_device;
> +	struct iommu *iommu;
> +	int ret;
> +
> +	iommu = iommu_init(variant->iommu_mode);
> +	pf_device = test_vfio_pci_device_init(pf_dev_bdf, iommu, UUID_1, &ret);
> +	vf_device = test_vfio_pci_device_init(self->vf_dev_bdf, iommu,
> +					      variant->vf_token, &ret);
> +
> +	ASSERT_VF_CREATION(ret);
> +
> +	test_vfio_pci_device_cleanup(vf_device);
> +	test_vfio_pci_device_cleanup(pf_device);
> +	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_device;
> +	struct vfio_pci_device *vf_device;
> +	struct iommu *iommu;
> +	int ret;
> +
> +	iommu = iommu_init(variant->iommu_mode);
> +	pf_device = test_vfio_pci_device_init(pf_dev_bdf, iommu, UUID_1, &ret);
> +	test_vfio_pci_device_cleanup(pf_device);
> +
> +	vf_device = test_vfio_pci_device_init(self->vf_dev_bdf, iommu,
> +					      variant->vf_token, &ret);
> +
> +	ASSERT_VF_CREATION(ret);
> +
> +	test_vfio_pci_device_cleanup(vf_device);
> +	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_device;
> +	struct vfio_pci_device *vf_device;
> +	struct iommu *iommu;
> +	int ret;
> +
> +	iommu = iommu_init(variant->iommu_mode);
> +	pf_device = test_vfio_pci_device_init(pf_dev_bdf, iommu, UUID_2, &ret);
> +	vfio_device_set_vf_token(pf_device->fd, UUID_1);
> +
> +	vf_device = test_vfio_pci_device_init(self->vf_dev_bdf, iommu,
> +					      variant->vf_token, &ret);
> +
> +	ASSERT_VF_CREATION(ret);
> +
> +	test_vfio_pci_device_cleanup(vf_device);
> +	test_vfio_pci_device_cleanup(pf_device);
> +	iommu_cleanup(iommu);
> +}
> +
> +int main(int argc, char *argv[])
> +{
> +	pf_dev_bdf = vfio_selftests_get_bdf(&argc, argv);
> +	return test_harness_run(argc, argv);
> +}
> -- 
> 2.52.0.239.gd5f0c6e74e-goog
> 

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

* Re: [PATCH v2 2/6] vfio: selftests: Introduce a sysfs lib
  2026-01-07 22:41   ` David Matlack
@ 2026-01-08 21:25     ` Raghavendra Rao Ananta
  0 siblings, 0 replies; 28+ messages in thread
From: Raghavendra Rao Ananta @ 2026-01-08 21:25 UTC (permalink / raw)
  To: David Matlack
  Cc: Alex Williamson, Alex Williamson, Josh Hilke, kvm, linux-kernel

On Wed, Jan 7, 2026 at 2:41 PM David Matlack <dmatlack@google.com> wrote:
>
> On 2025-12-10 06:14 PM, Raghavendra Rao Ananta wrote:
> > Introduce a sysfs liibrary to handle the common reads/writes to the
>                     library
>
> > PCI sysfs files, for example, getting the total number of VFs supported
> > by the device via /sys/bus/pci/devices/$BDF/sriov_totalvfs. The library
> > will be used in the upcoming test patch to configure the VFs for a given
> > PF device.
> >
> > Opportunistically, move vfio_pci_get_group_from_dev() to this library as
> > it falls under the same bucket. Rename it to sysfs_get_device_group() to
> > align with other function names.
> >
> > Signed-off-by: Raghavendra Rao Ananta <rananta@google.com>
> > ---
> >  .../selftests/vfio/lib/include/libvfio.h      |   1 +
> >  .../vfio/lib/include/libvfio/sysfs.h          |  16 ++
> >  tools/testing/selftests/vfio/lib/libvfio.mk   |   1 +
> >  tools/testing/selftests/vfio/lib/sysfs.c      | 151 ++++++++++++++++++
> >  .../selftests/vfio/lib/vfio_pci_device.c      |  22 +--
> >  5 files changed, 170 insertions(+), 21 deletions(-)
> >  create mode 100644 tools/testing/selftests/vfio/lib/include/libvfio/sysfs.h
> >  create mode 100644 tools/testing/selftests/vfio/lib/sysfs.c
> >
> > diff --git a/tools/testing/selftests/vfio/lib/include/libvfio.h b/tools/testing/selftests/vfio/lib/include/libvfio.h
> > index 279ddcd701944..bbe1d7616a648 100644
> > --- a/tools/testing/selftests/vfio/lib/include/libvfio.h
> > +++ b/tools/testing/selftests/vfio/lib/include/libvfio.h
> > @@ -5,6 +5,7 @@
> >  #include <libvfio/assert.h>
> >  #include <libvfio/iommu.h>
> >  #include <libvfio/iova_allocator.h>
> > +#include <libvfio/sysfs.h>
> >  #include <libvfio/vfio_pci_device.h>
> >  #include <libvfio/vfio_pci_driver.h>
> >
> > diff --git a/tools/testing/selftests/vfio/lib/include/libvfio/sysfs.h b/tools/testing/selftests/vfio/lib/include/libvfio/sysfs.h
> > new file mode 100644
> > index 0000000000000..1eca6b5cbcfcc
> > --- /dev/null
> > +++ b/tools/testing/selftests/vfio/lib/include/libvfio/sysfs.h
> > @@ -0,0 +1,16 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +#ifndef SELFTESTS_VFIO_LIB_INCLUDE_LIBVFIO_SYSFS_H
> > +#define SELFTESTS_VFIO_LIB_INCLUDE_LIBVFIO_SYSFS_H
> > +
> > +int sysfs_get_sriov_totalvfs(const char *bdf);
> > +int sysfs_get_sriov_numvfs(const char *bdf);
> > +void sysfs_set_sriov_numvfs(const char *bdfs, int numvfs);
> > +void sysfs_get_sriov_vf_bdf(const char *pf_bdf, int i, char *out_vf_bdf);
> > +bool sysfs_get_sriov_drivers_autoprobe(const char *bdf);
> > +void sysfs_set_sriov_drivers_autoprobe(const char *bdf, bool val);
> > +void sysfs_bind_driver(const char *bdf, const char *driver);
> > +void sysfs_unbind_driver(const char *bdf, const char *driver);
> > +int sysfs_get_driver(const char *bdf, char *out_driver);
> > +unsigned int sysfs_get_device_group(const char *bdf);
> > +
> > +#endif /* SELFTESTS_VFIO_LIB_INCLUDE_LIBVFIO_SYSFS_H */
> > diff --git a/tools/testing/selftests/vfio/lib/libvfio.mk b/tools/testing/selftests/vfio/lib/libvfio.mk
> > index 9f47bceed16f4..b7857319c3f1f 100644
> > --- a/tools/testing/selftests/vfio/lib/libvfio.mk
> > +++ b/tools/testing/selftests/vfio/lib/libvfio.mk
> > @@ -6,6 +6,7 @@ LIBVFIO_SRCDIR := $(selfdir)/vfio/lib
> >  LIBVFIO_C := iommu.c
> >  LIBVFIO_C += iova_allocator.c
> >  LIBVFIO_C += libvfio.c
> > +LIBVFIO_C += sysfs.c
> >  LIBVFIO_C += vfio_pci_device.c
> >  LIBVFIO_C += vfio_pci_driver.c
> >
> > diff --git a/tools/testing/selftests/vfio/lib/sysfs.c b/tools/testing/selftests/vfio/lib/sysfs.c
> > new file mode 100644
> > index 0000000000000..5551e8b981075
> > --- /dev/null
> > +++ b/tools/testing/selftests/vfio/lib/sysfs.c
> > @@ -0,0 +1,151 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +#include <fcntl.h>
> > +#include <unistd.h>
> > +#include <stdlib.h>
> > +#include <string.h>
> > +#include <linux/limits.h>
> > +
> > +#include <libvfio.h>
> > +
> > +static int sysfs_get_val(const char *component, const char *name,
>
> nit: I'm partial to putting the verbs at the end of function names for
> library calls.
>
> e.g.
>
>   vfio_pci_config_read()
>   vfio_pci_config_write()
>   vfio_pci_msi_enable()
>   vfio_pci_msi_disable()
>
> So these would be:
>
>   sysfs_val_set()
>   sysfs_val_get()
>   sysfs_device_val_set()
>   sysfs_device_val_get()
>   sysfs_sriov_numvfs_set()
>   sysfs_sriov_numvfs_get()
>   ...
>
> > +                      const char *file)
> > +{
> > +     char path[PATH_MAX] = {0};
> > +     char buf[32] = {0};
>
> nit: You don't need to zero-initialize these since you only use them
> after they are intitialized below.
>
> > +     int fd;
> > +
> > +     snprintf(path, PATH_MAX, "/sys/bus/pci/%s/%s/%s", component, name, file);
>
> Use the new snprintf_assert() :)
>
> > +     fd = open(path, O_RDONLY);
> > +     if (fd < 0)
> > +             return fd;
> > +
> > +     VFIO_ASSERT_GT(read(fd, buf, ARRAY_SIZE(buf)), 0);
> > +     VFIO_ASSERT_EQ(close(fd), 0);
> > +
> > +     return strtol(buf, NULL, 0);
> > +}
> > +
> > +static void sysfs_set_val(const char *component, const char *name,
> > +                       const char *file, const char *val)
> > +{
> > +     char path[PATH_MAX] = {0};
>
> Ditto here about zero-intialization being unnecessary.
>
> > +     int fd;
> > +
> > +     snprintf(path, PATH_MAX, "/sys/bus/pci/%s/%s/%s", component, name, file);
>
> Ditto here about snprintf_assert()
>
> You get the idea... I won't comment on the ones below.
>
> > +     VFIO_ASSERT_GT(fd = open(path, O_WRONLY), 0);
> > +
> > +     VFIO_ASSERT_EQ(write(fd, val, strlen(val)), strlen(val));
> > +     VFIO_ASSERT_EQ(close(fd), 0);
> > +}
> > +
> > +static int sysfs_get_device_val(const char *bdf, const char *file)
> > +{
> > +     sysfs_get_val("devices", bdf, file);
> > +}
> > +
> > +static void sysfs_set_device_val(const char *bdf, const char *file, const char *val)
> > +{
> > +     sysfs_set_val("devices", bdf, file, val);
> > +}
> > +
> > +static void sysfs_set_driver_val(const char *driver, const char *file, const char *val)
> > +{
> > +     sysfs_set_val("drivers", driver, file, val);
> > +}
> > +
> > +static void sysfs_set_device_val_int(const char *bdf, const char *file, int val)
> > +{
> > +     char val_str[32] = {0};
> > +
> > +     snprintf(val_str, sizeof(val_str), "%d", val);
> > +     sysfs_set_device_val(bdf, file, val_str);
> > +}
> > +
> > +int sysfs_get_sriov_totalvfs(const char *bdf)
> > +{
> > +     return sysfs_get_device_val(bdf, "sriov_totalvfs");
> > +}
> > +
> > +int sysfs_get_sriov_numvfs(const char *bdf)
> > +{
> > +     return sysfs_get_device_val(bdf, "sriov_numvfs");
> > +}
> > +
> > +void sysfs_set_sriov_numvfs(const char *bdf, int numvfs)
> > +{
> > +     sysfs_set_device_val_int(bdf, "sriov_numvfs", numvfs);
> > +}
> > +
> > +bool sysfs_get_sriov_drivers_autoprobe(const char *bdf)
> > +{
> > +     return (bool)sysfs_get_device_val(bdf, "sriov_drivers_autoprobe");
> > +}
> > +
> > +void sysfs_set_sriov_drivers_autoprobe(const char *bdf, bool val)
> > +{
> > +     sysfs_set_device_val_int(bdf, "sriov_drivers_autoprobe", val);
> > +}
> > +
> > +void sysfs_bind_driver(const char *bdf, const char *driver)
> > +{
> > +     sysfs_set_driver_val(driver, "bind", bdf);
> > +}
> > +
> > +void sysfs_unbind_driver(const char *bdf, const char *driver)
> > +{
> > +     sysfs_set_driver_val(driver, "unbind", bdf);
> > +}
> > +
> > +void sysfs_get_sriov_vf_bdf(const char *pf_bdf, int i, char *out_vf_bdf)
> > +{
> > +     char vf_path[PATH_MAX] = {0};
> > +     char path[PATH_MAX] = {0};
> > +     int ret;
> > +
> > +     snprintf(path, PATH_MAX, "/sys/bus/pci/devices/%s/virtfn%d", pf_bdf, i);
> > +
> > +     ret = readlink(path, vf_path, PATH_MAX);
> > +     VFIO_ASSERT_NE(ret, -1);
> > +
> > +     ret = sscanf(basename(vf_path), "%s", out_vf_bdf);
> > +     VFIO_ASSERT_EQ(ret, 1);
> > +}
> > +
> > +unsigned int sysfs_get_device_group(const char *bdf)
>
> nit: s/device_group/iommu_group/
>
> > +{
> > +     char dev_iommu_group_path[PATH_MAX] = {0};
> > +     char path[PATH_MAX] = {0};
> > +     unsigned int group;
> > +     int ret;
> > +
> > +     snprintf(path, PATH_MAX, "/sys/bus/pci/devices/%s/iommu_group", bdf);
> > +
> > +     ret = readlink(path, dev_iommu_group_path, sizeof(dev_iommu_group_path));
> > +     VFIO_ASSERT_NE(ret, -1, "Failed to get the IOMMU group for device: %s\n", bdf);
> > +
> > +     ret = sscanf(basename(dev_iommu_group_path), "%u", &group);
> > +     VFIO_ASSERT_EQ(ret, 1, "Failed to get the IOMMU group for device: %s\n", bdf);
> > +
> > +     return group;
> > +}
> > +
> > +int sysfs_get_driver(const char *bdf, char *out_driver)
> > +{
> > +     char driver_path[PATH_MAX] = {0};
> > +     char path[PATH_MAX] = {0};
> > +     int ret;
> > +
> > +     snprintf(path, PATH_MAX, "/sys/bus/pci/devices/%s/driver", bdf);
> > +     ret = readlink(path, driver_path, PATH_MAX);
> > +     if (ret == -1) {
> > +             if (errno == ENOENT)
> > +                     return -1;
> > +
> > +             VFIO_FAIL("Failed to read %s\n", path);
> > +     }
> > +
> > +     ret = sscanf(basename(driver_path), "%s", out_driver);
>
> I think this is equivalent to:
>
>   out_driver = basename(driver_path);
>
> ... which means out_driver to point within driver_path, which is stack
> allocated? I think you want to do strcpy() after basename() to copy the
> driver name to out_driver.
>
> Also how do you prevent overflowing out_driver? Maybe it would be
> cleaner for sysfs_get_driver() to allocate out_driver and return it to
> the caller?
>
> We can return an empty string for the ENOENT case.
>
Good point. Better to alloc 'out_driver'.

I'll take care of this and other nits that you pointed out in v3.

Thank you.
Raghavendra

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

* Re: [PATCH v2 3/6] vfio: selftests: Extend container/iommufd setup for passing vf_token
  2026-01-07 22:49   ` David Matlack
@ 2026-01-08 21:34     ` Raghavendra Rao Ananta
  0 siblings, 0 replies; 28+ messages in thread
From: Raghavendra Rao Ananta @ 2026-01-08 21:34 UTC (permalink / raw)
  To: David Matlack
  Cc: Alex Williamson, Alex Williamson, Josh Hilke, kvm, linux-kernel

On Wed, Jan 7, 2026 at 2:49 PM David Matlack <dmatlack@google.com> wrote:
> > diff --git a/tools/testing/selftests/vfio/lib/libvfio.mk b/tools/testing/selftests/vfio/lib/libvfio.mk
> > index b7857319c3f1f..459b14c6885a8 100644
> > --- a/tools/testing/selftests/vfio/lib/libvfio.mk
> > +++ b/tools/testing/selftests/vfio/lib/libvfio.mk
> > @@ -15,6 +15,8 @@ LIBVFIO_C += drivers/ioat/ioat.c
> >  LIBVFIO_C += drivers/dsa/dsa.c
> >  endif
> >
> > +LDLIBS += -luuid
> > +
> >  LIBVFIO_OUTPUT := $(OUTPUT)/libvfio
> >
> >  LIBVFIO_O := $(patsubst %.c, $(LIBVFIO_OUTPUT)/%.o, $(LIBVFIO_C))
> > @@ -25,6 +27,6 @@ $(shell mkdir -p $(LIBVFIO_O_DIRS))
> >  CFLAGS += -I$(LIBVFIO_SRCDIR)/include
> >
> >  $(LIBVFIO_O): $(LIBVFIO_OUTPUT)/%.o : $(LIBVFIO_SRCDIR)/%.c
> > -     $(CC) $(CFLAGS) $(CPPFLAGS) $(TARGET_ARCH) -c $< -o $@
> > +     $(CC) $(CFLAGS) $(CPPFLAGS) $(TARGET_ARCH) -c $< $(LDLIBS) -o $@
>
> Do we need $(LDLIBS) when compiling the intermediate .o files? I thought
> we would only need it when linking the selftests binaries.
>
You are right. We need it only during the final linking. I'll get rid of it.

I'll update this and other nits in the next version.

Thank you
Raghavendra

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

* Re: [PATCH v2 5/6] vfio: selftests: Add helper to set/override a vf_token
  2026-01-07 22:56   ` David Matlack
@ 2026-01-08 21:45     ` Raghavendra Rao Ananta
  2026-01-14 17:12       ` David Matlack
  0 siblings, 1 reply; 28+ messages in thread
From: Raghavendra Rao Ananta @ 2026-01-08 21:45 UTC (permalink / raw)
  To: David Matlack
  Cc: Alex Williamson, Alex Williamson, Josh Hilke, kvm, linux-kernel

On Wed, Jan 7, 2026 at 2:56 PM David Matlack <dmatlack@google.com> wrote:
>
> On 2025-12-10 06:14 PM, Raghavendra Rao Ananta wrote:
> > Add a helper function, vfio_device_set_vf_token(), to set or override a
> > vf_token. Not only at init, but a vf_token can also be set via the
> > VFIO_DEVICE_FEATURE ioctl, by setting the
> > VFIO_DEVICE_FEATURE_PCI_VF_TOKEN flag. Hence, add an API to utilize this
> > functionality from the test code. The subsequent commit will use this to
> > test the functionality of this method to set the vf_token.
> >
> > Signed-off-by: Raghavendra Rao Ananta <rananta@google.com>
> > ---
> >  .../lib/include/libvfio/vfio_pci_device.h     |  2 ++
> >  .../selftests/vfio/lib/vfio_pci_device.c      | 34 +++++++++++++++++++
> >  2 files changed, 36 insertions(+)
> >
> > diff --git a/tools/testing/selftests/vfio/lib/include/libvfio/vfio_pci_device.h b/tools/testing/selftests/vfio/lib/include/libvfio/vfio_pci_device.h
> > index 6186ca463ca6e..b370aa6a74d0b 100644
> > --- a/tools/testing/selftests/vfio/lib/include/libvfio/vfio_pci_device.h
> > +++ b/tools/testing/selftests/vfio/lib/include/libvfio/vfio_pci_device.h
> > @@ -129,4 +129,6 @@ void vfio_container_set_iommu(struct vfio_pci_device *device);
> >  void vfio_pci_iommufd_cdev_open(struct vfio_pci_device *device, const char *bdf);
> >  int __vfio_device_bind_iommufd(int device_fd, int iommufd, const char *vf_token);
> >
> > +void vfio_device_set_vf_token(int fd, const char *vf_token);
> > +
> >  #endif /* SELFTESTS_VFIO_LIB_INCLUDE_LIBVFIO_VFIO_PCI_DEVICE_H */
> > diff --git a/tools/testing/selftests/vfio/lib/vfio_pci_device.c b/tools/testing/selftests/vfio/lib/vfio_pci_device.c
> > index 208da2704d9e2..7725ecc62b024 100644
> > --- a/tools/testing/selftests/vfio/lib/vfio_pci_device.c
> > +++ b/tools/testing/selftests/vfio/lib/vfio_pci_device.c
> > @@ -109,6 +109,40 @@ static void vfio_pci_irq_get(struct vfio_pci_device *device, u32 index,
> >       ioctl_assert(device->fd, VFIO_DEVICE_GET_IRQ_INFO, irq_info);
> >  }
> >
> > +static int vfio_device_feature_ioctl(int fd, u32 flags, void *data,
> > +                                  size_t data_size)
> > +{
> > +     u8 buffer[sizeof(struct vfio_device_feature) + data_size] = {};
> > +     struct vfio_device_feature *feature = (void *)buffer;
> > +
> > +     memcpy(feature->data, data, data_size);
> > +
> > +     feature->argsz = sizeof(buffer);
> > +     feature->flags = flags;
> > +
> > +     return ioctl(fd, VFIO_DEVICE_FEATURE, feature);
> > +}
> > +
> > +static void vfio_device_feature_set(int fd, u16 feature, void *data, size_t data_size)
> > +{
> > +     u32 flags = VFIO_DEVICE_FEATURE_SET | feature;
> > +     int ret;
> > +
> > +     ret = vfio_device_feature_ioctl(fd, flags, data, data_size);
> > +     VFIO_ASSERT_EQ(ret, 0, "Failed to set feature %u\n", feature);
> > +}
> > +
> > +void vfio_device_set_vf_token(int fd, const char *vf_token)
> > +{
> > +     uuid_t token_uuid = {0};
> > +
> > +     VFIO_ASSERT_NOT_NULL(vf_token, "vf_token is NULL");
> > +     VFIO_ASSERT_EQ(uuid_parse(vf_token, token_uuid), 0);
> > +
> > +     vfio_device_feature_set(fd, VFIO_DEVICE_FEATURE_PCI_VF_TOKEN,
> > +                             token_uuid, sizeof(uuid_t));
> > +}
>
> Would it be useful to have a variant that returns an int for negative
> testing?
>
I couldn't see any interesting cases where the ioctl could fail that
would warrant a negative test.
The 'incorrect vf token set' is validated later during device init.
I've implemented a negative test for this.

However, please let me know if you can think of anything.

Thank you.
Raghavendra

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

* Re: [PATCH v2 4/6] vfio: selftests: Export more vfio_pci functions
  2026-01-07 23:05   ` David Matlack
@ 2026-01-08 21:47     ` Raghavendra Rao Ananta
  0 siblings, 0 replies; 28+ messages in thread
From: Raghavendra Rao Ananta @ 2026-01-08 21:47 UTC (permalink / raw)
  To: David Matlack
  Cc: Alex Williamson, Alex Williamson, Josh Hilke, kvm, linux-kernel

On Wed, Jan 7, 2026 at 3:05 PM David Matlack <dmatlack@google.com> wrote:
>
> On 2025-12-10 06:14 PM, Raghavendra Rao Ananta wrote:
>
> > -static void vfio_device_bind_iommufd(int device_fd, int iommufd,
> > -                                  const char *vf_token)
> > +int __vfio_device_bind_iommufd(int device_fd, int iommufd, const char *vf_token)
> >  {
> >       struct vfio_device_bind_iommufd args = {
> >               .argsz = sizeof(args),
> > @@ -314,7 +322,15 @@ static void vfio_device_bind_iommufd(int device_fd, int iommufd,
> >               args.token_uuid_ptr = (u64)token_uuid;
> >       }
> >
> > -     ioctl_assert(device_fd, VFIO_DEVICE_BIND_IOMMUFD, &args);
> > +     return ioctl(device_fd, VFIO_DEVICE_BIND_IOMMUFD, &args);
>
> For ioctls that return 0 on success and -1 on error, let's follow the
> precedent set in iommu.c and return -errno on error from our library
> functions. i.e.
>
>        if (ioctl(device_fd, VFIO_DEVICE_BIND_IOMMUFD, &args))
>                return -errno;
>
>        return 0;

Sure, will take care of this in v3.

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

* Re: [PATCH v2 6/6] vfio: selftests: Add tests to validate SR-IOV UAPI
  2026-01-07 23:22   ` David Matlack
@ 2026-01-09 19:05     ` Raghavendra Rao Ananta
  2026-01-14 17:09       ` David Matlack
  0 siblings, 1 reply; 28+ messages in thread
From: Raghavendra Rao Ananta @ 2026-01-09 19:05 UTC (permalink / raw)
  To: David Matlack
  Cc: Alex Williamson, Alex Williamson, Josh Hilke, kvm, linux-kernel

On Wed, Jan 7, 2026 at 3:22 PM David Matlack <dmatlack@google.com> wrote:
>
> On 2025-12-10 06:14 PM, Raghavendra Rao Ananta wrote:
> > Add a selfttest, 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>
> > ---
> >  tools/testing/selftests/vfio/Makefile         |   1 +
> >  .../selftests/vfio/vfio_pci_sriov_uapi_test.c | 215 ++++++++++++++++++
> >  2 files changed, 216 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 3c796ca99a509..f00a63902fbfb 100644
> > --- a/tools/testing/selftests/vfio/Makefile
> > +++ b/tools/testing/selftests/vfio/Makefile
> > @@ -4,6 +4,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..4c2951d6e049c
> > --- /dev/null
> > +++ b/tools/testing/selftests/vfio/vfio_pci_sriov_uapi_test.c
> > @@ -0,0 +1,215 @@
> > +// 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_dev_bdf;
>
> nit: I think you could simplify some of the names in this file. This
> code isn't in a library so the names dont' have to be globally unique
> and quite so long.
>
>   s/pf_dev_bdf/pf_bdf/
>   s/vf_dev_bdf/vf_bdf/
>   s/pf_device/pf/
>   s/vf_device/vf/
>   s/test_vfio_pci_container_setup/container_setup/
>   s/test_vfio_pci_iommufd_setup/iommufd_setup/
>   s/test_vfio_pci_device_init/device_init/
>   s/test_vfio_pci_device_cleanup/device_cleanup/
>
> Feel free to ignore this though if you think it makes the names too
> terse.
>
No, I think the short versions are fine. I can change in the next version.

> > +
> > +static int test_vfio_pci_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 test_vfio_pci_iommufd_setup(struct vfio_pci_device *device,
> > +                                    const char *bdf, const char *vf_token)
> > +{
> > +     vfio_pci_iommufd_cdev_open(device, bdf);
> > +     return __vfio_device_bind_iommufd(device->fd,
> > +                                       device->iommu->iommufd, vf_token);
> > +}
> > +
> > +static struct vfio_pci_device *test_vfio_pci_device_init(const char *bdf,
> > +                                                      struct iommu *iommu,
> > +                                                      const char *vf_token,
> > +                                                      int *out_ret)
> > +{
> > +     struct vfio_pci_device *device;
> > +
> > +     device = calloc(1, sizeof(*device));
> > +     VFIO_ASSERT_NOT_NULL(device);
> > +
> > +     device->iommu = iommu;
> > +     device->bdf = bdf;
>
> Can you put this in a helper exposed by vfio_pci_device.h? e.g.
> vfio_pci_device_alloc()
>
Is that just to wrap the ASSERT() within? Or were you thinking of
initializing the members as well in there?


> > +
> > +     if (iommu->mode->container_path)
> > +             *out_ret = test_vfio_pci_container_setup(device, bdf, vf_token);
> > +     else
> > +             *out_ret = test_vfio_pci_iommufd_setup(device, bdf, vf_token);
> > +
> > +     return device;
> > +}
> > +
> > +static void test_vfio_pci_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);
> > +
> > +     free(device);
> > +}
> > +
> > +FIXTURE(vfio_pci_sriov_uapi_test) {
> > +     char vf_dev_bdf[16];
> > +     char vf_driver[32];
> > +     bool sriov_drivers_autoprobe;
> > +};
> > +
> > +FIXTURE_SETUP(vfio_pci_sriov_uapi_test)
> > +{
> > +     int nr_vfs;
> > +     int ret;
> > +
> > +     nr_vfs = sysfs_get_sriov_totalvfs(pf_dev_bdf);
> > +     if (nr_vfs < 0)
> > +             SKIP(return, "SR-IOV may not be supported by the device\n");
>
> Should this be <= 0?
>
Yes, <= 0 should be better. I was only aiming for the case where
"Device doesn't support SR-IOV if the file is absent." Looking at the
pci code, I think there's a potential for returning 0, say for a VF or
an error in the PCI config.
I'll update this in v3.

> And replace "the device" with the BDF.
>
Sure

> > +
> > +     nr_vfs = sysfs_get_sriov_numvfs(pf_dev_bdf);
> > +     if (nr_vfs != 0)
> > +             SKIP(return, "SR-IOV already configured for the PF\n");
>
> Let's print the BDF and nr_vfs for the user.
>
Sure

> > +
> > +     self->sriov_drivers_autoprobe =
> > +             sysfs_get_sriov_drivers_autoprobe(pf_dev_bdf);
> > +     if (self->sriov_drivers_autoprobe)
> > +             sysfs_set_sriov_drivers_autoprobe(pf_dev_bdf, 0);
> > +
> > +     /* Export only one VF for testing */
>
> s/Export/Create/
>
Sure

> > +     sysfs_set_sriov_numvfs(pf_dev_bdf, 1);
> > +
> > +     sysfs_get_sriov_vf_bdf(pf_dev_bdf, 0, self->vf_dev_bdf);
> > +     if (sysfs_get_driver(self->vf_dev_bdf, self->vf_driver) == 0)
> > +             sysfs_unbind_driver(self->vf_dev_bdf, self->vf_driver);
>
> This should be impossible since we disabled autoprobing.
>
> > +     sysfs_bind_driver(self->vf_dev_bdf, "vfio-pci");
>
> Some devices also require setting driver_override to "vfio-pci" as well
> so the device can be bound to vfio-pci. Let's just do that
> unconditionally.
>
Sure, I'll include that in v3.

Thank you.
Raghavendra

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

* Re: [PATCH v2 6/6] vfio: selftests: Add tests to validate SR-IOV UAPI
  2026-01-09 19:05     ` Raghavendra Rao Ananta
@ 2026-01-14 17:09       ` David Matlack
  0 siblings, 0 replies; 28+ messages in thread
From: David Matlack @ 2026-01-14 17:09 UTC (permalink / raw)
  To: Raghavendra Rao Ananta
  Cc: Alex Williamson, Alex Williamson, Josh Hilke, kvm, linux-kernel

On 2026-01-09 11:05 AM, Raghavendra Rao Ananta wrote:
> On Wed, Jan 7, 2026 at 3:22 PM David Matlack <dmatlack@google.com> wrote:
> > On 2025-12-10 06:14 PM, Raghavendra Rao Ananta wrote:

> > > +static struct vfio_pci_device *test_vfio_pci_device_init(const char *bdf,
> > > +                                                      struct iommu *iommu,
> > > +                                                      const char *vf_token,
> > > +                                                      int *out_ret)
> > > +{
> > > +     struct vfio_pci_device *device;
> > > +
> > > +     device = calloc(1, sizeof(*device));
> > > +     VFIO_ASSERT_NOT_NULL(device);
> > > +
> > > +     device->iommu = iommu;
> > > +     device->bdf = bdf;
> >
> > Can you put this in a helper exposed by vfio_pci_device.h? e.g.
> > vfio_pci_device_alloc()
> >
> Is that just to wrap the ASSERT() within? Or were you thinking of
> initializing the members as well in there?

I was thinking it would include all of the above.

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

* Re: [PATCH v2 5/6] vfio: selftests: Add helper to set/override a vf_token
  2026-01-08 21:45     ` Raghavendra Rao Ananta
@ 2026-01-14 17:12       ` David Matlack
  0 siblings, 0 replies; 28+ messages in thread
From: David Matlack @ 2026-01-14 17:12 UTC (permalink / raw)
  To: Raghavendra Rao Ananta
  Cc: Alex Williamson, Alex Williamson, Josh Hilke, kvm, linux-kernel

On 2026-01-08 01:45 PM, Raghavendra Rao Ananta wrote:
> On Wed, Jan 7, 2026 at 2:56 PM David Matlack <dmatlack@google.com> wrote:
> > On 2025-12-10 06:14 PM, Raghavendra Rao Ananta wrote:

> > > +void vfio_device_set_vf_token(int fd, const char *vf_token)
> > > +{
> > > +     uuid_t token_uuid = {0};
> > > +
> > > +     VFIO_ASSERT_NOT_NULL(vf_token, "vf_token is NULL");

nit: Drop the "vf_token is NULL" message. It's unnecessary.

> > > +     VFIO_ASSERT_EQ(uuid_parse(vf_token, token_uuid), 0);
> > > +
> > > +     vfio_device_feature_set(fd, VFIO_DEVICE_FEATURE_PCI_VF_TOKEN,
> > > +                             token_uuid, sizeof(uuid_t));
> > > +}
> >
> > Would it be useful to have a variant that returns an int for negative
> > testing?
> >
> I couldn't see any interesting cases where the ioctl could fail that
> would warrant a negative test.
> The 'incorrect vf token set' is validated later during device init.
> I've implemented a negative test for this.
> 
> However, please let me know if you can think of anything.

I didn't have anything in mind, I was just curious.

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

* Re: [PATCH v2 6/6] vfio: selftests: Add tests to validate SR-IOV UAPI
  2026-01-06 19:47     ` Raghavendra Rao Ananta
@ 2026-02-05 21:51       ` David Matlack
  2026-02-23 18:57         ` David Matlack
  0 siblings, 1 reply; 28+ messages in thread
From: David Matlack @ 2026-02-05 21:51 UTC (permalink / raw)
  To: Raghavendra Rao Ananta
  Cc: Alex Williamson, Alex Williamson, Josh Hilke, kvm, linux-kernel,
	iommu, Bjorn Helgaas, linux-pci, David Woodhouse, Lu Baolu,
	Samiullah Khawaja

On 2026-01-06 11:47 AM, Raghavendra Rao Ananta wrote:
> On Thu, Dec 18, 2025 at 3:26 PM David Matlack <dmatlack@google.com> wrote:
> >
> > On 2025-12-10 06:14 PM, Raghavendra Rao Ananta wrote:
> > > Add a selfttest, 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>
> >
> > I hit the following kernel NULL pointer dereference after running the
> > new test a few times (nice!).
> >
> > Repro:
> >
> >   $ tools/testing/selftests/vfio/scripts/setup.sh 0000:16:00.1
> >   $ tools/testing/selftests/vfio/vfio_pci_sriov_uapi_test 0000:16:00.1
> >   $ tools/testing/selftests/vfio/scripts/cleanup.sh
> >   ... repeat ...
> >
> > The panic:
> >
> > [  553.245784][T27601] vfio-pci 0000:1a:00.0: probe with driver vfio-pci failed with error -22
> > [  553.256622][T27601] vfio-pci 0000:1a:00.0: probe with driver vfio-pci failed with error -22
> > [  574.857650][T27935] BUG: kernel NULL pointer dereference, address: 0000000000000008
> > [  574.865322][T27935] #PF: supervisor read access in kernel mode
> > [  574.871175][T27935] #PF: error_code(0x0000) - not-present page
> > [  574.877021][T27935] PGD 4116e63067 P4D 40fb0a3067 PUD 409597f067 PMD 0
> > [  574.883654][T27935] Oops: Oops: 0000 [#1] SMP NOPTI
> > [  574.888551][T27935] CPU: 100 UID: 0 PID: 27935 Comm: vfio_pci_sriov_ Tainted: G S      W           6.18.0-smp-DEV #1 NONE
> > [  574.899600][T27935] Tainted: [S]=CPU_OUT_OF_SPEC, [W]=WARN
> > [  574.905104][T27935] Hardware name: Google Izumi-EMR/izumi, BIOS 0.20250801.2-0 08/25/2025
> > [  574.913289][T27935] RIP: 0010:rb_insert_color+0x44/0x110
> > [  574.918623][T27935] Code: cc cc 48 89 cf 48 83 cf 01 48 89 3a 48 89 38 48 8b 01 48 89 cf 48 83 e0 fc 48 89 01 74 d7 48 8b 08 f6 c1 01 0f 85 c1 00 00 00 <48> 8b 51 08 48 39 c2 74 0c 48 85 d2 74 4f f6 02 01 74 c5 eb 48 48
> > [  574.938080][T27935] RSP: 0018:ff85113dcdd6bb08 EFLAGS: 00010046
> > [  574.944013][T27935] RAX: ff3f257594a99e80 RBX: ff3f25758af490c0 RCX: 0000000000000000
> > [  574.951857][T27935] RDX: 0000000000001a00 RSI: ff3f25360038eb70 RDI: ff3f2536658bbee0
> > [  574.959702][T27935] RBP: ff3f25360038ea00 R08: 0000000000000002 R09: ff85113dcdd6badc
> > [  574.967544][T27935] R10: ff3f257590ab8000 R11: ffffffffa78210a0 R12: ff3f2536658bbea0
> > [  574.975387][T27935] R13: 0000000000000286 R14: ff3f25758af49000 R15: ff3f25360038eb78
> > [  574.983230][T27935] FS:  00000000223403c0(0000) GS:ff3f25b4d4d83000(0000) knlGS:0000000000000000
> > [  574.992032][T27935] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > [  574.998488][T27935] CR2: 0000000000000008 CR3: 00000040fa254005 CR4: 0000000000f71ef0
> > [  575.006332][T27935] PKRU: 55555554
> > [  575.009753][T27935] Call Trace:
> > [  575.012919][T27935]  <TASK>
> > [  575.015730][T27935]  intel_iommu_probe_device+0x4c9/0x7b0
> > [  575.021153][T27935]  __iommu_probe_device+0x101/0x4c0
> > [  575.026231][T27935]  iommu_bus_notifier+0x37/0x100
> > [  575.031046][T27935]  blocking_notifier_call_chain+0x53/0xd0
> > [  575.036634][T27935]  bus_notify+0x99/0xc0
> > [  575.040666][T27935]  device_add+0x252/0x470
> > [  575.044872][T27935]  pci_device_add+0x414/0x5c0
> > [  575.049429][T27935]  pci_iov_add_virtfn+0x2f2/0x3e0
> > [  575.054326][T27935]  sriov_add_vfs+0x33/0x70
> > [  575.058613][T27935]  sriov_enable+0x2fc/0x490
> > [  575.062992][T27935]  vfio_pci_core_sriov_configure+0x16c/0x210
> > [  575.068843][T27935]  sriov_numvfs_store+0xc4/0x190
> > [  575.073652][T27935]  kernfs_fop_write_iter+0xfe/0x180
> > [  575.078724][T27935]  vfs_write+0x2d0/0x430
> > [  575.082846][T27935]  ksys_write+0x7f/0x100
> > [  575.086965][T27935]  do_syscall_64+0x6f/0x940
> > [  575.091339][T27935]  ? arch_exit_to_user_mode_prepare+0x9/0xb0
> > [  575.097193][T27935]  entry_SYSCALL_64_after_hwframe+0x76/0x7e

I think this is a use-after-free.

The VF used in this test matches quirk_intel_e2000_no_ats() which means
that ATS gets disabled (pdev->ats_cap = 0) via quirk after the device is
set up.

 drivers/pci/quirks.c:

 5651 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x1457, quirk_intel_e2000_no_ats);

The issue is this quirk is applied after the Intel IOMMU driver is
notified about the device.

So during intel_iommu_probe_device(), the Intel IOMMU driver sees that
ATS is enabled, and adds the device to the device rbtree:

 drivers/iommu/intel/iommu.c:

 3765 static struct iommu_device *intel_iommu_probe_device(struct device *dev)
 3766 {
 ...
 3826         if (pdev && pci_ats_supported(pdev)) {
 3827                 pci_prepare_ats(pdev, VTD_PAGE_SHIFT);
 3828                 ret = device_rbtree_insert(iommu, info);
 3829                 if (ret)
 3830                         goto free;
 3831         }
 ...
 3858 }


Then ATS is disabled via quirk:

 drivers/pci/iov.c:

 346 int pci_iov_add_virtfn(struct pci_dev *dev, int id)
 347 {
 ...
 383
 384         pci_device_add(virtfn, virtfn->bus);  <======= notifies Intel IOMMU
 385         rc = pci_iov_sysfs_link(dev, virtfn, id);
 386         if (rc)
 387                 goto failed1;
 388
 389         pci_bus_add_device(virtfn);  <==== Disables ATS via pci_fixup_final
 390
 391         return 0;
 ...
 401 }

Then later when the VF is destroyed (SR-IOV disabled on the PF), the
Intel IOMMU sees that ATS is disabled and does not remove the device
from its rbtree.

 drivers/iommu/intel/iommu.c:

 3889 static void intel_iommu_release_device(struct device *dev)
 3890 {
 ...
 3903         if (dev_is_pci(dev) && pci_ats_supported(to_pci_dev(dev)))
 3904                 device_rbtree_remove(info);
 ...
 3913         kfree(info);   <======= info is still reachable from device rbtree
 3914 }

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

* Re: [PATCH v2 6/6] vfio: selftests: Add tests to validate SR-IOV UAPI
  2026-02-05 21:51       ` David Matlack
@ 2026-02-23 18:57         ` David Matlack
  0 siblings, 0 replies; 28+ messages in thread
From: David Matlack @ 2026-02-23 18:57 UTC (permalink / raw)
  To: Raghavendra Rao Ananta
  Cc: Alex Williamson, Alex Williamson, Josh Hilke, kvm, linux-kernel,
	iommu, Bjorn Helgaas, linux-pci, David Woodhouse, Lu Baolu,
	Samiullah Khawaja

On Thu, Feb 5, 2026 at 1:52 PM David Matlack <dmatlack@google.com> wrote:
> On 2026-01-06 11:47 AM, Raghavendra Rao Ananta wrote:
> > On Thu, Dec 18, 2025 at 3:26 PM David Matlack <dmatlack@google.com> wrote:
> > > [  574.857650][T27935] BUG: kernel NULL pointer dereference, address: 0000000000000008
...
> > > [  575.009753][T27935] Call Trace:
> > > [  575.012919][T27935]  <TASK>
> > > [  575.015730][T27935]  intel_iommu_probe_device+0x4c9/0x7b0
> > > [  575.021153][T27935]  __iommu_probe_device+0x101/0x4c0
> > > [  575.026231][T27935]  iommu_bus_notifier+0x37/0x100
> > > [  575.031046][T27935]  blocking_notifier_call_chain+0x53/0xd0
> > > [  575.036634][T27935]  bus_notify+0x99/0xc0
> > > [  575.040666][T27935]  device_add+0x252/0x470
> > > [  575.044872][T27935]  pci_device_add+0x414/0x5c0
> > > [  575.049429][T27935]  pci_iov_add_virtfn+0x2f2/0x3e0
> > > [  575.054326][T27935]  sriov_add_vfs+0x33/0x70
> > > [  575.058613][T27935]  sriov_enable+0x2fc/0x490
> > > [  575.062992][T27935]  vfio_pci_core_sriov_configure+0x16c/0x210
> > > [  575.068843][T27935]  sriov_numvfs_store+0xc4/0x190
> > > [  575.073652][T27935]  kernfs_fop_write_iter+0xfe/0x180
> > > [  575.078724][T27935]  vfs_write+0x2d0/0x430
> > > [  575.082846][T27935]  ksys_write+0x7f/0x100
> > > [  575.086965][T27935]  do_syscall_64+0x6f/0x940
> > > [  575.091339][T27935]  ? arch_exit_to_user_mode_prepare+0x9/0xb0
> > > [  575.097193][T27935]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
>
> I think this is a use-after-free.

Fix proposed here:
https://lore.kernel.org/linux-pci/20260223184017.688212-1-dmatlack@google.com/

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

end of thread, other threads:[~2026-02-23 18:57 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-10 18:14 [PATCH v2 0/6] vfio: selftest: Add SR-IOV UAPI test Raghavendra Rao Ananta
2025-12-10 18:14 ` [PATCH v2 1/6] vfio: selftests: Introduce snprintf_assert() Raghavendra Rao Ananta
2026-01-07 22:21   ` David Matlack
2025-12-10 18:14 ` [PATCH v2 2/6] vfio: selftests: Introduce a sysfs lib Raghavendra Rao Ananta
2025-12-12 18:27   ` Raghavendra Rao Ananta
2025-12-18 21:52     ` David Matlack
2026-01-07 22:41   ` David Matlack
2026-01-08 21:25     ` Raghavendra Rao Ananta
2025-12-10 18:14 ` [PATCH v2 3/6] vfio: selftests: Extend container/iommufd setup for passing vf_token Raghavendra Rao Ananta
2026-01-07 22:49   ` David Matlack
2026-01-08 21:34     ` Raghavendra Rao Ananta
2025-12-10 18:14 ` [PATCH v2 4/6] vfio: selftests: Export more vfio_pci functions Raghavendra Rao Ananta
2026-01-07 22:55   ` David Matlack
2026-01-07 23:05   ` David Matlack
2026-01-08 21:47     ` Raghavendra Rao Ananta
2025-12-10 18:14 ` [PATCH v2 5/6] vfio: selftests: Add helper to set/override a vf_token Raghavendra Rao Ananta
2026-01-07 22:56   ` David Matlack
2026-01-08 21:45     ` Raghavendra Rao Ananta
2026-01-14 17:12       ` David Matlack
2025-12-10 18:14 ` [PATCH v2 6/6] vfio: selftests: Add tests to validate SR-IOV UAPI Raghavendra Rao Ananta
2025-12-12 18:21   ` Raghavendra Rao Ananta
2025-12-18 23:26   ` David Matlack
2026-01-06 19:47     ` Raghavendra Rao Ananta
2026-02-05 21:51       ` David Matlack
2026-02-23 18:57         ` David Matlack
2026-01-07 23:22   ` David Matlack
2026-01-09 19:05     ` Raghavendra Rao Ananta
2026-01-14 17:09       ` David Matlack

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox