diff mbox series

[U-Boot,2/2] rockchip: rk3288: Fix TPL_TEXT_BASE

Message ID 1518623406-22992-2-git-send-email-jagan@amarulasolutions.com
State Changes Requested
Delegated to: Philipp Tomsich
Headers show
Series [U-Boot,1/2] rockchip: rk3288: Add TPL_LDSCRIPT | expand

Commit Message

Jagan Teki Feb. 14, 2018, 3:50 p.m. UTC
rockchip boot0 add 4 bytes data (0xeaffffff) at beginning
of executable(0x800) in order to make generic and compatible
boot0 for all platforms and the resulting executable will be
input to mkimage and the output of mkimage with initial
4 bytes will overwritten by 'spl_hdr'

Since the TPL_TEXT_BASE in rk3288 is 0xff704004 which is improper
align to branch-to-next-instruction-word, so the resulting 4 bytes
are written at ff704020 of executable instead of beginning ff704000

Hexdump with overlaped bytes:
-----------------------------
0000000 0000 0000 0000 0000 0000 0000 0000 0000
0000010 0000 0000 0000 0000 0000 0000 ffff eaff

So, fix this improper TEXT_BASE which is wrong even before
and update it to 0xff704000 so-that the boot0 will add
4 bytes at beginning.

Disassembly:
-----------
with 0xff704020 TEXT_BASE:

ff704004 <__image_copy_start>:
	b 1f     /* if overwritten, entry-address is at the next word */
ff704020:       eaffffff        b       ff704024 <__image_copy_start+0x20>

with 0xff704000 TEXT_BASE:

ff704000 <__image_copy_start>:
	b 1f     /* if overwritten, entry-address is at the next word */
ff704000:       eaffffff        b       ff704004 <__image_copy_start+0x4>

This patch will also move TPL_TEXT_BASE into kconfig.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
 arch/arm/mach-rockchip/Kconfig | 3 +++
 configs/vyasa-rk3288_defconfig | 1 -
 2 files changed, 3 insertions(+), 1 deletion(-)

Comments

Philipp Tomsich Feb. 16, 2018, 5:12 p.m. UTC | #1
> rockchip boot0 add 4 bytes data (0xeaffffff) at beginning
> of executable(0x800) in order to make generic and compatible
> boot0 for all platforms and the resulting executable will be
> input to mkimage and the output of mkimage with initial
> 4 bytes will overwritten by 'spl_hdr'
> 
> Since the TPL_TEXT_BASE in rk3288 is 0xff704004 which is improper
> align to branch-to-next-instruction-word, so the resulting 4 bytes
> are written at ff704020 of executable instead of beginning ff704000
> 
> Hexdump with overlaped bytes:
> -----------------------------
> 0000000 0000 0000 0000 0000 0000 0000 0000 0000
> 0000010 0000 0000 0000 0000 0000 0000 ffff eaff
> 
> So, fix this improper TEXT_BASE which is wrong even before
> and update it to 0xff704000 so-that the boot0 will add
> 4 bytes at beginning.
> 
> Disassembly:
> -----------
> with 0xff704020 TEXT_BASE:
> 
> ff704004 <__image_copy_start>:
> 	b 1f     /* if overwritten, entry-address is at the next word */
> ff704020:       eaffffff        b       ff704024 <__image_copy_start+0x20>
> 
> with 0xff704000 TEXT_BASE:
> 
> ff704000 <__image_copy_start>:
> 	b 1f     /* if overwritten, entry-address is at the next word */
> ff704000:       eaffffff        b       ff704004 <__image_copy_start+0x4>
> 
> This patch will also move TPL_TEXT_BASE into kconfig.
> 
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> ---
>  arch/arm/mach-rockchip/Kconfig | 3 +++
>  configs/vyasa-rk3288_defconfig | 1 -
>  2 files changed, 3 insertions(+), 1 deletion(-)
> 

Acked-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
Philipp Tomsich Feb. 18, 2018, 7:33 p.m. UTC | #2
On Wed, 14 Feb 2018, Jagan Teki wrote:

> rockchip boot0 add 4 bytes data (0xeaffffff) at beginning
> of executable(0x800) in order to make generic and compatible
> boot0 for all platforms and the resulting executable will be
> input to mkimage and the output of mkimage with initial
> 4 bytes will overwritten by 'spl_hdr'

This is factually wrong (the beginning of the executable is neither at
0x800 nor are 4 bytes of data inserted by boot0 ... boot0 inserts a 
branch-instruction w/ an immediate offset).  Please rewrite.

>
> Since the TPL_TEXT_BASE in rk3288 is 0xff704004 which is improper
> align to branch-to-next-instruction-word, so the resulting 4 bytes
> are written at ff704020 of executable instead of beginning ff704000
>
> Hexdump with overlaped bytes:
> -----------------------------
> 0000000 0000 0000 0000 0000 0000 0000 0000 0000
> 0000010 0000 0000 0000 0000 0000 0000 ffff eaff
>
> So, fix this improper TEXT_BASE which is wrong even before
> and update it to 0xff704000 so-that the boot0 will add
> 4 bytes at beginning.
>
> Disassembly:
> -----------
> with 0xff704020 TEXT_BASE:
>
> ff704004 <__image_copy_start>:
> 	b 1f     /* if overwritten, entry-address is at the next word */
> ff704020:       eaffffff        b       ff704024 <__image_copy_start+0x20>
>
> with 0xff704000 TEXT_BASE:
>
> ff704000 <__image_copy_start>:
> 	b 1f     /* if overwritten, entry-address is at the next word */
> ff704000:       eaffffff        b       ff704004 <__image_copy_start+0x4>

This is confusing. Please rewrite so it is clear what the problem is and 
why the wrong TEXT_BASE is causing these effects.

> This patch will also move TPL_TEXT_BASE into kconfig.
>
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> Acked-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> ---
> arch/arm/mach-rockchip/Kconfig | 3 +++
> configs/vyasa-rk3288_defconfig | 1 -
> 2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig
> index d9218da..0adaed4 100644
> --- a/arch/arm/mach-rockchip/Kconfig
> +++ b/arch/arm/mach-rockchip/Kconfig
> @@ -77,6 +77,9 @@ if ROCKCHIP_RK3288
> config TPL_LDSCRIPT
> 	default "arch/arm/mach-rockchip/rk3288/u-boot-tpl.lds"
>
> +config TPL_TEXT_BASE
> +	default 0xff704000
> +
> endif
>
> config ROCKCHIP_RK3328
> diff --git a/configs/vyasa-rk3288_defconfig b/configs/vyasa-rk3288_defconfig
> index 1a8a9a8..4c76041 100644
> --- a/configs/vyasa-rk3288_defconfig
> +++ b/configs/vyasa-rk3288_defconfig
> @@ -5,7 +5,6 @@ CONFIG_ARCH_ROCKCHIP=y
> CONFIG_SYS_TEXT_BASE=0x00100000
> CONFIG_SYS_MALLOC_F_LEN=0x2000
> CONFIG_ROCKCHIP_RK3288=y
> -CONFIG_TPL_TEXT_BASE=0xff704004
> CONFIG_TARGET_VYASA_RK3288=y
> CONFIG_SPL_STACK_R_ADDR=0x80000
> CONFIG_DEFAULT_DEVICE_TREE="rk3288-vyasa"
>
diff mbox series

Patch

diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig
index d9218da..0adaed4 100644
--- a/arch/arm/mach-rockchip/Kconfig
+++ b/arch/arm/mach-rockchip/Kconfig
@@ -77,6 +77,9 @@  if ROCKCHIP_RK3288
 config TPL_LDSCRIPT
 	default "arch/arm/mach-rockchip/rk3288/u-boot-tpl.lds"
 
+config TPL_TEXT_BASE
+	default 0xff704000
+
 endif
 
 config ROCKCHIP_RK3328
diff --git a/configs/vyasa-rk3288_defconfig b/configs/vyasa-rk3288_defconfig
index 1a8a9a8..4c76041 100644
--- a/configs/vyasa-rk3288_defconfig
+++ b/configs/vyasa-rk3288_defconfig
@@ -5,7 +5,6 @@  CONFIG_ARCH_ROCKCHIP=y
 CONFIG_SYS_TEXT_BASE=0x00100000
 CONFIG_SYS_MALLOC_F_LEN=0x2000
 CONFIG_ROCKCHIP_RK3288=y
-CONFIG_TPL_TEXT_BASE=0xff704004
 CONFIG_TARGET_VYASA_RK3288=y
 CONFIG_SPL_STACK_R_ADDR=0x80000
 CONFIG_DEFAULT_DEVICE_TREE="rk3288-vyasa"