linux-kernel - Re: [PATCH 1/9] ARM i.MX51: Add ipu clock support

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: <20101215171246.GG9937@n2100.arm.linux.org.uk>
Date:	Wed, 15 Dec 2010 17:12:46 +0000
From:	Russell King - ARM Linux <linux@....linux.org.uk>
To:	Arnd Bergmann <arnd@...db.de>
Cc:	linux-arm-kernel@...ts.infradead.org,
	Sascha Hauer <s.hauer@...gutronix.de>,
	Zhang Lily-R58066 <r58066@...escale.com>,
	linux-fbdev@...r.kernel.org,
	Arnaud Patard <arnaud.patard@...-net.org>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/9] ARM i.MX51: Add ipu clock support

On Wed, Dec 15, 2010 at 05:49:59PM +0100, Arnd Bergmann wrote:
> On Wednesday 15 December 2010, Russell King - ARM Linux wrote:
> > Lastly, I don't see where atomicity comes into it - __raw_writel vs writel
> > have the same atomicity.  Both are single access atomic provided they're
> > naturally aligned.  Misaligned device accesses are not predictable.
> 
> This is just what gcc turns it into today. In theory, a future gcc or
> a future cpu might change that. If you mark a pointer as
> '__attribute__((packed))', it probably already does, even for aligned
> pointers, while it does not when using writel_{,relaxed}. The point is
> that __raw_* means just that -- we don't give any guarantees on what
> happens on the bus, so people should not use it.

No.  It does give guarantees on what happens on the bus.  The kind of
pointer you pass in has no bearing on what happens on the bus because it's
casted away immediately.

__raw_writel(v, ptr) doesn't just do *(ptr) = v - that doesn't work when
ptr is void.  Instead, it has to do a cast to ensure that we have a valid
C dereference (void pointers are illegal to dereference):

#define __raw_writel(v,a)  (__chk_io_ptr(a), \
	*(volatile unsigned int __force   *)(a) = (v))

Doesn't matter if 'a' was marked as packed or not - that's all casted away.
That's not something that should ever change - otherwise we'll all be
screwed as you could never cast away pointer attributes.

It _may_ possible that the compiler decides that accessing an 'unsigned int'
will not be done as a single word load, in which case we have exactly the
same problems with writel() too.

And in any case, if __raw_writel() was as you're suggesting, then it would
serve absolutely no purpose at all.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/