Subject: Re: bus_dmamap_sync not enough ?
To: Jason R Thorpe <thorpej@wasabisystems.com>
From: Stephan Uphoff <ups@stups.com>
List: tech-kern
Date: 02/11/2003 15:42:24
> Jason R. Thorpe wrote:
> On Tue, Feb 11, 2003 at 12:35:01PM -0500, Stephan Uphoff wrote:
> 
>  > The i82257 driver ignores this problem and just illegally calls 
>  > bus_dmamap_sync with
>  > BUS_DMASYNC_POSTXXXX flags with full knowledge that the DMA might not have 
>  > completed.
>  > ( When it finds out that the DMA did not complete it just calls bus_dmamap_sync
>  > again with BUS_DMASYNC_PREXXXX op flags)
> 
> This is not illegal... this is an acceptable/intended use of the interface
> (I should know -- I designed the interface and wrote the code that you're
> citing :-)
> 
> -- 
>         -- Jason R. Thorpe <thorpej@wasabisystems.com>
> 

I missed that you wrote the driver - excellent then I can point out some
of the details. (and btw, thanks for all the great code !)

The bus_dma(9) manual states for bus_dmamap_sync:
	
	This function must be called to synchronize DMA buffers before
        and after a DMA operation.
	[ ... ]	
	If DMA read and write operations are not preceded and followed by the appro-
        priate synchronization operations, behavior is undefined.

If the manual is correct then fxp_rxintr() has a race condition as a DMA read 
operation
might happen for example between lines  "/* ## A ## */" and "/* ## B ## */"

File i82257.c:
fxp_rxintr() 

	[...]	 
		FXP_RFASYNC(sc, m,
		    BUS_DMASYNC_POSTREAD|BUS_DMASYNC_POSTWRITE);

		/* ## A ##*/
		rxstat = le16toh(rfa->rfa_status);

		if ((rxstat & FXP_RFA_STATUS_C) == 0) {
			/*
			 * We have processed all of the
			 * receive buffers.
			 */
			 /* ## B ## */
			FXP_RFASYNC(sc, m, BUS_DMASYNC_PREREAD);
			return;
		}
	[...]

There are several more examples in the driver with possible read AND write DMA
operations on buffers last synchronized with  BUS_DMASYNC_POSTREAD|BUS_DMASYNC_
POSTWRITE
operations.

If the driver code is correct (and the manual page is wrong) the architecture
specific implementation of _bus_dmamap_sync for i386 must be converted from
a no-op to a serializing assembly instruction for BUS_DMASYNC_POSTREAD 
synchronization.
( Otherwise you might get out of order CPU reads of in order DMA read 
operations )

I also fail to see how bounce buffers (32bit PCI device and more than 4GB main 
memory)
would ever work if bus_dmamap_sync(... BUS_DMASYNC_PREWRITE) is allowed to be 
executed
concurrently with a DMA read operation on the same buffer.


Stephan

----------------------
Stephan Uphoff 
ups@stups.com
9275 Martin Road
Roswell, GA 30076
770-518-4058