From mboxrd@z Thu Jan 1 00:00:00 1970 From: Peng Fan Date: Wed, 21 Oct 2015 17:42:20 +0800 Subject: [U-Boot] [PATCH V2 1/3] arm: discard relocation entry for secure section In-Reply-To: <20151020145910.5e40f8cf@lilith> References: <1445320795-2235-1-git-send-email-Peng.Fan@freescale.com> <20151020090532.48426615@lilith> <20151020072039.GA26880@shlinux2> <20151020093251.468ebaa0@lilith> <20151020074127.GA31531@shlinux2> <20151020145910.5e40f8cf@lilith> Message-ID: <20151021094216.GA29761@shlinux2> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hello Albert, On Tue, Oct 20, 2015 at 02:59:10PM +0200, Albert ARIBAUD wrote: >Hello Peng, > >On Tue, 20 Oct 2015 15:41:30 +0800, Peng Fan >wrote: >> 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 >> >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 >> >> >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". > >Hmm, you're correct, the linker will use the first pattern match, not >the longest or best. :( > >But then, the secure reloc input section cannot go in line 55, because >that's still inside the binary part of the image, which means it will >waste space in there. > >So it's either use DISCARD at the very start of the SECTIONS (round >line 17) or back to not generating relocatable code at all for PSCI. I do not know how to generate non-relocatable code only for the secure text. Since -mword-relocations and -pie are global flags, I do not know how to disable mword-relocations when compiling PSCI and other secure text. Is this ok for you? diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds index 65986e8..fb2128a 100644 --- a/arch/arm/cpu/u-boot.lds +++ b/arch/arm/cpu/u-boot.lds @@ -14,6 +14,7 @@ OUTPUT_ARCH(arm) ENTRY(_start) SECTIONS { + /DISCARD/ : { *(.rel._secure*) } . = 0x00000000; . = ALIGN(4); Regards, Peng. > >> Regards, >> Peng. >> >> > >> >Amicalement, >> >-- >> >Albert. >> >> -- > > > >Amicalement, >-- >Albert. --