NetBSD Problem Report #42980
From www@NetBSD.org Mon Mar 15 23:31:18 2010
Return-Path: <www@NetBSD.org>
Received: from mail.netbsd.org (mail.netbsd.org [204.152.190.11])
by www.NetBSD.org (Postfix) with ESMTP id 58DF063C49B
for <gnats-bugs@gnats.NetBSD.org>; Mon, 15 Mar 2010 23:31:18 +0000 (UTC)
Message-Id: <20100315233117.EEEA263B873@www.NetBSD.org>
Date: Mon, 15 Mar 2010 23:31:17 +0000 (UTC)
From: julian.bourne@gmail.com
Reply-To: julian.bourne@gmail.com
To: gnats-bugs@NetBSD.org
Subject: satalink DMA fails under amd64
X-Send-Pr-Version: www-1.0
>Number: 42980
>Category: port-amd64
>Synopsis: satalink DMA fails under amd64
>Confidential: no
>Severity: serious
>Priority: medium
>Responsible: port-amd64-maintainer
>State: closed
>Class: sw-bug
>Submitter-Id: net
>Arrival-Date: Mon Mar 15 23:35:00 +0000 2010
>Closed-Date: Sat Aug 25 19:07:07 +0000 2012
>Last-Modified: Sat Aug 25 19:07:07 +0000 2012
>Originator: Julian Bourne
>Release: 5.0.2
>Organization:
>Environment:
NetBSD 5.0.2 (XEN3_DOM0) #0
>Description:
The satalink driver doesn't support DMA under NetBSD 5.0.2 amd64. It works properly on exactly the same hardware under NetBSD 5.0.2 i386.
satalink0 at pci1 dev 9 function 0
satalink0: Silicon Image SATALink 3512 (rev. 0x01)
satalink0: SATALink BA5 register space disabled
satalink0: bus-master DMA support present
satalink0: primary channel wired to native-PCI mode
satalink0: using ioapic0 pin 17, event channel 10 for native-PCI interrupt
atabus1 at satalink0 channel 0
satalink0: secondary channel wired to native-PCI mode
atabus2 at satalink0 channel 1
satalink0: port 0: device present, speed: 1.5Gb/s
satalink0: port 1: device present, speed: 1.5Gb/s
satalink0:0: unable to create xfer table DMA map for drive 0, error=22
wd0(satalink0:0:0): using PIO mode 4
satalink0:1: unable to create xfer table DMA map for drive 0, error=22
wd1(satalink0:1:0): using PIO mode 4
>How-To-Repeat:
Use an amd64 system equipped with a PCI bus and a SataLink 3512 card. Boot with NetBSD 5.0.2 amd64 (xen or native); it fails with the message described, and falls back to PIO mode 4 giving about 3MB/s for attached devices.
Boot with NetBSD 5.0.2 i386 (native); it succeeds for DMA and gives about 100MB/s for attached devices (in this case a pair of WD7500AALS drives).
>Fix:
Not sure exactly how to solve the problem, but I did look at the satalink code and it seems like it *might* be failing because it requests 32bit memory, rather than 64bit memory for DMA.
I have the time and energy to devote to fixing this - but I'd rather do it in a coordinated way with the port maintainer and owner of the satalink code. Contact me and I can test/debug etc.
>Release-Note:
>Audit-Trail:
From: Manuel Bouyer <bouyer@antioche.eu.org>
To: gnats-bugs@NetBSD.org
Cc: port-amd64-maintainer@NetBSD.org, gnats-admin@NetBSD.org,
netbsd-bugs@NetBSD.org
Subject: Re: port-amd64/42980: satalink DMA fails under amd64
Date: Tue, 16 Mar 2010 21:36:01 +0100
On Mon, Mar 15, 2010 at 11:35:01PM +0000, julian.bourne@gmail.com wrote:
> >Number: 42980
> >Category: port-amd64
> >Synopsis: satalink DMA fails under amd64
> >Confidential: no
> >Severity: serious
> >Priority: medium
> >Responsible: port-amd64-maintainer
> >State: open
> >Class: sw-bug
> >Submitter-Id: net
> >Arrival-Date: Mon Mar 15 23:35:00 +0000 2010
> >Originator: Julian Bourne
> >Release: 5.0.2
> >Organization:
> >Environment:
> NetBSD 5.0.2 (XEN3_DOM0) #0
> >Description:
> The satalink driver doesn't support DMA under NetBSD 5.0.2 amd64. It works properly on exactly the same hardware under NetBSD 5.0.2 i386.
>
> satalink0 at pci1 dev 9 function 0
> satalink0: Silicon Image SATALink 3512 (rev. 0x01)
> satalink0: SATALink BA5 register space disabled
> satalink0: bus-master DMA support present
> satalink0: primary channel wired to native-PCI mode
> satalink0: using ioapic0 pin 17, event channel 10 for native-PCI interrupt
> atabus1 at satalink0 channel 0
> satalink0: secondary channel wired to native-PCI mode
> atabus2 at satalink0 channel 1
> satalink0: port 0: device present, speed: 1.5Gb/s
> satalink0: port 1: device present, speed: 1.5Gb/s
> satalink0:0: unable to create xfer table DMA map for drive 0, error=22
> wd0(satalink0:0:0): using PIO mode 4
> satalink0:1: unable to create xfer table DMA map for drive 0, error=22
> wd1(satalink0:1:0): using PIO mode 4
>
>
>
> >How-To-Repeat:
> Use an amd64 system equipped with a PCI bus and a SataLink 3512 card. Boot with NetBSD 5.0.2 amd64 (xen or native); it fails with the message described, and falls back to PIO mode 4 giving about 3MB/s for attached devices.
>
> Boot with NetBSD 5.0.2 i386 (native); it succeeds for DMA and gives about 100MB/s for attached devices (in this case a pair of WD7500AALS drives).
> >Fix:
> Not sure exactly how to solve the problem, but I did look at the satalink code and it seems like it *might* be failing because it requests 32bit memory, rather than 64bit memory for DMA.
It's expected that the driver requests 32bit memory for DMA if the card
doesn't support 64bits. 22 is EINVAL, could you see which bus_dma call
fails, and what arguments were passed to it ? it looks like a
bus_dma(9) function is called with wrong arguments.
--
Manuel Bouyer <bouyer@antioche.eu.org>
NetBSD: 26 ans d'experience feront toujours la difference
--
From: Julian Bourne <julian.bourne@gmail.com>
To: gnats-bugs@netbsd.org
Cc: port-amd64-maintainer@netbsd.org, gnats-admin@netbsd.org,
netbsd-bugs@netbsd.org
Subject: Re: port-amd64/42980: satalink DMA fails under amd64
Date: Tue, 16 Mar 2010 17:05:19 -0400
Thanks for the guidance - I will get the details of those calls and params
back to you. Hopefully I can look at it this evening.
From: David Holland <dholland-gnats@netbsd.org>
To: gnats-bugs@netbsd.org
Cc:
Subject: Re: port-amd64/42980: satalink DMA fails under amd64
Date: Wed, 17 Mar 2010 19:42:30 +0000
X-Gnats-is-Stupid: only mail sent to gnats-bugs is "seen" automatically
------
From: Julian Bourne <julian.bourne@gmail.com>
To: port-amd64-maintainer@netbsd.org, gnats-admin@netbsd.org,
netbsd-bugs@netbsd.org, julian.bourne@gmail.com
Subject: Re: port-amd64/42980: satalink DMA fails under amd64
Date: Tue, 16 Mar 2010 22:02:56 -0400
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 haven't gone any deeper down the rabbit hole yet - let me know if you
want more info.
Thanks.
From: Julian Bourne <julian.bourne@gmail.com>
To: gnats-bugs@netbsd.org
Cc: port-amd64-maintainer@netbsd.org, gnats-admin@netbsd.org,
netbsd-bugs@netbsd.org
Subject: Re: port-amd64/42980: satalink DMA fails under amd64
Date: Wed, 17 Mar 2010 16:14:34 -0400
I found this morning that the EINVAL is due
to boundary (8192) being less than size (65536).
I've forced the boundary up to the size, as an experiment - metacode:
(boundary > 0 ? MAX(boundary, size) : boundary)
and now satalink DMA works. That correction was made to
pciide_common.c on line 597 where the
call uses the constant MAXPHYS (65536) for the
size of the allocation.
But it looks to me like the root cause is actually
here in satalink.c (line 480):
/*
* Rev. <= 0x01 of the 3112 have a bug that can cause data
* corruption if DMA transfers cross an 8K boundary. This is
* apparently hard to tickle, but we'll go ahead and play it
* safe.
*/
if (PCI_REVISION(pa->pa_class) <= 0x01) {
sc->sc_dma_maxsegsz = 8192;
sc->sc_dma_boundary = 8192;
}
Given that the 3512 uses the 3112 code, that's
probably not applicable; however, it would still
be good if 3112 DMA worked on AMDs - so what
should change to make that work?
Should pciide_common be using
sc_dma_maxsegsz instead of MAXPHYS?
Thanks,
Julian.
From: Manuel Bouyer <bouyer@antioche.eu.org>
To: Julian Bourne <julian.bourne@gmail.com>
Cc: port-amd64-maintainer@NetBSD.org, gnats-admin@NetBSD.org,
netbsd-bugs@NetBSD.org, gnats-bugs@NetBSD.org
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>
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--
From: Manuel Bouyer <bouyer@antioche.eu.org>
To: matthew green <mrg@eterna.com.au>
Cc: Julian Bourne <julian.bourne@gmail.com>, port-amd64-maintainer@NetBSD.org,
gnats-admin@NetBSD.org, netbsd-bugs@NetBSD.org, gnats-bugs@NetBSD.org
Subject: Re: port-amd64/42980: satalink DMA fails under amd64
Date: Thu, 18 Mar 2010 17:38:07 +0100
On Thu, Mar 18, 2010 at 12:45:36PM +1100, matthew green wrote:
> you're right.
>
> i guess the bus_dmamap_create() call needs to use this in place
> of the IDEDMA_TBL_ALIGN argument?
>
> min(dma_table_size, IDEDMA_TBL_ALIGN);
Now I'm confused. Why would this change anything (especially as AFAIK
dma_table_size can't be larger than IDEDMA_TBL_ALIGN) ?
Also, from what I understood, the problem comes from this call:
if ((error = bus_dmamap_create(sc->sc_dmat, MAXPHYS,
NIDEDMA_TABLES(sc), sc->sc_dma_maxsegsz, sc->sc_dma_boundary,
BUS_DMA_NOWAIT | BUS_DMA_ALLOCNOW,
&dma_maps->dmamap_xfer)) != 0) {
not from the one a few lines above, which uses IDEDMA_TBL_ALIGN.
I think this bus_dmamap_create() call is correct. The man page doesn't
prevent a boundary smaller than the total size (a boundary smaller than the
max segment size doesn't make sense, of course).
--
Manuel Bouyer <bouyer@antioche.eu.org>
NetBSD: 26 ans d'experience feront toujours la difference
--
From: Manuel Bouyer <bouyer@antioche.eu.org>
To: Julian Bourne <julian.bourne@gmail.com>
Cc: matthew green <mrg@eterna.com.au>, port-amd64-maintainer@NetBSD.org,
gnats-admin@NetBSD.org, netbsd-bugs@NetBSD.org, gnats-bugs@NetBSD.org
Subject: Re: port-amd64/42980: satalink DMA fails under amd64
Date: Thu, 18 Mar 2010 17:11:04 +0100
On Wed, Mar 17, 2010 at 09:59:58PM -0400, Julian Bourne wrote:
> Quick update - just verified that your exact patch works
> too Matthew - and its a good thing to have in there
> anyway (target the 8192 boundary workaround at the
> right chipset).
>
> I'll let you and Manuel work out the right way to address
> the 3112 - please keep me on the distribution - I'm
> curious about the answer.
>
> In the meantime, I looked into why the i386 isn't
> exhibiting the problem - it looks like it might be because
> the arch/x86/x86/bus_dma:_bus_dmamem_alloc_range()
> function simply doesn't make the same check
> (boundary >= size) as the xen implementation in arch/xen/x86/xen_bus_dma.c?
>
> Which begs the question, should it? (given that
> bus_dmamem_alloc man page implies its a not a legal
> argument (and thanks for the awesome man pages by the
> way - NetBSD rules)).
I think I explicitely added this check because the hypervisor
can't handle a boundary constraint explicitely, and the allocator
will return a memory that fits the constraint only if boundary >= size
(thanksfully the specs already said that :)
--
Manuel Bouyer <bouyer@antioche.eu.org>
NetBSD: 26 ans d'experience feront toujours la difference
--
From: Julian Bourne <julian.bourne@gmail.com>
To: gnats-bugs@netbsd.org
Cc: port-amd64-maintainer@netbsd.org, gnats-admin@netbsd.org,
netbsd-bugs@netbsd.org
Subject: Re: port-amd64/42980: satalink DMA fails under amd64
Date: Thu, 18 Mar 2010 13:22:57 -0400
Will try that patch out now - and will remove Matthew's patch to
expose the issue again. Give me 30 mins.
From: Julian Bourne <julian.bourne@gmail.com>
To: gnats-bugs@netbsd.org, port-amd64-maintainer@netbsd.org,
gnats-admin@netbsd.org, netbsd-bugs@netbsd.org
Cc:
Subject: Re: port-amd64/42980: satalink DMA fails under amd64
Date: Thu, 18 Mar 2010 13:54:20 -0400
That patch leaves us back where we started:
satalink0:0: unable to create xfer table DMA map for drive 0, error=22
wd0(satalink0:0:0): using PIO mode 4
satalink0:1: unable to create xfer table DMA map for drive 0, error=22
wd1(satalink0:1:0): using PIO mode 4
along with the subsequent performance issues.
Note also, I had the same behaviour with the regular amd64 (non xen);
so it may be that this boundary versus size check is made other places
as well.
I will verify a few things to make sure that I'm not giving you all
the wrong picture here - but I think its more general than just the
xen code.
From: Julian Bourne <julian.bourne@gmail.com>
To: gnats-bugs@netbsd.org, port-amd64-maintainer@netbsd.org,
gnats-admin@netbsd.org, netbsd-bugs@netbsd.org, julian.bourne@gmail.com
Cc:
Subject: Re: port-amd64/42980: satalink DMA fails under amd64
Date: Thu, 18 Mar 2010 14:27:42 -0400
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
See trace I added to the code from my kernel logs:
../../../../arch/xen/x86/xen_bus_dma.c:259
uvm_pglistalloc failed error=22, size=65536,
alignment=4096, boundary=8192.
satalink0:0: unable to create xfer table DMA map
for drive 0, error=22
wd0(satalink0:0:0): using PIO mode 4
Hope that helps.
If I can throw in 2c, I think the problem is that the bouncebuf
create call should be using the max segment size from the sc
structure, not MAXPHYS, but I admit, I'm way out of my depth.
From: Manuel Bouyer <bouyer@antioche.eu.org>
To: Julian Bourne <julian.bourne@gmail.com>
Cc: gnats-bugs@NetBSD.org, port-amd64-maintainer@NetBSD.org,
gnats-admin@NetBSD.org, netbsd-bugs@NetBSD.org
Subject: Re: port-amd64/42980: satalink DMA fails under amd64
Date: Thu, 18 Mar 2010 20:15:24 +0100
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
>
> If I can throw in 2c, I think the problem is that the bouncebuf
> create call should be using the max segment size from the sc
> structure, not MAXPHYS, but I admit, I'm way out of my depth.
It would then need to create the segment list by itself, where
we already have code to handle this.
--
Manuel Bouyer <bouyer@antioche.eu.org>
NetBSD: 26 ans d'experience feront toujours la difference
--
From: Manuel Bouyer <bouyer@antioche.eu.org>
To: Julian Bourne <julian.bourne@gmail.com>
Cc: gnats-bugs@NetBSD.org, port-amd64-maintainer@NetBSD.org,
gnats-admin@NetBSD.org, netbsd-bugs@NetBSD.org
Subject: Re: port-amd64/42980: satalink DMA fails under amd64
Date: Fri, 19 Mar 2010 14:16:19 +0100
--LZvS9be/3tNcYl/X
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
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>
NetBSD: 26 ans d'experience feront toujours la difference
--
--LZvS9be/3tNcYl/X
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="busdma.diff"
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;
--LZvS9be/3tNcYl/X--
From: Julian Bourne <julian.bourne@gmail.com>
To: gnats-bugs@netbsd.org
Cc: port-amd64-maintainer@netbsd.org, gnats-admin@netbsd.org,
netbsd-bugs@netbsd.org
Subject: Re: port-amd64/42980: satalink DMA fails under amd64
Date: Fri, 19 Mar 2010 09:47:39 -0400
I applied the patch to 5.0.2 (hope that was right - it
applied cleanly) and rebuilt my xen dom0 target.
It hit a __kern_assert() in uvm_pglistalloc():403
called from xen_bus_dmamem_alloc_range():
(boundary & (boundary - 1) == 0) failed
Thanks for the explanation btw - that makes some
sense to me. Hope this helps.
From: Manuel Bouyer <bouyer@antioche.eu.org>
To: Julian Bourne <julian.bourne@gmail.com>
Cc: gnats-bugs@NetBSD.org, port-amd64-maintainer@NetBSD.org,
gnats-admin@NetBSD.org, netbsd-bugs@NetBSD.org
Subject: Re: port-amd64/42980: satalink DMA fails under amd64
Date: Fri, 19 Mar 2010 16:41:16 +0100
On Fri, Mar 19, 2010 at 09:47:39AM -0400, Julian Bourne wrote:
> I applied the patch to 5.0.2 (hope that was right - it
> applied cleanly) and rebuilt my xen dom0 target.
>
> It hit a __kern_assert() in uvm_pglistalloc():403
> called from xen_bus_dmamem_alloc_range():
>
> (boundary & (boundary - 1) == 0) failed
that's strange, because in xen_bus_dmamem_alloc_range() this could be either
boundary (and this check is already done in xen_bus_dmamem_alloc_range())
or size (which is rounded to PAGE_SIZE). Could you print the
values for boundary and size here ?
--
Manuel Bouyer <bouyer@antioche.eu.org>
NetBSD: 26 ans d'experience feront toujours la difference
--
From: Manuel Bouyer <bouyer@antioche.eu.org>
To: Julian Bourne <julian.bourne@gmail.com>
Cc: gnats-bugs@NetBSD.org, port-amd64-maintainer@NetBSD.org,
gnats-admin@NetBSD.org, netbsd-bugs@NetBSD.org
Subject: Re: port-amd64/42980: satalink DMA fails under amd64
Date: Fri, 19 Mar 2010 17:06:25 +0100
--2oS5YaxWCcQjTEyO
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
On Fri, Mar 19, 2010 at 04:41:16PM +0100, Manuel Bouyer wrote:
> On Fri, Mar 19, 2010 at 09:47:39AM -0400, Julian Bourne wrote:
> > I applied the patch to 5.0.2 (hope that was right - it
> > applied cleanly) and rebuilt my xen dom0 target.
> >
> > It hit a __kern_assert() in uvm_pglistalloc():403
> > called from xen_bus_dmamem_alloc_range():
> >
> > (boundary & (boundary - 1) == 0) failed
>
> that's strange, because in xen_bus_dmamem_alloc_range() this could be either
> boundary (and this check is already done in xen_bus_dmamem_alloc_range())
> or size (which is rounded to PAGE_SIZE). Could you print the
> values for boundary and size here ?
ignore this, rounding up to PAGE_SIZE is not enough to make it a power of 2.
The attached patch should be correct (at last I hope so, it's becoming more
complex than I'd like ...)
--
Manuel Bouyer <bouyer@antioche.eu.org>
NetBSD: 26 ans d'experience feront toujours la difference
--
--2oS5YaxWCcQjTEyO
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="busdma.diff"
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 16:05:28 -0000
@@ -155,14 +155,28 @@ _bus_dmamem_alloc_range(bus_dma_tag_t t,
struct vm_page *m;
struct pglist mlist;
int curseg, error;
+ bus_size_t uboundary;
/* 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,
+ * We accept boundaries < size, splitting in multiple segments
+ * if needed. uvm_pglistalloc does not, so compute an appropriate
+ * boundary: next power of 2 >= size
+ */
+
+ if (boundary == 0)
+ uboundary = 0;
+ else {
+ uboundary = boundary;
+ while (uboundary < size)
+ uboundary = uboundary << 1;
+ }
+ error = uvm_pglistalloc(size, low, high, alignment, uboundary,
&mlist, nsegs, (flags & BUS_DMA_NOWAIT) == 0);
if (error)
return (error);
@@ -186,10 +200,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 16:05:28 -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);
@@ -244,21 +244,32 @@ _xen_bus_dmamem_alloc_range(bus_dma_tag_
struct pglist mlist;
int curseg, error;
int doingrealloc = 0;
+ bus_size_t uboundary;
/* Always round the size. */
size = round_page(size);
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.
+ * We accept boundaries < size, splitting in multiple segments
+ * if needed. uvm_pglistalloc does not, so compute an appropriate
+ * boundary: next power of 2 >= size
*/
- error = uvm_pglistalloc(size, 0, avail_end, alignment, boundary,
+ if (boundary == 0)
+ uboundary = 0;
+ else {
+ uboundary = boundary;
+ while (uboundary < size)
+ uboundary = uboundary << 1;
+ }
+ error = uvm_pglistalloc(size, 0, avail_end, alignment, uboundary,
&mlist, nsegs, (flags & BUS_DMA_NOWAIT) == 0);
if (error)
return (error);
@@ -282,14 +293,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 +358,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;
--2oS5YaxWCcQjTEyO--
From: Julian Bourne <julian.bourne@gmail.com>
To: gnats-bugs@netbsd.org
Cc: port-amd64-maintainer@netbsd.org, gnats-admin@netbsd.org,
netbsd-bugs@netbsd.org
Subject: Re: port-amd64/42980: satalink DMA fails under amd64
Date: Fri, 19 Mar 2010 12:39:25 -0400
This latest patch seems to work. Getting ~100MB/s which is reasonable
for the attached media, 1.5Gb/s PCI card and 8192 byte boundary issue.
To clarify for everyone not keeping up with the full
thread, this patch has been applied to a vanilla
5.0.2 sys src tree and is running under a stock
amd64 XEN3_DOM0 configuration in debug mode.
I will put this through its paces building a 5.0.2 amd64 GENERIC
kernel and then boot that and test it too, to see if it solves that
case as well.
Expect the answer to that in about 20 minutes.
From: Julian Bourne <julian.bourne@gmail.com>
To: gnats-bugs@netbsd.org
Cc: port-amd64-maintainer@netbsd.org, gnats-admin@netbsd.org,
netbsd-bugs@netbsd.org
Subject: Re: port-amd64/42980: satalink DMA fails under amd64
Date: Fri, 19 Mar 2010 13:10:34 -0400
The 5.0.2 amd64 GENERIC kernel works with the
patch too - 110MB/s read, 66MB/s write with DMA
enabled.
Would there be any sense in testing an i386 kernel
as well? It didn't exhibit the problem before, but I
assume it shares some code with the patch?
I'd also be happy to test the netbsd-5 branch, same
kernel scenarios, either with the patch or commits -
let me know.
From: Manuel Bouyer <bouyer@antioche.eu.org>
To: Julian Bourne <julian.bourne@gmail.com>
Cc: gnats-bugs@NetBSD.org, port-amd64-maintainer@NetBSD.org,
gnats-admin@NetBSD.org, netbsd-bugs@NetBSD.org
Subject: Re: port-amd64/42980: satalink DMA fails under amd64
Date: Mon, 22 Mar 2010 13:04:09 +0100
On Fri, Mar 19, 2010 at 01:10:34PM -0400, Julian Bourne wrote:
> The 5.0.2 amd64 GENERIC kernel works with the
> patch too - 110MB/s read, 66MB/s write with DMA
> enabled.
Thanks for the testings !
>
> Would there be any sense in testing an i386 kernel
> as well? It didn't exhibit the problem before, but I
> assume it shares some code with the patch?
You could test that the kernel boots and finds its root, but I don't
expect issues there.
>
> I'd also be happy to test the netbsd-5 branch, same
> kernel scenarios, either with the patch or commits -
> let me know.
You'll be notified when this gets pulled up - I fear it's late to make
it to 5.1_RELEASE though
--
Manuel Bouyer <bouyer@antioche.eu.org>
NetBSD: 26 ans d'experience feront toujours la difference
--
From: Julian Bourne <julian.bourne@gmail.com>
To: gnats-bugs@netbsd.org
Cc: port-amd64-maintainer@netbsd.org, gnats-admin@netbsd.org,
netbsd-bugs@netbsd.org
Subject: Re: port-amd64/42980: satalink DMA fails under amd64
Date: Mon, 22 Mar 2010 09:58:07 -0400
Cool - I will compile a 5.0.2 x86 kernel with that
last patch and see if it boots cleanly.
From: Julian Bourne <julian.bourne@gmail.com>
To: gnats-bugs@netbsd.org
Cc: port-amd64-maintainer@netbsd.org, gnats-admin@netbsd.org,
netbsd-bugs@netbsd.org
Subject: Re: port-amd64/42980: satalink DMA fails under amd64
Date: Mon, 22 Mar 2010 11:27:13 -0400
The i386 GENERIC kernel works with the patch
too - same performance as for the amd64 above.
Thanks Manuel for going so deep on this.
Thanks also Matthew for the response on the
satalink.c code itself - I think that patch is valid.
Last thing, IMHO this man page entry could do
with some adjustment:
bus_dmamem_alloc()
src/share/man/man9/bus_dma.9:555
.It Fa boundary
Each segment in the allocated memory must not cross this boundary
(relative to zero).
This value must be a power of two.
A boundary value less than the size of the allocation is invalid.
As I understand it, the boundary value is only
necessarily more than each segment size, not
the total allocation size?
Thanks everyone for taking the care to fix the root issue, not just the symptom.
From: Manuel Bouyer <bouyer@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/42980 CVS commit: src/sys/arch
Date: Mon, 22 Mar 2010 22:03:30 +0000
Module Name: src
Committed By: bouyer
Date: Mon Mar 22 22:03:30 UTC 2010
Modified Files:
src/sys/arch/x86/x86: bus_dma.c
src/sys/arch/xen/x86: xen_bus_dma.c
Log Message:
bus_dmamem_alloc() may not get a boundary smaller than size, but
it's perfectly valid for bus_dmamap_create() to do so (a contigous
transfers will then split in multiple segment).
Fix _xen_bus_dmamem_alloc_range() and _bus_dmamem_alloc_range() to
allow a boundary limit smaller than size:
- compute appropriate boundary for uvm_pglistalloc(), wich doesn't
accept boundary < size
- also take care of boundary when deciding to start a new segment.
While there, remove useless boundary argument to _xen_alloc_contig().
Fix the boundary-related issue of PR port-amd64/42980
To generate a diff of this commit:
cvs rdiff -u -r1.53 -r1.54 src/sys/arch/x86/x86/bus_dma.c
cvs rdiff -u -r1.20 -r1.21 src/sys/arch/xen/x86/xen_bus_dma.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
From: matthew green <mrg@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/42980 CVS commit: src/sys/dev/pci
Date: Tue, 23 Mar 2010 03:24:53 +0000
Module Name: src
Committed By: mrg
Date: Tue Mar 23 03:24:53 UTC 2010
Modified Files:
src/sys/dev/pci: satalink.c
Log Message:
only apply the satalink 3112 rev 0.1 and earlier to actual 3112 based
cards, not eg, 3512 cards.
should help performance for 3512 cards, derived from discussions in
PR#42980: satalink DMA fails under amd64.
To generate a diff of this commit:
cvs rdiff -u -r1.39 -r1.40 src/sys/dev/pci/satalink.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
From: "Jeff Rizzo" <riz@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/42980 CVS commit: [netbsd-5] src/sys/arch
Date: Fri, 19 Nov 2010 23:19:13 +0000
Module Name: src
Committed By: riz
Date: Fri Nov 19 23:19:13 UTC 2010
Modified Files:
src/sys/arch/x86/x86 [netbsd-5]: bus_dma.c
src/sys/arch/xen/x86 [netbsd-5]: xen_bus_dma.c
Log Message:
Pull up following revision(s) (requested by bouyer in ticket #1348):
sys/arch/x86/x86/bus_dma.c: revision 1.54
sys/arch/xen/x86/xen_bus_dma.c: revision 1.21
bus_dmamem_alloc() may not get a boundary smaller than size, but
it's perfectly valid for bus_dmamap_create() to do so (a contigous
transfers will then split in multiple segment).
Fix _xen_bus_dmamem_alloc_range() and _bus_dmamem_alloc_range() to
allow a boundary limit smaller than size:
- compute appropriate boundary for uvm_pglistalloc(), wich doesn't
accept boundary < size
- also take care of boundary when deciding to start a new segment.
While there, remove useless boundary argument to _xen_alloc_contig().
Fix the boundary-related issue of PR port-amd64/42980
To generate a diff of this commit:
cvs rdiff -u -r1.45 -r1.45.6.1 src/sys/arch/x86/x86/bus_dma.c
cvs rdiff -u -r1.11.8.2 -r1.11.8.3 src/sys/arch/xen/x86/xen_bus_dma.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
State-Changed-From-To: open->feedback
State-Changed-By: dholland@NetBSD.org
State-Changed-When: Thu, 01 Dec 2011 00:04:05 +0000
State-Changed-Why:
This appears to have been committed and pulled up a year ago - is it fixed
or is it still awaiting additional work?
From: Julian Bourne <julian.bourne@gmail.com>
To: gnats-bugs@netbsd.org
Cc:
Subject: Re: port-amd64/42980 (satalink DMA fails under amd64)
Date: Thu, 1 Dec 2011 11:55:40 -0500
--0015174c3418f653c204b30ab833
Content-Type: text/plain; charset=UTF-8
I raised the original issue and its fixed as far as I am concerned. There
might have been some manual pages that needed updating and so forth - so
perhaps the devs can answer that?
On Wed, Nov 30, 2011 at 7:04 PM, <dholland@netbsd.org> wrote:
> Synopsis: satalink DMA fails under amd64
>
> State-Changed-From-To: open->feedback
> State-Changed-By: dholland@NetBSD.org
> State-Changed-When: Thu, 01 Dec 2011 00:04:05 +0000
> State-Changed-Why:
> This appears to have been committed and pulled up a year ago - is it fixed
> or is it still awaiting additional work?
>
>
>
>
--0015174c3418f653c204b30ab833
Content-Type: text/html; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
I raised the original issue and its fixed as far as I am concerned.=C2=A0 T=
here might have been some manual pages that needed updating and so forth - =
so perhaps the devs can answer that?<br><br><div class=3D"gmail_quote">On W=
ed, Nov 30, 2011 at 7:04 PM, <span dir=3D"ltr"><<a href=3D"mailto:dholl=
and@netbsd.org">dholland@netbsd.org</a>></span> wrote:<br>
<blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;border-left:1p=
x #ccc solid;padding-left:1ex;">Synopsis: satalink DMA fails under amd64<br=
>
<br>
State-Changed-From-To: open->feedback<br>
State-Changed-By: dholland@NetBSD.org<br>
State-Changed-When: Thu, 01 Dec 2011 00:04:05 +0000<br>
State-Changed-Why:<br>
This appears to have been committed and pulled up a year ago - is it fixed<=
br>
or is it still awaiting additional work?<br>
<br>
<br>
<br>
</blockquote></div><br>
--0015174c3418f653c204b30ab833--
From: Julian Bourne <julian.bourne@gmail.com>
To: gnats-bugs@netbsd.org
Cc:
Subject: Re: port-amd64/42980
Date: Mon, 5 Dec 2011 08:37:23 -0500
Repeating this post after being reminded by NetBSD Problem Report DB
Administrator gnats@netbsd.org
I raised the original issue and its fixed as far as I am concerned. There
might have been some manual pages that needed updating and so forth - so
perhaps the other people involved can answer that?
State-Changed-From-To: feedback->open
State-Changed-By: dholland@NetBSD.org
State-Changed-When: Mon, 05 Dec 2011 16:36:46 +0000
State-Changed-Why:
feedback received.
I'll give it a week to see if anyone else speaks up, then close it...
State-Changed-From-To: open->closed
State-Changed-By: dholland@NetBSD.org
State-Changed-When: Sat, 25 Aug 2012 19:07:07 +0000
State-Changed-Why:
"I'll give it a week to see if anyone else speaks up, then close it..."
...8+ months later ...
>Unformatted:
(Contact us)
$NetBSD: query-full-pr,v 1.39 2013/11/01 18:47:49 spz Exp $
$NetBSD: gnats_config.sh,v 1.8 2006/05/07 09:23:38 tsutsui Exp $
Copyright © 1994-2007
The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.
Home