----Next_Part(Wed_Jun_12_19:23:22_2002_229)--
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit

>>> eeh@netbsd.org wrote

> After looking at what you did to _bus_dmamap_load_mbuf() I 
> cannot see anything that would change it's behavior.  Other
> than limiting the numver of segments used to map->_dm_segcnt
> it should result in the exact same data layout as the old
> version.

Thanks for reviewing my patch.

I saw that it is not suitable to check the segments limit by
map->_dm_segcnt in _bus_dmamap_load_mbuf().

In this case, only when segments fragmentation occurs, it passes
the limitation check in dev/iommu.c. So, we need to check like the
attached patch I revised. The important part is the 2nd chunk of
dev/iommu.c.

I also revised _bus_dmamap_load() more properly.

Please review my patch again.

-- Takeshi Nakayama

----Next_Part(Wed_Jun_12_19:23:22_2002_229)--
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment; filename="netra-x1.fix2.diff"

Index: sys/arch/sparc64/dev/iommu.c
===================================================================
RCS file: /cvsroot/syssrc/sys/arch/sparc64/dev/iommu.c,v
retrieving revision 1.51
diff -u -d -u -r1.51 iommu.c
--- iommu.c	2002/05/13 21:01:15	1.51
+++ iommu.c	2002/06/11 21:36:07
@@ -537,7 +553,7 @@
 			map->dm_segs[seg].ds_len));
 		map->dm_segs[seg].ds_len =
 		    boundary - (sgstart & (boundary - 1));
-		if (++seg > map->_dm_segcnt) {
+		if (++seg >= map->_dm_segcnt) {
 			/* Too many segments.  Fail the operation. */
 			DPRINTF(IDB_INFO, ("iommu_dvmamap_load: "
 				"too many segments %d\n", seg));
@@ -789,6 +805,10 @@
 					(long)map->dm_segs[j].ds_addr, 
 					map->dm_segs[j].ds_len));
 			} else {
+				if (j >= map->_dm_segcnt) {
+					iommu_dvmamap_unload(t, is, map);
+					return (E2BIG);
+				}
 				map->dm_segs[j].ds_addr = sgstart;
 				map->dm_segs[j].ds_len = left;
 				DPRINTF(IDB_INFO, ("iommu_dvmamap_load_raw: "
@@ -803,12 +823,12 @@
 				(sgend & ~(boundary - 1))) {
 				/* Need a new segment. */
 				map->dm_segs[j].ds_len =
-					sgstart & (boundary - 1);
+					boundary - (sgstart & (boundary - 1));
 				DPRINTF(IDB_INFO, ("iommu_dvmamap_load_raw: "
 					"seg %d start %lx size %lx\n", j,
 					(long)map->dm_segs[j].ds_addr, 
 					map->dm_segs[j].ds_len));
-				if (++j > map->_dm_segcnt) {
+				if (++j >= map->_dm_segcnt) {
 					iommu_dvmamap_unload(t, is, map);
 					return (E2BIG);
 				}
@@ -874,12 +894,12 @@
 	map->dm_segs[i].ds_addr = sgstart;
 	while ((sgstart & ~(boundary - 1)) != (sgend & ~(boundary - 1))) {
 		/* Oops.  We crossed a boundary.  Split the xfer. */
-		map->dm_segs[i].ds_len = sgstart & (boundary - 1);
+		map->dm_segs[i].ds_len = boundary - (sgstart & (boundary - 1));
 		DPRINTF(IDB_INFO, ("iommu_dvmamap_load_raw: "
 			"seg %d start %lx size %lx\n", i,
 			(long)map->dm_segs[i].ds_addr,
 			map->dm_segs[i].ds_len));
-		if (++i > map->_dm_segcnt) {
+		if (++i >= map->_dm_segcnt) {
 			/* Too many segments.  Fail the operation. */
 			s = splhigh();
 			/* How can this fail?  And if it does what can we do? */
Index: sys/arch/sparc64/sparc64/machdep.c
===================================================================
RCS file: /cvsroot/syssrc/sys/arch/sparc64/sparc64/machdep.c,v
retrieving revision 1.119.6.1
diff -u -d -u -r1.119.6.1 machdep.c
--- machdep.c	2002/06/05 04:09:19	1.119.6.1
+++ machdep.c	2002/06/11 21:36:08
@@ -1112,7 +1112,8 @@
 	bus_dma_tag_t t;
 	bus_dmamap_t map;
 {
-
+	if (map->dm_nsegs)
+		bus_dmamap_unload(t, map);
 	free(map, M_DMAMAP);
 }
 
@@ -1165,24 +1166,30 @@
 	i = 0;
 	map->dm_segs[i].ds_addr = NULL;
 	map->dm_segs[i].ds_len = 0;
-	while (sgsize > 0 && i < map->_dm_segcnt) {
+	while (sgsize > 0) {
 		paddr_t pa;
+		long incr;
 
+		incr = NBPG - (vaddr & PGOFSET);
+		incr = min(sgsize, incr);
+
 		(void) pmap_extract(pmap_kernel(), vaddr, &pa);
-		sgsize -= NBPG;
-		vaddr += NBPG;
+		sgsize -= incr;
+		vaddr += incr;
 		if (map->dm_segs[i].ds_len == 0)
 			map->dm_segs[i].ds_addr = pa;
 		if (pa == (map->dm_segs[i].ds_addr + map->dm_segs[i].ds_len)
-		    && ((map->dm_segs[i].ds_len + NBPG) < map->_dm_maxsegsz)) {
+		    && ((map->dm_segs[i].ds_len + incr) <= map->_dm_maxsegsz)) {
 			/* Hey, waddyaknow, they're contiguous */
-			map->dm_segs[i].ds_len += NBPG;
+			map->dm_segs[i].ds_len += incr;
 			continue;
 		}
-		map->dm_segs[++i].ds_addr = pa;
+		if (++i >= map->_dm_segcnt)
+			return (E2BIG);
+		map->dm_segs[i].ds_addr = pa;
 		map->dm_segs[i].ds_len = NBPG;
 	}
-	map->dm_nsegs = i;
+	map->dm_nsegs = i + 1;
 	/* Mapping is bus dependent */
 	return (0);
 }
@@ -1197,7 +1204,6 @@
 	struct mbuf *m;
 	int flags;
 {
-#if 1
 	bus_dma_segment_t segs[MAX_DMA_SEGS];
 	int i;
 	size_t len;
@@ -1217,15 +1223,17 @@
 			paddr_t pa;
 			long incr;
 
-			incr = NBPG - (vaddr&PGOFSET);
+			incr = NBPG - (vaddr & PGOFSET);
 			incr = min(buflen, incr);
 
 			(void) pmap_extract(pmap_kernel(), vaddr, &pa);
 			buflen -= incr;
 			vaddr += incr;
 
-			if (i > 0 && pa == (segs[i-1].ds_addr + segs[i-1].ds_len)
-			    && ((segs[i-1].ds_len + incr) < map->_dm_maxsegsz)) {
+			if (i > 0 && 
+				pa == (segs[i-1].ds_addr + segs[i-1].ds_len) &&
+				((segs[i-1].ds_len + incr) <= 
+					map->_dm_maxsegsz)) {
 				/* Hey, waddyaknow, they're contiguous */
 				segs[i-1].ds_len += incr;
 				continue;
@@ -1285,13 +1293,8 @@
 		}
 		return (retval);
 	}
-#endif
-	return (bus_dmamap_load_raw(t, map, segs, i,
-			    (bus_size_t)len, flags));
-#else
-	panic("_bus_dmamap_load_mbuf: not implemented");
-	return 0;
 #endif
+	return (bus_dmamap_load_raw(t, map, segs, i, (bus_size_t)len, flags));
 }
 
 /*
@@ -1365,7 +1368,7 @@
 
 
 			if (i > 0 && pa == (segs[i-1].ds_addr + segs[i-1].ds_len)
-			    && ((segs[i-1].ds_len + incr) < map->_dm_maxsegsz)) {
+			    && ((segs[i-1].ds_len + incr) <= map->_dm_maxsegsz)) {
 				/* Hey, waddyaknow, they're contiguous */
 				segs[i-1].ds_len += incr;
 				continue;

----Next_Part(Wed_Jun_12_19:23:22_2002_229)----