elfloader/risc-v: add helper function boot_hart()

Rework boot code flow to simplify it and avoid redundancy.

Signed-off-by: Axel Heider <axelheider@gmx.de>

elfloader-tool/src/arch-riscv/boot.c

@@ -211,28 +211,14 @@ static int is_core_ready(int core_id)
     return (0 != __atomic_load_n(&core_ready[core_id], __ATOMIC_RELAXED));
 }
 
-static void set_and_wait_for_ready(word_t hart_id, word_t core_id)
-{
-    /* Acquire lock to update core ready array */
-    acquire_multicore_lock();
-    printf("Hart ID %"PRIu_word" core ID %"PRIu_word"\n", hart_id, core_id);
-    mark_core_ready(core_id);
-    release_multicore_lock();
-
-    /* Wait until all cores are go */
-    for (int i = 0; i < CONFIG_MAX_NUM_NODES; i++) {
-        while (!is_core_ready(i)) {
-            /* busy waiting loop */
-        }
-    }
-}
-
 static void start_secondary_harts(void)
 {
+    /* Start all secondary cores. Since we take the multicore lock before that,
+     * the boot process on the primary core continues without concurrency issues
+     * until things can really run in parallel. Seems the main use case for this
+     * is printing nicely serialized boot messages,
+     */
     acquire_multicore_lock();
-    printf("Main entry hart_id:%"PRIu_word"\n", hart_id);
-    release_multicore_lock();
-    /* set global flag that secondary cores can run and bring them up. */
     set_secondary_cores_go();
     word_t i = 0;
     while (i < CONFIG_MAX_NUM_NODES && hsm_exists) {
@@ -241,12 +227,11 @@ static void start_secondary_harts(void)
             sbi_hart_start(i, secondary_harts, i);
         }
     }
-    set_and_wait_for_ready(hart_id, 0);
 }
 
 #endif /* CONFIG_MAX_NUM_NODES > 1 */
 
-static int run_elfloader(UNUSED word_t hart_id, void *bootloader_dtb)
+static int run_elfloader(void *bootloader_dtb)
 {
     int ret;
 
@@ -265,62 +250,86 @@ static int run_elfloader(UNUSED word_t hart_id, void *bootloader_dtb)
         return -1;
     }
 
+    /* Setup MMU tables. */
     ret = map_kernel_window(&kernel_info);
     if (0 != ret) {
         printf("ERROR: could not map kernel window, code %d\n", ret);
         return -1;
     }
 
+    return 0;
+}
+
+static NORETURN void boot_hart(word_t hart_id, word_t core_id)
+{
+    /* Caller must hold the multicore lock here. */
+
+    if (0 == core_id) {
+        printf("Enabling MMU and paging\n");
+    }
+    enable_virtual_memory();
+
 #if CONFIG_MAX_NUM_NODES > 1
-    start_secondary_harts();
+    /* We are ready to hand over control to the kernel on this hart. Sync with
+     * all other harts before doing this.
+     */
+    mark_core_ready(core_id);
+    release_multicore_lock();
+    for (int i = 0; i < CONFIG_MAX_NUM_NODES; i++) {
+        while (!is_core_ready(i)) {
+            /* busy waiting loop */
+        }
+    }
 #endif /* CONFIG_MAX_NUM_NODES > 1 */
 
-    printf("Enabling MMU and paging\n");
-    enable_virtual_memory();
+    if (0 == core_id) {
+        printf("Jumping to kernel-image entry point...\n\n");
+    }
 
-    printf("Jumping to kernel-image entry point...\n\n");
-    ((init_riscv_kernel_t)kernel_info.virt_entry)(user_info.phys_region_start,
-                                                  user_info.phys_region_end,
-                                                  user_info.phys_virt_offset,
-                                                  user_info.virt_entry,
-                                                  (word_t)dtb,
-                                                  dtb_size
+    /* The hand over interface is the same on all cores. We avoid making
+     * assumption how the parameters are used. The current seL4 kernel
+     * implementation only cares about the DTB on the primary core.
+     */
+    ((init_riscv_kernel_t)kernel_info.virt_entry)(
+        user_info.phys_region_start,
+        user_info.phys_region_end,
+        user_info.phys_virt_offset,
+        user_info.virt_entry,
+        (word_t)dtb,
+        dtb_size
 #if CONFIG_MAX_NUM_NODES > 1
-                                                  ,
-                                                  hart_id,
-                                                  0
+        ,
+        hart_id,
+        core_id
 #endif /* CONFIG_MAX_NUM_NODES > 1 */
-                                                 );
+    );
 
     /* We should never get here. */
-    printf("ERROR: Kernel returned back to the ELF Loader\n");
-    return -1;
+    printf("ERROR: back in ELF-loader hart %"PRIu_word" (core ID %"PRIu_word")\n",
+           hart_id, core_id);
+    abort();
+    UNREACHABLE();
 }
 
 #if CONFIG_MAX_NUM_NODES > 1
-
-void secondary_entry(word_t hart_id, word_t core_id)
+NORETURN void secondary_hart_main(word_t hart_id, word_t core_id)
 {
     block_until_secondary_cores_go();
     acquire_multicore_lock();
-    printf("Secondary entry hart_id:%"PRIu_word" core_id:%"PRIu_word"\n",
+    printf("started hart %"PRIu_word" (core id %"PRIu_word")\n",
            hart_id, core_id);
-    release_multicore_lock();
-    set_and_wait_for_ready(hart_id, core_id);
-    enable_virtual_memory();
-    /* If adding or modifying these parameters you will need to update
-        the registers in head.S */
-    ((init_riscv_kernel_t)kernel_info.virt_entry)(user_info.phys_region_start,
-                                                  user_info.phys_region_end,
-                                                  user_info.phys_virt_offset,
-                                                  user_info.virt_entry,
-                                                  (word_t)dtb,
-                                                  dtb_size,
-                                                  hart_id,
-                                                  core_id
-                                                 );
-}
 
+    if (core_id >= CONFIG_MAX_NUM_NODES) {
+        printf("ERROR: max number of harts exceeded (%d)\n",
+               (int)CONFIG_MAX_NUM_NODES);
+        abort();
+        UNREACHABLE();
+    }
+
+    boot_hart(hart_id, core_id);
+    UNREACHABLE();
+
+}
 #endif /* CONFIG_MAX_NUM_NODES > 1 */
 
 void main(word_t hart_id, void *bootloader_dtb)
@@ -331,19 +340,18 @@ void main(word_t hart_id, void *bootloader_dtb)
 
     printf("  paddr=[%p..%p]\n", _text, _end - 1);
 
-    /* Run the actual ELF loader, this is not expected to return unless there
-     * was an error.
-     */
-    int ret = run_elfloader(hart_id, bootloader_dtb);
+    int ret = run_elfloader(bootloader_dtb);
     if (0 != ret) {
         printf("ERROR: ELF-loader failed, code %d\n", ret);
         /* There is nothing we can do to recover. */
         abort();
         UNREACHABLE();
     }
 
-    /* We should never get here. */
-    printf("ERROR: ELF-loader didn't hand over control\n");
-    abort();
+#if CONFIG_MAX_NUM_NODES > 1
+    start_secondary_harts();
+#endif /* CONFIG_MAX_NUM_NODES > 1 */
+
+    boot_hart(hart_id, 0);
     UNREACHABLE();
 }

elfloader-tool/src/arch-riscv/crt0.S

@@ -12,6 +12,7 @@
 .extern elfloader_stack
 .extern hsm_exists
 #if CONFIG_MAX_NUM_NODES > 1
+.extern secondary_hart_main
 .extern next_logical_core_id
 #endif
 
@@ -152,7 +153,7 @@ secondary_harts:
   addi t0, a1, 1 /* increment by one because we need to set sp to the end */
   slli t0, t0, CONFIG_KERNEL_STACK_BITS /* t0 = t0 * BIT(CONFIG_KERNEL_STACK_BITS) */
   add sp, sp, t0
-  la s0, secondary_entry
+  la s0, secondary_hart_main
   jr s0
 #endif
 /* If we get here then the HSM extension exists and the current