CPU Cache and busdma usage in USB

[Piotr Zięcik]

Sunday 28 June 2009 11:54:40 Hans Petter Selasky napisał(a):
> Hi Piotr and Rafal,
>
> Your patch is not fully correct. It will break support for x86 and more
> when bounce pages are uses.
>
> Let's get the definitions right:
>
> man busdma
> (...)
>
> My view:
>
> XXX_PREXXX functions should be used prior to read/write device access.
>
> In other words, PRE has to be a flush operation.
>
> XXX_POSTXXX functions should be used after read/write device access.
>
> In other words, POST has to be an invalidate operation.
>
> Reading:
>
> src/sys/arm/arm/busdma_machdep.c
>
> I find bus_dmamap_sync_buf() to be coherent with this view.

If everything is OK, then why USB does not work on ARM and MIPS ?
Let's look into busdma for these platforms:

usb_pc_cpu_invalidate(): [BUS_DMASYNC_POSTREAD | BUS_DMASYNC_POSTWRITE]
	i386:		NOP
	arm:		Invalidate + hacks for VIPT cache.
	mips:		NOP

usb_pc_cpu_flush(): [BUS_DMASYNC_PREREAD | BUS_DMASYNC_PREWRITE]
	i386:		NOP
	arm:		Writeback invalidate
	mips:		Writeback invalidate.

I do not see direct coherency between flags passed to bus_dma and cache 
operations, which could be source of problem.

Let's also look at usb_pc_cpu_invalidate() usage in 
sys/dev/usb/controller/ehci.c:
<cite>
while (1) {
		usb_pc_cpu_invalidate(td->page_cache);
                status = hc32toh(sc, td->qtd_status);

		(...)

		if (status & EHCI_QTD_HALTED) {
			break;
		}

		(...)

		td = td->obj_next;
</cite>

In my oppinion usb_pc_cpu_invalidate() used here suggests that it is doing 
cache invalidation not "Perform any synchronization required after 					     
an update of host memory by the device and prior to CPU access to host 
memory".

As this function is implemented as bus_dmamap_sync() all busdma rules should 
be applied:
<cite>
If read and write operations are not preceded and followed by the
appropriate synchronization operations, behavior is undefined.
</cite>

In code shown above (and many more places in USB stack) there is no following
synchronization operation, which also could be source of problem.

My major question here is why bus_dma is used for flushing and invalidation
CPU caches instead of cpu_*() functions wich was written for this purpuose.

> Can you check if the COHERENT bit is set for your allocation?
>
>         if (map->flags & DMAMAP_COHERENT)
>                 return;
>

No. This bit is not set.

> Summed up:
>
> The existing code is doing correct. What is known is a problem with the
> memory mapping, so that the same memory page can get mapped with different
> attributes, which makes the problem appear.

I don't think so:
$ man busdma
<cite>
BUS_DMA_COHERENT
	Attempt to map this memory such that cache sync operations are as cheap as
	possible.  This flag is typically set on memory that will be accessed by both
	a CPU and a DMA engine, frequently.  Use of this flag does not remove the 
	requirement of using bus_dmamap_sync, but it may reduce the cost of 	
	performing these operations. 
</cite>

This means that BUS_DMA_COHERENT does not guarantee no-cache mapping - this is
only a hint for busdma subsystem that region will be synced frequently. Please 
look at discussion at 
http://kerneltrap.org/mailarchive/freebsd-current/2009/5/27/5817243

-- 
Best regards.
Piotr Ziecik