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

[Peng Fan <b51431@freescale.com>]

Hi Albert,

On Tue, Oct 20, 2015 at 09:32:51AM +0200, Albert ARIBAUD wrote:
>Hello Peng,
>
>On Tue, 20 Oct 2015 15:20:43 +0800, Peng Fan <b51431@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@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.

But ".rel._secure { *(.rel._secure*) }" can not be placed after
".dynamic : { *(.dynamic*) }". Actually ".rel._secure { *(.rel._secure*) }"
should be placed before ".rel_dyn_start" in u-boot.lds, otherwise
the secure relocation entries will be placed into ".rel.dyn", but not
".rel._secure".

Regards,
Peng.

>
>Amicalement,
>-- 
>Albert.

--