mbox series

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

Message ID 20181105184438.19494-1-ard.biesheuvel@linaro.org (mailing list archive)
Headers show
Series ARM: compressed: clean up section layout and enable EFI debugging | expand

Message

Ard Biesheuvel Nov. 5, 2018, 6:44 p.m. UTC
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

Comments

Russell King (Oracle) Nov. 5, 2018, 7:09 p.m. UTC | #1
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.
Ard Biesheuvel Nov. 5, 2018, 7:10 p.m. UTC | #2
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.
Russell King (Oracle) Nov. 5, 2018, 7:14 p.m. UTC | #3
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.
Ard Biesheuvel Nov. 5, 2018, 7:21 p.m. UTC | #4
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?