* [PATCH] MIPS: vmlinux.lds.S: align raw appended dtb to 8 bytes
@ 2021-03-07 18:23 Bjørn Mork
2021-03-08 10:55 ` Thomas Bogendoerfer
0 siblings, 1 reply; 6+ messages in thread
From: Bjørn Mork @ 2021-03-07 18:23 UTC (permalink / raw)
To: Thomas Bogendoerfer
Cc: linux-mips, Bjørn Mork, Rob Herring, Frank Rowand
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(+)
diff --git a/arch/mips/kernel/vmlinux.lds.S b/arch/mips/kernel/vmlinux.lds.S
index c1c345be04ff..850117efb09b 100644
--- a/arch/mips/kernel/vmlinux.lds.S
+++ b/arch/mips/kernel/vmlinux.lds.S
@@ -172,6 +172,11 @@ SECTIONS
#endif
#ifdef CONFIG_MIPS_RAW_APPENDED_DTB
+ .fill : {
+ FILL(0);
+ BYTE(0);
+ . = ALIGN(8);
+ }
__appended_dtb = .;
/* leave space for appended DTB */
. += 0x100000;
--
2.20.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] MIPS: vmlinux.lds.S: align raw appended dtb to 8 bytes
2021-03-07 18:23 [PATCH] MIPS: vmlinux.lds.S: align raw appended dtb to 8 bytes Bjørn Mork
@ 2021-03-08 10:55 ` Thomas Bogendoerfer
2021-03-08 11:10 ` Bjørn Mork
0 siblings, 1 reply; 6+ messages in thread
From: Thomas Bogendoerfer @ 2021-03-08 10:55 UTC (permalink / raw)
To: Bjørn Mork; +Cc: linux-mips, Rob Herring, Frank Rowand
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.
Thomas.
--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] MIPS: vmlinux.lds.S: align raw appended dtb to 8 bytes
2021-03-08 10:55 ` Thomas Bogendoerfer
@ 2021-03-08 11:10 ` Bjørn Mork
2021-03-08 12:53 ` Thomas Bogendoerfer
0 siblings, 1 reply; 6+ messages in thread
From: Bjørn Mork @ 2021-03-08 11:10 UTC (permalink / raw)
To: Thomas Bogendoerfer; +Cc: linux-mips, Rob Herring, Frank Rowand
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?
Bjørn
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] MIPS: vmlinux.lds.S: align raw appended dtb to 8 bytes
2021-03-08 11:10 ` Bjørn Mork
@ 2021-03-08 12:53 ` Thomas Bogendoerfer
2021-03-08 13:45 ` Bjørn Mork
0 siblings, 1 reply; 6+ messages in thread
From: Thomas Bogendoerfer @ 2021-03-08 12:53 UTC (permalink / raw)
To: Bjørn Mork; +Cc: linux-mips, Rob Herring, Frank Rowand
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 ]
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] MIPS: vmlinux.lds.S: align raw appended dtb to 8 bytes
2021-03-08 12:53 ` Thomas Bogendoerfer
@ 2021-03-08 13:45 ` Bjørn Mork
2021-03-08 17:47 ` Thomas Bogendoerfer
0 siblings, 1 reply; 6+ messages in thread
From: Bjørn Mork @ 2021-03-08 13:45 UTC (permalink / raw)
To: Thomas Bogendoerfer; +Cc: linux-mips, Rob Herring, Frank Rowand
[-- Attachment #1: Type: text/plain, Size: 3014 bytes --]
Thomas Bogendoerfer <tsbogend@alpha.franken.de> writes:
> 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 ?
I'll not pretend I know anything about this subject, so feel free to
adjust as necessary.
The problem with that patch is that it doesn't pad the image to the
aligment. So you can't do
cat my.dtb >> arch/mips/boot/vmlinux.bin
anymore. This used to work before commit 79edff12060f.
This is an image build with that patch and an initramfs:
bjorn@canardo:/usr/local/src/build-tmp/linux$ ls -l arch/mips/boot/vmlinux.bin
-rwxr-xr-x 1 bjorn src 15724964 Mar 8 14:21 arch/mips/boot/vmlinux.bin
So that's aligned to 4 bytes, because it's terminated by the 32bit
length of the initramfs:
15724964/8
1965620.50000000000000000000
Building with the attached patch results in:
bjorn@canardo:/usr/local/src/build-tmp/linux$ ls -l arch/mips/boot/vmlinux.bin
-rwxr-xr-x 1 bjorn src 15724992 Mar 8 14:29 arch/mips/boot/vmlinux.bin
Which is aligned to the 32 bytes expected after STRUCT_ALIGN():
15724992/8
1965624.00000000000000000000
The tail of the image shows the cpio trailer and cpio length
(0x0061f400) followed by enough padding to align an appended DTB to 32
bytes:
bjorn@canardo:/usr/local/src/build-tmp/linux$ ls -l arch/mips/boot/vmlinux.bin
-rwxr-xr-x 1 bjorn src 15724992 Mar 8 14:29 arch/mips/boot/vmlinux.bin
bjorn@canardo:/usr/local/src/build-tmp/linux$ hexdump -C arch/mips/boot/vmlinux.bin|tail
00eff090 30 30 30 30 30 30 30 30 30 31 30 30 30 30 30 30 |0000000001000000|
00eff0a0 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 |0000000000000000|
*
00eff0d0 30 42 30 30 30 30 30 30 30 30 54 52 41 49 4c 45 |0B00000000TRAILE|
00eff0e0 52 21 21 21 00 00 00 00 00 00 00 00 00 00 00 00 |R!!!............|
00eff0f0 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................|
*
00eff1a0 00 61 f4 00 00 00 00 00 00 00 00 00 00 00 00 00 |.a..............|
00eff1b0 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................|
00eff1c0
Yes, you can always pad the image yourself if you know about this
alignment requirement. But it gets more complicated. And it breaks my
home grown hackish build script. I know I'm not the only one...
Bjørn
[-- Attachment #2: 0001-MIPS-vmlinux.lds.S-fill-vmlinux.bin-to-DTB-alignment.patch --]
[-- Type: text/x-diff, Size: 1085 bytes --]
From c1bd611ae62db46db12d86196cade2613a291400 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B8rn=20Mork?= <bjorn@mork.no>
Date: Mon, 8 Mar 2021 14:24:17 +0100
Subject: [PATCH] MIPS: vmlinux.lds.S: fill vmlinux.bin to DTB alignment
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Padding the binary to the required alignment for raw appended
DTBs so that it is possible to append one without detailed
knowledge of this requirement.
Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
arch/mips/kernel/vmlinux.lds.S | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/arch/mips/kernel/vmlinux.lds.S b/arch/mips/kernel/vmlinux.lds.S
index 4b4e39b7c79b..1f98947fe715 100644
--- a/arch/mips/kernel/vmlinux.lds.S
+++ b/arch/mips/kernel/vmlinux.lds.S
@@ -173,7 +173,11 @@ SECTIONS
#endif
#ifdef CONFIG_MIPS_RAW_APPENDED_DTB
- STRUCT_ALIGN();
+ .fill : {
+ FILL(0);
+ BYTE(0);
+ STRUCT_ALIGN();
+ }
__appended_dtb = .;
/* leave space for appended DTB */
. += 0x100000;
--
2.20.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] MIPS: vmlinux.lds.S: align raw appended dtb to 8 bytes
2021-03-08 13:45 ` Bjørn Mork
@ 2021-03-08 17:47 ` Thomas Bogendoerfer
0 siblings, 0 replies; 6+ messages in thread
From: Thomas Bogendoerfer @ 2021-03-08 17:47 UTC (permalink / raw)
To: Bjørn Mork; +Cc: linux-mips, Rob Herring, Frank Rowand
On Mon, Mar 08, 2021 at 02:45:57PM +0100, Bjørn Mork wrote:
> Thomas Bogendoerfer <tsbogend@alpha.franken.de> writes:
>
> > 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 ?
>
> I'll not pretend I know anything about this subject, so feel free to
> adjust as necessary.
>
> The problem with that patch is that it doesn't pad the image to the
> aligment. So you can't do
>
> cat my.dtb >> arch/mips/boot/vmlinux.bin
>
> anymore. This used to work before commit 79edff12060f.
ok, took a little while to fully understand the problem. I've applied
your patch to mips-fixes with a Fixes: tag added.
Thomas.
--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-03-08 17:48 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-03-07 18:23 [PATCH] MIPS: vmlinux.lds.S: align raw appended dtb to 8 bytes Bjørn Mork
2021-03-08 10:55 ` Thomas Bogendoerfer
2021-03-08 11:10 ` Bjørn Mork
2021-03-08 12:53 ` Thomas Bogendoerfer
2021-03-08 13:45 ` Bjørn Mork
2021-03-08 17:47 ` Thomas Bogendoerfer
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox