arm: pick the r2 passed DTB over the appended one

RFC In review Valentin Longchamp 2 participants 8 messages Apr 24, 2015 – Apr 30, 2015 Arm (32-bit)

Share & tools

Valentin Longchamp

author

2015-04-24 12:53 UTC #0 lore raw

RFC 1/1 arm: pick the r2 passed DTB over the appended one

Thanks to the APPENDED_DTB option, it is possible to append the dtb to the kernel's binary image. This is very convenient to accomodate old bootloaders that do not pass a dtb.

We have updated a product with a new version of the bootloader that now passed the dtb properly in r2. However, we still want to support with the same software (i.e. kernel) the old bootloader, where the dtb must be appended, but give the priority to to the one passed in r2.

This patch tries cover the above use case. It works for the case a dtb is passed in r2 and one is appended (the one pointed by r2 is picked). However, when there is only an appended dtb and r2 points to an ATAG list, this fails. Somehow, the presumably updated r2 does not point to a valid dtb and it gets zeroed by __vet_atags. Below is an example of this (end of bootloader, start of kernel):

at 02000000 ...

  Image Name:   Linux-3.10.60-7.2.2-00002-gc6acc
  Image Type:   ARM Linux Kernel Image (uncompressed)
  Data Size:    2904550 Bytes = 2.8 MiB

Load Address: 00008000
Entry Point: 00008000
Verifying Checksum ... OK
Loading Kernel Image ... OK
OK
r2 (0x00000100) points to (size, header): 0x00000005, 0x54410001

Starting kernel ...

Uncompressing Linux... done, booting the kernel.
e5943000
trying to parse the FDT

dt_phys (r2) is NULL !

Error: unrecognized/unsupported machine ID (r1 = 0x000008cf).

Available machine support:

ID (hex) NAME
ffffffff Marvell Kirkwood (Flattened Device Tree)

Please check your kernel config and/or bootloader.

Is this patch a correct approach to implement the above use case ? If yes, does anyone has an idea why this fails ?

Note: this patch is for the 3.10 stable branch.

Signed-off-by Valentin Longchamp <[email protected]>

---
 arch/arm/boot/compressed/head.S | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

arch/arm/boot/compressed/head.S +5 -1

index 032a8d9..a43c8a0 100644
@@ -244,7 +244,7 @@ restart:	adr	r0, LC0
  * dtb data will get relocated along with the kernel if necessary.
  */
 
-		ldr	lr, [r6, #0]
+		ldr	lr, [r6, #0]		@ potential appended dtb ?
 #ifndef __ARMEB__
 		ldr	r1, =0xedfe0dd0		@ sig is 0xd00dfeed big endian
 #else

@@ -253,6 +253,10 @@ restart:	adr	r0, LC0
 		cmp	lr, r1
 		bne	dtb_check_done		@ not found
 
+		ldr	lr, [r8, #0] 		@ conventionaly passed dtb ?
+		cmp	lr, r1
+		beq	dtb_check_done		@ yes, do not manage it
+
 #ifdef CONFIG_ARM_ATAG_DTB_COMPAT
 		/*
 		 * OK... Let's do some funky business here.

Signature · 1 line

1.8.0.1

Nicolas Pitre

2015-04-29 03:50 UTC #1 lore raw

Quoted 9 lines · Valentin Longchamp

Thanks to the APPENDED_DTB option, it is possible to append the dtb to the kernel's binary image. This is very convenient to accomodate old bootloaders that do not pass a dtb.
We have updated a product with a new version of the bootloader that now passed the dtb properly in r2. However, we still want to support with the same software (i.e. kernel) the old bootloader, where the dtb must be appended, but give the priority to to the one passed in r2.

Quoted 5 lines

This patch tries cover the above use case. It works for the case a dtb is passed in r2 and one is appended (the one pointed by r2 is picked). However, when there is only an appended dtb and r2 points to an ATAG list, this fails. Somehow, the presumably updated r2 does not point to a valid dtb and it gets zeroed by __vet_atags.

Do you have CONFIG_ARM_ATAG_DTB_COMPAT set?

I don't see how this could be related, but you should consider adding commit c2607f74aa to your kernel if not already there.

Quoted 13 lines

Below is an example of this (end of bootloader, start of kernel):
at 02000000 ...
  Image Name:   Linux-3.10.60-7.2.2-00002-gc6acc
  Image Type:   ARM Linux Kernel Image (uncompressed)
  Data Size:    2904550 Bytes = 2.8 MiB
Load Address: 00008000
Entry Point: 00008000
Verifying Checksum ... OK
Loading Kernel Image ... OK
OK
r2 (0x00000100) points to (size, header): 0x00000005, 0x54410001

Quoted 4 lines

Starting kernel ...
Uncompressing Linux... done, booting the kernel.
e5943000

Where is this value from? This looks like an ARM opcode ("ldr r3, [r4]").

Quoted 51 lines

trying to parse the FDT
dt_phys (r2) is NULL !
Error: unrecognized/unsupported machine ID (r1 = 0x000008cf).
Available machine support:
ID (hex) NAME
ffffffff Marvell Kirkwood (Flattened Device Tree)
Please check your kernel config and/or bootloader.
Is this patch a correct approach to implement the above use case ? If yes, does anyone has an idea why this fails ?
Note: this patch is for the 3.10 stable branch.
Signed-off-by Valentin Longchamp <[email protected]>
---
 arch/arm/boot/compressed/head.S | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
arch/arm/boot/compressed/head.S +5 -1
index 032a8d9..a43c8a0 100644
@@ -244,7 +244,7 @@ restart:	adr	r0, LC0
  * dtb data will get relocated along with the kernel if necessary.
  */
 
-		ldr	lr, [r6, #0]
+		ldr	lr, [r6, #0]		@ potential appended dtb ?
 #ifndef __ARMEB__
 		ldr	r1, =0xedfe0dd0		@ sig is 0xd00dfeed big endian
 #else
@@ -253,6 +253,10 @@ restart:	adr	r0, LC0
 		cmp	lr, r1
 		bne	dtb_check_done		@ not found
 
+		ldr	lr, [r8, #0] 		@ conventionaly passed dtb ?
+		cmp	lr, r1
+		beq	dtb_check_done		@ yes, do not manage it
+
 #ifdef CONFIG_ARM_ATAG_DTB_COMPAT
 		/*
 		 * OK... Let's do some funky business here.
Signature · 1 line
1.8.0.1

Valentin Longchamp

author

2015-04-29 09:14 UTC #2 lore raw

Quoted 11 lines · Nicolas Pitre

Quoted 9 lines · Valentin Longchamp
Thanks to the APPENDED_DTB option, it is possible to append the dtb to the kernel's binary image. This is very convenient to accomodate old bootloaders that do not pass a dtb.
We have updated a product with a new version of the bootloader that now passed the dtb properly in r2. However, we still want to support with the same software (i.e. kernel) the old bootloader, where the dtb must be appended, but give the priority to to the one passed in r2.

Quoted 7 lines

Quoted 5 lines
This patch tries cover the above use case. It works for the case a dtb is passed in r2 and one is appended (the one pointed by r2 is picked). However, when there is only an appended dtb and r2 points to an ATAG list, this fails. Somehow, the presumably updated r2 does not point to a valid dtb and it gets zeroed by __vet_atags.
Do you have CONFIG_ARM_ATAG_DTB_COMPAT set?

Yes, it is set.

Quoted 3 lines

I don't see how this could be related, but you should consider adding commit c2607f74aa to your kernel if not already there.

I have included it in the meantime, thanks. It does not change the below behavior however.

Quoted 14 lines

Quoted 14 lines
Below is an example of this (end of bootloader, start of kernel):
at 02000000 ...
  Image Name:   Linux-3.10.60-7.2.2-00002-gc6acc
  Image Type:   ARM Linux Kernel Image (uncompressed)
  Data Size:    2904550 Bytes = 2.8 MiB
Load Address: 00008000
Entry Point: 00008000
Verifying Checksum ... OK
Loading Kernel Image ... OK
OK
r2 (0x00000100) points to (size, header): 0x00000005, 0x54410001

Quoted 6 lines

Quoted 4 lines
Starting kernel ...
Uncompressing Linux... done, booting the kernel.
e5943000
Where is this value from? This looks like an ARM opcode ("ldr r3, [r4]").

This value that I print out actually is the content of the memory pointed by the r2 register, printed from __vet_atags (head-common.S)

It can very well be an ARM opcode from the kernel binary since the r2 register does not point to a DTB but to some other location.

I now have further debugged this and I now understand why it does not work as I expect it: I have noticed that (at least with our kernel and our system) this code gets executed twice. This is has the consequence for the case where the DTB is appended and atags are passed to r2, my check is valid the first time this code is run. However, the second time, r8 (pointer to atags/dtb) was updated with r6 (pointer to _edata/appended dtb), which means that my additional check will cause the code to jump to dtb_check_done. So the DTB is not copied over with the rest of the kernel binary, because r6, r10 and sp and not "relocated" past the dtb !

Now my question is: is this normal that this code is executed twice or is a problem on our side ? If it is normal, I need to add further checks. If not, I would rather keep my implementation roughly as it is and rather troubleshoot this "double execution".

Maybe it is important to tell that we use a uImage for our kernel (U-boot wrapped zImage).

Quoted 52 lines

Quoted 52 lines
trying to parse the FDT
dt_phys (r2) is NULL !
Error: unrecognized/unsupported machine ID (r1 = 0x000008cf).
Available machine support:
ID (hex) NAME
ffffffff Marvell Kirkwood (Flattened Device Tree)
Please check your kernel config and/or bootloader.
Is this patch a correct approach to implement the above use case ? If yes, does anyone has an idea why this fails ?
Note: this patch is for the 3.10 stable branch.
Signed-off-by Valentin Longchamp <[email protected]>
---
 arch/arm/boot/compressed/head.S | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
arch/arm/boot/compressed/head.S +5 -1
index 032a8d9..a43c8a0 100644
@@ -244,7 +244,7 @@ restart:	adr	r0, LC0
  * dtb data will get relocated along with the kernel if necessary.
  */
 
-		ldr	lr, [r6, #0]
+		ldr	lr, [r6, #0]		@ potential appended dtb ?
 #ifndef __ARMEB__
 		ldr	r1, =0xedfe0dd0		@ sig is 0xd00dfeed big endian
 #else
@@ -253,6 +253,10 @@ restart:	adr	r0, LC0
 		cmp	lr, r1
 		bne	dtb_check_done		@ not found
 
+		ldr	lr, [r8, #0] 		@ conventionaly passed dtb ?
+		cmp	lr, r1
+		beq	dtb_check_done		@ yes, do not manage it
+
 #ifdef CONFIG_ARM_ATAG_DTB_COMPAT
 		/*
 		 * OK... Let's do some funky business here.
Signature · 1 line
1.8.0.1

Nicolas Pitre

2015-04-29 13:58 UTC #3 lore raw

Quoted 6 lines · Valentin Longchamp

Quoted 5 lines · Nicolas Pitre
Quoted 2 lines · Valentin Longchamp
Uncompressing Linux... done, booting the kernel.
e5943000

Quoted 7 lines

Quoted 1 line
Where is this value from? This looks like an ARM opcode ("ldr r3, [r4]").
This value that I print out actually is the content of the memory pointed by the r2 register, printed from __vet_atags (head-common.S)
It can very well be an ARM opcode from the kernel binary since the r2 register does not point to a DTB but to some other location.

What is the value of r2 at that point?

Quoted 10 lines

I now have further debugged this and I now understand why it does not work as I expect it: I have noticed that (at least with our kernel and our system) this code gets executed twice. This is has the consequence for the case where the DTB is appended and atags are passed to r2, my check is valid the first time this code is run. However, the second time, r8 (pointer to atags/dtb) was updated with r6 (pointer to _edata/appended dtb), which means that my additional check will cause the code to jump to dtb_check_done. So the DTB is not copied over with the rest of the kernel binary, because r6, r10 and sp and not "relocated" past the dtb !

Quoted 4 lines

Now my question is: is this normal that this code is executed twice or is a problem on our side ? If it is normal, I need to add further checks. If not, I would rather keep my implementation roughly as it is and rather troubleshoot this "double execution".

The double execution is "normal". The code checks if itself and the compressed payload is in the way of the decompressed kernel. If it is then it relocates itself to avoid being overwritten. Once relocated, execution starts over again to keep things simple.

When a DTB is appended to zImage, then it has to be relocated alongside the compressed payload for the same reasons. For this, we need to determine the actual size of the DTB. That's where r6 is adjusted to pretend the DTB is part of the actual zImage.

Yet if CONFIG_ARM_ATAG_DTB_COMPAT is set, this has to be performed before the relocation as it may well change the size of the DTB.

Now, to fix your test, you'd simply have to augment it with:

+ cmp r6, r8 @ is r8 pointing to the appended DTB?
+ beq 1f
ldr lr, [r8, #0] @ conventionaly passed dtb ?
cmp lr, r1
beq dtb_check_done @ yes, do not manage it
+1:

Nicolas

Valentin Longchamp

author

2015-04-29 15:49 UTC #4 lore raw

RFC 1/1 arm: pick the r2 passed DTB over the appended one

Quoted 10 lines · Nicolas Pitre

Quoted 8 lines · Valentin Longchamp
Quoted 6 lines · Nicolas Pitre
Quoted 2 lines · Valentin Longchamp
Uncompressing Linux... done, booting the kernel.
e5943000
Where is this value from? This looks like an ARM opcode ("ldr r3, [r4]").

Quoted 7 lines

Quoted 5 lines
This value that I print out actually is the content of the memory pointed by the r2 register, printed from __vet_atags (head-common.S)
It can very well be an ARM opcode from the kernel binary since the r2 register does not point to a DTB but to some other location.
What is the value of r2 at that point?

The value of r2 at that point ends up to be the value of r8 (please see below).

Quoted 33 lines

Quoted 11 lines
I now have further debugged this and I now understand why it does not work as I expect it: I have noticed that (at least with our kernel and our system) this code gets executed twice. This is has the consequence for the case where the DTB is appended and atags are passed to r2, my check is valid the first time this code is run. However, the second time, r8 (pointer to atags/dtb) was updated with r6 (pointer to _edata/appended dtb), which means that my additional check will cause the code to jump to dtb_check_done. So the DTB is not copied over with the rest of the kernel binary, because r6, r10 and sp and not "relocated" past the dtb !
Quoted 4 lines
Now my question is: is this normal that this code is executed twice or is a problem on our side ? If it is normal, I need to add further checks. If not, I would rather keep my implementation roughly as it is and rather troubleshoot this "double execution".
The double execution is "normal". The code checks if itself and the compressed payload is in the way of the decompressed kernel. If it is then it relocates itself to avoid being overwritten. Once relocated, execution starts over again to keep things simple.
When a DTB is appended to zImage, then it has to be relocated alongside the compressed payload for the same reasons. For this, we need to determine the actual size of the DTB. That's where r6 is adjusted to pretend the DTB is part of the actual zImage.
Yet if CONFIG_ARM_ATAG_DTB_COMPAT is set, this has to be performed before the relocation as it may well change the size of the DTB.
Now, to fix your test, you'd simply have to augment it with:
(quoted diff) +2 -0
+		cmp	r6, r8		@ is r8 pointing to the appended DTB?
+		beq	1f

Quoted 5 lines

ldr lr, [r8, #0] @ conventionaly passed dtb ?
cmp lr, r1
beq dtb_check_done @ yes, do not manage it
+1:

I had thought the same and implemented a similar test as you propose (patch attached, with some more debug code - please excuse my poor assembler). However it does not work ! The reason for it is that on the second run, r6 contains another value. As the output below seems to show, this 2nd run r6 value seems to point to a DTB since the first jump to dtb_check_done is not performed. However, since r8 now points to the "initial" appended DTB, the 2nd test jumps to dtb_check_done. Please see the output below.

Quoted 32 lines

## Current stack ends at 0x1fba0b68 ## Booting kernel from Legacy Image at 02000000 ...
   Image Name:   Linux-3.10.60-7.2.2-00005-g56840
   Image Type:   ARM Linux Kernel Image (uncompressed)
   Data Size:    2905014 Bytes = 2.8 MiB
Load Address: 00008000
Entry Point: 00008000
Verifying Checksum ... OK
Loading Kernel Image ... OK
OK
## Transferring control to Linux (at address 00008000) ... r2 (0x00000100) points to (size, header): 0x00000005, 0x54410001
Starting kernel ...
a
b
002CAE58
00000100
c
d
a
b
008AF9F8
002CAE58
c
Uncompressing Linux... done, booting the kernel.
trying to parse the FDT
dt_phys (r2) is NULL !

Quoted 10 lines

Error: unrecognized/unsupported machine ID (r1 = 0x000008cf).
Available machine support:
ID (hex) NAME
ffffffff Marvell Kirkwood (Flattened Device Tree)
Please check your kernel config and/or bootloader.

Why does r6 on the second run contain another value than r8 and points to an address that seem to be (another ?) DTB ?

Valentin

---
 arch/arm/boot/compressed/head.S | 37 ++++++++++++++++++++++++++++++++++---
 1 file changed, 34 insertions(+), 3 deletions(-)

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

index e9e312c..2677ccc 100644
@@ -19,6 +19,7 @@
  * 100% relocatable.  Any attempt to do so will result in a crash.
  * Please select one of the following when turning on debugging.
  */
+#define DEBUG
 #ifdef DEBUG
 
 #if defined(CONFIG_DEBUG_ICEDCC)

@@ -244,7 +245,12 @@ restart:   adr     r0, LC0
  * dtb data will get relocated along with the kernel if necessary.
  */
 
-               ldr     lr, [r6, #0]            @ potential appended dtb ?
+
+               stmfd   sp!, {r0-r1, lr}
+               kputc   #'a'
+               kputc   #'\n'
+               stmfd   sp!, {r0-r1, lr}
+               ldr     lr, [r6, #0]            @ potential appended dtb ?
 #ifndef __ARMEB__
                ldr     r1, =0xedfe0dd0         @ sig is 0xd00dfeed big endian
 #else

@@ -253,10 +259,35 @@ restart:  adr     r0, LC0
                cmp     lr, r1
                bne     dtb_check_done          @ not found
 
-               ldr     lr, [r8, #0]            @ conventionaly passed dtb ?
+               stmfd   sp!, {r0-r1, lr}
+               kputc   #'b'
+               kputc   #'\n'
+               kphex   r6, 8
+               kputc   #'\n'
+               kphex   r8, 8
+               kputc   #'\n'
+               stmfd   sp!, {r0-r1, lr}
+               cmp     r6, r8                  @ r8 already pointing to app
+               beq     skip_2nd_check          @ yes, leave 2nd check
+
+               stmfd   sp!, {r0-r1, lr}
+               kputc   #'c'
+               kputc   #'\n'
+               stmfd   sp!, {r0-r1, lr}
+               ldr     lr, [r8, #0]            @ conventionaly passed dtb ?
+#ifndef __ARMEB__
+               ldr     r1, =0xedfe0dd0         @ sig is 0xd00dfeed big endian
+#else
+               ldr     r1, =0xd00dfeed
+#endif
                cmp     lr, r1
-               beq     dtb_check_done          @ yes, do not manage it
+               beq     dtb_check_done          @ yes, use it instead of appended
 
+skip_2nd_check:
+               stmfd   sp!, {r0-r1, lr}
+               kputc   #'d'
+               kputc   #'\n'
+               stmfd   sp!, {r0-r1, lr}
 #ifdef CONFIG_ARM_ATAG_DTB_COMPAT
                /*
                 * OK... Let's do some funky business here.

Signature · 1 line

1.8.0.1

Nicolas Pitre

2015-04-29 16:43 UTC #5 lore raw

RFC 1/1 arm: pick the r2 passed DTB over the appended one

Quoted 18 lines · Valentin Longchamp

Quoted 9 lines · Nicolas Pitre
Now, to fix your test, you'd simply have to augment it with:
(quoted diff) +2 -0
+		cmp	r6, r8		@ is r8 pointing to the appended DTB?
+		beq	1f
ldr lr, [r8, #0] @ conventionaly passed dtb ?
cmp lr, r1
beq dtb_check_done @ yes, do not manage it
+1:
I had thought the same and implemented a similar test as you propose (patch attached, with some more debug code - please excuse my poor assembler). However it does not work ! The reason for it is that on the second run, r6 contains another value. As the output below seems to show, this 2nd run r6 value seems to point to a DTB since the first jump to dtb_check_done is not performed. However, since r8 now points to the "initial" appended DTB, the 2nd test jumps to dtb_check_done. Please see the output below.

Right. On the second run, r6 points at the relocated DTB. That's the one that should be used. r8 points at the initial, non relocated and about to be overwritten DTB. By giving priority to r8 with your patch, you're picking the wrong one.

The following should fix this issue:

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

index c41a793b51..bbce6a0f0d 100644
@@ -399,6 +399,16 @@ dtb_check_done:
 1:
 #endif
 
+#ifdef CONFIG_ARM_APPENDED_DTB
+		/*
+		 * If r8 refers to an appended DTB, it is no longer valid
+		 * and should be revalidated once relocated.
+		 */
+		cmp	r8, r5
+		cmpcs	r6, r8
+		movcs	r8, #0
+#endif
+
 		sub	r9, r6, r5		@ size to copy
 		add	r9, r9, #31		@ rounded up to a multiple
 		bic	r9, r9, #31		@ ... of 32 bytes

Nicolas

Valentin Longchamp

author

2015-04-30 12:39 UTC #6 lore raw

Quoted 51 lines · Nicolas Pitre

Quoted 18 lines · Valentin Longchamp
Quoted 9 lines · Nicolas Pitre
Now, to fix your test, you'd simply have to augment it with:
(quoted diff) +2 -0
+		cmp	r6, r8		@ is r8 pointing to the appended DTB?
+		beq	1f
ldr lr, [r8, #0] @ conventionaly passed dtb ?
cmp lr, r1
beq dtb_check_done @ yes, do not manage it
+1:
I had thought the same and implemented a similar test as you propose (patch attached, with some more debug code - please excuse my poor assembler). However it does not work ! The reason for it is that on the second run, r6 contains another value. As the output below seems to show, this 2nd run r6 value seems to point to a DTB since the first jump to dtb_check_done is not performed. However, since r8 now points to the "initial" appended DTB, the 2nd test jumps to dtb_check_done. Please see the output below.
Right. On the second run, r6 points at the relocated DTB. That's the one that should be used. r8 points at the initial, non relocated and about to be overwritten DTB. By giving priority to r8 with your patch, you're picking the wrong one.
The following should fix this issue:
arch/arm/boot/compressed/head.S +10 -0
index c41a793b51..bbce6a0f0d 100644
@@ -399,6 +399,16 @@ dtb_check_done:
 1:
 #endif
 
+#ifdef CONFIG_ARM_APPENDED_DTB
+		/*
+		 * If r8 refers to an appended DTB, it is no longer valid
+		 * and should be revalidated once relocated.
+		 */
+		cmp	r8, r5
+		cmpcs	r6, r8
+		movcs	r8, #0
+#endif
+
 		sub	r9, r6, r5		@ size to copy
 		add	r9, r9, #31		@ rounded up to a multiple
 		bic	r9, r9, #31		@ ... of 32 bytes

Quoted 2 lines

Nicolas

Thank you very much for your help Nicolas, this fixed the issue indeed. I now have the behavior I wanted.

Do you think it makes sense to send the updated patch for mainline inclusion or is this a too "exotic" use case ?

Valentin

Nicolas Pitre

2015-04-30 15:12 UTC #7 lore raw

Quoted 52 lines · Valentin Longchamp

Quoted 51 lines · Nicolas Pitre
Quoted 18 lines · Valentin Longchamp
Quoted 9 lines · Nicolas Pitre
Now, to fix your test, you'd simply have to augment it with:
(quoted diff) +2 -0
+		cmp	r6, r8		@ is r8 pointing to the appended DTB?
+		beq	1f
ldr lr, [r8, #0] @ conventionaly passed dtb ?
cmp lr, r1
beq dtb_check_done @ yes, do not manage it
+1:
I had thought the same and implemented a similar test as you propose (patch attached, with some more debug code - please excuse my poor assembler). However it does not work ! The reason for it is that on the second run, r6 contains another value. As the output below seems to show, this 2nd run r6 value seems to point to a DTB since the first jump to dtb_check_done is not performed. However, since r8 now points to the "initial" appended DTB, the 2nd test jumps to dtb_check_done. Please see the output below.
Right. On the second run, r6 points at the relocated DTB. That's the one that should be used. r8 points at the initial, non relocated and about to be overwritten DTB. By giving priority to r8 with your patch, you're picking the wrong one.
The following should fix this issue:
arch/arm/boot/compressed/head.S +10 -0
index c41a793b51..bbce6a0f0d 100644
@@ -399,6 +399,16 @@ dtb_check_done:
 1:
 #endif
 
+#ifdef CONFIG_ARM_APPENDED_DTB
+		/*
+		 * If r8 refers to an appended DTB, it is no longer valid
+		 * and should be revalidated once relocated.
+		 */
+		cmp	r8, r5
+		cmpcs	r6, r8
+		movcs	r8, #0
+#endif
+
 		sub	r9, r6, r5		@ size to copy
 		add	r9, r9, #31		@ rounded up to a multiple
 		bic	r9, r9, #31		@ ... of 32 bytes

Quoted 8 lines

Quoted 2 lines
Nicolas
Thank you very much for your help Nicolas, this fixed the issue indeed. I now have the behavior I wanted.
Do you think it makes sense to send the updated patch for mainline inclusion or is this a too "exotic" use case ?

I don't know. We can't be sure this won't break someone else's use case i.e. overriding the bootloader's DTB by appending one to zImage.

I'd suggest we wait to see if more people require this feature.

Nicolas