From mboxrd@z Thu Jan 1 00:00:00 1970 From: Albert ARIBAUD Date: Wed, 15 May 2013 09:31:37 +0200 Subject: [U-Boot] [PATCH v2 4/4] arm: factorize relocate_code routine In-Reply-To: <1489195069.818131.1368547310358.JavaMail.root@advansee.com> References: <1368223012-17609-1-git-send-email-albert.u.boot@aribaud.net> <1368525030-5162-1-git-send-email-albert.u.boot@aribaud.net> <1368525030-5162-2-git-send-email-albert.u.boot@aribaud.net> <1368525030-5162-3-git-send-email-albert.u.boot@aribaud.net> <1368525030-5162-4-git-send-email-albert.u.boot@aribaud.net> <1368525030-5162-5-git-send-email-albert.u.boot@aribaud.net> <1489195069.818131.1368547310358.JavaMail.root@advansee.com> Message-ID: <20130515093137.5c31a529@lilith> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 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.