[RFC] arm: pick the r2 passed DTB over the appended one
Valentin Longchamp
valentin.longchamp at keymile.com
Wed Apr 29 08:49:14 PDT 2015
On 04/29/2015 03:58 PM, Nicolas Pitre wrote:
> On Wed, 29 Apr 2015, Valentin Longchamp wrote:
>
>> On 04/29/2015 05:50 AM, Nicolas Pitre wrote:
>>> On Fri, 24 Apr 2015, Valentin Longchamp wrote:
>>>
>>>> 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.
>
> 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).
>
>> 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".
>
> 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:
>
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.
> ## 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 !
>
> 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(-)
diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
index e9e312c..2677ccc 100644
--- a/arch/arm/boot/compressed/head.S
+++ b/arch/arm/boot/compressed/head.S
@@ -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.
--
1.8.0.1
More information about the linux-arm-kernel
mailing list