Re: [PATCH 2/3] staging: binder: Support concurrent 32 bit and 64 bit processes.

[John Stultz <john.stultz@linaro.org>]

On 02/21/2014 01:56 PM, Arve Hjønnevåg wrote:
> On Fri, Feb 21, 2014 at 1:29 PM, John Stultz <john.stultz@linaro.org> wrote:
>> On 02/21/2014 12:59 PM, Arve Hjønnevåg wrote:
>>> On Fri, Feb 21, 2014 at 12:43 PM, John Stultz <john.stultz@linaro.org> wrote:
>>>> From: Arve Hjønnevåg <arve@android.com>
>>>>
>>>> For 64bit systems we want to use the same binder interface for 32bit and
>>>> 64bit processes. Thus the size and the layout of the structures passed
>>>> between the kernel and the userspace has to be the same for both 32 and
>>>> 64bit processes.
>>>>
>>>> This change replaces all the uses of void* and size_t with
>>>> binder_uintptr_t and binder_size_t. These are then typedefed to specific
>>>> sizes depending on the use of the interface, as follows:
>>>>        * __u32 - on legacy 32bit only userspace
>>>>        * __u64 - on mixed 32/64bit userspace where all processes use the same
>>>> interface.
>>>>
>>>> This change also increments the BINDER_CURRENT_PROTOCOL_VERSION to 8 and
>>>> hooks the compat_ioctl entry for the mixed 32/64bit Android userspace.
>>>>
>>> It only increments the version to 8 if the old 32 bit interface is not selected.
>>>
>>>> This patch also provides a CONFIG_ANDROID_BINDER_IPC_32BIT option for
>>>> compatability, which if set which enables the old protocol on 32 bit
>>>> systems.
>> Ok. I thought that point was covered by the detail on
>> CONFIG_ANDROID_BINDER_IPC_32BIT, but maybe its not explicit enough.
>>
>> Would you be ok with:
>>
>> This change also increments the BINDER_CURRENT_PROTOCOL_VERSION to 8 and
>> hooks the compat_ioctl entry for the mixed 32/64bit Android userspace.
>>
>> This patch also provides a CONFIG_ANDROID_BINDER_IPC_32BIT option for
>> compatability, which if set which enables the old protocol, setting
>> BINDER_CURRENT_PROTOCOL_VERSION to 7, on 32 bit systems.
>>
>> ?
>>
> Yes, but replacing "This change" with "Selecting the 64 bit interface"
> would also work.

Ok.. I might stick to my wording above since with this patch, the 64bit
interface is now the unconditional case, with the 32bit interface having
the config option. So it might be more clear as there is no 64bit
interface option to select.

I'll add that bit and resend. Everything else in the patch series ok by you?

thanks
-john