[U-Boot] [PATCH V2 1/3] arm: discard relocation entry for secure section

[Albert ARIBAUD]

Hello Peng,

On Tue, 20 Oct 2015 15:20:43 +0800, Peng Fan <b51431 at freescale.com>
wrote:
> Hi Albert,
> 
> On Tue, Oct 20, 2015 at 09:05:32AM +0200, Albert ARIBAUD wrote:
> >Hello Peng,
> >
> >On Tue, 20 Oct 2015 13:59:53 +0800, Peng Fan <Peng.Fan at freescale.com>
> >wrote:
> >> The code such as PSCI in section named secure is bundled with
> >> u-boot image, and when bootm, the code will be copied to their
> >> runtime address same to compliation/linking address -
> >> CONFIG_ARMV7_SECURE_BASE.
> >> 
> >> When compile the PSCI code and link it into the u-boot image,
> >> there will be relocation entries in .rel.dyn section for PSCI.
> >> Actually, we do not needs these relocation entries.
> >> 
> >> If still keep the relocation entries in .rel.dyn section,
> >> r0 at line 103 and 106 in arch/arm/lib/relocate.S may be an invalid
> >> address which may not support read/write for one SoC.
> >> 102         /* relative fix: increase location by offset */
> >> 103         add     r0, r0, r4
> >> 104         ldr     r1, [r0]
> >> 105         add     r1, r1, r4
> >> 106         str     r1, [r0]
> >> 
> >> So discard the relocation entries for code in secure section.
> >
> >Actually, I'll need you to do a slight change -- that's my fault, and
> >karma even ensured that my own self of two years ago would be the one
> >to come and kick me. See:
> >
> >http://lists.denx.de/pipermail/u-boot/2013-December/168652.html
> 
> Ok. Then arch/arm/cpu/armv8/u-boot.lds should also have such fix,
> since lots sections are discarded in u-boot.lds for armv8.

You are correct, armv8 needs to be fixed, as well as zynq (and x86 but
that's outside of my province). Anyway, that's a different problem
which does not need to be addressed in this series.

> >Which basically is about never discarding any section in the ELF file,
> >and only copying a subset of the ELF sections into the binary file.
> >
> >So rather than discarding the secure relocation records, let's move
> >them to another section as you had proposed, and thus change the line
> >
> >> +	/DISCARD/ : { *(.rel._secure*) }
> >
> >Into a
> >
> >	.rel._secure { *(.rel._secure*) }
> >
> >Placed right after the already present
> >
> >	.dynamic : { *(.dynamic*) }
> 
> It can not be placed after .dynamic, since the following is at front.
> 87         .rel.dyn : {
> 88                 *(.rel*)
> 89         }
> So relocation entry from secure text will first placed into .rel.dyn section.
> 
> If not DISCARD, then I prefer to put ".rel.secure : { *(.rel._secure*) }"
> at line 55 which is wrapped by CONFIG_ARMV7_NONSEC in arch/arm/cpu/u-boot.lds.

I prefer all 'unused' sections to be kept together near the end of the
LDS file -- I'll add an explicit comment in the LDS about it.

Besides, there no need to wrap such a section with a preprocessor
conditional, as the linker will simply not emit an output section if
there are no input sections at all for it; IOW, adding the '.rel._secure
{ *(.rel._secure*) }' line will be binary-invariant for platforms which
do not have PSCI or other secure code.

Amicalement,
-- 
Albert.