Date: Sun, 30 May 2010 21:50:15 +0200 From: Hans Petter Selasky <hselasky@c2i.net> To: freebsd-hackers@freebsd.org Subject: [FreeBSD 8/9] [64-bit IOCTL] Using the USB stack from a 32-bit application under a 64-bit kernel. Message-ID: <201005302150.15232.hselasky@c2i.net>
next in thread | raw e-mail | index | archive | help
--Boundary-00=_3FsAMIETJnfkjt6
Content-Type: text/plain;
charset="iso-8859-1"
Content-Transfer-Encoding: 7bit
Hi,
The USB team in FreeBSD has discussed this issue internally and would like
some advice how to best resolve the 32 bit to 64 bit IOCTL conversion issue.
Sometimes IOCTL requests contain userland pointers of type "void *". When
compiled on a 64-bit OS, sizeof(void *) is 8 bytes and when compiled on a 32-
bit OS sizeof(void *) is 4 bytes. This size difference makes it impossible to
forward IOCTLs 1:1 from a 32-bit application running under a 64-bit kernel.
This issue does not only apply to the USB subsystem, but also other kernel
IOCTL interfaces. I suggest a more general solution:
typedef uint64_t pvoid64;
When an IOCTL needs to reference pointers in userspace, then they should
always pad the pointer to 64-bit and use the pvoid64, which could have been a
compiler type, so that we can easily convert to "void *" without using casts.
When converting 32-bit userland pointers into 64-bit ones, there is no pointer
conversion except for the unsigned expansion and resulting zero-padding. I
assume this assumption will always be valid.
Please find attached an example patch to make the USB stack in 8 and 9 fully
64-bit compatible.
Any comments are welcome!
--HPS
--Boundary-00=_3FsAMIETJnfkjt6
Content-Type: text/plain; charset="iso-8859-1"; name="usb_64bit_patch.txt"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="usb_64bit_patch.txt"
--- 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;
--Boundary-00=_3FsAMIETJnfkjt6--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201005302150.15232.hselasky>