Re: [Minios-devel] [UNIKRAFT PATCH v2 4/4] lib/ukboot: Iterate through the inittab

[Simon Kuenzer <simon.kuenzer@xxxxxxxxx>]

Hey Sharan,

I will remove the comment then. Thanks!

Reviewed-by: Simon Kuenzer <simon.kuenzer@xxxxxxxxx>

On 24.09.19 10:40, Sharan Santhanam wrote:
> On 9/24/19 10:00 AM, Simon Kuenzer wrote:
> > On 23.09.19 14:09, Sharan Santhanam wrote:
> > > Add support to iterate through the inittab and call the init functions.
> > >
> > > Signed-off-by: Sharan Santhanam <sharan.santhanam@xxxxxxxxx>
> > > ---
> > >   include/uk/init.h |  6 ++++++
> > >   lib/ukboot/boot.c | 23 +++++++++++++++++++++++
> > >   2 files changed, 29 insertions(+)
> > >
> > > diff --git a/include/uk/init.h b/include/uk/init.h
> > > index bea4127..8b7a8aa 100644
> > > --- a/include/uk/init.h
> > > +++ b/include/uk/init.h
> > > @@ -92,6 +92,12 @@ 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;
> > > +
> > > +#define uk_inittab_foreach(init_start, init_end, itr)        \
> > > +    for (itr = DECONST(uk_init_t*, init_start); itr < &init_end; itr++)
> > > +
> > >   #ifdef __cplusplus
> > >   }
> > >   #endif /* __cplusplus */
> > > diff --git a/lib/ukboot/boot.c b/lib/ukboot/boot.c
> > > index 7fb0908..f6156ee 100644
> > > --- a/lib/ukboot/boot.c
> > > +++ b/lib/ukboot/boot.c
> > > @@ -56,6 +56,7 @@
> > >   #include <uk/essentials.h>
> > >   #include <uk/print.h>
> > >   #include <uk/ctors.h>
> > > +#include <uk/init.h>
> > >   #include <uk/argparse.h>
> > >   #if CONFIG_LIBUKBUS
> > >   #include <uk/bus.h>
> > > @@ -81,6 +82,22 @@ static void main_thread_func(void *arg)
> > >       int i;
> > >       int ret;
> > >       struct thread_main_arg *tma = arg;
> > > +    uk_init_t *itr;
> > > +
> > > +    /**
> > > +     * 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)();
> > > +        if (ret < 0) {
> > > +            uk_pr_err("Init function at %p returned error %d\n",
> > > +                  itr, ret);
> > > +            ret = UKPLAT_CRASH;
> > > +            goto exit;
> > > +        }
> > > +    }
> > >     #ifdef CONFIG_LIBUKBUS
> > >       uk_pr_info("Initialize bus handlers...\n");
> > > @@ -149,6 +166,8 @@ static void main_thread_func(void *arg)
> > >       ret = main(tma->argc, tma->argv);
> > >       uk_pr_info("main returned %d, halting system\n", ret);
> > >       ret = (ret != 0) ? UKPLAT_CRASH : UKPLAT_HALT;
> > > +
> > > +exit:
> > >       ukplat_terminate(ret); /* does not return */
> > >   }
> > >   @@ -239,6 +258,10 @@ void ukplat_entry(int argc, char *argv[])
> > >       }
> > >   #endif
> > >   +/**
> > > + * Adding the init table.
> > > + */
> > > +
> > I am not getting this comment... I am thinking that you mean to move 
> > the following code (irq init, platform time, scheduler) to init table 
> > entries - so it is a TODO, right?
> > I think, since because those items are really tricky, I tend to leave 
> > the comment for now. Alternatively, I propose that I change the 
> > comment to the following while upstreaming:
> No we can remove it. We can add it when the rest of the code is in place
> > /*
> >  * TODO: Move the following initialization calls to init table
> >  */
> >
> > Let me know what you think.
> >
> > >   #if CONFIG_LIBUKALLOC
> > >       uk_pr_info("Initialize IRQ subsystem...\n");
> > >       rc = ukplat_irq_init(a);
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel