[U-Boot] [PATCH 16/22] x86: board_f: Adjust x86 boot order for performance

[Graeme Russ]

Hi Simon & Bin,

On Tue, Jan 6, 2015 at 12:40 AM, Bin Meng <bmeng.cn at gmail.com> wrote:

> Hi Simon,
>
> On Mon, Jan 5, 2015 at 9:44 AM, Simon Glass <sjg at chromium.org> wrote:
> > Hi Bin,
> >
> > On 3 January 2015 at 20:26, Bin Meng <bmeng.cn at gmail.com> wrote:
> >> Hi Simon,
> >>
> >> On Fri, Jan 2, 2015 at 6:23 AM, Simon Glass <sjg at chromium.org> wrote:
> >>> Hi Bin,
> >>>
> >>> On 30 December 2014 at 22:51, Bin Meng <bmeng.cn at gmail.com> wrote:
> >>>>
> >>>> Hi Simon,
> >>>>
> >>>> On Sun, Dec 28, 2014 at 10:20 AM, Simon Glass <sjg at chromium.org>
> wrote:
> >>>> > For bare platforms we turn off ROM-caching before calling
> board_init_f_r()
> >>>> > It is then very slow to copy U-Boot from ROM to RAM. So adjust the
> order so
> >>>> > that the copying happens before we turn off ROM-caching.
> >>>> >
> >>>> > Signed-off-by: Simon Glass <sjg at chromium.org>
> >>>> > ---
> >>>> >
> >>>> >  common/board_f.c | 8 +++++---
> >>>> >  1 file changed, 5 insertions(+), 3 deletions(-)
> >>>> >
> >>>> > diff --git a/common/board_f.c b/common/board_f.c
> >>>> > index 98c9c72..1b65575 100644
> >>>> > --- a/common/board_f.c
> >>>> > +++ b/common/board_f.c
> >>>> > @@ -983,6 +983,11 @@ static init_fnc_t init_sequence_f[] = {
> >>>> >         INIT_FUNC_WATCHDOG_RESET
> >>>> >         reloc_fdt,
> >>>> >         setup_reloc,
> >>>> > +#ifdef CONFIG_X86
> >>>> > +       copy_uboot_to_ram,
> >>>> > +       clear_bss,
> >>>> > +       do_elf_reloc_fixups,
> >>>> > +#endif
> >>>> >  #if !defined(CONFIG_ARM) && !defined(CONFIG_SANDBOX)
> >>>> >         jump_to_copy,
> >>>> >  #endif
> >>>> > @@ -1042,9 +1047,6 @@ void board_init_f(ulong boot_flags)
> >>>> >   */
> >>>> >  static init_fnc_t init_sequence_f_r[] = {
> >>>> >         init_cache_f_r,
> >>>> > -       copy_uboot_to_ram,
> >>>> > -       clear_bss,
> >>>> > -       do_elf_reloc_fixups,
> >>>> >
> >>>> >         NULL,
> >>>> >  };
> >>>> > --
>

Wow, doesn't this bring back some memories. I've had a look over this code
as it is now and it took a while to sink in. Things have moved on in the
past 2 years :)


> >>>>
> >>>> I don't understand. Isn't the init_cache_f_r() which turns on the
> cache?
> >>>>
> >>>
> >>> Yes it turns on the cache, but turns off the ROM cache (they are
> >>> different things). So this ordering is much faster. I think I might
> >>> have more tuning to do though.
> >>>
> >>
> >> Got it. Since we moved these 3 routines from init_sequence_f_r[] to
> >> init_sequence_f[], how about we remove the whole init_sequence_f_r[]
> >> completely? If not possible, the comment block above
> >> init_sequence_f_r[] needs to be fixed?
> >
> > I'll remove the comment.
>

I think init_sequence_f_r can go... but I need to have a better look at the
code

If turning off the ROM cache by init_cache_f_r is the problem, then perhaps
the following order would be better:

  copy_uboot_to_ram,
  init_cache_f_r,
  clear_bss,
  do_elf_reloc_fixups,

Without enabling the CPU's cache, clear_bss and do_elf_reloc_fixups will
suffer.

>
> >>
> >>  *
> >>  * The '_f_r' sequence must, as a minimum, copy U-Boot to RAM (if
> >>  * supported).  It _should_, if possible, copy global data to RAM and
> >>  * initialise the CPU caches (to speed up the relocation process)
> >>  *
> >>  * NOTE: At present only x86 uses this route, but it is intended that
> >>  * all archs will move to this when generic relocation is implemented.
> >>  */
> >>
> >> So looks that we should either drop this _f_r stage, or make other
> >> arch use this _f_r?
> >
> > I think we should move to generic relocation, meaning that all archs
> > do relocation the same way with the same code. Then only arch-specific
> > stuff is the then ELF fixup function, which adjusts a relocation site,
> > and the code to actually change the stack pointer.
>

This was always my plan - have arch specific do_reloc_fixups and the rest
would be generic


> >
> > This board_init_f_r() code is part of one attempt to do this - the
> > premise was that turning the cache on before relocating U-Boot will
> > save time. That's true, but it would be even better to turn the cache
> > on much earlier. With pit (Chromebook 2) we turn on the cache in SPL.
> > So I think turning it on here is too late if we are trying to save
> > time. Still it is a good start and if we could make it happen
> > generally it would be nice.
>

And now you have me thinking board_init_f_r is not needed at all...

If we can setup the stack, clear BSS, copy U-Boot to RAM and perform
relocation fixups while running from ROM, what is left for board_init_f_r
to do?

The only thing I can think of is the caveats mentioned in the comment
('static' variables are read-only / Global Data (gd->xxx) is read/write).
But aren't these true when running from ROM?

Looking at non-x86 arches, relocate_code() looks to do what
copy_uboot_to_ram and clear_bss does in x86 land (not sure about
do_elf_reloc_fixups - seems to be arch specific as to whether
relocate_code() does this or it is done later (hence
the CONFIG_NEEDS_MANUAL_RELOC #define?)

>
> > See here for my original attempt, which was tied up with generic
> > board. In the end I untied them and we got one but not the other.
> >
> > http://lists.denx.de/pipermail/u-boot/2012-February/118409.html
> >
>

Ah, generic relocation... I really wish my life hadn't taken a hard-left
turn when it did. We got so close.

>From where I'm looking (fresh eyes - I might be missing something big) we
should be able to do the ROM->RAM copy, BSS clearing, and relocation fixups
in board_init_f.

When we return from board_init_f it should be a fairly simple matter of:
 - Copying the contents of the Global Data structure from CAR to RAM (or
from RAM to RAM if your on a platform which initialises RAM before U-Boot)
 - Set the gd pointer (reserved register) to point to the new copy
 - Figure out where board_init_r is and jump to it

board_init_r should be able to do any remaining cache tweaks. If cache
tweaks cannot be done while executing from RAM then they need to be done in
board_init_f

I just cannot see the point of board_init_f_r any more


> > Since then Albert has tidied up ARM start.S a lot which makes this much
> easier.
> >
> > So that's the background. One of these days I might take another look
> > at it if it doesn't get someone's attention.
>

Oh dear - it looks like I just put my hand up :)


> >
> > Regards,
> > Simon
>
> Thanks for the background information. I will take a look. Hope we can
> achieve generic board support as soon as we can.
>
> Regards,
> Bin
>
>
Regards,

Graeme