[PATCH 0/6] ARM: compressed: clean up section layout and enable EFI debugging

[PATCH 0/6] ARM: compressed: clean up section layout and enable EFI debugging

[Ard Biesheuvel]

Currently, the decompressor section layout is somewhat unusual: head.S
puts code into a .start and a .text section, which are expected to be
emitted back to back, unless other objects are included that define a
.start section as well, in which case execution is expected to proceed 
linearly from head.S's section .start section into the other .start
section and then onward into head.S's .text section.

This relies on the input files to be consumed by the linker in the same
order they were specified on the command line, but this is not actually
guaranteed by the linker, so it is better to make the inclusion of this
code specific by using function calls. Also, it is better to use a jump
instruction when going from one section to the next rather than relying
on code flow to proceed seamlessly from one to the other.

The reason I am cleaning this up is because I want to modify the section
layout slightly, so that the ELF and PE/COFF layouts are identical. This
permits a debug feature to be enabled that makes it possible to do single
step debugging from the EFI stub into the firmware and back, which is very
useful for debugging the handover from UEFI to the decompressor.

Ard Biesheuvel (6):
  ARM: compressed: move sharpsl startup code into subroutine
  ARM: compressed: move sa1100 startup code into subroutine
  ARM: compressed: move xscale startup code into subroutine
  ARM: compressed: move BE32 handling into head.S
  ARM: compressed: put zImage header and EFI header in dedicated section
  ARM: efi: add PE/COFF debug table to EFI header

 arch/arm/boot/compressed/Makefile       | 12 ++---
 arch/arm/boot/compressed/big-endian.S   | 14 ------
 arch/arm/boot/compressed/efi-header.S   | 47 ++++++++++++++++++++
 arch/arm/boot/compressed/head-sa1100.S  |  9 ++--
 arch/arm/boot/compressed/head-sharpsl.S | 24 +++++-----
 arch/arm/boot/compressed/head-xscale.S  |  7 +--
 arch/arm/boot/compressed/head.S         | 20 +++++++--
 arch/arm/boot/compressed/vmlinux.lds.S  |  4 +-
 8 files changed, 92 insertions(+), 45 deletions(-)
 delete mode 100644 arch/arm/boot/compressed/big-endian.S

-- 
2.19.1

[PATCH 1/6] ARM: compressed: move sharpsl startup code into subroutine

[Ard Biesheuvel]

Instead of relying on unspecified linker behavior, move the
SharpSL startup code into a subroutine and call it from the
location we expect the linker to put the code.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 arch/arm/boot/compressed/head-sharpsl.S | 24 ++++++++++----------
 arch/arm/boot/compressed/head.S         |  3 +++
 2 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/arch/arm/boot/compressed/head-sharpsl.S b/arch/arm/boot/compressed/head-sharpsl.S
index 992e784500fa..f4e6ad318376 100644
--- a/arch/arm/boot/compressed/head-sharpsl.S
+++ b/arch/arm/boot/compressed/head-sharpsl.S
@@ -20,9 +20,10 @@
 #error What am I doing here...
 #endif
 
-		.section        ".start", "ax"
+	.text
 
-__SharpSL_start:
+ENTRY(__SharpSL_start)
+	mov	ip, lr			@ preserve lr
 
 /* Check for TC6393 - if found we have a Tosa */
 	ldr	r7, .TOSAID
@@ -30,7 +31,7 @@ __SharpSL_start:
 	mov 	r6, #0x03
 	ldrh	r3, [r1, #8]		@ Load TC6393XB Revison: This is 0x0003
 	cmp	r6, r3
-	beq	.SHARPEND		@ Success -> tosa
+	moveq	pc, ip			@ Success -> tosa
 
 /* Check for pxa270 - if found, branch */
 	mrc p15, 0, r4, c0, c0		@ Get Processor ID
@@ -55,30 +56,30 @@ __SharpSL_start:
 	ldr	r3, .W100ID
 	ldr	r7, .POODLEID
 	cmp	r6, r3
-	bne	.SHARPEND			@ We have no w100 - Poodle
+	movne	pc, ip				@ We have no w100 - Poodle
 
 /* Check for pxa250 - if found we have a Corgi */
 	ldr	r7, .CORGIID
 	ldr	r3, .PXA255ID
 	cmp	r4, r3
-	blo	.SHARPEND			@ We have a PXA250 - Corgi
+	movlo	pc, ip				@ We have a PXA250 - Corgi
 
 /* Check for 64MiB flash - if found we have a Shepherd */
 	bl	get_flash_ids
 	ldr	r7, .SHEPHERDID
 	cmp	r3, #0x76			@ 64MiB flash
-	beq	.SHARPEND			@ We have Shepherd
+	moveq	pc, ip				@ We have Shepherd
 
 /* Must be a Husky */
 	ldr	r7, .HUSKYID		@ Must be Husky
-	b .SHARPEND
+	mov	pc, ip			@ return
 
 .PXA270:
 /* Check for 16MiB flash - if found we have Spitz */
 	bl	get_flash_ids
 	ldr	r7, .SPITZID
 	cmp	r3, #0x73			@ 16MiB flash
-	beq	.SHARPEND			@ We have Spitz
+	moveq	pc, ip				@ We have Spitz
 
 /* Check for a second SCOOP chip - if found we have Borzoi */
 	ldr	r1, .SCOOP2ADDR
@@ -87,11 +88,12 @@ __SharpSL_start:
 	strh	r6, [r1]
 	ldrh	r6, [r1]
 	cmp	r6, #0x0140
-	beq	.SHARPEND			@ We have Borzoi
+	moveq	pc, ip				@ We have Borzoi
 
 /* Must be Akita */
 	ldr	r7, .AKITAID
-	b	.SHARPEND			@ We have Borzoi
+	mov	pc, ip				@ We have Borzoi
+ENDPROC(__SharpSL_start)
 
 .PXA255ID:
 	.word	0x69052d00		@ PXA255 Processor ID
@@ -147,5 +149,3 @@ get_flash_ids:
 	ldrb	r2, [r1, #20]		@ NAND Manufacturer ID
 	ldrb	r3, [r1, #20]		@ NAND Chip ID
 	mov	pc, lr
-
-.SHARPEND:
diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
index 6c7ccb428c07..5067f287fa5a 100644
--- a/arch/arm/boot/compressed/head.S
+++ b/arch/arm/boot/compressed/head.S
@@ -213,6 +213,9 @@ not_angel:
 		 */
 
 		.text
+#ifdef CONFIG_PXA_SHARPSL_DETECT_MACH_ID
+		bl	__SharpSL_start
+#endif
 
 #ifdef CONFIG_AUTO_ZRELADDR
 		/*
-- 
2.19.1

[PATCH 1/6] ARM: compressed: move sharpsl startup code into subroutine

[Russell King - ARM Linux]

On Mon, Nov 05, 2018 at 07:44:33PM +0100, Ard Biesheuvel wrote:
> Instead of relying on unspecified linker behavior, move the
> SharpSL startup code into a subroutine and call it from the
> location we expect the linker to put the code.

Why is it "unspecified linker behaviour" ?  Lots of libraries and
programs rely on the linker building tables, whether it be data or
code from sections.

For example, programs build a section called .init which starts with
the code in crti.o, code from other files, and finishes with code
from crtn.o.

crti.o:

Disassembly of section .init:

00000000 <_init>:
   0:   e92d4008        push    {r3, lr}
   4:   ebfffffe        bl      0 <_init>
                        4: R_ARM_CALL   call_weak_fn

crtn.o:

Disassembly of section .init:

00000000 <.init>:
   0:   e8bd8008        pop     {r3, pc}

So, I don't think there's anything "unspecified" here.

We also rely heavily on this in the kernel to build exception tables,
symbol tables, and so forth.

> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  arch/arm/boot/compressed/head-sharpsl.S | 24 ++++++++++----------
>  arch/arm/boot/compressed/head.S         |  3 +++
>  2 files changed, 15 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/arm/boot/compressed/head-sharpsl.S b/arch/arm/boot/compressed/head-sharpsl.S
> index 992e784500fa..f4e6ad318376 100644
> --- a/arch/arm/boot/compressed/head-sharpsl.S
> +++ b/arch/arm/boot/compressed/head-sharpsl.S
> @@ -20,9 +20,10 @@
>  #error What am I doing here...
>  #endif
>  
> -		.section        ".start", "ax"
> +	.text
>  
> -__SharpSL_start:
> +ENTRY(__SharpSL_start)
> +	mov	ip, lr			@ preserve lr
>  
>  /* Check for TC6393 - if found we have a Tosa */
>  	ldr	r7, .TOSAID
> @@ -30,7 +31,7 @@ __SharpSL_start:
>  	mov 	r6, #0x03
>  	ldrh	r3, [r1, #8]		@ Load TC6393XB Revison: This is 0x0003
>  	cmp	r6, r3
> -	beq	.SHARPEND		@ Success -> tosa
> +	moveq	pc, ip			@ Success -> tosa
>  
>  /* Check for pxa270 - if found, branch */
>  	mrc p15, 0, r4, c0, c0		@ Get Processor ID
> @@ -55,30 +56,30 @@ __SharpSL_start:
>  	ldr	r3, .W100ID
>  	ldr	r7, .POODLEID
>  	cmp	r6, r3
> -	bne	.SHARPEND			@ We have no w100 - Poodle
> +	movne	pc, ip				@ We have no w100 - Poodle
>  
>  /* Check for pxa250 - if found we have a Corgi */
>  	ldr	r7, .CORGIID
>  	ldr	r3, .PXA255ID
>  	cmp	r4, r3
> -	blo	.SHARPEND			@ We have a PXA250 - Corgi
> +	movlo	pc, ip				@ We have a PXA250 - Corgi
>  
>  /* Check for 64MiB flash - if found we have a Shepherd */
>  	bl	get_flash_ids
>  	ldr	r7, .SHEPHERDID
>  	cmp	r3, #0x76			@ 64MiB flash
> -	beq	.SHARPEND			@ We have Shepherd
> +	moveq	pc, ip				@ We have Shepherd
>  
>  /* Must be a Husky */
>  	ldr	r7, .HUSKYID		@ Must be Husky
> -	b .SHARPEND
> +	mov	pc, ip			@ return
>  
>  .PXA270:
>  /* Check for 16MiB flash - if found we have Spitz */
>  	bl	get_flash_ids
>  	ldr	r7, .SPITZID
>  	cmp	r3, #0x73			@ 16MiB flash
> -	beq	.SHARPEND			@ We have Spitz
> +	moveq	pc, ip				@ We have Spitz
>  
>  /* Check for a second SCOOP chip - if found we have Borzoi */
>  	ldr	r1, .SCOOP2ADDR
> @@ -87,11 +88,12 @@ __SharpSL_start:
>  	strh	r6, [r1]
>  	ldrh	r6, [r1]
>  	cmp	r6, #0x0140
> -	beq	.SHARPEND			@ We have Borzoi
> +	moveq	pc, ip				@ We have Borzoi
>  
>  /* Must be Akita */
>  	ldr	r7, .AKITAID
> -	b	.SHARPEND			@ We have Borzoi
> +	mov	pc, ip				@ We have Borzoi
> +ENDPROC(__SharpSL_start)
>  
>  .PXA255ID:
>  	.word	0x69052d00		@ PXA255 Processor ID
> @@ -147,5 +149,3 @@ get_flash_ids:
>  	ldrb	r2, [r1, #20]		@ NAND Manufacturer ID
>  	ldrb	r3, [r1, #20]		@ NAND Chip ID
>  	mov	pc, lr
> -
> -.SHARPEND:
> diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
> index 6c7ccb428c07..5067f287fa5a 100644
> --- a/arch/arm/boot/compressed/head.S
> +++ b/arch/arm/boot/compressed/head.S
> @@ -213,6 +213,9 @@ not_angel:
>  		 */
>  
>  		.text
> +#ifdef CONFIG_PXA_SHARPSL_DETECT_MACH_ID
> +		bl	__SharpSL_start
> +#endif
>  
>  #ifdef CONFIG_AUTO_ZRELADDR
>  		/*
> -- 
> 2.19.1
> 

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

[PATCH 1/6] ARM: compressed: move sharpsl startup code into subroutine

[Ard Biesheuvel]

On 5 November 2018 at 19:59, Russell King - ARM Linux
<linux@armlinux.org.uk> wrote:
> On Mon, Nov 05, 2018 at 07:44:33PM +0100, Ard Biesheuvel wrote:
>> Instead of relying on unspecified linker behavior, move the
>> SharpSL startup code into a subroutine and call it from the
>> location we expect the linker to put the code.
>
> Why is it "unspecified linker behaviour" ?  Lots of libraries and
> programs rely on the linker building tables, whether it be data or
> code from sections.
>
> For example, programs build a section called .init which starts with
> the code in crti.o, code from other files, and finishes with code
> from crtn.o.
>
> crti.o:
>
> Disassembly of section .init:
>
> 00000000 <_init>:
>    0:   e92d4008        push    {r3, lr}
>    4:   ebfffffe        bl      0 <_init>
>                         4: R_ARM_CALL   call_weak_fn
>
> crtn.o:
>
> Disassembly of section .init:
>
> 00000000 <.init>:
>    0:   e8bd8008        pop     {r3, pc}
>
> So, I don't think there's anything "unspecified" here.
>

In that case, we should do what GCC's builtin linker script uses:

  .init           :
  {
    KEEP (*(SORT_NONE(.init)))
  } =0

> We also rely heavily on this in the kernel to build exception tables,
> symbol tables, and so forth.
>

No, not really. In that case, they are arrays of data structures, not
sequences of instructions, and if they need to appear in a particular
order, they are sorted explicitly.

>>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>>  arch/arm/boot/compressed/head-sharpsl.S | 24 ++++++++++----------
>>  arch/arm/boot/compressed/head.S         |  3 +++
>>  2 files changed, 15 insertions(+), 12 deletions(-)
>>
>> diff --git a/arch/arm/boot/compressed/head-sharpsl.S b/arch/arm/boot/compressed/head-sharpsl.S
>> index 992e784500fa..f4e6ad318376 100644
>> --- a/arch/arm/boot/compressed/head-sharpsl.S
>> +++ b/arch/arm/boot/compressed/head-sharpsl.S
>> @@ -20,9 +20,10 @@
>>  #error What am I doing here...
>>  #endif
>>
>> -             .section        ".start", "ax"
>> +     .text
>>
>> -__SharpSL_start:
>> +ENTRY(__SharpSL_start)
>> +     mov     ip, lr                  @ preserve lr
>>
>>  /* Check for TC6393 - if found we have a Tosa */
>>       ldr     r7, .TOSAID
>> @@ -30,7 +31,7 @@ __SharpSL_start:
>>       mov     r6, #0x03
>>       ldrh    r3, [r1, #8]            @ Load TC6393XB Revison: This is 0x0003
>>       cmp     r6, r3
>> -     beq     .SHARPEND               @ Success -> tosa
>> +     moveq   pc, ip                  @ Success -> tosa
>>
>>  /* Check for pxa270 - if found, branch */
>>       mrc p15, 0, r4, c0, c0          @ Get Processor ID
>> @@ -55,30 +56,30 @@ __SharpSL_start:
>>       ldr     r3, .W100ID
>>       ldr     r7, .POODLEID
>>       cmp     r6, r3
>> -     bne     .SHARPEND                       @ We have no w100 - Poodle
>> +     movne   pc, ip                          @ We have no w100 - Poodle
>>
>>  /* Check for pxa250 - if found we have a Corgi */
>>       ldr     r7, .CORGIID
>>       ldr     r3, .PXA255ID
>>       cmp     r4, r3
>> -     blo     .SHARPEND                       @ We have a PXA250 - Corgi
>> +     movlo   pc, ip                          @ We have a PXA250 - Corgi
>>
>>  /* Check for 64MiB flash - if found we have a Shepherd */
>>       bl      get_flash_ids
>>       ldr     r7, .SHEPHERDID
>>       cmp     r3, #0x76                       @ 64MiB flash
>> -     beq     .SHARPEND                       @ We have Shepherd
>> +     moveq   pc, ip                          @ We have Shepherd
>>
>>  /* Must be a Husky */
>>       ldr     r7, .HUSKYID            @ Must be Husky
>> -     b .SHARPEND
>> +     mov     pc, ip                  @ return
>>
>>  .PXA270:
>>  /* Check for 16MiB flash - if found we have Spitz */
>>       bl      get_flash_ids
>>       ldr     r7, .SPITZID
>>       cmp     r3, #0x73                       @ 16MiB flash
>> -     beq     .SHARPEND                       @ We have Spitz
>> +     moveq   pc, ip                          @ We have Spitz
>>
>>  /* Check for a second SCOOP chip - if found we have Borzoi */
>>       ldr     r1, .SCOOP2ADDR
>> @@ -87,11 +88,12 @@ __SharpSL_start:
>>       strh    r6, [r1]
>>       ldrh    r6, [r1]
>>       cmp     r6, #0x0140
>> -     beq     .SHARPEND                       @ We have Borzoi
>> +     moveq   pc, ip                          @ We have Borzoi
>>
>>  /* Must be Akita */
>>       ldr     r7, .AKITAID
>> -     b       .SHARPEND                       @ We have Borzoi
>> +     mov     pc, ip                          @ We have Borzoi
>> +ENDPROC(__SharpSL_start)
>>
>>  .PXA255ID:
>>       .word   0x69052d00              @ PXA255 Processor ID
>> @@ -147,5 +149,3 @@ get_flash_ids:
>>       ldrb    r2, [r1, #20]           @ NAND Manufacturer ID
>>       ldrb    r3, [r1, #20]           @ NAND Chip ID
>>       mov     pc, lr
>> -
>> -.SHARPEND:
>> diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
>> index 6c7ccb428c07..5067f287fa5a 100644
>> --- a/arch/arm/boot/compressed/head.S
>> +++ b/arch/arm/boot/compressed/head.S
>> @@ -213,6 +213,9 @@ not_angel:
>>                */
>>
>>               .text
>> +#ifdef CONFIG_PXA_SHARPSL_DETECT_MACH_ID
>> +             bl      __SharpSL_start
>> +#endif
>>
>>  #ifdef CONFIG_AUTO_ZRELADDR
>>               /*
>> --
>> 2.19.1
>>
>
> --
> RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
> According to speedtest.net: 11.9Mbps down 500kbps up

[PATCH 1/6] ARM: compressed: move sharpsl startup code into subroutine

[Russell King - ARM Linux]

On Mon, Nov 05, 2018 at 08:07:10PM +0100, Ard Biesheuvel wrote:
> On 5 November 2018 at 19:59, Russell King - ARM Linux
> <linux@armlinux.org.uk> wrote:
> > On Mon, Nov 05, 2018 at 07:44:33PM +0100, Ard Biesheuvel wrote:
> >> Instead of relying on unspecified linker behavior, move the
> >> SharpSL startup code into a subroutine and call it from the
> >> location we expect the linker to put the code.
> >
> > Why is it "unspecified linker behaviour" ?  Lots of libraries and
> > programs rely on the linker building tables, whether it be data or
> > code from sections.
> >
> > For example, programs build a section called .init which starts with
> > the code in crti.o, code from other files, and finishes with code
> > from crtn.o.
> >
> > crti.o:
> >
> > Disassembly of section .init:
> >
> > 00000000 <_init>:
> >    0:   e92d4008        push    {r3, lr}
> >    4:   ebfffffe        bl      0 <_init>
> >                         4: R_ARM_CALL   call_weak_fn
> >
> > crtn.o:
> >
> > Disassembly of section .init:
> >
> > 00000000 <.init>:
> >    0:   e8bd8008        pop     {r3, pc}
> >
> > So, I don't think there's anything "unspecified" here.
> >
> 
> In that case, we should do what GCC's builtin linker script uses:
> 
>   .init           :
>   {
>     KEEP (*(SORT_NONE(.init)))
>   } =0
> 
> > We also rely heavily on this in the kernel to build exception tables,
> > symbol tables, and so forth.
> >
> 
> No, not really. In that case, they are arrays of data structures, not
> sequences of instructions, and if they need to appear in a particular
> order, they are sorted explicitly.

Sorry, I disagree completely with you, as does the ld documentation:

   Normally, the linker will place files and sections matched by
   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
wildcards in the order in which they are seen during the link.  You can
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
change this by using the 'SORT_BY_NAME' keyword, which appears before a
wildcard pattern in parentheses (e.g., 'SORT_BY_NAME(.text*)').  When
the 'SORT_BY_NAME' keyword is used, the linker will sort the files or
sections into ascending order by name before placing them in the output
file.
...
   'SORT_NONE' disables section sorting by ignoring the command line
section sorting option.

Sorting by command line order is what we want.  Therefore, our existing
linker script is correct.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

[PATCH 1/6] ARM: compressed: move sharpsl startup code into subroutine

[Nicolas Pitre]

On Mon, 5 Nov 2018, Ard Biesheuvel wrote:

> Instead of relying on unspecified linker behavior, move the
> SharpSL startup code into a subroutine and call it from the
> location we expect the linker to put the code.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
[...]
> --- a/arch/arm/boot/compressed/head.S
> +++ b/arch/arm/boot/compressed/head.S
> @@ -213,6 +213,9 @@ not_angel:
>  		 */
>  
>  		.text
> +#ifdef CONFIG_PXA_SHARPSL_DETECT_MACH_ID
> +		bl	__SharpSL_start
> +#endif

Independently from Russell's points, I don't like the above. that will 
turn into an ugly list of #ifdefs and machine specific calls. I think 
you should use:

		bl	__early_arch_specific_start

and somewhere else in head.S:

WEAK(__early_arch_specific_start)
		mov	pc, lr
ENDPROC(__early_arch_specific_start)

That would make this change more palatable.



Nicolas

[PATCH 1/6] ARM: compressed: move sharpsl startup code into subroutine

[Ard Biesheuvel]

On 5 November 2018 at 20:25, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> On Mon, 5 Nov 2018, Ard Biesheuvel wrote:
>
>> Instead of relying on unspecified linker behavior, move the
>> SharpSL startup code into a subroutine and call it from the
>> location we expect the linker to put the code.
>>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
> [...]
>> --- a/arch/arm/boot/compressed/head.S
>> +++ b/arch/arm/boot/compressed/head.S
>> @@ -213,6 +213,9 @@ not_angel:
>>                */
>>
>>               .text
>> +#ifdef CONFIG_PXA_SHARPSL_DETECT_MACH_ID
>> +             bl      __SharpSL_start
>> +#endif
>
> Independently from Russell's points, I don't like the above. that will
> turn into an ugly list of #ifdefs and machine specific calls. I think
> you should use:
>
>                 bl      __early_arch_specific_start
>
> and somewhere else in head.S:
>
> WEAK(__early_arch_specific_start)
>                 mov     pc, lr
> ENDPROC(__early_arch_specific_start)
>
> That would make this change more palatable.
>

Yeah.

In any case, I will try a different approach that leaves the .start
and .text sections alone, given that the issue that I thought I was
addressing on the side does not turn out to exist in practice.

Thanks

[PATCH 2/6] ARM: compressed: move sa1100 startup code into subroutine

[Ard Biesheuvel]

Instead of relying on unspecified linker behavior, move the
SA1100 startup code into a subroutine and call it from the
location we expect the linker to put the code.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 arch/arm/boot/compressed/head-sa1100.S | 9 +++++----
 arch/arm/boot/compressed/head.S        | 3 +++
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boot/compressed/head-sa1100.S b/arch/arm/boot/compressed/head-sa1100.S
index 95abdd850fe3..89a7e9dfd5c0 100644
--- a/arch/arm/boot/compressed/head-sa1100.S
+++ b/arch/arm/boot/compressed/head-sa1100.S
@@ -4,18 +4,17 @@
  * 
  * Copyright (C) 1999 Nicolas Pitre <nico@fluxnic.net>
  * 
- * SA1100 specific tweaks.  This is merged into head.S by the linker.
+ * SA1100 specific tweaks.
  *
  */
 
 #include <linux/linkage.h>
 #include <asm/mach-types.h>
 
-		.section        ".start", "ax"
+		.text
 		.arch	armv4
 
-__SA1100_start:
-
+ENTRY(__SA1100_start)
 		@ Preserve r8/r7 i.e. kernel entry values
 #ifdef CONFIG_SA1100_COLLIE
 		mov	r7, #MACH_TYPE_COLLIE
@@ -47,3 +46,5 @@ __SA1100_start:
 		bic	r0, r0, #0x1000		@ clear Icache
 		mcr	p15, 0, r0, c1, c0, 0
 99:
+		mov	pc, lr
+ENDPROC(__SA1100_start)
diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
index 5067f287fa5a..c5355f929cee 100644
--- a/arch/arm/boot/compressed/head.S
+++ b/arch/arm/boot/compressed/head.S
@@ -216,6 +216,9 @@ not_angel:
 #ifdef CONFIG_PXA_SHARPSL_DETECT_MACH_ID
 		bl	__SharpSL_start
 #endif
+#ifdef CONFIG_ARCH_SA1100
+		bl	__SA1100_start
+#endif
 
 #ifdef CONFIG_AUTO_ZRELADDR
 		/*
-- 
2.19.1

[PATCH 2/6] ARM: compressed: move sa1100 startup code into subroutine

[Russell King - ARM Linux]

On Mon, Nov 05, 2018 at 07:44:34PM +0100, Ard Biesheuvel wrote:
> Instead of relying on unspecified linker behavior, move the
> SA1100 startup code into a subroutine and call it from the
> location we expect the linker to put the code.

Same comments as patch 1, I don't think this is warranted.

> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  arch/arm/boot/compressed/head-sa1100.S | 9 +++++----
>  arch/arm/boot/compressed/head.S        | 3 +++
>  2 files changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/boot/compressed/head-sa1100.S b/arch/arm/boot/compressed/head-sa1100.S
> index 95abdd850fe3..89a7e9dfd5c0 100644
> --- a/arch/arm/boot/compressed/head-sa1100.S
> +++ b/arch/arm/boot/compressed/head-sa1100.S
> @@ -4,18 +4,17 @@
>   * 
>   * Copyright (C) 1999 Nicolas Pitre <nico@fluxnic.net>
>   * 
> - * SA1100 specific tweaks.  This is merged into head.S by the linker.
> + * SA1100 specific tweaks.
>   *
>   */
>  
>  #include <linux/linkage.h>
>  #include <asm/mach-types.h>
>  
> -		.section        ".start", "ax"
> +		.text
>  		.arch	armv4
>  
> -__SA1100_start:
> -
> +ENTRY(__SA1100_start)
>  		@ Preserve r8/r7 i.e. kernel entry values
>  #ifdef CONFIG_SA1100_COLLIE
>  		mov	r7, #MACH_TYPE_COLLIE
> @@ -47,3 +46,5 @@ __SA1100_start:
>  		bic	r0, r0, #0x1000		@ clear Icache
>  		mcr	p15, 0, r0, c1, c0, 0
>  99:
> +		mov	pc, lr
> +ENDPROC(__SA1100_start)
> diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
> index 5067f287fa5a..c5355f929cee 100644
> --- a/arch/arm/boot/compressed/head.S
> +++ b/arch/arm/boot/compressed/head.S
> @@ -216,6 +216,9 @@ not_angel:
>  #ifdef CONFIG_PXA_SHARPSL_DETECT_MACH_ID
>  		bl	__SharpSL_start
>  #endif
> +#ifdef CONFIG_ARCH_SA1100
> +		bl	__SA1100_start
> +#endif
>  
>  #ifdef CONFIG_AUTO_ZRELADDR
>  		/*
> -- 
> 2.19.1
> 

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

[PATCH 3/6] ARM: compressed: move xscale startup code into subroutine

[Ard Biesheuvel]

Instead of relying on unspecified linker behavior, move the
XScale startup code into a subroutine and call it from the
location we expect the linker to put the code.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 arch/arm/boot/compressed/head-xscale.S | 7 ++++---
 arch/arm/boot/compressed/head.S        | 3 +++
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/compressed/head-xscale.S b/arch/arm/boot/compressed/head-xscale.S
index 20fa44d59f82..89e14a55ec3d 100644
--- a/arch/arm/boot/compressed/head-xscale.S
+++ b/arch/arm/boot/compressed/head-xscale.S
@@ -8,10 +8,9 @@
 
 #include <linux/linkage.h>
 
-		.section        ".start", "ax"
-
-__XScale_start:
+		.text
 
+ENTRY(__XScale_start)
 		@ Preserve r8/r7 i.e. kernel entry values
 
 		@ Data cache might be active.
@@ -33,3 +32,5 @@ __XScale_start:
 		bic	r0, r0, #0x1000		@ clear Icache
 		mcr	p15, 0, r0, c1, c0, 0
 
+		mov	pc, lr
+ENDPROC(__XScale_start)
diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
index c5355f929cee..da93f419d1f4 100644
--- a/arch/arm/boot/compressed/head.S
+++ b/arch/arm/boot/compressed/head.S
@@ -219,6 +219,9 @@ not_angel:
 #ifdef CONFIG_ARCH_SA1100
 		bl	__SA1100_start
 #endif
+#ifdef CONFIG_CPU_XSCALE
+		bl	__XScale_start
+#endif
 
 #ifdef CONFIG_AUTO_ZRELADDR
 		/*
-- 
2.19.1

[PATCH 3/6] ARM: compressed: move xscale startup code into subroutine

[Russell King - ARM Linux]

On Mon, Nov 05, 2018 at 07:44:35PM +0100, Ard Biesheuvel wrote:
> Instead of relying on unspecified linker behavior, move the
> XScale startup code into a subroutine and call it from the
> location we expect the linker to put the code.

Same as patch 1, I don't think this is warranted.

> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  arch/arm/boot/compressed/head-xscale.S | 7 ++++---
>  arch/arm/boot/compressed/head.S        | 3 +++
>  2 files changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/boot/compressed/head-xscale.S b/arch/arm/boot/compressed/head-xscale.S
> index 20fa44d59f82..89e14a55ec3d 100644
> --- a/arch/arm/boot/compressed/head-xscale.S
> +++ b/arch/arm/boot/compressed/head-xscale.S
> @@ -8,10 +8,9 @@
>  
>  #include <linux/linkage.h>
>  
> -		.section        ".start", "ax"
> -
> -__XScale_start:
> +		.text
>  
> +ENTRY(__XScale_start)
>  		@ Preserve r8/r7 i.e. kernel entry values
>  
>  		@ Data cache might be active.
> @@ -33,3 +32,5 @@ __XScale_start:
>  		bic	r0, r0, #0x1000		@ clear Icache
>  		mcr	p15, 0, r0, c1, c0, 0
>  
> +		mov	pc, lr
> +ENDPROC(__XScale_start)
> diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
> index c5355f929cee..da93f419d1f4 100644
> --- a/arch/arm/boot/compressed/head.S
> +++ b/arch/arm/boot/compressed/head.S
> @@ -219,6 +219,9 @@ not_angel:
>  #ifdef CONFIG_ARCH_SA1100
>  		bl	__SA1100_start
>  #endif
> +#ifdef CONFIG_CPU_XSCALE
> +		bl	__XScale_start
> +#endif
>  
>  #ifdef CONFIG_AUTO_ZRELADDR
>  		/*
> -- 
> 2.19.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

[PATCH 4/6] ARM: compressed: move BE32 handling into head.S

[Ard Biesheuvel]

The BE switch only takes 3 instructions, and is currently pulled into
the main startup code by the linker, which is not the nicest way to
do this. Let's just pull the code into head.S and enable it using
preprocessor conditionals.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 arch/arm/boot/compressed/Makefile     |  8 --------
 arch/arm/boot/compressed/big-endian.S | 14 --------------
 arch/arm/boot/compressed/head.S       |  5 +++++
 3 files changed, 5 insertions(+), 22 deletions(-)

diff --git a/arch/arm/boot/compressed/Makefile b/arch/arm/boot/compressed/Makefile
index 01bf2585a0fa..d1862621556f 100644
--- a/arch/arm/boot/compressed/Makefile
+++ b/arch/arm/boot/compressed/Makefile
@@ -47,14 +47,6 @@ ifeq ($(CONFIG_PXA_SHARPSL_DETECT_MACH_ID),y)
 OBJS		+= head-sharpsl.o
 endif
 
-ifeq ($(CONFIG_CPU_ENDIAN_BE32),y)
-ifeq ($(CONFIG_CPU_CP15),y)
-OBJS		+= big-endian.o
-else
-# The endian should be set by h/w design.
-endif
-endif
-
 #
 # We now have a PIC decompressor implementation.  Decompressors running
 # from RAM should not define ZTEXTADDR.  Decompressors running directly
diff --git a/arch/arm/boot/compressed/big-endian.S b/arch/arm/boot/compressed/big-endian.S
deleted file mode 100644
index 88e2a88d324b..000000000000
--- a/arch/arm/boot/compressed/big-endian.S
+++ /dev/null
@@ -1,14 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-/*
- *  linux/arch/arm/boot/compressed/big-endian.S
- *
- *  Switch CPU into big endian mode.
- *  Author: Nicolas Pitre
- */
-
-	.section ".start", #alloc, #execinstr
-
-	mrc	p15, 0, r0, c1, c0, 0	@ read control reg
-	orr	r0, r0, #(1 << 7)	@ enable big endian mode
-	mcr	p15, 0, r0, c1, c0, 0	@ write control reg
-
diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
index da93f419d1f4..55c227077207 100644
--- a/arch/arm/boot/compressed/head.S
+++ b/arch/arm/boot/compressed/head.S
@@ -222,6 +222,11 @@ not_angel:
 #ifdef CONFIG_CPU_XSCALE
 		bl	__XScale_start
 #endif
+#if defined(CONFIG_CPU_ENDIAN_BE32) && defined(CONFIG_CPU_CP15)
+		mrc	p15, 0, r0, c1, c0, 0	@ read control reg
+		orr	r0, r0, #(1 << 7)	@ enable big endian mode
+		mcr	p15, 0, r0, c1, c0, 0	@ write control reg
+#endif
 
 #ifdef CONFIG_AUTO_ZRELADDR
 		/*
-- 
2.19.1

[PATCH 5/6] ARM: compressed: put zImage header and EFI header in dedicated section

[Ard Biesheuvel]

To align the PE/COFF and the ELF headers of the decompressor binary, put
the zImage header and the EFI header in a separate .start section, and
emit it at the start of the Image. This change is necessary for UEFI
based debug tooling to be able to use the vmlinux ELF binary, since it
gets confused if the PE/COFF .text section and the ELF .text section live
at different offsets.

With this change, we also stop relying on the code to flow from one
section to the next simply because the order in which the linker emits
the sections happens to be what we expect.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 arch/arm/boot/compressed/head.S        | 6 +++---
 arch/arm/boot/compressed/vmlinux.lds.S | 4 +++-
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
index 55c227077207..e3c43fdb6371 100644
--- a/arch/arm/boot/compressed/head.S
+++ b/arch/arm/boot/compressed/head.S
@@ -176,6 +176,8 @@ start:
 		.word	_magic_table	@ additional data table
 
 		__EFI_HEADER
+
+		.text
 1:
  ARM_BE8(	setend	be		)	@ go BE8 if compiled for BE8
  AR_CLASS(	mrs	r9, cpsr	)
@@ -209,10 +211,8 @@ not_angel:
 
 		/*
 		 * some architecture specific code can be inserted
-		 * by the linker here, but it should preserve r7, r8, and r9.
+		 * here, but it should preserve r7, r8, and r9.
 		 */
-
-		.text
 #ifdef CONFIG_PXA_SHARPSL_DETECT_MACH_ID
 		bl	__SharpSL_start
 #endif
diff --git a/arch/arm/boot/compressed/vmlinux.lds.S b/arch/arm/boot/compressed/vmlinux.lds.S
index 2b963d8e76dd..ad9921490034 100644
--- a/arch/arm/boot/compressed/vmlinux.lds.S
+++ b/arch/arm/boot/compressed/vmlinux.lds.S
@@ -34,9 +34,11 @@ SECTIONS
   . = TEXT_START;
   _text = .;
 
-  .text : {
+  .start : {
     _start = .;
     *(.start)
+  }
+  .text : {
     *(.text)
     *(.text.*)
     *(.fixup)
-- 
2.19.1

[PATCH 6/6] ARM: efi: add PE/COFF debug table to EFI header

[Ard Biesheuvel]

This updates the PE/COFF header to emit the absolute path to the
decompressor vmlinux ELF file into a so-called NB10 Codeview entry.

On DEBUG builds of EDK2 for ARM, this will result in output to be
printed as follows

  add-symbol-file /home/ard/linux-build-arm/arch/arm/boot/compressed/vmlinux 0x43889000

which can be consumed by GDB directly, and load the decompressor
ELF symbols at the correct offset in the UEFI address space. With
both the decompressor and the firmware's ELF symbols loaded, we can
do single step debugging of calls made from the EFI stub into the
firmware and back, which is *really* helpful when debugging the
handover from UEFI to the decompressor.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 arch/arm/boot/compressed/Makefile     |  4 ++
 arch/arm/boot/compressed/efi-header.S | 47 ++++++++++++++++++++
 2 files changed, 51 insertions(+)

diff --git a/arch/arm/boot/compressed/Makefile b/arch/arm/boot/compressed/Makefile
index d1862621556f..b3c7aff96b92 100644
--- a/arch/arm/boot/compressed/Makefile
+++ b/arch/arm/boot/compressed/Makefile
@@ -194,3 +194,7 @@ AFLAGS_hyp-stub.o := -Wa,-march=armv7-a
 
 $(obj)/hyp-stub.S: $(srctree)/arch/$(SRCARCH)/kernel/hyp-stub.S
 	$(call cmd,shipped)
+
+ifeq ($(CONFIG_EFI)$(CONFIG_DEBUG_INFO),yy)
+AFLAGS_head.o += -DVMLINUX_PATH="\"$(realpath $(obj)/vmlinux)\""
+endif
diff --git a/arch/arm/boot/compressed/efi-header.S b/arch/arm/boot/compressed/efi-header.S
index c94a88ae834d..5c29b31e7080 100644
--- a/arch/arm/boot/compressed/efi-header.S
+++ b/arch/arm/boot/compressed/efi-header.S
@@ -98,6 +98,11 @@ extra_header_fields:
 		.quad	0				@ CertificationTable
 		.quad	0				@ BaseRelocationTable
 
+#ifdef CONFIG_DEBUG_INFO
+		.long	efi_debug_table - start		@ DebugTable
+		.long	efi_debug_table_size
+#endif
+
 section_table:
 		.ascii	".text\0\0\0"
 		.long	__pecoff_code_size		@ VirtualSize
@@ -127,6 +132,48 @@ section_table:
 
 		.set	section_count, (. - section_table) / 40
 
+#ifdef CONFIG_DEBUG_INFO
+		/*
+		 * The debug table is referenced via its Relative Virtual
+		 * Address (RVA), which is only defined for those parts of
+		 * the image that are covered by a section declaration. Since
+		 * this header is not covered by any section, the debug table
+		 * must be emitted elsewhere. So stick it in the .rodata
+		 * section instead.
+		 *
+		 * Note that the EFI debug entry itself may legally have a
+		 * zero RVA, which means we can simply put it right after the
+		 * section headers.
+		 */
+		.section	".rodata", #alloc
+
+		.align	2
+efi_debug_table:
+		// EFI_IMAGE_DEBUG_DIRECTORY_ENTRY
+		.long	0				@ Characteristics
+		.long	0				@ TimeDateStamp
+		.short	0				@ MajorVersion
+		.short	0				@ MinorVersion
+		.long	IMAGE_DEBUG_TYPE_CODEVIEW	@ Type
+		.long	efi_debug_entry_size		@ SizeOfData
+		.long	0				@ RVA
+		.long	efi_debug_entry - start		@ FileOffset
+
+		.set	efi_debug_table_size, . - efi_debug_table
+		.previous
+
+efi_debug_entry:
+		// EFI_IMAGE_DEBUG_CODEVIEW_NB10_ENTRY
+		.ascii	"NB10"				@ Signature
+		.long	0				@ Unknown
+		.long	0				@ Unknown2
+		.long	0				@ Unknown3
+
+		.asciz	VMLINUX_PATH
+
+		.set	efi_debug_entry_size, . - efi_debug_entry
+#endif
+
 		.align	12
 __efi_start:
 #endif
-- 
2.19.1

[PATCH 0/6] ARM: compressed: clean up section layout and enable EFI debugging

[Russell King - ARM Linux]

On Mon, Nov 05, 2018 at 07:44:32PM +0100, Ard Biesheuvel wrote:
> Currently, the decompressor section layout is somewhat unusual: head.S
> puts code into a .start and a .text section, which are expected to be
> emitted back to back, unless other objects are included that define a
> .start section as well, in which case execution is expected to proceed 
> linearly from head.S's section .start section into the other .start
> section and then onward into head.S's .text section.
> 
> This relies on the input files to be consumed by the linker in the same
> order they were specified on the command line, but this is not actually
> guaranteed by the linker, so it is better to make the inclusion of this
> code specific by using function calls.

I think you're wrong about that - as I've pointed out, this behaviour
is relied upon quite heavily when linking programs using GCC, so while
it may not be explicitly specified, the fact that GCC relies upon it
when creating shared libraries (using crti.o before any object files,
and crtn.o at the end) means that it's pretty much guaranteed.

What is less guaranteed is proceeding between two sections, but these
separate sections in object files are combined into one .text section
in the output file, and so we know that code in .start will be present
_before_ any code in .text.

Put the two things together, and the code from head.S will be listed
first, and therefore will be output first in the .start and .text
sections.  Other object files will have code placed into the output
.text section accordingly.

So really I see no point to your patch 1 to 5, other than change for
change's sake.

Can you _prove_ that there is a problem here - if you can, it's likely
that such a problem also affects gcc -shared too.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

[PATCH 0/6] ARM: compressed: clean up section layout and enable EFI debugging

[Ard Biesheuvel]

On 5 November 2018 at 20:09, Russell King - ARM Linux
<linux@armlinux.org.uk> wrote:
> On Mon, Nov 05, 2018 at 07:44:32PM +0100, Ard Biesheuvel wrote:
>> Currently, the decompressor section layout is somewhat unusual: head.S
>> puts code into a .start and a .text section, which are expected to be
>> emitted back to back, unless other objects are included that define a
>> .start section as well, in which case execution is expected to proceed
>> linearly from head.S's section .start section into the other .start
>> section and then onward into head.S's .text section.
>>
>> This relies on the input files to be consumed by the linker in the same
>> order they were specified on the command line, but this is not actually
>> guaranteed by the linker, so it is better to make the inclusion of this
>> code specific by using function calls.
>
> I think you're wrong about that - as I've pointed out, this behaviour
> is relied upon quite heavily when linking programs using GCC, so while
> it may not be explicitly specified, the fact that GCC relies upon it
> when creating shared libraries (using crti.o before any object files,
> and crtn.o at the end) means that it's pretty much guaranteed.
>
> What is less guaranteed is proceeding between two sections, but these
> separate sections in object files are combined into one .text section
> in the output file, and so we know that code in .start will be present
> _before_ any code in .text.
>
> Put the two things together, and the code from head.S will be listed
> first, and therefore will be output first in the .start and .text
> sections.  Other object files will have code placed into the output
> .text section accordingly.
>
> So really I see no point to your patch 1 to 5, other than change for
> change's sake.
>
> Can you _prove_ that there is a problem here - if you can, it's likely
> that such a problem also affects gcc -shared too.
>

As I replied in the other patch, GCC's builtin linker script uses
SORT_NONE(), and probably does so for a reason.

[PATCH 0/6] ARM: compressed: clean up section layout and enable EFI debugging

[Russell King - ARM Linux]

On Mon, Nov 05, 2018 at 08:10:48PM +0100, Ard Biesheuvel wrote:
> On 5 November 2018 at 20:09, Russell King - ARM Linux
> <linux@armlinux.org.uk> wrote:
> > On Mon, Nov 05, 2018 at 07:44:32PM +0100, Ard Biesheuvel wrote:
> >> Currently, the decompressor section layout is somewhat unusual: head.S
> >> puts code into a .start and a .text section, which are expected to be
> >> emitted back to back, unless other objects are included that define a
> >> .start section as well, in which case execution is expected to proceed
> >> linearly from head.S's section .start section into the other .start
> >> section and then onward into head.S's .text section.
> >>
> >> This relies on the input files to be consumed by the linker in the same
> >> order they were specified on the command line, but this is not actually
> >> guaranteed by the linker, so it is better to make the inclusion of this
> >> code specific by using function calls.
> >
> > I think you're wrong about that - as I've pointed out, this behaviour
> > is relied upon quite heavily when linking programs using GCC, so while
> > it may not be explicitly specified, the fact that GCC relies upon it
> > when creating shared libraries (using crti.o before any object files,
> > and crtn.o at the end) means that it's pretty much guaranteed.
> >
> > What is less guaranteed is proceeding between two sections, but these
> > separate sections in object files are combined into one .text section
> > in the output file, and so we know that code in .start will be present
> > _before_ any code in .text.
> >
> > Put the two things together, and the code from head.S will be listed
> > first, and therefore will be output first in the .start and .text
> > sections.  Other object files will have code placed into the output
> > .text section accordingly.
> >
> > So really I see no point to your patch 1 to 5, other than change for
> > change's sake.
> >
> > Can you _prove_ that there is a problem here - if you can, it's likely
> > that such a problem also affects gcc -shared too.
> >
> 
> As I replied in the other patch, GCC's builtin linker script uses
> SORT_NONE(), and probably does so for a reason.

To disregard any command line section sorting option that may be
supplied by the user.  That is not the case here.  Please read the
LD manual, specifically the "Input Section Wildcards" section.

Thanks.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

[PATCH 0/6] ARM: compressed: clean up section layout and enable EFI debugging

[Ard Biesheuvel]

On 5 November 2018 at 20:14, Russell King - ARM Linux
<linux@armlinux.org.uk> wrote:
> On Mon, Nov 05, 2018 at 08:10:48PM +0100, Ard Biesheuvel wrote:
>> On 5 November 2018 at 20:09, Russell King - ARM Linux
>> <linux@armlinux.org.uk> wrote:
>> > On Mon, Nov 05, 2018 at 07:44:32PM +0100, Ard Biesheuvel wrote:
>> >> Currently, the decompressor section layout is somewhat unusual: head.S
>> >> puts code into a .start and a .text section, which are expected to be
>> >> emitted back to back, unless other objects are included that define a
>> >> .start section as well, in which case execution is expected to proceed
>> >> linearly from head.S's section .start section into the other .start
>> >> section and then onward into head.S's .text section.
>> >>
>> >> This relies on the input files to be consumed by the linker in the same
>> >> order they were specified on the command line, but this is not actually
>> >> guaranteed by the linker, so it is better to make the inclusion of this
>> >> code specific by using function calls.
>> >
>> > I think you're wrong about that - as I've pointed out, this behaviour
>> > is relied upon quite heavily when linking programs using GCC, so while
>> > it may not be explicitly specified, the fact that GCC relies upon it
>> > when creating shared libraries (using crti.o before any object files,
>> > and crtn.o at the end) means that it's pretty much guaranteed.
>> >
>> > What is less guaranteed is proceeding between two sections, but these
>> > separate sections in object files are combined into one .text section
>> > in the output file, and so we know that code in .start will be present
>> > _before_ any code in .text.
>> >
>> > Put the two things together, and the code from head.S will be listed
>> > first, and therefore will be output first in the .start and .text
>> > sections.  Other object files will have code placed into the output
>> > .text section accordingly.
>> >
>> > So really I see no point to your patch 1 to 5, other than change for
>> > change's sake.
>> >
>> > Can you _prove_ that there is a problem here - if you can, it's likely
>> > that such a problem also affects gcc -shared too.
>> >
>>
>> As I replied in the other patch, GCC's builtin linker script uses
>> SORT_NONE(), and probably does so for a reason.
>
> To disregard any command line section sorting option that may be
> supplied by the user.  That is not the case here.  Please read the
> LD manual, specifically the "Input Section Wildcards" section.
>

Ok, fair enough. So it is indeed guaranteed by the linker, but it is
still unusual, and needlessly fragile IMHO (e.g., if anyone ever
enables sorting by alignment for code size reasons, this could break
subtly)

In any case, the changes aren't entirely pointless: they are also a
prerequisite for tweaking the ELF output section layout to match the
PE/COFF section layout (#5), which in turn is a prerequisite for patch
#6. Do you have another suggestion on how to approach that?