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

[Peng Fan]

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*) }
. = 0x;
   
. = ALIGN(4);

Regards,
Peng.
>
>> Regards,
>> Peng.
>> 
>> >
>> >Amicalement,
>> >-- 
>> >Albert.
>> 

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

[Peng Fan]

Hello Albert,

On Wed, Oct 21, 2015 at 01:42:27PM +0200, Albert ARIBAUD wrote:
>Hello Peng,
>
>On Wed, 21 Oct 2015 17:42:20 +0800, Peng Fan 
>wrote:
>> Hello Albert,
>
>> 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.
>
>Actually, I haven't even found a compiler or gcc option to entirely
>remove relocations altogether.
>
>> 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*) }
>> . = 0x;
>>
>> . = ALIGN(4);
>
>Functionally, it works -- I've just checked it by comparing u-boot.map
>files of a mx7dsabresd build with and without the DISCARD. Not a single
>symbol from the text and data section gets changed.
>
>The only missing thing left is a comment before the DISCARD line,
>explaining both why .rel._secure* input sections have to be discarded,
>and also explaining why this is done it right at the start of the output
>sections.

Ok. Will add the comments in V3.

Regards,
Peng.

>
>Thanks for your patience. :)
>
>> Regards,
>> Peng.
>
>Amicalement,
>-- 
>Albert.

-- 
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

[Albert ARIBAUD]

Hello Peng,

On Wed, 21 Oct 2015 17:42:20 +0800, Peng Fan 
wrote:
> Hello Albert,

> 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.

Actually, I haven't even found a compiler or gcc option to entirely
remove relocations altogether.

> 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*) }
> . = 0x;
>
> . = ALIGN(4);

Functionally, it works -- I've just checked it by comparing u-boot.map
files of a mx7dsabresd build with and without the DISCARD. Not a single
symbol from the text and data section gets changed.

The only missing thing left is a comment before the DISCARD line,
explaining both why .rel._secure* input sections have to be discarded,
and also explaining why this is done it right at the start of the output
sections.

Thanks for your patience. :)

> Regards,
> Peng.

Amicalement,
-- 
Albert.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

[Peng Fan]

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.

>
>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.

See following patch:

>From 4b950418835ff52cd7be8dd2b80fb8b0e055f9a2 Mon Sep 17 00:00:00 2001
From: Peng Fan 
Date: Tue, 20 Oct 2015 15:18:22 +0800
Subject: [PATCH] arm: move relocation entry for secure section into a single
 section

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 move them into a single section to avoid touching the relocation entry
in arch/arm/lib/relocate.S.

Signed-off-by: Peng Fan 
Cc: Tom Warren 
Cc: York Sun 
Cc: Hans De Goede 
Cc: Ian Campbell 
Cc: Albert Aribaud 
Cc: Tom Rini 
Cc: Jan Kiszka 
Cc: Stefano Babic 
---
 arch/arm/cpu/u-boot.lds | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds
index 03cd9f6..65986e8 100644
--- a/arch/arm/cpu/u-boot.lds
+++ b/arch/arm/cpu/u-boot.lds
@@ -51,6 +51,8 @@ SECTIONS
*(.__secure_end)
LONG(0x1d1071c);/* Must output something to reset LMA */
}
+
+   .rel.secure : { *(.rel._secure*) }
 #endif
 
. = ALIGN(4);

Regards,
Peng.

>
>With my apologies for the very late realization.
>
>Amicalement,
>-- 
>Albert.

-- 
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [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 
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.

Amicalement,
-- 
Albert.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

[Peng Fan]

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".

Regards,
Peng.

>
>Amicalement,
>-- 
>Albert.

-- 
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

[Albert ARIBAUD]

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

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*) }

With my apologies for the very late realization.

Amicalement,
-- 
Albert.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

[Albert ARIBAUD]

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.

> Regards,
> Peng.
> 
> >
> >Amicalement,
> >-- 
> >Albert.
> 
> -- 



Amicalement,
-- 
Albert.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot