[U-Boot] [PATCH 12/26] ARM: add relocation support

[Albert ARIBAUD]

Le 16/09/2010 23:26, Wolfgang Denk a écrit :
> Dear Albert ARIBAUD,
>
> In message<4C927C0C.1080007 at free.fr>  you wrote:
>>
>> I did not write 'the goal of using fPIC in u-boot', I wrote 'the goal of
>> fPIC', and as such, I think I read it right. I do agree though (and I
>> think I made it clear further in my post) that u-boot images are linked
>> for a fixed location and that their entry point is at the first address
>> of this location.
>
> Wrong. Their entry point is whereever the reset vector happens to be,
> and this is usually NOT at offset 0 into the text segment.

They key here is 'usually'; see the orion5x case for instance.

>> However, I disagree with the fact that u-boot's link address would
>> always be the reset vector location. This is not generally true, and
>> this is especially not true for the orion5x, which is linked for its RAM
>> location, not for its FLASH location, and for which the image is flashed
>
> You are probably looking at old, obsolete code.  Please check out the
> "next" branch, which has Heiko's ARM rework patches applied.

I am looking at code which was in effect until Heiko's patches; this is 
barely 'old, obsolete code', and I am precisely discussing the fact that 
Heiko's patches break building this code for orion5x, and require 
replacing a simple process with quite a complicated one.

>>> [I'm giving the generic, architecture independent view of U-Boot
>>> here. Actual implementations for some processors may behave
>>> differently, but this is nothing you should build on.]
>>
>> [Except when working with these processors. :) ]
>
> Not even then, because things might change under your feet - as just
> happened.

Things do change indeed, hence my attempt à 1) making sure I detect the 
change, 2) design things so that they are as resilient to change as 
possible. One example of such a resilience is making sure the u-boot 
code designed to run in FLASH can run *anywhere* in FLASH.

>> As for what I am trying to do: ironically, I am trying to find a way
>> that the entry point of u-boot be at the reset vector location, just as
>> it should.
>
> There it is.

Yes. And you are opposing this, which I do not understand since I'm 
trying to make this SoC/board perform exactly as you specify while 
maintaining balance with the hardware's constraints.

>> As for you suggestion of arranging the code so that the entry point ends
>> up at 0xffff0000, it requires much more than a little tuning of the
>> linker script. This tuning is actually a long and suboptimal process
>> akin to the bin packing problem, where you'd have to split sections,
>> notably .text, and then shuffle sections in memory areas while hoping
>> that no section is going to grow bigger than the memory region it was
>> assigned to. And then, change one thing in the config, and you may need
>> to solve the problem yet again.
>
> If you can live with the wasted 64 kB of empty space "behind" the
> reset vector, then it is not worse than counting the number of flash
> sectors needed for the image (which you have to do anyway to
> determine the TEXT_BASE). And moving a number of static,
> never-changing objects into that free area is no real rocket science
> either.  Say, half an hour of efforts including basic testing.

Precisely, I do not want to live with wasting 64 out of 512 KB of FLASH.

Regarding counting the flash sectors for mapping u-boot to flash to 
determine TEXT_BASE, I never had to, thanks to u-boot being (albeit 
accidentally) able to run from anywhere in FLASH (I can't help seeing 
irony in the fact that this ability was broken by adding a flag which is 
supposed to allow running from anywhere).

As for the half-hour of effort, the half-hour part is an assumption 
which would need supporting, and even if it is only half an hour, it is 
half an hours for each person configuring u-boot for arm; a burden that 
I am not willing to inflict on people who want to use u-boot on 
arm926ejs if I know a way to avoid it, and I know one which indeed 
requires a bit more effort but for me only, and will be just as 
(in)sensitive to things moving under feet.

>>> Hm... I don;t see global variables being used in timer_init() in
>>> "arch/arm/cpu/arm926ejs/orion5x/timer.c" - which exact code are you
>>> referring to?
>>
>> (*)
>>
>> timer_init() ends up calling reset_timer_masked(), which writes into
>> static variables timestamp and lastdec.
>
> This is indeed broken and needs to be fixed.
>
>> This is true, and reinforces my point that the comment in board.c is out
>> of sync with what is really going on, as it explicitely says the pointer
>> is passed as an argument to the init functions (lines 241 and 242 in
>> 'next') whereas it is not.
>
> Patch welcome...

Patches for both timer and board.c coming in soon. :)

>> I still think that with -fPIC u-boot should be able to run until at
>> least the end of board_init_f from anywhere in FLASH. And if all it
>> takes to get there is making sure that board_init_f-called code uses
>> only consts, then I think it would be worth asking board maintainers to
>> go check this while they're testing Heiko's relocation patches.
>
> We use PIC to make U-Boot relocatable to any RAM address, so we can
> auto-adjust to actual RAM sizes and always copy U-Boot to the
> (dynamically determined) end of RAM location.  When running from
> flash, U-Boot is linked to a fixed address map, which includes the
> (fix) reset vector as single entry point.

This statement does not contradict the proposal that U-boot, despite 
being linked for some address in FLASH, should be able to run, from 
_start to relocation, at any FLASH location at least for architectures, 
CPUs, SoCs or boards which trivially allow this. I do believe this 
requirement is both reasonable and useful.

Anyway: there is a bottom line on which I think we agree now:

1) init_sequence is a constant array and should thus be qualified 'const';

2) any data accessed between _start and relocation should be const as well.

Since enforcing these two constaints will fix the problem I have with 
(wrongly) running u-boot from any FLASH location, I'll happily cease 
arguing for it if so ordered; but as long as I can, I'll continue making 
sure changes made to u-boot avoid breaking this if they can.

(and I'll be sure to address the requirement that u-boot be linked for 
an address in flash which contains its entry point in my upcoming patch 
for using the last 64 KB of flash on orion5x)

Amicalement,
-- 
Albert.