lkml.org 
[lkml]   [2004]   [Sep]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: Add sparse "__iomem" infrastructure to check PCI address usage


On Sun, 12 Sep 2004, Jeff Garzik wrote:
>
> > No, although it's likely to be a strange combination. If you want to force
> > a static address space conversion to a volatile pointer, you can do so. I
> > don't see _why_ you'd want to do it ;)
>
> Well the reason I ask....
>
> static inline void writeb(unsigned char b, volatile void __iomem *addr)
> {
> *(volatile unsigned char __force *) addr = b;
> }

Right. Let's look a bit closer (more of an explanation than you need, but
hey, maybe somebody else is also wondering):

- for gcc, none of this matters one whit. We're just passing in a
"volatile void *", and we're storing the value "b" to the byte pointed
by it. Which is correct on x86, since memory-mapped PCI-space just
looks like memory on x86.

This is important to remember: for gcc, the sparse annotations are
meaningless. They can still be useful just to tell the _programmer_
that "hey, that pointer you got wasn't a normal pointer" in a fairly
readable manner, but in the end, unless you use sparse, they don't
actually _do_ anything.

HOWEVER. When you _do_ use parse, it is another matter entirely. For
"sparse", that "__iomem" has lots of meaning:

# define __iomem __attribute__((noderef, address_space(2)))

ie "iomem" means two separate things: it means that sparse should complain
if the pointer is ever dereferenced (it's a "noderef" pointer) directly,
and it's in "address space 2" as opposed to the normal address space (0).

Now, that means that _sparse_ will complain if such a pointer is ever
passed into a function that wants a regular pointer (because it is _not_ a
normal pointer, and you obviously shouldn't do things like "strcmp()" etc
on it), and sparse will also complain if you try to cast it to another
pointer in another address space.

So if you compile and install sparse, and build with "make C=1", you'll
get warnings like

drivers/video/aty/radeon_base.c:1725:42: warning: incorrect type in argument 2 (different address spaces)
drivers/video/aty/radeon_base.c:1725:42: expected void const *from
drivers/video/aty/radeon_base.c:1725:42: got void [noderef] *[assigned] base_addr<asn:2>

which is just another way sparse tells you that there is a bug in the
source code (in this case, we try to copy from PCI memory-mapped space
directly to user space using "copy_to_user()", which is a _bad_ idea).

So not only can you not dereference it by mistake, you can't even _turn_
it into a pointer that you could dereference by mistake. Sparse will
complain. Sparse will complain even if you use an explicit cast to make it
a normal "(void *)".

And that's good, because on some other architectures, if you try to
dereference the pointer, the machine just oopses. You need to do all the
special magic to actually read from memory-mapped PCI space.

HOWEVER. On x86, it just so happens that dereferencing the pointer _is_
actually the right thing to do, as long as you only do it with the proper
interfaces (ie readb/writeb and friends). And so that sparse won't be
upset, we use the "__force" directive - we're telling sparse that "I know
what I'm doing". And so sparse will quietly allow us to dereference that
pointer that was originally not dereferencable.

Generally, you shouldn't ever use __force in a driver or anything like
that. It's usually a valid thing to do only in the code that is defined
for that particular type. By definition, a driver isn't an entity that
should understand how architecture-specific data structures work, so it
shouldn't try to force things.

So a good use of "__force" is exactly the usage you quote: the
arch-specific code that actually really _knows_ what the magic address
space means, and knows what to do about it.

Linus
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2009-11-18 23:46    [from the cache]
©2003-2011 Jasper Spaans