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

[Max Filippov]

On Mon, Feb 12, 2018 at 1:03 PM, Tom Rini  wrote:
>> On Mon, Feb 12, 2018 at 6:23 AM, Tom Rini  wrote:
>> > I'm largely ok with the above, but:
>> > - For Xtensa (Max?), CONFIG_SYS_TEXT_ADDR needs to be renamed to
>> >   CONFIG_SYS_TEXT_BASE there
>>
>> For xtensa that address is defined as an expression, like
>> (CONFIG_SYS_MEMORY_TOP - CONFIG_SYS_MONITOR_LEN),
>> and for a single board it may vary with the CPU core on that board.
>
> OK.  But are those also really dynamic values?  SYS_MEMORY_TOP,
> SYS_MONITOR_LEN and SYS_TEXT_ADDR need to be converted to Kconfig, or
> removed from CONFIG namespace, whatever makes the most sense.  If the

At least two of the three -- yes, in a sense that they cannot be calculated
without the core-specific configuration overlay.
I'll move SYS_MONITOR_LEN to the Kconfig and move SYS_MEMORY_TOP
and SYS_TEXT_ADDR from the CONFIG namespace.

> notion of CONFIG_SYS_TEXT_BASE is just pointless to Xtensa, we add them
> to the ifneq(...,) test for the change Alexey is doing.  Thanks!

Yes, please.

-- 
Thanks.
-- Max
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

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

[Tom Rini]

On Mon, Feb 12, 2018 at 12:48:02PM -0800, Max Filippov wrote:
> Hello,
> 
> On Tue, Jan 30, 2018 at 7:23 AM, 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
> 
> Why? xtensa doesn't use any of it.
> 
> On Mon, Feb 12, 2018 at 6:23 AM, Tom Rini  wrote:
> > I'm largely ok with the above, but:
> > - For Xtensa (Max?), CONFIG_SYS_TEXT_ADDR needs to be renamed to
> >   CONFIG_SYS_TEXT_BASE there
> 
> For xtensa that address is defined as an expression, like
> (CONFIG_SYS_MEMORY_TOP - CONFIG_SYS_MONITOR_LEN),
> and for a single board it may vary with the CPU core on that board.
> If I just do this replacement then linking fails because of this symbolic
> definition:
> 
>   xtensa-dc233c-elf-ld.bfd--gc-sections -Bstatic
> --no-dynamic-linker -Ttext "(CONFIG_SYS_MEMORY_TOP -
> CONFIG_SYS_MONITOR_LEN)"
>   ...
>   xtensa-dc233c-elf-ld.bfd: invalid hex number `(CONFIG_SYS_MEMORY_TOP
> - CONFIG_SYS_MONITOR_LEN)'

OK.  But are those also really dynamic values?  SYS_MEMORY_TOP,
SYS_MONITOR_LEN and SYS_TEXT_ADDR need to be converted to Kconfig, or
removed from CONFIG namespace, whatever makes the most sense.  If the
notion of CONFIG_SYS_TEXT_BASE is just pointless to Xtensa, we add them
to the ifneq(...,) test for the change Alexey is doing.  Thanks!

-- 
Tom


signature.asc
Description: PGP signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

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

[Max Filippov]

Hello,

On Tue, Jan 30, 2018 at 7:23 AM, 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

Why? xtensa doesn't use any of it.

On Mon, Feb 12, 2018 at 6:23 AM, Tom Rini  wrote:
> I'm largely ok with the above, but:
> - For Xtensa (Max?), CONFIG_SYS_TEXT_ADDR needs to be renamed to
>   CONFIG_SYS_TEXT_BASE there

For xtensa that address is defined as an expression, like
(CONFIG_SYS_MEMORY_TOP - CONFIG_SYS_MONITOR_LEN),
and for a single board it may vary with the CPU core on that board.
If I just do this replacement then linking fails because of this symbolic
definition:

  xtensa-dc233c-elf-ld.bfd--gc-sections -Bstatic
--no-dynamic-linker -Ttext "(CONFIG_SYS_MEMORY_TOP -
CONFIG_SYS_MONITOR_LEN)"
  ...
  xtensa-dc233c-elf-ld.bfd: invalid hex number `(CONFIG_SYS_MEMORY_TOP
- CONFIG_SYS_MONITOR_LEN)'

-- 
Thanks.
-- Max
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

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

[Tom Rini]

On Mon, Feb 12, 2018 at 04:21:09PM +, Alexey Brodkin wrote:
> On Mon, 2018-02-12 at 09:23 -0500, Tom Rini wrote:
> > On Mon, Feb 12, 2018 at 01:27:30PM +, 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 
> > > > > Cc: Tom Rini 
> > > > > Acked-by: Masahiro Yamada 
> > > > > ---
> > > > >  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.
> 
> [snip]
> 
> > > 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
> > > https://urldefense.proofpoint.com/v2/url?u=http-3A__git.denx.de_-3Fp-3Du-2Dboot.git-3Ba-3Dblob-3Bf-3Darch_arc_cpu_u-2Dboot.lds-23l14=DwIBAg=DP
> > > L6_X_6JkXFx7AXWqB0tg=lqdeeSSEes0GFDDl656eViXO7breS55ytWkhpk5R81I=3oKsgMcKriZJ81auV4XDwr4ovIElxycDMpRlTqS7nhc=Yq0YW-
> > > MQvofxwQ961IRHzIS5xWYtHHzxMp_Fwn91h5s=
> > > 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?)
> 
> Looks like NIOS2 builds are not yet supported in TravisCI which is
> quite unfortunate... are there any instructions on how to build for Nios2
> so that I may test my changes before posting them?

So, part of the issue is that we need newer kernel.org toolchains
(https://www.kernel.org/pub/tools/crosstool/files/bin/x86_64/7.3.0/) in
order for buildman to be able to grab a nios2 toolchain, and that in
turn has its own issues.  You can however grab one of those (or the
6.4.0) version manually, extract into ~/.buildman/ and then just let
buildman build the arch.

-- 
Tom


signature.asc
Description: PGP signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

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

[Alexey Brodkin]

On Mon, 2018-02-12 at 09:23 -0500, Tom Rini wrote:
> On Mon, Feb 12, 2018 at 01:27:30PM +, 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 
> > > > Cc: Tom Rini 
> > > > Acked-by: Masahiro Yamada 
> > > > ---
> > > >  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.

[snip]

> > 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
> > https://urldefense.proofpoint.com/v2/url?u=http-3A__git.denx.de_-3Fp-3Du-2Dboot.git-3Ba-3Dblob-3Bf-3Darch_arc_cpu_u-2Dboot.lds-23l14=DwIBAg=DP
> > L6_X_6JkXFx7AXWqB0tg=lqdeeSSEes0GFDDl656eViXO7breS55ytWkhpk5R81I=3oKsgMcKriZJ81auV4XDwr4ovIElxycDMpRlTqS7nhc=Yq0YW-
> > MQvofxwQ961IRHzIS5xWYtHHzxMp_Fwn91h5s=
> > 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?)

Looks like NIOS2 builds are not yet supported in TravisCI which is
quite unfortunate... are there any instructions on how to build for Nios2
so that I may test my changes before posting them?

-Alexey
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

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

[Marek Vasut]

On 02/12/2018 03:23 PM, Tom Rini wrote:
> On Mon, Feb 12, 2018 at 01:27:30PM +, 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 
 Cc: Tom Rini 
 Acked-by: Masahiro Yamada 
 ---
  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_
>> jump 0x1000_1000
>> ...
>> --->8-
>>
>> And that's what we have for ARCv2:
>> --->8-
>> .ivt
>> 0x1000_
>> 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
>> .word0x1000
>> .word0x10001000
>> .align   256
>> .section .text
>> .word0x1000
>> .word0x10001000
>> --->8-
>>
>> That will be compiled into this:
>> --->8-
>> Disassembly of section .text:
>>  <.text>:
>>0: 1000   b   131072  ;0x2
>>4:1000 1000   ld  r0,[r8]
>>
>> Disassembly of section .ivt:
>>  <.ivt>:
>>0:00 00 00 10 .word   0x1000
>>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 0x_ 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 0x_ 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 

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

[Tom Rini]

On Mon, Feb 12, 2018 at 01:27:30PM +, 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 
> > > Cc: Tom Rini 
> > > Acked-by: Masahiro Yamada 
> > > ---
> > >  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_
> jump 0x1000_1000
> ...
> --->8-
> 
> And that's what we have for ARCv2:
> --->8-
> .ivt
> 0x1000_
> 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 0x1000
> .word 0x10001000
> .align256
> .section .text
> .word 0x1000
> .word 0x10001000
> --->8-
> 
> That will be compiled into this:
> --->8-
> Disassembly of section .text:
>  <.text>:
>0:  1000   b   131072  ;0x2
>4: 1000 1000   ld  r0,[r8]
> 
> Disassembly of section .ivt:
>  <.ivt>:
>0: 00 00 00 10 .word   0x1000
>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 0x_ 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 0x_ 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 

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

[Alexey Brodkin]

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 
> > Cc: Tom Rini 
> > Acked-by: Masahiro Yamada 
> > ---
> >  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_
jump 0x1000_1000
...
--->8-

And that's what we have for ARCv2:
--->8-
.ivt
0x1000_
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   0x1000
.word   0x10001000
.align  256
.section .text
.word   0x1000
.word   0x10001000
--->8-

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

Disassembly of section .ivt:
 <.ivt>:
   0:   00 00 00 10 .word   0x1000
   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 0x_ 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 0x_ 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 0x_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 

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

[Tom Rini]

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 
> Cc: Tom Rini 
> Acked-by: Masahiro Yamada 
> ---
>  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.

-- 
Tom
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot