diff --git a/Documentation/hal-interrupt.md b/Documentation/hal-interrupt.md index eb8f5e64..38480fb1 100644 --- a/Documentation/hal-interrupt.md +++ b/Documentation/hal-interrupt.md @@ -209,9 +209,144 @@ void bad_critical_section(void) { /* CORRECT */ void good_pattern(void) { mo_sem_wait(semaphore); /* Block outside critical section */ - + CRITICAL_ENTER(); /* Quick critical work */ CRITICAL_LEAVE(); } ``` + +## ISR Context Safety Guide + +### Overview +Interrupt Service Routines (ISRs), including timer callbacks, execute in interrupt context with special restrictions. +Violating these restrictions causes heap corruption, deadlocks, or undefined behavior. + +### ISR-Safe vs Task-Only Functions + +#### ISR-Safe Functions +These functions can be called from ISR context (timer callbacks, trap handlers): + +| Function | Protection | Notes | +|----------|------------|-------| +| `mo_task_id()` | None needed | Read-only | +| `mo_task_count()` | None needed | Read-only | +| `mo_ticks()` | None needed | Volatile read | +| `mo_uptime()` | None needed | Calculated | +| `mo_timer_start/cancel()` | NOSCHED | Timer control only | +| `mo_sem_trywait()` | None | Non-blocking | +| `mo_sem_signal()` | NOSCHED | Wakes waiting tasks | +| `mo_mutex_trylock()` | None | Non-blocking | +| `mo_mutex_unlock()` | NOSCHED | Releases lock | +| `mo_cond_signal/broadcast()` | NOSCHED | Wakes waiters | +| `mo_pipe_nbread/nbwrite()` | None | Non-blocking I/O | +| `mo_logger_enqueue()` | CRITICAL | Deferred logging | +| `isr_puts()` | None | Direct UART output | +| `isr_putx()` | None | Direct UART hex output | + +#### Task-Only Functions +These functions must NOT be called from ISR context: + +| Function | Reason | Alternative | +|----------|--------|-------------| +| `mo_task_spawn()` | Uses malloc | Pre-create tasks | +| `mo_task_cancel()` | Uses free | Defer to task | +| `mo_timer_create()` | Uses malloc | Pre-create timers | +| `mo_timer_destroy()` | Uses free | Defer to task | +| `mo_task_delay/yield()` | Invokes scheduler | Use timer callback | +| `mo_task_suspend/resume()` | Modifies scheduler | Use semaphore | +| `mo_sem_wait()` | Blocks caller | Use `mo_sem_trywait()` | +| `mo_mutex_lock()` | Blocks caller | Use `mo_mutex_trylock()` | +| `mo_cond_wait()` | Blocks caller | Signal from ISR, wait in task | +| `mo_pipe_read/write()` | Blocks caller | Use `mo_pipe_nbread/nbwrite()` | +| `mo_mq_enqueue/dequeue()` | Uses malloc | Use pipe instead | +| `printf()` | May deadlock | Use `mo_logger_enqueue()` | + +### Timer Callback Patterns + +Timer callbacks execute in ISR context. Example safe callback: +```c +void *my_callback(void *arg) { + /* Safe: ISR-protected logging */ + mo_logger_enqueue("Timer fired\n", 12); + + /* Safe: Non-blocking semaphore signal to wake task */ + mo_sem_signal(signal_sem); + + /* Safe: Non-blocking pipe write */ + char msg[] = "event"; + mo_pipe_nbwrite(event_pipe, msg, sizeof(msg)); + + /* UNSAFE - do not use in callbacks: + * mo_task_spawn(...); // Uses malloc + * mo_task_delay(10); // Invokes scheduler + * mo_sem_wait(sem); // Blocks caller + * printf(...); // May deadlock + */ + + return NULL; +} +``` + +### ISR-to-Task Communication Patterns + +#### Pattern 1: Semaphore Signaling +```c +/* ISR/callback: Signal event */ +void *timer_callback(void *arg) { + mo_sem_signal(event_sem); + return NULL; +} + +/* Task: Wait for event */ +void event_handler_task(void) { + while (1) { + mo_sem_wait(event_sem); /* Blocks until ISR signals */ + process_event(); + } +} +``` + +#### Pattern 2: Non-Blocking Pipe +```c +/* ISR/callback: Send data */ +void *sensor_callback(void *arg) { + sensor_data_t data = read_sensor(); + mo_pipe_nbwrite(sensor_pipe, &data, sizeof(data)); + return NULL; +} + +/* Task: Process data (non-blocking read, polls or uses semaphore wakeup) */ +void sensor_task(void) { + sensor_data_t data; + while (1) { + if (mo_pipe_nbread(sensor_pipe, &data, sizeof(data)) > 0) + process_sensor_data(&data); + mo_task_delay(1); /* Yield or use semaphore for event-driven */ + } +} +``` + +#### Pattern 3: Deferred Logging +```c +/* ISR/callback: Queue log message */ +void *debug_callback(void *arg) { + mo_logger_enqueue("Tick!\n", 6); + return NULL; +} +/* Logger task automatically outputs queued messages */ +``` + +### Emergency Debug Output + +For debugging trap handlers or when the logger is unavailable, use direct UART: +```c +void *debug_isr(void *arg) { + isr_puts("[ISR] Value=0x"); + isr_putx(some_value); + isr_puts("\r\n"); + return NULL; +} +``` + +Warning: Direct UART output is blocking and slow. Use only for emergency debugging. diff --git a/app/timer.c b/app/timer.c index 19f53466..58cecc60 100644 --- a/app/timer.c +++ b/app/timer.c @@ -2,11 +2,18 @@ * * It creates several timers with different periods and modes to verify * their correct operation. + * + * Note: Timer callbacks execute in ISR context. This test uses printf() + * which may fall back to blocking direct output when the queue is full + * or the message exceeds LOG_ENTRY_SZ. For production ISR callbacks, + * prefer mo_logger_enqueue() or isr_puts() which are always ISR-safe. */ #include -/* Helper function to print the current system uptime. */ -void print_time() +/* Helper function to print the current system uptime. + * Called from timer callback (ISR context) - uses ISR-safe logging. + */ +static void print_time(void) { /* mo_uptime() returns a 64-bit value to prevent rollover. */ uint64_t time_ms = mo_uptime(); @@ -17,8 +24,10 @@ void print_time() } /* Generic timer callback. - * We can use a single callback for multiple timers by passing a unique - * identifier as the argument. + * Executes in ISR context - only ISR-safe functions are permitted here. + * This example uses printf() for convenience in a test app; it may block + * on direct output fallback. For guaranteed ISR-safe logging, use + * mo_logger_enqueue(), isr_puts(), or isr_putx(). */ void *timer_callback(void *arg) { diff --git a/include/lib/libc.h b/include/lib/libc.h index 12f6f35d..b588cc1f 100644 --- a/include/lib/libc.h +++ b/include/lib/libc.h @@ -143,6 +143,48 @@ int random_r(struct random_data *buf, int32_t *result); int32_t puts(const char *str); int _putchar(int c); /* Low-level character output (used by logger) */ +/* ISR-Safe I/O Functions + * [ISR-SAFE] These functions are safe to call from interrupt context. + * They perform direct UART output without locks, buffering, or malloc. + * Use for emergency debug output in timer callbacks and trap handlers. + * + * Warning: Direct UART output is slow and blocks until complete. + * For normal logging, prefer mo_logger_enqueue() which is non-blocking. + */ + +/* Maximum characters isr_puts() will emit per call. + * Prevents unbounded blocking in ISR context on long strings. + */ +#define ISR_OUTPUT_MAX 128 + +/* ISR-safe string output - direct UART, no buffering. + * [ISR-SAFE] Safe to call from any context including ISR and timer callbacks. + * Output is capped at ISR_OUTPUT_MAX characters to bound ISR latency. + * + * @s : Null-terminated string to output + */ +static inline void isr_puts(const char *s) +{ + for (uint32_t i = 0; s && *s && i < ISR_OUTPUT_MAX; i++) + _putchar(*s++); +} + +/* ISR-safe hexadecimal output - direct UART, no buffering. + * [ISR-SAFE] Safe to call from any context including ISR and timer callbacks. + * + * Outputs a 32-bit value as 8 hex digits (e.g., "DEADBEEF"). + * Useful for printing addresses and error codes in trap handlers. + * + * @val : 32-bit value to output as hexadecimal + */ +static inline void isr_putx(uint32_t val) +{ + for (int i = 28; i >= 0; i -= 4) { + uint32_t nibble = (val >> i) & 0xF; + _putchar(nibble < 10 ? '0' + nibble : 'A' + nibble - 10); + } +} + /* Character and string input */ int getchar(void); char *gets(char *s); diff --git a/include/linmo.h b/include/linmo.h index e4cbecfc..2c7fcda3 100644 --- a/include/linmo.h +++ b/include/linmo.h @@ -1,5 +1,52 @@ #pragma once +/* Linmo Operating System - Main API Header + * + * This header includes all kernel APIs for task management, synchronization, + * IPC, and system services. + * + * Interrupt Service Routines (ISRs), including timer callbacks, execute in + * interrupt context with special restrictions. Violating these restrictions + * causes heap corruption, deadlocks, or undefined behavior. + * + * [ISR-SAFE] Functions (callable from interrupt context): + * ------------------------------------------------------- + * System Info: mo_task_id(), mo_task_count(), mo_ticks(), mo_uptime() + * Timer API: mo_timer_create/destroy/start/cancel() [NOSCHED protected] + * Semaphore: mo_sem_trywait(), mo_sem_signal() [non-blocking] + * Mutex: mo_mutex_trylock(), mo_mutex_unlock() [non-blocking] + * Condition Var: mo_cond_signal(), mo_cond_broadcast() [non-blocking] + * Pipe I/O: mo_pipe_nbread(), mo_pipe_nbwrite() [non-blocking] + * Logging: mo_logger_enqueue() [CRITICAL protected] + * Direct I/O: _putchar() for emergency/debug output + * + * [TASK-ONLY] Functions (must NOT call from ISR context): + * ------------------------------------------------------- + * Memory: mo_task_spawn(), mo_task_cancel() - use malloc/free + * Blocking: mo_task_delay(), mo_task_yield(), mo_task_suspend() + * Blocking Sync: mo_sem_wait(), mo_mutex_lock(), mo_cond_wait() + * Blocking I/O: mo_pipe_read(), mo_pipe_write() + * Message Queue: mo_mq_enqueue(), mo_mq_dequeue() - unprotected malloc + * Stdio: printf(), puts() - may deadlock in preemptive mode + * + * Timer Callback Rules: + * --------------------- + * Timer callbacks execute in ISR context. Example safe callback: + * + * void *my_callback(void *arg) { + * mo_logger_enqueue("Timer fired\n", 12); // Safe: ISR-protected + * mo_sem_signal(signal_sem); // Safe: non-blocking + * // UNSAFE: mo_task_spawn(...); // Uses malloc + * // UNSAFE: printf(...); // May deadlock + * return NULL; + * } + * + * For emergency debug output in ISR, use the ISR-safe I/O functions: + * isr_puts("message"); // Direct UART string output + * isr_putx(0xDEADBEEF); // Direct UART hex output + * These are defined in lib/libc.h and available via linmo.h. + */ + #include #include diff --git a/include/sys/logger.h b/include/sys/logger.h index e4a3df75..2d6977ea 100644 --- a/include/sys/logger.h +++ b/include/sys/logger.h @@ -37,8 +37,15 @@ int32_t mo_logger_init(void); /* Enqueue a log message for deferred output. + * [ISR-SAFE] Protected by CRITICAL_ENTER - safe to call from ISR context + * * Non-blocking: if queue is full, message is dropped. * Thread-safe: protected by short critical section. + * + * This is the RECOMMENDED way to log from timer callbacks and ISRs. + * The message is queued and output later by the logger task, avoiding + * the risk of deadlock from direct UART output in ISR context. + * * @msg : Null-terminated message string * @length : Length of message (excluding null terminator) * diff --git a/include/sys/mqueue.h b/include/sys/mqueue.h index ae191930..c55dbb4e 100644 --- a/include/sys/mqueue.h +++ b/include/sys/mqueue.h @@ -7,6 +7,17 @@ * Provides FIFO message passing between tasks with type-safe message * containers. Messages consist of a user-allocated payload, a type * discriminator, and size information. + * + * ISR-SAFETY WARNING: + * [TASK-ONLY] All message queue operations are task-context only. + * Message queue operations use underlying queue functions that may perform + * memory allocation. These operations are NOT protected by CRITICAL_ENTER + * and are NOT safe to call from ISR context. + * + * For ISR-to-task communication, use: + * - mo_pipe_nbwrite() for simple byte streams + * - mo_sem_signal() to wake a task + * - mo_logger_enqueue() for logging */ /* Message container structure */ diff --git a/include/sys/mutex.h b/include/sys/mutex.h index c2bb9fe7..fa7b7f3f 100644 --- a/include/sys/mutex.h +++ b/include/sys/mutex.h @@ -54,6 +54,8 @@ int32_t mo_mutex_destroy(mutex_t *m); /* Mutex Locking Operations */ /* Acquire mutex lock, blocking if necessary. + * [TASK-ONLY] May block caller - NOT safe from ISR context + * * If the mutex is free, acquires it immediately. If owned by another task, * the calling task is placed in the wait queue and blocked until the mutex * becomes available. Non-recursive - returns error if caller already owns @@ -65,6 +67,8 @@ int32_t mo_mutex_destroy(mutex_t *m); int32_t mo_mutex_lock(mutex_t *m); /* Attempt to acquire mutex lock without blocking. + * [ISR-SAFE] Non-blocking, returns immediately with success/failure + * * Returns immediately whether the lock was acquired or not. * @m : Pointer to mutex structure (must be valid) * @@ -73,6 +77,8 @@ int32_t mo_mutex_lock(mutex_t *m); int32_t mo_mutex_trylock(mutex_t *m); /* Attempt to acquire mutex lock with timeout. + * [TASK-ONLY] May block caller - NOT safe from ISR context + * * Blocks for up to 'ticks' scheduler ticks waiting for the mutex. * @m : Pointer to mutex structure (must be valid) * @ticks : Maximum time to wait in scheduler ticks (0 = trylock behavior) @@ -83,6 +89,8 @@ int32_t mo_mutex_trylock(mutex_t *m); int32_t mo_mutex_timedlock(mutex_t *m, uint32_t ticks); /* Release mutex lock. + * [ISR-SAFE] Protected by NOSCHED_ENTER - safe from any context + * * If tasks are waiting, ownership is transferred to the next task in FIFO * order. The released task is marked ready but may not run immediately * depending on scheduler priority. @@ -142,6 +150,8 @@ int32_t mo_cond_destroy(cond_t *c); /* Condition Variable Wait Operations */ /* Wait on condition variable (atomically releases mutex). + * [TASK-ONLY] Blocks caller - NOT safe from ISR context + * * The calling task must own the specified mutex. The mutex is atomically * released and the task blocks until another task signals the condition. * Upon waking, the mutex is re-acquired before returning. @@ -153,6 +163,8 @@ int32_t mo_cond_destroy(cond_t *c); int32_t mo_cond_wait(cond_t *c, mutex_t *m); /* Wait on condition variable with timeout. + * [TASK-ONLY] Blocks caller - NOT safe from ISR context + * * Like mo_cond_wait(), but limits wait time to specified number of ticks. * @c : Pointer to condition variable structure (must be valid) * @m : Pointer to mutex structure that caller must own @@ -166,6 +178,8 @@ int32_t mo_cond_timedwait(cond_t *c, mutex_t *m, uint32_t ticks); /* Condition Variable Signal Operations */ /* Signal one waiting task. + * [ISR-SAFE] Protected by NOSCHED_ENTER - safe for ISR-to-task signaling + * * Wakes up the oldest task waiting on the condition variable (FIFO order). * The signaled task will attempt to re-acquire the associated mutex. * @c : Pointer to condition variable structure (must be valid) @@ -175,6 +189,8 @@ int32_t mo_cond_timedwait(cond_t *c, mutex_t *m, uint32_t ticks); int32_t mo_cond_signal(cond_t *c); /* Signal all waiting tasks. + * [ISR-SAFE] Protected by NOSCHED_ENTER - safe from any context + * * Wakes up all tasks waiting on the condition variable. All tasks will * attempt to re-acquire their associated mutexes, but only one will succeed * immediately (others will block on the mutex). diff --git a/include/sys/pipe.h b/include/sys/pipe.h index 758b467e..8ecb01e6 100644 --- a/include/sys/pipe.h +++ b/include/sys/pipe.h @@ -72,9 +72,14 @@ int32_t mo_pipe_capacity(pipe_t *pipe); */ int32_t mo_pipe_free_space(pipe_t *pipe); -/* Blocking I/O Operations (runs in task context only) */ +/* Blocking I/O Operations + * [TASK-ONLY] These functions block the caller and invoke the scheduler. + * They must NOT be called from ISR context (including timer callbacks). + */ /* Read data from pipe, blocking until all requested bytes are available. + * [TASK-ONLY] Blocks caller - NOT safe from ISR context + * * @pipe : Pointer to pipe structure (must be valid) * @data : Buffer to store read data (must be non-NULL) * @size : Number of bytes to read (must be > 0) @@ -85,6 +90,8 @@ int32_t mo_pipe_free_space(pipe_t *pipe); int32_t mo_pipe_read(pipe_t *pipe, char *data, uint16_t size); /* Write data to pipe, blocking until all data is written. + * [TASK-ONLY] Blocks caller - NOT safe from ISR context + * * @pipe : Pointer to pipe structure (must be valid) * @data : Data to write (must be non-NULL) * @size : Number of bytes to write (must be > 0) @@ -94,9 +101,15 @@ int32_t mo_pipe_read(pipe_t *pipe, char *data, uint16_t size); */ int32_t mo_pipe_write(pipe_t *pipe, const char *data, uint16_t size); -/* Non-blocking I/O Operations (returns immediately with partial results) */ +/* Non-blocking I/O Operations + * [ISR-SAFE] These functions return immediately without blocking or invoking + * the scheduler. Safe to call from ISR context, including timer callbacks. + * Ideal for ISR-to-task communication patterns. + */ /* Read available data from pipe without blocking. + * [ISR-SAFE] Non-blocking, returns immediately + * * @pipe : Pointer to pipe structure (must be valid) * @data : Buffer to store read data (must be non-NULL) * @size : Maximum number of bytes to read @@ -107,6 +120,8 @@ int32_t mo_pipe_write(pipe_t *pipe, const char *data, uint16_t size); int32_t mo_pipe_nbread(pipe_t *pipe, char *data, uint16_t size); /* Write data to pipe without blocking. + * [ISR-SAFE] Non-blocking, returns immediately + * * @pipe : Pointer to pipe structure (must be valid) * @data : Data to write (must be non-NULL) * @size : Number of bytes to write diff --git a/include/sys/semaphore.h b/include/sys/semaphore.h index f7848781..6b67c9a8 100644 --- a/include/sys/semaphore.h +++ b/include/sys/semaphore.h @@ -43,6 +43,7 @@ int32_t mo_sem_destroy(sem_t *s); /* Semaphore Wait Operations */ /* Acquires the semaphore (a "P" or "pend" operation), blocking if necessary. + * [TASK-ONLY] May block caller - NOT safe from ISR context * * If the semaphore's count is > 0, it is decremented and the function returns * immediately. If the count is 0, the calling task is placed in the semaphore's @@ -54,6 +55,8 @@ int32_t mo_sem_destroy(sem_t *s); void mo_sem_wait(sem_t *s); /* Attempts to acquire the semaphore without blocking. + * [ISR-SAFE] Non-blocking, returns immediately with success/failure + * * @s : A pointer to the semaphore. Must not be NULL. * * Returns 0 if the semaphore was acquired successfully, or ERR_FAIL if it could @@ -65,11 +68,14 @@ int32_t mo_sem_trywait(sem_t *s); /* Releases the semaphore (a "V" or "post" operation), potentially unblocking * a waiter. + * [ISR-SAFE] Protected by NOSCHED_ENTER - safe for ISR-to-task signaling * * If there are tasks blocked in the wait queue, the oldest waiting task is * unblocked (FIFO order). If the wait queue is empty, the semaphore's * resource count is incremented up to SEM_MAX_COUNT. * @s : A pointer to the semaphore. Must not be NULL. + * + * Common ISR pattern: Timer callback signals semaphore to wake waiting task. */ void mo_sem_signal(sem_t *s); diff --git a/include/sys/task.h b/include/sys/task.h index f6b78e50..2cca9759 100644 --- a/include/sys/task.h +++ b/include/sys/task.h @@ -140,22 +140,41 @@ extern kcb_t *kcb; * 2. NOSCHED_* macros disable ONLY the scheduler timer interrupt */ +/* Critical section nesting support - defined in kernel/task.c. + * These enable ISR-safe critical sections by tracking nesting depth and + * saving/restoring the original interrupt state rather than unconditionally + * enabling interrupts on CRITICAL_LEAVE. + */ +extern volatile uint32_t critical_nesting; +extern volatile int32_t critical_saved_mie; + /* Disable/enable ALL maskable interrupts globally. * Provides strongest protection against concurrency from both other tasks * and all ISRs. Use when modifying data shared with any ISR. + * + * ISR-SAFE: These macros correctly handle nesting and restore the original + * interrupt state. Safe to call from ISR context (e.g., timer callbacks). + * * WARNING: Increases interrupt latency - use NOSCHED macros if protection * is only needed against task preemption. */ -#define CRITICAL_ENTER() \ - do { \ - if (kcb->preemptive) \ - _di(); \ +#define CRITICAL_ENTER() \ + do { \ + if (kcb->preemptive) { \ + int32_t _mie = _di(); \ + if (critical_nesting == 0) \ + critical_saved_mie = _mie; \ + critical_nesting++; \ + } \ } while (0) -#define CRITICAL_LEAVE() \ - do { \ - if (kcb->preemptive) \ - _ei(); \ +#define CRITICAL_LEAVE() \ + do { \ + if (kcb->preemptive && critical_nesting > 0) { \ + critical_nesting--; \ + if (critical_nesting == 0) \ + hal_interrupt_set(critical_saved_mie); \ + } \ } while (0) /* Flag indicating scheduler has started - prevents timer IRQ during early @@ -183,23 +202,37 @@ extern volatile bool scheduler_started; hal_timer_irq_enable(); \ } while (0) -/* Core Kernel and Task Management API */ +/* Core Kernel and Task Management API + * + * ISR-Safety Legend: + * [ISR-SAFE] - Safe to call from interrupt context + * [TASK-ONLY] - Must only be called from task context + * [ISR-INTERNAL]- Called by ISR infrastructure, not user code + */ /* System Control Functions */ -/* Prints a fatal error message and halts the system */ +/* Prints a fatal error message and halts the system. + * [ISR-SAFE] Safe from any context (terminates system) + */ void panic(int32_t ecode); -/* Main scheduler dispatch function, called by timer ISR or ecall */ +/* Main scheduler dispatch function, called by timer ISR or ecall. + * [ISR-INTERNAL] Not for user code - called by trap handler + */ void dispatcher(int from_timer); -/* Architecture-specific context switch implementations */ +/* Architecture-specific context switch implementations. + * [ISR-INTERNAL] Not for user code - called by scheduler + */ void _dispatch(void); void _yield(void); /* Task Lifecycle Management */ /* Application task creation macro. + * [TASK-ONLY] Uses malloc() - NOT safe from ISR context + * * @task_entry : Pointer to the task's entry function (void func(void)) * @stack_size : The desired stack size in bytes (minimum is enforced) * @@ -221,6 +254,8 @@ int sys_task_spawn(void *task, int stack_size); #endif /* Cancels and removes a task from the system. A task cannot cancel itself. + * [TASK-ONLY] Uses free() - NOT safe from ISR context + * * @id : The ID of the task to cancel * * Returns 0 on success, or a negative error code @@ -229,16 +264,22 @@ int32_t mo_task_cancel(uint16_t id); /* Task Scheduling Control */ -/* Voluntarily yields the CPU, allowing the scheduler to run another task */ +/* Voluntarily yields the CPU, allowing the scheduler to run another task. + * [TASK-ONLY] Invokes scheduler - NOT safe from ISR context + */ void mo_task_yield(void); /* Blocks the current task for a specified number of system ticks. + * [TASK-ONLY] Blocks caller - NOT safe from ISR context + * * @ticks : The number of system ticks to sleep. The task will be unblocked * after this duration has passed. */ void mo_task_delay(uint16_t ticks); /* Suspends a task, removing it from scheduling temporarily. + * [TASK-ONLY] Modifies scheduler state - NOT safe from ISR context + * * @id : The ID of the task to suspend. A task can suspend itself. * * Returns 0 on success, or a negative error code @@ -246,6 +287,8 @@ void mo_task_delay(uint16_t ticks); int32_t mo_task_suspend(uint16_t id); /* Resumes a previously suspended task. + * [TASK-ONLY] Modifies scheduler state - NOT safe from ISR context + * * @id : The ID of the task to resume * * Returns 0 on success, or a negative error code @@ -255,6 +298,8 @@ int32_t mo_task_resume(uint16_t id); /* Task Priority Management */ /* Changes a task's base priority. + * [TASK-ONLY] Modifies scheduler state - NOT safe from ISR context + * * @id : The ID of the task to modify * @priority : The new priority value (from enum task_priorities) * @@ -263,6 +308,8 @@ int32_t mo_task_resume(uint16_t id); int32_t mo_task_priority(uint16_t id, uint16_t priority); /* Assigns a task to a custom real-time scheduler. + * [TASK-ONLY] Modifies scheduler state - NOT safe from ISR context + * * @id : The ID of the task to modify * @priority : Opaque pointer to custom priority data for the RT scheduler * @@ -273,30 +320,41 @@ int32_t mo_task_rt_priority(uint16_t id, void *priority); /* Task Information and Status */ /* Gets the ID of the currently running task. + * [ISR-SAFE] Read-only access to current task pointer * * Returns the current task's ID */ uint16_t mo_task_id(void); /* Gets a task's ID from its entry function pointer. + * [ISR-SAFE] Read-only search through task list + * * @task_entry : Pointer to the task's entry function * * Returns the task's ID, or ERR_TASK_NOT_FOUND if no task matches */ int32_t mo_task_idref(void *task_entry); -/* Puts the CPU into a low-power state, waiting for the next scheduler tick */ +/* Puts the CPU into a low-power state, waiting for the next scheduler tick. + * [TASK-ONLY] Executes WFI instruction - context dependent + */ void mo_task_wfi(void); -/* Gets the total number of active tasks in the system */ +/* Gets the total number of active tasks in the system. + * [ISR-SAFE] Read-only access to cached counter + */ uint16_t mo_task_count(void); /* System Time Functions */ -/* Gets the current value of the system tick counter */ +/* Gets the current value of the system tick counter. + * [ISR-SAFE] Read-only access to volatile counter + */ uint32_t mo_ticks(void); -/* Gets the system uptime in milliseconds */ +/* Gets the system uptime in milliseconds. + * [ISR-SAFE] Calculated from tick counter + */ uint64_t mo_uptime(void); /* Internal Kernel Primitives */ diff --git a/include/sys/timer.h b/include/sys/timer.h index 397eb9dd..f2508796 100644 --- a/include/sys/timer.h +++ b/include/sys/timer.h @@ -6,6 +6,29 @@ * in one-shot or auto-reload modes and execute user-defined callbacks upon * expiration. All timers are managed by the kernel and serviced during * system timer interrupts. + * + * ISR-SAFETY: + * All timer management functions (create/destroy/start/cancel) are protected + * with NOSCHED_ENTER and are safe to call from both task and ISR context. + * + * CALLBACK CONSTRAINTS: + * Timer callbacks execute in ISR context (from the timer tick handler). + * Callbacks MUST: + * - Only call ISR-safe functions (see linmo.h for complete list) + * - Have bounded execution time to avoid starving other timers + * - Not call malloc/free, printf, or any blocking function + * + * Safe callback operations: + * - mo_sem_signal(), mo_cond_signal() for waking tasks + * - mo_logger_enqueue() for logging + * - mo_pipe_nbwrite() for non-blocking data output + * - Direct UART via _putchar() for debug output + * + * UNSAFE in callbacks (will cause corruption/deadlock): + * - mo_task_spawn(), mo_task_cancel() [use malloc/free] + * - mo_task_delay(), mo_task_yield() [invoke scheduler] + * - mo_sem_wait(), mo_mutex_lock() [block caller] + * - printf(), puts() [may deadlock in preemptive mode] */ #include @@ -44,11 +67,13 @@ typedef struct { /* Timer Management Functions */ /* Creates a new software timer. + * [ISR-SAFE] Protected by NOSCHED_ENTER - safe from any context * * The timer is created in a DISABLED state and must be started with * 'mo_timer_start()' before it will begin counting. * - * @callback : The function to execute upon expiry (cannot be NULL) + * @callback : The function to execute upon expiry (cannot be NULL). + * WARNING: Callback runs in ISR context - see constraints above. * @period_ms : The timer's period in milliseconds (must be > 0) * @arg : A user-defined argument to be passed to the callback * @@ -59,6 +84,7 @@ int32_t mo_timer_create(void *(*callback)(void *arg), void *arg); /* Destroys a software timer and frees its resources. + * [ISR-SAFE] Protected by NOSCHED_ENTER - safe from any context * * If the timer is active, it will be cancelled before being destroyed. * After destruction, the timer ID becomes invalid and should not be used. @@ -72,6 +98,7 @@ int32_t mo_timer_destroy(uint16_t id); /* Timer Control Functions */ /* Starts or restarts a software timer. + * [ISR-SAFE] Protected by NOSCHED_ENTER - safe from any context * * This function arms the timer and adds it to the active list. If the timer * was already running, its deadline is recalculated and it is rescheduled. @@ -85,6 +112,7 @@ int32_t mo_timer_destroy(uint16_t id); int32_t mo_timer_start(uint16_t id, uint8_t mode); /* Cancels a running software timer. + * [ISR-SAFE] Protected by NOSCHED_ENTER - safe from any context * * This function disarms the timer and removes it from the active list. The * timer object itself is not destroyed and can be restarted later with diff --git a/kernel/logger.c b/kernel/logger.c index 4174af8b..7e32818b 100644 --- a/kernel/logger.c +++ b/kernel/logger.c @@ -1,15 +1,14 @@ /* Deferred logging: async I/O pattern for thread-safe printf. * * Design rationale: - * - Ring buffer + mutex + * - Ring buffer protected by critical sections (ISR-safe) * - Logger task at IDLE priority: drains queue without blocking tasks - * - UART output outside lock: other tasks enqueue while we output + * - UART output outside critical section: other tasks enqueue while we output * - Graceful degradation: fallback to direct output on queue full */ #include #include -#include #include #include "private/error.h" @@ -21,18 +20,22 @@ typedef struct { char data[LOG_ENTRY_SZ]; } log_entry_t; -/* Logger state: single global instance, no dynamic allocation */ +/* Logger state: single global instance, no dynamic allocation. + * All queue fields protected by CRITICAL_ENTER/LEAVE (ISR-safe). + * + * FIXME(SMP): Single global logger assumes single-core. For SMP, use per-CPU + * log buffers or replace CRITICAL with spinlocks for cross-CPU safety. + */ typedef struct { log_entry_t queue[LOG_QSIZE]; uint32_t head, tail, count; uint32_t dropped; /* Diagnostic: tracks queue overflow events */ - mutex_t lock; /* Protects queue manipulation, not UART output */ int32_t task_id; bool initialized; /* When true, printf bypasses queue. * volatile: prevent compiler caching for lock-free read. Written under - * mutex, read without - safe on single-core. + * critical section, read without - safe on single-core. */ volatile bool direct_mode; } logger_state_t; @@ -47,20 +50,21 @@ static void logger_task(void) while (1) { bool have_message = false; - /* Critical section: only queue manipulation, not UART I/O */ - mo_mutex_lock(&logger.lock); + /* Critical section: only queue manipulation, not UART I/O. + * Uses CRITICAL to synchronize with ISR-context enqueuers. + */ + CRITICAL_ENTER(); if (logger.count > 0) { memcpy(&entry, &logger.queue[logger.tail], sizeof(log_entry_t)); logger.tail = (logger.tail + 1) % LOG_QSIZE; logger.count--; have_message = true; } - mo_mutex_unlock(&logger.lock); + CRITICAL_LEAVE(); if (have_message) { - /* Key design: UART output outside lock prevents blocking enqueuers. - * shorter UART write does not hold mutex - other tasks enqueue in - * parallel. + /* Key design: UART output outside critical section prevents + * blocking enqueuers. Other tasks/ISRs enqueue in parallel. */ for (uint16_t i = 0; i < entry.length; i++) _putchar(entry.data[i]); @@ -79,16 +83,11 @@ int32_t mo_logger_init(void) memset(&logger, 0, sizeof(logger_state_t)); - if (mo_mutex_init(&logger.lock) != ERR_OK) - return ERR_FAIL; - /* 1024B stack: space for log_entry_t (130B) + ISR frame (128B) + calls */ /* Kernel internal: always use direct API for kernel-mode task */ logger.task_id = mo_task_spawn_kernel(logger_task, 1024); - if (logger.task_id < 0) { - mo_mutex_destroy(&logger.lock); + if (logger.task_id < 0) return ERR_FAIL; - } /* IDLE priority: runs only when no application tasks are ready */ mo_task_priority(logger.task_id, TASK_PRIO_IDLE); @@ -97,7 +96,9 @@ int32_t mo_logger_init(void) return ERR_OK; } -/* Non-blocking enqueue: returns ERR_TASK_BUSY on overflow, never waits */ +/* Non-blocking enqueue: returns ERR_TASK_BUSY on overflow, never waits. + * [ISR-SAFE] Uses CRITICAL_ENTER for short critical section - safe from ISR. + */ int32_t mo_logger_enqueue(const char *msg, uint16_t length) { if (!logger.initialized || !msg || length == 0) @@ -107,14 +108,14 @@ int32_t mo_logger_enqueue(const char *msg, uint16_t length) if (length > LOG_ENTRY_SZ - 1) length = LOG_ENTRY_SZ - 1; - mo_mutex_lock(&logger.lock); + CRITICAL_ENTER(); /* Drop message on full queue: non-blocking design, caller falls back to * direct I/O */ if (logger.count >= LOG_QSIZE) { logger.dropped++; - mo_mutex_unlock(&logger.lock); + CRITICAL_LEAVE(); return ERR_TASK_BUSY; } @@ -127,40 +128,44 @@ int32_t mo_logger_enqueue(const char *msg, uint16_t length) logger.head = (logger.head + 1) % LOG_QSIZE; logger.count++; - mo_mutex_unlock(&logger.lock); + CRITICAL_LEAVE(); return ERR_OK; } -/* Diagnostic: monitor queue depth to detect sustained overflow conditions */ +/* Diagnostic: monitor queue depth to detect sustained overflow conditions. + * [ISR-SAFE] Read-only access to volatile counter with short critical section. + */ uint32_t mo_logger_queue_depth(void) { if (!logger.initialized) return 0; - mo_mutex_lock(&logger.lock); + CRITICAL_ENTER(); uint32_t depth = logger.count; - mo_mutex_unlock(&logger.lock); + CRITICAL_LEAVE(); return depth; } -/* Diagnostic: total messages lost since init (non-resettable counter) */ +/* Diagnostic: total messages lost since init (non-resettable counter). + * [ISR-SAFE] Read-only access to counter with short critical section. + */ uint32_t mo_logger_dropped_count(void) { if (!logger.initialized) return 0; - mo_mutex_lock(&logger.lock); + CRITICAL_ENTER(); uint32_t dropped = logger.dropped; - mo_mutex_unlock(&logger.lock); + CRITICAL_LEAVE(); return dropped; } /* Check if logger is in direct output mode. * Lock-free read: safe because direct_mode is only set atomically by flush - * and cleared by async_resume, both under mutex protection. Reading a stale + * and cleared by async_resume, both under critical section. Reading a stale * value is benign (worst case: one extra direct output or one queued message). */ bool mo_logger_direct_mode(void) @@ -183,17 +188,17 @@ void mo_logger_flush(void) while (1) { bool have_message = false; - mo_mutex_lock(&logger.lock); + CRITICAL_ENTER(); if (logger.count > 0) { memcpy(&entry, &logger.queue[logger.tail], sizeof(log_entry_t)); logger.tail = (logger.tail + 1) % LOG_QSIZE; logger.count--; have_message = true; } else { - /* Queue drained: enter direct mode while still holding lock */ + /* Queue drained: enter direct mode under CRITICAL */ logger.direct_mode = true; } - mo_mutex_unlock(&logger.lock); + CRITICAL_LEAVE(); if (!have_message) break; @@ -212,7 +217,7 @@ void mo_logger_async_resume(void) if (!logger.initialized) return; - mo_mutex_lock(&logger.lock); + CRITICAL_ENTER(); logger.direct_mode = false; - mo_mutex_unlock(&logger.lock); + CRITICAL_LEAVE(); } diff --git a/kernel/task.c b/kernel/task.c index 0bb5fb66..086a843a 100644 --- a/kernel/task.c +++ b/kernel/task.c @@ -32,10 +32,33 @@ kcb_t *kcb = &kernel_state; /* Flag to track if scheduler has started - prevents timer IRQ during early * init. NOSCHED_LEAVE checks this to avoid enabling timer before scheduler is * ready. + * + * FIXME(SMP): Global flag assumes single-core. For SMP, use per-CPU scheduler + * state or atomic operations with memory barriers. */ volatile bool scheduler_started = false; -/* timer work management for reduced latency */ +/* Critical section nesting support for ISR-safe CRITICAL_ENTER/LEAVE. + * Tracks nesting depth and saves the original interrupt state so that + * CRITICAL_LEAVE restores rather than unconditionally enables interrupts. + * + * FIXME(SMP): These variables are global, assuming single-core execution. + * For SMP support, move to per-CPU storage (e.g., CPU-local structure indexed + * by hart ID) or use atomic operations with spinlocks. Current assumptions: + * - Only one execution context runs at a time (task or ISR) + * - ISRs preempt tasks but ISRs do not nest (MIE stays cleared throughout + * ISR execution per RISC-V trap entry behavior) + * - These globals are only used in preemptive mode; in cooperative mode + * the CRITICAL macros are no-ops since tasks yield voluntarily + */ +volatile uint32_t critical_nesting = 0; +volatile int32_t critical_saved_mie = 0; + +/* Timer work management for reduced latency. + * + * FIXME(SMP): Global timer work state assumes single-core. For SMP, use per-CPU + * work queues or atomic operations to avoid cross-CPU races. + */ static volatile uint32_t timer_work_pending = 0; /* timer work types */ static volatile uint32_t timer_work_generation = 0; /* counter for coalescing */ diff --git a/lib/malloc.c b/lib/malloc.c index 3e9a7494..087cd4cb 100644 --- a/lib/malloc.c +++ b/lib/malloc.c @@ -17,6 +17,10 @@ * * This implementation prioritizes fast allocation/deallocation with proper * fragmentation management to minimize memory waste. + * + * FIXME(SMP): Global heap state assumes single-core execution. For SMP, either + * use per-CPU heaps to avoid contention, or replace CRITICAL_ENTER/LEAVE with + * spinlocks that provide cross-CPU mutual exclusion. */ typedef struct __memblock { @@ -190,6 +194,7 @@ void *malloc(uint32_t size) MARK_USED(p); if (unlikely(free_blocks_count <= 0)) { + CRITICAL_LEAVE(); panic(ERR_HEAP_CORRUPT); return NULL; } @@ -278,6 +283,8 @@ void *realloc(void *ptr, uint32_t size) old_size - size < sizeof(memblock_t) + MALLOC_MIN_SIZE) return ptr; + CRITICAL_ENTER(); + /* fast path for shrinking */ if (size <= old_size) { split_block(old_block, size); @@ -304,12 +311,17 @@ void *realloc(void *ptr, uint32_t size) return (void *) (old_block + 1); } - + /* Allocate new buffer while still holding critical section. + * This prevents race where another task/ISR frees ptr before we copy. + * malloc/free use nested CRITICAL_ENTER internally - safe due to nesting. + */ void *new_buf = malloc(size); if (new_buf) { memcpy(new_buf, ptr, min(old_size, size)); free(ptr); } + CRITICAL_LEAVE(); + return new_buf; }