Fix *dma_sync_single() and flush_dcache_all

Hi all,

I've had people trying to convince me of all sorts of things lately on this and finally went to the source (DaveM). The short is that DMA-mapping.txt means what it says in the section entitled "What memory is DMA'able?". That is, the only memory that can be used with the DMA API is memory acquired via *get_free_page, kmalloc, or kmem_cache_alloc. Period.

That means the ARM and MIPS implementations that use a bus_to_virt() on the dma_handle in *dma_sync_single are correct and our (performance killing) call to flush_dcache_all is unnecessary. flush_dcache_all is still used by 405lp pm code so it stays and I updated the comments and made it safe (per another thread) for 440gp's cache size. This is in case it is needed by some external callers on non-405lp.

If there are no objections, then I will commit to following patch to 2_4_devel and 2.5. The 2.5 pci_map_single/sync_single are horked up anyway for 4xx so that will get fixed too.

===== arch/ppc/kernel/misc.S 1.79 vs edited =====

@@ -607,25 +607,18 @@
 	blr
 
 #ifdef CONFIG_NOT_COHERENT_CACHE
-/* This is a bad one....It is used by 'consistent_sync' functions when
- * there isn't any handle on the virtual address needed by the usual
- * cache flush instructions.  On the MPC8xx, we can use the cache line
- * flush command, on others all we can do is read enough data to completely
- * reload the cache, flushing old data out.
- */
-
 /*
  * 40x cores have 8K or 16K dcache and 32 byte line size.
  * 440 has a 32K dcache and 32 byte line size.
  * 8xx has 1, 2, 4, 8K variants.
  * For now, cover the worst case of the 440.
- * When we get a cputable cache size entry we can do the right thing.
+ * Must be called with external interrupts disabled.
  */
 #define CACHE_NWAYS	64
 #define CACHE_NLINES	16
 
 _GLOBAL(flush_dcache_all)
-	li	r4, (CACHE_NWAYS * CACHE_NLINES)
+	li	r4, (2 * CACHE_NWAYS * CACHE_NLINES)
 	mtctr	r4
 	lis     r5, KERNELBASE@h
 1:	lwz	r3, 0(r5)		/* Load one word from every line */

===== include/asm-ppc/pci.h 1.30 vs edited =====

@@ -214,20 +214,8 @@
 {
 	if (direction == PCI_DMA_NONE)
 		BUG();
-#ifdef CONFIG_NOT_COHERENT_CACHE
-	/* The bus_to_virt() can't be used here, in case dma_handle
-	 * points to something that doesn't have the same cache attributes
-	 * as the 1:1 mapped kernel memory.  If we used it, we could
-	 * get a cache line alias with the wrong attributes.
-	 * Since there isn't any Linux way to get the real VA from a PA,
-	 * it is just easier to flush the whole cache.  The code to
-	 * determine VA from PA would probably do the same :-).
-	 * I don't know why these functions don't pass VA, since the
-	 * cache operations use VA and the caller has this information.
-	 *	-- Dan
-	 */
-	flush_dcache_all();
-#endif
+
+	consistent_sync(bus_to_virt(dma_handle), size, direction);
 }
 
 /* Make physical memory consistent for a set of streaming

Regards,
--
Matt Porter
[email protected]
This is Linux Country. On a quiet night, you can hear Windows reboot.

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Well, to be fair it seems this code has gone through a couple of revisions since it was originally written. I suspect a "better safe than sorry" approach was taken when changes were made. :-)

OK, go for it. A couple of comments though.........

I made these #defines in the original code and I believe they were set to values based upon the processor selection. When did they change to 'worst case' and the most popular and most performance challenged processors are paying the price for unnecessary cache operations?

	-- Dan

I figured that. :)

I don't know the answer that. It happened before I first touched it. I simply upped the worst case to cover the larger 440 core dcache.

Yes, I completely understand the comment. :)

As I said, the DMA API explicitly lists what memory can be passed to this family of API's. Memory allocated from pci_alloc_consistent cannot be passed in. Nor can ioremapped addresses. DaveM suggests another API be created to handle device to device transfers for 2.5+ for that specific case.

I listed the types of allocated addresses that the API documentation explicitly mentions in the previous mail. Memory allocated via those methods will always be able to be converted back to a virtual address via bus_to_virt() because the allocations come from 1:1 kernel lowmem and are guaranteed to be contiguous. highmem is not a consideration since the *_single() family of calls is only usable on lowmem...*_page() would be used for highmem.

Regards,
--
Matt Porter
[email protected]
This is Linux Country. On a quiet night, you can hear Windows reboot.

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/