From: dlg <dlg@openbsd.org>
Date: Sun, 25 Jan 2015 11:36:41 +0000 (+0000)
Subject: refactor loading of dmamaps.
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=880ce07f8829ad9f3295ccc4fb91f82df8df89b6;p=openbsd

refactor loading of dmamaps.

bus_dmama_load and bus_dmamap_load mbuf figure out the physical
addresses of the memory theyre given and then hand it to
_bus_dmamap_load_paddr to store in the dmamaps sg lists.

unfortunately bus_dmamap_load_mbuf assumes it is only given memory
from the kernels direct mapped region, and blindly translates
anything its given into phys addresses to hand to _load_paddr.

i recently committed change to pool asking them to allocate large
pages, which meant uvm allocated mbufs outside the direct map, which
meant bus_dmamap_load_mbuf was handing out bogus physical addresses.
the pool change got backed out until i could debug this.

now _load and _load_mbuf now call _bus_dmamap_load_vaddr for every
buffer theyve been given, which properly determines if the addresses
are in the direct map or via the tlb. _load_vaddr then feeds the
physical addresses into _bus_dmamap_load_paddr to store them in the
dmamap.

tldr; _load_mbuf doesnt make naive assumptions about its addresses
now.

ok miod@ kettenis@
---

diff --git a/sys/arch/landisk/landisk/bus_dma.c b/sys/arch/landisk/landisk/bus_dma.c
index 9422561dffc..2385133c0f2 100644
--- a/sys/arch/landisk/landisk/bus_dma.c
+++ b/sys/arch/landisk/landisk/bus_dma.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: bus_dma.c,v 1.12 2014/11/16 12:30:57 deraadt Exp $	*/
+/*	$OpenBSD: bus_dma.c,v 1.13 2015/01/25 11:36:41 dlg Exp $	*/
 /*	$NetBSD: bus_dma.c,v 1.1 2006/09/01 21:26:18 uwe Exp $	*/
 
 /*
@@ -67,6 +67,16 @@ struct _bus_dma_tag landisk_bus_dma = {
 	._dmamem_mmap = _bus_dmamem_mmap,
 };
 
+#define DMAMAP_RESET(_m) do { \
+	(_m)->dm_mapsize = 0; \
+	(_m)->dm_nsegs = 0; \
+} while (0)
+
+int	_bus_dmamap_load_vaddr(bus_dma_tag_t, bus_dmamap_t,
+	    void *, bus_size_t, pmap_t);
+int	_bus_dmamap_load_paddr(bus_dma_tag_t, bus_dmamap_t,
+	    paddr_t, vaddr_t, bus_size_t);
+
 /*
  * Create a DMA map.
  */
@@ -109,8 +119,7 @@ _bus_dmamap_create(bus_dma_tag_t t, bus_size_t size, int nsegments,
 	map->_dm_boundary = boundary;
 	map->_dm_flags = flags & ~(BUS_DMA_WAITOK|BUS_DMA_NOWAIT);
 
-	map->dm_mapsize = 0;		/* no valid mappings */
-	map->dm_nsegs = 0;
+	DMAMAP_RESET(map); /* no valid mappings */
 
 	*dmamp = map;
 
@@ -129,29 +138,23 @@ _bus_dmamap_destroy(bus_dma_tag_t t, bus_dmamap_t map)
 	free(map, M_DEVBUF, 0);
 }
 
-static inline int
+int
 _bus_dmamap_load_paddr(bus_dma_tag_t t, bus_dmamap_t map,
-    paddr_t paddr, vaddr_t vaddr, int size, int *segp, paddr_t *lastaddrp,
-    int first)
+    paddr_t paddr, vaddr_t vaddr, bus_size_t size)
 {
 	bus_dma_segment_t * const segs = map->dm_segs;
 	bus_addr_t bmask = ~(map->_dm_boundary - 1);
-	bus_addr_t lastaddr;
-	int nseg;
-	int sgsize;
 
-	nseg = *segp;
-	lastaddr = *lastaddrp;
+	int first = map->dm_mapsize == 0;
+	int nseg = map->dm_nsegs;
+	paddr_t lastaddr = SH3_P2SEG_TO_PHYS(segs[nseg].ds_addr);
 
-	DPRINTF(("_bus_dmamap_load_paddr: t = %p, map = %p, paddr = 0x%08lx, vaddr = 0x%08lx, size = %d\n", t, map, paddr, vaddr, size));
-        DPRINTF(("_bus_dmamap_load_paddr: nseg = %d, bmask = 0x%08lx, lastaddr = 0x%08lx\n", nseg, bmask, lastaddr));
+	map->dm_mapsize += size;
 
 	do {
-		sgsize = size;
+		bus_size_t sgsize = size;
 
-		/*
-		 * Make sure we don't cross any boundaries.
-		 */
+		/* Make sure we don't cross any boundaries. */
 		if (map->_dm_boundary > 0) {
 			bus_addr_t baddr; /* next boundary address */
 
@@ -160,113 +163,75 @@ _bus_dmamap_load_paddr(bus_dma_tag_t t, bus_dmamap_t map,
 				sgsize = (baddr - paddr);
 		}
 
-		DPRINTF(("_bus_dmamap_load_paddr: sgsize = %d\n", sgsize));
-
 		/*
 		 * Insert chunk into a segment, coalescing with
 		 * previous segment if possible.
 		 */
 		if (first) {
 			/* first segment */
-			DPRINTF(("_bus_dmamap_load_paddr: first\n"));
 			segs[nseg].ds_addr = SH3_PHYS_TO_P2SEG(paddr);
 			segs[nseg].ds_len = sgsize;
 			segs[nseg]._ds_vaddr = vaddr;
 			first = 0;
+		} else if ((paddr == lastaddr)
+		 && (segs[nseg].ds_len + sgsize <= map->_dm_maxsegsz)
+		 && (map->_dm_boundary == 0 ||
+		     (segs[nseg].ds_addr & bmask) == (paddr & bmask))) {
+			/* coalesce */
+			segs[nseg].ds_len += sgsize;
 		} else {
-			if ((paddr == lastaddr)
-			 && (segs[nseg].ds_len + sgsize <= map->_dm_maxsegsz)
-			 && (map->_dm_boundary == 0 ||
-			     (segs[nseg].ds_addr & bmask) == (paddr & bmask))) {
-				/* coalesce */
-				DPRINTF(("_bus_dmamap_load_paddr: coalesce\n"));
-				segs[nseg].ds_len += sgsize;
-			} else {
-				if (++nseg >= map->_dm_segcnt) {
-					break;
-				}
-				/* new segment */
-				DPRINTF(("_bus_dmamap_load_paddr: new\n"));
-				segs[nseg].ds_addr = SH3_PHYS_TO_P2SEG(paddr);
-				segs[nseg].ds_len = sgsize;
-				segs[nseg]._ds_vaddr = vaddr;
-			}
+			if (++nseg >= map->_dm_segcnt)
+				return (EFBIG);
+
+			/* new segment */
+			segs[nseg].ds_addr = SH3_PHYS_TO_P2SEG(paddr);
+			segs[nseg].ds_len = sgsize;
+			segs[nseg]._ds_vaddr = vaddr;
 		}
 
 		lastaddr = paddr + sgsize;
 		paddr += sgsize;
 		vaddr += sgsize;
 		size -= sgsize;
-		DPRINTF(("_bus_dmamap_load_paddr: lastaddr = 0x%08lx, paddr = 0x%08lx, vaddr = 0x%08lx, size = %d\n", lastaddr, paddr, vaddr, size));
 	} while (size > 0);
 
-	DPRINTF(("_bus_dmamap_load_paddr: nseg = %d\n", nseg));
-
-	*segp = nseg;
-	*lastaddrp = lastaddr;
-
-	/*
-	 * Did we fit?
-	 */
-	if (size != 0) {
-		/*
-		 * If there is a chained window, we will automatically
-		 * fall back to it.
-		 */
-		return (EFBIG);		/* XXX better return value here? */
-	}
+	map->dm_nsegs = nseg;
 
 	return (0);
 }
 
-static inline int
-_bus_bus_dmamap_load_buffer(bus_dma_tag_t t, bus_dmamap_t map, void *buf,
-    bus_size_t buflen, struct proc *p, int flags, int *segp)
+int
+_bus_dmamap_load_vaddr(bus_dma_tag_t t, bus_dmamap_t map,
+    void *buf, bus_size_t size, pmap_t pmap)
 {
-	bus_size_t sgsize;
-	bus_addr_t curaddr;
-	bus_size_t len;
-	paddr_t lastaddr;
-	vaddr_t vaddr = (vaddr_t)buf;
-	pmap_t pmap;
-	int first;
+	vaddr_t vaddr;
+	paddr_t paddr;
+	vaddr_t next, end;
 	int error;
 
-	DPRINTF(("_bus_dmamap_load_buffer: t = %p, map = %p, buf = %p, buflen = %ld, p = %p, flags = %x\n", t, map, buf, buflen, p, flags));
-
-	if (p != NULL)
-		pmap = p->p_vmspace->vm_map.pmap;
-	else
-		pmap = pmap_kernel();
-
-	*segp = 0;
-	first = 1;
-	len = buflen;
-	lastaddr = 0;		/* XXX: uwe: gag gcc4, but this is WRONG!!! */
-	while (len > 0) {
-		/*
-		 * Get the physical address for this segment.
-		 */
-		(void)pmap_extract(pmap, vaddr, &curaddr);
-
-		/*
-		 * Compute the segment size, and adjust counts.
-		 */
-		sgsize = PAGE_SIZE - ((u_long)vaddr & PGOFSET);
-		if (len < sgsize)
-			sgsize = len;
+	vaddr = (vaddr_t)buf;
+	end = vaddr + size;
+
+	if (pmap == pmap_kernel() &&
+	    vaddr >= SH3_P1SEG_BASE && end <= SH3_P2SEG_END)
+		paddr = SH3_P1SEG_TO_PHYS(vaddr);
+	else {
+		for (next = (vaddr + PAGE_SIZE) & ~PAGE_MASK;
+		    next < end; next += PAGE_SIZE) {
+			pmap_extract(pmap, vaddr, &paddr);
+			error = _bus_dmamap_load_paddr(t, map,
+			    paddr, vaddr, next - vaddr);
+			if (error != 0)
+				return (error);
 
-		error = _bus_dmamap_load_paddr(t, map, curaddr, vaddr, sgsize,
-		    segp, &lastaddr, first);
-		if (error)
-			return (error);
+			vaddr = next;
+		}
 
-		vaddr += sgsize;
-		len -= sgsize;
-		first = 0;
+		pmap_extract(pmap, vaddr, &paddr);
+		size = end - vaddr;
 	}
 
-	return (0);
+	return (_bus_dmamap_load_paddr(t, map, paddr, vaddr, size));
 }
 
 /*
@@ -276,65 +241,25 @@ int
 _bus_dmamap_load(bus_dma_tag_t t, bus_dmamap_t map, void *buf,
     bus_size_t buflen, struct proc *p, int flags)
 {
-	bus_addr_t addr = (bus_addr_t)buf;
-	paddr_t lastaddr;
-	int seg;
-	int first;
 	int error;
 
 	DPRINTF(("bus_dmamap_load: t = %p, map = %p, buf = %p, buflen = %ld, p = %p, flags = %x\n", t, map, buf, buflen, p, flags));
 
-	/*
-	 * Make sure on error condition we return "no valid mappings."
-	 */
-	map->dm_mapsize = 0;
-	map->dm_nsegs = 0;
+	DMAMAP_RESET(map);
 
 	if (buflen > map->_dm_size)
 		return (EINVAL);
 
-	lastaddr = 0;		/* XXX: uwe: gag gcc4, but this is WRONG!!! */
-
-	if ((addr >= SH3_P1SEG_BASE) && (addr + buflen <= SH3_P2SEG_END)) {
-		bus_addr_t curaddr;
-		bus_size_t sgsize;
-		bus_size_t len = buflen;
-
-		DPRINTF(("bus_dmamap_load: P[12]SEG (0x%08lx)\n", addr));
-
-		seg = 0;
-		first = 1;
-		while (len > 0) {
-			curaddr = SH3_P1SEG_TO_PHYS(addr);
-
-			sgsize = PAGE_SIZE - ((u_long)addr & PGOFSET);
-			if (len < sgsize)
-				sgsize = len;
-
-			error = _bus_dmamap_load_paddr(t, map, curaddr, addr,
-			    sgsize, &seg, &lastaddr, first);
-			if (error)
-				return (error);
-
-			addr += sgsize;
-			len -= sgsize;
-			first = 0;
-		}
-
-		map->dm_nsegs = seg + 1;
-		map->dm_mapsize = buflen;
-		return (0);
+	error = _bus_dmamap_load_vaddr(t, map, buf, buflen,
+	    p == NULL ? pmap_kernel() : p->p_vmspace->vm_map.pmap);
+	if (error != 0) {
+		DMAMAP_RESET(map); /* no valid mappings */
+		return (error);
 	}
 
-	error = _bus_bus_dmamap_load_buffer(t, map, buf, buflen, p, flags,
-	 &seg);
-	if (error == 0) {
-		map->dm_nsegs = seg + 1;
-		map->dm_mapsize = buflen;
-		return (0);
-	}
+	map->dm_nsegs++;
 
-	return (error);
+	return (0);
 }
 
 /*
@@ -345,18 +270,9 @@ _bus_dmamap_load_mbuf(bus_dma_tag_t t, bus_dmamap_t map, struct mbuf *m0,
     int flags)
 {
 	struct mbuf *m;
-	paddr_t lastaddr;
-	int seg;
-	int first;
 	int error;
 
-	DPRINTF(("bus_dmamap_load_mbuf: t = %p, map = %p, m0 = %p, flags = %x\n", t, map, m0, flags));
-
-	/*
-	 * Make sure on error condition we return "no valid mappings."
-	 */
-	map->dm_nsegs = 0;
-	map->dm_mapsize = 0;
+	DMAMAP_RESET(map);
 
 #ifdef DIAGNOSTIC
 	if ((m0->m_flags & M_PKTHDR) == 0)
@@ -366,33 +282,21 @@ _bus_dmamap_load_mbuf(bus_dma_tag_t t, bus_dmamap_t map, struct mbuf *m0,
 	if (m0->m_pkthdr.len > map->_dm_size)
 		return (EINVAL);
 
-	seg = 0;
-	first = 1;
-	error = 0;
-	lastaddr = 0;		/* XXX: uwe: gag gcc4, but this is WRONG!!! */
-	for (m = m0; m != NULL && error == 0; m = m->m_next) {
-		paddr_t paddr;
-		vaddr_t vaddr;
-		int size;
-
+	for (m = m0; m != NULL; m = m->m_next) {
 		if (m->m_len == 0)
 			continue;
 
-		vaddr = (vaddr_t)(m->m_data);
-		paddr = (paddr_t)(SH3_P1SEG_TO_PHYS(vaddr));
-		size = m->m_len;
-		error = _bus_dmamap_load_paddr(t, map, paddr, vaddr, size,
-		    &seg, &lastaddr, first);
-		first = 0;
+		error = _bus_dmamap_load_vaddr(t, map, m->m_data, m->m_len,
+		    pmap_kernel());
+		if (error != 0) {
+			DMAMAP_RESET(map);
+			return (error);
+		}
 	}
 
-	if (error == 0) {
-		map->dm_nsegs = seg + 1;
-		map->dm_mapsize = m0->m_pkthdr.len;
-		return (0);
-	}
+	map->dm_nsegs++;
 
-	return (error);
+	return (0);
 }
 
 /*