Re: [Minios-devel] [UNIKRAFT PATCH 09/10] lib/syscall_shim: introduce uk_syscall_dynamic

[Simon Kuenzer <simon.kuenzer@xxxxxxxxx>]

Hey Yuri,

thanks for this patch. As with the previous patches regarding 
Makefile.uk, can you apply the same changes here (build_cmd and variable 
name spacing)?
See my other comments inline.

On 04.06.19 18:28, Yuri Volchkov wrote:
> The macro layer is efficient, but the exact syscall must be known at
> the compilation stage. If a syscall number is kept in a variable macro
> layer will not be able to resolve it to the syscall function.
>
> For this case another file is generated - syscall_entry.c. Where
> syscalls are 'dynamically' resolved into a destination functions.
>
> Signed-off-by: Yuri Volchkov <yuri.volchkov@xxxxxxxxx>
> ---
>   lib/syscall_shim/Makefile.uk          |  9 +++++-
>   lib/syscall_shim/entry.c.in_end       | 15 ++++++++++
>   lib/syscall_shim/gen_entry.awk        | 40 +++++++++++++++++++++++++++
>   lib/syscall_shim/include/uk/syscall.h |  2 ++
>   4 files changed, 65 insertions(+), 1 deletion(-)
>   create mode 100644 lib/syscall_shim/entry.c.in_end
>   create mode 100644 lib/syscall_shim/gen_entry.awk
>
> diff --git a/lib/syscall_shim/Makefile.uk b/lib/syscall_shim/Makefile.uk
> index 67677fc6..5ea15963 100644
> --- a/lib/syscall_shim/Makefile.uk
> +++ b/lib/syscall_shim/Makefile.uk
> @@ -7,7 +7,8 @@ __PHONY_GEN_SRC := $(addprefix $(__GEN_INCLUDES_PATH)/, 
> $(__PHONY_GEN_SRC))
>   __PHONY_GEN_SRC += $(LIBSYSCALL_SHIM_BUILD)/provided_syscalls.h.in
>   __PHONY_GEN_SRC_NEW := $(addsuffix .new, $(__PHONY_GEN_SRC))
>   
> -__GEN_SRC := $(__GEN_INCLUDES_PATH)/provided_syscalls.h
> +__GEN_SRC := $(__GEN_INCLUDES_PATH)/provided_syscalls.h \
> +               $(LIBSYSCALL_SHIM_BUILD)/syscall_entry.c
>   
>   UK_PREPARE-$(CONFIG_LIBSYSCALL_SHIM) += $(__PHONY_GEN_SRC) $(__GEN_SRC)
>   
> @@ -43,6 +44,10 @@ $(__GEN_INCLUDES_PATH)/provided_syscalls.h: $(LIBSYSCALL_SHIM_BUILD)/provided_sy
>          printf "\nUK_SYSCALL_PROTO(%s, %s);\n", $$2, $$1;}' \
>         $^ > $@
>   
> +$(LIBSYSCALL_SHIM_BUILD)/syscall_entry.c: $(LIBSYSCALL_SHIM_BUILD)/provided_syscalls.h.in $(LIBSYSCALL_SHIM_BASE)/gen_entry.awk
> +       $(Q) awk -F '-' -f $(LIBSYSCALL_SHIM_BASE)/gen_entry.awk $< > $@
> +       $(Q) cat $(LIBSYSCALL_SHIM_BASE)/entry.c.in_end >> $@
> +
>   $(LIBSYSCALL_SHIM_BUILD)/provided_syscalls.h.in.new:
>         $Q echo $(UK_PROVIDED_SYSCALLS-y) | tr ' ' '\n' > $@
>   
> @@ -55,4 +60,6 @@ CXXINCLUDES-$(CONFIG_LIBSYSCALL_SHIM) += -I$(LIBSYSCALL_SHIM_BUILD)/include
>   CINCLUDES-y   += -I$(LIBSYSCALL_SHIM_BASE)/include
>   CXXINCLUDES-y += -I$(LIBSYSCALL_SHIM_BASE)/include
>   
> +LIBSYSCALL_SHIM_SRCS-y += $(LIBSYSCALL_SHIM_BUILD)/syscall_entry.c
> +
>   LIBSYSCALL_SHIM_CLEAN = $(__PHONY_GEN_SRC) $(__PHONY_GEN_SRC_NEW) $(__GEN_SRC)
> diff --git a/lib/syscall_shim/entry.c.in_end b/lib/syscall_shim/entry.c.in_end
> new file mode 100644
> index 00000000..58145bbc
> --- /dev/null
> +++ b/lib/syscall_shim/entry.c.in_end
> @@ -0,0 +1,15 @@
> +
Hum, I am not hundred percent happy with the name `dynamic`. What if you 
call this function just uk_syscall(long n, ...) instead? This would be 
more inline with what you know from Linux with long syscall(long 
numbner, ...) from <sys/syscall.h>.
> +long uk_syscall_dynamic(long n, ...)
> +{
> +       va_list ap;
> +       syscall_arg_t a,b,c,d,e,f;
> +       va_start(ap, n);
> +       a=va_arg(ap, long);
> +       b=va_arg(ap, long);
> +       c=va_arg(ap, long);
> +       d=va_arg(ap, long);
> +       e=va_arg(ap, long);
> +       f=va_arg(ap, long);
> +       va_end(ap);
> +       return __syscall_dynamic(n,a,b,c,d,e,f);
> +}
> diff --git a/lib/syscall_shim/gen_entry.awk b/lib/syscall_shim/gen_entry.awk
> new file mode 100644
> index 00000000..92457089
> --- /dev/null
> +++ b/lib/syscall_shim/gen_entry.awk
> @@ -0,0 +1,40 @@
> +BEGIN {
> +       max_args = 6
> +       print "/* Auto generated file. DO NOT EDIT */\n\n"
> +
> +       print "#include <uk/syscall.h>"
> +       print "#include <uk/print.h>\n"
> +
> +       printf "static inline long __syscall_dynamic(long nr, "
> +       for (i = 1; i < max_args; i++)
> +               printf "long arg%d, ",i
> +       printf "long arg%d)\n{\n", max_args
> +
> +       for (i = 1; i <= max_args; i++)
> +               printf "\t(void) arg%d;\n", i
> +
> +       print "\n\tswitch (nr) {"
> +}
> +
> +
> +/[a-zA-Z0-9]+-[0-9]+/{
> +       name = $1
> +       sys_name = "SYS_" name
> +       uk_name = "uk_syscall_" name
> +       args_nr = $2 + 0
> +       printf "\tcase %s:\n", sys_name;
> +       printf "\t\treturn %s(", uk_name;
> +       for (i = 1; i < args_nr; i++)
> +               printf("arg%d, ", i)
> +       if (args_nr > 0)
> +               printf("arg%d", args_nr)
> +       printf(");\n")
> +}
> +
> +END {
> +       printf "\tdefault:\n"
> +       printf "\t\tuk_pr_debug(\"syscall nr %%ld is not implemented\", nr);\n"
> +       printf "\t\terrno = -ENOSYS;\n"
> +       printf "\t\treturn -1;\n"
> +       printf "\t}\n}\n"
> +}
> diff --git a/lib/syscall_shim/include/uk/syscall.h 
> b/lib/syscall_shim/include/uk/syscall.h
> index 19161e5e..13913bab 100644
> --- a/lib/syscall_shim/include/uk/syscall.h
> +++ b/lib/syscall_shim/include/uk/syscall.h
> @@ -93,6 +93,8 @@ typedef long syscall_arg_t;
>   #include <uk/bits/provided_syscalls.h>
>   #include <uk/bits/syscall_stubs.h>
>   
> +long uk_syscall_dynamic(long n, ...);
> +
>   #define syscall(...)                                                  \
>         UK_CONCAT(__uk_syscall, __SYSCALL_NARGS(__VA_ARGS__))(__VA_ARGS__)
>   #endif
Thanks,

Simon

_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel