On Wed, Dec 4, 2013 at 2:02 PM, Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote: > On Wed, Dec 04, 2013 at 01:55:34PM -0800, Colin Cross wrote: >> On Wed, Dec 4, 2013 at 1:43 PM, Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote: >> > On Wed, Dec 04, 2013 at 12:46:42PM -0800, Colin Cross wrote: >> >> On Wed, Dec 4, 2013 at 10:35 AM, Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote: >> >> <snip> >> >> >> >> > And finally, is this all really needed? Why not just fix the structures >> >> > to be "correct", and then fix userspace to use the correct structures as >> >> > well, thereby not needing a compat layer at all? >> >> >> >> Some of the binder ioctls take userspace pointers. Are you suggesting >> >> storing those pointers in a __u64 to avoid having to have a >> >> compat_ioctl? >> > >> > Yes, that's the best way to solve the issue, right? >> >> It's the least code, but in exchange you lose all the type safety and >> warnings when copying in and out of the pointers, as well as sparse >> checking on the __user attribute. > > Not if you make the cast right at the beginning, when you first "touch" > the data, but yes, it does take some of the type saftey away, at the > expense of simpler code to mess up :) > >> That doesn't seem like a good tradeoff to me. In addition it requires >> modifying the existing heavily used 32 bit api, which means a >> mostly-equivalent compat layer added in libbinder to support old >> kernels. > > Wait, I thought that libbinder would have to be changed anyway here, to > handle 64bit kernels (in both 32 and 64bit userspace). Since you are > already changing it, why not just "do it correctly"? libbinder will need changes to support 64-bit userspace and especially a mixed 64-bit and 32-bit userspace, but this patch series is only addressing a pure 32-bit userspace on a 64-bit kernel. Support for a 64-bit userspace in Android is obviously going to require a future version of Android including, among other things, libbinder changes. As far as I know, those changes won't need to change the ioctl api, only the layout of the buffers that are passed through the ioctl api. > Or does this patch series mean that no userspace code is changed? Is > that a "requirement" here? Since this series only addresses 32-bit userspace on 64-bit kernel support there are no associated userspace changes. Changing the 32-bit api here means that combining the KitKat branch from http://android.googlesource.com with a newer kernel version will not work. _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
