Re: [PATCH v1 9/9] staging: android: binder: Add binder compat layer

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi all,

Thanks for your feedback! Sadly enough, being in a different time-zone, is not useful.

Sorry for the confusion related to why is this patch needed or not. I
should highlight a bit more what is the patch enabling and what would be the different alternatives, at least from my perspective.

*64bit kernel/ 32bit userspace*

This patch series adds support for 32bit userspace running on 64bit kernels. Thus by applying this patch to your kernel you will be able to use any existing 32bit Android userspace on your 64bit platform, running a 64bit kernel. That is pure 32bit userspace with no 64bit support!

This means *no modifications to the 32bit userspace*. Therefore any applications or userspace side drivers, that uses the binder interface at a native level will not have to be modified. These kind of applications are not "good citizens" - the native binder API is not exposed in the Android NDK. However I do not know how many applications do this and if breaking the compatibility is a concernt for 32bit userspace running on 64bit kernels.

Below you will find more information on one of the alternative solutions, the userspace compat.

*32bit kernel/ 32bit userspace*
*64bit kernel/ 64bit userspace*

My last series of binder patches, accepted into 3.12 revision of the Linux kernel, audits the binder interface for 64bit. A kernel with these changes applied can be used with a pure 64bit userspace (this does not include support for 32bit applications coexisting with 64bit ones). The userspace side needs trivial changes to libbinder.so and servicemanager, that I will upstream ASAP to AOSP, and that work for 32/32 systems and 64/64 systems without modifying the existing 32bit interface ABI (most of the changes are related to uint32_t != void *).

Author: Serban Constantinescu <serban.constantinescu@xxxxxxx>
Date:   Thu Jul 4 10:54:48 2013 +0100

staging: android: binder: fix binder interface for 64bit compat layer

*64bit kernel/ 64bit coexisting with 32bit userspace

Please note that *none of the above solutions fix this yet*. To understand why this is a special case please take a look at
how the binder driver works, seen from a high level perspective:

          ServiceManager
App1  <---------------------> App2

Thus, for two apps to exchange information between each other they will have to communicate with the userspace governor, ServiceManager. All the interaction between App1, App2 and ServiceManager is done through a combination of libbinder.so (Userspace HAL) and the binder driver. Note that there can only been one ServiceManager process, that is set during the userspace boot process.

Now lets consider the case that Arve described earlier 32bit Applications coexisting with 64bit ones. In this case all the commands and interface used will have to be the same, the ones used by the 64bit ServiceManager. For this the kernel entry point for 32bit compat will have to be changed to:

--- a/drivers/staging/android/binder.c
+++ b/drivers/staging/android/binder.c
@@ -3893,9 +3893,13 @@ static const struct file_operations binder_fops = {
        .owner = THIS_MODULE,
        .poll = binder_poll,
        .unlocked_ioctl = binder_ioctl,
-#ifdef CONFIG_COMPAT
+#if defined(CONFIG_COMPAT) && !defined(COEXIST_32BIT_64BIT)
        .compat_ioctl = compat_binder_ioctl,
 #endif
+
+#if defined(COEXIST_32BIT_64BIT)
+       .compat_ioctl = binder_ioctl,
+#endif
        .mmap = binder_mmap,
        .open = binder_open,
        .flush = binder_flush,

thus from the perspective of a kernel that works with a 64bit userspace where 64bit applications coexist with 32bit applications the handling will be the same for both 32bit and 64bit processes. This will also involve modifying libbinder.so to use 64bit structures like:

--- a/libs/binder/IPCThreadState.cpp
+++ b/libs/binder/IPCThreadState.cpp
+#if defined(COEXIST_32BIT_64BIT) && !defined(__LP64__)
+/*
+ * For running 32-bit processes on a 64-bit system, make the interaction with the
+ * binder driver the same from this, when built for 32-bit, as for a 64-bit process.
+ */
+struct coexist_binder_write_read {
+    uint64_t    write_size;     /* __LP64__ size_t: bytes to write */
+    uint64_t    write_consumed; /* __LP64__ size_t: bytes consumed by driver */
+    uint64_t    write_buffer;   /* __LP64__ unsigned long */
+    uint64_t    read_size;      /* __LP64__ size_t: bytes to read */
+    uint64_t    read_consumed;  /* __LP64__ size_t: bytes consumed by driver */
+    uint64_t    read_buffer;    /* __LP64__ unsigned long */
+};

"Good citizen" Android applications will go through these changes (since they should only use the binder Java API), and so shouldn't encounter any problem. Any 32bit applications that use the binder interface at a native level will not work with this model (they should not do so!)· This is exactly what the kernel compat "guarantees" *only* for 64/32bit systems.

The cleaner solution from the binder kernel perspective is to hook the compat_ioctl to binder_ioctl and do all this clean-up in the userspace. This way the same userspace compat can be used for 64bit kernel/32 bit userspace, 64bit kernel/ 64 bit coexisting with 32bit userspace. Note - this will mean that 64bit systems with 32bit userspace will be tied to a Android version >= the version with COEXIST_32BIT_64BIT support, and if the binder interface is used at a lower level than it should be your app will not work!

Or does this patch series mean that no userspace code is changed?  Is
that a "requirement" here?

This is what this series boils down to... Do we need to keep this compatibility for 64/32 and aford breaking it for 64/64-32 ?



I don't think we need to support old 32 bit userspace framework code
on a 64 bit system. I think it is more important to not prevent mixed
mode systems.

See above snippet for:
static const struct file_operations binder_fops.
a kernel build flag could differentiate between your target.

Plese let me know if I have missed any other solution and if there is a better alternative. Overall I intend to find a solution rather then create more mess. The binder driver is complicated as it is and does not need more complexity added, however moving forward we will have to support some of these systems.

Thanks for all your feedback,
Let me know your thoughts on this,
Serban Constantinescu

_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel





[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux