All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH 1/2] armv8: apply -mgeneral-regs-only
@ 2017-07-03 13:14 Peng Fan
  2017-07-03 13:14 ` [U-Boot] [PATCH 2/2] arm: config: enforce -fno-pic for SPL and normal U-Boot Peng Fan
  2017-07-04 13:32 ` [U-Boot] [PATCH 1/2] armv8: apply -mgeneral-regs-only Tom Rini
  0 siblings, 2 replies; 18+ messages in thread
From: Peng Fan @ 2017-07-03 13:14 UTC (permalink / raw)
  To: u-boot

Pass -mgeneral-regs-only to GCC, if not compiler may generate
instructions that use SIMD registers.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
Cc: Albert Aribaud <albert.u.boot@aribaud.net>
Cc: Tom Rini <trini@konsulko.com>
---
 arch/arm/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/Makefile b/arch/arm/Makefile
index 3e93fd6..e44e45c 100644
--- a/arch/arm/Makefile
+++ b/arch/arm/Makefile
@@ -18,7 +18,7 @@ arch-$(CONFIG_CPU_ARM1136)	=-march=armv5
 arch-$(CONFIG_CPU_ARM1176)	=-march=armv5t
 arch-$(CONFIG_CPU_V7)		=$(call cc-option, -march=armv7-a, \
 				 $(call cc-option, -march=armv7, -march=armv5))
-arch-$(CONFIG_ARM64)		=-march=armv8-a
+arch-$(CONFIG_ARM64)           =-march=armv8-a -mgeneral-regs-only
 
 # On Tegra systems we must build SPL for the armv4 core on the device
 # but otherwise we can use the value in CONFIG_SYS_ARM_ARCH
-- 
2.6.2

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [U-Boot] [PATCH 2/2] arm: config: enforce -fno-pic for SPL and normal U-Boot
  2017-07-03 13:14 [U-Boot] [PATCH 1/2] armv8: apply -mgeneral-regs-only Peng Fan
@ 2017-07-03 13:14 ` Peng Fan
  2017-07-03 16:16   ` Tom Rini
  2017-07-04 13:32 ` [U-Boot] [PATCH 1/2] armv8: apply -mgeneral-regs-only Tom Rini
  1 sibling, 1 reply; 18+ messages in thread
From: Peng Fan @ 2017-07-03 13:14 UTC (permalink / raw)
  To: u-boot

If not pass -fno-pic to toolchains, some toolchains may generate
.got and .got.plt sections, but when generate binaries, we
did not take .got and .got.plt into consideration, then
SPL or normal U-Boot boot failure because image corrupted.

Need to pass -fno-pic to disable generating .got and .got.plt
sections.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
Cc: Albert Aribaud <albert.u.boot@aribaud.net>
Cc: Tom Rini <trini@konsulko.com>
---
 arch/arm/config.mk | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/config.mk b/arch/arm/config.mk
index 1a77779..66ae403 100644
--- a/arch/arm/config.mk
+++ b/arch/arm/config.mk
@@ -130,9 +130,10 @@ ALL-y += checkarmreloc
 # instruction. Relocation is not supported for that case, so disable
 # such usage by requiring word relocations.
 PLATFORM_CPPFLAGS += $(call cc-option, -mword-relocations)
-PLATFORM_CPPFLAGS += $(call cc-option, -fno-pic)
 endif
 
+PLATFORM_CPPFLAGS += $(call cc-option, -fno-pic)
+
 # limit ourselves to the sections we want in the .bin.
 ifdef CONFIG_ARM64
 OBJCOPYFLAGS += -j .text -j .secure_text -j .secure_data -j .rodata -j .data \
-- 
2.6.2

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [U-Boot] [PATCH 2/2] arm: config: enforce -fno-pic for SPL and normal U-Boot
  2017-07-03 13:14 ` [U-Boot] [PATCH 2/2] arm: config: enforce -fno-pic for SPL and normal U-Boot Peng Fan
@ 2017-07-03 16:16   ` Tom Rini
  2017-07-04  1:09     ` Peng Fan
  0 siblings, 1 reply; 18+ messages in thread
From: Tom Rini @ 2017-07-03 16:16 UTC (permalink / raw)
  To: u-boot

On Mon, Jul 03, 2017 at 09:14:08PM +0800, Peng Fan wrote:

> If not pass -fno-pic to toolchains, some toolchains may generate
> .got and .got.plt sections, but when generate binaries, we
> did not take .got and .got.plt into consideration, then
> SPL or normal U-Boot boot failure because image corrupted.
> 
> Need to pass -fno-pic to disable generating .got and .got.plt
> sections.
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> Cc: Albert Aribaud <albert.u.boot@aribaud.net>
> Cc: Tom Rini <trini@konsulko.com>
> ---
>  arch/arm/config.mk | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/config.mk b/arch/arm/config.mk
> index 1a77779..66ae403 100644
> --- a/arch/arm/config.mk
> +++ b/arch/arm/config.mk
> @@ -130,9 +130,10 @@ ALL-y += checkarmreloc
>  # instruction. Relocation is not supported for that case, so disable
>  # such usage by requiring word relocations.
>  PLATFORM_CPPFLAGS += $(call cc-option, -mword-relocations)
> -PLATFORM_CPPFLAGS += $(call cc-option, -fno-pic)
>  endif
>  
> +PLATFORM_CPPFLAGS += $(call cc-option, -fno-pic)
> +
>  # limit ourselves to the sections we want in the .bin.
>  ifdef CONFIG_ARM64
>  OBJCOPYFLAGS += -j .text -j .secure_text -j .secure_data -j .rodata -j .data \

Something is "up" here and I need you to dig harder and perhaps see if
we're missing something in the linker scripts?  The very next line of
context here is:
                -j .u_boot_list -j .rela.dyn -j .got -j .got.plt

Meaning that we intentionally copy .got / .got.plt into the resulting
binary.  And I see that we took in 397d7d5a1be1 from you back in 2016
saying that we needed this in SPL.  But 5a942a152776 put the got/got.plt
sections (for 32bit ARM) in intentionally as some relocations do need
it.  And in 4b0d506ed3b4 Philipp seems to have seen the same problem you
have, but fixed it with adding got/got.plt to the sections list we copy
in (the above hunk of context).

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170703/c4d28b15/attachment.sig>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [U-Boot] [PATCH 2/2] arm: config: enforce -fno-pic for SPL and normal U-Boot
  2017-07-03 16:16   ` Tom Rini
@ 2017-07-04  1:09     ` Peng Fan
  2017-07-04  2:47       ` Tom Rini
  0 siblings, 1 reply; 18+ messages in thread
From: Peng Fan @ 2017-07-04  1:09 UTC (permalink / raw)
  To: u-boot

Hi Tom,

> -----Original Message-----
> From: Tom Rini [mailto:trini at konsulko.com]
> Sent: Tuesday, July 04, 2017 12:17 AM
> To: Peng Fan <peng.fan@nxp.com>; Simon Glass <sjg@chromium.org>; Philipp
> Tomsich <philipp.tomsich@theobroma-systems.com>
> Cc: albert.u.boot at aribaud.net; u-boot at lists.denx.de
> Subject: Re: [U-Boot] [PATCH 2/2] arm: config: enforce -fno-pic for SPL and
> normal U-Boot
> 
> On Mon, Jul 03, 2017 at 09:14:08PM +0800, Peng Fan wrote:
> 
> > If not pass -fno-pic to toolchains, some toolchains may generate .got
> > and .got.plt sections, but when generate binaries, we did not take
> > .got and .got.plt into consideration, then SPL or normal U-Boot boot
> > failure because image corrupted.
> >
> > Need to pass -fno-pic to disable generating .got and .got.plt
> > sections.
> >
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > Cc: Albert Aribaud <albert.u.boot@aribaud.net>
> > Cc: Tom Rini <trini@konsulko.com>
> > ---
> >  arch/arm/config.mk | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/arm/config.mk b/arch/arm/config.mk index
> > 1a77779..66ae403 100644
> > --- a/arch/arm/config.mk
> > +++ b/arch/arm/config.mk
> > @@ -130,9 +130,10 @@ ALL-y += checkarmreloc  # instruction. Relocation
> > is not supported for that case, so disable  # such usage by requiring
> > word relocations.
> >  PLATFORM_CPPFLAGS += $(call cc-option, -mword-relocations)
> > -PLATFORM_CPPFLAGS += $(call cc-option, -fno-pic)  endif
> >
> > +PLATFORM_CPPFLAGS += $(call cc-option, -fno-pic)
> > +
> >  # limit ourselves to the sections we want in the .bin.
> >  ifdef CONFIG_ARM64
> >  OBJCOPYFLAGS += -j .text -j .secure_text -j .secure_data -j .rodata
> > -j .data \
> 
> Something is "up" here and I need you to dig harder and perhaps see if we're
> missing something in the linker scripts?  The very next line of context here is:
>                 -j .u_boot_list -j .rela.dyn -j .got -j .got.plt
> 
> Meaning that we intentionally copy .got / .got.plt into the resulting binary.  And
> I see that we took in 397d7d5a1be1 from you back in 2016 saying that we
> needed this in SPL.  But 5a942a152776 put the got/got.plt sections (for 32bit
> ARM) in intentionally as some relocations do need it.  And in 4b0d506ed3b4
> Philipp seems to have seen the same problem you have, but fixed it with
> adding got/got.plt to the sections list we copy in (the above hunk of context).

If pass -fno-pic to compiler, there will be no .got and .got.plt sections.
The .got and .got.plt is usually used for dynamic link, such as linux "*.so" file.
We need to pass -fno-pic to compiler to remove .got and .got.plt sections.

Thanks,
Peng.

> 
> --
> Tom

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [U-Boot] [PATCH 2/2] arm: config: enforce -fno-pic for SPL and normal U-Boot
  2017-07-04  1:09     ` Peng Fan
@ 2017-07-04  2:47       ` Tom Rini
  2017-07-04  3:12         ` Peng Fan
  0 siblings, 1 reply; 18+ messages in thread
From: Tom Rini @ 2017-07-04  2:47 UTC (permalink / raw)
  To: u-boot

On Tue, Jul 04, 2017 at 01:09:36AM +0000, Peng Fan wrote:
> Hi Tom,
> 
> > -----Original Message-----
> > From: Tom Rini [mailto:trini at konsulko.com]
> > Sent: Tuesday, July 04, 2017 12:17 AM
> > To: Peng Fan <peng.fan@nxp.com>; Simon Glass <sjg@chromium.org>; Philipp
> > Tomsich <philipp.tomsich@theobroma-systems.com>
> > Cc: albert.u.boot at aribaud.net; u-boot at lists.denx.de
> > Subject: Re: [U-Boot] [PATCH 2/2] arm: config: enforce -fno-pic for SPL and
> > normal U-Boot
> > 
> > On Mon, Jul 03, 2017 at 09:14:08PM +0800, Peng Fan wrote:
> > 
> > > If not pass -fno-pic to toolchains, some toolchains may generate .got
> > > and .got.plt sections, but when generate binaries, we did not take
> > > .got and .got.plt into consideration, then SPL or normal U-Boot boot
> > > failure because image corrupted.
> > >
> > > Need to pass -fno-pic to disable generating .got and .got.plt
> > > sections.
> > >
> > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > Cc: Albert Aribaud <albert.u.boot@aribaud.net>
> > > Cc: Tom Rini <trini@konsulko.com>
> > > ---
> > >  arch/arm/config.mk | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/arch/arm/config.mk b/arch/arm/config.mk index
> > > 1a77779..66ae403 100644
> > > --- a/arch/arm/config.mk
> > > +++ b/arch/arm/config.mk
> > > @@ -130,9 +130,10 @@ ALL-y += checkarmreloc  # instruction. Relocation
> > > is not supported for that case, so disable  # such usage by requiring
> > > word relocations.
> > >  PLATFORM_CPPFLAGS += $(call cc-option, -mword-relocations)
> > > -PLATFORM_CPPFLAGS += $(call cc-option, -fno-pic)  endif
> > >
> > > +PLATFORM_CPPFLAGS += $(call cc-option, -fno-pic)
> > > +
> > >  # limit ourselves to the sections we want in the .bin.
> > >  ifdef CONFIG_ARM64
> > >  OBJCOPYFLAGS += -j .text -j .secure_text -j .secure_data -j .rodata
> > > -j .data \
> > 
> > Something is "up" here and I need you to dig harder and perhaps see if we're
> > missing something in the linker scripts?  The very next line of context here is:
> >                 -j .u_boot_list -j .rela.dyn -j .got -j .got.plt
> > 
> > Meaning that we intentionally copy .got / .got.plt into the resulting binary.  And
> > I see that we took in 397d7d5a1be1 from you back in 2016 saying that we
> > needed this in SPL.  But 5a942a152776 put the got/got.plt sections (for 32bit
> > ARM) in intentionally as some relocations do need it.  And in 4b0d506ed3b4
> > Philipp seems to have seen the same problem you have, but fixed it with
> > adding got/got.plt to the sections list we copy in (the above hunk of context).
> 
> If pass -fno-pic to compiler, there will be no .got and .got.plt sections.
> The .got and .got.plt is usually used for dynamic link, such as linux "*.so" file.
> We need to pass -fno-pic to compiler to remove .got and .got.plt sections.

"Usually" isn't the same as "always" or "only".  And this reminded me
that we had to deal with e391b1e64b0b because yes, we're not making a
shared library but we do have position independent code.  So, in the
case of SPL, since we can get away with -fno-pic (and get some space
savings) that's just not true of U-Boot itself.  We're enforcing -fpic
on other architectures, so what exactly is going on with what you're
seeing?  Where would we need to be taking .got/.got.plt into
consideration and why would it be a bad thing to do so?  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170703/47c0a4cf/attachment.sig>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [U-Boot] [PATCH 2/2] arm: config: enforce -fno-pic for SPL and normal U-Boot
  2017-07-04  2:47       ` Tom Rini
@ 2017-07-04  3:12         ` Peng Fan
  2017-07-04  4:15           ` J. William Campbell
  2017-07-04 13:35           ` Tom Rini
  0 siblings, 2 replies; 18+ messages in thread
From: Peng Fan @ 2017-07-04  3:12 UTC (permalink / raw)
  To: u-boot



> -----Original Message-----
> From: Tom Rini [mailto:trini at konsulko.com]
> Sent: Tuesday, July 04, 2017 10:47 AM
> To: Peng Fan <peng.fan@nxp.com>
> Cc: Simon Glass <sjg@chromium.org>; Philipp Tomsich
> <philipp.tomsich@theobroma-systems.com>; albert.u.boot at aribaud.net; u-
> boot at lists.denx.de
> Subject: Re: [U-Boot] [PATCH 2/2] arm: config: enforce -fno-pic for SPL and
> normal U-Boot
> 
> On Tue, Jul 04, 2017 at 01:09:36AM +0000, Peng Fan wrote:
> > Hi Tom,
> >
> > > -----Original Message-----
> > > From: Tom Rini [mailto:trini at konsulko.com]
> > > Sent: Tuesday, July 04, 2017 12:17 AM
> > > To: Peng Fan <peng.fan@nxp.com>; Simon Glass <sjg@chromium.org>;
> > > Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> > > Cc: albert.u.boot at aribaud.net; u-boot at lists.denx.de
> > > Subject: Re: [U-Boot] [PATCH 2/2] arm: config: enforce -fno-pic for
> > > SPL and normal U-Boot
> > >
> > > On Mon, Jul 03, 2017 at 09:14:08PM +0800, Peng Fan wrote:
> > >
> > > > If not pass -fno-pic to toolchains, some toolchains may generate
> > > > .got and .got.plt sections, but when generate binaries, we did not
> > > > take .got and .got.plt into consideration, then SPL or normal
> > > > U-Boot boot failure because image corrupted.
> > > >
> > > > Need to pass -fno-pic to disable generating .got and .got.plt
> > > > sections.
> > > >
> > > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > > Cc: Albert Aribaud <albert.u.boot@aribaud.net>
> > > > Cc: Tom Rini <trini@konsulko.com>
> > > > ---
> > > >  arch/arm/config.mk | 3 ++-
> > > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/arch/arm/config.mk b/arch/arm/config.mk index
> > > > 1a77779..66ae403 100644
> > > > --- a/arch/arm/config.mk
> > > > +++ b/arch/arm/config.mk
> > > > @@ -130,9 +130,10 @@ ALL-y += checkarmreloc  # instruction.
> > > > Relocation is not supported for that case, so disable  # such
> > > > usage by requiring word relocations.
> > > >  PLATFORM_CPPFLAGS += $(call cc-option, -mword-relocations)
> > > > -PLATFORM_CPPFLAGS += $(call cc-option, -fno-pic)  endif
> > > >
> > > > +PLATFORM_CPPFLAGS += $(call cc-option, -fno-pic)
> > > > +
> > > >  # limit ourselves to the sections we want in the .bin.
> > > >  ifdef CONFIG_ARM64
> > > >  OBJCOPYFLAGS += -j .text -j .secure_text -j .secure_data -j
> > > > .rodata -j .data \
> > >
> > > Something is "up" here and I need you to dig harder and perhaps see
> > > if we're missing something in the linker scripts?  The very next line of
> context here is:
> > >                 -j .u_boot_list -j .rela.dyn -j .got -j .got.plt
> > >
> > > Meaning that we intentionally copy .got / .got.plt into the
> > > resulting binary.  And I see that we took in 397d7d5a1be1 from you
> > > back in 2016 saying that we needed this in SPL.  But 5a942a152776
> > > put the got/got.plt sections (for 32bit
> > > ARM) in intentionally as some relocations do need it.  And in
> > > 4b0d506ed3b4 Philipp seems to have seen the same problem you have,
> > > but fixed it with adding got/got.plt to the sections list we copy in (the above
> hunk of context).
> >
> > If pass -fno-pic to compiler, there will be no .got and .got.plt sections.
> > The .got and .got.plt is usually used for dynamic link, such as linux "*.so" file.
> > We need to pass -fno-pic to compiler to remove .got and .got.plt sections.
> 
> "Usually" isn't the same as "always" or "only".  And this reminded me that we

From https://gcc.gnu.org/onlinedocs/gcc/Code-Gen-Options.html#Code-Gen-Options,
when -fpic is used for ARM, there will be GOT. The dynamic loader resolves the GOT entries
when the program starts (the dynamic loader is not part of GCC; it is part of the
operating system). 

> had to deal with e391b1e64b0b because yes, we're not making a shared library
> but we do have position independent code.  So, in the case of SPL, since we

For position independent code, but not making a shared library, we could use -fpie.

> can get away with -fno-pic (and get some space
> savings) that's just not true of U-Boot itself.  We're enforcing -fpic on other
> architectures, so what exactly is going on with what you're seeing?  Where

If not passing -fno-pic to gcc, the android toolchain will generate .got and .got.plt
section. I think these sections are for dynamic link, not for static link.

> would we need to be taking .got/.got.plt into consideration and why would it
> be a bad thing to do so?  Thanks!

We could keep .got and .got.plt, but personally I prefer use -fno-pic.
I think fpic is for shared library and fpie is for position independt code for static link.

Thanks,
Peng.

> 
> --
> Tom

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [U-Boot] [PATCH 2/2] arm: config: enforce -fno-pic for SPL and normal U-Boot
  2017-07-04  3:12         ` Peng Fan
@ 2017-07-04  4:15           ` J. William Campbell
  2017-07-04 13:35           ` Tom Rini
  1 sibling, 0 replies; 18+ messages in thread
From: J. William Campbell @ 2017-07-04  4:15 UTC (permalink / raw)
  To: u-boot

On 7/3/2017 8:12 PM, Peng Fan wrote:
>
>> -----Original Message-----
>> From: Tom Rini [mailto:trini at konsulko.com]
>> Sent: Tuesday, July 04, 2017 10:47 AM
>> To: Peng Fan <peng.fan@nxp.com>
>> Cc: Simon Glass <sjg@chromium.org>; Philipp Tomsich
>> <philipp.tomsich@theobroma-systems.com>; albert.u.boot at aribaud.net; u-
>> boot at lists.denx.de
>> Subject: Re: [U-Boot] [PATCH 2/2] arm: config: enforce -fno-pic for SPL and
>> normal U-Boot
>>
>> On Tue, Jul 04, 2017 at 01:09:36AM +0000, Peng Fan wrote:
>>> Hi Tom,
>>>
>>>> -----Original Message-----
>>>> From: Tom Rini [mailto:trini at konsulko.com]
>>>> Sent: Tuesday, July 04, 2017 12:17 AM
>>>> To: Peng Fan <peng.fan@nxp.com>; Simon Glass <sjg@chromium.org>;
>>>> Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
>>>> Cc: albert.u.boot at aribaud.net; u-boot at lists.denx.de
>>>> Subject: Re: [U-Boot] [PATCH 2/2] arm: config: enforce -fno-pic for
>>>> SPL and normal U-Boot
>>>>
>>>> On Mon, Jul 03, 2017 at 09:14:08PM +0800, Peng Fan wrote:
>>>>
>>>>> If not pass -fno-pic to toolchains, some toolchains may generate
>>>>> .got and .got.plt sections, but when generate binaries, we did not
>>>>> take .got and .got.plt into consideration, then SPL or normal
>>>>> U-Boot boot failure because image corrupted.
>>>>>
>>>>> Need to pass -fno-pic to disable generating .got and .got.plt
>>>>> sections.
>>>>>
>>>>> Signed-off-by: Peng Fan <peng.fan@nxp.com>
>>>>> Cc: Albert Aribaud <albert.u.boot@aribaud.net>
>>>>> Cc: Tom Rini <trini@konsulko.com>
>>>>> ---
>>>>>   arch/arm/config.mk | 3 ++-
>>>>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/arch/arm/config.mk b/arch/arm/config.mk index
>>>>> 1a77779..66ae403 100644
>>>>> --- a/arch/arm/config.mk
>>>>> +++ b/arch/arm/config.mk
>>>>> @@ -130,9 +130,10 @@ ALL-y += checkarmreloc  # instruction.
>>>>> Relocation is not supported for that case, so disable  # such
>>>>> usage by requiring word relocations.
>>>>>   PLATFORM_CPPFLAGS += $(call cc-option, -mword-relocations)
>>>>> -PLATFORM_CPPFLAGS += $(call cc-option, -fno-pic)  endif
>>>>>
>>>>> +PLATFORM_CPPFLAGS += $(call cc-option, -fno-pic)
>>>>> +
>>>>>   # limit ourselves to the sections we want in the .bin.
>>>>>   ifdef CONFIG_ARM64
>>>>>   OBJCOPYFLAGS += -j .text -j .secure_text -j .secure_data -j
>>>>> .rodata -j .data \
>>>> Something is "up" here and I need you to dig harder and perhaps see
>>>> if we're missing something in the linker scripts?  The very next line of
>> context here is:
>>>>                  -j .u_boot_list -j .rela.dyn -j .got -j .got.plt
>>>>
>>>> Meaning that we intentionally copy .got / .got.plt into the
>>>> resulting binary.  And I see that we took in 397d7d5a1be1 from you
>>>> back in 2016 saying that we needed this in SPL.  But 5a942a152776
>>>> put the got/got.plt sections (for 32bit
>>>> ARM) in intentionally as some relocations do need it.  And in
>>>> 4b0d506ed3b4 Philipp seems to have seen the same problem you have,
>>>> but fixed it with adding got/got.plt to the sections list we copy in (the above
>> hunk of context).
>>> If pass -fno-pic to compiler, there will be no .got and .got.plt sections.
>>> The .got and .got.plt is usually used for dynamic link, such as linux "*.so" file.
>>> We need to pass -fno-pic to compiler to remove .got and .got.plt sections.
>> "Usually" isn't the same as "always" or "only".  And this reminded me that we
>  From https://gcc.gnu.org/onlinedocs/gcc/Code-Gen-Options.html#Code-Gen-Options,
> when -fpic is used for ARM, there will be GOT. The dynamic loader resolves the GOT entries
> when the program starts (the dynamic loader is not part of GCC; it is part of the
> operating system).
>
>> had to deal with e391b1e64b0b because yes, we're not making a shared library
>> but we do have position independent code.  So, in the case of SPL, since we
> For position independent code, but not making a shared library, we could use -fpie.
>
>> can get away with -fno-pic (and get some space
>> savings) that's just not true of U-Boot itself.  We're enforcing -fpic on other
>> architectures, so what exactly is going on with what you're seeing?  Where
> If not passing -fno-pic to gcc, the android toolchain will generate .got and .got.plt
> section. I think these sections are for dynamic link, not for static link.
>
>> would we need to be taking .got/.got.plt into consideration and why would it
>> be a bad thing to do so?  Thanks!
> We could keep .got and .got.plt, but personally I prefer use -fno-pic.
> I think fpic is for shared library and fpie is for position independt code for static link.
>
> Thanks,
> Peng.
To clarify things, as I understand it, -pie produces the same code and 
segments as -pic does, plus a loader to load the code correctly into 
memory. u-boot probably doesn't want that loader. The loader is required 
to make the executable work under the OS, but stand-alone probably 
doesn't work right in general.  For u-boot, the advantage of -pic code 
is that there are no TEXTREL relocations in the binary. The code is 
generated to take care of bss (and code segment when required) 
references that require relocation. Things in the .got may need 
relocating by where bss ends up, but not the code. So relocation is 
short and simple.  Non pic code requires relocation processing that may 
be more complex than pic code, and requires a larger relocation table. 
If you are loading the code and bss at the location they were linked 
for, neither type requires relocation. If the code and or bss is located 
at a different address, -pic requires a little relocation and no pic 
requires a lot. In general, -pic code is slightly slower and larger than 
non-pic, but the relocation table for non-pic may be large enough to 
overcome the smaller code. YMMV, but either can be made to work. It 
depends on your exact situation. If you know the code will be loaded and 
executed as it was linked, non-pic is usually a winner, as you can 
discard all relocation. Otherwise, -pic is usually simpler to use, as 
the u-boot relocation code can be simpler. OTOH, if one already has a 
complete relocating loader in the u-boot code, either way will work. In 
any case, the relocation code must relocate itself as required for 
itself to work. Easy in -pic, not so easy otherwise.

Best Regards,
Bill Campbell
>> --
>> Tom
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [U-Boot] [PATCH 1/2] armv8: apply -mgeneral-regs-only
  2017-07-03 13:14 [U-Boot] [PATCH 1/2] armv8: apply -mgeneral-regs-only Peng Fan
  2017-07-03 13:14 ` [U-Boot] [PATCH 2/2] arm: config: enforce -fno-pic for SPL and normal U-Boot Peng Fan
@ 2017-07-04 13:32 ` Tom Rini
  2017-07-04 16:38   ` Dr. Philipp Tomsich
  1 sibling, 1 reply; 18+ messages in thread
From: Tom Rini @ 2017-07-04 13:32 UTC (permalink / raw)
  To: u-boot

On Mon, Jul 03, 2017 at 09:14:07PM +0800, Peng Fan wrote:

> Pass -mgeneral-regs-only to GCC, if not compiler may generate
> instructions that use SIMD registers.
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> Cc: Albert Aribaud <albert.u.boot@aribaud.net>
> Cc: Tom Rini <trini@konsulko.com>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170704/b8e05a91/attachment.sig>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [U-Boot] [PATCH 2/2] arm: config: enforce -fno-pic for SPL and normal U-Boot
  2017-07-04  3:12         ` Peng Fan
  2017-07-04  4:15           ` J. William Campbell
@ 2017-07-04 13:35           ` Tom Rini
  2017-07-05  1:02             ` Peng Fan
  1 sibling, 1 reply; 18+ messages in thread
From: Tom Rini @ 2017-07-04 13:35 UTC (permalink / raw)
  To: u-boot

On Tue, Jul 04, 2017 at 03:12:54AM +0000, Peng Fan wrote:
> 
> 
> > -----Original Message-----
> > From: Tom Rini [mailto:trini at konsulko.com]
> > Sent: Tuesday, July 04, 2017 10:47 AM
> > To: Peng Fan <peng.fan@nxp.com>
> > Cc: Simon Glass <sjg@chromium.org>; Philipp Tomsich
> > <philipp.tomsich@theobroma-systems.com>; albert.u.boot at aribaud.net; u-
> > boot at lists.denx.de
> > Subject: Re: [U-Boot] [PATCH 2/2] arm: config: enforce -fno-pic for SPL and
> > normal U-Boot
> > 
> > On Tue, Jul 04, 2017 at 01:09:36AM +0000, Peng Fan wrote:
> > > Hi Tom,
> > >
> > > > -----Original Message-----
> > > > From: Tom Rini [mailto:trini at konsulko.com]
> > > > Sent: Tuesday, July 04, 2017 12:17 AM
> > > > To: Peng Fan <peng.fan@nxp.com>; Simon Glass <sjg@chromium.org>;
> > > > Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> > > > Cc: albert.u.boot at aribaud.net; u-boot at lists.denx.de
> > > > Subject: Re: [U-Boot] [PATCH 2/2] arm: config: enforce -fno-pic for
> > > > SPL and normal U-Boot
> > > >
> > > > On Mon, Jul 03, 2017 at 09:14:08PM +0800, Peng Fan wrote:
> > > >
> > > > > If not pass -fno-pic to toolchains, some toolchains may generate
> > > > > .got and .got.plt sections, but when generate binaries, we did not
> > > > > take .got and .got.plt into consideration, then SPL or normal
> > > > > U-Boot boot failure because image corrupted.
> > > > >
> > > > > Need to pass -fno-pic to disable generating .got and .got.plt
> > > > > sections.
> > > > >
> > > > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > > > Cc: Albert Aribaud <albert.u.boot@aribaud.net>
> > > > > Cc: Tom Rini <trini@konsulko.com>
> > > > > ---
> > > > >  arch/arm/config.mk | 3 ++-
> > > > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/arch/arm/config.mk b/arch/arm/config.mk index
> > > > > 1a77779..66ae403 100644
> > > > > --- a/arch/arm/config.mk
> > > > > +++ b/arch/arm/config.mk
> > > > > @@ -130,9 +130,10 @@ ALL-y += checkarmreloc  # instruction.
> > > > > Relocation is not supported for that case, so disable  # such
> > > > > usage by requiring word relocations.
> > > > >  PLATFORM_CPPFLAGS += $(call cc-option, -mword-relocations)
> > > > > -PLATFORM_CPPFLAGS += $(call cc-option, -fno-pic)  endif
> > > > >
> > > > > +PLATFORM_CPPFLAGS += $(call cc-option, -fno-pic)
> > > > > +
> > > > >  # limit ourselves to the sections we want in the .bin.
> > > > >  ifdef CONFIG_ARM64
> > > > >  OBJCOPYFLAGS += -j .text -j .secure_text -j .secure_data -j
> > > > > .rodata -j .data \
> > > >
> > > > Something is "up" here and I need you to dig harder and perhaps see
> > > > if we're missing something in the linker scripts?  The very next line of
> > context here is:
> > > >                 -j .u_boot_list -j .rela.dyn -j .got -j .got.plt
> > > >
> > > > Meaning that we intentionally copy .got / .got.plt into the
> > > > resulting binary.  And I see that we took in 397d7d5a1be1 from you
> > > > back in 2016 saying that we needed this in SPL.  But 5a942a152776
> > > > put the got/got.plt sections (for 32bit
> > > > ARM) in intentionally as some relocations do need it.  And in
> > > > 4b0d506ed3b4 Philipp seems to have seen the same problem you have,
> > > > but fixed it with adding got/got.plt to the sections list we copy in (the above
> > hunk of context).
> > >
> > > If pass -fno-pic to compiler, there will be no .got and .got.plt sections.
> > > The .got and .got.plt is usually used for dynamic link, such as linux "*.so" file.
> > > We need to pass -fno-pic to compiler to remove .got and .got.plt sections.
> > 
> > "Usually" isn't the same as "always" or "only".  And this reminded me that we
> 
> From https://gcc.gnu.org/onlinedocs/gcc/Code-Gen-Options.html#Code-Gen-Options,
> when -fpic is used for ARM, there will be GOT. The dynamic loader resolves the GOT entries
> when the program starts (the dynamic loader is not part of GCC; it is part of the
> operating system). 
> 
> > had to deal with e391b1e64b0b because yes, we're not making a shared library
> > but we do have position independent code.  So, in the case of SPL, since we
> 
> For position independent code, but not making a shared library, we could use -fpie.

Note that "just" a switch to -fno-pic -fpie results in non-booting
platforms for me, so there'd be more work to be done there.

> > can get away with -fno-pic (and get some space
> > savings) that's just not true of U-Boot itself.  We're enforcing -fpic on other
> > architectures, so what exactly is going on with what you're seeing?  Where
> 
> If not passing -fno-pic to gcc, the android toolchain will generate
> .got and .got.plt section. I think these sections are for dynamic
> link, not for static link.

Right, but that's why we include these sections in the images now.  Have
you confirmed that top of tree doesn't work with that android toolchain,
without your second patch here?

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170704/1cdb84d7/attachment.sig>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [U-Boot] [PATCH 1/2] armv8: apply -mgeneral-regs-only
  2017-07-04 13:32 ` [U-Boot] [PATCH 1/2] armv8: apply -mgeneral-regs-only Tom Rini
@ 2017-07-04 16:38   ` Dr. Philipp Tomsich
  2017-07-04 19:38     ` Tom Rini
  0 siblings, 1 reply; 18+ messages in thread
From: Dr. Philipp Tomsich @ 2017-07-04 16:38 UTC (permalink / raw)
  To: u-boot

Tom,

Please note that some GCC versions had a code-generation bug
when -mgeneral-regs-only was used (we hit this for a customer in
a vendor GCC we maintain):
	https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64304

Until everyone upgrades to a recently recent GCC (or until UBoot
enforces this next January), some people may hit hard-to-reproduce
issues.

A better way to suppress SIMD generation would be to use:
	-march=armv8-a+nosimd

Regards,
Philipp.

> On 04 Jul 2017, at 15:32, Tom Rini <trini@konsulko.com> wrote:
> 
> On Mon, Jul 03, 2017 at 09:14:07PM +0800, Peng Fan wrote:
> 
>> Pass -mgeneral-regs-only to GCC, if not compiler may generate
>> instructions that use SIMD registers.
>> 
>> Signed-off-by: Peng Fan <peng.fan@nxp.com>
>> Cc: Albert Aribaud <albert.u.boot@aribaud.net>
>> Cc: Tom Rini <trini@konsulko.com>
> 
> Reviewed-by: Tom Rini <trini@konsulko.com>
> 
> -- 
> Tom
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [U-Boot] [PATCH 1/2] armv8: apply -mgeneral-regs-only
  2017-07-04 16:38   ` Dr. Philipp Tomsich
@ 2017-07-04 19:38     ` Tom Rini
  2017-07-05  1:04       ` Peng Fan
  2017-07-05  1:23       ` Peng Fan
  0 siblings, 2 replies; 18+ messages in thread
From: Tom Rini @ 2017-07-04 19:38 UTC (permalink / raw)
  To: u-boot

On Tue, Jul 04, 2017 at 06:38:10PM +0200, Dr. Philipp Tomsich wrote:
> Tom,
> 
> Please note that some GCC versions had a code-generation bug
> when -mgeneral-regs-only was used (we hit this for a customer in
> a vendor GCC we maintain):
> 	https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64304
> 
> Until everyone upgrades to a recently recent GCC (or until UBoot
> enforces this next January), some people may hit hard-to-reproduce
> issues.
> 
> A better way to suppress SIMD generation would be to use:
> 	-march=armv8-a+nosimd

Ah, good to know.  Peng, please re-spin.

> 
> Regards,
> Philipp.
> 
> > On 04 Jul 2017, at 15:32, Tom Rini <trini@konsulko.com> wrote:
> > 
> > On Mon, Jul 03, 2017 at 09:14:07PM +0800, Peng Fan wrote:
> > 
> >> Pass -mgeneral-regs-only to GCC, if not compiler may generate
> >> instructions that use SIMD registers.
> >> 
> >> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> >> Cc: Albert Aribaud <albert.u.boot@aribaud.net>
> >> Cc: Tom Rini <trini@konsulko.com>
> > 
> > Reviewed-by: Tom Rini <trini@konsulko.com>
> > 
> > -- 
> > Tom
> > _______________________________________________
> > U-Boot mailing list
> > U-Boot at lists.denx.de
> > https://lists.denx.de/listinfo/u-boot
> 

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170704/3c02b2b0/attachment.sig>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [U-Boot] [PATCH 2/2] arm: config: enforce -fno-pic for SPL and normal U-Boot
  2017-07-04 13:35           ` Tom Rini
@ 2017-07-05  1:02             ` Peng Fan
  2017-07-05  1:56               ` Tom Rini
  0 siblings, 1 reply; 18+ messages in thread
From: Peng Fan @ 2017-07-05  1:02 UTC (permalink / raw)
  To: u-boot



> -----Original Message-----
> From: Tom Rini [mailto:trini at konsulko.com]
> Sent: Tuesday, July 04, 2017 9:35 PM
> To: Peng Fan <peng.fan@nxp.com>
> Cc: Simon Glass <sjg@chromium.org>; Philipp Tomsich
> <philipp.tomsich@theobroma-systems.com>; albert.u.boot at aribaud.net; u-
> boot at lists.denx.de
> Subject: Re: [U-Boot] [PATCH 2/2] arm: config: enforce -fno-pic for SPL and
> normal U-Boot
> 
> On Tue, Jul 04, 2017 at 03:12:54AM +0000, Peng Fan wrote:
> >
> >
> > > -----Original Message-----
> > > From: Tom Rini [mailto:trini at konsulko.com]
> > > Sent: Tuesday, July 04, 2017 10:47 AM
> > > To: Peng Fan <peng.fan@nxp.com>
> > > Cc: Simon Glass <sjg@chromium.org>; Philipp Tomsich
> > > <philipp.tomsich@theobroma-systems.com>; albert.u.boot at aribaud.net;
> > > u- boot at lists.denx.de
> > > Subject: Re: [U-Boot] [PATCH 2/2] arm: config: enforce -fno-pic for
> > > SPL and normal U-Boot
> > >
> > > On Tue, Jul 04, 2017 at 01:09:36AM +0000, Peng Fan wrote:
> > > > Hi Tom,
> > > >
> > > > > -----Original Message-----
> > > > > From: Tom Rini [mailto:trini at konsulko.com]
> > > > > Sent: Tuesday, July 04, 2017 12:17 AM
> > > > > To: Peng Fan <peng.fan@nxp.com>; Simon Glass <sjg@chromium.org>;
> > > > > Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> > > > > Cc: albert.u.boot at aribaud.net; u-boot at lists.denx.de
> > > > > Subject: Re: [U-Boot] [PATCH 2/2] arm: config: enforce -fno-pic
> > > > > for SPL and normal U-Boot
> > > > >
> > > > > On Mon, Jul 03, 2017 at 09:14:08PM +0800, Peng Fan wrote:
> > > > >
> > > > > > If not pass -fno-pic to toolchains, some toolchains may
> > > > > > generate .got and .got.plt sections, but when generate
> > > > > > binaries, we did not take .got and .got.plt into
> > > > > > consideration, then SPL or normal U-Boot boot failure because image
> corrupted.
> > > > > >
> > > > > > Need to pass -fno-pic to disable generating .got and .got.plt
> > > > > > sections.
> > > > > >
> > > > > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > > > > Cc: Albert Aribaud <albert.u.boot@aribaud.net>
> > > > > > Cc: Tom Rini <trini@konsulko.com>
> > > > > > ---
> > > > > >  arch/arm/config.mk | 3 ++-
> > > > > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git a/arch/arm/config.mk b/arch/arm/config.mk index
> > > > > > 1a77779..66ae403 100644
> > > > > > --- a/arch/arm/config.mk
> > > > > > +++ b/arch/arm/config.mk
> > > > > > @@ -130,9 +130,10 @@ ALL-y += checkarmreloc  # instruction.
> > > > > > Relocation is not supported for that case, so disable  # such
> > > > > > usage by requiring word relocations.
> > > > > >  PLATFORM_CPPFLAGS += $(call cc-option, -mword-relocations)
> > > > > > -PLATFORM_CPPFLAGS += $(call cc-option, -fno-pic)  endif
> > > > > >
> > > > > > +PLATFORM_CPPFLAGS += $(call cc-option, -fno-pic)
> > > > > > +
> > > > > >  # limit ourselves to the sections we want in the .bin.
> > > > > >  ifdef CONFIG_ARM64
> > > > > >  OBJCOPYFLAGS += -j .text -j .secure_text -j .secure_data -j
> > > > > > .rodata -j .data \
> > > > >
> > > > > Something is "up" here and I need you to dig harder and perhaps
> > > > > see if we're missing something in the linker scripts?  The very
> > > > > next line of
> > > context here is:
> > > > >                 -j .u_boot_list -j .rela.dyn -j .got -j .got.plt
> > > > >
> > > > > Meaning that we intentionally copy .got / .got.plt into the
> > > > > resulting binary.  And I see that we took in 397d7d5a1be1 from
> > > > > you back in 2016 saying that we needed this in SPL.  But
> > > > > 5a942a152776 put the got/got.plt sections (for 32bit
> > > > > ARM) in intentionally as some relocations do need it.  And in
> > > > > 4b0d506ed3b4 Philipp seems to have seen the same problem you
> > > > > have, but fixed it with adding got/got.plt to the sections list
> > > > > we copy in (the above
> > > hunk of context).
> > > >
> > > > If pass -fno-pic to compiler, there will be no .got and .got.plt sections.
> > > > The .got and .got.plt is usually used for dynamic link, such as linux "*.so"
> file.
> > > > We need to pass -fno-pic to compiler to remove .got and .got.plt sections.
> > >
> > > "Usually" isn't the same as "always" or "only".  And this reminded
> > > me that we
> >
> > From
> > https://gcc.gnu.org/onlinedocs/gcc/Code-Gen-Options.html#Code-Gen-Opti
> > ons, when -fpic is used for ARM, there will be GOT. The dynamic loader
> > resolves the GOT entries when the program starts (the dynamic loader
> > is not part of GCC; it is part of the operating system).
> >
> > > had to deal with e391b1e64b0b because yes, we're not making a shared
> > > library but we do have position independent code.  So, in the case
> > > of SPL, since we
> >
> > For position independent code, but not making a shared library, we could use
> -fpie.
> 
> Note that "just" a switch to -fno-pic -fpie results in non-booting platforms for
> me, so there'd be more work to be done there.
> 
> > > can get away with -fno-pic (and get some space
> > > savings) that's just not true of U-Boot itself.  We're enforcing
> > > -fpic on other architectures, so what exactly is going on with what
> > > you're seeing?  Where
> >
> > If not passing -fno-pic to gcc, the android toolchain will generate
> > .got and .got.plt section. I think these sections are for dynamic
> > link, not for static link.
> 
> Right, but that's why we include these sections in the images now.  Have you
> confirmed that top of tree doesn't work with that android toolchain, without
> your second patch here?

I am using 2017.03 release.  Without the second patch, gcc will generate instructions
like "str q0, [x2]", and uboot runs into sync abort when booting .

Regards,
Peng.

> 
> --
> Tom

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [U-Boot] [PATCH 1/2] armv8: apply -mgeneral-regs-only
  2017-07-04 19:38     ` Tom Rini
@ 2017-07-05  1:04       ` Peng Fan
  2017-07-05  1:23       ` Peng Fan
  1 sibling, 0 replies; 18+ messages in thread
From: Peng Fan @ 2017-07-05  1:04 UTC (permalink / raw)
  To: u-boot



> -----Original Message-----
> From: Tom Rini [mailto:trini at konsulko.com]
> Sent: Wednesday, July 05, 2017 3:38 AM
> To: Dr. Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> Cc: Peng Fan <peng.fan@nxp.com>; u-boot at lists.denx.de
> Subject: Re: [U-Boot] [PATCH 1/2] armv8: apply -mgeneral-regs-only
> 
> On Tue, Jul 04, 2017 at 06:38:10PM +0200, Dr. Philipp Tomsich wrote:
> > Tom,
> >
> > Please note that some GCC versions had a code-generation bug when
> > -mgeneral-regs-only was used (we hit this for a customer in a vendor
> > GCC we maintain):
> > 	https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64304
> >
> > Until everyone upgrades to a recently recent GCC (or until UBoot
> > enforces this next January), some people may hit hard-to-reproduce
> > issues.
> >
> > A better way to suppress SIMD generation would be to use:
> > 	-march=armv8-a+nosimd
> 
> Ah, good to know.  Peng, please re-spin.

Actually I followed Linux to use "mgeneral-regs-only" here. Since
There is bug in some version gcc, I'll test and resend patch with using nosimd.

Thanks,
Peng.

> 
> >
> > Regards,
> > Philipp.
> >
> > > On 04 Jul 2017, at 15:32, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Mon, Jul 03, 2017 at 09:14:07PM +0800, Peng Fan wrote:
> > >
> > >> Pass -mgeneral-regs-only to GCC, if not compiler may generate
> > >> instructions that use SIMD registers.
> > >>
> > >> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > >> Cc: Albert Aribaud <albert.u.boot@aribaud.net>
> > >> Cc: Tom Rini <trini@konsulko.com>
> > >
> > > Reviewed-by: Tom Rini <trini@konsulko.com>
> > >
> > > --
> > > Tom
> > > _______________________________________________
> > > U-Boot mailing list
> > > U-Boot at lists.denx.de
> > > https://lists.denx.de/listinfo/u-boot
> >
> 
> --
> Tom

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [U-Boot] [PATCH 1/2] armv8: apply -mgeneral-regs-only
  2017-07-04 19:38     ` Tom Rini
  2017-07-05  1:04       ` Peng Fan
@ 2017-07-05  1:23       ` Peng Fan
  2017-07-05  8:37         ` Dr. Philipp Tomsich
  1 sibling, 1 reply; 18+ messages in thread
From: Peng Fan @ 2017-07-05  1:23 UTC (permalink / raw)
  To: u-boot



> -----Original Message-----
> From: Tom Rini [mailto:trini at konsulko.com]
> Sent: Wednesday, July 05, 2017 3:38 AM
> To: Dr. Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> Cc: Peng Fan <peng.fan@nxp.com>; u-boot at lists.denx.de
> Subject: Re: [U-Boot] [PATCH 1/2] armv8: apply -mgeneral-regs-only
> 
> On Tue, Jul 04, 2017 at 06:38:10PM +0200, Dr. Philipp Tomsich wrote:
> > Tom,
> >
> > Please note that some GCC versions had a code-generation bug when
> > -mgeneral-regs-only was used (we hit this for a customer in a vendor
> > GCC we maintain):
> > 	https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64304
> >
> > Until everyone upgrades to a recently recent GCC (or until UBoot
> > enforces this next January), some people may hit hard-to-reproduce
> > issues.
> >
> > A better way to suppress SIMD generation would be to use:
> > 	-march=armv8-a+nosimd
> 
> Ah, good to know.  Peng, please re-spin.

According to https://gcc.gnu.org/onlinedocs/gcc/ARM-Options.html,
There is no +nosimd for armv8-a. I also tried +nosimd,
There is still "str     q0, [x29,#112] ".  If I use armv8-a+nofp, there is no
such instructions. Then, do you agree using " armv8-a+nofp "?

Thanks,
Peng.

> 
> >
> > Regards,
> > Philipp.
> >
> > > On 04 Jul 2017, at 15:32, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Mon, Jul 03, 2017 at 09:14:07PM +0800, Peng Fan wrote:
> > >
> > >> Pass -mgeneral-regs-only to GCC, if not compiler may generate
> > >> instructions that use SIMD registers.
> > >>
> > >> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > >> Cc: Albert Aribaud <albert.u.boot@aribaud.net>
> > >> Cc: Tom Rini <trini@konsulko.com>
> > >
> > > Reviewed-by: Tom Rini <trini@konsulko.com>
> > >
> > > --
> > > Tom
> > > _______________________________________________
> > > U-Boot mailing list
> > > U-Boot at lists.denx.de
> > > https://lists.denx.de/listinfo/u-boot
> >
> 
> --
> Tom

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [U-Boot] [PATCH 2/2] arm: config: enforce -fno-pic for SPL and normal U-Boot
  2017-07-05  1:02             ` Peng Fan
@ 2017-07-05  1:56               ` Tom Rini
  0 siblings, 0 replies; 18+ messages in thread
From: Tom Rini @ 2017-07-05  1:56 UTC (permalink / raw)
  To: u-boot

On Wed, Jul 05, 2017 at 01:02:42AM +0000, Peng Fan wrote:
> 
> 
> > -----Original Message-----
> > From: Tom Rini [mailto:trini at konsulko.com]
> > Sent: Tuesday, July 04, 2017 9:35 PM
> > To: Peng Fan <peng.fan@nxp.com>
> > Cc: Simon Glass <sjg@chromium.org>; Philipp Tomsich
> > <philipp.tomsich@theobroma-systems.com>; albert.u.boot at aribaud.net; u-
> > boot at lists.denx.de
> > Subject: Re: [U-Boot] [PATCH 2/2] arm: config: enforce -fno-pic for SPL and
> > normal U-Boot
> > 
> > On Tue, Jul 04, 2017 at 03:12:54AM +0000, Peng Fan wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Tom Rini [mailto:trini at konsulko.com]
> > > > Sent: Tuesday, July 04, 2017 10:47 AM
> > > > To: Peng Fan <peng.fan@nxp.com>
> > > > Cc: Simon Glass <sjg@chromium.org>; Philipp Tomsich
> > > > <philipp.tomsich@theobroma-systems.com>; albert.u.boot at aribaud.net;
> > > > u- boot at lists.denx.de
> > > > Subject: Re: [U-Boot] [PATCH 2/2] arm: config: enforce -fno-pic for
> > > > SPL and normal U-Boot
> > > >
> > > > On Tue, Jul 04, 2017 at 01:09:36AM +0000, Peng Fan wrote:
> > > > > Hi Tom,
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Tom Rini [mailto:trini at konsulko.com]
> > > > > > Sent: Tuesday, July 04, 2017 12:17 AM
> > > > > > To: Peng Fan <peng.fan@nxp.com>; Simon Glass <sjg@chromium.org>;
> > > > > > Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> > > > > > Cc: albert.u.boot at aribaud.net; u-boot at lists.denx.de
> > > > > > Subject: Re: [U-Boot] [PATCH 2/2] arm: config: enforce -fno-pic
> > > > > > for SPL and normal U-Boot
> > > > > >
> > > > > > On Mon, Jul 03, 2017 at 09:14:08PM +0800, Peng Fan wrote:
> > > > > >
> > > > > > > If not pass -fno-pic to toolchains, some toolchains may
> > > > > > > generate .got and .got.plt sections, but when generate
> > > > > > > binaries, we did not take .got and .got.plt into
> > > > > > > consideration, then SPL or normal U-Boot boot failure because image
> > corrupted.
> > > > > > >
> > > > > > > Need to pass -fno-pic to disable generating .got and .got.plt
> > > > > > > sections.
> > > > > > >
> > > > > > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > > > > > Cc: Albert Aribaud <albert.u.boot@aribaud.net>
> > > > > > > Cc: Tom Rini <trini@konsulko.com>
> > > > > > > ---
> > > > > > >  arch/arm/config.mk | 3 ++-
> > > > > > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > > > > >
> > > > > > > diff --git a/arch/arm/config.mk b/arch/arm/config.mk index
> > > > > > > 1a77779..66ae403 100644
> > > > > > > --- a/arch/arm/config.mk
> > > > > > > +++ b/arch/arm/config.mk
> > > > > > > @@ -130,9 +130,10 @@ ALL-y += checkarmreloc  # instruction.
> > > > > > > Relocation is not supported for that case, so disable  # such
> > > > > > > usage by requiring word relocations.
> > > > > > >  PLATFORM_CPPFLAGS += $(call cc-option, -mword-relocations)
> > > > > > > -PLATFORM_CPPFLAGS += $(call cc-option, -fno-pic)  endif
> > > > > > >
> > > > > > > +PLATFORM_CPPFLAGS += $(call cc-option, -fno-pic)
> > > > > > > +
> > > > > > >  # limit ourselves to the sections we want in the .bin.
> > > > > > >  ifdef CONFIG_ARM64
> > > > > > >  OBJCOPYFLAGS += -j .text -j .secure_text -j .secure_data -j
> > > > > > > .rodata -j .data \
> > > > > >
> > > > > > Something is "up" here and I need you to dig harder and perhaps
> > > > > > see if we're missing something in the linker scripts?  The very
> > > > > > next line of
> > > > context here is:
> > > > > >                 -j .u_boot_list -j .rela.dyn -j .got -j .got.plt
> > > > > >
> > > > > > Meaning that we intentionally copy .got / .got.plt into the
> > > > > > resulting binary.  And I see that we took in 397d7d5a1be1 from
> > > > > > you back in 2016 saying that we needed this in SPL.  But
> > > > > > 5a942a152776 put the got/got.plt sections (for 32bit
> > > > > > ARM) in intentionally as some relocations do need it.  And in
> > > > > > 4b0d506ed3b4 Philipp seems to have seen the same problem you
> > > > > > have, but fixed it with adding got/got.plt to the sections list
> > > > > > we copy in (the above
> > > > hunk of context).
> > > > >
> > > > > If pass -fno-pic to compiler, there will be no .got and .got.plt sections.
> > > > > The .got and .got.plt is usually used for dynamic link, such as linux "*.so"
> > file.
> > > > > We need to pass -fno-pic to compiler to remove .got and .got.plt sections.
> > > >
> > > > "Usually" isn't the same as "always" or "only".  And this reminded
> > > > me that we
> > >
> > > From
> > > https://gcc.gnu.org/onlinedocs/gcc/Code-Gen-Options.html#Code-Gen-Opti
> > > ons, when -fpic is used for ARM, there will be GOT. The dynamic loader
> > > resolves the GOT entries when the program starts (the dynamic loader
> > > is not part of GCC; it is part of the operating system).
> > >
> > > > had to deal with e391b1e64b0b because yes, we're not making a shared
> > > > library but we do have position independent code.  So, in the case
> > > > of SPL, since we
> > >
> > > For position independent code, but not making a shared library, we could use
> > -fpie.
> > 
> > Note that "just" a switch to -fno-pic -fpie results in non-booting platforms for
> > me, so there'd be more work to be done there.
> > 
> > > > can get away with -fno-pic (and get some space
> > > > savings) that's just not true of U-Boot itself.  We're enforcing
> > > > -fpic on other architectures, so what exactly is going on with what
> > > > you're seeing?  Where
> > >
> > > If not passing -fno-pic to gcc, the android toolchain will generate
> > > .got and .got.plt section. I think these sections are for dynamic
> > > link, not for static link.
> > 
> > Right, but that's why we include these sections in the images now.  Have you
> > confirmed that top of tree doesn't work with that android toolchain, without
> > your second patch here?
> 
> I am using 2017.03 release.  Without the second patch, gcc will
> generate instructions like "str q0, [x2]", and uboot runs into sync
> abort when booting .

So you're missing the patch I mentioned that added .got/.got.plt then :)

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170704/9ac0d89c/attachment.sig>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [U-Boot] [PATCH 1/2] armv8: apply -mgeneral-regs-only
  2017-07-05  1:23       ` Peng Fan
@ 2017-07-05  8:37         ` Dr. Philipp Tomsich
  2017-07-05  8:59           ` Peng Fan
  0 siblings, 1 reply; 18+ messages in thread
From: Dr. Philipp Tomsich @ 2017-07-05  8:37 UTC (permalink / raw)
  To: u-boot


> On 05 Jul 2017, at 03:23, Peng Fan <peng.fan@nxp.com> wrote:
> 
> 
> 
>> -----Original Message-----
>> From: Tom Rini [mailto:trini at konsulko.com]
>> Sent: Wednesday, July 05, 2017 3:38 AM
>> To: Dr. Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
>> Cc: Peng Fan <peng.fan@nxp.com>; u-boot at lists.denx.de
>> Subject: Re: [U-Boot] [PATCH 1/2] armv8: apply -mgeneral-regs-only
>> 
>> On Tue, Jul 04, 2017 at 06:38:10PM +0200, Dr. Philipp Tomsich wrote:
>>> Tom,
>>> 
>>> Please note that some GCC versions had a code-generation bug when
>>> -mgeneral-regs-only was used (we hit this for a customer in a vendor
>>> GCC we maintain):
>>> 	https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64304
>>> 
>>> Until everyone upgrades to a recently recent GCC (or until UBoot
>>> enforces this next January), some people may hit hard-to-reproduce
>>> issues.
>>> 
>>> A better way to suppress SIMD generation would be to use:
>>> 	-march=armv8-a+nosimd
>> 
>> Ah, good to know.  Peng, please re-spin.
> 
> According to https://gcc.gnu.org/onlinedocs/gcc/ARM-Options.html,
> There is no +nosimd for armv8-a. I also tried +nosimd,

This is odd. When you look at
	gcc/config/aarch64/aarch64-option-extensions.def
the “simd” option was there since the beginning of time for AArch64 (e.g. on the
gcc_4.8-branch — see https://github.com/gcc-mirror/gcc/blob/gcc-4_8-branch/gcc/config/aarch64/aarch64-option-extensions.def)
and is still there today (see https://github.com/gcc-mirror/gcc/blob/master/gcc/config/aarch64/aarch64-option-extensions.def)

I just tested the “armv8-a+nosimd” specification on (configured as aarch64-*-linux-gnueabi)
4.9.1, 4.9.3, 5.4.0, 6.3.0 and (configured as aarch64-*-elf) 6.3.0 and 7.1.0…
…unfortunately this was all that I could find installed on our target systems.

> There is still "str     q0, [x29,#112] ".  If I use armv8-a+nofp, there is no
> such instructions. Then, do you agree using " armv8-a+nofp “?

I don’t mind if we go with +nofp, but I would like to understand how it comes
that your GCC differs from the FSF-released GCC source tree… this worries
me a bit, as users might encounter similar differences in the field.

> Thanks,
> Peng.
> 
>> 
>>> 
>>> Regards,
>>> Philipp.
>>> 
>>>> On 04 Jul 2017, at 15:32, Tom Rini <trini@konsulko.com> wrote:
>>>> 
>>>> On Mon, Jul 03, 2017 at 09:14:07PM +0800, Peng Fan wrote:
>>>> 
>>>>> Pass -mgeneral-regs-only to GCC, if not compiler may generate
>>>>> instructions that use SIMD registers.
>>>>> 
>>>>> Signed-off-by: Peng Fan <peng.fan@nxp.com>
>>>>> Cc: Albert Aribaud <albert.u.boot@aribaud.net>
>>>>> Cc: Tom Rini <trini@konsulko.com>
>>>> 
>>>> Reviewed-by: Tom Rini <trini@konsulko.com>
>>>> 
>>>> --
>>>> Tom
>>>> _______________________________________________
>>>> U-Boot mailing list
>>>> U-Boot at lists.denx.de
>>>> https://lists.denx.de/listinfo/u-boot
>>> 
>> 
>> --
>> Tom

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [U-Boot] [PATCH 1/2] armv8: apply -mgeneral-regs-only
  2017-07-05  8:37         ` Dr. Philipp Tomsich
@ 2017-07-05  8:59           ` Peng Fan
  2017-07-05  9:38             ` Dr. Philipp Tomsich
  0 siblings, 1 reply; 18+ messages in thread
From: Peng Fan @ 2017-07-05  8:59 UTC (permalink / raw)
  To: u-boot



> -----Original Message-----
> From: Dr. Philipp Tomsich [mailto:philipp.tomsich at theobroma-systems.com]
> Sent: Wednesday, July 05, 2017 4:38 PM
> To: Peng Fan <peng.fan@nxp.com>
> Cc: Tom Rini <trini@konsulko.com>; u-boot at lists.denx.de
> Subject: Re: [U-Boot] [PATCH 1/2] armv8: apply -mgeneral-regs-only
> 
> 
> > On 05 Jul 2017, at 03:23, Peng Fan <peng.fan@nxp.com> wrote:
> >
> >
> >
> >> -----Original Message-----
> >> From: Tom Rini [mailto:trini at konsulko.com]
> >> Sent: Wednesday, July 05, 2017 3:38 AM
> >> To: Dr. Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> >> Cc: Peng Fan <peng.fan@nxp.com>; u-boot at lists.denx.de
> >> Subject: Re: [U-Boot] [PATCH 1/2] armv8: apply -mgeneral-regs-only
> >>
> >> On Tue, Jul 04, 2017 at 06:38:10PM +0200, Dr. Philipp Tomsich wrote:
> >>> Tom,
> >>>
> >>> Please note that some GCC versions had a code-generation bug when
> >>> -mgeneral-regs-only was used (we hit this for a customer in a vendor
> >>> GCC we maintain):
> >>> 	https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64304
> >>>
> >>> Until everyone upgrades to a recently recent GCC (or until UBoot
> >>> enforces this next January), some people may hit hard-to-reproduce
> >>> issues.
> >>>
> >>> A better way to suppress SIMD generation would be to use:
> >>> 	-march=armv8-a+nosimd
> >>
> >> Ah, good to know.  Peng, please re-spin.
> >
> > According to https://gcc.gnu.org/onlinedocs/gcc/ARM-Options.html,
> > There is no +nosimd for armv8-a. I also tried +nosimd,
> 
> This is odd. When you look at
> 	gcc/config/aarch64/aarch64-option-extensions.def
> the “simd” option was there since the beginning of time for AArch64 (e.g. on
> the gcc_4.8-branch — see https://github.com/gcc-mirror/gcc/blob/gcc-4_8-
> branch/gcc/config/aarch64/aarch64-option-extensions.def)
> and is still there today (see https://github.com/gcc-
> mirror/gcc/blob/master/gcc/config/aarch64/aarch64-option-extensions.def)

I am not GCC expert, what I can follow is the doc, although doc is not always accurate.

> 
> I just tested the “armv8-a+nosimd” specification on (configured as aarch64-*-
> linux-gnueabi) 4.9.1, 4.9.3, 5.4.0, 6.3.0 and (configured as aarch64-*-elf) 6.3.0
> and 7.1.0… …unfortunately this was all that I could find installed on our target
> systems.
> 

Then did you see simd registers are used?

> > There is still "str     q0, [x29,#112] ".  If I use armv8-a+nofp, there is no
> > such instructions. Then, do you agree using " armv8-a+nofp “?
> 
> I don’t mind if we go with +nofp, but I would like to understand how it comes
> that your GCC differs from the FSF-released GCC source tree… this worries me
> a bit, as users might encounter similar differences in the field.

I tried yocto 6.2.0 and android 4.9.x toolchain, both could generate instructions that use
simd registers, not simd instructions.

Regards,
Peng.
> 
> > Thanks,
> > Peng.
> >
> >>
> >>>
> >>> Regards,
> >>> Philipp.
> >>>
> >>>> On 04 Jul 2017, at 15:32, Tom Rini <trini@konsulko.com> wrote:
> >>>>
> >>>> On Mon, Jul 03, 2017 at 09:14:07PM +0800, Peng Fan wrote:
> >>>>
> >>>>> Pass -mgeneral-regs-only to GCC, if not compiler may generate
> >>>>> instructions that use SIMD registers.
> >>>>>
> >>>>> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> >>>>> Cc: Albert Aribaud <albert.u.boot@aribaud.net>
> >>>>> Cc: Tom Rini <trini@konsulko.com>
> >>>>
> >>>> Reviewed-by: Tom Rini <trini@konsulko.com>
> >>>>
> >>>> --
> >>>> Tom
> >>>> _______________________________________________
> >>>> U-Boot mailing list
> >>>> U-Boot at lists.denx.de
> >>>> https://lists.denx.de/listinfo/u-boot
> >>>
> >>
> >> --
> >> Tom

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [U-Boot] [PATCH 1/2] armv8: apply -mgeneral-regs-only
  2017-07-05  8:59           ` Peng Fan
@ 2017-07-05  9:38             ` Dr. Philipp Tomsich
  0 siblings, 0 replies; 18+ messages in thread
From: Dr. Philipp Tomsich @ 2017-07-05  9:38 UTC (permalink / raw)
  To: u-boot


> On 05 Jul 2017, at 10:59, Peng Fan <peng.fan@nxp.com> wrote:
> 
>> I just tested the “armv8-a+nosimd” specification on (configured as aarch64-*-
>> linux-gnueabi) 4.9.1, 4.9.3, 5.4.0, 6.3.0 and (configured as aarch64-*-elf) 6.3.0
>> and 7.1.0… …unfortunately this was all that I could find installed on our target
>> systems.
>> 
> 
> Then did you see simd registers are used?

I used some of our compiler tests for vectorisable code and verified the assembly
output: the compiler didn’t try to vectorise and also emitted an assembler directive
that would trigger an assembler error on AdvSIMD instructions.

To check with the U-Boot source, I’ll have to dig a bit deeper and get the full set
of toolchain versions set up in my U-Boot development environment.  However,
I plan to do so in the near future to make sure that we don’t encounter any
surprises when moving to GCC-6.3 for next year’s releases.

>>> There is still "str     q0, [x29,#112] ".  If I use armv8-a+nofp, there is no
>>> such instructions. Then, do you agree using " armv8-a+nofp “?
>> 
>> I don’t mind if we go with +nofp, but I would like to understand how it comes
>> that your GCC differs from the FSF-released GCC source tree… this worries me
>> a bit, as users might encounter similar differences in the field.
> 
> I tried yocto 6.2.0 and android 4.9.x toolchain, both could generate instructions that use
> simd registers, not simd instructions.

Thanks, I’ll take a closer look.

Phil.

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2017-07-05  9:38 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-03 13:14 [U-Boot] [PATCH 1/2] armv8: apply -mgeneral-regs-only Peng Fan
2017-07-03 13:14 ` [U-Boot] [PATCH 2/2] arm: config: enforce -fno-pic for SPL and normal U-Boot Peng Fan
2017-07-03 16:16   ` Tom Rini
2017-07-04  1:09     ` Peng Fan
2017-07-04  2:47       ` Tom Rini
2017-07-04  3:12         ` Peng Fan
2017-07-04  4:15           ` J. William Campbell
2017-07-04 13:35           ` Tom Rini
2017-07-05  1:02             ` Peng Fan
2017-07-05  1:56               ` Tom Rini
2017-07-04 13:32 ` [U-Boot] [PATCH 1/2] armv8: apply -mgeneral-regs-only Tom Rini
2017-07-04 16:38   ` Dr. Philipp Tomsich
2017-07-04 19:38     ` Tom Rini
2017-07-05  1:04       ` Peng Fan
2017-07-05  1:23       ` Peng Fan
2017-07-05  8:37         ` Dr. Philipp Tomsich
2017-07-05  8:59           ` Peng Fan
2017-07-05  9:38             ` Dr. Philipp Tomsich

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.