Re: COMPAT_32BIT libusb ABI problem

2013-08-12 Thread Hans Petter Selasky

On 07/24/13 22:09, Damjan Jovanovic wrote:

Hi

The ioctl:
#define USB_FS_INIT _IOW ('U', 195, struct usb_fs_init)
when used by a COMPAT_32BIT libusb on amd64, fails (causing
libusb_open() to fail) due to:

freebsd32_ioctl(0x6,0x800c55c3,0xc710,0x0,0x0,0x0) ERR#25
'Inappropriate ioctl for device'
but when hacked a bit:
freebsd32_ioctl(0x6,0x801055c3,0xc710,0x0,0x0,0x0) = 0 (0x0)

because sizeof(struct usb_fs_init) is 12 bytes on i386, and 16 bytes on amd64.

But the failure of even libusb_open() - a fundamental libusb function
- must mean that the COMPAT_32BIT libusb never worked. Can we please
take this opportunity to kill it and do a real 32 bit compatibility
layer in the kernel that will work from 32 bit chroots and statically
linked 32 bit binaries? I'll help.


Hi,

Some further analysis shows that this is due to structures being aligned 
to their biggest element on AMD64 while on I386 not. This should now be 
fixed. Please see the following commit:


http://svnweb.freebsd.org/changeset/base/254243

You will need to use the LibUSB-32 built by the 64-bit system build, and 
not the I386 one, which has COMPAT_32BIT set.


Thank you!

--HPS

___
freebsd-usb@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-usb
To unsubscribe, send any mail to "freebsd-usb-unsubscr...@freebsd.org"


Re: COMPAT_32BIT libusb ABI problem

2013-07-26 Thread Hans Petter Selasky

Hi Damjan,

Does the attached patch fix the problem for you?

--HPS
=== sys/dev/usb/quirk/usb_quirk.c
==
--- sys/dev/usb/quirk/usb_quirk.c	(revision 253548)
+++ sys/dev/usb/quirk/usb_quirk.c	(local)
@@ -692,8 +692,8 @@
 	uint32_t y;
 	int err;
 
-	switch (cmd) {
-	case USB_DEV_QUIRK_GET:
+	switch (USB_IOCTL_SWAP(cmd)) {
+	case USB_IOCTL_RANGE(USB_DEV_QUIRK_GET):
 		pgq = (void *)data;
 		x = pgq->index % USB_SUB_QUIRKS_MAX;
 		y = pgq->index / USB_SUB_QUIRKS_MAX;
@@ -712,7 +712,7 @@
 		mtx_unlock(&usb_quirk_mtx);
 		return (0);		/* success */
 
-	case USB_QUIRK_NAME_GET:
+	case USB_IOCTL_RANGE(USB_QUIRK_NAME_GET):
 		pgq = (void *)data;
 		x = pgq->index;
 		if (x >= USB_QUIRK_MAX) {
@@ -722,7 +722,7 @@
 		usb_quirkstr(x), sizeof(pgq->quirkname));
 		return (0);		/* success */
 
-	case USB_DEV_QUIRK_ADD:
+	case USB_IOCTL_RANGE(USB_DEV_QUIRK_ADD):
 		pgq = (void *)data;
 
 		/* check privileges */
@@ -761,7 +761,7 @@
 		}
 		return (0);		/* success */
 
-	case USB_DEV_QUIRK_REMOVE:
+	case USB_IOCTL_RANGE(USB_DEV_QUIRK_REMOVE):
 		pgq = (void *)data;
 		/* check privileges */
 		err = priv_check(curthread, PRIV_DRIVER);
=== sys/dev/usb/usb_compat_linux.c
==
--- sys/dev/usb/usb_compat_linux.c	(revision 253548)
+++ sys/dev/usb/usb_compat_linux.c	(local)
@@ -48,7 +48,6 @@
 #include 
 
 #include 
-#include 
 #include 
 #include 
 
=== sys/dev/usb/usb_dev.c
==
--- sys/dev/usb/usb_dev.c	(revision 253548)
+++ sys/dev/usb/usb_dev.c	(local)
@@ -1497,30 +1497,30 @@
 	int err;
 
 	u.data = data;
-	switch (cmd) {
-		case USB_READ_DIR:
-			err = usb_read_symlink(u.urd->urd_data,
-			u.urd->urd_startentry, u.urd->urd_maxlen);
+	switch (USB_IOCTL_SWAP(cmd)) {
+	case USB_IOCTL_RANGE(USB_READ_DIR):
+		err = usb_read_symlink(u.urd->urd_data,
+		u.urd->urd_startentry, u.urd->urd_maxlen);
+		break;
+	case USB_IOCTL_RANGE(USB_DEV_QUIRK_GET):
+	case USB_IOCTL_RANGE(USB_QUIRK_NAME_GET):
+	case USB_IOCTL_RANGE(USB_DEV_QUIRK_ADD):
+	case USB_IOCTL_RANGE(USB_DEV_QUIRK_REMOVE):
+		err = usb_quirk_ioctl_p(cmd, data, fflag, td);
+		break;
+	case USB_IOCTL_RANGE(USB_GET_TEMPLATE):
+		*(int *)data = usb_template;
+		err = 0;
+		break;
+	case USB_IOCTL_RANGE(USB_SET_TEMPLATE):
+		err = priv_check(curthread, PRIV_DRIVER);
+		if (err)
 			break;
-		case USB_DEV_QUIRK_GET:
-		case USB_QUIRK_NAME_GET:
-		case USB_DEV_QUIRK_ADD:
-		case USB_DEV_QUIRK_REMOVE:
-			err = usb_quirk_ioctl_p(cmd, data, fflag, td);
-			break;
-		case USB_GET_TEMPLATE:
-			*(int *)data = usb_template;
-			err = 0;
-			break;
-		case USB_SET_TEMPLATE:
-			err = priv_check(curthread, PRIV_DRIVER);
-			if (err)
-break;
-			usb_template = *(int *)data;
-			break;
-		default:
-			err = ENOTTY;
-			break;
+		usb_template = *(int *)data;
+		break;
+	default:
+		err = ENOTTY;
+		break;
 	}
 	return (err);
 }
=== sys/dev/usb/usb_generic.c
==
--- sys/dev/usb/usb_generic.c	(revision 253548)
+++ sys/dev/usb/usb_generic.c	(local)
@@ -1424,8 +1424,8 @@
 
 	DPRINTFN(6, "cmd=0x%08lx\n", cmd);
 
-	switch (cmd) {
-	case USB_FS_COMPLETE:
+	switch (USB_IOCTL_SWAP(cmd)) {
+	case USB_IOCTL_RANGE(USB_FS_COMPLETE):
 		mtx_lock(f->priv_mtx);
 		error = ugen_fs_get_complete(f, &ep_index);
 		mtx_unlock(f->priv_mtx);
@@ -1438,7 +1438,7 @@
 		error = ugen_fs_copy_out(f, u.pcomp->ep_index);
 		break;
 
-	case USB_FS_START:
+	case USB_IOCTL_RANGE(USB_FS_START):
 		error = ugen_fs_copy_in(f, u.pstart->ep_index);
 		if (error)
 			break;
@@ -1448,7 +1448,7 @@
 		mtx_unlock(f->priv_mtx);
 		break;
 
-	case USB_FS_STOP:
+	case USB_IOCTL_RANGE(USB_FS_STOP):
 		if (u.pstop->ep_index >= f->fs_ep_max) {
 			error = EINVAL;
 			break;
@@ -1470,8 +1470,8 @@
 		mtx_unlock(f->priv_mtx);
 		break;
 
-	case USB_FS_OPEN:
-	case USB_FS_OPEN_STREAM:
+	case USB_IOCTL_RANGE(USB_FS_OPEN):
+	case USB_IOCTL_RANGE(USB_FS_OPEN_STREAM):
 		if (u.popen->ep_index >= f->fs_ep_max) {
 			error = EINVAL;
 			break;
@@ -1523,7 +1523,7 @@
 		usb_config[0].frames = u.popen->max_frames;
 		usb_config[0].bufsize = u.popen->max_bufsize;
 		usb_config[0].usb_mode = USB_MODE_DUAL;	/* both modes */
-		if (cmd == USB_FS_OPEN_STREAM)
+		if (IOCBASECMD(cmd) == IOCBASECMD(USB_FS_OPEN_STREAM))
 			usb_config[0].stream_id = u.popen_stream->stream_id;
 
 		if (usb_config[0].type == UE_CONTROL) {
@@ -1572,7 +1572,7 @@
 		}
 		break;
 
-	case USB_FS_CLOSE:
+	case USB_IOCTL_RANGE(USB_FS_CLOSE):
 		if (u.pclose->ep_index >= f->fs_ep_max) {
 			error = EINVAL;
 			break;
@@ -1584,7 +1584,7 @@
 		usbd_transfer_unsetup(f->fs_xfer + u.pclose->ep_index, 1);
 		break;
 
-	case USB_FS_CLEAR_STALL_SYNC:
+	case USB_IOCTL_RANGE(USB_FS_CLEAR_STALL_SYNC):
 		if (u.pstall->ep_index >= f->fs_ep_max) {
 			error = EINVAL;
 			break;
@@ -1941,8 +1941,8 @@
 	f_rx = f->udev->fifo[(f->fifo_index & 

RE: COMPAT_32BIT libusb ABI problem

2013-07-25 Thread Hans Petter Selasky
Hi,

LibUSB 32-bit is compiled using -m32 in a 64-bit environment. What happens when 
you pass -m32 to the compiler?

Thank you for your report.

Another alternative is to use __packed attribute, but that has some 
implications too.

--HPS
 
-Original message-
> From:Damjan Jovanovic mailto:damjan@gmail.com> >
> Sent: Thursday 25th July 2013 20:55
> To: Hans Petter Selasky  <mailto:hans.petter.sela...@bitfrost.no> >
> Cc: freebsd-usb@freebsd.org <mailto:freebsd-usb@freebsd.org> 
> Subject: Re: COMPAT_32BIT libusb ABI problem
> 
> On Thu, Jul 25, 2013 at 6:11 AM, Hans Petter Selasky  <mailto:h...@bitfrost.no> > wrote:
> > On 07/24/13 22:09, Damjan Jovanovic wrote:
> >>
> >> Hi
> >>
> >> The ioctl:
> >> #define USB_FS_INIT _IOW ('U', 195, struct usb_fs_init)
> >> when used by a COMPAT_32BIT libusb on amd64, fails (causing
> >> libusb_open() to fail) due to:
> >>
> >> freebsd32_ioctl(0x6,0x800c55c3,0xc710,0x0,0x0,0x0) ERR#25
> >> 'Inappropriate ioctl for device'
> >> but when hacked a bit:
> >> freebsd32_ioctl(0x6,0x801055c3,0xc710,0x0,0x0,0x0) = 0 (0x0)
> >>
> >> because sizeof(struct usb_fs_init) is 12 bytes on i386, and 16 bytes on
> >> amd64.
> >>
> >> But the failure of even libusb_open() - a fundamental libusb function
> >> - must mean that the COMPAT_32BIT libusb never worked. Can we please
> >> take this opportunity to kill it and do a real 32 bit compatibility
> >> layer in the kernel that will work from 32 bit chroots and statically
> >> linked 32 bit binaries? I'll help.
> >
> >
> > Hi,
> >
> > How did you compile it? Is the structure the same size, compiled with GCC
> > and LLVM?
> 
> GCC 4.2.1
> "gcc file.c -o file -lusb" in a 32 bit chroot = 12 bytes
> "gcc file.c -o file -lusb" 64 bit = 16 bytes
> 
> Clang 3.1
> "clang file.c -o file -lusb" in a 32 bit chroot = 12 bytes
> "clang file.c -o file -lusb" 64 bit = 16 bytes
> 
> The "uint8_t ep_index_max" field at the end of struct usb_fs_init must
> be getting padded to a 4 byte boundary on 32 bit, and 8 byte boundary
> on 64 bit.
> 
> > The LIB32 for USB has been tested. I did a quick test and found on 9-stable:
> >
> > cc -m32 -I . -L /usr/lib32 -lusb usbconfig.c dump.c
> > env LD_PRELOAD=/usr/lib32/libusb.so ./a.out
> > ugen0.1:  at usbus0, cfg=0 md=HOST spd=FULL (12Mbps)
> > pwr=SAVE (0mA)
> > ugen1.1:  at usbus1, cfg=0 md=HOST spd=FULL (12Mbps)
> > pwr=SAVE (0mA)
> >
> > ...
> 
> But usbconfig uses the libusb20 API and never seems to get to that broken 
> ioctl.
> 
> > Your approach requires much more code. It is not just about some structures,
> > but also about code accessing those structures. Feel free to submit a patch
> > however.
> 
> I'll see what I can do.
> 
> > I would rather fix this by adding the proper __aligned() to the structures
> > in question.
> 
> Do that in the meanwhile?
> 
> > --HPS
> >
> 
> Damjan
> 

___
freebsd-usb@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-usb
To unsubscribe, send any mail to "freebsd-usb-unsubscr...@freebsd.org"


Re: COMPAT_32BIT libusb ABI problem

2013-07-25 Thread Damjan Jovanovic
On Thu, Jul 25, 2013 at 6:11 AM, Hans Petter Selasky  wrote:
> On 07/24/13 22:09, Damjan Jovanovic wrote:
>>
>> Hi
>>
>> The ioctl:
>> #define USB_FS_INIT _IOW ('U', 195, struct usb_fs_init)
>> when used by a COMPAT_32BIT libusb on amd64, fails (causing
>> libusb_open() to fail) due to:
>>
>> freebsd32_ioctl(0x6,0x800c55c3,0xc710,0x0,0x0,0x0) ERR#25
>> 'Inappropriate ioctl for device'
>> but when hacked a bit:
>> freebsd32_ioctl(0x6,0x801055c3,0xc710,0x0,0x0,0x0) = 0 (0x0)
>>
>> because sizeof(struct usb_fs_init) is 12 bytes on i386, and 16 bytes on
>> amd64.
>>
>> But the failure of even libusb_open() - a fundamental libusb function
>> - must mean that the COMPAT_32BIT libusb never worked. Can we please
>> take this opportunity to kill it and do a real 32 bit compatibility
>> layer in the kernel that will work from 32 bit chroots and statically
>> linked 32 bit binaries? I'll help.
>
>
> Hi,
>
> How did you compile it? Is the structure the same size, compiled with GCC
> and LLVM?

GCC 4.2.1
"gcc file.c -o file -lusb" in a 32 bit chroot = 12 bytes
"gcc file.c -o file -lusb" 64 bit = 16 bytes

Clang 3.1
"clang file.c -o file -lusb" in a 32 bit chroot = 12 bytes
"clang file.c -o file -lusb" 64 bit = 16 bytes

The "uint8_t ep_index_max" field at the end of struct usb_fs_init must
be getting padded to a 4 byte boundary on 32 bit, and 8 byte boundary
on 64 bit.

> The LIB32 for USB has been tested. I did a quick test and found on 9-stable:
>
> cc -m32 -I . -L /usr/lib32 -lusb usbconfig.c dump.c
> env LD_PRELOAD=/usr/lib32/libusb.so ./a.out
> ugen0.1:  at usbus0, cfg=0 md=HOST spd=FULL (12Mbps)
> pwr=SAVE (0mA)
> ugen1.1:  at usbus1, cfg=0 md=HOST spd=FULL (12Mbps)
> pwr=SAVE (0mA)
>
> ...

But usbconfig uses the libusb20 API and never seems to get to that broken ioctl.

> Your approach requires much more code. It is not just about some structures,
> but also about code accessing those structures. Feel free to submit a patch
> however.

I'll see what I can do.

> I would rather fix this by adding the proper __aligned() to the structures
> in question.

Do that in the meanwhile?

> --HPS
>

Damjan
___
freebsd-usb@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-usb
To unsubscribe, send any mail to "freebsd-usb-unsubscr...@freebsd.org"


Re: COMPAT_32BIT libusb ABI problem

2013-07-24 Thread Hans Petter Selasky

On 07/24/13 22:09, Damjan Jovanovic wrote:

Hi

The ioctl:
#define USB_FS_INIT _IOW ('U', 195, struct usb_fs_init)
when used by a COMPAT_32BIT libusb on amd64, fails (causing
libusb_open() to fail) due to:

freebsd32_ioctl(0x6,0x800c55c3,0xc710,0x0,0x0,0x0) ERR#25
'Inappropriate ioctl for device'
but when hacked a bit:
freebsd32_ioctl(0x6,0x801055c3,0xc710,0x0,0x0,0x0) = 0 (0x0)

because sizeof(struct usb_fs_init) is 12 bytes on i386, and 16 bytes on amd64.

But the failure of even libusb_open() - a fundamental libusb function
- must mean that the COMPAT_32BIT libusb never worked. Can we please
take this opportunity to kill it and do a real 32 bit compatibility
layer in the kernel that will work from 32 bit chroots and statically
linked 32 bit binaries? I'll help.


Hi,

How did you compile it? Is the structure the same size, compiled with 
GCC and LLVM?


The LIB32 for USB has been tested. I did a quick test and found on 9-stable:

cc -m32 -I . -L /usr/lib32 -lusb usbconfig.c dump.c
env LD_PRELOAD=/usr/lib32/libusb.so ./a.out
ugen0.1:  at usbus0, cfg=0 md=HOST spd=FULL 
(12Mbps) pwr=SAVE (0mA)
ugen1.1:  at usbus1, cfg=0 md=HOST spd=FULL 
(12Mbps) pwr=SAVE (0mA)


...

Your approach requires much more code. It is not just about some 
structures, but also about code accessing those structures. Feel free to 
submit a patch however.


I would rather fix this by adding the proper __aligned() to the 
structures in question.


--HPS

___
freebsd-usb@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-usb
To unsubscribe, send any mail to "freebsd-usb-unsubscr...@freebsd.org"


COMPAT_32BIT libusb ABI problem

2013-07-24 Thread Damjan Jovanovic
Hi

The ioctl:
#define USB_FS_INIT _IOW ('U', 195, struct usb_fs_init)
when used by a COMPAT_32BIT libusb on amd64, fails (causing
libusb_open() to fail) due to:

freebsd32_ioctl(0x6,0x800c55c3,0xc710,0x0,0x0,0x0) ERR#25
'Inappropriate ioctl for device'
but when hacked a bit:
freebsd32_ioctl(0x6,0x801055c3,0xc710,0x0,0x0,0x0) = 0 (0x0)

because sizeof(struct usb_fs_init) is 12 bytes on i386, and 16 bytes on amd64.

But the failure of even libusb_open() - a fundamental libusb function
- must mean that the COMPAT_32BIT libusb never worked. Can we please
take this opportunity to kill it and do a real 32 bit compatibility
layer in the kernel that will work from 32 bit chroots and statically
linked 32 bit binaries? I'll help.

Regards
Damjan
___
freebsd-usb@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-usb
To unsubscribe, send any mail to "freebsd-usb-unsubscr...@freebsd.org"