[PATCH v5 0/4] Move io-pgtable-arm selftest to KUnit

[PATCH v5 0/4] Move io-pgtable-arm selftest to KUnit

[Mostafa Saleh]

This is a small series to clean up the io-pgtable-arm library.

The first patch is a small clean up to reduce the depedencies of the
test before moving it.

The second patch was originally part of the SMMUv3 KVM driver support[1],
which needed to factor out the kernel code from the io-pgtable-arm
library, and based on Jason’s suggestion this can be taken out as a
cleanup, and a step further to convert it to kunit.

The second patch just moves the code to a new file with no other changes,
so it can be easier to review with “--color-moved”

The third patch converts the sefltest to be modular, that is useful as
kunit can be a module and it can run anytime after boot.

The fourth patch registers the test using kunit, and converges some of
the APIs, some notes about that:

Granularity of tests:
---------------------
To make the series easier to review, the series changes the test to run in
kunit without making intrusive changes to the test itself.
It’s possible to refactor the tests to have smaller granularity (although
I think that would make it less efficient as we might create the same io-pgtable
config multiple times) and integrate them in kunit as multiple tests, that
change would be more intrusive, if you think that is the right approach,
I can add a couple of more patches re-writing the tests.

Other changes:
--------------
- Also, to make the test changes minimal, and the fail messages similar,
  “KUNIT_FAIL()” is used to fail all tests instead of using KUNIT specific
  assertions.

- Instead of using faux device, we rely on kunit_device_register()

- The WARN is removed when a test fails, as that doesn’t seem to be a pattern
  used with kunit.


You can find the instructions on how to run kunit in the last patch commit message.

A failure at the test with the new implementation look as
[  191.675985]     # arm_lpae_do_selftests: selftest: pgsize_bitmap 0x20010000, IAS 40 OAS 48
[  191.676057]     # arm_lpae_do_selftests: EXPECTATION FAILED at drivers/iommu/io-pgtable-arm-selftests.c:91
[  191.676087]     # arm_lpae_do_selftests: selftest: test failed for fmt idx 0

Changes in v5:
--------------
- Rebase on iommu/next tree
- Collected Jason and Praan Rbs
- Move back #include <slab.h> to the main file.

v4: https://lore.kernel.org/linux-iommu/20250922090003.686704-1-smostafa@google.com/

Main changes in v4:
-------------------
- Remove one missed __init.
- Change the kconfig dependencies back as it broke ARM
https://lore.kernel.org/all/202509201819.f369wBHc-lkp@intel.com/

v3: https://lore.kernel.org/all/20250919133316.2741279-1-smostafa@google.com/

Main changes in v3:
-------------------
- Move back unused code
- Simplify printing based on Jason comments + some renames
- Collect Jasons Rb
v2: https://lore.kernel.org/all/20250917191143.3847487-1-smostafa@google.com/

Main changes in v2:
-------------------
- Make the test modular
v1: https://lore.kernel.org/linux-iommu/20250917140216.2199055-1-smostafa@google.com/

[1] https://lore.kernel.org/all/20250819215156.2494305-5-smostafa@google.com/


Mostafa Saleh (4):
  iommu/io-pgtable-arm: Simplify error prints for selftests
  iommu/io-pgtable-arm: Move selftests to a separate file
  iommu/io-pgtable-arm-selftests: Modularize the test
  iommu/io-pgtable-arm-selftests: Use KUnit

 drivers/iommu/Kconfig                    |  11 +-
 drivers/iommu/Makefile                   |   1 +
 drivers/iommu/io-pgtable-arm-selftests.c | 221 +++++++++++++++++++++++
 drivers/iommu/io-pgtable-arm.c           | 203 ---------------------
 4 files changed, 228 insertions(+), 208 deletions(-)
 create mode 100644 drivers/iommu/io-pgtable-arm-selftests.c


base-commit: 5f4b8c03f41782f37d278946296d9443a7194153
-- 
2.51.0.536.g15c5d4f767-goog

[PATCH v5 1/4] iommu/io-pgtable-arm: Simplify error prints for selftests

[Mostafa Saleh]

At the moment, if the self test fails it prints a lot of information
about the page table (size, levels...) this requires access to many
internals, which has to be exposed in the next patch moving the
tests out.

Instead, we can simplify the print, using ias, oas, pgsize_bitmap
and fmt is enough to identify the failed case, and the rest can
be deduced from the code.

Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Reviewed-by: Pranjal Shrivastava <praan@google.com>
Signed-off-by: Mostafa Saleh <smostafa@google.com>
---
 drivers/iommu/io-pgtable-arm.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
index 7e8e2216c294..00218af5d5f7 100644
--- a/drivers/iommu/io-pgtable-arm.c
+++ b/drivers/iommu/io-pgtable-arm.c
@@ -1299,14 +1299,10 @@ static const struct iommu_flush_ops dummy_tlb_ops __initconst = {
 
 static void __init arm_lpae_dump_ops(struct io_pgtable_ops *ops)
 {
-	struct arm_lpae_io_pgtable *data = io_pgtable_ops_to_data(ops);
-	struct io_pgtable_cfg *cfg = &data->iop.cfg;
+	struct io_pgtable_cfg *cfg = &io_pgtable_ops_to_pgtable(ops)->cfg;
 
-	pr_err("cfg: pgsize_bitmap 0x%lx, ias %u-bit\n",
-		cfg->pgsize_bitmap, cfg->ias);
-	pr_err("data: %d levels, 0x%zx pgd_size, %u pg_shift, %u bits_per_level, pgd @ %p\n",
-		ARM_LPAE_MAX_LEVELS - data->start_level, ARM_LPAE_PGD_SIZE(data),
-		ilog2(ARM_LPAE_GRANULE(data)), data->bits_per_level, data->pgd);
+	pr_err("cfg: pgsize_bitmap 0x%lx, ias %u-bit oas %u-bit\n",
+		cfg->pgsize_bitmap, cfg->ias, cfg->oas);
 }
 
 #define __FAIL(ops, i)	({						\
-- 
2.51.0.536.g15c5d4f767-goog

[PATCH v5 2/4] iommu/io-pgtable-arm: Move selftests to a separate file

[Mostafa Saleh]

Clean up the io-pgtable-arm library by moving the selftests out.
Next the tests will be registered with kunit.

This is useful also to factor out kernel specific code out, so
it can compiled as part of the hypervisor object.

Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Reviewed-by: Pranjal Shrivastava <praan@google.com>
Signed-off-by: Mostafa Saleh <smostafa@google.com>
---
 drivers/iommu/Makefile                   |   1 +
 drivers/iommu/io-pgtable-arm-selftests.c | 207 +++++++++++++++++++++++
 drivers/iommu/io-pgtable-arm.c           | 199 ----------------------
 3 files changed, 208 insertions(+), 199 deletions(-)
 create mode 100644 drivers/iommu/io-pgtable-arm-selftests.c

diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
index 355294fa9033..5250a2eea13f 100644
--- a/drivers/iommu/Makefile
+++ b/drivers/iommu/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_IOMMU_DMA) += dma-iommu.o
 obj-$(CONFIG_IOMMU_IO_PGTABLE) += io-pgtable.o
 obj-$(CONFIG_IOMMU_IO_PGTABLE_ARMV7S) += io-pgtable-arm-v7s.o
 obj-$(CONFIG_IOMMU_IO_PGTABLE_LPAE) += io-pgtable-arm.o
+obj-$(CONFIG_IOMMU_IO_PGTABLE_LPAE_SELFTEST) += io-pgtable-arm-selftests.o
 obj-$(CONFIG_IOMMU_IO_PGTABLE_DART) += io-pgtable-dart.o
 obj-$(CONFIG_IOMMU_IOVA) += iova.o
 obj-$(CONFIG_OF_IOMMU)	+= of_iommu.o
diff --git a/drivers/iommu/io-pgtable-arm-selftests.c b/drivers/iommu/io-pgtable-arm-selftests.c
new file mode 100644
index 000000000000..6ee5329c06c2
--- /dev/null
+++ b/drivers/iommu/io-pgtable-arm-selftests.c
@@ -0,0 +1,207 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * CPU-agnostic ARM page table allocator.
+ *
+ * Copyright (C) 2014 ARM Limited
+ *
+ * Author: Will Deacon <will.deacon@arm.com>
+ */
+#include <linux/device/faux.h>
+#include <linux/io-pgtable.h>
+#include <linux/kernel.h>
+
+#include "io-pgtable-arm.h"
+
+static struct io_pgtable_cfg *cfg_cookie __initdata;
+
+static void __init dummy_tlb_flush_all(void *cookie)
+{
+	WARN_ON(cookie != cfg_cookie);
+}
+
+static void __init dummy_tlb_flush(unsigned long iova, size_t size,
+				   size_t granule, void *cookie)
+{
+	WARN_ON(cookie != cfg_cookie);
+	WARN_ON(!(size & cfg_cookie->pgsize_bitmap));
+}
+
+static void __init dummy_tlb_add_page(struct iommu_iotlb_gather *gather,
+				      unsigned long iova, size_t granule,
+				      void *cookie)
+{
+	dummy_tlb_flush(iova, granule, granule, cookie);
+}
+
+static const struct iommu_flush_ops dummy_tlb_ops __initconst = {
+	.tlb_flush_all	= dummy_tlb_flush_all,
+	.tlb_flush_walk	= dummy_tlb_flush,
+	.tlb_add_page	= dummy_tlb_add_page,
+};
+
+static void __init arm_lpae_dump_ops(struct io_pgtable_ops *ops)
+{
+	struct io_pgtable_cfg *cfg = &io_pgtable_ops_to_pgtable(ops)->cfg;
+
+	pr_err("cfg: pgsize_bitmap 0x%lx, ias %u-bit oas %u-bit\n",
+		cfg->pgsize_bitmap, cfg->ias, cfg->oas);
+}
+
+#define __FAIL(ops, i)	({						\
+		WARN(1, "selftest: test failed for fmt idx %d\n", (i));	\
+		arm_lpae_dump_ops(ops);					\
+		-EFAULT;						\
+})
+
+static int __init arm_lpae_run_tests(struct io_pgtable_cfg *cfg)
+{
+	static const enum io_pgtable_fmt fmts[] __initconst = {
+		ARM_64_LPAE_S1,
+		ARM_64_LPAE_S2,
+	};
+
+	int i, j;
+	unsigned long iova;
+	size_t size, mapped;
+	struct io_pgtable_ops *ops;
+
+	for (i = 0; i < ARRAY_SIZE(fmts); ++i) {
+		cfg_cookie = cfg;
+		ops = alloc_io_pgtable_ops(fmts[i], cfg, cfg);
+		if (!ops) {
+			pr_err("selftest: failed to allocate io pgtable ops\n");
+			return -ENOMEM;
+		}
+
+		/*
+		 * Initial sanity checks.
+		 * Empty page tables shouldn't provide any translations.
+		 */
+		if (ops->iova_to_phys(ops, 42))
+			return __FAIL(ops, i);
+
+		if (ops->iova_to_phys(ops, SZ_1G + 42))
+			return __FAIL(ops, i);
+
+		if (ops->iova_to_phys(ops, SZ_2G + 42))
+			return __FAIL(ops, i);
+
+		/*
+		 * Distinct mappings of different granule sizes.
+		 */
+		iova = 0;
+		for_each_set_bit(j, &cfg->pgsize_bitmap, BITS_PER_LONG) {
+			size = 1UL << j;
+
+			if (ops->map_pages(ops, iova, iova, size, 1,
+					   IOMMU_READ | IOMMU_WRITE |
+					   IOMMU_NOEXEC | IOMMU_CACHE,
+					   GFP_KERNEL, &mapped))
+				return __FAIL(ops, i);
+
+			/* Overlapping mappings */
+			if (!ops->map_pages(ops, iova, iova + size, size, 1,
+					    IOMMU_READ | IOMMU_NOEXEC,
+					    GFP_KERNEL, &mapped))
+				return __FAIL(ops, i);
+
+			if (ops->iova_to_phys(ops, iova + 42) != (iova + 42))
+				return __FAIL(ops, i);
+
+			iova += SZ_1G;
+		}
+
+		/* Full unmap */
+		iova = 0;
+		for_each_set_bit(j, &cfg->pgsize_bitmap, BITS_PER_LONG) {
+			size = 1UL << j;
+
+			if (ops->unmap_pages(ops, iova, size, 1, NULL) != size)
+				return __FAIL(ops, i);
+
+			if (ops->iova_to_phys(ops, iova + 42))
+				return __FAIL(ops, i);
+
+			/* Remap full block */
+			if (ops->map_pages(ops, iova, iova, size, 1,
+					   IOMMU_WRITE, GFP_KERNEL, &mapped))
+				return __FAIL(ops, i);
+
+			if (ops->iova_to_phys(ops, iova + 42) != (iova + 42))
+				return __FAIL(ops, i);
+
+			iova += SZ_1G;
+		}
+
+		/*
+		 * Map/unmap the last largest supported page of the IAS, this can
+		 * trigger corner cases in the concatednated page tables.
+		 */
+		mapped = 0;
+		size = 1UL << __fls(cfg->pgsize_bitmap);
+		iova = (1UL << cfg->ias) - size;
+		if (ops->map_pages(ops, iova, iova, size, 1,
+				   IOMMU_READ | IOMMU_WRITE |
+				   IOMMU_NOEXEC | IOMMU_CACHE,
+				   GFP_KERNEL, &mapped))
+			return __FAIL(ops, i);
+		if (mapped != size)
+			return __FAIL(ops, i);
+		if (ops->unmap_pages(ops, iova, size, 1, NULL) != size)
+			return __FAIL(ops, i);
+
+		free_io_pgtable_ops(ops);
+	}
+
+	return 0;
+}
+
+static int __init arm_lpae_do_selftests(void)
+{
+	static const unsigned long pgsize[] __initconst = {
+		SZ_4K | SZ_2M | SZ_1G,
+		SZ_16K | SZ_32M,
+		SZ_64K | SZ_512M,
+	};
+
+	static const unsigned int address_size[] __initconst = {
+		32, 36, 40, 42, 44, 48,
+	};
+
+	int i, j, k, pass = 0, fail = 0;
+	struct faux_device *dev;
+	struct io_pgtable_cfg cfg = {
+		.tlb = &dummy_tlb_ops,
+		.coherent_walk = true,
+		.quirks = IO_PGTABLE_QUIRK_NO_WARN,
+	};
+
+	dev = faux_device_create("io-pgtable-test", NULL, 0);
+	if (!dev)
+		return -ENOMEM;
+
+	cfg.iommu_dev = &dev->dev;
+
+	for (i = 0; i < ARRAY_SIZE(pgsize); ++i) {
+		for (j = 0; j < ARRAY_SIZE(address_size); ++j) {
+			/* Don't use ias > oas as it is not valid for stage-2. */
+			for (k = 0; k <= j; ++k) {
+				cfg.pgsize_bitmap = pgsize[i];
+				cfg.ias = address_size[k];
+				cfg.oas = address_size[j];
+				pr_info("selftest: pgsize_bitmap 0x%08lx, IAS %u OAS %u\n",
+					pgsize[i], cfg.ias, cfg.oas);
+				if (arm_lpae_run_tests(&cfg))
+					fail++;
+				else
+					pass++;
+			}
+		}
+	}
+
+	pr_info("selftest: completed with %d PASS %d FAIL\n", pass, fail);
+	faux_device_destroy(dev);
+
+	return fail ? -EFAULT : 0;
+}
+subsys_initcall(arm_lpae_do_selftests);
diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
index 00218af5d5f7..e6626004b323 100644
--- a/drivers/iommu/io-pgtable-arm.c
+++ b/drivers/iommu/io-pgtable-arm.c
@@ -12,8 +12,6 @@
 #include <linux/atomic.h>
 #include <linux/bitops.h>
 #include <linux/io-pgtable.h>
-#include <linux/kernel.h>
-#include <linux/device/faux.h>
 #include <linux/sizes.h>
 #include <linux/slab.h>
 #include <linux/types.h>
@@ -1267,200 +1265,3 @@ struct io_pgtable_init_fns io_pgtable_arm_mali_lpae_init_fns = {
 	.alloc	= arm_mali_lpae_alloc_pgtable,
 	.free	= arm_lpae_free_pgtable,
 };
-
-#ifdef CONFIG_IOMMU_IO_PGTABLE_LPAE_SELFTEST
-
-static struct io_pgtable_cfg *cfg_cookie __initdata;
-
-static void __init dummy_tlb_flush_all(void *cookie)
-{
-	WARN_ON(cookie != cfg_cookie);
-}
-
-static void __init dummy_tlb_flush(unsigned long iova, size_t size,
-				   size_t granule, void *cookie)
-{
-	WARN_ON(cookie != cfg_cookie);
-	WARN_ON(!(size & cfg_cookie->pgsize_bitmap));
-}
-
-static void __init dummy_tlb_add_page(struct iommu_iotlb_gather *gather,
-				      unsigned long iova, size_t granule,
-				      void *cookie)
-{
-	dummy_tlb_flush(iova, granule, granule, cookie);
-}
-
-static const struct iommu_flush_ops dummy_tlb_ops __initconst = {
-	.tlb_flush_all	= dummy_tlb_flush_all,
-	.tlb_flush_walk	= dummy_tlb_flush,
-	.tlb_add_page	= dummy_tlb_add_page,
-};
-
-static void __init arm_lpae_dump_ops(struct io_pgtable_ops *ops)
-{
-	struct io_pgtable_cfg *cfg = &io_pgtable_ops_to_pgtable(ops)->cfg;
-
-	pr_err("cfg: pgsize_bitmap 0x%lx, ias %u-bit oas %u-bit\n",
-		cfg->pgsize_bitmap, cfg->ias, cfg->oas);
-}
-
-#define __FAIL(ops, i)	({						\
-		WARN(1, "selftest: test failed for fmt idx %d\n", (i));	\
-		arm_lpae_dump_ops(ops);					\
-		-EFAULT;						\
-})
-
-static int __init arm_lpae_run_tests(struct io_pgtable_cfg *cfg)
-{
-	static const enum io_pgtable_fmt fmts[] __initconst = {
-		ARM_64_LPAE_S1,
-		ARM_64_LPAE_S2,
-	};
-
-	int i, j;
-	unsigned long iova;
-	size_t size, mapped;
-	struct io_pgtable_ops *ops;
-
-	for (i = 0; i < ARRAY_SIZE(fmts); ++i) {
-		cfg_cookie = cfg;
-		ops = alloc_io_pgtable_ops(fmts[i], cfg, cfg);
-		if (!ops) {
-			pr_err("selftest: failed to allocate io pgtable ops\n");
-			return -ENOMEM;
-		}
-
-		/*
-		 * Initial sanity checks.
-		 * Empty page tables shouldn't provide any translations.
-		 */
-		if (ops->iova_to_phys(ops, 42))
-			return __FAIL(ops, i);
-
-		if (ops->iova_to_phys(ops, SZ_1G + 42))
-			return __FAIL(ops, i);
-
-		if (ops->iova_to_phys(ops, SZ_2G + 42))
-			return __FAIL(ops, i);
-
-		/*
-		 * Distinct mappings of different granule sizes.
-		 */
-		iova = 0;
-		for_each_set_bit(j, &cfg->pgsize_bitmap, BITS_PER_LONG) {
-			size = 1UL << j;
-
-			if (ops->map_pages(ops, iova, iova, size, 1,
-					   IOMMU_READ | IOMMU_WRITE |
-					   IOMMU_NOEXEC | IOMMU_CACHE,
-					   GFP_KERNEL, &mapped))
-				return __FAIL(ops, i);
-
-			/* Overlapping mappings */
-			if (!ops->map_pages(ops, iova, iova + size, size, 1,
-					    IOMMU_READ | IOMMU_NOEXEC,
-					    GFP_KERNEL, &mapped))
-				return __FAIL(ops, i);
-
-			if (ops->iova_to_phys(ops, iova + 42) != (iova + 42))
-				return __FAIL(ops, i);
-
-			iova += SZ_1G;
-		}
-
-		/* Full unmap */
-		iova = 0;
-		for_each_set_bit(j, &cfg->pgsize_bitmap, BITS_PER_LONG) {
-			size = 1UL << j;
-
-			if (ops->unmap_pages(ops, iova, size, 1, NULL) != size)
-				return __FAIL(ops, i);
-
-			if (ops->iova_to_phys(ops, iova + 42))
-				return __FAIL(ops, i);
-
-			/* Remap full block */
-			if (ops->map_pages(ops, iova, iova, size, 1,
-					   IOMMU_WRITE, GFP_KERNEL, &mapped))
-				return __FAIL(ops, i);
-
-			if (ops->iova_to_phys(ops, iova + 42) != (iova + 42))
-				return __FAIL(ops, i);
-
-			iova += SZ_1G;
-		}
-
-		/*
-		 * Map/unmap the last largest supported page of the IAS, this can
-		 * trigger corner cases in the concatednated page tables.
-		 */
-		mapped = 0;
-		size = 1UL << __fls(cfg->pgsize_bitmap);
-		iova = (1UL << cfg->ias) - size;
-		if (ops->map_pages(ops, iova, iova, size, 1,
-				   IOMMU_READ | IOMMU_WRITE |
-				   IOMMU_NOEXEC | IOMMU_CACHE,
-				   GFP_KERNEL, &mapped))
-			return __FAIL(ops, i);
-		if (mapped != size)
-			return __FAIL(ops, i);
-		if (ops->unmap_pages(ops, iova, size, 1, NULL) != size)
-			return __FAIL(ops, i);
-
-		free_io_pgtable_ops(ops);
-	}
-
-	return 0;
-}
-
-static int __init arm_lpae_do_selftests(void)
-{
-	static const unsigned long pgsize[] __initconst = {
-		SZ_4K | SZ_2M | SZ_1G,
-		SZ_16K | SZ_32M,
-		SZ_64K | SZ_512M,
-	};
-
-	static const unsigned int address_size[] __initconst = {
-		32, 36, 40, 42, 44, 48,
-	};
-
-	int i, j, k, pass = 0, fail = 0;
-	struct faux_device *dev;
-	struct io_pgtable_cfg cfg = {
-		.tlb = &dummy_tlb_ops,
-		.coherent_walk = true,
-		.quirks = IO_PGTABLE_QUIRK_NO_WARN,
-	};
-
-	dev = faux_device_create("io-pgtable-test", NULL, 0);
-	if (!dev)
-		return -ENOMEM;
-
-	cfg.iommu_dev = &dev->dev;
-
-	for (i = 0; i < ARRAY_SIZE(pgsize); ++i) {
-		for (j = 0; j < ARRAY_SIZE(address_size); ++j) {
-			/* Don't use ias > oas as it is not valid for stage-2. */
-			for (k = 0; k <= j; ++k) {
-				cfg.pgsize_bitmap = pgsize[i];
-				cfg.ias = address_size[k];
-				cfg.oas = address_size[j];
-				pr_info("selftest: pgsize_bitmap 0x%08lx, IAS %u OAS %u\n",
-					pgsize[i], cfg.ias, cfg.oas);
-				if (arm_lpae_run_tests(&cfg))
-					fail++;
-				else
-					pass++;
-			}
-		}
-	}
-
-	pr_info("selftest: completed with %d PASS %d FAIL\n", pass, fail);
-	faux_device_destroy(dev);
-
-	return fail ? -EFAULT : 0;
-}
-subsys_initcall(arm_lpae_do_selftests);
-#endif
-- 
2.51.0.536.g15c5d4f767-goog

[PATCH v5 3/4] iommu/io-pgtable-arm-selftests: Modularize the test

[Mostafa Saleh]

Remove the __init constraint, as the test will be converted to KUnit,
it can run on-demand after later.

Also, as KUnit can be a module, make this test modular.

Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Reviewed-by: Pranjal Shrivastava <praan@google.com>
Signed-off-by: Mostafa Saleh <smostafa@google.com>
---
 drivers/iommu/Kconfig                    |  2 +-
 drivers/iommu/io-pgtable-arm-selftests.c | 36 +++++++++++++++---------
 2 files changed, 23 insertions(+), 15 deletions(-)

diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index 70d29b14d851..553522ef3ca9 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -41,7 +41,7 @@ config IOMMU_IO_PGTABLE_LPAE
 	  up to 48-bits in size.
 
 config IOMMU_IO_PGTABLE_LPAE_SELFTEST
-	bool "LPAE selftests"
+	tristate "LPAE selftests"
 	depends on IOMMU_IO_PGTABLE_LPAE
 	help
 	  Enable self-tests for LPAE page table allocator. This performs
diff --git a/drivers/iommu/io-pgtable-arm-selftests.c b/drivers/iommu/io-pgtable-arm-selftests.c
index 6ee5329c06c2..32c6a5c7af53 100644
--- a/drivers/iommu/io-pgtable-arm-selftests.c
+++ b/drivers/iommu/io-pgtable-arm-selftests.c
@@ -12,34 +12,34 @@
 
 #include "io-pgtable-arm.h"
 
-static struct io_pgtable_cfg *cfg_cookie __initdata;
+static struct io_pgtable_cfg *cfg_cookie;
 
-static void __init dummy_tlb_flush_all(void *cookie)
+static void dummy_tlb_flush_all(void *cookie)
 {
 	WARN_ON(cookie != cfg_cookie);
 }
 
-static void __init dummy_tlb_flush(unsigned long iova, size_t size,
-				   size_t granule, void *cookie)
+static void dummy_tlb_flush(unsigned long iova, size_t size,
+			    size_t granule, void *cookie)
 {
 	WARN_ON(cookie != cfg_cookie);
 	WARN_ON(!(size & cfg_cookie->pgsize_bitmap));
 }
 
-static void __init dummy_tlb_add_page(struct iommu_iotlb_gather *gather,
-				      unsigned long iova, size_t granule,
-				      void *cookie)
+static void dummy_tlb_add_page(struct iommu_iotlb_gather *gather,
+			       unsigned long iova, size_t granule,
+			       void *cookie)
 {
 	dummy_tlb_flush(iova, granule, granule, cookie);
 }
 
-static const struct iommu_flush_ops dummy_tlb_ops __initconst = {
+static const struct iommu_flush_ops dummy_tlb_ops = {
 	.tlb_flush_all	= dummy_tlb_flush_all,
 	.tlb_flush_walk	= dummy_tlb_flush,
 	.tlb_add_page	= dummy_tlb_add_page,
 };
 
-static void __init arm_lpae_dump_ops(struct io_pgtable_ops *ops)
+static void arm_lpae_dump_ops(struct io_pgtable_ops *ops)
 {
 	struct io_pgtable_cfg *cfg = &io_pgtable_ops_to_pgtable(ops)->cfg;
 
@@ -53,9 +53,9 @@ static void __init arm_lpae_dump_ops(struct io_pgtable_ops *ops)
 		-EFAULT;						\
 })
 
-static int __init arm_lpae_run_tests(struct io_pgtable_cfg *cfg)
+static int arm_lpae_run_tests(struct io_pgtable_cfg *cfg)
 {
-	static const enum io_pgtable_fmt fmts[] __initconst = {
+	static const enum io_pgtable_fmt fmts[] = {
 		ARM_64_LPAE_S1,
 		ARM_64_LPAE_S2,
 	};
@@ -156,15 +156,15 @@ static int __init arm_lpae_run_tests(struct io_pgtable_cfg *cfg)
 	return 0;
 }
 
-static int __init arm_lpae_do_selftests(void)
+static int arm_lpae_do_selftests(void)
 {
-	static const unsigned long pgsize[] __initconst = {
+	static const unsigned long pgsize[] = {
 		SZ_4K | SZ_2M | SZ_1G,
 		SZ_16K | SZ_32M,
 		SZ_64K | SZ_512M,
 	};
 
-	static const unsigned int address_size[] __initconst = {
+	static const unsigned int address_size[] = {
 		32, 36, 40, 42, 44, 48,
 	};
 
@@ -204,4 +204,12 @@ static int __init arm_lpae_do_selftests(void)
 
 	return fail ? -EFAULT : 0;
 }
+
+static void arm_lpae_exit_selftests(void)
+{
+}
+
 subsys_initcall(arm_lpae_do_selftests);
+module_exit(arm_lpae_exit_selftests);
+MODULE_DESCRIPTION("io-pgtable-arm library selftest");
+MODULE_LICENSE("GPL");
-- 
2.51.0.536.g15c5d4f767-goog

[PATCH v5 4/4] iommu/io-pgtable-arm-selftests: Use KUnit

[Mostafa Saleh]

Integrate the selftests as part of kunit.

Now instead of the test only being run at boot, it can run:

- With CONFIG_IOMMU_IO_PGTABLE_LPAE_KUNIT_TEST=y
  It will automatically run at boot as before.

- Otherwise with CONFIG_IOMMU_IO_PGTABLE_KUNIT_TEST=m:
  1) on module load:
     Once the module load the self test will run
     # modprobe io-pgtable-arm-selftests

  2) debugfs
     With CONFIG_KUNIT_DEBUGFS=y You can run the test with
     # echo 1 > /sys/kernel/debug/kunit/io-pgtable-arm-test/run

  3) Using kunit.py
     You can also use the helper script which uses Qemu in the background

     # ./tools/testing/kunit/kunit.py run --build_dir build_kunit_arm64 --arch arm64 \
       --make_options LLVM=1 --kunitconfig ./kunit/kunitconfig
      [18:01:09] ============= io-pgtable-arm-test (1 subtest) ==============
      [18:01:09] [PASSED] arm_lpae_do_selftests
      [18:01:09] =============== [PASSED] io-pgtable-arm-test ===============
      [18:01:09] ============================================================

Suggested-by: Jason Gunthorpe <jgg@ziepe.ca>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Reviewed-by: Pranjal Shrivastava <praan@google.com>
Signed-off-by: Mostafa Saleh <smostafa@google.com>
---
 drivers/iommu/Kconfig                    | 11 ++--
 drivers/iommu/Makefile                   |  2 +-
 drivers/iommu/io-pgtable-arm-selftests.c | 82 +++++++++++++-----------
 3 files changed, 51 insertions(+), 44 deletions(-)

diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index 553522ef3ca9..d50685433347 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -40,12 +40,13 @@ config IOMMU_IO_PGTABLE_LPAE
 	  sizes at both stage-1 and stage-2, as well as address spaces
 	  up to 48-bits in size.
 
-config IOMMU_IO_PGTABLE_LPAE_SELFTEST
-	tristate "LPAE selftests"
-	depends on IOMMU_IO_PGTABLE_LPAE
+config IOMMU_IO_PGTABLE_LPAE_KUNIT_TEST
+	tristate "KUnit tests for LPAE"
+	depends on IOMMU_IO_PGTABLE_LPAE && KUNIT
+	default KUNIT_ALL_TESTS
 	help
-	  Enable self-tests for LPAE page table allocator. This performs
-	  a series of page-table consistency checks during boot.
+	  Enable kunit tests for LPAE page table allocator. This performs
+	  a series of page-table consistency checks.
 
 	  If unsure, say N here.
 
diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
index 5250a2eea13f..ac3851570303 100644
--- a/drivers/iommu/Makefile
+++ b/drivers/iommu/Makefile
@@ -12,7 +12,7 @@ obj-$(CONFIG_IOMMU_DMA) += dma-iommu.o
 obj-$(CONFIG_IOMMU_IO_PGTABLE) += io-pgtable.o
 obj-$(CONFIG_IOMMU_IO_PGTABLE_ARMV7S) += io-pgtable-arm-v7s.o
 obj-$(CONFIG_IOMMU_IO_PGTABLE_LPAE) += io-pgtable-arm.o
-obj-$(CONFIG_IOMMU_IO_PGTABLE_LPAE_SELFTEST) += io-pgtable-arm-selftests.o
+obj-$(CONFIG_IOMMU_IO_PGTABLE_LPAE_KUNIT_TEST) += io-pgtable-arm-selftests.o
 obj-$(CONFIG_IOMMU_IO_PGTABLE_DART) += io-pgtable-dart.o
 obj-$(CONFIG_IOMMU_IOVA) += iova.o
 obj-$(CONFIG_OF_IOMMU)	+= of_iommu.o
diff --git a/drivers/iommu/io-pgtable-arm-selftests.c b/drivers/iommu/io-pgtable-arm-selftests.c
index 32c6a5c7af53..b61849a8a685 100644
--- a/drivers/iommu/io-pgtable-arm-selftests.c
+++ b/drivers/iommu/io-pgtable-arm-selftests.c
@@ -6,7 +6,8 @@
  *
  * Author: Will Deacon <will.deacon@arm.com>
  */
-#include <linux/device/faux.h>
+#include <kunit/device.h>
+#include <kunit/test.h>
 #include <linux/io-pgtable.h>
 #include <linux/kernel.h>
 
@@ -47,13 +48,14 @@ static void arm_lpae_dump_ops(struct io_pgtable_ops *ops)
 		cfg->pgsize_bitmap, cfg->ias, cfg->oas);
 }
 
-#define __FAIL(ops, i)	({						\
-		WARN(1, "selftest: test failed for fmt idx %d\n", (i));	\
-		arm_lpae_dump_ops(ops);					\
-		-EFAULT;						\
+#define __FAIL(test, ops, i)	({						\
+		KUNIT_FAIL(test, "");						\
+		kunit_err(test, "selftest: test failed for fmt idx %d\n", (i));	\
+		arm_lpae_dump_ops(ops);						\
+		-EFAULT;							\
 })
 
-static int arm_lpae_run_tests(struct io_pgtable_cfg *cfg)
+static int arm_lpae_run_tests(struct kunit *test, struct io_pgtable_cfg *cfg)
 {
 	static const enum io_pgtable_fmt fmts[] = {
 		ARM_64_LPAE_S1,
@@ -69,7 +71,7 @@ static int arm_lpae_run_tests(struct io_pgtable_cfg *cfg)
 		cfg_cookie = cfg;
 		ops = alloc_io_pgtable_ops(fmts[i], cfg, cfg);
 		if (!ops) {
-			pr_err("selftest: failed to allocate io pgtable ops\n");
+			kunit_err(test, "selftest: failed to allocate io pgtable ops\n");
 			return -ENOMEM;
 		}
 
@@ -78,13 +80,13 @@ static int arm_lpae_run_tests(struct io_pgtable_cfg *cfg)
 		 * Empty page tables shouldn't provide any translations.
 		 */
 		if (ops->iova_to_phys(ops, 42))
-			return __FAIL(ops, i);
+			return __FAIL(test, ops, i);
 
 		if (ops->iova_to_phys(ops, SZ_1G + 42))
-			return __FAIL(ops, i);
+			return __FAIL(test, ops, i);
 
 		if (ops->iova_to_phys(ops, SZ_2G + 42))
-			return __FAIL(ops, i);
+			return __FAIL(test, ops, i);
 
 		/*
 		 * Distinct mappings of different granule sizes.
@@ -97,16 +99,16 @@ static int arm_lpae_run_tests(struct io_pgtable_cfg *cfg)
 					   IOMMU_READ | IOMMU_WRITE |
 					   IOMMU_NOEXEC | IOMMU_CACHE,
 					   GFP_KERNEL, &mapped))
-				return __FAIL(ops, i);
+				return __FAIL(test, ops, i);
 
 			/* Overlapping mappings */
 			if (!ops->map_pages(ops, iova, iova + size, size, 1,
 					    IOMMU_READ | IOMMU_NOEXEC,
 					    GFP_KERNEL, &mapped))
-				return __FAIL(ops, i);
+				return __FAIL(test, ops, i);
 
 			if (ops->iova_to_phys(ops, iova + 42) != (iova + 42))
-				return __FAIL(ops, i);
+				return __FAIL(test, ops, i);
 
 			iova += SZ_1G;
 		}
@@ -117,18 +119,18 @@ static int arm_lpae_run_tests(struct io_pgtable_cfg *cfg)
 			size = 1UL << j;
 
 			if (ops->unmap_pages(ops, iova, size, 1, NULL) != size)
-				return __FAIL(ops, i);
+				return __FAIL(test, ops, i);
 
 			if (ops->iova_to_phys(ops, iova + 42))
-				return __FAIL(ops, i);
+				return __FAIL(test, ops, i);
 
 			/* Remap full block */
 			if (ops->map_pages(ops, iova, iova, size, 1,
 					   IOMMU_WRITE, GFP_KERNEL, &mapped))
-				return __FAIL(ops, i);
+				return __FAIL(test, ops, i);
 
 			if (ops->iova_to_phys(ops, iova + 42) != (iova + 42))
-				return __FAIL(ops, i);
+				return __FAIL(test, ops, i);
 
 			iova += SZ_1G;
 		}
@@ -144,11 +146,11 @@ static int arm_lpae_run_tests(struct io_pgtable_cfg *cfg)
 				   IOMMU_READ | IOMMU_WRITE |
 				   IOMMU_NOEXEC | IOMMU_CACHE,
 				   GFP_KERNEL, &mapped))
-			return __FAIL(ops, i);
+			return __FAIL(test, ops, i);
 		if (mapped != size)
-			return __FAIL(ops, i);
+			return __FAIL(test, ops, i);
 		if (ops->unmap_pages(ops, iova, size, 1, NULL) != size)
-			return __FAIL(ops, i);
+			return __FAIL(test, ops, i);
 
 		free_io_pgtable_ops(ops);
 	}
@@ -156,7 +158,7 @@ static int arm_lpae_run_tests(struct io_pgtable_cfg *cfg)
 	return 0;
 }
 
-static int arm_lpae_do_selftests(void)
+static void arm_lpae_do_selftests(struct kunit *test)
 {
 	static const unsigned long pgsize[] = {
 		SZ_4K | SZ_2M | SZ_1G,
@@ -169,18 +171,19 @@ static int arm_lpae_do_selftests(void)
 	};
 
 	int i, j, k, pass = 0, fail = 0;
-	struct faux_device *dev;
+	struct device *dev;
 	struct io_pgtable_cfg cfg = {
 		.tlb = &dummy_tlb_ops,
 		.coherent_walk = true,
 		.quirks = IO_PGTABLE_QUIRK_NO_WARN,
 	};
 
-	dev = faux_device_create("io-pgtable-test", NULL, 0);
-	if (!dev)
-		return -ENOMEM;
+	dev = kunit_device_register(test, "io-pgtable-test");
+	KUNIT_EXPECT_NOT_ERR_OR_NULL(test, dev);
+	if (IS_ERR_OR_NULL(dev))
+		return;
 
-	cfg.iommu_dev = &dev->dev;
+	cfg.iommu_dev = dev;
 
 	for (i = 0; i < ARRAY_SIZE(pgsize); ++i) {
 		for (j = 0; j < ARRAY_SIZE(address_size); ++j) {
@@ -189,9 +192,9 @@ static int arm_lpae_do_selftests(void)
 				cfg.pgsize_bitmap = pgsize[i];
 				cfg.ias = address_size[k];
 				cfg.oas = address_size[j];
-				pr_info("selftest: pgsize_bitmap 0x%08lx, IAS %u OAS %u\n",
-					pgsize[i], cfg.ias, cfg.oas);
-				if (arm_lpae_run_tests(&cfg))
+				kunit_info(test, "selftest: pgsize_bitmap 0x%08lx, IAS %u OAS %u\n",
+					   pgsize[i], cfg.ias, cfg.oas);
+				if (arm_lpae_run_tests(test, &cfg))
 					fail++;
 				else
 					pass++;
@@ -199,17 +202,20 @@ static int arm_lpae_do_selftests(void)
 		}
 	}
 
-	pr_info("selftest: completed with %d PASS %d FAIL\n", pass, fail);
-	faux_device_destroy(dev);
-
-	return fail ? -EFAULT : 0;
+	kunit_info(test, "selftest: completed with %d PASS %d FAIL\n", pass, fail);
 }
 
-static void arm_lpae_exit_selftests(void)
-{
-}
+static struct kunit_case io_pgtable_arm_test_cases[] = {
+	KUNIT_CASE(arm_lpae_do_selftests),
+	{},
+};
+
+static struct kunit_suite io_pgtable_arm_test = {
+	.name = "io-pgtable-arm-test",
+	.test_cases = io_pgtable_arm_test_cases,
+};
+
+kunit_test_suite(io_pgtable_arm_test);
 
-subsys_initcall(arm_lpae_do_selftests);
-module_exit(arm_lpae_exit_selftests);
 MODULE_DESCRIPTION("io-pgtable-arm library selftest");
 MODULE_LICENSE("GPL");
-- 
2.51.0.536.g15c5d4f767-goog

Re: [PATCH v5 0/4] Move io-pgtable-arm selftest to KUnit

[Mostafa Saleh]

On Mon, Sep 29, 2025 at 4:50 PM Mostafa Saleh <smostafa@google.com> wrote:
>
> This is a small series to clean up the io-pgtable-arm library.
>
> The first patch is a small clean up to reduce the depedencies of the
> test before moving it.
>
> The second patch was originally part of the SMMUv3 KVM driver support[1],
> which needed to factor out the kernel code from the io-pgtable-arm
> library, and based on Jason’s suggestion this can be taken out as a
> cleanup, and a step further to convert it to kunit.
>
> The second patch just moves the code to a new file with no other changes,
> so it can be easier to review with “--color-moved”
>
> The third patch converts the sefltest to be modular, that is useful as
> kunit can be a module and it can run anytime after boot.
>
> The fourth patch registers the test using kunit, and converges some of
> the APIs, some notes about that:
>
> Granularity of tests:
> ---------------------
> To make the series easier to review, the series changes the test to run in
> kunit without making intrusive changes to the test itself.
> It’s possible to refactor the tests to have smaller granularity (although
> I think that would make it less efficient as we might create the same io-pgtable
> config multiple times) and integrate them in kunit as multiple tests, that
> change would be more intrusive, if you think that is the right approach,
> I can add a couple of more patches re-writing the tests.
>
> Other changes:
> --------------
> - Also, to make the test changes minimal, and the fail messages similar,
>   “KUNIT_FAIL()” is used to fail all tests instead of using KUNIT specific
>   assertions.
>
> - Instead of using faux device, we rely on kunit_device_register()
>
> - The WARN is removed when a test fails, as that doesn’t seem to be a pattern
>   used with kunit.
>
>
> You can find the instructions on how to run kunit in the last patch commit message.
>
> A failure at the test with the new implementation look as
> [  191.675985]     # arm_lpae_do_selftests: selftest: pgsize_bitmap 0x20010000, IAS 40 OAS 48
> [  191.676057]     # arm_lpae_do_selftests: EXPECTATION FAILED at drivers/iommu/io-pgtable-arm-selftests.c:91
> [  191.676087]     # arm_lpae_do_selftests: selftest: test failed for fmt idx 0
>
> Changes in v5:
> --------------
> - Rebase on iommu/next tree
> - Collected Jason and Praan Rbs
> - Move back #include <slab.h> to the main file.
>
> v4: https://lore.kernel.org/linux-iommu/20250922090003.686704-1-smostafa@google.com/
>
> Main changes in v4:
> -------------------
> - Remove one missed __init.
> - Change the kconfig dependencies back as it broke ARM
> https://lore.kernel.org/all/202509201819.f369wBHc-lkp@intel.com/
>
> v3: https://lore.kernel.org/all/20250919133316.2741279-1-smostafa@google.com/
>
> Main changes in v3:
> -------------------
> - Move back unused code
> - Simplify printing based on Jason comments + some renames
> - Collect Jasons Rb
> v2: https://lore.kernel.org/all/20250917191143.3847487-1-smostafa@google.com/
>
> Main changes in v2:
> -------------------
> - Make the test modular
> v1: https://lore.kernel.org/linux-iommu/20250917140216.2199055-1-smostafa@google.com/
>
> [1] https://lore.kernel.org/all/20250819215156.2494305-5-smostafa@google.com/
>
>
> Mostafa Saleh (4):
>   iommu/io-pgtable-arm: Simplify error prints for selftests
>   iommu/io-pgtable-arm: Move selftests to a separate file
>   iommu/io-pgtable-arm-selftests: Modularize the test
>   iommu/io-pgtable-arm-selftests: Use KUnit
>
>  drivers/iommu/Kconfig                    |  11 +-
>  drivers/iommu/Makefile                   |   1 +
>  drivers/iommu/io-pgtable-arm-selftests.c | 221 +++++++++++++++++++++++
>  drivers/iommu/io-pgtable-arm.c           | 203 ---------------------
>  4 files changed, 228 insertions(+), 208 deletions(-)
>  create mode 100644 drivers/iommu/io-pgtable-arm-selftests.c
>
>
> base-commit: 5f4b8c03f41782f37d278946296d9443a7194153
> --

Pinging, as this has been reviewed for while, Is there anything
blocking it or any more feedback?

Thanks,
Mostafa

> 2.51.0.536.g15c5d4f767-goog
>

Re: [PATCH v5 1/4] iommu/io-pgtable-arm: Simplify error prints for selftests

[Robin Murphy]

On 2025-09-29 4:49 pm, Mostafa Saleh wrote:
> At the moment, if the self test fails it prints a lot of information
> about the page table (size, levels...) this requires access to many
> internals, which has to be exposed in the next patch moving the
> tests out.
> 
> Instead, we can simplify the print, using ias, oas, pgsize_bitmap
> and fmt is enough to identify the failed case, and the rest can
> be deduced from the code.

That is the same cfg information already logged at the start of the 
test, so if it's not useful to dump the actual internal details of the 
ops then just remove the whole function and make life even simpler. 
(Otherwise export it or turn it into a callback.)

Thanks,
Robin.
> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
> Reviewed-by: Pranjal Shrivastava <praan@google.com>
> Signed-off-by: Mostafa Saleh <smostafa@google.com>
> ---
>   drivers/iommu/io-pgtable-arm.c | 10 +++-------
>   1 file changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
> index 7e8e2216c294..00218af5d5f7 100644
> --- a/drivers/iommu/io-pgtable-arm.c
> +++ b/drivers/iommu/io-pgtable-arm.c
> @@ -1299,14 +1299,10 @@ static const struct iommu_flush_ops dummy_tlb_ops __initconst = {
>   
>   static void __init arm_lpae_dump_ops(struct io_pgtable_ops *ops)
>   {
> -	struct arm_lpae_io_pgtable *data = io_pgtable_ops_to_data(ops);
> -	struct io_pgtable_cfg *cfg = &data->iop.cfg;
> +	struct io_pgtable_cfg *cfg = &io_pgtable_ops_to_pgtable(ops)->cfg;
>   
> -	pr_err("cfg: pgsize_bitmap 0x%lx, ias %u-bit\n",
> -		cfg->pgsize_bitmap, cfg->ias);
> -	pr_err("data: %d levels, 0x%zx pgd_size, %u pg_shift, %u bits_per_level, pgd @ %p\n",
> -		ARM_LPAE_MAX_LEVELS - data->start_level, ARM_LPAE_PGD_SIZE(data),
> -		ilog2(ARM_LPAE_GRANULE(data)), data->bits_per_level, data->pgd);
> +	pr_err("cfg: pgsize_bitmap 0x%lx, ias %u-bit oas %u-bit\n",
> +		cfg->pgsize_bitmap, cfg->ias, cfg->oas);
>   }
>   
>   #define __FAIL(ops, i)	({						\

Re: [PATCH v5 4/4] iommu/io-pgtable-arm-selftests: Use KUnit

[Robin Murphy]

On 2025-09-29 4:49 pm, Mostafa Saleh wrote:
> Integrate the selftests as part of kunit.
> 
> Now instead of the test only being run at boot, it can run:
> 
> - With CONFIG_IOMMU_IO_PGTABLE_LPAE_KUNIT_TEST=y
>    It will automatically run at boot as before.
> 
> - Otherwise with CONFIG_IOMMU_IO_PGTABLE_KUNIT_TEST=m:
>    1) on module load:
>       Once the module load the self test will run
>       # modprobe io-pgtable-arm-selftests
> 
>    2) debugfs
>       With CONFIG_KUNIT_DEBUGFS=y You can run the test with
>       # echo 1 > /sys/kernel/debug/kunit/io-pgtable-arm-test/run
> 
>    3) Using kunit.py
>       You can also use the helper script which uses Qemu in the background
> 
>       # ./tools/testing/kunit/kunit.py run --build_dir build_kunit_arm64 --arch arm64 \
>         --make_options LLVM=1 --kunitconfig ./kunit/kunitconfig
>        [18:01:09] ============= io-pgtable-arm-test (1 subtest) ==============
>        [18:01:09] [PASSED] arm_lpae_do_selftests
>        [18:01:09] =============== [PASSED] io-pgtable-arm-test ===============
>        [18:01:09] ============================================================
> 
> Suggested-by: Jason Gunthorpe <jgg@ziepe.ca>
> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
> Reviewed-by: Pranjal Shrivastava <praan@google.com>
> Signed-off-by: Mostafa Saleh <smostafa@google.com>
> ---
>   drivers/iommu/Kconfig                    | 11 ++--
>   drivers/iommu/Makefile                   |  2 +-
>   drivers/iommu/io-pgtable-arm-selftests.c | 82 +++++++++++++-----------
>   3 files changed, 51 insertions(+), 44 deletions(-)
> 
> diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> index 553522ef3ca9..d50685433347 100644
> --- a/drivers/iommu/Kconfig
> +++ b/drivers/iommu/Kconfig
> @@ -40,12 +40,13 @@ config IOMMU_IO_PGTABLE_LPAE
>   	  sizes at both stage-1 and stage-2, as well as address spaces
>   	  up to 48-bits in size.
>   
> -config IOMMU_IO_PGTABLE_LPAE_SELFTEST
> -	tristate "LPAE selftests"
> -	depends on IOMMU_IO_PGTABLE_LPAE
> +config IOMMU_IO_PGTABLE_LPAE_KUNIT_TEST
> +	tristate "KUnit tests for LPAE"
> +	depends on IOMMU_IO_PGTABLE_LPAE && KUNIT
> +	default KUNIT_ALL_TESTS
>   	help
> -	  Enable self-tests for LPAE page table allocator. This performs
> -	  a series of page-table consistency checks during boot.
> +	  Enable kunit tests for LPAE page table allocator. This performs
> +	  a series of page-table consistency checks.
>   
>   	  If unsure, say N here.
>   
> diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
> index 5250a2eea13f..ac3851570303 100644
> --- a/drivers/iommu/Makefile
> +++ b/drivers/iommu/Makefile
> @@ -12,7 +12,7 @@ obj-$(CONFIG_IOMMU_DMA) += dma-iommu.o
>   obj-$(CONFIG_IOMMU_IO_PGTABLE) += io-pgtable.o
>   obj-$(CONFIG_IOMMU_IO_PGTABLE_ARMV7S) += io-pgtable-arm-v7s.o
>   obj-$(CONFIG_IOMMU_IO_PGTABLE_LPAE) += io-pgtable-arm.o
> -obj-$(CONFIG_IOMMU_IO_PGTABLE_LPAE_SELFTEST) += io-pgtable-arm-selftests.o
> +obj-$(CONFIG_IOMMU_IO_PGTABLE_LPAE_KUNIT_TEST) += io-pgtable-arm-selftests.o
>   obj-$(CONFIG_IOMMU_IO_PGTABLE_DART) += io-pgtable-dart.o
>   obj-$(CONFIG_IOMMU_IOVA) += iova.o
>   obj-$(CONFIG_OF_IOMMU)	+= of_iommu.o
> diff --git a/drivers/iommu/io-pgtable-arm-selftests.c b/drivers/iommu/io-pgtable-arm-selftests.c
> index 32c6a5c7af53..b61849a8a685 100644
> --- a/drivers/iommu/io-pgtable-arm-selftests.c
> +++ b/drivers/iommu/io-pgtable-arm-selftests.c
> @@ -6,7 +6,8 @@
>    *
>    * Author: Will Deacon <will.deacon@arm.com>
>    */
> -#include <linux/device/faux.h>
> +#include <kunit/device.h>
> +#include <kunit/test.h>
>   #include <linux/io-pgtable.h>
>   #include <linux/kernel.h>
>   
> @@ -47,13 +48,14 @@ static void arm_lpae_dump_ops(struct io_pgtable_ops *ops)
>   		cfg->pgsize_bitmap, cfg->ias, cfg->oas);
>   }
>   
> -#define __FAIL(ops, i)	({						\
> -		WARN(1, "selftest: test failed for fmt idx %d\n", (i));	\
> -		arm_lpae_dump_ops(ops);					\
> -		-EFAULT;						\
> +#define __FAIL(test, ops, i)	({						\
> +		KUNIT_FAIL(test, "");						\
> +		kunit_err(test, "selftest: test failed for fmt idx %d\n", (i));	\

This looks suspect - AFAICS open-coded kunit_err() is intended for test 
infrastucture errors (like the allocation in the next hunk below), while 
for an actual test report message it seems it should just be:

	KUNIT_FAIL(test, "selftest: test failed for fmt idx %d\n", (i));

> +		arm_lpae_dump_ops(ops);						\
> +		-EFAULT;							\
>   })
>   
> -static int arm_lpae_run_tests(struct io_pgtable_cfg *cfg)
> +static int arm_lpae_run_tests(struct kunit *test, struct io_pgtable_cfg *cfg)
>   {
>   	static const enum io_pgtable_fmt fmts[] = {
>   		ARM_64_LPAE_S1,
> @@ -69,7 +71,7 @@ static int arm_lpae_run_tests(struct io_pgtable_cfg *cfg)
>   		cfg_cookie = cfg;
>   		ops = alloc_io_pgtable_ops(fmts[i], cfg, cfg);
>   		if (!ops) {
> -			pr_err("selftest: failed to allocate io pgtable ops\n");
> +			kunit_err(test, "selftest: failed to allocate io pgtable ops\n");
>   			return -ENOMEM;
>   		}
>   
> @@ -78,13 +80,13 @@ static int arm_lpae_run_tests(struct io_pgtable_cfg *cfg)
>   		 * Empty page tables shouldn't provide any translations.
>   		 */
>   		if (ops->iova_to_phys(ops, 42))
> -			return __FAIL(ops, i);
> +			return __FAIL(test, ops, i);
>   
>   		if (ops->iova_to_phys(ops, SZ_1G + 42))
> -			return __FAIL(ops, i);
> +			return __FAIL(test, ops, i);
>   
>   		if (ops->iova_to_phys(ops, SZ_2G + 42))
> -			return __FAIL(ops, i);
> +			return __FAIL(test, ops, i);
>   
>   		/*
>   		 * Distinct mappings of different granule sizes.
> @@ -97,16 +99,16 @@ static int arm_lpae_run_tests(struct io_pgtable_cfg *cfg)
>   					   IOMMU_READ | IOMMU_WRITE |
>   					   IOMMU_NOEXEC | IOMMU_CACHE,
>   					   GFP_KERNEL, &mapped))
> -				return __FAIL(ops, i);
> +				return __FAIL(test, ops, i);
>   
>   			/* Overlapping mappings */
>   			if (!ops->map_pages(ops, iova, iova + size, size, 1,
>   					    IOMMU_READ | IOMMU_NOEXEC,
>   					    GFP_KERNEL, &mapped))
> -				return __FAIL(ops, i);
> +				return __FAIL(test, ops, i);
>   
>   			if (ops->iova_to_phys(ops, iova + 42) != (iova + 42))
> -				return __FAIL(ops, i);
> +				return __FAIL(test, ops, i);
>   
>   			iova += SZ_1G;
>   		}
> @@ -117,18 +119,18 @@ static int arm_lpae_run_tests(struct io_pgtable_cfg *cfg)
>   			size = 1UL << j;
>   
>   			if (ops->unmap_pages(ops, iova, size, 1, NULL) != size)
> -				return __FAIL(ops, i);
> +				return __FAIL(test, ops, i);
>   
>   			if (ops->iova_to_phys(ops, iova + 42))
> -				return __FAIL(ops, i);
> +				return __FAIL(test, ops, i);
>   
>   			/* Remap full block */
>   			if (ops->map_pages(ops, iova, iova, size, 1,
>   					   IOMMU_WRITE, GFP_KERNEL, &mapped))
> -				return __FAIL(ops, i);
> +				return __FAIL(test, ops, i);
>   
>   			if (ops->iova_to_phys(ops, iova + 42) != (iova + 42))
> -				return __FAIL(ops, i);
> +				return __FAIL(test, ops, i);
>   
>   			iova += SZ_1G;
>   		}
> @@ -144,11 +146,11 @@ static int arm_lpae_run_tests(struct io_pgtable_cfg *cfg)
>   				   IOMMU_READ | IOMMU_WRITE |
>   				   IOMMU_NOEXEC | IOMMU_CACHE,
>   				   GFP_KERNEL, &mapped))
> -			return __FAIL(ops, i);
> +			return __FAIL(test, ops, i);
>   		if (mapped != size)
> -			return __FAIL(ops, i);
> +			return __FAIL(test, ops, i);
>   		if (ops->unmap_pages(ops, iova, size, 1, NULL) != size)
> -			return __FAIL(ops, i);
> +			return __FAIL(test, ops, i);
>   
>   		free_io_pgtable_ops(ops);
>   	}
> @@ -156,7 +158,7 @@ static int arm_lpae_run_tests(struct io_pgtable_cfg *cfg)
>   	return 0;
>   }
>   
> -static int arm_lpae_do_selftests(void)
> +static void arm_lpae_do_selftests(struct kunit *test)
>   {
>   	static const unsigned long pgsize[] = {
>   		SZ_4K | SZ_2M | SZ_1G,
> @@ -169,18 +171,19 @@ static int arm_lpae_do_selftests(void)
>   	};
>   
>   	int i, j, k, pass = 0, fail = 0;
> -	struct faux_device *dev;
> +	struct device *dev;
>   	struct io_pgtable_cfg cfg = {
>   		.tlb = &dummy_tlb_ops,
>   		.coherent_walk = true,
>   		.quirks = IO_PGTABLE_QUIRK_NO_WARN,
>   	};
>   
> -	dev = faux_device_create("io-pgtable-test", NULL, 0);
> -	if (!dev)
> -		return -ENOMEM;
> +	dev = kunit_device_register(test, "io-pgtable-test");
> +	KUNIT_EXPECT_NOT_ERR_OR_NULL(test, dev);

Conversely, this is infrastructure, not an actual test of expected 
io-pgtable behaviour, so I think just:

	cfg.iommu_dev = kunit_device_register(test, "io-pgtable-test");
	if (IS_ERR(cfg.iommu_dev))
		return;

(it doesn't return NULLs either)

Otherwise, there's clearly scope for plenty more follow-up work 
streamlining and breaking the whole thing up into KUnit idioms, pulling 
the v7s test in, etc... However as a first step to at least set things 
firmly on the right KUnit-shaped path, I agree this seems enough for now.

Thanks,
Robin.

> +	if (IS_ERR_OR_NULL(dev))
> +		return;
>   
> -	cfg.iommu_dev = &dev->dev;
> +	cfg.iommu_dev = dev;
>   
>   	for (i = 0; i < ARRAY_SIZE(pgsize); ++i) {
>   		for (j = 0; j < ARRAY_SIZE(address_size); ++j) {
> @@ -189,9 +192,9 @@ static int arm_lpae_do_selftests(void)
>   				cfg.pgsize_bitmap = pgsize[i];
>   				cfg.ias = address_size[k];
>   				cfg.oas = address_size[j];
> -				pr_info("selftest: pgsize_bitmap 0x%08lx, IAS %u OAS %u\n",
> -					pgsize[i], cfg.ias, cfg.oas);
> -				if (arm_lpae_run_tests(&cfg))
> +				kunit_info(test, "selftest: pgsize_bitmap 0x%08lx, IAS %u OAS %u\n",
> +					   pgsize[i], cfg.ias, cfg.oas);
> +				if (arm_lpae_run_tests(test, &cfg))
>   					fail++;
>   				else
>   					pass++;
> @@ -199,17 +202,20 @@ static int arm_lpae_do_selftests(void)
>   		}
>   	}
>   
> -	pr_info("selftest: completed with %d PASS %d FAIL\n", pass, fail);
> -	faux_device_destroy(dev);
> -
> -	return fail ? -EFAULT : 0;
> +	kunit_info(test, "selftest: completed with %d PASS %d FAIL\n", pass, fail);
>   }
>   
> -static void arm_lpae_exit_selftests(void)
> -{
> -}
> +static struct kunit_case io_pgtable_arm_test_cases[] = {
> +	KUNIT_CASE(arm_lpae_do_selftests),
> +	{},
> +};
> +
> +static struct kunit_suite io_pgtable_arm_test = {
> +	.name = "io-pgtable-arm-test",
> +	.test_cases = io_pgtable_arm_test_cases,
> +};
> +
> +kunit_test_suite(io_pgtable_arm_test);
>   
> -subsys_initcall(arm_lpae_do_selftests);
> -module_exit(arm_lpae_exit_selftests);
>   MODULE_DESCRIPTION("io-pgtable-arm library selftest");
>   MODULE_LICENSE("GPL");

Re: [PATCH v5 1/4] iommu/io-pgtable-arm: Simplify error prints for selftests

[Mostafa Saleh]

On Tue, Oct 14, 2025 at 12:09:56PM +0100, Robin Murphy wrote:
> On 2025-09-29 4:49 pm, Mostafa Saleh wrote:
> > At the moment, if the self test fails it prints a lot of information
> > about the page table (size, levels...) this requires access to many
> > internals, which has to be exposed in the next patch moving the
> > tests out.
> > 
> > Instead, we can simplify the print, using ias, oas, pgsize_bitmap
> > and fmt is enough to identify the failed case, and the rest can
> > be deduced from the code.
> 
> That is the same cfg information already logged at the start of the test, so
> if it's not useful to dump the actual internal details of the ops then just
> remove the whole function and make life even simpler. (Otherwise export it
> or turn it into a callback.)

I see, I think it's better to just remove the whole function.

Thanks,
Mostafa

> 
> Thanks,
> Robin.
> > Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
> > Reviewed-by: Pranjal Shrivastava <praan@google.com>
> > Signed-off-by: Mostafa Saleh <smostafa@google.com>
> > ---
> >   drivers/iommu/io-pgtable-arm.c | 10 +++-------
> >   1 file changed, 3 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
> > index 7e8e2216c294..00218af5d5f7 100644
> > --- a/drivers/iommu/io-pgtable-arm.c
> > +++ b/drivers/iommu/io-pgtable-arm.c
> > @@ -1299,14 +1299,10 @@ static const struct iommu_flush_ops dummy_tlb_ops __initconst = {
> >   static void __init arm_lpae_dump_ops(struct io_pgtable_ops *ops)
> >   {
> > -	struct arm_lpae_io_pgtable *data = io_pgtable_ops_to_data(ops);
> > -	struct io_pgtable_cfg *cfg = &data->iop.cfg;
> > +	struct io_pgtable_cfg *cfg = &io_pgtable_ops_to_pgtable(ops)->cfg;
> > -	pr_err("cfg: pgsize_bitmap 0x%lx, ias %u-bit\n",
> > -		cfg->pgsize_bitmap, cfg->ias);
> > -	pr_err("data: %d levels, 0x%zx pgd_size, %u pg_shift, %u bits_per_level, pgd @ %p\n",
> > -		ARM_LPAE_MAX_LEVELS - data->start_level, ARM_LPAE_PGD_SIZE(data),
> > -		ilog2(ARM_LPAE_GRANULE(data)), data->bits_per_level, data->pgd);
> > +	pr_err("cfg: pgsize_bitmap 0x%lx, ias %u-bit oas %u-bit\n",
> > +		cfg->pgsize_bitmap, cfg->ias, cfg->oas);
> >   }
> >   #define __FAIL(ops, i)	({						\
> 

Re: [PATCH v5 4/4] iommu/io-pgtable-arm-selftests: Use KUnit

[Mostafa Saleh]

On Tue, Oct 14, 2025 at 02:43:53PM +0100, Robin Murphy wrote:
> On 2025-09-29 4:49 pm, Mostafa Saleh wrote:
> > Integrate the selftests as part of kunit.
> > 
> > Now instead of the test only being run at boot, it can run:
> > 
> > - With CONFIG_IOMMU_IO_PGTABLE_LPAE_KUNIT_TEST=y
> >    It will automatically run at boot as before.
> > 
> > - Otherwise with CONFIG_IOMMU_IO_PGTABLE_KUNIT_TEST=m:
> >    1) on module load:
> >       Once the module load the self test will run
> >       # modprobe io-pgtable-arm-selftests
> > 
> >    2) debugfs
> >       With CONFIG_KUNIT_DEBUGFS=y You can run the test with
> >       # echo 1 > /sys/kernel/debug/kunit/io-pgtable-arm-test/run
> > 
> >    3) Using kunit.py
> >       You can also use the helper script which uses Qemu in the background
> > 
> >       # ./tools/testing/kunit/kunit.py run --build_dir build_kunit_arm64 --arch arm64 \
> >         --make_options LLVM=1 --kunitconfig ./kunit/kunitconfig
> >        [18:01:09] ============= io-pgtable-arm-test (1 subtest) ==============
> >        [18:01:09] [PASSED] arm_lpae_do_selftests
> >        [18:01:09] =============== [PASSED] io-pgtable-arm-test ===============
> >        [18:01:09] ============================================================
> > 
> > Suggested-by: Jason Gunthorpe <jgg@ziepe.ca>
> > Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
> > Reviewed-by: Pranjal Shrivastava <praan@google.com>
> > Signed-off-by: Mostafa Saleh <smostafa@google.com>
> > ---
> >   drivers/iommu/Kconfig                    | 11 ++--
> >   drivers/iommu/Makefile                   |  2 +-
> >   drivers/iommu/io-pgtable-arm-selftests.c | 82 +++++++++++++-----------
> >   3 files changed, 51 insertions(+), 44 deletions(-)
> > 
> > diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> > index 553522ef3ca9..d50685433347 100644
> > --- a/drivers/iommu/Kconfig
> > +++ b/drivers/iommu/Kconfig
> > @@ -40,12 +40,13 @@ config IOMMU_IO_PGTABLE_LPAE
> >   	  sizes at both stage-1 and stage-2, as well as address spaces
> >   	  up to 48-bits in size.
> > -config IOMMU_IO_PGTABLE_LPAE_SELFTEST
> > -	tristate "LPAE selftests"
> > -	depends on IOMMU_IO_PGTABLE_LPAE
> > +config IOMMU_IO_PGTABLE_LPAE_KUNIT_TEST
> > +	tristate "KUnit tests for LPAE"
> > +	depends on IOMMU_IO_PGTABLE_LPAE && KUNIT
> > +	default KUNIT_ALL_TESTS
> >   	help
> > -	  Enable self-tests for LPAE page table allocator. This performs
> > -	  a series of page-table consistency checks during boot.
> > +	  Enable kunit tests for LPAE page table allocator. This performs
> > +	  a series of page-table consistency checks.
> >   	  If unsure, say N here.
> > diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
> > index 5250a2eea13f..ac3851570303 100644
> > --- a/drivers/iommu/Makefile
> > +++ b/drivers/iommu/Makefile
> > @@ -12,7 +12,7 @@ obj-$(CONFIG_IOMMU_DMA) += dma-iommu.o
> >   obj-$(CONFIG_IOMMU_IO_PGTABLE) += io-pgtable.o
> >   obj-$(CONFIG_IOMMU_IO_PGTABLE_ARMV7S) += io-pgtable-arm-v7s.o
> >   obj-$(CONFIG_IOMMU_IO_PGTABLE_LPAE) += io-pgtable-arm.o
> > -obj-$(CONFIG_IOMMU_IO_PGTABLE_LPAE_SELFTEST) += io-pgtable-arm-selftests.o
> > +obj-$(CONFIG_IOMMU_IO_PGTABLE_LPAE_KUNIT_TEST) += io-pgtable-arm-selftests.o
> >   obj-$(CONFIG_IOMMU_IO_PGTABLE_DART) += io-pgtable-dart.o
> >   obj-$(CONFIG_IOMMU_IOVA) += iova.o
> >   obj-$(CONFIG_OF_IOMMU)	+= of_iommu.o
> > diff --git a/drivers/iommu/io-pgtable-arm-selftests.c b/drivers/iommu/io-pgtable-arm-selftests.c
> > index 32c6a5c7af53..b61849a8a685 100644
> > --- a/drivers/iommu/io-pgtable-arm-selftests.c
> > +++ b/drivers/iommu/io-pgtable-arm-selftests.c
> > @@ -6,7 +6,8 @@
> >    *
> >    * Author: Will Deacon <will.deacon@arm.com>
> >    */
> > -#include <linux/device/faux.h>
> > +#include <kunit/device.h>
> > +#include <kunit/test.h>
> >   #include <linux/io-pgtable.h>
> >   #include <linux/kernel.h>
> > @@ -47,13 +48,14 @@ static void arm_lpae_dump_ops(struct io_pgtable_ops *ops)
> >   		cfg->pgsize_bitmap, cfg->ias, cfg->oas);
> >   }
> > -#define __FAIL(ops, i)	({						\
> > -		WARN(1, "selftest: test failed for fmt idx %d\n", (i));	\
> > -		arm_lpae_dump_ops(ops);					\
> > -		-EFAULT;						\
> > +#define __FAIL(test, ops, i)	({						\
> > +		KUNIT_FAIL(test, "");						\
> > +		kunit_err(test, "selftest: test failed for fmt idx %d\n", (i));	\
> 
> This looks suspect - AFAICS open-coded kunit_err() is intended for test
> infrastucture errors (like the allocation in the next hunk below), while for
> an actual test report message it seems it should just be:
> 
> 	KUNIT_FAIL(test, "selftest: test failed for fmt idx %d\n", (i));

I see, I used kunit_err, as KUNIT_FAIL adds extra information which was
noisy to be printed more than once as:
  # arm_lpae_do_selftests: EXPECTATION FAILED at drivers/iommu/io-pgtable-arm-selftests.c:91
I will check if there is a better way to do this.

> 
> > +		arm_lpae_dump_ops(ops);						\
> > +		-EFAULT;							\
> >   })
> > -static int arm_lpae_run_tests(struct io_pgtable_cfg *cfg)
> > +static int arm_lpae_run_tests(struct kunit *test, struct io_pgtable_cfg *cfg)
> >   {
> >   	static const enum io_pgtable_fmt fmts[] = {
> >   		ARM_64_LPAE_S1,
> > @@ -69,7 +71,7 @@ static int arm_lpae_run_tests(struct io_pgtable_cfg *cfg)
> >   		cfg_cookie = cfg;
> >   		ops = alloc_io_pgtable_ops(fmts[i], cfg, cfg);
> >   		if (!ops) {
> > -			pr_err("selftest: failed to allocate io pgtable ops\n");
> > +			kunit_err(test, "selftest: failed to allocate io pgtable ops\n");
> >   			return -ENOMEM;
> >   		}
> > @@ -78,13 +80,13 @@ static int arm_lpae_run_tests(struct io_pgtable_cfg *cfg)
> >   		 * Empty page tables shouldn't provide any translations.
> >   		 */
> >   		if (ops->iova_to_phys(ops, 42))
> > -			return __FAIL(ops, i);
> > +			return __FAIL(test, ops, i);
> >   		if (ops->iova_to_phys(ops, SZ_1G + 42))
> > -			return __FAIL(ops, i);
> > +			return __FAIL(test, ops, i);
> >   		if (ops->iova_to_phys(ops, SZ_2G + 42))
> > -			return __FAIL(ops, i);
> > +			return __FAIL(test, ops, i);
> >   		/*
> >   		 * Distinct mappings of different granule sizes.
> > @@ -97,16 +99,16 @@ static int arm_lpae_run_tests(struct io_pgtable_cfg *cfg)
> >   					   IOMMU_READ | IOMMU_WRITE |
> >   					   IOMMU_NOEXEC | IOMMU_CACHE,
> >   					   GFP_KERNEL, &mapped))
> > -				return __FAIL(ops, i);
> > +				return __FAIL(test, ops, i);
> >   			/* Overlapping mappings */
> >   			if (!ops->map_pages(ops, iova, iova + size, size, 1,
> >   					    IOMMU_READ | IOMMU_NOEXEC,
> >   					    GFP_KERNEL, &mapped))
> > -				return __FAIL(ops, i);
> > +				return __FAIL(test, ops, i);
> >   			if (ops->iova_to_phys(ops, iova + 42) != (iova + 42))
> > -				return __FAIL(ops, i);
> > +				return __FAIL(test, ops, i);
> >   			iova += SZ_1G;
> >   		}
> > @@ -117,18 +119,18 @@ static int arm_lpae_run_tests(struct io_pgtable_cfg *cfg)
> >   			size = 1UL << j;
> >   			if (ops->unmap_pages(ops, iova, size, 1, NULL) != size)
> > -				return __FAIL(ops, i);
> > +				return __FAIL(test, ops, i);
> >   			if (ops->iova_to_phys(ops, iova + 42))
> > -				return __FAIL(ops, i);
> > +				return __FAIL(test, ops, i);
> >   			/* Remap full block */
> >   			if (ops->map_pages(ops, iova, iova, size, 1,
> >   					   IOMMU_WRITE, GFP_KERNEL, &mapped))
> > -				return __FAIL(ops, i);
> > +				return __FAIL(test, ops, i);
> >   			if (ops->iova_to_phys(ops, iova + 42) != (iova + 42))
> > -				return __FAIL(ops, i);
> > +				return __FAIL(test, ops, i);
> >   			iova += SZ_1G;
> >   		}
> > @@ -144,11 +146,11 @@ static int arm_lpae_run_tests(struct io_pgtable_cfg *cfg)
> >   				   IOMMU_READ | IOMMU_WRITE |
> >   				   IOMMU_NOEXEC | IOMMU_CACHE,
> >   				   GFP_KERNEL, &mapped))
> > -			return __FAIL(ops, i);
> > +			return __FAIL(test, ops, i);
> >   		if (mapped != size)
> > -			return __FAIL(ops, i);
> > +			return __FAIL(test, ops, i);
> >   		if (ops->unmap_pages(ops, iova, size, 1, NULL) != size)
> > -			return __FAIL(ops, i);
> > +			return __FAIL(test, ops, i);
> >   		free_io_pgtable_ops(ops);
> >   	}
> > @@ -156,7 +158,7 @@ static int arm_lpae_run_tests(struct io_pgtable_cfg *cfg)
> >   	return 0;
> >   }
> > -static int arm_lpae_do_selftests(void)
> > +static void arm_lpae_do_selftests(struct kunit *test)
> >   {
> >   	static const unsigned long pgsize[] = {
> >   		SZ_4K | SZ_2M | SZ_1G,
> > @@ -169,18 +171,19 @@ static int arm_lpae_do_selftests(void)
> >   	};
> >   	int i, j, k, pass = 0, fail = 0;
> > -	struct faux_device *dev;
> > +	struct device *dev;
> >   	struct io_pgtable_cfg cfg = {
> >   		.tlb = &dummy_tlb_ops,
> >   		.coherent_walk = true,
> >   		.quirks = IO_PGTABLE_QUIRK_NO_WARN,
> >   	};
> > -	dev = faux_device_create("io-pgtable-test", NULL, 0);
> > -	if (!dev)
> > -		return -ENOMEM;
> > +	dev = kunit_device_register(test, "io-pgtable-test");
> > +	KUNIT_EXPECT_NOT_ERR_OR_NULL(test, dev);
> 
> Conversely, this is infrastructure, not an actual test of expected
> io-pgtable behaviour, so I think just:
> 
> 	cfg.iommu_dev = kunit_device_register(test, "io-pgtable-test");
> 	if (IS_ERR(cfg.iommu_dev))
> 		return;
> 
> (it doesn't return NULLs either)
> 

Yes, I was not sure about this one, when checking the code base, every test
handles kunit_device_register() failure differently, this seemed the
most strict one so I used it, I will update that in the next version.

> Otherwise, there's clearly scope for plenty more follow-up work streamlining
> and breaking the whole thing up into KUnit idioms, pulling the v7s test in,
> etc... However as a first step to at least set things firmly on the right
> KUnit-shaped path, I agree this seems enough for now.
> 

I agree, ultimately every configuration should be a test, which can be
even broken down to smaller cases. I will look in to that next.

Thanks,
Mostafa

> Thanks,
> Robin.
> 
> > +	if (IS_ERR_OR_NULL(dev))
> > +		return;
> > -	cfg.iommu_dev = &dev->dev;
> > +	cfg.iommu_dev = dev;
> >   	for (i = 0; i < ARRAY_SIZE(pgsize); ++i) {
> >   		for (j = 0; j < ARRAY_SIZE(address_size); ++j) {
> > @@ -189,9 +192,9 @@ static int arm_lpae_do_selftests(void)
> >   				cfg.pgsize_bitmap = pgsize[i];
> >   				cfg.ias = address_size[k];
> >   				cfg.oas = address_size[j];
> > -				pr_info("selftest: pgsize_bitmap 0x%08lx, IAS %u OAS %u\n",
> > -					pgsize[i], cfg.ias, cfg.oas);
> > -				if (arm_lpae_run_tests(&cfg))
> > +				kunit_info(test, "selftest: pgsize_bitmap 0x%08lx, IAS %u OAS %u\n",
> > +					   pgsize[i], cfg.ias, cfg.oas);
> > +				if (arm_lpae_run_tests(test, &cfg))
> >   					fail++;
> >   				else
> >   					pass++;
> > @@ -199,17 +202,20 @@ static int arm_lpae_do_selftests(void)
> >   		}
> >   	}
> > -	pr_info("selftest: completed with %d PASS %d FAIL\n", pass, fail);
> > -	faux_device_destroy(dev);
> > -
> > -	return fail ? -EFAULT : 0;
> > +	kunit_info(test, "selftest: completed with %d PASS %d FAIL\n", pass, fail);
> >   }
> > -static void arm_lpae_exit_selftests(void)
> > -{
> > -}
> > +static struct kunit_case io_pgtable_arm_test_cases[] = {
> > +	KUNIT_CASE(arm_lpae_do_selftests),
> > +	{},
> > +};
> > +
> > +static struct kunit_suite io_pgtable_arm_test = {
> > +	.name = "io-pgtable-arm-test",
> > +	.test_cases = io_pgtable_arm_test_cases,
> > +};
> > +
> > +kunit_test_suite(io_pgtable_arm_test);
> > -subsys_initcall(arm_lpae_do_selftests);
> > -module_exit(arm_lpae_exit_selftests);
> >   MODULE_DESCRIPTION("io-pgtable-arm library selftest");
> >   MODULE_LICENSE("GPL");

Re: [PATCH v5 4/4] iommu/io-pgtable-arm-selftests: Use KUnit

[Robin Murphy]

On 2025-10-15 10:53 am, Mostafa Saleh wrote:
[...]
>>> +		KUNIT_FAIL(test, "");						\
>>> +		kunit_err(test, "selftest: test failed for fmt idx %d\n", (i));	\
>>
>> This looks suspect - AFAICS open-coded kunit_err() is intended for test
>> infrastucture errors (like the allocation in the next hunk below), while for
>> an actual test report message it seems it should just be:
>>
>> 	KUNIT_FAIL(test, "selftest: test failed for fmt idx %d\n", (i));
> 
> I see, I used kunit_err, as KUNIT_FAIL adds extra information which was
> noisy to be printed more than once as:
>    # arm_lpae_do_selftests: EXPECTATION FAILED at drivers/iommu/io-pgtable-arm-selftests.c:91
> I will check if there is a better way to do this.

But isn't that exactly what we want? The tests should not fail, but if 
they ever do then inevitably you're going to be asked to debug it from 
nothing more than a dmesg snippet in a forwarded email. Loud and 
detailed is good ;)

If anything we can trim the generic message to just the minimal useful 
information about the format, if KUnit can now provide the rest of the 
attribution for us. Ultimately we should aim to log all the test 
conditions together in one place (and probably only upon a failure, such 
that normal successful runs are quiet), but that can be a job for the 
future follow-up refactoring.

[...]
>>> +	dev = kunit_device_register(test, "io-pgtable-test");
>>> +	KUNIT_EXPECT_NOT_ERR_OR_NULL(test, dev);
>>
>> Conversely, this is infrastructure, not an actual test of expected
>> io-pgtable behaviour, so I think just:
>>
>> 	cfg.iommu_dev = kunit_device_register(test, "io-pgtable-test");
>> 	if (IS_ERR(cfg.iommu_dev))
>> 		return;
>>
>> (it doesn't return NULLs either)
>>
> 
> Yes, I was not sure about this one, when checking the code base, every test
> handles kunit_device_register() failure differently, this seemed the
> most strict one so I used it, I will update that in the next version.

Yeah, my impression is that those have likely been copied from the 
lib/kunit/ code where it is actually testing its own API. I've now sent 
a patch for the example in the docs...

Cheers,
Robin.

Re: [PATCH v5 4/4] iommu/io-pgtable-arm-selftests: Use KUnit

[Jason Gunthorpe]

On Wed, Oct 15, 2025 at 02:51:15PM +0100, Robin Murphy wrote:
> On 2025-10-15 10:53 am, Mostafa Saleh wrote:
> > > Conversely, this is infrastructure, not an actual test of expected
> > > io-pgtable behaviour, so I think just:
> > > 
> > > 	cfg.iommu_dev = kunit_device_register(test, "io-pgtable-test");
> > > 	if (IS_ERR(cfg.iommu_dev))
> > > 		return;
> > > 
> > > (it doesn't return NULLs either)
> > > 
> > 
> > Yes, I was not sure about this one, when checking the code base, every test
> > handles kunit_device_register() failure differently, this seemed the
> > most strict one so I used it, I will update that in the next version.
> 
> Yeah, my impression is that those have likely been copied from the
> lib/kunit/ code where it is actually testing its own API. I've now sent a
> patch for the example in the docs...

I think any failure to run the test should be reported, either with an
err or a skip. Tests that didn't do anything and silently report
success are not a good design.

Looking at the existing users I see alot are in init functions, so
they propogate an error code and fail the init.

And the rest inside tests do something like this:

	dev = kunit_device_register(test, dev_name);			
	KUNIT_ASSERT_FALSE_MSG(test, IS_ERR(dev);
			       "Cannot register test device\n");

Jason

Re: [PATCH v5 4/4] iommu/io-pgtable-arm-selftests: Use KUnit

[Robin Murphy]

On 2025-10-15 4:10 pm, Jason Gunthorpe wrote:
> On Wed, Oct 15, 2025 at 02:51:15PM +0100, Robin Murphy wrote:
>> On 2025-10-15 10:53 am, Mostafa Saleh wrote:
>>>> Conversely, this is infrastructure, not an actual test of expected
>>>> io-pgtable behaviour, so I think just:
>>>>
>>>> 	cfg.iommu_dev = kunit_device_register(test, "io-pgtable-test");
>>>> 	if (IS_ERR(cfg.iommu_dev))
>>>> 		return;
>>>>
>>>> (it doesn't return NULLs either)
>>>>
>>>
>>> Yes, I was not sure about this one, when checking the code base, every test
>>> handles kunit_device_register() failure differently, this seemed the
>>> most strict one so I used it, I will update that in the next version.
>>
>> Yeah, my impression is that those have likely been copied from the
>> lib/kunit/ code where it is actually testing its own API. I've now sent a
>> patch for the example in the docs...
> 
> I think any failure to run the test should be reported, either with an
> err or a skip. Tests that didn't do anything and silently report
> success are not a good design.

Sure, a test that hasn't completed hasn't succeeded, but it hasn't 
failed either, in the sense of there being no fault found in the actual 
code being tested. Hence it seems intuitive to me that reporting a 
failure in the test infrastructure itself as if it were incorrect 
behaviour of the target code is not right.

In this case AFAICS kunit_device_register() can only fail due to OOM or 
something unexpectedly messed up in the kobject/sysfs hierarchy, all of 
which should already scream (and represent the system being sufficiently 
hosed that any actual test results probably no longer matter anyway) - 
otherwise I would have suggested a kunit_err() message too.

> Looking at the existing users I see alot are in init functions, so
> they propogate an error code and fail the init.

Indeed I think this one ultimately wants to end up in an init function 
too once everything is unpicked, but for now...
> And the rest inside tests do something like this:
> 
> 	dev = kunit_device_register(test, dev_name);			
> 	KUNIT_ASSERT_FALSE_MSG(test, IS_ERR(dev);
> 			       "Cannot register test device\n");

The confusion is that most of those KUNIT_ASSERTs are where the KUnit 
API *is* the thing under test, or have clearly copy-pasted the same 
example soon after, and some are in init functions which could fail 
gracefully, so it's far from clear how much considered intent they 
actually represent. In fact AFAICS it's only the two cases of that exact 
pattern in overflow and fortify that hint at being an older notion.

However, on further inspection, I see that there also about as many of 
examples of kunit_skip() being used when failing to allocate per-test 
resources, so I guess that's the pattern I was after, hurrah! I wouldn't 
consider the difference between "test skipped because it unexpectedly 
couldn't run" and "test skipped because it knowingly wouldn't have been 
meaningful or possible to run" significant enough to be worth trying to 
split further.

Thanks,
Robin.

Re: [PATCH v5 4/4] iommu/io-pgtable-arm-selftests: Use KUnit

[Jason Gunthorpe]

On Thu, Oct 16, 2025 at 06:17:35PM +0100, Robin Murphy wrote:

> In this case AFAICS kunit_device_register() can only fail due to OOM or
> something unexpectedly messed up in the kobject/sysfs hierarchy, all of
> which should already scream (and represent the system being sufficiently
> hosed that any actual test results probably no longer matter anyway) -
> otherwise I would have suggested a kunit_err() message too.

Yes, I think so too. Which is why I think the simple
KUNIT_ASSERT_FALSE is fine - we don't need to over think something
that should never happen.

Basically it is a simple logic for the test writer, any thing in the
test body that doesn't work as expected triggers a
KUNIT_ASSERT. Infrastructure included.

At least that is how I've written all my tests so far, including the
userspace ones..

Jason

Re: [PATCH v5 4/4] iommu/io-pgtable-arm-selftests: Use KUnit

[Mostafa Saleh]

On Thu, Oct 16, 2025 at 02:25:24PM -0300, Jason Gunthorpe wrote:
> On Thu, Oct 16, 2025 at 06:17:35PM +0100, Robin Murphy wrote:
> 
> > In this case AFAICS kunit_device_register() can only fail due to OOM or
> > something unexpectedly messed up in the kobject/sysfs hierarchy, all of
> > which should already scream (and represent the system being sufficiently
> > hosed that any actual test results probably no longer matter anyway) -
> > otherwise I would have suggested a kunit_err() message too.
> 
> Yes, I think so too. Which is why I think the simple
> KUNIT_ASSERT_FALSE is fine - we don't need to over think something
> that should never happen.
> 
> Basically it is a simple logic for the test writer, any thing in the
> test body that doesn't work as expected triggers a
> KUNIT_ASSERT. Infrastructure included.
> 
> At least that is how I've written all my tests so far, including the
> userspace ones..

Maybe as Robin suggested, something as:
	if (IS_ERR(dev)) {
		kunit_skip(test, "Failed to allocated device!\n");
		return;
	}

Is simple enough and verbose and can distinguished from test failures,
it will look like:
[    2.095812]     ok 1 arm_lpae_do_selftests # SKIP Failed to allocated device!

Thanks,
Mostafa


> 
> Jason

Re: [PATCH v5 4/4] iommu/io-pgtable-arm-selftests: Use KUnit

[Jason Gunthorpe]

On Fri, Oct 17, 2025 at 01:04:49PM +0000, Mostafa Saleh wrote:

> Is simple enough and verbose and can distinguished from test failures,
> it will look like:
> [    2.095812]     ok 1 arm_lpae_do_selftests # SKIP Failed to allocated device!
                   ^^^^^

The test "passed" though, and since we never expect this failure it
seems wrong to make it pass.

I think there is no point in distinguishing "infrastructure" from
anything else. Either the test runs to completion and does everything,
or it fails.

The use of skip is for things where we probe something and detect we
can't run the test. Like maybe you have a test that relies on
PAGE_SIZE=4096, or CONFIG_XX so skip other systems.

While, "I hit an OOM so I skip the test" seems wrong to me. Maybe the
OOM was caused by the "unit under test" leaking memory??

Jason

Re: [PATCH v5 4/4] iommu/io-pgtable-arm-selftests: Use KUnit

[Mostafa Saleh]

On Fri, Oct 17, 2025 at 11:13:12AM -0300, Jason Gunthorpe wrote:
> On Fri, Oct 17, 2025 at 01:04:49PM +0000, Mostafa Saleh wrote:
> 
> > Is simple enough and verbose and can distinguished from test failures,
> > it will look like:
> > [    2.095812]     ok 1 arm_lpae_do_selftests # SKIP Failed to allocated device!
>                    ^^^^^
> 
> The test "passed" though, and since we never expect this failure it
> seems wrong to make it pass.
> 
> I think there is no point in distinguishing "infrastructure" from
> anything else. Either the test runs to completion and does everything,
> or it fails.
> 
> The use of skip is for things where we probe something and detect we
> can't run the test. Like maybe you have a test that relies on
> PAGE_SIZE=4096, or CONFIG_XX so skip other systems.
> 
> While, "I hit an OOM so I skip the test" seems wrong to me. Maybe the
> OOM was caused by the "unit under test" leaking memory??

I see, both are fine with me, it seems the kunit maintainer is more
inclined twords the failure approach [1].

So, I can keep the test failing, is it OK Robin?

[1] https://lore.kernel.org/all/CABVgOS=NfRcXYzJVMMKqeXP8SyTewffwb7vdGN1D8esO2f0KOA@mail.gmail.com/

Thanks,
Mostafa

> 
> Jason