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 |   78 ++++++++++++++++++++++++++++++++++++++-
 2 files changed, 82 insertions(+), 3 deletions(-)

arch/arm/Kconfig +7 -0

index 92bae09..3556aea 100644
@@ -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 ""

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

index 200625c..034a654 100644
@@ -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
 
 		/*

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 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]>

---

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.

Quoted 7 lines · Shawn Guo

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 7 lines

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.

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.

Quoted 4 lines · Shawn Guo

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 ...

Quoted 6 lines · Shawn Guo

Quoted 4 lines · Shawn Guo

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.

Quoted 10 lines · Tony Lindgren

Quoted 6 lines · Shawn Guo

Quoted 4 lines · Shawn Guo

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?

Quoted 18 lines · Tony Lindgren

Quoted 10 lines · Tony Lindgren

Quoted 6 lines · Shawn Guo

Quoted 4 lines · Shawn Guo

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?

Quoted 4 lines

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.

Quoted 3 lines · Tony Lindgren

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).

Quoted 8 lines · Nicolas Pitre

Quoted 3 lines · Tony Lindgren

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..

Quoted 11 lines · Tony Lindgren

Quoted 8 lines · Nicolas Pitre

Quoted 3 lines · Tony Lindgren

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.

Quoted 3 lines · Tony Lindgren

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?

Quoted 10 lines · Tony Lindgren

* Tony Lindgren <[email protected]> [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.

Quoted 5 lines

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?

Quoted 17 lines · Grant Likely

Quoted 15 lines · Tony Lindgren

* Tony Lindgren <[email protected]> [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.

Quoted 2 lines

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.

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.

Quoted 8 lines · Tony Lindgren

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.

Quoted 5 lines

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.

Quoted 19 lines · Nicolas Pitre

Quoted 8 lines · Tony Lindgren

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.

Quoted 5 lines

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.

Quoted 3 lines

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.

Quoted 24 lines · Russell King - ARM Linux

Quoted 22 lines · Nicolas Pitre

Quoted 8 lines · Tony Lindgren

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.

Quoted 5 lines

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.

Quoted 3 lines

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.

Quoted 8 lines · Nicolas Pitre

Quoted 4 lines

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?

Quoted 3 lines · Nicolas Pitre

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?

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.

Quoted 6 lines · Tony Lindgren

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.

Quoted 10 lines · Tony Lindgren

Quoted 6 lines · Tony Lindgren

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

arch/arm/boot/compressed/Makefile +3 -0

@@ -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)

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

@@ -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
+
+		/*

Review

Quoted 2 lines · Tony Lindgren

(quoted diff) +1 -0

+		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.

Quoted 6 lines · Nicolas Pitre

Review

Quoted 2 lines · Tony Lindgren

(quoted diff) +1 -0

+		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.

Quoted 3 lines

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?

Quoted 2 lines · Tony Lindgren

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.

Quoted 6 lines · Nicolas Pitre

Quoted 2 lines · Tony Lindgren

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.

Quoted 8 lines · Tony Lindgren

Quoted 6 lines · Nicolas Pitre

Quoted 2 lines · Tony Lindgren

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.

Quoted 3 lines

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?

Quoted 9 lines · Tony Lindgren

Quoted 8 lines · Tony Lindgren

Quoted 6 lines · Nicolas Pitre

Quoted 2 lines · Tony Lindgren

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.

Quoted 6 lines

Quoted 3 lines

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.

From: Tony Lindgren <[email protected]>
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 <[email protected]>

Quoted 14 lines · Tony Lindgren

From: Tony Lindgren <[email protected]>
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.

Quoted 1 line

Signed-off-by Tony Lindgren <[email protected]>

[...]

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.

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).