Re: [PATCH 002/002] de2104x: support for systems lacking cache coherence

[Benjamin Herrenschmidt <benh@kernel.crashing.org>]

> I really don't see why this patch could possibly be necessary.
> 
> On systems that lack cache coherence:
> 
> 1) {pci,dma}_alloc_{consistent,coherent}() give kernel mappings of
>    the buffer with the cache disabled.  Therefore the device and
>    and cpu see the correct data.
> 
> 2) {pci,dma}_{map,unmap}_{single,sg}() do the appropriate cache
>    flushing.
> 
>    Explicit syncing between cpu and device can be performed
>    using {pci,dma}_sync_{single,sg}() as needed.
> 
> Therefore, this patch is superfluous.

Allright so in that case, I'm not entirely sure... There is some history
behind that (and btw, de4x5 and tulip probably need the same
"workaround"), see below.

First what I believe the problem to be is some kind of coherency bug in
a bridge in some of these old systems. I'm not 100% sure of the details
but it does have to do with partial cache line access iirc. By moving
the descriptors into separate cache lines, we avoid the bug.

It would be possible, I suppose, to work around it by treating those
machines as non-coherent but with two significant drawbacks:

 - One is simple and you may want to ignore it :-) Basically, those are
already pretty slow machines, and thus we would slow them down even more
by doing manual cache flush/invalidates all over the place (as a matter
of fact, I'm not even sure those CPUs support dcbi instructions for
cache inval, I need to dbl check).

 - The other is more subtle but also harder to avoid: It would be fairly
hard to add support for non-coherent mappings on these because of the
way the whole MMU stuff works for those 32-bit hash based powerpc's.
Basically, we cannot have something mapped both cached and non-cached
(in different virtual addresses), and the use of the BAT mappings in the
processor means that most of the linear mapping -will- be mapped cached
in chunks of 256MB. The code pretty much relies on that, it's not just
an optimisation that can be turned off.

So I'm of mixed opinion here... It looks like since only those 2 or 3
drivers are affected (ie, they are the only thing ever found behind
those weirdo bridges) and since those chips happen to support this
padding between descriptor, it looks like a good compromise to just add
this feature to the drivers. In fact, de4x5.c at least used to have it,
I don't know if it's still the case.

Now, there are other cache coherency related bugs I think on those old
machines, or at least what look like it -could- be cache coherency
related bugs, or maybe just bugs in the DBDMA engine. Mesh for example
gets unhappy if we give it a buffer that is not aligned on a cache line
boundary and corrupts the beginning of that cache line. But I don't
think that treating the platform as non-coherent will fix that, ie, it
seems to happens regardless of the line actually being in the cache or
not.  This is typically triggered by the SCSI ID at boot when using SLAB
debug which de-aligns kmalloc, or at least that's how I observed it a
while ago.

Cheers,
Ben.