[U-Boot] Where I'm going with x86 board.c

[Albert ARIBAUD]

Hi Simon,

Le 22/12/2011 16:49, Simon Glass a écrit :
> Hi Graeme,
>
> On Thu, Dec 22, 2011 at 3:27 AM, Graeme Russ<graeme.russ at gmail.com>  wrote:
>> Hi Simon,
>>
>> On 22/12/11 17:44, Simon Glass wrote:
>>> Hi Graeme,
>>>
>>> On Tue, Dec 20, 2011 at 4:06 AM, Graeme Russ<graeme.russ at gmail.com>  wrote:
>>>> With Simon's work on generic relocation support, I thought I would throw in
>>>> what I am planning for arch/x86/lib/board.c
>>>>
>>>> Now this is not a patch, it is a work-in-progress complete version of the
>>>> file (compiles, will test soon) - If feedback is positive, I'll add this to
>>>> an upcoming patch set
>>>
>>> Looks good to me+++
>>
>> Thanks for having a look :)
>>
>>>> Notice the amount of wrapping around void functions - If all other arch's
>>>> follow this lead, most of this wrapping can be removed by changing the
>>>> function signatures.
>>>>
>>>> Lines 428 - 585 are effectively the generic init sequence - The rest is
>>>> wrappers, init sequence arrays, or fluff that should be moved
>>>>
>>>> I noticed something along the way - gd is no longer special... let me
>>>> explain...
>>>>
>>>> Some arch's use a dedicated register for the gd pointer - This allows the
>>>> pointer to be written to prior to relocation. For x86, the gd pointer is
>>>> simply passed around as a function parameter early - If the init_sequence_f
>>>> functions accepted a gd pointer as a parameter, there would be no need for
>>>> it to be global prior to relocation and therefore no need to allocate a
>>>> permanent register for it - Of course do_init_loop() would no longer be
>>>> generic for both pre and post relocation. This does mess with the
>>>> stand-alone API, but as discussed before, stand alone applications should
>>>> not be accessing gd anyway, so there should be no API to break ;)
>>>
>>> Actually as it happens I did a bit of an experiment with this some
>>> weeks ago and my original board stuff had gd as a parameter for the
>>> pre-reloc functions (some with stubs to other ones like
>>> console_init_f()). On ARM it really doesn't make a lot of sense to use
>>> a global variable instead of a parameter. I decided that it was a bit
>>> much to bite off in one go :-) Partly that was because IMO gd really
>>> only makes sense prior to relocation - for the post-relocation init
>>> calls they don't have a lot of need for gd.
>>
>> I don't think this is 100% correct - I'm sure gd is used all over the place
>> post init
>
> Not enough that it needs to be in its own register. This compile-time
> decision affects the whole of U-Boot post-relocation. In some routines
> there is a small code size / performance advantage to having r8
> available.

 From measurements I did when working on ARM relocation, I remember 
having tried if reserving a(nother) global register would change code 
size, and IIRC, it did not have a big impact on code size/efficiency.

> Also it is a bit of a misnomer when used after relocation. There is
> lots of global data (the whole BSS) not just one structure. Arguably
> some things like the network code would be smaller/faster using
> global_data and without total reliance on global variables, but anyway
> it seems to me that gd is mostly a convenience for pre-relocation
> where memory may not be available.

Indeed, gd is a convenience for pre-relocation, but the point is not 
that memory may not be available : gd points to memory, so there must be 
some memory available anyway, be it SRAM, locked data cache or even 
SDRAM for some boards.

The convenience is that you cannot tell in advance where that memory 
will be, so you need a pointer, and gd is that pointer; it could indeed 
be passed as an argument from function to function, thus going from 
register to register to stack back and forth, or be allocated its own 
register, and considering most of pre-relocation code will want to 
access gd, allocating it globally to a register makes sense as it 
reduces stack usage (no passing of gd) and does not hurt performance 
that much.

>>
>>> So it has my vote. Once I get somewhere on the reboard series I will
>>> post my common/board.c. As I may have mentioned I have elected so far
>>> to leave the initcall list and local functions in
>>> arch/xxx/lib/board.c.
>>>
>>> Would be good to get all this moving.
>>
>> Well, I have a more thorough patch in the making - I creates
>> init_wrappers.c and init_helpers.c and board.c collapses very dramatically
>> (see below) - with a lot of #ifdef hell...
>
> I thing that all pre-reloc functions that are called should have the
> same signature - i.e. return an int. if we (later) move to passing gd
> around then they would change in that way also.

That's one way of seeing it, particularly when these functions are put 
in a table, but note that there is no *reason* for all functions to have 
the same signature, no more than there would be reason for post-reloc 
functions to either. If a design crops up for pre-reloc code where 
functions involved have different signatures, and it is simple, 
efficient and maintainable, I personally won't oppose it on the basis of 
signature heterogeneity.

>>
>> Notice the new board_init_f_r() - This is where U-Boot is running from
>> Flash, but SDRAM has been initialised and the stack is now in RAM - So this
>> is where we move gd to RAM and perform relocation. As a bonus, relocation
>> can be done with cache enabled :)

Not sure why this intermediary function/state is required.

> Yes I see. It is not so easy on ARM since we need the MMU also, but I
> think it can be done. Once data is set up / copied in its new location
> we can use it even prior to relocation. I also recently created a
> patch to delay console init until after relocation (cue screams on the
> list) which saves time.

Saving time may be a good thing, but for a bootloader, this would go 
second to always knowing what happens, so if delaying console init means 
risking the loss of some console output in case of stray code execution, 
then indeed there will be screaming.

> Regards,
> Simon

Amicalement,
-- 
Albert.