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

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

The following reply was made to PR port-amd64/42980; it has been noted by GNATS.

From: Manuel Bouyer <bouyer%antioche.eu.org@localhost>
To: Julian Bourne <julian.bourne%gmail.com@localhost>
Cc: port-amd64-maintainer%NetBSD.org@localhost, 
gnats-admin%NetBSD.org@localhost,
        netbsd-bugs%NetBSD.org@localhost, gnats-bugs%NetBSD.org@localhost
Subject: Re: port-amd64/42980: satalink DMA fails under amd64
Date: Thu, 18 Mar 2010 17:58:46 +0100

 --yrj/dFKFPuw6o+aM
 Content-Type: text/plain; charset=iso-8859-1
 Content-Disposition: inline
 Content-Transfer-Encoding: 8bit
 
 On Tue, Mar 16, 2010 at 10:02:56PM -0400, Julian Bourne wrote:
 > Further investigation: the EINVAL is being returned by the call to:
 > 
 > bus_dmamem_alloc(...,size=65536,
 >    alignment=4096,  boundary=8192,
 >    nsegs=17, flags=0x3)
 > 
 > from _bus_dma_alloc_bouncebuf() on line 896;
 > from _bus_dmamap_create() on line 283.
 > 
 > This is all from: arch/x86/x86/bus_dma.c revision 1.45 (NetBSD-5.0.2).
 > 
 > Ultimately, this is called from dev/pci/pciide_common.c revision 1.38
 > on line 597 via bus_dmamap_create() - the error is printed out a few
 > lines after.
 
 I think the bug is from _bus_dma_alloc_bouncebuf(), 
 which doesn't pay enough attention to the boundary value vs size, before
 calling bus_dmamem_alloc().
 It seems to me that the right solution is to properly handle the boundary
 in _xen_bus_dmamem_alloc_range() (the x86 version doens't need changes, this
 is handled by uvm_pglistalloc()). 
 I think the attached patch should do it; can you test (and make sure you
 removed your patch before, of course :) ?
 
 -- 
 Manuel Bouyer <bouyer%antioche.eu.org@localhost>
      NetBSD: 26 ans d'experience feront toujours la difference
 --
 
 --yrj/dFKFPuw6o+aM
 Content-Type: text/plain; charset=us-ascii
 Content-Disposition: attachment; filename=diff
 
 Index: 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_bus_dma.c      30 Jan 2010 19:14:20 -0000      1.11.8.1
 +++ xen_bus_dma.c      18 Mar 2010 16:53:52 -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);
 @@ -252,8 +252,6 @@ _xen_bus_dmamem_alloc_range(bus_dma_tag_
        KASSERT((boundary & (boundary - 1)) == 0);
        if (alignment < PAGE_SIZE)
                alignment = PAGE_SIZE;
 -      if (boundary != 0 && boundary < size)
 -              return (EINVAL);
  
        /*
         * Allocate pages from the VM system.
 @@ -282,10 +280,9 @@ 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)
 @@ -343,7 +340,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;
 
 --yrj/dFKFPuw6o+aM--