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

[Hans Petter Selasky]

On Saturday 16 October 2010 10:12:37 Kostik Belousov wrote:
> On Thu, Oct 14, 2010 at 08:38:18PM +0000, Hans Petter Selasky wrote:
> > Author: hselasky
> > Date: Thu Oct 14 20:38:18 2010
> > New Revision: 213852
> > URL: http://svn.freebsd.org/changeset/base/213852
> > 
> > Log:
> >   - Add support for LibUSB in 32-bit compatibility mode.
> >   
> >   Approved by:    thompsa (mentor)
> > 
> > Modified:
> >   head/lib/libusb/Makefile
> >   head/lib/libusb/libusb20.c
> >   head/lib/libusb/libusb20_int.h
> >   head/lib/libusb/libusb20_ugen20.c
> >   head/sys/dev/usb/usb_ioctl.h
> > 
> > Modified: head/lib/libusb/Makefile
> > =========================================================================
> > ===== --- head/lib/libusb/Makefile	Thu Oct 14 20:31:07 2010	(r213851)
> > +++ head/lib/libusb/Makefile	Thu Oct 14 20:38:18 2010	(r213852)
> > @@ -30,5 +30,9 @@ SRCS+=		libusb10.c
> > 
> >  SRCS+=		libusb10_desc.c
> >  SRCS+=		libusb10_io.c
> > 
> > +.if defined(COMPAT_32BIT)
> > +CFLAGS+=	-DCOMPAT_32BIT
> > +.endif
> > +
> > 
> >  .include <bsd.lib.mk>
> 
> The support is provided in a way that contradicts the established practice
> of doing 32-bit compat. Very nice that the support is provided, thank you
> for care about it. But, can it be changed so that the kernel emulates
> 32-bit ABI instead of library conforming to the kernel ABI ?

The short answer is yes, but it adds much more code than in the existing 
approach, with regard to USB. It is not all about IOCTL's it is also about 
shared memory layout. The existing approach means:

You need to compile /usr/lib32/ and use that with the 32-bit binaries. I.E. I 
want to have the 32->64 bit conversion in user-space, hence as per 
implementation in the kernel, there are no pointer mappings involved. It is 
simply a matter of zero-extending the pointer variable from 32-bit to 64-bit.

> 
> For COMPAT32, we aim in making the system where 32bit binaries and
> libraries just work on the 64bit host. Your change does not allow to take
> 32bit host into jail and run it on 64bit kernel, as example.

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?

> 
> Please see numerous examples of ioctl translations under
> #ifdef COMPAT_FREEBSD32 on how it is done.

Please find attached a patch to fix libusbhid world breakage. Sorry about 
that. I will do some more checing and see if more is broken. I will commit 
this as soon as I get a go.

--HPS
-------------- next part --------------
==== Patch <050.txt> level 1
Source: [No source]
Target: ccf9f872-aa2e-dd11-9fc8-001c23d0bc1f:/head/lib/libusbhid:208023 [mirrored]
        (svn+ssh://hselasky@svn.freebsd.org/base)
Log:
Fix worldbuild breakage.


=== Makefile
==================================================================
--- Makefile	(revision 208023)
+++ Makefile	(patch 050.txt level 1)
@@ -19,4 +19,8 @@
 
 INCS=	usbhid.h
 
+.if defined(COMPAT_32BIT)
+CFLAGS+=	-DCOMPAT_32BIT
+.endif
+
 .include <bsd.lib.mk>
=== descr.c
==================================================================
--- descr.c	(revision 208023)
+++ descr.c	(patch 050.txt level 1)
@@ -103,7 +103,7 @@
 	memset(&ugd, 0, sizeof(ugd));
 
 	/* get actual length first */
-	ugd.ugd_data = NULL;
+	ugd.ugd_data = hid_pass_ptr(NULL);
 	ugd.ugd_maxlen = 65535;
 	if (ioctl(fd, USB_GET_REPORT_DESC, &ugd) < 0) {
 #ifdef HID_COMPAT7
@@ -124,7 +124,7 @@
 		return (NULL);
 
 	/* fetch actual descriptor */
-	ugd.ugd_data = data;
+	ugd.ugd_data = hid_pass_ptr(data);
 	ugd.ugd_maxlen = ugd.ugd_actlen;
 	if (ioctl(fd, USB_GET_REPORT_DESC, &ugd) < 0) {
 		/* could not read descriptor */
@@ -133,7 +133,7 @@
 	}
 
 	/* check END_COLLECTION */
-	if (((unsigned char *)ugd.ugd_data)[ugd.ugd_actlen -1] != 0xC0) {
+	if (((unsigned char *)data)[ugd.ugd_actlen -1] != 0xC0) {
 		/* invalid end byte */
 		free(data);
 		return (NULL);
=== usbvar.h
==================================================================
--- usbvar.h	(revision 208023)
+++ usbvar.h	(patch 050.txt level 1)
@@ -29,6 +29,9 @@
  *
  */
 
+#ifndef _USBVAR_H_
+#define	_USBVAR_H_
+
 struct report_desc {
 	uint32_t size;
 	uint8_t data[1];
@@ -41,3 +44,11 @@
 int	hid_get_report_id_compat7(int fd);
 report_desc_t	hid_get_report_desc_compat7(int fd);
 #endif
+
+#ifdef COMPAT_32BIT
+#define	hid_pass_ptr(ptr)	((uint64_t)(uintptr_t)(ptr))
+#else
+#define	hid_pass_ptr(ptr)	(ptr)
+#endif
+
+#endif		/* _USBVAR_H_ */