All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alexey Brodkin <Alexey.Brodkin@synopsys.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] Build system: Don't check for CONFIG_SYS_TEXT_BASE being set
Date: Mon, 12 Feb 2018 13:27:30 +0000	[thread overview]
Message-ID: <1518442049.4974.50.camel@synopsys.com> (raw)
In-Reply-To: <20180211204733.GK3061@bill-the-cat>

Hi Tom, Simon,

On Sun, 2018-02-11 at 15:47 -0500, Tom Rini wrote:
> On Tue, Jan 30, 2018 at 06:23:13PM +0300, Alexey Brodkin wrote:
> 
> > CONFIG_SYS_TEXT_BASE must be set anyways and then it is used in many
> > places in the same Makefile without any checks so there's no point in
> > keeping this check araound just in one place.
> > 
> > Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com>
> > Cc: Tom Rini <trini@konsulko.com>
> > Acked-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> > ---
> >  Makefile | 2 --
> >  1 file changed, 2 deletions(-)
> > 
> > diff --git a/Makefile b/Makefile
> > index ab3453dcebdc..6f15612b4d07 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -820,9 +820,7 @@ LDFLAGS_u-boot += $(LDFLAGS_FINAL)
> >  # Avoid 'Not enough room for program headers' error on binutils 2.28 onwards.
> >  LDFLAGS_u-boot += $(call ld-option, --no-dynamic-linker)
> >  
> > -ifneq ($(CONFIG_SYS_TEXT_BASE),)
> >  LDFLAGS_u-boot += -Ttext $(CONFIG_SYS_TEXT_BASE)
> > -endif
> 
> This then causes xtensa to fail to build as it does not set
> CONFIG_SYS_TEXT_BASE.

And that also obviously breaks "efi-x86" target as well because CONFIG_SYS_TEXT_BASE
seems to not be defined for EFI and then LD gets a string like "-Ttext  -o u-boot"
where CONFIG_SYS_TEXT_BASE is supposed to be used as some value.

Frankly I'm not sure what to do with that - probably EFI is just a very special case...

But FWIW I'm not very happy with mandatory "-ttext $(CONFIG_SYS_TEXT_BASE)"
in case of ARC and here's why:
 1. In case of ARCv2 ISA interrupt vector table is not just another code section with jump
    instructions to corresponding handlers but instead it's just a set of addresses pointing
    to corresponding handlers.

    I.e. that's a traditional IVT (interrupt vector table) which among other architectures is
    used on older ARCompact (AKA ARCv1 ISA):
    ------------------->8-----------------
    .ivt
        jump 0x1000_0000
        jump 0x1000_1000
        ...
    ------------------->8-----------------

    And that's what we have for ARCv2:
    ------------------->8-----------------
    .ivt
        0x1000_0000
        0x1000_1000
        ...
    ------------------->8-----------------

 2. Now one may think there's no big difference in 2 cases above except content:
    it is either encoded instructions or literals. But that really matters because
    in case of ARC (regardless ISA version) instructions are encoded in __middle-endian__
    format while literals are normal little-endians.

    Consider the following example:
    ------------------->8-----------------
    .section .ivt
    .word	0x10000000
    .word	0x10001000
    .align	256
    .section .text
    .word	0x10000000
    .word	0x10001000
    ------------------->8-----------------

    That will be compiled into this:
    ------------------->8-----------------
    Disassembly of section .text:
    00000000 <.text>:
       0:	0000 1000           	b	131072	;0x20000
       4:	1000 1000           	ld	r0,[r8]

    Disassembly of section .ivt:
    00000000 <.ivt>:
       0:	00 00 00 10             	.word	0x10000000
       4:	00 10 00 10             	.word	0x10001000
    ------------------->8-----------------

    Note how bytes are swapped in .text section.

In the end that basically means we cannot put IVT in the beginning of .text section how
it is usually done. We need to keep .ivt and .text sections as separate substances.

And so far what we used to do we put .ivt section after .text.
It was done as a preparation for ARCv2 port introduction here:
http://git.denx.de/?p=u-boot.git;a=commit;h=20a58ac0d8e09d0bf1a74c6b68fea22784512b51

Now here comes another challenge - so far U-Boot was not the first piece of software
that was executed by CPU, but what's even more important U-Boot was started by boot-ROM
via jump to U-Boot's entry point (which happened to be it's start of .text section).

But now we're going to run U-Boot as the first ever thing on power on and for that we'll
put U-Boot in ROM such that CPU starts execution from "reset" vector and that will be
U-Boot.

In other words in hardware location of IVT is hard-coded as 0x0000_0000 and that's where
we'll put U-Boot. With explanation above I think it's quite clear that we cannot have .text
section there at 0x0000_0000 because what's going to happen is CPU will fetch the first "data"
word from ROM and will attempt to junp at address it "sees" there. Obviously that won't be
a correct address and so CPU will just jump into some unexpected location.

Which basically means we need to put .ivt section in the very beginning of the image and
have .text section at say 0x0000_1000. I.e. now we'll need to keep in mind at least 2 things:
 1) CONFIG_SYS_TEXT_BASE is not the base-address of the uboot.img
 2) .ivt's base-address is something just a couple of kB below CONFIG_SYS_TEXT_BASE

So if "-Ttext CONFIG_SYS_TEXT_BASE" is not used for each and every board I may use
CONFIG_SYS_TEXT_BASE directly in linker just as we have it now, see
http://git.denx.de/?p=u-boot.git;a=blob;f=arch/arc/cpu/u-boot.lds#l14
and then there might be any section like .ivt, .text, .myfunkysection etc.

In fact in the Linux kernel "-Ttext XXX" is not used for everybody - some
arches like MIPS and PPC indeed set it but others do other things.

The simplest thing might be is to add another #ifdef for ARC and X86 which both
use CONFIG_SYS_TEXT_BASE directly in their linker scripts like that:
------------------->8-----------------
--- a/Makefile
+++ b/Makefile
@@ -820,9 +820,11 @@ LDFLAGS_u-boot += $(LDFLAGS_FINAL)
 # Avoid 'Not enough room for program headers' error on binutils 2.28 onwards.
 LDFLAGS_u-boot += $(call ld-option, --no-dynamic-linker)
 
+ifeq($(CONFIG_ARC)$(CONFIG_X86),)
 LDFLAGS_u-boot += -Ttext $(CONFIG_SYS_TEXT_BASE)
+endif
 
 # Normally we fill empty space with 0xff
 quiet_cmd_objcopy = OBJCOPY $@
------------------->8-----------------

Any thoughts?

-Alexey

  reply	other threads:[~2018-02-12 13:27 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-30 15:23 [U-Boot] [PATCH] Build system: Don't check for CONFIG_SYS_TEXT_BASE being set Alexey Brodkin
2018-01-30 16:23 ` Masahiro Yamada
2018-01-30 16:28   ` Alexey Brodkin
2018-01-31 15:18     ` Masahiro Yamada
2018-01-31 15:21       ` Alexey Brodkin
2018-02-02 15:01         ` Masahiro Yamada
2018-02-11 20:47 ` [U-Boot] " Tom Rini
2018-02-12 13:27   ` Alexey Brodkin [this message]
2018-02-12 14:23     ` Tom Rini
2018-02-12 14:46       ` Marek Vasut
2018-02-12 16:21       ` Alexey Brodkin
2018-02-12 16:29         ` Tom Rini
2018-02-12 20:48       ` Max Filippov
2018-02-12 21:03         ` Tom Rini
2018-02-12 21:48           ` Max Filippov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1518442049.4974.50.camel@synopsys.com \
    --to=alexey.brodkin@synopsys.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.