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 <masahi...@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 < $<;                                                      \
> +     } > $@

_______________________________________________
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Reply via email to