* [PATCHv3] ARM:boot:device tree: Allow the device tree binary to be appended to zImage
@ 2011-03-24 23:18 John Bonesio
2011-03-24 23:37 ` Nicolas Pitre
` (2 more replies)
0 siblings, 3 replies; 36+ messages in thread
From: John Bonesio @ 2011-03-24 23:18 UTC (permalink / raw)
To: linux-arm-kernel
This patch provides the ability to boot using a device tree that is appended
to the raw binary zImage (e.g. cat zImage <filename>.dtb > zImage_w_dtb).
Signed-off-by: John Bonesio <bones@secretlab.ca>
---
arch/arm/Kconfig | 7 ++++
arch/arm/boot/compressed/head.S | 78 ++++++++++++++++++++++++++++++++++++++-
2 files changed, 82 insertions(+), 3 deletions(-)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 92bae09..3556aea 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1651,6 +1651,13 @@ config ZBOOT_ROM
Say Y here if you intend to execute your compressed kernel image
(zImage) directly from ROM or flash. If unsure, say N.
+config ARM_APPENDED_DTB
+ bool "Use appended device tree blob"
+ depends on OF && !ZBOOT_ROM
+ help
+ With this option, the boot code will look for a device tree binary
+ (dtb) appended to zImage.
+
config CMDLINE
string "Default kernel command string"
default ""
diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
index 200625c..034a654 100644
--- a/arch/arm/boot/compressed/head.S
+++ b/arch/arm/boot/compressed/head.S
@@ -211,6 +211,61 @@ restart: adr r0, LC0
mov r10, r6
#endif
+ mov lr, #0 @ init lr (dtb size) to 0
+#ifdef CONFIG_ARM_APPENDED_DTB
+/*
+ * r0 = delta
+ * r2 = BSS start
+ * r3 = BSS end
+ * r4 = final kernel address
+ * r5 = start of this image
+ * r6 = _edata
+ * r7 = architecture ID
+ * r8 = atags/device tree pointer
+ * r9 = size of decompressed image
+ * r10 = end of this image, including bss/stack/malloc space if non XIP
+ * r11 = GOT start
+ * r12 = GOT end
+ *
+ * if there are device trees (dtb) appended to zImage, advance r10 so that the
+ * dtb data will get relocated along with the kernel if necessary.
+ */
+
+ ldr lr, [r6, #0]
+#ifndef __ARMEB__
+ ldr r1, =0xedfe0dd0 @ sig is 0xd00dfeed big endian
+#else
+ ldr r1, =0xd00dfeed
+#endif
+ cmp lr, r1
+ mov lr, #0 @ dtb not found, it's size is 0
+ bne dtb_check_done
+
+ mov r8, r6 @ use the appended device tree
+
+ /* Get the dtb's size */
+ ldr lr, [r6, #4]
+
+#ifndef __ARMEB__
+ /* convert lr (dtb size) to little endian */
+ eor r1, lr, lr, ror #16
+ bic r1, r1, #0x00ff0000
+ mov lr, lr, ror #8
+ eor lr, lr, r1, lsr #8
+#endif
+ /*
+ * dtb size rounded up to a multiple of 8 bytes so a
+ * misaligned GOT entry doesn't cause the end of the
+ * dtb binary to be overwritten.
+ */
+ add lr, lr, #7
+ bic lr, lr, #7
+
+ add r10, r10, lr
+ add r6, r6, lr
+dtb_check_done:
+#endif
+
/*
* Check to see if we will overwrite ourselves.
* r4 = final kernel address
@@ -271,8 +326,9 @@ wont_overwrite:
* r11 = GOT start
* r12 = GOT end
* sp = stack pointer
+ * lr = appended dtb size (0 if not present)
*/
- teq r0, #0
+ orrs r1, r0, lr
beq not_relocated
add r11, r11, r0
add r12, r12, r0
@@ -288,12 +344,28 @@ wont_overwrite:
/*
* Relocate all entries in the GOT table.
+ * Bump bss entries to _edata + dtb size
*/
1: ldr r1, [r11, #0] @ relocate entries in the GOT
- add r1, r1, r0 @ table. This fixes up the
- str r1, [r11], #4 @ C references.
+ add r1, r1, r0 @ This fixes up C references
+ cmp r1, r2 @ if entry >= bss_start &&
+ cmphs r3, r1 @ bss_end > entry
+ addhi r1, r1, lr @ entry += dtb size
+ str r1, [r11], #4 @ next entry
cmp r11, r12
blo 1b
+
+ /* bump our bss registers too */
+ add r2, r2, lr
+ add r3, r3, lr
+
+ /*
+ * bump the stack pinter
+ *
+ * If the linker script changes so the stack is not after
+ * the bss section, this code will be wrong.
+ */
+ add sp, sp, lr
#else
/*
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCHv3] ARM:boot:device tree: Allow the device tree binary to be appended to zImage
2011-03-24 23:18 [PATCHv3] ARM:boot:device tree: Allow the device tree binary to be appended to zImage John Bonesio
@ 2011-03-24 23:37 ` Nicolas Pitre
2011-03-28 9:13 ` Shawn Guo
2011-05-09 11:23 ` [PATCHv3] ARM:boot:device tree: Allow the device tree binary to be appended to zImage Tony Lindgren
2 siblings, 0 replies; 36+ messages in thread
From: Nicolas Pitre @ 2011-03-24 23:37 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, 24 Mar 2011, John Bonesio wrote:
> This patch provides the ability to boot using a device tree that is appended
> to the raw binary zImage (e.g. cat zImage <filename>.dtb > zImage_w_dtb).
>
> Signed-off-by: John Bonesio <bones@secretlab.ca>
Reviewed-by: Nicolas Pitre <nicolas.pitre@linaro.org>
> ---
>
> arch/arm/Kconfig | 7 ++++
> arch/arm/boot/compressed/head.S | 78 ++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 82 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 92bae09..3556aea 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -1651,6 +1651,13 @@ config ZBOOT_ROM
> Say Y here if you intend to execute your compressed kernel image
> (zImage) directly from ROM or flash. If unsure, say N.
>
> +config ARM_APPENDED_DTB
> + bool "Use appended device tree blob"
> + depends on OF && !ZBOOT_ROM
> + help
> + With this option, the boot code will look for a device tree binary
> + (dtb) appended to zImage.
> +
> config CMDLINE
> string "Default kernel command string"
> default ""
> diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
> index 200625c..034a654 100644
> --- a/arch/arm/boot/compressed/head.S
> +++ b/arch/arm/boot/compressed/head.S
> @@ -211,6 +211,61 @@ restart: adr r0, LC0
> mov r10, r6
> #endif
>
> + mov lr, #0 @ init lr (dtb size) to 0
> +#ifdef CONFIG_ARM_APPENDED_DTB
> +/*
> + * r0 = delta
> + * r2 = BSS start
> + * r3 = BSS end
> + * r4 = final kernel address
> + * r5 = start of this image
> + * r6 = _edata
> + * r7 = architecture ID
> + * r8 = atags/device tree pointer
> + * r9 = size of decompressed image
> + * r10 = end of this image, including bss/stack/malloc space if non XIP
> + * r11 = GOT start
> + * r12 = GOT end
> + *
> + * if there are device trees (dtb) appended to zImage, advance r10 so that the
> + * dtb data will get relocated along with the kernel if necessary.
> + */
> +
> + ldr lr, [r6, #0]
> +#ifndef __ARMEB__
> + ldr r1, =0xedfe0dd0 @ sig is 0xd00dfeed big endian
> +#else
> + ldr r1, =0xd00dfeed
> +#endif
> + cmp lr, r1
> + mov lr, #0 @ dtb not found, it's size is 0
> + bne dtb_check_done
> +
> + mov r8, r6 @ use the appended device tree
> +
> + /* Get the dtb's size */
> + ldr lr, [r6, #4]
> +
> +#ifndef __ARMEB__
> + /* convert lr (dtb size) to little endian */
> + eor r1, lr, lr, ror #16
> + bic r1, r1, #0x00ff0000
> + mov lr, lr, ror #8
> + eor lr, lr, r1, lsr #8
> +#endif
> + /*
> + * dtb size rounded up to a multiple of 8 bytes so a
> + * misaligned GOT entry doesn't cause the end of the
> + * dtb binary to be overwritten.
> + */
> + add lr, lr, #7
> + bic lr, lr, #7
> +
> + add r10, r10, lr
> + add r6, r6, lr
> +dtb_check_done:
> +#endif
> +
> /*
> * Check to see if we will overwrite ourselves.
> * r4 = final kernel address
> @@ -271,8 +326,9 @@ wont_overwrite:
> * r11 = GOT start
> * r12 = GOT end
> * sp = stack pointer
> + * lr = appended dtb size (0 if not present)
> */
> - teq r0, #0
> + orrs r1, r0, lr
> beq not_relocated
> add r11, r11, r0
> add r12, r12, r0
> @@ -288,12 +344,28 @@ wont_overwrite:
>
> /*
> * Relocate all entries in the GOT table.
> + * Bump bss entries to _edata + dtb size
> */
> 1: ldr r1, [r11, #0] @ relocate entries in the GOT
> - add r1, r1, r0 @ table. This fixes up the
> - str r1, [r11], #4 @ C references.
> + add r1, r1, r0 @ This fixes up C references
> + cmp r1, r2 @ if entry >= bss_start &&
> + cmphs r3, r1 @ bss_end > entry
> + addhi r1, r1, lr @ entry += dtb size
> + str r1, [r11], #4 @ next entry
> cmp r11, r12
> blo 1b
> +
> + /* bump our bss registers too */
> + add r2, r2, lr
> + add r3, r3, lr
> +
> + /*
> + * bump the stack pinter
> + *
> + * If the linker script changes so the stack is not after
> + * the bss section, this code will be wrong.
> + */
> + add sp, sp, lr
> #else
>
> /*
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCHv3] ARM:boot:device tree: Allow the device tree binary to be appended to zImage
2011-03-24 23:18 [PATCHv3] ARM:boot:device tree: Allow the device tree binary to be appended to zImage John Bonesio
2011-03-24 23:37 ` Nicolas Pitre
@ 2011-03-28 9:13 ` Shawn Guo
2011-04-13 14:00 ` Tony Lindgren
2011-05-09 11:23 ` [PATCHv3] ARM:boot:device tree: Allow the device tree binary to be appended to zImage Tony Lindgren
2 siblings, 1 reply; 36+ messages in thread
From: Shawn Guo @ 2011-03-28 9:13 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Mar 24, 2011 at 04:18:41PM -0700, John Bonesio wrote:
> This patch provides the ability to boot using a device tree that is appended
> to the raw binary zImage (e.g. cat zImage <filename>.dtb > zImage_w_dtb).
>
> Signed-off-by: John Bonesio <bones@secretlab.ca>
> ---
>
Tested on mx51 babbage. The non-dt kernel works fine, but dt kernel
does not.
I tracked it down a little bit and found, before calling kernel from
arch/arm/boot/compressed/head.S, the dtb magic number (at address
pointed by r8) was interpolated to 0x73fbc000 which happens to be the
value of 'uart_base' assigned in plat-mxc/include/mach/uncompress.h.
--
Regards,
Shawn
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCHv3] ARM:boot:device tree: Allow the device tree binary to be appended to zImage
2011-03-28 9:13 ` Shawn Guo
@ 2011-04-13 14:00 ` Tony Lindgren
2011-04-20 5:47 ` Shawn Guo
0 siblings, 1 reply; 36+ messages in thread
From: Tony Lindgren @ 2011-04-13 14:00 UTC (permalink / raw)
To: linux-arm-kernel
* Shawn Guo <shawn.guo@freescale.com> [110328 12:10]:
> On Thu, Mar 24, 2011 at 04:18:41PM -0700, John Bonesio wrote:
> > This patch provides the ability to boot using a device tree that is appended
> > to the raw binary zImage (e.g. cat zImage <filename>.dtb > zImage_w_dtb).
> >
> > Signed-off-by: John Bonesio <bones@secretlab.ca>
> > ---
> >
> Tested on mx51 babbage. The non-dt kernel works fine, but dt kernel
> does not.
>
> I tracked it down a little bit and found, before calling kernel from
> arch/arm/boot/compressed/head.S, the dtb magic number (at address
> pointed by r8) was interpolated to 0x73fbc000 which happens to be the
> value of 'uart_base' assigned in plat-mxc/include/mach/uncompress.h.
Gave this a try on an omap3 based board, looks like decompress_kernel
will trash the first 16 bytes of the device tree data somehow.
John, can you please check if your current patch works properly?
Regards,
Tony
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCHv3] ARM:boot:device tree: Allow the device tree binary to be appended to zImage
2011-04-13 14:00 ` Tony Lindgren
@ 2011-04-20 5:47 ` Shawn Guo
2011-04-20 7:34 ` Shawn Guo
0 siblings, 1 reply; 36+ messages in thread
From: Shawn Guo @ 2011-04-20 5:47 UTC (permalink / raw)
To: linux-arm-kernel
Hi Tony,
On Wed, Apr 13, 2011 at 05:00:30PM +0300, Tony Lindgren wrote:
> * Shawn Guo <shawn.guo@freescale.com> [110328 12:10]:
> > On Thu, Mar 24, 2011 at 04:18:41PM -0700, John Bonesio wrote:
> > > This patch provides the ability to boot using a device tree that is appended
> > > to the raw binary zImage (e.g. cat zImage <filename>.dtb > zImage_w_dtb).
> > >
> > > Signed-off-by: John Bonesio <bones@secretlab.ca>
> > > ---
> > >
> > Tested on mx51 babbage. The non-dt kernel works fine, but dt kernel
> > does not.
> >
> > I tracked it down a little bit and found, before calling kernel from
> > arch/arm/boot/compressed/head.S, the dtb magic number (at address
> > pointed by r8) was interpolated to 0x73fbc000 which happens to be the
> > value of 'uart_base' assigned in plat-mxc/include/mach/uncompress.h.
>
I got it working by remove the 'static' from uart_base definition.
> Gave this a try on an omap3 based board, looks like decompress_kernel
> will trash the first 16 bytes of the device tree data somehow.
>
16 bytes corruption is a little beyond my guess. I expect only 8
bytes corruption on omap3. Can you please remove the 'static' in
uncompress.h to see if it helps?
--- a/arch/arm/plat-omap/include/plat/uncompress.h
+++ b/arch/arm/plat-omap/include/plat/uncompress.h
@@ -27,8 +27,8 @@
#define MDR1_MODE_MASK 0x07
-static volatile u8 *uart_base;
-static int uart_shift;
+volatile u8 *uart_base;
+int uart_shift;
---
Here is how I tracked it down with help from Dave Martin and Nicolas
Pitre.
http://lists.linaro.org/pipermail/linaro-kernel/2011-April/000259.html
With the comments given by Nicolas below, this seems a global issue in
platform uncompress.h.
--- quote begins ---
You must not use static variable in the decompressor. For one thing,
that breaks the ability to XIP the decompressor code and move writable
data elsewhere.
So the fix is indeed to _not_ declare any global variable as static in
this case.
--- quote ends ---
I would like to confirm it fixes your setup before I send fixing
patch to remove 'static' from global variables in all platform
uncompress.h.
--
Regards,
Shawn
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCHv3] ARM:boot:device tree: Allow the device tree binary to be appended to zImage
2011-04-20 5:47 ` Shawn Guo
@ 2011-04-20 7:34 ` Shawn Guo
2011-04-21 8:02 ` Tony Lindgren
0 siblings, 1 reply; 36+ messages in thread
From: Shawn Guo @ 2011-04-20 7:34 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Apr 20, 2011 at 01:47:47PM +0800, Shawn Guo wrote:
> I would like to confirm it fixes your setup before I send fixing
> patch to remove 'static' from global variables in all platform
> uncompress.h.
>
Just saw Nico's patch, so ignore above lines ...
--
Regards,
Shawn
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCHv3] ARM:boot:device tree: Allow the device tree binary to be appended to zImage
2011-04-20 7:34 ` Shawn Guo
@ 2011-04-21 8:02 ` Tony Lindgren
2011-04-21 12:46 ` Tony Lindgren
0 siblings, 1 reply; 36+ messages in thread
From: Tony Lindgren @ 2011-04-21 8:02 UTC (permalink / raw)
To: linux-arm-kernel
* Shawn Guo <shawn.guo@freescale.com> [110420 00:27]:
> On Wed, Apr 20, 2011 at 01:47:47PM +0800, Shawn Guo wrote:
> > I would like to confirm it fixes your setup before I send fixing
> > patch to remove 'static' from global variables in all platform
> > uncompress.h.
> >
> Just saw Nico's patch, so ignore above lines ...
Thanks looks like removing static only changes things so now
"Warning: Neither atags nor dtb found" error message printed out.
Regards,
Tony
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCHv3] ARM:boot:device tree: Allow the device tree binary to be appended to zImage
2011-04-21 8:02 ` Tony Lindgren
@ 2011-04-21 12:46 ` Tony Lindgren
2011-04-27 14:23 ` Tony Lindgren
0 siblings, 1 reply; 36+ messages in thread
From: Tony Lindgren @ 2011-04-21 12:46 UTC (permalink / raw)
To: linux-arm-kernel
* Tony Lindgren <tony@atomide.com> [110421 11:00]:
> * Shawn Guo <shawn.guo@freescale.com> [110420 00:27]:
> > On Wed, Apr 20, 2011 at 01:47:47PM +0800, Shawn Guo wrote:
> > > I would like to confirm it fixes your setup before I send fixing
> > > patch to remove 'static' from global variables in all platform
> > > uncompress.h.
> > >
> > Just saw Nico's patch, so ignore above lines ...
>
> Thanks looks like removing static only changes things so now
> "Warning: Neither atags nor dtb found" error message printed out.
But also Nico's patch fixes the corruption of the DT data during
uncompress, so I'm getting much further now.
Now the problem is that the DT data is zeroed out by the time code
gets to setup_machine_fdt. I wonder if I'm missing some patch?
I'm using grant's devicetree/arm branch + DT append patch + Nico's
static fix + my relocate fix.
Oh well have to look at it again next week..
Tony
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCHv3] ARM:boot:device tree: Allow the device tree binary to be appended to zImage
2011-04-21 12:46 ` Tony Lindgren
@ 2011-04-27 14:23 ` Tony Lindgren
2011-04-27 14:38 ` Tony Lindgren
2011-04-27 14:40 ` Nicolas Pitre
0 siblings, 2 replies; 36+ messages in thread
From: Tony Lindgren @ 2011-04-27 14:23 UTC (permalink / raw)
To: linux-arm-kernel
* Tony Lindgren <tony@atomide.com> [110421 05:44]:
> * Tony Lindgren <tony@atomide.com> [110421 11:00]:
> > * Shawn Guo <shawn.guo@freescale.com> [110420 00:27]:
> > > On Wed, Apr 20, 2011 at 01:47:47PM +0800, Shawn Guo wrote:
> > > > I would like to confirm it fixes your setup before I send fixing
> > > > patch to remove 'static' from global variables in all platform
> > > > uncompress.h.
> > > >
> > > Just saw Nico's patch, so ignore above lines ...
> >
> > Thanks looks like removing static only changes things so now
> > "Warning: Neither atags nor dtb found" error message printed out.
>
> But also Nico's patch fixes the corruption of the DT data during
> uncompress, so I'm getting much further now.
>
> Now the problem is that the DT data is zeroed out by the time code
> gets to setup_machine_fdt. I wonder if I'm missing some patch?
>
> I'm using grant's devicetree/arm branch + DT append patch + Nico's
> static fix + my relocate fix.
>
> Oh well have to look at it again next week..
I guess the issue is that when setup_machine_fdt gets called we only
have minimal MMU mapping done.
So I guess there's some other patch related to it?
Tony
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCHv3] ARM:boot:device tree: Allow the device tree binary to be appended to zImage
2011-04-27 14:23 ` Tony Lindgren
@ 2011-04-27 14:38 ` Tony Lindgren
2011-04-27 14:40 ` Nicolas Pitre
1 sibling, 0 replies; 36+ messages in thread
From: Tony Lindgren @ 2011-04-27 14:38 UTC (permalink / raw)
To: linux-arm-kernel
* Tony Lindgren <tony@atomide.com> [110427 07:20]:
>
> I guess the issue is that when setup_machine_fdt gets called we only
> have minimal MMU mapping done.
>
> So I guess there's some other patch related to it?
Hmm head.S should already map the boot params address in r2..
Looks like more debugging is needed.
Tony
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCHv3] ARM:boot:device tree: Allow the device tree binary to be appended to zImage
2011-04-27 14:23 ` Tony Lindgren
2011-04-27 14:38 ` Tony Lindgren
@ 2011-04-27 14:40 ` Nicolas Pitre
2011-04-27 14:43 ` Tony Lindgren
1 sibling, 1 reply; 36+ messages in thread
From: Nicolas Pitre @ 2011-04-27 14:40 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, 27 Apr 2011, Tony Lindgren wrote:
> * Tony Lindgren <tony@atomide.com> [110421 05:44]:
> > * Tony Lindgren <tony@atomide.com> [110421 11:00]:
> > > * Shawn Guo <shawn.guo@freescale.com> [110420 00:27]:
> > > > On Wed, Apr 20, 2011 at 01:47:47PM +0800, Shawn Guo wrote:
> > > > > I would like to confirm it fixes your setup before I send fixing
> > > > > patch to remove 'static' from global variables in all platform
> > > > > uncompress.h.
> > > > >
> > > > Just saw Nico's patch, so ignore above lines ...
> > >
> > > Thanks looks like removing static only changes things so now
> > > "Warning: Neither atags nor dtb found" error message printed out.
> >
> > But also Nico's patch fixes the corruption of the DT data during
> > uncompress, so I'm getting much further now.
> >
> > Now the problem is that the DT data is zeroed out by the time code
> > gets to setup_machine_fdt. I wonder if I'm missing some patch?
> >
> > I'm using grant's devicetree/arm branch + DT append patch + Nico's
> > static fix + my relocate fix.
> >
> > Oh well have to look at it again next week..
>
> I guess the issue is that when setup_machine_fdt gets called we only
> have minimal MMU mapping done.
In head.S (the final kernel image one) there is a 1MB mapping
established when r2 contains a valid ATAG/DTB pointer. See commit
4d901c42 (you even ACK'd it).
Nicolas
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCHv3] ARM:boot:device tree: Allow the device tree binary to be appended to zImage
2011-04-27 14:40 ` Nicolas Pitre
@ 2011-04-27 14:43 ` Tony Lindgren
2011-04-29 10:26 ` Tony Lindgren
0 siblings, 1 reply; 36+ messages in thread
From: Tony Lindgren @ 2011-04-27 14:43 UTC (permalink / raw)
To: linux-arm-kernel
* Nicolas Pitre <nico@fluxnic.net> [110427 07:37]:
> On Wed, 27 Apr 2011, Tony Lindgren wrote:
> >
> > I guess the issue is that when setup_machine_fdt gets called we only
> > have minimal MMU mapping done.
>
> In head.S (the final kernel image one) there is a 1MB mapping
> established when r2 contains a valid ATAG/DTB pointer. See commit
> 4d901c42 (you even ACK'd it).
Yeah thanks was just looking at that too :) Need to debug further..
Tony
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCHv3] ARM:boot:device tree: Allow the device tree binary to be appended to zImage
2011-04-27 14:43 ` Tony Lindgren
@ 2011-04-29 10:26 ` Tony Lindgren
2011-04-29 13:02 ` Grant Likely
2011-04-29 13:16 ` Nicolas Pitre
0 siblings, 2 replies; 36+ messages in thread
From: Tony Lindgren @ 2011-04-29 10:26 UTC (permalink / raw)
To: linux-arm-kernel
* Tony Lindgren <tony@atomide.com> [110427 07:41]:
> * Nicolas Pitre <nico@fluxnic.net> [110427 07:37]:
> > On Wed, 27 Apr 2011, Tony Lindgren wrote:
> > >
> > > I guess the issue is that when setup_machine_fdt gets called we only
> > > have minimal MMU mapping done.
> >
> > In head.S (the final kernel image one) there is a 1MB mapping
> > established when r2 contains a valid ATAG/DTB pointer. See commit
> > 4d901c42 (you even ACK'd it).
>
> Yeah thanks was just looking at that too :) Need to debug further..
OK figured this one out. Looks like we have an issue where kernel BSS
can easily overlap the appended DT data. Then kernel __mmap_switched
will clear the BSS and DT data.
This happens for example with omap2plus_defconfig where we have:
$ stat -c "%s" arch/arm/boot/Image
6405100
$ size kernel/built-in.o
text data bss dec hex filename
660576 110852 5475500 6246928 5f5210 kernel/built-in.o
If the compressed image is smaller than BSS, then we end up
having DT data in the BSS area. In this case the compressed
image is about 2.3 MB for LZMA.
The uncompress code does not know about the kernel BSS,
and does not necessarily relocate anything depending on the
compressed image load address.
So in which code do we want to relocate the DT data?
We could do it based on estimated BSS size in uncompress code,
or based on the real BSS size in __mmap_switched before BSS
gets reset.
Cheers,
Tony
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCHv3] ARM:boot:device tree: Allow the device tree binary to be appended to zImage
2011-04-29 10:26 ` Tony Lindgren
@ 2011-04-29 13:02 ` Grant Likely
2011-04-29 13:08 ` Grant Likely
2011-04-29 13:09 ` Tony Lindgren
2011-04-29 13:16 ` Nicolas Pitre
1 sibling, 2 replies; 36+ messages in thread
From: Grant Likely @ 2011-04-29 13:02 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Apr 29, 2011 at 4:26 AM, Tony Lindgren <tony@atomide.com> wrote:
> * Tony Lindgren <tony@atomide.com> [110427 07:41]:
>> * Nicolas Pitre <nico@fluxnic.net> [110427 07:37]:
>> > On Wed, 27 Apr 2011, Tony Lindgren wrote:
>> > >
>> > > I guess the issue is that when setup_machine_fdt gets called we only
>> > > have minimal MMU mapping done.
>> >
>> > In head.S (the final kernel image one) there is a 1MB mapping
>> > established when r2 contains a valid ATAG/DTB pointer. ?See commit
>> > 4d901c42 (you even ACK'd it).
>>
>> Yeah thanks was just looking at that too :) Need to debug further..
>
> OK figured this one out. Looks like we have an issue where kernel BSS
> can easily overlap the appended DT data. Then kernel __mmap_switched
> will clear the BSS and DT data.
>
> This happens for example with omap2plus_defconfig where we have:
>
> $ stat -c "%s" arch/arm/boot/Image
> 6405100
>
> $ size kernel/built-in.o
> ? text ? ?data ? ? bss ? ? dec ? ? hex filename
> ?660576 ?110852 5475500 6246928 ?5f5210 kernel/built-in.o
>
> If the compressed image is smaller than BSS, then we end up
> having DT data in the BSS area. In this case the compressed
> image is about 2.3 MB for LZMA.
>
> The uncompress code does not know about the kernel BSS,
> and does not necessarily relocate anything depending on the
> compressed image load address.
>
> So in which code do we want to relocate the DT data?
>
> We could do it based on estimated BSS size in uncompress code,
> or based on the real BSS size in __mmap_switched before BSS
> gets reset.
How about telling the wrapper what the final size will be by scraping
the __bss_end value out of System.map?
g.
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCHv3] ARM:boot:device tree: Allow the device tree binary to be appended to zImage
2011-04-29 13:02 ` Grant Likely
@ 2011-04-29 13:08 ` Grant Likely
2011-04-29 13:09 ` Tony Lindgren
1 sibling, 0 replies; 36+ messages in thread
From: Grant Likely @ 2011-04-29 13:08 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Apr 29, 2011 at 7:02 AM, Grant Likely <grant.likely@secretlab.ca> wrote:
> On Fri, Apr 29, 2011 at 4:26 AM, Tony Lindgren <tony@atomide.com> wrote:
>> We could do it based on estimated BSS size in uncompress code,
>> or based on the real BSS size in __mmap_switched before BSS
>> gets reset.
>
> How about telling the wrapper what the final size will be by scraping
> the __bss_end value out of System.map?
er, make that __bss_stop
g.
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCHv3] ARM:boot:device tree: Allow the device tree binary to be appended to zImage
2011-04-29 13:02 ` Grant Likely
2011-04-29 13:08 ` Grant Likely
@ 2011-04-29 13:09 ` Tony Lindgren
2011-04-29 13:21 ` Nicolas Pitre
1 sibling, 1 reply; 36+ messages in thread
From: Tony Lindgren @ 2011-04-29 13:09 UTC (permalink / raw)
To: linux-arm-kernel
* Grant Likely <grant.likely@secretlab.ca> [110429 05:59]:
> On Fri, Apr 29, 2011 at 4:26 AM, Tony Lindgren <tony@atomide.com> wrote:
> > * Tony Lindgren <tony@atomide.com> [110427 07:41]:
> >
> > If the compressed image is smaller than BSS, then we end up
> > having DT data in the BSS area. In this case the compressed
> > image is about 2.3 MB for LZMA.
> >
> > The uncompress code does not know about the kernel BSS,
> > and does not necessarily relocate anything depending on the
> > compressed image load address.
> >
> > So in which code do we want to relocate the DT data?
> >
> > We could do it based on estimated BSS size in uncompress code,
> > or based on the real BSS size in __mmap_switched before BSS
> > gets reset.
>
> How about telling the wrapper what the final size will be by scraping
> the __bss_end value out of System.map?
That sounds doable to me. It should probably be done in the uncompress
code so the kernel code can stay generic for the DT data handling.
BTW, with the relocation of DT data we may end up overwriting
an initrd, but I guess bootloaders typically place the initrd to
the end of the memory.
Regards,
Tony
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCHv3] ARM:boot:device tree: Allow the device tree binary to be appended to zImage
2011-04-29 10:26 ` Tony Lindgren
2011-04-29 13:02 ` Grant Likely
@ 2011-04-29 13:16 ` Nicolas Pitre
2011-04-29 13:53 ` Russell King - ARM Linux
1 sibling, 1 reply; 36+ messages in thread
From: Nicolas Pitre @ 2011-04-29 13:16 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, 29 Apr 2011, Tony Lindgren wrote:
> If the compressed image is smaller than BSS, then we end up
> having DT data in the BSS area. In this case the compressed
> image is about 2.3 MB for LZMA.
>
> The uncompress code does not know about the kernel BSS,
> and does not necessarily relocate anything depending on the
> compressed image load address.
>
> So in which code do we want to relocate the DT data?
>
> We could do it based on estimated BSS size in uncompress code,
> or based on the real BSS size in __mmap_switched before BSS
> gets reset.
Estimations for that kind of thing is always bound to create problems
some day.
The DT data should probably be moved out of the way from
arch/arm/kernel/head.S before the .bss is cleared, and even before
enabling the MMU, like in __vet_atags.
Nicolas
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCHv3] ARM:boot:device tree: Allow the device tree binary to be appended to zImage
2011-04-29 13:09 ` Tony Lindgren
@ 2011-04-29 13:21 ` Nicolas Pitre
0 siblings, 0 replies; 36+ messages in thread
From: Nicolas Pitre @ 2011-04-29 13:21 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, 29 Apr 2011, Tony Lindgren wrote:
> * Grant Likely <grant.likely@secretlab.ca> [110429 05:59]:
> > On Fri, Apr 29, 2011 at 4:26 AM, Tony Lindgren <tony@atomide.com> wrote:
> > > * Tony Lindgren <tony@atomide.com> [110427 07:41]:
> > >
> > > If the compressed image is smaller than BSS, then we end up
> > > having DT data in the BSS area. In this case the compressed
> > > image is about 2.3 MB for LZMA.
> > >
> > > The uncompress code does not know about the kernel BSS,
> > > and does not necessarily relocate anything depending on the
> > > compressed image load address.
> > >
> > > So in which code do we want to relocate the DT data?
> > >
> > > We could do it based on estimated BSS size in uncompress code,
> > > or based on the real BSS size in __mmap_switched before BSS
> > > gets reset.
> >
> > How about telling the wrapper what the final size will be by scraping
> > the __bss_end value out of System.map?
>
> That sounds doable to me. It should probably be done in the uncompress
> code so the kernel code can stay generic for the DT data handling.
Well, that's probably doable too. If the final kernel's .bss conflicts
with the DT data, then moving the DT data above the malloc space in the
decompressor is trivial too.
Nicolas
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCHv3] ARM:boot:device tree: Allow the device tree binary to be appended to zImage
2011-04-29 13:16 ` Nicolas Pitre
@ 2011-04-29 13:53 ` Russell King - ARM Linux
2011-04-29 19:14 ` Nicolas Pitre
0 siblings, 1 reply; 36+ messages in thread
From: Russell King - ARM Linux @ 2011-04-29 13:53 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Apr 29, 2011 at 09:16:39AM -0400, Nicolas Pitre wrote:
> On Fri, 29 Apr 2011, Tony Lindgren wrote:
>
> > If the compressed image is smaller than BSS, then we end up
> > having DT data in the BSS area. In this case the compressed
> > image is about 2.3 MB for LZMA.
> >
> > The uncompress code does not know about the kernel BSS,
> > and does not necessarily relocate anything depending on the
> > compressed image load address.
> >
> > So in which code do we want to relocate the DT data?
> >
> > We could do it based on estimated BSS size in uncompress code,
> > or based on the real BSS size in __mmap_switched before BSS
> > gets reset.
>
> Estimations for that kind of thing is always bound to create problems
> some day.
>
> The DT data should probably be moved out of the way from
> arch/arm/kernel/head.S before the .bss is cleared, and even before
> enabling the MMU, like in __vet_atags.
Err, no. Moving stuff around becomes quite expensive when the cache is
not on. It's far better to work out where to place it first time around
so its not in the way.
Remember that there is a section of the community which cares deeply about
boot time and has already put quite some resources into sorting that out.
CELF springs to mind.
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCHv3] ARM:boot:device tree: Allow the device tree binary to be appended to zImage
2011-04-29 13:53 ` Russell King - ARM Linux
@ 2011-04-29 19:14 ` Nicolas Pitre
2011-05-04 7:23 ` Tony Lindgren
0 siblings, 1 reply; 36+ messages in thread
From: Nicolas Pitre @ 2011-04-29 19:14 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, 29 Apr 2011, Russell King - ARM Linux wrote:
> On Fri, Apr 29, 2011 at 09:16:39AM -0400, Nicolas Pitre wrote:
> > On Fri, 29 Apr 2011, Tony Lindgren wrote:
> >
> > > If the compressed image is smaller than BSS, then we end up
> > > having DT data in the BSS area. In this case the compressed
> > > image is about 2.3 MB for LZMA.
> > >
> > > The uncompress code does not know about the kernel BSS,
> > > and does not necessarily relocate anything depending on the
> > > compressed image load address.
> > >
> > > So in which code do we want to relocate the DT data?
> > >
> > > We could do it based on estimated BSS size in uncompress code,
> > > or based on the real BSS size in __mmap_switched before BSS
> > > gets reset.
> >
> > Estimations for that kind of thing is always bound to create problems
> > some day.
> >
> > The DT data should probably be moved out of the way from
> > arch/arm/kernel/head.S before the .bss is cleared, and even before
> > enabling the MMU, like in __vet_atags.
>
> Err, no. Moving stuff around becomes quite expensive when the cache is
> not on. It's far better to work out where to place it first time around
> so its not in the way.
I don't think the DT data is that huge, but that's a point in favor of
doing it in the zImage code. We'll just need to feed the total size of
the uncompressed kernel .bss section to zImage when compiling it.
Nicolas
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCHv3] ARM:boot:device tree: Allow the device tree binary to be appended to zImage
2011-04-29 19:14 ` Nicolas Pitre
@ 2011-05-04 7:23 ` Tony Lindgren
2011-05-04 13:12 ` Tony Lindgren
2011-05-04 13:38 ` Nicolas Pitre
0 siblings, 2 replies; 36+ messages in thread
From: Tony Lindgren @ 2011-05-04 7:23 UTC (permalink / raw)
To: linux-arm-kernel
* Nicolas Pitre <nico@fluxnic.net> [110429 12:11]:
> On Fri, 29 Apr 2011, Russell King - ARM Linux wrote:
>
> > On Fri, Apr 29, 2011 at 09:16:39AM -0400, Nicolas Pitre wrote:
> > > On Fri, 29 Apr 2011, Tony Lindgren wrote:
> > >
> > > > If the compressed image is smaller than BSS, then we end up
> > > > having DT data in the BSS area. In this case the compressed
> > > > image is about 2.3 MB for LZMA.
> > > >
> > > > The uncompress code does not know about the kernel BSS,
> > > > and does not necessarily relocate anything depending on the
> > > > compressed image load address.
> > > >
> > > > So in which code do we want to relocate the DT data?
> > > >
> > > > We could do it based on estimated BSS size in uncompress code,
> > > > or based on the real BSS size in __mmap_switched before BSS
> > > > gets reset.
> > >
> > > Estimations for that kind of thing is always bound to create problems
> > > some day.
> > >
> > > The DT data should probably be moved out of the way from
> > > arch/arm/kernel/head.S before the .bss is cleared, and even before
> > > enabling the MMU, like in __vet_atags.
> >
> > Err, no. Moving stuff around becomes quite expensive when the cache is
> > not on. It's far better to work out where to place it first time around
> > so its not in the way.
>
> I don't think the DT data is that huge, but that's a point in favor of
> doing it in the zImage code. We'll just need to feed the total size of
> the uncompressed kernel .bss section to zImage when compiling it.
One more thing to consider though.. I don't think we want to copy the
DT data twice. It's not big right now, but could get large if we pass
all the clocks in it.
So this should be probably fixed in the original patch.. John got
any thoughts on that?
Regards,
Tony
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCHv3] ARM:boot:device tree: Allow the device tree binary to be appended to zImage
2011-05-04 7:23 ` Tony Lindgren
@ 2011-05-04 13:12 ` Tony Lindgren
2011-05-04 13:38 ` Nicolas Pitre
1 sibling, 0 replies; 36+ messages in thread
From: Tony Lindgren @ 2011-05-04 13:12 UTC (permalink / raw)
To: linux-arm-kernel
* Tony Lindgren <tony@atomide.com> [110504 00:20]:
> * Nicolas Pitre <nico@fluxnic.net> [110429 12:11]:
> > >
> > > Err, no. Moving stuff around becomes quite expensive when the cache is
> > > not on. It's far better to work out where to place it first time around
> > > so its not in the way.
> >
> > I don't think the DT data is that huge, but that's a point in favor of
> > doing it in the zImage code. We'll just need to feed the total size of
> > the uncompressed kernel .bss section to zImage when compiling it.
>
> One more thing to consider though.. I don't think we want to copy the
> DT data twice. It's not big right now, but could get large if we pass
> all the clocks in it.
>
> So this should be probably fixed in the original patch.. John got
> any thoughts on that?
Hmm actually the the easy fix is to relocate both the compressed kernel
and DT data with one copy to:
max(inflated_kernel_end + kernel_bss_size - compressed_kernel_size,
inflated_kernel_end)
So no need to relocate them separately or copy twice.
Tony
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCHv3] ARM:boot:device tree: Allow the device tree binary to be appended to zImage
2011-05-04 7:23 ` Tony Lindgren
2011-05-04 13:12 ` Tony Lindgren
@ 2011-05-04 13:38 ` Nicolas Pitre
2011-05-09 11:19 ` [PATCH] ARM: Make sure appended device tree data won't overlap kernel BSS Tony Lindgren
1 sibling, 1 reply; 36+ messages in thread
From: Nicolas Pitre @ 2011-05-04 13:38 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, 4 May 2011, Tony Lindgren wrote:
> * Nicolas Pitre <nico@fluxnic.net> [110429 12:11]:
> > I don't think the DT data is that huge, but that's a point in favor of
> > doing it in the zImage code. We'll just need to feed the total size of
> > the uncompressed kernel .bss section to zImage when compiling it.
>
> One more thing to consider though.. I don't think we want to copy the
> DT data twice. It's not big right now, but could get large if we pass
> all the clocks in it.
>
> So this should be probably fixed in the original patch.. John got
> any thoughts on that?
No. The original patch is good (and complex) enough as it is now. It
already required 3 rounds of reviews before it was right. This should be
fixed in a subsequent patch.
Nicolas
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH] ARM: Make sure appended device tree data won't overlap kernel BSS
2011-05-04 13:38 ` Nicolas Pitre
@ 2011-05-09 11:19 ` Tony Lindgren
2011-05-09 14:49 ` Tony Lindgren
0 siblings, 1 reply; 36+ messages in thread
From: Tony Lindgren @ 2011-05-09 11:19 UTC (permalink / raw)
To: linux-arm-kernel
Do this before relocating the compressed kernel + device tree data.
Otherwise we would have to split the copying into two parts, or copy
the device tree data twice.
As we only have one register available, pass the size of kernel BSS
via linker and do the following calculation using r1.
Signed-off-by: Tony Lindgren <tony@atomide.com>
--- a/arch/arm/boot/compressed/Makefile
+++ b/arch/arm/boot/compressed/Makefile
@@ -98,6 +98,9 @@ endif
ccflags-y := -fpic -fno-builtin
asflags-y := -Wa,-march=all
+# Supply kernel BSS size to the decompressor via a linker symbol.
+KBSS_SZ = $(shell size $(obj)/../../../../vmlinux | awk 'END{print $$3}')
+LDFLAGS_vmlinux = --defsym _kbss_sz=$(KBSS_SZ)
# Supply ZRELADDR to the decompressor via a linker symbol.
ifneq ($(CONFIG_AUTO_ZRELADDR),y)
LDFLAGS_vmlinux += --defsym zreladdr=$(ZRELADDR)
--- a/arch/arm/boot/compressed/head.S
+++ b/arch/arm/boot/compressed/head.S
@@ -273,17 +273,35 @@ dtb_check_done:
/*
* Check to see if we will overwrite ourselves.
+ * r1 = corrupted
* r4 = final kernel address
+ * r5 = start of this image
+ * r6 = _edata
* r9 = size of decompressed image
* r10 = end of this image, including bss/stack/malloc space if non XIP
* We basically want:
* r4 - 16k page directory >= r10 -> OK
* r4 + image length <= current position (pc) -> OK
+ * For the appended device tree case, check that the device tree data does
+ * not overlap the kernel BSS area.
*/
add r10, r10, #16384
cmp r4, r10
bhs wont_overwrite
add r10, r4, r9
+#if defined(CONFIG_ARM_APPENDED_DTB)
+ cmp lr, #0
+ beq no_kbss_check
+ adr r1, kbss
+ ldr r1, [r1, #0] @ kernel BSS size, _kbss_sz
+ add r1, r10, r1 @ inflated kernel end + kbss
+ sub r1, r1, r5 @ minus start of this data
+ add r1, r1, r6 @ plus end of this data
+ sub r1, r1, lr @ minus size of DT data
+ cmp r10, r1 @ DT start < kernel BSS end?
+ movlt r10, r1 @ yes, move past kernel BSS end
+no_kbss_check:
+#endif
cmp r10, pc
bls wont_overwrite
@@ -331,6 +349,8 @@ dtb_check_done:
add r0, r0, r6
mov pc, r0
+kbss: .word _kbss_sz
+
wont_overwrite:
/*
* If delta is zero, we are running@the address we were linked at.
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCHv3] ARM:boot:device tree: Allow the device tree binary to be appended to zImage
2011-03-24 23:18 [PATCHv3] ARM:boot:device tree: Allow the device tree binary to be appended to zImage John Bonesio
2011-03-24 23:37 ` Nicolas Pitre
2011-03-28 9:13 ` Shawn Guo
@ 2011-05-09 11:23 ` Tony Lindgren
2 siblings, 0 replies; 36+ messages in thread
From: Tony Lindgren @ 2011-05-09 11:23 UTC (permalink / raw)
To: linux-arm-kernel
* John Bonesio <bones@secretlab.ca> [110324 16:17]:
> This patch provides the ability to boot using a device tree that is appended
> to the raw binary zImage (e.g. cat zImage <filename>.dtb > zImage_w_dtb).
I suggest adding something like this to the description:
Note that a follow-up patch is needed to add checking for
kernel BSS overlapping the device tree data.
Other than that, I got /proc/device-tree entries showing up with
this patch + the uncompress fixes + the kernel BSS check I posted
as a reply to this thread, so:
Tested-by: Tony Lindgren <tony@atomide.com>
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH] ARM: Make sure appended device tree data won't overlap kernel BSS
2011-05-09 11:19 ` [PATCH] ARM: Make sure appended device tree data won't overlap kernel BSS Tony Lindgren
@ 2011-05-09 14:49 ` Tony Lindgren
2011-05-12 12:59 ` Tony Lindgren
0 siblings, 1 reply; 36+ messages in thread
From: Tony Lindgren @ 2011-05-09 14:49 UTC (permalink / raw)
To: linux-arm-kernel
* Tony Lindgren <tony@atomide.com> [110509 14:16]:
> Do this before relocating the compressed kernel + device tree data.
> Otherwise we would have to split the copying into two parts, or copy
> the device tree data twice.
>
> As we only have one register available, pass the size of kernel BSS
> via linker and do the following calculation using r1.
Blah, this still needs a bit more work.. The calculation is wrong
and we can get multiple relocations now.
Tony
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH] ARM: Make sure appended device tree data won't overlap kernel BSS
2011-05-09 14:49 ` Tony Lindgren
@ 2011-05-12 12:59 ` Tony Lindgren
2011-05-13 7:39 ` Nicolas Pitre
0 siblings, 1 reply; 36+ messages in thread
From: Tony Lindgren @ 2011-05-12 12:59 UTC (permalink / raw)
To: linux-arm-kernel
* Tony Lindgren <tony@atomide.com> [110509 07:47]:
> * Tony Lindgren <tony@atomide.com> [110509 14:16]:
> > Do this before relocating the compressed kernel + device tree data.
> > Otherwise we would have to split the copying into two parts, or copy
> > the device tree data twice.
> >
> > As we only have one register available, pass the size of kernel BSS
> > via linker and do the following calculation using r1.
>
> Blah, this still needs a bit more work.. The calculation is wrong
> and we can get multiple relocations now.
Here's a better version that also makes the stack usable early.
That might become handy for further changes.
Will still do a bit more testing on this on Friday.
Tony
From: Tony Lindgren <tony@atomide.com>
Date: Thu, 12 May 2011 05:29:49 -0700
Subject: [PATCH] ARM: Make sure appended device tree data won't overlap kernel BSS
Do this before relocating the compressed kernel + device tree data.
Otherwise we would have to split the copying into two parts, or copy
the device tree data twice.
As we only have one register available, pass the size of kernel BSS
via linker and do the calculation using r1, then save it to the stack.
Note that this patch now makes the stack also usable earlier for
CONFIG_ARM_APPENDED_DTB.
Not-Yet-Signed-off-by: Tony Lindgren <tony@atomide.com>
--- a/arch/arm/boot/compressed/Makefile
+++ b/arch/arm/boot/compressed/Makefile
@@ -98,6 +98,9 @@ endif
ccflags-y := -fpic -fno-builtin
asflags-y := -Wa,-march=all
+# Supply kernel BSS size to the decompressor via a linker symbol.
+KBSS_SZ = $(shell size $(obj)/../../../../vmlinux | awk 'END{print $$3}')
+LDFLAGS_vmlinux = --defsym _kbss_sz=$(KBSS_SZ)
# Supply ZRELADDR to the decompressor via a linker symbol.
ifneq ($(CONFIG_AUTO_ZRELADDR),y)
LDFLAGS_vmlinux += --defsym zreladdr=$(ZRELADDR)
--- a/arch/arm/boot/compressed/head.S
+++ b/arch/arm/boot/compressed/head.S
@@ -266,6 +266,25 @@ restart: adr r0, LC0
add lr, lr, #7
bic lr, lr, #7
+ /*
+ * Compensate for the appended device tree and make stack
+ * usable. Note if the linker script changes so the stack is
+ * not after the bss section, this code will be wrong.
+ */
+ add sp, sp, lr
+
+ /*
+ * Calculate and save the offset between kernel BSS end and
+ * device tree data start for later use to check they won't
+ * overlap.
+ */
+ adr r1, kbss_sz
+ ldr r1, [r1, #0] @ kernel BSS size
+ add r1, r1, r4 @ add inflated kernel start
+ add r1, r1, r9 @ add inflated kernel size
+ sub r1, r1, r6 @ kbss end - dt start
+ str r1, [sp, #0] @ save offset into stack
+
add r10, r10, lr
add r6, r6, lr
dtb_check_done:
@@ -273,17 +292,29 @@ dtb_check_done:
/*
* Check to see if we will overwrite ourselves.
+ * r1 = corrupted
* r4 = final kernel address
* r9 = size of decompressed image
* r10 = end of this image, including bss/stack/malloc space if non XIP
* We basically want:
* r4 - 16k page directory >= r10 -> OK
* r4 + image length <= current position (pc) -> OK
+ * For the appended device tree case, check that the device tree data does
+ * not overlap the kernel BSS area.
*/
add r10, r10, #16384
cmp r4, r10
bhs wont_overwrite
add r10, r4, r9
+#if defined(CONFIG_ARM_APPENDED_DTB)
+ cmp lr, #0 @ device tree appended?
+ beq no_kbss_check @ no, skip check
+ ldr r1, [sp, #0] @ get kbss offset
+ add r1, r10, r1 @ inflated end + kbss offset
+ cmp r10, r1 @ DT start < kernel BSS end?
+ movlt r10, r1 @ yes, move past kernel BSS end
+no_kbss_check:
+#endif
cmp r10, pc
bls wont_overwrite
@@ -331,6 +362,10 @@ dtb_check_done:
add r0, r0, r6
mov pc, r0
+#ifdef CONFIG_ARM_APPENDED_DTB
+kbss_sz: .word _kbss_sz @ kernel BSS size
+#endif
+
wont_overwrite:
/*
* If delta is zero, we are running@the address we were linked at.
@@ -376,13 +411,6 @@ wont_overwrite:
add r2, r2, lr
add r3, r3, lr
- /*
- * bump the stack pinter
- *
- * If the linker script changes so the stack is not after
- * the bss section, this code will be wrong.
- */
- add sp, sp, lr
#else
/*
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH] ARM: Make sure appended device tree data won't overlap kernel BSS
2011-05-12 12:59 ` Tony Lindgren
@ 2011-05-13 7:39 ` Nicolas Pitre
2011-05-13 11:21 ` Tony Lindgren
0 siblings, 1 reply; 36+ messages in thread
From: Nicolas Pitre @ 2011-05-13 7:39 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, 12 May 2011, Tony Lindgren wrote:
> * Tony Lindgren <tony@atomide.com> [110509 07:47]:
> > * Tony Lindgren <tony@atomide.com> [110509 14:16]:
> > > Do this before relocating the compressed kernel + device tree data.
> > > Otherwise we would have to split the copying into two parts, or copy
> > > the device tree data twice.
> > >
> > > As we only have one register available, pass the size of kernel BSS
> > > via linker and do the following calculation using r1.
> >
> > Blah, this still needs a bit more work.. The calculation is wrong
> > and we can get multiple relocations now.
>
> Here's a better version that also makes the stack usable early.
> That might become handy for further changes.
>
> Will still do a bit more testing on this on Friday.
>
> Tony
>
>
> From: Tony Lindgren <tony@atomide.com>
> Date: Thu, 12 May 2011 05:29:49 -0700
> Subject: [PATCH] ARM: Make sure appended device tree data won't overlap kernel BSS
>
> Do this before relocating the compressed kernel + device tree data.
> Otherwise we would have to split the copying into two parts, or copy
> the device tree data twice.
>
> As we only have one register available, pass the size of kernel BSS
> via linker and do the calculation using r1, then save it to the stack.
>
> Note that this patch now makes the stack also usable earlier for
> CONFIG_ARM_APPENDED_DTB.
>
> Not-Yet-Signed-off-by: Tony Lindgren <tony@atomide.com>
>
> --- a/arch/arm/boot/compressed/Makefile
> +++ b/arch/arm/boot/compressed/Makefile
> @@ -98,6 +98,9 @@ endif
> ccflags-y := -fpic -fno-builtin
> asflags-y := -Wa,-march=all
>
> +# Supply kernel BSS size to the decompressor via a linker symbol.
> +KBSS_SZ = $(shell size $(obj)/../../../../vmlinux | awk 'END{print $$3}')
> +LDFLAGS_vmlinux = --defsym _kbss_sz=$(KBSS_SZ)
> # Supply ZRELADDR to the decompressor via a linker symbol.
> ifneq ($(CONFIG_AUTO_ZRELADDR),y)
> LDFLAGS_vmlinux += --defsym zreladdr=$(ZRELADDR)
> --- a/arch/arm/boot/compressed/head.S
> +++ b/arch/arm/boot/compressed/head.S
> @@ -266,6 +266,25 @@ restart: adr r0, LC0
> add lr, lr, #7
> bic lr, lr, #7
>
> + /*
> + * Compensate for the appended device tree and make stack
> + * usable. Note if the linker script changes so the stack is
> + * not after the bss section, this code will be wrong.
> + */
> + add sp, sp, lr
> +
> + /*
> + * Calculate and save the offset between kernel BSS end and
> + * device tree data start for later use to check they won't
> + * overlap.
> + */
> + adr r1, kbss_sz
> + ldr r1, [r1, #0] @ kernel BSS size
> + add r1, r1, r4 @ add inflated kernel start
> + add r1, r1, r9 @ add inflated kernel size
> + sub r1, r1, r6 @ kbss end - dt start
> + str r1, [sp, #0] @ save offset into stack
This is actually outside the stack area if you want to be strictly
correct. Should be "str r1, [sp, #-4]!".
Anyway, both this patch tand the DT append patch won't apply or work
correctly anymore due to my latest cleanup series without minor
adjustments.
Nicolas
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH] ARM: Make sure appended device tree data won't overlap kernel BSS
2011-05-13 7:39 ` Nicolas Pitre
@ 2011-05-13 11:21 ` Tony Lindgren
2011-05-13 13:09 ` Nicolas Pitre
2011-06-12 6:14 ` Nicolas Pitre
0 siblings, 2 replies; 36+ messages in thread
From: Tony Lindgren @ 2011-05-13 11:21 UTC (permalink / raw)
To: linux-arm-kernel
* Nicolas Pitre <nico@fluxnic.net> [110513 00:35]:
> On Thu, 12 May 2011, Tony Lindgren wrote:
> >
> > + str r1, [sp, #0] @ save offset into stack
>
> This is actually outside the stack area if you want to be strictly
> correct. Should be "str r1, [sp, #-4]!".
Thanks, yes we should use it like stack now. Updated patch below.
> Anyway, both this patch tand the DT append patch won't apply or work
> correctly anymore due to my latest cleanup series without minor
> adjustments.
OK, no problem. Got those cleanup patches available somewhere?
Regards,
Tony
From: Tony Lindgren <tony@atomide.com>
Date: Thu, 12 May 2011 05:29:49 -0700
Subject: [PATCH] ARM: zImage: Make sure appended device tree data won't overlap kernel BSS
Do this before relocating the compressed kernel + device tree data.
Otherwise we would have to split the copying into two parts, or copy
the device tree data twice.
As we only have one register available, pass the size of kernel BSS
via linker and do the calculation using r1, then save it to the stack.
Note that this patch now makes the stack also usable earlier for
CONFIG_ARM_APPENDED_DTB.
Signed-off-by: Tony Lindgren <tony@atomide.com>
--- a/arch/arm/boot/compressed/Makefile
+++ b/arch/arm/boot/compressed/Makefile
@@ -98,6 +98,9 @@ endif
ccflags-y := -fpic -fno-builtin
asflags-y := -Wa,-march=all
+# Supply kernel BSS size to the decompressor via a linker symbol.
+KBSS_SZ = $(shell size $(obj)/../../../../vmlinux | awk 'END{print $$3}')
+LDFLAGS_vmlinux = --defsym _kbss_sz=$(KBSS_SZ)
# Supply ZRELADDR to the decompressor via a linker symbol.
ifneq ($(CONFIG_AUTO_ZRELADDR),y)
LDFLAGS_vmlinux += --defsym zreladdr=$(ZRELADDR)
--- a/arch/arm/boot/compressed/head.S
+++ b/arch/arm/boot/compressed/head.S
@@ -266,6 +266,25 @@ restart: adr r0, LC0
add lr, lr, #7
bic lr, lr, #7
+ /*
+ * Compensate for the appended device tree and make stack
+ * usable. Note if the linker script changes so the stack is
+ * not after the bss section, this code will be wrong.
+ */
+ add sp, sp, lr
+
+ /*
+ * Calculate and save the offset between kernel BSS end and
+ * device tree data start for later use to check they won't
+ * overlap.
+ */
+ adr r1, kbss_sz
+ ldr r1, [r1, #0] @ kernel BSS size
+ add r1, r1, r4 @ add inflated kernel start
+ add r1, r1, r9 @ add inflated kernel size
+ sub r1, r1, r6 @ kbss end - dt start
+ str r1, [sp, #-4]! @ save offset into stack
+
add r10, r10, lr
add r6, r6, lr
dtb_check_done:
@@ -273,17 +292,29 @@ dtb_check_done:
/*
* Check to see if we will overwrite ourselves.
+ * r1 = corrupted
* r4 = final kernel address
* r9 = size of decompressed image
* r10 = end of this image, including bss/stack/malloc space if non XIP
* We basically want:
* r4 - 16k page directory >= r10 -> OK
* r4 + image length <= current position (pc) -> OK
+ * For the appended device tree case, check that the device tree data does
+ * not overlap the kernel BSS area.
*/
add r10, r10, #16384
cmp r4, r10
bhs wont_overwrite
add r10, r4, r9
+#if defined(CONFIG_ARM_APPENDED_DTB)
+ cmp lr, #0 @ device tree appended?
+ beq no_kbss_check @ no, skip check
+ ldr r1, [sp], #4 @ get kbss offset from stack
+ add r1, r10, r1 @ inflated end + kbss offset
+ cmp r10, r1 @ DT start < kernel BSS end?
+ movlt r10, r1 @ yes, move past kernel BSS end
+no_kbss_check:
+#endif
ARM( cmp r10, pc )
THUMB( mov lr, pc )
THUMB( cmp r10, lr )
@@ -333,6 +364,10 @@ dtb_check_done:
add r0, r0, r6
mov pc, r0
+#ifdef CONFIG_ARM_APPENDED_DTB
+kbss_sz: .word _kbss_sz @ kernel BSS size
+#endif
+
wont_overwrite:
/*
* If delta is zero, we are running@the address we were linked at.
@@ -378,13 +413,6 @@ wont_overwrite:
add r2, r2, lr
add r3, r3, lr
- /*
- * bump the stack pinter
- *
- * If the linker script changes so the stack is not after
- * the bss section, this code will be wrong.
- */
- add sp, sp, lr
#else
/*
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH] ARM: Make sure appended device tree data won't overlap kernel BSS
2011-05-13 11:21 ` Tony Lindgren
@ 2011-05-13 13:09 ` Nicolas Pitre
2011-05-13 13:28 ` Tony Lindgren
2011-06-12 6:14 ` Nicolas Pitre
1 sibling, 1 reply; 36+ messages in thread
From: Nicolas Pitre @ 2011-05-13 13:09 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, 13 May 2011, Tony Lindgren wrote:
> * Nicolas Pitre <nico@fluxnic.net> [110513 00:35]:
> > On Thu, 12 May 2011, Tony Lindgren wrote:
> > >
> > > + str r1, [sp, #0] @ save offset into stack
> >
> > This is actually outside the stack area if you want to be strictly
> > correct. Should be "str r1, [sp, #-4]!".
>
> Thanks, yes we should use it like stack now. Updated patch below.
>
> > Anyway, both this patch tand the DT append patch won't apply or work
> > correctly anymore due to my latest cleanup series without minor
> > adjustments.
>
> OK, no problem. Got those cleanup patches available somewhere?
They,re available in branch 'zImage_fixes' of git://git.linaro.org/people/nico/linux
and in RMK's devel-stable branch as well.
Nicolas
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH] ARM: Make sure appended device tree data won't overlap kernel BSS
2011-05-13 13:09 ` Nicolas Pitre
@ 2011-05-13 13:28 ` Tony Lindgren
2011-06-07 12:43 ` Tony Lindgren
0 siblings, 1 reply; 36+ messages in thread
From: Tony Lindgren @ 2011-05-13 13:28 UTC (permalink / raw)
To: linux-arm-kernel
* Nicolas Pitre <nico@fluxnic.net> [110513 06:06]:
> On Fri, 13 May 2011, Tony Lindgren wrote:
> >
> > OK, no problem. Got those cleanup patches available somewhere?
>
> They,re available in branch 'zImage_fixes' of git://git.linaro.org/people/nico/linux
> and in RMK's devel-stable branch as well.
OK, in that case I already did the rebase last night so the
version I posted should apply. I think the original append
patch applied without modifications.
Regards,
Tony
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH] ARM: Make sure appended device tree data won't overlap kernel BSS
2011-05-13 13:28 ` Tony Lindgren
@ 2011-06-07 12:43 ` Tony Lindgren
2011-06-07 13:14 ` Nicolas Pitre
0 siblings, 1 reply; 36+ messages in thread
From: Tony Lindgren @ 2011-06-07 12:43 UTC (permalink / raw)
To: linux-arm-kernel
* Tony Lindgren <tony@atomide.com> [110513 16:25]:
> * Nicolas Pitre <nico@fluxnic.net> [110513 06:06]:
> > On Fri, 13 May 2011, Tony Lindgren wrote:
> > >
> > > OK, no problem. Got those cleanup patches available somewhere?
> >
> > They,re available in branch 'zImage_fixes' of git://git.linaro.org/people/nico/linux
> > and in RMK's devel-stable branch as well.
>
> OK, in that case I already did the rebase last night so the
> version I posted should apply. I think the original append
> patch applied without modifications.
ping, Nico, are you planning to queue these device tree append
patches?
Regards,
Tony
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH] ARM: Make sure appended device tree data won't overlap kernel BSS
2011-06-07 12:43 ` Tony Lindgren
@ 2011-06-07 13:14 ` Nicolas Pitre
2011-06-07 13:22 ` Tony Lindgren
0 siblings, 1 reply; 36+ messages in thread
From: Nicolas Pitre @ 2011-06-07 13:14 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, 7 Jun 2011, Tony Lindgren wrote:
> * Tony Lindgren <tony@atomide.com> [110513 16:25]:
> > * Nicolas Pitre <nico@fluxnic.net> [110513 06:06]:
> > > On Fri, 13 May 2011, Tony Lindgren wrote:
> > > >
> > > > OK, no problem. Got those cleanup patches available somewhere?
> > >
> > > They,re available in branch 'zImage_fixes' of git://git.linaro.org/people/nico/linux
> > > and in RMK's devel-stable branch as well.
> >
> > OK, in that case I already did the rebase last night so the
> > version I posted should apply. I think the original append
> > patch applied without modifications.
>
> ping, Nico, are you planning to queue these device tree append
> patches?
Yes.
Nicolas
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH] ARM: Make sure appended device tree data won't overlap kernel BSS
2011-06-07 13:14 ` Nicolas Pitre
@ 2011-06-07 13:22 ` Tony Lindgren
0 siblings, 0 replies; 36+ messages in thread
From: Tony Lindgren @ 2011-06-07 13:22 UTC (permalink / raw)
To: linux-arm-kernel
* Nicolas Pitre <nico@fluxnic.net> [110607 06:10]:
> On Tue, 7 Jun 2011, Tony Lindgren wrote:
>
> > * Tony Lindgren <tony@atomide.com> [110513 16:25]:
> > > * Nicolas Pitre <nico@fluxnic.net> [110513 06:06]:
> > > > On Fri, 13 May 2011, Tony Lindgren wrote:
> > > > >
> > > > > OK, no problem. Got those cleanup patches available somewhere?
> > > >
> > > > They,re available in branch 'zImage_fixes' of git://git.linaro.org/people/nico/linux
> > > > and in RMK's devel-stable branch as well.
> > >
> > > OK, in that case I already did the rebase last night so the
> > > version I posted should apply. I think the original append
> > > patch applied without modifications.
> >
> > ping, Nico, are you planning to queue these device tree append
> > patches?
>
> Yes.
OK thanks for the update.
Tony
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH] ARM: Make sure appended device tree data won't overlap kernel BSS
2011-05-13 11:21 ` Tony Lindgren
2011-05-13 13:09 ` Nicolas Pitre
@ 2011-06-12 6:14 ` Nicolas Pitre
2011-06-13 10:49 ` Tony Lindgren
1 sibling, 1 reply; 36+ messages in thread
From: Nicolas Pitre @ 2011-06-12 6:14 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, 13 May 2011, Tony Lindgren wrote:
> From: Tony Lindgren <tony@atomide.com>
> Date: Thu, 12 May 2011 05:29:49 -0700
> Subject: [PATCH] ARM: zImage: Make sure appended device tree data won't overlap kernel BSS
>
> Do this before relocating the compressed kernel + device tree data.
> Otherwise we would have to split the copying into two parts, or copy
> the device tree data twice.
>
> As we only have one register available, pass the size of kernel BSS
> via linker and do the calculation using r1, then save it to the stack.
>
> Note that this patch now makes the stack also usable earlier for
> CONFIG_ARM_APPENDED_DTB.
>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
[...]
Please tell me what you think of this alternative:
http://article.gmane.org/gmane.linux.ports.arm.kernel/120182
I think it is a cleaner solution.
Nicolas
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH] ARM: Make sure appended device tree data won't overlap kernel BSS
2011-06-12 6:14 ` Nicolas Pitre
@ 2011-06-13 10:49 ` Tony Lindgren
0 siblings, 0 replies; 36+ messages in thread
From: Tony Lindgren @ 2011-06-13 10:49 UTC (permalink / raw)
To: linux-arm-kernel
* Nicolas Pitre <nico@fluxnic.net> [110611 23:09]:
> On Fri, 13 May 2011, Tony Lindgren wrote:
>
> > From: Tony Lindgren <tony@atomide.com>
> > Date: Thu, 12 May 2011 05:29:49 -0700
> > Subject: [PATCH] ARM: zImage: Make sure appended device tree data won't overlap kernel BSS
> >
> > Do this before relocating the compressed kernel + device tree data.
> > Otherwise we would have to split the copying into two parts, or copy
> > the device tree data twice.
> >
> > As we only have one register available, pass the size of kernel BSS
> > via linker and do the calculation using r1, then save it to the stack.
> >
> > Note that this patch now makes the stack also usable earlier for
> > CONFIG_ARM_APPENDED_DTB.
> >
> > Signed-off-by: Tony Lindgren <tony@atomide.com>
> [...]
>
> Please tell me what you think of this alternative:
> http://article.gmane.org/gmane.linux.ports.arm.kernel/120182
> I think it is a cleaner solution.
Yeah that's nice, I acked that one in the thread.
Regards,
Tony
^ permalink raw reply [flat|nested] 36+ messages in thread
end of thread, other threads:[~2011-06-13 10:49 UTC | newest]
Thread overview: 36+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-03-24 23:18 [PATCHv3] ARM:boot:device tree: Allow the device tree binary to be appended to zImage John Bonesio
2011-03-24 23:37 ` Nicolas Pitre
2011-03-28 9:13 ` Shawn Guo
2011-04-13 14:00 ` Tony Lindgren
2011-04-20 5:47 ` Shawn Guo
2011-04-20 7:34 ` Shawn Guo
2011-04-21 8:02 ` Tony Lindgren
2011-04-21 12:46 ` Tony Lindgren
2011-04-27 14:23 ` Tony Lindgren
2011-04-27 14:38 ` Tony Lindgren
2011-04-27 14:40 ` Nicolas Pitre
2011-04-27 14:43 ` Tony Lindgren
2011-04-29 10:26 ` Tony Lindgren
2011-04-29 13:02 ` Grant Likely
2011-04-29 13:08 ` Grant Likely
2011-04-29 13:09 ` Tony Lindgren
2011-04-29 13:21 ` Nicolas Pitre
2011-04-29 13:16 ` Nicolas Pitre
2011-04-29 13:53 ` Russell King - ARM Linux
2011-04-29 19:14 ` Nicolas Pitre
2011-05-04 7:23 ` Tony Lindgren
2011-05-04 13:12 ` Tony Lindgren
2011-05-04 13:38 ` Nicolas Pitre
2011-05-09 11:19 ` [PATCH] ARM: Make sure appended device tree data won't overlap kernel BSS Tony Lindgren
2011-05-09 14:49 ` Tony Lindgren
2011-05-12 12:59 ` Tony Lindgren
2011-05-13 7:39 ` Nicolas Pitre
2011-05-13 11:21 ` Tony Lindgren
2011-05-13 13:09 ` Nicolas Pitre
2011-05-13 13:28 ` Tony Lindgren
2011-06-07 12:43 ` Tony Lindgren
2011-06-07 13:14 ` Nicolas Pitre
2011-06-07 13:22 ` Tony Lindgren
2011-06-12 6:14 ` Nicolas Pitre
2011-06-13 10:49 ` Tony Lindgren
2011-05-09 11:23 ` [PATCHv3] ARM:boot:device tree: Allow the device tree binary to be appended to zImage Tony Lindgren
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).