public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Albert ARIBAUD <albert.aribaud@free.fr>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 12/26] ARM: add relocation support
Date: Fri, 17 Sep 2010 08:16:36 +0200	[thread overview]
Message-ID: <4C9307C4.40208@free.fr> (raw)
In-Reply-To: <20100916212646.5F5A415242D@gemini.denx.de>

Le 16/09/2010 23:26, Wolfgang Denk a ?crit :
> Dear Albert ARIBAUD,
>
> In message<4C927C0C.1080007@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.

  reply	other threads:[~2010-09-17  6:16 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-08-11 18:16 [U-Boot] [PATCH 12/26] ARM: add relocation support Heiko Schocher
2010-09-15 21:06 ` Albert ARIBAUD
2010-09-16  5:09   ` Heiko Schocher
2010-09-16  6:23     ` Alessandro Rubini
2010-09-16  7:06       ` Wolfgang Denk
2010-09-16  7:18         ` Graeme Russ
2010-09-16  8:23           ` Wolfgang Denk
2010-09-16  9:54             ` Graeme Russ
2010-09-16 10:18               ` Wolfgang Wegner
2010-09-16 10:49                 ` Albert ARIBAUD
2010-09-16 11:06                 ` Wolfgang Denk
2010-09-16 11:24                   ` Wolfgang Wegner
2010-09-16 10:50     ` Albert ARIBAUD
2010-09-16 11:29       ` Wolfgang Denk
2010-09-16 20:20         ` Albert ARIBAUD
2010-09-16 21:26           ` Wolfgang Denk
2010-09-17  6:16             ` Albert ARIBAUD [this message]
2010-09-17 11:05               ` Wolfgang Denk
2010-09-17 12:58                 ` Albert ARIBAUD
2010-09-17 14:52                   ` Wolfgang Denk
2010-09-17 16:39                     ` Albert ARIBAUD
2010-09-17 19:04                       ` Wolfgang Denk
2010-09-17 22:19                         ` Albert ARIBAUD
2010-09-17 22:42                           ` Wolfgang Denk
2010-09-17 23:25                             ` Albert ARIBAUD

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4C9307C4.40208@free.fr \
    --to=albert.aribaud@free.fr \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox