Re: [U-Boot] [PATCH 2/4] arm: add %function attribute to assembly functions
On Monday 20 February 2012 09:38 PM, Aneesh V wrote:
On Saturday 18 February 2012 10:18 PM, Albert ARIBAUD wrote:
Hi Aneesh,
[...]
I will get back with more details on the Linaro GCC 2012.01 later.
I meant the Linaro GCC 2012.01 tool-chain problem
This is a different problem. Some of the .rodata symbols are given an
odd address although they should be aligned to at least 2-byte boundary
). In fact the data is actually put at the even address but the symbol's
value is +1 of the actual address. This is the ARM convention for Thumb
functions, but they have applied it here for data too. That's the
problem. I see that this doesn't happen to all the .rodata in SPL.
Neither could I reproduce it with a small program. But the workaround
for this problem is to avoid -fdata-sections. The following patch works
around it.
diff --git a/config.mk b/config.mk
index ddaa477..723286a 100644
--- a/config.mk
+++ b/config.mk
@@ -190,7 +190,7 @@ CPPFLAGS := $(DBGFLAGS) $(OPTFLAGS) $(RELFLAGS) \
# Enable garbage collection of un-used sections for SPL
ifeq ($(CONFIG_SPL_BUILD),y)
-CPPFLAGS += -ffunction-sections -fdata-sections
+CPPFLAGS += -ffunction-sections
LDFLAGS_FINAL += --gc-sections
endif
Will you take a patch to make -fdata-sections optional, that is, having
it under something like CONFIG_SYS_SPL_NO_FDATA_SECTIONS?
Hmm... considering you're seeing the issue in a fairly new toolchain
release, I prefer notifying the toolchain makers, rather than removing
the -fdata-sections from SPL even for specific boards. Can you go and
see why the Linaro toolchain generated odd thumb data at all and get
this fixed?
I tried investigating a bit. As I mentioned earlier it doesn't happen
with some other files that use the same compiler and linker commands.
So, I don't know what's going on. Also, I couldn't reproduce it with a
simple program unlike in the other cases. Anyway, I have notified
tool-chain folks at Linaro:
http://article.gmane.org/gmane.linux.linaro.toolchain/2096
Ulrich Weigand has identified the root-cause [1]. The problem was due
to __attribute__ ((packed)) used in a structure. Let me quote him:
**
I can reproduce the odd addresses of .rodata symbols. However, this
occurs simply because the linker put *no* alignment requirement whatsoever
on those sections:
Section Headers:
[Nr] Name TypeAddr OffSize ES Flg Lk
Inf Al
[snip]
[11] .rodata.wkup_padc PROGBITS 000100 04 00 A 0
0 1
[12] .rodata.wkup_padc PROGBITS 000104 48 00 A 0
0 1
[13] .rodata.wkup_padc PROGBITS 00014c 0c 00 A 0
0 1
[14] .rodata.wkup_padc PROGBITS 000158 04 00 A 0
0 1
Note the Al column values of 1. In the final executable, those sections
happen to end up immediately following a .rodata.str string section with
odd
lenght, and since they don't have any alignment requirement, they start out
at an odd address.
The reason for the lack of alignment requirement is actually in the source:
const struct pad_conf_entry core_padconf_array_essential[] = {
where
struct pad_conf_entry {
u16 offset;
u16 val;
} __attribute__ ((packed));
The packed attribute specifies that all struct elements ought to be
considered to have alignment requirement 1 instead of their default
alignment. Thus the whole struct ends up having alignment requirement 1;
and since the section contains only a single variable of such struct
type, this is then also the alignment requirement of the section.
**
I tried removing packed and it works. I will send an updated series
with this fix.
br,
Aneesh
[1] http://article.gmane.org/gmane.linux.linaro.toolchain/2099
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 2/4] arm: add %function attribute to assembly functions
On Mon, Feb 20, 2012 at 11:19:14PM -0500, Mike Frysinger wrote: On Monday 20 February 2012 16:53:40 Simon Glass wrote: On Mon, Feb 20, 2012 at 12:07 PM, Tom Rini wrote: On Sun, Feb 19, 2012 at 02:15:30AM -0500, Mike Frysinger wrote: On Saturday 18 February 2012 17:03:59 Simon Glass wrote: On Wed, Feb 15, 2012 at 5:57 AM, Aneesh V wrote: -.globl reset_cpu +.type reset_cpu, %function +.globalreset_cpu Should we introduce a macro to deal with this rather than writing it out each time? EXPORT()? we have it already with the linux/linkage.h header :) Well, unless my tree is out of date (or stuff is in-flight) we don't have the full compliment here. We have linux/linkage.h for all and asm/linkage.h for bfin. That said, yes, we should grab at least the ARM version and make use of ENTRY/END_FUNC ala the kernel. I'm behind on my convert __attribute__((...)) to __attr series already or I'd say more :) In case one of you is going to look at this, can we try to use asm-generic as much as possible? i don't know what you mean ... we already have linux/linkage.h with sane defaults for pretty much everyone. the Blackfin asm/linkage.h is an empty file to satisfy building. Well, the kernel version for blackfin sets ALIGN/ALIGN_STR, so are they out of sync? -- Tom ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 2/4] arm: add %function attribute to assembly functions
On Tuesday 21 February 2012 09:33:31 Tom Rini wrote: On Mon, Feb 20, 2012 at 11:19:14PM -0500, Mike Frysinger wrote: On Monday 20 February 2012 16:53:40 Simon Glass wrote: On Mon, Feb 20, 2012 at 12:07 PM, Tom Rini wrote: On Sun, Feb 19, 2012 at 02:15:30AM -0500, Mike Frysinger wrote: On Saturday 18 February 2012 17:03:59 Simon Glass wrote: On Wed, Feb 15, 2012 at 5:57 AM, Aneesh V wrote: -.globl reset_cpu +.type reset_cpu, %function +.globalreset_cpu Should we introduce a macro to deal with this rather than writing it out each time? EXPORT()? we have it already with the linux/linkage.h header :) Well, unless my tree is out of date (or stuff is in-flight) we don't have the full compliment here. We have linux/linkage.h for all and asm/linkage.h for bfin. That said, yes, we should grab at least the ARM version and make use of ENTRY/END_FUNC ala the kernel. I'm behind on my convert __attribute__((...)) to __attr series already or I'd say more :) In case one of you is going to look at this, can we try to use asm-generic as much as possible? i don't know what you mean ... we already have linux/linkage.h with sane defaults for pretty much everyone. the Blackfin asm/linkage.h is an empty file to satisfy building. Well, the kernel version for blackfin sets ALIGN/ALIGN_STR, so are they out of sync? the overall linkage.h concept is the same, but we've unified things better in the u-boot code. Linux's common linkage.h has x86-centric defaults which we specifically avoided in the u-boot version. we might want to tweak the ENDPROC() in u-boot's linkage.h to use % rather than @ since the latter is a comment char in arm asm and the former should work for all the arches i know of just the same as @. i expect the arm guys to submit a patch though at the same time they add a stub asm/linkage.h ;). -mike signature.asc Description: This is a digitally signed message part. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 2/4] arm: add %function attribute to assembly functions
On Tuesday 21 February 2012 09:12 PM, Mike Frysinger wrote: On Tuesday 21 February 2012 09:33:31 Tom Rini wrote: On Mon, Feb 20, 2012 at 11:19:14PM -0500, Mike Frysinger wrote: On Monday 20 February 2012 16:53:40 Simon Glass wrote: On Mon, Feb 20, 2012 at 12:07 PM, Tom Rini wrote: On Sun, Feb 19, 2012 at 02:15:30AM -0500, Mike Frysinger wrote: On Saturday 18 February 2012 17:03:59 Simon Glass wrote: On Wed, Feb 15, 2012 at 5:57 AM, Aneesh V wrote: -.globl reset_cpu +.type reset_cpu, %function +.globalreset_cpu Should we introduce a macro to deal with this rather than writing it out each time? EXPORT()? we have it already with the linux/linkage.h header :) Well, unless my tree is out of date (or stuff is in-flight) we don't have the full compliment here. We havelinux/linkage.h for all and asm/linkage.h for bfin. That said, yes, we should grab at least the ARM version and make use of ENTRY/END_FUNC ala the kernel. I'm behind on my convert __attribute__((...)) to __attr series already or I'd say more :) In case one of you is going to look at this, can we try to use asm-generic as much as possible? i don't know what you mean ... we already have linux/linkage.h with sane defaults for pretty much everyone. the Blackfin asm/linkage.h is an empty file to satisfy building. Well, the kernel version for blackfin sets ALIGN/ALIGN_STR, so are they out of sync? the overall linkage.h concept is the same, but we've unified things better in the u-boot code. Linux's common linkage.h has x86-centric defaults which we specifically avoided in the u-boot version. we might want to tweak the ENDPROC() in u-boot's linkage.h to use % rather than @ since the latter is a comment char in arm asm and the former should work for all the arches i know of just the same as @. i expect the arm guys to submit a patch though at the same time they add a stub asm/linkage.h ;). I was planning to do that in the next revision of this series. BTW, I guess the following in the arm linkage.h of kernel is useful too. Any thoughts? #define __ALIGN .align 0 #define __ALIGN_STR .align 0 BTW, I am not volunteering to convert all the assembly functions in ARM to the new format:) I shall do it for armv7. br, Aneesh ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 2/4] arm: add %function attribute to assembly functions
On Tuesday 21 February 2012 13:03:55 Aneesh V wrote: I was planning to do that in the next revision of this series. BTW, I guess the following in the arm linkage.h of kernel is useful too. Any thoughts? #define __ALIGN .align 0 #define __ALIGN_STR .align 0 arm allows unaligned instruction execution ? i would have thought it'd require higher alignment than that. i don't think that'd be a good default for everyone ... -mike signature.asc Description: This is a digitally signed message part. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 2/4] arm: add %function attribute to assembly functions
On Wednesday 22 February 2012 12:58 AM, Mike Frysinger wrote: On Tuesday 21 February 2012 13:03:55 Aneesh V wrote: I was planning to do that in the next revision of this series. BTW, I guess the following in the arm linkage.h of kernel is useful too. Any thoughts? #define __ALIGN .align 0 #define __ALIGN_STR .align 0 arm allows unaligned instruction execution ? i would have thought it'd require higher alignment than that. i don't think that'd be a good default for everyone ... Unaligned instruction execution - No. ARM instruction should be 4 byte aligned and Thumb instruction should be 2 byte aligned. Unaligned data access - to a certain extent yes and is programmable. IIRC, access to 32 bit integer is possible at 2 byte boundary but not at an odd address. Yes, I was also a little puzzled by that one. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 2/4] arm: add %function attribute to assembly functions
On Saturday 18 February 2012 10:18 PM, Albert ARIBAUD wrote: Hi Aneesh, [...] I will get back with more details on the Linaro GCC 2012.01 later. I meant the Linaro GCC 2012.01 tool-chain problem This is a different problem. Some of the .rodata symbols are given an odd address although they should be aligned to at least 2-byte boundary ). In fact the data is actually put at the even address but the symbol's value is +1 of the actual address. This is the ARM convention for Thumb functions, but they have applied it here for data too. That's the problem. I see that this doesn't happen to all the .rodata in SPL. Neither could I reproduce it with a small program. But the workaround for this problem is to avoid -fdata-sections. The following patch works around it. diff --git a/config.mk b/config.mk index ddaa477..723286a 100644 --- a/config.mk +++ b/config.mk @@ -190,7 +190,7 @@ CPPFLAGS := $(DBGFLAGS) $(OPTFLAGS) $(RELFLAGS) \ # Enable garbage collection of un-used sections for SPL ifeq ($(CONFIG_SPL_BUILD),y) -CPPFLAGS += -ffunction-sections -fdata-sections +CPPFLAGS += -ffunction-sections LDFLAGS_FINAL += --gc-sections endif Will you take a patch to make -fdata-sections optional, that is, having it under something like CONFIG_SYS_SPL_NO_FDATA_SECTIONS? Hmm... considering you're seeing the issue in a fairly new toolchain release, I prefer notifying the toolchain makers, rather than removing the -fdata-sections from SPL even for specific boards. Can you go and see why the Linaro toolchain generated odd thumb data at all and get this fixed? I tried investigating a bit. As I mentioned earlier it doesn't happen with some other files that use the same compiler and linker commands. So, I don't know what's going on. Also, I couldn't reproduce it with a simple program unlike in the other cases. Anyway, I have notified tool-chain folks at Linaro: http://article.gmane.org/gmane.linux.linaro.toolchain/2096 br, Aneesh ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 2/4] arm: add %function attribute to assembly functions
On Sun, Feb 19, 2012 at 02:15:30AM -0500, Mike Frysinger wrote: On Saturday 18 February 2012 17:03:59 Simon Glass wrote: On Wed, Feb 15, 2012 at 5:57 AM, Aneesh V wrote: -.globl reset_cpu +.type reset_cpu, %function +.globalreset_cpu Should we introduce a macro to deal with this rather than writing it out each time? EXPORT()? we have it already with the linux/linkage.h header :) Well, unless my tree is out of date (or stuff is in-flight) we don't have the full compliment here. We have linux/linkage.h for all and asm/linkage.h for bfin. That said, yes, we should grab at least the ARM version and make use of ENTRY/END_FUNC ala the kernel. I'm behind on my convert __attribute__((...)) to __attr series already or I'd say more :) -- Tom ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 2/4] arm: add %function attribute to assembly functions
Hi Tom, Aneesh, On Mon, Feb 20, 2012 at 12:07 PM, Tom Rini tr...@ti.com wrote: On Sun, Feb 19, 2012 at 02:15:30AM -0500, Mike Frysinger wrote: On Saturday 18 February 2012 17:03:59 Simon Glass wrote: On Wed, Feb 15, 2012 at 5:57 AM, Aneesh V wrote: -.globl reset_cpu +.type reset_cpu, %function +.global reset_cpu Should we introduce a macro to deal with this rather than writing it out each time? EXPORT()? we have it already with the linux/linkage.h header :) Well, unless my tree is out of date (or stuff is in-flight) we don't have the full compliment here. We have linux/linkage.h for all and asm/linkage.h for bfin. That said, yes, we should grab at least the ARM version and make use of ENTRY/END_FUNC ala the kernel. I'm behind on my convert __attribute__((...)) to __attr series already or I'd say more :) In case one of you is going to look at this, can we try to use asm-generic as much as possible? -- Tom ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot Regards, Simon ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 2/4] arm: add %function attribute to assembly functions
On Monday 20 February 2012 15:07:46 Tom Rini wrote: On Sun, Feb 19, 2012 at 02:15:30AM -0500, Mike Frysinger wrote: On Saturday 18 February 2012 17:03:59 Simon Glass wrote: On Wed, Feb 15, 2012 at 5:57 AM, Aneesh V wrote: -.globl reset_cpu +.type reset_cpu, %function +.globalreset_cpu Should we introduce a macro to deal with this rather than writing it out each time? EXPORT()? we have it already with the linux/linkage.h header :) Well, unless my tree is out of date (or stuff is in-flight) we don't have the full compliment here. We have linux/linkage.h for all and asm/linkage.h for bfin. That said, yes, we should grab at least the ARM version and make use of ENTRY/END_FUNC ala the kernel. I'm behind on my convert __attribute__((...)) to __attr series already or I'd say more :) yes, each arch is responsible for bringing in their asm/linkage.h needs into u-boot. makes more sense to me to do that in arm than trying to hand modify a bunch of random funcs that people happen to notice issues with. -mike signature.asc Description: This is a digitally signed message part. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 2/4] arm: add %function attribute to assembly functions
On Monday 20 February 2012 16:53:40 Simon Glass wrote: On Mon, Feb 20, 2012 at 12:07 PM, Tom Rini wrote: On Sun, Feb 19, 2012 at 02:15:30AM -0500, Mike Frysinger wrote: On Saturday 18 February 2012 17:03:59 Simon Glass wrote: On Wed, Feb 15, 2012 at 5:57 AM, Aneesh V wrote: -.globl reset_cpu +.type reset_cpu, %function +.globalreset_cpu Should we introduce a macro to deal with this rather than writing it out each time? EXPORT()? we have it already with the linux/linkage.h header :) Well, unless my tree is out of date (or stuff is in-flight) we don't have the full compliment here. We have linux/linkage.h for all and asm/linkage.h for bfin. That said, yes, we should grab at least the ARM version and make use of ENTRY/END_FUNC ala the kernel. I'm behind on my convert __attribute__((...)) to __attr series already or I'd say more :) In case one of you is going to look at this, can we try to use asm-generic as much as possible? i don't know what you mean ... we already have linux/linkage.h with sane defaults for pretty much everyone. the Blackfin asm/linkage.h is an empty file to satisfy building. -mike signature.asc Description: This is a digitally signed message part. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 2/4] arm: add %function attribute to assembly functions
Hi Mike, On Mon, Feb 20, 2012 at 8:19 PM, Mike Frysinger vap...@gentoo.org wrote: On Monday 20 February 2012 16:53:40 Simon Glass wrote: On Mon, Feb 20, 2012 at 12:07 PM, Tom Rini wrote: On Sun, Feb 19, 2012 at 02:15:30AM -0500, Mike Frysinger wrote: On Saturday 18 February 2012 17:03:59 Simon Glass wrote: On Wed, Feb 15, 2012 at 5:57 AM, Aneesh V wrote: -.globl reset_cpu +.type reset_cpu, %function +.global reset_cpu Should we introduce a macro to deal with this rather than writing it out each time? EXPORT()? we have it already with the linux/linkage.h header :) Well, unless my tree is out of date (or stuff is in-flight) we don't have the full compliment here. We have linux/linkage.h for all and asm/linkage.h for bfin. That said, yes, we should grab at least the ARM version and make use of ENTRY/END_FUNC ala the kernel. I'm behind on my convert __attribute__((...)) to __attr series already or I'd say more :) In case one of you is going to look at this, can we try to use asm-generic as much as possible? i don't know what you mean ... we already have linux/linkage.h with sane defaults for pretty much everyone. the Blackfin asm/linkage.h is an empty file to satisfy building. That's fine then, thanks. -mike Regards Simon ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 2/4] arm: add %function attribute to assembly functions
Hi Aneesh, Le 17/02/2012 12:09, Aneesh V a écrit : Hi Albert, On Wednesday 15 February 2012 07:27 PM, Aneesh V wrote: This is done using the following directive preceding each function definition: .typefunc-name, %function This marks the symbol as a function in the object header which in turn helps the linker in some cases. In particular this was found needed for resolving ARM/Thumb calls correctly in a build with Thumb interworking enabled. This solves the following problem I had reported earlier: When U-Boot/SPL is built using the Thumb instruction set the toolchain has a potential issue with weakly linked symbols. If a function has a weakly linked default implementation in C and a real implementation in assembly GCC is confused about the instruction set of the assembly implementation. As a result the assembly function that is built in ARM is executed as if it is Thumb. This results in a crash Signed-off-by: Aneesh Vane...@ti.com Does this look good to you. I was a bit nervous about touching so many files. Please let me know if you would prefer to change only the OMAP function that was creating the ARM/Thumb problem. I did a MAKEALL -a arm and didn't see any new errors. Let me know if this is an acceptable solution to the problem. Regarding the solution: it is quite ok to me. I would just like to understand the exact effect of the .function directive, what its options are and if some of these should not be explicitly specified. Regarding touching many files: I won't be worried as long as you check that the first three patches have no effect on existing boards. This can be verified as follows -- if you haven't done so already: - build your OMAP target without the patch set and do a hex dump of u-boot.bin; - apply the first three patches of your set, rebuild your OMAP target without the patch set and do a hex dump of u-boot.bin; - compare both dumps. Normally you should only see one difference, in the build version and date -- if .function does not actually alter the assembly code, which I hope it indeed does not when building for ARM. If there are more changes than build version and date, then they might be due to .function requiring some yet unknown additional option, or to some change in patch 1 or 3 not being completely conditioned on CONFIG_SYS_THUMB_BUILD. regards, Aneesh Amicalement, -- Albert. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 2/4] arm: add %function attribute to assembly functions
On Friday 17 February 2012 10:43 PM, Mike Frysinger wrote: On Wednesday 15 February 2012 08:57:31 Aneesh V wrote: This is done using the following directive preceding each function definition: .typefunc-name, %function This marks the symbol as a function in the object header which in turn helps the linker in some cases. In particular this was found needed for resolving ARM/Thumb calls correctly in a build with Thumb interworking enabled. ideally arm would use the new linkage.h header and then these would change to doing what Linux does: ENTRY(foo) asm ENDPROC(foo) Thanks for the info. So, what do you suggest? Fix only the problematic function and leave the rest to be converted to the above form later? br, Aneesh ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 2/4] arm: add %function attribute to assembly functions
Hi Aneesh, Le 18/02/2012 12:12, Aneesh V a écrit : On Friday 17 February 2012 10:43 PM, Mike Frysinger wrote: On Wednesday 15 February 2012 08:57:31 Aneesh V wrote: This is done using the following directive preceding each function definition: .typefunc-name, %function This marks the symbol as a function in the object header which in turn helps the linker in some cases. In particular this was found needed for resolving ARM/Thumb calls correctly in a build with Thumb interworking enabled. ideally arm would use the new linkage.h header and then these would change to doing what Linux does: ENTRY(foo) asm ENDPROC(foo) Thanks for the info. So, what do you suggest? Fix only the problematic function and leave the rest to be converted to the above form later? Please at the very least do not leave any file half-baked, with some symbols fixed and other not. Any file you start fixing should be entirely fixed. Regarding fixing other ASM files unrelated to your patch set, then no, you don't need to fix them. Others will have to do any outstanding fixes them when they add Thumb support for their own target. Amicalement, -- Albert. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 2/4] arm: add %function attribute to assembly functions
Hi Albert,
On Saturday 18 February 2012 03:43 PM, Albert ARIBAUD wrote:
Hi Aneesh,
Le 17/02/2012 12:09, Aneesh V a écrit :
Hi Albert,
On Wednesday 15 February 2012 07:27 PM, Aneesh V wrote:
This is done using the following directive preceding
each function definition:
.typefunc-name, %function
This marks the symbol as a function in the object
header which in turn helps the linker in some cases.
In particular this was found needed for resolving ARM/Thumb
calls correctly in a build with Thumb interworking enabled.
This solves the following problem I had reported earlier:
When U-Boot/SPL is built using the Thumb instruction set the
toolchain has a potential issue with weakly linked symbols.
If a function has a weakly linked default implementation in C
and a real implementation in assembly GCC is confused about the
instruction set of the assembly implementation. As a result
the assembly function that is built in ARM is executed as
if it is Thumb. This results in a crash
Signed-off-by: Aneesh Vane...@ti.com
Does this look good to you. I was a bit nervous about touching so many
files. Please let me know if you would prefer to change only the OMAP
function that was creating the ARM/Thumb problem. I did a MAKEALL -a
arm and didn't see any new errors.
Let me know if this is an acceptable solution to the problem.
Regarding the solution: it is quite ok to me. I would just like to
understand the exact effect of the .function directive, what its options
are and if some of these should not be explicitly specified.
Regarding touching many files: I won't be worried as long as you check
that the first three patches have no effect on existing boards. This can
be verified as follows -- if you haven't done so already:
- build your OMAP target without the patch set and do a hex dump of
u-boot.bin;
- apply the first three patches of your set, rebuild your OMAP target
without the patch set and do a hex dump of u-boot.bin;
- compare both dumps. Normally you should only see one difference, in
the build version and date -- if .function does not actually alter the
assembly code, which I hope it indeed does not when building for ARM.
If there are more changes than build version and date, then they might
be due to .function requiring some yet unknown additional option, or to
some change in patch 1 or 3 not being completely conditioned on
CONFIG_SYS_THUMB_BUILD.
I can reproduce the problem with a simple test program.
Note: I can reproduce this with Sourcery G++ Lite 2010q1-202 (GCC 4.4.1
- Binutils 2.19.51.20090709)
But I *can not* reproduce reproduce this with Linaro GCC 2012.01 (GCC
4.6.3 , Binutils 2.22)
So apparently the issue has been fixed recently. Unfortunately Linaro
GCC 2012.01 creates a new Thumb problem that I am investigating now.
Somehow I missed this when I tested earlier. So, my Thumb build is
not working with Linaro GCC 2012.01. But this one is not reproduced on
Sourcery G++ Lite 2010q1-202!
Here is the program I used to reproduce the problem in Sourcery G++
Lite 2010q1-202 that this patch is addressing
a.c:
extern void foo (void) __attribute__ ((weak, alias (__foo)));
void __foo (void)
{
}
extern void call_foo(void);
int main (void)
{
call_foo ();
}
b.S:
.text
.align 2
.global foo
foo:
push{r7}
add r7, sp, #0
mov sp, r7
pop {r7}
bx lr
.size foo, .-foo
c.S:
.text
.align 2
.global call_foo
call_foo:
bl foo
bx lr
.global __aeabi_unwind_cpp_pr0
__aeabi_unwind_cpp_pr0:
bx lr
Now build it and take the assembly dump using the following commands:
arm-none-linux-gnueabi-gcc -mthumb -mthumb-interwork -c a.c
arm-none-linux-gnueabi-gcc -mthumb -mthumb-interwork -c b.S
arm-none-linux-gnueabi-gcc -mthumb -mthumb-interwork -c c.S
arm-none-linux-gnueabi-ld -r a.o -o alib.o
arm-none-linux-gnueabi-ld -r b.o -o blib.o
arm-none-linux-gnueabi-ld -r c.o -o clib.o
arm-none-linux-gnueabi-ld --start-group clib.o alib.o blib.o
--end-group -o a.out
arm-none-linux-gnueabi-objdump -S --reloc a.out
You will get something like this in the assembly dump:
8094 call_foo:
8094: fa06blx 80b4 foo
8098: e12fff1ebx lr
The blx is wrong as we are jumping to an ARM function from ARM.
Now if you change b.S like this:
.text
.align 2
+.type foo, %function
.global foo
foo:
push{r7}
And compile it again in the same way you will see:
8094 call_foo:
8094: eb06bl 80b4 foo
8098: e12fff1ebx lr
Please note that the branch to foo is correct now.
I hope this convinces you that %function indeed has an effect.
I will get back with more details on the Linaro GCC 2012.01 later.
br,
Aneesh
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 2/4] arm: add %function attribute to assembly functions
Hi Aneesh,
Le 18/02/2012 14:24, Aneesh V a écrit :
Hi Albert,
On Saturday 18 February 2012 03:43 PM, Albert ARIBAUD wrote:
Hi Aneesh,
Le 17/02/2012 12:09, Aneesh V a écrit :
Hi Albert,
On Wednesday 15 February 2012 07:27 PM, Aneesh V wrote:
This is done using the following directive preceding
each function definition:
.typefunc-name, %function
This marks the symbol as a function in the object
header which in turn helps the linker in some cases.
In particular this was found needed for resolving ARM/Thumb
calls correctly in a build with Thumb interworking enabled.
This solves the following problem I had reported earlier:
When U-Boot/SPL is built using the Thumb instruction set the
toolchain has a potential issue with weakly linked symbols.
If a function has a weakly linked default implementation in C
and a real implementation in assembly GCC is confused about the
instruction set of the assembly implementation. As a result
the assembly function that is built in ARM is executed as
if it is Thumb. This results in a crash
Signed-off-by: Aneesh Vane...@ti.com
Does this look good to you. I was a bit nervous about touching so many
files. Please let me know if you would prefer to change only the OMAP
function that was creating the ARM/Thumb problem. I did a MAKEALL -a
arm and didn't see any new errors.
Let me know if this is an acceptable solution to the problem.
Regarding the solution: it is quite ok to me. I would just like to
understand the exact effect of the .function directive, what its options
are and if some of these should not be explicitly specified.
Regarding touching many files: I won't be worried as long as you check
that the first three patches have no effect on existing boards. This can
be verified as follows -- if you haven't done so already:
- build your OMAP target without the patch set and do a hex dump of
u-boot.bin;
- apply the first three patches of your set, rebuild your OMAP target
without the patch set and do a hex dump of u-boot.bin;
- compare both dumps. Normally you should only see one difference, in
the build version and date -- if .function does not actually alter the
assembly code, which I hope it indeed does not when building for ARM.
If there are more changes than build version and date, then they might
be due to .function requiring some yet unknown additional option, or to
some change in patch 1 or 3 not being completely conditioned on
CONFIG_SYS_THUMB_BUILD.
I can reproduce the problem with a simple test program.
Note: I can reproduce this with Sourcery G++ Lite 2010q1-202 (GCC 4.4.1
- Binutils 2.19.51.20090709)
But I *can not* reproduce reproduce this with Linaro GCC 2012.01 (GCC
4.6.3 , Binutils 2.22)
So apparently the issue has been fixed recently. Unfortunately Linaro
GCC 2012.01 creates a new Thumb problem that I am investigating now.
Somehow I missed this when I tested earlier. So, my Thumb build is
not working with Linaro GCC 2012.01. But this one is not reproduced on
Sourcery G++ Lite 2010q1-202!
Here is the program I used to reproduce the problem in Sourcery G++
Lite 2010q1-202 that this patch is addressing
a.c:
extern void foo (void) __attribute__ ((weak, alias (__foo)));
void __foo (void)
{
}
extern void call_foo(void);
int main (void)
{
call_foo ();
}
b.S:
.text
.align 2
.global foo
foo:
push {r7}
add r7, sp, #0
mov sp, r7
pop {r7}
bx lr
.size foo, .-foo
c.S:
.text
.align 2
.global call_foo
call_foo:
bl foo
bx lr
.global __aeabi_unwind_cpp_pr0
__aeabi_unwind_cpp_pr0:
bx lr
Now build it and take the assembly dump using the following commands:
arm-none-linux-gnueabi-gcc -mthumb -mthumb-interwork -c a.c
arm-none-linux-gnueabi-gcc -mthumb -mthumb-interwork -c b.S
arm-none-linux-gnueabi-gcc -mthumb -mthumb-interwork -c c.S
arm-none-linux-gnueabi-ld -r a.o -o alib.o
arm-none-linux-gnueabi-ld -r b.o -o blib.o
arm-none-linux-gnueabi-ld -r c.o -o clib.o
arm-none-linux-gnueabi-ld --start-group clib.o alib.o blib.o --end-group
-o a.out
arm-none-linux-gnueabi-objdump -S --reloc a.out
You will get something like this in the assembly dump:
8094 call_foo:
8094: fa06 blx 80b4 foo
8098: e12fff1e bx lr
The blx is wrong as we are jumping to an ARM function from ARM.
Now if you change b.S like this:
.text
.align 2
+.type foo, %function
.global foo
foo:
push {r7}
And compile it again in the same way you will see:
8094 call_foo:
8094: eb06 bl 80b4 foo
8098: e12fff1e bx lr
Please note that the branch to foo is correct now.
I hope this convinces you that %function indeed has an effect.
This convinces me that .function as you used it had the effect that you
needed for Thumb compilation, but I had no doubts about that. What I
want to be sure is that it also has no other, unexpected, effect,
especially when still building ARM code only; and the test I suggest
would demonstrate this. :)
I will get back with more details on the Linaro GCC 2012.01 later.
Thanks. Anyway, this patch series is not going to end up
Re: [U-Boot] [PATCH 2/4] arm: add %function attribute to assembly functions
On Saturday 18 February 2012 06:54 PM, Aneesh V wrote:
Hi Albert,
On Saturday 18 February 2012 03:43 PM, Albert ARIBAUD wrote:
Hi Aneesh,
Le 17/02/2012 12:09, Aneesh V a écrit :
Hi Albert,
On Wednesday 15 February 2012 07:27 PM, Aneesh V wrote:
This is done using the following directive preceding
each function definition:
.typefunc-name, %function
This marks the symbol as a function in the object
header which in turn helps the linker in some cases.
In particular this was found needed for resolving ARM/Thumb
calls correctly in a build with Thumb interworking enabled.
This solves the following problem I had reported earlier:
When U-Boot/SPL is built using the Thumb instruction set the
toolchain has a potential issue with weakly linked symbols.
If a function has a weakly linked default implementation in C
and a real implementation in assembly GCC is confused about the
instruction set of the assembly implementation. As a result
the assembly function that is built in ARM is executed as
if it is Thumb. This results in a crash
Signed-off-by: Aneesh Vane...@ti.com
Does this look good to you. I was a bit nervous about touching so many
files. Please let me know if you would prefer to change only the OMAP
function that was creating the ARM/Thumb problem. I did a MAKEALL -a
arm and didn't see any new errors.
Let me know if this is an acceptable solution to the problem.
Regarding the solution: it is quite ok to me. I would just like to
understand the exact effect of the .function directive, what its options
are and if some of these should not be explicitly specified.
Regarding touching many files: I won't be worried as long as you check
that the first three patches have no effect on existing boards. This can
be verified as follows -- if you haven't done so already:
- build your OMAP target without the patch set and do a hex dump of
u-boot.bin;
- apply the first three patches of your set, rebuild your OMAP target
without the patch set and do a hex dump of u-boot.bin;
- compare both dumps. Normally you should only see one difference, in
the build version and date -- if .function does not actually alter the
assembly code, which I hope it indeed does not when building for ARM.
If there are more changes than build version and date, then they might
be due to .function requiring some yet unknown additional option, or to
some change in patch 1 or 3 not being completely conditioned on
CONFIG_SYS_THUMB_BUILD.
I can reproduce the problem with a simple test program.
Note: I can reproduce this with Sourcery G++ Lite 2010q1-202 (GCC 4.4.1
- Binutils 2.19.51.20090709)
But I *can not* reproduce reproduce this with Linaro GCC 2012.01 (GCC
4.6.3 , Binutils 2.22)
Linaro GCC 2012.01 has the same problem when assembly function(ARM is
called from C (Thumb). I can reproduce it using this program:
a.c:
int main (void)
{
foo ();
}
b.S:
.text
.align 2
.global foo
foo:
push{r7}
add r7, sp, #0
mov sp, r7
pop {r7}
bx lr
.size foo, .-foo
.global __aeabi_unwind_cpp_pr0
__aeabi_unwind_cpp_pr0:
bx lr
arm-linux-gnueabi-gcc -mthumb -mthumb-interwork -c a.c
arm-linux-gnueabi-gcc -mthumb -mthumb-interwork -c b.S
arm-linux-gnueabi-ld -r a.o -o alib.o
arm-linux-gnueabi-ld -r b.o -o blib.o
arm-linux-gnueabi-ld --start-group alib.o blib.o --end-group -o a.out
arm-linux-gnueabi-objdump -S --reloc a.out
gives:
8076: af00add r7, sp, #0
8078: f000 f802 bl 8080 foo
807c: 4618mov r0, r3
It should have been blx foo
Again, %function solves it. Conclusion: %function is necessary with
both old and new tool-chains when building for Thumb.
It should have been blx 8080 foo, isn't it? Again, %function
solves it.
Conclusion: %function is necessary with both old and new tool-chains
when building for Thumb.
So apparently the issue has been fixed recently. Unfortunately Linaro
GCC 2012.01 creates a new Thumb problem that I am investigating now.
Somehow I missed this when I tested earlier. So, my Thumb build is
not working with Linaro GCC 2012.01. But this one is not reproduced on
Sourcery G++ Lite 2010q1-202!
Here is the program I used to reproduce the problem in Sourcery G++
Lite 2010q1-202 that this patch is addressing
a.c:
extern void foo (void) __attribute__ ((weak, alias (__foo)));
void __foo (void)
{
}
extern void call_foo(void);
int main (void)
{
call_foo ();
}
b.S:
.text
.align 2
.global foo
foo:
push {r7}
add r7, sp, #0
mov sp, r7
pop {r7}
bx lr
.size foo, .-foo
c.S:
.text
.align 2
.global call_foo
call_foo:
bl foo
bx lr
.global __aeabi_unwind_cpp_pr0
__aeabi_unwind_cpp_pr0:
bx lr
Now build it and take the assembly dump using the following commands:
arm-none-linux-gnueabi-gcc -mthumb -mthumb-interwork -c a.c
arm-none-linux-gnueabi-gcc -mthumb -mthumb-interwork -c b.S
arm-none-linux-gnueabi-gcc -mthumb -mthumb-interwork -c c.S
Re: [U-Boot] [PATCH 2/4] arm: add %function attribute to assembly functions
Hi Aneesh,
Le 18/02/2012 17:34, Aneesh V a écrit :
On Saturday 18 February 2012 06:54 PM, Aneesh V wrote:
Hi Albert,
On Saturday 18 February 2012 03:43 PM, Albert ARIBAUD wrote:
Hi Aneesh,
Le 17/02/2012 12:09, Aneesh V a écrit :
Hi Albert,
On Wednesday 15 February 2012 07:27 PM, Aneesh V wrote:
This is done using the following directive preceding
each function definition:
.typefunc-name, %function
This marks the symbol as a function in the object
header which in turn helps the linker in some cases.
In particular this was found needed for resolving ARM/Thumb
calls correctly in a build with Thumb interworking enabled.
This solves the following problem I had reported earlier:
When U-Boot/SPL is built using the Thumb instruction set the
toolchain has a potential issue with weakly linked symbols.
If a function has a weakly linked default implementation in C
and a real implementation in assembly GCC is confused about the
instruction set of the assembly implementation. As a result
the assembly function that is built in ARM is executed as
if it is Thumb. This results in a crash
Signed-off-by: Aneesh Vane...@ti.com
Does this look good to you. I was a bit nervous about touching so many
files. Please let me know if you would prefer to change only the OMAP
function that was creating the ARM/Thumb problem. I did a MAKEALL -a
arm and didn't see any new errors.
Let me know if this is an acceptable solution to the problem.
Regarding the solution: it is quite ok to me. I would just like to
understand the exact effect of the .function directive, what its options
are and if some of these should not be explicitly specified.
Regarding touching many files: I won't be worried as long as you check
that the first three patches have no effect on existing boards. This can
be verified as follows -- if you haven't done so already:
- build your OMAP target without the patch set and do a hex dump of
u-boot.bin;
- apply the first three patches of your set, rebuild your OMAP target
without the patch set and do a hex dump of u-boot.bin;
- compare both dumps. Normally you should only see one difference, in
the build version and date -- if .function does not actually alter the
assembly code, which I hope it indeed does not when building for ARM.
If there are more changes than build version and date, then they might
be due to .function requiring some yet unknown additional option, or to
some change in patch 1 or 3 not being completely conditioned on
CONFIG_SYS_THUMB_BUILD.
I can reproduce the problem with a simple test program.
Note: I can reproduce this with Sourcery G++ Lite 2010q1-202 (GCC 4.4.1
- Binutils 2.19.51.20090709)
But I *can not* reproduce reproduce this with Linaro GCC 2012.01 (GCC
4.6.3 , Binutils 2.22)
Linaro GCC 2012.01 has the same problem when assembly function(ARM is
called from C (Thumb). I can reproduce it using this program:
a.c:
int main (void)
{
foo ();
}
b.S:
.text
.align 2
.global foo
foo:
push {r7}
add r7, sp, #0
mov sp, r7
pop {r7}
bx lr
.size foo, .-foo
.global __aeabi_unwind_cpp_pr0
__aeabi_unwind_cpp_pr0:
bx lr
arm-linux-gnueabi-gcc -mthumb -mthumb-interwork -c a.c
arm-linux-gnueabi-gcc -mthumb -mthumb-interwork -c b.S
arm-linux-gnueabi-ld -r a.o -o alib.o
arm-linux-gnueabi-ld -r b.o -o blib.o
arm-linux-gnueabi-ld --start-group alib.o blib.o --end-group -o a.out
arm-linux-gnueabi-objdump -S --reloc a.out
gives:
8076: af00 add r7, sp, #0
8078: f000 f802 bl 8080 foo
807c: 4618 mov r0, r3
It should have been blx foo
Again, %function solves it. Conclusion: %function is necessary with
both old and new tool-chains when building for Thumb.
It should have been blx 8080 foo, isn't it? Again, %function
solves it.
Conclusion: %function is necessary with both old and new tool-chains
when building for Thumb.
So apparently the issue has been fixed recently. Unfortunately Linaro
GCC 2012.01 creates a new Thumb problem that I am investigating now.
Somehow I missed this when I tested earlier. So, my Thumb build is
not working with Linaro GCC 2012.01. But this one is not reproduced on
Sourcery G++ Lite 2010q1-202!
Here is the program I used to reproduce the problem in Sourcery G++
Lite 2010q1-202 that this patch is addressing
a.c:
extern void foo (void) __attribute__ ((weak, alias (__foo)));
void __foo (void)
{
}
extern void call_foo(void);
int main (void)
{
call_foo ();
}
b.S:
.text
.align 2
.global foo
foo:
push {r7}
add r7, sp, #0
mov sp, r7
pop {r7}
bx lr
.size foo, .-foo
c.S:
.text
.align 2
.global call_foo
call_foo:
bl foo
bx lr
.global __aeabi_unwind_cpp_pr0
__aeabi_unwind_cpp_pr0:
bx lr
Now build it and take the assembly dump using the following commands:
arm-none-linux-gnueabi-gcc -mthumb -mthumb-interwork -c a.c
arm-none-linux-gnueabi-gcc -mthumb -mthumb-interwork -c b.S
arm-none-linux-gnueabi-gcc -mthumb -mthumb-interwork -c c.S
arm-none-linux-gnueabi-ld -r a.o -o alib.o
arm-none-linux-gnueabi-ld -r b.o -o blib.o
arm-none-linux-gnueabi-ld
Re: [U-Boot] [PATCH 2/4] arm: add %function attribute to assembly functions
Hi Anesh, On Wed, Feb 15, 2012 at 5:57 AM, Aneesh V ane...@ti.com wrote: This is done using the following directive preceding each function definition: .type func-name, %function This marks the symbol as a function in the object header which in turn helps the linker in some cases. In particular this was found needed for resolving ARM/Thumb calls correctly in a build with Thumb interworking enabled. This solves the following problem I had reported earlier: When U-Boot/SPL is built using the Thumb instruction set the toolchain has a potential issue with weakly linked symbols. If a function has a weakly linked default implementation in C and a real implementation in assembly GCC is confused about the instruction set of the assembly implementation. As a result the assembly function that is built in ARM is executed as if it is Thumb. This results in a crash Signed-off-by: Aneesh V ane...@ti.com --- Changes from RFC to V1: - This change completely replaces the previous workaround for the ARM/Thumb interwork problem, which was to wrap around the assembly function in question with a naked C function --- arch/arm/cpu/arm1136/omap24xx/reset.S | 3 +- arch/arm/cpu/arm1136/start.S | 7 +++- arch/arm/cpu/arm1176/s3c64xx/cpu_init.S | 3 +- arch/arm/cpu/arm1176/s3c64xx/reset.S | 3 +- arch/arm/cpu/arm1176/start.S | 9 +++-- arch/arm/cpu/arm1176/tnetv107x/lowlevel_init.S | 3 +- arch/arm/cpu/arm720t/lpc2292/iap_entry.S | 3 +- arch/arm/cpu/arm720t/start.S | 12 -- arch/arm/cpu/arm920t/a320/reset.S | 1 + arch/arm/cpu/arm920t/at91/lowlevel_init.S | 3 +- arch/arm/cpu/arm920t/ep93xx/lowlevel_init.S | 3 +- arch/arm/cpu/arm920t/ks8695/lowlevel_init.S | 3 +- arch/arm/cpu/arm920t/start.S | 6 ++- arch/arm/cpu/arm925t/start.S | 9 +++-- arch/arm/cpu/arm926ejs/at91/lowlevel_init.S | 4 +- arch/arm/cpu/arm926ejs/davinci/lowlevel_init.S | 3 +- arch/arm/cpu/arm926ejs/davinci/reset.S | 3 +- arch/arm/cpu/arm926ejs/mx28/start.S | 3 +- arch/arm/cpu/arm926ejs/nomadik/reset.S | 3 +- arch/arm/cpu/arm926ejs/omap/reset.S | 3 +- arch/arm/cpu/arm926ejs/orion5x/lowlevel_init.S | 3 +- arch/arm/cpu/arm926ejs/start.S | 9 +++-- arch/arm/cpu/arm926ejs/versatile/reset.S | 3 +- arch/arm/cpu/arm946es/start.S | 9 +++-- arch/arm/cpu/arm_intcm/start.S | 15 ++-- arch/arm/cpu/armv7/mx5/lowlevel_init.S | 3 +- arch/arm/cpu/armv7/mx6/lowlevel_init.S | 3 +- arch/arm/cpu/armv7/omap-common/lowlevel_init.S | 9 +++-- arch/arm/cpu/armv7/omap-common/reset.S | 1 + arch/arm/cpu/armv7/omap3/lowlevel_init.S | 45 arch/arm/cpu/armv7/s5pc1xx/cache.S | 2 + arch/arm/cpu/armv7/s5pc1xx/reset.S | 3 +- arch/arm/cpu/armv7/start.S | 9 +++-- arch/arm/cpu/armv7/tegra2/lowlevel_init.S | 1 + arch/arm/cpu/armv7/u8500/lowlevel.S | 6 ++- arch/arm/cpu/ixp/start.S | 9 +++-- arch/arm/cpu/lh7a40x/start.S | 9 +++-- arch/arm/cpu/pxa/start.S | 6 ++- arch/arm/cpu/s3c44b0/start.S | 6 ++- arch/arm/cpu/sa1100/start.S | 9 +++-- arch/arm/lib/_ashldi3.S | 6 ++- arch/arm/lib/_ashrdi3.S | 6 ++- arch/arm/lib/_divsi3.S | 6 ++- arch/arm/lib/_lshrdi3.S | 6 ++- arch/arm/lib/_modsi3.S | 3 +- arch/arm/lib/_udivsi3.S | 6 ++- arch/arm/lib/memcpy.S | 3 +- arch/arm/lib/memset.S | 3 +- 48 files changed, 186 insertions(+), 100 deletions(-) diff --git a/arch/arm/cpu/arm1136/omap24xx/reset.S b/arch/arm/cpu/arm1136/omap24xx/reset.S index 5f8343f..917a934 100644 --- a/arch/arm/cpu/arm1136/omap24xx/reset.S +++ b/arch/arm/cpu/arm1136/omap24xx/reset.S @@ -30,7 +30,8 @@ #include asm/arch/omap2420.h -.globl reset_cpu +.type reset_cpu, %function +.global reset_cpu Should we introduce a macro to deal with this rather than writing it out each time? EXPORT()? [snip] Regards, Simon ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 2/4] arm: add %function attribute to assembly functions
On Saturday 18 February 2012 17:03:59 Simon Glass wrote: On Wed, Feb 15, 2012 at 5:57 AM, Aneesh V wrote: -.globl reset_cpu +.type reset_cpu, %function +.globalreset_cpu Should we introduce a macro to deal with this rather than writing it out each time? EXPORT()? we have it already with the linux/linkage.h header :) -mike signature.asc Description: This is a digitally signed message part. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 2/4] arm: add %function attribute to assembly functions
Hi Albert, On Wednesday 15 February 2012 07:27 PM, Aneesh V wrote: This is done using the following directive preceding each function definition: .typefunc-name, %function This marks the symbol as a function in the object header which in turn helps the linker in some cases. In particular this was found needed for resolving ARM/Thumb calls correctly in a build with Thumb interworking enabled. This solves the following problem I had reported earlier: When U-Boot/SPL is built using the Thumb instruction set the toolchain has a potential issue with weakly linked symbols. If a function has a weakly linked default implementation in C and a real implementation in assembly GCC is confused about the instruction set of the assembly implementation. As a result the assembly function that is built in ARM is executed as if it is Thumb. This results in a crash Signed-off-by: Aneesh Vane...@ti.com Does this look good to you. I was a bit nervous about touching so many files. Please let me know if you would prefer to change only the OMAP function that was creating the ARM/Thumb problem. I did a MAKEALL -a arm and didn't see any new errors. Let me know if this is an acceptable solution to the problem. regards, Aneesh --- Changes from RFC to V1: - This change completely replaces the previous workaround for the ARM/Thumb interwork problem, which was to wrap around the assembly function in question with a naked C function --- arch/arm/cpu/arm1136/omap24xx/reset.S |3 +- arch/arm/cpu/arm1136/start.S |7 +++- arch/arm/cpu/arm1176/s3c64xx/cpu_init.S|3 +- arch/arm/cpu/arm1176/s3c64xx/reset.S |3 +- arch/arm/cpu/arm1176/start.S |9 +++-- arch/arm/cpu/arm1176/tnetv107x/lowlevel_init.S |3 +- arch/arm/cpu/arm720t/lpc2292/iap_entry.S |3 +- arch/arm/cpu/arm720t/start.S | 12 -- arch/arm/cpu/arm920t/a320/reset.S |1 + arch/arm/cpu/arm920t/at91/lowlevel_init.S |3 +- arch/arm/cpu/arm920t/ep93xx/lowlevel_init.S|3 +- arch/arm/cpu/arm920t/ks8695/lowlevel_init.S|3 +- arch/arm/cpu/arm920t/start.S |6 ++- arch/arm/cpu/arm925t/start.S |9 +++-- arch/arm/cpu/arm926ejs/at91/lowlevel_init.S|4 +- arch/arm/cpu/arm926ejs/davinci/lowlevel_init.S |3 +- arch/arm/cpu/arm926ejs/davinci/reset.S |3 +- arch/arm/cpu/arm926ejs/mx28/start.S|3 +- arch/arm/cpu/arm926ejs/nomadik/reset.S |3 +- arch/arm/cpu/arm926ejs/omap/reset.S|3 +- arch/arm/cpu/arm926ejs/orion5x/lowlevel_init.S |3 +- arch/arm/cpu/arm926ejs/start.S |9 +++-- arch/arm/cpu/arm926ejs/versatile/reset.S |3 +- arch/arm/cpu/arm946es/start.S |9 +++-- arch/arm/cpu/arm_intcm/start.S | 15 ++-- arch/arm/cpu/armv7/mx5/lowlevel_init.S |3 +- arch/arm/cpu/armv7/mx6/lowlevel_init.S |3 +- arch/arm/cpu/armv7/omap-common/lowlevel_init.S |9 +++-- arch/arm/cpu/armv7/omap-common/reset.S |1 + arch/arm/cpu/armv7/omap3/lowlevel_init.S | 45 arch/arm/cpu/armv7/s5pc1xx/cache.S |2 + arch/arm/cpu/armv7/s5pc1xx/reset.S |3 +- arch/arm/cpu/armv7/start.S |9 +++-- arch/arm/cpu/armv7/tegra2/lowlevel_init.S |1 + arch/arm/cpu/armv7/u8500/lowlevel.S|6 ++- arch/arm/cpu/ixp/start.S |9 +++-- arch/arm/cpu/lh7a40x/start.S |9 +++-- arch/arm/cpu/pxa/start.S |6 ++- arch/arm/cpu/s3c44b0/start.S |6 ++- arch/arm/cpu/sa1100/start.S|9 +++-- arch/arm/lib/_ashldi3.S|6 ++- arch/arm/lib/_ashrdi3.S|6 ++- arch/arm/lib/_divsi3.S |6 ++- arch/arm/lib/_lshrdi3.S|6 ++- arch/arm/lib/_modsi3.S |3 +- arch/arm/lib/_udivsi3.S|6 ++- arch/arm/lib/memcpy.S |3 +- arch/arm/lib/memset.S |3 +- 48 files changed, 186 insertions(+), 100 deletions(-) diff --git a/arch/arm/cpu/arm1136/omap24xx/reset.S b/arch/arm/cpu/arm1136/omap24xx/reset.S index 5f8343f..917a934 100644 --- a/arch/arm/cpu/arm1136/omap24xx/reset.S +++ b/arch/arm/cpu/arm1136/omap24xx/reset.S @@ -30,7 +30,8 @@ #includeasm/arch/omap2420.h -.globl reset_cpu +.type reset_cpu, %function +.globalreset_cpu reset_cpu: ldr r1, rstctl /* get addr for global reset reg */ mov r3, #0x2/* full reset pll+mpu */ diff --git a/arch/arm/cpu/arm1136/start.S b/arch/arm/cpu/arm1136/start.S index c0db96c..972fc0e 100644 ---
Re: [U-Boot] [PATCH 2/4] arm: add %function attribute to assembly functions
On Wednesday 15 February 2012 08:57:31 Aneesh V wrote: This is done using the following directive preceding each function definition: .type func-name, %function This marks the symbol as a function in the object header which in turn helps the linker in some cases. In particular this was found needed for resolving ARM/Thumb calls correctly in a build with Thumb interworking enabled. ideally arm would use the new linkage.h header and then these would change to doing what Linux does: ENTRY(foo) asm ENDPROC(foo) -mike signature.asc Description: This is a digitally signed message part. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 2/4] arm: add %function attribute to assembly functions
This is done using the following directive preceding each function definition: .type func-name, %function This marks the symbol as a function in the object header which in turn helps the linker in some cases. In particular this was found needed for resolving ARM/Thumb calls correctly in a build with Thumb interworking enabled. This solves the following problem I had reported earlier: When U-Boot/SPL is built using the Thumb instruction set the toolchain has a potential issue with weakly linked symbols. If a function has a weakly linked default implementation in C and a real implementation in assembly GCC is confused about the instruction set of the assembly implementation. As a result the assembly function that is built in ARM is executed as if it is Thumb. This results in a crash Signed-off-by: Aneesh V ane...@ti.com --- Changes from RFC to V1: - This change completely replaces the previous workaround for the ARM/Thumb interwork problem, which was to wrap around the assembly function in question with a naked C function --- arch/arm/cpu/arm1136/omap24xx/reset.S |3 +- arch/arm/cpu/arm1136/start.S |7 +++- arch/arm/cpu/arm1176/s3c64xx/cpu_init.S|3 +- arch/arm/cpu/arm1176/s3c64xx/reset.S |3 +- arch/arm/cpu/arm1176/start.S |9 +++-- arch/arm/cpu/arm1176/tnetv107x/lowlevel_init.S |3 +- arch/arm/cpu/arm720t/lpc2292/iap_entry.S |3 +- arch/arm/cpu/arm720t/start.S | 12 -- arch/arm/cpu/arm920t/a320/reset.S |1 + arch/arm/cpu/arm920t/at91/lowlevel_init.S |3 +- arch/arm/cpu/arm920t/ep93xx/lowlevel_init.S|3 +- arch/arm/cpu/arm920t/ks8695/lowlevel_init.S|3 +- arch/arm/cpu/arm920t/start.S |6 ++- arch/arm/cpu/arm925t/start.S |9 +++-- arch/arm/cpu/arm926ejs/at91/lowlevel_init.S|4 +- arch/arm/cpu/arm926ejs/davinci/lowlevel_init.S |3 +- arch/arm/cpu/arm926ejs/davinci/reset.S |3 +- arch/arm/cpu/arm926ejs/mx28/start.S|3 +- arch/arm/cpu/arm926ejs/nomadik/reset.S |3 +- arch/arm/cpu/arm926ejs/omap/reset.S|3 +- arch/arm/cpu/arm926ejs/orion5x/lowlevel_init.S |3 +- arch/arm/cpu/arm926ejs/start.S |9 +++-- arch/arm/cpu/arm926ejs/versatile/reset.S |3 +- arch/arm/cpu/arm946es/start.S |9 +++-- arch/arm/cpu/arm_intcm/start.S | 15 ++-- arch/arm/cpu/armv7/mx5/lowlevel_init.S |3 +- arch/arm/cpu/armv7/mx6/lowlevel_init.S |3 +- arch/arm/cpu/armv7/omap-common/lowlevel_init.S |9 +++-- arch/arm/cpu/armv7/omap-common/reset.S |1 + arch/arm/cpu/armv7/omap3/lowlevel_init.S | 45 arch/arm/cpu/armv7/s5pc1xx/cache.S |2 + arch/arm/cpu/armv7/s5pc1xx/reset.S |3 +- arch/arm/cpu/armv7/start.S |9 +++-- arch/arm/cpu/armv7/tegra2/lowlevel_init.S |1 + arch/arm/cpu/armv7/u8500/lowlevel.S|6 ++- arch/arm/cpu/ixp/start.S |9 +++-- arch/arm/cpu/lh7a40x/start.S |9 +++-- arch/arm/cpu/pxa/start.S |6 ++- arch/arm/cpu/s3c44b0/start.S |6 ++- arch/arm/cpu/sa1100/start.S|9 +++-- arch/arm/lib/_ashldi3.S|6 ++- arch/arm/lib/_ashrdi3.S|6 ++- arch/arm/lib/_divsi3.S |6 ++- arch/arm/lib/_lshrdi3.S|6 ++- arch/arm/lib/_modsi3.S |3 +- arch/arm/lib/_udivsi3.S|6 ++- arch/arm/lib/memcpy.S |3 +- arch/arm/lib/memset.S |3 +- 48 files changed, 186 insertions(+), 100 deletions(-) diff --git a/arch/arm/cpu/arm1136/omap24xx/reset.S b/arch/arm/cpu/arm1136/omap24xx/reset.S index 5f8343f..917a934 100644 --- a/arch/arm/cpu/arm1136/omap24xx/reset.S +++ b/arch/arm/cpu/arm1136/omap24xx/reset.S @@ -30,7 +30,8 @@ #include asm/arch/omap2420.h -.globl reset_cpu +.type reset_cpu, %function +.globalreset_cpu reset_cpu: ldr r1, rstctl /* get addr for global reset reg */ mov r3, #0x2/* full reset pll+mpu */ diff --git a/arch/arm/cpu/arm1136/start.S b/arch/arm/cpu/arm1136/start.S index c0db96c..972fc0e 100644 --- a/arch/arm/cpu/arm1136/start.S +++ b/arch/arm/cpu/arm1136/start.S @@ -31,7 +31,8 @@ #include asm-offsets.h #include config.h #include version.h -.globl _start +.type _start, %function +.global_start _start: b reset #ifdef CONFIG_SPL_BUILD ldr pc, _hang @@ -178,7 +179,8 @@ call_board_init_f: * after relocating the monitor code. * */ - .globl relocate_code +.type relocate_code, %function +.global
