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/13/2003 10:55:55
Stephan Uphoff wrote:
> Perhaps I should first try to correct/clarify the bus_dma manual page
> to have a base for discussions ? 

I guess I should stop opening interesting looking boxes ...

1) Some of the ugly problems that are not mentioned on the manual page for 
bus_dmamap_sync:


a)  bus_dmamap_sync might sync additional data outside the [offset,len] 
interval due to
   cache line granularity.
   Let us call the whole area the bus_dmamap_sync footprint.
                                 ---------------------------

b) Unfortunately there is currently no machine independent way to determine the
   bus_dmamap_sync footprint of a given bus_dmamap_sync call.

c) Data in the bus_dmamap_sync footprint should not be accessed by the 
processor
   if a BUS_DMASYNC_PREREAD is in effect for the bus_dmamap_sync footprint.
   ( Example: XSCALE architecture?)

   Problems:
    Reading data that falls in the bus_dmamap_sync footprint might cause a 
cacheline to be loaded.
    This cacheline might persist in the cache and present outdated memory 
state to the CPU
    even after a BUS_DMASYNC_POSTREAD sync operation.
    The outdated state in the cacheline can be flushed by a 
BUS_DMASYNC_PREREAD sync.
    ( This could be fixed by  flushing cache lines on BUS_DMASYNC_POSTREAD)

    Writing data also might lead to a stale cache line - however this condition
    can not be reverted on a write-back cache (write through should be ok)
    since the write marks the cacheline as dirty. (and a flush will just save 
it to memory)
    ( This might revert dma region memory with which it shares a cache line to 
an earlier state)

d) DMA_READ operation on memory not currently synced as BUS_DMASYNC_PREREAD.

   Same problems as (c) and the following additional problem that the
   CPU might see some (but not all)  data from dma transfers and the CPU might 
not see the
   data in the order it was transfered by DMA.
   Again BUS_DMASYNC_PREREAD should synchronize.

e) DMA_WRITE operation on memory not currently synced as BUS_DMASYNC_PREWRITE. 

    
   DMA access could see some data written by the CPU before a 
BUS_DMASYNC_PREWRITE operation
   and it might see the data out of order.


-------------------------------------------------------------------------------
----------

2) Examples of race conditions in the i82557 driver:

(I)
Adding a new receive buffer at the end of the DMA queue.
File i82257var.h:  
	#define	FXP_INIT_RFABUF(sc, m  \
	      [...]
		FXP_RFASYNC((sc), __p_m,				\
		    BUS_DMASYNC_POSTREAD|BUS_DMASYNC_POSTWRITE);	\
		/* ## a ## */
		memcpy((void *)&__p_rfa->link_addr, &__v,		\
		    sizeof(__v));					\
		__p_rfa->rfa_control &= htole16(~FXP_RFA_CONTROL_EL);	\
	        /* ## b ## */
		FXP_RFASYNC((sc), __p_m,				\
		    BUS_DMASYNC_PREREAD|BUS_DMASYNC_PREWRITE);		\


Between /* ## a ## */ and /* ## b ## */ DMA reads might be active for the RFA 
and
for the frame data area.

Due to problem d) dma updates to the RFA might be (partially) lost.
If the RFA shares a cacheline with the frame data - parts of the frame data 
might also be lost.

Another problem might be that a DMA write is active for the RFA.
With problem e) the device might observe an RFA where the rfa_control but not 
the link_addr
field has been updated (DMAing the text rfa from a random address)

Fix: < No clean fix >
If the bus_dmamap_sync footprint would always be equal to the requested 
bus_dmamap_sync area
this code could possibly be fixed by sequentially updating and syncing the 
updated fields.
Non-cacheable memory would do the trick but:
Jason R Thorpe wrote: [..] driver should not rely on BUS_DMA_COHERENT for 
correct operation [...]

(II)
File i82257.c
fxp_rxintr(struct fxp_softc *sc)
	[...]

	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.
			 */
			FXP_RFASYNC(sc, m, BUS_DMASYNC_PREREAD);
			return;
		}

		IF_DEQUEUE(&sc->sc_rxq, m);

		FXP_RXBUFSYNC(sc, m, BUS_DMASYNC_POSTREAD);
		/** B **/
		len = le16toh(rfa->actual_size) &
		    (m->m_ext.ext_size - 1);

		if (len < sizeof(struct ether_header)) {
			/*
			 * Runt packet; drop it now.
			 */
			FXP_INIT_RFABUF(sc, m);
			continue;
		}


The rfa might be dma-updated at point /** A **/
Due to problem d) the driver might see the update of the rfa_status field but 
not
the update of the actual_size field at point /** B **/

Fix:
	Add another pair of 
		FXP_RFASYNC(BUS_DMASYNC_PREREAD);
		FXP_RFASYNC(BUS_DMASYNC_POSTREAD);
	at point /** B **/

(This is needed in addition to the i386 architecture specific fix for 
bus_dmamap_sync
mentioned in my last email)