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

[David Matlack <dmatlack@google.com>]

On 2026-07-01 08:33 PM, Aaron Lewis wrote:
> 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         | 132 +++++++++++++++++-
>  1 file changed, 129 insertions(+), 3 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 4639bda7ebaa..6c025e9c4420 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>
> @@ -19,6 +20,12 @@
>  
>  static const char *device_bdf;
>  
> +struct test_params {
> +	u64 size;
> +};

Move device_bdf into test_params as well.

> +
> +struct test_params test_params;

static

> +
>  struct iommu_mapping {
>  	u64 pgd;
>  	u64 p4d;
> @@ -80,7 +87,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;
>  	struct timespec start;
> @@ -204,7 +211,7 @@ 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 u64 size = test_params.size;
>  	struct dma_region region;
>  	struct timespec start;
>  	u64 unmapped;
> @@ -235,8 +242,127 @@ TEST_F(vfio_dma_mapping_perf_memfd_test, dma_map_unmap_from_file)
>  	teardown_memfd(fd, size, region.vaddr);
>  }
>  
> +/*
> + * Parses "[0-9]+[kmgt]?".
> + */
> +size_t parse_size(const char *size)

nit: u64 aligns with the types used in the test.

> +{
> +	size_t base;
> +	char *scale;
> +	int shift = 0;
> +
> +	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;
> +}

I'd like to put this in the library but it's going to conflict with KVM
selftests parse_size() helper when we link libvfio into KVM selftests.

I'd like to unify the common helpers between libvfio and KVM selftests
library but that is future work. In the meantime I guess we can keep
this local.

> +
> +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");

Should this also call test_harness_run(1, "-h") to print its help
message?

> +}
> +
> +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)
> +{
> +	if (wordexp(argv_0, &args->exp, WRDE_NOCMD) == 0 &&
> +	    wordexp(cmdlne, &args->exp, WRDE_APPEND | WRDE_NOCMD) == 0) {
> +		args->argc = args->exp.we_wordc;
> +		args->argv = args->exp.we_wordv;
> +	}

Is this missing error handling? Also maybe add some comments since
wordexp() is not a very commonly used helper.

> +}
> +
> +static void setup_test(struct harness_args *args, int argc, char *argv[])
> +{
> +	int opt;
> +
> +	test_params = (struct test_params) {
> +		.size = SZ_1G,
> +	};

Do this where test_params is declared, not here.

  static struct test_params test_params = {
          .size = SZ_1G,
  };

BTW, this change will force all tests to use 1GiB instead of their
current defaults (PAGE_SIZE, 2M, and 1G respectively).

So this change is not purely adding a command line option. I think this
change should probably go into the first commit that introduces the
test, not this one.

> +
> +	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]);
> +			goto out;

This should exit instead of continuing right?

> +		}
> +	}
> +
> +out:
> +	// 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[])
>  {
> +	struct harness_args args = (struct harness_args) {
> +		.argc = 0,
> +		.argv = NULL,
> +	};

This can just be:

  struct harness_args args = {};

> +	int r;

Please be consistent about variable names for return codes. There's a
mix of r and rc I've noticed.

> +
> +	setup_test(&args, argc, argv);
>  	device_bdf = vfio_selftests_get_bdf(&argc, argv);

Move this into setup_test() too.

> -	return test_harness_run(argc, argv);
> +	r = test_harness_run(args.argc, args.argv);
> +	teardown_test(&args);
> +
> +	return r;
>  }
> -- 
> 2.55.0.rc0.799.gd6f94ed593-goog
>