Skip to contentSkip to conversation

What's up with cpu_is_noncoherent_r10000() ?

DISCUSSION David Daney 4 participants 5 messages Aug 25, 2008 – Aug 26, 2008 MIPS

lore
Share & tools
lore
#0 lore raw

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

#1 lore raw
Quoted 3 lines · 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.

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

Thiemo

#2 lore raw
In reply to David Daney
Quoted 2 lines · David Daney

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.

Signature · 2 lines
Crap can work. Given enough thrust pigs will fly, but it's not necessary a
good idea.                                                [ RFC1925, 2.3 ]
#3 lore raw
Quoted 11 lines · Thomas Bogendoerfer
Quoted 2 lines · David Daney

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

#4 lore raw
In reply to David Daney
Quoted 9 lines · 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?

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