|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Minios-devel] [UNIKRAFT PATCH 2/9] include: Clean-ups to <uk/ctors.h>
Hi Simon,
Thanks for the patch! Please see my comments inline.
Thanks,
Vlad
On 29.01.2020 23:48, Simon Kuenzer wrote:
> This commit brings the Unikraft constructor definitions closer to the
> one of the Unikraft Init table.
>
> UK_CTOR_FUNC() is basically replaced by UK_CTOR_PRIO(). Naming and
> argument order is derived from Init table definition <uk/init.h>.
>
> The table iterator is now similar as the one found in the Init
> table. Instead of providing an integer as index, the foreach-macro is
> directly iterating with a function pointer.
>
> Because of compatibility reasons, the previous macro UK_CTOR_FUNC() is
> still provided. The idea is that it gets removed as soon all depending
> code was updated (internal and external).
>
> Signed-off-by: Simon Kuenzer <simon.kuenzer@xxxxxxxxx>
> ---
> include/uk/ctors.h | 60 ++++++++++++-------
> lib/ukboot/boot.c | 46 +++++++-------
> lib/uklibparam/include/uk/libparam.h | 2 +-
> .../include/uk/plat/common/common.lds.h | 2 +-
> 4 files changed, 64 insertions(+), 46 deletions(-)
>
> diff --git a/include/uk/ctors.h b/include/uk/ctors.h
> index e7d0dece..d8a1c838 100644
> --- a/include/uk/ctors.h
> +++ b/include/uk/ctors.h
> @@ -53,41 +53,55 @@ extern const uk_ctor_func_t __preinit_array_start[];
> extern const uk_ctor_func_t __preinit_array_end;
> extern const uk_ctor_func_t __init_array_start[];
> extern const uk_ctor_func_t __init_array_end;
> -extern const uk_ctor_func_t uk_ctortab[];
> +extern const uk_ctor_func_t uk_ctortab_start[];
> extern const uk_ctor_func_t uk_ctortab_end;
>
> /**
> * Register a Unikraft constructor function that is
> * called during bootstrap (uk_ctortab)
> *
> - * @param lvl
> - * Priority level (0 (higher) to 9 (least))
> - * Note: Any other value for level will be ignored
> - * @param ctorf
> + * @param fn
> * Constructor function to be called
> + * @param prio
> + * Priority level (0 (earliest) to 9 (latest))
> + * Note: Any other value for level will be ignored
> + */
> +#define __UK_CTORTAB(fn, libname, prio) \
> + static const uk_ctor_func_t \
> + __used __section(".uk_ctortab" #prio) \
> + __uk_ctortab ## lvl ## _ ## libname ## _ ## fn = (fn)
Since we have renamed lvl, here we should have prio not lvl.
If libname has special characters such as "-" the macro won't compile and
will result in an error such as: error: expected ‘=’, ‘,’, ‘;’, ‘asm’
or ‘__attribute__’ before ‘-’ token.
Maybe we should not concatenate the library's name.
> +
> +#define _UK_CTORTAB(fn, libname, prio) \
> + __UK_CTORTAB(fn, libname, prio)
> +
> +#define UK_CTOR_PRIO(fn, prio) \
> + _UK_CTORTAB(fn, __LIBNAME__, prio)
> +
> +/**
> + * Similar interface without priority.
> */
> -#define __UK_CTOR_FUNC(lvl, ctorf) \
> - static const uk_ctor_func_t \
> - __used __section(".uk_ctortab" #lvl) \
> - __uk_ctab ## lvl ## _ ## ctorf = (ctorf)
> -#define UK_CTOR_FUNC(lvl, ctorf) __UK_CTOR_FUNC(lvl, ctorf)
> +#define UK_CTOR(fn) UK_CTOR_PRIO(fn, 9)
> +
> +/* DELETEME: Compatibility wrapper for existing code, to be removed! */
> +#define UK_CTOR_FUNC(lvl, ctorf) \
> + _UK_CTORTAB(ctorf, __LIBNAME__, lvl)
There are two __ missing here, it should be __UK_CTOR_FUNC.
>
> /**
> - * Helper macro for iterating over constructor pointer arrays
> - * Please note that the array may contain NULL pointer entries
> + * Helper macro for iterating over constructor pointer tables
> + * Please note that the table may contain NULL pointer entries
> *
> - * @param arr_start
> - * Start address of pointer array (type: const uk_ctor_func_t const [])
> - * @param arr_end
> - * End address of pointer array
> - * @param i
> - * Iterator variable (integer) which should be used to access the
> - * individual fields
> + * @param itr
> + * Iterator variable (uk_ctor_func_t *) which points to the individual
> + * table entries during iteration
> + * @param ctortab_start
> + * Start address of table (type: const uk_ctor_func_t[])
> + * @param ctortab_end
> + * End address of table (type: const uk_ctor_func_t)
> */
> -#define uk_ctor_foreach(arr_start, arr_end, i) \
> - for ((i) = 0; \
> - &((arr_start)[i]) < &(arr_end); \
> - ++(i))
> +#define uk_ctortab_foreach(itr, ctortab_start, ctortab_end) \
> + for ((itr) = DECONST(uk_ctor_func_t*, ctortab_start); \
> + (itr) < &(ctortab_end); \
> + (itr)++)
>
> #ifdef __cplusplus
> }
> diff --git a/lib/ukboot/boot.c b/lib/ukboot/boot.c
> index 3f5046ca..aa6999cc 100644
> --- a/lib/ukboot/boot.c
> +++ b/lib/ukboot/boot.c
> @@ -77,6 +77,7 @@ static void main_thread_func(void *arg)
> int ret;
> struct thread_main_arg *tma = arg;
> uk_init_t *itr;
> + uk_ctor_func_t *ctorfn;
>
> /**
> * Run init table
> @@ -111,25 +112,24 @@ static void main_thread_func(void *arg)
> * from its OS being initialized.
> */
> uk_pr_info("Pre-init table at %p - %p\n",
> - __preinit_array_start, &__preinit_array_end);
> - uk_ctor_foreach(__preinit_array_start, __preinit_array_end, i) {
> - if (__preinit_array_start[i]) {
> - uk_pr_debug("Call pre-init constructor (entry %d (%p):
> %p())...\n",
> - i, &__preinit_array_start[i],
> - __preinit_array_start[i]);
> - __preinit_array_start[i]();
> - }
> + &__preinit_array_start[0], &__preinit_array_end);
> + uk_ctortab_foreach(ctorfn,
> + __preinit_array_start, __preinit_array_end) {
> + if (!*ctorfn)
> + continue;
Normally, ctorfn should never be NULL, by passing over NULLs we might hide
bugs that cause this section to be malformed.
> +
> + uk_pr_debug("Call pre-init constructor: %p()...\n", *ctorfn);
> + (*ctorfn)();
> }
>
> uk_pr_info("Constructor table at %p - %p\n",
> - __init_array_start, &__init_array_end);
> - uk_ctor_foreach(__init_array_start, __init_array_end, i) {
> - if (__init_array_start[i]) {
> - uk_pr_debug("Call constructor (entry %d (%p):
> %p())...\n",
> - i, &__init_array_start[i],
> - __init_array_start[i]);
> - __init_array_start[i]();
> - }
> + &__init_array_start[0], &__init_array_end);
> + uk_ctortab_foreach(ctorfn, __init_array_start, __init_array_end) {
> + if (!*ctorfn)
> + continue;
Same comment as above.
> +
> + uk_pr_debug("Call constructor: %p()...\n", *ctorfn);
> + (*ctorfn)();
> }
>
> uk_pr_info("Calling main(%d, [", tma->argc);
> @@ -170,7 +170,6 @@ void ukplat_entry_argp(char *arg0, char *argb, __sz
> argb_len)
> void ukplat_entry(int argc, char *argv[])
> {
> struct thread_main_arg tma;
> - int i;
> int kern_args = 0;
> int rc __maybe_unused = 0;
> #if CONFIG_LIBUKALLOC
> @@ -183,11 +182,16 @@ void ukplat_entry(int argc, char *argv[])
> struct uk_sched *s = NULL;
> struct uk_thread *main_thread = NULL;
> #endif
> + uk_ctor_func_t *ctorfn;
> +
> + uk_pr_info("Unikraft constructor table at %p - %p\n",
> + &uk_ctortab_start[0], &uk_ctortab_end);
> + uk_ctortab_foreach(ctorfn, uk_ctortab_start, uk_ctortab_end) {
> + if (!*ctorfn)
> + continue;
Same comment as above.
>
> - uk_pr_info("Unikraft constructors table at %p\n", uk_ctortab);
> - uk_ctor_foreach(uk_ctortab, uk_ctortab_end, i) {
> - uk_pr_debug("Call constructor %p\n", uk_ctortab[i]);
> - uk_ctortab[i]();
> + uk_pr_debug("Call constructor: %p())...\n", *ctorfn);
> + (*ctorfn)();
> }
>
> #ifdef CONFIG_LIBUKLIBPARAM
> diff --git a/lib/uklibparam/include/uk/libparam.h
> b/lib/uklibparam/include/uk/libparam.h
> index 2a271ed3..f8f8ba4b 100644
> --- a/lib/uklibparam/include/uk/libparam.h
> +++ b/lib/uklibparam/include/uk/libparam.h
> @@ -300,7 +300,7 @@ void _uk_libparam_lib_add(struct uk_lib_section *lib_sec);
> #define UK_LIB_CTOR_PRIO 1
>
> #define UK_LIB_CONSTRUCTOR_SETUP(prio, name)
> \
> - __UK_CTOR_FUNC(prio, name)
> + UK_CTOR_PRIO(name, prio)
>
> /**
> * Create a constructor to initialize the parameters in the library.
> diff --git a/plat/common/include/uk/plat/common/common.lds.h
> b/plat/common/include/uk/plat/common/common.lds.h
> index 593e6699..d31aa1dd 100644
> --- a/plat/common/include/uk/plat/common/common.lds.h
> +++ b/plat/common/include/uk/plat/common/common.lds.h
> @@ -87,7 +87,7 @@
>
> #define CTORTAB_SECTION
> \
> . = ALIGN(__PAGE_SIZE); \
> - uk_ctortab = .; \
> + uk_ctortab_start = .; \
> .uk_ctortab : \
> { \
> KEEP(*(SORT_BY_NAME(.uk_ctortab[0-9]))) \
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |