ARM:boot:device tree: Allow the device tree binary to be appended to zImage

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 <[email protected]>

---

 arch/arm/Kconfig                |    7 ++++
 arch/arm/boot/compressed/head.S |   75 +++++++++++++++++++++++++++++++++++++--
 2 files changed, 79 insertions(+), 3 deletions(-)

arch/arm/Kconfig +7 -0

index d8a330f..1a55e3e 100644
@@ -1593,6 +1593,13 @@ config USE_OF
 	help
 	  Include support for flattened device tree machine descriptions.
 
+config ARM_APPENDED_DTB
+	bool "Use appended device tree blob"
+	depends on OF
+	help
+	  With this option, the boot code will look for a device tree binary
+	  (dtb) appended to zImage.
+
 # Compressed boot loader in ROM.  Yes, we really want to ask about
 # TEXT and BSS so we preserve their values in the config files.
 config ZBOOT_ROM_TEXT

arch/arm/boot/compressed/head.S +72 -3

index 200625c..611719e 100644
@@ -210,6 +210,58 @@ restart:	adr	r0, LC0
 		 */
 		mov	r10, r6
 #endif
+#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
+		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.

@@ -272,7 +324,8 @@ wont_overwrite:
  *   r12 = GOT end
  *   sp  = stack pointer
  */
-		teq	r0, #0
+		add	r1, r0, lr
+		teq	r1, #0
 		beq	not_relocated
 		add	r11, r11, r0
 		add	r12, r12, r0

@@ -288,12 +341,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, 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
 
 		/*

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Quoted 2 lines · John Bonesio

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'd much rather see something that wrappers the kernel and passes the DT through an ATAG. So possibly a pre-amble as well?

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 <[email protected]>

---

 arch/arm/Kconfig                |    7 ++++
 arch/arm/boot/compressed/head.S |   75 +++++++++++++++++++++++++++++++++++++--
 2 files changed, 79 insertions(+), 3 deletions(-)

Quoted 6 lines · John Bonesio

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 <[email protected]>

---

Quoted 4 lines

 arch/arm/Kconfig                |    7 ++++
 arch/arm/boot/compressed/head.S |   75 +++++++++++++++++++++++++++++++++++++--
 2 files changed, 79 insertions(+), 3 deletions(-)

I played this patch on mx51 babbage a little bit, and noticed that it introduces one difference on both non-dt and dt kernel booting. The

serial terminal stalls for a few seconds after printing 'Uncompressing Linux...', and then continue to work with all message output. That's something like:

Uncompressing Linux... (stall for seconds) done, booting the kernel. Linux version 2.6.38-rc6+ (r65073 at S2101-09) (gcc version 4.4.5 (Ubuntu/Linaro 4. 4.4-14ubuntu4) ) #91 Tue Mar 15 11:29:03 CST 2011
CPU: ARMv7 Processor [412fc085] revision 5 (ARMv7), cr=10c53c7f
CPU: VIPT nonaliasing data cache, VIPT aliasing instruction cache
Machine: Freescale MX51 Babbage Board
Machine: Freescale MX51 Babbage Board
Memory policy: ECC disabled, Data cache writeback
Built 1 zonelists in Zone order, mobility grouping on. Total pages: 130048
......

I'm unsure if this is expected ...

Quoted 11 lines · Shawn Guo

Quoted 10 lines · John Bonesio

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 <[email protected]>

---

 arch/arm/Kconfig                |    7 ++++
 arch/arm/boot/compressed/head.S |   75 +++++++++++++++++++++++++++++++++++++--
 2 files changed, 79 insertions(+), 3 deletions(-)

Quoted 2 lines

I played this patch on mx51 babbage a little bit, and noticed that it introduces one difference on both non-dt and dt kernel booting. The

To be clear, the difference is being seen when I do not even enable CONFIG_ARM_APPENDED_DTB.

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 <[email protected]>

---

 arch/arm/Kconfig                |    7 ++++
 arch/arm/boot/compressed/head.S |   75 +++++++++++++++++++++++++++++++++++++--
 2 files changed, 79 insertions(+), 3 deletions(-)

arch/arm/Kconfig +0 -0

index d8a330f..1a55e3e 100644

arch/arm/boot/compressed/head.S +42 -0

index 200625c..611719e 100644
@@ -210,6 +210,58 @@ restart:	adr	r0, LC0
 		 */
 		mov	r10, r6
 #endif
+#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
+		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

arch/arm/boot/compressed/head.S +11 -2

 		add	r11, r11, r0
 		add	r12, r12, r0

@@ -288,12 +341,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, 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
+

Quoted 4 lines · John Bonesio

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 <[email protected]>

You're almost there, but there are still remaining issues with this patch.

Quoted 11 lines

---

 arch/arm/Kconfig                |    7 ++++
 arch/arm/boot/compressed/head.S |   75 +++++++++++++++++++++++++++++++++++++--
 2 files changed, 79 insertions(+), 3 deletions(-)

arch/arm/Kconfig +0 -0

index d8a330f..1a55e3e 100644

arch/arm/Kconfig config USE_OF · line 1602

Quoted 10 lines

arch/arm/Kconfig +7 -0

@@ -1593,6 +1593,13 @@ config USE_OF
 	help
 	  Include support for flattened device tree machine descriptions.
 
+config ARM_APPENDED_DTB
+	bool "Use appended device tree blob"
+	depends on OF
+	help
+	  With this option, the boot code will look for a device tree binary
+	  (dtb) appended to zImage.
+

I'd put this option below the ZBOOT_ROM option, and make it dependent on !ZBOOT_ROM, e.g.:

	depends on OF && !ZBOOT_ROM

Quoted 57 lines

arch/arm/boot/compressed/head.S +49 -0

index 200625c..611719e 100644
@@ -210,6 +210,58 @@ restart:	adr	r0, LC0
 		 */
 		mov	r10, r6
 #endif
+#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
+		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

arch/arm/boot/compressed/head.S restart: adr r0, LC0 · line 264

Quoted 3 lines

arch/arm/boot/compressed/head.S +3 -0

+		add	r6, r6, lr
+dtb_check_done:
+#endif

Now if no dtb was found, or if that code is not configured, then lr contains garbage and that may have random consequences with the code that follows, which would explain the reported bad behaviors with this patch applied even if not configured in the kernel.

Nicolas

Hi Nicolas,

Thanks for the comments. I have a question (below).

Quoted 49 lines

Quoted 49 lines

arch/arm/boot/compressed/head.S +49 -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
+		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

Quoted 8 lines

arch/arm/boot/compressed/head.S restart: adr r0, LC0 · line 264

Quoted 3 lines

arch/arm/boot/compressed/head.S +3 -0

+		add	r6, r6, lr
+dtb_check_done:
+#endif

Now if no dtb was found, or if that code is not configured, then lr contains garbage and that may have random consequences with the code that follows, which would explain the reported bad behaviors with this patch applied even if not configured in the kernel.

I'm probably not understanding something here. When I look at the code prior to my patch, it looks like lr is not initialized by head.S.

It also looks like this code makes use of lr when it relocates the code and leaves it uninitialized when it jumps back to 'restart'.

Is this a bug in the previous code too?

What should lr be initialized to, or should we just be preserving it's value?

I saw the comment about the performance is different with this patch - something about a pause between 'Uncompressing Linux...' and the kernel boot output. I'm not sure what's going on here. By the time 'Uncompressing Linux...' comes out all relocations and dtb discovery is complete. Do you really think having lr uninitialized would cause this?

Quoted 4 lines · John Bonesio

Hi Nicolas,

Thanks for the comments. I have a question (below).

[...]

Quoted 8 lines

Quoted 5 lines

Now if no dtb was found, or if that code is not configured, then lr contains garbage and that may have random consequences with the code that follows, which would explain the reported bad behaviors with this patch applied even if not configured in the kernel.

I'm probably not understanding something here. When I look at the code prior to my patch, it looks like lr is not initialized by head.S.

Exact.

Quoted 2 lines

It also looks like this code makes use of lr when it relocates the code and leaves it uninitialized when it jumps back to 'restart'.

Exact.

Quoted 1 line

Is this a bug in the previous code too?

No.

Quoted 2 lines

What should lr be initialized to, or should we just be preserving it's value?

No. The problem is that you do use lr further down in your patch when adjusting GOT entries, assuming it contains the size of the dtb. If no DTB is found then lr contains random stuff that will mess up the GOT for .bss references.

Quoted 6 lines

I saw the comment about the performance is different with this patch - something about a pause between 'Uncompressing Linux...' and the kernel boot output. I'm not sure what's going on here. By the time 'Uncompressing Linux...' comes out all relocations and dtb discovery is complete. Do you really think having lr uninitialized would cause this?

Certainly. See above.

Nicolas