Re: [U-Boot] [RFC] board_f: generalize code for case of no relocation
Hi Aexey, On 15 December 2015 at 03:06, Alexey Brodkinwrote: > 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 > --- > > 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. > > 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. > > 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); > + > /* > * x86 is special, but in a nice way. It uses a trampoline which > * enables the dcache if possible. > @@ -1017,8 +1023,7 @@ void board_init_f(ulong boot_flags) > if (initcall_run_list(init_sequence_f)) > hang(); > > -#if !defined(CONFIG_ARM) && !defined(CONFIG_SANDBOX) && \ > - !defined(CONFIG_EFI_APP) > +#if !defined(CONFIG_ARM) && !defined(CONFIG_SANDBOX) > /* NOTREACHED - jump_to_copy() does not return */ > hang(); Actually the hope is that all boards will work this way soon, so I'd rather not remove one. I'd like to see board_init_f() return normally, and then board_init_r() be called by the caller. In the end, jump_to_copy() will be removed. So can you adjust your patch to just return from jump_to_copy() and skip this hang()? Then (I hope) on ARM it will also do the right thing...I can help test on x86 also. It's Christmas where I am so I may not be around much for the next few weeks. > #endif > diff --git a/lib/efi/efi_app.c b/lib/efi/efi_app.c > index 452ab5d..5e41b7f 100644 > --- a/lib/efi/efi_app.c > +++ b/lib/efi/efi_app.c > @@ -123,7 +123,7 @@ efi_status_t efi_main(efi_handle_t image, struct > efi_system_table *sys_table) > printf("starting\n"); > > board_init_f(GD_FLG_SKIP_RELOC); > - board_init_r(NULL, 0); > + /* board_init_r() is called in the end of board_init_f() */ > free_memory(priv); > > return EFI_SUCCESS; > -- > 2.4.3 > Regards, Simon ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [RFC] board_f: generalize code for case of no relocation
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 >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 > > > 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 > > > > --- > > > > > > > > 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 > > > > > > Tested on QEMU, booting u-boot-app.efi with EFI firmware > > > Tested-by: Bin Meng > > > > > > > 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
Re: [U-Boot] [RFC] board_f: generalize code for case of no relocation
Hi Alexey, On Thu, Dec 17, 2015 at 3:13 AM, Alexey Brodkinwrote: > 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 >> 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 >> > --- >> > >> > 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 >> >> Tested on QEMU, booting u-boot-app.efi with EFI firmware >> Tested-by: Bin Meng >> >> > 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. Regards, Bin ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [RFC] board_f: generalize code for case of no relocation
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 >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 > > --- > > > > 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 > > Tested on QEMU, booting u-boot-app.efi with EFI firmware > Tested-by: Bin Meng > > > 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 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [RFC] board_f: generalize code for case of no relocation
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--- 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. 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. 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); + /* * x86 is special, but in a nice way. It uses a trampoline which * enables the dcache if possible. @@ -1017,8 +1023,7 @@ void board_init_f(ulong boot_flags) if (initcall_run_list(init_sequence_f)) hang(); -#if !defined(CONFIG_ARM) && !defined(CONFIG_SANDBOX) && \ - !defined(CONFIG_EFI_APP) +#if !defined(CONFIG_ARM) && !defined(CONFIG_SANDBOX) /* NOTREACHED - jump_to_copy() does not return */ hang(); #endif diff --git a/lib/efi/efi_app.c b/lib/efi/efi_app.c index 452ab5d..5e41b7f 100644 --- a/lib/efi/efi_app.c +++ b/lib/efi/efi_app.c @@ -123,7 +123,7 @@ efi_status_t efi_main(efi_handle_t image, struct efi_system_table *sys_table) printf("starting\n"); board_init_f(GD_FLG_SKIP_RELOC); - board_init_r(NULL, 0); + /* board_init_r() is called in the end of board_init_f() */ free_memory(priv); return EFI_SUCCESS; -- 2.4.3 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [RFC] board_f: generalize code for case of no relocation
On Tue, Dec 15, 2015 at 6:06 PM, Alexey Brodkinwrote: > 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 > --- > > 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? > 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 Tested on QEMU, booting u-boot-app.efi with EFI firmware Tested-by: Bin Meng > 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); > + > /* > * x86 is special, but in a nice way. It uses a trampoline which > * enables the dcache if possible. > @@ -1017,8 +1023,7 @@ void board_init_f(ulong boot_flags) > if (initcall_run_list(init_sequence_f)) > hang(); > > -#if !defined(CONFIG_ARM) && !defined(CONFIG_SANDBOX) && \ > - !defined(CONFIG_EFI_APP) > +#if !defined(CONFIG_ARM) && !defined(CONFIG_SANDBOX) > /* NOTREACHED - jump_to_copy() does not return */ > hang(); > #endif > diff --git a/lib/efi/efi_app.c b/lib/efi/efi_app.c > index 452ab5d..5e41b7f 100644 > --- a/lib/efi/efi_app.c > +++ b/lib/efi/efi_app.c > @@ -123,7 +123,7 @@ efi_status_t efi_main(efi_handle_t image, struct > efi_system_table *sys_table) > printf("starting\n"); > > board_init_f(GD_FLG_SKIP_RELOC); > - board_init_r(NULL, 0); > + /* board_init_r() is called in the end of board_init_f() */ > free_memory(priv); > > return EFI_SUCCESS; > -- Regards, Bin ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot