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: <m3d1rxqa4p.fsf@t19.piap.pl>
Date:	Tue, 16 Feb 2016 10:26:14 +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:

> The barriers on a spinlock synchronize between CPUs but not an external
> bus, so (on some architectures) a spinlock protecting an MMIO register
> does not guarantee that two CPUs doing
>
> 	spin_lock();
> 	__raw_writel(address);
> 	__raw_writel(data);
> 	spin_unlock();
>
> would cause pairs of address/data to be seen on the bus.
>
> Of course this is meaningless on ixp4xx, as there is only one CPU.

I still don't get it. If the spinlocks synchronize between CPUs, there
can only be one CPU (or core) doing the pair of raw_writel(), so how
would it be possible to not get the address/data pair written out?
IOW, how is it different from a system with a single CPU?

> On powerpc, we have in_le32/in_be32 for SoC-internal register access,
> while only PCI devices are allowed to be accessed using readl().

Yeah, this seems like a sane solution.

> I would suggest using an ixp4xx specific set of accessors that comes down
> to either readl() or ioread32_be(), depending on whether CONFIG_CPU_BIG_ENDIAN
> is set. That makes it clear that there is a magic bus involved and that it
> works on this platform but not in portable code.

Hmm. This is actually the opposite - while there may be some magic
(swapping) in readl() and friends, there is absolutely no magic in the
__raw_readl() etc. They are essentially equivalent to
*(volatile u32 *)ptr. This is constant and doesn't depend on endianess,
PCI, anything.
-- 
Krzysztof Halasa

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