Date: Fri, 17 Jan 2003 17:08:57 +0100 From: Thomas Moestl <tmm@freebsd.org> To: Harti Brandt <brandt@fokus.gmd.de> Cc: sparc@freebsd.org Subject: Re: Problem with iommu_dvmamap_create Message-ID: <20030117160857.GB304@crow.dom2ip.de> In-Reply-To: <20030117151958.U715@beagle.fokus.gmd.de> References: <20030117151958.U715@beagle.fokus.gmd.de>
next in thread | previous in thread | raw e-mail | index | archive | help
[-- Attachment #1 --]
On Fri, 2003/01/17 at 15:34:37 +0100, Harti Brandt wrote:
> Hi,
>
> it seems, there is a problem in this function. I have a case, where my
> driver creates a dma_tag with a maxsize of 64k-1, a maximum segment size
> of 64k-1 and a large maximum number of segments (say 30...40). As soon, as
> I create a DMA map with this tag, the io gets botched (up to the point,
> that the boot prom reports a nvram checksum error). When I comment out the
> code in iommu.c as follows I can at least create and destroy the maps
> without any bad effect (I did not try to load them yet):
Can you please make try the attached patch? I mostly posted it to this
list before it fixes some bogons which may be related (bugs in the
lowaddr and boundary handling) and adds some diagnostics. I also fixed
the two issues you noted below and KASSERT()ed that presz can't be 0.
Can you also please post the exact map and tag creation calls?
> int
> iommu_dvmamap_create(bus_dma_tag_t pt, bus_dma_tag_t dt, struct iommu_state *is,
> int flags, bus_dmamap_t *mapp)
> {
> bus_size_t totsz, presz, currsz;
> int error, i, maxpre;
>
> if ((error = sparc64_dmamap_create(pt->dt_parent, dt, flags, mapp)) != 0)
> return (error);
> KASSERT(SLIST_EMPTY(&(*mapp)->dm_reslist),
> ("iommu_dvmamap_create: hierarchy botched"));
> iommu_map_insq(*mapp);
> /*
> * Preallocate DVMA space; if this fails now, it is retried at load
> * time. Through bus_dmamap_load_mbuf() and bus_dmamap_load_uio(), it
> * is possible to have multiple discontiguous segments in a single map,
> * which is handled by allocating additional resources, instead of
> * increasing the size, to avoid fragmentation.
> * Clamp preallocation to BUS_SPACE_MAXSIZE. In some situations we can
> * handle more; that case is handled by reallocating at map load time.
> */
> totsz = ulmin(IOMMU_SIZE_ROUNDUP(dt->dt_maxsize), IOMMU_MAX_PRE);
> error = iommu_dvma_valloc(dt, is, *mapp, totsz);
> if (error != 0)
> return (0);
> #if 0
> /*
> * Try to be smart about preallocating some additional segments if
> * needed.
> */
> maxpre = imin(dt->dt_nsegments, IOMMU_MAX_PRE_SEG);
> presz = dt->dt_maxsize / maxpre;
> for (i = 0; i < maxpre && totsz < IOMMU_MAX_PRE; i++) {
> currsz = round_io_page(ulmin(presz, IOMMU_MAX_PRE - totsz));
> error = iommu_dvma_valloc(dt, is, *mapp, currsz);
> if (error != 0)
> break;
> totsz += currsz;
> }
> #endif
> return (0);
> }
>
> The problem seems to be the dt_maxsize / maxpre. In my case this evaluates
> to 0 and the call to valloc will use a currsz of 0. This seems to have bad
> effects on the resource manager.
Hmmm, allocating with a size of 0 would of course be a bug, but I fail to
see what would cause presz to be 0 in the first place in your example -
maxpre will be IOMMU_MAX_PRE_SEG = 3 then, so (64k-1) / 3 = 21845.
It will always be > 0 if maxsize > min(maxseg, IOMMU_PRE_SEG), and all
else would be a bogus tag.
> Also the loop will allocate one segment more than it needs (it does not
> count the first allocation). This is, however, not critical.
Doh, right.
> I suggest also changing the comment above - it mentions BUS_SPACE_MAXSIZE,
> but BUS_SPACE_MAXSIZE does not figure in the code (should be
> IOMMU_MAX_PRE probably, which happens to be BUS_SPACE_MAXSIZE).
Will do (it used to be BUS_SPACE_MAXSIZE directly, but I overlooked
the comment when changing things).
Thanks,
- Thomas
--
Thomas Moestl <tmoestl@gmx.net> http://www.tu-bs.de/~y0015675/
<tmm@FreeBSD.org> http://people.FreeBSD.org/~tmm/
PGP fingerprint: 1C97 A604 2BD0 E492 51D0 9C0F 1FE6 4F1D 419C 776C
[-- Attachment #2 --]
Index: sparc64/sparc64/iommu.c
===================================================================
RCS file: /ncvs/src/sys/sparc64/sparc64/iommu.c,v
retrieving revision 1.14
diff -u -r1.14 iommu.c
--- sparc64/sparc64/iommu.c 6 Jan 2003 21:59:54 -0000 1.14
+++ sparc64/sparc64/iommu.c 17 Jan 2003 15:52:16 -0000
@@ -517,10 +517,13 @@
{
struct resource *res;
struct bus_dmamap_res *bdr;
- bus_size_t align, bound, sgsize;
+ bus_size_t align, sgsize;
- if ((bdr = malloc(sizeof(*bdr), M_IOMMU, M_NOWAIT)) == NULL)
+ if ((bdr = malloc(sizeof(*bdr), M_IOMMU, M_NOWAIT)) == NULL) {
+ printf("iommu_dvma_valloc: res descriptor allocation "
+ "failed.\n");
return (EAGAIN);
+ }
/*
* If a boundary is specified, a map cannot be larger than it; however
* we do not clip currently, as that does not play well with the lazy
@@ -531,9 +534,9 @@
sgsize = round_io_page(size) >> IO_PAGE_SHIFT;
if (t->dt_boundary > 0 && t->dt_boundary < IO_PAGE_SIZE)
panic("iommu_dvmamap_load: illegal boundary specified");
- bound = ulmax(t->dt_boundary >> IO_PAGE_SHIFT, 1);
res = rman_reserve_resource_bound(&iommu_dvma_rman, 0L,
- t->dt_lowaddr, sgsize, bound >> IO_PAGE_SHIFT,
+ t->dt_lowaddr >> IO_PAGE_SHIFT, sgsize,
+ t->dt_boundary >> IO_PAGE_SHIFT,
RF_ACTIVE | rman_make_alignment_flags(align), NULL);
if (res == NULL)
return (ENOMEM);
@@ -719,7 +722,7 @@
* is possible to have multiple discontiguous segments in a single map,
* which is handled by allocating additional resources, instead of
* increasing the size, to avoid fragmentation.
- * Clamp preallocation to BUS_SPACE_MAXSIZE. In some situations we can
+ * Clamp preallocation to IOMMU_MAX_PRE. In some situations we can
* handle more; that case is handled by reallocating at map load time.
*/
totsz = ulmin(IOMMU_SIZE_ROUNDUP(dt->dt_maxsize), IOMMU_MAX_PRE);
@@ -732,7 +735,10 @@
*/
maxpre = imin(dt->dt_nsegments, IOMMU_MAX_PRE_SEG);
presz = dt->dt_maxsize / maxpre;
- for (i = 0; i < maxpre && totsz < IOMMU_MAX_PRE; i++) {
+ KASSERT(presz != 0, ("iommu_dvmamap_create: bogus preallocation size "
+ ", nsegments = %d, maxpre = %d, maxsize = %lu", dt->dt_nsegments,
+ maxpre, dt->dt_maxsize));
+ for (i = 1; i < maxpre && totsz < IOMMU_MAX_PRE; i++) {
currsz = round_io_page(ulmin(presz, IOMMU_MAX_PRE - totsz));
error = iommu_dvma_valloc(dt, is, *mapp, currsz);
if (error != 0)
@@ -766,8 +772,10 @@
pmap_t pmap = NULL;
KASSERT(buflen != 0, ("iommu_dvmamap_load_buffer: buflen == 0!"));
- if (buflen > dt->dt_maxsize)
+ if (buflen > dt->dt_maxsize) {
+ printf("iommu_dvmamap_load_buffer: buffer too long.\n");
return (EINVAL);
+ }
if (td != NULL)
pmap = vmspace_pmap(td->td_proc->p_vmspace);
@@ -813,6 +821,8 @@
sgcnt++;
if (sgcnt >= dt->dt_nsegments ||
sgcnt >= BUS_DMAMAP_NSEGS) {
+ printf("iommu_dvmamap_load_buffer: too many "
+ "segments.\n");
error = EFBIG;
break;
}
@@ -860,7 +870,7 @@
if (error != 0) {
iommu_dvmamap_vunload(is, map);
- (*cb)(cba, NULL, 0, error);
+ (*cb)(cba, sgs, 0, error);
} else {
/* Move the map to the end of the LRU queue. */
iommu_map_insq(map);
Index: kern/subr_rman.c
===================================================================
RCS file: /ncvs/src/sys/kern/subr_rman.c,v
retrieving revision 1.27
diff -u -r1.27 subr_rman.c
--- kern/subr_rman.c 27 Nov 2002 03:55:22 -0000 1.27
+++ kern/subr_rman.c 12 Jan 2003 22:45:20 -0000
@@ -229,7 +229,7 @@
*/
do {
rstart = (rstart + amask) & ~amask;
- if (((rstart ^ (rstart + count)) & bmask) != 0)
+ if (((rstart ^ (rstart + count - 1)) & bmask) != 0)
rstart += bound - (rstart & ~bmask);
} while ((rstart & amask) != 0 && rstart < end &&
rstart < s->r_end);
@@ -263,8 +263,11 @@
* two new allocations; the second requires but one.
*/
rv = malloc(sizeof *rv, M_RMAN, M_NOWAIT | M_ZERO);
- if (rv == 0)
+ if (rv == 0) {
+ printf("rman_reserve_resource: out of "
+ "memory.\n");
goto out;
+ }
rv->r_start = rstart;
rv->r_end = rstart + count - 1;
rv->r_flags = flags | RF_ALLOCATED;
@@ -282,6 +285,8 @@
*/
r = malloc(sizeof *r, M_RMAN, M_NOWAIT|M_ZERO);
if (r == 0) {
+ printf("rman_reserve_resource: out of "
+ "memory.\n");
free(rv, M_RMAN);
rv = 0;
goto out;
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20030117160857.GB304>