2013-03-05 08:36:48

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] staging: android: ashmem: Add support for 32bit ashmem calls in a 64bit kernel

On Mon, Feb 25, 2013 at 06:44:36PM +0000, Serban Constantinescu wrote:
> Android's shared memory subsystem, Ashmem, does not support calls from a
> 32bit userspace in a 64 bit kernel. This patch adds support for syscalls
> coming from a 32bit userspace in a 64bit kernel.
>
> The patch has been successfully tested on ARMv8 AEM(64bit
> platform model) and Versatile Express A9(32bit platform).
>
> Signed-off-by: Serban Constantinescu <[email protected]>
> Acked-by: Arve Hjønnevåg <[email protected]>
> Acked-by: John Stultz <[email protected]>

This patch breaks the build on my machine:
drivers/staging/android/ashmem.c: In function ‘compat_ashmem_ioctl’:
drivers/staging/android/ashmem.c:711:7: error: ‘compat_size_t’ undeclared (first use in this function)
drivers/staging/android/ashmem.c:711:7: note: each undeclared identifier is reported only once for each function it appears in

so of course, I can't take it :(

greg k-h


2013-03-05 10:20:16

by Serban Constantinescu

[permalink] [raw]
Subject: [PATCH] staging: android: ashmem: Add support for 32bit ashmem calls in a 64bit kernel

Android's shared memory subsystem, Ashmem, does not support calls from a
32bit userspace in a 64 bit kernel. This patch adds support for syscalls
coming from a 32bit userspace in a 64bit kernel.

The patch has been successfully tested on ARMv8 AEM(64bit
platform model) and Versatile Express A9(32bit platform).

Signed-off-by: Serban Constantinescu <[email protected]>
---
drivers/staging/android/ashmem.c | 22 +++++++++++++++++++++-
drivers/staging/android/ashmem.h | 6 ++++++
2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/android/ashmem.c b/drivers/staging/android/ashmem.c
index 72064fc..e96f381 100644
--- a/drivers/staging/android/ashmem.c
+++ b/drivers/staging/android/ashmem.c
@@ -31,6 +31,7 @@
#include <linux/bitops.h>
#include <linux/mutex.h>
#include <linux/shmem_fs.h>
+#include <linux/compat.h>
#include "ashmem.h"

#define ASHMEM_NAME_PREFIX "dev/ashmem/"
@@ -674,6 +675,23 @@ static long ashmem_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
return ret;
}

+/* support of 32bit userspace on 64bit platforms */
+#ifdef CONFIG_COMPAT
+static long compat_ashmem_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
+{
+
+ switch (cmd) {
+ case COMPAT_ASHMEM_SET_SIZE:
+ cmd = ASHMEM_SET_SIZE;
+ break;
+ case COMPAT_ASHMEM_SET_PROT_MASK:
+ cmd = ASHMEM_SET_PROT_MASK;
+ break;
+ }
+ return ashmem_ioctl(file, cmd, arg);
+}
+#endif
+
static const struct file_operations ashmem_fops = {
.owner = THIS_MODULE,
.open = ashmem_open,
@@ -682,7 +700,9 @@ static const struct file_operations ashmem_fops = {
.llseek = ashmem_llseek,
.mmap = ashmem_mmap,
.unlocked_ioctl = ashmem_ioctl,
- .compat_ioctl = ashmem_ioctl,
+#ifdef CONFIG_COMPAT
+ .compat_ioctl = compat_ashmem_ioctl,
+#endif
};

static struct miscdevice ashmem_misc = {
diff --git a/drivers/staging/android/ashmem.h b/drivers/staging/android/ashmem.h
index 1976b10..2809d31 100644
--- a/drivers/staging/android/ashmem.h
+++ b/drivers/staging/android/ashmem.h
@@ -45,4 +45,10 @@ struct ashmem_pin {
#define ASHMEM_GET_PIN_STATUS _IO(__ASHMEMIOC, 9)
#define ASHMEM_PURGE_ALL_CACHES _IO(__ASHMEMIOC, 10)

+/* support of 32bit userspace on 64bit platforms */
+#ifdef CONFIG_COMPAT
+#define COMPAT_ASHMEM_SET_SIZE _IOW(__ASHMEMIOC, 3, compat_size_t)
+#define COMPAT_ASHMEM_SET_PROT_MASK _IOW(__ASHMEMIOC, 5, unsigned int)
+#endif
+
#endif /* _LINUX_ASHMEM_H */
--
1.7.9.5

2013-03-05 10:25:53

by Serban Constantinescu

[permalink] [raw]
Subject: Re: [PATCH] staging: android: ashmem: Add support for 32bit ashmem calls in a 64bit kernel

On 05/03/13 08:37, Greg KH wrote:
> On Mon, Feb 25, 2013 at 06:44:36PM +0000, Serban Constantinescu wrote:
>> Android's shared memory subsystem, Ashmem, does not support calls from a
>> 32bit userspace in a 64 bit kernel. This patch adds support for syscalls
>> coming from a 32bit userspace in a 64bit kernel.
>>
>> The patch has been successfully tested on ARMv8 AEM(64bit
>> platform model) and Versatile Express A9(32bit platform).
>>
>> Signed-off-by: Serban Constantinescu <[email protected]>
>> Acked-by: Arve Hjønnevåg <[email protected]>
>> Acked-by: John Stultz <[email protected]>
>
> This patch breaks the build on my machine:
> drivers/staging/android/ashmem.c: In function ‘compat_ashmem_ioctl’:
> drivers/staging/android/ashmem.c:711:7: error: ‘compat_size_t’ undeclared (first use in this function)
> drivers/staging/android/ashmem.c:711:7: note: each undeclared identifier is reported only once for each function it appears in

When building for x86_64 we need <linux/compat.h>. Sorry I forgot it,
should build fine now.

>
> so of course, I can't take it :(

Thanks,
Serban

2013-03-05 10:27:28

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] staging: android: ashmem: Add support for 32bit ashmem calls in a 64bit kernel

On Tue, Mar 05, 2013 at 10:18:27AM +0000, Serban Constantinescu wrote:
> Android's shared memory subsystem, Ashmem, does not support calls from a
> 32bit userspace in a 64 bit kernel. This patch adds support for syscalls
> coming from a 32bit userspace in a 64bit kernel.
>
> The patch has been successfully tested on ARMv8 AEM(64bit
> platform model) and Versatile Express A9(32bit platform).
>
> Signed-off-by: Serban Constantinescu <[email protected]>
> ---
> drivers/staging/android/ashmem.c | 22 +++++++++++++++++++++-
> drivers/staging/android/ashmem.h | 6 ++++++
> 2 files changed, 27 insertions(+), 1 deletion(-)

Ok, what has changed from your previous version that I rejected that is
going to actually allow this to build?

Hint, you gotta tell me what version of the patch this is, and what
changed, otherwise I'll just assume this is a resend of the previous
patch and go ahead and delete it from my queue.

In fact, I might as well do that, now deleted :)

greg k-h

2013-03-05 10:38:45

by Serban Constantinescu

[permalink] [raw]
Subject: Re: [PATCH] staging: android: ashmem: Add support for 32bit ashmem calls in a 64bit kernel

On 05/03/13 10:27, Greg KH wrote:
> On Tue, Mar 05, 2013 at 10:18:27AM +0000, Serban Constantinescu wrote:
>> Android's shared memory subsystem, Ashmem, does not support calls from a
>> 32bit userspace in a 64 bit kernel. This patch adds support for syscalls
>> coming from a 32bit userspace in a 64bit kernel.
>>
>> The patch has been successfully tested on ARMv8 AEM(64bit
>> platform model) and Versatile Express A9(32bit platform).
>>
>> Signed-off-by: Serban Constantinescu <[email protected]>
>> ---
>> drivers/staging/android/ashmem.c | 22 +++++++++++++++++++++-
>> drivers/staging/android/ashmem.h | 6 ++++++
>> 2 files changed, 27 insertions(+), 1 deletion(-)
>
> Ok, what has changed from your previous version that I rejected that is
> going to actually allow this to build?

I had to add <linux/compat.h> for a successful build on x86_64. The
attached hunk is the only change added.

> diff --git a/drivers/staging/android/ashmem.c b/drivers/staging/android/ashmem.c
> index 72064fc..e96f381 100644
> --- a/drivers/staging/android/ashmem.c
> +++ b/drivers/staging/android/ashmem.c
> @@ -31,6 +31,7 @@
> #include <linux/bitops.h>
> #include <linux/mutex.h>
> #include <linux/shmem_fs.h>
> +#include <linux/compat.h>
> #include "ashmem.h"
>
> #define ASHMEM_NAME_PREFIX "dev/ashmem/"


>
> Hint, you gotta tell me what version of the patch this is, and what
> changed, otherwise I'll just assume this is a resend of the previous
> patch and go ahead and delete it from my queue.

Sorry - same patch as before, the one that had John and Arve's ack, plus
the above hunk.
>
> In fact, I might as well do that, now deleted :)
>
> greg k-h
>

Thanks,
Serban

2013-03-05 10:47:10

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] staging: android: ashmem: Add support for 32bit ashmem calls in a 64bit kernel

On Tue, Mar 05, 2013 at 10:38:48AM +0000, Serban Constantinescu wrote:
> On 05/03/13 10:27, Greg KH wrote:
> >On Tue, Mar 05, 2013 at 10:18:27AM +0000, Serban Constantinescu wrote:
> >>Android's shared memory subsystem, Ashmem, does not support calls from a
> >>32bit userspace in a 64 bit kernel. This patch adds support for syscalls
> >>coming from a 32bit userspace in a 64bit kernel.
> >>
> >>The patch has been successfully tested on ARMv8 AEM(64bit
> >>platform model) and Versatile Express A9(32bit platform).
> >>
> >>Signed-off-by: Serban Constantinescu <[email protected]>
> >>---
> >> drivers/staging/android/ashmem.c | 22 +++++++++++++++++++++-
> >> drivers/staging/android/ashmem.h | 6 ++++++
> >> 2 files changed, 27 insertions(+), 1 deletion(-)
> >
> >Ok, what has changed from your previous version that I rejected that is
> >going to actually allow this to build?
>
> I had to add <linux/compat.h> for a successful build on x86_64. The
> attached hunk is the only change added.

Ok, please say that. I deal with over 7000 patches a year, there is no
way I can remember what the difference is between this one, and the
previous one I rejected. Especially as I reviewed about 30 patches
inbetween the time I rejected it and you sent this update.

> >diff --git a/drivers/staging/android/ashmem.c b/drivers/staging/android/ashmem.c
> >index 72064fc..e96f381 100644
> >--- a/drivers/staging/android/ashmem.c
> >+++ b/drivers/staging/android/ashmem.c
> >@@ -31,6 +31,7 @@
> > #include <linux/bitops.h>
> > #include <linux/mutex.h>
> > #include <linux/shmem_fs.h>
> >+#include <linux/compat.h>
> > #include "ashmem.h"
> >
> > #define ASHMEM_NAME_PREFIX "dev/ashmem/"
>
>
> >
> >Hint, you gotta tell me what version of the patch this is, and what
> >changed, otherwise I'll just assume this is a resend of the previous
> >patch and go ahead and delete it from my queue.
>
> Sorry - same patch as before, the one that had John and Arve's ack,
> plus the above hunk.

That is not what I meant, please go read Section 2 of the
Documentation/SubmittingPatches file for what I need to see here.

thanks,

greg k-h