Re: [PATCH 2/3] ARM: convert to generated system call tables

Re: [PATCH 2/3] ARM: convert to generated system call tables

[Arnd Bergmann]

On Wednesday, October 19, 2016 4:53:25 PM CEST Russell King - ARM Linux wrote:
> On Wed, Oct 19, 2016 at 05:30:49PM +0200, Arnd Bergmann wrote:
> > On Tuesday, October 18, 2016 8:31:38 PM CEST Russell King wrote:
> > > Convert ARM to use a similar mechanism to x86 to generate the unistd.h
> > > system call numbers and the various kernel system call tables.  This
> > > means that rather than having to edit three places (asm/unistd.h for
> > > the total number of system calls, uapi/asm/unistd.h for the system call
> > > numbers, and arch/arm/kernel/calls.S for the call table) we have only
> > > one place to edit, making the process much more simple.
> > > 
> > > The scripts have knowledge of the table padding requirements, so there's
> > > no need to worry about __NR_syscalls not fitting within the immediate
> > > constant field of ALU instructions anymore.
> > > 
> > > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> > 
> > Ah, very nice!
> > 
> > I have some vague plans to do something like this for all architectures,
> > so having it done for one of the more complex examples (there are very
> > few architectures with more than one table) simplifies it a lot.
> > 
> > The next step is probably to do it for asm-generic/unistd.h, which
> > covers a lot of architectures, and then we can introduce a shared
> > table for all future additions so we only have to add the new calls
> > in one place, and change the scripts so they can merge two input
> > files into one.
> 
> Architecture maintainers like to verify that the system call works on
> their architecture before they push it out into the wild; your idea
> effectively bypasses architecture maintainer review and testing, so
> is bad.  For something as critical as system call interfaces, that
> step is critical: introducing a new system call across all architectures
> that then fails to work correctly on a particular architecture invites
> userspace to work around the problem, and the brokenness then becomes
> user API which can't be fixed.

I see your point, but I think there are serious issues with the current
approach as well:

- a lot of the less common architectures just don't get updated
  in time, out of 22 architectures that don't use asm-generic/unistd.h,
  only 12 have pwritev2 in linux-next, and only three have pkey_mprotect

- some architectures that add all syscalls sometimes make a mistake
  and forget one, e.g. alpha apparently never added __NR_bpf, but it
  did add the later __NR_execveat.

- when glibc updates their minimum supported kernel version, they
  would like to drop obsolete syscalls, but when each architecture
  adds the calls at a different time, it's hard to tell when a
  replacement syscall is guaranteed to be available

- linux-next produces warnings about missing syscalls on most
  architectures half of the time since it's impossible for an
  arch maintainer to hook up the number before the implementation
  is merged 

Regarding the review process, I'd really hope we've improved enough
that we can rely on the review on linux-arch/linux-api to catch
all serious issues like system call that cannot be defined the same
way on all architectures. If we fail at this, there is a more
serious issue with the review process.

Since all syscalls now go through SYSCALL_DEFINEx(), we have
covered the hardest part (sign/zero extending short arguments),
and a lot more people are aware of problems with struct alignment
since it differs between i386 and x86_64 and also affects all
ioctl interfaces. I think the last time a syscall made it in that
didn't just work on all architectures was sync_file_range, and
that was nine years ago.

> > > +# Where abi is:
> > > +#  common - for system calls shared between oabi and eabi
> > > +#  oabi   - for oabi-only system calls (may have compat)
> > > +#  eabi   - for eabi-only system calls
> > 
> > Why do we need all three? I would have guessed that these two are
> > sufficient to cover all cases:
> > 
> > arm	- one entry for eabi, optional second entry for oabi if different
> > oabi	- only one entry for oabi, syscall is not used on eabi
> 
> You haven't quite understood if you think the second entry gets used
> for OABI - but that's not surprising because the issues here are
> quite complex.
> 
> For OABI-only, all the oabi and first entry in common gets used.
> For EABI-only, all the eabi and first entry in common gets used.
> For EABI with OABI compat, EABI uses eabi and the first entry in common,
> but the OABI compat table uses the oabi and common entries, prefering
> the second entry where present.

Got it, I missed the fact that we support native OABI kernels.

> Yes, for the cases where we list the oabi and eabi together like you
> quoted, currently there are no differences between the system calls,
> and in my latest version, they've already been modified down to just
> a single "common" entry, leaving us without any eabi entries.
>
> However, I want to retain the ability to have separate eabi entries
> if needs be.  Such a case would be a system call which needs a helper
> for arguments passed in >4 registers on EABI but not OABI (eg, because
> of an non-naturally aligned 64-bit quantity passed in r1/r2 on OABI
> but r2/r3 in EABI.)

If we hit this case, why not just use the wrapper on both EABI
and OABI for simplicity? It's not like we care a lot about
micro-optimizing OABI any more.

> You'll find the latest version in the next linux-next, or my current
> for-next branch.

Ok. After rebasing my randconfig tree on today's linux-next, I needed
this hunk to avoid a warning:

<stdin>:1143:2: error: #warning syscall sync_file_range not implemented [-Werror=cpp]

diff --git a/arch/arm/tools/syscall.tbl b/arch/arm/tools/syscall.tbl
index 70558e4459fd..7da1bbe69e56 100644
--- a/arch/arm/tools/syscall.tbl
+++ b/arch/arm/tools/syscall.tbl
@@ -355,7 +355,8 @@
 338	common	set_robust_list		sys_set_robust_list
 339	common	get_robust_list		sys_get_robust_list
 340	common	splice			sys_splice
-341	common	arm_sync_file_range	sys_sync_file_range2
+341	common	sync_file_range2		sys_sync_file_range2
+341	common	arm_sync_file_range
 342	common	tee			sys_tee
 343	common	vmsplice		sys_vmsplice
 344	common	move_pages		sys_move_pages

(or alternatively, add "#define sync_file_range2 arm_sync_file_range"
to uapi/asm/unistd.h).

> > > diff --git a/arch/arm/tools/syscallhdr.sh b/arch/arm/tools/syscallhdr.sh
> > > new file mode 100644
> > > index 000000000000..72d4b2e3bdec
> > > --- /dev/null
> > > +++ b/arch/arm/tools/syscallhdr.sh
> > 
> > The scripts are still very similar to the x86 version. Any chance
> > we can move them to a top-level scripts/syscall/ directory and make
> > them work for both architectures? It would be good to avoid duplicating
> > them for all the other architectures too, so starting out with a common
> > version could make that easier.
> 
> The fileguard prefix would have to be specified as an additional
> argument to achieve that, but I don't see that as a big problem.

Agreed, I saw the same thing there.

> The syscalltbl.sh script is particularly architecture specific, as
> our "compat" isn't the same as x86's "compat" requirements.

That brings up an interesting issue: it would be nice to use the
same input file for arch/arm/ and the compat mode of arch/arm64,
like x86 does. If we handle both oabi and arm64-compat in the same
file, we end up with a superset of what x86 does, and we could
use a single script again, and generate all four tables for
ARM (native OABI, OABI-on-EABI, native EABI, EABI-on-arm64).

Another related case in asm-generic, which defines three tables:
native 32-bit, native 64-bit and compat 32-bit. This one not only
needs to have three different function pointers (e.g. sys_fcntl64,
sys_fcntl and compat_sys_fcntl64) but also different macros (e.g.
__NR_fcntl64 and __NR_fcntl).

Anything wrong with this approach:?

/* ARM */
221  oabi  fcntl64                 sys_fcntl64             sys_oabi_fcntl64
221  eabi  fcntl64                 sys_fcntl64             compat_sys_fcntl64

/* asm-generic */
25   32    fcntl64                 sys_fcntl64             compat_sys_fcntl64
25   64    fcntl                   sys_fcntl

> The syscallnr.sh script kind-of looks like a candidate, but it has
> ARM arch specifics to it (knowing that the number of system calls
> needs to fit within the 8-bit value plus 4-bit shift constant
> representation of ARM ALU instructions.)  Maybe a generic version
> without that knowledge would work, provided architectures can
> override it.

syscallnr.sh isn't used on x86, and probably won't be needed on
most (or all) others, right? 

Generally speaking I'd think that having a check for the ${ARCH}
variable and doing this conditionally in that one script is fine
here, if we need other architecture specific versions, they we can
use case/esac.

Similar checks exist in scripts/recordmcount.pl,
scripts/package/builddeb, scripts/tags.sh etc.

	Arnd

Re: [PATCH 2/3] ARM: convert to generated system call tables

[Russell King - ARM Linux]

On Fri, Oct 21, 2016 at 03:06:45PM +0200, Arnd Bergmann wrote:
> Regarding the review process, I'd really hope we've improved enough
> that we can rely on the review on linux-arch/linux-api to catch
> all serious issues like system call that cannot be defined the same
> way on all architectures. If we fail at this, there is a more
> serious issue with the review process.

Well, forget linux-arch, that's hopeless because that became a very
x86-centric linux-kernel-v2, and as such I refuse to subscribe to it -
it would be a total waste of my network bandwidth because I wouldn't
have time to read it.

I somehow suspect that linux-api isn't that much better either.  What
I want from any "arch" specific thing is a heads-up to alert me to
something so that I can then choose whether to look deeper at the
subject or just ignore it completely.  I don't want to be buried under
lots of x86 specific drivel about a new feature.

So, the reality is, that I don't see any of the new syscall discussions
anymore.  The first that I'm aware of them is when they hit in some way
that becomes visible to me - which is normally sometime during the merge
window.

> Since all syscalls now go through SYSCALL_DEFINEx(), we have
> covered the hardest part (sign/zero extending short arguments),
> and a lot more people are aware of problems with struct alignment
> since it differs between i386 and x86_64 and also affects all
> ioctl interfaces. I think the last time a syscall made it in that
> didn't just work on all architectures was sync_file_range, and
> that was nine years ago.

It's not really about struct alignment, although that is a concern.
For ARM, it's more about argument alignment, and whether a 64-bit
argument gets passed in (eg) r1/r2 or r2/r3, and whether we run out
of registers to pass the arguments.

> If we hit this case, why not just use the wrapper on both EABI
> and OABI for simplicity? It's not like we care a lot about
> micro-optimizing OABI any more.

I'd still like to retain the ability to only add to EABI in the future.

> > You'll find the latest version in the next linux-next, or my current
> > for-next branch.
> 
> Ok. After rebasing my randconfig tree on today's linux-next, I needed
> this hunk to avoid a warning:
> 
> <stdin>:1143:2: error: #warning syscall sync_file_range not implemented [-Werror=cpp]

I don't get that on my builds, for EABI or OABI - for EABI:

  CHK     include/generated/bounds.h
  CC      arch/arm/kernel/asm-offsets.s
  CHK     include/generated/asm-offsets.h
  CALL    /home/rmk/git/linux-rmk/scripts/checksyscalls.sh
make[1]: Leaving directory '/home/rmk/git/build/hdrtst'

and for OABI:

  CHK     include/generated/bounds.h
  CC      arch/arm/kernel/asm-offsets.s
  CHK     include/generated/asm-offsets.h
  CALL    /home/rmk/git/linux-rmk/scripts/checksyscalls.sh
make[1]: Leaving directory '/home/rmk/git/build/hdrtst-oabi'

So, I'd like to know how you're seeing that warning.  We have never
provided sync_file_range on ARM, and we must never define it, because
the user API for it is broken.

> diff --git a/arch/arm/tools/syscall.tbl b/arch/arm/tools/syscall.tbl
> index 70558e4459fd..7da1bbe69e56 100644
> --- a/arch/arm/tools/syscall.tbl
> +++ b/arch/arm/tools/syscall.tbl
> @@ -355,7 +355,8 @@
>  338	common	set_robust_list		sys_set_robust_list
>  339	common	get_robust_list		sys_get_robust_list
>  340	common	splice			sys_splice
> -341	common	arm_sync_file_range	sys_sync_file_range2
> +341	common	sync_file_range2		sys_sync_file_range2
> +341	common	arm_sync_file_range
>  342	common	tee			sys_tee
>  343	common	vmsplice		sys_vmsplice
>  344	common	move_pages		sys_move_pages
> 
> (or alternatively, add "#define sync_file_range2 arm_sync_file_range"
> to uapi/asm/unistd.h).

Well, I think you have a mis-merge somewhere, beacuse uapi/asm/unistd.h
does have:

#define __NR_sync_file_range2           __NR_arm_sync_file_range

in it, which triggers this in scripts/checksyscalls.sh:

/* sync_file_range had a stupid ABI. Allow sync_file_range2 instead */
#ifdef __NR_sync_file_range2
#define __IGNORE_sync_file_range
#endif

Hence why I don't see the warning you're seeing.

> That brings up an interesting issue: it would be nice to use the
> same input file for arch/arm/ and the compat mode of arch/arm64,
> like x86 does. If we handle both oabi and arm64-compat in the same
> file, we end up with a superset of what x86 does, and we could
> use a single script again, and generate all four tables for
> ARM (native OABI, OABI-on-EABI, native EABI, EABI-on-arm64).

OABI-compat != ARM64-EABI-compat though.  They're two completely
different things.

Moreover, the syscall numbers ARM64 uses natively are completely
different from the syscall numbers 32-bit ARM uses, either for EABI
or OABI.  So I really don't see this working.

I've no idea how ARM64 deals with 32-bit binaries, so I can't comment
on how it deals with those syscalls, sorry.

> Another related case in asm-generic, which defines three tables:
> native 32-bit, native 64-bit and compat 32-bit. This one not only
> needs to have three different function pointers (e.g. sys_fcntl64,
> sys_fcntl and compat_sys_fcntl64) but also different macros (e.g.
> __NR_fcntl64 and __NR_fcntl).
> 
> Anything wrong with this approach:?
> 
> /* ARM */
> 221  oabi  fcntl64                 sys_fcntl64             sys_oabi_fcntl64
> 221  eabi  fcntl64                 sys_fcntl64             compat_sys_fcntl64
> 
> /* asm-generic */
> 25   32    fcntl64                 sys_fcntl64             compat_sys_fcntl64
> 25   64    fcntl                   sys_fcntl

Don't know, sorry.  I know virtually nothing about the differences
between the 64-bit and 32-bit ABIs, so I can't comment on anything
to do with the compat_* interfaces.

> > The syscallnr.sh script kind-of looks like a candidate, but it has
> > ARM arch specifics to it (knowing that the number of system calls
> > needs to fit within the 8-bit value plus 4-bit shift constant
> > representation of ARM ALU instructions.)  Maybe a generic version
> > without that knowledge would work, provided architectures can
> > override it.
> 
> syscallnr.sh isn't used on x86, and probably won't be needed on
> most (or all) others, right? 

Why not - the point of syscallnr.sh is to remove the need to manually
update the __NR_syscalls definition, which is a generic kernel thing.
Unless other architectures just define a fixed-size table with plenty
of extra space, they'd need to adjust __NR_syscalls when adding new
calls.


-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH 2/3] ARM: convert to generated system call tables

[Arnd Bergmann]

On Friday, October 21, 2016 2:37:08 PM CEST Russell King - ARM Linux wrote:
> On Fri, Oct 21, 2016 at 03:06:45PM +0200, Arnd Bergmann wrote:

> > If we hit this case, why not just use the wrapper on both EABI
> > and OABI for simplicity? It's not like we care a lot about
> > micro-optimizing OABI any more.
> 
> I'd still like to retain the ability to only add to EABI in the future.

Do you mean to add an EABI specific workaround for a specific syscall
if necessary, or to stop adding OABI syscalls altogether?

For the first one, the way that the asm-generic unistd.h handles this is
to have a range of syscall numbers reserved for architecture specific
additions. I was planning to do the same here and have a number of
reserved numbers after the last architecture specific call before
the start of the newly added generic numbers.

> > (or alternatively, add "#define sync_file_range2 arm_sync_file_range"
> > to uapi/asm/unistd.h).
> 
> Well, I think you have a mis-merge somewhere, beacuse uapi/asm/unistd.h
> does have:
> 
> #define __NR_sync_file_range2           __NR_arm_sync_file_range
> 
> in it, which triggers this in scripts/checksyscalls.sh:

That was it, I had merged in my y2038 syscall series for testing and
accidentally dropped that line while rebasing the newly added
syscalls.

> > That brings up an interesting issue: it would be nice to use the
> > same input file for arch/arm/ and the compat mode of arch/arm64,
> > like x86 does. If we handle both oabi and arm64-compat in the same
> > file, we end up with a superset of what x86 does, and we could
> > use a single script again, and generate all four tables for
> > ARM (native OABI, OABI-on-EABI, native EABI, EABI-on-arm64).
> 
> OABI-compat != ARM64-EABI-compat though.  They're two completely
> different things.

For the purpose of generating the tables, I don't see much
difference: we either use the fourth column only in native
mode, or we use the fifth column to override values from
the fourth one when emulating the ABI on the "other" kernel.

> Moreover, the syscall numbers ARM64 uses natively are completely
> different from the syscall numbers 32-bit ARM uses, either for EABI
> or OABI.  So I really don't see this working.

That's similar to x86, 32-bit syscalls use the traditional numbers
with an optionally different entry point for compat mode, while
64-bit syscalls use a somewhat reduced table that now has support
for both native 64-bit and "x32" mode. x86-64 and x32 share a
syscall table but not the unistd.h list, all three generated
from syscall_64.tbl.

> I've no idea how ARM64 deals with 32-bit binaries, so I can't comment
> on how it deals with those syscalls, sorry.

ARM64 has a separate list of syscalls for compat mode in 
arch/arm64/include/asm/unistd32.h, this list has the same format
as include/asm-generic/uapi/unistd.h and must be updated manually
to match the arch/arm/ table today.

ARM64 will also likely gain native (A64 instruction set) ILP32
support soon, which will use the same numbers as the 64-bit
mode but point to the compat syscalls. This is similar to
how ARM OABI emulation works.

> > Another related case in asm-generic, which defines three tables:
> > native 32-bit, native 64-bit and compat 32-bit. This one not only
> > needs to have three different function pointers (e.g. sys_fcntl64,
> > sys_fcntl and compat_sys_fcntl64) but also different macros (e.g.
> > __NR_fcntl64 and __NR_fcntl).
> > 
> > Anything wrong with this approach:?
> > 
> > /* ARM */
> > 221  oabi  fcntl64                 sys_fcntl64             sys_oabi_fcntl64
> > 221  eabi  fcntl64                 sys_fcntl64             compat_sys_fcntl64
> > 
> > /* asm-generic */
> > 25   32    fcntl64                 sys_fcntl64             compat_sys_fcntl64
> > 25   64    fcntl                   sys_fcntl
> 
> Don't know, sorry.  I know virtually nothing about the differences
> between the 64-bit and 32-bit ABIs, so I can't comment on anything
> to do with the compat_* interfaces.

The only important factor here is that the first three columns are used
to generate unistd.h, which is the same for both native and emulated
mode, while the last two columns are used to generate two sets of
syscall tables using the same numbers. This is a common requirement
for OABI mode (native or emulated), EABI (on 32-bit or 64-bit kernels)
and the generic ABI (native 32-bit or emulated 32-bit). 

The 64-bit unistd.h is differs only in those syscalls that got replaced
with when loff_t support was added:

#define __NR_fcntl __NR3264_fcntl
#define __NR_statfs __NR3264_statfs
#define __NR_fstatfs __NR3264_fstatfs
#define __NR_truncate __NR3264_truncate
#define __NR_ftruncate __NR3264_ftruncate
#define __NR_lseek __NR3264_lseek
#define __NR_sendfile __NR3264_sendfile
#define __NR_newfstatat __NR3264_fstatat
#define __NR_fstat __NR3264_fstat
#define __NR_mmap __NR3264_mmap
#define __NR_fadvise64 __NR3264_fadvise64
#define __NR_stat __NR3264_stat
#define __NR_lstat __NR3264_lstat

64-bit architectures still use __NR_fcntl, while 32-bit architectures
use __NR_fcntl64 etc.

> > > The syscallnr.sh script kind-of looks like a candidate, but it has
> > > ARM arch specifics to it (knowing that the number of system calls
> > > needs to fit within the 8-bit value plus 4-bit shift constant
> > > representation of ARM ALU instructions.)  Maybe a generic version
> > > without that knowledge would work, provided architectures can
> > > override it.
> > 
> > syscallnr.sh isn't used on x86, and probably won't be needed on
> > most (or all) others, right? 
> 
> Why not - the point of syscallnr.sh is to remove the need to manually
> update the __NR_syscalls definition, which is a generic kernel thing.
> Unless other architectures just define a fixed-size table with plenty
> of extra space, they'd need to adjust __NR_syscalls when adding new
> calls.

Ah, makes sense. I see that x86 does this in
arch/x86/kernel/asm-offsets_64.c in a way that would also work on
other architectures, but being able to share a single script across
all architectures would also help avoid duplicating that code.

	Arnd

Re: [PATCH 2/3] ARM: convert to generated system call tables

[Russell King - ARM Linux]

On Fri, Oct 21, 2016 at 05:18:30PM +0200, Arnd Bergmann wrote:
> On Friday, October 21, 2016 2:37:08 PM CEST Russell King - ARM Linux wrote:
> > On Fri, Oct 21, 2016 at 03:06:45PM +0200, Arnd Bergmann wrote:
> 
> > > If we hit this case, why not just use the wrapper on both EABI
> > > and OABI for simplicity? It's not like we care a lot about
> > > micro-optimizing OABI any more.
> > 
> > I'd still like to retain the ability to only add to EABI in the future.
> 
> Do you mean to add an EABI specific workaround for a specific syscall
> if necessary, or to stop adding OABI syscalls altogether?

To stop adding OABI syscalls altogether.  I'm sure that there will
come a point (if it hasn't already) that glibc no longer supports
OABI, and at that point it probably becomes rather silly to keep
adding OABI syscalls.

> > > That brings up an interesting issue: it would be nice to use the
> > > same input file for arch/arm/ and the compat mode of arch/arm64,
> > > like x86 does. If we handle both oabi and arm64-compat in the same
> > > file, we end up with a superset of what x86 does, and we could
> > > use a single script again, and generate all four tables for
> > > ARM (native OABI, OABI-on-EABI, native EABI, EABI-on-arm64).
> > 
> > OABI-compat != ARM64-EABI-compat though.  They're two completely
> > different things.
> 
> For the purpose of generating the tables, I don't see much
> difference: we either use the fourth column only in native
> mode, or we use the fifth column to override values from
> the fourth one when emulating the ABI on the "other" kernel.

The table generation method can be shared, but I've no idea about the
feasibility of sharing the table between ARM64 and ARM - I don't know
enough about ARM64 to know whether things like an "long" argument to
a syscall (which would be 64-bit on ARM64) would be or would not be a
problem if called from a 32-bit user application.

I've zero knowledge of the whole 32-bit application on 64-bit CPUs
thing, so it's pointless trying to discuss this aspect with me.  Even
for x86, all I care there is that it works, I've no knowledge of how
it works.

> That's similar to x86, 32-bit syscalls use the traditional numbers
> with an optionally different entry point for compat mode, while
> 64-bit syscalls use a somewhat reduced table that now has support
> for both native 64-bit and "x32" mode. x86-64 and x32 share a
> syscall table but not the unistd.h list, all three generated
> from syscall_64.tbl.

What's the point of the x32 mode?

> ARM64 has a separate list of syscalls for compat mode in 
> arch/arm64/include/asm/unistd32.h, this list has the same format
> as include/asm-generic/uapi/unistd.h and must be updated manually
> to match the arch/arm/ table today.

Looking through it, sort-of.  It could have re-used the numbering
from the arch/arm include file, but because ARM64 wanted to be an
entirely separate architecture, it duplicates a lot from 32-bit ARM.
I pointed that out at the time, and was shouted down (which is why
today I have absolutely nothing to do with ARM64, and as a result
have very little knowledge about ARM64 - I lost interest in it as
a result of the responses I got to my comments.)

So... if you don't mind, this isn't an issue I care one iota about.

In order for something to work like what you're alluding to, ARM64
would have to ferret around in arch/arm to pull out the bits it
wants, and I see zero reason for that to be acceptable on either
side of the ARM64 vs ARM divide - it will make my job harder because
I'm then into the position where I need acks from ARM64 people to
change ARM code, and that doesn't interest me at all.  I'm not going
to put myself into a position where I'm at the mercy of ARM64 folk.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH 2/3] ARM: convert to generated system call tables

[Joseph Myers]

On Fri, 21 Oct 2016, Russell King - ARM Linux wrote:

> To stop adding OABI syscalls altogether.  I'm sure that there will
> come a point (if it hasn't already) that glibc no longer supports
> OABI, and at that point it probably becomes rather silly to keep
> adding OABI syscalls.

glibc hasn't supported OABI since 2012 (support removed in version 2.16 
released 2012-06-30).  GCC support was also removed in 2012 (removal 
released in version 4.8, released 2013-03-22).

-- 
Joseph S. Myers
joseph-qD8j1LwMmJjtCj0u4l0SBw@public.gmane.org

Re: [PATCH 2/3] ARM: convert to generated system call tables

[Russell King - ARM Linux]

On Fri, Oct 21, 2016 at 04:48:36PM +0000, Joseph Myers wrote:
> On Fri, 21 Oct 2016, Russell King - ARM Linux wrote:
> 
> > To stop adding OABI syscalls altogether.  I'm sure that there will
> > come a point (if it hasn't already) that glibc no longer supports
> > OABI, and at that point it probably becomes rather silly to keep
> > adding OABI syscalls.
> 
> glibc hasn't supported OABI since 2012 (support removed in version 2.16 
> released 2012-06-30).  GCC support was also removed in 2012 (removal 
> released in version 4.8, released 2013-03-22).

Right, so arguably its pointless adding new syscalls to OABI anymore.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH 2/3] ARM: convert to generated system call tables

[Arnd Bergmann]

On Friday, October 21, 2016 4:48:56 PM CEST Russell King - ARM Linux wrote:
> On Fri, Oct 21, 2016 at 05:18:30PM +0200, Arnd Bergmann wrote:
> > On Friday, October 21, 2016 2:37:08 PM CEST Russell King - ARM Linux wrote:
> > > On Fri, Oct 21, 2016 at 03:06:45PM +0200, Arnd Bergmann wrote:
> > 
> > > > If we hit this case, why not just use the wrapper on both EABI
> > > > and OABI for simplicity? It's not like we care a lot about
> > > > micro-optimizing OABI any more.
> > > 
> > > I'd still like to retain the ability to only add to EABI in the future.
> > 
> > Do you mean to add an EABI specific workaround for a specific syscall
> > if necessary, or to stop adding OABI syscalls altogether?
> 
> To stop adding OABI syscalls altogether.  I'm sure that there will
> come a point (if it hasn't already) that glibc no longer supports
> OABI, and at that point it probably becomes rather silly to keep
> adding OABI syscalls.

Ok. While uClibc still supports OABI, it seems rather unlikely that
there are active users of OABI uClibc that need any of the future
syscalls.

> > > > That brings up an interesting issue: it would be nice to use the
> > > > same input file for arch/arm/ and the compat mode of arch/arm64,
> > > > like x86 does. If we handle both oabi and arm64-compat in the same
> > > > file, we end up with a superset of what x86 does, and we could
> > > > use a single script again, and generate all four tables for
> > > > ARM (native OABI, OABI-on-EABI, native EABI, EABI-on-arm64).
> > > 
> > > OABI-compat != ARM64-EABI-compat though.  They're two completely
> > > different things.
> > 
> > For the purpose of generating the tables, I don't see much
> > difference: we either use the fourth column only in native
> > mode, or we use the fifth column to override values from
> > the fourth one when emulating the ABI on the "other" kernel.
> 
> The table generation method can be shared, but I've no idea about the
> feasibility of sharing the table between ARM64 and ARM - I don't know
> enough about ARM64 to know whether things like an "long" argument to
> a syscall (which would be 64-bit on ARM64) would be or would not be a
> problem if called from a 32-bit user application.
> 
> I've zero knowledge of the whole 32-bit application on 64-bit CPUs
> thing, so it's pointless trying to discuss this aspect with me.  Even
> for x86, all I care there is that it works, I've no knowledge of how
> it works.

Ok. I'm sure it works, but we can definitely leave that for the
time when I or someone else gets around to do the same conversion
for ARM64 that you are doing for ARM32. From all I can tell, it's
a natural extension of the file you already have.

> > That's similar to x86, 32-bit syscalls use the traditional numbers
> > with an optionally different entry point for compat mode, while
> > 64-bit syscalls use a somewhat reduced table that now has support
> > for both native 64-bit and "x32" mode. x86-64 and x32 share a
> > syscall table but not the unistd.h list, all three generated
> > from syscall_64.tbl.
> 
> What's the point of the x32 mode?

On x86, the motivation is faster code for most use cases that
don't need a lot of memory, as the 64-bit opcodes have 16 registers
rather than 8 in 32-bit mode but 32-bit pointers have lower
cache footprint than 64-bit pointers. 

Interestingly, while ARM64 is doing basically the same thing now,
the motivation is almost entirely different: going from around 16
to around 32 registers has a much lower impact on performance
than starting out with 8, but there are ARMv8 implementations that
lack aarch32 mode but still want to run legacy 32-bit code that
is too time-consuming to get 64-bit clean.

> > ARM64 has a separate list of syscalls for compat mode in 
> > arch/arm64/include/asm/unistd32.h, this list has the same format
> > as include/asm-generic/uapi/unistd.h and must be updated manually
> > to match the arch/arm/ table today.
> 
> Looking through it, sort-of.  It could have re-used the numbering
> from the arch/arm include file, but because ARM64 wanted to be an
> entirely separate architecture, it duplicates a lot from 32-bit ARM.
> I pointed that out at the time, and was shouted down (which is why
> today I have absolutely nothing to do with ARM64, and as a result
> have very little knowledge about ARM64 - I lost interest in it as
> a result of the responses I got to my comments.)
> 
> So... if you don't mind, this isn't an issue I care one iota about.
> 
> In order for something to work like what you're alluding to, ARM64
> would have to ferret around in arch/arm to pull out the bits it
> wants, and I see zero reason for that to be acceptable on either
> side of the ARM64 vs ARM divide - it will make my job harder because
> I'm then into the position where I need acks from ARM64 people to
> change ARM code, and that doesn't interest me at all.  I'm not going
> to put myself into a position where I'm at the mercy of ARM64 folk.

Well, if we don't need to worry about OABI any more, using a shared
file for all architectures to add new syscalls (with their compat
handlers) would also mean that you don't have to worry about the
compat handler either, since that gets added in the shared file
along with the syscall.

Then again, if we move the a shared file for new syscalls, we also
don't need to worry about synchronizing the syscall tables for
ARM and compat ARM64, since the files would not get changed any
more.

	Arnd

Re: [PATCH 2/3] ARM: convert to generated system call tables

[Robert Jarzmik]

Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> writes:

> On Friday, October 21, 2016 4:48:56 PM CEST Russell King - ARM Linux wrote:
>> What's the point of the x32 mode?
>
> On x86, the motivation is faster code for most use cases that
> don't need a lot of memory, as the 64-bit opcodes have 16 registers
> rather than 8 in 32-bit mode but 32-bit pointers have lower
> cache footprint than 64-bit pointers.

For completness, the second point of x32 AFAIU is the IP-relative addressing
which is not available in standard 32 bit mode, which improves PIC code. For
simple not algorithmic code (think Android HAL for example) with many shared
libraries, it's better in the Hardware Abstraction Layer Libraries, instead of
the push-to-stack and pop register.

Cheers.

--
Robert

Re: [PATCH 2/3] ARM: convert to generated system call tables

[Geert Uytterhoeven]

On Sat, Oct 22, 2016 at 10:23 PM, Robert Jarzmik <robert.jarzmik@free.fr> wrote:
> Arnd Bergmann <arnd@arndb.de> writes:
>
>> On Friday, October 21, 2016 4:48:56 PM CEST Russell King - ARM Linux wrote:
>>> What's the point of the x32 mode?
>>
>> On x86, the motivation is faster code for most use cases that
>> don't need a lot of memory, as the 64-bit opcodes have 16 registers
>> rather than 8 in 32-bit mode but 32-bit pointers have lower
>> cache footprint than 64-bit pointers.
>
> For completness, the second point of x32 AFAIU is the IP-relative addressing
> which is not available in standard 32 bit mode, which improves PIC code. For
> simple not algorithmic code (think Android HAL for example) with many shared
> libraries, it's better in the Hardware Abstraction Layer Libraries, instead of
> the push-to-stack and pop register.

But that's not an advantage compared to full am64 mode, right?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 2/3] ARM: convert to generated system call tables

[Robert Jarzmik]

Geert Uytterhoeven <geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org> writes:

> On Sat, Oct 22, 2016 at 10:23 PM, Robert Jarzmik <robert.jarzmik-GANU6spQydw@public.gmane.org> wrote:
>> Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> writes:
>>
>>> On Friday, October 21, 2016 4:48:56 PM CEST Russell King - ARM Linux wrote:
>>>> What's the point of the x32 mode?
>>>
>>> On x86, the motivation is faster code for most use cases that
>>> don't need a lot of memory, as the 64-bit opcodes have 16 registers
>>> rather than 8 in 32-bit mode but 32-bit pointers have lower
>>> cache footprint than 64-bit pointers.
>>
>> For completness, the second point of x32 AFAIU is the IP-relative addressing
>> which is not available in standard 32 bit mode, which improves PIC code. For
>> simple not algorithmic code (think Android HAL for example) with many shared
>> libraries, it's better in the Hardware Abstraction Layer Libraries, instead of
>> the push-to-stack and pop register.
>
> But that's not an advantage compared to full am64 mode, right?
Nope, the amd64 (that's what you have in mind with am64, not aarch64 right ?)
mode has the IP-relative has well, so x32 has no advantage over amd64 in this
area.

x32 over amd64 advantage is cache and memory footprint AFAICT, I don't see other
benefits. There doesn't seem to be any ISA differences, or I didn't really
notice in my daily system performance analysis job.

There was a nice presentation made by Peter Anvin at a Plumber conference, here
: http://linuxplumbersconf.org/2011/ocw/sessions/531

Cheers.

-- 
Robert

Re: [PATCH 2/3] ARM: convert to generated system call tables

[Geert Uytterhoeven]

Hi Arnd,

On Fri, Oct 21, 2016 at 3:06 PM, Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> wrote:
> - a lot of the less common architectures just don't get updated
>   in time, out of 22 architectures that don't use asm-generic/unistd.h,
>   only 12 have pwritev2 in linux-next, and only three have pkey_mprotect

Should everybody implement pkey_mprotect?

"x86, pkeys: remove cruft from never-merged syscalls" and

arch/powerpc/include/asm/unistd.h:#define __IGNORE_pkey_mprotect

aren't that reassuring...

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 2/3] ARM: convert to generated system call tables

[Michael Cree]

On Fri, Oct 21, 2016 at 03:06:45PM +0200, Arnd Bergmann wrote:
> I see your point, but I think there are serious issues with the current
> approach as well:
> 
> - a lot of the less common architectures just don't get updated
>   in time, out of 22 architectures that don't use asm-generic/unistd.h,
>   only 12 have pwritev2 in linux-next, and only three have pkey_mprotect
> 
> - some architectures that add all syscalls sometimes make a mistake
>   and forget one, e.g. alpha apparently never added __NR_bpf, but it
>   did add the later __NR_execveat.

__NR_bpf was not forgotten on Alpha.  It was not wired up because
extra architecture support is needed which has not been implemented.

But maybe we should just wire it up to sys_ni_syscall in the meantime
so a syscall number is reserved for it, and user space can call it to
get -ENOSYS returned.

Cheers
Michael.

Re: [PATCH 2/3] ARM: convert to generated system call tables

[Arnd Bergmann]

On Tuesday, October 25, 2016 10:12:10 PM CEST Michael Cree wrote:
> On Fri, Oct 21, 2016 at 03:06:45PM +0200, Arnd Bergmann wrote:
> > I see your point, but I think there are serious issues with the current
> > approach as well:
> > 
> > - a lot of the less common architectures just don't get updated
> >   in time, out of 22 architectures that don't use asm-generic/unistd.h,
> >   only 12 have pwritev2 in linux-next, and only three have pkey_mprotect
> > 
> > - some architectures that add all syscalls sometimes make a mistake
> >   and forget one, e.g. alpha apparently never added __NR_bpf, but it
> >   did add the later __NR_execveat.
> 
> __NR_bpf was not forgotten on Alpha.  It was not wired up because
> extra architecture support is needed which has not been implemented.
> 
> But maybe we should just wire it up to sys_ni_syscall in the meantime
> so a syscall number is reserved for it, and user space can call it to
> get -ENOSYS returned.

Ah, I must have misinterpreted the code then. I assumed that the
bpf syscall always works on all architectures, but that only the
jit compiler for it required architecture specific code to make it
more efficient.

The implementation of sys_bfp is compile-time selectable at the moment
and falls back to sys_no_syscall if CONFIG_BPF_SYSCALL is disabled.
If it doesn't work on Alpha, maybe that symbol could have a "depends
on !ALPHA" or "depends on BPF_SUPPORT"?

sys_seccomp is another one that falls into a similar category, but
it already depends on HAVE_ARCH_SECCOMP_FILTER, and most other
architectures have assigned a syscall number but not set this symbol.
This one will actually allow you to set strict seccomp mode even
without the Kconfig symbol, just not allow to set a filter.

	Arnd

Re: [PATCH 2/3] ARM: convert to generated system call tables

[Richard Henderson]

On 10/25/2016 03:28 AM, Arnd Bergmann wrote:
> On Tuesday, October 25, 2016 10:12:10 PM CEST Michael Cree wrote:
>> On Fri, Oct 21, 2016 at 03:06:45PM +0200, Arnd Bergmann wrote:
>>> I see your point, but I think there are serious issues with the current
>>> approach as well:
>>>
>>> - a lot of the less common architectures just don't get updated
>>>   in time, out of 22 architectures that don't use asm-generic/unistd.h,
>>>   only 12 have pwritev2 in linux-next, and only three have pkey_mprotect
>>>
>>> - some architectures that add all syscalls sometimes make a mistake
>>>   and forget one, e.g. alpha apparently never added __NR_bpf, but it
>>>   did add the later __NR_execveat.
>>
>> __NR_bpf was not forgotten on Alpha.  It was not wired up because
>> extra architecture support is needed which has not been implemented.
>>
>> But maybe we should just wire it up to sys_ni_syscall in the meantime
>> so a syscall number is reserved for it, and user space can call it to
>> get -ENOSYS returned.
> 
> Ah, I must have misinterpreted the code then. I assumed that the
> bpf syscall always works on all architectures, but that only the
> jit compiler for it required architecture specific code to make it
> more efficient.

That was my interpretation as well.  What's the problem, Michael?


r~

Re: [PATCH 2/3] ARM: convert to generated system call tables

[Geert Uytterhoeven]

On Tue, Oct 25, 2016 at 12:28 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Tuesday, October 25, 2016 10:12:10 PM CEST Michael Cree wrote:
>> On Fri, Oct 21, 2016 at 03:06:45PM +0200, Arnd Bergmann wrote:
>> > I see your point, but I think there are serious issues with the current
>> > approach as well:
>> >
>> > - a lot of the less common architectures just don't get updated
>> >   in time, out of 22 architectures that don't use asm-generic/unistd.h,
>> >   only 12 have pwritev2 in linux-next, and only three have pkey_mprotect
>> >
>> > - some architectures that add all syscalls sometimes make a mistake
>> >   and forget one, e.g. alpha apparently never added __NR_bpf, but it
>> >   did add the later __NR_execveat.
>>
>> __NR_bpf was not forgotten on Alpha.  It was not wired up because
>> extra architecture support is needed which has not been implemented.
>>
>> But maybe we should just wire it up to sys_ni_syscall in the meantime
>> so a syscall number is reserved for it, and user space can call it to
>> get -ENOSYS returned.
>
> Ah, I must have misinterpreted the code then. I assumed that the
> bpf syscall always works on all architectures, but that only the
> jit compiler for it required architecture specific code to make it
> more efficient.
>
> The implementation of sys_bfp is compile-time selectable at the moment
> and falls back to sys_no_syscall if CONFIG_BPF_SYSCALL is disabled.
> If it doesn't work on Alpha, maybe that symbol could have a "depends
> on !ALPHA" or "depends on BPF_SUPPORT"?

Yes, BPF should just work (m68k has it).

> sys_seccomp is another one that falls into a similar category, but
> it already depends on HAVE_ARCH_SECCOMP_FILTER, and most other
> architectures have assigned a syscall number but not set this symbol.
> This one will actually allow you to set strict seccomp mode even
> without the Kconfig symbol, just not allow to set a filter.

Seccomp needs architecture support (m68k doesn't have it).

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 2/3] ARM: convert to generated system call tables

[Michael Cree]

On Tue, Oct 25, 2016 at 12:28:16PM +0200, Arnd Bergmann wrote:
> On Tuesday, October 25, 2016 10:12:10 PM CEST Michael Cree wrote:
> > On Fri, Oct 21, 2016 at 03:06:45PM +0200, Arnd Bergmann wrote:
> > > I see your point, but I think there are serious issues with the current
> > > approach as well:
> > > 
> > > - a lot of the less common architectures just don't get updated
> > >   in time, out of 22 architectures that don't use asm-generic/unistd.h,
> > >   only 12 have pwritev2 in linux-next, and only three have pkey_mprotect
> > > 
> > > - some architectures that add all syscalls sometimes make a mistake
> > >   and forget one, e.g. alpha apparently never added __NR_bpf, but it
> > >   did add the later __NR_execveat.
> > 
> > __NR_bpf was not forgotten on Alpha.  It was not wired up because
> > extra architecture support is needed which has not been implemented.
> > 
> > But maybe we should just wire it up to sys_ni_syscall in the meantime
> > so a syscall number is reserved for it, and user space can call it to
> > get -ENOSYS returned.
> 
> Ah, I must have misinterpreted the code then. I assumed that the
> bpf syscall always works on all architectures, but that only the
> jit compiler for it required architecture specific code to make it
> more efficient.

Oh.  When someone posted wiring up of syscalls on Alpha some time
back I raised a query about seccomp then someone else (I can't be
bothered looking up the old emails, it doesn't really matter)
said bpf was in the same basket, so the patch was re-submitted with
neither of those syscalls.

> sys_seccomp is another one that falls into a similar category, but
> it already depends on HAVE_ARCH_SECCOMP_FILTER, and most other
> architectures have assigned a syscall number but not set this symbol.
> This one will actually allow you to set strict seccomp mode even
> without the Kconfig symbol, just not allow to set a filter.

We have got way behind on syscalls on Alpha and I was just in the
process of wiring them up and testing them, so I will include both
seccomp and bpf in that.

Cheers
Michael.

bpf on Alpha [was Re: [PATCH 2/3] ARM: convert to generated system call tables]

[Michael Cree]

On Wed, Oct 26, 2016 at 08:04:30PM +1300, Michael Cree wrote:
> On Tue, Oct 25, 2016 at 12:28:16PM +0200, Arnd Bergmann wrote:
> > On Tuesday, October 25, 2016 10:12:10 PM CEST Michael Cree wrote:
> > > On Fri, Oct 21, 2016 at 03:06:45PM +0200, Arnd Bergmann wrote:
> > > > I see your point, but I think there are serious issues with the current
> > > > approach as well:
> > > > 
> > > > - a lot of the less common architectures just don't get updated
> > > >   in time, out of 22 architectures that don't use asm-generic/unistd.h,
> > > >   only 12 have pwritev2 in linux-next, and only three have pkey_mprotect
> > > > 
> > > > - some architectures that add all syscalls sometimes make a mistake
> > > >   and forget one, e.g. alpha apparently never added __NR_bpf, but it
> > > >   did add the later __NR_execveat.
> > > 
> > > __NR_bpf was not forgotten on Alpha.  It was not wired up because
> > > extra architecture support is needed which has not been implemented.
> > > 
> > > But maybe we should just wire it up to sys_ni_syscall in the meantime
> > > so a syscall number is reserved for it, and user space can call it to
> > > get -ENOSYS returned.
> > 
> > Ah, I must have misinterpreted the code then. I assumed that the
> > bpf syscall always works on all architectures, but that only the
> > jit compiler for it required architecture specific code to make it
> > more efficient.
> 
> Oh.  When someone posted wiring up of syscalls on Alpha some time
> back I raised a query about seccomp then someone else (I can't be
> bothered looking up the old emails, it doesn't really matter)
> said bpf was in the same basket, so the patch was re-submitted with
> neither of those syscalls.
> 
> > sys_seccomp is another one that falls into a similar category, but
> > it already depends on HAVE_ARCH_SECCOMP_FILTER, and most other
> > architectures have assigned a syscall number but not set this symbol.
> > This one will actually allow you to set strict seccomp mode even
> > without the Kconfig symbol, just not allow to set a filter.
> 
> We have got way behind on syscalls on Alpha and I was just in the
> process of wiring them up and testing them, so I will include both
> seccomp and bpf in that.

Having just wired up bpf on an Alpha and run samples/bpf/test_verifier
I get:

#0 add+sub+mul OK
#1 unreachable OK
#2 unreachable2 OK
#3 out of range jump OK

[snip many passing tests]

#69 unpriv: check that printk is disallowed FAIL
failed to load prog 'Invalid argument'
0: (7a) *(u64 *)(r10 -8) = 0
1: (bf) r1 = r10
2: (07) r1 += -8
3: (b7) r2 = 8
4: (bf) r3 = r1
5: (85) call 6
unknown func 6

[snip many more passing tests]

Summary: 101 PASSED, 1 FAILED

Should I be concerned about the failing #69 test?

Cheers
Michael.