[U-Boot] Build system: Don't check for CONFIG_SYS_TEXT_BASE being set

[Tom Rini <trini@konsulko.com>]

On Mon, Feb 12, 2018 at 01:27:30PM +0000, Alexey Brodkin wrote:
> 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

Thanks for explaining what's going on with ARC.

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

I'm largely ok with the above, but:
- You need to exclude CONFIG_NIOS2 in the above as well, or convert the
  usage of CONFIG_SYS_MONITOR_BASE to CONFIG_SYS_TEXT_BASE (Thomas?)
- For Xtensa (Max?), CONFIG_SYS_TEXT_ADDR needs to be renamed to
  CONFIG_SYS_TEXT_BASE there

Thanks!

-- 
Tom