Message ID | 1390213839-14018-1-git-send-email-abrodkin@synopsys.com |
---|---|
State | Accepted |
Delegated to: | Tom Rini |
Headers | show |
Hi, On 20 January 2014 03:30, Alexey Brodkin <abrodkin@synopsys.com> wrote: > > This is only required for "PIC" relocation and doesn't apply to modern > "PIE" relocation which does data relocation as well as code. > > "init_sequence_r" is just an array that consists of compile-time > adresses of init functions. Since this is basically an array of integers > (pointers to "void" to be more precise) it won't be modified during > relocation - it will be just copied to new location as it is. > > As a consequence on execution after relocation "initcall_run_list" will > be jumping to pre-relocation addresses. As long as we don't overwrite > pre-relocation memory area init calls are executed correctly. But still > it is dangerous because after relocation we don't expect initially used > memory to stay untouched. > > Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com> It's unfortunate that we need to carry over CONFIG_NEEDS_MANUAL_RELOC to the new common board init code. But if we do, this patch looks reasonable, so: Acked-by: Simon Glass <sjg@chromium.org> > > Cc: Tom Rini <trini@ti.com> > Cc: Simon Glass <sjg@chromium.org> > Cc: Masahiro Yamada <yamada.m@jp.panasonic.com> > Cc: Doug Anderson <dianders@chromium.org> > Cc: Thomas Langer <thomas.langer@lantiq.com> > Cc: Albert ARIBAUD <albert.u.boot@aribaud.net> > > Changes to v2: > * Use ARRAY_SIZE as suggested by Simon Glass > * Added CONFIG_NEEDS_MANUAL_RELOC wrapper for temporary index variable > "i" to fix compile-time warning for boards without > CONFIG_NEEDS_MANUAL_RELOC > --- > common/board_r.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/common/board_r.c b/common/board_r.c > index 86ca1cb..c2d0763 100644 > --- a/common/board_r.c > +++ b/common/board_r.c > @@ -903,9 +903,19 @@ init_fnc_t init_sequence_r[] = { > > void board_init_r(gd_t *new_gd, ulong dest_addr) > { > +#ifdef CONFIG_NEEDS_MANUAL_RELOC > + int i; > +#endif > + > #ifndef CONFIG_X86 > gd = new_gd; > #endif > + > +#ifdef CONFIG_NEEDS_MANUAL_RELOC > + for (i = 0; i < ARRAY_SIZE(init_sequence_r); i++) > + init_sequence_r[i] += gd->reloc_off; > +#endif > + > if (initcall_run_list(init_sequence_r)) > hang(); > > -- > 1.8.4.2 >
Hi Simon, On Tue, 2014-01-21 at 09:13 -0700, Simon Glass wrote: > Hi, > > It's unfortunate that we need to carry over CONFIG_NEEDS_MANUAL_RELOC > to the new common board init code. But if we do, this patch looks > reasonable, so: I think as long as PIC relocation is supported in U-Boot (for some architectures) it's good to accept fixes for it. And I have to confess that this kind of fixes are missing in many other places. That's why I switched to PIE as well so this particular patch doesn't change anything for me now. But for those who still use PIC it might be helpful - that's why I did another re-spin. -Alexey
On Mon, Jan 20, 2014 at 02:30:39PM +0400, Alexey Brodkin wrote: > This is only required for "PIC" relocation and doesn't apply to modern > "PIE" relocation which does data relocation as well as code. > > "init_sequence_r" is just an array that consists of compile-time > adresses of init functions. Since this is basically an array of integers > (pointers to "void" to be more precise) it won't be modified during > relocation - it will be just copied to new location as it is. > > As a consequence on execution after relocation "initcall_run_list" will > be jumping to pre-relocation addresses. As long as we don't overwrite > pre-relocation memory area init calls are executed correctly. But still > it is dangerous because after relocation we don't expect initially used > memory to stay untouched. > > Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com> > > Cc: Tom Rini <trini@ti.com> > Cc: Simon Glass <sjg@chromium.org> > Cc: Masahiro Yamada <yamada.m@jp.panasonic.com> > Cc: Doug Anderson <dianders@chromium.org> > Cc: Thomas Langer <thomas.langer@lantiq.com> > Cc: Albert ARIBAUD <albert.u.boot@aribaud.net> Applied to u-boot/master, thanks!
diff --git a/common/board_r.c b/common/board_r.c index 86ca1cb..c2d0763 100644 --- a/common/board_r.c +++ b/common/board_r.c @@ -903,9 +903,19 @@ init_fnc_t init_sequence_r[] = { void board_init_r(gd_t *new_gd, ulong dest_addr) { +#ifdef CONFIG_NEEDS_MANUAL_RELOC + int i; +#endif + #ifndef CONFIG_X86 gd = new_gd; #endif + +#ifdef CONFIG_NEEDS_MANUAL_RELOC + for (i = 0; i < ARRAY_SIZE(init_sequence_r); i++) + init_sequence_r[i] += gd->reloc_off; +#endif + if (initcall_run_list(init_sequence_r)) hang();
This is only required for "PIC" relocation and doesn't apply to modern "PIE" relocation which does data relocation as well as code. "init_sequence_r" is just an array that consists of compile-time adresses of init functions. Since this is basically an array of integers (pointers to "void" to be more precise) it won't be modified during relocation - it will be just copied to new location as it is. As a consequence on execution after relocation "initcall_run_list" will be jumping to pre-relocation addresses. As long as we don't overwrite pre-relocation memory area init calls are executed correctly. But still it is dangerous because after relocation we don't expect initially used memory to stay untouched. Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com> Cc: Tom Rini <trini@ti.com> Cc: Simon Glass <sjg@chromium.org> Cc: Masahiro Yamada <yamada.m@jp.panasonic.com> Cc: Doug Anderson <dianders@chromium.org> Cc: Thomas Langer <thomas.langer@lantiq.com> Cc: Albert ARIBAUD <albert.u.boot@aribaud.net> Changes to v2: * Use ARRAY_SIZE as suggested by Simon Glass * Added CONFIG_NEEDS_MANUAL_RELOC wrapper for temporary index variable "i" to fix compile-time warning for boards without CONFIG_NEEDS_MANUAL_RELOC --- common/board_r.c | 10 ++++++++++ 1 file changed, 10 insertions(+)