[U-Boot] [RFC PATCH] arm: provide a CONFIG flag for disabling relocation

[U-Boot] [RFC PATCH] arm: provide a CONFIG flag for disabling relocation

[GROYER, 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.

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
===================================================================
--- arch/arm/cpu/arm926ejs/start.S	(r?vision 1083)
+++ arch/arm/cpu/arm926ejs/start.S	(r?vision 1113)
@@ -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
===================================================================
--- arch/arm/include/asm/global_data.h	(r?vision 1083)
+++ arch/arm/include/asm/global_data.h	(copie de travail)
@@ -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
===================================================================
--- arch/arm/lib/board.c	(r?vision 1138)
+++ arch/arm/lib/board.c	(copie de travail)
@@ -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

>On Wed, Apr 20, 2011 at 11:56 PM, Aneesh V <ane...@ti.com> wrote:
>> Hi Simon, Wolfgang,
>>
>> On Thursday 21 April 2011 12:04 AM, Simon Glass wrote:
>>>
>>> On Fri, Mar 25, 2011 at 11:35 AM, Albert ARIBAUD<albert.arib...@free.fr>
>>>  wrote:
>>>>
>>>> Le 25/03/2011 17:12, Aneesh V a ?crit :
>>>>
>>>>> 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.
>>>>
>>>> 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.
>>>
>>> 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.
>>
>> 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
>
>>
>>>
>>> Looking at the December thread
>>> http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/88067/focus=88262
>>>
>>> Aneesh:
>>>>>
>>>>> Shouldn't we provide a CONFIG option by which users can disable
>>>>> Elf relocation?
>>>
>>> Wolfgang:
>>>>
>>>> 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
>>>
>>>>
>>>> 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
>

[U-Boot] [RFC PATCH] arm: provide a CONFIG flag for disabling relocation

[Wolfgang Denk]

Dear "GROYER, Anthony",

In message <BC0A2F434D4F39448D24A68EA6EFFB9F0194D534@EU-FR-EXBE07.eu.corp.airliquide.com> you wrote:
> 
> 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.

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

>  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

This is a broken memory layout then, i. e. another configuration
error.


So actually both your "issues" are caused by configuration errors and
should go away if you fix your system configuration.


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

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Any sufficiently advanced technology is indistinguishable from magic.
                                                   - Arthur C. Clarke

[U-Boot] [RFC PATCH] arm: provide a CONFIG flag for disabling relocation

[Albert ARIBAUD]

Le 20/09/2011 20:09, Wolfgang Denk a ?crit :
> Dear "GROYER, Anthony",
>
> In message<BC0A2F434D4F39448D24A68EA6EFFB9F0194D534@EU-FR-EXBE07.eu.corp.airliquide.com>  you wrote:
>>
>> 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.
>
>> 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.

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

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

> Best regards,
>
> Wolfgang Denk

Amicalement,
-- 
Albert.

[U-Boot] [RFC PATCH] arm: provide a CONFIG flag for disabling relocation

[GROYER, Anthony]

> -----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 :
> > Dear "GROYER, Anthony",
> >
> > In message<BC0A2F434D4F39448D24A68EA6EFFB9F0194D534@EU-FR-
> EXBE07.eu.corp.airliquide.com>  you wrote:
> >>
> >> 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.
> >
> >> 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.
> 
> 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 */


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 ?

Anthony

> 
> > 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.
> 
> > Best regards,
> >
> > Wolfgang Denk
> 
> Amicalement,
> --
> Albert.
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [RFC PATCH] arm: provide a CONFIG flag for disabling relocation

[Wolfgang Denk]

Dear "GROYER, Anthony",

In message <BC0A2F434D4F39448D24A68EA6EFFB9F0194DA79@EU-FR-EXBE07.eu.corp.airliquide.com> you wrote:
>
> 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).

Please better not confuse these two...

Best regards,

Wolfgang Denk

--
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
A committee is a life form with six or more legs and no brain.
                              -- Lazarus Long, "Time Enough For Love"

[U-Boot] [RFC PATCH] arm: provide a CONFIG flag for disabling relocation

[Albert ARIBAUD]

Le 21/09/2011 12:45, Wolfgang Denk a ?crit :
> Dear "GROYER, Anthony",
>
> In message<BC0A2F434D4F39448D24A68EA6EFFB9F0194DA79@EU-FR-EXBE07.eu.corp.airliquide.com>  you wrote:
>>
>> 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).

This would be boards

- where U-Boot boots from Flash without a SPL,

- which boot at FFFF0000,

- and which don't have a tiny piece of code at FFFF0000 which jumps to a 
fixed location at which _start resides.

Thus, typically boards with a very small FLASH that forces the 
maintainer to fill the last 64 KB with _start and some code, then put 
the rest of U-Boot below FFFF0000.

But my edminiv2, with only 512 KB flash, already provides enough space 
that such a complicated linker mapping is unneeded -- FFFF0000 just has 
a permanent jump instruction to FFF90000, and U-Boot is linked linearly, 
with _start at FFF9000.

I wonder which ARM boards we have that still require a complex mapping 
with _start in the middle of the code.

Amicalement,
-- 
Albert.

[U-Boot] [RFC PATCH] arm: provide a CONFIG flag for disabling relocation

[Albert ARIBAUD]

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

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

The diff?rence is that when using "ldr r0,_TEXT_BASE", you depend on 
U-Boot having been actually loaded at the address specified by 
CONFIG_SYS_TEXT_BASE -- that *should* be the case, but if it is not, the 
code won't work.

OTOH, using "adr r0,_start" is actually translated into "sub r0, pc, 
#nnn", thus always setting r0 to the actual base address of U-Boot -- it 
will work even when U-Boot is not run at CONFIG_SYS_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 ?

Upon *third* look, it is a bit more complicated.

Granted, you want r9 set even if you don't relocate.
But the current relocation code *trashes* r9 in the copy loop, before 
setting it right before the relocation loop.
And *then* it is used for jumping to board_init_r.

So the correct fix would be to

1) set r9 early, before checking if relocation is needed so that a 
direct jump to clear_bss has the correct r9 whichever way.

2) remove the late setting of r9 between the copy and relocation loop, 
as it was already set up in 1)

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

4) Replace all "ldr rNN, _TEXT_BASE" with "adr rNN, _start".

Best is you submit a new RFC patch and when it is OK and you have 
validated through JTAG that it works, then re-submit it as a non-RFC 
patch to all start.S files in arch/arm/cpu/*.

Amicalement,
-- 
Albert.

[U-Boot] [RFC PATCH] arm: provide a CONFIG flag for disabling relocation

[Andreas Bießmann]

Dear "GROYER, Anthony",
Dear Albert,

Am Mi 21 Sep 2011 12:51:33 CEST schrieb Albert ARIBAUD:
> Le 21/09/2011 11:29, GROYER, Anthony a ?crit :
>

<snip>

> 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

[U-Boot] [RFC PATCH] arm: provide a CONFIG flag for disabling relocation

[Albert ARIBAUD]

Le 21/09/2011 13:20, Andreas Bie?mann a ?crit :
> Dear "GROYER, Anthony",
> Dear Albert,
>
> Am Mi 21 Sep 2011 12:51:33 CEST schrieb Albert ARIBAUD:
>> Le 21/09/2011 11:29, GROYER, Anthony a ?crit :
>>
>
> <snip>
>
>> 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?

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.

> best regards
>
> Andreas Bie?mann

Amicalement,
-- 
Albert.

[U-Boot] [RFC PATCH] arm: provide a CONFIG flag for disabling relocation

[Andreas Bießmann]

Dear Albert,

Am Mi 21 Sep 2011 14:03:09 CEST schrieb Albert ARIBAUD:
> Le 21/09/2011 13:20, Andreas Bie?mann a ?crit :
>> Dear "GROYER, Anthony",
>> Dear Albert,
>>
>> Am Mi 21 Sep 2011 12:51:33 CEST schrieb Albert ARIBAUD:
>>> Le 21/09/2011 11:29, GROYER, Anthony a ?crit :
>>>
>>
>> <snip>
>>
>>> 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?
>
> 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.

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

> 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

[U-Boot] [RFC PATCH] arm: provide a CONFIG flag for disabling relocation

[Albert ARIBAUD]

Le 21/09/2011 14:31, Andreas Bie?mann a ?crit :
> Dear Albert,
>
> Am Mi 21 Sep 2011 14:03:09 CEST schrieb Albert ARIBAUD:
>> Le 21/09/2011 13:20, Andreas Bie?mann a ?crit :
>>> Dear "GROYER, Anthony",
>>> Dear Albert,
>>>
>>> Am Mi 21 Sep 2011 12:51:33 CEST schrieb Albert ARIBAUD:
>>>> Le 21/09/2011 11:29, GROYER, Anthony a ?crit :
>>>>
>>>
>>> <snip>
>>>
>>>> 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?
>>
>> 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.

I see -- you want to do a ldmia/stmia with a lot of regs.

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

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

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

> 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

Amicalement,
-- 
Albert.

[U-Boot] [RFC PATCH] arm: provide a CONFIG flag for disabling relocation

[Andreas Bießmann]

Dear Albert,

Am Mi 21 Sep 2011 16:23:56 CEST schrieb Albert ARIBAUD:
> Le 21/09/2011 14:31, Andreas Bie?mann a ?crit :
>> Dear Albert,
>>
>> Am Mi 21 Sep 2011 14:03:09 CEST schrieb Albert ARIBAUD:
>>> Le 21/09/2011 13:20, Andreas Bie?mann a ?crit :
>>>> Dear "GROYER, Anthony",
>>>> Dear Albert,
>>>>
>>>> Am Mi 21 Sep 2011 12:51:33 CEST schrieb Albert ARIBAUD:
>>>>> Le 21/09/2011 11:29, GROYER, Anthony a ?crit :

<snip>

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

Good point. Well, we could implement some 'rewind' apart from .bss
zeroing, if we care about it. I guess this could be done without any
impact to copy time, if we really save time by using more regs for
ldmia/stmia.

But how about reading beyond the _end symbol from flash crossing some
border e.g. end of address space, end of flash space ... ?

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

There should no one else left, my last changeset fixed it globally:
http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/90010/focus=90013

best regards

Andreas Bie?mann

[U-Boot] [RFC PATCH] arm: provide a CONFIG flag for disabling relocation

[Andreas Bießmann]

Dear Albert,

Am 21.09.2011 um 14:31 schrieb Andreas Bie?mann:

> Dear Albert,
> 
> Am Mi 21 Sep 2011 14:03:09 CEST schrieb Albert ARIBAUD:
>> Le 21/09/2011 13:20, Andreas Bie?mann a ?crit :
>>> Dear "GROYER, Anthony",
>>> Dear Albert,
>>> 
>>> Am Mi 21 Sep 2011 12:51:33 CEST schrieb Albert ARIBAUD:
>>>> Le 21/09/2011 11:29, GROYER, Anthony a ?crit :

<snip>

> 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

[U-Boot] [RFC PATCH] arm: provide a CONFIG flag for disabling relocation

[Simon Schwarz]

On 09/29/2011 06:14 PM, Andreas Bie?mann wrote:
[SNIP]
> 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
>
>
Yes, devkit8000.

Regards
Simon

[U-Boot] [RFC PATCH] arm: provide a CONFIG flag for disabling relocation

[Albert ARIBAUD]

Le 30/09/2011 09:21, Simon Schwarz a ?crit :
> On 09/29/2011 06:14 PM, Andreas Bie?mann wrote:
> [SNIP]
>> 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
>>
>>
> Yes, devkit8000.
>
> Regards
> Simon

Thanks to both of you. I'd somehow expected there not to be a great 
increase as the true limit is bus width.

So let us stick with two registers and avoid register pushes/pops.

Amicalement,
-- 
Albert.

[U-Boot] [RFC PATCH] arm: provide a CONFIG flag for disabling relocation

[Simon Glass]

Hi Anthony,

On Tue, Sep 20, 2011 at 7:22 AM, GROYER, Anthony
<Anthony.GROYER@airliquide.com> wrote:
>
> 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().

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

>
> Index: arch/arm/cpu/arm926ejs/start.S
> ===================================================================
> --- arch/arm/cpu/arm926ejs/start.S ? ? ?(r?vision 1083)
> +++ arch/arm/cpu/arm926ejs/start.S ? ? ?(r?vision 1113)
> @@ -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
> ===================================================================
> --- arch/arm/include/asm/global_data.h ?(r?vision 1083)
> +++ arch/arm/include/asm/global_data.h ?(copie de travail)
> @@ -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
> ===================================================================
> --- arch/arm/lib/board.c ? ? ? ?(r?vision 1138)
> +++ arch/arm/lib/board.c ? ? ? ?(copie de travail)
> @@ -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
>
>>On Wed, Apr 20, 2011 at 11:56 PM, Aneesh V <ane...@ti.com> wrote:
>>> Hi Simon, Wolfgang,
>>>
>>> On Thursday 21 April 2011 12:04 AM, Simon Glass wrote:
>>>>
>>>> On Fri, Mar 25, 2011 at 11:35 AM, Albert ARIBAUD<albert.arib...@free.fr>
>>>> ?wrote:
>>>>>
>>>>> Le 25/03/2011 17:12, Aneesh V a ?crit :
>>>>>
>>>>>> 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.
>>>>>
>>>>> 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.
>>>>
>>>> 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.
>>>
>>> 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
>>
>>>
>>>>
>>>> Looking at the December thread
>>>> http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/88067/focus=88262
>>>>
>>>> Aneesh:
>>>>>>
>>>>>> Shouldn't we provide a CONFIG option by which users can disable
>>>>>> Elf relocation?
>>>>
>>>> Wolfgang:
>>>>>
>>>>> 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
>>>>
>>>>>
>>>>> 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
>>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>

[U-Boot] [RFC PATCH] arm: provide a CONFIG flag for disabling relocation

[Aneesh V]

Hi Simon,

On Wednesday 21 September 2011 03:04 AM, Simon Glass wrote:
> Hi Anthony,
> 
> On Tue, Sep 20, 2011 at 7:22 AM, GROYER, Anthony
> <Anthony.GROYER@airliquide.com> wrote:
>>
>> 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().
> 
> 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

> 
> Regards,
> Simon
> 
>>
>> Index: arch/arm/cpu/arm926ejs/start.S
>> ===================================================================
>> --- arch/arm/cpu/arm926ejs/start.S      (r?vision 1083)
>> +++ arch/arm/cpu/arm926ejs/start.S      (r?vision 1113)
>> @@ -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
>> ===================================================================
>> --- arch/arm/include/asm/global_data.h  (r?vision 1083)
>> +++ arch/arm/include/asm/global_data.h  (copie de travail)
>> @@ -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
>> ===================================================================
>> --- arch/arm/lib/board.c        (r?vision 1138)
>> +++ arch/arm/lib/board.c        (copie de travail)
>> @@ -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
>>
>>> On Wed, Apr 20, 2011 at 11:56 PM, Aneesh V <ane...@ti.com> wrote:
>>>> Hi Simon, Wolfgang,
>>>>
>>>> On Thursday 21 April 2011 12:04 AM, Simon Glass wrote:
>>>>>
>>>>> On Fri, Mar 25, 2011 at 11:35 AM, Albert ARIBAUD<albert.arib...@free.fr>
>>>>>  wrote:
>>>>>>
>>>>>> Le 25/03/2011 17:12, Aneesh V a ?crit :
>>>>>>
>>>>>>> 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.
>>>>>>
>>>>>> 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.
>>>>>
>>>>> 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.
>>>>
>>>> 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
>>>
>>>>
>>>>>
>>>>> Looking at the December thread
>>>>> http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/88067/focus=88262
>>>>>
>>>>> Aneesh:
>>>>>>>
>>>>>>> Shouldn't we provide a CONFIG option by which users can disable
>>>>>>> Elf relocation?
>>>>>
>>>>> Wolfgang:
>>>>>>
>>>>>> 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
>>>>>
>>>>>>
>>>>>> 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
>>>
>> _______________________________________________
>> 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

[U-Boot] [RFC PATCH] arm: provide a CONFIG flag for disabling relocation

[Simon Glass]

Hi Aneesh,

On Wed, Sep 21, 2011 at 7:21 AM, Aneesh V <aneesh@ti.com> wrote:
> Hi Simon,
>
> On Wednesday 21 September 2011 03:04 AM, Simon Glass wrote:
>> Hi Anthony,
>>
>> On Tue, Sep 20, 2011 at 7:22 AM, GROYER, Anthony
>> <Anthony.GROYER@airliquide.com> wrote:
>>>
>>> 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().
>>
>> 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.

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

>
> best regards,
> Aneesh
>
>>
>> Regards,
>> Simon
>>
>>>
>>> Index: arch/arm/cpu/arm926ejs/start.S
>>> ===================================================================
>>> --- arch/arm/cpu/arm926ejs/start.S ? ? ?(r?vision 1083)
>>> +++ arch/arm/cpu/arm926ejs/start.S ? ? ?(r?vision 1113)
>>> @@ -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
>>> ===================================================================
>>> --- arch/arm/include/asm/global_data.h ?(r?vision 1083)
>>> +++ arch/arm/include/asm/global_data.h ?(copie de travail)
>>> @@ -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
>>> ===================================================================
>>> --- arch/arm/lib/board.c ? ? ? ?(r?vision 1138)
>>> +++ arch/arm/lib/board.c ? ? ? ?(copie de travail)
>>> @@ -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
>>>
>>>> On Wed, Apr 20, 2011 at 11:56 PM, Aneesh V <ane...@ti.com> wrote:
>>>>> Hi Simon, Wolfgang,
>>>>>
>>>>> On Thursday 21 April 2011 12:04 AM, Simon Glass wrote:
>>>>>>
>>>>>> On Fri, Mar 25, 2011 at 11:35 AM, Albert ARIBAUD<albert.arib...@free.fr>
>>>>>> ?wrote:
>>>>>>>
>>>>>>> Le 25/03/2011 17:12, Aneesh V a ?crit :
>>>>>>>
>>>>>>>> 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.
>>>>>>>
>>>>>>> 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.
>>>>>>
>>>>>> 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.
>>>>>
>>>>> 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
>>>>
>>>>>
>>>>>>
>>>>>> Looking at the December thread
>>>>>> http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/88067/focus=88262
>>>>>>
>>>>>> Aneesh:
>>>>>>>>
>>>>>>>> Shouldn't we provide a CONFIG option by which users can disable
>>>>>>>> Elf relocation?
>>>>>>
>>>>>> Wolfgang:
>>>>>>>
>>>>>>> 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
>>>>>>
>>>>>>>
>>>>>>> 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
>>>>
>>> _______________________________________________
>>> 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
>

[U-Boot] [RFC PATCH] arm: provide a CONFIG flag for disabling relocation

[Albert ARIBAUD]

Hi Simon,

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

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

> Regards,
> Simon

Amicalement,
-- 
Albert.

[U-Boot] [RFC PATCH] arm: provide a CONFIG flag for disabling relocation

[Simon Glass]

Hi Albert,

On Sat, Oct 1, 2011 at 12:01 AM, Albert ARIBAUD
<albert.u.boot@aribaud.net> wrote:
> Hi Simon,
>
> Le 23/09/2011 18:04, Simon Glass a ?crit :
>
>>> 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?

In other words, still relocation, but we 'know' where it will relocate
to after it does its maths.

It works, but it's a little brittle, and I would prefer a simple
option to disable relocation for devs. I keep such a patch around for
when I am using an ICE. Is the concern that people might use it in
their boards? Should be easy enough to refuse patches which do include
it.

Regards,
Simon
>
> Amicalement,
> --
> Albert.
>

[U-Boot] [RFC PATCH] arm: provide a CONFIG flag for disabling relocation

[Aneesh V]

Relocation may not be needed and desirable in many cases:
 * For many boards the amount of SDRAM is fixed.
   So relocation is not needed.
 * Relocation adds un-necessary additional overhead when
   it's not needed. This delay is singificant on slower
   platforms such as FPGA
 * Many boards have ample memory. So reserving enough memory
   for Linux in the first half is not a challenge even without
   relocation

Add a CONFIG option to disable relocation on platforms that
do not need it. When this flag is enabled allocate memory
for stack heap etc at the end of memory as usual, but U-Boot
itself is not moved from TEXT_BASE.

Additionally, -pie is removed from the final link step because
it was causing the absolute value of all symbols coming from
the linker script to be 0. This affects find_cmd()

Tested on OMAP4430 SDP

Cc: Albert Aribaud <albert.aribaud@free.fr>
Cc: Wolfgang Denk <wd@denx.de>

Signed-off-by: Aneesh V <aneesh@ti.com>
---
 arch/arm/config.mk              |    2 ++
 arch/arm/lib/board.c            |    5 +++++
 board/ti/sdp4430/config.mk      |    9 +++++++--
 include/configs/omap4_sdp4430.h |    2 ++
 4 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/arch/arm/config.mk b/arch/arm/config.mk
index 1785e73..2315bcb 100644
--- a/arch/arm/config.mk
+++ b/arch/arm/config.mk
@@ -71,5 +71,7 @@ LDSCRIPT := $(SRCTREE)/$(CPUDIR)/u-boot.lds
 
 # needed for relocation
 ifndef CONFIG_NAND_SPL
+ifndef CONFIG_SYS_SKIP_ARM_RELOCATION
 LDFLAGS_u-boot += -pie
 endif
+endif
diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
index 72ee108..ed3898f 100644
--- a/arch/arm/lib/board.c
+++ b/arch/arm/lib/board.c
@@ -361,6 +361,7 @@ void board_init_f (ulong bootflag)
 	gd->fb_base = addr;
 #endif /* CONFIG_LCD */
 
+#ifndef CONFIG_SYS_SKIP_ARM_RELOCATION
 	/*
 	 * reserve memory for U-Boot code, data & bss
 	 * round down to next 4 kB limit
@@ -369,6 +370,7 @@ void board_init_f (ulong bootflag)
 	addr &= ~(4096 - 1);
 
 	debug ("Reserving %ldk for U-Boot at: %08lx\n", gd->mon_len >> 10, addr);
+#endif
 
 #ifndef CONFIG_PRELOADER
 	/*
@@ -420,6 +422,9 @@ void board_init_f (ulong bootflag)
 	dram_init_banksize();
 	display_dram_config();	/* and display it */
 
+#ifdef CONFIG_SYS_SKIP_ARM_RELOCATION
+	addr = _TEXT_BASE;
+#endif
 	gd->relocaddr = addr;
 	gd->start_addr_sp = addr_sp;
 	gd->reloc_off = addr - _TEXT_BASE;
diff --git a/board/ti/sdp4430/config.mk b/board/ti/sdp4430/config.mk
index c62965d..8846732 100644
--- a/board/ti/sdp4430/config.mk
+++ b/board/ti/sdp4430/config.mk
@@ -28,5 +28,10 @@
 # Linux-Kernel is expected to be at 8000'8000, entry 8000'8000
 # (mem base + reserved)
 
-# 1MB into the SDRAM to allow for SPL's bss at the beginning of SDRAM
-CONFIG_SYS_TEXT_BASE = 0x80100000
+# 64MB into the SDRAM
+# Assuming a minimum of 128 MB on the board:
+# - 64MB before U-Boot is more than enough for Linux when relocation is
+#   disabled
+# - ~63MB after the U-Boot is more than enough for U-Boot to relocate
+#   itself without stepping on itself
+CONFIG_SYS_TEXT_BASE = 0x84000000
diff --git a/include/configs/omap4_sdp4430.h b/include/configs/omap4_sdp4430.h
index fff67d8..e9f76d3 100644
--- a/include/configs/omap4_sdp4430.h
+++ b/include/configs/omap4_sdp4430.h
@@ -278,4 +278,6 @@
 #define CONFIG_SYS_SPL_BSS_MAX_SIZE		0x80000		/* 512 KB */
 
 #define CONFIG_SYS_THUMB_BUILD
+
+#define CONFIG_SYS_SKIP_ARM_RELOCATION
 #endif /* __CONFIG_H */
-- 
1.7.0.4

[U-Boot] [RFC PATCH] arm: provide a CONFIG flag for disabling relocation

[Aneesh V]

Forgot to mention that this patch depends on my previous series for MMC
spl:
http://article.gmane.org/gmane.comp.boot-loaders.u-boot/96352

This one was anyway intended to initiate the discussion. If approved,
I shall create a cleaner patch.

On Friday 25 March 2011 06:42 PM, Aneesh V wrote:
> Relocation may not be needed and desirable in many cases:
>   * For many boards the amount of SDRAM is fixed.
>     So relocation is not needed.
>   * Relocation adds un-necessary additional overhead when
>     it's not needed. This delay is singificant on slower
>     platforms such as FPGA
>   * Many boards have ample memory. So reserving enough memory
>     for Linux in the first half is not a challenge even without
>     relocation
>
> Add a CONFIG option to disable relocation on platforms that
> do not need it. When this flag is enabled allocate memory
> for stack heap etc at the end of memory as usual, but U-Boot
> itself is not moved from TEXT_BASE.
>
> Additionally, -pie is removed from the final link step because
> it was causing the absolute value of all symbols coming from
> the linker script to be 0. This affects find_cmd()
>
> Tested on OMAP4430 SDP
>
> Cc: Albert Aribaud<albert.aribaud@free.fr>
> Cc: Wolfgang Denk<wd@denx.de>
>
> Signed-off-by: Aneesh V<aneesh@ti.com>
> ---
>   arch/arm/config.mk              |    2 ++
>   arch/arm/lib/board.c            |    5 +++++
>   board/ti/sdp4430/config.mk      |    9 +++++++--
>   include/configs/omap4_sdp4430.h |    2 ++
>   4 files changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/config.mk b/arch/arm/config.mk
> index 1785e73..2315bcb 100644
> --- a/arch/arm/config.mk
> +++ b/arch/arm/config.mk
> @@ -71,5 +71,7 @@ LDSCRIPT := $(SRCTREE)/$(CPUDIR)/u-boot.lds
>
>   # needed for relocation
>   ifndef CONFIG_NAND_SPL
> +ifndef CONFIG_SYS_SKIP_ARM_RELOCATION
>   LDFLAGS_u-boot += -pie
>   endif
> +endif
> diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
> index 72ee108..ed3898f 100644
> --- a/arch/arm/lib/board.c
> +++ b/arch/arm/lib/board.c
> @@ -361,6 +361,7 @@ void board_init_f (ulong bootflag)
>   	gd->fb_base = addr;
>   #endif /* CONFIG_LCD */
>
> +#ifndef CONFIG_SYS_SKIP_ARM_RELOCATION
>   	/*
>   	 * reserve memory for U-Boot code, data&  bss
>   	 * round down to next 4 kB limit
> @@ -369,6 +370,7 @@ void board_init_f (ulong bootflag)
>   	addr&= ~(4096 - 1);
>
>   	debug ("Reserving %ldk for U-Boot at: %08lx\n", gd->mon_len>>  10, addr);
> +#endif
>
>   #ifndef CONFIG_PRELOADER
>   	/*
> @@ -420,6 +422,9 @@ void board_init_f (ulong bootflag)
>   	dram_init_banksize();
>   	display_dram_config();	/* and display it */
>
> +#ifdef CONFIG_SYS_SKIP_ARM_RELOCATION
> +	addr = _TEXT_BASE;
> +#endif
>   	gd->relocaddr = addr;
>   	gd->start_addr_sp = addr_sp;
>   	gd->reloc_off = addr - _TEXT_BASE;
> diff --git a/board/ti/sdp4430/config.mk b/board/ti/sdp4430/config.mk
> index c62965d..8846732 100644
> --- a/board/ti/sdp4430/config.mk
> +++ b/board/ti/sdp4430/config.mk
> @@ -28,5 +28,10 @@
>   # Linux-Kernel is expected to be at 8000'8000, entry 8000'8000
>   # (mem base + reserved)
>
> -# 1MB into the SDRAM to allow for SPL's bss at the beginning of SDRAM
> -CONFIG_SYS_TEXT_BASE = 0x80100000
> +# 64MB into the SDRAM
> +# Assuming a minimum of 128 MB on the board:
> +# - 64MB before U-Boot is more than enough for Linux when relocation is
> +#   disabled
> +# - ~63MB after the U-Boot is more than enough for U-Boot to relocate
> +#   itself without stepping on itself
> +CONFIG_SYS_TEXT_BASE = 0x84000000
> diff --git a/include/configs/omap4_sdp4430.h b/include/configs/omap4_sdp4430.h
> index fff67d8..e9f76d3 100644
> --- a/include/configs/omap4_sdp4430.h
> +++ b/include/configs/omap4_sdp4430.h
> @@ -278,4 +278,6 @@
>   #define CONFIG_SYS_SPL_BSS_MAX_SIZE		0x80000		/* 512 KB */
>
>   #define CONFIG_SYS_THUMB_BUILD
> +
> +#define CONFIG_SYS_SKIP_ARM_RELOCATION
>   #endif /* __CONFIG_H */

[U-Boot] [RFC PATCH] arm: provide a CONFIG flag for disabling relocation

[Wolfgang Denk]

Dear Aneesh V,

In message <1301058732-30898-1-git-send-email-aneesh@ti.com> you wrote:
> Relocation may not be needed and desirable in many cases:
>  * For many boards the amount of SDRAM is fixed.
>    So relocation is not needed.

This is plain wrong.  This has been explained a couple of times
before.  Please read the archives.

>  * Relocation adds un-necessary additional overhead when
>    it's not needed. This delay is singificant on slower
>    platforms such as FPGA

Do you have numbers?  How much delay do you see caused by the
relocation of U-Boot?

>  * Many boards have ample memory. So reserving enough memory
>    for Linux in the first half is not a challenge even without
>    relocation

You appear not to understand what relocation is needed for.


I tend to reject your patch, unless you can come up with really
convincing arguments. Also, your patch should then work for all
boards - not only ARM, but also PowerPC (one of the arguments for
relocation was that we want to unify the code).


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"Beware of bugs in the above code; I have only proved it correct, not
tried it."                                             - Donald Knuth

[U-Boot] [RFC PATCH] arm: provide a CONFIG flag for disabling relocation

[Aneesh V]

Dear Wolfgang,

On Friday 25 March 2011 07:42 PM, Wolfgang Denk wrote:
> Dear Aneesh V,
>
> In message<1301058732-30898-1-git-send-email-aneesh@ti.com>  you wrote:
>> Relocation may not be needed and desirable in many cases:
>>   * For many boards the amount of SDRAM is fixed.
>>     So relocation is not needed.
>
> This is plain wrong.  This has been explained a couple of times
> before.  Please read the archives.
>

I read this one:
http://tree.celinuxforum.org/pipermail/celinux-dev/2009-December/001916.html

I also read again this discussion I had started sometime back:
http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/88067/focus=88262

I don't think any of these features will suffer due to my change.
Please note that the memory for pRAM, frame buffer etc would still
be allocated at the end of available memory. I am preventing only code
relocation.

My strategy is to leave enough space before and after the U-boot in the
non-relocation case. Before the U-Boot for Linux, after U-boot for
stack, heap and all other dynamic needs. Wouldn't that work? Am I
missing something?

>>   * Relocation adds un-necessary additional overhead when
>>     it's not needed. This delay is singificant on slower
>>     platforms such as FPGA
>
> Do you have numbers?  How much delay do you see caused by the
> relocation of U-Boot?

No, I don't have any numbers. In fact, if I enable relocation I get
into some issues on the FPGA platform I am working on currently. But
that seems to be a problem with the platform (For some reason, it
doesn't work well when some code is copied to a new area and
executed from there - maybe some issue with the memory model).

But the platform is so slow that booting U-boot itself takes couple of
minutes. The overhead due to relocation will surely get magnified in
such cases.

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.

>
>>   * Many boards have ample memory. So reserving enough memory
>>     for Linux in the first half is not a challenge even without
>>     relocation
>
> You appear not to understand what relocation is needed for.
>
>
> I tend to reject your patch, unless you can come up with really
> convincing arguments. Also, your patch should then work for all
> boards - not only ARM, but also PowerPC (one of the arguments for
> relocation was that we want to unify the code).

I am afraid I will not be able to do that for all the archs primarily
because I will not be able to test anything other than armv7 and also I do
not understand their assembly languages. Sometimes, fixes in
relocate_code() will be needed to make sure the special case of
relocation offset = 0 works well. I had to do that for armv7(It's part
of the SPL series). However, the same principles as in this patch
should apply everywhere it shouldn't be very difficult to make it work
for any architecture when there is a need.

best regards,
Aneesh

[U-Boot] [RFC PATCH] arm: provide a CONFIG flag for disabling relocation

[Albert ARIBAUD]

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

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

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.

Amicalement,
-- 
Albert.

[U-Boot] [RFC PATCH] arm: provide a CONFIG flag for disabling relocation

[Simon Glass]

On Fri, Mar 25, 2011 at 11:35 AM, Albert ARIBAUD <albert.aribaud@free.fr> wrote:
> Le 25/03/2011 17:12, Aneesh V a ?crit :
>
>> 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.
>
> 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.

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.

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

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

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

[U-Boot] [RFC PATCH] arm: provide a CONFIG flag for disabling relocation

[Aneesh V]

Hi Simon, Wolfgang,

On Thursday 21 April 2011 12:04 AM, Simon Glass wrote:
> On Fri, Mar 25, 2011 at 11:35 AM, Albert ARIBAUD<albert.aribaud@free.fr>  wrote:
>> Le 25/03/2011 17:12, Aneesh V a ?crit :
>>
>>> 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.
>>
>> 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.
>
> 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.

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?

>
> Looking at the December thread
> http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/88067/focus=88262
>
> Aneesh:
>>> Shouldn't we provide a CONFIG option by which users can disable
>>> Elf relocation?
>
> Wolfgang:
>> 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
>
>>
>> Amicalement,
>> --
>> Albert.
>> _______________________________________________
>> U-Boot mailing list
>> U-Boot at lists.denx.de
>> http://lists.denx.de/mailman/listinfo/u-boot
>>

[U-Boot] [RFC PATCH] arm: provide a CONFIG flag for disabling relocation

[Simon Glass]

On Wed, Apr 20, 2011 at 11:56 PM, Aneesh V <aneesh@ti.com> wrote:
> Hi Simon, Wolfgang,
>
> On Thursday 21 April 2011 12:04 AM, Simon Glass wrote:
>>
>> On Fri, Mar 25, 2011 at 11:35 AM, Albert ARIBAUD<albert.aribaud@free.fr>
>> ?wrote:
>>>
>>> Le 25/03/2011 17:12, Aneesh V a ?crit :
>>>
>>>> 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.
>>>
>>> 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.
>>
>> 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.
>
> 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

>
>>
>> Looking at the December thread
>> http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/88067/focus=88262
>>
>> Aneesh:
>>>>
>>>> Shouldn't we provide a CONFIG option by which users can disable
>>>> Elf relocation?
>>
>> Wolfgang:
>>>
>>> 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
>>
>>>
>>> Amicalement,
>>> --
>>> Albert.
>>> _______________________________________________
>>> U-Boot mailing list
>>> U-Boot at lists.denx.de
>>> http://lists.denx.de/mailman/listinfo/u-boot
>>>
>