Re: port-amd64/42980: satalink DMA fails under amd64

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

On Thu, Mar 18, 2010 at 08:15:24PM +0100, Manuel Bouyer wrote:
> On Thu, Mar 18, 2010 at 02:27:42PM -0400, Julian Bourne wrote:
> > OK, I verified several things with the 5.0.2 kernel:
> > 
> > (1) amd64 GENERIC (non xen) suffers as well
> > (2) amd64 XEN_DOM0 suffers with or without Manuel's patch
> > (3) uvm_pglistalloc has the boundary >= size check as well
> 
> OK; if uvm_pglistalloc had this check too, my patch won't help.
> I'll see how this can be fixed. uvm(9) doesn't say that
> uvm_pglistalloc() should not be called with a boundary smaller than
> size. Maybe it needs to be fixed too

uvm_pglistalloc() is not easy to fix, it's really not designed to hanble
it this way (the man page should probably be fixed instead).

But fixing the x86 bus_dma(9) isn't hard: we can call uvm_pglistalloc()
with a larger boundary, and split the result in multiple segments if needed
here (uvm_pglistalloc() doesn't split anyway).

The attached patch should do it, can you try it ?

-- 
Manuel Bouyer <bouyer%antioche.eu.org@localhost>
     NetBSD: 26 ans d'experience feront toujours la difference
--

Index: x86/x86/bus_dma.c
===================================================================
RCS file: /cvsroot/src/sys/arch/x86/x86/bus_dma.c,v
retrieving revision 1.45
diff -u -p -u -r1.45 bus_dma.c
--- x86/x86/bus_dma.c   28 Jun 2008 17:23:01 -0000      1.45
+++ x86/x86/bus_dma.c   19 Mar 2010 13:05:05 -0000
@@ -159,10 +159,13 @@ _bus_dmamem_alloc_range(bus_dma_tag_t t,
        /* Always round the size. */
        size = round_page(size);
 
+       KASSERT(boundary >= PAGE_SIZE || boundary == 0);
+
        /*
         * Allocate pages from the VM system.
         */
-       error = uvm_pglistalloc(size, low, high, alignment, boundary,
+       error = uvm_pglistalloc(size, low, high, alignment,
+           (boundary > size) ? boundary : size,
            &mlist, nsegs, (flags & BUS_DMA_NOWAIT) == 0);
        if (error)
                return (error);
@@ -186,10 +189,13 @@ _bus_dmamem_alloc_range(bus_dma_tag_t t,
                        panic("_bus_dmamem_alloc_range");
                }
 #endif
-               if (curaddr == (lastaddr + PAGE_SIZE))
+               if (curaddr == (lastaddr + PAGE_SIZE) &&
+                   (lastaddr & boundary) == (curaddr & boundary)) {
                        segs[curseg].ds_len += PAGE_SIZE;
-               else {
+               } else {
                        curseg++;
+                       if (curseg >= nsegs)
+                               return EFBIG;
                        segs[curseg].ds_addr = curaddr;
                        segs[curseg].ds_len = PAGE_SIZE;
                }
Index: xen/x86/xen_bus_dma.c
===================================================================
RCS file: /cvsroot/src/sys/arch/xen/x86/xen_bus_dma.c,v
retrieving revision 1.11.8.1
diff -u -p -u -r1.11.8.1 xen_bus_dma.c
--- xen/x86/xen_bus_dma.c       30 Jan 2010 19:14:20 -0000      1.11.8.1
+++ xen/x86/xen_bus_dma.c       19 Mar 2010 13:05:05 -0000
@@ -61,7 +61,7 @@ static inline int get_order(unsigned lon
 }
 
 static int
-_xen_alloc_contig(bus_size_t size, bus_size_t alignment, bus_size_t boundary,
+_xen_alloc_contig(bus_size_t size, bus_size_t alignment,
     struct pglist *mlistp, int flags, bus_addr_t low, bus_addr_t high)
 {
        int order, i;
@@ -75,9 +75,9 @@ _xen_alloc_contig(bus_size_t size, bus_s
 
        /*
         * When requesting a contigous memory region, the hypervisor will
-        * return a memory range aligned on size. This will automagically
-        * handle "boundary", but the only way to enforce alignment
-        * is to request a memory region of size max(alignment, size).
+        * return a memory range aligned on size. 
+        * The only way to enforce alignment is to request a memory region
+        * of size max(alignment, size).
         */
        order = max(get_order(size), get_order(alignment));
        npages = (1 << order);
@@ -250,15 +250,16 @@ _xen_bus_dmamem_alloc_range(bus_dma_tag_
 
        KASSERT((alignment & (alignment - 1)) == 0);
        KASSERT((boundary & (boundary - 1)) == 0);
+       KASSERT(boundary >= PAGE_SIZE || boundary == 0);
+                   
        if (alignment < PAGE_SIZE)
                alignment = PAGE_SIZE;
-       if (boundary != 0 && boundary < size)
-               return (EINVAL);
 
        /*
         * Allocate pages from the VM system.
         */
-       error = uvm_pglistalloc(size, 0, avail_end, alignment, boundary,
+       error = uvm_pglistalloc(size, 0, avail_end, alignment,
+           (boundary > size) ? boundary : size,
            &mlist, nsegs, (flags & BUS_DMA_NOWAIT) == 0);
        if (error)
                return (error);
@@ -282,14 +283,18 @@ again:
                curaddr = _BUS_VM_PAGE_TO_BUS(m);
                if (curaddr < low || curaddr >= high)
                        goto badaddr;
-               if (curaddr == (lastaddr + PAGE_SIZE)) {
+               if (curaddr == (lastaddr + PAGE_SIZE) &&
+                   (lastaddr & boundary) == (curaddr & boundary)) {
                        segs[curseg].ds_len += PAGE_SIZE;
-                       if ((lastaddr & boundary) != (curaddr & boundary))
-                               goto dorealloc;
                } else {
                        curseg++;
-                       if (curseg >= nsegs || (curaddr & (alignment - 1)) != 0)
-                               goto dorealloc;
+                       if (curseg >= nsegs ||
+                           (curaddr & (alignment - 1)) != 0) {
+                               if (doingrealloc)
+                                       return EFBIG;
+                               else
+                                       goto dorealloc;
+                       }
                        segs[curseg].ds_addr = curaddr;
                        segs[curseg].ds_len = PAGE_SIZE;
                }
@@ -343,7 +348,7 @@ dorealloc:
                segs[curseg].ds_len = 0;
        }
        error = _xen_alloc_contig(size, alignment,
-           boundary, &mlist, flags, low, high);
+           &mlist, flags, low, high);
        if (error)
                return error;
        goto again;