| Message ID | 20240923075704.3567313-1-masahiroy@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v3,1/2] kbuild: move non-boot built-in DTBs to .rodata section | expand |
On Mon, Sep 23, 2024 at 04:56:02PM +0900, Masahiro Yamada wrote: > Commit aab94339cd85 ("of: Add support for linking device tree blobs > into vmlinux") introduced a mechanism to embed DTBs into vmlinux. > > Initially, it was used for wrapping boot DTBs in arch/*/boot/dts/, but > it is now reused for more generic purposes, such as testing. > > Built-in DTBs are discarded because KERNEL_DTB() is part of INIT_DATA, > as defined in include/asm-generic/vmlinux.lds.h. > > This has not been an issue so far because OF unittests are triggered > during boot, as defined by late_initcall(of_unittest). > > However, the recent clk KUnit test additions have caused problems > because KUnit can execute test suites after boot. > > For example: > > # echo > /sys/kernel/debug/kunit/clk_register_clk_parent_data_device/run > > This command triggers a stack trace because built-in DTBs have already > been freed. > > While it is possible to move such test suites from kunit_test_suites to > kunit_test_init_section_suites, it would be preferable to avoid usage > limitations. > > This commit moves non-boot built-in DTBs to the .rodata section. Since > these generic DTBs are looked up by name, they do not need to be placed > in the special .dtb.init.rodata section. > > Boot DTBs should remain in .dtb.init.rodata because the arch boot code > generally does not know the DT name, thus it uses the __dtb_start symbol > to locate it. > > This separation also ensures that the __dtb_start symbol references the > boot DTB. Currently, the .dtb.init.rodata is a mixture of both boot and > non-boot DTBs. The __dtb_start symbol must be followed by the boot DTB, > but we currently rely on the link order (i.e., the order in Makefiles), > which is very fragile. > > Fixes: 5c9dd72d8385 ("of: Add a KUnit test for overlays and test managed APIs") > Fixes: 5776526beb95 ("clk: Add KUnit tests for clk fixed rate basic type") > Fixes: 274aff8711b2 ("clk: Add KUnit tests for clks registered with struct clk_parent_data") > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > --- > > Changes in v3: > - Move to .rodata section instead of .init.rodata > > scripts/Makefile.dtbs | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) Acked-by: Rob Herring (Arm) <robh@kernel.org>
On Mon, Sep 23, 2024 at 4:57 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > Commit aab94339cd85 ("of: Add support for linking device tree blobs > into vmlinux") introduced a mechanism to embed DTBs into vmlinux. > > Initially, it was used for wrapping boot DTBs in arch/*/boot/dts/, but > it is now reused for more generic purposes, such as testing. > > Built-in DTBs are discarded because KERNEL_DTB() is part of INIT_DATA, > as defined in include/asm-generic/vmlinux.lds.h. > > This has not been an issue so far because OF unittests are triggered > during boot, as defined by late_initcall(of_unittest). > > However, the recent clk KUnit test additions have caused problems > because KUnit can execute test suites after boot. > > For example: > > # echo > /sys/kernel/debug/kunit/clk_register_clk_parent_data_device/run > > This command triggers a stack trace because built-in DTBs have already > been freed. > > While it is possible to move such test suites from kunit_test_suites to > kunit_test_init_section_suites, it would be preferable to avoid usage > limitations. > > This commit moves non-boot built-in DTBs to the .rodata section. Since > these generic DTBs are looked up by name, they do not need to be placed > in the special .dtb.init.rodata section. > > Boot DTBs should remain in .dtb.init.rodata because the arch boot code > generally does not know the DT name, thus it uses the __dtb_start symbol > to locate it. > > This separation also ensures that the __dtb_start symbol references the > boot DTB. Currently, the .dtb.init.rodata is a mixture of both boot and > non-boot DTBs. The __dtb_start symbol must be followed by the boot DTB, > but we currently rely on the link order (i.e., the order in Makefiles), > which is very fragile. > > Fixes: 5c9dd72d8385 ("of: Add a KUnit test for overlays and test managed APIs") > Fixes: 5776526beb95 ("clk: Add KUnit tests for clk fixed rate basic type") > Fixes: 274aff8711b2 ("clk: Add KUnit tests for clks registered with struct clk_parent_data") > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > --- > > Changes in v3: > - Move to .rodata section instead of .init.rodata > > scripts/Makefile.dtbs | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/scripts/Makefile.dtbs b/scripts/Makefile.dtbs > index 46009d5f1486..3e81cca6f68a 100644 > --- a/scripts/Makefile.dtbs > +++ b/scripts/Makefile.dtbs > @@ -34,12 +34,14 @@ $(obj)/dtbs-list: $(dtb-y) FORCE > # Assembly file to wrap dtb(o) > # --------------------------------------------------------------------------- > > +builtin-dtb-section = $(if $(filter arch/%, $(obj)),.dtb.init.rodata,.rodata) For more precise matching, I will change as follows: diff --git a/scripts/Makefile.dtbs b/scripts/Makefile.dtbs index 3e81cca6f68a..8d56c0815f33 100644 --- a/scripts/Makefile.dtbs +++ b/scripts/Makefile.dtbs @@ -34,7 +34,7 @@ $(obj)/dtbs-list: $(dtb-y) FORCE # Assembly file to wrap dtb(o) # --------------------------------------------------------------------------- -builtin-dtb-section = $(if $(filter arch/%, $(obj)),.dtb.init.rodata,.rodata) +builtin-dtb-section = $(if $(filter arch/$(SRCARCH)/boot/dts%, $(obj)),.dtb.init.rodata,.rodata) # Generate an assembly file to wrap the output of the device tree compiler quiet_cmd_wrap_S_dtb = WRAP $@ Applied to linux-kbuild/fixes. > + > # Generate an assembly file to wrap the output of the device tree compiler > quiet_cmd_wrap_S_dtb = WRAP $@ > cmd_wrap_S_dtb = { \ > symbase=__$(patsubst .%,%,$(suffix $<))_$(subst -,_,$(notdir $*)); \ > echo '\#include <asm-generic/vmlinux.lds.h>'; \ > - echo '.section .dtb.init.rodata,"a"'; \ > + echo '.section $(builtin-dtb-section),"a"'; \ > echo '.balign STRUCT_ALIGNMENT'; \ > echo ".global $${symbase}_begin"; \ > echo "$${symbase}_begin:"; \ > -- > 2.43.0 >
diff --git a/scripts/Makefile.dtbs b/scripts/Makefile.dtbs index 46009d5f1486..3e81cca6f68a 100644 --- a/scripts/Makefile.dtbs +++ b/scripts/Makefile.dtbs @@ -34,12 +34,14 @@ $(obj)/dtbs-list: $(dtb-y) FORCE # Assembly file to wrap dtb(o) # --------------------------------------------------------------------------- +builtin-dtb-section = $(if $(filter arch/%, $(obj)),.dtb.init.rodata,.rodata) + # Generate an assembly file to wrap the output of the device tree compiler quiet_cmd_wrap_S_dtb = WRAP $@ cmd_wrap_S_dtb = { \ symbase=__$(patsubst .%,%,$(suffix $<))_$(subst -,_,$(notdir $*)); \ echo '\#include <asm-generic/vmlinux.lds.h>'; \ - echo '.section .dtb.init.rodata,"a"'; \ + echo '.section $(builtin-dtb-section),"a"'; \ echo '.balign STRUCT_ALIGNMENT'; \ echo ".global $${symbase}_begin"; \ echo "$${symbase}_begin:"; \
Commit aab94339cd85 ("of: Add support for linking device tree blobs into vmlinux") introduced a mechanism to embed DTBs into vmlinux. Initially, it was used for wrapping boot DTBs in arch/*/boot/dts/, but it is now reused for more generic purposes, such as testing. Built-in DTBs are discarded because KERNEL_DTB() is part of INIT_DATA, as defined in include/asm-generic/vmlinux.lds.h. This has not been an issue so far because OF unittests are triggered during boot, as defined by late_initcall(of_unittest). However, the recent clk KUnit test additions have caused problems because KUnit can execute test suites after boot. For example: # echo > /sys/kernel/debug/kunit/clk_register_clk_parent_data_device/run This command triggers a stack trace because built-in DTBs have already been freed. While it is possible to move such test suites from kunit_test_suites to kunit_test_init_section_suites, it would be preferable to avoid usage limitations. This commit moves non-boot built-in DTBs to the .rodata section. Since these generic DTBs are looked up by name, they do not need to be placed in the special .dtb.init.rodata section. Boot DTBs should remain in .dtb.init.rodata because the arch boot code generally does not know the DT name, thus it uses the __dtb_start symbol to locate it. This separation also ensures that the __dtb_start symbol references the boot DTB. Currently, the .dtb.init.rodata is a mixture of both boot and non-boot DTBs. The __dtb_start symbol must be followed by the boot DTB, but we currently rely on the link order (i.e., the order in Makefiles), which is very fragile. Fixes: 5c9dd72d8385 ("of: Add a KUnit test for overlays and test managed APIs") Fixes: 5776526beb95 ("clk: Add KUnit tests for clk fixed rate basic type") Fixes: 274aff8711b2 ("clk: Add KUnit tests for clks registered with struct clk_parent_data") Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> --- Changes in v3: - Move to .rodata section instead of .init.rodata scripts/Makefile.dtbs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)