Replacing sysinit

[Christopher Collins <cc...@apache.org>]

Hello all,

Sysinit is the process Mynewt uses to initialize all the packages in a
project.  Typically, sysinit gets invoked at the very start of main().

There are a few aspects of sysinit that I really dislike, and I think we
can replace it with something better.  In this email, I will (1) summarize the
mechanism as it exists today, (2) describe the problems with the current
mechanism, and (3) propose a new solution.

(1) CURRENT SYSINIT PROCESS

    * A package's pkg.yml file optionally specifies "init_function" and
      "init_stage"
    * At build time, the newt tool generates sysinit C files which
      define a single function.  This function calls the packages' init
      functions in the correct order.
        * Stage 0 runs first, stage 1 next, etc.
        * Within a stage, initializations are ordered alphabetically by
          package name.
    * For split images, two sysinit files get generated: one for the
      loader and one for the app.  The sysinit() macro (defined in
      sys/sysinit/include/sysinit.h) ensures the correct sysinit
      function gets called, depending on whether the loader or app is
      running.

The generated sysinit files are created with the following paths:
    * bin/targets/<target>/generated/src/<target>-sysinit-loader.c
    * bin/targets/<target>/generated/src/<target>-sysinit-app.c

For non-split-images, only the "app" file is created.

Here is an example of a generated sysinit function:

    void
    sysinit_app(void)
    {
        os_init();

        /*** Stage 0 */
        /* 0.0: kernel/os */
        os_pkg_init();
        /* 0.2: sys/flash_map */
        flash_map_init();

        /*** Stage 1 */
        /* 1.0: net/nimble/transport/ram */
        ble_hci_ram_pkg_init();
        /* 1.1: sys/log */
        log_init();

        /* [...] */
    }

(2) ISSUES WITH CURRENT IMPLEMENTATION

So that is how sysinit works today.  There are two aspects of the
current mechanism that I strongly dislike:
    (1) Generated code.
    (2) C identifiers specified in YAML files.

I think everyone hates generated code, so I probably don't have to say
much about (1).  As a general principle, I think it is good to minimize
the amount of magic newt performs during builds.

The reason I dislike (2) is that it creates an opportunity for really
confusing build errors.  Newt doesn't ensure each init_function setting
specifies a real function with the correct type.  If a setting is wrong,
the user gets a linker error which points at a generated sysinit file.

(3) PROPOSED SOLUTION

    * In C code, a package optionally defines an "init entry" struct
      containing:
        o Pointer to its init function.
        o Stage number.
    * Init entry structs are placed in a special section via
      __attribute__((section(...))).
    * At startup, the sysinit() function walks the list of entries in
      the special section and calls their corresponding init functions
      in the correct order.

This solution addresses both the issues I mentioned above:
    * No more init_function or init_stage settings in pkg.yml files.
    * No generated sysinit code.  The sysinit() function is a generic
      function that exists in apache-mynewt-core.

I should mention that this solution introduces a new issue:
indeterminate initialization order within each stage.  In the current
mechanism, newt enforces a consistent ordering of packages with the same
stage (alphabetical).  In the proposed solution, packages within a stage
do not have a predictable ordering.  Presumably, the entries are
arranged in the order the linker encounters them.  I don't think this is
a huge problem, but it does mean package authors would need to be
careful when assigning stage numbers.

Here is the API that I came up with:

    /* Package initialization function. */
    typedef void sysinit_init_fn(struct sysinit_init_ctxt *ctxt);

    struct sysinit_entry {
        /* Initializes a package. */
        sysinit_init_fn *init_fn;

        /* Specifies when the init function gets called.  0=first, * 1=next, etc. */
        uint8_t stage;
    };

    struct sysinit_init_ctxt {
        /* Corresponds to the init function currently executing. */
        const struct sysinit_entry *entry;

        /* The stage that sysinit is currently processing. */
        uint8_t cur_stage;
    };

    #define SYSINIT_REGISTER_INIT(init_cb, init_stage) /* ... */

Here is a usage example.  This is the existing mgmt/newtmgr init
function modified to use the proposed mechanism.

    void
    nmgr_pkg_init(struct sysinit_init_ctxt *ctxt)
    {
        int rc;

        /* Ensure this function only gets called by sysinit. */
        SYSINIT_ASSERT_ACTIVE();

        rc = nmgr_task_init();
        SYSINIT_PANIC_ASSERT(rc == 0);
    }

    SYSINIT_REGISTER_INIT(nmgr_pkg_init, 5);

I anticipate most initialization functions would not need the ctxt
parameter.  Some functions may find it useful, though.  For example, a
package might register a function several times, specifying a different
stage with each registration.  Such a function might need to know the
current stage to know which initialization phase to execute.

Anyway, thanks for reading.  All feedback is welcom.

Chris