Re: [RFC PATCH 07/33] vfio: selftests: Use command line to set hugepage size for DMA mapping test

[Jason Gunthorpe <jgg@nvidia.com>]

On Fri, May 30, 2025 at 09:50:22AM -0700, David Matlack wrote:
> On Mon, May 26, 2025 at 10:15 AM Jason Gunthorpe <jgg@nvidia.com> wrote:
> >
> > On Fri, May 23, 2025 at 11:29:52PM +0000, David Matlack wrote:
> > > From: Josh Hilke <jrhilke@google.com>
> > >
> > > Add a command line arg to vfio_dma_mapping_test to choose the size of
> > > page which is mapped in VFIO.
> >
> > This doesn't seem right..
> >
> > Tests should run automously, test all possible sizes using a fixture.
> 
> This test uses a fixture already. I assume you're referring to
> FIXTURE_VARIANT()?

Yes


> I'll explore doing this. For a single dimension this looks possible.
> But for multiple dimensions (e.g. cross product of iommu_mode and
> backing_src) I don't see a clear way to do it. But that's just after a
> cursory look.

Explicitly list all the combinations with macros?

Enhance the userspace tests allow code to generate the
variants? Kernel tests can do this:

/**
 * KUNIT_CASE_PARAM - A helper for creation a parameterized &struct kunit_case
 *
 * @test_name: a reference to a test case function.
 * @gen_params: a reference to a parameter generator function.
 *
 * The generator function::
 *
 *	const void* gen_params(const void *prev, char *desc)
 *
 * is used to lazily generate a series of arbitrarily typed values that fit into
 * a void*. The argument @prev is the previously returned value, which should be
 * used to derive the next value; @prev is set to NULL on the initial generator
 * call. When no more values are available, the generator must return NULL.
 * Optionally write a string into @desc (size of KUNIT_PARAM_DESC_SIZE)
 * describing the parameter.
 */
#define KUNIT_CASE_PARAM(test_name, gen_params)			\
		{ .run_case = test_name, .name = #test_name,	\
		  .generate_params = gen_params, .module_name = KBUILD_MODNAME}

> For context, the pattern of passing in test configuration via flags
> rather than automatically testing all combinations is something
> inherited from KVM selftests. That's the common pattern there. There's
> some work happening there to encode configurations at a higher level
> using testcase files and a runner [1].

IMHO it is not good, it should be done with fixtures and variants
slicing the test by matching the test fixture/name/etc.

It doesn't really make sense to have a C test runner that you have to
invoke from bash just because the C stuff is not very good..

>  - The library needs to know which device to use. In this RFC that
> works by the user passing in BDF as a positional argument to each
> test.

I'd probably say it should scan the available vfio devices and pick
the first that it understands how to use. Command line would be an
option.

>  - For tests that use HugeTLB (like this one), the test requires the
> user to have already allocated HugeTLB memory for it to use.

Other tests do this already, like the iommufd test assumes it can get
hugetlb mmaps. Skip the tests on allocation failure is the best I
think you can do.

It would be nice to have test metadata so a runner frame work could
know to provide this stuff in the environment.

Jason