port-i386/38935: x86 bus_dmamap_sync() should use memory barrier

[Manuel Bouyer <bouyer%antioche.eu.org@localhost>]

>Number:         38935
>Category:       port-i386
>Synopsis:       x86 bus_dmamap_sync() should use memory barrier
>Confidential:   no
>Severity:       critical
>Priority:       high
>Responsible:    port-i386-maintainer
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue Jun 10 18:30:00 +0000 2008
>Originator:     Manuel Bouyer
>Release:        NetBSD current
>Organization:
>Environment:
System: NetBSD 
Architecture: i386/amd64/xen
Machine: i386/amd64/xen
>Description:
        the x86 implementation of bus_dmamap_sync() turn into a NOP if
        we're not using bouce-buffers (which is the common case).
        Yet some DMA devices rely on the read or write operations
        happening in order (e.g. when the driver can add DMA descripors to
        a queue while the device may also be looking at the queue).
        Typical examples are the uhci and ehci drivers (but it's not
        the only example in our tree). Without appropriate memory barrier,
        the CPU's write buffers can reorder writes in a may that makes
        the DMA descriptors in an inconsistent state from the device's view.
        speculative reads can also make memory read occur in a different
        order than the one indended, and cause the driver to misread status
        words.
>How-To-Repeat:
        code inspection
        open/close a umodem device in a loop, try to understand why the
        uhci device sometimes sees bogus DMA descriptors and halts.
>Fix:


Index: x86/bus_dma.c
===================================================================
RCS file: /cvsroot/src/sys/arch/x86/x86/bus_dma.c,v
retrieving revision 1.43
diff -u -r1.43 bus_dma.c
--- x86/bus_dma.c       4 Jun 2008 12:41:42 -0000       1.43
+++ x86/bus_dma.c       10 Jun 2008 18:22:40 -0000
@@ -722,7 +732,7 @@
         */
        if (len == 0 || cookie == NULL ||
            (cookie->id_flags & X86_DMA_IS_BOUNCING) == 0)
-               return;
+               goto end;
 
        switch (cookie->id_buftype) {
        case X86_DMA_BUFTYPE_LINEAR:
@@ -844,6 +854,24 @@
                printf("unknown buffer type %d\n", cookie->id_buftype);
                panic("_bus_dmamap_sync");
        }
+end:
+       if ((ops & (BUS_DMASYNC_PREWRITE|BUS_DMASYNC_POSTWRITE)) &&
+           (ops & (BUS_DMASYNC_PREREAD|BUS_DMASYNC_POSTREAD))) {
+               /* both read and write, we need a full barrier */
+               x86_mfence();
+       } else if (ops & (BUS_DMASYNC_PREWRITE|BUS_DMASYNC_POSTWRITE)) {
+               /*
+                * all past writes should have completed before this point,
+                * and futur writes should not have started yet.
+                */
+               x86_sfence();
+       } else if (ops & (BUS_DMASYNC_PREREAD|BUS_DMASYNC_POSTREAD)) {
+               /*
+                * all past reads should have completed before this point,
+                * and futur reads should not have started yet.
+                */
+               x86_lfence();
+       }
 }
 
 /*