U-Boot Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Lucas Stach <dev@lynxeye.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] arm: enable unaligned access on ARMv7
Date: Fri, 22 Jun 2012 13:47:17 +0200	[thread overview]
Message-ID: <1340365637.1381.38.camel@tellur> (raw)
In-Reply-To: <20120622131654.2c694a82@aari01-12>

Hi Albert,

Am Freitag, den 22.06.2012, 13:16 +0200 schrieb Albert ARIBAUD:
> Hi Lucas,
> 
> > > > Linux in particular does reinitialize this state and I expect any
> > > > reasonable OS to do so.
> > > 
> > > Then what is the point of enabling it on U-Boot? Does it fix some
> > > issue whereby some mis-aligned piece of data cannot be properly
> > > aligned?
> > > 
> > Yes, it fixes U-Boot USB on Tegra, when built with a recent toolchain.
> > Fixing the alignment of some of the structures in the USB code should
> > also be done, but this is a whole lot more invasive and requires some
> > more thought, as the discussion about this on LKML shows. The issue
> > doesn't show for older toolchains, as they by default emit code to
> > work around unaligned accesses.
> > 
> > This patch fixes all unaligned issues, that may appear with recent
> > toolchains. We avoid having to instruct the toolchain to work around
> > unaligned accesses and gain better performance in cases where it is
> > needed.
> 
> I am not too happy with enabling a lax behavior only to avoid an
> issue which apparently is diagnosed and could / should be fixed at
> its root. Can you point me to the relevant LKML thread
> so that I get the details and understand what prevents fixing this by
> 'simply' aligning the USB structures?

I'm with you that the issue for this particular fault that I ran into
should be fixed at it's root and I will do so as soon as I have enough
time to do so, i.e. within the next three weeks.
You can find a thread about this here:
https://lkml.org/lkml/2011/4/27/278
The problem here is that simply removing the packed attribute is not the
right thing to do for structures that are used for accessing hardware
registers. I have to read a bit more of the USB code to come up with the
right thing to do for every structure there.

But apart from this, we certainly have situations where we have
unaligned accesses that are justified and could not be removed.
Activating the aligned access hardware check is crippling a feature of
the ARMv7 architecture that's apparently useful enough that all recent
toolchains default to using it and not using compiler side workarounds.
Furthermore setting the check bit doesn't even deactivate unaligned
access (there is also a bit for this, which is forced to 1 by all v7
implementations), but just traps the processor in case you care about
such unaligned accesses. Linux for example only sets this check bit if
you choose to install a trap handler for this.

I cannot see how enabling a hardware feature can be seen as allowing of
lax behaviour. As some of the USB structs are used to access hardware
registers, we can not align every struct there. Our options are either:
1. instruct the toolchain to insert workaround code or
2. allow v7 hardware to do the unaligned access on it's own
My comment about fixing the USB code applies only to part of the structs
used there as some of them generate _unnecessary_ unaligned accesses,
the issue that all unaligned accesses fail with current U-Boot built
with a recent toolchain has to be fixed either way and is exactly what
this patch does.

Thanks,
Lucas

  reply	other threads:[~2012-06-22 11:47 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-06-05 17:47 [U-Boot] [PATCH] arm: enable unaligned access on ARMv7 Lucas Stach
2012-06-05 18:42 ` Stephen Warren
2012-06-05 19:06   ` Lucas Stach
2012-06-22  9:15     ` Albert ARIBAUD
2012-06-22  9:36       ` Lucas Stach
2012-06-22 11:16         ` Albert ARIBAUD
2012-06-22 11:47           ` Lucas Stach [this message]
2012-06-22 22:11             ` Aneesh V
2012-06-22 22:13               ` Aneesh V
2012-06-23  9:01                 ` Albert ARIBAUD
2012-06-23 17:43                   ` V, Aneesh
2012-06-25 20:34                     ` Albert ARIBAUD
2012-06-25 21:49                       ` Aneesh V
2012-06-25 22:02                         ` Wolfgang Denk
2012-06-23 19:50                   ` Måns Rullgård
2012-06-24  6:30                   ` Lucas Stach
     [not found]                     ` <20120625221741.3a32790e@lilith>
2012-06-25 21:34                       ` Lucas Stach
2012-06-26 20:56       ` Rob Herring
2012-06-27 10:14         ` Tetsuyuki Kobayashi
2012-07-02  9:42           ` [U-Boot] [PATCH] arm: armv7: add compile option -mno-unaligned-access if available Tetsuyuki Kobayashi
2012-07-02  9:53             ` Måns Rullgård
2012-07-02 15:16               ` Lucas Stach
2012-07-02 16:14                 ` Måns Rullgård
2012-07-03  7:10                   ` Tetsuyuki Kobayashi
2012-07-05  7:57                   ` Albert ARIBAUD
2012-07-18 21:37                     ` Albert ARIBAUD
2012-07-19  4:31                     ` Mike Frysinger
2012-07-19  4:29                   ` Mike Frysinger
2012-07-19  6:28                     ` Albert ARIBAUD
2012-07-19 14:27                       ` Mike Frysinger
2012-07-20  7:12                         ` Albert ARIBAUD
2012-07-12 15:12             ` Gary Thomas

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=1340365637.1381.38.camel@tellur \
    --to=dev@lynxeye.de \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
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