public inbox for u-boot@lists.denx.de
 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: Mon, 25 Jun 2012 23:34:21 +0200	[thread overview]
Message-ID: <1340660061.1337.26.camel@antimon> (raw)
In-Reply-To: <20120625221741.3a32790e@lilith>

Hi Albert,

Am Montag, den 25.06.2012, 22:17 +0200 schrieb Albert ARIBAUD:
> Hi Lucas,
> 
> On Sun, 24 Jun 2012 08:30:19 +0200, Lucas Stach <dev@lynxeye.de> wrote:
> > Hi Albert,
> > 
> > Am Samstag, den 23.06.2012, 11:01 +0200 schrieb Albert ARIBAUD:
> > [snip]
> > > > >> But apart from this, we certainly have situations where we have
> > > > >> unaligned accesses that are justified and could not be removed.
> > > > >> [...]
> > > > >> 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.
> > > 
> > > If the access is in true RAM, then we can always realign the data;
> > > and I don't know of memory-mapped registers which would be
> > > unaligned wrt their width. If some USB controller is designed so,
> > > then the fix should only and explicitly affect that controller,
> > > because we don't know it it will always be used with an ARM
> > > implementation that can do unaligned accesses.
> > 
> > My point is: on ARM platforms that can't do unaligned access in
> > hardware the toolchain will automaticly emit helper code to work
> > around this. On an ARMv7 platform hardware assisted unaligned access
> > is mandatory, so toolchains will not emit helper code and rather let
> > the hardware do it's job.
> 
> My point is that there should be a reason for doing unaligned access to
> boot (pun half-intended) and I don't see any.
> 
I see your point here.

> > Now the situation is as follows: hardware platforms without the
> > ability to do unaligned access in hardware will just work even though
> > the code is suboptimal, but users of an ARMv7 platform will encounter
> > unexplained system hangs, which is bad imho. This patch just makes
> > behaviour consistent across ARMv5 and ARMv7 platforms.
> 
> Wouldn't aligning data properly make behaviour consistent as well?
> 
Consistent in the way of "it works on all platforms", but not in the way
of programming practice. ARMv7 people will have to care about alignment
to not encounter runtime errors, while on ARMv5 it just doesn't matter.
(Which certainly does not make it the right thing to do on ARMv5, but as
the existing USB code shows people just haven't cared about this.)

> > If we really want to disallow the use of unaligned memory operations
> > in U-Boot we should make all platforms fail at compile time, not make
> > one platform fail randomly at runtime. Do you think this is the way
> > to go? I'm fine either way, I'm just not okay with the current
> > situation where one platform fails while another just works.
> 
> I do not want to disallow unaligned accesses, because this assumes
> they were allowed in the first place; I want to keep not allowing them,
> just like they were unallowed far, and just like they can keep on
> being. If a misaligned access causes one platform to fail while another
> just works, then we should avoid the misaligned access by re-aligning
> it properly so as to make both platforms work.
> 
The thing I wanted to say is: if we generally consider unaligned access
a no-go for ARMv7 (which I'm fine with), we should equally consider it a
no-go for ARMv5. So I would rather want to see both platforms fail at
compile time if we have unaligned data, to avoid the unintended
introduction of such code in the codebase as has happened with the usb
code. Sadly I have not found a way to tell gcc to error out if
encounters an unaligned access. Currently ARMv5 "allows" unaligned data,
as toolchains insert helper code which makes it work.

Also it is really unfortunate that coding errors (which the unjustified
use of unaligned data certainly is) manifest as a runtime error, rather
than a compiletime failure, which make them a lot harder to spot.

> IOW, things will not change until I am shown some actual situation where
> a misaligned access is necessary and the core prevents it.
> 
I'm okay with you dropping this patch and will try to come up with
something that make programming constraints more consistent across both
ARM platforms.

Thanks,
Lucas

  parent reply	other threads:[~2012-06-25 21:34 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
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 [this message]
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=1340660061.1337.26.camel@antimon \
    --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