Re: [PATCH] ANDROID: binder: remove 32-bit binder interface.
On Mon, May 14, 2018 at 4:00 PM, Geert Uytterhoevenwrote: > Patch sent. Thanks for the quick turn-around! > > BTW, sh also doesn't seem to have 64-bit get_user(). > There may be others. I checked quickly, nios2 is the only other arch that explicitly doesn't support it and would result in a build error; some other archs don't define __get_user, but in that case they just fall back to raw_copy_from_user(). > > BTW2, does the Android Binder need to care about endianness when talking > to userspace? No, I don't think it should. Thanks, Martijn > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- > ge...@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like > that. > -- Linus Torvalds
Re: [PATCH] ANDROID: binder: remove 32-bit binder interface.
On Mon, May 14, 2018 at 4:00 PM, Geert Uytterhoeven wrote: > Patch sent. Thanks for the quick turn-around! > > BTW, sh also doesn't seem to have 64-bit get_user(). > There may be others. I checked quickly, nios2 is the only other arch that explicitly doesn't support it and would result in a build error; some other archs don't define __get_user, but in that case they just fall back to raw_copy_from_user(). > > BTW2, does the Android Binder need to care about endianness when talking > to userspace? No, I don't think it should. Thanks, Martijn > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- > ge...@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like > that. > -- Linus Torvalds
Re: [PATCH] ANDROID: binder: remove 32-bit binder interface.
Hi Christoph, On Mon, May 14, 2018 at 2:03 PM, Christoph Hellwigwrote: > On Fri, May 11, 2018 at 09:57:52AM +0200, Martijn Coenen wrote: >> On Sat, May 5, 2018 at 2:10 PM, kbuild test robot wrote: >> >drivers/android/binder.o: In function `binder_thread_write': >> >>> binder.c:(.text+0x6a16): undefined reference to `__get_user_bad' >> >> Looks like m68k doesn't support 64-bit get_user(). I could just have >> binder depend on !CONFIG_M68K, but there may be other architectures >> still that don't support this. Another alternative would be to >> whitelist the architectures Android supports - eg arm, arm64, x86, >> x86_64. But I'm not sure if arch-limited drivers are considered bad >> form. Does anybody have suggestions for how to deal with this? > > The proper fix is to just support 640bit get/put_user on m68k instead I hope we'll never need 640bit support in {get,put}_user() ;-) > of working around this. Patch sent. BTW, sh also doesn't seem to have 64-bit get_user(). There may be others. BTW2, does the Android Binder need to care about endianness when talking to userspace? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Re: [PATCH] ANDROID: binder: remove 32-bit binder interface.
Hi Christoph, On Mon, May 14, 2018 at 2:03 PM, Christoph Hellwig wrote: > On Fri, May 11, 2018 at 09:57:52AM +0200, Martijn Coenen wrote: >> On Sat, May 5, 2018 at 2:10 PM, kbuild test robot wrote: >> >drivers/android/binder.o: In function `binder_thread_write': >> >>> binder.c:(.text+0x6a16): undefined reference to `__get_user_bad' >> >> Looks like m68k doesn't support 64-bit get_user(). I could just have >> binder depend on !CONFIG_M68K, but there may be other architectures >> still that don't support this. Another alternative would be to >> whitelist the architectures Android supports - eg arm, arm64, x86, >> x86_64. But I'm not sure if arch-limited drivers are considered bad >> form. Does anybody have suggestions for how to deal with this? > > The proper fix is to just support 640bit get/put_user on m68k instead I hope we'll never need 640bit support in {get,put}_user() ;-) > of working around this. Patch sent. BTW, sh also doesn't seem to have 64-bit get_user(). There may be others. BTW2, does the Android Binder need to care about endianness when talking to userspace? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Re: [PATCH] ANDROID: binder: remove 32-bit binder interface.
On Fri, May 11, 2018 at 09:57:52AM +0200, Martijn Coenen wrote: > On Sat, May 5, 2018 at 2:10 PM, kbuild test robotwrote: > >drivers/android/binder.o: In function `binder_thread_write': > >>> binder.c:(.text+0x6a16): undefined reference to `__get_user_bad' > > Looks like m68k doesn't support 64-bit get_user(). I could just have > binder depend on !CONFIG_M68K, but there may be other architectures > still that don't support this. Another alternative would be to > whitelist the architectures Android supports - eg arm, arm64, x86, > x86_64. But I'm not sure if arch-limited drivers are considered bad > form. Does anybody have suggestions for how to deal with this? The proper fix is to just support 640bit get/put_user on m68k instead of working around this.
Re: [PATCH] ANDROID: binder: remove 32-bit binder interface.
On Fri, May 11, 2018 at 09:57:52AM +0200, Martijn Coenen wrote: > On Sat, May 5, 2018 at 2:10 PM, kbuild test robot wrote: > >drivers/android/binder.o: In function `binder_thread_write': > >>> binder.c:(.text+0x6a16): undefined reference to `__get_user_bad' > > Looks like m68k doesn't support 64-bit get_user(). I could just have > binder depend on !CONFIG_M68K, but there may be other architectures > still that don't support this. Another alternative would be to > whitelist the architectures Android supports - eg arm, arm64, x86, > x86_64. But I'm not sure if arch-limited drivers are considered bad > form. Does anybody have suggestions for how to deal with this? The proper fix is to just support 640bit get/put_user on m68k instead of working around this.
Re: [PATCH] ANDROID: binder: remove 32-bit binder interface.
On Fri, May 11, 2018 at 10:08 AM, Greg KHwrote: > I think using !CONFIG_M68K is a good start. We can blacklist any other > arch that doesn't support this, and that list should be small as I doubt > any new ones will be added without this support. Thanks, I will send a v2. > > thanks, > > greg k-h
Re: [PATCH] ANDROID: binder: remove 32-bit binder interface.
On Fri, May 11, 2018 at 10:08 AM, Greg KH wrote: > I think using !CONFIG_M68K is a good start. We can blacklist any other > arch that doesn't support this, and that list should be small as I doubt > any new ones will be added without this support. Thanks, I will send a v2. > > thanks, > > greg k-h
Re: [PATCH] ANDROID: binder: remove 32-bit binder interface.
On Fri, May 11, 2018 at 09:57:52AM +0200, Martijn Coenen wrote: > On Sat, May 5, 2018 at 2:10 PM, kbuild test robotwrote: > >drivers/android/binder.o: In function `binder_thread_write': > >>> binder.c:(.text+0x6a16): undefined reference to `__get_user_bad' > > Looks like m68k doesn't support 64-bit get_user(). I could just have > binder depend on !CONFIG_M68K, but there may be other architectures > still that don't support this. Another alternative would be to > whitelist the architectures Android supports - eg arm, arm64, x86, > x86_64. But I'm not sure if arch-limited drivers are considered bad > form. Does anybody have suggestions for how to deal with this? I think using !CONFIG_M68K is a good start. We can blacklist any other arch that doesn't support this, and that list should be small as I doubt any new ones will be added without this support. thanks, greg k-h
Re: [PATCH] ANDROID: binder: remove 32-bit binder interface.
On Fri, May 11, 2018 at 09:57:52AM +0200, Martijn Coenen wrote: > On Sat, May 5, 2018 at 2:10 PM, kbuild test robot wrote: > >drivers/android/binder.o: In function `binder_thread_write': > >>> binder.c:(.text+0x6a16): undefined reference to `__get_user_bad' > > Looks like m68k doesn't support 64-bit get_user(). I could just have > binder depend on !CONFIG_M68K, but there may be other architectures > still that don't support this. Another alternative would be to > whitelist the architectures Android supports - eg arm, arm64, x86, > x86_64. But I'm not sure if arch-limited drivers are considered bad > form. Does anybody have suggestions for how to deal with this? I think using !CONFIG_M68K is a good start. We can blacklist any other arch that doesn't support this, and that list should be small as I doubt any new ones will be added without this support. thanks, greg k-h
Re: [PATCH] ANDROID: binder: remove 32-bit binder interface.
On Sat, May 5, 2018 at 2:10 PM, kbuild test robotwrote: >drivers/android/binder.o: In function `binder_thread_write': >>> binder.c:(.text+0x6a16): undefined reference to `__get_user_bad' Looks like m68k doesn't support 64-bit get_user(). I could just have binder depend on !CONFIG_M68K, but there may be other architectures still that don't support this. Another alternative would be to whitelist the architectures Android supports - eg arm, arm64, x86, x86_64. But I'm not sure if arch-limited drivers are considered bad form. Does anybody have suggestions for how to deal with this? Thanks, Martijn >binder.c:(.text+0x6c9a): undefined reference to `__get_user_bad' >binder.c:(.text+0x701e): undefined reference to `__get_user_bad' >binder.c:(.text+0x7414): undefined reference to `__get_user_bad' > > --- > 0-DAY kernel test infrastructureOpen Source Technology Center > https://lists.01.org/pipermail/kbuild-all Intel Corporation
Re: [PATCH] ANDROID: binder: remove 32-bit binder interface.
On Sat, May 5, 2018 at 2:10 PM, kbuild test robot wrote: >drivers/android/binder.o: In function `binder_thread_write': >>> binder.c:(.text+0x6a16): undefined reference to `__get_user_bad' Looks like m68k doesn't support 64-bit get_user(). I could just have binder depend on !CONFIG_M68K, but there may be other architectures still that don't support this. Another alternative would be to whitelist the architectures Android supports - eg arm, arm64, x86, x86_64. But I'm not sure if arch-limited drivers are considered bad form. Does anybody have suggestions for how to deal with this? Thanks, Martijn >binder.c:(.text+0x6c9a): undefined reference to `__get_user_bad' >binder.c:(.text+0x701e): undefined reference to `__get_user_bad' >binder.c:(.text+0x7414): undefined reference to `__get_user_bad' > > --- > 0-DAY kernel test infrastructureOpen Source Technology Center > https://lists.01.org/pipermail/kbuild-all Intel Corporation
Re: [PATCH] ANDROID: binder: remove 32-bit binder interface.
Hi Martijn, I love your patch! Yet something to improve: [auto build test ERROR on staging/staging-testing] [also build test ERROR on v4.17-rc3 next-20180504] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Martijn-Coenen/ANDROID-binder-remove-32-bit-binder-interface/20180505-130632 config: m68k-allmodconfig (attached as .config) compiler: m68k-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=m68k All errors (new ones prefixed by >>): drivers/android/binder.o: In function `binder_thread_write': >> binder.c:(.text+0x6a16): undefined reference to `__get_user_bad' binder.c:(.text+0x6c9a): undefined reference to `__get_user_bad' binder.c:(.text+0x701e): undefined reference to `__get_user_bad' binder.c:(.text+0x7414): undefined reference to `__get_user_bad' --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
Re: [PATCH] ANDROID: binder: remove 32-bit binder interface.
Hi Martijn, I love your patch! Yet something to improve: [auto build test ERROR on staging/staging-testing] [also build test ERROR on v4.17-rc3 next-20180504] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Martijn-Coenen/ANDROID-binder-remove-32-bit-binder-interface/20180505-130632 config: m68k-allmodconfig (attached as .config) compiler: m68k-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=m68k All errors (new ones prefixed by >>): drivers/android/binder.o: In function `binder_thread_write': >> binder.c:(.text+0x6a16): undefined reference to `__get_user_bad' binder.c:(.text+0x6c9a): undefined reference to `__get_user_bad' binder.c:(.text+0x701e): undefined reference to `__get_user_bad' binder.c:(.text+0x7414): undefined reference to `__get_user_bad' --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
[PATCH] ANDROID: binder: remove 32-bit binder interface.
New devices launching with Android P need to use the 64-bit binder interface, even on 32-bit SoCs [0]. This change removes the Kconfig option to select the 32-bit binder interface. We don't think this will affect existing userspace for the following reasons: 1) The latest Android common tree is 4.14, so we don't believe any Android devices are on kernels >4.14. 2) Android devices launch on an LTS release and stick with it, so we wouldn't expect devices running on <= 4.14 now to upgrade to 4.17 or later. But even if they did, they'd rebuild the world (kernel + userspace) anyway. 3) Other userspaces like 'anbox' are already using the 64-bit interface. Note that this change doesn't remove the 32-bit UAPI itself; the reason for that is that Android userspace always uses the latest UAPI headers from upstream, and userspace retains 32-bit support for devices that are upgrading. This will be removed as well in 2-3 years, at which point we can remove the code from the UAPI as well. [0]: https://android-review.googlesource.com/c/platform/build/+/595193 Signed-off-by: Martijn Coenen--- drivers/android/Kconfig | 13 - drivers/android/binder.c | 4 2 files changed, 17 deletions(-) diff --git a/drivers/android/Kconfig b/drivers/android/Kconfig index 7dce3795b887..432e9ad77070 100644 --- a/drivers/android/Kconfig +++ b/drivers/android/Kconfig @@ -32,19 +32,6 @@ config ANDROID_BINDER_DEVICES created. Each binder device has its own context manager, and is therefore logically separated from the other devices. -config ANDROID_BINDER_IPC_32BIT - bool "Use old (Android 4.4 and earlier) 32-bit binder API" - depends on !64BIT && ANDROID_BINDER_IPC - default y - ---help--- - The Binder API has been changed to support both 32 and 64bit - applications in a mixed environment. - - Enable this to support an old 32-bit Android user-space (v4.4 and - earlier). - - Note that enabling this will break newer Android user-space. - config ANDROID_BINDER_IPC_SELFTEST bool "Android Binder IPC Driver Selftest" depends on ANDROID_BINDER_IPC diff --git a/drivers/android/binder.c b/drivers/android/binder.c index e578eee31589..2ee9fb02dfb8 100644 --- a/drivers/android/binder.c +++ b/drivers/android/binder.c @@ -72,10 +72,6 @@ #include #include -#ifdef CONFIG_ANDROID_BINDER_IPC_32BIT -#define BINDER_IPC_32BIT 1 -#endif - #include #include "binder_alloc.h" #include "binder_trace.h" -- 2.17.0.441.gb46fe60e1d-goog
[PATCH] ANDROID: binder: remove 32-bit binder interface.
New devices launching with Android P need to use the 64-bit binder interface, even on 32-bit SoCs [0]. This change removes the Kconfig option to select the 32-bit binder interface. We don't think this will affect existing userspace for the following reasons: 1) The latest Android common tree is 4.14, so we don't believe any Android devices are on kernels >4.14. 2) Android devices launch on an LTS release and stick with it, so we wouldn't expect devices running on <= 4.14 now to upgrade to 4.17 or later. But even if they did, they'd rebuild the world (kernel + userspace) anyway. 3) Other userspaces like 'anbox' are already using the 64-bit interface. Note that this change doesn't remove the 32-bit UAPI itself; the reason for that is that Android userspace always uses the latest UAPI headers from upstream, and userspace retains 32-bit support for devices that are upgrading. This will be removed as well in 2-3 years, at which point we can remove the code from the UAPI as well. [0]: https://android-review.googlesource.com/c/platform/build/+/595193 Signed-off-by: Martijn Coenen --- drivers/android/Kconfig | 13 - drivers/android/binder.c | 4 2 files changed, 17 deletions(-) diff --git a/drivers/android/Kconfig b/drivers/android/Kconfig index 7dce3795b887..432e9ad77070 100644 --- a/drivers/android/Kconfig +++ b/drivers/android/Kconfig @@ -32,19 +32,6 @@ config ANDROID_BINDER_DEVICES created. Each binder device has its own context manager, and is therefore logically separated from the other devices. -config ANDROID_BINDER_IPC_32BIT - bool "Use old (Android 4.4 and earlier) 32-bit binder API" - depends on !64BIT && ANDROID_BINDER_IPC - default y - ---help--- - The Binder API has been changed to support both 32 and 64bit - applications in a mixed environment. - - Enable this to support an old 32-bit Android user-space (v4.4 and - earlier). - - Note that enabling this will break newer Android user-space. - config ANDROID_BINDER_IPC_SELFTEST bool "Android Binder IPC Driver Selftest" depends on ANDROID_BINDER_IPC diff --git a/drivers/android/binder.c b/drivers/android/binder.c index e578eee31589..2ee9fb02dfb8 100644 --- a/drivers/android/binder.c +++ b/drivers/android/binder.c @@ -72,10 +72,6 @@ #include #include -#ifdef CONFIG_ANDROID_BINDER_IPC_32BIT -#define BINDER_IPC_32BIT 1 -#endif - #include #include "binder_alloc.h" #include "binder_trace.h" -- 2.17.0.441.gb46fe60e1d-goog