[U-Boot] [PATCH] ARM: fixed relocation using proper alignment

[Lokesh Vutla]

On Wednesday 10 May 2017 07:11 PM, Manfred Schlaegl wrote:
> Using u-boot-2017.05 on i.MX6UL we ran into following problem:
> Initially U-Boot could be started normally.
> If we added one random command in configuration, the newly generated
> image hung at startup (last output was DRAM:  256 MiB).
> 
> We tracked this down to a data abort within relocation (relocated_code).
> 
> relocated_code in arch/arm/lib/relocate.S copies 8 bytes per loop
> iteration until the source pointer is equal to __image_copy_end.
> In a good case __image_copy_end was aligned to 8 bytes, so the loop
> stopped as suggested, but in an errornous case __image_copy_end was
> not aligned to 8 bytes, so the loop ran out of bounds and caused a
> data abort exception.

Well I agree with this patch but I have a small query. Looking at the
relocation code:

copy_loop:
	ldmia	r1!, {r10-r11}		/* copy from source address [r1]    */
	stmia	r0!, {r10-r11}		/* copy to   target address [r0]    */
	cmp  	r1, r2			/* until source end address [r2]    */
	blo		copy_loop

IIUC, this loops exits as soon as r1 >= r2

In your case
r1 is __image_copy_start
r0 is relocation address
r2 is __image_copy_end (which is 4 byte aligned)

In the corner case you mentioned there will be extra memcpy of 4 bytes
from address in r1 to address in r0. I assume you are running from DDR
and relocation offset is calculated such that (aligned to previous 4K
page) it should be able to accommodate extra 4 bytes(I assume). I am
wondering why should this give a data abort.

Thanks and regards,
Lokesh



> 
> This patches solves the issue by aligning __image_copy_end to 8 byte
> using the linker script related to arm.
> 
> I don't know if it's the correct way to solve this, so some review would
> be very appreciated.
> ---
>  arch/arm/cpu/u-boot.lds | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds
> index 37d4c60..70bee1a 100644
> --- a/arch/arm/cpu/u-boot.lds
> +++ b/arch/arm/cpu/u-boot.lds
> @@ -165,7 +165,7 @@ SECTIONS
>  		*(.__efi_runtime_rel_stop)
>  	}
>  
> -	. = ALIGN(4);
> +	. = ALIGN(8);
>  
>  	.image_copy_end :
>  	{
>