[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Minios-devel] [UNIKRAFT PATCH 3/9] include: Clean-ups to <uk/init.h>





On 30.01.20 17:59, Vlad-Andrei BĂDOIU (78692) wrote:
Hey Simon,

Please see my comments inline.

Thank you,

Vlad

On 29.01.2020 23:48, Simon Kuenzer wrote:
This commit brings the Unikraft init table definitions to the same
style as the updated one of the Unikraft Constructor table.

The new macro uk_initcall_class_prio() enables specifying the
target initialization class with its corresponding number 1 to 6.

The changes to this header should not break existing code.

Signed-off-by: Simon Kuenzer <simon.kuenzer@xxxxxxxxx>
---
   include/uk/init.h                             | 71 ++++++++++++++-----
   lib/ukboot/boot.c                             | 16 +++--
   .../include/uk/plat/common/common.lds.h       |  2 +-
   3 files changed, 63 insertions(+), 26 deletions(-)

diff --git a/include/uk/init.h b/include/uk/init.h
index 8b7a8aa2..149b4926 100644
--- a/include/uk/init.h
+++ b/include/uk/init.h
@@ -41,46 +41,65 @@
   extern "C" {
   #endif
-typedef int (*uk_init_t)(void);
+typedef int (*uk_init_func_t)(void);
-#define INITTAB_STR_VAR(libname, fn, base, prio) libname ## fn ## base ## prio
-#define INITTAB_SECTION(base, prio)  .uk_inittab_ ## base ## prio
-#define INITTAB_SECTION_NAME(name) STRINGIFY(name)
+/**
+ * Register a Unikraft init function that is
+ * called during bootstrap (uk_inittab)
+ *
+ * @param fn
+ *   Initialization function to be called
+ * @param class
+ *   Initialization class (1 (earliest) to 6 (latest))
+ * @param prio
+ *   Priority level (0 (earliest) to 9 (latest)), must be a constant.
+ *   Note: Any other value for level will be ignored
+ */
+#define __UK_INITTAB(libname, fn, base, prio)                          \
+       static const uk_init_func_t                                     \
+       __used __section(".uk_inittab" #base #prio)                   \
+       __uk_inittab ## base ## prio ## _ ## libname ## _ ## fn = (fn)

As pointed out in the previous patch, concatenating the libname seems

to cause problems. Until this patch there was a bug hid this problem,

we were using LIBNAME in place of __LIBNAME__. Please see my error

when compiling pthreads-embedded below:

error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘-’ token

You are right. I am going to remove it. See also my comment in patch 2/9.

+
+#define _UK_INITTAB(libname, fn, base, prio)                           \
+       __UK_INITTAB(libname, fn, base, prio)
-#define __inittab(libname, fn, base, prio) \
-       static  const __used __section(INITTAB_SECTION_NAME(            \
-                                       INITTAB_SECTION(base, prio))    \
-                                     )                                 \
-               uk_init_t INITTAB_STR_VAR(libname, fn, base, prio) = fn
+#define uk_initcall_class_prio(fn, class, prio)                                
\
+       _UK_INITTAB(__LIBNAME__, fn, class, prio)
/**
    * Define a library initialization. At this point in time some platform
    * component may not be initialized, so it wise to initializes those 
component
    * to initialized.
    */
-#define uk_early_initcall_prio(fn, prio)  __inittab(LIBNAME, fn, 1, prio)
+#define uk_early_initcall_prio(fn, prio) \
+       uk_initcall_class_prio(fn, 1, prio)
   /**
    * Define a stage for platform initialization. Platform at this point read
    * all the device and device are initialized.
    */
-#define uk_plat_initcall_prio(fn, prio)  __inittab(LIBNAME, fn, 2, prio)
+#define uk_plat_initcall_prio(fn, prio) \
+       uk_initcall_class_prio(fn, 2, prio)
   /**
    * Define a stage for performing library initialization. This library
    * initialization is performed after the platform is completely initialized.
    */
-#define uk_lib_initcall_prio(fn, prio) __inittab(LIBNAME, fn, 3, prio)
+#define uk_lib_initcall_prio(fn, prio) \
+       uk_initcall_class_prio(fn, 3, prio)
   /**
    * Define a stage for filesystem initialization.
    */
-#define uk_rootfs_initcall_prio(fn, prio) __inittab(LIBNAME, fn, 4, prio)
+#define uk_rootfs_initcall_prio(fn, prio) \
+       uk_initcall_class_prio(fn, 4, prio)
   /**
    * Define a stage for device initialization
    */
-#define uk_sys_initcall_prio(fn, prio) __inittab(LIBNAME, fn, 5, prio)
+#define uk_sys_initcall_prio(fn, prio) \
+       uk_initcall_class_prio(fn, 5, prio)
   /**
    * Define a stage for application pre-initialization
    */
-#define uk_late_initcall_prio(fn, prio)  __inittab(LIBNAME, fn, 6, prio)
+#define uk_late_initcall_prio(fn, prio) \
+       uk_initcall_class_prio(fn, 6, prio)
/**
    * Similar interface without priority.
@@ -92,11 +111,25 @@ typedef int (*uk_init_t)(void);
   #define uk_sys_initcall(fn)       uk_sys_initcall_prio(fn, 9)
   #define uk_late_initcall(fn)      uk_late_initcall_prio(fn, 9)
-extern const uk_init_t uk_inittab_start[];
-extern const uk_init_t uk_inittab_end;
+extern const uk_init_func_t uk_inittab_start[];
+extern const uk_init_func_t uk_inittab_end;
-#define uk_inittab_foreach(init_start, init_end, itr) \
-       for (itr = DECONST(uk_init_t*, init_start); itr < &init_end; itr++)
+/**
+ * Helper macro for iterating over init pointer tables
+ * Please note that the table may contain NULL pointer entries
+ *
+ * @param itr
+ *   Iterator variable (uk_init_func_t *) which points to the individual
+ *   table entries during iteration
+ * @param inittab_start
+ *   Start address of table (type: const uk_init_func_t[])
+ * @param inittab_end
+ *   End address of table (type: const uk_init_func_t)
+ */
+#define uk_inittab_foreach(itr, inittab_start, inittab_end)    \
+       for ((itr) = DECONST(uk_init_func_t*, inittab_start);   \
+            (itr) < &(inittab_end);                             \
+            (itr)++)
#ifdef __cplusplus
   }
diff --git a/lib/ukboot/boot.c b/lib/ukboot/boot.c
index aa6999cc..c6dac1a8 100644
--- a/lib/ukboot/boot.c
+++ b/lib/ukboot/boot.c
@@ -76,19 +76,23 @@ static void main_thread_func(void *arg)
        int i;
        int ret;
        struct thread_main_arg *tma = arg;
-       uk_init_t *itr;
        uk_ctor_func_t *ctorfn;
+       uk_init_func_t *initfn;
/**
         * Run init table
         */
-       uk_pr_info("Init Table @ %p - %p\n", &uk_inittab_start[0],
-                  &uk_inittab_end);
-       uk_inittab_foreach(uk_inittab_start, uk_inittab_end, itr) {
-               ret = (*itr)();
+       uk_pr_info("Init Table @ %p - %p\n",
+                  &uk_inittab_start[0], &uk_inittab_end);
+       uk_inittab_foreach(initfn, uk_inittab_start, uk_inittab_end) {
+               if (!*initfn)
+                       continue;
This might hide possible bugs.

I will replace it with an assertion.

+
+               uk_pr_debug("Call init function: %p()...\n", *initfn);
+               ret = (*initfn)();
                if (ret < 0) {
                        uk_pr_err("Init function at %p returned error %d\n",
-                                 itr, ret);
+                                 *initfn, ret);
                        ret = UKPLAT_CRASH;
                        goto exit;
                }
diff --git a/plat/common/include/uk/plat/common/common.lds.h 
b/plat/common/include/uk/plat/common/common.lds.h
index d31aa1dd..676c20b3 100644
--- a/plat/common/include/uk/plat/common/common.lds.h
+++ b/plat/common/include/uk/plat/common/common.lds.h
@@ -98,7 +98,7 @@
        uk_inittab_start = .;                                           \
        .uk_inittab :                                                   \
        {                                                               \
-               KEEP(*(SORT_BY_NAME(.uk_inittab_[1-6][0-9])))           \
+               KEEP(*(SORT_BY_NAME(.uk_inittab[1-6][0-9])))            \
        }                                                               \
        uk_inittab_end = .;

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.