[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <52A0CB04.8030502@arm.com>
Date: Thu, 05 Dec 2013 18:50:44 +0000
From: Serban Constantinescu <Serban.Constantinescu@....com>
To: Dan Carpenter <dan.carpenter@...cle.com>
CC: "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
"arve@...roid.com" <arve@...roid.com>,
"devel@...verdev.osuosl.org" <devel@...verdev.osuosl.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"john.stultz@...aro.org" <john.stultz@...aro.org>,
"ccross@...roid.com" <ccross@...roid.com>,
Dave Butcher <Dave.Butcher@....com>,
"irogers@...gle.com" <irogers@...gle.com>,
"romlem@...roid.com" <romlem@...roid.com>
Subject: Re: [PATCH v1 3/9] staging: android: binder: Add cmd == CMD_NAME
handling
On 05/12/13 08:40, Dan Carpenter wrote:
> On Wed, Dec 04, 2013 at 06:09:35PM +0000, Serban Constantinescu wrote:
>> This patch modifies the functions that need to be passed the explicit
>> command to use a boolean flag. This way we can reuse the code for 64bit
>> compat commands.
>>
>
> I don't understand this at all. cmd seems like it should be 32 bits
> on both arches.
Command is 32bit for both arches but does not have the same value. Here
is what happens, this patch make more sense when looking at the compat
layer.
The binder commands differ between 32bit userspace and 64bit kernels. E.g:
> BR_INCREFS = _IOR('r', 7, struct binder_ptr_cookie)
> COMPAT_BR_INCREFS = _IOR('r', 7, struct compat_binder_ptr_cookie)
> struct compat_binder_ptr_cookie {
> compat_uptr_t ptr;
> compat_uptr_t cookie;
> };
> struct binder_ptr_cookie {
> void *ptr;
> void *cookie;
> };
(the IOR() macro encodes the size of the transaction - struct
binder_ptr_cookie).
This enables me to use the same handler for:
* native 64bit kernel/ 64bit userspace
> case BC_INCREFS_DONE:
> case BC_ACQUIRE_DONE: {
> void __user *node_ptr;
> void *cookie;
>
> if (get_user(node_ptr, (void * __user *)ptr))
> return -EFAULT;
> ptr += sizeof(void *);
> if (get_user(cookie, (void * __user *)ptr))
> return -EFAULT;
> ptr += sizeof(void *);
> bc_increfs_done(proc, thread, cmd == BC_ACQUIRE_DONE, node_ptr, cookie);
> break;
> }
* compat 64bit kernel/ 32bit userspace.
> + case COMPAT_BC_INCREFS_DONE:
> + case COMPAT_BC_ACQUIRE_DONE: {
> + compat_uptr_t node_ptr;
> + compat_uptr_t cookie;
> +
> + if (get_user(node_ptr, (compat_uptr_t __user *)*ptr))
> + return -EFAULT;
> + *ptr += sizeof(compat_uptr_t);
> + if (get_user(cookie, (compat_uptr_t __user *)*ptr))
> + return -EFAULT;
> + *ptr += sizeof(compat_uptr_t);
> + bc_increfs_done(proc, thread, cmd == COMPAT_BC_ACQUIRE_DONE,
> + compat_ptr(node_ptr), compat_ptr(cookie));
> + break;
where the prototype for bc_increfs_done() has changed to:
> static void bc_increfs_done(struct binder_proc *proc,
> - struct binder_thread *thread, uint32_t cmd,
> + struct binder_thread *thread, bool acquire,
Obsucre on its own, I should add more details in the commit message.
Thanks,
Serban C
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists