lkml.org 
[lkml]   [2013]   [Dec]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v1 9/9] staging: android: binder: Add binder compat layer
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@arm.com>
> 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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2013-12-05 20:01    [W:1.391 / U:0.340 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site