From mboxrd@z Thu Jan 1 00:00:00 1970 From: Lokesh Vutla Date: Thu, 11 May 2017 12:23:00 +0530 Subject: [U-Boot] [PATCH] ARM: fixed relocation using proper alignment In-Reply-To: References: Message-ID: <81d82db6-d9a9-4400-0acf-5014f38c1668@ti.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 : > { >