[U-Boot] [RFC] board_f: generalize code for case of no relocation

[Alexey Brodkin]

Hi Simon,

On Thu, 2015-12-17 at 18:36 +0800, Bin Meng wrote:
> Hi Alexey,
> 
> On Thu, Dec 17, 2015 at 3:13 AM, Alexey Brodkin
> <Alexey.Brodkin at synopsys.com> wrote:
> > Hi Bin,
> > 
> > On Tue, 2015-12-15 at 20:45 +0800, Bin Meng wrote:
> > > On Tue, Dec 15, 2015 at 6:06 PM, Alexey Brodkin
> > > <Alexey.Brodkin at synopsys.com> wrote:
> > > > Current implementation of disabled relocation only works for EFI.
> > > > 
> > > > In case of GD_FLG_SKIP_RELOC jump_to_copy() will return instead of
> > > > jumping further in board_init_r() etc. And jump_to_copy() being the last
> > > > call in init_sequence_f when returning simply triggers hang() in
> > > > board_init_f(). Well for everything except ARM, SANDBOX and EFI_APP.
> > > > 
> > > > Not sure why ARM and SANBOX are here - I would assume it's all on
> > > > purpose but as for EFI_APP this is an essential need for getting out of
> > > > board_init_f() and jumping in board_init_r() immediately afterwards, see
> > > > efi_main().
> > > > 
> > > > But what if in case of no relocation we jump in board_init_r() right
> > > > from jump_to_copy()? In that case we remove one ifdef from
> > > > board_init_f() and leave a chance to seamlessly re-use disabled
> > > > relocation for other (non-EFI) cases.
> > > > 
> > > > Signed-off-by: Alexey Brodkin <abrodkin at synopsys.com>
> > > > ---
> > > > 
> > > > Note I didn't test it for EFI because I don't know how to do that in
> > > > simulation, please let me know if there's a simple way to do it.
> > > > 
> > > 
> > > Does doc/README.efi not help?
> > 
> > Yeah thanks for that obvious pointer.
> > Still it requires some extra steps like obtaining/building EFI BIOS etc.
> > Anyways I'll try to get this setup up and running.
> > 
> > > 
> > > > But I did test it for ARC boards (with additional patches) that enable
> > > > disabled relocation - these patches to follow once something similar to
> > > > my proposal here is implemented.
> > > > 
> > > 
> > > Reviewed-by: Bin Meng <bmeng.cn at gmail.com>
> > > 
> > > Tested on QEMU, booting u-boot-app.efi with EFI firmware
> > > Tested-by: Bin Meng <bmeng.cn at gmail.com>
> > > 
> > > >  common/board_f.c  | 11 ++++++++---
> > > >  lib/efi/efi_app.c |  2 +-
> > > >  2 files changed, 9 insertions(+), 4 deletions(-)
> > > > 
> > > > diff --git a/common/board_f.c b/common/board_f.c
> > > > index eac7c5e..2d60ed9 100644
> > > > --- a/common/board_f.c
> > > > +++ b/common/board_f.c
> > > > @@ -720,8 +720,14 @@ static int setup_reloc(void)
> > > > 
> > > >  static int jump_to_copy(void)
> > > >  {
> > > > +       /*
> > > > +        * In case of no relocation nothing to do between "running from flash"
> > > > +        * (init_f) and "running from ram" (init_r), so just jumping in
> > > > +        * board_init_r().
> > > > +        */
> > > >         if (gd->flags & GD_FLG_SKIP_RELOC)
> > > > -               return 0;
> > > > +               board_init_r((gd_t *)gd, gd->relocaddr);
> > 
> > I tried to do more complicated things compared to booting in console
> > like "usb start" and at that point faced an unexpected problem.
> > 
> > The thing is usually in between board_init_f() and board_init_r()
> > we do a couple of things, most important for us here is stack pointer
> > update. See in board_init_f() we use init stack which is usually
> > (for most of arches except x86) is located at hardcoded address
> > CONFIG_SYS_INIT_SP_ADDR which might easily point to quite limited special
> > memory like on-chip SRAM or (which is the case) be in the very beginning of
> > RAM.
> > 
> > This init stack as said above could be quite small - just enough for every
> > everything in board_init_f(). But when something heavy is executed what may
> > easily happen (and that happens for me on "usb start") - we'll get in unexpected
> > memory location. In my case I'm hitting non-existing memory which precedes
> > DDR. And that was quite fortunate because I was hitting exception and so
> > was able to figure out what's wrong.
> > 
> > For me solution was in stack-pointer update right before calling board_init_r()
> > in my start.S. And that required another line addition to jump_to_copy():
> > So now I'm having this:
> > ------------------>8-----------------
> >         if (gd->flags & GD_FLG_SKIP_RELOC) {
> >                 board_init_f_stack_update(gd->start_addr_sp); <-- Updating SP
> >                 board_init_r((gd_t *)gd, gd->relocaddr);
> >         }
> > ------------------>8-----------------
> > 
> > I'm not sure if all that makes sense for x86 EFI but would like to know
> > your opinion if potential run out out stack may happen there as well.
> > 
> 
> For u-boot-app.efi, the stack is allocated by the EFI firmware, so I
> think we are fine here. If we change its SP without making the EFI
> firmware aware, I believe subsequent call to EFI boot services will
> fail.

Any thoughts on that patch?
If no comments from your side please consider applying.

-Alexey