Re: [Minios-devel] [UNIKRAFT PATCH 05/10] lib/syscall_shim: generate base headers

[Yuri Volchkov <yuri.volchkov@xxxxxxxxx>]

Simon Kuenzer <simon.kuenzer@xxxxxxxxx> writes:

> On 04.06.19 18:28, Yuri Volchkov wrote:
>> The following headers are generated by the code in this patch:
>>    - syscall_nrs.h - just definitions of all the syscalls with their
>>      numbers
>>    - syscall_map.h  - provides maps from syscall number to a concrete
>>      function in the Unikarft
>>    - syscall_stubs.h - if no library is providing a syscall, it will be
>>      resolved to a stub, defined in this file
>> 
>> Signed-off-by: Yuri Volchkov <yuri.volchkov@xxxxxxxxx>
>> ---
>> @@ -0,0 +1,3 @@
>> +config LIBSYSCALL_SHIM
>> +    bool "Syscall shim layer"
>> +    default n
>> diff --git a/lib/syscall_shim/Makefile.uk b/lib/syscall_shim/Makefile.uk
>> new file mode 100644
>> index 00000000..52a12f18
>> --- /dev/null
>> +++ b/lib/syscall_shim/Makefile.uk
>> @@ -0,0 +1,40 @@
>> +$(eval $(call addlib_s,libsyscall_shim,$(CONFIG_LIBSYSCALL_SHIM)))
>> +
>> +__GEN_INCLUDES_PATH := $(LIBSYSCALL_SHIM_BUILD)/include/uk/bits
>
> One comment to the `__` prefixed variables: I prefer all of them to be 
> namespaced with `LIBSYSCALL_SHIM_` instead. We have just this as 
> namespacing scheme so far and it also makes `make print-vars` more 
> understandable.
Makes sense. That was quit an adventure, but I have re-prefixed all the
variables in this makefile :)

>
>> +
>> +__PHONY_GEN_SRC := syscall_map.h syscall_stubs.h syscall_nrs.h
>> +__PHONY_GEN_SRC := $(addprefix $(__GEN_INCLUDES_PATH)/, $(__PHONY_GEN_SRC))
>> +__PHONY_GEN_SRC_NEW := $(addsuffix .new, $(__PHONY_GEN_SRC))
>> +
>> +UK_PREPARE-$(CONFIG_LIBSYSCALL_SHIM) += $(__PHONY_GEN_SRC)
>> +
>> +
>> +__SYSCALL_SHIM_TEMPL := 
>> $(LIBSYSCALL_SHIM_BASE)/arch/$(CONFIG_UK_ARCH)/syscall.h.in
>> +
>> +$(call uk_mk_dir, $(__GEN_INCLUDES_PATH))
>
> Hum, now I understand what you want to do. In this case you are lucky 
> and create a directory under the build directory. Maybe a 
> `mk_lib_sub_dir` would make more sense than a general `uk_mk_dir`. I 
> could imagine the following syntax within this file:
>
> $(call mk_lib_sub_dir,libsyscall_shim,include/uk/bits)
>
> Or maybe even:
>
> __GEN_INCLUDES_PATH := $(call lib_sub_dir,libsyscall_shim,include/uk/bits)
>
>> +
>> +.PHONY: $(__PHONY_GEN_SRC_NEW)
>> +
>> +$(__PHONY_GEN_SRC): %: %.new
>> +    @cmp -s $^ $@; if [ $$? -ne 0 ]; then cp $^ $@; fi
>> +
>> +$(__GEN_INCLUDES_PATH)/syscall_nrs.h.new:
>> +    $(Q) awk 'BEGIN {print "/* Automatically generated file; DO NOT EDIT 
>> */"} \
>> +            {printf "\n#define SYS_%s\t\t%s", \
>> +            substr($$2,6),$$3}' \
>> +            $(__SYSCALL_SHIM_TEMPL) > $@
>> +
>> +$(__GEN_INCLUDES_PATH)/syscall_map.h.new:
>> +    $(Q) awk 'BEGIN \
>> +            {print "/* Automatically generated file; DO NOT EDIT */\n"} \
>> +            /#define __NR_/{\
>> +                    printf "#define uk_syscall_fn_%s(...) 
>> uk_syscall_%s(__VA_ARGS__)\n", \
>> +                            $$3,substr($$2,6)\
>> +            }' \
>> +            $(__SYSCALL_SHIM_TEMPL) > $@
>
> You used `$(AWK)` instead of `awk` in the following rule. Could you use 
> `$(AWK)` everywhere instead?
> Instead of making all these rules quiet, could you make them as a 
> pretty-printed build command (`build_cmd`)?
Ok, but no more inline awk scripts, because it does not work with
build_cmd. I moved everything to awk scripts

> Also, wouldn't be `.gen` be a nicer extension than `.new`?
It follows the existing pattern for $(KCONFIG_ELIB_IN).

>
>> +
>> +$(__GEN_INCLUDES_PATH)/syscall_stubs.h.new:
>> +    $(Q) $(AWK) -f $(LIBSYSCALL_SHIM_BASE)/gen_stubs.awk \
>> +            $(__SYSCALL_SHIM_TEMPL) > $@
>> +
>
> I think all of those generated files should become part of the prepare 
> step, right?:
>
>   UK_PREPARE-$(CONFIG_LIBSYSCALL_SHIM) += $(__PHONY_GEN_SRC_NEW)
>
>> +LIBSYSCALL_SHIM_CLEAN = $(__PHONY_GEN_SRC) $(__PHONY_GEN_SRC_NEW)
>> diff --git a/lib/syscall_shim/gen_stubs.awk b/lib/syscall_shim/gen_stubs.awk
>> new file mode 100644
>> index 00000000..9e0ad6c6
>> --- /dev/null
>> +++ b/lib/syscall_shim/gen_stubs.awk
>> @@ -0,0 +1,8 @@
>> +BEGIN { print "/* Auto generated file. Do not edit */" }
>> +{
>> +    name = substr($2,6);
>> +    uk_name = "uk_syscall_" name
>> +    printf "\n#ifndef HAVE_%s", uk_name;
>> +    printf "\n#define %s(...) uk_syscall_stub(\"%s\")", uk_name, name;
>> +    printf "\n#endif /* HAVE_%s */\n", uk_name;
>> +}
>> 

-- 
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