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

John Stultz
2014-02-21T22:35:29Z

On 02/21/2014 02:10 PM, Arve Hjønnevåg wrote:
> On Fri, Feb 21, 2014 at 2:04 PM, John Stultz <john.stultz@linaro.org> wrote:
>> 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?
>>
> Assuming you did not change the code, yes.

Nothing but whitespace formatting for checkpatch as noted in the commit log.

After these three patches, the binder diff from android-3.10 is only
whitespace and the SELinux "Add security hooks to binder" patch which
makes changes out of the staging directory.

thanks
-john