Re: arch/x86/Kconfig selects invalid HAVE_READQ, HAVE_WRITEQ vars

[Roland Dreier]

 > from arch/x86/Kconfig:
 > 	...
 >         select HAVE_READQ
 >         select HAVE_WRITEQ
 > 	...
 > 
 > yet there are no such defined Kconfig vars anywhere.  thoughts?

git blame shows that this came in from 2c5643b1 ("x86: provide
readq()/writeq() on 32-bit too").  And that commit looks very dubious
indeed to me -- it defines readq() and writeq() in a way that is not
atomic and probably won't generate single 64-bit bus cycles.

Now, many drivers do "#ifndef readq <define my own implementation> #endif"
but exactly what is required is very hardware-dependent, and accessing
32-bit halves in the wrong order may lead to very subtle bugs.  For
example, the changelog for e23a59e1 ("niu: Fix readq implementation when
architecture does not provide one.") says:

    In particular one of the issues is whether the top 32-bits
    or the bottom 32-bits of the 64-bit register should be read
    first.  There could be side effects, and in fact that is
    exactly the problem here.

By coincidence, the 32-bit x86 implementation is actually OK for niu,
but I didn't audit every similar driver, and I don't think any
implementation of readq()/writeq() that generates multiple bus cycles is
suitable in general -- it doesn't meet the requirements of the API.

So I would strongly suggest reverting 2c5643b1 since as far as I can
tell it just sets a trap for subtle bugs that only show up on 32-bit
x86 -- any portable driver still needs to provide readq()/writeq() for
other 32-bit architectures, so it doesn't really help anyone.

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