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

[Yuri Volchkov <yuri.volchkov@xxxxxxxxx>]

Simon Kuenzer <simon.kuenzer@xxxxxxxxx> writes:

> 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)?
Ok
>
> 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>.
OK
>
>> +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

-- 
Yuri Volchkov
Software Specialist

NEC Europe Ltd
Kurfürsten-Anlage 36
D-69115 Heidelberg

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