PERFORCE change 129954 for review

[Hans Petter Selasky]

http://perforce.freebsd.org/chv.cgi?CH=129954

Change 129954 by hselasky at hselasky_laptop001 on 2007/12/02 02:26:59

	
	Bugfixes:
	
	1) The "usbd_get_page()" function should only be used after
	that a virtual buffer has been loaded into DMA.
	
	2) Recursive locking in BUS-DMA callback.

Affected files ...

.. //depot/projects/usb/src/sys/dev/usb/usb_subr.c#61 edit

Differences ...

==== //depot/projects/usb/src/sys/dev/usb/usb_subr.c#61 (text+ko) ====

@@ -1776,6 +1776,9 @@
 
 /*------------------------------------------------------------------------*
  *  usbd_get_page - lookup DMA-able memory for the given offset
+ *
+ * NOTE: Only call this function when the "page_cache" structure has
+ * been properly initialized !
  *------------------------------------------------------------------------*/
 void
 usbd_get_page(struct usbd_page_cache *pc, uint32_t offset,
@@ -1787,6 +1790,8 @@
 
 	offset += pc->page_offset_buf;
 
+	/* range check */
+
 	if ((offset >= pc->page_offset_end) ||
 	    (offset < pc->page_offset_buf)) {
 
@@ -1801,22 +1806,13 @@
 
 	page = pc->page_start;
 
-	if (page) {
+	page += (offset / USB_PAGE_SIZE);
 
-		page += (offset / USB_PAGE_SIZE);
+	offset %= USB_PAGE_SIZE;
 
-		offset %= USB_PAGE_SIZE;
+	res->length = USB_PAGE_SIZE - offset;
+	res->physaddr = page->physaddr + offset;
 
-		res->length = USB_PAGE_SIZE - offset;
-		res->physaddr = page->physaddr + offset;
-
-	} else {
-
-		offset %= USB_PAGE_SIZE;
-
-		res->length = USB_PAGE_SIZE - offset;
-		res->physaddr = 0;
-	}
 	return;
 }
 
@@ -1827,25 +1823,10 @@
 usbd_copy_in(struct usbd_page_cache *cache, uint32_t offset,
     const void *ptr, uint32_t len)
 {
-	struct usbd_page_search res;
+	void *dst;
 
-	while (len) {
-
-		usbd_get_page(cache, offset, &res);
-
-		if (res.length == 0) {
-			panic("%s:%d invalid offset!\n",
-			    __FUNCTION__, __LINE__);
-		}
-		if (res.length > len) {
-			res.length = len;
-		}
-		bcopy(ptr, res.buffer, res.length);
-
-		offset += res.length;
-		len -= res.length;
-		ptr = USBD_ADD_BYTES(ptr, res.length);
-	}
+	dst = USBD_ADD_BYTES(cache->buffer, offset);
+	bcopy(ptr, dst, len);
 	return;
 }
 
@@ -1889,33 +1870,10 @@
 usbd_uiomove(struct usbd_page_cache *pc, struct uio *uio,
     uint32_t pc_offset, uint32_t len)
 {
-	struct usbd_page_search res;
-	int error = 0;
+	void *ptr;
 
-	while (len > 0) {
-
-		usbd_get_page(pc, pc_offset, &res);
-
-		if (res.length == 0) {
-			panic("%s:%d invalid offset!\n",
-			    __FUNCTION__, __LINE__);
-		}
-		if (res.length > len) {
-			res.length = len;
-		}
-		/*
-		 * "uiomove()" can sleep so one needs to make a wrapper,
-		 * exiting the mutex and checking things
-		 */
-		error = uiomove(res.buffer, len, uio);
-
-		if (error) {
-			break;
-		}
-		pc_offset += res.length;
-		len -= res.length;
-	}
-	return (error);
+	ptr = USBD_ADD_BYTES(pc->buffer, pc_offset);
+	return (uiomove(ptr, len, uio));
 }
 
 /*------------------------------------------------------------------------*
@@ -1925,25 +1883,10 @@
 usbd_copy_out(struct usbd_page_cache *cache, uint32_t offset,
     void *ptr, uint32_t len)
 {
-	struct usbd_page_search res;
+	void *src;
 
-	while (len) {
-
-		usbd_get_page(cache, offset, &res);
-
-		if (res.length == 0) {
-			panic("%s:%d invalid offset!\n",
-			    __FUNCTION__, __LINE__);
-		}
-		if (res.length > len) {
-			res.length = len;
-		}
-		bcopy(res.buffer, ptr, res.length);
-
-		offset += res.length;
-		len -= res.length;
-		ptr = USBD_ADD_BYTES(ptr, res.length);
-	}
+	src = USBD_ADD_BYTES(cache->buffer, offset);
+	bcopy(src, ptr, len);
 	return;
 }
 
@@ -1953,24 +1896,10 @@
 void
 usbd_bzero(struct usbd_page_cache *cache, uint32_t offset, uint32_t len)
 {
-	struct usbd_page_search res;
+	void *ptr;
 
-	while (len) {
-
-		usbd_get_page(cache, offset, &res);
-
-		if (res.length == 0) {
-			panic("%s:%d invalid offset!\n",
-			    __FUNCTION__, __LINE__);
-		}
-		if (res.length > len) {
-			res.length = len;
-		}
-		bzero(res.buffer, res.length);
-
-		offset += res.length;
-		len -= res.length;
-	}
+	ptr = USBD_ADD_BYTES(cache->buffer, offset);
+	bzero(ptr, len);
 	return;
 }
 
@@ -2029,21 +1958,30 @@
 	struct usbd_page_cache *pc;
 	struct usbd_page *pg;
 	uint32_t rem;
+	uint8_t owned;
 
+	pc = arg;
 	xfer = pc->xfer;
 
-	/* XXX sometimes recursive locking here */
+	/*
+	 * XXX There is sometimes recursive locking here.
+	 * XXX We should try to find a better solution.
+	 * XXX Until further the "owned" variable does
+	 * XXX the trick.
+	 */
 
 	if (error) {
 		if (xfer) {
-			mtx_lock(xfer->priv_mtx);
+			owned = mtx_owned(xfer->priv_mtx);
+			if (!owned)
+				mtx_lock(xfer->priv_mtx);
 			xfer->usb_root->dma_error = 1;
 			usbd_bdma_done_event(xfer->usb_root);
-			mtx_unlock(xfer->priv_mtx);
+			if (!owned)
+				mtx_unlock(xfer->priv_mtx);
 		}
 		return;
 	}
-	pc = arg;
 	pg = pc->page_start;
 	pg->physaddr = segs->ds_addr & ~(USB_PAGE_SIZE - 1);
 	rem = segs->ds_addr & (USB_PAGE_SIZE - 1);
@@ -2059,9 +1997,12 @@
 	}
 
 	if (xfer) {
-		mtx_lock(xfer->priv_mtx);
+		owned = mtx_owned(xfer->priv_mtx);
+		if (!owned)
+			mtx_lock(xfer->priv_mtx);
 		usbd_bdma_done_event(xfer->usb_root);
-		mtx_unlock(xfer->priv_mtx);
+		if (!owned)
+			mtx_unlock(xfer->priv_mtx);
 	}
 	return;
 }