arm: provide a CONFIG flag for disabling relocation

Quoted 35 lines

Hi Simon, Wolfgang,

Quoted 25 lines · Simon Glass

Quoted 10 lines · Albert ARIBAUD

Le 25/03/2011 17:12, Aneesh V a ?crit :

Quoted 6 lines

Another problem I have with relocation is that it makes debugging with JTAG debugers more difficult. The addresses of symbols in the ELF target are no longer valid. Of course, you can load the symbols at an offset from the original location. But one has to first enable debug prints, find the relocation offset, use it while loading the symbols before you can do source level debugging.

Quoted 6 lines

Actually you don't need recompiling: simply set a breakpoint at the entry of relocate_code and once you hit the bp, look up r2: it contains the destination. If you want the offset rather than the absolute address, set the breakpoint right after the 'sub r9, r6, r0' round line 222: r9 will then give you the offset. Unload the current symbols, reload the symbols with the relevant offset, and there you go.

I would like to revisit this thread. I'm not sure how other people do development in U-Boot but I like to use an ICE and a source-level debugger for any significant effort. I think it should be possible to use a JTAG debugging just by loading the u-boot ELF file and running.

Quoted 6 lines

With this patch (or something similar) this is possible. Without it, it is painful.

To be clear, we are not talking here about creating a platform that doesn't use relocation, just that for development purposes it is convenient to be able to disable it.

Quoted 5 lines

Actually, I am not even sure why relocation shouldn't be disabled in my platform for good. It may be useful to have things such as the frame buffer at the end of available memory. But, IMHO, that can still be done without relocating u-boot. That's what the patch does.Am I missing something?

Well relocation does remove a lot of this ad-hoc positioning of things at compile time. I think it is desirable. My point is that it is not engineer-friendly during development, and we should have an easy way to disable it for debugging / JTAG purposes.

Regards,
Simon

Quoted 27 lines

Quoted 27 lines

Looking at the December thread
http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/88067/focus=88262

Aneesh:

Quoted 3 lines

Quoted 3 lines

Shouldn't we provide a CONFIG option by which users can disable Elf relocation?

Wolfgang:

Quoted 3 lines

Why should we? It would just make the code even more complicated, and require a lot of additional test cases.

From what I can see this is a new CONFIG option, two ifdefs in the
board.c file, and optionally disabling the -pie position-independent executable option to reduce size. It is pretty trivial:

 arch/arm/config.mk   |    2 ++
 arch/arm/lib/board.c |    5 +++++
 2 files changed, 7 insertions(+), 0 deletions(-)

Regards,
Simon

Quoted 1 line

Quoted 9 lines

Quoted 8 lines

Quoted 8 lines

Amicalement,
--
Albert.
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

The use of the initial patches for the CONFIG_SYS_SKIP_ARM_RELOCATION featu res has revealed two issues.

First issue: the calculation of the relocation offset was done only if the relocation is actually done. So we could reach a point where r9 has a wrong
value, since it has never been used before (in my case, this bug happens w

the test of a relocation need.

Second issue: board_init_r was thinking the memory area for the malloc is j ust below the code, whereas the board_init_f had allocated some space for t he malloc at the end of the SDRAM. If the code is located at the base of th

Dear "GROYER, Anthony",

Quoted 3 lines · In message<BC0A2F434D4F39448D24A68EA6EFFB9F0194D534@EU-FR-EXBE07.eu.corp.airliq…

The use of the initial patches for the CONFIG_SYS_SKIP_ARM_RELOCATION featu res has revealed two issues.

Could you please restict your line length to some 70 characters or so? Thanks.

Quoted 3 lines

First issue: the calculation of the relocation offset was done only if the relocation is actually done. So we could reach a point where r9 has a wrong
value, since it has never been used before (in my case, this bug happens w

This is a configuration error then, isn't it? The relocation offset should be either the intended value, or eventually zero, if no relocation is intended.

BTW: your patch has a number ofd coduing style errors, and the
Signed-off-by: line is missing.

Best regards,

Wolfgang Denk

-----Message d'origine-----
De?: u-boot-bounces at lists.denx.de [mailto:u-boot- bounces at lists.denx.de] De la part de Albert ARIBAUD Envoy??: mardi 20 septembre 2011 21:14
??: u-boot at lists.denx.de
Objet?: Re: [U-Boot] [RFC PATCH] arm: provide a CONFIG flag for disabling relocation

Le 20/09/2011 20:09, Wolfgang Denk a ?crit :

Quoted 3 lines

Dear "GROYER, Anthony",

In message<BC0A2F434D4F39448D24A68EA6EFFB9F0194D534@EU-FR-

Quoted 2 lines · EXBE07.eu.corp.airliquide.com> you wrote:

Quoted 2 lines

The use of the initial patches for the

CONFIG_SYS_SKIP_ARM_RELOCATION featu

Quoted 3 lines

Quoted 1 line

res has revealed two issues.

Could you please restict your line length to some 70 characters or

so?

Quoted 3 lines

Thanks.

Quoted 1 line

First issue: the calculation of the relocation offset was done

only if the

Quoted 1 line

Quoted 1 line

relocation is actually done. So we could reach a point where r9

has a wrong

Quoted 1 line

Quoted 1 line

value, since it has never been used before (in my case, this

bug happens w

Quoted 2 lines

This is a configuration error then, isn't it? The relocation

offset

Quoted 2 lines

should be either the intended value, or eventually zero, if no relocation is intended.

Actually, even though "revision 1083" and "revision 1113" are not git
references (and thus I can't be sure Anthony is referring to up-to- date
mainline code), there is a point to what Anthony says: in the case where
relocation is unneeded (r0 equals r6) then r9 is not set, but is still
used when branching to board_init_r().

for this bug to have any effect, relocation would have to be unneeded,
which is a rare case, *and* r9 has to be nonzero, which may or may not
happen depending on the code executed until relocate_code() is called,
and thus makes the whole condition rarer yet; probably the rarity of these two conjunct conditions explains why it was not noticed until now.

However, since start.S has a code path to handle the non-relocating case, this path ought to be bug-free. But then, I want it to be
consistent: if the relocation offset is computed in r9, then testing whether relocation is needed would be done on r9 once computed, not before, by replacing

	adr	r0, _start
	cmp	r0, r6
	beq	clear_bss		/* skip relocation */

With

	adr	r0, _start
	sub	r9, r6, r0
	cmp	r0, #0
	beq	clear_bss		/* skip relocation */

Quoted 3 lines

BTW: your patch has a number ofd coduing style errors, and the
Signed-off-by: line is missing.

Plus it did not have the commit message separator either. I suspect it
was not produced using git format-patch / git send-email.

Anthony, please submit a proper [PATCH], without [RFC], and with the setting of r9 done as shown above, and applied to all relevant start.S
files in arch/arm/cpu/*/ -- in next merge window we should try and factorize those start.S files, BTW.

Quoted 3 lines

Best regards,

Wolfgang Denk

Amicalement,
--
Albert.
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

What is the difference between _start and _TEXT_BASE ? I do not see any differences and the former relocation offset calculation was using _TEXT_BASE.

Dear "GROYER, Anthony",

Quoted 1 line · In message<BC0A2F434D4F39448D24A68EA6EFFB9F0194DA79@EU-FR-EXBE07.eu.corp.airliq…

Quoted 2 lines

What is the difference between _start and _TEXT_BASE ? I do not see any differences and the former relocation offset calculation was using _TEXT_BASE.

The former is the entry point address, while the latter is the start of the text segment. These may be the same (and on many ARM systems they are), but they have actually no direct relation to each other (and some ARM systems do use an entry point that is not the same as the start of the code).

Quoted 10 lines

However, since start.S has a code path to handle the non-relocating case, this path ought to be bug-free. But then, I want it to be
consistent: if the relocation offset is computed in r9, then testing whether relocation is needed would be done on r9 once computed, not before, by replacing

	adr	r0, _start
	cmp	r0, r6
	beq	clear_bss		/* skip relocation */

Quoted 6 lines

With

	adr	r0, _start
	sub	r9, r6, r0
	cmp	r0, #0
	beq	clear_bss		/* skip relocation */

I guess the code is this:
adr r0, _start
sub r9, r6, r0
cmp r9, #0
beq clear_bss /* skip relocation */

What is the difference between _start and _TEXT_BASE ? I do not see any differences and the former relocation offset calculation was using _TEXT_BASE.

May I remove the following code in all arch/arm/cpu/*/start.S ?
ldr r0, _TEXT_BASE /* r0<- Text base */
sub r9, r6, r0 /* r9<- relocation offset */
and expect than the "adr r0, _start" is sufficient ?

Le 21/09/2011 11:29, GROYER, Anthony a ?crit :

3) replace use of r9-r10 with e.g. r10-r11 in the copy loop, to preserve r9 during relocation.

Dear "GROYER, Anthony",
Dear Albert,

Am Mi 21 Sep 2011 12:51:33 CEST schrieb Albert ARIBAUD:

Quoted 2 lines

Le 21/09/2011 11:29, GROYER, Anthony a ?crit :

<snip>

Quoted 2 lines

3) replace use of r9-r10 with e.g. r10-r11 in the copy loop, to preserve r9 during relocation.

If one is changing this place I would like to discuss another point here. In my last changeset for relocation I found some implementation in a/a/c/pxa/start.S which do save the register to stack before copy_loop, use almost all registers (only r8 is not used which is gd_t for arm, but I think it could be used here too cause it is saved on the stack) and save the registers back later on.
I guess this could fasten the copy_loop a bit but needs to be proven. Anthony, if you change all start.S could you consider this also?

best regards

Andreas Bie?mann

Le 21/09/2011 13:20, Andreas Bie?mann a ?crit :

Quoted 13 lines

Dear "GROYER, Anthony",
Dear Albert,

Am Mi 21 Sep 2011 12:51:33 CEST schrieb Albert ARIBAUD:

Quoted 2 lines

Le 21/09/2011 11:29, GROYER, Anthony a ?crit :

<snip>

Quoted 3 lines

3) replace use of r9-r10 with e.g. r10-r11 in the copy loop, to preserve
r9 during relocation.

Quoted 9 lines

If one is changing this place I would like to discuss another point here.
In my last changeset for relocation I found some implementation in a/a/c/pxa/start.S which do save the register to stack before copy_loop, use almost all registers (only r8 is not used which is gd_t for arm, but I think it could be used here too cause it is saved on the stack) and save the registers back later on.
I guess this could fasten the copy_loop a bit but needs to be proven. Anthony, if you change all start.S could you consider this also?

I am not 100% sure I get your point, but I assume that you are asking for *removal* of the saving and restoring, right?

I would tend to
agree that saving and restoring registers in relocate_code is moot, as this function does not return in the usual sense.

As for r8, it should be preserved as it points to gd, but that is ensured by the C code already IIRC.

Dear Albert,

Am Mi 21 Sep 2011 14:03:09 CEST schrieb Albert ARIBAUD:

Quoted 24 lines

Le 21/09/2011 13:20, Andreas Bie?mann a ?crit :

Quoted 22 lines

Dear "GROYER, Anthony",
Dear Albert,

Am Mi 21 Sep 2011 12:51:33 CEST schrieb Albert ARIBAUD:

Quoted 2 lines

Le 21/09/2011 11:29, GROYER, Anthony a ?crit :

<snip>

Quoted 3 lines

3) replace use of r9-r10 with e.g. r10-r11 in the copy loop, to preserve
r9 during relocation.

If one is changing this place I would like to discuss another point here.
In my last changeset for relocation I found some implementation in a/a/c/pxa/start.S which do save the register to stack before copy_loop, use almost all registers (only r8 is not used which is gd_t for arm, but I think it could be used here too cause it is saved on the stack) and save the registers back later on.
I guess this could fasten the copy_loop a bit but needs to be proven. Anthony, if you change all start.S could you consider this also?

Quoted 2 lines

I am not 100% sure I get your point, but I assume that you are asking for *removal* of the saving and restoring, right?

No, that was not the point. I think the 'save registers before copy_loop to use more registers for ldmia/stmia instructions' is a good solution which could improve the copy_loop for all arm implementations.

Quoted 3 lines

I would tend to
agree that saving and restoring registers in relocate_code is moot, as this function does not return in the usual sense.

No the code does register save before copy_loop and restore them right afterwards. Therefore even r8 could be used in the copy_loop cause it is preserved on the (newly created) stack. Have a look at a/a/c/pxa/start.S from line 241 (relocate_code) to 263 (end of copy_loop).
But I guess the ldmia/stmia instructions could even use r3-r11, only r0-r2 needs to be preserved for loop counting.
I wonder if this could improve the copy_loop ... will try to test it these days, if no one else can do it (Anthony?).

Quoted 2 lines

As for r8, it should be preserved as it points to gd, but that is ensured by the C code already IIRC.

We use -ffixed-r8 therefore the compiler takes care for the C part, but we need to respect this in asm.

Well, if we preserve r8 for the copy_loop and restore it right afterwards we could use it in the copy_loop for copy purposes. Cause there is no dereferencing of r8 in copy_loop.

best regards

Andreas Bie?mann

Le 21/09/2011 14:31, Andreas Bie?mann a ?crit :

Quoted 9 lines

Dear Albert,

Am Mi 21 Sep 2011 14:03:09 CEST schrieb Albert ARIBAUD:

Quoted 6 lines

Le 21/09/2011 13:20, Andreas Bie?mann a ?crit :

Quoted 5 lines

Dear "GROYER, Anthony",
Dear Albert,

Am Mi 21 Sep 2011 12:51:33 CEST schrieb Albert ARIBAUD:

Quoted 1 line

Le 21/09/2011 11:29, GROYER, Anthony a ?crit :

Apart from your question (how are the number of registers in ldmia and stmia speed related to the speed of the copy loop?) there is another
one: how do we handle the fact that the length to copy may not be a multiple of the ldmia/stmia 'width'? Even in arm926ejs/start.S, two registers are used, but the alignment for text+data is 4 bytes, not 8.

This did not bite us so far, and should not, since we're going to copy into the space after .text, which *should* be .bss, which we'll zero right after. But Murphy's law could hit...

Quoted 5 lines

Quoted 2 lines

As for r8, it should be preserved as it points to gd, but that is ensured by the C code already IIRC.

We use -ffixed-r8 therefore the compiler takes care for the C part, but we need to respect this in asm.

in arm926ejs/start.S we do. If there are other start.S files where r8 is trashed, they should be fixed indeed.

Dear Albert,

Am Mi 21 Sep 2011 14:03:09 CEST schrieb Albert ARIBAUD:

Quoted 6 lines

Le 21/09/2011 13:20, Andreas Bie?mann a ?crit :

Quoted 5 lines

Dear "GROYER, Anthony",
Dear Albert,

Am Mi 21 Sep 2011 12:51:33 CEST schrieb Albert ARIBAUD:

Quoted 1 line

Le 21/09/2011 11:29, GROYER, Anthony a ?crit :

No the code does register save before copy_loop and restore them right afterwards. Therefore even r8 could be used in the copy_loop cause it is preserved on the (newly created) stack. Have a look at a/a/c/pxa/start.S from line 241 (relocate_code) to 263 (end of copy_loop).
But I guess the ldmia/stmia instructions could even use r3-r11, only r0-r2 needs to be preserved for loop counting.
I wonder if this could improve the copy_loop ... will try to test it these days, if no one else can do it (Anthony?).

Simon, who has just finished his bachelor thesis did this test for me (thanks to this way ...). It seems using more register for ldmia/stmia does _not_ improve the copy
time that much. Simon measured 10.8 ms for r9-r10 and 10.65 ms for r3-r10 (unfortunately I do not know which system he tested, but I guess devkit8000).

best regards

Andreas Bie?mann

[SNIP]

Quoted 9 lines

Simon, who has just finished his bachelor thesis did this test for me (thanks to this way ...).
It seems using more register for ldmia/stmia does _not_ improve the copy time that much. Simon measured 10.8 ms for r9-r10 and 10.65 ms for r3-r10 (unfortunately
I do not know which system he tested, but I guess devkit8000).

best regards

Quoted 3 lines

Andreas Bie?mann

Yes, devkit8000.

Regards
Simon

Hello,

I came back on a discussion started on April 2011.

The use of the initial patches for the CONFIG_SYS_SKIP_ARM_RELOCATION features has revealed two issues.

First issue: the calculation of the relocation offset was done only if the relocation is actually done. So we could reach a point where r9 has a wrong value, since it has never been used before (in my case, this bug happens without JTAG). The first diff moves the relocation offset calculation before the test of a relocation need.

Second issue: board_init_r was thinking the memory area for the malloc is just below the code, whereas the board_init_f had allocated some space for the malloc at the end of the SDRAM. If the code is located at the base of the SDRAM with CONFIG_SYS_SKIP_ARM_RELOCATION defined, the malloc area does not point to a correct memory address. The 2 other diff store the calculated malloc start address in a global data member so that it can be used in board_init_r().

Index: arch/arm/cpu/arm926ejs/start.S
===================================================================

arm/cpu/arm926ejs/start.S ? ? ?(r?vision 1113) renamed from arm/cpu/arm926ejs/start.S ? ? ?(r?vision 1083) +0 -0

@@ -196,6 +196,10 @@

? ? ? ?mov ? ? r4, r0 ?/* save addr_sp */
? ? ? ?mov ? ? r5, r1 ?/* save addr of gd */
? ? ? ?mov ? ? r6, r2 ?/* save addr of destination */
+ ? ?/* set relocation offset here for all cases:
+ ? ? ? ?relocation or not */
+ ? ? ? ldr ? ? r0, _TEXT_BASE ?/* r0 <- Text base */
+ ? ? ? sub ? ? r9, r6, r0 ? ? ? ? ? ? ?/* r9 <- relocation offset */

? ? ? ?/* Set up the stack ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? */ ?stack_setup:

arm/cpu/arm926ejs/start.S ? ? ?(r?vision 1113) renamed from arm/cpu/arm926ejs/start.S ? ? ?(r?vision 1083) +0 -0

@@ -218,8 +222,6 @@

? ? ? ?/*
? ? ? ? * fix .rel.dyn relocations
? ? ? ? */
- ? ? ? ldr ? ? r0, _TEXT_BASE ? ? ? ? ?/* r0 <- Text base */
- ? ? ? sub ? ? r9, r6, r0 ? ? ? ? ? ? ?/* r9 <- relocation offset */ ? ? ? ?ldr ? ? r10, _dynsym_start_ofs ?/* r10 <- sym table ofs */ ? ? ? ?add ? ? r10, r10, r0 ? ? ? ? ? ?/* r10 <- sym table in FLASH */ ? ? ? ?ldr ? ? r2, _rel_dyn_start_ofs ?/* r2 <- rel dyn start ofs */
Index: arch/arm/include/asm/global_data.h
===================================================================

arm/include/asm/global_data.h ?(copie de travail) renamed from arm/include/asm/global_data.h ?(r?vision 1083) +0 -0

@@ -69,6 +69,7 @@

? ? ? ?unsigned long ? mon_len; ? ? ? ?/* monitor len */
? ? ? ?unsigned long ? irq_sp; ? ? ? ? /* irq stack pointer */ ? ? ? ?unsigned long ? start_addr_sp; ?/* start_addr_stackpointer */
+ ? ? ? unsigned long ? start_addr_malloc; ? ? ?/* start_addr_malloc */ ? ? ? ?unsigned long ? reloc_off;
?#if !(defined(CONFIG_SYS_NO_ICACHE) && defined(CONFIG_SYS_NO_DCACHE)) ? ? ? ?unsigned long ? tlb_addr;
Index: arch/arm/lib/board.c
===================================================================

arm/lib/board.c ? ? ? ?(copie de travail) renamed from arm/lib/board.c ? ? ? ?(r?vision 1138) +0 -0

@@ -367,6 +367,7 @@

? ? ? ? * reserve memory for malloc() arena
? ? ? ? */
? ? ? ?addr_sp = addr - TOTAL_MALLOC_LEN;
+ ? ?gd->start_addr_malloc = addr_sp;
? ? ? ?debug ("Reserving %dk for malloc() at: %08lx\n", ? ? ? ? ? ? ? ? ? ? ? ?TOTAL_MALLOC_LEN >> 10, addr_sp); ? ? ? ?/*

arm/lib/board.c ? ? ? ?(copie de travail) renamed from arm/lib/board.c ? ? ? ?(r?vision 1138) +0 -1

@@ -445,7 +446,6 @@
 ?{
 ? ? ? ?char *s;
 ? ? ? ?bd_t *bd;
- ? ? ? ulong malloc_start;
 ?#if !defined(CONFIG_SYS_NO_FLASH)
 ? ? ? ?ulong flash_size;

?#endif

arm/lib/board.c ? ? ? ?(copie de travail) renamed from arm/lib/board.c ? ? ? ?(r?vision 1138) +0 -0

@@ -473,9 +473,7 @@
 ? ? ? ?post_output_backlog ();

?#endif

- ? ? ? /* The Malloc area is immediately below the monitor copy in DRAM */
- ? ? ? malloc_start = dest_addr - TOTAL_MALLOC_LEN;
- ? ? ? mem_malloc_init (malloc_start, TOTAL_MALLOC_LEN);
+ ? ? ? mem_malloc_init (gd->start_addr_malloc, TOTAL_MALLOC_LEN);

?#if !defined(CONFIG_SYS_NO_FLASH)
? ? ? ?puts ("Flash: ");

Regards,
Anthony Groyer

Quoted 86 lines · Aneesh V

Quoted 35 lines

Hi Simon, Wolfgang,

Quoted 25 lines · Simon Glass

Quoted 10 lines · On Fri, Mar 25, 2011 at 11:35 AM, Albert ARIBAUD<[email protected]> ?wrote:

Le 25/03/2011 17:12, Aneesh V a ?crit :

Quoted 6 lines

Another problem I have with relocation is that it makes debugging with JTAG debugers more difficult. The addresses of symbols in the ELF target are no longer valid. Of course, you can load the symbols at an offset from the original location. But one has to first enable debug prints, find the relocation offset, use it while loading the symbols before you can do source level debugging.

Quoted 6 lines

Actually you don't need recompiling: simply set a breakpoint at the entry of relocate_code and once you hit the bp, look up r2: it contains the destination. If you want the offset rather than the absolute address, set the breakpoint right after the 'sub r9, r6, r0' round line 222: r9 will then give you the offset. Unload the current symbols, reload the symbols with the relevant offset, and there you go.

I would like to revisit this thread. I'm not sure how other people do development in U-Boot but I like to use an ICE and a source-level debugger for any significant effort. I think it should be possible to use a JTAG debugging just by loading the u-boot ELF file and running.

Quoted 6 lines

With this patch (or something similar) this is possible. Without it, it is painful.

To be clear, we are not talking here about creating a platform that doesn't use relocation, just that for development purposes it is convenient to be able to disable it.

Quoted 5 lines

Actually, I am not even sure why relocation shouldn't be disabled in my platform for good. It may be useful to have things such as the frame buffer at the end of available memory. But, IMHO, that can still be done without relocating u-boot. That's what the patch does.Am I missing something?

Well relocation does remove a lot of this ad-hoc positioning of things at compile time. I think it is desirable. My point is that it is not engineer-friendly during development, and we should have an easy way to disable it for debugging / JTAG purposes.

Regards,
Simon

Quoted 27 lines

Quoted 27 lines

Looking at the December thread
http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/88067/focus=88262

Aneesh:

Quoted 3 lines

Quoted 3 lines

Shouldn't we provide a CONFIG option by which users can disable Elf relocation?

Wolfgang:

Quoted 3 lines

Why should we? ?It would just make the code even more complicated, and require a lot of additional test cases.

?From what I can see this is a new CONFIG option, two ifdefs in the board.c file, and optionally disabling the -pie position-independent executable option to reduce size. It is pretty trivial:

?arch/arm/config.mk ? | ? ?2 ++
?arch/arm/lib/board.c | ? ?5 +++++
?2 files changed, 7 insertions(+), 0 deletions(-)

Regards,
Simon

Quoted 1 line

Quoted 9 lines

Quoted 8 lines

Quoted 8 lines

Amicalement,
--
Albert.
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Quoted 5 lines

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Hi Anthony,

Quoted 9 lines · Anthony

Hello,

I came back on a discussion started on April 2011.

The use of the initial patches for the CONFIG_SYS_SKIP_ARM_RELOCATION features has revealed two issues.

First issue: the calculation of the relocation offset was done only if the relocation is actually done. So we could reach a point where r9 has a wrong value, since it has never been used before (in my case, this bug happens without JTAG). The first diff moves the relocation offset calculation before the test of a relocation need.

Quoted 2 lines

Second issue: board_init_r was thinking the memory area for the malloc is just below the code, whereas the board_init_f had allocated some space for the malloc at the end of the SDRAM. If the code is located at the base of the SDRAM with CONFIG_SYS_SKIP_ARM_RELOCATION defined, the malloc area does not point to a correct memory address. The 2 other diff store the calculated malloc start address in a global data member so that it can be used in board_init_r().

Thanks for coming back to this. Hopefully you can do patches for the other CPU types also as Albert requests - and I can test armv7 when you do that. This feature is one that I use a lot with an ICE and I would like to see it in U-Boot.

Regards,
Simon

Quoted 163 lines

Index: arch/arm/cpu/arm926ejs/start.S
===================================================================

arm/cpu/arm926ejs/start.S (r?vision 1113) renamed from arm/cpu/arm926ejs/start.S (r?vision 1083) +4 -2

@@ -196,6 +196,10 @@
       mov     r4, r0  /* save addr_sp */
       mov     r5, r1  /* save addr of gd */
       mov     r6, r2  /* save addr of destination */
+    /* set relocation offset here for all cases:
+        relocation or not */
+       ldr     r0, _TEXT_BASE  /* r0 <- Text base */
+       sub     r9, r6, r0              /* r9 <- relocation offset */
 
       /* Set up the stack                                                 */
 stack_setup:

@@ -218,8 +222,6 @@
       /*
        * fix .rel.dyn relocations
        */
-       ldr     r0, _TEXT_BASE          /* r0 <- Text base */
-       sub     r9, r6, r0              /* r9 <- relocation offset */
       ldr     r10, _dynsym_start_ofs  /* r10 <- sym table ofs */
       add     r10, r10, r0            /* r10 <- sym table in FLASH */
       ldr     r2, _rel_dyn_start_ofs  /* r2 <- rel dyn start ofs */

Index: arch/arm/include/asm/global_data.h
===================================================================

arm/include/asm/global_data.h (copie de travail) renamed from arm/include/asm/global_data.h (r?vision 1083) +1 -0

@@ -69,6 +69,7 @@
       unsigned long   mon_len;        /* monitor len */
       unsigned long   irq_sp;         /* irq stack pointer */
       unsigned long   start_addr_sp;  /* start_addr_stackpointer */
+       unsigned long   start_addr_malloc;      /* start_addr_malloc */
       unsigned long   reloc_off;
 #if !(defined(CONFIG_SYS_NO_ICACHE) && defined(CONFIG_SYS_NO_DCACHE))
       unsigned long   tlb_addr;

Index: arch/arm/lib/board.c
===================================================================

arm/lib/board.c (copie de travail) renamed from arm/lib/board.c (r?vision 1138) +2 -4

@@ -367,6 +367,7 @@
        * reserve memory for malloc() arena
        */
       addr_sp = addr - TOTAL_MALLOC_LEN;
+    gd->start_addr_malloc = addr_sp;
       debug ("Reserving %dk for malloc() at: %08lx\n",
                       TOTAL_MALLOC_LEN >> 10, addr_sp);
       /*

@@ -445,7 +446,6 @@
 {
       char *s;
       bd_t *bd;
-       ulong malloc_start;
 #if !defined(CONFIG_SYS_NO_FLASH)
       ulong flash_size;
 #endif

@@ -473,9 +473,7 @@
       post_output_backlog ();
 #endif
 
-       /* The Malloc area is immediately below the monitor copy in DRAM */
-       malloc_start = dest_addr - TOTAL_MALLOC_LEN;
-       mem_malloc_init (malloc_start, TOTAL_MALLOC_LEN);
+       mem_malloc_init (gd->start_addr_malloc, TOTAL_MALLOC_LEN);
 
 #if !defined(CONFIG_SYS_NO_FLASH)
       puts ("Flash: ");

Regards,
Anthony Groyer

Quoted 86 lines · Aneesh V

Quoted 35 lines

Hi Simon, Wolfgang,

Quoted 25 lines · Simon Glass

Quoted 10 lines · Albert ARIBAUD

Le 25/03/2011 17:12, Aneesh V a ?crit :

Quoted 6 lines

Another problem I have with relocation is that it makes debugging with JTAG debugers more difficult. The addresses of symbols in the ELF target are no longer valid. Of course, you can load the symbols at an offset from the original location. But one has to first enable debug prints, find the relocation offset, use it while loading the symbols before you can do source level debugging.

Quoted 6 lines

Actually you don't need recompiling: simply set a breakpoint at the entry of relocate_code and once you hit the bp, look up r2: it contains the destination. If you want the offset rather than the absolute address, set the breakpoint right after the 'sub r9, r6, r0' round line 222: r9 will then give you the offset. Unload the current symbols, reload the symbols with the relevant offset, and there you go.

I would like to revisit this thread. I'm not sure how other people do development in U-Boot but I like to use an ICE and a source-level debugger for any significant effort. I think it should be possible to use a JTAG debugging just by loading the u-boot ELF file and running.

Quoted 6 lines

With this patch (or something similar) this is possible. Without it, it is painful.

To be clear, we are not talking here about creating a platform that doesn't use relocation, just that for development purposes it is convenient to be able to disable it.

Quoted 5 lines

Actually, I am not even sure why relocation shouldn't be disabled in my platform for good. It may be useful to have things such as the frame buffer at the end of available memory. But, IMHO, that can still be done without relocating u-boot. That's what the patch does.Am I missing something?

Well relocation does remove a lot of this ad-hoc positioning of things at compile time. I think it is desirable. My point is that it is not engineer-friendly during development, and we should have an easy way to disable it for debugging / JTAG purposes.

Regards,
Simon

Quoted 27 lines

Quoted 27 lines

Looking at the December thread
http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/88067/focus=88262

Aneesh:

Quoted 3 lines

Quoted 3 lines

Shouldn't we provide a CONFIG option by which users can disable Elf relocation?

Wolfgang:

Quoted 3 lines

Why should we? It would just make the code even more complicated, and require a lot of additional test cases.

From what I can see this is a new CONFIG option, two ifdefs in the
board.c file, and optionally disabling the -pie position-independent executable option to reduce size. It is pretty trivial:

 arch/arm/config.mk   |    2 ++
 arch/arm/lib/board.c |    5 +++++
 2 files changed, 7 insertions(+), 0 deletions(-)

Regards,
Simon

Quoted 1 line

Quoted 9 lines

Quoted 8 lines

Quoted 8 lines

Amicalement,
--
Albert.
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Quoted 5 lines

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Quoted 5 lines

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Hi Simon,

Quoted 13 lines · Simon Glass

Hi Anthony,

Quoted 9 lines · Anthony

Hello,

I came back on a discussion started on April 2011.

The use of the initial patches for the CONFIG_SYS_SKIP_ARM_RELOCATION features has revealed two issues.

First issue: the calculation of the relocation offset was done only if the relocation is actually done. So we could reach a point where r9 has a wrong value, since it has never been used before (in my case, this bug happens without JTAG). The first diff moves the relocation offset calculation before the test of a relocation need.

Quoted 7 lines

Quoted 2 lines

Second issue: board_init_r was thinking the memory area for the malloc is just below the code, whereas the board_init_f had allocated some space for the malloc at the end of the SDRAM. If the code is located at the base of the SDRAM with CONFIG_SYS_SKIP_ARM_RELOCATION defined, the malloc area does not point to a correct memory address. The 2 other diff store the calculated malloc start address in a global data member so that it can be used in board_init_r().

Thanks for coming back to this. Hopefully you can do patches for the other CPU types also as Albert requests - and I can test armv7 when you do that. This feature is one that I use a lot with an ICE and I would like to see it in U-Boot.

Are you looking for CONFIG_SYS_SKIP_ARM_RELOCATION? I think Anthony is only fixing couple of issues uncovered by the original 'skip relocation' patch but I don't think CONFIG_SYS_SKIP_ARM_RELOCATION itself is getting accepted.

best regards,
Aneesh

Quoted 176 lines

Regards,
Simon

Quoted 163 lines

Index: arch/arm/cpu/arm926ejs/start.S
===================================================================

arm/cpu/arm926ejs/start.S ? ? ?(r?vision 1113) renamed from arm/cpu/arm926ejs/start.S ? ? ?(r?vision 1083) +0 -0

@@ -196,6 +196,10 @@

? ? ? ?mov ? ? r4, r0 ?/* save addr_sp */
? ? ? ?mov ? ? r5, r1 ?/* save addr of gd */
? ? ? ?mov ? ? r6, r2 ?/* save addr of destination */
+ ? ?/* set relocation offset here for all cases:
+ ? ? ? ?relocation or not */
+ ? ? ? ldr ? ? r0, _TEXT_BASE ?/* r0 <- Text base */
+ ? ? ? sub ? ? r9, r6, r0 ? ? ? ? ? ? ?/* r9 <- relocation offset */

? ? ? ?/* Set up the stack ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? */ ?stack_setup:

arm/cpu/arm926ejs/start.S ? ? ?(r?vision 1113) renamed from arm/cpu/arm926ejs/start.S ? ? ?(r?vision 1083) +0 -0

@@ -218,8 +222,6 @@

? ? ? ?/*
? ? ? ? * fix .rel.dyn relocations
? ? ? ? */
- ? ? ? ldr ? ? r0, _TEXT_BASE ? ? ? ? ?/* r0 <- Text base */
- ? ? ? sub ? ? r9, r6, r0 ? ? ? ? ? ? ?/* r9 <- relocation offset */ ? ? ? ?ldr ? ? r10, _dynsym_start_ofs ?/* r10 <- sym table ofs */ ? ? ? ?add ? ? r10, r10, r0 ? ? ? ? ? ?/* r10 <- sym table in FLASH */ ? ? ? ?ldr ? ? r2, _rel_dyn_start_ofs ?/* r2 <- rel dyn start ofs */
Index: arch/arm/include/asm/global_data.h
===================================================================

arm/include/asm/global_data.h ?(copie de travail) renamed from arm/include/asm/global_data.h ?(r?vision 1083) +0 -0

@@ -69,6 +69,7 @@

? ? ? ?unsigned long ? mon_len; ? ? ? ?/* monitor len */
? ? ? ?unsigned long ? irq_sp; ? ? ? ? /* irq stack pointer */ ? ? ? ?unsigned long ? start_addr_sp; ?/* start_addr_stackpointer */
+ ? ? ? unsigned long ? start_addr_malloc; ? ? ?/* start_addr_malloc */ ? ? ? ?unsigned long ? reloc_off;
?#if !(defined(CONFIG_SYS_NO_ICACHE) && defined(CONFIG_SYS_NO_DCACHE)) ? ? ? ?unsigned long ? tlb_addr;
Index: arch/arm/lib/board.c
===================================================================

arm/lib/board.c ? ? ? ?(copie de travail) renamed from arm/lib/board.c ? ? ? ?(r?vision 1138) +0 -0

@@ -367,6 +367,7 @@

? ? ? ? * reserve memory for malloc() arena
? ? ? ? */
? ? ? ?addr_sp = addr - TOTAL_MALLOC_LEN;
+ ? ?gd->start_addr_malloc = addr_sp;
? ? ? ?debug ("Reserving %dk for malloc() at: %08lx\n", ? ? ? ? ? ? ? ? ? ? ? ?TOTAL_MALLOC_LEN >> 10, addr_sp); ? ? ? ?/*

arm/lib/board.c ? ? ? ?(copie de travail) renamed from arm/lib/board.c ? ? ? ?(r?vision 1138) +0 -1

@@ -445,7 +446,6 @@
 ?{
 ? ? ? ?char *s;
 ? ? ? ?bd_t *bd;
- ? ? ? ulong malloc_start;
 ?#if !defined(CONFIG_SYS_NO_FLASH)
 ? ? ? ?ulong flash_size;

?#endif

arm/lib/board.c ? ? ? ?(copie de travail) renamed from arm/lib/board.c ? ? ? ?(r?vision 1138) +0 -0

@@ -473,9 +473,7 @@
 ? ? ? ?post_output_backlog ();

?#endif

- ? ? ? /* The Malloc area is immediately below the monitor copy in DRAM */
- ? ? ? malloc_start = dest_addr - TOTAL_MALLOC_LEN;
- ? ? ? mem_malloc_init (malloc_start, TOTAL_MALLOC_LEN);
+ ? ? ? mem_malloc_init (gd->start_addr_malloc, TOTAL_MALLOC_LEN);

?#if !defined(CONFIG_SYS_NO_FLASH)
? ? ? ?puts ("Flash: ");

Regards,
Anthony Groyer

Quoted 86 lines · Aneesh V

Quoted 35 lines

Hi Simon, Wolfgang,

Quoted 25 lines · Simon Glass

Quoted 10 lines · On Fri, Mar 25, 2011 at 11:35 AM, Albert ARIBAUD<[email protected]> ?wrote:

Le 25/03/2011 17:12, Aneesh V a ?crit :

Quoted 6 lines

Another problem I have with relocation is that it makes debugging with JTAG debugers more difficult. The addresses of symbols in the ELF target are no longer valid. Of course, you can load the symbols at an offset from the original location. But one has to first enable debug prints, find the relocation offset, use it while loading the symbols before you can do source level debugging.

Quoted 6 lines

Actually you don't need recompiling: simply set a breakpoint at the entry of relocate_code and once you hit the bp, look up r2: it contains the destination. If you want the offset rather than the absolute address, set the breakpoint right after the 'sub r9, r6, r0' round line 222: r9 will then give you the offset. Unload the current symbols, reload the symbols with the relevant offset, and there you go.

I would like to revisit this thread. I'm not sure how other people do development in U-Boot but I like to use an ICE and a source-level debugger for any significant effort. I think it should be possible to use a JTAG debugging just by loading the u-boot ELF file and running.

Quoted 6 lines

With this patch (or something similar) this is possible. Without it, it is painful.

To be clear, we are not talking here about creating a platform that doesn't use relocation, just that for development purposes it is convenient to be able to disable it.

Quoted 5 lines

Actually, I am not even sure why relocation shouldn't be disabled in my platform for good. It may be useful to have things such as the frame buffer at the end of available memory. But, IMHO, that can still be done without relocating u-boot. That's what the patch does.Am I missing something?

Well relocation does remove a lot of this ad-hoc positioning of things at compile time. I think it is desirable. My point is that it is not engineer-friendly during development, and we should have an easy way to disable it for debugging / JTAG purposes.

Regards,
Simon

Quoted 27 lines

Quoted 27 lines

Looking at the December thread
http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/88067/focus=88262

Aneesh:

Quoted 3 lines

Quoted 3 lines

Shouldn't we provide a CONFIG option by which users can disable Elf relocation?

Wolfgang:

Quoted 3 lines

Why should we? ?It would just make the code even more complicated, and require a lot of additional test cases.

?From what I can see this is a new CONFIG option, two ifdefs in the board.c file, and optionally disabling the -pie position-independent executable option to reduce size. It is pretty trivial:

?arch/arm/config.mk ? | ? ?2 ++
?arch/arm/lib/board.c | ? ?5 +++++
?2 files changed, 7 insertions(+), 0 deletions(-)

Regards,
Simon

Quoted 1 line

Quoted 9 lines

Quoted 8 lines

Quoted 8 lines

Amicalement,
--
Albert.
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Quoted 5 lines

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Quoted 5 lines

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Quoted 4 lines

Are you looking for CONFIG_SYS_SKIP_ARM_RELOCATION? I think Anthony is only fixing couple of issues uncovered by the original 'skip relocation' patch but I don't think CONFIG_SYS_SKIP_ARM_RELOCATION itself is getting accepted.

I see. That is sad, because skipping relocation is very useful for development. Why do we make things harder for devs than they need to be?

Regards,
Simon

Hi Simon,

Le 23/09/2011 18:04, Simon Glass a ?crit :

Quoted 8 lines

Quoted 4 lines

Are you looking for CONFIG_SYS_SKIP_ARM_RELOCATION? I think Anthony is only fixing couple of issues uncovered by the original 'skip relocation' patch but I don't think CONFIG_SYS_SKIP_ARM_RELOCATION itself is getting accepted.

I see. That is sad, because skipping relocation is very useful for development. Why do we make things harder for devs than they need to be?

There is at least a possibility to avoid relocation without introducing the CONFIG_SYS_SKIP_ARM_RELOCATION flag; set CONFIG_SKIP_LOWLEVEL_INIT and adjust CONFIG_SYS_TEXT_BASE to be the base address where your code will be located. It is a two-round process (you need a first run with the 'wrong' CONFIG_SYS_TEXT_BASE in order to find its 'right' value) and will work only for a given board variant (same available RAM amount always) but might be enough to cover the use case(s) you are looking for?

Amicalement,
--
Albert.