* [PATCH v3 2/2] kbuild: add generic support for built-in boot DTBs
2024-09-23 7:56 [PATCH v3 1/2] kbuild: move non-boot built-in DTBs to .rodata section Masahiro Yamada
@ 2024-09-23 7:56 ` Masahiro Yamada
2024-09-24 22:15 ` [PATCH v3 1/2] kbuild: move non-boot built-in DTBs to .rodata section Rob Herring
2024-09-27 14:51 ` Masahiro Yamada
2 siblings, 0 replies; 4+ messages in thread
From: Masahiro Yamada @ 2024-09-23 7:56 UTC (permalink / raw)
To: linux-kbuild
Cc: linux-kernel, Andrea della Porta, linux-clk, Stephen Boyd,
Brendan Higgins, Rob Herring, devicetree, Masahiro Yamada,
Alex Gaynor, Alice Ryhl, Andreas Hindborg, Benno Lossin,
Björn Roy Baron, Boqun Feng, Gary Guo, Miguel Ojeda,
Nathan Chancellor, Nicolas Schier, Saravana Kannan,
Wedson Almeida Filho, rust-for-linux
Some architectures embed boot DTBs in vmlinux. A potential issue for
these architectures is a race condition during parallel builds because
Kbuild descends into arch/*/boot/dts/ twice.
One build thread is initiated by the 'dtbs' target, which is a
prerequisite of the 'all' target in the top-level Makefile:
ifdef CONFIG_OF_EARLY_FLATTREE
all: dtbs
endif
For architectures that support the built-in boot dtb, arch/*/boot/dts/
is visited also during the ordinary directory traversal in order to
build obj-y objects that wrap DTBs.
Since these build threads are unaware of each other, they can run
simultaneously during parallel builds.
This commit introduces a generic build rule to scripts/Makefile.vmlinux
to support embedded boot DTBs in a race-free way. Architectures that
want to use this rule need to select CONFIG_GENERIC_BUILTIN_DTB.
After the migration, Makefiles under arch/*/boot/dts/ will be visited
only once to build only *.dtb files.
This change also aims to unify the CONFIG options used for built-in DTBs
support. Currently, different architectures use different CONFIG options
for the same purposes.
With this commit, the CONFIG options will be unified as follows:
- CONFIG_GENERIC_BUILTIN_DTB
This enables the generic rule for built-in boot DTBs. This will be
renamed to CONFIG_BUILTIN_DTB after all architectures migrate to the
generic rule.
- CONFIG_BUILTIN_DTB_NAME
This specifies the path to the embedded DTB.
(relative to arch/*/boot/dts/)
- CONFIG_BUILTIN_DTB_ALL
If this is enabled, all DTB files compiled under arch/*/boot/dts/ are
embedded into vmlinux. Only used by MIPS.
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---
(no changes since v1)
Makefile | 7 ++++++-
drivers/of/Kconfig | 6 ++++++
scripts/Makefile.vmlinux | 44 ++++++++++++++++++++++++++++++++++++++++
scripts/link-vmlinux.sh | 4 ++++
4 files changed, 60 insertions(+), 1 deletion(-)
diff --git a/Makefile b/Makefile
index 0d3d45a88a71..35b8392d2bef 100644
--- a/Makefile
+++ b/Makefile
@@ -1413,6 +1413,10 @@ ifdef CONFIG_OF_EARLY_FLATTREE
all: dtbs
endif
+ifdef CONFIG_GENERIC_BUILTIN_DTB
+vmlinux: dtbs
+endif
+
endif
PHONY += scripts_dtc
@@ -1480,7 +1484,8 @@ CLEAN_FILES += vmlinux.symvers modules-only.symvers \
modules.builtin modules.builtin.modinfo modules.nsdeps \
modules.builtin.ranges vmlinux.o.map \
compile_commands.json rust/test \
- rust-project.json .vmlinux.objs .vmlinux.export.c
+ rust-project.json .vmlinux.objs .vmlinux.export.c \
+ .builtin-dtbs-list .builtin-dtb.S
# Directories & files removed with 'make mrproper'
MRPROPER_FILES += include/config include/generated \
diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
index dd726c7056bf..5142e7d7fef8 100644
--- a/drivers/of/Kconfig
+++ b/drivers/of/Kconfig
@@ -2,6 +2,12 @@
config DTC
bool
+config GENERIC_BUILTIN_DTB
+ bool
+
+config BUILTIN_DTB_ALL
+ bool
+
menuconfig OF
bool "Device Tree and Open Firmware support"
help
diff --git a/scripts/Makefile.vmlinux b/scripts/Makefile.vmlinux
index 1284f05555b9..9ef0480ed755 100644
--- a/scripts/Makefile.vmlinux
+++ b/scripts/Makefile.vmlinux
@@ -17,6 +17,50 @@ quiet_cmd_cc_o_c = CC $@
%.o: %.c FORCE
$(call if_changed_dep,cc_o_c)
+quiet_cmd_as_o_S = AS $@
+ cmd_as_o_S = $(CC) $(a_flags) -c -o $@ $<
+
+%.o: %.S FORCE
+ $(call if_changed_dep,as_o_S)
+
+# Built-in dtb
+# ---------------------------------------------------------------------------
+
+quiet_cmd_wrap_dtbs = WRAP $@
+ cmd_wrap_dtbs = { \
+ echo '\#include <asm-generic/vmlinux.lds.h>'; \
+ echo '.section .dtb.init.rodata,"a"'; \
+ while read dtb; do \
+ symbase=__dtb_$$(basename -s .dtb "$${dtb}" | tr - _); \
+ echo '.balign STRUCT_ALIGNMENT'; \
+ echo ".global $${symbase}_begin"; \
+ echo "$${symbase}_begin:"; \
+ echo '.incbin "'$$dtb'" '; \
+ echo ".global $${symbase}_end"; \
+ echo "$${symbase}_end:"; \
+ done < $<; \
+ } > $@
+
+.builtin-dtbs.S: .builtin-dtbs-list FORCE
+ $(call if_changed,wrap_dtbs)
+
+quiet_cmd_gen_dtbs_list = GEN $@
+ cmd_gen_dtbs_list = \
+ $(if $(CONFIG_BUILTIN_DTB_NAME), echo "arch/$(SRCARCH)/boot/dts/$(CONFIG_BUILTIN_DTB_NAME).dtb",:) > $@
+
+.builtin-dtbs-list: arch/$(SRCARCH)/boot/dts/dtbs-list FORCE
+ $(call if_changed,$(if $(CONFIG_BUILTIN_DTB_ALL),copy,gen_dtbs_list))
+
+targets += .builtin-dtbs-list
+
+ifdef CONFIG_GENERIC_BUILTIN_DTB
+targets += .builtin-dtbs.S .builtin-dtbs.o
+vmlinux: .builtin-dtbs.o
+endif
+
+# vmlinux
+# ---------------------------------------------------------------------------
+
ifdef CONFIG_MODULES
targets += .vmlinux.export.o
vmlinux: .vmlinux.export.o
diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index c27b4e969f20..bd196944e350 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -68,6 +68,10 @@ vmlinux_link()
libs="${KBUILD_VMLINUX_LIBS}"
fi
+ if is_enabled CONFIG_GENERIC_BUILTIN_DTB; then
+ objs="${objs} .builtin-dtbs.o"
+ fi
+
if is_enabled CONFIG_MODULES; then
objs="${objs} .vmlinux.export.o"
fi
--
2.43.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH v3 1/2] kbuild: move non-boot built-in DTBs to .rodata section
2024-09-23 7:56 [PATCH v3 1/2] kbuild: move non-boot built-in DTBs to .rodata section Masahiro Yamada
2024-09-23 7:56 ` [PATCH v3 2/2] kbuild: add generic support for built-in boot DTBs Masahiro Yamada
@ 2024-09-24 22:15 ` Rob Herring
2024-09-27 14:51 ` Masahiro Yamada
2 siblings, 0 replies; 4+ messages in thread
From: Rob Herring @ 2024-09-24 22:15 UTC (permalink / raw)
To: Masahiro Yamada
Cc: linux-kbuild, linux-kernel, Andrea della Porta, linux-clk,
Stephen Boyd, Brendan Higgins, devicetree, Nathan Chancellor,
Nicolas Schier
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>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v3 1/2] kbuild: move non-boot built-in DTBs to .rodata section
2024-09-23 7:56 [PATCH v3 1/2] kbuild: move non-boot built-in DTBs to .rodata section Masahiro Yamada
2024-09-23 7:56 ` [PATCH v3 2/2] kbuild: add generic support for built-in boot DTBs Masahiro Yamada
2024-09-24 22:15 ` [PATCH v3 1/2] kbuild: move non-boot built-in DTBs to .rodata section Rob Herring
@ 2024-09-27 14:51 ` Masahiro Yamada
2 siblings, 0 replies; 4+ messages in thread
From: Masahiro Yamada @ 2024-09-27 14:51 UTC (permalink / raw)
To: linux-kbuild
Cc: linux-kernel, Andrea della Porta, linux-clk, Stephen Boyd,
Brendan Higgins, Rob Herring, devicetree, Nathan Chancellor,
Nicolas Schier
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
>
--
Best Regards
Masahiro Yamada
^ permalink raw reply related [flat|nested] 4+ messages in thread