Re: [PATCH] MIPS: vmlinux.lds.S: align raw appended dtb to 8 bytes

[Thomas Bogendoerfer <tsbogend@alpha.franken.de>]

On Mon, Mar 08, 2021 at 12:10:33PM +0100, Bjørn Mork wrote:
> Thomas Bogendoerfer <tsbogend@alpha.franken.de> writes:
> 
> > On Sun, Mar 07, 2021 at 07:23:01PM +0100, Bjørn Mork wrote:
> >> The devicetree specification requires 8-byte alignment in
> >> memory. This is now enforced by libfdt since commit 79edff12060f
> >> ("scripts/dtc: Update to upstream version v1.6.0-51-g183df9e9c2b9")
> >> which included the upstream commit 5e735860c478 ("libfdt: Check for
> >> 8-byte address alignment in fdt_ro_probe_()").
> >> 
> >> This broke the MIPS raw appended DTBs which would be appended to
> >> the image immediately following the initramfs section.  This ends
> >> with a 32bit size, resulting in a 4-byte alignment of the DTB.
> >> 
> >> Fix by padding with zeroes to 8-bytes when MIPS_RAW_APPENDED_DTB
> >> is defined.
> >> 
> >> Cc: Rob Herring <robh+dt@kernel.org>
> >> Cc: Frank Rowand <frowand.list@gmail.com>
> >> Signed-off-by: Bjørn Mork <bjorn@mork.no>
> >> ---
> >>  arch/mips/kernel/vmlinux.lds.S | 5 +++++
> >>  1 file changed, 5 insertions(+)
> >
> > thank you for your patch, but there already was a fix for the problem
> > pending from Paul, which I've applied to mips-fixes a few minutes ago.
> 
> Yes, I see.  That does look much nicer.  But I don't think it addresses
> the problem with an uncompressed kernel?  Could we have the padding in
> vmlinux.lds.S  as well?  Or some other solution to ensure that it is
> possible to cat the DTB to the end of vmlinux.bin without manually
> aligning it to 8 bytes?

I see

diff --git a/arch/mips/kernel/vmlinux.lds.S b/arch/mips/kernel/vmlinux.lds.S
index c1c345be04ff..4b4e39b7c79b 100644
--- a/arch/mips/kernel/vmlinux.lds.S
+++ b/arch/mips/kernel/vmlinux.lds.S
@@ -145,6 +145,7 @@ SECTIONS
        }
 
 #ifdef CONFIG_MIPS_ELF_APPENDED_DTB
+       STRUCT_ALIGN();
        .appended_dtb : AT(ADDR(.appended_dtb) - LOAD_OFFSET) {
                *(.appended_dtb)
                KEEP(*(.appended_dtb))
@@ -172,6 +173,7 @@ SECTIONS
 #endif
 
 #ifdef CONFIG_MIPS_RAW_APPENDED_DTB
+       STRUCT_ALIGN();
        __appended_dtb = .;
        /* leave space for appended DTB */
        . += 0x100000;

in that patch, and IMHO this does align the appended_dtb. What do I miss ?

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]