From owner-freebsd-sparc Fri Jan 17 8: 7:22 2003 Delivered-To: freebsd-sparc@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 87A7337B401 for ; Fri, 17 Jan 2003 08:07:16 -0800 (PST) Received: from mail.gmx.net (mail.gmx.net [213.165.64.20]) by mx1.FreeBSD.org (Postfix) with SMTP id BBC9243F18 for ; Fri, 17 Jan 2003 08:07:14 -0800 (PST) (envelope-from tmoestl@gmx.net) Received: (qmail 11648 invoked by uid 0); 17 Jan 2003 16:07:12 -0000 Received: from p508e75c4.dip.t-dialin.net (HELO galatea.local) (80.142.117.196) by mail.gmx.net (mp016-rz3) with SMTP; 17 Jan 2003 16:07:12 -0000 Received: from localhost ([127.0.0.1] helo=galatea.local) by galatea.local with esmtp (Exim 4.12 #1) id 18ZZ3G-0000o3-00; Fri, 17 Jan 2003 17:09:02 +0100 Received: (from tmm@localhost) by galatea.local (8.12.6/8.12.6/Submit) id h0HG8v27003102; Fri, 17 Jan 2003 17:08:57 +0100 (CET) Date: Fri, 17 Jan 2003 17:08:57 +0100 From: Thomas Moestl To: Harti Brandt Cc: sparc@freebsd.org Subject: Re: Problem with iommu_dvmamap_create Message-ID: <20030117160857.GB304@crow.dom2ip.de> Mail-Followup-To: Harti Brandt , sparc@freebsd.org References: <20030117151958.U715@beagle.fokus.gmd.de> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="sdtB3X0nJg68CQEu" Content-Disposition: inline In-Reply-To: <20030117151958.U715@beagle.fokus.gmd.de> User-Agent: Mutt/1.4i Sender: owner-freebsd-sparc@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.org --sdtB3X0nJg68CQEu Content-Type: text/plain; charset=us-ascii Content-Disposition: inline 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 http://www.tu-bs.de/~y0015675/ http://people.FreeBSD.org/~tmm/ PGP fingerprint: 1C97 A604 2BD0 E492 51D0 9C0F 1FE6 4F1D 419C 776C --sdtB3X0nJg68CQEu Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="iommu-misc.diff" 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; --sdtB3X0nJg68CQEu-- To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-sparc" in the body of the message