linux-kernel - Re: [PATCH] Revert "regmap-mmio: Use native endianness for read/write"

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <15472016.EVMMrUafC6@wuerfel>
Date:	Tue, 26 Jan 2016 00:01:25 +0100
From:	Arnd Bergmann <arnd@...db.de>
To:	Mark Brown <broonie@...nel.org>
Cc:	Johannes Berg <johannes@...solutions.net>,
	Simon Arlott <simon@...e.lp0.eu>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Revert "regmap-mmio: Use native endianness for read/write"

On Monday 25 January 2016 22:47:10 Mark Brown wrote:
> On Mon, Jan 25, 2016 at 11:24:44PM +0100, Arnd Bergmann wrote:
> > On Monday 25 January 2016 23:07:55 Johannes Berg wrote:
> > > Consequently, this commit broke my HummingBoard i.MX6 in big
> > > endian mode since it would now try to talk to the little
> > > endian hardware with a big endian CPU without conversion.
> 
> What I'd expect to be happening here is that either the driver or the
> DT should be specifying the endianness of the hardware.  If the device
> is always a given endianness then I'd expect that to turn up in the
> driver rather than the DT.
> 
> > > What the patch really would have to do is introduce some kind
> > > of "device-endian" readl/writel, that takes the endianness of
> > > the device as an argument. That seems a bit overkill though,
> > > and would likely not generate any better code than the double
> > > byte-swaps that MIPS is getting now.
> 
> The problem here is that regmap already has that functionality (it needs
> it for non-MMIO buses anyway) and so it knows what's going on really
> wants the I/O accessors to get out of the way.

I think we need to at least document the default behavior for
syscon (without changing behavior on ARM), and allow the DT to
specify cpu-endian as an override (or make MIPS default to that,
but that would be rather inconsistent and a pain to document
in the syscon binding).

> > This means that the devices are in fact CPU-endian, and we need
> > some way for Linux to represent this. The patch to
> > drivers/base/regmap/regmap-mmio.c is clearly wrong, as we
> > must never use __raw_*() accessors in an architecture independent
> > driver (for a number of reasons), but we still need a fix for
> > MIPS so it can specify a way to do the double-swap without
> > faking the endianess of the registers.
> 
> I can't identify *anything* which says we shouldn't use the __raw
> accessors in architecture neutral code with the possible exception of
> the __.  Seriously, how is anyone supposed to use this stuff if we have
> hidden assumptions like this?

I thought everyone knew by now.

> > Also, defaulting syscon to "native-endian" when nothing else is
> > specified sounds like a bad idea, but we may already be stuck there
> > with the precedent in existing bindings after 6a55244e897d
> > ("regmap: mmio: request native endian formatting"), we'll have
> > to think about that some more.
> 
> Yeah, the native endian formatting is causing a lot of trouble.  The
> MIPS folks really should also have come and talked to me rather than
> writing such obvious nonsense in the DT in the first place.

They should also have talked to me as the syscon maintainer...

	Arnd