[PATCH v2 0/4] Introduce vfio_dma_mapping_perf_test

[PATCH v2 0/4] Introduce vfio_dma_mapping_perf_test

[Aaron Lewis]

Add a new VFIO selftest, vfio_dma_mapping_perf_test, to provide a
configurable tool for measuring DMA mapping and unmapping latencies.

This selftest introduces command-line parameters to provide a convenient
way to tune the test for specific situations. For example, it can be used
to observe the lengthy unmap times of the Type 1 IOMMU on large memory
regions, or to compare DMA mapping performance between the Type 1 IOMMU
and IOMMUFD. These scenarios can now be easily configured and run via the
command line.

Changes in V2:
- Assert expected unmap size in iommu_unmap().
- Remove unused struct iommu_mapping
- Create a reusable TIME() macro and move timer-related helpers to libvfio to
  output nanoseconds
- Unified latency reporting by pulling over the TIME() macro instead of
  distinct test-specific prints.
- Drop confusing prints mixing size/addresses in favor of parsing-friendly
  outputs.
- Fix potentially undefined behavior on 32-bit caused by passing u64 directly
  to %lu.
- Refactored memfd tests to run only on MODE_IOMMUFD and drop redundant
  variants.
- Removed MAP_SHARED from memfd_create flags to avoid inadvertently aliasing
  MFD_CLOEXEC.
- Fixed memfd_create() return validation to correctly verify against < 0.
- Updated the map/unmap failure block to properly abort/SKIP on ENOMEM hugepage
  failures.
- Store the device_bdf parameter statically within test_params to unify
  configuration definitions.
- Eliminate memory leaks in populate_harness_args by correctly piping argv into
  wordexp with WRDE_APPEND on subsequent uses.  Subsequent uses now works whether
- Halt execution and exit gracefully if invalid configurations or the
  "-h" flag are supplied, preventing test execution dropout.

Aaron Lewis (4):
  vfio: selftests: Assert the region was unmapped in iommu_unmap()
  vfio: selftests: Introduce vfio_dma_mapping_perf_test
  vfio: selftests: Add memfd test to vfio_dma_mapping_perf_test
  vfio: selftests: Allow a size for vfio_dma_mapping_perf_test

 tools/testing/selftests/vfio/Makefile         |   1 +
 .../selftests/vfio/lib/include/libvfio.h      |  28 ++
 .../vfio/lib/include/libvfio/iommu.h          |  12 +-
 tools/testing/selftests/vfio/lib/iommu.c      |  24 ++
 .../vfio/vfio_dma_mapping_perf_test.c         | 331 ++++++++++++++++++
 .../selftests/vfio/vfio_dma_mapping_test.c    |  11 +-
 6 files changed, 397 insertions(+), 10 deletions(-)
 create mode 100644 tools/testing/selftests/vfio/vfio_dma_mapping_perf_test.c

-- 
2.55.0.654.g21b8a5bc05-goog

[PATCH v2 1/4] vfio: selftests: Assert the region was unmapped in iommu_unmap()

[Aaron Lewis]

Add an assertion in iommu_unmap() to verify that the region was fully
unmapped. Centralizing this check simplifies the selftests by removing
the need for individual tests to handle the verification themselves.

Suggested-by: David Matlack <dmatlack@google.com>
Signed-off-by: Aaron Lewis <aaronlewis@google.com>
---
 .../selftests/vfio/lib/include/libvfio/iommu.h        |  5 ++++-
 tools/testing/selftests/vfio/vfio_dma_mapping_test.c  | 11 ++---------
 2 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/tools/testing/selftests/vfio/lib/include/libvfio/iommu.h b/tools/testing/selftests/vfio/lib/include/libvfio/iommu.h
index e9a3386a4719..5c77875240f4 100644
--- a/tools/testing/selftests/vfio/lib/include/libvfio/iommu.h
+++ b/tools/testing/selftests/vfio/lib/include/libvfio/iommu.h
@@ -46,7 +46,10 @@ int __iommu_unmap(struct iommu *iommu, struct dma_region *region, u64 *unmapped)
 
 static inline void iommu_unmap(struct iommu *iommu, struct dma_region *region)
 {
-	VFIO_ASSERT_EQ(__iommu_unmap(iommu, region, NULL), 0);
+	u64 unmapped;
+
+	VFIO_ASSERT_EQ(__iommu_unmap(iommu, region, &unmapped), 0);
+	VFIO_ASSERT_EQ(unmapped, region->size);
 }
 
 int __iommu_unmap_all(struct iommu *iommu, u64 *unmapped);
diff --git a/tools/testing/selftests/vfio/vfio_dma_mapping_test.c b/tools/testing/selftests/vfio/vfio_dma_mapping_test.c
index 7d0de8c79de1..1b90441644a2 100644
--- a/tools/testing/selftests/vfio/vfio_dma_mapping_test.c
+++ b/tools/testing/selftests/vfio/vfio_dma_mapping_test.c
@@ -138,7 +138,6 @@ TEST_F(vfio_dma_mapping_test, dma_map_unmap)
 	struct dma_region region;
 	struct iommu_mapping mapping;
 	u64 mapping_size = size;
-	u64 unmapped;
 	int rc;
 
 	region.vaddr = mmap(NULL, size, PROT_READ | PROT_WRITE, flags, -1, 0);
@@ -190,9 +189,7 @@ TEST_F(vfio_dma_mapping_test, dma_map_unmap)
 	}
 
 unmap:
-	rc = __iommu_unmap(self->iommu, &region, &unmapped);
-	ASSERT_EQ(rc, 0);
-	ASSERT_EQ(unmapped, region.size);
+	iommu_unmap(self->iommu, &region);
 	printf("Unmapped IOVA 0x%lx\n", region.iova);
 	ASSERT_NE(0, __to_iova(self->device, region.vaddr, NULL));
 	ASSERT_NE(0, iommu_mapping_get(device_bdf, region.iova, &mapping));
@@ -260,15 +257,11 @@ FIXTURE_TEARDOWN(vfio_dma_map_limit_test)
 TEST_F(vfio_dma_map_limit_test, unmap_range)
 {
 	struct dma_region *region = &self->region;
-	u64 unmapped;
-	int rc;
 
 	iommu_map(self->iommu, region);
 	ASSERT_EQ(region->iova, to_iova(self->device, region->vaddr));
 
-	rc = __iommu_unmap(self->iommu, region, &unmapped);
-	ASSERT_EQ(rc, 0);
-	ASSERT_EQ(unmapped, region->size);
+	iommu_unmap(self->iommu, region);
 }
 
 TEST_F(vfio_dma_map_limit_test, unmap_all)
-- 
2.55.0.654.g21b8a5bc05-goog

[PATCH v2 2/4] vfio: selftests: Introduce vfio_dma_mapping_perf_test

[Aaron Lewis]

Introduce vfio_dma_mapping_perf_test to aid in latency testing. Rather
than a traditional pass/fail test, which tends to be flaky when
enforcing latency bounds, this test directly reports the latency of
various stages in the mapping and unmapping process.

Start with a basic test that performs a simple DMA map/unmap and tracks
the duration of each operation. The test reports the following metrics:

1. Time taken by mmap().
2. Time taken to map a DMA region.
3. Time taken to unmap a DMA region.
4. Time taken by munmap().

Subsequent commits in this series will introduce more features to allow
for a wider variety of testing.

To ensure consistent DMA mapping times, the test uses MAP_POPULATE when
calling mmap(). This forces page faults to be handled upfront, isolating
the DMA mapping times from them.

Signed-off-by: Aaron Lewis <aaronlewis@google.com>
---
 tools/testing/selftests/vfio/Makefile         |  1 +
 .../selftests/vfio/lib/include/libvfio.h      | 28 ++++++
 .../vfio/vfio_dma_mapping_perf_test.c         | 90 +++++++++++++++++++
 3 files changed, 119 insertions(+)
 create mode 100644 tools/testing/selftests/vfio/vfio_dma_mapping_perf_test.c

diff --git a/tools/testing/selftests/vfio/Makefile b/tools/testing/selftests/vfio/Makefile
index 2c32c48db509..ed5856af5f9a 100644
--- a/tools/testing/selftests/vfio/Makefile
+++ b/tools/testing/selftests/vfio/Makefile
@@ -8,6 +8,7 @@ else
 CFLAGS = $(KHDR_INCLUDES)
 TEST_GEN_PROGS += vfio_dma_mapping_test
 TEST_GEN_PROGS += vfio_dma_mapping_mmio_test
+TEST_GEN_PROGS += vfio_dma_mapping_perf_test
 TEST_GEN_PROGS += vfio_iommufd_setup_test
 TEST_GEN_PROGS += vfio_pci_device_test
 TEST_GEN_PROGS += vfio_pci_device_init_perf_test
diff --git a/tools/testing/selftests/vfio/lib/include/libvfio.h b/tools/testing/selftests/vfio/lib/include/libvfio.h
index 07862b470777..cf322b499cfb 100644
--- a/tools/testing/selftests/vfio/lib/include/libvfio.h
+++ b/tools/testing/selftests/vfio/lib/include/libvfio.h
@@ -9,6 +9,34 @@
 #include <libvfio/vfio_pci_device.h>
 #include <libvfio/vfio_pci_driver.h>
 
+#include <stdint.h>
+#include <time.h>
+#include <linux/time64.h>
+
+static inline void timer_start(struct timespec *start)
+{
+	clock_gettime(CLOCK_MONOTONIC, start);
+}
+
+static inline uint64_t timer_elapsed_ns(struct timespec start)
+{
+	struct timespec end;
+
+	clock_gettime(CLOCK_MONOTONIC, &end);
+
+	return (uint64_t)(end.tv_sec - start.tv_sec) * NSEC_PER_SEC +
+	       (uint64_t)(end.tv_nsec - start.tv_nsec);
+}
+
+#define TIME(_name, _expression) do {				   \
+	struct timespec __start;				   \
+								   \
+	timer_start(&__start);					   \
+	_expression;						   \
+	printf(_name " = %.2lfms\n",				   \
+	       (double)timer_elapsed_ns(__start) / NSEC_PER_MSEC); \
+} while (0)
+
 /*
  * Return the BDF string of the device that the test should use.
  *
diff --git a/tools/testing/selftests/vfio/vfio_dma_mapping_perf_test.c b/tools/testing/selftests/vfio/vfio_dma_mapping_perf_test.c
new file mode 100644
index 000000000000..26c04cabef61
--- /dev/null
+++ b/tools/testing/selftests/vfio/vfio_dma_mapping_perf_test.c
@@ -0,0 +1,90 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <limits.h>
+#include <stdio.h>
+#include <sys/mman.h>
+#include <time.h>
+#include <unistd.h>
+
+#include <linux/iommufd.h>
+#include <linux/limits.h>
+#include <linux/memfd.h>
+#include <linux/mman.h>
+#include <linux/sizes.h>
+#include <linux/time64.h>
+#include <linux/vfio.h>
+
+#include <libvfio.h>
+
+#include "kselftest_harness.h"
+
+static const char *device_bdf;
+
+FIXTURE(vfio_dma_mapping_perf_test) {
+	struct iommu *iommu;
+	struct vfio_pci_device *device;
+	struct iova_allocator *iova_allocator;
+};
+
+FIXTURE_VARIANT(vfio_dma_mapping_perf_test) {
+	const char *iommu_mode;
+	int mmap_flags;
+};
+
+#define FIXTURE_VARIANT_ADD_IOMMU_MODE(_iommu_mode, _name, _mmap_flags)		  \
+FIXTURE_VARIANT_ADD(vfio_dma_mapping_perf_test, _iommu_mode ## _ ## _name) {	  \
+	.iommu_mode = #_iommu_mode,						  \
+	.mmap_flags = MAP_ANONYMOUS | MAP_PRIVATE | MAP_POPULATE | (_mmap_flags), \
+}
+
+FIXTURE_VARIANT_ADD_ALL_IOMMU_MODES(anonymous, 0);
+FIXTURE_VARIANT_ADD_ALL_IOMMU_MODES(anonymous_hugetlb_2mb, MAP_HUGETLB | MAP_HUGE_2MB);
+FIXTURE_VARIANT_ADD_ALL_IOMMU_MODES(anonymous_hugetlb_1gb, MAP_HUGETLB | MAP_HUGE_1GB);
+
+#undef FIXTURE_VARIANT_ADD_IOMMU_MODE
+
+FIXTURE_SETUP(vfio_dma_mapping_perf_test)
+{
+	self->iommu = iommu_init(variant->iommu_mode);
+	self->device = vfio_pci_device_init(device_bdf, self->iommu);
+	self->iova_allocator = iova_allocator_init(self->iommu);
+}
+
+FIXTURE_TEARDOWN(vfio_dma_mapping_perf_test)
+{
+	iova_allocator_cleanup(self->iova_allocator);
+	vfio_pci_device_cleanup(self->device);
+	iommu_cleanup(self->iommu);
+}
+
+TEST_F(vfio_dma_mapping_perf_test, dma_map_unmap)
+{
+	const u64 size = SZ_1G;
+	const int flags = variant->mmap_flags;
+	struct dma_region region;
+
+	printf("mmap size = %lluG\n", (unsigned long long)(size / SZ_1G));
+
+	TIME("mmap",
+	     region.vaddr = mmap(NULL, size, PROT_READ | PROT_WRITE, flags, -1, 0));
+
+	/* Skip the test if there aren't enough HugeTLB pages available. */
+	if (flags & MAP_HUGETLB && region.vaddr == MAP_FAILED)
+		SKIP(return, "mmap() failed: %s (%d)\n", strerror(errno), errno);
+	else
+		ASSERT_NE(region.vaddr, MAP_FAILED);
+
+	region.iova = iova_allocator_alloc(self->iova_allocator, size);
+	region.size = size;
+
+	TIME("IOMMU map", iommu_map(self->iommu, &region));
+	ASSERT_EQ(region.iova, to_iova(self->device, region.vaddr));
+
+	TIME("IOMMU unmap", iommu_unmap(self->iommu, &region));
+	TIME("munmap", ASSERT_EQ(0, munmap(region.vaddr, size)));
+}
+
+int main(int argc, char *argv[])
+{
+	device_bdf = vfio_selftests_get_bdf(&argc, argv);
+	return test_harness_run(argc, argv);
+}
-- 
2.55.0.654.g21b8a5bc05-goog

[PATCH v2 3/4] vfio: selftests: Add memfd test to vfio_dma_mapping_perf_test

[Aaron Lewis]

Add a second test to "vfio_dma_mapping_perf_test" to evaluate memfd
latencies. A key motivator for adding this test is to demonstrate that
the IOMMU_IOAS_MAP_FILE ioctl is significantly faster than other methods
of mapping DMA regions. While this performance difference is not fully
apparent with the test's current capabilities, it will become evident
as more features are introduced later in the series.

To support this, add IOMMU_IOAS_MAP_FILE ioctl support to the VFIO
selftest library via iommufd_map_file().

Signed-off-by: Aaron Lewis <aaronlewis@google.com>
---
 .../vfio/lib/include/libvfio/iommu.h          |   7 ++
 tools/testing/selftests/vfio/lib/iommu.c      |  24 +++++
 .../vfio/vfio_dma_mapping_perf_test.c         | 102 ++++++++++++++++++
 3 files changed, 133 insertions(+)

diff --git a/tools/testing/selftests/vfio/lib/include/libvfio/iommu.h b/tools/testing/selftests/vfio/lib/include/libvfio/iommu.h
index 5c77875240f4..515976fb19be 100644
--- a/tools/testing/selftests/vfio/lib/include/libvfio/iommu.h
+++ b/tools/testing/selftests/vfio/lib/include/libvfio/iommu.h
@@ -35,6 +35,13 @@ struct iommu {
 struct iommu *iommu_init(const char *iommu_mode);
 void iommu_cleanup(struct iommu *iommu);
 
+int __iommufd_map_file(struct iommu *iommu, struct dma_region *region, int fd);
+
+static inline void iommufd_map_file(struct iommu *iommu, struct dma_region *region, int fd)
+{
+	VFIO_ASSERT_EQ(__iommufd_map_file(iommu, region, fd), 0);
+}
+
 int __iommu_map(struct iommu *iommu, struct dma_region *region);
 
 static inline void iommu_map(struct iommu *iommu, struct dma_region *region)
diff --git a/tools/testing/selftests/vfio/lib/iommu.c b/tools/testing/selftests/vfio/lib/iommu.c
index b6f3c5c84e01..b76ae0a91c68 100644
--- a/tools/testing/selftests/vfio/lib/iommu.c
+++ b/tools/testing/selftests/vfio/lib/iommu.c
@@ -97,6 +97,30 @@ iova_t iommu_hva2iova(struct iommu *iommu, void *vaddr)
 	return iova;
 }
 
+int __iommufd_map_file(struct iommu *iommu, struct dma_region *region, int fd)
+{
+	VFIO_ASSERT_TRUE(iommu->iommufd, "IOMMU_IOAS_MAP_FILE is an IOMMUFD IOCTL.");
+
+	struct iommu_ioas_map_file args = {
+		.size = sizeof(args),
+		.flags = IOMMU_IOAS_MAP_READABLE |
+			IOMMU_IOAS_MAP_WRITEABLE |
+			IOMMU_IOAS_MAP_FIXED_IOVA,
+		.ioas_id = iommu->ioas_id,
+		.fd = fd,
+		.start = 0,
+		.iova = region->iova,
+		.length = region->size,
+	};
+
+	if (ioctl(iommu->iommufd, IOMMU_IOAS_MAP_FILE, &args))
+		return -errno;
+
+	list_add(&region->link, &iommu->dma_regions);
+
+	return 0;
+}
+
 static int vfio_iommu_map(struct iommu *iommu, struct dma_region *region)
 {
 	struct vfio_iommu_type1_dma_map args = {
diff --git a/tools/testing/selftests/vfio/vfio_dma_mapping_perf_test.c b/tools/testing/selftests/vfio/vfio_dma_mapping_perf_test.c
index 26c04cabef61..5ef85deba4ee 100644
--- a/tools/testing/selftests/vfio/vfio_dma_mapping_perf_test.c
+++ b/tools/testing/selftests/vfio/vfio_dma_mapping_perf_test.c
@@ -83,6 +83,108 @@ TEST_F(vfio_dma_mapping_perf_test, dma_map_unmap)
 	TIME("munmap", ASSERT_EQ(0, munmap(region.vaddr, size)));
 }
 
+FIXTURE(vfio_dma_mapping_perf_memfd_test) {
+	struct iommu *iommu;
+	struct vfio_pci_device *device;
+	struct iova_allocator *iova_allocator;
+};
+
+FIXTURE_VARIANT(vfio_dma_mapping_perf_memfd_test) {
+	const char *iommu_mode;
+	int mmap_flags;
+	int memfd_flags;
+};
+
+#define FIXTURE_VARIANT_ADD_MEMFD_MODE(_name, _mmap_flags, _memfd_flags)      \
+FIXTURE_VARIANT_ADD(vfio_dma_mapping_perf_memfd_test, iommufd ## _ ## _name) {\
+	.iommu_mode = MODE_IOMMUFD,					      \
+	.mmap_flags = MAP_SHARED | MAP_POPULATE | (_mmap_flags),	      \
+	.memfd_flags = (_memfd_flags),					      \
+}
+
+FIXTURE_VARIANT_ADD_MEMFD_MODE(memfd, 0, 0);
+FIXTURE_VARIANT_ADD_MEMFD_MODE(memfd_hugetlb_2mb,
+				    MAP_HUGETLB | MAP_HUGE_2MB,
+				    MFD_HUGETLB | MFD_HUGE_2MB);
+FIXTURE_VARIANT_ADD_MEMFD_MODE(memfd_hugetlb_1gb,
+				    MAP_HUGETLB | MAP_HUGE_1GB,
+				    MFD_HUGETLB | MFD_HUGE_1GB);
+
+#undef FIXTURE_VARIANT_ADD_MEMFD_MODE
+
+FIXTURE_SETUP(vfio_dma_mapping_perf_memfd_test)
+{
+	self->iommu = iommu_init(variant->iommu_mode);
+	self->device = vfio_pci_device_init(device_bdf, self->iommu);
+	self->iova_allocator = iova_allocator_init(self->iommu);
+}
+
+FIXTURE_TEARDOWN(vfio_dma_mapping_perf_memfd_test)
+{
+	iova_allocator_cleanup(self->iova_allocator);
+	vfio_pci_device_cleanup(self->device);
+	iommu_cleanup(self->iommu);
+}
+
+static void *setup_memfd(int *fd, u64 size, int mmap_flags, int mfd_flags)
+{
+	void *buf = MAP_FAILED;
+
+	TIME("memfd_create",
+	     *fd = memfd_create("vfio_dma_mapping_perf_memfd_test", mfd_flags));
+	if (*fd < 0)
+		return MAP_FAILED;
+
+	if (ftruncate(*fd, size))
+		goto out;
+
+	TIME("mmap",
+	     buf = mmap(NULL, size, PROT_READ | PROT_WRITE, mmap_flags, *fd, 0));
+
+out:
+	if (buf == MAP_FAILED)
+		close(*fd);
+
+	return buf;
+}
+
+static void teardown_memfd(int fd, u64 size, void *vaddr)
+{
+	if (vaddr != MAP_FAILED)
+		TIME("munmap", VFIO_ASSERT_EQ(0, munmap(vaddr, size)));
+
+	if (fd != -1)
+		TIME("close", VFIO_ASSERT_EQ(0, close(fd)));
+}
+
+TEST_F(vfio_dma_mapping_perf_memfd_test, dma_map_unmap_from_file)
+{
+	const u64 size = SZ_1G;
+	const int flags = variant->mmap_flags;
+	struct dma_region region;
+	int fd;
+
+	printf("mmap size = %lluG\n", (unsigned long long)(size / SZ_1G));
+
+	region.vaddr = setup_memfd(&fd, size, variant->mmap_flags, variant->memfd_flags);
+
+	/* Skip the test if there aren't enough HugeTLB pages available. */
+	if (flags & MAP_HUGETLB && region.vaddr == MAP_FAILED)
+		SKIP(return, "setup_memfd() failed: %s (%d)\n", strerror(errno), errno);
+	else
+		ASSERT_NE(region.vaddr, MAP_FAILED);
+
+	region.iova = iova_allocator_alloc(self->iova_allocator, size);
+	region.size = size;
+
+	TIME("IOMMU map", iommufd_map_file(self->iommu, &region, fd));
+	ASSERT_EQ(region.iova, to_iova(self->device, region.vaddr));
+
+	TIME("IOMMU unmap", iommu_unmap(self->iommu, &region));
+
+	teardown_memfd(fd, size, region.vaddr);
+}
+
 int main(int argc, char *argv[])
 {
 	device_bdf = vfio_selftests_get_bdf(&argc, argv);
-- 
2.55.0.654.g21b8a5bc05-goog

[PATCH v2 4/4] vfio: selftests: Allow a size for vfio_dma_mapping_perf_test

[Aaron Lewis]

Allow the user to specify a DMA region size via the command line for
vfio_dma_mapping_perf_test.

Because the selftest harness also parses command-line parameters, sharing
them directly is problematic. Adding options directly to the test could
create conflicts with harness-defined options. Even without conflicts, the
harness would need to be updated to recognize test-specific options to avoid
failing on unknown parameters.

Resolve this by isolating the two sets of parameters. The standard command-line
options are consumed by the test itself. To pass options through to the test
harness, introduce a new '-a' option.

For example, both the test size and the test harness options can be set
like this:

./vfio_dma_mapping_perf_test -b 16G -a "-v vfio_type1_iommu_memfd_hugetlb_1gb"

This invocation configures a 16G DMA region and restricts execution to the
specified test variant, which is useful when debugging DMA mapping latency
issues for a specific IOMMU type.

Signed-off-by: Aaron Lewis <aaronlewis@google.com>
---
 .../vfio/vfio_dma_mapping_perf_test.c         | 159 ++++++++++++++++--
 1 file changed, 149 insertions(+), 10 deletions(-)

diff --git a/tools/testing/selftests/vfio/vfio_dma_mapping_perf_test.c b/tools/testing/selftests/vfio/vfio_dma_mapping_perf_test.c
index 5ef85deba4ee..af2273a0c6f5 100644
--- a/tools/testing/selftests/vfio/vfio_dma_mapping_perf_test.c
+++ b/tools/testing/selftests/vfio/vfio_dma_mapping_perf_test.c
@@ -4,6 +4,7 @@
 #include <sys/mman.h>
 #include <time.h>
 #include <unistd.h>
+#include <wordexp.h>
 
 #include <linux/iommufd.h>
 #include <linux/limits.h>
@@ -17,7 +18,12 @@
 
 #include "kselftest_harness.h"
 
-static const char *device_bdf;
+static struct {
+	u64 size;
+	const char *device_bdf;
+} test_params = {
+	.size = SZ_1G,
+};
 
 FIXTURE(vfio_dma_mapping_perf_test) {
 	struct iommu *iommu;
@@ -45,7 +51,7 @@ FIXTURE_VARIANT_ADD_ALL_IOMMU_MODES(anonymous_hugetlb_1gb, MAP_HUGETLB | MAP_HUG
 FIXTURE_SETUP(vfio_dma_mapping_perf_test)
 {
 	self->iommu = iommu_init(variant->iommu_mode);
-	self->device = vfio_pci_device_init(device_bdf, self->iommu);
+	self->device = vfio_pci_device_init(test_params.device_bdf, self->iommu);
 	self->iova_allocator = iova_allocator_init(self->iommu);
 }
 
@@ -58,7 +64,7 @@ FIXTURE_TEARDOWN(vfio_dma_mapping_perf_test)
 
 TEST_F(vfio_dma_mapping_perf_test, dma_map_unmap)
 {
-	const u64 size = SZ_1G;
+	const u64 size = test_params.size;
 	const int flags = variant->mmap_flags;
 	struct dma_region region;
 
@@ -115,7 +121,7 @@ FIXTURE_VARIANT_ADD_MEMFD_MODE(memfd_hugetlb_1gb,
 FIXTURE_SETUP(vfio_dma_mapping_perf_memfd_test)
 {
 	self->iommu = iommu_init(variant->iommu_mode);
-	self->device = vfio_pci_device_init(device_bdf, self->iommu);
+	self->device = vfio_pci_device_init(test_params.device_bdf, self->iommu);
 	self->iova_allocator = iova_allocator_init(self->iommu);
 }
 
@@ -159,17 +165,17 @@ static void teardown_memfd(int fd, u64 size, void *vaddr)
 
 TEST_F(vfio_dma_mapping_perf_memfd_test, dma_map_unmap_from_file)
 {
-	const u64 size = SZ_1G;
-	const int flags = variant->mmap_flags;
+	const u64 size = test_params.size;
+	const int mmap_flags = variant->mmap_flags;
 	struct dma_region region;
 	int fd;
 
 	printf("mmap size = %lluG\n", (unsigned long long)(size / SZ_1G));
 
-	region.vaddr = setup_memfd(&fd, size, variant->mmap_flags, variant->memfd_flags);
+	region.vaddr = setup_memfd(&fd, size, mmap_flags, variant->memfd_flags);
 
 	/* Skip the test if there aren't enough HugeTLB pages available. */
-	if (flags & MAP_HUGETLB && region.vaddr == MAP_FAILED)
+	if (mmap_flags & MAP_HUGETLB && region.vaddr == MAP_FAILED)
 		SKIP(return, "setup_memfd() failed: %s (%d)\n", strerror(errno), errno);
 	else
 		ASSERT_NE(region.vaddr, MAP_FAILED);
@@ -185,8 +191,141 @@ TEST_F(vfio_dma_mapping_perf_memfd_test, dma_map_unmap_from_file)
 	teardown_memfd(fd, size, region.vaddr);
 }
 
+/*
+ * Parses "[0-9]+[kmgt]?".
+ */
+u64 parse_size(const char *size)
+{
+	int shift = 0;
+	char *scale;
+	u64 base;
+
+	VFIO_ASSERT_TRUE(size && isdigit(size[0]),
+			 "Need at least one digit in '%s'.", size);
+
+	base = strtoull(size, &scale, 0);
+
+	VFIO_ASSERT_TRUE(base != ULLONG_MAX, "Overflow parsing size!");
+
+	switch (tolower(*scale)) {
+	case 't':
+		shift = 40;
+		break;
+	case 'g':
+		shift = 30;
+		break;
+	case 'm':
+		shift = 20;
+		break;
+	case 'k':
+		shift = 10;
+		break;
+	case 'b':
+	case '\0':
+		shift = 0;
+		break;
+	default:
+		VFIO_FAIL("Unknown size letter '%c'.", *scale);
+	}
+
+	VFIO_ASSERT_TRUE((base << shift) >> shift == base,
+			 "Overflow scaling size!");
+
+	return base << shift;
+}
+
+static void help(char *name)
+{
+	puts("");
+	printf("usage: %s [-h] [-b bytes] [-a \"test harness args\"]\n", name);
+	puts("");
+	printf(" -h: Display this help message.\n"
+	       " -b: Specify the size of the DMA region to be mapped\n"
+	       "     and unmapped. e.g. 16M or 8G, (default: 1G)\n"
+	       " -a: Args that are forwarded to the test harness,\n"
+	       "     e.g. -a \"-t dma_map_unmap_from_file\"\n");
+}
+
+struct harness_args {
+	int argc;
+	char **argv;
+	wordexp_t exp;
+};
+
+static void populate_harness_args(struct harness_args *args, const char *argv_0,
+				  const char *cmdlne)
+{
+	int flags = WRDE_NOCMD;
+
+	if (!args->argv) {
+		/*
+		 * Initialize the argument list with the program name (argv[0]).
+		 * WRDE_NOCMD disables command substitution for safety.
+		 */
+		if (wordexp(argv_0, &args->exp, flags) != 0)
+			VFIO_FAIL("Failed to evaluate test harness argv_0 args!");
+	}
+
+	flags |= WRDE_APPEND;
+
+	/*
+	 * Use wordexp() to reliably parse the user-supplied command line string
+	 * into individual arguments, respecting shell quoting and escaping rules.
+	 * WRDE_APPEND merges these new arguments with the earlier argv[0].
+	 */
+	if (wordexp(cmdlne, &args->exp, flags) != 0)
+		VFIO_FAIL("Failed to evaluate test harness cmdlne args!");
+
+	args->argc = args->exp.we_wordc;
+	args->argv = args->exp.we_wordv;
+}
+
+static void setup_test(struct harness_args *args, int *argc, char *argv[])
+{
+	char *h_argv[] = { argv[0], "-h" };
+	int opt;
+
+	test_params.device_bdf = vfio_selftests_get_bdf(argc, argv);
+
+	while ((opt = getopt(*argc, argv, "a:b:h")) != -1) {
+		switch (opt) {
+		case 'a':
+			populate_harness_args(args, argv[0], optarg);
+			break;
+		case 'b':
+			test_params.size = parse_size(optarg);
+			break;
+		case 'h':
+		default:
+			help(argv[0]);
+			exit(test_harness_run(2, h_argv));
+		}
+	}
+
+	// Reset getopt() state to allow the test harness to use it.
+	optind = 1;
+}
+
+static void teardown_test(struct harness_args *args)
+{
+	if (args->argv) {
+		args->argc = 0;
+		args->argv = NULL;
+		wordfree(&args->exp);
+	}
+}
+
 int main(int argc, char *argv[])
 {
-	device_bdf = vfio_selftests_get_bdf(&argc, argv);
-	return test_harness_run(argc, argv);
+	char *default_hargs[] = { argv[0], NULL };
+	struct harness_args args = {};
+	int r;
+
+	setup_test(&args, &argc, argv);
+
+	r = test_harness_run(args.argc ?: 1, args.argv ?: default_hargs);
+
+	teardown_test(&args);
+
+	return r;
 }
-- 
2.55.0.654.g21b8a5bc05-goog

Re: [PATCH v2 4/4] vfio: selftests: Allow a size for vfio_dma_mapping_perf_test

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Using wordexp() on argv[0] without quoting causes incorrect argument splitting, and using wordexp() on test arguments allows unintended shell expansion.
- [Low] The diagnostic print statement for the mapped region size statically calculates the size in Gigabytes, truncating custom sizes smaller than 1GB to 0G.
--

--- Patch [4]: [PATCH v2 4/4] vfio: selftests: Allow a size for vfio_dma_mapping_perf_test ---
commit cff64ba01747807f5a7be83ede29a96ad3859dc3
Author: Aaron Lewis <aaronlewis@google.com>

vfio: selftests: Allow a size for vfio_dma_mapping_perf_test

This commit allows the user to specify a custom DMA region size via the
command line for vfio_dma_mapping_perf_test. It also introduces a way to
pass arguments through to the test harness using a new '-a' option.

> diff --git a/tools/testing/selftests/vfio/vfio_dma_mapping_perf_test.c b/tools/testing/selftests/vfio/vfio_dma_mapping_perf_test.c
> index 5ef85deba4ee0..af2273a0c6f52 100644
> --- a/tools/testing/selftests/vfio/vfio_dma_mapping_perf_test.c
> +++ b/tools/testing/selftests/vfio/vfio_dma_mapping_perf_test.c

[ ... ]

> @@ -159,17 +165,17 @@ static void teardown_memfd(int fd, u64 size, void *vaddr)
>  
>  TEST_F(vfio_dma_mapping_perf_memfd_test, dma_map_unmap_from_file)
>  {
> -	const u64 size = SZ_1G;
> -	const int flags = variant->mmap_flags;
> +	const u64 size = test_params.size;
> +	const int mmap_flags = variant->mmap_flags;
>  	struct dma_region region;
>  	int fd;
>  
>  	printf("mmap size = %lluG\n", (unsigned long long)(size / SZ_1G));

[Severity: Low]
Does this print statement output "0G" when a size smaller than 1GB is
provided? It looks like the division by SZ_1G truncates custom sizes like
512M to zero.

> -	region.vaddr = setup_memfd(&fd, size, variant->mmap_flags, variant->memfd_flags);
> +	region.vaddr = setup_memfd(&fd, size, mmap_flags, variant->memfd_flags);
>  
>  	/* Skip the test if there aren't enough HugeTLB pages available. */

[ ... ]

> @@ -185,8 +191,141 @@ TEST_F(vfio_dma_mapping_perf_memfd_test, dma_map_unmap_from_file)

[ ... ]

> +static void populate_harness_args(struct harness_args *args, const char *argv_0,
> +				  const char *cmdlne)
> +{
> +	int flags = WRDE_NOCMD;
> +
> +	if (!args->argv) {
> +		/*
> +		 * Initialize the argument list with the program name (argv[0]).
> +		 * WRDE_NOCMD disables command substitution for safety.
> +		 */
> +		if (wordexp(argv_0, &args->exp, flags) != 0)

[Severity: Medium]
Will this wordexp() call split the test executable's path into multiple
arguments if the path contains spaces? If a user runs this from a directory
like '/opt/my tests/test', it seems it would corrupt the test harness
argument list and cause test_harness_run() to fail.

> +			VFIO_FAIL("Failed to evaluate test harness argv_0 args!");
> +	}
> +
> +	flags |= WRDE_APPEND;
> +
> +	/*
> +	 * Use wordexp() to reliably parse the user-supplied command line string
> +	 * into individual arguments, respecting shell quoting and escaping rules.
> +	 * WRDE_APPEND merges these new arguments with the earlier argv[0].
> +	 */
> +	if (wordexp(cmdlne, &args->exp, flags) != 0)

[Severity: Medium]
Does using wordexp() on the user-provided arguments allow unintended shell
globbing? For instance, if a user passes an argument like '-a "-v test*"',
would wordexp() expand this against local files if a match exists, rather
than passing the literal wildcard to the test harness filter?

> +		VFIO_FAIL("Failed to evaluate test harness cmdlne args!");
> +
> +	args->argc = args->exp.we_wordc;
> +	args->argv = args->exp.we_wordv;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260804165748.1060476-1-aaronlewis@google.com?part=4

Re: [PATCH v2 2/4] vfio: selftests: Introduce vfio_dma_mapping_perf_test

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The `TIME` macro can clobber `errno` before the caller evaluates it.
- [Low] Diagnostic output uses `printf` instead of `TH_LOG` or `ksft_print_msg`, violating the Test Anything Protocol (TAP) format.
--

--- Patch [2]: [PATCH v2 2/4] vfio: selftests: Introduce vfio_dma_mapping_perf_test ---
commit 3d215ae55639b2f45faac3bfc752e16beff09489
Author: Aaron Lewis <aaronlewis@google.com>

vfio: selftests: Introduce vfio_dma_mapping_perf_test
    
Introduce vfio_dma_mapping_perf_test to aid in latency testing. Rather
than a traditional pass/fail test, which tends to be flaky when
enforcing latency bounds, this test directly reports the latency of
various stages in the mapping and unmapping process.

> diff --git a/tools/testing/selftests/vfio/lib/include/libvfio.h b/tools/testing/selftests/vfio/lib/include/libvfio.h
> index 07862b470777b..cf322b499cfba 100644
> --- a/tools/testing/selftests/vfio/lib/include/libvfio.h
> +++ b/tools/testing/selftests/vfio/lib/include/libvfio.h

[ ... ]

> @@ -9,6 +9,34 @@
>  #include <libvfio/vfio_pci_device.h>
>  #include <libvfio/vfio_pci_driver.h>
>  
> +#include <stdint.h>
> +#include <time.h>
> +#include <linux/time64.h>

[ ... ]

> +static inline uint64_t timer_elapsed_ns(struct timespec start)
> +{
> +	struct timespec end;
> +
> +	clock_gettime(CLOCK_MONOTONIC, &end);
> +
> +	return (uint64_t)(end.tv_sec - start.tv_sec) * NSEC_PER_SEC +
> +	       (uint64_t)(end.tv_nsec - start.tv_nsec);
> +}
> +
> +#define TIME(_name, _expression) do {				   \
> +	struct timespec __start;				   \
> +								   \
> +	timer_start(&__start);					   \
> +	_expression;						   \
> +	printf(_name " = %.2lfms\n",				   \

[Severity: Medium]
Can this TIME macro clobber errno before the caller evaluates it?

If the target _expression (like mmap) fails and sets errno, the macro then
calls clock_gettime() inside timer_elapsed_ns() and printf(). These libc
functions can modify errno, causing the caller to read an incorrect errno
value.

For example, this happens in dma_map_unmap(), where the SKIP() check reads
errno after mmap() is executed through this macro:

tools/testing/selftests/vfio/vfio_dma_mapping_perf_test.c:dma_map_unmap() {
    ...
    TIME("mmap",
         region.vaddr = mmap(NULL, size, PROT_READ | PROT_WRITE, flags, -1, 0));

    /* Skip the test if there aren't enough HugeTLB pages available. */
    if (flags & MAP_HUGETLB && region.vaddr == MAP_FAILED)
        SKIP(return, "mmap() failed: %s (%d)\n", strerror(errno), errno);
    ...
}


[Severity: Low]
Does this output violate the Test Anything Protocol (TAP) format?

Since it outputs directly to stdout without the TAP diagnostic prefix
("# "), it might confuse TAP parsers. Would it be better to use TH_LOG() or
ksft_print_msg() here?

> +	       (double)timer_elapsed_ns(__start) / NSEC_PER_MSEC); \
> +} while (0)
> +

> diff --git a/tools/testing/selftests/vfio/vfio_dma_mapping_perf_test.c b/tools/testing/selftests/vfio/vfio_dma_mapping_perf_test.c
> new file mode 100644
> index 0000000000000..26c04cabef616
> --- /dev/null
> +++ b/tools/testing/selftests/vfio/vfio_dma_mapping_perf_test.c

[ ... ]

> +TEST_F(vfio_dma_mapping_perf_test, dma_map_unmap)
> +{
> +	const u64 size = SZ_1G;
> +	const int flags = variant->mmap_flags;
> +	struct dma_region region;
> +
> +	printf("mmap size = %lluG\n", (unsigned long long)(size / SZ_1G));

[Severity: Low]
Similar to the TIME macro above, does this directly output to stdout without
the TAP diagnostic prefix?

Using ksft_print_msg() or TH_LOG() instead would ensure the diagnostic output
complies with the TAP specification.

> +
> +	TIME("mmap",
> +	     region.vaddr = mmap(NULL, size, PROT_READ | PROT_WRITE, flags, -1, 0));
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260804165748.1060476-1-aaronlewis@google.com?part=2