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

Reply via email to