| Message ID | 20240904234803.698424-5-masahiroy@kernel.org (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | kbuild: refactor DTB build rules, introduce a generic built-in boot DTB support | expand |
On Thu, Sep 05, 2024 at 08:47:40AM +0900, Masahiro Yamada wrote: > 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 embedded 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 embedded DTBs > support. Currently, different architectures use different CONFIG options > for the same purposes. > > The CONFIG options are unified as follows: > > - CONFIG_GENERIC_BUILTIN_DTB > > This enables the generic rule for embedded 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. I started to do this a long time ago, but then decided we didn't want to encourage this feature. IMO it should only be for legacy bootloaders or development/debug. And really, appended DTB is more flexible for the legacy bootloader case. In hindsight, a common config would have been easier to limit new arches... > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > --- > > 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 145112bf281a..1c765c12ab9e 100644 > --- a/Makefile > +++ b/Makefile > @@ -1417,6 +1417,10 @@ ifdef CONFIG_OF_EARLY_FLATTREE > all: dtbs > endif > > +ifdef CONFIG_GENERIC_BUILTIN_DTB > +vmlinux: dtbs > +endif > + > endif > > PHONY += scripts_dtc > @@ -1483,7 +1487,8 @@ endif # CONFIG_MODULES > CLEAN_FILES += vmlinux.symvers modules-only.symvers \ > modules.builtin modules.builtin.modinfo modules.nsdeps \ > 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 So that we don't add new architectures to this, I would like something like: # Do not add new architectures to this list depends on MIPS || RISCV || MICROBLAZE ... Yes, it's kind of odd since the arch selects the option... For sure, we don't want this option on arm64. For that, I can rely on Will and Catalin rejecting a select, but on some new arch I can't. > + > +config BUILTIN_DTB_ALL > + bool Can this be limited to MIPS? > + > menuconfig OF > bool "Device Tree and Open Firmware support" > help > diff --git a/scripts/Makefile.vmlinux b/scripts/Makefile.vmlinux > index 5ceecbed31eb..4626b472da49 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'; \ Is this always guaranteed to be at least 8 bytes? That's the required alignment for dtbs and assumed by libfdt. > + echo ".global $${symbase}_begin"; \ > + echo "$${symbase}_begin:"; \ > + echo '.incbin "'$$dtb'" '; \ > + echo ".global $${symbase}_end"; \ > + echo "$${symbase}_end:"; \ > + done < $<; \ > + } > $@
On Thu, Sep 5, 2024 at 11:17 PM Rob Herring <robh@kernel.org> wrote: > > On Thu, Sep 05, 2024 at 08:47:40AM +0900, Masahiro Yamada wrote: > > 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 embedded 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 embedded DTBs > > support. Currently, different architectures use different CONFIG options > > for the same purposes. > > > > The CONFIG options are unified as follows: > > > > - CONFIG_GENERIC_BUILTIN_DTB > > > > This enables the generic rule for embedded 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. > > I started to do this a long time ago, but then decided we didn't want to > encourage this feature. IMO it should only be for legacy bootloaders or > development/debug. And really, appended DTB is more flexible for the > legacy bootloader case. I learned CONFIG_ARM_APPENDED_DTB today. > In hindsight, a common config would have been easier to limit new > arches... > > > > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > > --- > > > > 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 145112bf281a..1c765c12ab9e 100644 > > --- a/Makefile > > +++ b/Makefile > > @@ -1417,6 +1417,10 @@ ifdef CONFIG_OF_EARLY_FLATTREE > > all: dtbs > > endif > > > > +ifdef CONFIG_GENERIC_BUILTIN_DTB > > +vmlinux: dtbs > > +endif > > + > > endif > > > > PHONY += scripts_dtc > > @@ -1483,7 +1487,8 @@ endif # CONFIG_MODULES > > CLEAN_FILES += vmlinux.symvers modules-only.symvers \ > > modules.builtin modules.builtin.modinfo modules.nsdeps \ > > 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 > > So that we don't add new architectures to this, I would like something > like: > > # Do not add new architectures to this list > depends on MIPS || RISCV || MICROBLAZE ... > > Yes, it's kind of odd since the arch selects the option... > > For sure, we don't want this option on arm64. For that, I can rely on > Will and Catalin rejecting a select, but on some new arch I can't. > > > + > > +config BUILTIN_DTB_ALL > > + bool > > Can this be limited to MIPS? I am fine with hard-coded "depends on" if this feature is discouraged. > > + > > menuconfig OF > > bool "Device Tree and Open Firmware support" > > help > > diff --git a/scripts/Makefile.vmlinux b/scripts/Makefile.vmlinux > > index 5ceecbed31eb..4626b472da49 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'; \ > > Is this always guaranteed to be at least 8 bytes? That's the required > alignment for dtbs and assumed by libfdt. I think so. include/asm-generic/vmlinux.lds.h defines it as 32. We can loosen the alignment to 8, but for safety, I just copied this from scripts/Makefile.lib because 32-byte alignment is what we do now.
On Thu, Sep 5, 2024 at 11:17 PM Rob Herring <robh@kernel.org> wrote: > > On Thu, Sep 05, 2024 at 08:47:40AM +0900, Masahiro Yamada wrote: > > 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 embedded 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 embedded DTBs > > support. Currently, different architectures use different CONFIG options > > for the same purposes. > > > > The CONFIG options are unified as follows: > > > > - CONFIG_GENERIC_BUILTIN_DTB > > > > This enables the generic rule for embedded 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. > > I started to do this a long time ago, but then decided we didn't want to > encourage this feature. IMO it should only be for legacy bootloaders or > development/debug. And really, appended DTB is more flexible for the > legacy bootloader case. > > In hindsight, a common config would have been easier to limit new > arches... > > > > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > > --- > > > > 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 145112bf281a..1c765c12ab9e 100644 > > --- a/Makefile > > +++ b/Makefile > > @@ -1417,6 +1417,10 @@ ifdef CONFIG_OF_EARLY_FLATTREE > > all: dtbs > > endif > > > > +ifdef CONFIG_GENERIC_BUILTIN_DTB > > +vmlinux: dtbs > > +endif > > + > > endif > > > > PHONY += scripts_dtc > > @@ -1483,7 +1487,8 @@ endif # CONFIG_MODULES > > CLEAN_FILES += vmlinux.symvers modules-only.symvers \ > > modules.builtin modules.builtin.modinfo modules.nsdeps \ > > 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 > > So that we don't add new architectures to this, I would like something > like: > > # Do not add new architectures to this list > depends on MIPS || RISCV || MICROBLAZE ... This will not work after 14/15 is applied. For example, if arch/arm/Kconfig has config BUILTIN_DTB bool "enable BUILTIN_DTB" No warning is displayed.
diff --git a/Makefile b/Makefile index 145112bf281a..1c765c12ab9e 100644 --- a/Makefile +++ b/Makefile @@ -1417,6 +1417,10 @@ ifdef CONFIG_OF_EARLY_FLATTREE all: dtbs endif +ifdef CONFIG_GENERIC_BUILTIN_DTB +vmlinux: dtbs +endif + endif PHONY += scripts_dtc @@ -1483,7 +1487,8 @@ endif # CONFIG_MODULES CLEAN_FILES += vmlinux.symvers modules-only.symvers \ modules.builtin modules.builtin.modinfo modules.nsdeps \ 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 5ceecbed31eb..4626b472da49 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
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 embedded 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 embedded DTBs support. Currently, different architectures use different CONFIG options for the same purposes. The CONFIG options are unified as follows: - CONFIG_GENERIC_BUILTIN_DTB This enables the generic rule for embedded 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> --- Makefile | 7 ++++++- drivers/of/Kconfig | 6 ++++++ scripts/Makefile.vmlinux | 44 ++++++++++++++++++++++++++++++++++++++++ scripts/link-vmlinux.sh | 4 ++++ 4 files changed, 60 insertions(+), 1 deletion(-)