| Message ID | 20130911143102.GA8825@darko.cambridge.arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Sep 11, 2013 at 7:32 AM, Catalin Marinas <catalin.marinas@arm.com> wrote: > On Mon, Sep 09, 2013 at 10:32:59PM +0100, Andrew Pinski wrote: >> This patch adds full support of the ABI to the ARM64 target. > > This description is too short. Please describe what the ABI is, what are > the commonalities with AArch64 and AArch32, what other non-obvious > things had to be done (like __kernel_long_t being long long). Split this > patch into multiple patches like base syscall handling, signal handling, > pselect6/ppoll, vdso etc. It's too much to review at once. Ok. I will do so after my vacation next week. > > On top of these, I would really like to see > Documentation/arm64/ilp32.txt describing the ABI. No other target does not, not even x86_64 for x32. > > I would also like to know (you can state this in the cover letter) the > level of testing for all 3 types of ABI. I'm worried that at least this > patch breaks the current compat ABI (has LTP reported anything?). We did test LTP on an earlier version of this patch for all three ABIs, I will make sure that the next version I send out is tested on all three ABIs also. We also tested ILP32/LP64 on big-endian at the same time which we will continue to do (I should push for our team here to push out the big-endian patches). > >> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig >> index cc64df5..7fdc994 100644 >> --- a/arch/arm64/Kconfig >> +++ b/arch/arm64/Kconfig >> @@ -248,7 +248,7 @@ source "fs/Kconfig.binfmt" >> >> config COMPAT >> def_bool y >> - depends on ARM64_AARCH32 >> + depends on ARM64_AARCH32 || ARM64_ILP32 >> select COMPAT_BINFMT_ELF >> >> config ARM64_AARCH32 >> @@ -263,7 +263,14 @@ config ARM64_AARCH32 >> the user helper functions, VFP support and the ptrace interface are >> handled appropriately by the kernel. >> >> - If you want to execute 32-bit userspace applications, say Y. >> + If you want to execute Aarch32 userspace applications, say Y. >> + >> +config ARM64_ILP32 >> + bool "Kernel support for ILP32" >> + help >> + This option enables support for AArch64 ILP32 user space. These are >> + 64-bit binaries using 32-bit quantities for addressing and certain >> + data that would normally be 64-bit. > > You could be even more precise by mentioning that longs and pointers are > 32-bit, together with the derived types. I copied this from the MIPS64 n32 description in Kconfig. But I will make it more explicit. > >> diff --git a/arch/arm64/include/asm/compat.h b/arch/arm64/include/asm/compat.h >> index 5ab2676..91bcf13 100644 >> --- a/arch/arm64/include/asm/compat.h >> +++ b/arch/arm64/include/asm/compat.h >> @@ -62,6 +62,8 @@ typedef u32 compat_ulong_t; >> typedef u64 compat_u64; >> typedef u32 compat_uptr_t; >> >> +typedef s64 ilp32_clock_t; >> + >> struct compat_timespec { >> compat_time_t tv_sec; >> s32 tv_nsec; >> @@ -180,6 +182,15 @@ typedef struct compat_siginfo { >> compat_clock_t _stime; >> } _sigchld; >> >> + /* SIGCHLD (ILP32 version) */ >> + struct { >> + compat_pid_t _pid; /* which child */ >> + __compat_uid32_t _uid; /* sender's uid */ >> + int _status; /* exit code */ >> + ilp32_clock_t _utime; >> + ilp32_clock_t _stime; >> + } _sigchld_ilp32; > > This *breaks* the compat_siginfo alignment. ilp32_clock_t is 64-bit > which forces the _sigchld_ilp32 to be 64-bit which makes the preamble 16 > instead of 12 bytes. This ilp32_clock_t needs > __attribute__((aligned(4))). I will check on this. > > The other approach I've been looking at is just using the native siginfo > instead of the compat one for ILP32. But this requires wider debate > (cc'ed Arnd if he has time). This is not useful and as you shown can be very messy and even worse when it comes taking into account big and little-endian. Even x32 does not do that. > > Basically if you use the current siginfo in the ILP32 context with > __kernel_clock_t being 64-bit you end up with a structure that doesn't > match any of the native or compat siginfo. This is because we have some > pointers which will turn into 32-bit values in ILP32: > > void __user *sival_ptr; /* accessed via si_ptr */ > void __user *_addr; /* accessed via si_addr */ > void __user *_call_addr; /* accessed via si_call_addr */ > > We also have __ARCH_SI_BAND_T defined as long. I had first thought about this and even started to implement it but I found the glibc and the kernel messier than it was already. > > AFAICT, Linux only does a put_user() on these and never reads them from > user space. This means that we can add the right padding on either side > of these pointers (for endianness reasons) and Linux would write 0 as > the top part of a 64-bit pointer (since the user address is restricted > to 32-bit anyway). User ILP32 would only access the corresponding > pointer as a 32-bit value and ignore the padding. And I am not a fan of changing the generic UAPI files just so it is no longer generic like you are doing. > > It's easier to explain with some code (untested): > > diff --git a/arch/arm64/include/uapi/asm/siginfo.h b/arch/arm64/include/uapi/asm/siginfo.h > index 5a74a08..e3848be 100644 > --- a/arch/arm64/include/uapi/asm/siginfo.h > +++ b/arch/arm64/include/uapi/asm/siginfo.h > @@ -18,6 +18,13 @@ > > #define __ARCH_SI_PREAMBLE_SIZE (4 * sizeof(int)) > > +#ifndef __LP64__ /* ILP32 */ > +#define __ARCH_SI_BAND_T long long > +#define VOID_USER_PTR(x) \ > + void __user *x __attribute__((aligned(8))); \ > + char _pad[4] > +#endif > + > #include <asm-generic/siginfo.h> > > #endif > diff --git a/include/uapi/asm-generic/siginfo.h b/include/uapi/asm-generic/siginfo.h > index ba5be7f..9c50257 100644 > --- a/include/uapi/asm-generic/siginfo.h > +++ b/include/uapi/asm-generic/siginfo.h > @@ -4,9 +4,13 @@ > #include <linux/compiler.h> > #include <linux/types.h> > > +#ifndef VOID_USER_PTR > +#define VOID_USER_PTR(x) void __user *x > +#endif > + > typedef union sigval { > int sival_int; > - void __user *sival_ptr; > + VOID_USER_PTR(sival_ptr); > } sigval_t; > > /* > @@ -86,7 +90,7 @@ typedef struct siginfo { > > /* SIGILL, SIGFPE, SIGSEGV, SIGBUS */ > struct { > - void __user *_addr; /* faulting insn/memory ref. */ > + VOID_USER_PTR(_addr); /* faulting insn/memory ref. */ > #ifdef __ARCH_SI_TRAPNO > int _trapno; /* TRAP # which caused the signal */ > #endif > @@ -101,7 +105,7 @@ typedef struct siginfo { > > /* SIGSYS */ > struct { > - void __user *_call_addr; /* calling user insn */ > + VOID_USER_PTR(_call_addr); /* calling user insn */ > int _syscall; /* triggering system call number */ > unsigned int _arch; /* AUDIT_ARCH_* of syscall */ > } _sigsys; > > > __LP64__ is always defined for AArch64 (kernel and native applications). > ILP32 user would not get this symbol and compat use a separate > compat_siginfo anyway. > > I'm not entirely sure about defining __ARCH_SI_BAND_T to long long but > as it also seems to be just written by the kernel, we can use some > padding as for the void __user *. > > > So, I'm looking for feedback on this proposal. As I mentioned before even x32 does not do that and it is very messy to make sure things get zero'd on the glibc and kernel sides. > >> diff --git a/arch/arm64/include/asm/stat.h b/arch/arm64/include/asm/stat.h >> index 989128a..f2e0d3c 100644 >> --- a/arch/arm64/include/asm/stat.h >> +++ b/arch/arm64/include/asm/stat.h >> @@ -18,9 +18,11 @@ >> >> #include <uapi/asm/stat.h> >> >> -#ifdef CONFIG_ARM64_AARCH32 >> - >> +#ifdef CONFIG_COMPAT >> #include <asm/compat.h> >> +#endif > > Doesn't asm/compat.h have guards already? Do you get a conflict with > is_compat_task()? The guard was there already, I am just changing it back to be under CONFIG_COMPAT from when I added CONFIG_ARM64_AARCH32. When I submit the next version of the set of patches, this change will be done correctly in the patch which adds CONFIG_ARM64_AARCH32. > >> diff --git a/arch/arm64/include/uapi/asm/siginfo.h b/arch/arm64/include/uapi/asm/siginfo.h >> index 5a74a08..297fb4f 100644 >> --- a/arch/arm64/include/uapi/asm/siginfo.h >> +++ b/arch/arm64/include/uapi/asm/siginfo.h >> @@ -16,7 +16,13 @@ >> #ifndef __ASM_SIGINFO_H >> #define __ASM_SIGINFO_H >> >> +#ifdef __LP64__ >> #define __ARCH_SI_PREAMBLE_SIZE (4 * sizeof(int)) >> +#else /* ILP32 */ >> +typedef long long __kernel_si_clock_t __attribute__((aligned(4))); >> +#define __ARCH_SI_CLOCK_T __kernel_si_clock_t >> +#define __ARCH_SI_ATTRIBUTES __attribute__((aligned(8))) >> +#endif > > This could go away if we manage to use the native siginfo. See above why I think this is a bad thing and even worse since even x32 did not do that already; it was the last added ABI like ILP32 to the kernel. > >> --- /dev/null >> +++ b/arch/arm64/kernel/signalilp32.c >> @@ -0,0 +1,30 @@ >> + >> +#ifdef CONFIG_ARM64_ILP32 >> + >> +#define rt_sigframe rt_sigframe_ilp32 > > Can we have the same native rt_sigframe (if we go for the native > siginfo)? ucontext is an arm64 structure, so we can add padding for > pointers and long. No again this is messy due to the zeroing of the values. As I mentioned, I started to implement that but it got messy and there was no right way of having the headers be sane. > >> --- /dev/null >> +++ b/arch/arm64/kernel/sys_ilp32.c >> @@ -0,0 +1,274 @@ >> +/* >> + * AArch64- ILP32 specific system calls implementation >> + * >> + * Copyright (C) 2013 Cavium Inc. >> + * Author: Andrew Pinski <apinski@cavium.com> >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License version 2 as >> + * published by the Free Software Foundation. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + * >> + * You should have received a copy of the GNU General Public License >> + * along with this program. If not, see <http://www.gnu.org/licenses/>. >> + */ >> + >> +/* Adjust unistd.h to provide 32-bit numbers and functions. */ >> +#define __SYSCALL_COMPAT > > No. We need to use as many native syscalls as possible and only define > those absolutely necessary. In my investigation, I only ended up needing > these: No using __SYSCALL_COMPAT is the correct thing to do and then only reverting back what is needed. The main reason is due to using the generic support inside glibc already. > > #define sys_ioctl compat_sys_ioctl > #define sys_readv compat_sys_readv > #define sys_writev compat_sys_writev > #define sys_preadv compat_sys_preadv64 > #define sys_pwritev compat_sys_pwritev64 > #define sys_vmsplice compat_sys_vmsplice > #define sys_waitid compat_sys_waitid > #define sys_set_robust_list compat_sys_set_robust_list > #define sys_get_robust_list compat_sys_get_robust_list > #define sys_kexec_load compat_sys_kexec_load > #define sys_timer_create compat_sys_timer_create > #define sys_ptrace compat_sys_ptrace > #define sys_sigaltstack compat_sys_sigaltstack > #define sys_rt_sigaction compat_sys_rt_sigaction > #define sys_rt_sigpending compat_sys_rt_sigpending > #define sys_rt_sigtimedwait compat_sys_rt_sigtimedwait > #define sys_rt_sigqueueinfo compat_sys_rt_sigqueueinfo > #define sys_rt_sigreturn compat_sys_rt_sigreturn_wrapper > #define sys_mq_notify compat_sys_mq_notify > #define sys_recvfrom compat_sys_recvfrom > #define sys_setsockopt compat_sys_setsockopt > #define sys_getsockopt compat_sys_getsockopt > #define sys_sendmsg compat_sys_sendmsg > #define sys_recvmsg compat_sys_recvmsg > #define sys_execve compat_sys_execve > #define sys_move_pages compat_sys_move_pages > #define sys_rt_tgsigqueueinfo compat_sys_rt_tgsigqueueinfo > #define sys_recvmmsg compat_sys_recvmmsg > #define sys_sendmmsg compat_sys_sendmmsg > #define sys_process_vm_readv compat_sys_process_vm_readv > #define sys_process_vm_writev compat_sys_process_vm_writev You even forgot compat_sys_openat (where O_LARGEFILE differences does make a difference). > >> diff --git a/arch/arm64/kernel/vdsoilp32/Makefile b/arch/arm64/kernel/vdsoilp32/Makefile >> new file mode 100644 >> index 0000000..ec93f3f >> --- /dev/null >> +++ b/arch/arm64/kernel/vdsoilp32/Makefile > > Could we not keep vdso in the same directory? I started out that way but "make clean ARCH=arm64" did not clean the vdso files all the time. > >> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c >> index 67e8d7c..17b9c39 100644 >> --- a/arch/arm64/mm/init.c >> +++ b/arch/arm64/mm/init.c >> @@ -35,6 +35,7 @@ >> #include <asm/sections.h> >> #include <asm/setup.h> >> #include <asm/sizes.h> >> +#include <asm/compat.h> >> #include <asm/tlb.h> > > Same issue, other header files doesn't include what is necessary. I will double check these but I think it was a fall out due to my change that I had originally did for asm/stat.h . Thanks, Andrew Pinski
On Fri, Sep 13, 2013 at 07:18:48AM +0100, Andrew Pinski wrote: > On Wed, Sep 11, 2013 at 7:32 AM, Catalin Marinas > <catalin.marinas@arm.com> wrote: > > On Mon, Sep 09, 2013 at 10:32:59PM +0100, Andrew Pinski wrote: > >> This patch adds full support of the ABI to the ARM64 target. > > > > This description is too short. Please describe what the ABI is, what are > > the commonalities with AArch64 and AArch32, what other non-obvious > > things had to be done (like __kernel_long_t being long long). Split this > > patch into multiple patches like base syscall handling, signal handling, > > pselect6/ppoll, vdso etc. It's too much to review at once. > > Ok. I will do so after my vacation next week. > > > > > On top of these, I would really like to see > > Documentation/arm64/ilp32.txt describing the ABI. > > No other target does not, not even x86_64 for x32. Well, I'm sure they wouldn't mind if you submitted documentation for them too. > > I would also like to know (you can state this in the cover letter) the > > level of testing for all 3 types of ABI. I'm worried that at least this > > patch breaks the current compat ABI (has LTP reported anything?). > > We did test LTP on an earlier version of this patch for all three > ABIs, I will make sure that the next version I send out is tested on > all three ABIs also. We also tested ILP32/LP64 on big-endian at the > same time which we will continue to do (I should push for our team > here to push out the big-endian patches). We also have some BE patches internally, but obviously they just target LP64 and AArch32 compat. I'd hope to get these out shortly (the current issue is extensive testing, since we don't have much of a BE userspace). > >> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > >> index cc64df5..7fdc994 100644 > >> --- a/arch/arm64/Kconfig > >> +++ b/arch/arm64/Kconfig > >> @@ -248,7 +248,7 @@ source "fs/Kconfig.binfmt" > >> > >> config COMPAT > >> def_bool y > >> - depends on ARM64_AARCH32 > >> + depends on ARM64_AARCH32 || ARM64_ILP32 > >> select COMPAT_BINFMT_ELF > >> > >> config ARM64_AARCH32 (nitpick) We used to have an option like this, called CONFIG_AARCH32_EMULATION, which I think is clearer than CONFIG_ARM64_AARCH32. > >> diff --git a/arch/arm64/kernel/vdsoilp32/Makefile b/arch/arm64/kernel/vdsoilp32/Makefile > >> new file mode 100644 > >> index 0000000..ec93f3f > >> --- /dev/null > >> +++ b/arch/arm64/kernel/vdsoilp32/Makefile > > > > Could we not keep vdso in the same directory? > > I started out that way but "make clean ARCH=arm64" did not clean the > vdso files all the time. Can you elaborate please? I'd much rather we fix broken make rules instead of botching around the issue by creating new directories. Will
On Fri, Sep 13, 2013 at 10:47:12AM +0100, Will Deacon wrote: > On Fri, Sep 13, 2013 at 07:18:48AM +0100, Andrew Pinski wrote: > > On Wed, Sep 11, 2013 at 7:32 AM, Catalin Marinas > > <catalin.marinas@arm.com> wrote: > > > On Mon, Sep 09, 2013 at 10:32:59PM +0100, Andrew Pinski wrote: > > >> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > > >> index cc64df5..7fdc994 100644 > > >> --- a/arch/arm64/Kconfig > > >> +++ b/arch/arm64/Kconfig > > >> @@ -248,7 +248,7 @@ source "fs/Kconfig.binfmt" > > >> > > >> config COMPAT > > >> def_bool y > > >> - depends on ARM64_AARCH32 > > >> + depends on ARM64_AARCH32 || ARM64_ILP32 > > >> select COMPAT_BINFMT_ELF > > >> > > >> config ARM64_AARCH32 > > (nitpick) We used to have an option like this, called > CONFIG_AARCH32_EMULATION, which I think is clearer than CONFIG_ARM64_AARCH32. I think avoiding "EMULATION" is better, we don't actually emulate the instruction set ;).
On Fri, Sep 13, 2013 at 10:57:40AM +0100, Catalin Marinas wrote: > On Fri, Sep 13, 2013 at 10:47:12AM +0100, Will Deacon wrote: > > On Fri, Sep 13, 2013 at 07:18:48AM +0100, Andrew Pinski wrote: > > > On Wed, Sep 11, 2013 at 7:32 AM, Catalin Marinas > > > <catalin.marinas@arm.com> wrote: > > > > On Mon, Sep 09, 2013 at 10:32:59PM +0100, Andrew Pinski wrote: > > > >> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > > > >> index cc64df5..7fdc994 100644 > > > >> --- a/arch/arm64/Kconfig > > > >> +++ b/arch/arm64/Kconfig > > > >> @@ -248,7 +248,7 @@ source "fs/Kconfig.binfmt" > > > >> > > > >> config COMPAT > > > >> def_bool y > > > >> - depends on ARM64_AARCH32 > > > >> + depends on ARM64_AARCH32 || ARM64_ILP32 > > > >> select COMPAT_BINFMT_ELF > > > >> > > > >> config ARM64_AARCH32 > > > > (nitpick) We used to have an option like this, called > > CONFIG_AARCH32_EMULATION, which I think is clearer than CONFIG_ARM64_AARCH32. > > I think avoiding "EMULATION" is better, we don't actually emulate the > instruction set ;). Bah, you suggest something better then! Will
On Fri, Sep 13, 2013 at 07:18:48AM +0100, Andrew Pinski wrote: > On Wed, Sep 11, 2013 at 7:32 AM, Catalin Marinas > <catalin.marinas@arm.com> wrote: > > On Mon, Sep 09, 2013 at 10:32:59PM +0100, Andrew Pinski wrote: > > > > On top of these, I would really like to see > > Documentation/arm64/ilp32.txt describing the ABI. > > No other target does not, not even x86_64 for x32. That's not really a good argument. > > The other approach I've been looking at is just using the native siginfo > > instead of the compat one for ILP32. But this requires wider debate > > (cc'ed Arnd if he has time). > > This is not useful and as you shown can be very messy and even worse > when it comes taking into account big and little-endian. Even x32 > does not do that. Well, please don't bring the "x32 does not do that" argument. It doesn't mean we shouldn't investigate better ways. Initially x32 got the siginfo members alignment wrong and they ended up __ARCH_SI_CLOCK_T and __ARCH_SI_ATTRIBUTES, changing the generic uapi files. > > Basically if you use the current siginfo in the ILP32 context with > > __kernel_clock_t being 64-bit you end up with a structure that doesn't > > match any of the native or compat siginfo. This is because we have some > > pointers which will turn into 32-bit values in ILP32: > > > > void __user *sival_ptr; /* accessed via si_ptr */ > > void __user *_addr; /* accessed via si_addr */ > > void __user *_call_addr; /* accessed via si_call_addr */ > > > > We also have __ARCH_SI_BAND_T defined as long. > > I had first thought about this and even started to implement it but I > found the glibc and the kernel messier than it was already. The kernel part wasn't bad IMO (of course, needs ack from generic headers maintainer). I can't talk about glibc but wouldn't it just access these members explicitly? > > AFAICT, Linux only does a put_user() on these and never reads them from > > user space. This means that we can add the right padding on either side > > of these pointers (for endianness reasons) and Linux would write 0 as > > the top part of a 64-bit pointer (since the user address is restricted > > to 32-bit anyway). User ILP32 would only access the corresponding > > pointer as a 32-bit value and ignore the padding. > > And I am not a fan of changing the generic UAPI files just so it is no > longer generic like you are doing. As I said above, x32 did that already and your are doing similar things for __ARCH_SI_CLOCK_T. > > So, I'm looking for feedback on this proposal. > > As I mentioned before even x32 does not do that and it is very messy > to make sure things get zero'd on the glibc and kernel sides. (not the x32 argument again) On the kernel side, they get zeroed automatically because the kernel assumes it is a 64-bit address for user space, which is restricted to 32-bit only. Are these members ever read back by the kernel? That's where glibc zeroing would be needed (and I wouldn't like it either). > >> diff --git a/arch/arm64/include/uapi/asm/siginfo.h b/arch/arm64/include/uapi/asm/siginfo.h > >> index 5a74a08..297fb4f 100644 > >> --- a/arch/arm64/include/uapi/asm/siginfo.h > >> +++ b/arch/arm64/include/uapi/asm/siginfo.h > >> @@ -16,7 +16,13 @@ > >> #ifndef __ASM_SIGINFO_H > >> #define __ASM_SIGINFO_H > >> > >> +#ifdef __LP64__ > >> #define __ARCH_SI_PREAMBLE_SIZE (4 * sizeof(int)) > >> +#else /* ILP32 */ > >> +typedef long long __kernel_si_clock_t __attribute__((aligned(4))); > >> +#define __ARCH_SI_CLOCK_T __kernel_si_clock_t > >> +#define __ARCH_SI_ATTRIBUTES __attribute__((aligned(8))) > >> +#endif > > > > This could go away if we manage to use the native siginfo. > > See above why I think this is a bad thing and even worse since even > x32 did not do that already; it was the last added ABI like ILP32 to > the kernel. The x32 thing is becoming the central theme. > >> --- /dev/null > >> +++ b/arch/arm64/kernel/sys_ilp32.c > >> @@ -0,0 +1,274 @@ > >> +/* > >> + * AArch64- ILP32 specific system calls implementation > >> + * > >> + * Copyright (C) 2013 Cavium Inc. > >> + * Author: Andrew Pinski <apinski@cavium.com> > >> + * > >> + * This program is free software; you can redistribute it and/or modify > >> + * it under the terms of the GNU General Public License version 2 as > >> + * published by the Free Software Foundation. > >> + * > >> + * This program is distributed in the hope that it will be useful, > >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of > >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > >> + * GNU General Public License for more details. > >> + * > >> + * You should have received a copy of the GNU General Public License > >> + * along with this program. If not, see <http://www.gnu.org/licenses/>. > >> + */ > >> + > >> +/* Adjust unistd.h to provide 32-bit numbers and functions. */ > >> +#define __SYSCALL_COMPAT > > > > No. We need to use as many native syscalls as possible and only define > > those absolutely necessary. In my investigation, I only ended up needing > > these: > > No using __SYSCALL_COMPAT is the correct thing to do and then only > reverting back what is needed. I _disagree_. "Even x32 does not to that". Some past discussions: http://thread.gmane.org/gmane.linux.kernel/1184913 > > #define sys_ioctl compat_sys_ioctl > > #define sys_readv compat_sys_readv > > #define sys_writev compat_sys_writev > > #define sys_preadv compat_sys_preadv64 > > #define sys_pwritev compat_sys_pwritev64 > > #define sys_vmsplice compat_sys_vmsplice > > #define sys_waitid compat_sys_waitid > > #define sys_set_robust_list compat_sys_set_robust_list > > #define sys_get_robust_list compat_sys_get_robust_list > > #define sys_kexec_load compat_sys_kexec_load > > #define sys_timer_create compat_sys_timer_create > > #define sys_ptrace compat_sys_ptrace > > #define sys_sigaltstack compat_sys_sigaltstack > > #define sys_rt_sigaction compat_sys_rt_sigaction > > #define sys_rt_sigpending compat_sys_rt_sigpending > > #define sys_rt_sigtimedwait compat_sys_rt_sigtimedwait > > #define sys_rt_sigqueueinfo compat_sys_rt_sigqueueinfo > > #define sys_rt_sigreturn compat_sys_rt_sigreturn_wrapper > > #define sys_mq_notify compat_sys_mq_notify > > #define sys_recvfrom compat_sys_recvfrom > > #define sys_setsockopt compat_sys_setsockopt > > #define sys_getsockopt compat_sys_getsockopt > > #define sys_sendmsg compat_sys_sendmsg > > #define sys_recvmsg compat_sys_recvmsg > > #define sys_execve compat_sys_execve > > #define sys_move_pages compat_sys_move_pages > > #define sys_rt_tgsigqueueinfo compat_sys_rt_tgsigqueueinfo > > #define sys_recvmmsg compat_sys_recvmmsg > > #define sys_sendmmsg compat_sys_sendmmsg > > #define sys_process_vm_readv compat_sys_process_vm_readv > > #define sys_process_vm_writev compat_sys_process_vm_writev > > You even forgot compat_sys_openat (where O_LARGEFILE differences does > make a difference). Just read the above thread. "Even x32 does not do that". > >> diff --git a/arch/arm64/kernel/vdsoilp32/Makefile b/arch/arm64/kernel/vdsoilp32/Makefile > >> new file mode 100644 > >> index 0000000..ec93f3f > >> --- /dev/null > >> +++ b/arch/arm64/kernel/vdsoilp32/Makefile > > > > Could we not keep vdso in the same directory? > > I started out that way but "make clean ARCH=arm64" did not clean the > vdso files all the time. I'll leave vdso comments to Will.
On Fri, Sep 13, 2013 at 11:04:53AM +0100, Will Deacon wrote: > On Fri, Sep 13, 2013 at 10:57:40AM +0100, Catalin Marinas wrote: > > On Fri, Sep 13, 2013 at 10:47:12AM +0100, Will Deacon wrote: > > > On Fri, Sep 13, 2013 at 07:18:48AM +0100, Andrew Pinski wrote: > > > > On Wed, Sep 11, 2013 at 7:32 AM, Catalin Marinas > > > > <catalin.marinas@arm.com> wrote: > > > > > On Mon, Sep 09, 2013 at 10:32:59PM +0100, Andrew Pinski wrote: > > > > >> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > > > > >> index cc64df5..7fdc994 100644 > > > > >> --- a/arch/arm64/Kconfig > > > > >> +++ b/arch/arm64/Kconfig > > > > >> @@ -248,7 +248,7 @@ source "fs/Kconfig.binfmt" > > > > >> > > > > >> config COMPAT > > > > >> def_bool y > > > > >> - depends on ARM64_AARCH32 > > > > >> + depends on ARM64_AARCH32 || ARM64_ILP32 > > > > >> select COMPAT_BINFMT_ELF > > > > >> > > > > >> config ARM64_AARCH32 > > > > > > (nitpick) We used to have an option like this, called > > > CONFIG_AARCH32_EMULATION, which I think is clearer than CONFIG_ARM64_AARCH32. > > > > I think avoiding "EMULATION" is better, we don't actually emulate the > > instruction set ;). > > Bah, you suggest something better then! CONFIG_AARCH32_EL0.
On Wed, Sep 11, 2013 at 7:32 AM, Catalin Marinas <catalin.marinas@arm.com> wrote: > On Mon, Sep 09, 2013 at 10:32:59PM +0100, Andrew Pinski wrote: >> This patch adds full support of the ABI to the ARM64 target. > > This description is too short. Please describe what the ABI is, what are > the commonalities with AArch64 and AArch32, what other non-obvious > things had to be done (like __kernel_long_t being long long). Split this > patch into multiple patches like base syscall handling, signal handling, > pselect6/ppoll, vdso etc. It's too much to review at once. I will try to do this. > > On top of these, I would really like to see > Documentation/arm64/ilp32.txt describing the ABI. Ok, I will add some documentation. > > I would also like to know (you can state this in the cover letter) the > level of testing for all 3 types of ABI. I'm worried that at least this > patch breaks the current compat ABI (has LTP reported anything?). When I submit the next version, I will cover this in my cover letter. > >> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig >> index cc64df5..7fdc994 100644 >> --- a/arch/arm64/Kconfig >> +++ b/arch/arm64/Kconfig >> @@ -248,7 +248,7 @@ source "fs/Kconfig.binfmt" >> >> config COMPAT >> def_bool y >> - depends on ARM64_AARCH32 >> + depends on ARM64_AARCH32 || ARM64_ILP32 >> select COMPAT_BINFMT_ELF >> >> config ARM64_AARCH32 >> @@ -263,7 +263,14 @@ config ARM64_AARCH32 >> the user helper functions, VFP support and the ptrace interface are >> handled appropriately by the kernel. >> >> - If you want to execute 32-bit userspace applications, say Y. >> + If you want to execute Aarch32 userspace applications, say Y. >> + >> +config ARM64_ILP32 >> + bool "Kernel support for ILP32" >> + help >> + This option enables support for AArch64 ILP32 user space. These are >> + 64-bit binaries using 32-bit quantities for addressing and certain >> + data that would normally be 64-bit. > > You could be even more precise by mentioning that longs and pointers are > 32-bit, together with the derived types. Will fix this as mentioned before. > >> diff --git a/arch/arm64/include/asm/compat.h b/arch/arm64/include/asm/compat.h >> index 5ab2676..91bcf13 100644 >> --- a/arch/arm64/include/asm/compat.h >> +++ b/arch/arm64/include/asm/compat.h >> @@ -62,6 +62,8 @@ typedef u32 compat_ulong_t; >> typedef u64 compat_u64; >> typedef u32 compat_uptr_t; >> >> +typedef s64 ilp32_clock_t; >> + >> struct compat_timespec { >> compat_time_t tv_sec; >> s32 tv_nsec; >> @@ -180,6 +182,15 @@ typedef struct compat_siginfo { >> compat_clock_t _stime; >> } _sigchld; >> >> + /* SIGCHLD (ILP32 version) */ >> + struct { >> + compat_pid_t _pid; /* which child */ >> + __compat_uid32_t _uid; /* sender's uid */ >> + int _status; /* exit code */ >> + ilp32_clock_t _utime; >> + ilp32_clock_t _stime; >> + } _sigchld_ilp32; > > This *breaks* the compat_siginfo alignment. ilp32_clock_t is 64-bit > which forces the _sigchld_ilp32 to be 64-bit which makes the preamble 16 > instead of 12 bytes. This ilp32_clock_t needs > __attribute__((aligned(4))). Will fix this. > > The other approach I've been looking at is just using the native siginfo > instead of the compat one for ILP32. But this requires wider debate > (cc'ed Arnd if he has time). I will try to do this though it will take a week to get how much I can do with siginfo as the glibc side can be a pain to work with. The kernel is cleaner here since it is always 64bit. > > Basically if you use the current siginfo in the ILP32 context with > __kernel_clock_t being 64-bit you end up with a structure that doesn't > match any of the native or compat siginfo. This is because we have some > pointers which will turn into 32-bit values in ILP32: > > void __user *sival_ptr; /* accessed via si_ptr */ > void __user *_addr; /* accessed via si_addr */ > void __user *_call_addr; /* accessed via si_call_addr */ > > We also have __ARCH_SI_BAND_T defined as long. This has to stay long as POSIX defines it as a long type as mentioned in the header file. I will use an anonymous union/struct to get the correct definition for ilp32. I have to also fix the ptrace interface after I do this. > > AFAICT, Linux only does a put_user() on these and never reads them from > user space. This means that we can add the right padding on either side > of these pointers (for endianness reasons) and Linux would write 0 as > the top part of a 64-bit pointer (since the user address is restricted > to 32-bit anyway). User ILP32 would only access the corresponding > pointer as a 32-bit value and ignore the padding. > > It's easier to explain with some code (untested): > > diff --git a/arch/arm64/include/uapi/asm/siginfo.h b/arch/arm64/include/uapi/asm/siginfo.h > index 5a74a08..e3848be 100644 > --- a/arch/arm64/include/uapi/asm/siginfo.h > +++ b/arch/arm64/include/uapi/asm/siginfo.h > @@ -18,6 +18,13 @@ > > #define __ARCH_SI_PREAMBLE_SIZE (4 * sizeof(int)) > > +#ifndef __LP64__ /* ILP32 */ > +#define __ARCH_SI_BAND_T long long > +#define VOID_USER_PTR(x) \ > + void __user *x __attribute__((aligned(8))); \ > + char _pad[4] > +#endif > + > #include <asm-generic/siginfo.h> > > #endif > diff --git a/include/uapi/asm-generic/siginfo.h b/include/uapi/asm-generic/siginfo.h > index ba5be7f..9c50257 100644 > --- a/include/uapi/asm-generic/siginfo.h > +++ b/include/uapi/asm-generic/siginfo.h > @@ -4,9 +4,13 @@ > #include <linux/compiler.h> > #include <linux/types.h> > > +#ifndef VOID_USER_PTR > +#define VOID_USER_PTR(x) void __user *x > +#endif > + > typedef union sigval { > int sival_int; > - void __user *sival_ptr; > + VOID_USER_PTR(sival_ptr); > } sigval_t; > > /* > @@ -86,7 +90,7 @@ typedef struct siginfo { > > /* SIGILL, SIGFPE, SIGSEGV, SIGBUS */ > struct { > - void __user *_addr; /* faulting insn/memory ref. */ > + VOID_USER_PTR(_addr); /* faulting insn/memory ref. */ > #ifdef __ARCH_SI_TRAPNO > int _trapno; /* TRAP # which caused the signal */ > #endif > @@ -101,7 +105,7 @@ typedef struct siginfo { > > /* SIGSYS */ > struct { > - void __user *_call_addr; /* calling user insn */ > + VOID_USER_PTR(_call_addr); /* calling user insn */ > int _syscall; /* triggering system call number */ > unsigned int _arch; /* AUDIT_ARCH_* of syscall */ > } _sigsys; > > > __LP64__ is always defined for AArch64 (kernel and native applications). > ILP32 user would not get this symbol and compat use a separate > compat_siginfo anyway. > > I'm not entirely sure about defining __ARCH_SI_BAND_T to long long but > as it also seems to be just written by the kernel, we can use some > padding as for the void __user *. POSIX defines it as long as mentioned above. I don't think we want ILP32 to violate POSIX here if we can not help it. > > > So, I'm looking for feedback on this proposal. > >> diff --git a/arch/arm64/include/asm/stat.h b/arch/arm64/include/asm/stat.h >> index 989128a..f2e0d3c 100644 >> --- a/arch/arm64/include/asm/stat.h >> +++ b/arch/arm64/include/asm/stat.h >> @@ -18,9 +18,11 @@ >> >> #include <uapi/asm/stat.h> >> >> -#ifdef CONFIG_ARM64_AARCH32 >> - >> +#ifdef CONFIG_COMPAT >> #include <asm/compat.h> >> +#endif > > Doesn't asm/compat.h have guards already? Do you get a conflict with > is_compat_task()? This was due to the order of the patches were applied and I did not notice the breakage until after I finished the last one. I will fix this so it is correctly done before I submit the next version of the patches. > >> diff --git a/arch/arm64/include/uapi/asm/siginfo.h b/arch/arm64/include/uapi/asm/siginfo.h >> index 5a74a08..297fb4f 100644 >> --- a/arch/arm64/include/uapi/asm/siginfo.h >> +++ b/arch/arm64/include/uapi/asm/siginfo.h >> @@ -16,7 +16,13 @@ >> #ifndef __ASM_SIGINFO_H >> #define __ASM_SIGINFO_H >> >> +#ifdef __LP64__ >> #define __ARCH_SI_PREAMBLE_SIZE (4 * sizeof(int)) >> +#else /* ILP32 */ >> +typedef long long __kernel_si_clock_t __attribute__((aligned(4))); >> +#define __ARCH_SI_CLOCK_T __kernel_si_clock_t >> +#define __ARCH_SI_ATTRIBUTES __attribute__((aligned(8))) >> +#endif > > This could go away if we manage to use the native siginfo. Yes I agree. > >> --- /dev/null >> +++ b/arch/arm64/kernel/signalilp32.c >> @@ -0,0 +1,30 @@ >> + >> +#ifdef CONFIG_ARM64_ILP32 >> + >> +#define rt_sigframe rt_sigframe_ilp32 > > Can we have the same native rt_sigframe (if we go for the native > siginfo)? ucontext is an arm64 structure, so we can add padding for > pointers and long. This one is a little harder at least in glibc itself. I tried a few times but I always failed due to not having that much experience with the code there but I will try one more time. > >> --- /dev/null >> +++ b/arch/arm64/kernel/sys_ilp32.c >> @@ -0,0 +1,274 @@ >> +/* >> + * AArch64- ILP32 specific system calls implementation >> + * >> + * Copyright (C) 2013 Cavium Inc. >> + * Author: Andrew Pinski <apinski@cavium.com> >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License version 2 as >> + * published by the Free Software Foundation. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + * >> + * You should have received a copy of the GNU General Public License >> + * along with this program. If not, see <http://www.gnu.org/licenses/>. >> + */ >> + >> +/* Adjust unistd.h to provide 32-bit numbers and functions. */ >> +#define __SYSCALL_COMPAT > > No. We need to use as many native syscalls as possible and only define > those absolutely necessary. In my investigation, I only ended up needing > these: > > #define sys_ioctl compat_sys_ioctl > #define sys_readv compat_sys_readv > #define sys_writev compat_sys_writev > #define sys_preadv compat_sys_preadv64 > #define sys_pwritev compat_sys_pwritev64 > #define sys_vmsplice compat_sys_vmsplice > #define sys_waitid compat_sys_waitid > #define sys_set_robust_list compat_sys_set_robust_list > #define sys_get_robust_list compat_sys_get_robust_list > #define sys_kexec_load compat_sys_kexec_load > #define sys_timer_create compat_sys_timer_create > #define sys_ptrace compat_sys_ptrace > #define sys_sigaltstack compat_sys_sigaltstack > #define sys_rt_sigaction compat_sys_rt_sigaction > #define sys_rt_sigpending compat_sys_rt_sigpending > #define sys_rt_sigtimedwait compat_sys_rt_sigtimedwait > #define sys_rt_sigqueueinfo compat_sys_rt_sigqueueinfo > #define sys_rt_sigreturn compat_sys_rt_sigreturn_wrapper > #define sys_mq_notify compat_sys_mq_notify > #define sys_recvfrom compat_sys_recvfrom > #define sys_setsockopt compat_sys_setsockopt > #define sys_getsockopt compat_sys_getsockopt > #define sys_sendmsg compat_sys_sendmsg > #define sys_recvmsg compat_sys_recvmsg > #define sys_execve compat_sys_execve > #define sys_move_pages compat_sys_move_pages > #define sys_rt_tgsigqueueinfo compat_sys_rt_tgsigqueueinfo > #define sys_recvmmsg compat_sys_recvmmsg > #define sys_sendmmsg compat_sys_sendmmsg > #define sys_process_vm_readv compat_sys_process_vm_readv > #define sys_process_vm_writev compat_sys_process_vm_writev Here is some additional ones I can up with after understanding this code better:\ /* Pointer in struct */ #define sys_mount compat_sys_mount /* NUMA */ /* unsigned long bitmaps */ #define sys_get_mempolicy compat_sys_get_mempolicy #define sys_set_mempolicy compat_sys_set_mempolicy #define sys_mbind compat_sys_mbind /* array of pointers */ /* unsigned long bitmaps */ #define sys_migrate_pages compat_sys_migrate_pages /* Scheduler */ /* unsigned long bitmaps */ #define sys_sched_setaffinity compat_sys_sched_setaffinity #define sys_sched_getaffinity compat_sys_sched_getaffinity /* iov usage */ #define sys_keyctl compat_sys_keyctl /* aio */ /* Pointer to Pointer */ #define sys_io_setup compat_sys_io_setup /* Array of pointers */ #define sys_io_submit compat_sys_io_submit --- CUT ---- I added a couple of comments on why the ones listed above are needed. Also I don't think ilp32 needs its own sys_ptrace rather than using the compat one. > >> diff --git a/arch/arm64/kernel/vdsoilp32/Makefile b/arch/arm64/kernel/vdsoilp32/Makefile >> new file mode 100644 >> index 0000000..ec93f3f >> --- /dev/null >> +++ b/arch/arm64/kernel/vdsoilp32/Makefile > > Could we not keep vdso in the same directory? > >> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c >> index 67e8d7c..17b9c39 100644 >> --- a/arch/arm64/mm/init.c >> +++ b/arch/arm64/mm/init.c >> @@ -35,6 +35,7 @@ >> #include <asm/sections.h> >> #include <asm/setup.h> >> #include <asm/sizes.h> >> +#include <asm/compat.h> >> #include <asm/tlb.h> > > Same issue, other header files doesn't include what is necessary. > > -- > Catalin
On Fri, Sep 13, 2013 at 5:12 AM, Catalin Marinas <catalin.marinas@arm.com> wrote: > On Fri, Sep 13, 2013 at 07:18:48AM +0100, Andrew Pinski wrote: >> On Wed, Sep 11, 2013 at 7:32 AM, Catalin Marinas >> <catalin.marinas@arm.com> wrote: >> > On Mon, Sep 09, 2013 at 10:32:59PM +0100, Andrew Pinski wrote: >> > >> > On top of these, I would really like to see >> > Documentation/arm64/ilp32.txt describing the ABI. >> >> No other target does not, not even x86_64 for x32. > > That's not really a good argument. > >> > The other approach I've been looking at is just using the native siginfo >> > instead of the compat one for ILP32. But this requires wider debate >> > (cc'ed Arnd if he has time). >> >> This is not useful and as you shown can be very messy and even worse >> when it comes taking into account big and little-endian. Even x32 >> does not do that. > > Well, please don't bring the "x32 does not do that" argument. It doesn't > mean we shouldn't investigate better ways. Initially x32 got the siginfo > members alignment wrong and they ended up __ARCH_SI_CLOCK_T and > __ARCH_SI_ATTRIBUTES, changing the generic uapi files. > >> > Basically if you use the current siginfo in the ILP32 context with >> > __kernel_clock_t being 64-bit you end up with a structure that doesn't >> > match any of the native or compat siginfo. This is because we have some >> > pointers which will turn into 32-bit values in ILP32: >> > >> > void __user *sival_ptr; /* accessed via si_ptr */ >> > void __user *_addr; /* accessed via si_addr */ >> > void __user *_call_addr; /* accessed via si_call_addr */ >> > >> > We also have __ARCH_SI_BAND_T defined as long. >> >> I had first thought about this and even started to implement it but I >> found the glibc and the kernel messier than it was already. > > The kernel part wasn't bad IMO (of course, needs ack from generic > headers maintainer). I can't talk about glibc but wouldn't it just > access these members explicitly? > >> > AFAICT, Linux only does a put_user() on these and never reads them from >> > user space. This means that we can add the right padding on either side >> > of these pointers (for endianness reasons) and Linux would write 0 as >> > the top part of a 64-bit pointer (since the user address is restricted >> > to 32-bit anyway). User ILP32 would only access the corresponding >> > pointer as a 32-bit value and ignore the padding. This is not true for sigevent which is defined to include "union sigval" by the POSIX standard. sigval is also included in siginfo so it needs to be using 64bit pointers here. The padding issue does come into play for some syscalls (mq_notify and timer_create). They both would need a special wrapper syscall to correctly zero fill the upper bits of that union. Would it be ok for those two system calls to have a wapper? The rest is correct we only write to those fields and never read from them. Thanks, Andrew Pinski >> >> And I am not a fan of changing the generic UAPI files just so it is no >> longer generic like you are doing. > > As I said above, x32 did that already and your are doing similar things > for __ARCH_SI_CLOCK_T. > >> > So, I'm looking for feedback on this proposal. >> >> As I mentioned before even x32 does not do that and it is very messy >> to make sure things get zero'd on the glibc and kernel sides. > > (not the x32 argument again) > > On the kernel side, they get zeroed automatically because the kernel > assumes it is a 64-bit address for user space, which is restricted to > 32-bit only. Are these members ever read back by the kernel? That's > where glibc zeroing would be needed (and I wouldn't like it either). > >> >> diff --git a/arch/arm64/include/uapi/asm/siginfo.h b/arch/arm64/include/uapi/asm/siginfo.h >> >> index 5a74a08..297fb4f 100644 >> >> --- a/arch/arm64/include/uapi/asm/siginfo.h >> >> +++ b/arch/arm64/include/uapi/asm/siginfo.h >> >> @@ -16,7 +16,13 @@ >> >> #ifndef __ASM_SIGINFO_H >> >> #define __ASM_SIGINFO_H >> >> >> >> +#ifdef __LP64__ >> >> #define __ARCH_SI_PREAMBLE_SIZE (4 * sizeof(int)) >> >> +#else /* ILP32 */ >> >> +typedef long long __kernel_si_clock_t __attribute__((aligned(4))); >> >> +#define __ARCH_SI_CLOCK_T __kernel_si_clock_t >> >> +#define __ARCH_SI_ATTRIBUTES __attribute__((aligned(8))) >> >> +#endif >> > >> > This could go away if we manage to use the native siginfo. >> >> See above why I think this is a bad thing and even worse since even >> x32 did not do that already; it was the last added ABI like ILP32 to >> the kernel. > > The x32 thing is becoming the central theme. > >> >> --- /dev/null >> >> +++ b/arch/arm64/kernel/sys_ilp32.c >> >> @@ -0,0 +1,274 @@ >> >> +/* >> >> + * AArch64- ILP32 specific system calls implementation >> >> + * >> >> + * Copyright (C) 2013 Cavium Inc. >> >> + * Author: Andrew Pinski <apinski@cavium.com> >> >> + * >> >> + * This program is free software; you can redistribute it and/or modify >> >> + * it under the terms of the GNU General Public License version 2 as >> >> + * published by the Free Software Foundation. >> >> + * >> >> + * This program is distributed in the hope that it will be useful, >> >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> >> + * GNU General Public License for more details. >> >> + * >> >> + * You should have received a copy of the GNU General Public License >> >> + * along with this program. If not, see <http://www.gnu.org/licenses/>. >> >> + */ >> >> + >> >> +/* Adjust unistd.h to provide 32-bit numbers and functions. */ >> >> +#define __SYSCALL_COMPAT >> > >> > No. We need to use as many native syscalls as possible and only define >> > those absolutely necessary. In my investigation, I only ended up needing >> > these: >> >> No using __SYSCALL_COMPAT is the correct thing to do and then only >> reverting back what is needed. > > I _disagree_. "Even x32 does not to that". Some past discussions: > > http://thread.gmane.org/gmane.linux.kernel/1184913 > >> > #define sys_ioctl compat_sys_ioctl >> > #define sys_readv compat_sys_readv >> > #define sys_writev compat_sys_writev >> > #define sys_preadv compat_sys_preadv64 >> > #define sys_pwritev compat_sys_pwritev64 >> > #define sys_vmsplice compat_sys_vmsplice >> > #define sys_waitid compat_sys_waitid >> > #define sys_set_robust_list compat_sys_set_robust_list >> > #define sys_get_robust_list compat_sys_get_robust_list >> > #define sys_kexec_load compat_sys_kexec_load >> > #define sys_timer_create compat_sys_timer_create >> > #define sys_ptrace compat_sys_ptrace >> > #define sys_sigaltstack compat_sys_sigaltstack >> > #define sys_rt_sigaction compat_sys_rt_sigaction >> > #define sys_rt_sigpending compat_sys_rt_sigpending >> > #define sys_rt_sigtimedwait compat_sys_rt_sigtimedwait >> > #define sys_rt_sigqueueinfo compat_sys_rt_sigqueueinfo >> > #define sys_rt_sigreturn compat_sys_rt_sigreturn_wrapper >> > #define sys_mq_notify compat_sys_mq_notify >> > #define sys_recvfrom compat_sys_recvfrom >> > #define sys_setsockopt compat_sys_setsockopt >> > #define sys_getsockopt compat_sys_getsockopt >> > #define sys_sendmsg compat_sys_sendmsg >> > #define sys_recvmsg compat_sys_recvmsg >> > #define sys_execve compat_sys_execve >> > #define sys_move_pages compat_sys_move_pages >> > #define sys_rt_tgsigqueueinfo compat_sys_rt_tgsigqueueinfo >> > #define sys_recvmmsg compat_sys_recvmmsg >> > #define sys_sendmmsg compat_sys_sendmmsg >> > #define sys_process_vm_readv compat_sys_process_vm_readv >> > #define sys_process_vm_writev compat_sys_process_vm_writev >> >> You even forgot compat_sys_openat (where O_LARGEFILE differences does >> make a difference). > > Just read the above thread. "Even x32 does not do that". > >> >> diff --git a/arch/arm64/kernel/vdsoilp32/Makefile b/arch/arm64/kernel/vdsoilp32/Makefile >> >> new file mode 100644 >> >> index 0000000..ec93f3f >> >> --- /dev/null >> >> +++ b/arch/arm64/kernel/vdsoilp32/Makefile >> > >> > Could we not keep vdso in the same directory? >> >> I started out that way but "make clean ARCH=arm64" did not clean the >> vdso files all the time. > > I'll leave vdso comments to Will. > > -- > Catalin
diff --git a/arch/arm64/include/uapi/asm/siginfo.h b/arch/arm64/include/uapi/asm/siginfo.h index 5a74a08..e3848be 100644 --- a/arch/arm64/include/uapi/asm/siginfo.h +++ b/arch/arm64/include/uapi/asm/siginfo.h @@ -18,6 +18,13 @@ #define __ARCH_SI_PREAMBLE_SIZE (4 * sizeof(int)) +#ifndef __LP64__ /* ILP32 */ +#define __ARCH_SI_BAND_T long long +#define VOID_USER_PTR(x) \ + void __user *x __attribute__((aligned(8))); \ + char _pad[4] +#endif + #include <asm-generic/siginfo.h> #endif diff --git a/include/uapi/asm-generic/siginfo.h b/include/uapi/asm-generic/siginfo.h index ba5be7f..9c50257 100644 --- a/include/uapi/asm-generic/siginfo.h +++ b/include/uapi/asm-generic/siginfo.h @@ -4,9 +4,13 @@ #include <linux/compiler.h> #include <linux/types.h> +#ifndef VOID_USER_PTR +#define VOID_USER_PTR(x) void __user *x +#endif + typedef union sigval { int sival_int; - void __user *sival_ptr; + VOID_USER_PTR(sival_ptr); } sigval_t; /* @@ -86,7 +90,7 @@ typedef struct siginfo { /* SIGILL, SIGFPE, SIGSEGV, SIGBUS */ struct { - void __user *_addr; /* faulting insn/memory ref. */ + VOID_USER_PTR(_addr); /* faulting insn/memory ref. */ #ifdef __ARCH_SI_TRAPNO int _trapno; /* TRAP # which caused the signal */ #endif @@ -101,7 +105,7 @@ typedef struct siginfo { /* SIGSYS */ struct { - void __user *_call_addr; /* calling user insn */ + VOID_USER_PTR(_call_addr); /* calling user insn */ int _syscall; /* triggering system call number */ unsigned int _arch; /* AUDIT_ARCH_* of syscall */ } _sigsys;