From 67df7311f44390807a66b256ac3be0661275a9f3 Mon Sep 17 00:00:00 2001 From: Cemil ILIK Date: Tue, 21 Apr 2026 10:12:03 +0300 Subject: [PATCH 01/10] =?UTF-8?q?docs(roadmap):=20T-003=20=E2=86=92=20Done?= =?UTF-8?q?;=20open=20T-004=20=E2=80=94=20cooperative=20scheduler=20(A5)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - T-003 marked Done; PR merged to main 2026-04-21. - A4 milestone closed; A5 (cooperative scheduler) opened. - T-004 task file created: cooperative yield-based scheduler with context switch, IPC blocking integration, and QEMU smoke test. ADR-0019 (scheduler shape) and ADR-0020 (Cpu trait v2) required before implementation begins. - current.md, phase-a.md, and tasks README updated accordingly. Co-Authored-By: Claude Sonnet 4.6 --- docs/analysis/tasks/phase-a/README.md | 3 +- .../tasks/phase-a/T-003-ipc-primitives.md | 3 +- .../phase-a/T-004-cooperative-scheduler.md | 93 +++++++++++++++++++ docs/roadmap/current.md | 18 ++-- docs/roadmap/phases/phase-a.md | 6 +- 5 files changed, 111 insertions(+), 12 deletions(-) create mode 100644 docs/analysis/tasks/phase-a/T-004-cooperative-scheduler.md diff --git a/docs/analysis/tasks/phase-a/README.md b/docs/analysis/tasks/phase-a/README.md index 0b1a78e..268f36e 100644 --- a/docs/analysis/tasks/phase-a/README.md +++ b/docs/analysis/tasks/phase-a/README.md @@ -8,6 +8,7 @@ Tasks belonging to [Phase A — Kernel core on QEMU `virt`](../../../roadmap/pha |----|-------|-----------|--------| | [T-001](T-001-capability-table-foundation.md) | Capability table foundation | A2 | Done | | [T-002](T-002-kernel-object-storage.md) | Kernel object storage foundation | A3 | Done | -| [T-003](T-003-ipc-primitives.md) | IPC primitives | A4 | In Review | +| [T-003](T-003-ipc-primitives.md) | IPC primitives | A4 | Done | +| [T-004](T-004-cooperative-scheduler.md) | Cooperative scheduler | A5 | Draft | Tasks are added here as they become active. See [`../../../roadmap/phases/phase-a.md`](../../../roadmap/phases/phase-a.md) for the full phase plan. diff --git a/docs/analysis/tasks/phase-a/T-003-ipc-primitives.md b/docs/analysis/tasks/phase-a/T-003-ipc-primitives.md index 0985ae9..0610895 100644 --- a/docs/analysis/tasks/phase-a/T-003-ipc-primitives.md +++ b/docs/analysis/tasks/phase-a/T-003-ipc-primitives.md @@ -2,7 +2,7 @@ - **Phase:** A - **Milestone:** A4 — IPC primitives -- **Status:** In Review +- **Status:** Done - **Created:** 2026-04-21 - **Author:** @cemililik - **Dependencies:** T-002 — Kernel object storage foundation (Done) @@ -94,3 +94,4 @@ Design is pinned in ADR-0017. At a sketch level: | 2026-04-21 | @cemililik | ADR-0017 Accepted; status → Ready. Implementation may begin. | | 2026-04-21 | @cemililik | status → In Progress; implementation begins on `development`. | | 2026-04-21 | @cemililik | status → In Review; 55/55 tests pass, all clippy clean. | +| 2026-04-21 | @cemililik | PR merged to main; status → Done. | diff --git a/docs/analysis/tasks/phase-a/T-004-cooperative-scheduler.md b/docs/analysis/tasks/phase-a/T-004-cooperative-scheduler.md new file mode 100644 index 0000000..8d47e04 --- /dev/null +++ b/docs/analysis/tasks/phase-a/T-004-cooperative-scheduler.md @@ -0,0 +1,93 @@ +# T-004 — Cooperative scheduler + +- **Phase:** A +- **Milestone:** A5 — Cooperative scheduler and context switch +- **Status:** Draft +- **Created:** 2026-04-21 +- **Author:** @cemililik +- **Dependencies:** T-003 — IPC primitives (Done) +- **Informs:** T-005 (two-task IPC demo — A6) +- **ADRs required:** ADR-0019 (Scheduler shape) — must be Accepted before implementation; ADR-0020 (`Cpu` trait v2 / context-switch extension) — must be Accepted before assembly or context-switch code lands. + +--- + +## User story + +As the Umbrix kernel, I want a cooperative, yield-based scheduler that can switch register context between two kernel-level task stubs and can unblock a task that was waiting on IPC — so that the A6 two-task IPC demo can run both tasks on a single CPU core with observable console output from inside QEMU. + +## Context + +T-003 gave us `ipc_send` / `ipc_recv` / `ipc_notify` with correct waiter-state management. What the A4 IPC layer cannot do is actually suspend a calling task and resume it when the other side arrives: "blocking" in A4 is a state recorded in `IpcQueues`; the caller's execution still continues. A5 wires the scheduler so that blocking means the task is removed from the ready queue, the CPU switches to another ready task, and the blocked task is re-enqueued when its condition is satisfied. + +The scheduler itself is deliberately simple: cooperative yield (no preemption, no timer tick in A5). Two design decisions drive the shape: + +- **ADR-0019** settles the scheduler's data structure (queue type, yield semantics, blocked-task representation). +- **ADR-0020** extends the [`umbrix-hal::Cpu`](../../../hal/src/cpu.rs) trait with `save_context` / `restore_context` and a `TaskContext` associated type, so the context-switch assembly lives in the BSP rather than the kernel crate. + +The actual assembly for saving and restoring aarch64 register state (callee-saved registers, SP, LR/PC) lives in `bsp-qemu-virt` behind a safe Rust wrapper with a documented `# Safety` contract. The kernel crate calls the HAL trait and stays `unsafe`-free. + +**Scope constraint.** A5 has no timer: preemption, time-sliced round-robin, and real-time guarantees are Phase B work. The QEMU smoke test is deliberately minimal — two tasks that yield back and forth and print to the console, proving context switch works. + +## Acceptance criteria + +- [ ] **ADR-0019 Accepted** before any scheduler implementation lands. Settles: queue structure (single FIFO vs. priority queues), yield semantics, blocked-task lifecycle. +- [ ] **ADR-0020 Accepted** before any context-switch code lands. Settles: `Cpu` trait v2 shape (`save_context`, `restore_context`, `TaskContext` associated type), safety contract. +- [ ] **`Cpu` trait v2** lands in `umbrix-hal`; the BSP `QemuVirtCpu` implements it. +- [ ] **Context-switch assembly** in `bsp-qemu-virt`, behind a safe Rust wrapper; `unsafe` block audited per [`unsafe-policy.md`](../../../standards/unsafe-policy.md). +- [ ] **Scheduler queue** in `kernel::sched`: bounded, heap-free. Shape decided by ADR-0019. +- [ ] **`yield_now` kernel operation**: moves the current task to the back of the ready queue and switches to the head. +- [ ] **IPC integration**: when `ipc_recv` finds no sender (returns `RecvOutcome::Pending`), the scheduler removes the calling task from the ready queue and parks it. When `ipc_send` delivers to a waiting receiver (returns `SendOutcome::Delivered`), the scheduler re-enqueues the receiver. +- [ ] **Host tests** for scheduler data structures (enqueue, dequeue, block, unblock). +- [ ] **QEMU smoke test**: two kernel-level tasks yield back and forth; console shows alternating output from each task. +- [ ] **No new `unsafe`** beyond the context-switch wrapper. If any additional `unsafe` lands, audit entry per [`unsafe-policy.md`](../../../standards/unsafe-policy.md). + +## Out of scope + +- Timer interrupts and preemption — Phase B. +- Priority scheduling beyond what ADR-0019 decides for v1 (single FIFO is acceptable). +- SMP / multi-core — deferred indefinitely. +- Userspace task creation and switching — Phase B or later. +- `reply_recv` fastpath integration — deferred to when A6 reveals a concrete need. + +## Approach + +Design is delegated to ADR-0019 and ADR-0020. At a sketch level: + +1. **Queue.** A `SchedQueue` in `kernel::sched` — bounded, heap-free, holding task handles. ADR-0019 picks N and whether per-priority buckets are needed for v1. +2. **`TaskContext`.** An associated type on `Cpu` (ADR-0020) holding callee-saved registers + SP + PC/LR. The BSP implements the concrete type and the save/restore assembly. +3. **`yield_now(scheduler, cpu)`.** Saves the current task's context, moves the task handle to the back of the ready queue, pops the head, restores its context. +4. **IPC bridge.** After `ipc_recv` returns `Pending`, the caller's scheduler-layer wrapper parks the task (removes from ready queue, records it as waiting on the endpoint). After `ipc_send` returns `Delivered`, the scheduler-layer wrapper unparks the previously waiting receiver. +5. **QEMU smoke.** Two tasks created in `kernel_main`; each prints its ID and calls `yield_now`; loop runs until both have printed N times. + +## Definition of done + +- [ ] `cargo fmt --all -- --check` clean. +- [ ] `cargo host-clippy` clean. +- [ ] `cargo kernel-clippy` clean. +- [ ] `cargo host-test` passes with new scheduler unit tests. +- [ ] QEMU smoke test runs and prints alternating task output (manual check or CI run). +- [ ] `unsafe` in context-switch wrapper has a `# Safety` section and an audit entry. +- [ ] Commit(s) follow [`commit-style.md`](../../../standards/commit-style.md). +- [ ] [`current.md`](../../../roadmap/current.md) updated on each status transition. + +## Design notes + +- **Why cooperative-only?** Preemption requires a timer IRQ and safe IRQ entry/exit, which pulls in interrupt handling before the scheduler is even proven. Starting cooperative keeps the first context switch auditable and testable without hardware interrupt complexity. +- **Why `Cpu` trait extension rather than a separate trait?** The context-switch primitive is a fundamental CPU operation, like `write_bytes` on `Console`. Extending `Cpu` keeps the HAL surface minimal and avoids a proliferation of single-method traits. ADR-0020 may decide otherwise if the extension is large or awkward. +- **Safety of context switch.** The save/restore assembly is the first `unsafe` in the kernel that is not structurally impossible to make safe. The invariants (stack pointer valid, registers stable, interrupts disabled during switch) must be stated explicitly and checked in review. +- **IPC bridge complexity.** Parking a task on `RecvOutcome::Pending` requires knowing which task is the caller — in A5, "task" is a kernel-level stub with an ID and a `TaskContext`; the scheduler maps task ID to ready/blocked state. This is the first time the kernel has a concept of "current task." + +## References + +- [ADR-0017: IPC primitive set](../../../decisions/0017-ipc-primitive-set.md) — Accepted; A5 wires its blocking semantics to the scheduler. +- [ADR-0019: Scheduler shape](../../../decisions/0019-scheduler-shape.md) *(to be written before implementation)*. +- [ADR-0020: Cpu trait v2 / context-switch extension](../../../decisions/0020-cpu-trait-v2.md) *(to be written before context-switch code lands)*. +- [Phase A plan](../../../roadmap/phases/phase-a.md) — A5 sub-breakdown and acceptance criteria. +- [T-003](T-003-ipc-primitives.md) — delivers the IPC waiter states this task wires to the scheduler. +- seL4 scheduler model — priority-based, cooperative within a priority band (prior art; full model deferred). + +## Review history + +| Date | Reviewer | Note | +|------|----------|------| +| 2026-04-21 | @cemililik | opened; status Draft — ADR-0019 and ADR-0020 not yet written; A5 blocked until both Accepted. | diff --git a/docs/roadmap/current.md b/docs/roadmap/current.md index f07c7d0..74822a7 100644 --- a/docs/roadmap/current.md +++ b/docs/roadmap/current.md @@ -5,18 +5,18 @@ A short pointer file updated as work progresses. For the full plan see [`phases/ --- - **Active phase:** A — Kernel core on QEMU `virt`. -- **Active milestone:** A4 — IPC primitives. -- **Active task:** [T-003 — IPC primitives](../analysis/tasks/phase-a/T-003-ipc-primitives.md) (status: **In Review**). +- **Active milestone:** A5 — Cooperative scheduler and context switch. +- **Active task:** [T-004 — Cooperative scheduler](../analysis/tasks/phase-a/T-004-cooperative-scheduler.md) (status: **Draft**). - **Working branch:** `development`. -- **Last completed milestone:** A3 — Kernel objects, on 2026-04-21 (PR merged to `main`). -- **Last completed task:** [T-002 — Kernel object storage foundation](../analysis/tasks/phase-a/T-002-kernel-object-storage.md) — `Done` 2026-04-21. +- **Last completed milestone:** A4 — IPC primitives, on 2026-04-21 (PR merged to `main`). +- **Last completed task:** [T-003 — IPC primitives](../analysis/tasks/phase-a/T-003-ipc-primitives.md) — `Done` 2026-04-21. - **Last review:** [A2 completion business review](../analysis/reviews/business-reviews/2026-04-21-A2-completion.md) — 2026-04-21. -- **Next review trigger:** PR merge of T-003 to `main` (code + security review currently in progress); A4 business review waits for A6 per [phase-a.md closure](phases/phase-a.md). +- **Next review trigger:** code + security review of T-004 when it reaches `In Review`; A4/A5 business review waits for A6 per [phase-a.md closure](phases/phase-a.md). ## Notes -- The capability subsystem (T-001), kernel-object subsystem (T-002), and IPC-primitive subsystem (T-003) form the Phase A stack. T-001 and T-002 both shipped with zero `unsafe` and no heap. Neither subsystem is wired into `run` yet; that is Phase-A later-milestone work. -- [ADR-0014](../decisions/0014-capability-representation.md) and [ADR-0016](../decisions/0016-kernel-object-storage.md) both Accepted. -- T-002 introduced `obj::{Arena, Task, Endpoint, Notification}` with typed handles and rewired `CapObject` to a typed enum paralleling `CapKind`. `Capability::new` lost its redundant `kind` parameter (kind is now derived from the object's variant). 44 host tests green (kernel crate). -- ADR-0017 Accepted: `send` + `recv` + `notify`, fixed-size 4-word `Message`, ≤ 1 cap per message, `reply_recv` and badge scheme deferred to ADR-0018. T-003 implementation complete; 55/55 tests pass; status → In Review. +- The capability subsystem (T-001), kernel-object subsystem (T-002), and IPC-primitive subsystem (T-003) form the Phase A stack. All three shipped with zero `unsafe` and no heap. None is wired into `run` yet; that is Phase-A later-milestone work. +- [ADR-0014](../decisions/0014-capability-representation.md), [ADR-0016](../decisions/0016-kernel-object-storage.md), and [ADR-0017](../decisions/0017-ipc-primitive-set.md) all Accepted. +- T-003 (A4) delivered `ipc_send` / `ipc_recv` / `ipc_notify` with generation-tracked `IpcQueues`, atomic capability transfer, and TRANSFER-right enforcement. 64 host tests green; status → Done 2026-04-21. +- A5 (T-004) opens: cooperative scheduler with context switch. Requires ADR-0019 (scheduler shape) and ADR-0020 (`Cpu` trait v2 / context-switch extension). - The maintainer updates this file when the active task changes. AI agents update it when they move a task to `In Progress` or `Done` via the [`start-task`](../../.claude/skills/start-task/SKILL.md) and [`conduct-review`](../../.claude/skills/conduct-review/SKILL.md) skills. diff --git a/docs/roadmap/phases/phase-a.md b/docs/roadmap/phases/phase-a.md index 42bef2b..8c7a8dd 100644 --- a/docs/roadmap/phases/phase-a.md +++ b/docs/roadmap/phases/phase-a.md @@ -118,7 +118,7 @@ Synchronous rendezvous endpoints and asynchronous notifications. Capability tran ### Tasks under A4 -- [T-003 — IPC primitives](../../analysis/tasks/phase-a/T-003-ipc-primitives.md) — In Review. +- [T-003 — IPC primitives](../../analysis/tasks/phase-a/T-003-ipc-primitives.md) — Done. ### Informs @@ -148,6 +148,10 @@ The first real scheduler: cooperative yield-based, with a context-switch primiti - Two kernel-level tasks yield back and forth; this is observable via console output from inside QEMU. - `unsafe` around the context switch is audited; the safe wrapper's invariants are stated in its `# Safety` doc. +### Tasks under A5 + +- [T-004 — Cooperative scheduler](../../analysis/tasks/phase-a/T-004-cooperative-scheduler.md) — Draft. + ### Informs Milestone A6 integrates IPC + scheduling to demonstrate Phase A end-to-end. From 2fd990cc3cff2c59da105b40345bb1529bec58ad Mon Sep 17 00:00:00 2001 From: Cemil ILIK Date: Tue, 21 Apr 2026 10:14:50 +0300 Subject: [PATCH 02/10] =?UTF-8?q?docs(adr):=20propose=20ADR-0019=20?= =?UTF-8?q?=E2=80=94=20scheduler=20shape?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Settles four inter-related design axes for Milestone A5: - Queue structure: single bounded FIFO (SchedQueue, N = TASK_ARENA_CAPACITY). - Yield semantics: yield to next ready task (FIFO order); no targeted yield in v1. - Blocked-task state: per-task TaskState enum { Idle, Ready, Blocked { on: EndpointHandle } } in kernel::sched; O(N) unblock scan accepted at A5 scale (N ≤ 16). - IPC bridge: scheduler is the orchestration layer; kernel::ipc remains free of scheduling concerns and its 64 tests are unaffected. Open questions: yield_to(target) for reply_recv (deferred to ADR-0018), initial TaskContext setup (ADR-0020), idle task for double-block deadlock (Phase B). Co-Authored-By: Claude Sonnet 4.6 --- docs/decisions/0019-scheduler-shape.md | 218 +++++++++++++++++++++++++ docs/decisions/README.md | 1 + 2 files changed, 219 insertions(+) create mode 100644 docs/decisions/0019-scheduler-shape.md diff --git a/docs/decisions/0019-scheduler-shape.md b/docs/decisions/0019-scheduler-shape.md new file mode 100644 index 0000000..3cc3d02 --- /dev/null +++ b/docs/decisions/0019-scheduler-shape.md @@ -0,0 +1,218 @@ +# 0019 — Scheduler shape + +- **Status:** Proposed +- **Date:** 2026-04-21 +- **Deciders:** @cemililik + +## Context + +[T-004](../analysis/tasks/phase-a/T-004-cooperative-scheduler.md) opens Milestone A5: the first real scheduler in Umbrix. The IPC layer (T-003, [ADR-0017](0017-ipc-primitive-set.md)) records waiter state in `IpcQueues` but has no mechanism to suspend a calling task or resume it when the other side arrives. A5 wires that mechanism. + +Before any implementation lands, four inter-related questions must be settled: + +1. **Queue structure.** What data structure holds the set of tasks that are ready to run? +2. **Yield semantics.** What does "yield" mean — give up the CPU to anyone, or to a specific task? +3. **Blocked-task representation.** How is a task that is waiting on IPC recorded, and how is it unblocked? +4. **IPC bridge ownership.** Which layer is responsible for calling `ipc_send`/`ipc_recv` and reacting to `SendOutcome::Delivered` / `RecvOutcome::Pending`? + +**Constraints inherited from earlier ADRs.** + +- All kernel state is statically bounded — no heap ([ADR-0016](0016-kernel-object-storage.md)). +- The kernel crate is `no_std`, HAL-dependent only through traits ([ADR-0006](0006-workspace-layout.md), [ADR-0008](0008-cpu-trait.md)). +- IPC is cooperative in A4/A5; preemption is Phase B ([ADR-0017](0017-ipc-primitive-set.md)). +- Context-switch assembly lives in the BSP, exposed through the `Cpu` trait (ADR-0020); the scheduler calls the trait, not the assembly. +- Only one CPU core is in scope (single-core aarch64 QEMU `virt`). + +**A5 scale.** Two kernel-level task stubs, cooperative yield, no timer tick. Complexity should match scale: a design that works perfectly for 2 tasks and is straightforward to extend later is better than a general scheduler written before there is a workload to validate it against. + +## Decision drivers + +- **Simplest correct first.** Single-core, cooperative, two tasks — the scheduler should be the least complex structure that satisfies the A5 and A6 acceptance criteria. +- **Bounded and heap-free.** The ready queue must have a compile-time capacity, just like `CapabilityTable` and the kernel-object arenas. +- **No circular module dependency.** `kernel::ipc` imports from `kernel::cap` and `kernel::obj`. A `kernel::sched` that imports from `kernel::ipc` is fine. The reverse (`ipc` importing `sched`) is not — it would create a cycle. +- **Auditable `unsafe` boundary.** The context-switch primitive will carry `unsafe`; the scheduler's data structures and logic must be safe Rust so the `unsafe` surface stays minimal and localised to the BSP wrapper. +- **Host-testable scheduler logic.** Queue operations (enqueue, dequeue, block, unblock) must be exercisable without QEMU, assembly, or a running loop. +- **Forward compatibility.** The chosen shape should not require a rewrite to add priority queues or a third task in A6 or Phase B. + +## Considered options + +### Queue structure + +**Option A — Single bounded FIFO (`SchedQueue`).** +One array-backed ring buffer of `TaskHandle`s with capacity `N = TASK_ARENA_CAPACITY`. `enqueue` appends to the back; `dequeue` pops from the front. All ready tasks are treated equally — no priority. + +- Pro: minimal complexity; consistent with the arena/bounded-table pattern established across the codebase. +- Pro: O(1) enqueue and dequeue; no sorting, no heap. +- Pro: the only failure mode is "queue full" (more than `N` ready tasks simultaneously), which cannot happen when `N` equals the total task capacity. +- Con: no priority differentiation. A latency-sensitive task cannot cut the queue. +- Neutral: adding per-priority queues later is additive (a new ADR supersedes this one); nothing in the FIFO design precludes it. + +**Option B — Per-priority bounded FIFOs.** +Multiple queues, one per priority level; `dequeue` picks the highest non-empty queue. + +- Pro: enables latency differentiation. +- Con: no A5/A6 scenario requires it; adds complexity (priority assignment, multiple queues) before there is a workload that exposes the need. +- Con: priority inversion is a known hazard that requires protocol-level mitigation (priority inheritance, ceiling); introducing priority before those mitigations are in place is premature. + +**Option C — Round-robin ring buffer (no explicit FIFO order).** +Each task gets a fixed slot; `yield_now` advances a cursor. + +- Pro: deterministic per-task slot; no reordering. +- Con: slots are allocated statically regardless of whether a task exists, wasting capacity. +- Con: blocking a task leaves a gap in the ring; handling gaps adds complexity. + +### Yield semantics + +**Option D — Yield to next ready (FIFO order).** +`yield_now` moves the current task to the back of the ready queue and runs the task at the front. The caller does not specify a target. + +- Pro: simple, composable; the scheduler owns the scheduling decision. +- Pro: with a FIFO queue, two tasks alternate deterministically — exactly the A5 smoke-test scenario. +- Con: a server task cannot yield specifically to a known client (needed for `reply_recv` fastpath, which is deferred to ADR-0018). + +**Option E — Yield to specific task handle.** +`yield_to(target: TaskHandle)` moves the current task to the back of the ready queue and specifically moves `target` to the front (or directly runs it). + +- Pro: supports the `reply_recv` pattern without a scheduler round-trip. +- Con: the caller must know the target's `TaskHandle`; this couples IPC and scheduling tightly. +- Con: if `target` is blocked, the call fails or silently becomes a `yield_to_next` — ambiguous semantics. +- Neutral: can be added later as a `yield_to` extension without changing the base `yield_now` semantics. + +**Option F — Yield to highest priority ready.** +`yield_now` runs the highest-priority ready task regardless of order. + +- Con: requires per-priority queues (Option B); excluded while Option A is chosen. + +### Blocked-task representation + +**Option G — Per-task state enum in the scheduler.** +The scheduler owns an array `task_states: [TaskState; TASK_ARENA_CAPACITY]` where `TaskState` is: + +```rust +enum TaskState { + Ready, + Blocked { on: EndpointHandle }, + Idle, // slot not in use +} +``` + +When `ipc_recv` returns `RecvOutcome::Pending`, the scheduler sets the task's state to `Blocked { on: ep_handle }` and removes it from the ready queue. When `ipc_send` returns `SendOutcome::Delivered`, the scheduler finds the task blocked on that endpoint, sets it back to `Ready`, and enqueues it. + +- Pro: all scheduling state in one place; the scheduler is the single source of truth. +- Pro: O(N) unblock scan is acceptable at A5 scale (N ≤ 16). +- Pro: consistent pattern with `IpcQueues` (parallel state array indexed by slot index). +- Con: the blocked-state scan is O(N); a priority-indexed blocked list would be faster but unnecessary at A5 scale. + +**Option H — Blocked tasks stored inside `IpcQueues`.** +`IpcQueues` tracks which `TaskHandle` is waiting at each endpoint alongside the `EndpointState`. + +- Con: creates a `kernel::ipc` → `kernel::obj::task` dependency for `TaskHandle`. `kernel::cap` already imports from `kernel::obj`; adding `TaskHandle` to `ipc` does not create a cycle, but it couples two subsystems that are currently clean of each other. +- Con: the scheduler would need to query `IpcQueues` to learn which task to unblock — mixing IPC state and scheduling state in one structure. + +**Option I — Blocked tasks on a per-endpoint wait list inside `Endpoint`.** +`obj::Endpoint` grows a `waiting_task: Option` field. + +- Con: `obj` would need to import from `cap` (for `TaskHandle`) and potentially `sched`, creating circular dependencies. +- Con: the `Endpoint` struct already deliberately defers waiter state to `IpcQueues` (see ADR-0017 rationale); adding a task handle there reverses that decision. + +### IPC bridge ownership + +**Option J — Scheduler as the IPC orchestration layer.** +`kernel::sched` calls `ipc_send` / `ipc_recv` on behalf of tasks, inspects the outcomes, and performs the ready-queue and state transitions. Tasks do not call `ipc_*` directly; they request IPC through the scheduler. + +- Pro: clean ownership hierarchy — `sched` is the top-level orchestrator; `ipc` is a pure state-machine layer with no scheduling concerns. +- Pro: no circular dependency (`sched` → `ipc` → `cap`, `obj`). +- Pro: `ipc_send` and `ipc_recv` remain pure, side-effect-free with respect to scheduling — easy to test independently. + +**Option K — IPC functions call scheduler callbacks.** +`ipc_send` / `ipc_recv` accept a callback (trait object or function pointer) invoked on `Delivered` / `Pending`. + +- Con: introduces dynamic dispatch or a generic parameter that threads through every IPC call site; adds complexity for no benefit in A5. +- Con: the IPC layer was deliberately designed without scheduling knowledge (ADR-0017); adding a callback parameter couples the two. + +## Decision outcome + +**Chosen:** + +| Axis | Choice | +|------|--------| +| Queue structure | **Option A — Single bounded FIFO** | +| Yield semantics | **Option D — Yield to next ready** | +| Blocked-task state | **Option G — Per-task state enum in scheduler** | +| IPC bridge | **Option J — Scheduler as orchestration layer** | + +**Rationale.** The four choices form a coherent, minimal design: + +- A single FIFO ready queue mirrors the arena/bounded-table pattern already used throughout the codebase. Capacity equals `TASK_ARENA_CAPACITY` (currently 16), so the queue can never be "full" relative to the number of tasks that can exist. +- "Yield to next ready" is the simplest semantics that produces the A5 smoke-test behaviour (two tasks alternating deterministically). `reply_recv`-style targeted yield is deferred to ADR-0018. +- Keeping all scheduler state (ready queue + per-task `TaskState`) in `kernel::sched` avoids polluting `IpcQueues` or `obj::Endpoint` with scheduling concerns. The O(N) unblock scan is negligible at N ≤ 16. +- Making the scheduler the IPC orchestration layer keeps `kernel::ipc` free of scheduling knowledge, preserving its independent testability and the existing module boundary established in ADR-0017. + +### Public API sketch + +```rust +// kernel/src/sched/mod.rs + +pub struct Scheduler { + ready: SchedQueue, + task_states: [TaskState; TASK_ARENA_CAPACITY], + current: Option, +} + +pub enum TaskState { + Idle, // slot not occupied by a live task + Ready, // in the ready queue + Blocked { on: EndpointHandle }, +} + +impl Scheduler { + pub fn add_task(&mut self, handle: TaskHandle); + pub fn yield_now(&mut self, cpu: &mut C, contexts: &mut TaskContexts); + pub fn ipc_send_and_yield( + &mut self, cpu: &mut C, contexts: &mut TaskContexts, + ep_arena: &mut EndpointArena, queues: &mut IpcQueues, + caller_table: &mut CapabilityTable, ep_cap: CapHandle, + msg: Message, transfer: Option, + ) -> Result; + pub fn ipc_recv_and_yield( + &mut self, cpu: &mut C, contexts: &mut TaskContexts, + ep_arena: &mut EndpointArena, queues: &mut IpcQueues, + caller_table: &mut CapabilityTable, ep_cap: CapHandle, + ) -> Result; +} +``` + +`TaskContexts` is a bounded array of `C::TaskContext` values, indexed by raw task slot index — the same parallel-array pattern used by `IpcQueues`. It depends on ADR-0020 for the `C::TaskContext` associated type. + +## Consequences + +**Positive:** + +- The scheduler data structures are safe Rust; only the context-switch call (ADR-0020) crosses the `unsafe` boundary. +- `kernel::ipc` remains testable in isolation with no scheduler dependency — existing 64 tests are unaffected. +- A5's QEMU smoke test (two tasks yielding back and forth) falls directly out of `yield_now` on a two-element FIFO. +- Adding priority queues in a future ADR is additive: `SchedQueue` can be replaced by `[SchedQueue; PRIORITY_LEVELS]` without changing the `yield_now` / `add_task` signatures. + +**Negative:** + +- O(N) unblock scan. With N ≤ 16 this is negligible; at larger N a hash map or endpoint-indexed wait list would be more efficient. Accepted as a known limitation at A5 scale. +- `yield_to(specific_task)` is not supported. The `reply_recv` fastpath (ADR-0017 open question, ADR-0018) requires it; until ADR-0018 is written, server-pattern tasks pay a scheduler round-trip on every reply. + +**Neutral:** + +- The `Scheduler` struct owns `current: Option`. In a real preemptive kernel this would be per-CPU; for single-core A5 a single field is sufficient and straightforward to change when SMP arrives. +- `TaskContexts` is a parallel array to `IpcQueues` — same indexing discipline. This is consistent with the established pattern but means a third parallel array joins the two already present. + +## Open questions + +- **`yield_to(target)` for `reply_recv`.** Deferred to ADR-0018 (badge/reply scheme). If ADR-0018 adds `reply_recv`, it must decide whether to implement it as `yield_to(sender)` or as a scheduler-side fastpath. +- **Task context initialisation.** How is a new task's initial `TaskContext` set up before it runs for the first time? The first `restore_context` must point at a known entry function. The ADR-0020 and the T-004 implementation will specify the initialisation convention; it is not settled here. +- **Idle task.** With two tasks and cooperative scheduling, if both block on IPC simultaneously (a deadlock), the ready queue is empty and `yield_now` has nothing to run. A5 will panic in this case; a real idle task (spin-loop or WFI) is Phase B work. + +## References + +- [ADR-0017: IPC primitive set](0017-ipc-primitive-set.md) — the IPC layer this scheduler wires up. +- [ADR-0008: `Cpu` HAL trait v1](0008-cpu-trait.md) — v2 (ADR-0020) extends this with context-switch primitives. +- [T-004: Cooperative scheduler](../analysis/tasks/phase-a/T-004-cooperative-scheduler.md) — the task this ADR gates. +- seL4 scheduler — strict priority, bounded queues; Phase B reference. +- Hubris task-dispatch model — cooperative, bounded task set; closest prior art to the A5 design. diff --git a/docs/decisions/README.md b/docs/decisions/README.md index 4570f5c..013890b 100644 --- a/docs/decisions/README.md +++ b/docs/decisions/README.md @@ -46,6 +46,7 @@ Each ADR contains: | 0015 | [AI integration stance: userspace-only, kernel-neutral](0015-ai-integration-stance.md) | Accepted | 2026-04-20 | | 0016 | [Kernel object storage](0016-kernel-object-storage.md) | Accepted | 2026-04-21 | | 0017 | [IPC primitive set](0017-ipc-primitive-set.md) | Accepted | 2026-04-21 | +| 0019 | [Scheduler shape](0019-scheduler-shape.md) | Proposed | 2026-04-21 | ## Creating a new ADR From b92f5b3f5e8bc3352ef6ef534d2884d8b3c70a59 Mon Sep 17 00:00:00 2001 From: Cemil ILIK Date: Tue, 21 Apr 2026 10:17:27 +0300 Subject: [PATCH 03/10] =?UTF-8?q?docs(adr):=20record=20ADR-0018=20?= =?UTF-8?q?=E2=80=94=20badge=20scheme=20and=20reply=5Frecv=20deferral?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Formally closes the two open questions that ADR-0017 left to this ADR: - Badge scheme: deferred (Option A). No Phase A/A6 scenario requires kernel-injected badges; user-space emulation via CapHandle keying is available today. Revisit trigger: multiplexed-server scenario or formal security argument requiring kernel-TCB injection. - reply_recv fastpath: deferred (Option D). The operation is semantically undefined without a running scheduler (A5) and unmeasurable without real context switches (A6 timings). Revisit trigger: A5/A6 benchmarks showing the extra context switch dominates server-loop latency. Message::label, Capability, SlotEntry, and all IPC operations are unchanged; both features remain additive for a successor ADR. Co-Authored-By: Claude Sonnet 4.6 --- ...18-badge-scheme-and-reply-recv-deferral.md | 98 +++++++++++++++++++ docs/decisions/README.md | 1 + 2 files changed, 99 insertions(+) create mode 100644 docs/decisions/0018-badge-scheme-and-reply-recv-deferral.md diff --git a/docs/decisions/0018-badge-scheme-and-reply-recv-deferral.md b/docs/decisions/0018-badge-scheme-and-reply-recv-deferral.md new file mode 100644 index 0000000..f8cc874 --- /dev/null +++ b/docs/decisions/0018-badge-scheme-and-reply-recv-deferral.md @@ -0,0 +1,98 @@ +# 0018 — Badge scheme and `reply_recv` fastpath: formal deferral + +- **Status:** Accepted +- **Date:** 2026-04-21 +- **Deciders:** @cemililik + +## Context + +[ADR-0017](0017-ipc-primitive-set.md) settled the v1 IPC primitive set (`send`, `recv`, `notify`) and explicitly left two questions open for this ADR: + +1. **Badge scheme.** Should the kernel inject a per-derivation discriminator into `Message::label` automatically? A badge is a small integer stamped onto a capability at derivation time; when the bearer sends a message through that capability, the kernel replaces (or augments) `label` with the badge value. The receiver can therefore distinguish which derived capability a message arrived on — useful for servers that share a single endpoint across many clients. + +2. **`reply_recv` fastpath.** Should the kernel expose a combined "reply-to-current-client + wait-for-next-client" operation (`reply_recv`)? This eliminates one context switch per server-pattern round trip by merging the outbound `send` and the next inbound `recv` into a single kernel entry. + +Both features are well-established in the L4 / seL4 lineage. Neither is required by the Phase A or A6 acceptance criteria. ADR-0017 required that this ADR either accept them or record a principled deferral. + +**State of the codebase at decision time.** + +- `Message::label` is a caller-controlled `u64`; the kernel does not inspect or modify it. +- The capability derivation tree tracks `parent` / `first_child` / `next_sibling` indices but stores no badge value. +- `ipc_send` / `ipc_recv` / `ipc_notify` are implemented; the `CapabilityTable` has no "badge" field. +- No scheduler exists yet (A5 is next); `reply_recv` would require a live scheduler to be meaningful. +- The A6 two-task demo involves exactly two tasks, one endpoint, and one direction of reply — no multiplexed server pattern. + +## Decision drivers + +- **No scenario requires either feature in Phase A or A6.** Badge injection is useful when a single endpoint serves multiple clients distinguished by their derivation path; A6 has exactly one client and one server. `reply_recv` optimises away a context switch that does not yet exist. +- **Both features modify the security-critical capability derivation path.** Adding a badge field to `Capability` or `SlotEntry` and making the kernel stamp it on message delivery changes how the kernel propagates authority. Introducing that change without a concrete use case that can validate correct behaviour is a risk that does not pay off in Phase A. +- **`reply_recv` requires scheduler concepts that do not exist until A5.** The operation must atomically reply to the current caller and suspend waiting for the next; this is meaningless without a running scheduler and task identities. The earliest it can be implemented correctly is A5; the earliest it can be *measured* is A6. +- **The `label` field is forward-compatible.** A badge scheme that injects into `label` is additive: current code that writes `label` explicitly continues to work if the scheme is introduced later (the badge would OR into or replace the value — the exact semantics are an ADR-0018-successor question). No ABI break. +- **Deferral preserves design freedom.** A badge scheme for a pure rendezvous kernel may look different from one designed alongside `reply_recv`. Deciding them together — after A6 produces a concrete use case — yields a more coherent design. + +## Considered options + +### Badge scheme + +**Option A — Kernel-injected badge (defer).** Add a `badge: u64` field to `SlotEntry`; set it at `cap_derive` time; stamp it into `Message::label` on `ipc_send`. Defer to a successor ADR once A6 or Phase B produces a server-with-multiple-clients scenario. + +**Option B — User-space badge emulation.** The server stores the badge in its own data structure keyed on `CapHandle`. No kernel change needed. Always available without a new ADR. + +**Option C — Kernel-injected badge now.** Add the badge immediately alongside T-003. Rejected: no test case in Phase A validates correct badge injection; adding a kernel-side mutation of a user-visible message field before it is tested is unsafe practice for a high-assurance kernel. + +### `reply_recv` fastpath + +**Option D — Defer `reply_recv`.** Implement it in a successor ADR once A5 gives us a running scheduler and A6 lets us measure the context-switch cost it would eliminate. + +**Option E — Implement `reply_recv` now (A5).** The operation would be part of T-004 / A5. Rejected: A5 is already complex (context-switch assembly, HAL extension, scheduler data structures); adding a combined IPC-plus-scheduling operation before the base scheduler is proven stable adds risk without a measured motivation. + +**Option F — Replace `send` + `recv` with `reply_recv` as the primary primitive.** Adopted by seL4 as a design choice. Rejected: it couples IPC and scheduling more tightly than the current architecture allows; it would require revisiting ADR-0017's operation set wholesale. + +## Decision outcome + +**Chosen:** +- Badge scheme: **Option A — defer to a successor ADR.** +- `reply_recv` fastpath: **Option D — defer to a successor ADR.** + +**Rationale.** + +Both features share the same deferral logic: no Phase A or A6 scenario requires them, both touch security-critical code paths that should be introduced with a concrete test case, and both are genuinely additive — the existing `Message` struct, `CapabilityTable`, and IPC operations are forward-compatible with either feature landing later. + +User-space badge emulation (Option B) is available today at zero kernel cost and is sufficient for the A6 demo. If the A6 demo reveals that the server pattern genuinely needs kernel-injected badges, the evidence will be in the code and can motivate a focused successor ADR with a real test. + +`reply_recv` is explicitly gated on A5 being complete: the operation is nonsensical without a scheduler and unmeasurable without real context switches. The right time to write its ADR is after A5/A6 produce timings. + +### Revisit triggers + +A successor ADR superseding this one should be written when **any one** of the following is true: + +1. A6 or Phase B introduces a server that multiplexes a single endpoint across more than one client, and distinguishing clients at the receiver side requires kernel-level badge injection (rather than user-space keying on `CapHandle`). +2. A5/A6 QEMU measurements show that the extra context switch in the server loop is a meaningful bottleneck (e.g., it dominates the round-trip latency in a benchmark that Phase B cares about). +3. A formal security argument for Umbrix requires that badge injection happen inside the kernel TCB rather than in user space. + +## Consequences + +**Positive:** + +- No change to `Capability`, `SlotEntry`, `Message`, or any IPC operation. T-003's implementation is complete and unaffected. +- The deferral is now formally recorded; future contributors will not re-open the question without a concrete trigger. +- User-space badge emulation is available immediately: a server allocates one `CapHandle` per client and stores the client identity in a local table keyed by handle. This pattern works correctly today. + +**Negative:** + +- Multiplexed-server protocols that rely on kernel-injected badges cannot be written until a successor ADR lands. This is not a Phase A or A6 concern. +- The server-pattern round-trip has one extra context switch compared to a `reply_recv` design. This cost is not measurable until A5; it is not paid in Phase A. + +**Neutral:** + +- `Message::label` remains a plain caller-supplied `u64`. A badge scheme that writes into `label` is additive; a badge scheme that adds a separate `badge` field is also additive (a new public field in `Message` with a default of `0` is backwards-compatible). Both options remain open to the successor ADR. +- The `CapabilityTable::cap_derive` signature is unchanged. A badge-aware `cap_derive` would gain a `badge: u64` parameter; that is an additive API change that does not break existing call sites (they can pass `0`). + +## References + +- [ADR-0017: IPC primitive set](0017-ipc-primitive-set.md) — the ADR that opened these questions. +- [T-003: IPC primitives](../analysis/tasks/phase-a/T-003-ipc-primitives.md) — the implemented task; `Message::label` is the badge-compatible field. +- [T-004: Cooperative scheduler](../analysis/tasks/phase-a/T-004-cooperative-scheduler.md) — prerequisite for any `reply_recv` implementation. +- seL4 badge scheme — https://sel4.systems/ — kernel-injected per-endpoint badge; the design this ADR defers borrowing from. +- seL4 `seL4_ReplyRecv` — combined reply + receive fastpath; the operation Option D defers. +- Hubris IPC — no badge concept; server distinguishes callers by task identity rather than capability derivation. A simpler model worth revisiting when the use case is clearer. diff --git a/docs/decisions/README.md b/docs/decisions/README.md index 013890b..1bce262 100644 --- a/docs/decisions/README.md +++ b/docs/decisions/README.md @@ -46,6 +46,7 @@ Each ADR contains: | 0015 | [AI integration stance: userspace-only, kernel-neutral](0015-ai-integration-stance.md) | Accepted | 2026-04-20 | | 0016 | [Kernel object storage](0016-kernel-object-storage.md) | Accepted | 2026-04-21 | | 0017 | [IPC primitive set](0017-ipc-primitive-set.md) | Accepted | 2026-04-21 | +| 0018 | [Badge scheme and `reply_recv` fastpath: formal deferral](0018-badge-scheme-and-reply-recv-deferral.md) | Accepted | 2026-04-21 | | 0019 | [Scheduler shape](0019-scheduler-shape.md) | Proposed | 2026-04-21 | ## Creating a new ADR From 178d318b48283fef80d734aaf8bc1688eaf9ba15 Mon Sep 17 00:00:00 2001 From: Cemil ILIK Date: Tue, 21 Apr 2026 10:30:12 +0300 Subject: [PATCH 04/10] =?UTF-8?q?docs(adr):=20accept=20ADR-0019;=20propose?= =?UTF-8?q?=20ADR-0020=20=E2=80=94=20ContextSwitch=20trait=20and=20Cpu=20v?= =?UTF-8?q?2?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ADR-0019 (scheduler shape) is now Accepted: single bounded FIFO, yield-to-next-ready, per-task TaskState enum, scheduler as IPC orchestration layer. ADR-0020 (Proposed): introduces a separate ContextSwitch trait (preserving Cpu object-safety), unsafe context_switch / init_context operations, and a TaskContext associated type. Defines the aarch64 register frame (x19–x28, fp, lr, sp = 104 bytes) and the TestHal no-op for host tests. Both ADRs are prerequisites for T-004 (A5). Co-Authored-By: Claude Sonnet 4.6 --- docs/decisions/0019-scheduler-shape.md | 2 +- .../0020-cpu-trait-v2-context-switch.md | 322 ++++++++++++++++++ docs/decisions/README.md | 3 +- 3 files changed, 325 insertions(+), 2 deletions(-) create mode 100644 docs/decisions/0020-cpu-trait-v2-context-switch.md diff --git a/docs/decisions/0019-scheduler-shape.md b/docs/decisions/0019-scheduler-shape.md index 3cc3d02..fb33d07 100644 --- a/docs/decisions/0019-scheduler-shape.md +++ b/docs/decisions/0019-scheduler-shape.md @@ -1,6 +1,6 @@ # 0019 — Scheduler shape -- **Status:** Proposed +- **Status:** Accepted - **Date:** 2026-04-21 - **Deciders:** @cemililik diff --git a/docs/decisions/0020-cpu-trait-v2-context-switch.md b/docs/decisions/0020-cpu-trait-v2-context-switch.md new file mode 100644 index 0000000..0b1be1f --- /dev/null +++ b/docs/decisions/0020-cpu-trait-v2-context-switch.md @@ -0,0 +1,322 @@ +# 0020 — `Cpu` trait v2: context-switch extension + +- **Status:** Proposed +- **Date:** 2026-04-21 +- **Deciders:** @cemililik + +## Context + +[ADR-0008](0008-cpu-trait.md) introduced the `Cpu` HAL trait and explicitly deferred context-switch primitives with the note: *"They will be pinned down in a dedicated ADR at the start of scheduler work."* This is that ADR. + +[ADR-0019](0019-scheduler-shape.md) settled the scheduler's data structure and described a `TaskContexts` parallel array indexed by raw task slot — meaning the scheduler is generic over some type `C` that provides both the context-switch operation and the concrete `TaskContext` storage type. This ADR decides the shape of that extension. + +**The core tension.** The existing `Cpu` trait is object-safe: the kernel holds `&dyn Cpu` and dispatches through a vtable. Context switch requires an **associated type** (`TaskContext`) to be useful — the scheduler must be able to store and index an array of task contexts, which means knowing the concrete size and layout at compile time. Associated types break `dyn` compatibility. + +Three design paths exist: + +1. Extend `Cpu` with associated type, abandon `dyn Cpu` for the scheduler. +2. Introduce a separate `ContextSwitch` trait with the associated type; `Cpu` remains object-safe and unchanged. +3. Keep `Cpu` object-safe by expressing context save/restore through raw pointers to caller-managed buffers of a fixed architecture-defined size. + +The right choice depends on which consumers need `dyn Cpu` and whether the scheduler specifically can afford to be generic instead. + +**Who uses `dyn Cpu`?** In Phase A, `kernel::run` receives `&C: Console` and calls `cpu.disable_irqs()` for critical sections. The interrupt-mask operations (`disable_irqs`, `restore_irq_state`, `wait_for_interrupt`, `instruction_barrier`) are genuinely object-safe and benefit from dynamic dispatch at the kernel entry point. The scheduler, however, is a static component that knows the BSP at link time — it does not need dynamic dispatch. + +**aarch64 calling convention.** A cooperative context switch only needs to save callee-saved registers: `x19`–`x28`, the frame pointer (`x29 / fp`), the link register (`x30 / lr`), and the stack pointer (`sp`). The compiler already saves caller-saved registers around any ordinary function call; from the compiler's view, `context_switch` is an ordinary function. This means the assembly is minimal: save 13 values (12 registers + sp), restore 13 values, return. + +## Decision drivers + +- **Preserve `Cpu` object-safety.** The interrupt-mask surface is used with `&dyn Cpu` throughout the kernel; widening that trait to include associated types would force generic parameters everywhere `Cpu` is passed, which is an unnecessary churn on stable code. +- **Scheduler is generic, not dynamic.** The scheduler is instantiated once per BSP; it never needs to dispatch across multiple CPU implementations at runtime. Generic bounds (``) are natural and add zero runtime cost. +- **No heap, no allocation.** `TaskContext` must be a plain `Copy`/`Default` struct that can live in a bounded array. No `Box`, no `dyn`, no allocation. +- **Minimal `unsafe` surface.** The context-switch assembly is unavoidably `unsafe`. Everything else must be safe. The trait's methods carry `unsafe` only where the assembly invariants cannot be expressed in Rust's type system. +- **`no_std` compatible.** The trait and its implementations live in HAL and BSP crates that are `#![no_std]`. +- **Auditable.** Each `unsafe impl` block must have a `# Safety` section that states the invariants. See [unsafe-policy.md](../standards/unsafe-policy.md). +- **Testable without QEMU.** The `TestHal` fake can implement `ContextSwitch` with a `TaskContext` that is a simple counter or flag, allowing scheduler logic to be exercised in host tests without real assembly. + +## Considered options + +### Shape of the extension + +**Option A — Separate `ContextSwitch` trait with associated `TaskContext`.** + +```rust +// In umbrix-hal + +pub trait ContextSwitch { + type TaskContext: Default + Send; + + /// Save the current CPU register state into `current` and restore + /// the state from `next`, switching execution to the task that + /// previously saved into `next`. + /// + /// # Safety + /// + /// - `current` must point at storage that remains valid until this + /// task is switched back to and `context_switch` returns. + /// - `next` must contain a valid context previously written by + /// `context_switch` or initialised by `init_context`. + /// - Interrupts must be disabled by the caller for the duration of + /// the switch to prevent a preempting IRQ from observing a + /// partially saved state. + unsafe fn context_switch( + &self, + current: &mut Self::TaskContext, + next: &Self::TaskContext, + ); + + /// Initialise a `TaskContext` so that restoring it begins execution + /// at `entry` with `stack_top` as the initial stack pointer. + /// + /// # Safety + /// + /// `stack_top` must point at the top of a sufficiently-sized, + /// properly aligned stack that remains valid for the task's + /// lifetime. + unsafe fn init_context( + &self, + ctx: &mut Self::TaskContext, + entry: fn() -> !, + stack_top: *mut u8, + ); +} +``` + +`QemuVirtCpu` implements both `Cpu` (unchanged, object-safe) and `ContextSwitch`. The scheduler is generic over `C: ContextSwitch`. `Cpu` gains no new methods. + +- Pro: `Cpu` is fully stable; no existing callsite changes. +- Pro: clear separation of concerns — interrupt management vs. context management. +- Pro: `ContextSwitch` can be implemented by the `TestHal` with a no-op or a trivial fake. +- Con: two separate traits for the same BSP struct; a caller that needs both must bound `C: Cpu + ContextSwitch`. +- Neutral: associated types on a non-`dyn` trait are idiomatic and add no runtime cost. + +**Option B — Extend `Cpu` with `save_context_raw` / `restore_context_raw` using raw pointers and a const size.** + +```rust +pub trait Cpu: Send + Sync { + // … existing methods … + const CONTEXT_SIZE: usize; + unsafe fn save_context_raw(&self, buf: *mut u8); + unsafe fn restore_context_raw(&self, buf: *const u8) -> !; +} +``` + +The scheduler allocates `[[u8; MAX_CONTEXT_SIZE]; N]` and passes raw pointers. `MAX_CONTEXT_SIZE` is a project-wide constant set conservatively large. + +- Pro: `Cpu` stays a single trait; no second trait. +- Con: `const CONTEXT_SIZE` on a trait method breaks `dyn Cpu` (associated constants are not object-safe without `where Self: Sized`). Either abandons object-safety or adds a carve-out. +- Con: raw pointer buffers lose all type safety; the scheduler cannot statically verify it is passing a correctly-typed buffer. +- Con: `MAX_CONTEXT_SIZE` is a global constant that must be correct for every architecture — fragile. + +**Option C — Extend `Cpu` with associated type `TaskContext`; abandon `dyn Cpu` universally.** + +```rust +pub trait Cpu: Send + Sync { + type TaskContext: Default + Send; + // … existing + new methods … +} +``` + +Every consumer of `Cpu` becomes generic: `kernel::run(cpu: &C, …)`, etc. + +- Pro: one unified trait. +- Con: all existing code that takes `&dyn Cpu` must be rewritten to `` — a large, low-value change touching stable, tested code. +- Con: `dyn Cpu` becomes impossible; if the kernel ever needs to swap BSPs at runtime, it loses that option entirely. + +**Option D — Context switch through BSP-owned function pointers rather than a trait.** + +The BSP exposes `static CONTEXT_SWITCH: fn(*mut TaskCtx, *const TaskCtx)`. The scheduler calls the function pointer directly. + +- Pro: no trait complexity. +- Con: not object-oriented; hard to fake for host tests; no type safety; the scheduler must know the BSP's concrete `TaskCtx` type anyway. +- Con: contradicts the HAL-isolation principle established in [ADR-0006](0006-workspace-layout.md). + +### `unsafe` placement + +**Option E — Both `context_switch` and `init_context` are `unsafe fn`.** + +Callers must uphold stack validity and context initialisation invariants. + +**Option F — Wrapper safe API with `unsafe` inside the trait implementation.** + +`context_switch` is `fn` (safe); the trait implementation uses `unsafe` internally. Safety relies on the implementation being correct. + +- Con: a safe `context_switch` API implies the compiler guarantees correctness, but the invariants (valid stack, disabled interrupts) cannot be expressed in Rust's type system. A safe API would be misleading. + +## Decision outcome + +**Chosen: Option A — separate `ContextSwitch` trait with associated `TaskContext`; Option E — `unsafe fn` at the trait boundary.** + +### Trait definition + +```rust +// umbrix-hal/src/cpu.rs (addition; existing Cpu trait unchanged) + +/// Context-switch extension for BSPs that support cooperative task switching. +/// +/// Separate from [`Cpu`] to preserve `Cpu`'s object-safety. The scheduler +/// is generic over `C: ContextSwitch`; it never needs dynamic dispatch. +/// +/// # Safety contract +/// +/// Implementations must ensure that `context_switch` atomically saves +/// all callee-saved registers of the current execution context and +/// restores all callee-saved registers of the next context. On aarch64 +/// that is `x19`–`x28`, `x29` (fp), `x30` (lr), and `sp`. The switch +/// appears to return normally to both the saving call site (when this +/// task is resumed) and the resuming call site returns immediately. +pub trait ContextSwitch { + /// The saved register state for one task. + /// + /// Must be `Default` so the scheduler can zero-initialise a slot + /// before `init_context` fills it in. Must be `Send` so contexts + /// can be moved between (future) CPU cores. + type TaskContext: Default + Send; + + /// Save the calling task's register state into `current` and resume + /// the task whose state was saved in `next`. + /// + /// When this task is later resumed (by another call to + /// `context_switch` with `current` as the `next` argument), + /// execution continues as if `context_switch` returned normally. + /// + /// # Safety + /// + /// - Interrupts must be disabled before this call and remain + /// disabled until the caller re-enables them after the switch. + /// An IRQ firing mid-switch would observe a partially saved state. + /// - `current` must be valid for the entire time this task is + /// suspended; the caller is responsible for keeping the + /// `TaskContexts` array alive. + /// - `next` must contain a context previously written by + /// `context_switch` or fully initialised by `init_context`. + /// Restoring an uninitialised or partially written context is + /// undefined behaviour. + unsafe fn context_switch( + &self, + current: &mut Self::TaskContext, + next: &Self::TaskContext, + ); + + /// Write an initial register state into `ctx` so that the first + /// restore of `ctx` begins executing `entry` with `stack_top` as + /// the stack pointer. + /// + /// # Safety + /// + /// - `stack_top` must point one byte past the top of a + /// sufficiently-sized (≥ 512 bytes recommended for aarch64), + /// 16-byte-aligned stack region that remains valid for the + /// task's entire lifetime. + /// - `entry` must be a `fn() -> !` that never returns; returning + /// from a task entry function is undefined behaviour. + unsafe fn init_context( + &self, + ctx: &mut Self::TaskContext, + entry: fn() -> !, + stack_top: *mut u8, + ); +} +``` + +### aarch64 concrete type (in `bsp-qemu-virt`) + +```rust +// bsp-qemu-virt/src/cpu.rs + +/// Saved callee-register state for one cooperative task on aarch64. +/// +/// Layout must match the offsets used by the `context_switch_asm` +/// routine in `context_switch.s`. `#[repr(C)]` ensures the compiler +/// does not reorder fields. +#[derive(Default)] +#[repr(C)] +pub struct Aarch64TaskContext { + /// x19–x28: callee-saved general-purpose registers. + pub x19_x28: [u64; 10], + /// x29 — frame pointer (callee-saved). + pub fp: u64, + /// x30 — link register (return address; callee-saved in AAPCS64). + pub lr: u64, + /// Stack pointer — saved explicitly (not a general-purpose register). + pub sp: u64, +} +// Total: 13 × 8 = 104 bytes per task context. +``` + +The `context_switch` implementation calls an assembly routine `context_switch_asm(current: *mut Aarch64TaskContext, next: *const Aarch64TaskContext)` via `core::arch::asm!` or a separate `.s` file. The routine: + +1. Saves `x19`–`x28`, `x29`, `x30` into `[current]`. +2. Saves `sp` (via `mov x2, sp`) into `[current + offset_of(sp)]`. +3. Loads `sp`, `x29`, `x30`, `x19`–`x28` from `[next]` in reverse order. +4. Returns via `ret` — which now jumps to the `lr` loaded from `next`. + +For the first run of a task, `init_context` sets `lr` to the entry function address and `sp` to `stack_top`. The first `ret` in the assembly begins executing the entry function. + +### Scheduler integration + +The scheduler becomes `Scheduler`. The `TaskContexts` parallel array (from ADR-0019) is: + +```rust +struct TaskContexts { + contexts: [C::TaskContext; TASK_ARENA_CAPACITY], +} +``` + +`Scheduler::yield_now` calls `cpu.context_switch(&mut contexts[current_idx], &contexts[next_idx])` inside a critical section (interrupts disabled via `cpu.disable_irqs()`). + +### `TestHal` fake + +```rust +// For host-side tests: no assembly, just records which context was last switched to. +#[derive(Default)] +pub struct FakeTaskContext { switched_to: usize } + +pub struct FakeCpu { /* existing fields */ } + +impl ContextSwitch for FakeCpu { + type TaskContext = FakeTaskContext; + unsafe fn context_switch(&self, _current: &mut Self::TaskContext, _next: &Self::TaskContext) { + // No-op in host tests; scheduler logic is tested without real switching. + } + unsafe fn init_context(&self, _ctx: &mut Self::TaskContext, _entry: fn() -> !, _stack_top: *mut u8) {} +} +``` + +## Consequences + +### Positive + +- **`Cpu` is fully unchanged.** All existing callsites, tests, and implementations compile without modification. +- **Scheduler is zero-overhead.** Monomorphisation of `Scheduler` produces direct calls with no vtable indirection. +- **`unsafe` is localised.** The `context_switch_asm` routine and its safe wrapper in the BSP are the only new `unsafe` code. The scheduler's `yield_now` calls `unsafe { cpu.context_switch(…) }` — one `unsafe` block with a documented invariant. +- **Host tests for scheduler logic work without QEMU.** `FakeCpu: ContextSwitch` no-ops the switch; queue management, blocked-task transitions, and IPC bridge logic can all be tested in `cargo test`. +- **Forward-compatible with multi-core.** A future `ContextSwitch` implementation for multi-core simply provides a different `TaskContext` and assembly; the scheduler's generic parameter absorbs the change. + +### Negative + +- **Two bounds needed.** Callers that need both interrupt management and context switching must write `C: Cpu + ContextSwitch`. For `kernel_main` and the scheduler entry, this is one extra trait bound — acceptable. +- **One more `unsafe` audit entry.** The `context_switch_asm` assembly block is new `unsafe` that must be audited per [unsafe-policy.md](../standards/unsafe-policy.md). This is unavoidable for any real context switch. +- **`init_context` initialises `lr` to a `fn() -> !` raw address.** This is safe in practice (function pointers are always valid addresses in Rust) but requires care: the entry function must truly never return, or the `ret` at the end of the assembly falls into garbage. + +### Neutral + +- **`Aarch64TaskContext` is 104 bytes.** With `TASK_ARENA_CAPACITY = 16`, `TaskContexts` occupies 1 664 bytes — well within any reasonable stack or `.bss` section. +- **NEON / FP registers deferred.** The aarch64 AAPCS64 callee-saved NEON registers (`d8`–`d15`) are not saved in v1 because Phase A kernel tasks do not use floating point. A Phase B ADR will add them when userspace tasks run with FP enabled. +- **`sp` alignment.** The aarch64 ABI requires `sp` to be 16-byte-aligned at all `bl` / function call boundaries. `init_context` must receive a `stack_top` that is already 16-byte-aligned; callers that provide odd values trigger undefined behaviour. The `# Safety` doc notes this requirement. + +## Open questions + +- **Interrupt state across switch.** This ADR requires interrupts disabled during `context_switch`. A future ADR may permit switching with interrupts enabled (for preemption), but that changes the assembly invariants substantially; deferred. +- **FP / NEON context save.** Deferred to Phase B as noted above. +- **Per-task kernel stack allocation.** Where do task stacks come from? In A5 they are static arrays allocated at link time (one per task, sized conservatively). A Phase B memory-management ADR will provide dynamic stack allocation. + +## References + +- [ADR-0008: `Cpu` HAL trait signature (v1)](0008-cpu-trait.md) — the existing trait this ADR extends by addition. +- [ADR-0019: Scheduler shape](0019-scheduler-shape.md) — the scheduler that consumes `ContextSwitch`. +- [T-004: Cooperative scheduler](../analysis/tasks/phase-a/T-004-cooperative-scheduler.md) — the task implementing this ADR. +- ARM Architecture Reference Manual, ARMv8-A — §C5 "AAPCS64 procedure call standard": callee-saved register list (`x19`–`x28`, `x29`, `x30`, `sp`). +- Hubris context-switch model — saves/restores callee-saved GPRs cooperatively; closest prior art. +- seL4 `ksSwitchToThread` — saves full register file (preemptive); not adopted in v1. +- Rust `core::arch::asm!` — the mechanism for inline assembly in the BSP implementation. diff --git a/docs/decisions/README.md b/docs/decisions/README.md index 1bce262..ab1afef 100644 --- a/docs/decisions/README.md +++ b/docs/decisions/README.md @@ -47,7 +47,8 @@ Each ADR contains: | 0016 | [Kernel object storage](0016-kernel-object-storage.md) | Accepted | 2026-04-21 | | 0017 | [IPC primitive set](0017-ipc-primitive-set.md) | Accepted | 2026-04-21 | | 0018 | [Badge scheme and `reply_recv` fastpath: formal deferral](0018-badge-scheme-and-reply-recv-deferral.md) | Accepted | 2026-04-21 | -| 0019 | [Scheduler shape](0019-scheduler-shape.md) | Proposed | 2026-04-21 | +| 0019 | [Scheduler shape](0019-scheduler-shape.md) | Accepted | 2026-04-21 | +| 0020 | [`ContextSwitch` trait and `Cpu` v2](0020-cpu-trait-v2-context-switch.md) | Proposed | 2026-04-21 | ## Creating a new ADR From bdf982ee4e8582ac2d54a91377882336c07c4f34 Mon Sep 17 00:00:00 2001 From: Cemil ILIK Date: Tue, 21 Apr 2026 10:35:10 +0300 Subject: [PATCH 05/10] =?UTF-8?q?docs(adr,roadmap):=20accept=20ADR-0020;?= =?UTF-8?q?=20advance=20T-004=20Draft=20=E2=86=92=20Ready?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ADR-0020 (`ContextSwitch` trait and `Cpu` v2) accepted: both ADRs that gate T-004 are now Accepted. T-004 status advances from Draft → Ready; all blocking prerequisites recorded in the task file. Roadmap pointers updated in current.md, phase-a.md, and the task index. Co-Authored-By: Claude Sonnet 4.6 --- docs/analysis/tasks/phase-a/README.md | 2 +- .../tasks/phase-a/T-004-cooperative-scheduler.md | 9 +++++---- docs/decisions/0020-cpu-trait-v2-context-switch.md | 2 +- docs/decisions/README.md | 2 +- docs/roadmap/current.md | 4 ++-- docs/roadmap/phases/phase-a.md | 2 +- 6 files changed, 11 insertions(+), 10 deletions(-) diff --git a/docs/analysis/tasks/phase-a/README.md b/docs/analysis/tasks/phase-a/README.md index 268f36e..61e9306 100644 --- a/docs/analysis/tasks/phase-a/README.md +++ b/docs/analysis/tasks/phase-a/README.md @@ -9,6 +9,6 @@ Tasks belonging to [Phase A — Kernel core on QEMU `virt`](../../../roadmap/pha | [T-001](T-001-capability-table-foundation.md) | Capability table foundation | A2 | Done | | [T-002](T-002-kernel-object-storage.md) | Kernel object storage foundation | A3 | Done | | [T-003](T-003-ipc-primitives.md) | IPC primitives | A4 | Done | -| [T-004](T-004-cooperative-scheduler.md) | Cooperative scheduler | A5 | Draft | +| [T-004](T-004-cooperative-scheduler.md) | Cooperative scheduler | A5 | Ready | Tasks are added here as they become active. See [`../../../roadmap/phases/phase-a.md`](../../../roadmap/phases/phase-a.md) for the full phase plan. diff --git a/docs/analysis/tasks/phase-a/T-004-cooperative-scheduler.md b/docs/analysis/tasks/phase-a/T-004-cooperative-scheduler.md index 8d47e04..299ffaa 100644 --- a/docs/analysis/tasks/phase-a/T-004-cooperative-scheduler.md +++ b/docs/analysis/tasks/phase-a/T-004-cooperative-scheduler.md @@ -2,12 +2,12 @@ - **Phase:** A - **Milestone:** A5 — Cooperative scheduler and context switch -- **Status:** Draft +- **Status:** Ready - **Created:** 2026-04-21 - **Author:** @cemililik - **Dependencies:** T-003 — IPC primitives (Done) - **Informs:** T-005 (two-task IPC demo — A6) -- **ADRs required:** ADR-0019 (Scheduler shape) — must be Accepted before implementation; ADR-0020 (`Cpu` trait v2 / context-switch extension) — must be Accepted before assembly or context-switch code lands. +- **ADRs required:** ADR-0019 (Scheduler shape) — Accepted 2026-04-21; ADR-0020 (`Cpu` trait v2 / context-switch extension) — Accepted 2026-04-21. --- @@ -30,8 +30,8 @@ The actual assembly for saving and restoring aarch64 register state (callee-save ## Acceptance criteria -- [ ] **ADR-0019 Accepted** before any scheduler implementation lands. Settles: queue structure (single FIFO vs. priority queues), yield semantics, blocked-task lifecycle. -- [ ] **ADR-0020 Accepted** before any context-switch code lands. Settles: `Cpu` trait v2 shape (`save_context`, `restore_context`, `TaskContext` associated type), safety contract. +- [x] **ADR-0019 Accepted** — 2026-04-21. Settles: single bounded FIFO queue, yield-to-next-ready, `TaskState { Idle, Ready, Blocked }`, scheduler as IPC orchestration layer. +- [x] **ADR-0020 Accepted** — 2026-04-21. Settles: separate `ContextSwitch` trait, `unsafe context_switch` / `init_context`, aarch64 frame (x19–x28 + fp + lr + sp = 104 bytes). - [ ] **`Cpu` trait v2** lands in `umbrix-hal`; the BSP `QemuVirtCpu` implements it. - [ ] **Context-switch assembly** in `bsp-qemu-virt`, behind a safe Rust wrapper; `unsafe` block audited per [`unsafe-policy.md`](../../../standards/unsafe-policy.md). - [ ] **Scheduler queue** in `kernel::sched`: bounded, heap-free. Shape decided by ADR-0019. @@ -91,3 +91,4 @@ Design is delegated to ADR-0019 and ADR-0020. At a sketch level: | Date | Reviewer | Note | |------|----------|------| | 2026-04-21 | @cemililik | opened; status Draft — ADR-0019 and ADR-0020 not yet written; A5 blocked until both Accepted. | +| 2026-04-21 | @cemililik | ADR-0019 and ADR-0020 both Accepted; status → Ready. Implementation may begin. | diff --git a/docs/decisions/0020-cpu-trait-v2-context-switch.md b/docs/decisions/0020-cpu-trait-v2-context-switch.md index 0b1be1f..5cf9845 100644 --- a/docs/decisions/0020-cpu-trait-v2-context-switch.md +++ b/docs/decisions/0020-cpu-trait-v2-context-switch.md @@ -1,6 +1,6 @@ # 0020 — `Cpu` trait v2: context-switch extension -- **Status:** Proposed +- **Status:** Accepted - **Date:** 2026-04-21 - **Deciders:** @cemililik diff --git a/docs/decisions/README.md b/docs/decisions/README.md index ab1afef..558f619 100644 --- a/docs/decisions/README.md +++ b/docs/decisions/README.md @@ -48,7 +48,7 @@ Each ADR contains: | 0017 | [IPC primitive set](0017-ipc-primitive-set.md) | Accepted | 2026-04-21 | | 0018 | [Badge scheme and `reply_recv` fastpath: formal deferral](0018-badge-scheme-and-reply-recv-deferral.md) | Accepted | 2026-04-21 | | 0019 | [Scheduler shape](0019-scheduler-shape.md) | Accepted | 2026-04-21 | -| 0020 | [`ContextSwitch` trait and `Cpu` v2](0020-cpu-trait-v2-context-switch.md) | Proposed | 2026-04-21 | +| 0020 | [`ContextSwitch` trait and `Cpu` v2](0020-cpu-trait-v2-context-switch.md) | Accepted | 2026-04-21 | ## Creating a new ADR diff --git a/docs/roadmap/current.md b/docs/roadmap/current.md index 74822a7..4097318 100644 --- a/docs/roadmap/current.md +++ b/docs/roadmap/current.md @@ -6,7 +6,7 @@ A short pointer file updated as work progresses. For the full plan see [`phases/ - **Active phase:** A — Kernel core on QEMU `virt`. - **Active milestone:** A5 — Cooperative scheduler and context switch. -- **Active task:** [T-004 — Cooperative scheduler](../analysis/tasks/phase-a/T-004-cooperative-scheduler.md) (status: **Draft**). +- **Active task:** [T-004 — Cooperative scheduler](../analysis/tasks/phase-a/T-004-cooperative-scheduler.md) (status: **Ready**). - **Working branch:** `development`. - **Last completed milestone:** A4 — IPC primitives, on 2026-04-21 (PR merged to `main`). - **Last completed task:** [T-003 — IPC primitives](../analysis/tasks/phase-a/T-003-ipc-primitives.md) — `Done` 2026-04-21. @@ -18,5 +18,5 @@ A short pointer file updated as work progresses. For the full plan see [`phases/ - The capability subsystem (T-001), kernel-object subsystem (T-002), and IPC-primitive subsystem (T-003) form the Phase A stack. All three shipped with zero `unsafe` and no heap. None is wired into `run` yet; that is Phase-A later-milestone work. - [ADR-0014](../decisions/0014-capability-representation.md), [ADR-0016](../decisions/0016-kernel-object-storage.md), and [ADR-0017](../decisions/0017-ipc-primitive-set.md) all Accepted. - T-003 (A4) delivered `ipc_send` / `ipc_recv` / `ipc_notify` with generation-tracked `IpcQueues`, atomic capability transfer, and TRANSFER-right enforcement. 64 host tests green; status → Done 2026-04-21. -- A5 (T-004) opens: cooperative scheduler with context switch. Requires ADR-0019 (scheduler shape) and ADR-0020 (`Cpu` trait v2 / context-switch extension). +- A5 (T-004) opens: cooperative scheduler with context switch. ADR-0019 (scheduler shape) and ADR-0020 (`Cpu` trait v2 / context-switch extension) both Accepted 2026-04-21. T-004 → Ready. - The maintainer updates this file when the active task changes. AI agents update it when they move a task to `In Progress` or `Done` via the [`start-task`](../../.claude/skills/start-task/SKILL.md) and [`conduct-review`](../../.claude/skills/conduct-review/SKILL.md) skills. diff --git a/docs/roadmap/phases/phase-a.md b/docs/roadmap/phases/phase-a.md index 8c7a8dd..2b73c4d 100644 --- a/docs/roadmap/phases/phase-a.md +++ b/docs/roadmap/phases/phase-a.md @@ -150,7 +150,7 @@ The first real scheduler: cooperative yield-based, with a context-switch primiti ### Tasks under A5 -- [T-004 — Cooperative scheduler](../../analysis/tasks/phase-a/T-004-cooperative-scheduler.md) — Draft. +- [T-004 — Cooperative scheduler](../../analysis/tasks/phase-a/T-004-cooperative-scheduler.md) — Ready. ### Informs From 6180af42b4781017c4ad5ce21fe99d68781511ed Mon Sep 17 00:00:00 2001 From: Cemil ILIK Date: Tue, 21 Apr 2026 11:30:09 +0300 Subject: [PATCH 06/10] feat(a5): implement ContextSwitch trait, QemuVirtCpu, and cooperative scheduler MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Milestone A5 implementation (T-004): hal/src/context_switch.rs — new ContextSwitch trait (ADR-0020): TaskContext associated type, unsafe context_switch / init_context with full Safety docs. bsp-qemu-virt/src/cpu.rs — QemuVirtCpu implementing Cpu + ContextSwitch; Aarch64TaskContext (repr(C), 104 bytes: x19–x28 + fp + lr + sp); inline-asm context_switch_asm saving/restoring callee-saved registers and switching via ret. kernel/src/sched/mod.rs — Scheduler: - SchedQueue: bounded FIFO with wrap-around, O(1) enqueue/dequeue - TaskState: Idle / Ready / Blocked { on: EndpointHandle } - add_task, start, yield_now: cooperative context-switch operations - ipc_send_and_yield, ipc_recv_and_yield: IPC bridge over kernel::ipc - unblock_receiver_on: O(N) scan to re-enqueue blocked tasks - 11 host tests: queue FIFO, wrap-around, state transitions, unblock logic docs/audits/unsafe-log.md — audit entries UNSAFE-2026-0006 through 0009. All checks clean: cargo fmt, cargo host-clippy, cargo host-test (75 passed). Co-Authored-By: Claude Sonnet 4.6 --- bsp-qemu-virt/src/cpu.rs | 233 +++++++ bsp-qemu-virt/src/main.rs | 8 +- docs/analysis/tasks/phase-a/README.md | 2 +- .../phase-a/T-004-cooperative-scheduler.md | 2 +- docs/audits/unsafe-log.md | 56 ++ docs/roadmap/current.md | 2 +- docs/roadmap/phases/phase-a.md | 2 +- hal/src/context_switch.rs | 70 ++ hal/src/lib.rs | 5 +- kernel/src/lib.rs | 15 +- kernel/src/sched/mod.rs | 655 ++++++++++++++++++ 11 files changed, 1038 insertions(+), 12 deletions(-) create mode 100644 bsp-qemu-virt/src/cpu.rs create mode 100644 hal/src/context_switch.rs create mode 100644 kernel/src/sched/mod.rs diff --git a/bsp-qemu-virt/src/cpu.rs b/bsp-qemu-virt/src/cpu.rs new file mode 100644 index 0000000..1458bf7 --- /dev/null +++ b/bsp-qemu-virt/src/cpu.rs @@ -0,0 +1,233 @@ +//! `Cpu` and `ContextSwitch` implementations for the QEMU `virt` aarch64 target. +//! +//! `QemuVirtCpu` implements: +//! - [`umbrix_hal::Cpu`] — interrupt masking and core identity (object-safe). +//! - [`umbrix_hal::ContextSwitch`] — cooperative register-state save/restore +//! (generic; see [ADR-0020]). +//! +//! # Safety overview +//! +//! The context-switch assembly (`context_switch_asm`) is the only intrinsically +//! unsafe operation in this file. Every other `unsafe impl` is a marker that +//! follows from the struct's invariants. See the individual `// SAFETY:` comments +//! and the audit entries `UNSAFE-2026-0006` through `UNSAFE-2026-0009`. +//! +//! [ADR-0020]: https://github.com/cemililik/UmbrixOS/blob/main/docs/decisions/0020-cpu-trait-v2-context-switch.md + +use core::arch::asm; + +use umbrix_hal::{ContextSwitch, CoreId, Cpu, IrqState}; + +// ─── QemuVirtCpu ──────────────────────────────────────────────────────────── + +/// The QEMU `virt` aarch64 CPU implementation. +/// +/// A zero-size type — all behaviour comes from DAIF register manipulation +/// and the context-switch assembly stub. Construct via [`QemuVirtCpu::new`]. +pub struct QemuVirtCpu { + _priv: (), +} + +impl QemuVirtCpu { + /// Construct the CPU handle. + /// + /// # Safety + /// + /// There must be at most one `QemuVirtCpu` instance driving a given + /// physical core. Creating a second instance on the same core and calling + /// `restore_irq_state` on both may produce inconsistent DAIF state. + /// In v1 (single-core), construct exactly once in `kernel_entry`. + #[must_use] + pub const fn new() -> Self { + Self { _priv: () } + } +} + +// SAFETY: `QemuVirtCpu` is a zero-size marker; it has no interior mutability +// and holds no pointers. Sending it between threads is safe — the only shared +// hardware resource (DAIF) is accessed via per-core system registers that are +// inherently thread-local in a single-core system. Audit: UNSAFE-2026-0006. +unsafe impl Send for QemuVirtCpu {} + +// SAFETY: Same reasoning as the `Send` impl — no interior mutability; DAIF +// reads/writes are atomic per-core register operations. Audit: UNSAFE-2026-0006. +unsafe impl Sync for QemuVirtCpu {} + +impl Cpu for QemuVirtCpu { + fn current_core_id(&self) -> CoreId { + let mpidr: u64; + // SAFETY: `MRS x, MPIDR_EL1` is a non-privileged read of a read-only + // system register. It does not modify any state and is always available + // in EL1. Audit: UNSAFE-2026-0007. + unsafe { + asm!("mrs {}, mpidr_el1", out(reg) mpidr, options(nostack, nomem)); + } + // AFF0 (bits 7:0) identifies the core within a cluster. QEMU virt + // presents a flat topology where AFF0 == core index. + #[allow(clippy::cast_possible_truncation, reason = "AFF0 fits in u32")] + let id = (mpidr & 0xFF) as u32; + id + } + + fn disable_irqs(&self) -> IrqState { + let daif: usize; + // SAFETY: `MRS x, DAIF` reads the current interrupt mask; `MSR DAIF, #0xF` + // masks all DAIF bits (D, A, I, F). Both are EL1-privileged register + // operations. The returned `IrqState` captures the prior value so that + // `restore_irq_state` can restore it exactly. Audit: UNSAFE-2026-0007. + unsafe { + asm!( + "mrs {daif}, daif", + "msr daifset, #0xf", + daif = out(reg) daif, + options(nostack, nomem), + ); + } + IrqState(daif) + } + + fn restore_irq_state(&self, state: IrqState) { + // SAFETY: `MSR DAIF, x` writes the full DAIF register. `state` must be + // a value previously returned by `disable_irqs`; the caller is + // contractually bound to pass it unmodified. Writing an arbitrary value + // could enable or suppress interrupts unexpectedly, but the contract + // documents this requirement. Audit: UNSAFE-2026-0007. + unsafe { + asm!("msr daif, {}", in(reg) state.0, options(nostack, nomem)); + } + } + + fn wait_for_interrupt(&self) { + // SAFETY: `WFI` halts the core until an interrupt arrives. It does not + // modify registers or memory; it only affects CPU power state. + // Audit: UNSAFE-2026-0007. + unsafe { + asm!("wfi", options(nostack, nomem)); + } + } + + fn instruction_barrier(&self) { + // SAFETY: `ISB` synchronizes the instruction stream. It is always safe + // to call; it cannot cause memory corruption. Audit: UNSAFE-2026-0007. + unsafe { + asm!("isb", options(nostack, nomem)); + } + } +} + +// ─── Aarch64TaskContext ────────────────────────────────────────────────────── + +/// Saved callee-register state for one cooperative task on aarch64. +/// +/// Layout must match the field offsets used by [`context_switch_asm`]. +/// `#[repr(C)]` prevents field reordering. +/// +/// Total size: 13 × 8 = 104 bytes per task context. +#[derive(Default)] +#[repr(C)] +pub struct Aarch64TaskContext { + /// `x19`–`x28`: callee-saved general-purpose registers (10 × u64). + pub x19_x28: [u64; 10], + /// `x29` — frame pointer (callee-saved in AAPCS64). + pub fp: u64, + /// `x30` — link register / return address (callee-saved in AAPCS64). + pub lr: u64, + /// Stack pointer — saved explicitly (not a general-purpose register). + pub sp: u64, +} + +// ─── context_switch_asm ────────────────────────────────────────────────────── + +/// Save `x19`–`x28`, fp, lr, sp into `*current` and restore from `*next`. +/// +/// # Safety +/// +/// - Both pointers must be 8-byte-aligned and valid for the duration of the +/// switch. +/// - `next` must have been written by a prior call to `context_switch_asm` +/// or fully initialised by `init_context_inner`. +/// - The caller is responsible for disabling interrupts before calling this +/// function. +/// +/// Audit: UNSAFE-2026-0008. +unsafe fn context_switch_asm(current: *mut Aarch64TaskContext, next: *const Aarch64TaskContext) { + // Field offsets within Aarch64TaskContext (repr(C)): + // x19_x28 offset 0 (10 × 8 = 80 bytes) + // fp offset 80 + // lr offset 88 + // sp offset 96 + // + // We save sp via `mov x2, sp` because `str sp, [x0, #96]` is not valid + // in AArch64 — sp cannot be used as a source in most store instructions. + // SAFETY: inline assembly that saves/restores callee-saved registers. + // The register constraints and memory clobbers are stated explicitly. + // Audit: UNSAFE-2026-0008. + unsafe { + asm!( + // ── save current ──────────────────────────────────────────────── + "stp x19, x20, [{cur}, #0]", + "stp x21, x22, [{cur}, #16]", + "stp x23, x24, [{cur}, #32]", + "stp x25, x26, [{cur}, #48]", + "stp x27, x28, [{cur}, #64]", + "stp x29, x30, [{cur}, #80]", // fp, lr + "mov x8, sp", + "str x8, [{cur}, #96]", // sp + + // ── restore next ───────────────────────────────────────────────── + "ldr x8, [{nxt}, #96]", // sp + "mov sp, x8", + "ldp x29, x30, [{nxt}, #80]", // fp, lr + "ldp x27, x28, [{nxt}, #64]", + "ldp x25, x26, [{nxt}, #48]", + "ldp x23, x24, [{nxt}, #32]", + "ldp x21, x22, [{nxt}, #16]", + "ldp x19, x20, [{nxt}, #0]", + + // ret jumps to the lr we just loaded from `next`. + // For a task's first run, that lr was set to the entry fn by + // init_context_inner. + "ret", + + cur = in(reg) current, + nxt = in(reg) next, + // All callee-saved regs are clobbered by the restore. + out("x8") _, + options(nostack), + ); + } +} + +// ─── ContextSwitch impl ─────────────────────────────────────────────────────── + +impl ContextSwitch for QemuVirtCpu { + type TaskContext = Aarch64TaskContext; + + unsafe fn context_switch(&self, current: &mut Self::TaskContext, next: &Self::TaskContext) { + // SAFETY: caller guarantees interrupts are disabled and both contexts + // are valid. We forward directly to the assembly stub which upholds + // the AAPCS64 callee-save contract. Audit: UNSAFE-2026-0008. + unsafe { + context_switch_asm( + current as *mut Aarch64TaskContext, + next as *const Aarch64TaskContext, + ); + } + } + + unsafe fn init_context( + &self, + ctx: &mut Self::TaskContext, + entry: fn() -> !, + stack_top: *mut u8, + ) { + // Set lr to the entry function — the first `ret` in context_switch_asm + // will jump here to begin the task. + // Set sp to stack_top — caller must guarantee 16-byte alignment. + // All other callee-saved registers are zero (from Default), which is + // safe: the entry function establishes its own frame. + // Audit: UNSAFE-2026-0009. + ctx.lr = entry as usize as u64; + ctx.sp = stack_top as u64; + } +} diff --git a/bsp-qemu-virt/src/main.rs b/bsp-qemu-virt/src/main.rs index f30a0f2..96bf391 100644 --- a/bsp-qemu-virt/src/main.rs +++ b/bsp-qemu-virt/src/main.rs @@ -28,8 +28,10 @@ use core::panic::PanicInfo; use umbrix_hal::{Console, FmtWriter}; mod console; +mod cpu; use console::Pl011Uart; +use cpu::QemuVirtCpu; /// MMIO base of the QEMU `virt` machine's PL011 UART. /// @@ -56,8 +58,12 @@ pub extern "C" fn kernel_entry() -> ! { // window are guaranteed by the machine. // Audit: UNSAFE-2026-0001. let console = unsafe { Pl011Uart::new(PL011_UART_BASE) }; + // SAFETY: `QemuVirtCpu::new` requires at most one instance per physical + // core. We are single-core and this is the only call site. + // Audit: UNSAFE-2026-0006. + let cpu = QemuVirtCpu::new(); - umbrix_kernel::run(&console) + umbrix_kernel::run(&console, &cpu) } #[panic_handler] diff --git a/docs/analysis/tasks/phase-a/README.md b/docs/analysis/tasks/phase-a/README.md index 61e9306..25ba054 100644 --- a/docs/analysis/tasks/phase-a/README.md +++ b/docs/analysis/tasks/phase-a/README.md @@ -9,6 +9,6 @@ Tasks belonging to [Phase A — Kernel core on QEMU `virt`](../../../roadmap/pha | [T-001](T-001-capability-table-foundation.md) | Capability table foundation | A2 | Done | | [T-002](T-002-kernel-object-storage.md) | Kernel object storage foundation | A3 | Done | | [T-003](T-003-ipc-primitives.md) | IPC primitives | A4 | Done | -| [T-004](T-004-cooperative-scheduler.md) | Cooperative scheduler | A5 | Ready | +| [T-004](T-004-cooperative-scheduler.md) | Cooperative scheduler | A5 | In Progress | Tasks are added here as they become active. See [`../../../roadmap/phases/phase-a.md`](../../../roadmap/phases/phase-a.md) for the full phase plan. diff --git a/docs/analysis/tasks/phase-a/T-004-cooperative-scheduler.md b/docs/analysis/tasks/phase-a/T-004-cooperative-scheduler.md index 299ffaa..e3d6eff 100644 --- a/docs/analysis/tasks/phase-a/T-004-cooperative-scheduler.md +++ b/docs/analysis/tasks/phase-a/T-004-cooperative-scheduler.md @@ -2,7 +2,7 @@ - **Phase:** A - **Milestone:** A5 — Cooperative scheduler and context switch -- **Status:** Ready +- **Status:** In Progress - **Created:** 2026-04-21 - **Author:** @cemililik - **Dependencies:** T-003 — IPC primitives (Done) diff --git a/docs/audits/unsafe-log.md b/docs/audits/unsafe-log.md index 9a84d41..cf81ece 100644 --- a/docs/audits/unsafe-log.md +++ b/docs/audits/unsafe-log.md @@ -61,3 +61,59 @@ Entries are **append-only**. When an `unsafe` region is removed, its entry gains - **Rejected alternatives:** Using a `volatile_register` crate would wrap these in typed abstractions at some ergonomic cost; the plain-MMIO form is small enough and easy enough to audit here. Revisit if more registers join the picture. - **Reviewed by:** @cemililik. - **Status:** Active. + +### UNSAFE-2026-0006 — `Send` + `Sync` for `QemuVirtCpu` + +- **Introduced:** 2026-04-21, T-004 / A5 context-switch implementation. +- **Location:** [`bsp-qemu-virt/src/cpu.rs`](../../bsp-qemu-virt/src/cpu.rs) — `unsafe impl Send for QemuVirtCpu` and `unsafe impl Sync for QemuVirtCpu`. +- **Operation:** Declares that `QemuVirtCpu` can be transferred between threads and that shared references to it are safe to use concurrently. +- **Invariants relied on:** + - `QemuVirtCpu` is a zero-size type with no fields, no heap allocation, and no interior mutability. + - The hardware resources it accesses (DAIF interrupt-mask register, MPIDR) are per-core system registers — inherently core-local in a single-core v1 system. + - In a multi-core system, each core would construct its own `QemuVirtCpu`; a future ADR will revisit this. +- **Rejected alternatives:** The compiler cannot derive `Send`/`Sync` for structs containing raw pointers; since `QemuVirtCpu` uses inline assembly to access system registers rather than storing raw pointers, this is a marker assertion rather than a pointer-safety claim. +- **Reviewed by:** @cemililik (self-review, solo phase). +- **Status:** Active. + +### UNSAFE-2026-0007 — inline assembly in `QemuVirtCpu::Cpu` methods + +- **Introduced:** 2026-04-21, T-004 / A5 context-switch implementation. +- **Location:** [`bsp-qemu-virt/src/cpu.rs`](../../bsp-qemu-virt/src/cpu.rs) — `current_core_id`, `disable_irqs`, `restore_irq_state`, `wait_for_interrupt`, `instruction_barrier`. +- **Operation:** `MRS`/`MSR` DAIF and MPIDR_EL1 register accesses, `WFI`, and `ISB` via `core::arch::asm!`. +- **Invariants relied on:** + - All instructions are EL1-privileged; the kernel runs at EL1 on QEMU `virt`. + - `MRS` reads are non-destructive; `MSR DAIFSET` masks interrupts atomically. + - `MSR DAIF, x` in `restore_irq_state` writes exactly the value returned by a prior `disable_irqs` call — the caller is contractually bound to pass the value unmodified. + - `WFI` and `ISB` do not modify registers or memory; `options(nostack, nomem)` is correct. +- **Rejected alternatives:** No safe Rust abstraction exists for EL1 system-register access; the HAL trait is the safe abstraction wrapping these blocks. +- **Reviewed by:** @cemililik. +- **Status:** Active. + +### UNSAFE-2026-0008 — context-switch assembly in `context_switch_asm` and callers + +- **Introduced:** 2026-04-21, T-004 / A5 context-switch implementation. +- **Location:** [`bsp-qemu-virt/src/cpu.rs`](../../bsp-qemu-virt/src/cpu.rs) — `context_switch_asm` and `QemuVirtCpu::context_switch`; [`kernel/src/sched/mod.rs`](../../kernel/src/sched/mod.rs) — `Scheduler::start`, `yield_now`, `ipc_recv_and_yield`. +- **Operation:** Saves `x19`–`x28`, `x29` (fp), `x30` (lr), `sp` to `*current` and restores from `*next` via `STP`/`LDP`/`STR`/`LDR` instructions; returns via `RET` which jumps to the loaded `lr`. +- **Invariants relied on:** + - `current` and `next` are distinct (different task indices) wherever the split-borrow pattern is used in `Scheduler`. + - Both pointers are 8-byte aligned — `Aarch64TaskContext` is `#[repr(C)]` with all `u64` fields. + - Interrupts are disabled by `IrqGuard` before `context_switch` is called. An IRQ mid-switch would observe partially saved registers. + - `next` was either written by a prior `context_switch_asm` call or fully initialised by `init_context` (UNSAFE-2026-0009). + - The `ret` instruction will jump to `next.lr`; for a task's first run, `lr` is the entry function address set by `init_context`. The entry function is `fn() -> !` and truly never returns. +- **Rejected alternatives:** Context switching requires register-level manipulation that cannot be expressed in safe Rust. The assembly is minimal (13 saves + 13 restores + ret). +- **Reviewed by:** @cemililik. +- **Status:** Active. + +### UNSAFE-2026-0009 — context initialisation in `QemuVirtCpu::init_context` and callers + +- **Introduced:** 2026-04-21, T-004 / A5 context-switch implementation. +- **Location:** [`bsp-qemu-virt/src/cpu.rs`](../../bsp-qemu-virt/src/cpu.rs) — `QemuVirtCpu::init_context`; [`kernel/src/sched/mod.rs`](../../kernel/src/sched/mod.rs) — `Scheduler::add_task`. +- **Operation:** Writes `entry` (cast to `u64`) into `ctx.lr` and `stack_top` (cast to `u64`) into `ctx.sp`. The first restore of this context will begin executing `entry` with `stack_top` as the stack pointer. +- **Invariants relied on:** + - `stack_top` must be 16-byte aligned and point one byte past the top of at least 512 bytes of stack memory that remains valid for the task's lifetime. Callers are contractually bound by the `# Safety` doc. + - Function pointers are always valid addresses in Rust — casting `fn() -> !` to `usize` then `u64` is safe. + - The entry function truly never returns; if it did, the `ret` in `context_switch_asm` would jump to garbage. + - `ctx` is at a valid, exclusively-owned index within `Scheduler::contexts`. +- **Rejected alternatives:** Initialising a context requires writing raw register values; no safe abstraction exists. +- **Reviewed by:** @cemililik. +- **Status:** Active. diff --git a/docs/roadmap/current.md b/docs/roadmap/current.md index 4097318..af2ec82 100644 --- a/docs/roadmap/current.md +++ b/docs/roadmap/current.md @@ -6,7 +6,7 @@ A short pointer file updated as work progresses. For the full plan see [`phases/ - **Active phase:** A — Kernel core on QEMU `virt`. - **Active milestone:** A5 — Cooperative scheduler and context switch. -- **Active task:** [T-004 — Cooperative scheduler](../analysis/tasks/phase-a/T-004-cooperative-scheduler.md) (status: **Ready**). +- **Active task:** [T-004 — Cooperative scheduler](../analysis/tasks/phase-a/T-004-cooperative-scheduler.md) (status: **In Progress**). - **Working branch:** `development`. - **Last completed milestone:** A4 — IPC primitives, on 2026-04-21 (PR merged to `main`). - **Last completed task:** [T-003 — IPC primitives](../analysis/tasks/phase-a/T-003-ipc-primitives.md) — `Done` 2026-04-21. diff --git a/docs/roadmap/phases/phase-a.md b/docs/roadmap/phases/phase-a.md index 2b73c4d..1e43e31 100644 --- a/docs/roadmap/phases/phase-a.md +++ b/docs/roadmap/phases/phase-a.md @@ -150,7 +150,7 @@ The first real scheduler: cooperative yield-based, with a context-switch primiti ### Tasks under A5 -- [T-004 — Cooperative scheduler](../../analysis/tasks/phase-a/T-004-cooperative-scheduler.md) — Ready. +- [T-004 — Cooperative scheduler](../../analysis/tasks/phase-a/T-004-cooperative-scheduler.md) — In Progress. ### Informs diff --git a/hal/src/context_switch.rs b/hal/src/context_switch.rs new file mode 100644 index 0000000..6132dca --- /dev/null +++ b/hal/src/context_switch.rs @@ -0,0 +1,70 @@ +//! Cooperative context-switch extension for BSPs. +//! +//! See [ADR-0020] for the design rationale. This trait is deliberately +//! separate from [`Cpu`][crate::Cpu] to preserve `Cpu`'s object-safety; +//! the scheduler is generic over `C: ContextSwitch` and does not use +//! dynamic dispatch. +//! +//! [ADR-0020]: https://github.com/cemililik/UmbrixOS/blob/main/docs/decisions/0020-cpu-trait-v2-context-switch.md + +/// Context-switch extension for BSPs that support cooperative task switching. +/// +/// Separate from [`Cpu`][crate::Cpu] to preserve `Cpu`'s object-safety. +/// The scheduler is generic over `C: ContextSwitch`; it never needs +/// dynamic dispatch. +/// +/// # Safety contract +/// +/// Implementations must ensure that `context_switch` atomically saves +/// all callee-saved registers of the current execution context and +/// restores all callee-saved registers of the next context. On aarch64 +/// that is `x19`–`x28`, `x29` (fp), `x30` (lr), and `sp`. From the +/// perspective of both call sites, `context_switch` appears to return +/// normally — the saving side resumes here when it is later selected as +/// `next`. +pub trait ContextSwitch { + /// The saved register state for one cooperative task. + /// + /// Must be `Default` so the scheduler can zero-initialise a slot + /// before `init_context` fills it in. Must be `Send` so contexts + /// can be moved between (future) CPU cores. + type TaskContext: Default + Send; + + /// Save the calling task's register state into `current` and resume + /// the task whose state was saved in `next`. + /// + /// When this task is later resumed (by another call to + /// `context_switch` with this `current` as the `next` argument), + /// execution continues as if `context_switch` returned normally. + /// + /// # Safety + /// + /// - Interrupts must be disabled before this call. An IRQ firing + /// mid-switch would observe a partially saved state. + /// - `current` must be valid for the entire time this task is + /// suspended; the caller is responsible for keeping the context + /// array alive. + /// - `next` must contain a context previously written by + /// `context_switch` or fully initialised by `init_context`. + /// Restoring an uninitialised context is undefined behaviour. + unsafe fn context_switch(&self, current: &mut Self::TaskContext, next: &Self::TaskContext); + + /// Write an initial register state into `ctx` so that the first + /// restore begins executing `entry` with `stack_top` as the initial + /// stack pointer. + /// + /// # Safety + /// + /// - `stack_top` must point one byte past the top of a + /// sufficiently-sized (≥ 512 bytes recommended for aarch64), + /// 16-byte-aligned stack region that remains valid for the + /// task's entire lifetime. + /// - `entry` must be a `fn() -> !` that never returns; returning + /// from a task entry function is undefined behaviour. + unsafe fn init_context( + &self, + ctx: &mut Self::TaskContext, + entry: fn() -> !, + stack_top: *mut u8, + ); +} diff --git a/hal/src/lib.rs b/hal/src/lib.rs index 3ad6dde..7f95a61 100644 --- a/hal/src/lib.rs +++ b/hal/src/lib.rs @@ -16,19 +16,22 @@ //! //! In progress. Traits are pinned down one at a time, each behind a dedicated //! ADR. Accepted so far: [`Console`] (ADR-0007), [`Cpu`] (ADR-0008), -//! [`Mmu`] (ADR-0009), [`Timer`] (ADR-0010), [`IrqController`] (ADR-0011). +//! [`Mmu`] (ADR-0009), [`Timer`] (ADR-0010), [`IrqController`] (ADR-0011), +//! [`ContextSwitch`] (ADR-0020). //! The remaining trait stub below is a placeholder whose method surface //! will be pinned by its own ADR when a concrete caller needs it. #![no_std] mod console; +mod context_switch; mod cpu; mod irq_controller; mod mmu; mod timer; pub use console::{Console, FmtWriter}; +pub use context_switch::ContextSwitch; pub use cpu::{CoreId, Cpu, IrqGuard, IrqState}; pub use irq_controller::{IrqController, IrqNumber}; pub use mmu::{ diff --git a/kernel/src/lib.rs b/kernel/src/lib.rs index a4e56b7..b4d16cb 100644 --- a/kernel/src/lib.rs +++ b/kernel/src/lib.rs @@ -24,10 +24,13 @@ //! later subsystem refers through for authority. //! - [`ipc`] — IPC subsystem (Phase A4 / [T-003]): `send` / `recv` / `notify` //! primitives over the A3 kernel objects, gated by capabilities. +//! - [`sched`] — cooperative scheduler (Phase A5 / [T-004]): bounded FIFO +//! ready queue, per-task state, and IPC bridge. //! //! [T-001]: https://github.com/cemililik/UmbrixOS/blob/main/docs/analysis/tasks/phase-a/T-001-capability-table-foundation.md //! [T-002]: https://github.com/cemililik/UmbrixOS/blob/main/docs/analysis/tasks/phase-a/T-002-kernel-object-storage.md //! [T-003]: https://github.com/cemililik/UmbrixOS/blob/main/docs/analysis/tasks/phase-a/T-003-ipc-primitives.md +//! [T-004]: https://github.com/cemililik/UmbrixOS/blob/main/docs/analysis/tasks/phase-a/T-004-cooperative-scheduler.md #![cfg_attr(not(test), no_std)] // Kernel-specific stricter lints on top of the workspace set. @@ -42,21 +45,21 @@ pub mod cap; pub mod ipc; pub mod obj; +pub mod sched; -use umbrix_hal::Console; +use umbrix_hal::{Console, Cpu}; /// Portable kernel entry, called by the BSP after early init. /// -/// In Phase 4c v0.0.1 this writes a greeting to the console and idles -/// the CPU in a `spin_loop`. Subsequent phases will bring up the -/// scheduler, IPC, capability system, and userspace init here before -/// reaching steady state. +/// Accepts the BSP's console and CPU implementations. In the current phase +/// (A5) this prints a greeting and idles. Future milestones will wire up +/// the scheduler, task creation, and IPC here. /// /// # Never returns /// /// This function is `-> !`. A return would be a kernel bug; the BSP's /// reset stub halts defensively if it ever does. -pub fn run(console: &C) -> ! { +pub fn run(console: &Con, _cpu: &C) -> ! { console.write_bytes(b"umbrix: hello from kernel_main\n"); loop { diff --git a/kernel/src/sched/mod.rs b/kernel/src/sched/mod.rs new file mode 100644 index 0000000..55aeafb --- /dev/null +++ b/kernel/src/sched/mod.rs @@ -0,0 +1,655 @@ +//! Cooperative scheduler — Milestone A5, [T-004]. +//! +//! Design settled in [ADR-0019] (queue structure, yield semantics, +//! blocked-task representation, IPC bridge ownership). Generic over +//! `C: ContextSwitch` per [ADR-0020]. +//! +//! # Overview +//! +//! - [`SchedQueue`] — bounded FIFO of [`TaskHandle`]s. +//! - [`TaskState`] — per-task state enum (`Idle / Ready / Blocked`). +//! - [`Scheduler`] — ready queue, per-task state, saved contexts, and +//! the identity of the currently running task. +//! +//! The IPC bridge (`ipc_send_and_yield` / `ipc_recv_and_yield`) is an +//! orchestration layer on top of [`crate::ipc`]; the IPC module itself +//! remains ignorant of scheduling concerns. +//! +//! [T-004]: https://github.com/cemililik/UmbrixOS/blob/main/docs/analysis/tasks/phase-a/T-004-cooperative-scheduler.md +//! [ADR-0019]: https://github.com/cemililik/UmbrixOS/blob/main/docs/decisions/0019-scheduler-shape.md +//! [ADR-0020]: https://github.com/cemililik/UmbrixOS/blob/main/docs/decisions/0020-cpu-trait-v2-context-switch.md + +use umbrix_hal::{ContextSwitch, Cpu, IrqGuard}; + +use crate::cap::{CapHandle, CapObject, CapabilityTable}; +use crate::ipc::{ipc_recv, ipc_send, IpcError, IpcQueues, Message, RecvOutcome, SendOutcome}; +use crate::obj::endpoint::EndpointArena; +use crate::obj::{EndpointHandle, TaskHandle, TASK_ARENA_CAPACITY}; + +// ─── SchedQueue ─────────────────────────────────────────────────────────────── + +/// Bounded FIFO queue of [`TaskHandle`]s, capacity `N`. +/// +/// Capacity equals [`TASK_ARENA_CAPACITY`] so the queue can never be full +/// relative to the number of tasks that can exist. +pub struct SchedQueue { + buf: [Option; N], + head: usize, + len: usize, +} + +impl Default for SchedQueue { + fn default() -> Self { + Self::new() + } +} + +impl SchedQueue { + /// Construct an empty queue. + #[must_use] + pub const fn new() -> Self { + Self { + buf: [None; N], + head: 0, + len: 0, + } + } + + /// Push `handle` to the back of the queue. + /// + /// # Errors + /// + /// Returns `Err(handle)` when the queue is full. + pub fn enqueue(&mut self, handle: TaskHandle) -> Result<(), TaskHandle> { + if self.len == N { + return Err(handle); + } + // N is the queue capacity; head and len are both < N when not full. + // wrapping_add followed by % N is safe: head < N, len < N, so their + // sum fits in usize before the modulo. + #[allow( + clippy::arithmetic_side_effects, + reason = "N > 0 enforced by caller; head < N and len < N" + )] + let tail = self.head.wrapping_add(self.len) % N; + self.buf[tail] = Some(handle); + self.len = self.len.wrapping_add(1); + Ok(()) + } + + /// Pop the front handle from the queue, or `None` if empty. + pub fn dequeue(&mut self) -> Option { + if self.len == 0 { + return None; + } + let handle = self.buf[self.head].take(); + // head wraps around N — N > 0 because len > 0. + #[allow( + clippy::arithmetic_side_effects, + reason = "N > 0 because len > 0 (queue not empty)" + )] + { + self.head = self.head.wrapping_add(1) % N; + } + self.len = self.len.wrapping_sub(1); + handle + } + + /// Number of handles currently in the queue. + #[must_use] + pub fn len(&self) -> usize { + self.len + } + + /// True when the queue contains no handles. + #[must_use] + pub fn is_empty(&self) -> bool { + self.len == 0 + } +} + +// ─── TaskState ──────────────────────────────────────────────────────────────── + +/// Scheduling state of one task slot. +#[derive(Copy, Clone, Debug, Eq, PartialEq)] +pub enum TaskState { + /// Slot is not occupied by a live task. + Idle, + /// Task is in the ready queue or currently running. + Ready, + /// Task is waiting for a message on an endpoint. + Blocked { + /// The endpoint the task is blocked on. + on: EndpointHandle, + }, +} + +// ─── SchedError ────────────────────────────────────────────────────────────── + +/// Errors returned by scheduler operations. +#[derive(Copy, Clone, Debug, Eq, PartialEq)] +pub enum SchedError { + /// No task is currently running; the operation requires a current task. + NoCurrentTask, + /// The ready queue is full. + QueueFull, + /// IPC operation failed. + Ipc(IpcError), +} + +impl From for SchedError { + fn from(e: IpcError) -> Self { + Self::Ipc(e) + } +} + +// ─── Scheduler ─────────────────────────────────────────────────────────────── + +/// Cooperative, single-core scheduler. +/// +/// Generic over `C: ContextSwitch + Cpu` — the BSP provides both the +/// interrupt-masking needed for safe context switches and the register-save +/// assembly. +pub struct Scheduler { + ready: SchedQueue, + task_states: [TaskState; TASK_ARENA_CAPACITY], + /// Stored handles, indexed by slot index, so the scheduler can find + /// a `TaskHandle` when unblocking without re-querying the arena. + task_handles: [Option; TASK_ARENA_CAPACITY], + current: Option, + /// Saved register contexts, one per task arena slot. + /// + /// Invariant: `contexts[i]` is valid for every slot `i` that has + /// `task_states[i] != Idle` — either zero-initialised by `Default` and + /// then filled by `init_context`, or saved by a prior `context_switch`. + contexts: [C::TaskContext; TASK_ARENA_CAPACITY], +} + +impl Default for Scheduler { + fn default() -> Self { + Self::new() + } +} + +impl Scheduler { + /// Construct an empty scheduler with all contexts zero-initialised. + #[must_use] + pub fn new() -> Self { + Self { + ready: SchedQueue::new(), + task_states: [TaskState::Idle; TASK_ARENA_CAPACITY], + task_handles: [None; TASK_ARENA_CAPACITY], + current: None, + contexts: core::array::from_fn(|_| C::TaskContext::default()), + } + } + + /// Register a new task and enqueue it as ready. + /// + /// Initialises the task's context so that the first restore begins + /// executing `entry` with `stack_top` as the initial stack pointer. + /// + /// # Errors + /// + /// [`SchedError::QueueFull`] if the ready queue is already at capacity + /// (cannot happen when `TASK_ARENA_CAPACITY` slots exist and only one + /// task occupies each slot). + /// + /// # Safety + /// + /// `stack_top` must satisfy [`ContextSwitch::init_context`]'s contract: + /// 16-byte aligned, at least 512 bytes of backing memory, valid for the + /// task's entire lifetime. + pub unsafe fn add_task( + &mut self, + cpu: &C, + handle: TaskHandle, + entry: fn() -> !, + stack_top: *mut u8, + ) -> Result<(), SchedError> { + let idx = handle.slot().index() as usize; + // SAFETY: caller guarantees stack_top validity per the # Safety doc. + // Forwarding to the BSP's init_context which writes lr and sp into + // the context slot at `idx`. Audit: UNSAFE-2026-0009. + unsafe { + cpu.init_context(&mut self.contexts[idx], entry, stack_top); + } + self.task_states[idx] = TaskState::Ready; + self.task_handles[idx] = Some(handle); + self.ready + .enqueue(handle) + .map_err(|_| SchedError::QueueFull) + } + + /// Start the scheduler by switching to the first ready task. + /// + /// Saves throwaway state for the bootstrap context (which is never + /// resumed) and restores the first task. Intended to be called exactly + /// once from `kernel_main` after tasks have been added. + /// + /// # Panics + /// + /// Panics if no tasks have been added (the ready queue is empty). + pub fn start(&mut self, cpu: &C) { + #[allow( + clippy::panic, + reason = "empty ready queue is a kernel programming error" + )] + let Some(next_handle) = self.ready.dequeue() else { + panic!("Scheduler::start called with empty ready queue"); + }; + let next_idx = next_handle.slot().index() as usize; + self.task_states[next_idx] = TaskState::Ready; + self.current = Some(next_handle); + + let mut throwaway = C::TaskContext::default(); + let _guard = IrqGuard::new(cpu); + // SAFETY: `next` was written by `add_task` → `init_context`. + // Interrupts are disabled by the IrqGuard for the duration of the + // switch. The throwaway current context is never restored. + // Audit: UNSAFE-2026-0008. + unsafe { + cpu.context_switch(&mut throwaway, &self.contexts[next_idx]); + } + } + + /// Yield the current task: re-enqueue it as ready and switch to the + /// next task at the head of the ready queue. + /// + /// If only one task exists (the queue is empty after re-enqueue), the + /// function returns without switching — the single task keeps running. + /// + /// # Errors + /// + /// [`SchedError::NoCurrentTask`] if called before [`Scheduler::start`]. + pub fn yield_now(&mut self, cpu: &C) -> Result<(), SchedError> { + let current_handle = self.current.ok_or(SchedError::NoCurrentTask)?; + let current_idx = current_handle.slot().index() as usize; + + // Re-enqueue current as ready. Cannot be full: the running task was + // not in the ready queue (it was dequeued when it started running), + // so at most TASK_ARENA_CAPACITY-1 other tasks are queued. + self.task_states[current_idx] = TaskState::Ready; + let _ = self.ready.enqueue(current_handle); + + // Dequeue the next task. + let next_handle = match self.ready.dequeue() { + Some(h) if h != current_handle => h, + _ => { + // Only one task exists or same handle returned — no switch. + return Ok(()); + } + }; + + let next_idx = next_handle.slot().index() as usize; + self.task_states[next_idx] = TaskState::Ready; + self.current = Some(next_handle); + + let _guard = IrqGuard::new(cpu); + // SAFETY: current_idx != next_idx — they are two distinct tasks; + // current was the running task (not in the queue) while next was + // dequeued from the ready queue. Both indices are within + // [0, TASK_ARENA_CAPACITY). Interrupts disabled by IrqGuard. + // Audit: UNSAFE-2026-0008. + unsafe { + let ctx_ptr = self.contexts.as_mut_ptr(); + // Split-borrow: current and next are at distinct indices, + // so the two resulting references do not alias. + let cur_ctx = &mut *ctx_ptr.add(current_idx); + let nxt_ctx = &*ctx_ptr.add(next_idx); + cpu.context_switch(cur_ctx, nxt_ctx); + } + + Ok(()) + } + + // ── IPC bridge ──────────────────────────────────────────────────────────── + + /// Send a message; if it was delivered to a waiting receiver, unblock + /// that receiver and yield the current task. + /// + /// # Errors + /// + /// Propagates [`IpcError`] as [`SchedError::Ipc`]. + #[allow( + clippy::too_many_arguments, + reason = "IPC bridge must forward all parameters that ipc_send requires" + )] + pub fn ipc_send_and_yield( + &mut self, + cpu: &C, + ep_arena: &mut EndpointArena, + queues: &mut IpcQueues, + caller_table: &mut CapabilityTable, + ep_cap: CapHandle, + msg: Message, + transfer: Option, + ) -> Result { + // Resolve the endpoint handle before calling ipc_send so we can + // identify the blocked receiver even after state has changed. + let ep_handle = Self::resolve_ep_cap(caller_table, ep_cap)?; + + let outcome = ipc_send(ep_arena, queues, ep_cap, caller_table, msg, transfer)?; + + if outcome == SendOutcome::Delivered { + self.unblock_receiver_on(ep_handle); + self.yield_now(cpu)?; + } + + Ok(outcome) + } + + /// Receive a message; if none is ready, block and yield to another task. + /// + /// When the blocked task is resumed (after a sender delivers), calls + /// `ipc_recv` again to collect the delivered message. + /// + /// # Panics + /// + /// Panics when all tasks (including this one) are blocked on IPC + /// simultaneously, producing a deadlock with no idle task to run. + /// + /// # Errors + /// + /// Propagates [`IpcError`] as [`SchedError::Ipc`]. + pub fn ipc_recv_and_yield( + &mut self, + cpu: &C, + ep_arena: &mut EndpointArena, + queues: &mut IpcQueues, + caller_table: &mut CapabilityTable, + ep_cap: CapHandle, + ) -> Result { + let ep_handle = Self::resolve_ep_cap(caller_table, ep_cap)?; + + let outcome = ipc_recv(ep_arena, queues, ep_cap, caller_table)?; + + if matches!(outcome, RecvOutcome::Pending) { + let current_handle = self.current.ok_or(SchedError::NoCurrentTask)?; + let current_idx = current_handle.slot().index() as usize; + self.task_states[current_idx] = TaskState::Blocked { on: ep_handle }; + self.current = None; + + #[allow( + clippy::panic, + reason = "deadlock with no idle task is a fatal A5 condition; \ + an idle task is Phase B work (ADR-0019 open questions)" + )] + let Some(next_handle) = self.ready.dequeue() else { + panic!("deadlock: all tasks blocked on IPC and no idle task available"); + }; + let next_idx = next_handle.slot().index() as usize; + self.task_states[next_idx] = TaskState::Ready; + self.current = Some(next_handle); + + let _guard = IrqGuard::new(cpu); + // SAFETY: current_idx != next_idx; both valid indices; IRQs + // disabled. When this task is later resumed (by another task + // calling ipc_send_and_yield → unblock_receiver_on → yield_now), + // execution resumes after this context_switch call. + // Audit: UNSAFE-2026-0008. + unsafe { + let ctx_ptr = self.contexts.as_mut_ptr(); + let cur_ctx = &mut *ctx_ptr.add(current_idx); + let nxt_ctx = &*ctx_ptr.add(next_idx); + cpu.context_switch(cur_ctx, nxt_ctx); + } + + // Resumed here: the sender has delivered; collect the message. + return ipc_recv(ep_arena, queues, ep_cap, caller_table).map_err(SchedError::Ipc); + } + + Ok(outcome) + } + + // ── Private helpers ─────────────────────────────────────────────────────── + + /// Resolve a capability handle to an [`EndpointHandle`]. + fn resolve_ep_cap( + caller_table: &CapabilityTable, + ep_cap: CapHandle, + ) -> Result { + let cap = caller_table + .lookup(ep_cap) + .map_err(|_| SchedError::Ipc(IpcError::InvalidCapability))?; + match cap.object() { + CapObject::Endpoint(h) => Ok(h), + _ => Err(SchedError::Ipc(IpcError::InvalidCapability)), + } + } + + /// Scan `task_states` for a task blocked on `ep` and re-enqueue it. + /// + /// O(N) scan — acceptable at `TASK_ARENA_CAPACITY ≤ 16` (ADR-0019). + fn unblock_receiver_on(&mut self, ep: EndpointHandle) { + for idx in 0..TASK_ARENA_CAPACITY { + if let TaskState::Blocked { on } = self.task_states[idx] { + if on == ep { + if let Some(handle) = self.task_handles[idx] { + self.task_states[idx] = TaskState::Ready; + let _ = self.ready.enqueue(handle); + return; + } + } + } + } + } +} + +// ─── Tests ──────────────────────────────────────────────────────────────────── + +#[cfg(test)] +#[allow( + clippy::unwrap_used, + clippy::panic, + clippy::expect_used, + reason = "test pragmas not permitted in production kernel code" +)] +mod tests { + use super::*; + use crate::obj::arena::SlotId; + use crate::obj::endpoint::EndpointHandle; + + // ── FakeCpu ─────────────────────────────────────────────────────────────── + + struct FakeCpu; + + #[derive(Default, Debug, PartialEq)] + struct FakeCtx { + switched: bool, + } + + // SAFETY: FakeCpu is a zero-size marker with no interior mutability + // and no shared mutable state. Send + Sync are safe. + unsafe impl Send for FakeCpu {} + // SAFETY: same reasoning as Send impl above. + unsafe impl Sync for FakeCpu {} + + impl Cpu for FakeCpu { + fn current_core_id(&self) -> umbrix_hal::CoreId { + 0 + } + fn disable_irqs(&self) -> umbrix_hal::IrqState { + umbrix_hal::IrqState(0) + } + fn restore_irq_state(&self, _: umbrix_hal::IrqState) {} + fn wait_for_interrupt(&self) {} + fn instruction_barrier(&self) {} + } + + impl ContextSwitch for FakeCpu { + type TaskContext = FakeCtx; + + unsafe fn context_switch( + &self, + current: &mut Self::TaskContext, + _next: &Self::TaskContext, + ) { + current.switched = true; + } + + unsafe fn init_context( + &self, + _ctx: &mut Self::TaskContext, + _entry: fn() -> !, + _stack_top: *mut u8, + ) { + } + } + + // ── Helpers ─────────────────────────────────────────────────────────────── + + fn task_handle(index: u16) -> TaskHandle { + TaskHandle::from_slot(SlotId::from_parts(index, 0)) + } + + fn ep_handle(index: u16) -> EndpointHandle { + EndpointHandle::from_slot(SlotId::from_parts(index, 0)) + } + + fn spin_entry() -> fn() -> ! { + || loop { + core::hint::spin_loop(); + } + } + + // ── SchedQueue tests ────────────────────────────────────────────────────── + + #[test] + fn queue_enqueue_dequeue_fifo_order() { + let mut q: SchedQueue<4> = SchedQueue::new(); + let h0 = task_handle(0); + let h1 = task_handle(1); + q.enqueue(h0).unwrap(); + q.enqueue(h1).unwrap(); + assert_eq!(q.dequeue(), Some(h0)); + assert_eq!(q.dequeue(), Some(h1)); + assert_eq!(q.dequeue(), None); + } + + #[test] + fn queue_full_returns_error() { + let mut q: SchedQueue<2> = SchedQueue::new(); + q.enqueue(task_handle(0)).unwrap(); + q.enqueue(task_handle(1)).unwrap(); + assert!(q.enqueue(task_handle(2)).is_err()); + } + + #[test] + fn queue_empty_dequeue_is_none() { + let mut q: SchedQueue<4> = SchedQueue::new(); + assert!(q.dequeue().is_none()); + } + + #[test] + fn queue_wraps_around() { + let mut q: SchedQueue<2> = SchedQueue::new(); + q.enqueue(task_handle(0)).unwrap(); + q.dequeue(); + q.enqueue(task_handle(1)).unwrap(); + assert_eq!(q.dequeue(), Some(task_handle(1))); + } + + #[test] + fn queue_len_and_is_empty() { + let mut q: SchedQueue<4> = SchedQueue::new(); + assert!(q.is_empty()); + assert_eq!(q.len(), 0); + q.enqueue(task_handle(0)).unwrap(); + assert!(!q.is_empty()); + assert_eq!(q.len(), 1); + } + + // ── Scheduler state-transition tests ───────────────────────────────────── + + #[test] + fn add_task_sets_ready_state_and_stores_handle() { + let cpu = FakeCpu; + let mut sched: Scheduler = Scheduler::new(); + let h = task_handle(0); + let mut stack = [0u8; 512]; + let stack_top = stack.as_mut_ptr_range().end; + // SAFETY: stack is 512 bytes; FakeCpu::init_context is a no-op. + unsafe { sched.add_task(&cpu, h, spin_entry(), stack_top).unwrap() }; + assert_eq!(sched.task_states[0], TaskState::Ready); + assert_eq!(sched.task_handles[0], Some(h)); + assert_eq!(sched.ready.len(), 1); + } + + #[test] + fn yield_now_switches_context_and_updates_current() { + let cpu = FakeCpu; + let mut sched: Scheduler = Scheduler::new(); + let h0 = task_handle(0); + let h1 = task_handle(1); + let mut s0 = [0u8; 512]; + let mut s1 = [0u8; 512]; + // SAFETY: stacks are 512 bytes; FakeCpu::init_context is a no-op. + unsafe { + sched + .add_task(&cpu, h0, spin_entry(), s0.as_mut_ptr_range().end) + .unwrap(); + sched + .add_task(&cpu, h1, spin_entry(), s1.as_mut_ptr_range().end) + .unwrap(); + } + // Simulate h0 running: it was dequeued when it started running. + sched.ready.dequeue(); // removes h0 (head of queue) + sched.current = Some(h0); + // h1 is still in the queue. + assert_eq!(sched.ready.len(), 1); + + sched.yield_now(&cpu).unwrap(); + + assert_eq!(sched.current, Some(h1)); + assert_eq!(sched.task_states[0], TaskState::Ready); + assert_eq!(sched.task_states[1], TaskState::Ready); + // FakeCpu::context_switch marks the saved (h0) context as switched. + assert!(sched.contexts[0].switched); + } + + #[test] + fn yield_now_with_no_current_returns_error() { + let cpu = FakeCpu; + let mut sched: Scheduler = Scheduler::new(); + assert_eq!(sched.yield_now(&cpu), Err(SchedError::NoCurrentTask)); + } + + #[test] + fn unblock_receiver_on_moves_task_to_ready() { + let mut sched: Scheduler = Scheduler::new(); + let h0 = task_handle(0); + let ep = ep_handle(0); + sched.task_states[0] = TaskState::Blocked { on: ep }; + sched.task_handles[0] = Some(h0); + + sched.unblock_receiver_on(ep); + + assert_eq!(sched.task_states[0], TaskState::Ready); + assert_eq!(sched.ready.len(), 1); + } + + #[test] + fn unblock_receiver_on_wrong_ep_is_noop() { + let mut sched: Scheduler = Scheduler::new(); + let h0 = task_handle(0); + let ep0 = ep_handle(0); + let ep1 = ep_handle(1); + sched.task_states[0] = TaskState::Blocked { on: ep0 }; + sched.task_handles[0] = Some(h0); + + sched.unblock_receiver_on(ep1); + + assert_eq!(sched.task_states[0], TaskState::Blocked { on: ep0 }); + assert!(sched.ready.is_empty()); + } + + #[test] + fn task_state_variants_are_distinct() { + let ep = ep_handle(0); + assert_ne!(TaskState::Idle, TaskState::Ready); + assert_ne!(TaskState::Ready, TaskState::Blocked { on: ep }); + assert_ne!(TaskState::Idle, TaskState::Blocked { on: ep }); + assert_eq!(TaskState::Blocked { on: ep }, TaskState::Blocked { on: ep }); + } +} From 4fe547cf47beb54a51ad09f8a5705d2f30841a29 Mon Sep 17 00:00:00 2001 From: Cemil ILIK Date: Tue, 21 Apr 2026 16:13:56 +0300 Subject: [PATCH 07/10] =?UTF-8?q?feat(a5):=20cooperative=20context=20switc?= =?UTF-8?q?h=20=E2=80=94=20fix=20three=20boot=20bugs;=20QEMU=20smoke=20pas?= =?UTF-8?q?ses?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three bugs prevented task_a from executing after `start()` transferred control. All three are fixed in this commit; QEMU now shows interleaved output from both tasks across all three iterations. Bug 1 — IrqGuard fat-pointer vtable (hal/src/cpu.rs) `IrqGuard::new(cpu)` where `cpu: &QemuVirtCpu` was coerced to `&dyn Cpu`. The compiler placed the vtable adjacent to a panic string in .rodata; the vtable offset landed on ASCII bytes, so `blr` jumped into garbage. Fix: make `IrqGuard` generic — `IrqGuard<'a, C: Cpu>` takes `cpu: &'a C` directly, eliminating the fat-pointer coercion site. Bug 2 — CPACR_EL1.FPEN not initialised (bsp-qemu-virt/src/boot.s) QEMU virt drops the kernel to EL1. CPACR_EL1.FPEN resets to 0b00, which traps FP/SIMD at EL1 as Undefined Instruction. The compiler emits `movi v0.2d, #0` for zero-initialising the throwaway scheduler context; this trapped immediately. No VBAR was set, so the exception vector at 0x200 fetched zeros → silent hang. Fix: set CPACR_EL1.FPEN = 0b11 (bits[21:20] = 0x300000) in boot.s before zeroing BSS, followed by ISB. Bug 3 — context_switch_asm compiler prologue corrupts saved sp (bsp-qemu-virt/src/cpu.rs) Even with `#[inline(never)]`, the compiler generated a standard function prologue (`stp x29, x30, [sp, #-0x10]!`) before our asm, adjusting sp by -16 before we saved it. On context restore, sp was 16 bytes too low; the caller's (yield_now) epilogue read callee-saved registers from the wrong stack addresses, then `ret`-ed to garbage. Fix: convert `context_switch_asm` to `#[unsafe(naked)] extern "C"` with `naked_asm!`. No prologue/epilogue is generated; sp is saved and restored exactly as the caller left it. QEMU smoke output (debug build): umbrix: hello from kernel_main umbrix: starting cooperative scheduler umbrix: task A — iteration 0 umbrix: task B — iteration 0 umbrix: task A — iteration 1 umbrix: task B — iteration 1 umbrix: task A — iteration 2 umbrix: task B — iteration 2 umbrix: task A done; spinning Also: remove debug raw volatile UART writes from task_a; update T-004 → Done; advance current.md and phase-a.md to A6. Audit entries: UNSAFE-2026-0006 through UNSAFE-2026-0009 (existing). Host tests: 75 passed (umbrix-kernel --lib). Co-Authored-By: Claude Sonnet 4.6 --- bsp-qemu-virt/src/boot.s | 23 +- bsp-qemu-virt/src/cpu.rs | 89 +++---- bsp-qemu-virt/src/main.rs | 236 ++++++++++++++++-- .../phase-a/T-004-cooperative-scheduler.md | 35 +-- docs/roadmap/current.md | 15 +- docs/roadmap/phases/phase-a.md | 4 +- hal/src/cpu.rs | 14 +- 7 files changed, 321 insertions(+), 95 deletions(-) diff --git a/bsp-qemu-virt/src/boot.s b/bsp-qemu-virt/src/boot.s index ff18f8c..7fbf9e8 100644 --- a/bsp-qemu-virt/src/boot.s +++ b/bsp-qemu-virt/src/boot.s @@ -6,14 +6,17 @@ * * Responsibilities, in order: * 1. Set the stack pointer to __stack_top (from linker.ld). - * 2. Zero the BSS region [__bss_start, __bss_end), which is 8-byte + * 2. Enable FP/SIMD at EL1: set CPACR_EL1.FPEN = 0b11 so that the + * compiler-generated NEON instructions (e.g. movi/stp q-regs for + * zero-initialisation) do not trap as Undefined Instruction. + * 3. Zero the BSS region [__bss_start, __bss_end), which is 8-byte * aligned at both ends so 8-byte stores are safe. - * 3. Branch to kernel_entry (a Rust function marked extern "C"). - * 4. If kernel_entry ever returns (it should not), halt defensively. + * 4. Branch to kernel_entry (a Rust function marked extern "C"). + * 5. If kernel_entry ever returns (it should not), halt defensively. * - * The Exception Level is whatever QEMU hands us at entry; no EL - * manipulation here (per ADR-0012 v1). The DTB pointer in x0 is - * currently ignored. + * QEMU virt drops the kernel to EL1 before execution. The DTB pointer + * in x0 is currently ignored. No EL transition performed here + * (per ADR-0012 v1). */ .section .text.boot, "ax" @@ -24,6 +27,14 @@ _start: add x0, x0, :lo12:__stack_top mov sp, x0 + /* Enable FP/SIMD at EL1 and EL0 (CPACR_EL1.FPEN = 0b11 = bits[21:20]). + * 0x300000 = 3 << 20. CPACR_EL1 resets to zero, so only FPEN needs + * setting; all other fields remain 0 (no ZEN, no TTA traps). + * ISB ensures the write is visible before the first NEON instruction. */ + mov x0, #0x300000 + msr cpacr_el1, x0 + isb + adrp x0, __bss_start add x0, x0, :lo12:__bss_start adrp x1, __bss_end diff --git a/bsp-qemu-virt/src/cpu.rs b/bsp-qemu-virt/src/cpu.rs index 1458bf7..7bbaf1e 100644 --- a/bsp-qemu-virt/src/cpu.rs +++ b/bsp-qemu-virt/src/cpu.rs @@ -14,7 +14,7 @@ //! //! [ADR-0020]: https://github.com/cemililik/UmbrixOS/blob/main/docs/decisions/0020-cpu-trait-v2-context-switch.md -use core::arch::asm; +use core::arch::{asm, naked_asm}; use umbrix_hal::{ContextSwitch, CoreId, Cpu, IrqState}; @@ -149,53 +149,56 @@ pub struct Aarch64TaskContext { /// - The caller is responsible for disabling interrupts before calling this /// function. /// +/// `#[unsafe(naked)]` suppresses the compiler-generated prologue/epilogue so +/// that `sp` is saved and restored exactly, with no hidden adjustment. Without +/// it the compiler pushes a frame onto the stack before our asm runs, causing +/// the saved `sp` to be 16 bytes too low — the caller's epilogue then reads +/// callee-saved registers from the wrong stack addresses after a context switch. +/// +/// Registers arrive per AAPCS64: `current` → x0, `next` → x1. +/// x8 is used as a scratch register (caller-saved; the asm clobbers it). +/// /// Audit: UNSAFE-2026-0008. -unsafe fn context_switch_asm(current: *mut Aarch64TaskContext, next: *const Aarch64TaskContext) { +#[unsafe(naked)] +unsafe extern "C" fn context_switch_asm( + current: *mut Aarch64TaskContext, + next: *const Aarch64TaskContext, +) { // Field offsets within Aarch64TaskContext (repr(C)): // x19_x28 offset 0 (10 × 8 = 80 bytes) // fp offset 80 // lr offset 88 // sp offset 96 // - // We save sp via `mov x2, sp` because `str sp, [x0, #96]` is not valid - // in AArch64 — sp cannot be used as a source in most store instructions. - // SAFETY: inline assembly that saves/restores callee-saved registers. - // The register constraints and memory clobbers are stated explicitly. - // Audit: UNSAFE-2026-0008. - unsafe { - asm!( - // ── save current ──────────────────────────────────────────────── - "stp x19, x20, [{cur}, #0]", - "stp x21, x22, [{cur}, #16]", - "stp x23, x24, [{cur}, #32]", - "stp x25, x26, [{cur}, #48]", - "stp x27, x28, [{cur}, #64]", - "stp x29, x30, [{cur}, #80]", // fp, lr - "mov x8, sp", - "str x8, [{cur}, #96]", // sp - - // ── restore next ───────────────────────────────────────────────── - "ldr x8, [{nxt}, #96]", // sp - "mov sp, x8", - "ldp x29, x30, [{nxt}, #80]", // fp, lr - "ldp x27, x28, [{nxt}, #64]", - "ldp x25, x26, [{nxt}, #48]", - "ldp x23, x24, [{nxt}, #32]", - "ldp x21, x22, [{nxt}, #16]", - "ldp x19, x20, [{nxt}, #0]", - - // ret jumps to the lr we just loaded from `next`. - // For a task's first run, that lr was set to the entry fn by - // init_context_inner. - "ret", - - cur = in(reg) current, - nxt = in(reg) next, - // All callee-saved regs are clobbered by the restore. - out("x8") _, - options(nostack), - ); - } + // sp cannot appear as a source operand in most AArch64 store instructions, + // so we move it through x8 (a caller-saved scratch register). + naked_asm!( + // ── save current (x0) ─────────────────────────────────────────── + "stp x19, x20, [x0, #0]", + "stp x21, x22, [x0, #16]", + "stp x23, x24, [x0, #32]", + "stp x25, x26, [x0, #48]", + "stp x27, x28, [x0, #64]", + "stp x29, x30, [x0, #80]", // fp, lr + "mov x8, sp", + "str x8, [x0, #96]", // sp + + // ── restore next (x1) ─────────────────────────────────────────── + "ldr x8, [x1, #96]", // sp + "mov sp, x8", + "ldp x29, x30, [x1, #80]", // fp, lr + "ldp x27, x28, [x1, #64]", + "ldp x25, x26, [x1, #48]", + "ldp x23, x24, [x1, #32]", + "ldp x21, x22, [x1, #16]", + "ldp x19, x20, [x1, #0]", + + // ret jumps to the lr just loaded from `next`. + // On a task's first run that lr was set by init_context to the + // entry function; on subsequent runs it is the return address + // stored by the `bl context_switch_asm` in the previous yield. + "ret", + ); } // ─── ContextSwitch impl ─────────────────────────────────────────────────────── @@ -209,8 +212,8 @@ impl ContextSwitch for QemuVirtCpu { // the AAPCS64 callee-save contract. Audit: UNSAFE-2026-0008. unsafe { context_switch_asm( - current as *mut Aarch64TaskContext, - next as *const Aarch64TaskContext, + core::ptr::from_mut::(current), + core::ptr::from_ref::(next), ); } } diff --git a/bsp-qemu-virt/src/main.rs b/bsp-qemu-virt/src/main.rs index 96bf391..d971714 100644 --- a/bsp-qemu-virt/src/main.rs +++ b/bsp-qemu-virt/src/main.rs @@ -7,9 +7,10 @@ //! This crate is the bootable binary: it provides the reset vector //! (`_start`, assembled from `boot.s` via [`core::arch::global_asm!`]), //! the Rust entry `kernel_entry`, a panic handler, and the hardware -//! implementations of the HAL traits (currently only -//! [`umbrix_hal::Console`] via [`console::Pl011Uart`]; the remaining -//! trait implementations follow in later phases). +//! implementations of the HAL traits. The A5 milestone adds the +//! [`cpu::QemuVirtCpu`] implementation of [`umbrix_hal::Cpu`] and +//! [`umbrix_hal::ContextSwitch`], and a two-task cooperative scheduler +//! smoke test. //! //! The boot flow is documented in [`docs/architecture/boot.md`][boot-doc] //! and the memory-layout decisions in [ADR-0012][adr-0012]. @@ -20,12 +21,19 @@ #![no_std] #![no_main] +// Binary crate: `pub` items serve the linker (`#[no_mangle]`) rather than +// external consumers; `unreachable_pub` is therefore expected throughout. +#![allow(unreachable_pub, reason = "binary crate; pub items are for the linker")] use core::arch::global_asm; +use core::cell::UnsafeCell; use core::fmt::Write; +use core::mem::MaybeUninit; use core::panic::PanicInfo; use umbrix_hal::{Console, FmtWriter}; +use umbrix_kernel::obj::task::{create_task, Task, TaskArena}; +use umbrix_kernel::sched::Scheduler; mod console; mod cpu; @@ -42,30 +50,230 @@ use cpu::QemuVirtCpu; /// [adr-0012]: https://github.com/cemililik/UmbrixOS/blob/main/docs/decisions/0012-boot-flow-qemu-virt.md const PL011_UART_BASE: usize = 0x0900_0000; +// ─── StaticCell ─────────────────────────────────────────────────────────────── +// +// Task entry functions are `fn() -> !` — they cannot capture environment. +// The scheduler, CPU, and console are stored as immutable statics wrapping +// `UnsafeCell>` so all tasks can reach them without `static mut`. +// All accesses remain `unsafe`; safety is ensured by the single-core, +// cooperative execution model (no two tasks run simultaneously). + +/// `Sync` wrapper around `UnsafeCell>` for write-once globals. +/// +/// Written exactly once from `kernel_entry` (before `start()` is called). +/// Tasks then access the value through `assume_init_ref` / `assume_init_mut`. +/// All accesses are `unsafe`; this type only satisfies the `Sync` bound that +/// `static` requires. +struct StaticCell(UnsafeCell>); + +// SAFETY: Umbrix v1 is single-core and cooperative. No two tasks ever run +// simultaneously, so there are no data races on `StaticCell` contents. +// Audit: UNSAFE-2026-0001. +unsafe impl Sync for StaticCell {} + +impl StaticCell { + const fn new() -> Self { + Self(UnsafeCell::new(MaybeUninit::uninit())) + } +} + +// ─── Task-stack storage ─────────────────────────────────────────────────────── + +/// Aligned storage for one task's call stack. +/// +/// `#[repr(C, align(16))]` guarantees the 16-byte sp alignment required by +/// AAPCS64 at every function-call boundary. The inner array is wrapped in +/// `UnsafeCell` so the static need not be `mut`; all access is still `unsafe`. +#[repr(C, align(16))] +struct TaskStack(UnsafeCell<[u8; 4096]>); + +// SAFETY: single-core cooperative kernel; only one task touches each stack +// at a time. Audit: UNSAFE-2026-0001. +unsafe impl Sync for TaskStack {} + +impl TaskStack { + const fn new() -> Self { + Self(UnsafeCell::new([0u8; 4096])) + } + + /// Return a pointer one past the end of the stack (the initial sp value). + /// + /// # Safety + /// + /// The caller must ensure this `TaskStack` outlives every task that uses it. + unsafe fn top(&self) -> *mut u8 { + // SAFETY: add(4096) is the one-past-end sentinel, not a dereference. + // Caller guarantees the stack's lifetime exceeds the task. + unsafe { (*self.0.get()).as_mut_ptr().add(4096) } + } +} + +/// Stack for task A. +static TASK_A_STACK: TaskStack = TaskStack::new(); +/// Stack for task B. +static TASK_B_STACK: TaskStack = TaskStack::new(); + +/// The cooperative scheduler, concrete over the QEMU BSP CPU type. +/// +/// Written once in `kernel_entry` before `start()`. Tasks access it via +/// `SCHED` to yield control cooperatively. +static SCHED: StaticCell> = StaticCell::new(); + +/// The CPU handle — needed by `yield_now` to mask IRQs during the switch. +static CPU: StaticCell = StaticCell::new(); + +/// The PL011 console — used by task functions for diagnostic output. +static CONSOLE: StaticCell = StaticCell::new(); + +// ─── Task A ─────────────────────────────────────────────────────────────────── + +/// First smoke-test task. Prints its iteration index and yields three times, +/// then spins. The alternating output with task B confirms context switching. +fn task_a() -> ! { + for i in 0u32..3 { + // SAFETY: CONSOLE is fully initialised in `kernel_entry` before + // `start()` transfers control; no other task writes concurrently + // (cooperative scheduling). Audit: UNSAFE-2026-0001. + let console = unsafe { (*CONSOLE.0.get()).assume_init_ref() }; + let mut w = FmtWriter(console); + let _ = writeln!(w, "umbrix: task A — iteration {i}"); + + // SAFETY: SCHED and CPU are fully initialised before `start()`. + // Cooperative scheduling ensures no concurrent mutation. + // Audit: UNSAFE-2026-0001. + let sched = unsafe { (*SCHED.0.get()).assume_init_mut() }; + // SAFETY: same invariants as SCHED above. Audit: UNSAFE-2026-0001. + let cpu = unsafe { (*CPU.0.get()).assume_init_ref() }; + // yield_now returns Err only if current == None, which cannot + // happen once the scheduler has started. + let _ = sched.yield_now(cpu); + } + + // SAFETY: CONSOLE is fully initialised; no concurrent access. + // Audit: UNSAFE-2026-0001. + let console = unsafe { (*CONSOLE.0.get()).assume_init_ref() }; + console.write_bytes(b"umbrix: task A done; spinning\n"); + loop { + core::hint::spin_loop(); + } +} + +// ─── Task B ─────────────────────────────────────────────────────────────────── + +/// Second smoke-test task. Symmetric to task A. +fn task_b() -> ! { + for i in 0u32..3 { + // SAFETY: same invariants as task_a. Audit: UNSAFE-2026-0001. + let console = unsafe { (*CONSOLE.0.get()).assume_init_ref() }; + let mut w = FmtWriter(console); + let _ = writeln!(w, "umbrix: task B — iteration {i}"); + + // SAFETY: same invariants as task_a. Audit: UNSAFE-2026-0001. + let sched = unsafe { (*SCHED.0.get()).assume_init_mut() }; + // SAFETY: same invariants as task_a. Audit: UNSAFE-2026-0001. + let cpu = unsafe { (*CPU.0.get()).assume_init_ref() }; + let _ = sched.yield_now(cpu); + } + + // SAFETY: CONSOLE is fully initialised; no concurrent access. + // Audit: UNSAFE-2026-0001. + let console = unsafe { (*CONSOLE.0.get()).assume_init_ref() }; + console.write_bytes(b"umbrix: task B done; spinning\n"); + loop { + core::hint::spin_loop(); + } +} + +// ─── Boot entry ─────────────────────────────────────────────────────────────── + // Reset entry (`_start`). See `boot.s` and `docs/architecture/boot.md`. global_asm!(include_str!("boot.s")); /// First Rust entry after the assembly stub. /// -/// Constructs the BSP's concrete HAL implementations and hands the -/// portable kernel its console. This function never returns; the boot -/// stub halts defensively if it somehow does. +/// Initialises the console, CPU, and cooperative scheduler, registers two +/// smoke-test tasks, then transfers control to the scheduler. This function +/// never returns — the BSP reset stub halts defensively if it somehow does. +/// +/// # Panics +/// +/// Panics if the `TaskArena` cannot accommodate two tasks. The arena capacity +/// is 16, so in practice this branch is unreachable. #[unsafe(no_mangle)] pub extern "C" fn kernel_entry() -> ! { + // ── Hardware setup ──────────────────────────────────────────────────────── + // SAFETY: 0x0900_0000 is the well-known QEMU virt PL011 UART MMIO // base, exclusively owned by this kernel in v1 (single-core, no - // concurrent drivers). Alignment and addressability of the register - // window are guaranteed by the machine. - // Audit: UNSAFE-2026-0001. + // concurrent drivers). Audit: UNSAFE-2026-0001. let console = unsafe { Pl011Uart::new(PL011_UART_BASE) }; - // SAFETY: `QemuVirtCpu::new` requires at most one instance per physical - // core. We are single-core and this is the only call site. - // Audit: UNSAFE-2026-0006. let cpu = QemuVirtCpu::new(); - umbrix_kernel::run(&console, &cpu) + // Publish the console and CPU before any task can run. + // SAFETY: single-core; no concurrent writer exists before `start()`. + // Audit: UNSAFE-2026-0001. + unsafe { + (*CONSOLE.0.get()).write(console); + (*CPU.0.get()).write(cpu); + } + + // SAFETY: both cells were initialised in the block above. + // Audit: UNSAFE-2026-0001. + let console = unsafe { (*CONSOLE.0.get()).assume_init_ref() }; + // SAFETY: CPU cell was initialised above. Audit: UNSAFE-2026-0001. + let cpu = unsafe { (*CPU.0.get()).assume_init_ref() }; + + console.write_bytes(b"umbrix: hello from kernel_main\n"); + + // ── Kernel-object setup ─────────────────────────────────────────────────── + + let mut arena = TaskArena::default(); + // Infallible: arena capacity is 16 and we allocate 2 tasks. + let handle_a = create_task(&mut arena, Task::new(0)).ok().unwrap(); + let handle_b = create_task(&mut arena, Task::new(1)).ok().unwrap(); + + // ── Scheduler setup ─────────────────────────────────────────────────────── + + let mut sched = Scheduler::::new(); + + // SAFETY: add_task calls init_context; the stack tops are 16-byte aligned + // (guaranteed by TaskStack's repr) and remain valid for the process + // lifetime. Entry functions are `fn() -> !`. Audit: UNSAFE-2026-0009. + // + // Stack tops are computed inline to avoid introducing two local bindings + // with similar names that would trigger the `similar_names` lint. + unsafe { + sched + .add_task(cpu, handle_a, task_a, TASK_A_STACK.top()) + .ok(); + sched + .add_task(cpu, handle_b, task_b, TASK_B_STACK.top()) + .ok(); + } + + // Publish the scheduler before transferring control. + // SAFETY: single-core; no task is running yet. Audit: UNSAFE-2026-0001. + unsafe { + (*SCHED.0.get()).write(sched); + } + + console.write_bytes(b"umbrix: starting cooperative scheduler\n"); + + // Transfer control to the first ready task. Does not return. + // SAFETY: SCHED is fully initialised; the first task's context was set + // up by add_task above. Audit: UNSAFE-2026-0008. + unsafe { (*SCHED.0.get()).assume_init_mut() }.start(cpu); + + // Unreachable — start() switches away and the bootstrap context is never + // restored. The BSP reset stub halts the core if this line is somehow + // reached, which is a kernel bug. + loop { + core::hint::spin_loop(); + } } +// ─── Panic handler ──────────────────────────────────────────────────────────── + #[panic_handler] fn panic(info: &PanicInfo) -> ! { // SAFETY: constructing a fresh Pl011Uart in the panic path is @@ -78,8 +286,6 @@ fn panic(info: &PanicInfo) -> ! { console.write_bytes(b"\n!! umbrix panic !!\n"); let mut w = FmtWriter(&console); - // The Result is infallible for FmtWriter (ADR-0007) but the clippy - // lint `write_literal` wants the outcome handled explicitly. let _ = writeln!(w, "{info}"); loop { diff --git a/docs/analysis/tasks/phase-a/T-004-cooperative-scheduler.md b/docs/analysis/tasks/phase-a/T-004-cooperative-scheduler.md index e3d6eff..de75729 100644 --- a/docs/analysis/tasks/phase-a/T-004-cooperative-scheduler.md +++ b/docs/analysis/tasks/phase-a/T-004-cooperative-scheduler.md @@ -2,7 +2,7 @@ - **Phase:** A - **Milestone:** A5 — Cooperative scheduler and context switch -- **Status:** In Progress +- **Status:** Done - **Created:** 2026-04-21 - **Author:** @cemililik - **Dependencies:** T-003 — IPC primitives (Done) @@ -32,14 +32,14 @@ The actual assembly for saving and restoring aarch64 register state (callee-save - [x] **ADR-0019 Accepted** — 2026-04-21. Settles: single bounded FIFO queue, yield-to-next-ready, `TaskState { Idle, Ready, Blocked }`, scheduler as IPC orchestration layer. - [x] **ADR-0020 Accepted** — 2026-04-21. Settles: separate `ContextSwitch` trait, `unsafe context_switch` / `init_context`, aarch64 frame (x19–x28 + fp + lr + sp = 104 bytes). -- [ ] **`Cpu` trait v2** lands in `umbrix-hal`; the BSP `QemuVirtCpu` implements it. -- [ ] **Context-switch assembly** in `bsp-qemu-virt`, behind a safe Rust wrapper; `unsafe` block audited per [`unsafe-policy.md`](../../../standards/unsafe-policy.md). -- [ ] **Scheduler queue** in `kernel::sched`: bounded, heap-free. Shape decided by ADR-0019. -- [ ] **`yield_now` kernel operation**: moves the current task to the back of the ready queue and switches to the head. -- [ ] **IPC integration**: when `ipc_recv` finds no sender (returns `RecvOutcome::Pending`), the scheduler removes the calling task from the ready queue and parks it. When `ipc_send` delivers to a waiting receiver (returns `SendOutcome::Delivered`), the scheduler re-enqueues the receiver. -- [ ] **Host tests** for scheduler data structures (enqueue, dequeue, block, unblock). -- [ ] **QEMU smoke test**: two kernel-level tasks yield back and forth; console shows alternating output from each task. -- [ ] **No new `unsafe`** beyond the context-switch wrapper. If any additional `unsafe` lands, audit entry per [`unsafe-policy.md`](../../../standards/unsafe-policy.md). +- [x] **`Cpu` trait v2** lands in `umbrix-hal`; the BSP `QemuVirtCpu` implements it. +- [x] **Context-switch assembly** in `bsp-qemu-virt`, behind a safe Rust wrapper; `unsafe` block audited per [`unsafe-policy.md`](../../../standards/unsafe-policy.md). +- [x] **Scheduler queue** in `kernel::sched`: bounded, heap-free. Shape decided by ADR-0019. +- [x] **`yield_now` kernel operation**: moves the current task to the back of the ready queue and switches to the head. +- [x] **IPC integration**: when `ipc_recv` finds no sender (returns `RecvOutcome::Pending`), the scheduler removes the calling task from the ready queue and parks it. When `ipc_send` delivers to a waiting receiver (returns `SendOutcome::Delivered`), the scheduler re-enqueues the receiver. +- [x] **Host tests** for scheduler data structures (enqueue, dequeue, block, unblock). +- [x] **QEMU smoke test**: two kernel-level tasks yield back and forth; console shows alternating output from each task. +- [x] **No new `unsafe`** beyond the context-switch wrapper. If any additional `unsafe` lands, audit entry per [`unsafe-policy.md`](../../../standards/unsafe-policy.md). ## Out of scope @@ -61,14 +61,14 @@ Design is delegated to ADR-0019 and ADR-0020. At a sketch level: ## Definition of done -- [ ] `cargo fmt --all -- --check` clean. -- [ ] `cargo host-clippy` clean. -- [ ] `cargo kernel-clippy` clean. -- [ ] `cargo host-test` passes with new scheduler unit tests. -- [ ] QEMU smoke test runs and prints alternating task output (manual check or CI run). -- [ ] `unsafe` in context-switch wrapper has a `# Safety` section and an audit entry. -- [ ] Commit(s) follow [`commit-style.md`](../../../standards/commit-style.md). -- [ ] [`current.md`](../../../roadmap/current.md) updated on each status transition. +- [x] `cargo fmt --all -- --check` clean. +- [x] `cargo host-clippy` clean. +- [x] `cargo kernel-clippy` clean. +- [x] `cargo host-test` passes with new scheduler unit tests. +- [x] QEMU smoke test runs and prints alternating task output (manual check or CI run). +- [x] `unsafe` in context-switch wrapper has a `# Safety` section and an audit entry. +- [x] Commit(s) follow [`commit-style.md`](../../../standards/commit-style.md). +- [x] [`current.md`](../../../roadmap/current.md) updated on each status transition. ## Design notes @@ -92,3 +92,4 @@ Design is delegated to ADR-0019 and ADR-0020. At a sketch level: |------|----------|------| | 2026-04-21 | @cemililik | opened; status Draft — ADR-0019 and ADR-0020 not yet written; A5 blocked until both Accepted. | | 2026-04-21 | @cemililik | ADR-0019 and ADR-0020 both Accepted; status → Ready. Implementation may begin. | +| 2026-04-21 | @cemililik | Implementation complete; QEMU smoke confirms alternating A/B output × 3 iterations. Three bugs fixed: IrqGuard fat-pointer vtable → generic `IrqGuard`; CPACR_EL1.FPEN not set → NEON trap at EL1; context_switch_asm compiler prologue corrupted saved sp → `#[unsafe(naked)]`. Status → Done. | diff --git a/docs/roadmap/current.md b/docs/roadmap/current.md index af2ec82..293679c 100644 --- a/docs/roadmap/current.md +++ b/docs/roadmap/current.md @@ -5,18 +5,19 @@ A short pointer file updated as work progresses. For the full plan see [`phases/ --- - **Active phase:** A — Kernel core on QEMU `virt`. -- **Active milestone:** A5 — Cooperative scheduler and context switch. -- **Active task:** [T-004 — Cooperative scheduler](../analysis/tasks/phase-a/T-004-cooperative-scheduler.md) (status: **In Progress**). +- **Active milestone:** A6 — Two-task IPC demo. +- **Active task:** none yet; T-004 just closed. - **Working branch:** `development`. -- **Last completed milestone:** A4 — IPC primitives, on 2026-04-21 (PR merged to `main`). -- **Last completed task:** [T-003 — IPC primitives](../analysis/tasks/phase-a/T-003-ipc-primitives.md) — `Done` 2026-04-21. +- **Last completed milestone:** A5 — Cooperative scheduler and context switch, on 2026-04-21. +- **Last completed task:** [T-004 — Cooperative scheduler](../analysis/tasks/phase-a/T-004-cooperative-scheduler.md) — `Done` 2026-04-21. - **Last review:** [A2 completion business review](../analysis/reviews/business-reviews/2026-04-21-A2-completion.md) — 2026-04-21. -- **Next review trigger:** code + security review of T-004 when it reaches `In Review`; A4/A5 business review waits for A6 per [phase-a.md closure](phases/phase-a.md). +- **Next review trigger:** code + security review before A6 work lands; A4/A5/A6 business review at Phase A closure. ## Notes -- The capability subsystem (T-001), kernel-object subsystem (T-002), and IPC-primitive subsystem (T-003) form the Phase A stack. All three shipped with zero `unsafe` and no heap. None is wired into `run` yet; that is Phase-A later-milestone work. +- The capability subsystem (T-001), kernel-object subsystem (T-002), and IPC-primitive subsystem (T-003) form the Phase A stack. All three shipped with zero `unsafe` and no heap. - [ADR-0014](../decisions/0014-capability-representation.md), [ADR-0016](../decisions/0016-kernel-object-storage.md), and [ADR-0017](../decisions/0017-ipc-primitive-set.md) all Accepted. - T-003 (A4) delivered `ipc_send` / `ipc_recv` / `ipc_notify` with generation-tracked `IpcQueues`, atomic capability transfer, and TRANSFER-right enforcement. 64 host tests green; status → Done 2026-04-21. -- A5 (T-004) opens: cooperative scheduler with context switch. ADR-0019 (scheduler shape) and ADR-0020 (`Cpu` trait v2 / context-switch extension) both Accepted 2026-04-21. T-004 → Ready. +- T-004 (A5) delivered cooperative context switch (`#[unsafe(naked)]` aarch64 asm), `ContextSwitch` HAL trait, `Scheduler` with `yield_now`, and a QEMU smoke test showing two tasks alternating output across 3 iterations. 75 host tests green; QEMU smoke confirmed 2026-04-21. Status → Done 2026-04-21. +- Three implementation bugs required fixing in A5: IrqGuard fat-pointer vtable corruption → generic `IrqGuard`; CPACR_EL1.FPEN not initialised → NEON trap at EL1; context_switch_asm compiler prologue corrupted saved sp → `#[unsafe(naked)]`. - The maintainer updates this file when the active task changes. AI agents update it when they move a task to `In Progress` or `Done` via the [`start-task`](../../.claude/skills/start-task/SKILL.md) and [`conduct-review`](../../.claude/skills/conduct-review/SKILL.md) skills. diff --git a/docs/roadmap/phases/phase-a.md b/docs/roadmap/phases/phase-a.md index 1e43e31..17fd211 100644 --- a/docs/roadmap/phases/phase-a.md +++ b/docs/roadmap/phases/phase-a.md @@ -126,7 +126,7 @@ Milestone A5 needs IPC so that yield-to-peer makes sense; A6 demonstrates A4's o --- -## Milestone A5 — Cooperative scheduler and context switch +## Milestone A5 — Cooperative scheduler and context switch ✓ (done 2026-04-21) The first real scheduler: cooperative yield-based, with a context-switch primitive that swaps register state between kernel-level "tasks." No preemption, no timer tick yet. @@ -150,7 +150,7 @@ The first real scheduler: cooperative yield-based, with a context-switch primiti ### Tasks under A5 -- [T-004 — Cooperative scheduler](../../analysis/tasks/phase-a/T-004-cooperative-scheduler.md) — In Progress. +- [T-004 — Cooperative scheduler](../../analysis/tasks/phase-a/T-004-cooperative-scheduler.md) — Done. ### Informs diff --git a/hal/src/cpu.rs b/hal/src/cpu.rs index 08f9ee4..6ec3466 100644 --- a/hal/src/cpu.rs +++ b/hal/src/cpu.rs @@ -83,6 +83,10 @@ pub trait Cpu: Send + Sync { /// correctly — dropping an inner guard leaves the outer section's mask /// in place rather than fully re-enabling interrupts. /// +/// Generic over the concrete CPU type `C` to avoid fat-pointer vtable +/// dispatch, which can generate incorrect vtable references when the +/// coercion site is inlined near large `.rodata` constants. +/// /// # Example /// /// ```ignore @@ -92,23 +96,23 @@ pub trait Cpu: Send + Sync { /// // critical section: interrupts are masked here /// // `_g` drops at end of scope; interrupts restored to previous state /// ``` -pub struct IrqGuard<'a> { - cpu: &'a dyn Cpu, +pub struct IrqGuard<'a, C: Cpu> { + cpu: &'a C, prev: IrqState, } -impl<'a> IrqGuard<'a> { +impl<'a, C: Cpu> IrqGuard<'a, C> { /// Enter a critical section by masking interrupts on the current core. /// /// The interrupt state at the moment of construction is remembered and /// restored when the returned guard is dropped. - pub fn new(cpu: &'a dyn Cpu) -> Self { + pub fn new(cpu: &'a C) -> Self { let prev = cpu.disable_irqs(); Self { cpu, prev } } } -impl Drop for IrqGuard<'_> { +impl Drop for IrqGuard<'_, C> { fn drop(&mut self) { self.cpu.restore_irq_state(self.prev); } From 6f258fbe0a4c1c01938d201a50ac1b18c3d62407 Mon Sep 17 00:00:00 2001 From: Cemil ILIK Date: Tue, 21 Apr 2026 16:42:47 +0300 Subject: [PATCH 08/10] docs(standards): add BSP boot checklist and naked-fn rule; run-qemu --int-log MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three silent hangs during A5 debugging revealed gaps in documented boot requirements and unsafe coding rules. This commit closes those gaps so the same bugs cannot repeat unnoticed on future BSP targets. docs/standards/bsp-boot-checklist.md (new) Six-item ordered checklist for every new BSP: EL verification, CPACR_EL1 FP/SIMD enable, VBAR, SP alignment, BSS zeroing, and the naked-asm rule for context-switch functions. Each item explains what goes wrong if skipped and how to diagnose it. Includes a diagnostic cheat sheet mapping symptom → first check, and the QEMU -d int invocation. docs/standards/unsafe-policy.md — new §5a Codifies that any function saving/restoring SP in inline asm must be #[unsafe(naked)] with naked_asm!. Shows a correct and a broken example. Explains why #[inline(never)] is not sufficient (compiler still emits the stp x29,x30,[sp,#-N]! prologue). tools/run-qemu.sh — --int-log flag Passes -d int -D /tmp/qemu_int.log to QEMU when requested. Eliminates the need to remember or look up the QEMU exception-log flags mid-debug session. Co-Authored-By: Claude Sonnet 4.6 --- docs/standards/bsp-boot-checklist.md | 170 +++++++++++++++++++++++++++ docs/standards/unsafe-policy.md | 40 +++++++ tools/run-qemu.sh | 16 +++ 3 files changed, 226 insertions(+) create mode 100644 docs/standards/bsp-boot-checklist.md diff --git a/docs/standards/bsp-boot-checklist.md b/docs/standards/bsp-boot-checklist.md new file mode 100644 index 0000000..1bf3c90 --- /dev/null +++ b/docs/standards/bsp-boot-checklist.md @@ -0,0 +1,170 @@ +# BSP boot checklist + +Every new BSP target must satisfy this checklist before any Rust code runs. +Learned from A5 debugging: three silent hangs, each caused by a missing boot +step that was invisible without the previous one being fixed first. + +Work through the items in order — each failure mode masks the next. + +--- + +## 1. Exception level + +**Question:** At what EL does QEMU (or hardware) drop us? + +- QEMU `virt` → EL1 (not EL2; verified 2026-04-21). +- RPi4 → EL2 by default; must drop to EL1 before enabling kernel features. + +**Action:** Confirm EL in the BSP header comment and in the boot sequence ADR. +If EL transition is needed, do it before any other boot step. + +**What goes wrong if skipped:** System-register writes target the wrong EL; +writes silently have no effect or trap. + +--- + +## 2. FP/SIMD enable + +**Question:** Are FP/SIMD instructions enabled at the current EL? + +On aarch64 the reset value of `CPACR_EL1` is `0`, which traps every +FP/SIMD instruction at EL1 as an Undefined Instruction exception. The Rust +compiler routinely emits NEON instructions for zero-initialisation +(`movi v0.2d, #0`), struct copy, and even some integer operations at +higher optimization levels. + +**Action:** Set `CPACR_EL1.FPEN = 0b11` (bits[21:20]) before zeroing BSS +and before any Rust code runs. Follow with `ISB`. + +```asm +mov x0, #0x300000 // FPEN = 0b11 +msr cpacr_el1, x0 +isb +``` + +**What goes wrong if skipped:** The first NEON instruction raises an +Undefined Instruction exception. Without a configured VBAR the exception +vector is address 0x200; fetching zeros there causes an infinite hang with +no output and no error. + +**Diagnostic:** `qemu-system-aarch64 -d int -D /tmp/q.log`; look for +`Taking exception 1 [Undefined Instruction]` with `ESR 0x1fe00000` +(ISS = 0, IL = 1, EC = 0x07 = FP/SIMD trap at EL1). + +--- + +## 3. Exception vector (VBAR) + +**Question:** Is a vector base address configured? + +Until `VBAR_EL1` is set, any exception (undefined instruction, alignment, +prefetch abort) jumps to the reset value of VBAR (typically 0x0 or 0x200 +on QEMU `virt`), fetches whatever is there, and hangs silently. + +**Action:** Install a minimal exception vector **or** ensure that no +exception can occur in the boot path before Rust sets up a real handler. + +For A-phase kernel (no MMU, no interrupts): document that no exception is +expected in boot; rely on CPACR_EL1 and SP alignment to prevent them. +For B-phase and beyond: configure VBAR before enabling interrupts. + +**What goes wrong if skipped:** Any unexpected exception causes a silent hang. +The CPACR_EL1 bug (item 2) is the example: it was silent because VBAR was 0. + +--- + +## 4. Stack pointer alignment + +**Question:** Is SP 16-byte aligned before the first `bl` into Rust? + +AAPCS64 requires 16-byte SP alignment at every function-call boundary. +The linker symbol `__stack_top` must itself be 16-byte aligned; verify in +`linker.ld`. + +**Action:** Check that `__stack_top` alignment in the linker script is +`ALIGN(16)` or equivalent, and that the boot assembly sets `sp` from that +symbol before calling Rust. + +**What goes wrong if skipped:** Misaligned SP causes a stack-alignment +exception on the first `stp` or `ldp` with SP-relative addressing +(SP alignment check is optional in EL1 but enabled by some CPU configs). + +--- + +## 5. BSS zeroed before Rust entry + +**Question:** Is BSS zero-initialised before `kernel_entry` is called? + +Rust assumes `.bss` is zero. Any `static` or `static mut` with a zero +initializer lives in BSS. If BSS is not zeroed, those statics have garbage +values. + +**Action:** Zero `[__bss_start, __bss_end)` in the assembly stub, after SP +and CPACR_EL1 setup but before the first `bl` into Rust. Use 8-byte stores +(`str xzr, [x0], #8`) and confirm both symbols are 8-byte aligned in the +linker script. + +--- + +## 6. Context-switch assembly uses `#[unsafe(naked)]` + +**Question:** Does any function manipulate SP directly in inline asm? + +The compiler generates a standard function prologue +(`stp x29, x30, [sp, #-N]!`) for every non-naked function, adjusting SP +before inline asm runs. A context-switch routine that reads SP after the +prologue saves the wrong value; on restore the caller's stack frame is +misaligned and its epilogue reads saved registers from incorrect addresses. + +**Action:** Any function whose asm body saves or restores SP (or whose +correctness depends on SP having the caller's exact value) **must** be +`#[unsafe(naked)]`. Use `naked_asm!` as the sole function body. + +```rust +#[unsafe(naked)] +unsafe extern "C" fn context_switch_asm( + current: *mut TaskContext, + next: *const TaskContext, +) { + naked_asm!( + "mov x8, sp", + "str x8, [x0, #96]", // save caller's exact sp + // ... + "ret", + ); +} +``` + +`#[inline(never)]` alone is insufficient: the compiler still emits a prologue +for a regular function, even when it is not inlined. + +**What goes wrong if skipped:** Saved SP is 16 bytes (or N bytes) too low. +After a context restore the caller's epilogue reads callee-saved registers +from the wrong stack offsets, then `ret`s to a garbage address. Output stops +after the first yield. + +**Diagnostic:** Disassemble the function and check for +`stp x29, x30, [sp, #-N]!` before your asm. If present, add `#[unsafe(naked)]`. + +--- + +## Diagnostic cheat sheet + +| Symptom | First thing to check | +|---------|---------------------| +| Hangs before any output | CPACR_EL1.FPEN; run QEMU with `-d int` | +| Exception with ESR EC=0x07 | FP/SIMD trap — item 2 | +| Output stops after first yield | SP corruption in context switch — item 6; disassemble the switch function | +| Garbage `ret` address | Stack misalignment or wrong saved lr — item 6 | +| Static has non-zero garbage value | BSS not zeroed — item 5 | +| Exception jumps to 0x200 | VBAR not set — item 3 | + +### Enabling QEMU exception logging + +Add `--debug` to `tools/run-qemu.sh` or pass flags directly: + +```sh +qemu-system-aarch64 ... -d int -D /tmp/qemu_int.log +``` + +Then `grep "Taking exception" /tmp/qemu_int.log` to see what fired. diff --git a/docs/standards/unsafe-policy.md b/docs/standards/unsafe-policy.md index 1aa2582..b36c490 100644 --- a/docs/standards/unsafe-policy.md +++ b/docs/standards/unsafe-policy.md @@ -92,6 +92,46 @@ unsafe impl Send for PageFrame {} - **FFI at the kernel/HAL boundary.** Calls to assembly stubs or firmware services (PSCI, SMC). - **Intrinsics and inline assembly** that `cargo check` cannot reason about. +### 5a. Context-switch functions must use `#[unsafe(naked)]` + +Any function whose asm body saves or restores the stack pointer (SP), or +whose correctness depends on SP having the caller's exact value on entry, +**must** be declared `#[unsafe(naked)]` and use `naked_asm!` as its sole +body. Use `extern "C"` so arguments arrive in x0, x1, … per AAPCS64. + +**Why `#[inline(never)]` is not enough.** The compiler generates a standard +function prologue (`stp x29, x30, [sp, #-N]!`) for every non-naked function, +even when `#[inline(never)]` is set. This adjusts SP *before* inline asm +runs. A context-switch routine that reads SP after the prologue saves the +wrong value; on restore the caller's stack frame is misaligned by N bytes and +its epilogue reads callee-saved registers from incorrect addresses. + +```rust +// CORRECT — no prologue/epilogue; sp is exactly the caller's sp. +#[unsafe(naked)] +unsafe extern "C" fn context_switch_asm( + current: *mut TaskContext, + next: *const TaskContext, +) { + naked_asm!( + "mov x8, sp", + "str x8, [x0, #96]", + // … save/restore … + "ret", + ); +} + +// WRONG — compiler adds stp x29,x30,[sp,#-16]! before the asm; +// saved sp is 16 bytes too low. +#[inline(never)] +unsafe fn context_switch_asm_broken(…) { + unsafe { asm!("mov x8, sp", "str x8, [x0, #96]", …, options(nostack)); } +} +``` + +This rule is documented in `docs/standards/bsp-boot-checklist.md` §6 with +the diagnostic procedure. + ### 6. Where `unsafe` is not permitted - **Ergonomic shortcuts.** Bypassing a borrow check because it is inconvenient. diff --git a/tools/run-qemu.sh b/tools/run-qemu.sh index 3eb32c0..4ec1170 100755 --- a/tools/run-qemu.sh +++ b/tools/run-qemu.sh @@ -4,8 +4,13 @@ # Usage: # tools/run-qemu.sh — debug build # tools/run-qemu.sh --release — release build +# tools/run-qemu.sh --int-log — log exceptions to /tmp/qemu_int.log # tools/run-qemu.sh — explicit ELF path # +# --int-log adds -d int -D /tmp/qemu_int.log to the QEMU invocation. +# Use it when the kernel hangs silently to see what exception fired. +# After the run: grep "Taking exception" /tmp/qemu_int.log +# # See docs/guides/run-under-qemu.md for the full walkthrough and the # manual invocation used under the hood. @@ -13,12 +18,16 @@ set -euo pipefail BUILD_PROFILE="debug" KERNEL="" +INT_LOG="" for arg in "$@"; do case "$arg" in --release) BUILD_PROFILE="release" ;; + --int-log) + INT_LOG="yes" + ;; *) KERNEL="$arg" ;; @@ -42,6 +51,12 @@ if ! command -v qemu-system-aarch64 >/dev/null 2>&1; then exit 1 fi +INT_LOG_FLAGS=() +if [[ -n "$INT_LOG" ]]; then + INT_LOG_FLAGS=(-d int -D /tmp/qemu_int.log) + echo "exception log → /tmp/qemu_int.log (grep 'Taking exception' to inspect)" >&2 +fi + exec qemu-system-aarch64 \ -M virt \ -cpu cortex-a72 \ @@ -49,4 +64,5 @@ exec qemu-system-aarch64 \ -smp 1 \ -nographic \ -serial mon:stdio \ + "${INT_LOG_FLAGS[@]}" \ -kernel "$KERNEL" From f8dcc4bc278d7ddac31d76db157bffd693993b70 Mon Sep 17 00:00:00 2001 From: Cemil ILIK Date: Tue, 21 Apr 2026 17:04:25 +0300 Subject: [PATCH 09/10] fix(review): address post-A5 code review findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All findings from the post-A5 review verified against current code and fixed or explicitly documented. Summary by category: Safety / correctness - bsp-qemu-virt/src/cpu.rs: extend Aarch64TaskContext with d8_d15 [u64;8] and update context_switch_asm to save/restore AAPCS64 callee-saved SIMD registers d8–d15. Total context size: 104 → 168 bytes. Without this fix a task that causes NEON register allocation in d8–d15 would corrupt state across a cooperative yield. - kernel/src/sched/mod.rs: reorder add_task so ready.enqueue() is called before task_states/task_handles are set; a QueueFull error no longer leaves partial registration. - bsp-qemu-virt/src/main.rs: change add_task .ok() to .expect() so setup failures panic with a message instead of silently continuing. - bsp-qemu-virt/src/main.rs: restructure task_a/task_b yield calls so &mut Scheduler is not bound to a named variable that persists as a loop local across context switches. Add UNSAFE-2026-0012 audit entry documenting the deliberate aliasing relaxation under the cooperative model. API / type correctness - bsp-qemu-virt/src/cpu.rs: make QemuVirtCpu::new() unsafe to match its # Safety invariant (at most one instance per core). - kernel/src/sched/mod.rs: add Cpu bound to Scheduler struct declaration to match the existing impl blocks. Documentation / comments - bsp-qemu-virt/src/cpu.rs: fix disable_irqs SAFETY comment — the instruction is MSR DAIFSet, not MSR DAIF. - bsp-qemu-virt/src/cpu.rs: add (c) "why safer alternatives rejected" to Send/Sync SAFETY comments. - bsp-qemu-virt/src/boot.s: remove "CPACR_EL1 resets to zero" assertion; state we write FPEN explicitly without relying on reset value. - bsp-qemu-virt/src/main.rs: add (c) to StaticCell and TaskStack SAFETY comments; update audit IDs to UNSAFE-2026-0010 and UNSAFE-2026-0011. - hal/src/cpu.rs: reword IrqGuard doc comment to avoid an overly specific codegen claim about .rodata layout. - kernel/src/sched/mod.rs: add IRQ-state-on-entry doc to Scheduler::start; add self-yield fast-path invariant comment in yield_now; add single-waiter semantics doc to unblock_receiver_on. - docs/analysis/tasks/phase-a/T-004-cooperative-scheduler.md: fix ADR-0020 API references (save_context/restore_context → context_switch/ init_context; Cpu trait → ContextSwitch trait); update frame size comment. - docs/decisions/0019-scheduler-shape.md: replace "Cpu trait" with "ContextSwitch trait" in constraints and references sections. - docs/analysis/tasks/phase-a/README.md: T-004 status → Done (2026-04-21). Tests - kernel/src/sched/mod.rs: replace plain [u8; 512] stacks with AlignedStack<512> (#[repr(C, align(16))]) in Scheduler tests so the stack_top pointer satisfies init_context's 16-byte alignment contract. - kernel/src/sched/mod.rs: add debug_assert in ipc_recv_and_yield that the second ipc_recv call after resume never returns Pending. Audit log - docs/audits/unsafe-log.md: add UNSAFE-2026-0010 (StaticCell Sync), UNSAFE-2026-0011 (TaskStack Sync), UNSAFE-2026-0012 (&mut Scheduler aliasing across cooperative yield). Dead code - kernel/src/lib.rs: remove the stale pub fn run() stub, which printed a greeting and spun. kernel_entry() is the real entry point and the stub was never called. Co-Authored-By: Claude Sonnet 4.6 --- bsp-qemu-virt/src/boot.s | 7 +- bsp-qemu-virt/src/cpu.rs | 86 ++++++++++++++----- bsp-qemu-virt/src/main.rs | 79 +++++++++++------ docs/analysis/tasks/phase-a/README.md | 2 +- .../phase-a/T-004-cooperative-scheduler.md | 8 +- docs/audits/unsafe-log.md | 39 +++++++++ docs/decisions/0019-scheduler-shape.md | 4 +- hal/src/cpu.rs | 7 +- kernel/src/lib.rs | 20 ----- kernel/src/sched/mod.rs | 76 ++++++++++++---- 10 files changed, 233 insertions(+), 95 deletions(-) diff --git a/bsp-qemu-virt/src/boot.s b/bsp-qemu-virt/src/boot.s index 7fbf9e8..b7646c4 100644 --- a/bsp-qemu-virt/src/boot.s +++ b/bsp-qemu-virt/src/boot.s @@ -28,9 +28,10 @@ _start: mov sp, x0 /* Enable FP/SIMD at EL1 and EL0 (CPACR_EL1.FPEN = 0b11 = bits[21:20]). - * 0x300000 = 3 << 20. CPACR_EL1 resets to zero, so only FPEN needs - * setting; all other fields remain 0 (no ZEN, no TTA traps). - * ISB ensures the write is visible before the first NEON instruction. */ + * 0x300000 = 3 << 20. We do not rely on the reset value of CPACR_EL1; + * we write FPEN = 0b11 explicitly and leave all other fields zero (no + * ZEN, no TTA traps). ISB ensures the write takes effect before the + * first NEON instruction in BSS zeroing or Rust code. */ mov x0, #0x300000 msr cpacr_el1, x0 isb diff --git a/bsp-qemu-virt/src/cpu.rs b/bsp-qemu-virt/src/cpu.rs index 7bbaf1e..6e9ae41 100644 --- a/bsp-qemu-virt/src/cpu.rs +++ b/bsp-qemu-virt/src/cpu.rs @@ -35,10 +35,12 @@ impl QemuVirtCpu { /// /// There must be at most one `QemuVirtCpu` instance driving a given /// physical core. Creating a second instance on the same core and calling - /// `restore_irq_state` on both may produce inconsistent DAIF state. - /// In v1 (single-core), construct exactly once in `kernel_entry`. + /// `restore_irq_state` on both may produce inconsistent DAIF state — + /// the second instance's saved `IrqState` would reflect a different DAIF + /// snapshot and restoring it would silently override the first instance's + /// state. In v1 (single-core), construct exactly once in `kernel_entry`. #[must_use] - pub const fn new() -> Self { + pub const unsafe fn new() -> Self { Self { _priv: () } } } @@ -46,11 +48,19 @@ impl QemuVirtCpu { // SAFETY: `QemuVirtCpu` is a zero-size marker; it has no interior mutability // and holds no pointers. Sending it between threads is safe — the only shared // hardware resource (DAIF) is accessed via per-core system registers that are -// inherently thread-local in a single-core system. Audit: UNSAFE-2026-0006. +// inherently thread-local in a single-core system. +// Rejected alternatives: wrapping in a `Mutex` or `AtomicUsize` would add +// overhead with no benefit — DAIF is already a per-core register, not shared +// memory; there is nothing to protect with a software lock. +// Audit: UNSAFE-2026-0006. unsafe impl Send for QemuVirtCpu {} // SAFETY: Same reasoning as the `Send` impl — no interior mutability; DAIF -// reads/writes are atomic per-core register operations. Audit: UNSAFE-2026-0006. +// reads/writes are atomic per-core register operations. A `RefCell` or similar +// interior-mutability wrapper would not help because the resource (DAIF) is +// a hardware register, not a Rust data structure; the safe abstraction is +// already the `Cpu` trait methods. +// Audit: UNSAFE-2026-0006. unsafe impl Sync for QemuVirtCpu {} impl Cpu for QemuVirtCpu { @@ -71,9 +81,11 @@ impl Cpu for QemuVirtCpu { fn disable_irqs(&self) -> IrqState { let daif: usize; - // SAFETY: `MRS x, DAIF` reads the current interrupt mask; `MSR DAIF, #0xF` - // masks all DAIF bits (D, A, I, F). Both are EL1-privileged register - // operations. The returned `IrqState` captures the prior value so that + // SAFETY: `MRS x, DAIF` reads the current interrupt mask; `MSR DAIFSet, #0xf` + // sets all four DAIF mask bits (D, A, I, F) atomically via the write-only + // DAIFSet encoding — this is distinct from `MSR DAIF, #imm` which would + // require a 9-bit immediate. Both are EL1-privileged register operations. + // The returned `IrqState` captures the prior value so that // `restore_irq_state` can restore it exactly. Audit: UNSAFE-2026-0007. unsafe { asm!( @@ -122,7 +134,15 @@ impl Cpu for QemuVirtCpu { /// Layout must match the field offsets used by [`context_switch_asm`]. /// `#[repr(C)]` prevents field reordering. /// -/// Total size: 13 × 8 = 104 bytes per task context. +/// Per AAPCS64, callee-saved registers are: +/// - General-purpose: x19–x28, x29 (fp), x30 (lr), sp +/// - SIMD/FP: the lower 64 bits of v8–v15 (i.e. d8–d15) +/// +/// d8–d15 must be saved whenever CPACR_EL1.FPEN is non-zero, because the +/// compiler may allocate those registers for any kernel-level task and will +/// not emit callee-save spills across a cooperative yield. +/// +/// Total size: (10 + 1 + 1 + 1) × 8 + 8 × 8 = 104 + 64 = 168 bytes. #[derive(Default)] #[repr(C)] pub struct Aarch64TaskContext { @@ -134,18 +154,25 @@ pub struct Aarch64TaskContext { pub lr: u64, /// Stack pointer — saved explicitly (not a general-purpose register). pub sp: u64, + /// Lower 64 bits of `v8`–`v15` (`d8`–`d15`): AAPCS64 callee-saved + /// SIMD/FP registers. Only the lower 8 bytes need to be preserved; + /// the upper 64 bits are caller-saved. + pub d8_d15: [u64; 8], } // ─── context_switch_asm ────────────────────────────────────────────────────── -/// Save `x19`–`x28`, fp, lr, sp into `*current` and restore from `*next`. +/// Save all AAPCS64 callee-saved registers into `*current` and restore from `*next`. +/// +/// Saves: x19–x28, x29 (fp), x30 (lr), sp, d8–d15. +/// Restores: same set from `*next`, then `ret`s to the restored lr. /// /// # Safety /// /// - Both pointers must be 8-byte-aligned and valid for the duration of the /// switch. /// - `next` must have been written by a prior call to `context_switch_asm` -/// or fully initialised by `init_context_inner`. +/// or fully initialised by [`QemuVirtCpu::init_context`]. /// - The caller is responsible for disabling interrupts before calling this /// function. /// @@ -156,7 +183,7 @@ pub struct Aarch64TaskContext { /// callee-saved registers from the wrong stack addresses after a context switch. /// /// Registers arrive per AAPCS64: `current` → x0, `next` → x1. -/// x8 is used as a scratch register (caller-saved; the asm clobbers it). +/// x8 is used as a scratch register (caller-saved; clobbered by the asm). /// /// Audit: UNSAFE-2026-0008. #[unsafe(naked)] @@ -169,29 +196,42 @@ unsafe extern "C" fn context_switch_asm( // fp offset 80 // lr offset 88 // sp offset 96 + // d8_d15 offset 104 ( 8 × 8 = 64 bytes) + // total 168 bytes // // sp cannot appear as a source operand in most AArch64 store instructions, // so we move it through x8 (a caller-saved scratch register). + // + // d8–d15 are saved as 64-bit values (lower half of v8–v15). AAPCS64 + // requires preserving only the lower 8 bytes of each v8–v15 register. naked_asm!( - // ── save current (x0) ─────────────────────────────────────────── + // ── save current (x0) ───────────────────────────────────────── "stp x19, x20, [x0, #0]", "stp x21, x22, [x0, #16]", "stp x23, x24, [x0, #32]", "stp x25, x26, [x0, #48]", "stp x27, x28, [x0, #64]", - "stp x29, x30, [x0, #80]", // fp, lr + "stp x29, x30, [x0, #80]", // fp, lr "mov x8, sp", - "str x8, [x0, #96]", // sp + "str x8, [x0, #96]", // sp + "stp d8, d9, [x0, #104]", + "stp d10, d11, [x0, #120]", + "stp d12, d13, [x0, #136]", + "stp d14, d15, [x0, #152]", - // ── restore next (x1) ─────────────────────────────────────────── - "ldr x8, [x1, #96]", // sp + // ── restore next (x1) ───────────────────────────────────────── + "ldp d14, d15, [x1, #152]", + "ldp d12, d13, [x1, #136]", + "ldp d10, d11, [x1, #120]", + "ldp d8, d9, [x1, #104]", + "ldr x8, [x1, #96]", // sp "mov sp, x8", - "ldp x29, x30, [x1, #80]", // fp, lr - "ldp x27, x28, [x1, #64]", - "ldp x25, x26, [x1, #48]", - "ldp x23, x24, [x1, #32]", - "ldp x21, x22, [x1, #16]", - "ldp x19, x20, [x1, #0]", + "ldp x29, x30, [x1, #80]", // fp, lr + "ldp x27, x28, [x1, #64]", + "ldp x25, x26, [x1, #48]", + "ldp x23, x24, [x1, #32]", + "ldp x21, x22, [x1, #16]", + "ldp x19, x20, [x1, #0]", // ret jumps to the lr just loaded from `next`. // On a task's first run that lr was set by init_context to the diff --git a/bsp-qemu-virt/src/main.rs b/bsp-qemu-virt/src/main.rs index d971714..20392eb 100644 --- a/bsp-qemu-virt/src/main.rs +++ b/bsp-qemu-virt/src/main.rs @@ -66,9 +66,13 @@ const PL011_UART_BASE: usize = 0x0900_0000; /// `static` requires. struct StaticCell(UnsafeCell>); -// SAFETY: Umbrix v1 is single-core and cooperative. No two tasks ever run +// SAFETY: Umbrix v1 is single-core and cooperative; no two tasks ever run // simultaneously, so there are no data races on `StaticCell` contents. -// Audit: UNSAFE-2026-0001. +// Rejected alternatives: `Mutex` / `RwLock` require a runtime (heap, OS) or +// a spin implementation that itself relies on `unsafe` and adds overhead +// inappropriate for a bare-metal `static`. `OnceCell` / `LazyCell` from +// `core` are not available in `no_std` without an allocator in A5. +// Audit: UNSAFE-2026-0010. unsafe impl Sync for StaticCell {} impl StaticCell { @@ -87,8 +91,14 @@ impl StaticCell { #[repr(C, align(16))] struct TaskStack(UnsafeCell<[u8; 4096]>); -// SAFETY: single-core cooperative kernel; only one task touches each stack -// at a time. Audit: UNSAFE-2026-0001. +// SAFETY: single-core cooperative kernel; only one task touches each stack at +// a time, and no task can interrupt another (cooperative scheduling). +// Rejected alternatives: wrapping in `Mutex` would add lock overhead and +// require a runtime or spin implementation. Making the static `mut` would +// expose the interior to safe code via `static mut` aliasing, which is +// worse. `UnsafeCell` with manual discipline is the standard bare-metal +// pattern and is the minimal wrapper that satisfies the `Sync` bound. +// Audit: UNSAFE-2026-0011. unsafe impl Sync for TaskStack {} impl TaskStack { @@ -133,24 +143,40 @@ fn task_a() -> ! { for i in 0u32..3 { // SAFETY: CONSOLE is fully initialised in `kernel_entry` before // `start()` transfers control; no other task writes concurrently - // (cooperative scheduling). Audit: UNSAFE-2026-0001. + // (cooperative scheduling). Audit: UNSAFE-2026-0010. let console = unsafe { (*CONSOLE.0.get()).assume_init_ref() }; let mut w = FmtWriter(console); let _ = writeln!(w, "umbrix: task A — iteration {i}"); - // SAFETY: SCHED and CPU are fully initialised before `start()`. - // Cooperative scheduling ensures no concurrent mutation. - // Audit: UNSAFE-2026-0001. - let sched = unsafe { (*SCHED.0.get()).assume_init_mut() }; - // SAFETY: same invariants as SCHED above. Audit: UNSAFE-2026-0001. - let cpu = unsafe { (*CPU.0.get()).assume_init_ref() }; - // yield_now returns Err only if current == None, which cannot - // happen once the scheduler has started. - let _ = sched.yield_now(cpu); + // SAFETY: SCHED and CPU are both fully initialised before `start()`. + // + // Aliasing note (Audit: UNSAFE-2026-0012): `assume_init_mut` creates + // a `&mut Scheduler` that is technically alive when `yield_now` + // suspends this task and another task creates its own `&mut Scheduler`. + // This relaxes Rust's strict aliasing rules. It is safe under the + // single-core cooperative model because: + // (a) no two tasks execute simultaneously — there is no concurrent + // memory access; + // (b) `yield_now` does not observe `self` after the context switch + // returns (the only post-switch code is the `IrqGuard` drop and + // the `Ok(())` return, both of which operate on stack locals + // within yield_now's own frame, not on `self`). + // The `&mut` is not bound to a named variable so its scope is limited + // to the duration of the `yield_now` call expression. + // A raw-pointer API would eliminate the aliasing entirely; that refactor + // is deferred to a future ADR. Audit: UNSAFE-2026-0010. + // + // yield_now returns Err only when current == None, which cannot happen + // once the scheduler has started. + unsafe { + let _ = (*SCHED.0.get()) + .assume_init_mut() + .yield_now((*CPU.0.get()).assume_init_ref()); + } } // SAFETY: CONSOLE is fully initialised; no concurrent access. - // Audit: UNSAFE-2026-0001. + // Audit: UNSAFE-2026-0010. let console = unsafe { (*CONSOLE.0.get()).assume_init_ref() }; console.write_bytes(b"umbrix: task A done; spinning\n"); loop { @@ -163,20 +189,21 @@ fn task_a() -> ! { /// Second smoke-test task. Symmetric to task A. fn task_b() -> ! { for i in 0u32..3 { - // SAFETY: same invariants as task_a. Audit: UNSAFE-2026-0001. + // SAFETY: same invariants as task_a. Audit: UNSAFE-2026-0010. let console = unsafe { (*CONSOLE.0.get()).assume_init_ref() }; let mut w = FmtWriter(console); let _ = writeln!(w, "umbrix: task B — iteration {i}"); - // SAFETY: same invariants as task_a. Audit: UNSAFE-2026-0001. - let sched = unsafe { (*SCHED.0.get()).assume_init_mut() }; - // SAFETY: same invariants as task_a. Audit: UNSAFE-2026-0001. - let cpu = unsafe { (*CPU.0.get()).assume_init_ref() }; - let _ = sched.yield_now(cpu); + // SAFETY: same aliasing invariants as task_a. Audit: UNSAFE-2026-0012. + unsafe { + let _ = (*SCHED.0.get()) + .assume_init_mut() + .yield_now((*CPU.0.get()).assume_init_ref()); + } } // SAFETY: CONSOLE is fully initialised; no concurrent access. - // Audit: UNSAFE-2026-0001. + // Audit: UNSAFE-2026-0010. let console = unsafe { (*CONSOLE.0.get()).assume_init_ref() }; console.write_bytes(b"umbrix: task B done; spinning\n"); loop { @@ -207,7 +234,9 @@ pub extern "C" fn kernel_entry() -> ! { // base, exclusively owned by this kernel in v1 (single-core, no // concurrent drivers). Audit: UNSAFE-2026-0001. let console = unsafe { Pl011Uart::new(PL011_UART_BASE) }; - let cpu = QemuVirtCpu::new(); + // SAFETY: constructed exactly once in kernel_entry; single-core v1. + // See QemuVirtCpu::new # Safety. Audit: UNSAFE-2026-0006. + let cpu = unsafe { QemuVirtCpu::new() }; // Publish the console and CPU before any task can run. // SAFETY: single-core; no concurrent writer exists before `start()`. @@ -245,10 +274,10 @@ pub extern "C" fn kernel_entry() -> ! { unsafe { sched .add_task(cpu, handle_a, task_a, TASK_A_STACK.top()) - .ok(); + .expect("add_task A failed: queue full or arena exhausted"); sched .add_task(cpu, handle_b, task_b, TASK_B_STACK.top()) - .ok(); + .expect("add_task B failed: queue full or arena exhausted"); } // Publish the scheduler before transferring control. diff --git a/docs/analysis/tasks/phase-a/README.md b/docs/analysis/tasks/phase-a/README.md index 25ba054..cab35cf 100644 --- a/docs/analysis/tasks/phase-a/README.md +++ b/docs/analysis/tasks/phase-a/README.md @@ -9,6 +9,6 @@ Tasks belonging to [Phase A — Kernel core on QEMU `virt`](../../../roadmap/pha | [T-001](T-001-capability-table-foundation.md) | Capability table foundation | A2 | Done | | [T-002](T-002-kernel-object-storage.md) | Kernel object storage foundation | A3 | Done | | [T-003](T-003-ipc-primitives.md) | IPC primitives | A4 | Done | -| [T-004](T-004-cooperative-scheduler.md) | Cooperative scheduler | A5 | In Progress | +| [T-004](T-004-cooperative-scheduler.md) | Cooperative scheduler | A5 | Done (2026-04-21) | Tasks are added here as they become active. See [`../../../roadmap/phases/phase-a.md`](../../../roadmap/phases/phase-a.md) for the full phase plan. diff --git a/docs/analysis/tasks/phase-a/T-004-cooperative-scheduler.md b/docs/analysis/tasks/phase-a/T-004-cooperative-scheduler.md index de75729..030d177 100644 --- a/docs/analysis/tasks/phase-a/T-004-cooperative-scheduler.md +++ b/docs/analysis/tasks/phase-a/T-004-cooperative-scheduler.md @@ -22,7 +22,7 @@ T-003 gave us `ipc_send` / `ipc_recv` / `ipc_notify` with correct waiter-state m The scheduler itself is deliberately simple: cooperative yield (no preemption, no timer tick in A5). Two design decisions drive the shape: - **ADR-0019** settles the scheduler's data structure (queue type, yield semantics, blocked-task representation). -- **ADR-0020** extends the [`umbrix-hal::Cpu`](../../../hal/src/cpu.rs) trait with `save_context` / `restore_context` and a `TaskContext` associated type, so the context-switch assembly lives in the BSP rather than the kernel crate. +- **ADR-0020** introduces a separate [`umbrix-hal::ContextSwitch`](../../../hal/src/context_switch.rs) trait with `unsafe context_switch` / `init_context` and a `TaskContext` associated type, so the context-switch assembly lives in the BSP rather than the kernel crate. The actual assembly for saving and restoring aarch64 register state (callee-saved registers, SP, LR/PC) lives in `bsp-qemu-virt` behind a safe Rust wrapper with a documented `# Safety` contract. The kernel crate calls the HAL trait and stays `unsafe`-free. @@ -31,8 +31,8 @@ The actual assembly for saving and restoring aarch64 register state (callee-save ## Acceptance criteria - [x] **ADR-0019 Accepted** — 2026-04-21. Settles: single bounded FIFO queue, yield-to-next-ready, `TaskState { Idle, Ready, Blocked }`, scheduler as IPC orchestration layer. -- [x] **ADR-0020 Accepted** — 2026-04-21. Settles: separate `ContextSwitch` trait, `unsafe context_switch` / `init_context`, aarch64 frame (x19–x28 + fp + lr + sp = 104 bytes). -- [x] **`Cpu` trait v2** lands in `umbrix-hal`; the BSP `QemuVirtCpu` implements it. +- [x] **ADR-0020 Accepted** — 2026-04-21. Settles: separate `ContextSwitch` trait, `unsafe context_switch` / `init_context`, aarch64 frame (x19–x28 + fp + lr + sp + d8–d15 = 168 bytes). +- [x] **`ContextSwitch` trait** lands in `umbrix-hal`; the BSP `QemuVirtCpu` implements it alongside `Cpu`. - [x] **Context-switch assembly** in `bsp-qemu-virt`, behind a safe Rust wrapper; `unsafe` block audited per [`unsafe-policy.md`](../../../standards/unsafe-policy.md). - [x] **Scheduler queue** in `kernel::sched`: bounded, heap-free. Shape decided by ADR-0019. - [x] **`yield_now` kernel operation**: moves the current task to the back of the ready queue and switches to the head. @@ -73,7 +73,7 @@ Design is delegated to ADR-0019 and ADR-0020. At a sketch level: ## Design notes - **Why cooperative-only?** Preemption requires a timer IRQ and safe IRQ entry/exit, which pulls in interrupt handling before the scheduler is even proven. Starting cooperative keeps the first context switch auditable and testable without hardware interrupt complexity. -- **Why `Cpu` trait extension rather than a separate trait?** The context-switch primitive is a fundamental CPU operation, like `write_bytes` on `Console`. Extending `Cpu` keeps the HAL surface minimal and avoids a proliferation of single-method traits. ADR-0020 may decide otherwise if the extension is large or awkward. +- **Why a separate `ContextSwitch` trait rather than extending `Cpu`?** ADR-0020 settled this: `Cpu` is object-safe (`&dyn Cpu` is used in the kernel for IRQ masking) whereas `ContextSwitch` has an associated type (`TaskContext`) that makes it non-object-safe. Merging the two would require removing `&dyn Cpu` usage or adding complex workarounds. A separate trait keeps each interface coherent and independently usable. - **Safety of context switch.** The save/restore assembly is the first `unsafe` in the kernel that is not structurally impossible to make safe. The invariants (stack pointer valid, registers stable, interrupts disabled during switch) must be stated explicitly and checked in review. - **IPC bridge complexity.** Parking a task on `RecvOutcome::Pending` requires knowing which task is the caller — in A5, "task" is a kernel-level stub with an ID and a `TaskContext`; the scheduler maps task ID to ready/blocked state. This is the first time the kernel has a concept of "current task." diff --git a/docs/audits/unsafe-log.md b/docs/audits/unsafe-log.md index cf81ece..8dc7520 100644 --- a/docs/audits/unsafe-log.md +++ b/docs/audits/unsafe-log.md @@ -117,3 +117,42 @@ Entries are **append-only**. When an `unsafe` region is removed, its entry gains - **Rejected alternatives:** Initialising a context requires writing raw register values; no safe abstraction exists. - **Reviewed by:** @cemililik. - **Status:** Active. + +### UNSAFE-2026-0010 — `unsafe impl Sync for StaticCell` + +- **Introduced:** 2026-04-21, T-004 / A5 BSP bootstrap. +- **Location:** [`bsp-qemu-virt/src/main.rs`](../../bsp-qemu-virt/src/main.rs) — `unsafe impl Sync for StaticCell`. +- **Operation:** Declares that `&StaticCell` can be shared across threads, allowing `StaticCell` to appear in `static` position. +- **Invariants relied on:** + - Umbrix v1 is single-core and cooperative: no two tasks ever run simultaneously, so no two threads can reach a `StaticCell` concurrently. + - Each cell is written exactly once from `kernel_entry` before `start()` is called; subsequent accesses are read-only (via `assume_init_ref`) or guarded by the cooperative schedule. +- **Rejected alternatives:** `Mutex` / `RwLock` require a runtime or a spin implementation that itself uses `unsafe`; using them would defer rather than eliminate the unsafety. `OnceCell` / `LazyLock` are not available without `std` in A5. `static mut` would expose the interior to safe code via aliasing. +- **Reviewed by:** @cemililik. +- **Status:** Active. + +### UNSAFE-2026-0011 — `unsafe impl Sync for TaskStack` + +- **Introduced:** 2026-04-21, T-004 / A5 BSP bootstrap. +- **Location:** [`bsp-qemu-virt/src/main.rs`](../../bsp-qemu-virt/src/main.rs) — `unsafe impl Sync for TaskStack`. +- **Operation:** Declares that `&TaskStack` can be shared across threads, allowing `static TASK_A_STACK` / `TASK_B_STACK` to satisfy the `Sync` bound on `static`. +- **Invariants relied on:** + - Single-core cooperative kernel: only one task uses each stack at a time. + - The inner `UnsafeCell<[u8; 4096]>` is only accessed via `TaskStack::top`, which returns a raw pointer; no safe reference to the interior is ever materialised. + - Stack lifetimes exceed the tasks that use them (static storage). +- **Rejected alternatives:** Wrapping in `Mutex` adds lock overhead inappropriate for a bare-metal stack. `static mut` exposes the interior unsafely and makes aliasing analysis harder. `UnsafeCell` with manual discipline is the minimal and standard pattern for bare-metal static storage. +- **Reviewed by:** @cemililik. +- **Status:** Active. + +### UNSAFE-2026-0012 — `&mut Scheduler` aliasing across cooperative yield + +- **Introduced:** 2026-04-21, T-004 / A5 BSP bootstrap. +- **Location:** [`bsp-qemu-virt/src/main.rs`](../../bsp-qemu-virt/src/main.rs) — `task_a`, `task_b` — `assume_init_mut().yield_now(...)` call. +- **Operation:** `(*SCHED.0.get()).assume_init_mut()` creates a `&mut Scheduler` that is technically alive across the cooperative context switch inside `yield_now`. When another task calls the same expression, a second `&mut Scheduler` is derived from the same `UnsafeCell`, creating aliased mutable references — undefined behaviour under Rust's strict aliasing rules. +- **Invariants relied on:** + - Single-core cooperative model: no two tasks execute simultaneously; there is no concurrent access to the Scheduler's memory. + - The `&mut` is not bound to a named variable; its scope is limited to the single `yield_now` call expression. After `yield_now` suspends, the scheduler's data is not modified by the suspended frame's stack. + - `yield_now` does not read `self` after the `cpu.context_switch` call within its body (only the `IrqGuard` drop and `Ok(())` return occur, both stack-local). + - LLVM's context switch (`naked_asm!` with `ret`) acts as a full memory barrier, preventing the compiler from caching or reordering accesses across the switch point. +- **Rejected alternatives:** A raw-pointer API (`yield_raw(*mut Scheduler, &C)`) would eliminate the aliasing entirely by ensuring no `&mut Scheduler` is live across the context switch. This refactor is the correct long-term fix but requires restructuring the BSP task functions and potentially the Scheduler API; it is deferred to a future ADR. A `Mutex` would introduce lock overhead and a blocking primitive before the kernel has blocking support. +- **Reviewed by:** @cemililik. +- **Status:** Active — to be resolved by raw-pointer API refactor (future ADR). diff --git a/docs/decisions/0019-scheduler-shape.md b/docs/decisions/0019-scheduler-shape.md index fb33d07..f83fb82 100644 --- a/docs/decisions/0019-scheduler-shape.md +++ b/docs/decisions/0019-scheduler-shape.md @@ -20,7 +20,7 @@ Before any implementation lands, four inter-related questions must be settled: - All kernel state is statically bounded — no heap ([ADR-0016](0016-kernel-object-storage.md)). - The kernel crate is `no_std`, HAL-dependent only through traits ([ADR-0006](0006-workspace-layout.md), [ADR-0008](0008-cpu-trait.md)). - IPC is cooperative in A4/A5; preemption is Phase B ([ADR-0017](0017-ipc-primitive-set.md)). -- Context-switch assembly lives in the BSP, exposed through the `Cpu` trait (ADR-0020); the scheduler calls the trait, not the assembly. +- Context-switch assembly lives in the BSP, exposed through the `ContextSwitch` trait (ADR-0020); the scheduler calls the trait, not the assembly. - Only one CPU core is in scope (single-core aarch64 QEMU `virt`). **A5 scale.** Two kernel-level task stubs, cooperative yield, no timer tick. Complexity should match scale: a design that works perfectly for 2 tasks and is straightforward to extend later is better than a general scheduler written before there is a workload to validate it against. @@ -212,7 +212,7 @@ impl Scheduler { ## References - [ADR-0017: IPC primitive set](0017-ipc-primitive-set.md) — the IPC layer this scheduler wires up. -- [ADR-0008: `Cpu` HAL trait v1](0008-cpu-trait.md) — v2 (ADR-0020) extends this with context-switch primitives. +- [ADR-0008: `Cpu` HAL trait v1](0008-cpu-trait.md) — ADR-0020 introduces the separate `ContextSwitch` trait alongside it. - [T-004: Cooperative scheduler](../analysis/tasks/phase-a/T-004-cooperative-scheduler.md) — the task this ADR gates. - seL4 scheduler — strict priority, bounded queues; Phase B reference. - Hubris task-dispatch model — cooperative, bounded task set; closest prior art to the A5 design. diff --git a/hal/src/cpu.rs b/hal/src/cpu.rs index 6ec3466..3f69649 100644 --- a/hal/src/cpu.rs +++ b/hal/src/cpu.rs @@ -84,8 +84,11 @@ pub trait Cpu: Send + Sync { /// in place rather than fully re-enabling interrupts. /// /// Generic over the concrete CPU type `C` to avoid fat-pointer vtable -/// dispatch, which can generate incorrect vtable references when the -/// coercion site is inlined near large `.rodata` constants. +/// dispatch on critical-section paths. Dynamic dispatch (`&dyn Cpu`) is also +/// avoided because coercing a concrete type to a trait object at certain +/// inlining depths can produce vtable references that alias unrelated data +/// in `.rodata`; using a concrete type parameter eliminates the coercion site +/// entirely. /// /// # Example /// diff --git a/kernel/src/lib.rs b/kernel/src/lib.rs index b4d16cb..4184b08 100644 --- a/kernel/src/lib.rs +++ b/kernel/src/lib.rs @@ -46,23 +46,3 @@ pub mod cap; pub mod ipc; pub mod obj; pub mod sched; - -use umbrix_hal::{Console, Cpu}; - -/// Portable kernel entry, called by the BSP after early init. -/// -/// Accepts the BSP's console and CPU implementations. In the current phase -/// (A5) this prints a greeting and idles. Future milestones will wire up -/// the scheduler, task creation, and IPC here. -/// -/// # Never returns -/// -/// This function is `-> !`. A return would be a kernel bug; the BSP's -/// reset stub halts defensively if it ever does. -pub fn run(console: &Con, _cpu: &C) -> ! { - console.write_bytes(b"umbrix: hello from kernel_main\n"); - - loop { - core::hint::spin_loop(); - } -} diff --git a/kernel/src/sched/mod.rs b/kernel/src/sched/mod.rs index 55aeafb..b07a6ef 100644 --- a/kernel/src/sched/mod.rs +++ b/kernel/src/sched/mod.rs @@ -150,7 +150,7 @@ impl From for SchedError { /// Generic over `C: ContextSwitch + Cpu` — the BSP provides both the /// interrupt-masking needed for safe context switches and the register-save /// assembly. -pub struct Scheduler { +pub struct Scheduler { ready: SchedQueue, task_states: [TaskState; TASK_ARENA_CAPACITY], /// Stored handles, indexed by slot index, so the scheduler can find @@ -214,11 +214,14 @@ impl Scheduler { unsafe { cpu.init_context(&mut self.contexts[idx], entry, stack_top); } - self.task_states[idx] = TaskState::Ready; - self.task_handles[idx] = Some(handle); + // Enqueue before writing task_states / task_handles so that a + // QueueFull error leaves no partial registration in those arrays. self.ready .enqueue(handle) - .map_err(|_| SchedError::QueueFull) + .map_err(|_| SchedError::QueueFull)?; + self.task_states[idx] = TaskState::Ready; + self.task_handles[idx] = Some(handle); + Ok(()) } /// Start the scheduler by switching to the first ready task. @@ -227,6 +230,16 @@ impl Scheduler { /// resumed) and restores the first task. Intended to be called exactly /// once from `kernel_main` after tasks have been added. /// + /// # IRQ state on task entry + /// + /// This method creates an `IrqGuard` immediately before the context switch. + /// The guard is on the bootstrap stack frame, which is abandoned (never + /// resumed). Tasks therefore begin executing with interrupts **masked** + /// (DAIF = 0xF). In A5 this is acceptable because no interrupt sources + /// are configured; a task that needs interrupts enabled must call + /// `cpu.restore_irq_state(IrqState(0))` explicitly. This will be + /// revisited when Phase B introduces a timer or other interrupt source. + /// /// # Panics /// /// Panics if no tasks have been added (the ready queue is empty). @@ -276,7 +289,9 @@ impl Scheduler { let next_handle = match self.ready.dequeue() { Some(h) if h != current_handle => h, _ => { - // Only one task exists or same handle returned — no switch. + // Only one ready task exists. The queue is transiently empty + // and self.current is unchanged. The next yield will re-enqueue + // the current task; no switch is performed here. return Ok(()); } }; @@ -396,7 +411,15 @@ impl Scheduler { } // Resumed here: the sender has delivered; collect the message. - return ipc_recv(ep_arena, queues, ep_cap, caller_table).map_err(SchedError::Ipc); + // A second Pending result would be a scheduler bug — the sender + // should have queued a message before unblocking this task. + let result = ipc_recv(ep_arena, queues, ep_cap, caller_table); + debug_assert!( + !matches!(result, Ok(RecvOutcome::Pending)), + "ipc_recv returned Pending after context-switch resume — \ + sender must deliver before unblocking receiver" + ); + return result.map_err(SchedError::Ipc); } Ok(outcome) @@ -420,6 +443,11 @@ impl Scheduler { /// Scan `task_states` for a task blocked on `ep` and re-enqueue it. /// + /// **Single-waiter semantics.** Only the first blocked task found is + /// woken; subsequent blocked tasks (if any) remain blocked. In A5 at + /// most one task waits per endpoint at a time (ADR-0019), so this is + /// correct. Multi-waiter wake-up is deferred to a future ADR. + /// /// O(N) scan — acceptable at `TASK_ARENA_CAPACITY ≤ 16` (ADR-0019). fn unblock_receiver_on(&mut self, ep: EndpointHandle) { for idx in 0..TASK_ARENA_CAPACITY { @@ -513,6 +541,23 @@ mod tests { } } + /// Test-only stack with guaranteed 16-byte alignment, satisfying the + /// contract of [`ContextSwitch::init_context`]. `FakeCpu::init_context` + /// is a no-op, so the alignment is not strictly required by the tests, + /// but it is stated here so the SAFETY comment is accurate and so the + /// helper is reusable if a real init_context is ever wired into tests. + #[repr(C, align(16))] + struct AlignedStack([u8; N]); + + impl AlignedStack { + fn new() -> Self { + Self([0u8; N]) + } + fn top(&mut self) -> *mut u8 { + self.0.as_mut_ptr_range().end + } + } + // ── SchedQueue tests ────────────────────────────────────────────────────── #[test] @@ -567,10 +612,10 @@ mod tests { let cpu = FakeCpu; let mut sched: Scheduler = Scheduler::new(); let h = task_handle(0); - let mut stack = [0u8; 512]; - let stack_top = stack.as_mut_ptr_range().end; - // SAFETY: stack is 512 bytes; FakeCpu::init_context is a no-op. - unsafe { sched.add_task(&cpu, h, spin_entry(), stack_top).unwrap() }; + let mut stack = AlignedStack::<512>::new(); + // SAFETY: stack is 512 bytes and 16-byte aligned (AlignedStack repr). + // FakeCpu::init_context is a no-op so the stack is never actually used. + unsafe { sched.add_task(&cpu, h, spin_entry(), stack.top()).unwrap() }; assert_eq!(sched.task_states[0], TaskState::Ready); assert_eq!(sched.task_handles[0], Some(h)); assert_eq!(sched.ready.len(), 1); @@ -582,15 +627,16 @@ mod tests { let mut sched: Scheduler = Scheduler::new(); let h0 = task_handle(0); let h1 = task_handle(1); - let mut s0 = [0u8; 512]; - let mut s1 = [0u8; 512]; - // SAFETY: stacks are 512 bytes; FakeCpu::init_context is a no-op. + let mut s0 = AlignedStack::<512>::new(); + let mut s1 = AlignedStack::<512>::new(); + // SAFETY: stacks are 512 bytes and 16-byte aligned (AlignedStack repr). + // FakeCpu::init_context is a no-op so the stacks are never actually used. unsafe { sched - .add_task(&cpu, h0, spin_entry(), s0.as_mut_ptr_range().end) + .add_task(&cpu, h0, spin_entry(), s0.top()) .unwrap(); sched - .add_task(&cpu, h1, spin_entry(), s1.as_mut_ptr_range().end) + .add_task(&cpu, h1, spin_entry(), s1.top()) .unwrap(); } // Simulate h0 running: it was dequeued when it started running. From b4879c7082580ca293a58d02217d3dcba1350187 Mon Sep 17 00:00:00 2001 From: Cemil ILIK Date: Tue, 21 Apr 2026 17:09:46 +0300 Subject: [PATCH 10/10] docs(skills): add add-bsp skill Covers the full procedure for adding a new BSP crate to the workspace: boot-flow ADR first, crate skeleton, the six-item boot checklist from bsp-boot-checklist.md, cpu.rs with naked context switch and d8-d15, console.rs, linker script, run script with --int-log, and smoke test verification. Acceptance criteria require the naked-fn rule and SIMD register coverage to be met before the task is considered done. Co-Authored-By: Claude Sonnet 4.6 --- .claude/skills/README.md | 1 + .claude/skills/add-bsp/SKILL.md | 213 ++++++++++++++++++++++++++++++++ 2 files changed, 214 insertions(+) create mode 100644 .claude/skills/add-bsp/SKILL.md diff --git a/.claude/skills/README.md b/.claude/skills/README.md index bfb67a6..cee30ee 100644 --- a/.claude/skills/README.md +++ b/.claude/skills/README.md @@ -73,6 +73,7 @@ Skills are short. If a skill needs more than ~200 lines, either (a) the underlyi | [sync-adr-index](sync-adr-index/SKILL.md) | Rebuild the ADR index table from the files on disk. | | [start-task](start-task/SKILL.md) | Open a new roadmap task from the user-story template. | | [conduct-review](conduct-review/SKILL.md) | Produce a milestone retrospective in `docs/roadmap/reviews/`. | +| [add-bsp](add-bsp/SKILL.md) | Add a new Board Support Package crate — crate skeleton, boot checklist, console, context switch, smoke test. | ## Conventions for adding a new skill diff --git a/.claude/skills/add-bsp/SKILL.md b/.claude/skills/add-bsp/SKILL.md new file mode 100644 index 0000000..decc9e6 --- /dev/null +++ b/.claude/skills/add-bsp/SKILL.md @@ -0,0 +1,213 @@ +--- +name: add-bsp +description: Add a new Board Support Package (BSP) crate to the Umbrix workspace — from crate skeleton through boot checklist to first QEMU or hardware boot. +when-to-use: When adding support for a new hardware target (e.g. Raspberry Pi 4, a custom board) or when porting the kernel to a new QEMU machine type. +--- + +# Add BSP + +## Inputs + +Before starting, the agent must have: + +- **Target name** — a short kebab-case identifier (e.g. `rpi4`, `qemu-virt`). +- **Architecture** — the Rust target triple (e.g. `aarch64-unknown-none`). +- **Boot EL** — the exception level the hardware or QEMU drops the kernel to (EL1, EL2, …). Verify this before writing any boot code; do not assume. +- **Peripheral map** — UART base address, any other peripherals needed for a boot console. +- **ADR for boot flow** — a new ADR covering reset-vector design and memory map for this target (model on [ADR-0012](../../../docs/decisions/0012-boot-flow-qemu-virt.md)). + +If the boot EL or peripheral map is unknown, stop and ask the maintainer before proceeding. + +## Procedure + +### 1. Write the boot-flow ADR + +Follow the [write-adr](../write-adr/SKILL.md) skill. The ADR must document: + +- Which EL the target enters at. +- Stack address and linker symbol convention. +- Memory map (ROM/RAM ranges visible at reset). +- Any privileged setup required before Rust entry (EL transition, cache init, …). + +Do not write boot code before this ADR is Accepted. + +### 2. Create the crate skeleton + +``` +bsp-/ + Cargo.toml + build.rs (if a linker script needs to be emitted) + linker.ld + src/ + boot.s (reset vector — assembly only) + main.rs (kernel_entry and panic handler) + console.rs (Console impl for the target's UART) + cpu.rs (Cpu + ContextSwitch impl for the target) +``` + +`Cargo.toml` must set: +```toml +[package] +name = "umbrix-bsp-" +edition = "2021" + +[[bin]] +name = "umbrix-bsp-" +path = "src/main.rs" + +[dependencies] +umbrix-hal = { path = "../hal" } +umbrix-kernel = { path = "../kernel" } +``` + +Add the crate to `[workspace]` in the root `Cargo.toml`. + +### 3. Work through the BSP boot checklist — in order + +Read [`docs/standards/bsp-boot-checklist.md`](../../../docs/standards/bsp-boot-checklist.md) in full. +Execute each item before moving to the next. Do **not** assume any item is already satisfied on a new target: + +| # | Item | Common mistake | +|---|------|----------------| +| 1 | Exception level confirmed | Assuming EL1 when hardware enters EL2 | +| 2 | CPACR_EL1.FPEN = 0b11 set in `boot.s` | Forgetting → NEON trap, silent hang | +| 3 | VBAR configured before enabling IRQs | Missing → any exception = silent hang | +| 4 | SP 16-byte aligned at first `bl` | Wrong linker alignment → AAPCS64 fault | +| 5 | BSS zeroed before `kernel_entry` | Uninitialised statics → subtle UB | +| 6 | Context-switch fn is `#[unsafe(naked)]` | Using `#[inline(never)]` → sp corruption | + +### 4. Write `boot.s` + +Minimum content (aarch64 EL1 example): + +```asm + .section .text.boot, "ax" + .global _start +_start: + /* 1. Set stack pointer */ + adrp x0, __stack_top + add x0, x0, :lo12:__stack_top + mov sp, x0 + + /* 2. Enable FP/SIMD — do not rely on CPACR_EL1 reset value */ + mov x0, #0x300000 // FPEN = 0b11 + msr cpacr_el1, x0 + isb + + /* 3. Zero BSS */ + adrp x0, __bss_start + add x0, x0, :lo12:__bss_start + adrp x1, __bss_end + add x1, x1, :lo12:__bss_end +0: cmp x0, x1 + b.hs 1f + str xzr, [x0], #8 + b 0b +1: + bl kernel_entry +2: wfe + b 2b +``` + +Adjust for EL2→EL1 transition if the target enters at EL2 (add `msr hcr_el2, …` / `eret` sequence before step 1). + +### 5. Implement `cpu.rs` + +Copy the structure from [`bsp-qemu-virt/src/cpu.rs`](../../../bsp-qemu-virt/src/cpu.rs). + +- Implement `Cpu` trait: `current_core_id`, `disable_irqs`, `restore_irq_state`, `wait_for_interrupt`, `instruction_barrier`. +- Implement `ContextSwitch` trait with `#[unsafe(naked)]` `context_switch_asm` that saves **all** AAPCS64 callee-saved registers: x19–x28, fp, lr, sp, **and d8–d15**. +- Use the same `Aarch64TaskContext` layout (168 bytes, repr(C)) and the same field offsets. +- Add every `unsafe` block to the audit log per the [justify-unsafe](../justify-unsafe/SKILL.md) skill. + +### 6. Implement `console.rs` + +Implement `umbrix_hal::Console` for the target UART. Follow the existing `Pl011Uart` implementation. Each UART model is different; check the datasheet for the FIFO-full flag and data-register offsets. + +### 7. Wire `main.rs` + +`kernel_entry` must: +1. Initialise the console and print the greeting (`umbrix: hello from kernel_main`). +2. Call `umbrix_kernel::sched` to register tasks and start the scheduler. +3. Never return (`-> !`). + +Provide a `#[panic_handler]` that writes to the console and loops. + +### 8. Add a linker script + +Model on [`bsp-qemu-virt/linker.ld`](../../../bsp-qemu-virt/linker.ld). At minimum: + +- Place `.text.boot` first so `_start` is at the load address. +- Align `__bss_start` and `__bss_end` to 8 bytes. +- Align `__stack_top` to 16 bytes. + +### 9. Add a run script + +Create `tools/run-.sh`. For QEMU targets, model on [`tools/run-qemu.sh`](../../../tools/run-qemu.sh): + +- Include the `--int-log` flag (`-d int -D /tmp/qemu_int.log`) for silent-hang debugging. +- Document the QEMU machine flags in the script header. + +For real hardware, document the flashing command (e.g. `openocd`, `rpiboot`, `cargo flash`). + +### 10. Verify the smoke test + +Boot the kernel and confirm: + +``` +umbrix: hello from kernel_main +umbrix: starting cooperative scheduler +umbrix: task A — iteration 0 +umbrix: task B — iteration 0 +... +umbrix: task A done; spinning +``` + +If the kernel hangs silently, run with `--int-log` (QEMU) or attach a JTAG debugger (real hardware) and check for the following before anything else: + +1. Exception log: `grep "Taking exception" /tmp/qemu_int.log` +2. CPACR_EL1 — is FPEN set? (ESR EC=0x07 = FP/SIMD trap) +3. SP misalignment — AAPCS64 stack-alignment fault? +4. BSS not zeroed — statics have garbage values? + +See [`docs/standards/bsp-boot-checklist.md`](../../../docs/standards/bsp-boot-checklist.md) for the full diagnostic table. + +### 11. Commit + +Per [commit-style.md](../../../docs/standards/commit-style.md): + +``` +feat(bsp-): initial BSP — boot to kernel_entry on +``` + +Body: one sentence on what the BSP proves (e.g. "boots to kernel_entry on RPi4 CM4 at EL2→EL1; PL011 console confirmed"). + +Trailer: `Refs: ADR-NNNN` (the boot-flow ADR from step 1). + +## Acceptance criteria + +- [ ] Boot-flow ADR Accepted before any code lands. +- [ ] All six items in the BSP boot checklist verified. +- [ ] `cargo build --target -p umbrix-bsp-` succeeds with zero warnings. +- [ ] QEMU or hardware boots to the cooperative scheduler smoke-test output. +- [ ] Every `unsafe` block has a `// SAFETY:` comment with (a) why needed, (b) invariants, (c) why alternatives rejected; audit log updated. +- [ ] `context_switch_asm` is `#[unsafe(naked)]` and saves d8–d15 in addition to x19–x28, fp, lr, sp. +- [ ] Run script exists and `--int-log` (or equivalent) is documented. +- [ ] Commit follows `commit-style.md`. + +## Anti-patterns + +- **Skipping the boot-flow ADR.** Every BSP has target-specific boot decisions; writing code before documenting them produces undocumented assumptions. +- **Copying `boot.s` without checking the EL.** QEMU `virt` enters EL1; RPi4 enters EL2. The CPACR_EL1 sequence is only needed if the kernel runs at EL1; at EL2 you need CPTR_EL2. +- **Using `#[inline(never)]` instead of `#[unsafe(naked)]` for the context switch.** The compiler will still emit a prologue. See `docs/standards/unsafe-policy.md §5a`. +- **Omitting d8–d15 from `Aarch64TaskContext`.** NEON is enabled at boot; the compiler may allocate d8–d15 in any function. Omitting them silently corrupts task state at higher optimisation levels. +- **Not zeroing BSS.** Rust guarantees `.bss` is zero; if the hardware or QEMU does not zero it, every static initialised to zero will have garbage values. + +## References + +- [`docs/standards/bsp-boot-checklist.md`](../../../docs/standards/bsp-boot-checklist.md) — ordered checklist with diagnostic table. +- [`docs/standards/unsafe-policy.md`](../../../docs/standards/unsafe-policy.md) — `#[unsafe(naked)]` rule (§5a) and general unsafe discipline. +- [`bsp-qemu-virt/`](../../../bsp-qemu-virt/) — reference BSP implementation. +- [ADR-0012](../../../docs/decisions/0012-boot-flow-qemu-virt.md) — boot-flow ADR for the reference BSP. +- [ADR-0020](../../../docs/decisions/0020-cpu-trait-v2-context-switch.md) — `ContextSwitch` trait contract. +- [AAPCS64](https://github.com/ARM-software/abi-aa/releases) — callee-saved register list (x19–x28, x29, x30, d8–d15).