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

[Simon Kuenzer <simon.kuenzer@xxxxxxxxx>]

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>
> ---
>   lib/Config.uk                  |  1 +
>   lib/Makefile.uk                |  1 +
>   lib/syscall_shim/Config.uk     |  3 +++
>   lib/syscall_shim/Makefile.uk   | 40 ++++++++++++++++++++++++++++++++++
>   lib/syscall_shim/gen_stubs.awk |  8 +++++++
>   5 files changed, 53 insertions(+)
>   create mode 100644 lib/syscall_shim/Config.uk
>   create mode 100644 lib/syscall_shim/Makefile.uk
>   create mode 100644 lib/syscall_shim/gen_stubs.awk
>
> diff --git a/lib/Config.uk b/lib/Config.uk
> index 822c624f..b517abd8 100644
> --- a/lib/Config.uk
> +++ b/lib/Config.uk
> @@ -47,3 +47,4 @@ source "lib/ukswrand/Config.uk"
>   source "lib/ukbus/Config.uk"
>   source "lib/uksglist/Config.uk"
>   source "lib/uknetdev/Config.uk"
> +source "lib/syscall_shim/Config.uk"
> diff --git a/lib/Makefile.uk b/lib/Makefile.uk
> index d06837f1..e6664952 100644
> --- a/lib/Makefile.uk
> +++ b/lib/Makefile.uk
> @@ -24,3 +24,4 @@ $(eval $(call _import_lib,$(CONFIG_UK_BASE)/lib/ukmpi))
>   $(eval $(call _import_lib,$(CONFIG_UK_BASE)/lib/ukbus))
>   $(eval $(call _import_lib,$(CONFIG_UK_BASE)/lib/uksglist))
>   $(eval $(call _import_lib,$(CONFIG_UK_BASE)/lib/uknetdev))
> +$(eval $(call _import_lib,$(CONFIG_UK_BASE)/lib/syscall_shim))
> diff --git a/lib/syscall_shim/Config.uk b/lib/syscall_shim/Config.uk
> new file mode 100644
> index 00000000..f88cf970
> --- /dev/null
> +++ b/lib/syscall_shim/Config.uk
> @@ -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.
> +
> +__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`)?
Also, wouldn't be `.gen` be a nicer extension than `.new`?

> +
> +$(__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;
> +}
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel