Re: [PATCH v2 5/5] KVM: selftests: List the known IOMMU modes in irq_test's help output

[Sean Christopherson <seanjc@google.com>]

On Tue, Nov 03, 2026, David Matlack wrote:
> List the names of all known IOMMU modes in the help output for irq_test's
> -t option, using iommu_mode_names() from the VFIO selftests library, and
> clarify that the IOMMU mode is auto-detected by default.
> 
> The help output currently claims that the only valid modes are
> vfio_type1_iommu and iommufd, which was never true, and would get stale
> any time a mode is added to the VFIO selftests library.
> 
> Signed-off-by: David Matlack <dmatlack@google.com>
> ---
>  tools/testing/selftests/kvm/irq_test.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/kvm/irq_test.c b/tools/testing/selftests/kvm/irq_test.c
> index 8a345472df5f..ef69f9d7ac3d 100644
> --- a/tools/testing/selftests/kvm/irq_test.c
> +++ b/tools/testing/selftests/kvm/irq_test.c
> @@ -159,7 +159,8 @@ static void help(const char *name)

If you're going to do s/type/mode below, then the initial help lines should also
use iommu_mode.

>  	printf("-i	The number of IRQs to generate during the test\n");
>  	printf("-m	Pin target vCPU to random physical CPU before triggering interrupt\n");
>  	printf("-n	Deliver 50 percent of IRQs as non-maskable interrupts\n");
> -	printf("-t	Override the IOMMU type to use (vfio_type1_iommu or iommufd)\n");
> +	printf("-t	Override the IOMMU mode to use instead of auto-detecting it (%s)\n",

IMO, the "auto-detecting" blurb does more harm than good.  It makes the code harder
to read, and for me at least it's just extra noise in understanding what the '-t'
option does.  It also makes the line reaaaaaaally long.

I know the parentheses are pre-existing, but that was somewhat ok when there were
only two options.  With a big list, it's again noisy to reach the end and have to
think about that the closing ')' is terminating.

What about this?

	printf("-t      Override the IOMMU mode.  Options: %s\n", iommu_mode_names());

To go from:

  -t	Override the IOMMU mode to use instead of auto-detecting it (iommufd, vfio_type1v2_iommu, vfio_type1_iommu, iommufd_compat_type1v2, iommufd_compat_type1)

to

  -t	Override the IOMMU mode.  Options: iommufd, vfio_type1v2_iommu, vfio_type1_iommu, iommufd_compat_type1v2, iommufd_compat_type1

> +	       iommu_mode_names());

This really should spit out only the available modes.  For the internal assert,
it makes sense to print all modes since that's just doing a lookup, but it's
annoying to have to manually probe to see what's available.

Juggling all vs. available is mildly tricky to avoid having to dynamically
allocate, but not that hard in the end.  And IMO, it's totally ok to print a
misleading message if a mode becomes (un)available mid-test, especially since
this helper is unlikely to be used except in terminal situations.

E.g.

static const char *__iommu_mode_names(bool include_unavailable)
{
	static char all_names[256];
	static char avl_names[sizeof(all_names)];
	char *names = include_unavailable ? all_names : avl_names;
	int i, len = 0;

	if (names[0])
		return names;

	for (i = 0; i < ARRAY_SIZE(iommu_modes); i++) {
		if (!include_unavailable && !iommu_mode_probe(&iommu_modes[i]))
			continue;

		len += snprintf(names + len, sizeof(all_names) - len, "%s%s",
				names[0] ? ", " : "", iommu_modes[i].name);
		VFIO_ASSERT_LT(len, (int)sizeof(all_names));
	}

	if (!names[0])
		return NO_IOMMU_MODE_AVAILABLE_MSG;

	return names;
}

const char *iommu_mode_names(void)
{
	return __iommu_mode_names(false);
}

Side topic, the use of parantheses in iommu_mode_lookup() also yields hard to
read output.  E.g. if there's a typo in the command line, the error message is
rather hard to visually parse:

  Unrecognized IOMMU mode: a (known modes: iommufd, vfio_type1v2_iommu, vfio_type1_iommu, iommufd_compat_type1v2, iommufd_compat_type1)

Coupled with the above, how about this:

	VFIO_FAIL("Unrecognized IOMMU mode '%s'.  Known modes: %s.\n",
		  iommu_mode, __iommu_mode_names(true));

to generate:

  Unrecognized IOMMU mode 'a'.  Known modes: iommufd, vfio_type1v2_iommu, vfio_type1_iommu, iommufd_compat_type1v2, iommufd_compat_type1.

Which to me makes it much easier to identify the bad input.

>  	printf("-v	Number of vCPUS to run\n");
>  	printf("-x	Use xAPIC mode instead of x2APIC mode in the guest\n");
>  	printf("\n");
> -- 
> 2.56.0.385.gac671c819a-goog
>