Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1395825 > unrolled thread
| Started by | "Zhangjian (Bamvor)" <bamvor.zhangjian@huawei.com> |
|---|---|
| First post | 2016-05-06 14:30 +0200 |
| Last post | 2016-05-12 11:30 +0200 |
| Articles | 19 — 3 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it "Zhangjian (Bamvor)" <bamvor.zhangjian@huawei.com> - 2016-05-06 14:30 +0200
Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it "Zhangjian (Bamvor)" <bamvor.zhangjian@huawei.com> - 2016-05-10 09:50 +0200
Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it "Zhangjian (Bamvor)" <bamvor.zhangjian@huawei.com> - 2016-05-10 10:00 +0200
Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it Arnd Bergmann <arnd@arndb.de> - 2016-05-10 10:40 +0200
Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it "Zhangjian (Bamvor)" <bamvor.zhangjian@huawei.com> - 2016-05-10 11:50 +0200
Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it Arnd Bergmann <arnd@arndb.de> - 2016-05-10 13:50 +0200
Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it "Zhangjian (Bamvor)" <bamvor.zhangjian@huawei.com> - 2016-05-10 14:50 +0200
Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it Arnd Bergmann <arnd@arndb.de> - 2016-05-10 15:00 +0200
Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it "Zhangjian (Bamvor)" <bamvor.zhangjian@huawei.com> - 2016-05-11 04:10 +0200
Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it Arnd Bergmann <arnd@arndb.de> - 2016-05-11 10:20 +0200
Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it "Zhangjian (Bamvor)" <bamvor.zhangjian@huawei.com> - 2016-05-11 12:20 +0200
Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it "Zhangjian (Bamvor)" <bamvor.zhangjian@huawei.com> - 2016-05-11 13:20 +0200
Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it Arnd Bergmann <arnd@arndb.de> - 2016-05-11 17:00 +0200
Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it "Zhangjian (Bamvor)" <bamvor.zhangjian@huawei.com> - 2016-05-12 05:50 +0200
Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it Arnd Bergmann <arnd@arndb.de> - 2016-05-11 13:00 +0200
Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it Catalin Marinas <catalin.marinas@arm.com> - 2016-05-11 19:00 +0200
Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it Arnd Bergmann <arnd@arndb.de> - 2016-05-11 21:40 +0200
Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it Catalin Marinas <catalin.marinas@arm.com> - 2016-05-12 11:20 +0200
Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it Arnd Bergmann <arnd@arndb.de> - 2016-05-12 11:30 +0200
| From | "Zhangjian (Bamvor)" <bamvor.zhangjian@huawei.com> |
|---|---|
| Date | 2016-05-06 14:30 +0200 |
| Subject | Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it |
| Message-ID | <rvN3k-51u-13@gated-at.bofh.it> |
Hi, On 2016/4/6 6:08, Yury Norov wrote: > From: Andrew Pinski <apinski@cavium.com> > > Add a separate syscall-table for ILP32, which dispatches either to native > LP64 system call implementation or to compat-syscalls, as appropriate. > > Signed-off-by: Andrew Pinski <Andrew.Pinski@caviumnetworks.com> > Signed-off-by: Yury Norov <ynorov@caviumnetworks.com> > --- > arch/arm64/include/asm/unistd.h | 11 ++++++- > arch/arm64/kernel/Makefile | 2 +- > arch/arm64/kernel/entry.S | 12 +++++++- > arch/arm64/kernel/sys_ilp32.c | 65 +++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 87 insertions(+), 3 deletions(-) > create mode 100644 arch/arm64/kernel/sys_ilp32.c > > diff --git a/arch/arm64/include/asm/unistd.h b/arch/arm64/include/asm/unistd.h > index 2971dea..5ea18ef 100644 > --- a/arch/arm64/include/asm/unistd.h > +++ b/arch/arm64/include/asm/unistd.h > @@ -13,9 +13,18 @@ > * You should have received a copy of the GNU General Public License > * along with this program. If not, see <http://www.gnu.org/licenses/>. > */ > + > +#ifdef CONFIG_COMPAT > +#define __ARCH_WANT_COMPAT_STAT64 > +#endif > + > +#ifdef CONFIG_ARM64_ILP32 > +#define __ARCH_WANT_COMPAT_SYS_PREADV64 > +#define __ARCH_WANT_COMPAT_SYS_PWRITEV64 > +#endif > + > #ifdef CONFIG_AARCH32_EL0 > #define __ARCH_WANT_COMPAT_SYS_GETDENTS64 > -#define __ARCH_WANT_COMPAT_STAT64 > #define __ARCH_WANT_SYS_GETHOSTNAME > #define __ARCH_WANT_SYS_PAUSE > #define __ARCH_WANT_SYS_GETPGRP > diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile > index 9dfdf86..7aa65ea 100644 > --- a/arch/arm64/kernel/Makefile > +++ b/arch/arm64/kernel/Makefile > @@ -28,7 +28,7 @@ $(obj)/%.stub.o: $(obj)/%.o FORCE > arm64-obj-$(CONFIG_AARCH32_EL0) += sys32.o kuser32.o signal32.o \ > sys_compat.o entry32.o \ > ../../arm/kernel/opcodes.o binfmt_elf32.o > -arm64-obj-$(CONFIG_ARM64_ILP32) += binfmt_ilp32.o > +arm64-obj-$(CONFIG_ARM64_ILP32) += binfmt_ilp32.o sys_ilp32.o > arm64-obj-$(CONFIG_FUNCTION_TRACER) += ftrace.o entry-ftrace.o > arm64-obj-$(CONFIG_MODULES) += arm64ksyms.o module.o > arm64-obj-$(CONFIG_ARM64_MODULE_PLTS) += module-plts.o > diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S > index cf4d1ae..1f7a145 100644 > --- a/arch/arm64/kernel/entry.S > +++ b/arch/arm64/kernel/entry.S > @@ -715,9 +715,13 @@ ENDPROC(ret_from_fork) > */ > .align 6 > el0_svc: > - adrp stbl, sys_call_table // load syscall table pointer > uxtw scno, w8 // syscall number in w8 > mov sc_nr, #__NR_syscalls > +#ifdef CONFIG_ARM64_ILP32 > + ldr x16, [tsk, #TI_FLAGS] > + tbnz x16, #TIF_32BIT_AARCH64, el0_ilp32_svc // We are using ILP32 > +#endif > + adrp stbl, sys_call_table // load syscall table pointer > el0_svc_naked: // compat entry point > stp x0, scno, [sp, #S_ORIG_X0] // save the original x0 and syscall number > enable_dbg_and_irq > @@ -737,6 +741,12 @@ ni_sys: > b ret_fast_syscall > ENDPROC(el0_svc) > > +#ifdef CONFIG_ARM64_ILP32 > +el0_ilp32_svc: > + adrp stbl, sys_call_ilp32_table // load syscall table pointer > + b el0_svc_naked > +#endif > + > /* > * This is the really slow path. We're going to be doing context > * switches, and waiting for our parent to respond. > diff --git a/arch/arm64/kernel/sys_ilp32.c b/arch/arm64/kernel/sys_ilp32.c > new file mode 100644 > index 0000000..0996d8e > --- /dev/null > +++ b/arch/arm64/kernel/sys_ilp32.c > @@ -0,0 +1,65 @@ > +/* > + * AArch64- ILP32 specific system calls implementation > + * > + * Copyright (C) 2016 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/>. > + */ > + > +#include <linux/compiler.h> > +#include <linux/errno.h> > +#include <linux/fs.h> > +#include <linux/mm.h> > +#include <linux/msg.h> > +#include <linux/export.h> > +#include <linux/sched.h> > +#include <linux/slab.h> > +#include <linux/syscalls.h> > +#include <linux/compat.h> > +#include <asm-generic/syscalls.h> > + > +/* Using non-compat syscalls where necessary */ > +#define compat_sys_fadvise64_64 sys_fadvise64_64 > +#define compat_sys_fallocate sys_fallocate > +#define compat_sys_ftruncate64 sys_ftruncate > +#define compat_sys_lookup_dcookie sys_lookup_dcookie > +#define compat_sys_pread64 sys_pread64 > +#define compat_sys_pwrite64 sys_pwrite64 > +#define compat_sys_readahead sys_readahead > +#define compat_sys_shmat sys_shmat > +#define compat_sys_sync_file_range sys_sync_file_range > +#define compat_sys_truncate64 sys_truncate > +#define sys_llseek sys_lseek > +#define sys_mmap2 sys_mmap I am a little bit confused here. We wrap the mmap to mmap2 in glibc without shift the 4096 and We map mmap2 to mmap in kernel which means we shift with the real page size. It works unless the application want to mmap the offset bigger then 2G. In ILP32 app, if the offset is bigger than 2G(e.g. 0xfb000000), it is a negative number and extend to 64bit nagetive number in kernel (0xfffffff fb000000). I add the "COMPAT_SYSCALL_WRAP6(mmap, ...)" in kernel/compat_wrapper.c. But it is not works. I am not sure if it is already sign extended in userspace. On the other hand, I read the code of mmap in arm and other architecture. Usually, they will shift 4096 in userspace and shift others in kernel if needed. Should we follow the similar ways or we could call mmap_pgoff in glibc and do the shift according the real page shift(getpages())? Thanks Bamvor
[toc] | [next] | [standalone]
| From | "Zhangjian (Bamvor)" <bamvor.zhangjian@huawei.com> |
|---|---|
| Date | 2016-05-10 09:50 +0200 |
| Message-ID | <rxaAz-6l0-53@gated-at.bofh.it> |
| In reply to | #1395825 |
Hi, Yury
On 2016/5/6 20:37, Yury Norov wrote:
> On Fri, May 06, 2016 at 08:16:48PM +0800, Zhangjian (Bamvor) wrote:
>> Hi,
>>
>> On 2016/4/6 6:08, Yury Norov wrote:
>>> From: Andrew Pinski <apinski@cavium.com>
>>>
>>> Add a separate syscall-table for ILP32, which dispatches either to native
>>> LP64 system call implementation or to compat-syscalls, as appropriate.
>>>
>>> Signed-off-by: Andrew Pinski <Andrew.Pinski@caviumnetworks.com>
>>> Signed-off-by: Yury Norov <ynorov@caviumnetworks.com>
>>> ---
>>> arch/arm64/include/asm/unistd.h | 11 ++++++-
>>> arch/arm64/kernel/Makefile | 2 +-
>>> arch/arm64/kernel/entry.S | 12 +++++++-
>>> arch/arm64/kernel/sys_ilp32.c | 65 +++++++++++++++++++++++++++++++++++++++++
>>> 4 files changed, 87 insertions(+), 3 deletions(-)
>>> create mode 100644 arch/arm64/kernel/sys_ilp32.c
>>>
>>> diff --git a/arch/arm64/include/asm/unistd.h b/arch/arm64/include/asm/unistd.h
>>> index 2971dea..5ea18ef 100644
>>> --- a/arch/arm64/include/asm/unistd.h
>>> +++ b/arch/arm64/include/asm/unistd.h
>>> @@ -13,9 +13,18 @@
>>> * You should have received a copy of the GNU General Public License
>>> * along with this program. If not, see <http://www.gnu.org/licenses/>.
>>> */
>>> +
>>> +#ifdef CONFIG_COMPAT
>>> +#define __ARCH_WANT_COMPAT_STAT64
>>> +#endif
>>> +
>>> +#ifdef CONFIG_ARM64_ILP32
>>> +#define __ARCH_WANT_COMPAT_SYS_PREADV64
>>> +#define __ARCH_WANT_COMPAT_SYS_PWRITEV64
>>> +#endif
>>> +
>>> #ifdef CONFIG_AARCH32_EL0
>>> #define __ARCH_WANT_COMPAT_SYS_GETDENTS64
>>> -#define __ARCH_WANT_COMPAT_STAT64
>>> #define __ARCH_WANT_SYS_GETHOSTNAME
>>> #define __ARCH_WANT_SYS_PAUSE
>>> #define __ARCH_WANT_SYS_GETPGRP
>>> diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
>>> index 9dfdf86..7aa65ea 100644
>>> --- a/arch/arm64/kernel/Makefile
>>> +++ b/arch/arm64/kernel/Makefile
>>> @@ -28,7 +28,7 @@ $(obj)/%.stub.o: $(obj)/%.o FORCE
>>> arm64-obj-$(CONFIG_AARCH32_EL0) += sys32.o kuser32.o signal32.o \
>>> sys_compat.o entry32.o \
>>> ../../arm/kernel/opcodes.o binfmt_elf32.o
>>> -arm64-obj-$(CONFIG_ARM64_ILP32) += binfmt_ilp32.o
>>> +arm64-obj-$(CONFIG_ARM64_ILP32) += binfmt_ilp32.o sys_ilp32.o
>>> arm64-obj-$(CONFIG_FUNCTION_TRACER) += ftrace.o entry-ftrace.o
>>> arm64-obj-$(CONFIG_MODULES) += arm64ksyms.o module.o
>>> arm64-obj-$(CONFIG_ARM64_MODULE_PLTS) += module-plts.o
>>> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
>>> index cf4d1ae..1f7a145 100644
>>> --- a/arch/arm64/kernel/entry.S
>>> +++ b/arch/arm64/kernel/entry.S
>>> @@ -715,9 +715,13 @@ ENDPROC(ret_from_fork)
>>> */
>>> .align 6
>>> el0_svc:
>>> - adrp stbl, sys_call_table // load syscall table pointer
>>> uxtw scno, w8 // syscall number in w8
>>> mov sc_nr, #__NR_syscalls
>>> +#ifdef CONFIG_ARM64_ILP32
>>> + ldr x16, [tsk, #TI_FLAGS]
>>> + tbnz x16, #TIF_32BIT_AARCH64, el0_ilp32_svc // We are using ILP32
>>> +#endif
>>> + adrp stbl, sys_call_table // load syscall table pointer
>>> el0_svc_naked: // compat entry point
>>> stp x0, scno, [sp, #S_ORIG_X0] // save the original x0 and syscall number
>>> enable_dbg_and_irq
>>> @@ -737,6 +741,12 @@ ni_sys:
>>> b ret_fast_syscall
>>> ENDPROC(el0_svc)
>>>
>>> +#ifdef CONFIG_ARM64_ILP32
>>> +el0_ilp32_svc:
>>> + adrp stbl, sys_call_ilp32_table // load syscall table pointer
>>> + b el0_svc_naked
>>> +#endif
>>> +
>>> /*
>>> * This is the really slow path. We're going to be doing context
>>> * switches, and waiting for our parent to respond.
>>> diff --git a/arch/arm64/kernel/sys_ilp32.c b/arch/arm64/kernel/sys_ilp32.c
>>> new file mode 100644
>>> index 0000000..0996d8e
>>> --- /dev/null
>>> +++ b/arch/arm64/kernel/sys_ilp32.c
>>> @@ -0,0 +1,65 @@
>>> +/*
>>> + * AArch64- ILP32 specific system calls implementation
>>> + *
>>> + * Copyright (C) 2016 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/>.
>>> + */
>>> +
>>> +#include <linux/compiler.h>
>>> +#include <linux/errno.h>
>>> +#include <linux/fs.h>
>>> +#include <linux/mm.h>
>>> +#include <linux/msg.h>
>>> +#include <linux/export.h>
>>> +#include <linux/sched.h>
>>> +#include <linux/slab.h>
>>> +#include <linux/syscalls.h>
>>> +#include <linux/compat.h>
>>> +#include <asm-generic/syscalls.h>
>>> +
>>> +/* Using non-compat syscalls where necessary */
>>> +#define compat_sys_fadvise64_64 sys_fadvise64_64
>>> +#define compat_sys_fallocate sys_fallocate
>>> +#define compat_sys_ftruncate64 sys_ftruncate
>>> +#define compat_sys_lookup_dcookie sys_lookup_dcookie
>>> +#define compat_sys_pread64 sys_pread64
>>> +#define compat_sys_pwrite64 sys_pwrite64
>>> +#define compat_sys_readahead sys_readahead
>>> +#define compat_sys_shmat sys_shmat
>>> +#define compat_sys_sync_file_range sys_sync_file_range
>>> +#define compat_sys_truncate64 sys_truncate
>>> +#define sys_llseek sys_lseek
>>> +#define sys_mmap2 sys_mmap
>> I am a little bit confused here. We wrap the mmap to mmap2 in glibc
>> without shift the 4096 and We map mmap2 to mmap in kernel which
>> means we shift with the real page size. It works unless the
>> application want to mmap the offset bigger then 2G. In ILP32 app,
>> if the offset is bigger than 2G(e.g. 0xfb000000), it is a negative
>> number and extend to 64bit nagetive number in kernel
>> (0xfffffff fb000000). I add the "COMPAT_SYSCALL_WRAP6(mmap, ...)" in
>> kernel/compat_wrapper.c. But it is not works. I am not sure if it is
>> already sign extended in userspace.
>>
>> On the other hand, I read the code of mmap in arm and other
>> architecture. Usually, they will shift 4096 in userspace and shift
>> others in kernel if needed. Should we follow the similar ways or we
>> could call mmap_pgoff in glibc and do the shift according the real
>> page shift(getpages())?
>>
>> Thanks
>>
>> Bamvor
>>
>>
>
> Hi,
>
> AFAIR, here we don't shift offset, as it's 64-bit both in user-
> and kernel-space,
In your ilp32-2.22 branch, you wrapper mmap to mmap2 in which type of
offset is off_t. And off_t is 32bit in ilp32, correct?
"sysdeps/unix/sysv/linux/aarch64/ilp32/mmap64.c"
/* mmap is provided by mmap as they are the same. */
void *__mmap (void *__addr, size_t __len, int __prot,
int __flags, int __fd, __off_t __offset)
{
void *result;
result = (void *)
INLINE_SYSCALL (mmap2, 6, __addr,
__len, __prot, __flags, __fd, __offset);
return result;
}
> and just pass it from user to kernel thru glibc
> with no changes.
>
> It definitely works, as there are many mappings made by linker and
> libc in 2G+ area, and there are no problems with them. This is a
> typical ILP32 application map:
Ok, the different is I am talking about the offset in mmap. I am NOT
talking about the map result.
If I run my test case with strace:
"strace -e trace=mmap ./mmap.arm64_ilp32 0xfb000000 0x1000", here is
the part of log:
1 mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0xf7721000
2 mmap(NULL, 65536, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0xf7557000
3 page size<0x1000>, offset is <0xfb000000>
4 mmap(NULL, 4096, PROT_READ, MAP_SHARED, 3, 0xfffffffffb000000) = -1 EINVAL (Invalid argument)
5 mmap of mmapfile failed
As you said, line 1 and 2 show that mmap could map above 2G. But it
is NOT what I want to discussion.
As I said, when I pass the offset above 2G(e.g. 0xfb0000000), we
could find that the actual offset pass to kernel is
0xfffffffffb000000(reference line 4).
It will fail if I map in /dev/mmem. It will not fail if the fd is
a normal file. But in both of case the offset is wrong.
Regards
Bamvor
> 00400000-00401000 r-xp 00000000 08:00 130400 /root/mykill
> 00410000-00411000 rwxp 00000000 08:00 130400 /root/mykill
> 00527000-00549000 rwxp 00000000 00:00 0 [heap]
> c6278000-c6298000 rwxp 00000000 00:00 0
> c6298000-c63d0000 r-xp 00000000 08:00 135293 /root/sys-root/libilp32/libc-2.22.so
> c63d0000-c63e0000 ---p 00138000 08:00 135293 /root/sys-root/libilp32/libc-2.22.so
> c63e0000-c63e2000 r-xp 00138000 08:00 135293 /root/sys-root/libilp32/libc-2.22.so
> c63e2000-c63e3000 rwxp 0013a000 08:00 135293 /root/sys-root/libilp32/libc-2.22.so
> c63e3000-c63e6000 rwxp 00000000 00:00 0
> c63e6000-c63fc000 r-xp 00000000 08:00 135313 /root/sys-root/libilp32/libpthread-2.22.so
> c63fc000-c640b000 ---p 00016000 08:00 135313 /root/sys-root/libilp32/libpthread-2.22.so
> c640b000-c640c000 r-xp 00015000 08:00 135313 /root/sys-root/libilp32/libpthread-2.22.so
> c640c000-c640d000 rwxp 00016000 08:00 135313 /root/sys-root/libilp32/libpthread-2.22.so
> c640d000-c640f000 rwxp 00000000 00:00 0
> c640f000-c642c000 r-xp 00000000 08:00 135288 /root/sys-root/libilp32/ld-2.22.so
> c6437000-c6439000 rwxp 00000000 00:00 0
> c6439000-c643a000 r--p 00000000 00:00 0 [vvar]
> c643a000-c643b000 r-xp 00000000 00:00 0 [vdso]
> c643b000-c643c000 r-xp 0001c000 08:00 135288 /root/sys-root/libilp32/ld-2.22.so
> c643c000-c643d000 rwxp 0001d000 08:00 135288 /root/sys-root/libilp32/ld-2.22.so
> ffe2d000-ffe4e000 rw-p 00000000 00:00 0 [stack]
>
>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
[toc] | [prev] | [next] | [standalone]
| From | "Zhangjian (Bamvor)" <bamvor.zhangjian@huawei.com> |
|---|---|
| Date | 2016-05-10 10:00 +0200 |
| Message-ID | <rxaKd-6py-1@gated-at.bofh.it> |
| In reply to | #1397801 |
Hi,
Sorry I forget to paste my test code:
#include <stdio.h>
#include <stdlib.h>
#include <fcntl.h>
#include <sys/mman.h>
#define TEMPFILE "mmapfile"
int main(int argc, char *argv[])
{
int fd;
void *addr;
unsigned long offset;
unsigned long size;
if (argc == 3) {
if (argv[1][0] == '0' && argv[1][1] == 'x')
offset = strtoll(&argv[1][2], NULL, 16);
else
offset = atoi(argv[1]);
if (argv[2][0] == '0' && argv[2][1] == 'x')
size = strtoll(&argv[2][2], NULL, 16);
else
size = atoi(argv[2]);
} else {
exit(2);
}
printf("page size<0x%x>, offset is <0x%x>\n", size, offset);
// if ((fd = open(TEMPFILE, O_RDWR | O_CREAT, 0666)) < 0) {
// fprintf(stderr, "opening %s failed\n", TEMPFILE);
// exit(2);
// }
fd = open("/dev/mem", O_RDWR | O_SYNC);
if (-1 == fd)
{
printf( "open /dev/mem fail!\n" );
return 1;
}
//addr = mmap(0, size, PROT_READ | PROT_WRITE, MAP_FILE | MAP_SHARED, fd, offset);
addr = mmap(0, size, PROT_READ, MAP_FILE | MAP_SHARED, fd, offset);
if(addr == MAP_FAILED) {
fprintf(stderr, "mmap of %s failed\n", TEMPFILE);
exit(2);
}
printf("addr: <0x%x>\n", addr);
return 0;
}
Regards
Bamvor
On 2016/5/10 15:42, Zhangjian (Bamvor) wrote:
> Hi, Yury
>
> On 2016/5/6 20:37, Yury Norov wrote:
>> On Fri, May 06, 2016 at 08:16:48PM +0800, Zhangjian (Bamvor) wrote:
>>> Hi,
>>>
>>> On 2016/4/6 6:08, Yury Norov wrote:
>>>> From: Andrew Pinski <apinski@cavium.com>
>>>>
>>>> Add a separate syscall-table for ILP32, which dispatches either to native
>>>> LP64 system call implementation or to compat-syscalls, as appropriate.
>>>>
>>>> Signed-off-by: Andrew Pinski <Andrew.Pinski@caviumnetworks.com>
>>>> Signed-off-by: Yury Norov <ynorov@caviumnetworks.com>
>>>> ---
>>>> arch/arm64/include/asm/unistd.h | 11 ++++++-
>>>> arch/arm64/kernel/Makefile | 2 +-
>>>> arch/arm64/kernel/entry.S | 12 +++++++-
>>>> arch/arm64/kernel/sys_ilp32.c | 65 +++++++++++++++++++++++++++++++++++++++++
>>>> 4 files changed, 87 insertions(+), 3 deletions(-)
>>>> create mode 100644 arch/arm64/kernel/sys_ilp32.c
>>>>
>>>> diff --git a/arch/arm64/include/asm/unistd.h b/arch/arm64/include/asm/unistd.h
>>>> index 2971dea..5ea18ef 100644
>>>> --- a/arch/arm64/include/asm/unistd.h
>>>> +++ b/arch/arm64/include/asm/unistd.h
>>>> @@ -13,9 +13,18 @@
>>>> * You should have received a copy of the GNU General Public License
>>>> * along with this program. If not, see <http://www.gnu.org/licenses/>.
>>>> */
>>>> +
>>>> +#ifdef CONFIG_COMPAT
>>>> +#define __ARCH_WANT_COMPAT_STAT64
>>>> +#endif
>>>> +
>>>> +#ifdef CONFIG_ARM64_ILP32
>>>> +#define __ARCH_WANT_COMPAT_SYS_PREADV64
>>>> +#define __ARCH_WANT_COMPAT_SYS_PWRITEV64
>>>> +#endif
>>>> +
>>>> #ifdef CONFIG_AARCH32_EL0
>>>> #define __ARCH_WANT_COMPAT_SYS_GETDENTS64
>>>> -#define __ARCH_WANT_COMPAT_STAT64
>>>> #define __ARCH_WANT_SYS_GETHOSTNAME
>>>> #define __ARCH_WANT_SYS_PAUSE
>>>> #define __ARCH_WANT_SYS_GETPGRP
>>>> diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
>>>> index 9dfdf86..7aa65ea 100644
>>>> --- a/arch/arm64/kernel/Makefile
>>>> +++ b/arch/arm64/kernel/Makefile
>>>> @@ -28,7 +28,7 @@ $(obj)/%.stub.o: $(obj)/%.o FORCE
>>>> arm64-obj-$(CONFIG_AARCH32_EL0) += sys32.o kuser32.o signal32.o \
>>>> sys_compat.o entry32.o \
>>>> ../../arm/kernel/opcodes.o binfmt_elf32.o
>>>> -arm64-obj-$(CONFIG_ARM64_ILP32) += binfmt_ilp32.o
>>>> +arm64-obj-$(CONFIG_ARM64_ILP32) += binfmt_ilp32.o sys_ilp32.o
>>>> arm64-obj-$(CONFIG_FUNCTION_TRACER) += ftrace.o entry-ftrace.o
>>>> arm64-obj-$(CONFIG_MODULES) += arm64ksyms.o module.o
>>>> arm64-obj-$(CONFIG_ARM64_MODULE_PLTS) += module-plts.o
>>>> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
>>>> index cf4d1ae..1f7a145 100644
>>>> --- a/arch/arm64/kernel/entry.S
>>>> +++ b/arch/arm64/kernel/entry.S
>>>> @@ -715,9 +715,13 @@ ENDPROC(ret_from_fork)
>>>> */
>>>> .align 6
>>>> el0_svc:
>>>> - adrp stbl, sys_call_table // load syscall table pointer
>>>> uxtw scno, w8 // syscall number in w8
>>>> mov sc_nr, #__NR_syscalls
>>>> +#ifdef CONFIG_ARM64_ILP32
>>>> + ldr x16, [tsk, #TI_FLAGS]
>>>> + tbnz x16, #TIF_32BIT_AARCH64, el0_ilp32_svc // We are using ILP32
>>>> +#endif
>>>> + adrp stbl, sys_call_table // load syscall table pointer
>>>> el0_svc_naked: // compat entry point
>>>> stp x0, scno, [sp, #S_ORIG_X0] // save the original x0 and syscall number
>>>> enable_dbg_and_irq
>>>> @@ -737,6 +741,12 @@ ni_sys:
>>>> b ret_fast_syscall
>>>> ENDPROC(el0_svc)
>>>>
>>>> +#ifdef CONFIG_ARM64_ILP32
>>>> +el0_ilp32_svc:
>>>> + adrp stbl, sys_call_ilp32_table // load syscall table pointer
>>>> + b el0_svc_naked
>>>> +#endif
>>>> +
>>>> /*
>>>> * This is the really slow path. We're going to be doing context
>>>> * switches, and waiting for our parent to respond.
>>>> diff --git a/arch/arm64/kernel/sys_ilp32.c b/arch/arm64/kernel/sys_ilp32.c
>>>> new file mode 100644
>>>> index 0000000..0996d8e
>>>> --- /dev/null
>>>> +++ b/arch/arm64/kernel/sys_ilp32.c
>>>> @@ -0,0 +1,65 @@
>>>> +/*
>>>> + * AArch64- ILP32 specific system calls implementation
>>>> + *
>>>> + * Copyright (C) 2016 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/>.
>>>> + */
>>>> +
>>>> +#include <linux/compiler.h>
>>>> +#include <linux/errno.h>
>>>> +#include <linux/fs.h>
>>>> +#include <linux/mm.h>
>>>> +#include <linux/msg.h>
>>>> +#include <linux/export.h>
>>>> +#include <linux/sched.h>
>>>> +#include <linux/slab.h>
>>>> +#include <linux/syscalls.h>
>>>> +#include <linux/compat.h>
>>>> +#include <asm-generic/syscalls.h>
>>>> +
>>>> +/* Using non-compat syscalls where necessary */
>>>> +#define compat_sys_fadvise64_64 sys_fadvise64_64
>>>> +#define compat_sys_fallocate sys_fallocate
>>>> +#define compat_sys_ftruncate64 sys_ftruncate
>>>> +#define compat_sys_lookup_dcookie sys_lookup_dcookie
>>>> +#define compat_sys_pread64 sys_pread64
>>>> +#define compat_sys_pwrite64 sys_pwrite64
>>>> +#define compat_sys_readahead sys_readahead
>>>> +#define compat_sys_shmat sys_shmat
>>>> +#define compat_sys_sync_file_range sys_sync_file_range
>>>> +#define compat_sys_truncate64 sys_truncate
>>>> +#define sys_llseek sys_lseek
>>>> +#define sys_mmap2 sys_mmap
>>> I am a little bit confused here. We wrap the mmap to mmap2 in glibc
>>> without shift the 4096 and We map mmap2 to mmap in kernel which
>>> means we shift with the real page size. It works unless the
>>> application want to mmap the offset bigger then 2G. In ILP32 app,
>>> if the offset is bigger than 2G(e.g. 0xfb000000), it is a negative
>>> number and extend to 64bit nagetive number in kernel
>>> (0xfffffff fb000000). I add the "COMPAT_SYSCALL_WRAP6(mmap, ...)" in
>>> kernel/compat_wrapper.c. But it is not works. I am not sure if it is
>>> already sign extended in userspace.
>>>
>>> On the other hand, I read the code of mmap in arm and other
>>> architecture. Usually, they will shift 4096 in userspace and shift
>>> others in kernel if needed. Should we follow the similar ways or we
>>> could call mmap_pgoff in glibc and do the shift according the real
>>> page shift(getpages())?
>>>
>>> Thanks
>>>
>>> Bamvor
>>>
>>>
>>
>> Hi,
>>
>> AFAIR, here we don't shift offset, as it's 64-bit both in user-
>> and kernel-space,
> In your ilp32-2.22 branch, you wrapper mmap to mmap2 in which type of
> offset is off_t. And off_t is 32bit in ilp32, correct?
> "sysdeps/unix/sysv/linux/aarch64/ilp32/mmap64.c"
> /* mmap is provided by mmap as they are the same. */
> void *__mmap (void *__addr, size_t __len, int __prot,
> int __flags, int __fd, __off_t __offset)
> {
> void *result;
> result = (void *)
> INLINE_SYSCALL (mmap2, 6, __addr,
> __len, __prot, __flags, __fd, __offset);
> return result;
> }
>> and just pass it from user to kernel thru glibc
>> with no changes.
>>
>> It definitely works, as there are many mappings made by linker and
>> libc in 2G+ area, and there are no problems with them. This is a
>> typical ILP32 application map:
> Ok, the different is I am talking about the offset in mmap. I am NOT
> talking about the map result.
> If I run my test case with strace:
> "strace -e trace=mmap ./mmap.arm64_ilp32 0xfb000000 0x1000", here is
> the part of log:
>
> 1 mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0xf7721000
> 2 mmap(NULL, 65536, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0xf7557000
> 3 page size<0x1000>, offset is <0xfb000000>
> 4 mmap(NULL, 4096, PROT_READ, MAP_SHARED, 3, 0xfffffffffb000000) = -1 EINVAL (Invalid argument)
> 5 mmap of mmapfile failed
>
> As you said, line 1 and 2 show that mmap could map above 2G. But it
> is NOT what I want to discussion.
> As I said, when I pass the offset above 2G(e.g. 0xfb0000000), we
> could find that the actual offset pass to kernel is
> 0xfffffffffb000000(reference line 4).
> It will fail if I map in /dev/mmem. It will not fail if the fd is
> a normal file. But in both of case the offset is wrong.
>
> Regards
>
> Bamvor
>
>> 00400000-00401000 r-xp 00000000 08:00 130400 /root/mykill
>> 00410000-00411000 rwxp 00000000 08:00 130400 /root/mykill
>> 00527000-00549000 rwxp 00000000 00:00 0 [heap]
>> c6278000-c6298000 rwxp 00000000 00:00 0
>> c6298000-c63d0000 r-xp 00000000 08:00 135293 /root/sys-root/libilp32/libc-2.22.so
>> c63d0000-c63e0000 ---p 00138000 08:00 135293 /root/sys-root/libilp32/libc-2.22.so
>> c63e0000-c63e2000 r-xp 00138000 08:00 135293 /root/sys-root/libilp32/libc-2.22.so
>> c63e2000-c63e3000 rwxp 0013a000 08:00 135293 /root/sys-root/libilp32/libc-2.22.so
>> c63e3000-c63e6000 rwxp 00000000 00:00 0
>> c63e6000-c63fc000 r-xp 00000000 08:00 135313 /root/sys-root/libilp32/libpthread-2.22.so
>> c63fc000-c640b000 ---p 00016000 08:00 135313 /root/sys-root/libilp32/libpthread-2.22.so
>> c640b000-c640c000 r-xp 00015000 08:00 135313 /root/sys-root/libilp32/libpthread-2.22.so
>> c640c000-c640d000 rwxp 00016000 08:00 135313 /root/sys-root/libilp32/libpthread-2.22.so
>> c640d000-c640f000 rwxp 00000000 00:00 0
>> c640f000-c642c000 r-xp 00000000 08:00 135288 /root/sys-root/libilp32/ld-2.22.so
>> c6437000-c6439000 rwxp 00000000 00:00 0
>> c6439000-c643a000 r--p 00000000 00:00 0 [vvar]
>> c643a000-c643b000 r-xp 00000000 00:00 0 [vdso]
>> c643b000-c643c000 r-xp 0001c000 08:00 135288 /root/sys-root/libilp32/ld-2.22.so
>> c643c000-c643d000 rwxp 0001d000 08:00 135288 /root/sys-root/libilp32/ld-2.22.so
>> ffe2d000-ffe4e000 rw-p 00000000 00:00 0 [stack]
>>
>>
>>> _______________________________________________
>>> linux-arm-kernel mailing list
>>> linux-arm-kernel@lists.infradead.org
>>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
[toc] | [prev] | [next] | [standalone]
| From | Arnd Bergmann <arnd@arndb.de> |
|---|---|
| Date | 2016-05-10 10:40 +0200 |
| Subject | Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it |
| Message-ID | <rxbmW-7dG-19@gated-at.bofh.it> |
| In reply to | #1397801 |
On Tuesday 10 May 2016 15:42:07 Zhangjian wrote:
> On 2016/5/6 20:37, Yury Norov wrote:
> > On Fri, May 06, 2016 at 08:16:48PM +0800, Zhangjian (Bamvor) wrote:
> >
> > AFAIR, here we don't shift offset, as it's 64-bit both in user-
> > and kernel-space,
> In your ilp32-2.22 branch, you wrapper mmap to mmap2 in which type of
> offset is off_t. And off_t is 32bit in ilp32, correct?
> "sysdeps/unix/sysv/linux/aarch64/ilp32/mmap64.c"
> /* mmap is provided by mmap as they are the same. */
> void *__mmap (void *__addr, size_t __len, int __prot,
> int __flags, int __fd, __off_t __offset)
> {
> void *result;
> result = (void *)
> INLINE_SYSCALL (mmap2, 6, __addr,
> __len, __prot, __flags, __fd, __offset);
> return result;
> }
__off_t should be 'long long' on new architectures, and map
to __kernel_loff_t.
Can you see how it is defined?
Arnd
[toc] | [prev] | [next] | [standalone]
| From | "Zhangjian (Bamvor)" <bamvor.zhangjian@huawei.com> |
|---|---|
| Date | 2016-05-10 11:50 +0200 |
| Message-ID | <rxcsF-8ff-9@gated-at.bofh.it> |
| In reply to | #1397852 |
Hi, Arnd
On 2016/5/10 16:36, Arnd Bergmann wrote:
> On Tuesday 10 May 2016 15:42:07 Zhangjian wrote:
>> On 2016/5/6 20:37, Yury Norov wrote:
>>> On Fri, May 06, 2016 at 08:16:48PM +0800, Zhangjian (Bamvor) wrote:
>>>
>>> AFAIR, here we don't shift offset, as it's 64-bit both in user-
>>> and kernel-space,
>> In your ilp32-2.22 branch, you wrapper mmap to mmap2 in which type of
>> offset is off_t. And off_t is 32bit in ilp32, correct?
>> "sysdeps/unix/sysv/linux/aarch64/ilp32/mmap64.c"
>> /* mmap is provided by mmap as they are the same. */
>> void *__mmap (void *__addr, size_t __len, int __prot,
>> int __flags, int __fd, __off_t __offset)
>> {
>> void *result;
>> result = (void *)
>> INLINE_SYSCALL (mmap2, 6, __addr,
>> __len, __prot, __flags, __fd, __offset);
>> return result;
>> }
>
> __off_t should be 'long long' on new architectures, and map
> to __kernel_loff_t.
>
> Can you see how it is defined?
For kernel part, in "include/uapi/asm-generic/posix_types.h":
#ifndef __kernel_long_t
typedef long __kernel_long_t;
typedef unsigned long __kernel_ulong_t;
#endif
typedef __kernel_long_t __kernel_off_t;
in "include/linux/types.h":
typedef __kernel_off_t off_t;
"include/uapi/asm-generic/posix_types.h" is uapi, we could not check
"ARCH_32BIT_OFF_T" here. Besides, the `__kernel_long_t` is long which
mean it is 32bit in ILP32. should we define something like x32?
```
diff --git a/arch/arm64/include/uapi/asm/posix_types.h b/arch/arm64/include/uapi/asm/posix_types.h
index 7985ff6..9baa8d3 100644
--- a/arch/arm64/include/uapi/asm/posix_types.h
+++ b/arch/arm64/include/uapi/asm/posix_types.h
@@ -5,6 +5,9 @@ typedef unsigned short __kernel_old_uid_t;
typedef unsigned short __kernel_old_gid_t;
#define __kernel_old_uid_t __kernel_old_uid_t
+typedef long long __kernel_long_t;
+typedef unsigned long long __kernel_ulong_t;
+
#include <asm-generic/posix_types.h>
#endif /* __ASM_POSIX_TYPES_H */u
```
After this definition, the following kernel types is 64bit in ILP32:
typedef __kernel_long_t __kernel_suseconds_t;
typedef __kernel_long_t __kernel_ssize_t;
typedef __kernel_long_t __kernel_ptrdiff_t;
typedef __kernel_long_t __kernel_off_t;
typedef __kernel_long_t __kernel_time_t;
typedef __kernel_long_t __kernel_clock_t;
typedef __kernel_ulong_t __kernel_ino_t;
typedef __kernel_ulong_t __kernel_size_t;
But it is not a generic way to define off_t to 64bit if
ARCH_32BIT_OFF_T is not defined. How about this one? We need to
define `__kernel_off_t` on all the old 32bit architecture like
arm:
```
diff --git a/arch/arm/include/uapi/asm/posix_types.h b/arch/arm/include/uapi/asm/posix_types.h
index d2de9cb..f9d065c 100644
--- a/arch/arm/include/uapi/asm/posix_types.h
+++ b/arch/arm/include/uapi/asm/posix_types.h
@@ -32,6 +32,9 @@ typedef unsigned short __kernel_gid_t;
typedef unsigned short __kernel_old_dev_t;
#define __kernel_old_dev_t __kernel_old_dev_t
+typedef __kernel_long_t __kernel_off_t;
+#define __kernel_off_t
+
#include <asm-generic/posix_types.h>
#endif
```
And We could change the generic posix_types.h a little bit:
```
diff --git a/include/uapi/asm-generic/posix_types.h b/include/uapi/asm-generic/posix_types.h
index fe74fcc..7bbaf04 100644
--- a/include/uapi/asm-generic/posix_types.h
+++ b/include/uapi/asm-generic/posix_types.h
@@ -80,10 +80,13 @@ typedef struct {
} __kernel_fsid_t;
#endif
+#ifndef __kernel_off_t
+typedef long long __kernel_off_t;
+#endif
+
/*
* anything below here should be completely generic
*/
-typedef __kernel_long_t __kernel_off_t;
typedef long long __kernel_loff_t;
typedef __kernel_long_t __kernel_time_t;
typedef __kernel_long_t __kernel_clock_t;
```
On the other hand, glibc define it own off_t in "bits/types.h":
__STD_TYPE __OFF_T_TYPE __off_t; /* Type of file sizes and offsets. */
__STD_TYPE __OFF64_T_TYPE __off64_t; /* Type of file sizes and offsets (LFS). */
in "sysdeps/unix/sysv/linux/aarch64/bits/typesizes.h":
#define __OFF_T_TYPE __SLONGWORD_TYPE
#define __OFF64_T_TYPE __SQUAD_TYPE
If we define off_t as 64bit in glibc:
#define __OFF_T_TYPE __SQUAD_TYPE
Should We need to align all the off_t syscall to 64bit syscall in
kernel?
Regards
Bamvor
> Arnd
>
[toc] | [prev] | [next] | [standalone]
| From | Arnd Bergmann <arnd@arndb.de> |
|---|---|
| Date | 2016-05-10 13:50 +0200 |
| Subject | Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it |
| Message-ID | <rxekP-1Bh-27@gated-at.bofh.it> |
| In reply to | #1397908 |
On Tuesday 10 May 2016 17:47:26 Zhangjian wrote: > On 2016/5/10 16:36, Arnd Bergmann wrote: > > On Tuesday 10 May 2016 15:42:07 Zhangjian wrote: > >> On 2016/5/6 20:37, Yury Norov wrote: > > "include/uapi/asm-generic/posix_types.h" is uapi, we could not check > "ARCH_32BIT_OFF_T" here. Besides, the `__kernel_long_t` is long which > mean it is 32bit in ILP32. should we define something like x32? > ``` > diff --git a/arch/arm64/include/uapi/asm/posix_types.h b/arch/arm64/include/uapi/asm/posix_types.h > index 7985ff6..9baa8d3 100644 > --- a/arch/arm64/include/uapi/asm/posix_types.h > +++ b/arch/arm64/include/uapi/asm/posix_types.h glibc does not use the definition of __kernel_off_t, it has its own copy, so changing the kernel headers would do nothing. > @@ -5,6 +5,9 @@ typedef unsigned short __kernel_old_uid_t; > typedef unsigned short __kernel_old_gid_t; > #define __kernel_old_uid_t __kernel_old_uid_t > > +typedef long long __kernel_long_t; > +typedef unsigned long long __kernel_ulong_t; > + > #include <asm-generic/posix_types.h> > > #endif /* __ASM_POSIX_TYPES_H */u > ``` This would break all sorts of things, because __kernel_long_t/__kernel_ulong_t are not just used for off_t but also other things. > > On the other hand, glibc define it own off_t in "bits/types.h": > __STD_TYPE __OFF_T_TYPE __off_t; /* Type of file sizes and offsets. */ > __STD_TYPE __OFF64_T_TYPE __off64_t; /* Type of file sizes and offsets (LFS). */ > > in "sysdeps/unix/sysv/linux/aarch64/bits/typesizes.h": > #define __OFF_T_TYPE __SLONGWORD_TYPE > #define __OFF64_T_TYPE __SQUAD_TYPE > > If we define off_t as 64bit in glibc: > #define __OFF_T_TYPE __SQUAD_TYPE > > Should We need to align all the off_t syscall to 64bit syscall in > kernel? > Yes, this is the change that I think we need to make, along with the same change for __INO_T_TYPE and #define __OFF_T_MATCHES_OFF64_T 1 #define __INO_T_MATCHES_INO64_T 1 If I read the rest of the glibc headers right, that should be all we need to ensure that both off_t and off64_t match the __kernel_loff_t based syscalls. Arnd
[toc] | [prev] | [next] | [standalone]
| From | "Zhangjian (Bamvor)" <bamvor.zhangjian@huawei.com> |
|---|---|
| Date | 2016-05-10 14:50 +0200 |
| Message-ID | <rxfgS-2rD-13@gated-at.bofh.it> |
| In reply to | #1398016 |
Hi, On 2016/5/10 19:48, Arnd Bergmann wrote: > On Tuesday 10 May 2016 17:47:26 Zhangjian wrote: >> On 2016/5/10 16:36, Arnd Bergmann wrote: >>> On Tuesday 10 May 2016 15:42:07 Zhangjian wrote: >>>> On 2016/5/6 20:37, Yury Norov wrote: >> >> "include/uapi/asm-generic/posix_types.h" is uapi, we could not check >> "ARCH_32BIT_OFF_T" here. Besides, the `__kernel_long_t` is long which >> mean it is 32bit in ILP32. should we define something like x32? >> ``` >> diff --git a/arch/arm64/include/uapi/asm/posix_types.h b/arch/arm64/include/uapi/asm/posix_types.h >> index 7985ff6..9baa8d3 100644 >> --- a/arch/arm64/include/uapi/asm/posix_types.h >> +++ b/arch/arm64/include/uapi/asm/posix_types.h > > glibc does not use the definition of __kernel_off_t, it has its own > copy, so changing the kernel headers would do nothing. Yes, I think so. I am puzzle that how do we ensure that all the new 32bit architecture will define off_t as 64bit after introduce ARCH_32BIT_OFF_T and do not change any other code? > >> @@ -5,6 +5,9 @@ typedef unsigned short __kernel_old_uid_t; >> typedef unsigned short __kernel_old_gid_t; >> #define __kernel_old_uid_t __kernel_old_uid_t >> >> +typedef long long __kernel_long_t; >> +typedef unsigned long long __kernel_ulong_t; >> + >> #include <asm-generic/posix_types.h> >> >> #endif /* __ASM_POSIX_TYPES_H */u >> ``` > > This would break all sorts of things, because __kernel_long_t/__kernel_ulong_t > are not just used for off_t but also other things. Yes. But if we do not change __kernel_long_t and __kernel_ulong_t, the application of ilp32 will think size of long in kernel is 32bit. Is that correct? > >> >> On the other hand, glibc define it own off_t in "bits/types.h": >> __STD_TYPE __OFF_T_TYPE __off_t; /* Type of file sizes and offsets. */ >> __STD_TYPE __OFF64_T_TYPE __off64_t; /* Type of file sizes and offsets (LFS). */ >> >> in "sysdeps/unix/sysv/linux/aarch64/bits/typesizes.h": >> #define __OFF_T_TYPE __SLONGWORD_TYPE >> #define __OFF64_T_TYPE __SQUAD_TYPE >> >> If we define off_t as 64bit in glibc: >> #define __OFF_T_TYPE __SQUAD_TYPE >> >> Should We need to align all the off_t syscall to 64bit syscall in >> kernel? >> > > Yes, this is the change that I think we need to make, along with > the same change for __INO_T_TYPE and > > #define __OFF_T_MATCHES_OFF64_T 1 > #define __INO_T_MATCHES_INO64_T 1 > > If I read the rest of the glibc headers right, that should be all we need > to ensure that both off_t and off64_t match the __kernel_loff_t based > syscalls. Ok, I will test the ltp syscall test. With this changes, the issue I mentioned should be fixed. But we still use mmap2 syscall for ILP32 application when we pass the offset instead of page offset. Is it correct? Regards Bamvor > > Arnd >
[toc] | [prev] | [next] | [standalone]
| From | Arnd Bergmann <arnd@arndb.de> |
|---|---|
| Date | 2016-05-10 15:00 +0200 |
| Subject | Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it |
| Message-ID | <rxfqy-2xu-1@gated-at.bofh.it> |
| In reply to | #1398056 |
On Tuesday 10 May 2016 20:39:41 Zhangjian wrote: > Hi, > > On 2016/5/10 19:48, Arnd Bergmann wrote: > > On Tuesday 10 May 2016 17:47:26 Zhangjian wrote: > >> On 2016/5/10 16:36, Arnd Bergmann wrote: > >>> On Tuesday 10 May 2016 15:42:07 Zhangjian wrote: > >>>> On 2016/5/6 20:37, Yury Norov wrote: > >> > >> "include/uapi/asm-generic/posix_types.h" is uapi, we could not check > >> "ARCH_32BIT_OFF_T" here. Besides, the `__kernel_long_t` is long which > >> mean it is 32bit in ILP32. should we define something like x32? > >> ``` > >> diff --git a/arch/arm64/include/uapi/asm/posix_types.h b/arch/arm64/include/uapi/asm/posix_types.h > >> index 7985ff6..9baa8d3 100644 > >> --- a/arch/arm64/include/uapi/asm/posix_types.h > >> +++ b/arch/arm64/include/uapi/asm/posix_types.h > > > > glibc does not use the definition of __kernel_off_t, it has its own > > copy, so changing the kernel headers would do nothing. > Yes, I think so. I am puzzle that how do we ensure that all the new > 32bit architecture will define off_t as 64bit after introduce > ARCH_32BIT_OFF_T and do not change any other code? This is for historic reasons: we have two sets of system calls passing file offsets: the original ones passing a 'long' are named e.g. lseek, pread, pwrite, fadvise, ... The replacement syscalls that pass a 'long long' __kernel_loff_t are only available on 32-bit architectures and have different names, e.g. llseek, pread64, pwrite64, fadvise64, ... On 64-bit architectures, we only provide the first set, because __kernel_off_t was already wide enough. On old 32-bit architectures we have both, and on new 32-bit architectures we only have the second set, which lets us use the same definitions on all 32-bit architectures. > >> @@ -5,6 +5,9 @@ typedef unsigned short __kernel_old_uid_t; > >> typedef unsigned short __kernel_old_gid_t; > >> #define __kernel_old_uid_t __kernel_old_uid_t > >> > >> +typedef long long __kernel_long_t; > >> +typedef unsigned long long __kernel_ulong_t; > >> + > >> #include <asm-generic/posix_types.h> > >> > >> #endif /* __ASM_POSIX_TYPES_H */u > >> ``` > > > > This would break all sorts of things, because __kernel_long_t/__kernel_ulong_t > > are not just used for off_t but also other things. > Yes. But if we do not change __kernel_long_t and __kernel_ulong_t, the > application of ilp32 will think size of long in kernel is 32bit. Is that > correct? No, __kernel_long_t refers to the definition of 'long' in the kernel/user ABI, not in the kernel (except on the x86-64 "x32" ABI, which is weird). A 32-bit user space application must not care about how 'long' is defined in the kernel, it should run on either 32-bit kernels or 64-bit kernels. We don't have any plans to do an ilp32-mode kernel, but if we ever want one, it must use the exact same ABI as the 64-bit kernel when running ilp32 user space. > >> > >> On the other hand, glibc define it own off_t in "bits/types.h": > >> __STD_TYPE __OFF_T_TYPE __off_t; /* Type of file sizes and offsets. */ > >> __STD_TYPE __OFF64_T_TYPE __off64_t; /* Type of file sizes and offsets (LFS). */ > >> > >> in "sysdeps/unix/sysv/linux/aarch64/bits/typesizes.h": > >> #define __OFF_T_TYPE __SLONGWORD_TYPE > >> #define __OFF64_T_TYPE __SQUAD_TYPE > >> > >> If we define off_t as 64bit in glibc: > >> #define __OFF_T_TYPE __SQUAD_TYPE > >> > >> Should We need to align all the off_t syscall to 64bit syscall in > >> kernel? > >> > > > > Yes, this is the change that I think we need to make, along with > > the same change for __INO_T_TYPE and > > > > #define __OFF_T_MATCHES_OFF64_T 1 > > #define __INO_T_MATCHES_INO64_T 1 > > > > If I read the rest of the glibc headers right, that should be all we need > > to ensure that both off_t and off64_t match the __kernel_loff_t based > > syscalls. > Ok, I will test the ltp syscall test. > With this changes, the issue I mentioned should be fixed. But we still > use mmap2 syscall for ILP32 application when we pass the offset instead > of page offset. Is it correct? I don't remember. It's probably not important whether we have the shift in there, as long as it's independent of the actual kernel page size and user space and kernel agree on the calling conventions. Arnd
[toc] | [prev] | [next] | [standalone]
| From | "Zhangjian (Bamvor)" <bamvor.zhangjian@huawei.com> |
|---|---|
| Date | 2016-05-11 04:10 +0200 |
| Message-ID | <rxrL4-714-19@gated-at.bofh.it> |
| In reply to | #1398064 |
Hi, On 2016/5/10 20:50, Arnd Bergmann wrote: > On Tuesday 10 May 2016 20:39:41 Zhangjian wrote: >> Hi, >> >> On 2016/5/10 19:48, Arnd Bergmann wrote: >>> On Tuesday 10 May 2016 17:47:26 Zhangjian wrote: >>>> On 2016/5/10 16:36, Arnd Bergmann wrote: >>>>> On Tuesday 10 May 2016 15:42:07 Zhangjian wrote: >>>>>> On 2016/5/6 20:37, Yury Norov wrote: >>>> >>>> "include/uapi/asm-generic/posix_types.h" is uapi, we could not check >>>> "ARCH_32BIT_OFF_T" here. Besides, the `__kernel_long_t` is long which >>>> mean it is 32bit in ILP32. should we define something like x32? >>>> ``` >>>> diff --git a/arch/arm64/include/uapi/asm/posix_types.h b/arch/arm64/include/uapi/asm/posix_types.h >>>> index 7985ff6..9baa8d3 100644 >>>> --- a/arch/arm64/include/uapi/asm/posix_types.h >>>> +++ b/arch/arm64/include/uapi/asm/posix_types.h >>> >>> glibc does not use the definition of __kernel_off_t, it has its own >>> copy, so changing the kernel headers would do nothing. >> Yes, I think so. I am puzzle that how do we ensure that all the new >> 32bit architecture will define off_t as 64bit after introduce >> ARCH_32BIT_OFF_T and do not change any other code? > > This is for historic reasons: we have two sets of system calls passing > file offsets: the original ones passing a 'long' are named e.g. lseek, > pread, pwrite, fadvise, ... > > The replacement syscalls that pass a 'long long' __kernel_loff_t are only > available on 32-bit architectures and have different names, e.g. llseek, > pread64, pwrite64, fadvise64, ... > > On 64-bit architectures, we only provide the first set, because > __kernel_off_t was already wide enough. On old 32-bit architectures > we have both, and on new 32-bit architectures we only have the second > set, which lets us use the same definitions on all 32-bit architectures. > >>>> @@ -5,6 +5,9 @@ typedef unsigned short __kernel_old_uid_t; >>>> typedef unsigned short __kernel_old_gid_t; >>>> #define __kernel_old_uid_t __kernel_old_uid_t >>>> >>>> +typedef long long __kernel_long_t; >>>> +typedef unsigned long long __kernel_ulong_t; >>>> + >>>> #include <asm-generic/posix_types.h> >>>> >>>> #endif /* __ASM_POSIX_TYPES_H */u >>>> ``` >>> >>> This would break all sorts of things, because __kernel_long_t/__kernel_ulong_t >>> are not just used for off_t but also other things. >> Yes. But if we do not change __kernel_long_t and __kernel_ulong_t, the >> application of ilp32 will think size of long in kernel is 32bit. Is that >> correct? > > No, __kernel_long_t refers to the definition of 'long' in the kernel/user > ABI, not in the kernel (except on the x86-64 "x32" ABI, which is weird). Thanks, that is answer my question. I ask this because I saw the posix_types.h for x32. > A 32-bit user space application must not care about how 'long' is defined > in the kernel, it should run on either 32-bit kernels or 64-bit kernels. > We don't have any plans to do an ilp32-mode kernel, but if we ever want > one, it must use the exact same ABI as the 64-bit kernel when running > ilp32 user space. > >>>> >>>> On the other hand, glibc define it own off_t in "bits/types.h": >>>> __STD_TYPE __OFF_T_TYPE __off_t; /* Type of file sizes and offsets. */ >>>> __STD_TYPE __OFF64_T_TYPE __off64_t; /* Type of file sizes and offsets (LFS). */ >>>> >>>> in "sysdeps/unix/sysv/linux/aarch64/bits/typesizes.h": >>>> #define __OFF_T_TYPE __SLONGWORD_TYPE >>>> #define __OFF64_T_TYPE __SQUAD_TYPE >>>> >>>> If we define off_t as 64bit in glibc: >>>> #define __OFF_T_TYPE __SQUAD_TYPE >>>> >>>> Should We need to align all the off_t syscall to 64bit syscall in >>>> kernel? >>>> >>> >>> Yes, this is the change that I think we need to make, along with >>> the same change for __INO_T_TYPE and >>> >>> #define __OFF_T_MATCHES_OFF64_T 1 >>> #define __INO_T_MATCHES_INO64_T 1 >>> >>> If I read the rest of the glibc headers right, that should be all we need >>> to ensure that both off_t and off64_t match the __kernel_loff_t based >>> syscalls. >> Ok, I will test the ltp syscall test. >> With this changes, the issue I mentioned should be fixed. But we still >> use mmap2 syscall for ILP32 application when we pass the offset instead >> of page offset. Is it correct? > > I don't remember. It's probably not important whether we have the shift > in there, as long as it's independent of the actual kernel page size and > user space and kernel agree on the calling conventions. Well. I am ok with where to shift the pages size because we get the same result. I was just thinking if we should get rid of the name of mmap2 in our ILP32 porting. Actually, it is mmap but we name it as mmap2. User may confused if they do not know the implementations. Regards Bamvor > > Arnd >
[toc] | [prev] | [next] | [standalone]
| From | Arnd Bergmann <arnd@arndb.de> |
|---|---|
| Date | 2016-05-11 10:20 +0200 |
| Subject | Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it |
| Message-ID | <rxxx8-4bl-1@gated-at.bofh.it> |
| In reply to | #1398638 |
On Wednesday 11 May 2016 10:04:16 Zhangjian wrote: > > I don't remember. It's probably not important whether we have the shift > > in there, as long as it's independent of the actual kernel page size and > > user space and kernel agree on the calling conventions. > Well. I am ok with where to shift the pages size because we get the same > result. I was just thinking if we should get rid of the name of mmap2 in our > ILP32 porting. Actually, it is mmap but we name it as mmap2. User may confused > if they do not know the implementations. That is a good point: If the implementation matches the mmap() behavior rather than mmap2(), we should rename the macro by doing #undef __NR_mmap2 #define __NR_mmap 222 in the uapi/asm/unistd.h file for ilp32 mode. Alternatively we can keep the __NR_mmap2 definition but then we need to pass the pgoff (value shifted by 12 bits) argument rather than the size in bytes. Arnd
[toc] | [prev] | [next] | [standalone]
| From | "Zhangjian (Bamvor)" <bamvor.zhangjian@huawei.com> |
|---|---|
| Date | 2016-05-11 12:20 +0200 |
| Message-ID | <rxzpg-6fl-5@gated-at.bofh.it> |
| In reply to | #1398755 |
Hi, Arnd On 2016/5/11 16:09, Arnd Bergmann wrote: > On Wednesday 11 May 2016 10:04:16 Zhangjian wrote: >>> I don't remember. It's probably not important whether we have the shift >>> in there, as long as it's independent of the actual kernel page size and >>> user space and kernel agree on the calling conventions. >> Well. I am ok with where to shift the pages size because we get the same >> result. I was just thinking if we should get rid of the name of mmap2 in our >> ILP32 porting. Actually, it is mmap but we name it as mmap2. User may confused >> if they do not know the implementations. > > That is a good point: If the implementation matches the mmap() behavior rather than > mmap2(), we should rename the macro by doing > > #undef __NR_mmap2 > #define __NR_mmap 222 > > in the uapi/asm/unistd.h file for ilp32 mode. Do you mean define the following things in kernel: ``` diff --git a/arch/arm64/include/uapi/asm/unistd.h b/arch/arm64/include/uapi/asm/unistd.h index 1caadc2..3f79640 100644 --- a/arch/arm64/include/uapi/asm/unistd.h +++ b/arch/arm64/include/uapi/asm/unistd.h @@ -14,3 +14,9 @@ * along with this program. If not, see <http://www.gnu.org/licenses/>. */ #include <asm-generic/unistd.h> + +#ifdef __ILP32__ +#undef __NR_mmap2 +#define __NR_mmap 222 +#endif /* #ifdef __ILP32__ */ + ``` Then glibc could call mmap instead of mmap2. I could not try it now. Because after change off_t to 64bit in glibc, stat is fail. I may need to revert the stat relative patch. > Alternatively we can keep the > __NR_mmap2 definition but then we need to pass the pgoff (value shifted by > 12 bits) argument rather than the size in bytes. It means that we could reuse the existing code of mmap2 in kernel and glibc. But we need to shift twice when kernel is 64k page. It seems that the first method is more clear. Suggestion? Regards Bamvor > > Arnd >
[toc] | [prev] | [next] | [standalone]
| From | "Zhangjian (Bamvor)" <bamvor.zhangjian@huawei.com> |
|---|---|
| Date | 2016-05-11 13:20 +0200 |
| Message-ID | <rxAll-7bN-29@gated-at.bofh.it> |
| In reply to | #1398895 |
Hi, On 2016/5/11 18:12, Zhangjian (Bamvor) wrote: > Hi, Arnd > > On 2016/5/11 16:09, Arnd Bergmann wrote: > > On Wednesday 11 May 2016 10:04:16 Zhangjian wrote: > >>> I don't remember. It's probably not important whether we have the shift > >>> in there, as long as it's independent of the actual kernel page size and > >>> user space and kernel agree on the calling conventions. > >> Well. I am ok with where to shift the pages size because we get the same > >> result. I was just thinking if we should get rid of the name of mmap2 in our > >> ILP32 porting. Actually, it is mmap but we name it as mmap2. User may confused > >> if they do not know the implementations. > > > > That is a good point: If the implementation matches the mmap() behavior rather than > > mmap2(), we should rename the macro by doing > > > > #undef __NR_mmap2 > > #define __NR_mmap 222 > > > > in the uapi/asm/unistd.h file for ilp32 mode. > Do you mean define the following things in kernel: > ``` > diff --git a/arch/arm64/include/uapi/asm/unistd.h b/arch/arm64/include/uapi/asm/unistd.h > index 1caadc2..3f79640 100644 > --- a/arch/arm64/include/uapi/asm/unistd.h > +++ b/arch/arm64/include/uapi/asm/unistd.h > @@ -14,3 +14,9 @@ > * along with this program. If not, see <http://www.gnu.org/licenses/>. > */ > #include <asm-generic/unistd.h> > + > +#ifdef __ILP32__ > +#undef __NR_mmap2 > +#define __NR_mmap 222 > +#endif /* #ifdef __ILP32__ */ > + > ``` > Then glibc could call mmap instead of mmap2. > I could not try it now. Because after change off_t to 64bit in glibc, stat > is fail. I may need to revert the stat relative patch. After revert stat relative patch in glibc, mmap01-mmap14 success. But mmap16 success with segfault. I will investigate it later. There is pointer and size_t in mmap, so, IIUC, we need to clear the top halves of register by using COMPAT_SYSCALL_WRAP6. And after check the function in arch/s390/kernel/compat_linux.c, I feel that we need to do the same thing for pread64 and pwrite64. But I got following error when I try to add COMPAT_SYSCALL_WRAP4(pread64, unsigned int, fd, char __user *, buf, size_t, count, loff_t, pos); COMPAT_SYSCALL_WRAP4(pwrite64, unsigned int, fd, const char __user *, buf, size_t, count, loff_t, pos); The error message: kernel/compat_wrapper.c: In function 'compat_SyS_pread64': include/linux/compiler.h:429:38: error: call to '__compiletime_assert_308' declared with attribute error: BUILD_BUG_ON failed: (sizeof(loff_t) > 4) && !__TYPE_IS_L(loff_t) && !__TYPE_IS_UL(loff_t) && !__TYPE_IS_PTR(loff_t) _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__) ^ include/linux/compiler.h:412:4: note: in definition of macro '__compiletime_assert' prefix ## suffix(); \ ^ include/linux/compiler.h:429:2: note: in expansion of macro '_compiletime_assert' _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__) ^ include/linux/bug.h:50:37: note: in expansion of macro 'compiletime_assert' #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg) ^ include/linux/bug.h:74:2: note: in expansion of macro 'BUILD_BUG_ON_MSG' BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition) ^ include/linux/compat.h:749:2: note: in expansion of macro 'BUILD_BUG_ON' BUILD_BUG_ON((sizeof(t) > 4) && !__TYPE_IS_L(t) && \ ^ include/linux/syscalls.h:38:23: note: in expansion of macro '__SC_COMPAT_CAST' #define __MAP1(m,t,a) m(t,a) ^ include/linux/syscalls.h:39:35: note: in expansion of macro '__MAP1' #define __MAP2(m,t,a,...) m(t,a), __MAP1(m,__VA_ARGS__) ^ include/linux/syscalls.h:40:35: note: in expansion of macro '__MAP2' #define __MAP3(m,t,a,...) m(t,a), __MAP2(m,__VA_ARGS__) ^ include/linux/syscalls.h:41:35: note: in expansion of macro '__MAP3' #define __MAP4(m,t,a,...) m(t,a), __MAP3(m,__VA_ARGS__) ^ include/linux/syscalls.h:44:22: note: in expansion of macro '__MAP4' #define __MAP(n,...) __MAP##n(__VA_ARGS__) ^ include/linux/compat.h:777:19: note: in expansion of macro '__MAP' return sys##name(__MAP(x, __SC_COMPAT_CAST, __VA_ARGS__)); \ ^ include/linux/compat.h:735:41: note: in expansion of macro 'COMPAT_SYSCALL_WRAPx' #define COMPAT_SYSCALL_WRAP4(name, ...) COMPAT_SYSCALL_WRAPx(4, _##name,\ ^ kernel/compat_wrapper.c:307:1: note: in expansion of macro 'COMPAT_SYSCALL_WRAP4' COMPAT_SYSCALL_WRAP4(pread64, unsigned int, fd, char __user *, buf, ^ Because the loff_t is not the long, unsigned long or pointer. Regards Bamvor > > Alternatively we can keep the > > __NR_mmap2 definition but then we need to pass the pgoff (value shifted by > > 12 bits) argument rather than the size in bytes. > It means that we could reuse the existing code of mmap2 in kernel and glibc. > But we need to shift twice when kernel is 64k page. > It seems that the first method is more clear. Suggestion? > > Regards > > Bamvor > > > > Arnd > > >
[toc] | [prev] | [next] | [standalone]
| From | Arnd Bergmann <arnd@arndb.de> |
|---|---|
| Date | 2016-05-11 17:00 +0200 |
| Subject | Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it |
| Message-ID | <rxDMe-1Nm-19@gated-at.bofh.it> |
| In reply to | #1398961 |
On Wednesday 11 May 2016 19:16:44 Zhangjian wrote: > Hi, > > On 2016/5/11 18:12, Zhangjian (Bamvor) wrote: > > Hi, Arnd > > > > On 2016/5/11 16:09, Arnd Bergmann wrote: > > > On Wednesday 11 May 2016 10:04:16 Zhangjian wrote: > > >>> I don't remember. It's probably not important whether we have the shift > > >>> in there, as long as it's independent of the actual kernel page size and > > >>> user space and kernel agree on the calling conventions. > > >> Well. I am ok with where to shift the pages size because we get the same > > >> result. I was just thinking if we should get rid of the name of mmap2 in our > > >> ILP32 porting. Actually, it is mmap but we name it as mmap2. User may confused > > >> if they do not know the implementations. > > > > > > That is a good point: If the implementation matches the mmap() behavior rather than > > > mmap2(), we should rename the macro by doing > > > > > > #undef __NR_mmap2 > > > #define __NR_mmap 222 > > > > > > in the uapi/asm/unistd.h file for ilp32 mode. > > Do you mean define the following things in kernel: > > ``` > > diff --git a/arch/arm64/include/uapi/asm/unistd.h b/arch/arm64/include/uapi/asm/unistd.h > > index 1caadc2..3f79640 100644 > > --- a/arch/arm64/include/uapi/asm/unistd.h > > +++ b/arch/arm64/include/uapi/asm/unistd.h > > @@ -14,3 +14,9 @@ > > * along with this program. If not, see <http://www.gnu.org/licenses/>. > > */ > > #include <asm-generic/unistd.h> > > + > > +#ifdef __ILP32__ > > +#undef __NR_mmap2 > > +#define __NR_mmap 222 > > +#endif /* #ifdef __ILP32__ */ > > + > > ``` > > Then glibc could call mmap instead of mmap2. > > I could not try it now. Because after change off_t to 64bit in glibc, stat > > is fail. I may need to revert the stat relative patch. > After revert stat relative patch in glibc, mmap01-mmap14 success. But mmap16 > success with segfault. I will investigate it later. > > There is pointer and size_t in mmap, so, IIUC, we need to clear the top halves > of register by using COMPAT_SYSCALL_WRAP6. Correct, good catch! > And after check the function in > arch/s390/kernel/compat_linux.c, I feel that we need to do the same thing for > pread64 and pwrite64. > > But I got following error when I try to add > COMPAT_SYSCALL_WRAP4(pread64, unsigned int, fd, char __user *, buf, > size_t, count, loff_t, pos); > COMPAT_SYSCALL_WRAP4(pwrite64, unsigned int, fd, const char __user *, buf, > size_t, count, loff_t, pos); > Hmm, that is indeed tricky. I think COMPAT_SYSCALL_WRAP4 rightfully refuses the loff_t argument here, as the common case is that this is not possible. Can you open-code this using a COMPAT_SYSCALL4 definition similar to what arch/tile has, but without the merging of the two halves of the argument? Arnd
[toc] | [prev] | [next] | [standalone]
| From | "Zhangjian (Bamvor)" <bamvor.zhangjian@huawei.com> |
|---|---|
| Date | 2016-05-12 05:50 +0200 |
| Message-ID | <rxPNn-5yL-3@gated-at.bofh.it> |
| In reply to | #1399208 |
Hi, Arnd
On 2016/5/11 22:50, Arnd Bergmann wrote:
> On Wednesday 11 May 2016 19:16:44 Zhangjian wrote:
>> Hi,
>>
>> On 2016/5/11 18:12, Zhangjian (Bamvor) wrote:
>>> Hi, Arnd
>>>
>>> On 2016/5/11 16:09, Arnd Bergmann wrote:
>>> > On Wednesday 11 May 2016 10:04:16 Zhangjian wrote:
>>> >>> I don't remember. It's probably not important whether we have the shift
>>> >>> in there, as long as it's independent of the actual kernel page size and
>>> >>> user space and kernel agree on the calling conventions.
>>> >> Well. I am ok with where to shift the pages size because we get the same
>>> >> result. I was just thinking if we should get rid of the name of mmap2 in our
>>> >> ILP32 porting. Actually, it is mmap but we name it as mmap2. User may confused
>>> >> if they do not know the implementations.
>>> >
>>> > That is a good point: If the implementation matches the mmap() behavior rather than
>>> > mmap2(), we should rename the macro by doing
>>> >
>>> > #undef __NR_mmap2
>>> > #define __NR_mmap 222
>>> >
>>> > in the uapi/asm/unistd.h file for ilp32 mode.
>>> Do you mean define the following things in kernel:
>>> ```
>>> diff --git a/arch/arm64/include/uapi/asm/unistd.h b/arch/arm64/include/uapi/asm/unistd.h
>>> index 1caadc2..3f79640 100644
>>> --- a/arch/arm64/include/uapi/asm/unistd.h
>>> +++ b/arch/arm64/include/uapi/asm/unistd.h
>>> @@ -14,3 +14,9 @@
>>> * along with this program. If not, see <http://www.gnu.org/licenses/>.
>>> */
>>> #include <asm-generic/unistd.h>
>>> +
>>> +#ifdef __ILP32__
>>> +#undef __NR_mmap2
>>> +#define __NR_mmap 222
>>> +#endif /* #ifdef __ILP32__ */
>>> +
>>> ```
>>> Then glibc could call mmap instead of mmap2.
>>> I could not try it now. Because after change off_t to 64bit in glibc, stat
>>> is fail. I may need to revert the stat relative patch.
>> After revert stat relative patch in glibc, mmap01-mmap14 success. But mmap16
>> success with segfault. I will investigate it later.
>>
>> There is pointer and size_t in mmap, so, IIUC, we need to clear the top halves
>> of register by using COMPAT_SYSCALL_WRAP6.
>
> Correct, good catch!
>
>> And after check the function in
>> arch/s390/kernel/compat_linux.c, I feel that we need to do the same thing for
>> pread64 and pwrite64.
>>
>
>> But I got following error when I try to add
>> COMPAT_SYSCALL_WRAP4(pread64, unsigned int, fd, char __user *, buf,
>> size_t, count, loff_t, pos);
>> COMPAT_SYSCALL_WRAP4(pwrite64, unsigned int, fd, const char __user *, buf,
>> size_t, count, loff_t, pos);
>>
>
> Hmm, that is indeed tricky. I think COMPAT_SYSCALL_WRAP4 rightfully
> refuses the loff_t argument here, as the common case is that this is
> not possible.
It works if I apply the following patch, I defined the wrong `__TYPE_IS_xxx`
yesterday. Should we merge this into ILP32 series or send the compat.h
and syscalls.h individually? The current series of ILP32 is a little bit
long and hard to review.
diff --git a/include/linux/compat.h b/include/linux/compat.h
index ba6ebe0..22a9565 100644
--- a/include/linux/compat.h
+++ b/include/linux/compat.h
@@ -747,7 +747,8 @@ asmlinkage long compat_sys_fanotify_mark(int, unsigned int, __u32, __u32,
#ifndef __SC_COMPAT_CAST
#define __SC_COMPAT_CAST(t, a) ({ \
BUILD_BUG_ON((sizeof(t) > 4) && !__TYPE_IS_L(t) && \
- !__TYPE_IS_UL(t) && !__TYPE_IS_PTR(t)); \
+ !__TYPE_IS_UL(t) && !__TYPE_IS_PTR(t) && \
+ !__TYPE_IS_LOFFT(t)); \
((t) ((t)(-1) < 0 ? (s64)(s32)(a) : (u64)(u32)(a))); \
})
#endif
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 6e57d9c..66eb85d 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -47,6 +47,7 @@
#define __TYPE_IS_L(t) (__same_type((t)0, 0L))
#define __TYPE_IS_UL(t) (__same_type((t)0, 0UL))
#define __TYPE_IS_LL(t) (__same_type((t)0, 0LL) || __same_type((t)0, 0ULL))
+#define __TYPE_IS_LOFFT(t) (__same_type((t)0, (loff_t)0))
#define __SC_LONG(t, a) __typeof(__builtin_choose_expr(__TYPE_IS_LL(t), 0LL, 0L)) a
#define __SC_CAST(t, a) (t) a
#define __SC_ARGS(t, a) a
diff --git a/kernel/compat_wrapper.c b/kernel/compat_wrapper.c
index 98b68b8..28f02d0 100644
--- a/kernel/compat_wrapper.c
+++ b/kernel/compat_wrapper.c
@@ -304,3 +304,7 @@ COMPAT_SYSCALL_WRAP3(getpeername, int, fd, struct sockaddr __user *, usockaddr,
COMPAT_SYSCALL_WRAP6(sendto, int, fd, void __user *, buff, size_t, len,
unsigned int, flags, struct sockaddr __user *, addr,
int, addr_len);
+COMPAT_SYSCALL_WRAP4(pread64, unsigned int, fd, char __user *, buf,
+ size_t, count, loff_t, pos);
+COMPAT_SYSCALL_WRAP4(pwrite64, unsigned int, fd, const char __user *, buf,
+ size_t, count, loff_t, pos);
>
> Can you open-code this using a COMPAT_SYSCALL4 definition similar to what
> arch/tile has, but without the merging of the two halves of the argument?
I am lost here. Tile do not use the wrapper, and it do not use the loff_t
either:
COMPAT_SYSCALL_DEFINE6(pread64, unsigned int, fd, char __user *, ubuf,
size_t, count, u32, dummy, u32, low, u32, high)
Regards
Bamvor
> Arnd
>
[toc] | [prev] | [next] | [standalone]
| From | Arnd Bergmann <arnd@arndb.de> |
|---|---|
| Date | 2016-05-11 13:00 +0200 |
| Subject | Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it |
| Message-ID | <rxA1Y-6AZ-17@gated-at.bofh.it> |
| In reply to | #1398638 |
On Wednesday 11 May 2016 11:04:38 Yury Norov wrote: > On Wed, May 11, 2016 at 10:04:16AM +0800, Zhangjian (Bamvor) wrote: > [...] > > > >>Ok, I will test the ltp syscall test. > > >>With this changes, the issue I mentioned should be fixed. But we still > > >>use mmap2 syscall for ILP32 application when we pass the offset instead > > >>of page offset. Is it correct? > > > > > >I don't remember. It's probably not important whether we have the shift > > >in there, as long as it's independent of the actual kernel page size and > > >user space and kernel agree on the calling conventions. > > Well. I am ok with where to shift the pages size because we get the same > > result. I was just thinking if we should get rid of the name of mmap2 in our > > ILP32 porting. Actually, it is mmap but we name it as mmap2. User may confused > > if they do not know the implementations. > > > > This is what generic unistd.h does. If you want to change it, you'd > change each arch that uses generic unistd.h. > Generic unistd.h has this: #ifdef __SYSCALL_COMPAT #define __SC_COMP_3264(_nr, _32, _64, _comp) __SYSCALL(_nr, _comp) #else #define __SC_COMP_3264(_nr, _32, _64, _comp) __SC_3264(_nr, _32, _64) #endif #define __NR3264_mmap 222 __SC_3264(__NR3264_mmap, sys_mmap2, sys_mmap) #if __BITS_PER_LONG == 64 && !defined(__SYSCALL_COMPAT) #define __NR_mmap __NR3264_mmap #else #define __NR_mmap2 __NR3264_mmap #endif So by default we get __NR_mmap2 and sys_mmap2 on 32-bit ABIs, but __NR_mmap and sys_mmap on 64-bit ABIs, as it should be. The problem is that arch/arm64/kernel/sys_ilp32.c now overrides this to use __NR_mmap2 with sys_mmap, so we have a mismatch. I think we should either override both the implementation and the number, or neither of them. I think what happened is that you chose to override sys_mmap because sys_mmap2 sometimes takes an off_t argument, however the kernel just treats it as 'unsigned long', with a range of 44 bits for the offset. Using sys_mmap with a __kernel_loff_t argument gives us 64-bit range, but if we do that, I think it should be __NR_mmap. Arnd
[toc] | [prev] | [next] | [standalone]
| From | Catalin Marinas <catalin.marinas@arm.com> |
|---|---|
| Date | 2016-05-11 19:00 +0200 |
| Message-ID | <rxFEm-3D1-17@gated-at.bofh.it> |
| In reply to | #1398945 |
On Wed, May 11, 2016 at 12:55:01PM +0200, Arnd Bergmann wrote: > On Wednesday 11 May 2016 11:04:38 Yury Norov wrote: > > On Wed, May 11, 2016 at 10:04:16AM +0800, Zhangjian (Bamvor) wrote: > > [...] > > > > > >>Ok, I will test the ltp syscall test. > > > >>With this changes, the issue I mentioned should be fixed. But we still > > > >>use mmap2 syscall for ILP32 application when we pass the offset instead > > > >>of page offset. Is it correct? > > > > > > > >I don't remember. It's probably not important whether we have the shift > > > >in there, as long as it's independent of the actual kernel page size and > > > >user space and kernel agree on the calling conventions. > > > Well. I am ok with where to shift the pages size because we get the same > > > result. I was just thinking if we should get rid of the name of mmap2 in our > > > ILP32 porting. Actually, it is mmap but we name it as mmap2. User may confused > > > if they do not know the implementations. > > > > > > > This is what generic unistd.h does. If you want to change it, you'd > > change each arch that uses generic unistd.h. > > Generic unistd.h has this: > > #ifdef __SYSCALL_COMPAT > #define __SC_COMP_3264(_nr, _32, _64, _comp) __SYSCALL(_nr, _comp) > #else > #define __SC_COMP_3264(_nr, _32, _64, _comp) __SC_3264(_nr, _32, _64) > #endif > > #define __NR3264_mmap 222 > __SC_3264(__NR3264_mmap, sys_mmap2, sys_mmap) > > > #if __BITS_PER_LONG == 64 && !defined(__SYSCALL_COMPAT) > #define __NR_mmap __NR3264_mmap > #else > #define __NR_mmap2 __NR3264_mmap > #endif > > So by default we get __NR_mmap2 and sys_mmap2 on 32-bit ABIs, but > __NR_mmap and sys_mmap on 64-bit ABIs, as it should be. > > The problem is that arch/arm64/kernel/sys_ilp32.c now overrides > this to use __NR_mmap2 with sys_mmap, so we have a mismatch. I think > we should either override both the implementation and the number, > or neither of them. I would vote for "neither of them" (so we use __NR_mmap2 and sys_mmap2) to keep it close to new 32-bit architectures, even though we would have some shifts by 12 in both glibc and kernel. -- Catalin
[toc] | [prev] | [next] | [standalone]
| From | Arnd Bergmann <arnd@arndb.de> |
|---|---|
| Date | 2016-05-11 21:40 +0200 |
| Subject | Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it |
| Message-ID | <rxI9b-6rJ-1@gated-at.bofh.it> |
| In reply to | #1399364 |
On Wednesday 11 May 2016 17:59:01 Catalin Marinas wrote: > On Wed, May 11, 2016 at 12:55:01PM +0200, Arnd Bergmann wrote: > > On Wednesday 11 May 2016 11:04:38 Yury Norov wrote: > > > On Wed, May 11, 2016 at 10:04:16AM +0800, Zhangjian (Bamvor) wrote: > > > [...] > > > > > > > >>Ok, I will test the ltp syscall test. > > > > >>With this changes, the issue I mentioned should be fixed. But we still > > > > >>use mmap2 syscall for ILP32 application when we pass the offset instead > > > > >>of page offset. Is it correct? > > > > > > > > > >I don't remember. It's probably not important whether we have the shift > > > > >in there, as long as it's independent of the actual kernel page size and > > > > >user space and kernel agree on the calling conventions. > > > > Well. I am ok with where to shift the pages size because we get the same > > > > result. I was just thinking if we should get rid of the name of mmap2 in our > > > > ILP32 porting. Actually, it is mmap but we name it as mmap2. User may confused > > > > if they do not know the implementations. > > > > > > > > > > This is what generic unistd.h does. If you want to change it, you'd > > > change each arch that uses generic unistd.h. > > > > Generic unistd.h has this: > > > > #ifdef __SYSCALL_COMPAT > > #define __SC_COMP_3264(_nr, _32, _64, _comp) __SYSCALL(_nr, _comp) > > #else > > #define __SC_COMP_3264(_nr, _32, _64, _comp) __SC_3264(_nr, _32, _64) > > #endif > > > > #define __NR3264_mmap 222 > > __SC_3264(__NR3264_mmap, sys_mmap2, sys_mmap) > > > > > > #if __BITS_PER_LONG == 64 && !defined(__SYSCALL_COMPAT) > > #define __NR_mmap __NR3264_mmap > > #else > > #define __NR_mmap2 __NR3264_mmap > > #endif > > > > So by default we get __NR_mmap2 and sys_mmap2 on 32-bit ABIs, but > > __NR_mmap and sys_mmap on 64-bit ABIs, as it should be. > > > > The problem is that arch/arm64/kernel/sys_ilp32.c now overrides > > this to use __NR_mmap2 with sys_mmap, so we have a mismatch. I think > > we should either override both the implementation and the number, > > or neither of them. > > I would vote for "neither of them" (so we use __NR_mmap2 and sys_mmap2) > to keep it close to new 32-bit architectures, even though we would have > some shifts by 12 in both glibc and kernel. I don't think the shifts are a problem, the main downside would be the limit to 44 bits of file offsets (16TB files), but it's also unclear if that is a practical problem at all. If it is, we run into the same problem on all other 32-bit architectures too. Arnd
[toc] | [prev] | [next] | [standalone]
| From | Catalin Marinas <catalin.marinas@arm.com> |
|---|---|
| Date | 2016-05-12 11:20 +0200 |
| Message-ID | <rxUWK-2AG-3@gated-at.bofh.it> |
| In reply to | #1399448 |
On Wed, May 11, 2016 at 09:30:07PM +0200, Arnd Bergmann wrote: > On Wednesday 11 May 2016 17:59:01 Catalin Marinas wrote: > > On Wed, May 11, 2016 at 12:55:01PM +0200, Arnd Bergmann wrote: > > > On Wednesday 11 May 2016 11:04:38 Yury Norov wrote: > > > > On Wed, May 11, 2016 at 10:04:16AM +0800, Zhangjian (Bamvor) wrote: > > > > [...] > > > > > > > > > >>Ok, I will test the ltp syscall test. > > > > > >>With this changes, the issue I mentioned should be fixed. But we still > > > > > >>use mmap2 syscall for ILP32 application when we pass the offset instead > > > > > >>of page offset. Is it correct? > > > > > > > > > > > >I don't remember. It's probably not important whether we have the shift > > > > > >in there, as long as it's independent of the actual kernel page size and > > > > > >user space and kernel agree on the calling conventions. > > > > > Well. I am ok with where to shift the pages size because we get the same > > > > > result. I was just thinking if we should get rid of the name of mmap2 in our > > > > > ILP32 porting. Actually, it is mmap but we name it as mmap2. User may confused > > > > > if they do not know the implementations. > > > > > > > > > > > > > This is what generic unistd.h does. If you want to change it, you'd > > > > change each arch that uses generic unistd.h. > > > > > > Generic unistd.h has this: > > > > > > #ifdef __SYSCALL_COMPAT > > > #define __SC_COMP_3264(_nr, _32, _64, _comp) __SYSCALL(_nr, _comp) > > > #else > > > #define __SC_COMP_3264(_nr, _32, _64, _comp) __SC_3264(_nr, _32, _64) > > > #endif > > > > > > #define __NR3264_mmap 222 > > > __SC_3264(__NR3264_mmap, sys_mmap2, sys_mmap) > > > > > > > > > #if __BITS_PER_LONG == 64 && !defined(__SYSCALL_COMPAT) > > > #define __NR_mmap __NR3264_mmap > > > #else > > > #define __NR_mmap2 __NR3264_mmap > > > #endif > > > > > > So by default we get __NR_mmap2 and sys_mmap2 on 32-bit ABIs, but > > > __NR_mmap and sys_mmap on 64-bit ABIs, as it should be. > > > > > > The problem is that arch/arm64/kernel/sys_ilp32.c now overrides > > > this to use __NR_mmap2 with sys_mmap, so we have a mismatch. I think > > > we should either override both the implementation and the number, > > > or neither of them. > > > > I would vote for "neither of them" (so we use __NR_mmap2 and sys_mmap2) > > to keep it close to new 32-bit architectures, even though we would have > > some shifts by 12 in both glibc and kernel. > > I don't think the shifts are a problem, the main downside would be > the limit to 44 bits of file offsets (16TB files), but it's also > unclear if that is a practical problem at all. If it is, we run > into the same problem on all other 32-bit architectures too. I hope people are seriously thinking of moving to an LP64 ABI if they have such large file offset needs. -- Catalin
[toc] | [prev] | [next] | [standalone]
| From | Arnd Bergmann <arnd@arndb.de> |
|---|---|
| Date | 2016-05-12 11:30 +0200 |
| Subject | Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it |
| Message-ID | <rxV6q-2GU-7@gated-at.bofh.it> |
| In reply to | #1399813 |
On Thursday 12 May 2016 10:17:58 Catalin Marinas wrote: > On Wed, May 11, 2016 at 09:30:07PM +0200, Arnd Bergmann wrote: > > On Wednesday 11 May 2016 17:59:01 Catalin Marinas wrote: > > > > I don't think the shifts are a problem, the main downside would be > > the limit to 44 bits of file offsets (16TB files), but it's also > > unclear if that is a practical problem at all. If it is, we run > > into the same problem on all other 32-bit architectures too. > > I hope people are seriously thinking of moving to an LP64 ABI if they > have such large file offset needs. Good point. 44 bits of file size is certainly enough for mmap() on a 32-bit task: you would only be able to map a very small fraction of the file anyway, and if you want to map larger files, and should move to 64-bit tasks long before this becomes a limitation. Arnd
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web