Merge pull request #53 from HeatCrab/u-mode/basic-support

Add basic U-mode support with ecall-based syscalls

Author: jserv

.ci/run-app-tests.sh

@@ -47,7 +47,7 @@ else
 fi
 
 # Filter excluded apps
-EXCLUDED_APPS=""
+EXCLUDED_APPS="umode"
 if [ -n "$EXCLUDED_APPS" ]; then
     FILTERED_APPS=""
     for app in $APPS; do

.ci/run-functional-tests.sh

@@ -11,6 +11,7 @@ TOOLCHAIN_TYPE=${TOOLCHAIN_TYPE:-gnu}
 declare -A FUNCTIONAL_TESTS
 FUNCTIONAL_TESTS["mutex"]="Fairness: PASS,Mutual Exclusion: PASS,Data Consistency: PASS,Overall: PASS"
 FUNCTIONAL_TESTS["semaphore"]="Overall: PASS"
+FUNCTIONAL_TESTS["umode"]="PASS: sys_tid() returned,PASS: sys_uptime() returned,[EXCEPTION] Illegal instruction"
 #FUNCTIONAL_TESTS["test64"]="Unsigned Multiply: PASS,Unsigned Divide: PASS,Signed Multiply: PASS,Signed Divide: PASS,Left Shifts: PASS,Logical Right Shifts: PASS,Arithmetic Right Shifts: PASS,Overall: PASS"
 #FUNCTIONAL_TESTS["suspend"]="Suspend: PASS,Resume: PASS,Self-Suspend: PASS,Overall: PASS"
 
@@ -75,7 +76,8 @@ test_functional_app() {
     IFS=',' read -ra PASS_CRITERIA <<< "$expected_passes"
 
     # Check for crashes first
-    if echo "$output" | grep -qiE "(trap|exception|fault|panic|illegal|segfault)"; then
+    # Special case: umode test expects an illegal instruction exception
+    if [ "$test" != "umode" ] && echo "$output" | grep -qiE "(trap|exception|fault|panic|illegal|segfault)"; then
         echo "[!] Crash detected"
 
         # Mark all criteria as crashed

Documentation/hal-calling-convention.md

@@ -109,13 +109,14 @@ void hal_context_restore(jmp_buf env, int32_t val); /* Restore context + process
 The ISR in `boot.c` performs a complete context save of all registers:
 
 ```
-Stack Frame Layout (128 bytes, offsets from sp):
+Stack Frame Layout (144 bytes, 33 words × 4 bytes, offsets from sp):
   0: ra,   4: gp,   8: tp,  12: t0,  16: t1,  20: t2
- 24: s0,  28: s1,  32: a0,  36: a1,  40: a2,  44: a3  
+ 24: s0,  28: s1,  32: a0,  36: a1,  40: a2,  44: a3
  48: a4,  52: a5,  56: a6,  60: a7,  64: s2,  68: s3
  72: s4,  76: s5,  80: s6,  84: s7,  88: s8,  92: s9
  96: s10, 100:s11, 104:t3, 108: t4, 112: t5, 116: t6
-120: mcause, 124: mepc
+120: mcause, 124: mepc, 128: mstatus
+132-143: padding (12 bytes for 16-byte alignment)
 ```
 
 Why full context save in ISR?
@@ -128,7 +129,7 @@ Why full context save in ISR?
 
 Each task stack must reserve space for the ISR frame:
 ```c
-#define ISR_STACK_FRAME_SIZE 128  /* 32 registers × 4 bytes */
+#define ISR_STACK_FRAME_SIZE 144  /* 33 words × 4 bytes, 16-byte aligned */
 ```
 
 This "red zone" is reserved at the top of every task stack to guarantee ISR safety.
@@ -147,10 +148,20 @@ int32_t result = mo_task_spawn(task_function, 2048);
 
 ### System Call Interface
 
-Linmo uses standard function calls (not trap instructions) for system services:
-- Arguments passed in `a0-a7` registers
-- Return values in `a0`
-- No special calling convention required
+Linmo provides system calls through the RISC-V trap mechanism for privilege
+boundary crossing. User mode tasks invoke system calls using the environment
+call instruction, which triggers a synchronous exception handled by the kernel.
+
+System call convention:
+- Arguments passed in `a0-a7` registers before trap
+- System call number in `a7` register
+- Trap handler preserves all registers except return value
+- Return value delivered in `a0` register after trap return
+- Standard RISC-V calling convention maintained across privilege boundary
+
+The trap-based interface allows user mode tasks to safely access kernel
+services without requiring privileged instruction execution. The kernel
+validates all parameters and mediates access to protected resources.
 
 ### Task Entry Points
 
@@ -174,9 +185,9 @@ Each task has its own stack with this layout:
 
 ```
 High Address
-+------------------+ <- stack_base + stack_size  
-| ISR Red Zone     | <- 128 bytes reserved for ISR
-| (128 bytes)      |
++------------------+ <- stack_base + stack_size
+| ISR Red Zone     | <- 144 bytes reserved for ISR
+| (144 bytes)      |
 +------------------+ <- Initial SP (16-byte aligned)
 |                  |
 | Task Stack       | <- Grows downward
@@ -251,8 +262,8 @@ Minimal context (jmp_buf):
 - 17 × 32-bit loads/stores = 68 bytes
 - Essential for cooperative scheduling
 
-Full context (ISR):  
-- 32 × 32-bit loads/stores = 128 bytes  
+Full context (ISR):
+- 33 × 32-bit loads/stores = 144 bytes (includes padding for alignment)
 - Required for preemptive interrupts
 
 ### Function Call Overhead

Documentation/hal-riscv-context-switch.md

@@ -96,7 +96,11 @@ State Preservation:
 - Nested interrupts are handled correctly by hardware's automatic state stacking
 
 ### Task Initialization
-New tasks are initialized with proper processor state:
+Task initialization differs between cooperative and preemptive modes due to
+their distinct context management approaches.
+
+In cooperative mode, tasks use lightweight context structures for voluntary
+yielding. New tasks are initialized with execution context only:
 
 ```c
 void hal_context_init(jmp_buf *ctx, size_t sp, size_t ss, size_t ra)
@@ -109,7 +113,58 @@ void hal_context_init(jmp_buf *ctx, size_t sp, size_t ss, size_t ra)
 }
 ```
 
-This ensures new tasks start with interrupts enabled in machine mode.
+This lightweight approach uses standard calling conventions where tasks
+return control through normal function returns.
+
+Preemptive mode requires interrupt frame structures to support trap-based
+context switching and privilege mode transitions. Task initialization builds
+a complete interrupt service routine frame:
+
+```c
+void *hal_build_initial_frame(void *stack_top,
+                              void (*task_entry)(void),
+                              int user_mode)
+{
+    /* Place frame in stack with initial reserve below for proper startup */
+    uint32_t *frame = (uint32_t *) ((uint8_t *) stack_top - 256 -
+                                    ISR_STACK_FRAME_SIZE);
+
+    /* Initialize all general purpose registers to zero */
+    for (int i = 0; i < 32; i++)
+        frame[i] = 0;
+
+    /* Compute thread pointer: aligned to 64 bytes from _end */
+    uint32_t tp_val = ((uint32_t) &_end + 63) & ~63U;
+
+    /* Set essential pointers */
+    frame[FRAME_GP] = (uint32_t) &_gp;  /* Global pointer */
+    frame[FRAME_TP] = tp_val;            /* Thread pointer */
+
+    /* Configure processor state for task entry:
+     * - MPIE=1: Interrupts will enable when task starts
+     * - MPP: Target privilege level (user or machine mode)
+     * - MIE=0: Keep interrupts disabled during frame restoration
+     */
+    uint32_t mstatus_val =
+        MSTATUS_MPIE | (user_mode ? MSTATUS_MPP_USER : MSTATUS_MPP_MACH);
+    frame[FRAME_MSTATUS] = mstatus_val;
+
+    /* Set entry point */
+    frame[FRAME_EPC] = (uint32_t) task_entry;
+
+    return frame;  /* Return frame base as initial stack pointer */
+}
+```
+
+The interrupt frame layout reserves space for all register state, control
+registers, and alignment padding. When the scheduler first dispatches this
+task, the trap return mechanism restores the frame and transfers control to
+the entry point with the configured privilege level.
+
+Key differences from cooperative mode include full register state allocation
+rather than minimal callee-saved registers, trap return semantics rather than
+function return, support for privilege level transitions through MPP
+configuration, and proper interrupt state initialization through MPIE bit.
 
 ## Implementation Details
 
@@ -168,10 +223,19 @@ New Task Creation:
 4. Processor state initialized with interrupts enabled
 
 First Task Launch:
-1. `hal_dispatch_init` transfers control from kernel to first task
+
+**Cooperative Mode**:
+1. `hal_dispatch_init` receives lightweight context structure
 2. Global interrupts enabled just before task execution
-3. Timer interrupts activated for preemptive scheduling
-4. Task begins execution at its entry point
+3. Control transfers to first task through standard function call
+4. Task begins execution and voluntarily yields control
+
+**Preemptive Mode**:
+1. `hal_dispatch_init` receives interrupt frame pointer
+2. Timer interrupt enabled for periodic preemption
+3. Dispatcher loads frame and executes trap return instruction
+4. Hardware restores registers and transitions to configured privilege level
+5. Task begins execution and can be preempted by timer
 
 Context Switch Cycle:
 1. Timer interrupt triggers scheduler entry

Makefile

@@ -29,7 +29,7 @@ deps += $(LIB_OBJS:%.o=%.o.d)
 APPS := coop echo hello mqueues semaphore mutex cond \
         pipes pipes_small pipes_struct prodcons progress \
         rtsched suspend test64 timer timer_kill \
-        cpubench test_libc
+        cpubench test_libc umode
 
 # Output files for __link target
 IMAGE_BASE := $(BUILD_DIR)/image
@@ -66,9 +66,9 @@ $(APPS): %: rebuild $(BUILD_APP_DIR)/%.o linmo
 # Link target - creates all output files
 __link: $(IMAGE_FILES)
 
-$(IMAGE_BASE).elf: $(BUILD_APP_DIR)/*.o $(BUILD_DIR)/liblinmo.a
+$(IMAGE_BASE).elf: $(BUILD_APP_DIR)/*.o $(BUILD_DIR)/liblinmo.a $(ENTRY_OBJ)
 	$(VECHO) "  LD\t$@\n"
-	$(Q)$(LD) $(LDFLAGS) -T$(LDSCRIPT) -Map $(IMAGE_BASE).map -o $@ $(BUILD_APP_DIR)/*.o -L$(BUILD_DIR) -llinmo
+	$(Q)$(LD) $(LDFLAGS) -T$(LDSCRIPT) -Map $(IMAGE_BASE).map -o $@ $(BUILD_APP_DIR)/*.o $(ENTRY_OBJ) -L$(BUILD_DIR) -llinmo
 
 $(IMAGE_BASE).lst: $(IMAGE_BASE).elf
 	$(VECHO) "  DUMP\t$@\n"

app/umode.c

@@ -0,0 +1,75 @@
+#include <linmo.h>
+
+/* U-mode Validation Task
+ *
+ * Integrates two tests into a single task flow to ensure sequential execution:
+ * 1. Phase 1: Mechanism Check - Verify syscalls work.
+ * 2. Phase 2: Security Check - Verify privileged instructions trigger a trap.
+ */
+void umode_validation_task(void)
+{
+    /* --- Phase 1: Mechanism Check (Syscalls) --- */
+    umode_printf("[umode] Phase 1: Testing Syscall Mechanism\n");
+
+    /* Test 1: sys_tid() - Simplest read-only syscall. */
+    int my_tid = sys_tid();
+    if (my_tid > 0) {
+        umode_printf("[umode] PASS: sys_tid() returned %d\n", my_tid);
+    } else {
+        umode_printf("[umode] FAIL: sys_tid() failed (ret=%d)\n", my_tid);
+    }
+
+    /* Test 2: sys_uptime() - Verify value transmission is correct. */
+    int uptime = sys_uptime();
+    if (uptime >= 0) {
+        umode_printf("[umode] PASS: sys_uptime() returned %d\n", uptime);
+    } else {
+        umode_printf("[umode] FAIL: sys_uptime() failed (ret=%d)\n", uptime);
+    }
+
+    /* Note: Skipping sys_tadd for now, as kernel user pointer checks might
+     * block function pointers in the .text segment, avoiding distraction.
+     */
+
+    /* --- Phase 2: Security Check (Privileged Access) --- */
+    umode_printf("[umode] ========================================\n");
+    umode_printf("[umode] Phase 2: Testing Security Isolation\n");
+    umode_printf(
+        "[umode] Action: Attempting to read 'mstatus' CSR from U-mode.\n");
+    umode_printf("[umode] Expect: Kernel Panic with 'Illegal instruction'.\n");
+    umode_printf("[umode] ========================================\n");
+
+    /* CRITICAL: Delay before suicide to ensure logs are flushed from
+     * buffer to UART.
+     */
+    sys_tdelay(10);
+
+    /* Privileged Instruction Trigger */
+    uint32_t mstatus;
+    asm volatile("csrr %0, mstatus" : "=r"(mstatus));
+
+    /* If execution reaches here, U-mode isolation failed (still has
+     * privileges).
+     */
+    umode_printf(
+        "[umode] FAIL: Privileged instruction executed! (mstatus=0x%lx)\n",
+        (long) mstatus);
+
+    /* Spin loop to prevent further execution. */
+    while (1)
+        sys_tyield();
+}
+
+int32_t app_main(void)
+{
+    umode_printf("[Kernel] Spawning U-mode validation task...\n");
+
+    /* app_main is called from kernel context during bootstrap.
+     * Use mo_task_spawn_user to create the validation task in user mode.
+     * This ensures privilege isolation is properly tested.
+     */
+    mo_task_spawn_user(umode_validation_task, DEFAULT_STACK_SIZE);
+
+    /* Return 1 to enable preemptive scheduler */
+    return 1;
+}

arch/riscv/boot.c

@@ -94,10 +94,10 @@ __attribute__((naked, section(".text.prologue"))) void _entry(void)
 }
 
 /* Size of the full trap context frame saved on the stack by the ISR.
- * 30 GPRs (x1, x3-x31) + mcause + mepc = 32 registers * 4 bytes = 128 bytes.
- * This provides a 16-byte aligned full context save.
+ * 30 GPRs (x1, x3-x31) + mcause + mepc + mstatus = 33 words * 4 bytes = 132
+ * bytes. Round up to 144 bytes for 16-byte alignment.
  */
-#define ISR_CONTEXT_SIZE 128
+#define ISR_CONTEXT_SIZE 144
 
 /* Low-level Interrupt Service Routine (ISR) trampoline.
  *
@@ -154,11 +154,15 @@ __attribute__((naked, aligned(4))) void _isr(void)
         "sw  t6,  29*4(sp)\n"
 
         /* Save trap-related CSRs and prepare arguments for do_trap */
-        "csrr   a0, mcause\n" /* Arg 1: cause */
-        "csrr   a1, mepc\n"   /* Arg 2: epc */
-        "mv     a2, sp\n"     /* Arg 3: isr_sp (current stack frame) */
+        "csrr   a0, mcause\n"
+        "csrr   a1, mepc\n"
+        "csrr   a2, mstatus\n" /* For context switching in privilege change */
+
         "sw     a0,  30*4(sp)\n"
         "sw     a1,  31*4(sp)\n"
+        "sw     a2,  32*4(sp)\n"
+
+        "mv     a2, sp\n" /* a2 = isr_sp */
 
         /* Call the high-level C trap handler.
          * Returns: a0 = SP to use for restoring context (may be different
@@ -169,9 +173,13 @@ __attribute__((naked, aligned(4))) void _isr(void)
         /* Use returned SP for context restore (enables context switching) */
         "mv     sp, a0\n"
 
-        /* Restore context. mepc might have been modified by the handler */
-        "lw     a1,  31*4(sp)\n"
-        "csrw   mepc, a1\n"
+        /* Restore mstatus from frame[32] */
+        "lw     t0, 32*4(sp)\n"
+        "csrw   mstatus, t0\n"
+
+        /* Restore mepc from frame[31] (might have been modified by handler) */
+        "lw     t1, 31*4(sp)\n"
+        "csrw   mepc, t1\n"
         "lw  ra,   0*4(sp)\n"
         "lw  gp,   1*4(sp)\n"
         "lw  tp,   2*4(sp)\n"
@@ -208,7 +216,7 @@ __attribute__((naked, aligned(4))) void _isr(void)
 
         /* Return from trap */
         "mret\n"
-        : /* no outputs */
-        : "i"(ISR_CONTEXT_SIZE)
+        :                       /* no outputs */
+        : "i"(ISR_CONTEXT_SIZE) /* +16 for mcause, mepc, mstatus */
         : "memory");
 }

arch/riscv/build.mk

@@ -74,6 +74,14 @@ HAL_OBJS := boot.o hal.o muldiv.o
 HAL_OBJS := $(addprefix $(BUILD_KERNEL_DIR)/,$(HAL_OBJS))
 deps += $(HAL_OBJS:%.o=%.o.d)
 
+# Architecture-specific syscall entry point requiring direct linkage.
+# Archives only extract objects when symbols are unresolved. Since the generic
+# syscall dispatcher provides a weak symbol, the archive mechanism would skip
+# the strong override. Direct linking ensures the architecture-specific
+# implementation takes precedence at link time.
+ENTRY_OBJ := $(BUILD_KERNEL_DIR)/entry.o
+deps += $(ENTRY_OBJ).d
+
 $(BUILD_KERNEL_DIR)/%.o: $(ARCH_DIR)/%.c | $(BUILD_DIR)
 	$(VECHO) "  CC\t$@\n"
 	$(Q)$(CC) $(CFLAGS) -o $@ -c -MMD -MF $@.d $<