What's up with cpu_is_noncoherent_r10000() ?

What's up with cpu_is_noncoherent_r10000() ?

[David Daney]

I am bringing up the git HEAD on an old ATI Xilleon X226.  This nice 
system claims to be 4KEc, but for some reason doesn't support mips32r2, 
but I digress.

Among its other problems this is a CONFIG_DMA_NONCOHERENT system, so 
drivers like net/e100.c do not function properly if the cache is not 
appropriately flushed/invalidated when they are doing DMA.  Fortunately 
the authors of said driver have used 
pci_dma_sync_single_for_{cpu,device} in what seems like the appropriate 
  manner.

pci_dma_sync_single_for_device() ends up in dma_sync_single_for_device() 
(in mm/dme-default.c) and is does the cache flush as expected.  The 
problem is with dma_sync_single_for_cpu() which for some reason only 
does the cache flush/invalidate  if cpu_is_noncoherent_r10000() returns 
true (which it does only for R10K CPUs).  When I hack it up so that it 
returns true unconditionally, e100 starts functioning normally for me. 
This leads me to think that the cache operation should be done for all 
CONFIG_DMA_NONCOHERENT systems not just R10K based systems.

What is the reasoning for only doing the cache operation on  R10K based 
systems?

David Daney

Re: What's up with cpu_is_noncoherent_r10000() ?

[Thiemo Seufer]

David Daney wrote:
> I am bringing up the git HEAD on an old ATI Xilleon X226.  This nice  
> system claims to be 4KEc, but for some reason doesn't support mips32r2,  
> but I digress.

FYI, early revisions of the 4KEc, most notably the TI AR7, are MIPS32R1.


Thiemo

Re: What's up with cpu_is_noncoherent_r10000() ?

[Thomas Bogendoerfer]

On Mon, Aug 25, 2008 at 09:34:29AM -0700, David Daney wrote:
> What is the reasoning for only doing the cache operation on  R10K based 
> systems?

non coherent R10k need after DMA operations to get rid of remains
of load/store speculations. Other CPUs don't pollute the cache
after it got flushed.

But this optimization is wrong, we need to do the flush for
every non coherent device otherwise polling a descriptor via
a cached mapping can't work. And this exactly what E100 does.

Instead of if (cpu_is_noncoherent_r10000(deva)) something like

if (cpu_is_noncoherent_r10000(dev) || 
    (!plat_device_is_coherent(dev) && (direction != DMA_TO_DEVICE)))


should do the trick with minimum flushes for non R10k CPUs. But probably
a simple

if ((!plat_device_is_coherent(dev))

is the safest approach.

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessary a
good idea.                                                [ RFC1925, 2.3 ]

Re: What's up with cpu_is_noncoherent_r10000() ?

[Ralf Baechle]

On Mon, Aug 25, 2008 at 08:46:01PM +0200, Thomas Bogendoerfer wrote:

> On Mon, Aug 25, 2008 at 09:34:29AM -0700, David Daney wrote:
> > What is the reasoning for only doing the cache operation on  R10K based 
> > systems?
> 
> non coherent R10k need after DMA operations to get rid of remains
> of load/store speculations. Other CPUs don't pollute the cache
> after it got flushed.
> 
> But this optimization is wrong, we need to do the flush for
> every non coherent device otherwise polling a descriptor via
> a cached mapping can't work. And this exactly what E100 does.

When polling the buffer basically changes ownership between CPU and device
and buffer all the time, so a drivers needs to do a
dma_sync_*_for_cpu call before looking at the buffer, then 
dma_sync_*_for_device to return the buffer to the device.  So to polling
loop will work fine as long as one of the two calls does the flush operation.
In fact we're even doing double flushes for the case of non-coherent
R10000s ...

  Ralf

Re: What's up with cpu_is_noncoherent_r10000() ?

[David Daney]

David Daney wrote:
> I am bringing up the git HEAD on an old ATI Xilleon X226.  This nice 
> system claims to be 4KEc, but for some reason doesn't support mips32r2, 
> but I digress.
> 
> Among its other problems this is a CONFIG_DMA_NONCOHERENT system, so 
> drivers like net/e100.c do not function properly if the cache is not 
> appropriately flushed/invalidated when they are doing DMA.  Fortunately 
> the authors of said driver have used 
> pci_dma_sync_single_for_{cpu,device} in what seems like the appropriate 
>  manner.
> 
> pci_dma_sync_single_for_device() ends up in dma_sync_single_for_device() 
> (in mm/dme-default.c) and is does the cache flush as expected.  The 
> problem is with dma_sync_single_for_cpu() which for some reason only 
> does the cache flush/invalidate  if cpu_is_noncoherent_r10000() returns 
> true (which it does only for R10K CPUs).  When I hack it up so that it 
> returns true unconditionally, e100 starts functioning normally for me. 
> This leads me to think that the cache operation should be done for all 
> CONFIG_DMA_NONCOHERENT systems not just R10K based systems.
> 
> What is the reasoning for only doing the cache operation on  R10K based 
> systems?
> 

OK, Ralf straightened me out on dma_sync_*.  It would appear that 
mm/dme-default.c is correct and drivers/net/e100.c is missing a 
pci_dma_sync_single_for_device().

I am preparing a patch for e100.c

David Daney