Re: [Qemu-devel] [PATCH v3 12/19] linux-user: Setup split syscall infrastructure

2018-08-12 Thread Richard Henderson 
On 08/12/2018 01:45 PM, Laurent Vivier wrote:
> Le 12/06/2018 à 02:51, Richard Henderson a écrit :
>> Defines a unified structure for implementation and strace.
>> Supplies a generator script to build the declarations and
>> the lookup function.
>>
>> Signed-off-by: Richard Henderson 
>> ---
>>  linux-user/syscall.h   | 178 +++
>>  linux-user/strace.c| 386 -
>>  linux-user/syscall.c   | 113 --
>>  linux-user/Makefile.objs   |  10 +
>>  linux-user/gen_syscall_list.py |  82 +++
>>  5 files changed, 595 insertions(+), 174 deletions(-)
>>  create mode 100644 linux-user/syscall.h
>>  create mode 100644 linux-user/gen_syscall_list.py
> ...
>> diff --git a/linux-user/Makefile.objs b/linux-user/Makefile.objs
>> index 59a5c17354..afa69ed6d2 100644
>> --- a/linux-user/Makefile.objs
>> +++ b/linux-user/Makefile.objs
>> @@ -7,3 +7,13 @@ obj-$(TARGET_HAS_BFLT) += flatload.o
>>  obj-$(TARGET_I386) += vm86.o
>>  obj-$(TARGET_ARM) += arm/nwfpe/
>>  obj-$(TARGET_M68K) += m68k-sim.o
>> +
>> +GEN_SYSCALL_LIST = $(SRC_PATH)/linux-user/gen_syscall_list.py
>> +SYSCALL_LIST = linux-user/syscall_list.h linux-user/syscall_list.inc.c
>> +
>> +$(SYSCALL_LIST): $(GEN_SYSCALL_LIST)
>> +$(call quiet-command,\
>> +  $(PYTHON) $(GEN_SYSCALL_LIST) $@, "GEN", $(TARGET_DIR)$@)
>> +
>> +linux-user/syscall.o \
>> +linux-user/strace.o: $(SYSCALL_LIST)
>> diff --git a/linux-user/gen_syscall_list.py b/linux-user/gen_syscall_list.py
>> new file mode 100644
>> index 00..2e0fc39100
>> --- /dev/null
>> +++ b/linux-user/gen_syscall_list.py
>> @@ -0,0 +1,82 @@
>> +#
>> +# Linux syscall table generator
>> +# Copyright (c) 2018 Linaro, Limited.
>> +#
>> +# This program is free software; you can redistribute it and/or modify
>> +# it under the terms of the GNU General Public License as published by
>> +# the Free Software Foundation; either version 2 of the License, or
>> +# (at your option) any later version.
>> +#
>> +# 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 .
>> +#
>> +
>> +from __future__ import print_function
>> +import sys
>> +
>> +# These are sets of all syscalls that have been converted.
>> +# The lists are in collation order with '_' ignored.
>> +
>> +# These syscalls are supported by all targets.
>> +# Avoiding ifdefs for these can diagnose typos in $cpu/syscall_nr.h
>> +unconditional_syscalls = [
>> +]
>> +
>> +# These syscalls are only supported by some target or abis.
>> +conditional_syscalls = [
>> +]
>> +
>> +
>> +def header(f):
>> +# Do not ifdef the declarations -- their use may be complicated.
>> +all = unconditional_syscalls + conditional_syscalls
>> +all.sort()
>> +for s in all:
>> +print("extern const SyscallDef def_", s, ";", sep = '', file = f)
>> +
>> +
>> +def def_syscall(s, f):
>> +print("case TARGET_NR_", s, ": return _", s, ";",
>> +  sep = '', file = f);
>> +
>> +
>> +def source(f):
>> +print("static const SyscallDef *syscall_table(int num)",
>> +  "{",
>> +  "switch (num) {",
>> +  sep = '\n', file = f)
>> +
>> +for s in unconditional_syscalls:
>> +def_syscall(s, f)
>> +for s in conditional_syscalls:
>> +print("#ifdef TARGET_NR_", s, sep = '', file = f)
>> +def_syscall(s, f)
>> +print("#endif", file = f)
>> +
>> +print("}",
>> +  "return NULL;",
>> +  "}",
>> +  sep = '\n', file = f);
>> +
>> +
>> +def main():
>> +p = sys.argv[1]
>> +f = open(p, "w")
>> +
>> +print("/* This file is autogenerated by gensyscall.py.  */\n\n",
>> +  file = f)
>> +
>> +if p[len(p) - 1] == 'h':
>> +header(f)
>> +else:
>> +source(f)
>> +
>> +f.close();
>> +
>> +
>> +main()
>>
> 
> As we can see in patch 19/19 it's easy to forget to update the syscalls
> lists.
> 
> Should it be possible to generate syscalls switch from the macro we
> already have?

I didn't see how, right off.

It was possible when all of the syscalls were still in the same file, as we
could get defined-but-unused warnings.  With them in different files, we don't
get that.

Perhaps there's a way that

> static const SyscallDef *syscall_table(int num)
> {
> switch (num) {
> #include "syscall_file.def"
> #include "syscall_ipc.def"
> #include "syscall_mem.def"
> #include "syscall_proc.def"
> }
> return NULL;
> }
> 
> and in syscall_proc.def:

something like this achieves that, via missing-prototype warnings.

But if we have a structure like this, I wonder if we're better off with

#include "syscall_file.inc.c"
#include

Re: [Qemu-devel] [PATCH v3 12/19] linux-user: Setup split syscall infrastructure

2018-08-12 Thread Laurent Vivier 
Le 12/06/2018 à 02:51, Richard Henderson a écrit :
> Defines a unified structure for implementation and strace.
> Supplies a generator script to build the declarations and
> the lookup function.
> 
> Signed-off-by: Richard Henderson 
> ---
>  linux-user/syscall.h   | 178 +++
>  linux-user/strace.c| 386 -
>  linux-user/syscall.c   | 113 --
>  linux-user/Makefile.objs   |  10 +
>  linux-user/gen_syscall_list.py |  82 +++
>  5 files changed, 595 insertions(+), 174 deletions(-)
>  create mode 100644 linux-user/syscall.h
>  create mode 100644 linux-user/gen_syscall_list.py
...
> diff --git a/linux-user/Makefile.objs b/linux-user/Makefile.objs
> index 59a5c17354..afa69ed6d2 100644
> --- a/linux-user/Makefile.objs
> +++ b/linux-user/Makefile.objs
> @@ -7,3 +7,13 @@ obj-$(TARGET_HAS_BFLT) += flatload.o
>  obj-$(TARGET_I386) += vm86.o
>  obj-$(TARGET_ARM) += arm/nwfpe/
>  obj-$(TARGET_M68K) += m68k-sim.o
> +
> +GEN_SYSCALL_LIST = $(SRC_PATH)/linux-user/gen_syscall_list.py
> +SYSCALL_LIST = linux-user/syscall_list.h linux-user/syscall_list.inc.c
> +
> +$(SYSCALL_LIST): $(GEN_SYSCALL_LIST)
> + $(call quiet-command,\
> +   $(PYTHON) $(GEN_SYSCALL_LIST) $@, "GEN", $(TARGET_DIR)$@)
> +
> +linux-user/syscall.o \
> +linux-user/strace.o: $(SYSCALL_LIST)
> diff --git a/linux-user/gen_syscall_list.py b/linux-user/gen_syscall_list.py
> new file mode 100644
> index 00..2e0fc39100
> --- /dev/null
> +++ b/linux-user/gen_syscall_list.py
> @@ -0,0 +1,82 @@
> +#
> +# Linux syscall table generator
> +# Copyright (c) 2018 Linaro, Limited.
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# 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 .
> +#
> +
> +from __future__ import print_function
> +import sys
> +
> +# These are sets of all syscalls that have been converted.
> +# The lists are in collation order with '_' ignored.
> +
> +# These syscalls are supported by all targets.
> +# Avoiding ifdefs for these can diagnose typos in $cpu/syscall_nr.h
> +unconditional_syscalls = [
> +]
> +
> +# These syscalls are only supported by some target or abis.
> +conditional_syscalls = [
> +]
> +
> +
> +def header(f):
> +# Do not ifdef the declarations -- their use may be complicated.
> +all = unconditional_syscalls + conditional_syscalls
> +all.sort()
> +for s in all:
> +print("extern const SyscallDef def_", s, ";", sep = '', file = f)
> +
> +
> +def def_syscall(s, f):
> +print("case TARGET_NR_", s, ": return _", s, ";",
> +  sep = '', file = f);
> +
> +
> +def source(f):
> +print("static const SyscallDef *syscall_table(int num)",
> +  "{",
> +  "switch (num) {",
> +  sep = '\n', file = f)
> +
> +for s in unconditional_syscalls:
> +def_syscall(s, f)
> +for s in conditional_syscalls:
> +print("#ifdef TARGET_NR_", s, sep = '', file = f)
> +def_syscall(s, f)
> +print("#endif", file = f)
> +
> +print("}",
> +  "return NULL;",
> +  "}",
> +  sep = '\n', file = f);
> +
> +
> +def main():
> +p = sys.argv[1]
> +f = open(p, "w")
> +
> +print("/* This file is autogenerated by gensyscall.py.  */\n\n",
> +  file = f)
> +
> +if p[len(p) - 1] == 'h':
> +header(f)
> +else:
> +source(f)
> +
> +f.close();
> +
> +
> +main()
> 

As we can see in patch 19/19 it's easy to forget to update the syscalls
lists.

Should it be possible to generate syscalls switch from the macro we
already have?

Something like (it should not be as simple as this but...):

#define SYSCALL_DEF(NAME, ...) \
case TARGET_NR_##NAME: return @def_##NAME

static const SyscallDef *syscall_table(int num)
{
switch (num) {
#include "syscall_file.def"
#include "syscall_ipc.def"
#include "syscall_mem.def"
#include "syscall_proc.def"
}
return NULL;
}

and in syscall_proc.def:

...
SYSCALL_DEF(clone)
#ifdef TARGET_NR_fork
SYSCALL_DEF(fork);
#endif
#ifdef TARGET_NR_vfork
SYSCALL_DEF(vfork);
#endif
...
SYSCALL_DEF(set_tid_address, ARG_PTR);

and in syscall_proc.c:
...
SYSCALL_IMPL(set_tid_address)
{
return get_errno(set_tid_address((int *)g2h(arg1)));
}
#include "syscall_proc.def".

So compiler will detect any unused references (if SYSCALL_DEF doesn't
match SYSCALL_IMPL), the syscall_table has automatically all implemented
syscalls,

Re: [Qemu-devel] [PATCH v3 12/19] linux-user: Setup split syscall infrastructure

2018-06-22 Thread Richard Henderson 
On 06/22/2018 03:30 AM, Peter Maydell wrote:
>> +struct SyscallDef {
>> +const char *name;
>> +SyscallArgsFn *args;
>> +SyscallImplFn *impl;
>> +SyscallPrintFn *print;
>> +SyscallPrintRetFn *print_ret;
> 
> Are all these hook functions mandatory, or can a syscall
> implementation leave some of them NULL for a default behaviour?

Only impl is mandatory.  You're right that there should be more doco here at
the structure definition.  Here, the docs got split down to the macros that
define the most common forms of the structures.  Will fix.


>>  static void
>>  print_flags(const struct flags *f, abi_long flags, int last)
>>  {
>> -const char *sep = "";
>> -int n;
>> +char buf[256];
>> +add_flags(buf, sizeof(buf), f, flags, false);
>> +gemu_log("%s%s", buf, get_comma(last));
>> +}
> 
> All this refactoring of the strace functions feels like it
> should have been in a separate patch...

Fair.


r~

Re: [Qemu-devel] [PATCH v3 12/19] linux-user: Setup split syscall infrastructure

2018-06-22 Thread Peter Maydell 
On 12 June 2018 at 01:51, Richard Henderson
 wrote:
> Defines a unified structure for implementation and strace.
> Supplies a generator script to build the declarations and
> the lookup function.
>
> Signed-off-by: Richard Henderson 
> ---
>  linux-user/syscall.h   | 178 +++
>  linux-user/strace.c| 386 -
>  linux-user/syscall.c   | 113 --
>  linux-user/Makefile.objs   |  10 +
>  linux-user/gen_syscall_list.py |  82 +++
>  5 files changed, 595 insertions(+), 174 deletions(-)
>  create mode 100644 linux-user/syscall.h
>  create mode 100644 linux-user/gen_syscall_list.py
>
> diff --git a/linux-user/syscall.h b/linux-user/syscall.h
> new file mode 100644
> index 00..7eb078c3e5
> --- /dev/null
> +++ b/linux-user/syscall.h
> @@ -0,0 +1,178 @@
> +/*
> + *  Linux syscalls internals
> + *  Copyright (c) 2018 Linaro, Limited.
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation; either version 2 of the License, or
> + *  (at your option) any later version.
> + *
> + *  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 .
> + */
> +
> +typedef struct SyscallDef SyscallDef;
> +
> +/* This hook extracts max 6 arguments from max 8 input registers.
> + * In the process, register pairs that store 64-bit arguments are merged.
> + * Finally, syscalls are demultipliexed; e.g. the hook for socketcall will

"demultiplexed"

> + * return the SyscallDef for bind, listen, etc.  In the process the hook
> + * may need to read from guest memory, or otherwise validate operands.
> + * On failure, set errno (to a host value) and return NULL;
> + * the (target adjusted) errno will be returned to the guest.
> + */
> +typedef const SyscallDef *SyscallArgsFn(const SyscallDef *, int64_t out[6],
> +abi_long in[8]);
> +
> +/* This hook implements the syscall.  */
> +typedef abi_long SyscallImplFn(CPUArchState *, int64_t, int64_t, int64_t,
> +   int64_t, int64_t, int64_t);
> +
> +/* This hook prints the arguments to the syscall for strace.  */
> +typedef void SyscallPrintFn(const SyscallDef *, int64_t arg[6]);
> +
> +/* This hook print the return value from the syscall for strace.  */
> +typedef void SyscallPrintRetFn(const SyscallDef *, abi_long);
> +
> +/* These flags describe the arguments for the generic fallback to
> + * SyscallPrintFn.  ARG_NONE indicates that the argument is not present.
> + */
> +typedef enum {
> +ARG_NONE = 0,
> +
> +/* These print as numbers of abi_long.  */
> +ARG_DEC,
> +ARG_HEX,
> +ARG_OCT,
> +
> +/* These print as sets of flags.  */
> +ARG_ATDIRFD,
> +ARG_MODEFLAG,
> +ARG_OPENFLAG,
> +
> +/* These are interpreted as pointers.  */
> +ARG_PTR,
> +ARG_STR,
> +ARG_BUF,
> +
> +/* For a 32-bit host, force printing as a 64-bit operand.  */
> +#if TARGET_ABI_BITS == 32
> +ARG_DEC64,
> +#else
> +ARG_DEC64 = ARG_DEC,
> +#endif
> +} SyscallArgType;
> +
> +struct SyscallDef {
> +const char *name;
> +SyscallArgsFn *args;
> +SyscallImplFn *impl;
> +SyscallPrintFn *print;
> +SyscallPrintRetFn *print_ret;

Are all these hook functions mandatory, or can a syscall
implementation leave some of them NULL for a default behaviour?

> +SyscallArgType arg_type[6];
> +};
> +
> +void print_syscall_def(const SyscallDef *def, int64_t args[6]);
> +void print_syscall_def_ret(const SyscallDef *def, abi_long ret);
> +void print_syscall_ptr_ret(const SyscallDef *def, abi_long ret);
> +
> +/* Emit the signature for a SyscallArgsFn.  */
> +#define SYSCALL_ARGS(NAME) \
> +static const SyscallDef *args_##NAME(const SyscallDef *def, \
> + int64_t out[6], abi_long in[8])
> +
> +/* Emit the signature for a SyscallImplFn.  */
> +#define SYSCALL_IMPL(NAME) \
> +static abi_long impl_##NAME(CPUArchState *cpu_env, int64_t arg1, \
> +int64_t arg2, int64_t arg3, int64_t arg4, \
> +int64_t arg5, int64_t arg6)
> +
> +/* Emit the definition for a "simple" syscall.  Such does not use
> + * SyscallArgsFn and only uses arg_type for strace.
> + */
> +#define SYSCALL_DEF(NAME, ...) \
> +const SyscallDef def_##NAME = { \
> +.name = #NAME, .impl = impl_##NAME, .arg_type = { __VA_ARGS__ } \
> +}
> +
> +/* Emit the definition for a syscall that also has an args hook,
> + * and uses arg_type for strace.
> + */

[Qemu-devel] [PATCH v3 12/19] linux-user: Setup split syscall infrastructure

2018-06-11 Thread Richard Henderson 
Defines a unified structure for implementation and strace.
Supplies a generator script to build the declarations and
the lookup function.

Signed-off-by: Richard Henderson 
---
 linux-user/syscall.h   | 178 +++
 linux-user/strace.c| 386 -
 linux-user/syscall.c   | 113 --
 linux-user/Makefile.objs   |  10 +
 linux-user/gen_syscall_list.py |  82 +++
 5 files changed, 595 insertions(+), 174 deletions(-)
 create mode 100644 linux-user/syscall.h
 create mode 100644 linux-user/gen_syscall_list.py

diff --git a/linux-user/syscall.h b/linux-user/syscall.h
new file mode 100644
index 00..7eb078c3e5
--- /dev/null
+++ b/linux-user/syscall.h
@@ -0,0 +1,178 @@
+/*
+ *  Linux syscalls internals
+ *  Copyright (c) 2018 Linaro, Limited.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  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 .
+ */
+
+typedef struct SyscallDef SyscallDef;
+
+/* This hook extracts max 6 arguments from max 8 input registers.
+ * In the process, register pairs that store 64-bit arguments are merged.
+ * Finally, syscalls are demultipliexed; e.g. the hook for socketcall will
+ * return the SyscallDef for bind, listen, etc.  In the process the hook
+ * may need to read from guest memory, or otherwise validate operands.
+ * On failure, set errno (to a host value) and return NULL;
+ * the (target adjusted) errno will be returned to the guest.
+ */
+typedef const SyscallDef *SyscallArgsFn(const SyscallDef *, int64_t out[6],
+abi_long in[8]);
+
+/* This hook implements the syscall.  */
+typedef abi_long SyscallImplFn(CPUArchState *, int64_t, int64_t, int64_t,
+   int64_t, int64_t, int64_t);
+
+/* This hook prints the arguments to the syscall for strace.  */
+typedef void SyscallPrintFn(const SyscallDef *, int64_t arg[6]);
+
+/* This hook print the return value from the syscall for strace.  */
+typedef void SyscallPrintRetFn(const SyscallDef *, abi_long);
+
+/* These flags describe the arguments for the generic fallback to
+ * SyscallPrintFn.  ARG_NONE indicates that the argument is not present.
+ */
+typedef enum {
+ARG_NONE = 0,
+
+/* These print as numbers of abi_long.  */
+ARG_DEC,
+ARG_HEX,
+ARG_OCT,
+
+/* These print as sets of flags.  */
+ARG_ATDIRFD,
+ARG_MODEFLAG,
+ARG_OPENFLAG,
+
+/* These are interpreted as pointers.  */
+ARG_PTR,
+ARG_STR,
+ARG_BUF,
+
+/* For a 32-bit host, force printing as a 64-bit operand.  */
+#if TARGET_ABI_BITS == 32
+ARG_DEC64,
+#else
+ARG_DEC64 = ARG_DEC,
+#endif
+} SyscallArgType;
+
+struct SyscallDef {
+const char *name;
+SyscallArgsFn *args;
+SyscallImplFn *impl;
+SyscallPrintFn *print;
+SyscallPrintRetFn *print_ret;
+SyscallArgType arg_type[6];
+};
+
+void print_syscall_def(const SyscallDef *def, int64_t args[6]);
+void print_syscall_def_ret(const SyscallDef *def, abi_long ret);
+void print_syscall_ptr_ret(const SyscallDef *def, abi_long ret);
+
+/* Emit the signature for a SyscallArgsFn.  */
+#define SYSCALL_ARGS(NAME) \
+static const SyscallDef *args_##NAME(const SyscallDef *def, \
+ int64_t out[6], abi_long in[8])
+
+/* Emit the signature for a SyscallImplFn.  */
+#define SYSCALL_IMPL(NAME) \
+static abi_long impl_##NAME(CPUArchState *cpu_env, int64_t arg1, \
+int64_t arg2, int64_t arg3, int64_t arg4, \
+int64_t arg5, int64_t arg6)
+
+/* Emit the definition for a "simple" syscall.  Such does not use
+ * SyscallArgsFn and only uses arg_type for strace.
+ */
+#define SYSCALL_DEF(NAME, ...) \
+const SyscallDef def_##NAME = { \
+.name = #NAME, .impl = impl_##NAME, .arg_type = { __VA_ARGS__ } \
+}
+
+/* Emit the definition for a syscall that also has an args hook,
+ * and uses arg_type for strace.
+ */
+#define SYSCALL_DEF_ARGS(NAME, ...) \
+const SyscallDef def_##NAME = { \
+.name = #NAME, .args = args_##NAME, .impl = impl_##NAME, \
+.arg_type = { __VA_ARGS__ } \
+}
+
+/* Declarations from the main syscall.c for use in syscall_foo.c,
+ * or for the moment, vice versa.
+ */
+
+int host_to_target_errno(int err);
+
+static inline abi_long get_errno(abi_long ret)
+{
+return unlikely(ret == -1) ?