From mboxrd@z Thu Jan 1 00:00:00 1970 From: Albert ARIBAUD Date: Tue, 20 Sep 2011 21:13:34 +0200 Subject: [U-Boot] [RFC PATCH] arm: provide a CONFIG flag for disabling relocation In-Reply-To: <20110920180959.8008F1208F1E@gemini.denx.de> References: <20110920180959.8008F1208F1E@gemini.denx.de> Message-ID: <4E78E5DE.60300@aribaud.net> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Le 20/09/2011 20:09, Wolfgang Denk a ?crit : > Dear "GROYER, Anthony", > > In message you wrote: >> >> The use of the initial patches for the CONFIG_SYS_SKIP_ARM_RELOCATION featu >> res has revealed two issues. > > Could you please restict your line length to some 70 characters or so? > Thanks. > >> First issue: the calculation of the relocation offset was done only if the >> relocation is actually done. So we could reach a point where r9 has a wrong >> value, since it has never been used before (in my case, this bug happens w > > This is a configuration error then, isn't it? The relocation offset > should be either the intended value, or eventually zero, if no > relocation is intended. Actually, even though "revision 1083" and "revision 1113" are not git references (and thus I can't be sure Anthony is referring to up-to-date mainline code), there is a point to what Anthony says: in the case where relocation is unneeded (r0 equals r6) then r9 is not set, but is still used when branching to board_init_r(). for this bug to have any effect, relocation would have to be unneeded, which is a rare case, *and* r9 has to be nonzero, which may or may not happen depending on the code executed until relocate_code() is called, and thus makes the whole condition rarer yet; probably the rarity of these two conjunct conditions explains why it was not noticed until now. However, since start.S has a code path to handle the non-relocating case, this path ought to be bug-free. But then, I want it to be consistent: if the relocation offset is computed in r9, then testing whether relocation is needed would be done on r9 once computed, not before, by replacing adr r0, _start cmp r0, r6 beq clear_bss /* skip relocation */ With adr r0, _start sub r9, r6, r0 cmp r0, #0 beq clear_bss /* skip relocation */ > BTW: your patch has a number ofd coduing style errors, and the > Signed-off-by: line is missing. Plus it did not have the commit message separator either. I suspect it was not produced using git format-patch / git send-email. Anthony, please submit a proper [PATCH], without [RFC], and with the setting of r9 done as shown above, and applied to all relevant start.S files in arch/arm/cpu/*/ -- in next merge window we should try and factorize those start.S files, BTW. > Best regards, > > Wolfgang Denk Amicalement, -- Albert.