On 02/12/2018 03:23 PM, Tom Rini wrote: > 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 <abrod...@synopsys.com> >>>> Cc: Tom Rini <tr...@konsulko.com> >>>> Acked-by: Masahiro Yamada <yamada.masah...@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?)
I _think_ converting nios2 from monitor_base to text_base should be easy enough, U-Boot is linked to monitor_base via the linker script anyway, so just change that to text_base . > - For Xtensa (Max?), CONFIG_SYS_TEXT_ADDR needs to be renamed to > CONFIG_SYS_TEXT_BASE there > > Thanks! > -- Best regards, Marek Vasut _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
