[U-Boot] [PATCH v3] board_r - fixup functions table after relocation

[U-Boot] [PATCH v3] board_r - fixup functions table after relocation

[Alexey Brodkin]

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(+)

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

[U-Boot] [PATCH v3] board_r - fixup functions table after relocation

[Simon Glass]

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
>

[U-Boot] [PATCH v3] board_r - fixup functions table after relocation

[Alexey Brodkin]

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

[U-Boot] [U-Boot, v3] board_r - fixup functions table after relocation

[Tom Rini]

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!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140127/2deb1dc2/attachment.pgp>