linux-kernel - Re: [PATCH] microblaze: migrate to the generic rule for built-in DTB

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAK7LNAQDj9x7hHeSO=5PHpswRm6ku3bb1RW9fVZw9N+xswPQrA@mail.gmail.com>
Date: Tue, 14 Jan 2025 11:41:07 +0900
From: Masahiro Yamada <masahiroy@...nel.org>
To: Michal Simek <michal.simek@....com>
Cc: linux-kernel@...r.kernel.org, Conor Dooley <conor+dt@...nel.org>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Rob Herring <robh@...nel.org>, devicetree@...r.kernel.org
Subject: Re: [PATCH] microblaze: migrate to the generic rule for built-in DTB

On Wed, Jan 8, 2025 at 1:37 AM Michal Simek <michal.simek@....com> wrote:
>
> Hi Masahiro,
>
> On 12/22/24 10:46, Masahiro Yamada wrote:
> > Commit 654102df2ac2 ("kbuild: add generic support for built-in boot
> > DTBs") introduced generic support for built-in DTBs.
> >
> > Select GENERIC_BUILTIN_DTB to use the generic rule.
> >
> > MicroBlaze is the only architecture that places the built-in DTB to its
> > own section, __fdt_blob, rather than the standard location defined by
> > the KERNEL_DTB() macro.
> >
> > For backward compatibility, arch/microblaze/boot/dts/system.dtb is still
> > placed in the __fdt_blob section, and possibly overwritten by an externel
> > DTB passed via the r7 register.
> >
> > Signed-off-by: Masahiro Yamada <masahiroy@...nel.org>
> > ---
> >
> >   arch/microblaze/Kbuild                | 1 -
> >   arch/microblaze/Kconfig               | 5 +++++
> >   arch/microblaze/boot/dts/Makefile     | 5 -----
> >   arch/microblaze/boot/dts/linked_dtb.S | 2 --
> >   arch/microblaze/kernel/vmlinux.lds.S  | 2 +-
> >   5 files changed, 6 insertions(+), 9 deletions(-)
> >   delete mode 100644 arch/microblaze/boot/dts/linked_dtb.S
> >
> > diff --git a/arch/microblaze/Kbuild b/arch/microblaze/Kbuild
> > index 077a0b8e9615..70510389eb92 100644
> > --- a/arch/microblaze/Kbuild
> > +++ b/arch/microblaze/Kbuild
> > @@ -2,7 +2,6 @@
> >   obj-y                       += kernel/
> >   obj-y                       += mm/
> >   obj-$(CONFIG_PCI)   += pci/
> > -obj-y                        += boot/dts/
> >
> >   # for cleaning
> >   subdir- += boot
> > diff --git a/arch/microblaze/Kconfig b/arch/microblaze/Kconfig
> > index f18ec02ddeb2..4ed8ca89f0c9 100644
> > --- a/arch/microblaze/Kconfig
> > +++ b/arch/microblaze/Kconfig
> > @@ -10,6 +10,7 @@ config MICROBLAZE
> >       select ARCH_MIGHT_HAVE_PC_PARPORT
> >       select ARCH_WANT_IPC_PARSE_VERSION
> >       select BUILDTIME_TABLE_SORT
> > +     select GENERIC_BUILTIN_DTB
> >       select TIMER_OF
> >       select CLONE_BACKWARDS3
> >       select COMMON_CLK
> > @@ -47,6 +48,10 @@ config MICROBLAZE
> >       select TRACE_IRQFLAGS_SUPPORT
> >       select GENERIC_IRQ_MULTI_HANDLER
> >
> > +config BUILTIN_DTB_NAME
> > +     string
> > +     default "system"
> > +
> >   # Endianness selection
> >   choice
> >       prompt "Endianness selection"
> > diff --git a/arch/microblaze/boot/dts/Makefile b/arch/microblaze/boot/dts/Makefile
> > index b84e2cbb20ee..87c1d25ff096 100644
> > --- a/arch/microblaze/boot/dts/Makefile
> > +++ b/arch/microblaze/boot/dts/Makefile
> > @@ -4,11 +4,6 @@
> >   dtb-y := system.dtb
> >
> >   ifneq ($(DTB),)
> > -obj-y += linked_dtb.o
> > -
> > -# Ensure system.dtb exists
> > -$(obj)/linked_dtb.o: $(obj)/system.dtb
> > -
> >   # Generate system.dtb from $(DTB).dtb
> >   ifneq ($(DTB),system)
> >   $(obj)/system.dtb: $(obj)/$(DTB).dtb
> > diff --git a/arch/microblaze/boot/dts/linked_dtb.S b/arch/microblaze/boot/dts/linked_dtb.S
> > deleted file mode 100644
> > index 23345af3721f..000000000000
> > --- a/arch/microblaze/boot/dts/linked_dtb.S
> > +++ /dev/null
> > @@ -1,2 +0,0 @@
> > -.section __fdt_blob,"a"
> > -.incbin "arch/microblaze/boot/dts/system.dtb"
> > diff --git a/arch/microblaze/kernel/vmlinux.lds.S b/arch/microblaze/kernel/vmlinux.lds.S
> > index ae50d3d04a7d..e86f9ca8e979 100644
> > --- a/arch/microblaze/kernel/vmlinux.lds.S
> > +++ b/arch/microblaze/kernel/vmlinux.lds.S
> > @@ -47,7 +47,7 @@ SECTIONS {
> >       . = ALIGN (8) ;
> >       __fdt_blob : AT(ADDR(__fdt_blob) - LOAD_OFFSET) {
> >               _fdt_start = . ;                /* place for fdt blob */
> > -             *(__fdt_blob) ;                 /* Any link-placed DTB */
> > +             *(.dtb.init.rodata) ;           /* Any link-placed DTB */
> >               . = _fdt_start + 0x10000;       /* Pad up to 64kbyte */
> >               _fdt_end = . ;
> >       }
>
> This patch is better then previous one but still it is changing behavior of
> build. When this patch is applied linux.bin contains dtb which is not the same
> behavior as before (which was empty).
> DTB should be filled when simpleImage.<dt> is built.

Why is it a problem?

Microblaze always keeps 64kbyte space in case
an external DTB is passed from a boot-loader.

Even if the default system.dtb is placed there,
nothing is wasted.


One more thing, Microblaze is only the architecture
that determines the built-in DTB by the command line.

I do not think it is a sensible way.

    $ make linux.bin simpleImage.foo
      --> linux.bin also contains the builtin device tree

    $ make simpleImage.foo simpleImage.bar
      --> does not work


All other architectures determinantes the built-in DTB
by CONFIG option.


-- 
Best Regards
Masahiro Yamada