Changeset ed7e057 in mainline

Timestamp:

2024-01-16T15:46:47Z (12 months ago)

Author:

Jiří Zárevúcky <zarevucky.jiri@…>

Branches:

master

Children:

29029ac0, a5b5f17

Parents:

4ed7870

git-author:

Jiří Zárevúcky <zarevucky.jiri@…> (2024-01-15 20:09:30)

git-committer:

Jiří Zárevúcky <zarevucky.jiri@…> (2024-01-16 15:46:47)

Message:

Add functions context_create(), context_replace() and context_swap()

and use them where appropriate, removing context_save() in the process.
Much like in userspace, context_swap() maintains natural control flow
as opposed to context_save()'s return-twice mechanic.

Beyond that, in the future, context_replace() and context_swap()
can be implemented more efficiently than the context_save()/
context_restore() pair. As of now, the original implementation is
retained.

Location:

kernel/generic

Files:

• include/context.h (modified) (3 diffs)
• src/main/main.c (modified) (3 diffs)
• src/proc/scheduler.c (modified) (3 diffs)
• src/proc/thread.c (modified) (2 diffs)

kernel/generic/include/context.h

@@ -36 +36 @@
 #define KERN_CONTEXT_H_
 
+#include <panic.h>
 #include <trace.h>
 #include <arch/context.h>
+#include <arch/faddr.h>
 
 #define context_set_generic(ctx, _pc, stack, size) \
@@ -47 +49 @@
 extern int context_save_arch(context_t *ctx) __attribute__((returns_twice));
 extern void context_restore_arch(context_t *ctx) __attribute__((noreturn));
-
-/** Save register context.
- *
- * Save the current register context (including stack pointer) to a context
- * structure. A subsequent call to context_restore() will return to the same
- * address as the corresponding call to context_save().
- *
- * Note that context_save_arch() must reuse the stack frame of the function
- * which called context_save(). We guarantee this by:
- *
- *   a) implementing context_save_arch() in assembly so that it does not create
- *      its own stack frame, and by
- *   b) defining context_save() as a macro because the inline keyword is just a
- *      hint for the compiler, not a real constraint; the application of a macro
- *      will definitely not create a stack frame either.
- *
- * To imagine what could happen if there were some extra stack frames created
- * either by context_save() or context_save_arch(), we need to realize that the
- * sp saved in the contex_t structure points to the current stack frame as it
- * existed when context_save_arch() was executing. After the return from
- * context_save_arch() and context_save(), any extra stack frames created by
- * these functions will be destroyed and their contents sooner or later
- * overwritten by functions called next. Any attempt to restore to a context
- * saved like that would therefore lead to a disaster.
- *
- * @param ctx Context structure.
- *
- * @return context_save() returns 1, context_restore() returns 0.
- *
- */
-#define context_save(ctx)  context_save_arch(ctx)
 
 /** Restore register context.
@@ -91 +62 @@
  *
  */
-_NO_TRACE static inline void context_restore(context_t *ctx)
+_NO_TRACE __attribute__((noreturn))
+    static inline void context_restore(context_t *ctx)
 {
         context_restore_arch(ctx);
+}
+
+/**
+ * Saves current context to the variable pointed to by `self`,
+ * and restores the context denoted by `other`.
+ *
+ * When the `self` context is later restored by another call to
+ * `context_swap()`, the control flow behaves as if the earlier call to
+ * `context_swap()` just returned.
+ */
+_NO_TRACE static inline void context_swap(context_t *self, context_t *other)
+{
+        if (context_save_arch(self))
+                context_restore_arch(other);
+}
+
+_NO_TRACE static inline void context_create(context_t *context,
+    void (*fn)(void), void *stack_base, size_t stack_size)
+{
+        *context = (context_t) { 0 };
+        context_set(context, FADDR(fn), stack_base, stack_size);
+}
+
+__attribute__((noreturn)) static inline void context_replace(void (*fn)(void),
+    void *stack_base, size_t stack_size)
+{
+        context_t ctx;
+        context_create(&ctx, fn, stack_base, stack_size);
+        context_restore(&ctx);
 }
 

kernel/generic/src/main/main.c

@@ -80 +80 @@
 #include <arch/arch.h>
 #include <arch.h>
-#include <arch/faddr.h>
 #include <ipc/ipc.h>
 #include <macros.h>
@@ -174 +173 @@
             ALIGN_UP((uintptr_t) kdata_end - config.base, PAGE_SIZE);
 
-        context_save(&ctx);
-        context_set(&ctx, FADDR(main_bsp_separated_stack),
+        context_create(&ctx, main_bsp_separated_stack,
             bootstrap_stack, bootstrap_stack_size);
         context_restore(&ctx);
@@ -336 +334 @@
          * switch to this cpu's private stack prior to waking kmp up.
          */
-        context_t ctx;
-        context_save(&ctx);
-        context_set(&ctx, FADDR(main_ap_separated_stack),
-            (uintptr_t) CPU_LOCAL->stack, STACK_SIZE);
-        context_restore(&ctx);
+        context_replace(main_ap_separated_stack, CPU_LOCAL->stack, STACK_SIZE);
         /* not reached */
 }

kernel/generic/src/proc/scheduler.c

@@ -309 +309 @@
 
         current_copy(CURRENT, (current_t *) CPU_LOCAL->stack);
-
-        context_t ctx;
-        context_save(&ctx);
-        context_set(&ctx, FADDR(scheduler_separated_stack),
-            (uintptr_t) CPU_LOCAL->stack, STACK_SIZE);
-        context_restore(&ctx);
-
+        context_replace(scheduler_separated_stack, CPU_LOCAL->stack, STACK_SIZE);
         unreachable();
 }
@@ -452 +446 @@
                 /* Prefer the thread after it's woken up. */
                 THREAD->priority = -1;
-        }
-
-        if (!context_save(&THREAD->saved_context)) {
-                /*
-                 * This is the place where threads leave scheduler();
-                 */
-
-                irq_spinlock_unlock(&THREAD->lock, false);
-                interrupts_restore(ipl);
-                return;
         }
 
@@ -486 +470 @@
          */
         context_t ctx;
-        context_save(&ctx);
-        context_set(&ctx, FADDR(scheduler_separated_stack),
-            (uintptr_t) CPU_LOCAL->stack, STACK_SIZE);
-        context_restore(&ctx);
-
-        /* Not reached */
+        context_create(&ctx, scheduler_separated_stack,
+            CPU_LOCAL->stack, STACK_SIZE);
+
+        /* Switch to scheduler context and store current thread's context. */
+        context_swap(&THREAD->saved_context, &ctx);
+
+        /* Returned from scheduler. */
+
+        irq_spinlock_unlock(&THREAD->lock, false);
+        interrupts_restore(ipl);
 }
 

kernel/generic/src/proc/thread.c

@@ -60 +60 @@
 #include <arch/interrupt.h>
 #include <smp/ipi.h>
-#include <arch/faddr.h>
 #include <atomic.h>
 #include <memw.h>
@@ -310 +309 @@
         irq_spinlock_unlock(&tidlock, true);
 
-        memset(&thread->saved_context, 0, sizeof(thread->saved_context));
-        context_set(&thread->saved_context, FADDR(cushion),
-            (uintptr_t) thread->kstack, STACK_SIZE);
+        context_create(&thread->saved_context, cushion, thread->kstack, STACK_SIZE);
 
         current_initialize((current_t *) thread->kstack);