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

[Albert ARIBAUD]

Le 16/09/2010 13:29, Wolfgang Denk a écrit :
> Dear Albert ARIBAUD,
>
> In message<4C91F659.7020607 at free.fr>  you wrote:
>>
>> Well, the main goal of -fPIC is that the code should be able to run from
>> anywhere, so it should be able to run from anywhere in FLASH. Besides,
>
> You misread that. We actually make no attempts that the code can be
> run from any arbitrary address. On contrary - we explicitly link it
> for a fixed address map, which is defined by the location of the reset
> vector, which is the one and only defined entry point into U-Boot.

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.

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 
so that _start does not end up at the reset address. And in that 
context, u-boot is designed to execute -- once jumped at from the reset 
vector -- to start running at a location (in FLASH) different from the 
one it was linked for (the RAM) until it has relocated itself to its 
intended RAM address, at which point it jumps there.

> [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. :) ]

>> that can be useful for designs where two (possibly differently built)
>> images of u-boot are located in FLASH and some mechanism allows booting
>> from either.
>
> The only reasonable use cases for such a szenario that I am aware of
> would be best implemented in hardware, by being able to toggle between
> two banks of flash memory, selecting one or the other as boot device.
> Then you can even use the same U-Boot binary images, which is a good
> thing in such setups. [eventualkly you want to store the environment
> somewhere else, say in some storage device on the backplane, so all
> instances of U-Boot refer to the same data.]
>
>> (I personally have another motive for having at least the _f part of
>> u-boot able to run fully PIC, as I want to implement direct-to-u-boot
>> resetting on targets that have a reset vector rather far away from the
>> end of the addressable space but too near to fit u-boot in, e.g. the
>> orion5x which resets as 0xffff0000: 64 KB is too small for u-boot. I
>> have submitted a patch for this then withdrawn it because of the
>> relocation patches; however for this conversation, we can keep this
>> point aside)
>
> I do not understand this what you mean by that, or what the problem
> might be. It is trivial to arrange the code such that no significant
> gaps remain. This requires just a little tuning of the linker
> script.

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.

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.

This would be a huge inconvenience and a clumsy way of doing things, 
when another solution allows keeping the current linear, single-region, 
all-initialized-content-in-one-block mapping and requires only flashing 
the image in a clever way (although I'm saying it myself) and adding 
around twenty instructions to the existing start.S.

>> As for what would or would not work, ATM it boils down to 'access to
>> const variables will be position-independent, access to others will
>> not'. Non-initialized as well as initialized-but-not-const globals are
>> always accessed at their link location before and after relocation, and
>> thus will work only if you're running the image at its linked location.
>
> Again, please keep in mind that (1) this depends on the linker script,
> and (2) that before relocation to RAM _all_ access to global variables
> is extremely limited as we have only a read-only data segment and no
> BSS segment at all.

I do have 2 in mind -- I thought that was clear from the rest of my 
post. For 1, well, yes that depends on the linker script, but here I am 
talking about only one linker script, that of the arm926ejs. I see an 
issue there with relocating while init_sequence is not const, but maybe 
it does not occur for other CPUs than ARM, and maybe it does not even 
occur on ARMs for compilers other than the one provided in ELDK42 -- 
that's the one I'm using. But I do think that it does happen with all 
gcc versions, because this is not a compiler bug, this is an overlook in 
board.c -- fundamentally, init_sequence *is* a const and should be 
qualified as such.

>> This means that non-const data obviously can't work when the image is
>> linked to FLASH; and they can even wreak havoc if the image linked for
>> RAM and relocated near, but not exactly at, its linked location. The
>
> I don't understand what you mean. Are you aware of the restrictions
> of the execution environment before relocation to RAM?

I am. More precisely, I am more aware if it than some code in u-boot is. :)

>> fact that it did not break so far in u-boot without reloc (and yes,
>> several drivers do access globals during in-FLASH board init) is due to
>> the fact that DRAM is already initialized when drivers access these
>> globals at their linked location; this breaks when the image is linked
>> for a location in FLASH.
>
> If any drivers do such things, and if these drivers are used before
> relocation to RAM, these are serious bugs that must be fixed.

Uhm... That's what I am implying here, and what I explicitely offer 
solutions for further in my post.

> If you are aware of such bugs please post this information, so people
> can start working on fixes.

I will certainly.

>> Of course solving the init data issue (by making them const) will not
>> solve the issue of rw data. For this one I see two solutions:
>>
>> 1) forbid using such data in drivers during _f phase if that is
>
> This is, and has always been, the case.

Then in at least two cases it was missed by reviews. See below (*).

>> possible. For instance, in the timer.c driver of orion5x, the timer_init
>> code accesses two writable variables because it wants to have a first
>> reference for rollover detection; this can obviously be postponed to the
>> _r init phase.
>
> 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.

The second case where it was missed is in the kirkwood timer, which is 
quite logical as the orion5x code is based on the kirkwood one.

>> 2) in case where the _f phase *has* to store data in globals, then this
>> data should go to the globals space allocated below the stack, where gd
>> also resides, and be later copied to usual globals if reqired.
>
> It seems you really minunderstand the excution environment before
> relocation.  Ther eis also no steck there, and of course no "globals
> space allocated below the stack", because we don't even have the RAM
> initialized yet.

I stand corrected--Indeed, we don't have RAM inittialized at this point. 
This makes my second suggestion inapplicable... And the first one, as 
you noted, is already a requirement.

>> BTW, the comments in board.c say that the _f init functions receive a
>> pointer to gd; actually they dont, they're int (*) (void). Were the
>> comments always out-of-sync with the code, or was there a removal of the
>> gd argument for some reason?
>
> Please read the code, and the README. Please pay special attention to
> section "Initial Stack, Global Data:".
>
> In short: the pointer to the global data is passed in a register,
> which we reserve for this purpose.

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.

> Best regards,
>
> Wolfgang Denk

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.

Amicalement,
-- 
Albert.