On Mon, Feb 10, 2014 at 06:26:46PM +0100, Albert ARIBAUD wrote: > Hi Tom, > > On Mon, 10 Feb 2014 11:24:03 -0500, Tom Rini <tr...@ti.com> wrote: > > > On Mon, Feb 10, 2014 at 05:12:24PM +0100, Albert ARIBAUD wrote: > > > Hi Måns, > > > > > > On Mon, 10 Feb 2014 15:14:49 +0000, Måns Rullgård <m...@mansr.com> > > > wrote: > > > > > > > Albert ARIBAUD <albert.u.b...@aribaud.net> writes: > > > > > > > > > Hi Tom, > > > > > > > > > > On Mon, 10 Feb 2014 08:21:39 -0500, Tom Rini <tr...@ti.com> wrote: > > > > > > > > > >> On Mon, Feb 10, 2014 at 10:24:47AM +0100, Albert ARIBAUD wrote: > > > > >> > Hi Tom, > > > > >> > > > > > >> > On Tue, 4 Feb 2014 12:05:33 -0500, Tom Rini <tr...@ti.com> wrote: > > > > >> > > > > > >> > > When we tell the compiler to optimize for ARMv7 it assumes a > > > > >> > > default of > > > > >> > > unaligned accesses being supported at the hardware level and can > > > > >> > > make > > > > >> > > use of this to perform what it deems as an optimization in any > > > > >> > > case, > > > > >> > > including allowing for data to become unaligned. We explicitly > > > > >> > > disallow > > > > >> > > this hardware feature so we must tell the compiler. > > > > >> > > > > > > >> > > Cc: Albert ARIBAUD <albert.u.b...@aribaud.net> > > > > >> > > Cc: Mans Rullgard <m...@mansr.com> > > > > >> > > Signed-off-by: Tom Rini <tr...@ti.com> > > > > >> > > > > > >> > NAK -- the discrepancy between the compiler being told to allow > > > > >> > native > > > > >> > unaligned accesses while at the same time telling the hardware to > > > > >> > trap > > > > >> > them is conscious and voluntary. It was chosen to help detect > > > > >> > unaligned > > > > >> > accesses which are rarely necessary and can be explicitly > > > > >> > performed by > > > > >> > software on a case by case basis. > > > > >> > > > > > >> > If and when a specific file requires unaligned access which cannot > > > > >> > be > > > > >> > made by some other mean than enabling -mno-unaligned-access, then > > > > >> > this > > > > >> > file should have it added, not the whole of U-Boot. > > > > >> > > > > >> Right, I recall the discussion, and we chose wrong. > > > > > > > > > > I am quite prepared to discuss whether we chose wrong or right, and > > > > > to change my mind when the conditions are right, but I'll need more > > > > > than > > > > > such a short and simple statement. :) > > > > > > > > I already gave you a detailed explanation some months ago. You refused > > > > to read it. > > > > > > I can hardly have "refused to read" a message which I *answered*, laid > > > out how I thought the issue should be solved... and got no answer after > > > this. > > > > > > Now, are we going to discuss the technical issue or is this going to go > > > debian-TC -- which I wouldn't see the point of. > > > > Well, here's the point that I haven't seen an answer to. If we tell the > > compiler "you may choose to use unaligned accesses as an optimization, > > we support this", the compiler says "OK, I shall do that", and then we > > fail at run time because we don't actually allow the unaligned access, > > how is this not a problem on our end for the first part of the equation, > > keeping in mind that the real world is poorly designed and when we write > > code to this reality the compiler does the correct thing in all cases > > (or it's a compiler bug). > > Both in the thread you are referring to, and in this current thread, I > have already explained why we allow native unaligned accesses in the > compiler and prevent them in the hardware, and how this may seem > contradictory but is not ; but I'm ok with explaining it again, so here > comes. > > U-Boot runs on all sorts of hardware. Some hardware can perform native > unaligned accesses without any penalty; some can, but with a penalty; > some just can't. > > Therefore, except for some parts of it which are specific to a given > architecture, the U-Boot source code should not assume that native > unaligned accesses are possible. In fact, it should assume they are > impossible. > > Now if we restrict ourselves to a subset of ARM architectures, then > native unaligned accesses could be used. And if we restrict ourselves > to ARMv7, they are almost costless. > > So on ARMv7 we could write code which does not care about alignment > because native unaligned accesses are just as cheap as aligned ones. > > The problem is, since U-Boot runs on a lot of platforms, such > alignment-agnostic code might end up built and run on hardware which > does not allow native unaligned accesses. Granted, this hardware will > detect it; but this might happen quite some time after the code was > pulled in.
Right. > Therefore, we should try and detect unaligned accesses as early as > possible. Then what I'm saying is that our proposal for catching them isn't valid, which I'll get to. > To perform such a detection, there is a bit in ARM architectures which, > when set, causes native unaligned accesses to trap, allowing us to > see them as early as possible, which is good: we want to catch such > accesses. > > Now, there is also a compiler flag related to alignments, and that is > -m[no-]unaligned-access. Its effect is not to allow or disallow > unaligned accesses, but to allow or disallow *native* unaligned > accesses. If -mno-unaligned-access is in effect, then native unaligned > accesses will be replaced by software unaligned accesses, e.g. an > even-address dword access will be actually performed as a combination > of byte and word accesses, all aligned. This will prevent the hardware > from detecting the dword unaligned access, which is bad: we don't want > to miss such accesses. Then gcc has a bug and you need to convince them to fix it. What gcc does, as Mans has explained, and this invalidates the "lets catch unaligned access problems" notion, is for ARMv6 and higher say "we assume by default the hardware can perform native unaligned access, so make use of this in our optimization choices". > *That* is the reason for forbidding unaligned accesses at the hardware > level while enabling them at the compiler level: it is the only > combination which ensures that unsuspected unaligned accesses are > detected at run time. And it has the unintended side-effect of generating native unaligned accesses in cases where we have annotated the sources correctly and when native unaligned access would be disallowed, the compiler would work correctly. > Now back to your question, "how is this not a problem on our end for the > first part of the equation": > > - first, if "the first part of the equation" means "the compiler > setting" as opposed to the hardware setting, then the question fails > to realize that we don't (and should not) consider the compiler and > hardware settings separately; they work in *combination*. That they work in combination is my point. We're saying -march=armv7a -munaligned-access (the latter implicitly as it is the default from the compiler) and _NOT_ honoring the requirements. > - second, assuming the question is "how is it not a problem on our end > that some code traps due to the combined hardware and compiler > settings", the answer is: because the setting was not designed to > catch *ARMv7* issues; it was designed to catch *U-Boot* issues. In > other words, such traps show that there is code which won't work > elsewhere than on ARMv7-like hardware which does not care about > alignment. But this part isn't true. Or rather, it is (or may, at the whim of the compiler) catching every case where we say __attribute__((packed)) and then don't follow up ensuring that every access is then fixed up by hand, rather than letting the compiler do it. We've essentially picked "lets blow things up at times" over "lets keep an eye out for __packed being used in code, and be careful there". > The only drawback of this setup is that while some code does unneeded > unaligned accesses (and is thus obviously wrong), some other code > *requires* unaligned accesses (because of standards, or of hardware). > Such code will trap... but *only* if it performs unaligned accesses > *natively*, which it should not since it might run on a hardware not > capable of native unaligned accesses. > > That is why I consider that the fault is in the trapped software, not > in the trap. The solution is to make the software use software, not > native, unaligned accesses. The exact solution depends on whether the > code has only a few such unaligned accesses (in which case we should > use explicit unaligned access macros) or many (in which case, for the > file considered, we can enable -mno-unaligned-access). You'll find > instances of both in the U-Boot code. The problem is that __packed means we can see this problem whenever its used. This is the design practice we need to be wary of, and make sure we're coding to an unfortunate reality rather than misoptimizing things. [snip] > Regarding the EFI code, there was a patch submitted (see the thread you > have pointed me to) which had the proper unaligned access macros and > thus did not require -mno-unaligned-access for this file, and certainly > does not require it for the whole of ARM. Right, the EFI patch takes valid code and makes it differently valid. -- Tom
signature.asc
Description: Digital signature
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
