[U-Boot] [PATCH 2/2] MIPS: Stop building position independent code
Paul Burton
paul.burton at imgtec.com
Fri Jun 16 19:40:13 UTC 2017
Hi Daniel,
On Friday, 16 June 2017 06:49:55 PDT Daniel Schwierzeck wrote:
> Hi Paul,
>
> Am 16.06.2017 um 02:05 schrieb Paul Burton:
> > U-Boot has up until now built with -fpic for the MIPS architecture,
> > producing position independent code which uses indirection through a
> > global offset table, making relocation fairly straightforward as it
> > simply involves patching up GOT entries.
> >
> > Using -fpic does however have some downsides. The biggest of these is
> > that generated code is bloated in various ways. For example, function
> > calls are indirected through the GOT & the t9 register:
> >
> > 8f998064 lw t9,-32668(gp)
> > 0320f809 jalr t9
> >
> > Without -fpic the call is simply:
> >
> > 0f803f01 jal be00fc04 <puts>
> >
> > This is more compact & faster (due to the lack of the load & the
> > dependency the jump has on its result). It is also easier to read &
> > debug because the disassembly shows what function is being called,
> > rather than just an offset from gp which would then have to be looked up
> > in the ELF to discover the target function.
> >
> > Another disadvantage of -fpic is that each function begins with a
> > sequence to calculate the value of the gp register, for example:
> >
> > 3c1c0004 lui gp,0x4
> > 279c3384 addiu gp,gp,13188
> > 0399e021 addu gp,gp,t9
> >
> > Without using -fpic this sequence no longer appears at the start of each
> > function, reducing code size considerably.
> >
> > This patch switches U-Boot from building with -fpic to building with
> > -fno-pic, in order to gain the benefits described above. The cost of
> > this is an extra step during the build process to extract relocation
> > data from the ELF & write it into a new .rel section in a compact
> > format, plus the added complexity of dealing with multiple types of
> > relocation rather than the single type that applied to the GOT. The
> > benefit is smaller, cleaner, more debuggable code. The relocate_code()
> > function is reimplemented in C to handle the new relocation scheme,
> > which also makes it easier to read & debug.
> >
> > Taking maltael_defconfig as an example the size of u-boot.bin built
> > using the Codescape MIPS 2016.05-06 toolchain (gcc 4.9.2, binutils
> > 2.24.90) shrinks from 254KiB to 224KiB.
> >
> > Signed-off-by: Paul Burton <paul.burton at imgtec.com>
> > Cc: Daniel Schwierzeck <daniel.schwierzeck at gmail.com>
> > Cc: u-boot at lists.denx.de
>
> nice work, thanks. Nits below
Thanks for reviewing. Hope to get more submitted soon..!
> > ---
> >
> > arch/mips/Makefile.postlink | 23 +++
> > arch/mips/config.mk | 19 +-
> > arch/mips/cpu/start.S | 130 -------------
> > arch/mips/cpu/u-boot.lds | 41 +---
> > arch/mips/include/asm/relocs.h | 24 +++
> > arch/mips/lib/Makefile | 1 +
> > arch/mips/lib/reloc.c | 164 ++++++++++++++++
> > common/board_f.c | 2 +-
> > tools/.gitignore | 1 +
> > tools/Makefile | 2 +
> > tools/mips-relocs.c | 426
> > +++++++++++++++++++++++++++++++++++++++++ 11 files changed, 656
> > insertions(+), 177 deletions(-)
> > create mode 100644 arch/mips/Makefile.postlink
> > create mode 100644 arch/mips/include/asm/relocs.h
> > create mode 100644 arch/mips/lib/reloc.c
> > create mode 100644 tools/mips-relocs.c
> >
> > diff --git a/arch/mips/Makefile.postlink b/arch/mips/Makefile.postlink
> > new file mode 100644
> > index 0000000000..d6fbc0d404
> > --- /dev/null
> > +++ b/arch/mips/Makefile.postlink
> > @@ -0,0 +1,23 @@
> > +#
> > +# Copyright (c) 2017 Imagination Technologies Ltd.
> > +#
> > +# SPDX-License-Identifier: GPL-2.0+
> > +#
> > +
> > +PHONY := __archpost
> > +__archpost:
> > +
> > +-include include/config/auto.conf
> > +include scripts/Kbuild.include
> > +
> > +CMD_RELOCS = tools/mips-relocs
> > +quiet_cmd_relocs = RELOCS $@
> > + cmd_relocs = $(CMD_RELOCS) $@ .$@.relocs
>
> what's the purpose of .$@.relocs? The mips-relocs tool only has one
> arguments and the kernel Makefile doesn't have this
Well spotted. At one point I experimented with having the mips-relocs tool
output a separate file then inserting it into the ELF using objcopy, but that
didn't work out so well. Will remove for v2.
> > +
> > +u-boot: FORCE
> > + @true
> > + $(call if_changed,relocs)
> > +
> > +.PHONY: FORCE
> > +
> > +FORCE:
> > diff --git a/arch/mips/config.mk b/arch/mips/config.mk
> > index 2c72c1553d..56d150171e 100644
> > --- a/arch/mips/config.mk
> > +++ b/arch/mips/config.mk
> > @@ -56,25 +56,16 @@ PLATFORM_ELFFLAGS += -B mips $(OBJCOPYFLAGS)
> >
> > # LDFLAGS_vmlinux += -G 0 -static -n -nostdlib
> > # MODFLAGS += -mlong-calls
> > #
> >
> > -# On the other hand, we want PIC in the U-Boot code to relocate it from
> > ROM -# to RAM. $28 is always used as gp.
> > -#
> > -ifdef CONFIG_SPL_BUILD
> > -PF_ABICALLS := -mno-abicalls
> > -PF_PIC := -fno-pic
> > -PF_PIE :=
> > -else
> > -PF_ABICALLS := -mabicalls
> > -PF_PIC := -fpic
> > -PF_PIE := -pie
> > -PF_OBJCOPY := -j .got -j .rel.dyn -j .padding
> > +ifndef CONFIG_SPL_BUILD
> > +PF_OBJCOPY := -j .got -j .rel -j .padding
> >
> > PF_OBJCOPY += -j .dtb.init.rodata
> >
> > +LDFLAGS_FINAL += --emit-relocs
> >
> > endif
>
> I think we could now drop the extra PF_OBJCOPY variable and directly
> assign the values to OBJCOPYFLAGS
Sure, will do in v2.
> > -PLATFORM_CPPFLAGS += -G 0 $(PF_ABICALLS) $(PF_PIC)
> > +PLATFORM_CPPFLAGS += -G 0 -mno-abicalls -fno-pic
> >
> > PLATFORM_CPPFLAGS += -msoft-float
> > PLATFORM_LDFLAGS += -G 0 -static -n -nostdlib
> > PLATFORM_RELFLAGS += -ffunction-sections -fdata-sections
> >
> > -LDFLAGS_FINAL += --gc-sections $(PF_PIE)
> > +LDFLAGS_FINAL += --gc-sections
> >
> > OBJCOPYFLAGS += -j .text -j .rodata -j .data -j .u_boot_list
> > OBJCOPYFLAGS += $(PF_OBJCOPY)
> >
> > diff --git a/arch/mips/lib/reloc.c b/arch/mips/lib/reloc.c
> > new file mode 100644
> > index 0000000000..b7ae56df5a
> > --- /dev/null
> > +++ b/arch/mips/lib/reloc.c
> > @@ -0,0 +1,164 @@
> > +/*
> > + * MIPS Relocation
> > + *
> > + * Copyright (c) 2017 Imagination Technologies Ltd.
> > + *
> > + * SPDX-License-Identifier: GPL-2.0+
> > + */
> > +
> > +#include <common.h>
> > +#include <asm/relocs.h>
> > +#include <asm/sections.h>
> > +
> > +/*
> > + * __rel_start: Relocation data generated by the mips-relocs tool
> > + *
> > + * This data, found in the .rel section, is generated by the mips-relocs
> > tool & + * contains a record of all locations in the U-Boot binary that
> > need to be + * fixed up during relocation.
> > + *
> > + * The data is a sequence of unsigned integers, which are of somewhat
> > arbitrary + * size. This is achieved by encoding integers as a sequence
> > of bytes, each of + * which contains 7 bits of data with the most
> > significant bit indicating + * whether any further bytes need to be read.
> > The least significant bits of the + * integer are found in the first byte
> > - ie. it somewhat resembles little + * endian.
> > + *
> > + * Each pair of two integers represents a relocation that must be
> > applied. The + * first integer represents the type of relocation as a
> > standard ELF relocation + * type (ie. R_MIPS_*). The second integer
> > represents the offset at which to + * apply the relocation, relative to
> > the previous relocation or for the first + * relocation the start of the
> > relocated .text section.
> > + *
> > + * The end of the relocation data is indicated when type R_MIPS_NONE (0)
> > is + * read, at which point no further integers should be read. That is,
> > the + * terminating R_MIPS_NONE reloc includes no offset.
> > + */
> > +extern uint8_t __rel_start[];
>
> could you move this to asm/sections.h (or asm-generic/sections.h
> perhaps) to suppress a checkpatch.pl warning and to have linker script
> exports in one place?
OK, will do.
Thanks,
Paul
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170616/a9d5caaa/attachment.sig>
More information about the U-Boot
mailing list