lkml.org 
[lkml]   [2023]   [Mar]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH 2/8] of: Enable DTB loading on UML for KUnit tests
    On Thu, 2 Mar 2023 at 09:38, Stephen Boyd <sboyd@kernel.org> 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 <richard@nod.at>
    > Cc: Anton Ivanov <anton.ivanov@cambridgegreys.com>
    > Cc: Johannes Berg <johannes@sipsolutions.net>
    > Cc: Vincent Whitchurch <vincent.whitchurch@axis.com>
    > Cc: Rob Herring <robh+dt@kernel.org>
    > Cc: Frank Rowand <frowand.list@gmail.com>
    > Signed-off-by: Stephen Boyd <sboyd@kernel.org>
    > ---
    > 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 <linux/kconfig.h>
    > +#include <linux/of.h>
    > +#include <linux/of_fdt.h>
    > +
    > +#include <kunit/test.h>
    > +
    > +/*
    > + * 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(
    > + &uml_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.
    [unhandled content-type:application/pkcs7-signature]
    \
     
     \ /
      Last update: 2023-03-27 00:42    [W:4.213 / U:0.104 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site