Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it

[Yury Norov]

On Wed, May 18, 2016 at 12:21:46PM +0100, Catalin Marinas wrote:
> On Tue, May 17, 2016 at 10:05:26PM +0300, Yury Norov wrote:
> > On Mon, May 16, 2016 at 06:06:05PM +0100, Catalin Marinas wrote:
> > > On Sat, May 14, 2016 at 06:03:52PM +0300, Yury Norov wrote:
> > > > +SYSCALL_DEFINE6(mmap2, unsigned long, addr, unsigned long, len,
> > > > +   unsigned long, prot, unsigned long, flags, unsigned long, fd,
> > > > +   unsigned long, pgoff)
> > > 
> > > To avoid the types confusion we could add __SC_WRAP to mmap2 in unistd.h
> > > and use COMPAT_SYSCALL_DEFINE6 here (together with compat_ptr_t etc.).
> > > 
> > > > +{
> > > > +   if (pgoff & (~PAGE_MASK >> 12))
> > > > +   return -EINVAL;
> > > > +
> > > > +   return sys_mmap_pgoff((compat_uptr_t) addr, (compat_size_t) len,
> > > > +  (int) prot, (int) flags, (int) fd,
> > > > +  pgoff >> (PAGE_SHIFT-12));
> > > 
> > > Then we wouldn't need the explicit casting here.
> > 
> > See below
> > 
> > > > +}
> > > > +
> > > > +COMPAT_SYSCALL_DEFINE4(pread64, unsigned int, fd, compat_uptr_t __user 
> > > > *, ubuf,
> > > > +  compat_size_t, count, off_t, offset)
> > > > +{
> > > > +   return sys_pread64(fd, (char *) ubuf, count, offset);
> > > > +}
> > > > +
> > > > +COMPAT_SYSCALL_DEFINE4(pwrite64, unsigned int, fd, compat_uptr_t 
> > > > __user *, ubuf,
> > > > +  compat_size_t, count, off_t, offset)
> > > > +{
> > > > +   return sys_pwrite64(fd, (char *) ubuf, count, offset);
> > > 
> > > Nitpick: no space between cast type and variable name: (char *)ubuf, ...
> > 
> > I think it's really a matter of taste. I prefer to have a space, and
> > there's no solid rule in coding style.
> > 
> > And there are 13032 insertions of my version vs 35030 of yours:
> > ~/work/linux$ git grep ' \*)[a-zA-Z]'|wc -l
> > 35030
> > ~/work/linux$ git grep ' \*) [a-zA-Z]'|wc -l
> > 13032
> > 
> > Of course, I will change it if you insist.
> 
> Not really, I thought it's covered by CodingStyle but it doesn't seem
> to.
> 
> > > We can also make these functions static as they are not used outside
> > > this file.
> > 
> > If it's OK, we can use just compat_sys_xxx() instead of
> > COMPAT_SYSCALL_DEFINEx(xxx),
> 
> I got lost in macros, what difference would COMPAT_SYSCALL_DEFINE vs
> compat_sys_*() make? Is this the delouse stuff?
> 

Hmm... I looked again. It seems that COMPAT delouses all arguments,
including off_t, and that's what we try to avoid. So we *should* use
naked functions. 

include/linux/compat.h:
53 #define COMPAT_SYSCALL_DEFINEx(x, name, ...) \
54 asmlinkage long compat_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__))\
55  __attribute__((alias(__stringify(compat_SyS##name;  \
56 static inline long C_SYSC##name(__MAP(x,__SC_DECL,__VA_ARGS__));\
57 asmlinkage long compat_SyS##name(__MAP(x,__SC_LONG,__VA_ARGS__));\
58 asmlinkage long compat_SyS##name(__MAP(x,__SC_LONG,__VA_ARGS__))\
59 { \
60 return C_SYSC##name(__MAP(x,__SC_DELOUSE,__VA_ARGS__)); \
61 } \
62 static inline long C_SYSC##name(__MAP(x,__SC_DECL,__VA_ARGS__))


> > and for mmap2 we'll not need to change _SYSCALL to _SC_WRAP in
> > unistd.h that way.
> 
> Looking at the generic unistd.h, adding _SC_WRAP for sys_mmap2 is
> indeed not easy:
> 
> #define __NR3264_mmap 222
> __SC_3264(__NR3264_mmap, sys_mmap2, sys_mmap)
> 
> So defining a compat_sys_mmap2() would work but I think you'd also need:
> 

> #define sys_mmap2 compat_sys_mmap2()

OK.

> 
> -- 
> Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it

[Yury Norov]

On Mon, May 16, 2016 at 06:06:05PM +0100, Catalin Marinas wrote:
> On Sat, May 14, 2016 at 06:03:52PM +0300, Yury Norov wrote:
> > +SYSCALL_DEFINE6(mmap2, unsigned long, addr, unsigned long, len,
> > +   unsigned long, prot, unsigned long, flags, unsigned long, fd,
> > +   unsigned long, pgoff)
> 
> To avoid the types confusion we could add __SC_WRAP to mmap2 in unistd.h
> and use COMPAT_SYSCALL_DEFINE6 here (together with compat_ptr_t etc.).
> 
> > +{
> > +   if (pgoff & (~PAGE_MASK >> 12))
> > +   return -EINVAL;
> > +
> > +   return sys_mmap_pgoff((compat_uptr_t) addr, (compat_size_t) len,
> > +  (int) prot, (int) flags, (int) fd,
> > +  pgoff >> (PAGE_SHIFT-12));
> 
> Then we wouldn't need the explicit casting here.

See below

> 
> > +}
> > +
> > +COMPAT_SYSCALL_DEFINE4(pread64, unsigned int, fd, compat_uptr_t __user *, 
> > ubuf,
> > +  compat_size_t, count, off_t, offset)
> > +{
> > +   return sys_pread64(fd, (char *) ubuf, count, offset);
> > +}
> > +
> > +COMPAT_SYSCALL_DEFINE4(pwrite64, unsigned int, fd, compat_uptr_t __user *, 
> > ubuf,
> > +  compat_size_t, count, off_t, offset)
> > +{
> > +   return sys_pwrite64(fd, (char *) ubuf, count, offset);
> 
> Nitpick: no space between cast type and variable name: (char *)ubuf, ...

I think it's really a matter of taste. I prefer to have a space, and
there's no solid rule in coding style.

And there are 13032 insertions of my version vs 35030 of yours:
~/work/linux$ git grep ' \*)[a-zA-Z]'|wc -l
35030
~/work/linux$ git grep ' \*) [a-zA-Z]'|wc -l
13032

Of course, I will change it if you insist.

> We can also make these functions static as they are not used outside
> this file.

If it's OK, we can use just compat_sys_xxx() instead of
COMPAT_SYSCALL_DEFINEx(xxx), and for mmap2 we'll not need to change 
_SYSCALL to _SC_WRAP in unistd.h that way.

> > -- 
> Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it

[Catalin Marinas]

On Sat, May 14, 2016 at 06:03:52PM +0300, Yury Norov wrote:
> +SYSCALL_DEFINE6(mmap2, unsigned long, addr, unsigned long, len,
> +   unsigned long, prot, unsigned long, flags, unsigned long, fd,
> +   unsigned long, pgoff)

To avoid the types confusion we could add __SC_WRAP to mmap2 in unistd.h
and use COMPAT_SYSCALL_DEFINE6 here (together with compat_ptr_t etc.).

> +{
> +   if (pgoff & (~PAGE_MASK >> 12))
> +   return -EINVAL;
> +
> +   return sys_mmap_pgoff((compat_uptr_t) addr, (compat_size_t) len,
> +(int) prot, (int) flags, (int) fd,
> +pgoff >> (PAGE_SHIFT-12));

Then we wouldn't need the explicit casting here.

> +}
> +
> +COMPAT_SYSCALL_DEFINE4(pread64, unsigned int, fd, compat_uptr_t __user *, 
> ubuf,
> +compat_size_t, count, off_t, offset)
> +{
> + return sys_pread64(fd, (char *) ubuf, count, offset);
> +}
> +
> +COMPAT_SYSCALL_DEFINE4(pwrite64, unsigned int, fd, compat_uptr_t __user *, 
> ubuf,
> +compat_size_t, count, off_t, offset)
> +{
> + return sys_pwrite64(fd, (char *) ubuf, count, offset);

Nitpick: no space between cast type and variable name: (char *)ubuf, ...

We can also make these functions static as they are not used outside
this file.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it

[Yury Norov]

So, after all discussions, this patch is looking like this.

Changes:
 - assembler part reworked to be more clear, as Catalin recommended;
 - mmap now points to mmap2, as in 1st versions (suggested by Bamvor),
   and ilp32 wrapper delouses required arguments;
 - pread64 and pwrite64 wrappers introduced to delouse args as well;
 - removed unneeded #undefs;

Did I miss something?

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 .
  */
+
+#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..0f651bc 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -500,6 +500,7 @@ el0_svc_compat:
 * AArch32 syscall handling
 */
adrpstbl, compat_sys_call_table // load compat syscall table 
pointer
+   ldr x16, [tsk, #TI_FLAGS]
uxtwscno, w7// syscall number in w7 (r7)
mov sc_nr, #__NR_compat_syscalls
b   el0_svc_naked
@@ -716,15 +717,20 @@ ENDPROC(ret_from_fork)
.align  6
 el0_svc:
adrpstbl, sys_call_table// load syscall table pointer
+   ldr x16, [tsk, #TI_FLAGS]
uxtwscno, w8// syscall number in w8
mov sc_nr, #__NR_syscalls
+#ifdef CONFIG_ARM64_ILP32
+   adrpx17, sys_call_ilp32_table   // load ilp32 syscall table 
pointer
+   tst x16, #_TIF_32BIT_AARCH64
+   cselstbl, stbl, x17, eq // We are using ILP32
+#endif
 el0_svc_naked: // compat entry point
stp x0, scno, [sp, #S_ORIG_X0]  // save the original x0 and 
syscall number
enable_dbg_and_irq
ct_user_exit 1
 
-   ldr x16, [tsk, #TI_FLAGS]   // check for syscall hooks
-   tst x16, #_TIF_SYSCALL_WORK
+   tst x16, #_TIF_SYSCALL_WORK // check for syscall hooks
b.ne__sys_trace
cmp scno, sc_nr // check upper syscall limit
b.hsni_sys
diff --git a/arch/arm64/kernel/sys_ilp32.c b/arch/arm64/kernel/sys_ilp32.c
new file mode 100644
index 000..64db612
--- /dev/null
+++ b/arch/arm64/kernel/sys_ilp32.c
@@ -0,0 +1,84 @@
+/*
+ * AArch64- ILP32 specific system calls implementation
+ *
+ * Copyright (C) 2016 Cavium Inc.
+ * Author: Andrew Pinski 
+ *
+ * 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 .
+ */
+
+#define __SYSCALL_COMPAT
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* Using non-compat syscalls where necessary */
+#define compat_sys_fadvise64_64sys_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_readahead   sys_readahead
+#define compat_sys_shmat   sys_shmat
+#define 

Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it

[Yury Norov]

On Thu, May 12, 2016 at 04:19:48PM +0300, Yury Norov wrote:

[...]

> > I think that's a good idea. The function used to be slightly different
> > for each architecture, but now it seems we have a significant number
> > of identical implementations that we could just merge them together
> > into one.
> > 
> > sys_mmap_pgoff was originally introduced as the common implementation
> > and it reduced the amount of duplication a lot, but as its units
> > are based on PAGE_SIZE rather than hardwired 4096 bytes, it's
> > not as useful.
> > 
> 
> microblaze and mips (twice) are doing like this. And aarh32 as well,
> in arch/arm64/kernel/entry32.S
> 
> In previous submissions it was a patch that shares aarch32 code to
> ilp32. If we decided turn around again, I think, we'd pick up that patch.
> 
> The other option is to make this hack generic, as so many arches use it.

Hi again,

I picked up that old patch from Jan, and found that it doesn't delouse
addr and length, which is wrong for ilp32. So now I think we'd pick
Bamvor version.

Jan's patch (rebased) is attached for reference.

Yury.


>From 02ad258662c40d457ac041634e67e1cbdbb800f3 Mon Sep 17 00:00:00 2001
From: Jan Dakinevich 
Date: Tue, 3 Nov 2015 02:30:41 +0300
Subject: ilp32: common 32-bit wrappers

Signed-off-by: Jan Dakinevich 
Signed-off-by: Yury Norov 

diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
index 9dfdf86..2319042 100644
--- a/arch/arm64/kernel/Makefile
+++ b/arch/arm64/kernel/Makefile
@@ -30,6 +30,7 @@ arm64-obj-$(CONFIG_AARCH32_EL0)   += sys32.o 
kuser32.o signal32.o \
   ../../arm/kernel/opcodes.o 
binfmt_elf32.o
 arm64-obj-$(CONFIG_ARM64_ILP32)+= binfmt_ilp32.o
 arm64-obj-$(CONFIG_FUNCTION_TRACER)+= ftrace.o entry-ftrace.o
+arm64-obj-$(CONFIG_COMPAT) += entry32-common.o
 arm64-obj-$(CONFIG_MODULES)+= arm64ksyms.o module.o
 arm64-obj-$(CONFIG_ARM64_MODULE_PLTS)  += module-plts.o
 arm64-obj-$(CONFIG_PERF_EVENTS)+= perf_regs.o perf_callchain.o
diff --git a/arch/arm64/kernel/entry32-common.S 
b/arch/arm64/kernel/entry32-common.S
new file mode 100644
index 000..1a0abe3
--- /dev/null
+++ b/arch/arm64/kernel/entry32-common.S
@@ -0,0 +1,23 @@
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+
+/*
+ * Note: off_4k (w5) is always in units of 4K. If we can't do the
+ * requested offset because it is not page-aligned, we return -EINVAL.
+ */
+ENTRY(compat_sys_mmap2_wrapper)
+#if PAGE_SHIFT > 12
+   tst w5, #~PAGE_MASK >> 12
+   b.ne1f
+   lsr w5, w5, #PAGE_SHIFT - 12
+#endif
+   b   sys_mmap_pgoff
+1: mov x0, #-EINVAL
+   ret
+ENDPROC(compat_sys_mmap2_wrapper)
+
diff --git a/arch/arm64/kernel/entry32.S b/arch/arm64/kernel/entry32.S
index f332d5d..58cb5b0 100644
--- a/arch/arm64/kernel/entry32.S
+++ b/arch/arm64/kernel/entry32.S
@@ -55,21 +55,6 @@ ENTRY(compat_sys_fstatfs64_wrapper)
 ENDPROC(compat_sys_fstatfs64_wrapper)
 
 /*
- * Note: off_4k (w5) is always in units of 4K. If we can't do the
- * requested offset because it is not page-aligned, we return -EINVAL.
- */
-ENTRY(compat_sys_mmap2_wrapper)
-#if PAGE_SHIFT > 12
-   tst w5, #~PAGE_MASK >> 12
-   b.ne1f
-   lsr w5, w5, #PAGE_SHIFT - 12
-#endif
-   b   sys_mmap_pgoff
-1: mov x0, #-EINVAL
-   ret
-ENDPROC(compat_sys_mmap2_wrapper)
-
-/*
  * Wrappers for AArch32 syscalls that either take 64-bit parameters
  * in registers or that take 32-bit parameters which require sign
  * extension.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it

[Yury Norov]

On Thu, May 12, 2016 at 03:06:39PM +0200, Arnd Bergmann wrote:
> On Thursday 12 May 2016 20:49:24 Zhangjian wrote:
> > Hi,
> > 
> > On 2016/5/12 17:21, Arnd Bergmann wrote:
> > > 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.
> > Hi,
> > 
> > I apply the following patch in order to make use of the REAL mmmap2. LTP
> > test pass in litle endian. mmap16 successful with segfault in big endian.
> > 
> > BTW, I saw the similar code in tile, mips, microblaze and s390 compat. 
> > Should
> > we merge these code into a common syscall wrapper?
> 
> I think that's a good idea. The function used to be slightly different
> for each architecture, but now it seems we have a significant number
> of identical implementations that we could just merge them together
> into one.
> 
> sys_mmap_pgoff was originally introduced as the common implementation
> and it reduced the amount of duplication a lot, but as its units
> are based on PAGE_SIZE rather than hardwired 4096 bytes, it's
> not as useful.
> 

microblaze and mips (twice) are doing like this. And aarh32 as well,
in arch/arm64/kernel/entry32.S

In previous submissions it was a patch that shares aarch32 code to
ilp32. If we decided turn around again, I think, we'd pick up that patch.

The other option is to make this hack generic, as so many arches use it.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it

[Arnd Bergmann]

On Thursday 12 May 2016 20:49:24 Zhangjian wrote:
> Hi,
> 
> On 2016/5/12 17:21, Arnd Bergmann wrote:
> > 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.
> Hi,
> 
> I apply the following patch in order to make use of the REAL mmmap2. LTP
> test pass in litle endian. mmap16 successful with segfault in big endian.
> 
> BTW, I saw the similar code in tile, mips, microblaze and s390 compat. Should
> we merge these code into a common syscall wrapper?

I think that's a good idea. The function used to be slightly different
for each architecture, but now it seems we have a significant number
of identical implementations that we could just merge them together
into one.

sys_mmap_pgoff was originally introduced as the common implementation
and it reduced the amount of duplication a lot, but as its units
are based on PAGE_SIZE rather than hardwired 4096 bytes, it's
not as useful.

> diff --git a/arch/arm64/kernel/sys_ilp32.c b/arch/arm64/kernel/sys_ilp32.c
> index d85fe94..2cd72eb 100644
> --- a/arch/arm64/kernel/sys_ilp32.c
> +++ b/arch/arm64/kernel/sys_ilp32.c
> @@ -41,7 +41,16 @@
>   #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
> +
> +SYSCALL_DEFINE6(mmap2, unsigned long, addr, unsigned long, len,
> +   unsigned long, prot, unsigned long, flags, unsigned long, fd,
> +   unsigned long, pgoff)
> +{
> +   if (pgoff & (~PAGE_MASK >> 12))
> +   return -EINVAL;
> +
> +   return sys_mmap_pgoff(addr, len, prot, flags, fd, pgoff >> 
> (PAGE_SHIFT-12));
> +}
> 

Looks good to me.

Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it

[Zhangjian (Bamvor)]

Hi,

On 2016/5/12 16:24, Yury Norov wrote:

On Thu, May 12, 2016 at 11:45:53AM +0800, Zhangjian (Bamvor) wrote:

[...]


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));  \


I think it's wrong, as loff_t is 64-bit in 32-bit userspace, and this
will clear meaningful data in top halve.

Yes. It is my fault. The original thoughts is clear the up 32bit for size_t.
How should we skip the loff_t?

Regards

Bamvor

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it

[Zhangjian (Bamvor)]

Hi,

On 2016/5/12 17:21, Arnd Bergmann wrote:

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.

Hi,

I apply the following patch in order to make use of the REAL mmmap2. LTP
test pass in litle endian. mmap16 successful with segfault in big endian.

BTW, I saw the similar code in tile, mips, microblaze and s390 compat. Should
we merge these code into a common syscall wrapper?

kernel:
diff --git a/arch/arm64/kernel/sys_ilp32.c b/arch/arm64/kernel/sys_ilp32.c
index d85fe94..2cd72eb 100644
--- a/arch/arm64/kernel/sys_ilp32.c
+++ b/arch/arm64/kernel/sys_ilp32.c
@@ -41,7 +41,16 @@
 #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
+
+SYSCALL_DEFINE6(mmap2, unsigned long, addr, unsigned long, len,
+   unsigned long, prot, unsigned long, flags, unsigned long, fd,
+   unsigned long, pgoff)
+{
+   if (pgoff & (~PAGE_MASK >> 12))
+   return -EINVAL;
+
+   return sys_mmap_pgoff(addr, len, prot, flags, fd, pgoff >> 
(PAGE_SHIFT-12));
+}

 asmlinkage long ilp32_sys_rt_sigreturn_wrapper(void);
 #define compat_sys_rt_sigreturnilp32_sys_rt_sigreturn_wrapper

glibc:
diff --git a/sysdeps/unix/sysv/linux/aarch64/ilp32/mmap.c 
b/sysdeps/unix/sysv/linux/aarch64/ilp32/mmap.c
index e69de29..f75e251 100644
--- a/sysdeps/unix/sysv/linux/aarch64/ilp32/mmap.c
+++ b/sysdeps/unix/sysv/linux/aarch64/ilp32/mmap.c
@@ -0,0 +1,2 @@
+#include 
+
diff --git a/sysdeps/unix/sysv/linux/aarch64/ilp32/mmap64.c 
b/sysdeps/unix/sysv/linux/aarch64/ilp32/mmap64.c
index c6c7f1d..6f1a141 100644
--- a/sysdeps/unix/sysv/linux/aarch64/ilp32/mmap64.c
+++ b/sysdeps/unix/sysv/linux/aarch64/ilp32/mmap64.c
@@ -1,29 +1 @@
-#include 
-#include 
-#include 
-#include 
-
-#include 
-#include 
-
-/* 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,
-   return result;
-}
-/* mmap64 is provided by mmap as they are the same. */
-void *__mmap64 (void *__addr, size_t __len, int __prot,
-int __flags, int __fd, __off64_t __offset)
-{
-   void *result;
-   result = (void *)
- INLINE_SYSCALL (mmap2, 6, __addr,
- __len, __prot, __flags, __fd, __offset);
-   return result;
-}
-weak_alias (__mmap, mmap)
-weak_alias (__mmap64, mmap64)
+#include 

Regards

Bamvor

Arnd



--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it

[Arnd Bergmann]

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
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it

[Catalin Marinas]

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
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it

[Yury Norov]

On Thu, May 12, 2016 at 11:45:53AM +0800, Zhangjian (Bamvor) wrote:

[...]

> >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));  \

I think it's wrong, as loff_t is 64-bit in 32-bit userspace, and this
will clear meaningful data in top halve.

> ((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);

For cases like this I think we should write wrappers by hands. In
unistd.h we can use __SC_WRAP, so they will work like wrappers
generated by COMPAT_SYSCALL_WRAPx() 
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it

[Zhangjian (Bamvor)]

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 .
*/
   #include 
+
+#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, 

Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it

[Arnd Bergmann]

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
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it

[Catalin Marinas]

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
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it

[Arnd Bergmann]

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 .
> >*/
> >   #include 
> > +
> > +#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
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it

[Zhangjian (Bamvor)]

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 .
   */
  #include 
+
+#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
 >



--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it

[Zhangjian (Bamvor)]

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 .
  */
 #include 
+
+#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
>

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it

[Yury Norov]

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.

> Regards
> 
> Bamvor
> 
> >
> > Arnd
> >
> 
> 
> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it

[Zhangjian (Bamvor)]

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 

 #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_T1
#define __INO_T_MATCHES_INO64_T1

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



--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it

[Arnd Bergmann]

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 
> 
>#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_T1
#define __INO_T_MATCHES_INO64_T1

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
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it

[Zhangjian (Bamvor)]

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 

  #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 

 #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



--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it

[Arnd Bergmann]

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
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it

[Zhangjian (Bamvor)]

Hi,

Sorry I forget to paste my test code:

#include 
#include 
#include 
#include 

#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([1][2], NULL, 16);
else
offset = atoi(argv[1]);

if (argv[2][0] == '0' && argv[2][1] == 'x')
size = strtoll([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 

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 
Signed-off-by: Yury Norov 
---
  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 .
   */
+
+#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)
   */
  .align6
  el0_svc:
-adrpstbl, sys_call_table// load syscall table pointer
  uxtwscno, w8// syscall number in w8
  movsc_nr, #__NR_syscalls
+#ifdef CONFIG_ARM64_ILP32
+ldrx16, [tsk, #TI_FLAGS]
+tbnzx16, #TIF_32BIT_AARCH64, el0_ilp32_svc // We are using ILP32
+#endif
+adrpstbl, sys_call_table// load syscall table pointer
  el0_svc_naked:// compat entry point
  stpx0, scno, [sp, #S_ORIG_X0]// save the original x0 and syscall 
number
  enable_dbg_and_irq
@@ -737,6 +741,12 @@ ni_sys:
  bret_fast_syscall
  ENDPROC(el0_svc)

+#ifdef CONFIG_ARM64_ILP32
+el0_ilp32_svc:
+adrpstbl, 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 

Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it

[Zhangjian (Bamvor)]

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 

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 
Signed-off-by: Yury Norov 
---
  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 .
   */
+
+#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:
-   adrpstbl, sys_call_table// load syscall table pointer
uxtwscno, w8// syscall number in w8
mov sc_nr, #__NR_syscalls
+#ifdef CONFIG_ARM64_ILP32
+   ldr x16, [tsk, #TI_FLAGS]
+   tbnzx16, #TIF_32BIT_AARCH64, el0_ilp32_svc // We are using ILP32
+#endif
+   adrpstbl, 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:
+   adrpstbl, 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 000..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 
+ *
+ * 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 .
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* Using non-compat syscalls where necessary */
+#define compat_sys_fadvise64_64sys_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 

Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it

[Zhangjian (Bamvor)]

Hi,

On 2016/4/6 6:08, Yury Norov wrote:

From: Andrew Pinski 

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 
Signed-off-by: Yury Norov 
---
  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 .
   */
+
+#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:
-   adrpstbl, sys_call_table// load syscall table pointer
uxtwscno, w8// syscall number in w8
mov sc_nr, #__NR_syscalls
+#ifdef CONFIG_ARM64_ILP32
+   ldr x16, [tsk, #TI_FLAGS]
+   tbnzx16, #TIF_32BIT_AARCH64, el0_ilp32_svc // We are using ILP32
+#endif
+   adrpstbl, 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:
+   adrpstbl, 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 000..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 
+ *
+ * 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 .
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* Using non-compat syscalls where necessary */
+#define compat_sys_fadvise64_64sys_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_pwrite64sys_pwrite64
+#define compat_sys_readahead   sys_readahead
+#define compat_sys_shmat 

Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it

[Arnd Bergmann]

On Friday 29 April 2016 16:13:38 Yury Norov wrote:
> On Fri, Apr 29, 2016 at 12:43:41AM +0200, Arnd Bergmann wrote:
> > On Friday 29 April 2016 01:21:37 Yury Norov wrote:

> > > 
> > > arch/arm64/kernel/sys_ilp32.c:59:0: warning: "__SYSCALL" redefined
> > >  #define __SYSCALL(nr, sym) [nr] = sym,
> > >  ^
> > > In file included from include/asm-generic/unistd.h:1:0,
> > > 
> > 
> > Ok, I think I see it now. Can you #undef the two symbols at the
> > end of arch/arm64/include/uapi/asm/unistd.h
> 
> I think it doesn't look better than what we have now, but not worse
> as well. If you like it, I'll change.

I looked again and saw that the existing architectures also #undef __SYSCALL,
and they don't have __SC_WRAP. It's probably fine to just #undef the
two here (don't undef SC_COMP, __SC_3264 and SC_COMP_3264).

Changing the asm-generic header to not require the #undef would be nice,
but then we should do that for all 12 users of that file.

Arnd

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it

[Yury Norov]

On Fri, Apr 29, 2016 at 12:43:41AM +0200, Arnd Bergmann wrote:
> On Friday 29 April 2016 01:21:37 Yury Norov wrote:
> > index 1458ad7..410d817 100644
> > --- a/arch/arm64/kernel/sys_ilp32.c
> > +++ b/arch/arm64/kernel/sys_ilp32.c
> > @@ -17,6 +17,8 @@
> >  * along with this program.  If not, see
> >  * .
> >  */
> > 
> > +#define __SYSCALL_COMPAT
> > +
> >  #include 
> >  #include 
> >  #include 
> > @@ -48,13 +50,12 @@ asmlinkage long
> >  ilp32_sys_rt_sigreturn_wrapper(void);
> >
> >  #include 
> >  
> > -#undef __SYSCALL
> > -#undef __SC_COMP
> > -#undef __SC_WRAP
> > -#undef __SC_3264
> > -#undef __SC_COMP_3264
> >  
> > -#define __SYSCALL_COMPAT
> >  #define __SYSCALL(nr, sym) [nr] = sym,
> >  #define __SC_WRAP(nr, sym) [nr] = compat_##sym,
> >  
> > This patch makes gcc warn about redefinition.
> > 
> > arch/arm64/kernel/sys_ilp32.c:59:0: warning: "__SYSCALL" redefined
> >  #define __SYSCALL(nr, sym) [nr] = sym,
> >  ^
> > In file included from include/asm-generic/unistd.h:1:0,
> > 
> 
> Ok, I think I see it now. Can you #undef the two symbols at the
> end of arch/arm64/include/uapi/asm/unistd.h

I think it doesn't look better than what we have now, but not worse
as well. If you like it, I'll change.

> or possibly
> include/uapi/asm-generic/unistd.h?
> 
>   Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it

[Arnd Bergmann]

On Friday 29 April 2016 01:21:37 Yury Norov wrote:
> index 1458ad7..410d817 100644
> --- a/arch/arm64/kernel/sys_ilp32.c
> +++ b/arch/arm64/kernel/sys_ilp32.c
> @@ -17,6 +17,8 @@
>  * along with this program.  If not, see
>  * .
>  */
> 
> +#define __SYSCALL_COMPAT
> +
>  #include 
>  #include 
>  #include 
> @@ -48,13 +50,12 @@ asmlinkage long
>  ilp32_sys_rt_sigreturn_wrapper(void);
>
>  #include 
>  
> -#undef __SYSCALL
> -#undef __SC_COMP
> -#undef __SC_WRAP
> -#undef __SC_3264
> -#undef __SC_COMP_3264
>  
> -#define __SYSCALL_COMPAT
>  #define __SYSCALL(nr, sym) [nr] = sym,
>  #define __SC_WRAP(nr, sym) [nr] = compat_##sym,
>  
> This patch makes gcc warn about redefinition.
> 
> arch/arm64/kernel/sys_ilp32.c:59:0: warning: "__SYSCALL" redefined
>  #define __SYSCALL(nr, sym) [nr] = sym,
>  ^
> In file included from include/asm-generic/unistd.h:1:0,
> 

Ok, I think I see it now. Can you #undef the two symbols at the
end of arch/arm64/include/uapi/asm/unistd.h or possibly
include/uapi/asm-generic/unistd.h?

Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it

[Yury Norov]

On Thu, Apr 28, 2016 at 10:43:59PM +0200, Arnd Bergmann wrote:
> On Thursday 28 April 2016 22:19:14 Yury Norov wrote:
> > 
> > Yes, we need. Otherwise we have circular dependency like this:
> > arch/arm64/kernel/sys_ilp32.c:60:0: warning: "__SC_WRAP" redefined
> >  #define __SC_WRAP(nr, sym) [nr] = compat_##sym,
> >   ^
> >   In file included from include/asm-generic/unistd.h:1:0,
> >from ./arch/arm64/include/uapi/asm/unistd.h:16,
> >from ./arch/arm64/include/asm/unistd.h:62,
> >from ./include/uapi/linux/unistd.h:7,
> >from include/linux/syscalls.h:23,
> >from arch/arm64/kernel/sys_ilp32.c:30:
> > include/uapi/asm-generic/unistd.h:33:0: note: this is the location of the 
> > previous definition
> >  #define __SC_WRAP __SYSCALL
> > 
> > Defining __SYSCALL_COMPAT at the top of the file does not help much.
> 
> Hmm, this sounds like something that we should fix in the asm-generic/unistd.h
> file. Is it just for __SC_WRAP, or also the other macros?
> 
>   Arnd

For __SYSCALL and __SC_WRAP:

diff --git a/arch/arm64/kernel/sys_ilp32.c
b/arch/arm64/kernel/sys_ilp32.c
index 1458ad7..410d817 100644
--- a/arch/arm64/kernel/sys_ilp32.c
+++ b/arch/arm64/kernel/sys_ilp32.c
@@ -17,6 +17,8 @@
 * along with this program.  If not, see
 * .
 */

+#define __SYSCALL_COMPAT
+
 #include 
 #include 
 #include 
@@ -48,13 +50,12 @@ asmlinkage long
 ilp32_sys_rt_sigreturn_wrapper(void);
   
 #include 
 
-#undef __SYSCALL
-#undef __SC_COMP
-#undef __SC_WRAP
-#undef __SC_3264
-#undef __SC_COMP_3264
 
-#define __SYSCALL_COMPAT
 #define __SYSCALL(nr, sym) [nr] = sym,
 #define __SC_WRAP(nr, sym) [nr] = compat_##sym,
 
This patch makes gcc warn about redefinition.

arch/arm64/kernel/sys_ilp32.c:59:0: warning: "__SYSCALL" redefined
 #define __SYSCALL(nr, sym) [nr] = sym,
 ^
In file included from include/asm-generic/unistd.h:1:0,
 from ./arch/arm64/include/uapi/asm/unistd.h:16,
 from ./arch/arm64/include/asm/unistd.h:62,
 from ./include/uapi/linux/unistd.h:7,
 from include/linux/syscalls.h:23,
 from arch/arm64/kernel/sys_ilp32.c:30:
include/uapi/asm-generic/unistd.h:15:0: note: this is the location of the 
previous definition
 #define __SYSCALL(x, y)
 ^
arch/arm64/kernel/sys_ilp32.c:60:0: warning: "__SC_WRAP" redefined
 #define __SC_WRAP(nr, sym) [nr] = compat_##sym,
 ^
In file included from include/asm-generic/unistd.h:1:0,
 from ./arch/arm64/include/uapi/asm/unistd.h:16,
 from ./arch/arm64/include/asm/unistd.h:62,
 from ./include/uapi/linux/unistd.h:7,
 from include/linux/syscalls.h:23,
 from arch/arm64/kernel/sys_ilp32.c:30:
include/uapi/asm-generic/unistd.h:33:0: note: this is the location of the 
previous definition
 #define __SC_WRAP __SYSCALL
 ^
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it

[Arnd Bergmann]

On Thursday 28 April 2016 22:19:14 Yury Norov wrote:
> 
> Yes, we need. Otherwise we have circular dependency like this:
> arch/arm64/kernel/sys_ilp32.c:60:0: warning: "__SC_WRAP" redefined
>  #define __SC_WRAP(nr, sym) [nr] = compat_##sym,
>   ^
>   In file included from include/asm-generic/unistd.h:1:0,
>from ./arch/arm64/include/uapi/asm/unistd.h:16,
>from ./arch/arm64/include/asm/unistd.h:62,
>from ./include/uapi/linux/unistd.h:7,
>from include/linux/syscalls.h:23,
>from arch/arm64/kernel/sys_ilp32.c:30:
> include/uapi/asm-generic/unistd.h:33:0: note: this is the location of the 
> previous definition
>  #define __SC_WRAP __SYSCALL
> 
> Defining __SYSCALL_COMPAT at the top of the file does not help much.

Hmm, this sounds like something that we should fix in the asm-generic/unistd.h
file. Is it just for __SC_WRAP, or also the other macros?

Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it

[Yury Norov]

On Tue, Apr 26, 2016 at 05:57:01PM +0100, Catalin Marinas wrote:
> On Wed, Apr 06, 2016 at 01:08:42AM +0300, Yury Norov wrote:
> > +/* Using non-compat syscalls where necessary */
> > +#define compat_sys_fadvise64_64sys_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_pwrite64sys_pwrite64
> > +#define compat_sys_readahead   sys_readahead
> > +#define compat_sys_shmat   sys_shmat
> 
> Why don't we use compat_sys_shmat? Is it because of COMPAT_SHMLBA?

Yes. COMPAT_SHMLBA is 4 pages, and it's aarch32-only limitation.

> 
> > +#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
> 
> Nitpick: there are some whitespace inconsistencies above (just convert
> all spaces to tabs).
> 
> I think you should also update Documentation/arm64/ilp32.txt to include
> the list above.

OK

> 
> > +
> > +#include 
> > +
> > +#undef __SYSCALL
> > +#undef __SC_COMP
> > +#undef __SC_WRAP
> > +#undef __SC_3264
> > +#undef __SC_COMP_3264
> 
> Minor detail: do we actually need to undef all these? Maybe we can get
> away with just defining __SYSCALL_COMPAT at the top of the file.
> 

Yes, we need. Otherwise we have circular dependency like this:
arch/arm64/kernel/sys_ilp32.c:60:0: warning: "__SC_WRAP" redefined
 #define __SC_WRAP(nr, sym) [nr] = compat_##sym,
  ^
  In file included from include/asm-generic/unistd.h:1:0,
   from ./arch/arm64/include/uapi/asm/unistd.h:16,
   from ./arch/arm64/include/asm/unistd.h:62,
   from ./include/uapi/linux/unistd.h:7,
   from include/linux/syscalls.h:23,
   from arch/arm64/kernel/sys_ilp32.c:30:
include/uapi/asm-generic/unistd.h:33:0: note: this is the location of the 
previous definition
 #define __SC_WRAP __SYSCALL

Defining __SYSCALL_COMPAT at the top of the file does not help much.

> > +
> > +#define __SYSCALL_COMPAT
> > +#define __SYSCALL(nr, sym) [nr] = sym,
> > +#define __SC_WRAP(nr, sym) [nr] = compat_##sym,
> > +
> > +/*
> > + * The sys_call_ilp32_table array must be 4K aligned to be accessible from
> > + * kernel/entry.S.
> > + */
> > +void *sys_call_ilp32_table[__NR_syscalls] __aligned(4096) = {
> > +   [0 ... __NR_syscalls - 1] = sys_ni_syscall,
> > +#include 
> > +};
> 
> -- 
> Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it

[Catalin Marinas]

On Wed, Apr 06, 2016 at 01:08:42AM +0300, Yury Norov wrote:
> +/* Using non-compat syscalls where necessary */
> +#define compat_sys_fadvise64_64sys_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_pwrite64sys_pwrite64
> +#define compat_sys_readahead   sys_readahead
> +#define compat_sys_shmat   sys_shmat

Why don't we use compat_sys_shmat? Is it because of COMPAT_SHMLBA?

> +#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

Nitpick: there are some whitespace inconsistencies above (just convert
all spaces to tabs).

I think you should also update Documentation/arm64/ilp32.txt to include
the list above.

> +
> +#include 
> +
> +#undef __SYSCALL
> +#undef __SC_COMP
> +#undef __SC_WRAP
> +#undef __SC_3264
> +#undef __SC_COMP_3264

Minor detail: do we actually need to undef all these? Maybe we can get
away with just defining __SYSCALL_COMPAT at the top of the file.

> +
> +#define __SYSCALL_COMPAT
> +#define __SYSCALL(nr, sym)   [nr] = sym,
> +#define __SC_WRAP(nr, sym)   [nr] = compat_##sym,
> +
> +/*
> + * The sys_call_ilp32_table array must be 4K aligned to be accessible from
> + * kernel/entry.S.
> + */
> +void *sys_call_ilp32_table[__NR_syscalls] __aligned(4096) = {
> + [0 ... __NR_syscalls - 1] = sys_ni_syscall,
> +#include 
> +};

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it

[Catalin Marinas]

On Mon, Apr 25, 2016 at 09:47:40PM +0300, Yury Norov wrote:
> On Mon, Apr 25, 2016 at 09:19:13PM +0300, Yury Norov wrote:
> > On Mon, Apr 25, 2016 at 06:26:56PM +0100, Catalin Marinas wrote:
> > > On Wed, Apr 06, 2016 at 01:08:42AM +0300, Yury Norov wrote:
> > > > --- a/arch/arm64/kernel/entry.S
> > > > +++ b/arch/arm64/kernel/entry.S
> > > > @@ -715,9 +715,13 @@ ENDPROC(ret_from_fork)
> > > >   */
> > > > .align  6
> > > >  el0_svc:
> > > > -   adrpstbl, sys_call_table// load syscall table 
> > > > pointer
> > > > uxtwscno, w8// syscall number in w8
> > > > mov sc_nr, #__NR_syscalls
> > > > +#ifdef CONFIG_ARM64_ILP32
> > > > +   ldr x16, [tsk, #TI_FLAGS]
> > > > +   tbnzx16, #TIF_32BIT_AARCH64, el0_ilp32_svc // We are using 
> > > > ILP32
> > > > +#endif
> > > 
> > > There is another ldr x16, [tsk, #TI_FLAGS] load further down in the
> > > el0_svc_naked block. We should rework these a bit to avoid loading the
> > > same location twice unnecessarily. E.g. move the ldr x16 just before
> > > el0_svc_naked and branch one line after in case of the ILP32 syscall.
> > > 
> > 
> > Yes, I thiks we can refactor it. Thanks for a catch.
> 
> Now it's better, I think
> 
> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
> index cf4d1ae..21312bb 100644
> --- a/arch/arm64/kernel/entry.S
> +++ b/arch/arm64/kernel/entry.S
> @@ -715,16 +715,22 @@ ENDPROC(ret_from_fork)
>   */
>   .align  6
>  el0_svc:
> - adrpstbl, sys_call_table// load syscall table pointer
>   uxtwscno, w8// syscall number in w8
>   mov sc_nr, #__NR_syscalls
> + ldr x16, [tsk, #TI_FLAGS]

You can move this higher up for interlocking reasons (though these days
CPUs do a lot of speculative loads).

> +#ifdef CONFIG_ARM64_ILP32
> + tbz x16, #TIF_32BIT_AARCH64, el0_lp64_svc // We are using ILP32

// We are *not* using ILP32

> + adrpstbl, sys_call_ilp32_table  // load ilp32 syscall table 
> pointer
> + b el0_svc_naked
> +el0_lp64_svc:
> +#endif
> + adrpstbl, sys_call_table// load syscall table pointer

You can avoid the branches by using csel, something like this:

ldr x16, [tsk, #TI_FLAGS]
adrpstbl, sys_call_table
...
#ifdef CONFIG_ARM64_ILP32
adrpx17, sys_call_ilp32_table
tst x16, #_TIF_32BIT_AARCH64
cselstbl, stbl, x17, eq
#endif
el0_svc_naked:
...

>  el0_svc_naked:   // compat entry point
>   stp x0, scno, [sp, #S_ORIG_X0]  // save the original x0 and 
> syscall number
>   enable_dbg_and_irq
>   ct_user_exit 1
>  
> - ldr x16, [tsk, #TI_FLAGS]   // check for syscall hooks
> - tst x16, #_TIF_SYSCALL_WORK
> + tst x16, #_TIF_SYSCALL_WORK // check for syscall hooks
>   b.ne__sys_trace
>   cmp scno, sc_nr // check upper syscall limit
>   b.hsni_sys

There is el0_svc_compat branching to el0_svc_naked and it won't have x16
set anymore. So you need to add an ldr x16 to el0_svc_compat as well.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it

[Yury Norov]

On Mon, Apr 25, 2016 at 09:19:13PM +0300, Yury Norov wrote:
> On Mon, Apr 25, 2016 at 06:26:56PM +0100, Catalin Marinas wrote:
> > On Wed, Apr 06, 2016 at 01:08:42AM +0300, Yury Norov wrote:
> > > --- a/arch/arm64/kernel/entry.S
> > > +++ b/arch/arm64/kernel/entry.S
> > > @@ -715,9 +715,13 @@ ENDPROC(ret_from_fork)
> > >   */
> > >   .align  6
> > >  el0_svc:
> > > - adrpstbl, sys_call_table// load syscall table pointer
> > >   uxtwscno, w8// syscall number in w8
> > >   mov sc_nr, #__NR_syscalls
> > > +#ifdef CONFIG_ARM64_ILP32
> > > + ldr x16, [tsk, #TI_FLAGS]
> > > + tbnzx16, #TIF_32BIT_AARCH64, el0_ilp32_svc // We are using ILP32
> > > +#endif
> > 
> > There is another ldr x16, [tsk, #TI_FLAGS] load further down in the
> > el0_svc_naked block. We should rework these a bit to avoid loading the
> > same location twice unnecessarily. E.g. move the ldr x16 just before
> > el0_svc_naked and branch one line after in case of the ILP32 syscall.
> > 
> 
> Yes, I thiks we can refactor it. Thanks for a catch.

Now it's better, I think


diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index cf4d1ae..21312bb 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -715,16 +715,22 @@ ENDPROC(ret_from_fork)
  */
.align  6
 el0_svc:
-   adrpstbl, sys_call_table// load syscall table pointer
uxtwscno, w8// syscall number in w8
mov sc_nr, #__NR_syscalls
+   ldr x16, [tsk, #TI_FLAGS]
+#ifdef CONFIG_ARM64_ILP32
+   tbz x16, #TIF_32BIT_AARCH64, el0_lp64_svc // We are using ILP32
+   adrpstbl, sys_call_ilp32_table  // load ilp32 syscall table 
pointer
+   b el0_svc_naked
+el0_lp64_svc:
+#endif
+   adrpstbl, 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
ct_user_exit 1
 
-   ldr x16, [tsk, #TI_FLAGS]   // check for syscall hooks
-   tst x16, #_TIF_SYSCALL_WORK
+   tst x16, #_TIF_SYSCALL_WORK // check for syscall hooks
b.ne__sys_trace
cmp scno, sc_nr // check upper syscall limit
b.hsni_sys
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it

[Yury Norov]

On Mon, Apr 25, 2016 at 06:26:56PM +0100, Catalin Marinas wrote:
> On Wed, Apr 06, 2016 at 01:08:42AM +0300, Yury Norov wrote:
> > --- a/arch/arm64/kernel/entry.S
> > +++ b/arch/arm64/kernel/entry.S
> > @@ -715,9 +715,13 @@ ENDPROC(ret_from_fork)
> >   */
> > .align  6
> >  el0_svc:
> > -   adrpstbl, sys_call_table// load syscall table pointer
> > uxtwscno, w8// syscall number in w8
> > mov sc_nr, #__NR_syscalls
> > +#ifdef CONFIG_ARM64_ILP32
> > +   ldr x16, [tsk, #TI_FLAGS]
> > +   tbnzx16, #TIF_32BIT_AARCH64, el0_ilp32_svc // We are using ILP32
> > +#endif
> 
> There is another ldr x16, [tsk, #TI_FLAGS] load further down in the
> el0_svc_naked block. We should rework these a bit to avoid loading the
> same location twice unnecessarily. E.g. move the ldr x16 just before
> el0_svc_naked and branch one line after in case of the ILP32 syscall.
> 

Yes, I thiks we can refactor it. Thanks for a catch.

> > +   adrpstbl, 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:
> > +   adrpstbl, 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.
> 
> -- 
> Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html