linux-kernel - Re: [PATCH 3/7] usb: gadget: pxa25x

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: <m3y4amqdym.fsf@t19.piap.pl>
Date:	Mon, 15 Feb 2016 14:51:13 +0100
From:	khalasa@...p.pl (Krzysztof Hałasa)
To:	Arnd Bergmann <arnd@...db.de>
Cc:	linux-arm-kernel@...ts.infradead.org,
	Felipe Balbi <balbi@...nel.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
	Felipe Balbi <balbi@...com>,
	Haojian Zhuang <haojian.zhuang@...il.com>,
	Daniel Mack <daniel@...que.org>,
	Imre Kaloz <kaloz@...nwrt.org>,
	Robert Jarzmik <robert.jarzmik@...e.fr>
Subject: Re: [PATCH 3/7] usb: gadget: pxa25x_udc: use readl/writel for mmio

Arnd Bergmann <arnd@...db.de> writes:

> I consider the use of __raw_* accessors a bug, I don't think we should
> ever do that because it hides how the hardware actually works, it doesn't
> work with spinlocks, and it can lead to the compiler splitting up accesses
> into byte sized ones (not on ARM with the current definition, but
> possible in general).

Well, then maybe we should fix them, or add another set.
Why don't they work with spinlocks?

To be honest, I remember this was already discussed a bit years ago.
I think I proposed back then a set of read_le32 (which would be
equivalent of current readl(), and could be named pci_readl() as well),
read_be32, read_host (without swapping).
The names could be better, though.

> Almost all hardware is fixed-endian, so you have to use swapping accessors
> when the CPU is the other way, except for device RAM and FIFO registers
> that are always used to transfer a byte stream (see the definition of
> readsl() and memcpy_fromio()). When you have hardware that adds byteswaps
> on the bus interface, you typically end up with MMIO registers requiring
> no swap (or double swap) and readsl()/memcpy_fromio()) suddenly requiring
> a swap that is counterintuitive.

Sure, but the __raw_* are used just to be sure there is absolutely no
swapping.
E.g. for IXP4xx, the registers never require swapping, thus readl() etc.
are not suitable for this. What is needed here is simple "atomic" 32-bit
straight to/from register (MM)IO (assuming 4-byte address alignment).

If not __raw_* then what?
-- 
Krzysztof Halasa

Industrial Research Institute for Automation and Measurements PIAP
Al. Jerozolimskie 202, 02-486 Warsaw, Poland