public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Alexey Brodkin <Alexey.Brodkin@synopsys.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [RFC] board_f: generalize code for case of no relocation
Date: Wed, 16 Dec 2015 19:13:14 +0000	[thread overview]
Message-ID: <1450293194.6453.24.camel@synopsys.com> (raw)
In-Reply-To: <CAEUhbmXrhDtcGUAV2VmqX1JHNUoD+48mLR_r-rC5jW=u+=_Ctg@mail.gmail.com>

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@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@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@gmail.com>
> 
> Tested on QEMU, booting u-boot-app.efi with EFI firmware
> Tested-by: Bin Meng <bmeng.cn@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.

-Alexey

  reply	other threads:[~2015-12-16 19:13 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-12-15 10:06 [U-Boot] [RFC] board_f: generalize code for case of no relocation Alexey Brodkin
2015-12-15 12:45 ` Bin Meng
2015-12-16 19:13   ` Alexey Brodkin [this message]
2015-12-17 10:36     ` Bin Meng
2015-12-21  9:09       ` Alexey Brodkin
2015-12-22  4:38 ` Simon Glass

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=1450293194.6453.24.camel@synopsys.com \
    --to=alexey.brodkin@synopsys.com \
    --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