svn commit: r213852 - in head: lib/libusb sys/dev/usb

Hans Petter Selasky hselasky at c2i.net
Sat Oct 16 11:29:16 UTC 2010


On Saturday 16 October 2010 12:12:43 Hans Petter Selasky wrote:
> On Saturday 16 October 2010 12:00:51 Kostik Belousov wrote:
> > > USB has some shared memory structures which are used in both user-land
> > > and  kernel, which are not part of IOCTLs. Your approach means that
> > > there are two sets of IOCTL's of all kinds, one for 32-bit and one for
> > > 64-bit?
> > 
> > For all kinds of structures that are not ABI-invariant, yes.
> 

Hi,

I've committed a patch to fix the buildworld breakage after feedback from 
Andreas Tobler.

http://svn.freebsd.org/changeset/base/213920

> The approach that was discussed by me and Andrew earlier this year, was to
> use uint64_t instead of "void *" in shared memory structures. The only
> disadvantage is that this will force you to recompile libusb when you
> update the kernel, and so we kind of put that approach aside to keep
> seamless upgrade compatibility.

This will also break the ABI on 8-stable and that was the main reason for 
going the other route. However, most applications access USB via libusb, so 
the breakage would probably be minimal. Do you have any opinions here? Should 
we make an exception for the general rule to not change the ABI within a 
stable branch?

I'm attaching the other approach, which allows both 32 and 64 bit applications 
to use USB using the same IOCTL's.

See thread: [FreeBSD 8/9] [64-bit IOCTL] Using the USB stack from a 32-bit 
application under a 64-bit kernel.

--HPS
-------------- next part --------------
--- src/lib/libusb/libusb20.c	2010-03-08 16:57:53.000000000 0000
+++ src/lib/libusb/libusb20.c	2010-03-08 16:57:53.000000000 0000
@@ -320,7 +320,7 @@
 void
 libusb20_tr_set_buffer(struct libusb20_transfer *xfer, void *buffer, uint16_t frIndex)
 {
-	xfer->ppBuffer[frIndex] = buffer;
+	xfer->ppBuffer[frIndex] = libusb20_u64ptr(buffer);
 	return;
 }
 
@@ -386,7 +386,7 @@
 void
 libusb20_tr_setup_bulk(struct libusb20_transfer *xfer, void *pBuf, uint32_t length, uint32_t timeout)
 {
-	xfer->ppBuffer[0] = pBuf;
+	xfer->ppBuffer[0] = libusb20_u64ptr(pBuf);
 	xfer->pLength[0] = length;
 	xfer->timeout = timeout;
 	xfer->nFrames = 1;
@@ -398,7 +398,7 @@
 {
 	uint16_t len;
 
-	xfer->ppBuffer[0] = psetup;
+	xfer->ppBuffer[0] = libusb20_u64ptr(psetup);
 	xfer->pLength[0] = 8;		/* fixed */
 	xfer->timeout = timeout;
 
@@ -406,7 +406,7 @@
 
 	if (len != 0) {
 		xfer->nFrames = 2;
-		xfer->ppBuffer[1] = pBuf;
+		xfer->ppBuffer[1] = libusb20_u64ptr(pBuf);
 		xfer->pLength[1] = len;
 	} else {
 		xfer->nFrames = 1;
@@ -417,7 +417,7 @@
 void
 libusb20_tr_setup_intr(struct libusb20_transfer *xfer, void *pBuf, uint32_t length, uint32_t timeout)
 {
-	xfer->ppBuffer[0] = pBuf;
+	xfer->ppBuffer[0] = libusb20_u64ptr(pBuf);
 	xfer->pLength[0] = length;
 	xfer->timeout = timeout;
 	xfer->nFrames = 1;
@@ -431,7 +431,7 @@
 		/* should not happen */
 		return;
 	}
-	xfer->ppBuffer[frIndex] = pBuf;
+	xfer->ppBuffer[frIndex] = libusb20_u64ptr(pBuf);
 	xfer->pLength[frIndex] = length;
 	return;
 }
--- src/lib/libusb/libusb20_int.h	2010-03-08 16:57:53.000000000 0000
+++ src/lib/libusb/libusb20_int.h	2010-03-08 16:57:53.000000000 0000
@@ -31,6 +31,8 @@
 #ifndef _LIBUSB20_INT_H_
 #define	_LIBUSB20_INT_H_
 
+#define	libusb20_u64ptr(ptr)	((uint64_t)(uintptr_t)(ptr))
+
 struct libusb20_device;
 struct libusb20_backend;
 struct libusb20_transfer;
@@ -148,7 +150,7 @@
 	/*
 	 * Pointer to a list of buffer pointers:
 	 */
-	void  **ppBuffer;
+	uint64_t *ppBuffer;
 	/*
 	 * Pointer to frame lengths, which are updated to actual length
 	 * after the USB transfer completes:
--- src/lib/libusb/libusb20_ugen20.c	2010-03-08 16:57:53.000000000 0000
+++ src/lib/libusb/libusb20_ugen20.c	2010-03-08 16:57:53.000000000 0000
@@ -763,8 +763,8 @@
 	xfer->maxPacketLen = temp.max_packet_length;
 
 	/* setup buffer and length lists */
-	fsep->ppBuffer = xfer->ppBuffer;/* zero copy */
-	fsep->pLength = xfer->pLength;	/* zero copy */
+	fsep->ppBuffer = libusb20_u64ptr(xfer->ppBuffer);	/* zero copy */
+	fsep->pLength = libusb20_u64ptr(xfer->pLength);		/* zero copy */
 
 	return (0);			/* success */
 }
--- src/sys/dev/usb/usb_generic.c	2010-05-17 04:19:10.000000000 0000
+++ src/sys/dev/usb/usb_generic.c	2010-05-17 04:19:10.000000000 0000
@@ -76,6 +76,7 @@
 #define	UGEN_BULK_FS_BUFFER_SIZE	(64*32)	/* bytes */
 #define	UGEN_BULK_HS_BUFFER_SIZE	(1024*32)	/* bytes */
 #define	UGEN_HW_FRAMES	50		/* number of milliseconds per transfer */
+#define	UGEN_HW_PTR(u64)		((void *)(uintptr_t)(u64))
 
 /* function prototypes */
 
@@ -134,7 +135,6 @@
 TUNABLE_INT("hw.usb.ugen.debug", &ugen_debug);
 #endif
 
-
 /* prototypes */
 
 static int
@@ -1039,7 +1039,7 @@
 	struct usb_device_request *req;
 	struct usb_xfer *xfer;
 	struct usb_fs_endpoint fs_ep;
-	void *uaddr;			/* userland pointer */
+	uint64_t uaddr;			/* userland pointer */
 	void *kaddr;
 	usb_frlength_t offset;
 	usb_frlength_t rem;
@@ -1077,11 +1077,13 @@
 		xfer->error = USB_ERR_INVAL;
 		goto complete;
 	}
-	error = copyin(fs_ep.ppBuffer,
+
+	/* read frame buffer pointer */
+	error = copyin(UGEN_HW_PTR(fs_ep.ppBuffer),
 	    &uaddr, sizeof(uaddr));
-	if (error) {
+	if (error)
 		return (error);
-	}
+
 	/* reset first frame */
 	usbd_xfer_set_frame_offset(xfer, 0, 0);
 
@@ -1089,7 +1091,8 @@
 
 		req = xfer->frbuffers[0].buffer;
 
-		error = copyin(fs_ep.pLength,
+		/* read frame buffer length */
+		error = copyin(UGEN_HW_PTR(fs_ep.pLength),
 		    &length, sizeof(length));
 		if (error) {
 			return (error);
@@ -1099,7 +1102,7 @@
 			goto complete;
 		}
 		if (length != 0) {
-			error = copyin(uaddr, req, length);
+			error = copyin(UGEN_HW_PTR(uaddr), req, length);
 			if (error) {
 				return (error);
 			}
@@ -1159,11 +1162,12 @@
 
 	for (; n != xfer->nframes; n++) {
 
-		error = copyin(fs_ep.pLength + n,
+		/* read frame buffer length */
+		error = copyin(UGEN_HW_PTR(fs_ep.pLength + (4 * n)),
 		    &length, sizeof(length));
-		if (error) {
+		if (error)
 			break;
-		}
+
 		usbd_xfer_set_frame_len(xfer, n, length);
 
 		if (length > rem) {
@@ -1174,12 +1178,12 @@
 
 		if (!isread) {
 
-			/* we need to know the source buffer */
-			error = copyin(fs_ep.ppBuffer + n,
+			/* read frame buffer pointer */
+			error = copyin(UGEN_HW_PTR(fs_ep.ppBuffer + (8 * n)),
 			    &uaddr, sizeof(uaddr));
-			if (error) {
+			if (error)
 				break;
-			}
+
 			if (xfer->flags_int.isochronous_xfr) {
 				/* get kernel buffer address */
 				kaddr = xfer->frbuffers[0].buffer;
@@ -1193,7 +1197,7 @@
 			}
 
 			/* move data */
-			error = copyin(uaddr, kaddr, length);
+			error = copyin(UGEN_HW_PTR(uaddr), kaddr, length);
 			if (error) {
 				break;
 			}
@@ -1216,7 +1220,7 @@
 	struct usb_xfer *xfer;
 	struct usb_fs_endpoint fs_ep;
 	struct usb_fs_endpoint *fs_ep_uptr;	/* userland ptr */
-	void *uaddr;			/* userland ptr */
+	uint64_t uaddr;			/* userland ptr */
 	void *kaddr;
 	usb_frlength_t offset;
 	usb_frlength_t rem;
@@ -1281,12 +1285,12 @@
 
 	for (; n != xfer->nframes; n++) {
 
-		/* get initial length into "temp" */
-		error = copyin(fs_ep.pLength + n,
+		/* get initial frame buffer length into "temp" */
+		error = copyin(UGEN_HW_PTR(fs_ep.pLength + (4 * n)),
 		    &temp, sizeof(temp));
-		if (error) {
+		if (error)
 			return (error);
-		}
+
 		if (temp > rem) {
 			/* the userland length has been corrupted */
 			DPRINTF("corrupt userland length "
@@ -1307,12 +1311,12 @@
 		}
 		if (isread) {
 
-			/* we need to know the destination buffer */
-			error = copyin(fs_ep.ppBuffer + n,
+			/* read destination frame buffer pointer */
+			error = copyin(UGEN_HW_PTR(fs_ep.ppBuffer + (8 * n)),
 			    &uaddr, sizeof(uaddr));
-			if (error) {
+			if (error)
 				return (error);
-			}
+
 			if (xfer->flags_int.isochronous_xfr) {
 				/* only one frame buffer */
 				kaddr = USB_ADD_BYTES(
@@ -1323,7 +1327,7 @@
 			}
 
 			/* move data */
-			error = copyout(kaddr, uaddr, length);
+			error = copyout(kaddr, UGEN_HW_PTR(uaddr), length);
 			if (error) {
 				return (error);
 			}
@@ -1334,12 +1338,11 @@
 		 */
 		offset += temp;
 
-		/* update length */
+		/* update frame buffer length */
 		error = copyout(&length,
-		    fs_ep.pLength + n, sizeof(length));
-		if (error) {
+		    UGEN_HW_PTR(fs_ep.pLength + (4 * n)), sizeof(length));
+		if (error)
 			return (error);
-		}
 	}
 
 complete:
--- src/sys/dev/usb/usb_ioctl.h	2010-02-14 12:03:51.000000000 0000
+++ src/sys/dev/usb/usb_ioctl.h	2010-02-14 12:03:51.000000000 0000
@@ -131,9 +131,10 @@
 	 * NOTE: isochronous USB transfer only use one buffer, but can have
 	 * multiple frame lengths !
 	 */
-	void  **ppBuffer;		/* pointer to userland buffers */
-	uint32_t *pLength;		/* pointer to frame lengths, updated
-					 * to actual length */
+	uint64_t ppBuffer;		/* pointer to 64-bit userland buffer pointers */
+	uint64_t pLength;		/* pointer to 32-bit frame lengths, which
+					 * get updated to the actual length after
+					 * the transfer is complete. */
 	uint32_t nFrames;		/* number of frames */
 	uint32_t aFrames;		/* actual number of frames */
 	uint16_t flags;


More information about the svn-src-all mailing list