On Thu, 2 Mar 2023 at 09:38, Stephen Boyd wrote: > > To fully exercise common clk framework code in KUnit we need to > associate 'struct device' pointers with 'struct device_node' pointers so > that things like clk_get() can parse DT nodes for 'clocks' and so that > clk providers can use DT to provide clks; the most common mode of > operation for clk providers. > > Adding support to KUnit so that it loads a DTB is fairly simple after > commit b31297f04e86 ("um: Add devicetree support"). We can simply pass a > pre-compiled deviectree blob (DTB) on the kunit.py commandline and UML > will load it. The problem is that tests won't know that the commandline > has been modified, nor that a DTB has been loaded. Take a different > approach so that tests can skip if a DTB hasn't been loaded. > > Reuse the Makefile logic from the OF unittests to build a DTB into the > kernel. This DTB will be for the mythical machine "linux,kunit", i.e. > the devicetree for the KUnit "board". In practice, it is a dtsi file > that will gather includes for kunit tests that rely in part on a > devicetree being loaded. The devicetree should only be loaded if > CONFIG_OF_KUNIT=y. Make that a choice config parallel to the existing > CONFIG_OF_UNITTEST so that only one devicetree can be loaded in the > system at a time. Similarly, the kernel commandline option to load a > DTB is ignored if CONFIG_OF_KUNIT is enabled so that only one DTB is > loaded at a time. This feels a little bit like it's just papering over the real problem, which is that there's no way tests can skip themselves if no DTB is loaded. That being said, I do think that there's probably some sense in supporting the compiled-in DTB as well (it's definitely simpler than patching kunit.py to always pass the extra command-line option in, for example). But maybe it'd be nice to have the command-line option override the built-in one if present. > > Add a simple unit test to confirm that the DTB loading worked. Future > tests will add to the kunit.dtsi file to include their specific test > nodes. > > Cc: Richard Weinberger > Cc: Anton Ivanov > Cc: Johannes Berg > Cc: Vincent Whitchurch > Cc: Rob Herring > Cc: Frank Rowand > Signed-off-by: Stephen Boyd > --- > arch/um/kernel/dtb.c | 29 +++++++++++++++-- > drivers/of/Kconfig | 26 ++++++++++++++++ > drivers/of/Makefile | 1 + > drivers/of/kunit/.kunitconfig | 4 +++ > drivers/of/kunit/Makefile | 4 +++ > drivers/of/kunit/kunit.dtsi | 8 +++++ > drivers/of/kunit/kunit.dtso | 4 +++ > drivers/of/kunit/uml_dtb_test.c | 55 +++++++++++++++++++++++++++++++++ > 8 files changed, 128 insertions(+), 3 deletions(-) > create mode 100644 drivers/of/kunit/.kunitconfig > create mode 100644 drivers/of/kunit/Makefile > create mode 100644 drivers/of/kunit/kunit.dtsi > create mode 100644 drivers/of/kunit/kunit.dtso > create mode 100644 drivers/of/kunit/uml_dtb_test.c > > diff --git a/arch/um/kernel/dtb.c b/arch/um/kernel/dtb.c > index 484141b06938..ee63951b12df 100644 > --- a/arch/um/kernel/dtb.c > +++ b/arch/um/kernel/dtb.c > @@ -15,9 +15,32 @@ void uml_dtb_init(void) > long long size; > void *area; > > - area = uml_load_file(dtb, &size); > - if (!area) > - return; > + if (IS_ENABLED(CONFIG_OF_KUNIT)) { > + /* > + * __dtbo_kunit_begin[] and __dtbo_kunit_end[] are magically > + * created by cmd_dt_S_dtbo in scripts/Makefile.lib from the > + * drivers/of/kunit/kunit.dtsi file. > + */ > + extern uint8_t __dtbo_kunit_begin[]; > + extern uint8_t __dtbo_kunit_end[]; > + > + size = __dtbo_kunit_end - __dtbo_kunit_begin; > + if (!size) { > + pr_warn("%s: kunit testcases is empty\n", __func__); > + return; > + } > + > + /* creating copy */ > + area = memblock_alloc(size, 8); > + if (!area) > + return; > + > + memcpy(area, __dtbo_kunit_begin, size); > + } else { I think this should probably override the KUnit dtb if present (so, try to load the dtb, and fallback to the builtin one). If not, I think we should at least print a warning if a DTB is specified on the command-line, but we're not using it. > + area = uml_load_file(dtb, &size); > + if (!area) > + return; > + } > > if (!early_init_dt_scan(area)) { > pr_err("invalid DTB %s\n", dtb); > diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig > index 80b5fd44ab1c..1f968b6a3dde 100644 > --- a/drivers/of/Kconfig > +++ b/drivers/of/Kconfig > @@ -12,6 +12,20 @@ menuconfig OF > > if OF > > +choice > + prompt "Devicetree Runtime Tests" > + default OF_UNITTEST > + > +config OF_KUNIT > + bool "Devicetree KUnit support" if KUNIT > + depends on UML > + select IRQ_DOMAIN > + select OF_EARLY_FLATTREE > + help > + This option builds in KUnit test cases that rely on device tree infrastructure. > + A fake Device Tree Blob (DTB) is loaded on the UML kernel running KUnit so that > + KUnit tests can test device tree dependent code. > + > config OF_UNITTEST > bool "Device Tree runtime unit tests" > depends on !SPARC > @@ -25,6 +39,18 @@ config OF_UNITTEST > > If unsure, say N here, but this option is safe to enable. > > +endchoice > + > +config OF_DTB_KUNIT_TEST > + tristate "Devicetree KUnit DTB Test" if !KUNIT_ALL_TESTS > + depends on KUNIT > + default KUNIT_ALL_TESTS > + help > + This option builds unit tests for the "linux,kunit" DTB built into > + the UML kernel image. > + > + If unsure, say N here, but this option is safe to enable. > + > config OF_ALL_DTBS > bool "Build all Device Tree Blobs" > depends on COMPILE_TEST > diff --git a/drivers/of/Makefile b/drivers/of/Makefile > index e0360a44306e..16eef3fdf60a 100644 > --- a/drivers/of/Makefile > +++ b/drivers/of/Makefile > @@ -19,4 +19,5 @@ obj-y += kexec.o > endif > endif > > +obj-y += kunit/ > obj-$(CONFIG_OF_UNITTEST) += unittest-data/ > diff --git a/drivers/of/kunit/.kunitconfig b/drivers/of/kunit/.kunitconfig > new file mode 100644 > index 000000000000..1def0ad30d29 > --- /dev/null > +++ b/drivers/of/kunit/.kunitconfig > @@ -0,0 +1,4 @@ > +CONFIG_KUNIT=y > +CONFIG_OF=y > +CONFIG_OF_KUNIT=y > +CONFIG_OF_DTB_KUNIT_TEST=y > diff --git a/drivers/of/kunit/Makefile b/drivers/of/kunit/Makefile > new file mode 100644 > index 000000000000..ffe0447e1ac7 > --- /dev/null > +++ b/drivers/of/kunit/Makefile > @@ -0,0 +1,4 @@ > +# SPDX-License-Identifier: GPL-2.0 > +obj-$(CONFIG_OF_KUNIT) += kunit.dtbo.o > + > +obj-$(CONFIG_OF_DTB_KUNIT_TEST) += uml_dtb_test.o > diff --git a/drivers/of/kunit/kunit.dtsi b/drivers/of/kunit/kunit.dtsi > new file mode 100644 > index 000000000000..82f6c3e2b8d5 > --- /dev/null > +++ b/drivers/of/kunit/kunit.dtsi > @@ -0,0 +1,8 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +/ { > + model = "KUnit UML"; > + compatible = "linux,kunit"; > +}; > + > +/* Include testcase dtsi files below */ > diff --git a/drivers/of/kunit/kunit.dtso b/drivers/of/kunit/kunit.dtso > new file mode 100644 > index 000000000000..50187e8d1422 > --- /dev/null > +++ b/drivers/of/kunit/kunit.dtso > @@ -0,0 +1,4 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/dts-v1/; > + > +#include "kunit.dtsi" > diff --git a/drivers/of/kunit/uml_dtb_test.c b/drivers/of/kunit/uml_dtb_test.c > new file mode 100644 > index 000000000000..8966c9ebf51f > --- /dev/null > +++ b/drivers/of/kunit/uml_dtb_test.c > @@ -0,0 +1,55 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * KUnit tests for DTB loading on UML > + */ > +#include > +#include > +#include > + > +#include > + > +/* > + * Test that of_machine_is_compatible() returns positive int when loaded DTB > + * matches. > + */ > +static void uml_dtb_of_machine_compatible_test(struct kunit *test) > +{ > + KUNIT_EXPECT_GT(test, of_machine_is_compatible("linux,kunit"), 0); > +} > + > +/* > + * Test that of_flat_dt_get_machine_name() returns the expected 'model' from the > + * loaded DTB. > + */ > +static void uml_dtb_of_flat_dt_get_machine_name_test(struct kunit *test) > +{ > + KUNIT_EXPECT_STREQ(test, of_flat_dt_get_machine_name(), "KUnit UML"); > +} > + > +static struct kunit_case uml_dtb_test_cases[] = { > + KUNIT_CASE(uml_dtb_of_machine_compatible_test), > + KUNIT_CASE(uml_dtb_of_flat_dt_get_machine_name_test), > + {} > +}; > + > +static int uml_dtb_test_init(struct kunit *test) > +{ > + if (!IS_ENABLED(CONFIG_OF_KUNIT)) > + kunit_skip(test, "requires CONFIG_OF_KUNIT"); > + > + return 0; > +} > + > +/* > + * Test suite to confirm DTB is loaded on UML. > + */ > +static struct kunit_suite uml_dtb_suite = { > + .name = "uml_dtb", > + .init = uml_dtb_test_init, > + .test_cases = uml_dtb_test_cases, > +}; > + > +kunit_test_suites( > + ¨_dtb_suite, > +); > +MODULE_LICENSE("GPL"); > -- > https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git/ > https://git.kernel.org/pub/scm/linux/kernel/git/sboyd/spmi.git > > -- > You received this message because you are subscribed to the Google Groups "KUnit Development" group. > To unsubscribe from this group and stop receiving emails from it, send an email to kunit-dev+unsubscribe@googlegroups.com. > To view this discussion on the web visit https://groups.google.com/d/msgid/kunit-dev/20230302013822.1808711-3-sboyd%40kernel.org.