| Message ID | 20240112200750.4062441-7-sboyd@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | of: populate of_root node if bootloader doesn't | expand |
On Sat, 13 Jan 2024 at 04:07, Stephen Boyd <sboyd@kernel.org> wrote: > > Add a KUnit test that confirms a DTB has been loaded, i.e. there is a > root node, and that the of_have_populated_dt() API works properly. > > Cc: Rob Herring <robh+dt@kernel.org> > Cc: Frank Rowand <frowand.list@gmail.com> > Cc: David Gow <davidgow@google.com> > Cc: Brendan Higgins <brendan.higgins@linux.dev> > Signed-off-by: Stephen Boyd <sboyd@kernel.org> > --- I won't pretend to be a devicetree expert, but this looks good to me from a KUnit point of view, and passes comfortably here. checkpatch seems to have one complaint about the kconfig help text. Personally, I think the brief description is fine. Reviewed-by: David Gow <davidgow@google.com> Cheers, -- David > drivers/of/.kunitconfig | 3 ++ > drivers/of/Kconfig | 9 ++++ > drivers/of/Makefile | 2 + > drivers/of/of_test.c | 98 +++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 112 insertions(+) > create mode 100644 drivers/of/.kunitconfig > create mode 100644 drivers/of/of_test.c > > diff --git a/drivers/of/.kunitconfig b/drivers/of/.kunitconfig > new file mode 100644 > index 000000000000..5a8fee11978c > --- /dev/null > +++ b/drivers/of/.kunitconfig > @@ -0,0 +1,3 @@ > +CONFIG_KUNIT=y > +CONFIG_OF=y > +CONFIG_OF_KUNIT_TEST=y > diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig > index 9628e48baa15..a527ba8451d9 100644 > --- a/drivers/of/Kconfig > +++ b/drivers/of/Kconfig > @@ -37,6 +37,15 @@ config OF_UNITTEST > > If unsure, say N here. This option is not safe to enable. > > +config OF_KUNIT_TEST > + tristate "Devicetree KUnit DTB Test" if !KUNIT_ALL_TESTS > + depends on KUNIT > + default KUNIT_ALL_TESTS > + help > + This option builds KUnit unit tests for device tree infrastructure. > + > + If unsure, say N here, but this option is safe to enable. > + FYI: WARNING: please write a help paragraph that fully describes the config symbol #111: FILE: drivers/of/Kconfig:40: > 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 df305348d1cb..251d33532148 100644 > --- a/drivers/of/Makefile > +++ b/drivers/of/Makefile > @@ -19,4 +19,6 @@ obj-y += kexec.o > endif > endif > > +obj-$(CONFIG_OF_KUNIT_TEST) += of_test.o > + > obj-$(CONFIG_OF_UNITTEST) += unittest-data/ > diff --git a/drivers/of/of_test.c b/drivers/of/of_test.c > new file mode 100644 > index 000000000000..7609ad3081b9 > --- /dev/null > +++ b/drivers/of/of_test.c > @@ -0,0 +1,98 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * KUnit tests for OF APIs > + */ > +#include <linux/module.h> > +#include <linux/of.h> > + > +#include <kunit/test.h> > + > +/* > + * Test that the root node "/" exists. > + */ > +static void dtb_root_node_found_by_path(struct kunit *test) > +{ > + struct device_node *np; > + > + np = of_find_node_by_path("/"); > + KUNIT_EXPECT_NOT_ERR_OR_NULL(test, np); > + of_node_put(np); > +} > + > +/* > + * Test that the of_root global variable is always populated when DT > + * code is enabled. > + */ > +static void dtb_root_node_populates_of_root(struct kunit *test) > +{ > + KUNIT_EXPECT_NOT_ERR_OR_NULL(test, of_root); > +} > + > +static struct kunit_case dtb_test_cases[] = { > + KUNIT_CASE(dtb_root_node_found_by_path), > + KUNIT_CASE(dtb_root_node_populates_of_root), > + {} > +}; > + > +/* > + * Test suite to confirm a live DTB is loaded. > + */ > +static struct kunit_suite dtb_suite = { > + .name = "dtb", > + .test_cases = dtb_test_cases, > +}; > + > +/* > + * Test that calling of_have_populated_dt() returns false when > + * the OF_EMPTY_ROOT flag isn't set. > + */ > +static void of_have_populated_dt_false_when_flag_set(struct kunit *test) > +{ > + bool was_set; > + > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, of_root); > + > + was_set = of_node_test_and_set_flag(of_root, OF_EMPTY_ROOT); > + KUNIT_EXPECT_FALSE(test, of_have_populated_dt()); > + > + if (!was_set) > + of_node_clear_flag(of_root, OF_EMPTY_ROOT); > +} > + > +/* > + * Test that calling of_have_populated_dt() returns false when > + * the OF_EMPTY_ROOT flag isn't set. > + */ > +static void of_have_populated_dt_true_when_flag_clear(struct kunit *test) > +{ > + bool was_set; > + > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, of_root); > + > + was_set = of_node_check_flag(of_root, OF_EMPTY_ROOT); > + of_node_set_flag(of_root, OF_EMPTY_ROOT); > + KUNIT_EXPECT_FALSE(test, of_have_populated_dt()); > + > + if (was_set) > + of_node_set_flag(of_root, OF_EMPTY_ROOT); > +} > + > +static struct kunit_case of_have_populated_dt_test_cases[] = { > + KUNIT_CASE(of_have_populated_dt_false_when_flag_set), > + KUNIT_CASE(of_have_populated_dt_true_when_flag_clear), > + {} > +}; > + > +/* > + * Test suite to confirm behavior of of_have_populated_dt(). > + */ > +static struct kunit_suite of_have_populated_dt_suite = { > + .name = "of_have_populated_dt", > + .test_cases = of_have_populated_dt_test_cases, > +}; > + > +kunit_test_suites( > + &dtb_suite, > + &of_have_populated_dt_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 >
Quoting David Gow (2024-01-15 21:03:12) > On Sat, 13 Jan 2024 at 04:07, Stephen Boyd <sboyd@kernel.org> wrote: > > > > Add a KUnit test that confirms a DTB has been loaded, i.e. there is a > > root node, and that the of_have_populated_dt() API works properly. > > > > Cc: Rob Herring <robh+dt@kernel.org> > > Cc: Frank Rowand <frowand.list@gmail.com> > > Cc: David Gow <davidgow@google.com> > > Cc: Brendan Higgins <brendan.higgins@linux.dev> > > Signed-off-by: Stephen Boyd <sboyd@kernel.org> > > --- > > I won't pretend to be a devicetree expert, but this looks good to me > from a KUnit point of view, and passes comfortably here. > > checkpatch seems to have one complaint about the kconfig help text. > Personally, I think the brief description is fine. > > Reviewed-by: David Gow <davidgow@google.com> > Thanks! I noticed that x86 has some devicetree init code. Did you happen to try on an x86 kvm instance? Or only run on UML? ----8<---- diff --git a/arch/x86/kernel/devicetree.c b/arch/x86/kernel/devicetree.c index afd09924094e..650752d112a6 100644 --- a/arch/x86/kernel/devicetree.c +++ b/arch/x86/kernel/devicetree.c @@ -283,22 +283,24 @@ void __init x86_flattree_get_config(void) u32 size, map_len; void *dt; - if (!initial_dtb) - return; + if (initial_dtb) { + map_len = max(PAGE_SIZE - (initial_dtb & ~PAGE_MASK), (u64)128); - map_len = max(PAGE_SIZE - (initial_dtb & ~PAGE_MASK), (u64)128); + dt = early_memremap(initial_dtb, map_len); + size = fdt_totalsize(dt); + if (map_len < size) { + early_memunmap(dt, map_len); + dt = early_memremap(initial_dtb, size); + map_len = size; + } - dt = early_memremap(initial_dtb, map_len); - size = fdt_totalsize(dt); - if (map_len < size) { - early_memunmap(dt, map_len); - dt = early_memremap(initial_dtb, size); - map_len = size; + early_init_dt_verify(dt); } - early_init_dt_verify(dt); unflatten_and_copy_device_tree(); - early_memunmap(dt, map_len); + + if (initial_dtb) + early_memunmap(dt, map_len); } #endif
On Tue, 23 Jan 2024 at 06:48, Stephen Boyd <sboyd@kernel.org> wrote: > > Quoting David Gow (2024-01-15 21:03:12) > > On Sat, 13 Jan 2024 at 04:07, Stephen Boyd <sboyd@kernel.org> wrote: > > > > > > Add a KUnit test that confirms a DTB has been loaded, i.e. there is a > > > root node, and that the of_have_populated_dt() API works properly. > > > > > > Cc: Rob Herring <robh+dt@kernel.org> > > > Cc: Frank Rowand <frowand.list@gmail.com> > > > Cc: David Gow <davidgow@google.com> > > > Cc: Brendan Higgins <brendan.higgins@linux.dev> > > > Signed-off-by: Stephen Boyd <sboyd@kernel.org> > > > --- > > > > I won't pretend to be a devicetree expert, but this looks good to me > > from a KUnit point of view, and passes comfortably here. > > > > checkpatch seems to have one complaint about the kconfig help text. > > Personally, I think the brief description is fine. > > > > Reviewed-by: David Gow <davidgow@google.com> > > > > Thanks! I noticed that x86 has some devicetree init code. Did you happen > to try on an x86 kvm instance? Or only run on UML? > I admit, I only tested on UML, plus a quick check that the default kunitconfig wasn't broken on anything else. A further look is showing: aarch64: PASSED UML: PASSED UML LLVM: PASSED powerpc64: PASSED m68k: FAILED i386: FAILED x86_64: FAILED x86_64 KASAN: FAILED The failures are all of the form: > [15:18:34] ============ of_have_populated_dt (2 subtests) ============= > [15:18:34] # of_have_populated_dt_false_when_flag_set: ASSERTION FAILED at drivers/of/of_test.c:53 > [15:18:34] Expected of_root is not null, but is > [15:18:34] [FAILED] of_have_populated_dt_false_when_flag_set > [15:18:34] # of_have_populated_dt_true_when_flag_clear: ASSERTION FAILED at drivers/of/of_test.c:70 > [15:18:34] Expected of_root is not null, but is > [15:18:34] [FAILED] of_have_populated_dt_true_when_flag_clear > [15:18:34] # module: of_test > [15:18:34] # of_have_populated_dt: pass:0 fail:2 skip:0 total:2 > [15:18:34] # Totals: pass:0 fail:2 skip:0 total:2 > [15:18:34] ============== [FAILED] of_have_populated_dt =============== -- David > ----8<---- > diff --git a/arch/x86/kernel/devicetree.c b/arch/x86/kernel/devicetree.c > index afd09924094e..650752d112a6 100644 > --- a/arch/x86/kernel/devicetree.c > +++ b/arch/x86/kernel/devicetree.c > @@ -283,22 +283,24 @@ void __init x86_flattree_get_config(void) > u32 size, map_len; > void *dt; > > - if (!initial_dtb) > - return; > + if (initial_dtb) { > + map_len = max(PAGE_SIZE - (initial_dtb & ~PAGE_MASK), (u64)128); > > - map_len = max(PAGE_SIZE - (initial_dtb & ~PAGE_MASK), (u64)128); > + dt = early_memremap(initial_dtb, map_len); > + size = fdt_totalsize(dt); > + if (map_len < size) { > + early_memunmap(dt, map_len); > + dt = early_memremap(initial_dtb, size); > + map_len = size; > + } > > - dt = early_memremap(initial_dtb, map_len); > - size = fdt_totalsize(dt); > - if (map_len < size) { > - early_memunmap(dt, map_len); > - dt = early_memremap(initial_dtb, size); > - map_len = size; > + early_init_dt_verify(dt); > } > > - early_init_dt_verify(dt); > unflatten_and_copy_device_tree(); > - early_memunmap(dt, map_len); > + > + if (initial_dtb) > + early_memunmap(dt, map_len); > } > #endif
diff --git a/drivers/of/.kunitconfig b/drivers/of/.kunitconfig new file mode 100644 index 000000000000..5a8fee11978c --- /dev/null +++ b/drivers/of/.kunitconfig @@ -0,0 +1,3 @@ +CONFIG_KUNIT=y +CONFIG_OF=y +CONFIG_OF_KUNIT_TEST=y diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig index 9628e48baa15..a527ba8451d9 100644 --- a/drivers/of/Kconfig +++ b/drivers/of/Kconfig @@ -37,6 +37,15 @@ config OF_UNITTEST If unsure, say N here. This option is not safe to enable. +config OF_KUNIT_TEST + tristate "Devicetree KUnit DTB Test" if !KUNIT_ALL_TESTS + depends on KUNIT + default KUNIT_ALL_TESTS + help + This option builds KUnit unit tests for device tree infrastructure. + + 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 df305348d1cb..251d33532148 100644 --- a/drivers/of/Makefile +++ b/drivers/of/Makefile @@ -19,4 +19,6 @@ obj-y += kexec.o endif endif +obj-$(CONFIG_OF_KUNIT_TEST) += of_test.o + obj-$(CONFIG_OF_UNITTEST) += unittest-data/ diff --git a/drivers/of/of_test.c b/drivers/of/of_test.c new file mode 100644 index 000000000000..7609ad3081b9 --- /dev/null +++ b/drivers/of/of_test.c @@ -0,0 +1,98 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * KUnit tests for OF APIs + */ +#include <linux/module.h> +#include <linux/of.h> + +#include <kunit/test.h> + +/* + * Test that the root node "/" exists. + */ +static void dtb_root_node_found_by_path(struct kunit *test) +{ + struct device_node *np; + + np = of_find_node_by_path("/"); + KUNIT_EXPECT_NOT_ERR_OR_NULL(test, np); + of_node_put(np); +} + +/* + * Test that the of_root global variable is always populated when DT + * code is enabled. + */ +static void dtb_root_node_populates_of_root(struct kunit *test) +{ + KUNIT_EXPECT_NOT_ERR_OR_NULL(test, of_root); +} + +static struct kunit_case dtb_test_cases[] = { + KUNIT_CASE(dtb_root_node_found_by_path), + KUNIT_CASE(dtb_root_node_populates_of_root), + {} +}; + +/* + * Test suite to confirm a live DTB is loaded. + */ +static struct kunit_suite dtb_suite = { + .name = "dtb", + .test_cases = dtb_test_cases, +}; + +/* + * Test that calling of_have_populated_dt() returns false when + * the OF_EMPTY_ROOT flag isn't set. + */ +static void of_have_populated_dt_false_when_flag_set(struct kunit *test) +{ + bool was_set; + + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, of_root); + + was_set = of_node_test_and_set_flag(of_root, OF_EMPTY_ROOT); + KUNIT_EXPECT_FALSE(test, of_have_populated_dt()); + + if (!was_set) + of_node_clear_flag(of_root, OF_EMPTY_ROOT); +} + +/* + * Test that calling of_have_populated_dt() returns false when + * the OF_EMPTY_ROOT flag isn't set. + */ +static void of_have_populated_dt_true_when_flag_clear(struct kunit *test) +{ + bool was_set; + + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, of_root); + + was_set = of_node_check_flag(of_root, OF_EMPTY_ROOT); + of_node_set_flag(of_root, OF_EMPTY_ROOT); + KUNIT_EXPECT_FALSE(test, of_have_populated_dt()); + + if (was_set) + of_node_set_flag(of_root, OF_EMPTY_ROOT); +} + +static struct kunit_case of_have_populated_dt_test_cases[] = { + KUNIT_CASE(of_have_populated_dt_false_when_flag_set), + KUNIT_CASE(of_have_populated_dt_true_when_flag_clear), + {} +}; + +/* + * Test suite to confirm behavior of of_have_populated_dt(). + */ +static struct kunit_suite of_have_populated_dt_suite = { + .name = "of_have_populated_dt", + .test_cases = of_have_populated_dt_test_cases, +}; + +kunit_test_suites( + &dtb_suite, + &of_have_populated_dt_suite, +); +MODULE_LICENSE("GPL");
Add a KUnit test that confirms a DTB has been loaded, i.e. there is a root node, and that the of_have_populated_dt() API works properly. Cc: Rob Herring <robh+dt@kernel.org> Cc: Frank Rowand <frowand.list@gmail.com> Cc: David Gow <davidgow@google.com> Cc: Brendan Higgins <brendan.higgins@linux.dev> Signed-off-by: Stephen Boyd <sboyd@kernel.org> --- drivers/of/.kunitconfig | 3 ++ drivers/of/Kconfig | 9 ++++ drivers/of/Makefile | 2 + drivers/of/of_test.c | 98 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 112 insertions(+) create mode 100644 drivers/of/.kunitconfig create mode 100644 drivers/of/of_test.c