Hi Benoît, (I had indeed missed remarks in your reply; apologies for this)
On Tue, 14 May 2013 18:01:50 +0200 (CEST), Benoît Thébaudeau <benoit.thebaud...@advansee.com> wrote: > Hi Albert, > > +/* > > + * These are defined in the board-specific linker script. > > + * Subtracting _start from them lets the linker put their > > + * relative position in the executable instead of leaving > > + * them null. > > + */ > > This comment is obsolete. It should either be removed or updated. Correct. > > + > > +/* > > + * void relocate_code(addr_moni) > > + * > > + * This function relocates the monitor code. > > + */ > > +ENTRY(relocate_code) > > + mov r6, r0 /* save addr of destination */ > > + > > + ldr r0, =_start > > If you are avoiding the "ldr =" construct below, why do you use it > here? You could "ldr r0, _TEXT_BASE". _start is defined in a compilation unit, not in the linker file. References to _start such as the one above are therefore correct before as well as after relocation. > > + subs r9, r6, r0 /* r9 <- relocation offset */ > > + beq relocate_done /* skip relocation */ > > + mov r1, r6 /* r1 <- scratch for copy loop */ > > + ldr r3, _image_copy_end_ofs > > + add r2, r0, r3 /* r2 <- source end address */ > > Adding _start to a relocate_code-relative _image_copy_end_ofs?! You're correct. For some reason I did not complete the rewrite here. :( I'd made the offsets relative to relocate_code in order to avoid the linker issues with subtracting symbols not defined in the current compilation unit. Then I should add =relocate_code to r3, not =_start, and also -- as r9 is not the right offset here -- compute r7 as the delta between the link-time =_start and the run-time relocate_code (r7 becomes useless once R10, r2 and r3 are fixed). I'd run-tested tested each commit but apparently this bug did not cause any immediately visible issue. This time I'll step-test it. > > + /* > > + * fix .rel.dyn relocations > > + */ > > + ldr r10, _dynsym_start_ofs /* r10 <- sym table ofs */ > > + add r10, r10, r9 /* r10 <- sym table in FLASH */ > > + ldr r2, _rel_dyn_start_ofs /* r2 <- rel dyn start ofs */ > > + add r2, r2, r9 /* r2 <- rel dyn start in FLASH */ > > + ldr r3, _rel_dyn_end_ofs /* r3 <- rel dyn end ofs */ > > + add r3, r3, r9 /* r3 <- rel dyn end in FLASH */ > > This is mixing relocate_code-relative offsets with the relocation offset! Correct, that's where r7 kicks in to replace r9. Again, apologies for missing this. > Best regards, > Benoît Amicalement, -- Albert. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
