From: dave.martin@linaro•org (Dave Martin)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH 3/3] ARM: tegra: make sure the pointer on 4 byte align when THUMB2_KERNEL enabled
Date: Wed, 17 Apr 2013 11:30:07 +0100 [thread overview]
Message-ID: <20130417102957.GA2249@linaro.org> (raw)
In-Reply-To: <516D7A3B.9030404@wwwdotorg.org>
On Tue, Apr 16, 2013 at 10:20:11AM -0600, Stephen Warren wrote:
> On 04/16/2013 08:13 AM, Dave Martin wrote:
> > On Mon, Apr 15, 2013 at 04:50:55PM -0600, Stephen Warren wrote:
> >> From: Joseph Lo <josephl@nvidia•com>
> >>
> >> When building kernel with CONFIG_THUMB2_KERNEL, the data pointer in the
> >> assembly may not on the 4 byte alignment. Then causing a data abort when
> >> accessing the pointer. This patch add a ".align" flag in the head of the
> >> pointer. And always using 32-bit ADR Thumb instruction to make sure it
> >> won't build failure.
> >>
> >> Signed-off-by: Joseph Lo <josephl@nvidia•com>
> >> Signed-off-by: Stephen Warren <swarren@nvidia•com>
> >> ---
> >> arch/arm/mach-tegra/reset-handler.S | 1 +
> >> arch/arm/mach-tegra/sleep.h | 3 ++-
> >> 2 files changed, 3 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/arch/arm/mach-tegra/reset-handler.S b/arch/arm/mach-tegra/reset-handler.S
> >> index e6de88a..519a8c5 100644
> >> --- a/arch/arm/mach-tegra/reset-handler.S
> >> +++ b/arch/arm/mach-tegra/reset-handler.S
> >> @@ -83,6 +83,7 @@ ENDPROC(tegra_resume)
> >>
> >> #ifdef CONFIG_CACHE_L2X0
> >> .globl l2x0_saved_regs_addr
> >> + .align
> >> l2x0_saved_regs_addr:
> >> .long 0
> >> #endif
> >> diff --git a/arch/arm/mach-tegra/sleep.h b/arch/arm/mach-tegra/sleep.h
> >> index 4ffae54..8e9b6af 100644
> >> --- a/arch/arm/mach-tegra/sleep.h
> >> +++ b/arch/arm/mach-tegra/sleep.h
> >> @@ -92,7 +92,8 @@
> >>
> >> #ifdef CONFIG_CACHE_L2X0
> >> .macro l2_cache_resume, tmp1, tmp2, tmp3, phys_l2x0_saved_regs
> >> - adr \tmp1, \phys_l2x0_saved_regs
> >> + ARM( adr \tmp1, \phys_l2x0_saved_regs )
> >> + THUMB( adr.w \tmp1, \phys_l2x0_saved_regs )
> >
> > Can you give an example of the assembler error you get without this?
>
> arch/arm/mach-tegra/reset-handler.S: Assembler messages:
> arch/arm/mach-tegra/reset-handler.S:78: Error: invalid immediate for
> address calculation (value = 0x00000004)
The immediate in the 16-bit form of ADR is alignment-sensitive, and
can't address anything that's not on a word boundary. What if you just
have the .align, without adr.w?
> This is with gcc-4.5.3, as:
> GNU assembler (crosstool-NG hg_unknown at 20110628.165246) 2.20.1.20100303
>
> > The target symbol is local and the assembler can see where it is,
> > so it should choose the correct variant for the adr instruction
> > with no need for the ".w" suffix.
> >
> > If not, it could mean that there is a bug in the version of the
> > assembler you're using.
>
> True. Switching to the Ubuntu-packaged ARM cross-compiler that ships
> with Ubuntu 12.10, I do not see this problem.
>
> Still, many people probably still use older compilers.
>
> > If it's definitely needed, you can append a Thumb-only .w suffix with
> >
> > W(adr) \tmp1, \phys_l2x0_saved_regs
> >
> > ...which is slightly neater.
>
> Yes, that certainly works too, even without the .align change in
> reset-handler.S.
Sure, although the unaligned access is not such a good idea anyway.
Keeping the adr fix is harmless, I just wanted to understand why it's
needed.
Cheers
---Dave
next prev parent reply other threads:[~2013-04-17 10:30 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-04-15 22:50 [PATCH 1/3] ARM: tegra: fix build error when THUMB2_KERNEL enabled Stephen Warren
2013-04-15 22:50 ` [PATCH 2/3] ARM: tegra: fix relocation truncated " Stephen Warren
2013-04-17 17:01 ` Dave Martin
2013-04-18 16:32 ` Olof Johansson
2013-04-15 22:50 ` [PATCH 3/3] ARM: tegra: make sure the pointer on 4 byte align " Stephen Warren
2013-04-16 14:13 ` Dave Martin
2013-04-16 16:20 ` Stephen Warren
2013-04-17 10:30 ` Dave Martin [this message]
2013-04-17 16:22 ` Stephen Warren
2013-04-17 16:57 ` Dave Martin
2013-04-18 16:32 ` Olof Johansson
2013-04-17 16:59 ` Dave Martin
2013-04-17 17:00 ` [PATCH 1/3] ARM: tegra: fix build error " Dave Martin
2013-04-18 16:31 ` Olof Johansson
-- strict thread matches above, loose matches on Subject: below --
2013-04-15 12:42 Joseph Lo
2013-04-15 12:42 ` [PATCH 3/3] ARM: tegra: make sure the pointer on 4 byte align " Joseph Lo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130417102957.GA2249@linaro.org \
--to=dave.martin@linaro$(echo .)org \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox