Re: [PATCH 2/2] powerpc/syscalls: Split SPU-ness out of ABI

[Arnd Bergmann]

On Tue, Jun 16, 2020 at 3:56 PM Michael Ellerman <mpe@xxxxxxxxxxxxxx> wrote:
>
> Using the ABI field to encode whether a syscall is usable by SPU
> programs or not is a bit of kludge.
>
> The ABI of the syscall doesn't change depending on the SPU-ness, but
> in order to make the syscall generation work we have to pretend that
> it does.

The idea of the ABI field is not to identify which ABI a syscall follows
but which ABIs do or do not implement it. This is the same with e.g.
the x32 ABI on x86.

> It also means we have more duplicated syscall lines than we need to,
> and the SPU logic is not well contained, instead all of the syscall
> generation targets need to know if they are spu or nospu.
>
> So instead add a separate file which contains the information on which
> syscalls are available for SPU programs. It's just a list of syscall
> numbers with a single "spu" field. If the field has the value "spu"
> then the syscall is available to SPU programs, any other value or no
> entry entirely means the syscall is not available to SPU programs.
>
> Signed-off-by: Michael Ellerman <mpe@xxxxxxxxxxxxxx>

I have a patch series originally from Firoz that was never quite finished
to unify the scripts across all architectures. I think making the format of
the table format more powerpc specific like you do here takes it a step
backwards and makes it harder to do that eventually.

>  4 files changed, 523 insertions(+), 128 deletions(-)
>  create mode 100644 arch/powerpc/kernel/syscalls/spu.tbl
>
>
> I'm inclined to put this in next and ask Linus to pull it before rc2, that seems
> like the least disruptive way to get this in, unless anyone objects?

I still hope we can get a better solution.

> diff --git a/arch/powerpc/kernel/syscalls/spu.tbl b/arch/powerpc/kernel/syscalls/spu.tbl
> new file mode 100644
> index 000000000000..5eac04919303
> --- /dev/null
> +++ b/arch/powerpc/kernel/syscalls/spu.tbl
> @@ -0,0 +1,430 @@
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# The format is:
> +# <number> <name> <spu>
> +#
> +# To indicate a syscall can be used by SPU programs use "spu" for the spu column.
> +#
> +# Syscalls that are not to be used by SPU programs can be left out of the file
> +# entirely, or an entry with a value other than "spu" can be added.
> +0      restart_syscall                 -
> +1      exit                            -
> +2      fork                            -
> +3      read                            spu
> +4      write                           spu
> +5      open                            spu

Having a new table format here also makes it harder for others to add
a new system call, both because it doesn't follow the syscall*.tbl naming
and because one has to first understand what the format is.

If you absolutely want to split it out, could you at least make the format
compatible with the existing scripts and avoid the change to
the syscalltbl.sh file?

       Arnd