feat: T-018 — AddressSpace kernel object + cap-gated Mmu::map/unmap wrappers + activation-on-context-switch (B3 §§2-7 closure) - #28
Merged
Conversation
… enum (kernel/src/mm/)
Pure data-structure landing per [ADR-0028 §Dependency chain steps
1-2][adr-0028] and [T-018 §Approach commit 1][t-018]. No
capability integration, no scheduler hook, no BSP wiring — those
land in commits 2, 4, and 5 respectively. The cap-gated wrapper
surface (cap_create_address_space / cap_map / cap_unmap) and the
remaining AddressSpaceError variants (OutOfFrames, CapError,
MmuMapError, MmuUnmapError) land in commit 3.
What this commit adds:
- **`kernel/src/mm/address_space.rs`** (new file, ~430 lines incl.
tests). Module structure mirrors `kernel/src/obj/endpoint.rs` /
`kernel/src/obj/task.rs` / `kernel/src/obj/notification.rs`
per [ADR-0016][adr-0016] (per-type fixed-size-block arenas,
generation-tagged typed handles, safe-Rust-only surface):
- `pub struct AddressSpace<M: Mmu> { inner: M::AddressSpace }`
— generic over `M: Mmu` per [ADR-0028 §Decision outcome][adr-0028]
Option A. The struct holds only the BSP-specific inner value
for v1; the per-AS generation tag lives in the arena slot
per the existing pattern. Forward-compat fields (asid,
reverse-mapping pointers) land additively when ADR-0033
(high-half migration placeholder) opens — not added today
(CLAUDE.md non-negotiable #6).
- `AddressSpace::wrap_bootstrap(inner: M::AddressSpace) -> Self`
— wraps an already-active `Mmu::AddressSpace` (the one
`mmu_bootstrap` activated post-T-016) into a kernel-object
value WITHOUT calling `Mmu::create_address_space` (which
would re-zero the live L0 frame). Used exactly once at boot
in commit 5's BSP wiring; all subsequent address spaces are
constructed via `cap_create_address_space` →
`PMM.alloc_frame()` → `Mmu::create_address_space(root)` in
commit 3.
- `AddressSpace::root_frame(&self, mmu: &M) -> PhysFrame` —
diagnostic accessor for the bootstrap banner + host tests;
delegates to `Mmu::address_space_root`.
- `AddressSpace::inner()` / `inner_mut()` — crate-internal
accessors that the cap-gated wrappers in commit 3 will use
to pass `&` / `&mut M::AddressSpace` to `Mmu::map` /
`Mmu::unmap` / `Mmu::activate`. Landed in commit 1 (with
`#[allow(dead_code, reason = ...)]` until commit 3 lights
them up) so commit 3 adds only wrapper bodies, not accessor
surface — keeps commit 3 focused.
- `pub struct AddressSpaceHandle(SlotId)` newtype — typed
handle exact-shape mirror of `EndpointHandle` / `TaskHandle`
/ `NotificationHandle`. Generation-tagged via the underlying
[`crate::obj::arena::SlotId`]; stale handles fail lookup
with `AddressSpaceError::StaleHandle`. `test_handle(index,
generation)` constructor for unit-test scaffolding mirrors
the existing `EndpointHandle::test_handle` / `TaskHandle::test_handle`
pattern.
- `pub type AddressSpaceArena<M> = Arena<AddressSpace<M>,
ADDRESS_SPACE_ARENA_CAPACITY>` — reuses
`crate::obj::arena::Arena<T, N>` (ADR-0016's audited generic
fixed-capacity arena). No new arena infrastructure;
`AddressSpace<M>` plugs into the existing generic-arena
surface as the value type.
- `pub const ADDRESS_SPACE_ARENA_CAPACITY: usize = 8` —
bootstrap AS + headroom for T-018's two-AS isolation tests
(commits 3 + 4 will exercise; commit 5 will publish the
`StaticCell` for the BSP).
- `#[non_exhaustive] pub enum AddressSpaceError { ArenaFull,
StaleHandle }` — v1 commit-1 variant set. Commit 3 extends
the enum additively (`#[non_exhaustive]` makes the extension
a non-breaking change for any external matcher).
- Free functions: `create_address_space` /
`destroy_address_space` / `get_address_space` /
`get_address_space_mut`. Mirrors the
`create_endpoint` / `destroy_endpoint` / `get_endpoint`
shape from `kernel/src/obj/endpoint.rs`. `destroy_address_space`
is gated behind `#[allow(dead_code, ...)]` per the
cap_revoke-deferral note in [ADR-0028 §"Out of scope"][adr-0028]
— destroy path is B4+ (when first userspace destroy lands);
v1 has no caller but the function is landed in commit 1 for
symmetric arena-surface completeness.
- **`kernel/src/mm/mod.rs`** — `pub mod address_space;` declaration
+ re-exports of `AddressSpace` / `AddressSpaceArena` /
`AddressSpaceError` / `AddressSpaceHandle` /
`ADDRESS_SPACE_ARENA_CAPACITY`. Module-doc header updated to
reference ADR-0028 + T-018 alongside the existing ADR-0035 +
T-017 references; the "placeholder" note for ADR-0028 lifts
to the live reference.
Host tests (6 new in `kernel/src/mm/address_space::tests`):
1. `wrap_bootstrap_returns_address_space_with_root` — pins
[ADR-0028 §Simulation row 0][adr-0028]: given a BSP-side
`M::AddressSpace` value, `wrap_bootstrap(inner)` returns a
kernel-object whose `root_frame()` matches the original
root. Uses `tyrne_test_hal::FakeMmu` as the M impl;
`FakeMmu::create_address_space` is the FakeMmu's pure
"wrap existing root" surrogate (host-side HashMap-backed
mock; no zero-fill).
2. `arena_alloc_returns_distinct_handles` — alloc two
address spaces, assert distinct handles, assert both
resolve to the expected roots.
3. `arena_get_with_stale_handle_returns_none` — pins the
generation-tag contract: alloc + free + alloc-again at the
same slot; the original handle's generation no longer
matches; `get` / `get_mut` return None; the new handle
resolves; the slot reuse is detectable.
4. `arena_full_returns_arena_full_error` — fill the arena to
`ADDRESS_SPACE_ARENA_CAPACITY`, assert overflow alloc
returns `Err(AddressSpaceError::ArenaFull)`.
5. `destroy_with_stale_handle_returns_stale_handle_error` —
first destroy succeeds; second on the same handle returns
`Err(AddressSpaceError::StaleHandle)`. Uses `matches!`
rather than `assert_eq!` because `AddressSpace<M>` does
not implement `Debug + PartialEq` (would require an
`M::AddressSpace: Debug + PartialEq` bound the `Mmu`
trait does not impose).
6. `inner_accessors_provide_borrow_and_borrow_mut` —
exercises the crate-internal `inner()` / `inner_mut()`
accessors that commit 3's cap-gated wrappers will use,
verifying borrow + borrow-mut round-trip behaviour
within the host-test scope.
Verification:
- cargo fmt --all -- --check clean
- cargo host-clippy clean (-D warnings)
- cargo kernel-clippy clean (-D warnings)
- cargo host-test 121/121 host (kernel crate;
was 115 — +6 AddressSpace tests).
Workspace total 206 (was 200).
- cargo kernel-build clean (the kernel image picks
up the new module; no BSP-side
caller until commit 5, so the
`.bss` / `.text` deltas are
bounded by the module's data
+ the test-only references).
No `unsafe` in this commit. No HAL trait surface change. No BSP
file touched. No capability code touched.
Refs: ADR-0028, T-018
[adr-0016]: https://github.com/cemililik/Tyrne/blob/main/docs/decisions/0016-kernel-object-storage.md
[adr-0028]: https://github.com/cemililik/Tyrne/blob/main/docs/decisions/0028-address-space-data-structure.md
[t-018]: https://github.com/cemililik/Tyrne/blob/main/docs/analysis/tasks/phase-b/T-018-address-space-kernel-object.md
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…nt + CapError::WrongKind
Capability-system surface extension for the [ADR-0028][adr-0028]
`AddressSpace` kernel object. Pure additive enum changes:
- **`CapKind::AddressSpace`** — new discriminator variant,
inserted between `Notification` and `MemoryRegion`. Matches
the semantic order (kernel-object kinds with live storage
first; `MemoryRegion` is the Phase-B4+ frame-ownership
reservation that still has no `CapObject` variant).
- **`CapObject::AddressSpace(AddressSpaceHandle)`** — new typed
variant carrying the handle from commit 1's
[`crate::mm::AddressSpaceHandle`] newtype. The typed-handle
discipline prevents wrong-kind invocation at compile time
(passing a `TaskHandle` where an `AddressSpaceHandle` is
expected is a compile error, mirroring how
`CapObject::Endpoint(EndpointHandle)` keeps endpoint and
task capabilities distinct).
- **`CapObject::kind()` extension** — exhaustive match gains
`Self::AddressSpace(_) => CapKind::AddressSpace` arm; no
behavioural change for existing variants.
- **`CapError::WrongKind`** — new variant on the
`#[non_exhaustive]` `CapError` enum. The existing variants
(`CapsExhausted` / `InvalidHandle` / `WidenedRights` / ...)
don't cleanly express "the capability exists and resolves
but its kind is not what this operation requires" — which
is the discriminant T-018's cap-gated wrappers
(`cap_create_address_space` / `cap_map` / `cap_unmap`, landing
in commit 3) need to surface. Previously the IPC fast-path
in `kernel/src/sched/mod.rs::resolve_ep_cap` collapsed
wrong-kind into `SchedError::Ipc(IpcError::InvalidCapability)`
— that flatten was acceptable because IPC has only one
expected cap kind (`Endpoint`), but the AddressSpace wrappers
need a clean wrong-kind error to wrap through
`AddressSpaceError::CapError(CapError::WrongKind)` per the
unified-return contract documented in [T-018:41,52-54,88][t-018].
- **`CapKind` doc-comment update** — names the AddressSpace
variant + ADR-0028 reference; clarifies that `MemoryRegion`
is Phase-B4+, not "Phase B".
- **Module-doc status update** — `CapObject` doc now lists
`AddressSpaceHandle` alongside the existing three typed
handles; `MemoryRegion` deferral narrowed from "Phase B" to
"Phase B4+".
- **Import** — `use crate::mm::AddressSpaceHandle` brought into
scope at the module top so the `CapObject::AddressSpace`
variant compiles without a fully-qualified path.
Note on `CapKind`'s lack of `#[non_exhaustive]`: this enum was
not annotated when introduced in T-001 (capabilities pre-date
the `#[non_exhaustive]` convention the project adopted later for
error enums). The matchers that pattern on `CapKind` (currently
just `CapObject::kind()` + one test assertion at
`kernel/src/cap/table.rs:645`) are kernel-internal and updated
in lockstep with this variant addition. Promoting the enum to
`#[non_exhaustive]` is out of scope for T-018; worth a hygiene
task when a future cap-variant addition feels structurally
material.
Verification (commit 2 is structural-only; no behavioural change
to existing tests):
- cargo fmt --all -- --check clean
- cargo host-clippy clean (-D warnings)
- cargo kernel-clippy clean (-D warnings)
- cargo host-test 121/121 kernel (unchanged
from commit 1) — no new
tests; the new variants will
be exercised by commit 3's
cap-gated wrapper tests.
- cargo kernel-build clean
No `unsafe`. No HAL trait surface change. No BSP file touched.
No scheduler code touched. The capability-variant landing is
isolated to `kernel/src/cap/mod.rs`.
Refs: ADR-0028, T-018
[adr-0028]: https://github.com/cemililik/Tyrne/blob/main/docs/decisions/0028-address-space-data-structure.md
[t-018]: https://github.com/cemililik/Tyrne/blob/main/docs/analysis/tasks/phase-b/T-018-address-space-kernel-object.md
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…_space / cap_map / cap_unmap)
Capability-gated wrapper surface per [ADR-0028 §Simulation rows 1-2][adr-0028]
+ [T-018 §Approach commit 3][t-018]. Lights up the `inner_mut`
accessor + `destroy_address_space` (the rollback-on-cap-mint-failure
caller). The scheduler activation hook still pending (commit 4),
keeps `inner()` (immutable) dead-code-gated until commit 4 lights it.
What this commit adds:
- **`AddressSpaceError` enum extension** — four new variants on the
existing `#[non_exhaustive]` enum:
- `OutOfFrames` — PMM exhausted in `cap_create_address_space`.
- `CapError(CapError)` — passthrough from cap-resolution (wraps
`InvalidHandle` / `WrongKind` / `WidenedRights` /
`CapsExhausted` without flattening).
- `MmuMapError(MmuError)` — passthrough from `Mmu::map`
(`OutOfFrames` / `AlreadyMapped` / `MisalignedAddress` /
`InvalidFlags`, etc.).
- `MmuUnmapError(MmuError)` — passthrough from `Mmu::unmap`
(`NotMapped` / `MisalignedAddress`).
Wrappers expose one unified return type; the unified-return
contract is now honest (resolves the PR #27 round-1 finding).
- **`AddressSpace::from_mmu_address_space(inner)` constructor** —
structurally identical to `wrap_bootstrap` (both wrap an
`M::AddressSpace`) but the name documents the caller's intent:
`wrap_bootstrap` for the already-live bootstrap topology (BSP
commit 5 path); `from_mmu_address_space` for the post-
`Mmu::create_address_space` path (this commit's
`cap_create_address_space`). Avoids the misnamed-bootstrap
confusion future readers would hit if `cap_create_address_space`
called `wrap_bootstrap`.
- **`resolve_address_space_cap` helper** — file-internal `fn`
(not `pub(crate)`; used only inside this module). Looks up the
cap by handle, validates `kind() == CapKind::AddressSpace`,
extracts the typed handle from `CapObject::AddressSpace(_)`.
Two error paths: `CapError(InvalidHandle)` (table miss) and
`CapError(WrongKind)` (cap exists but wrong kind). T-018:48
originally specified the helper in `kernel/src/sched/mod.rs`
alongside `resolve_ep_cap` — but that helper is scheduler-
internal (`fn` on `Scheduler<C>` returning `SchedError`); the
AddressSpace resolution is mm-internal (used by the cap-gated
wrappers in this same file). Local-to-the-wrappers placement
is the better locality. The T-018 §Acceptance criteria line
flagging "sched/mod.rs" location is a Doc drift to clean up in
commit 6.
- **`cap_create_address_space`** — 6-step capability-gated AS
creation:
1. Resolve parent_cap, validate `kind == CapKind::AddressSpace`.
No state change. v1 simplification: "any AS cap grants AS
creation authority" — Phase-C `Untyped` discipline replaces
this.
2. No-widening check on `new_rights` per ADR-0014's
`cap_copy`/`cap_derive` discipline (mirrors
`CapabilityTable::cap_copy` at `kernel/src/cap/table.rs:191`).
3. `pmm.alloc_frame()` for the root. Fails-fast with
`OutOfFrames`.
4. `unsafe { mmu.create_address_space(root) }` — SAFETY block
names the existing UNSAFE-2026-0023 umbrella (the unsafe is
on the HAL trait surface, not introduced by this wrapper).
Invariants upheld: page-alignment (statically by `PhysFrame`)
+ zero-fill (UNSAFE-2026-0026 PMM contract).
5. Arena slot via `create_address_space` (commit 1 free
function). On `ArenaFull`: PMM frame leaks (v1 limitation
— `FrameProvider` trait has no `free_frame` method;
structurally unreachable in v1's sized arena).
6. Mint cap via `table.insert_root`. On cap-table failure:
rollback the arena slot via `destroy_address_space` (the
dead-code-gated function from commit 1, now ungated for
this caller); PMM frame still leaks (same limitation).
Returns the new `CapHandle` (not `AddressSpaceHandle` — T-018:52
drift; the new cap is what subsequent `cap_map`/`cap_unmap`
callers need).
- **`cap_map`** — 4-step capability-gated mapping installation:
resolve cap → `get_address_space_mut` → `mmu.map(inner_mut, va,
pa, flags, pmm)` → `token.flush(mmu)`. Returns `Ok(())` on
success, wraps `MmuError` as `AddressSpaceError::MmuMapError`
on failure. Lifts UNSAFE-2026-0025's `Pending QEMU smoke
verification` note via commit 6's audit-log Amendment.
- **`cap_unmap`** — mirrors `cap_map` inversely: resolve cap →
`get_address_space_mut` → `mmu.unmap(inner_mut, va)` →
`token.flush(mmu)` → return the orphaned `PhysFrame` for
caller-side handling (typically PMM `free_frame` when
`cap_revoke(MemoryRegionCap)` lands in B4+; v1's tests just
verify the return value matches the originally-mapped PA).
- **Doc-comment for `destroy_address_space`** updated to name
the cap-mint-failure rollback caller (no longer "v1 has no
caller"); `#[allow(dead_code)]` removed.
Host tests (11 new in `mm::address_space::tests`):
- `resolve_address_space_cap_returns_handle_on_correct_kind`
- `resolve_address_space_cap_returns_wrong_kind_on_endpoint_cap`
- `cap_create_address_space_consumes_one_pmm_frame_and_mints_cap`
- `cap_create_address_space_returns_out_of_frames_on_pmm_exhaustion`
- `cap_create_address_space_rejects_wrong_parent_kind`
- `cap_create_address_space_rejects_widened_rights`
- `cap_map_installs_mapping_and_flushes_tlb` — uses FakeMmu's
`tlb_address_invalidations()` recorder to verify the flush
token was discharged.
- `cap_map_wraps_mmu_error_passthrough` — FakeMmu returns
`MisalignedAddress` for non-page-aligned VAs; assert the
wrapper wraps it as `MmuMapError(MisalignedAddress)`.
- `cap_map_rejects_wrong_kind` — Endpoint cap rejected.
- `cap_unmap_returns_unmapped_frame` — map + unmap round-trip;
returned `PhysFrame` matches; two TLB invalidations recorded
(one from map, one from unmap).
- `cap_unmap_wraps_mmu_error_passthrough` — unmap of unmapped
VA returns `MmuUnmapError(NotMapped)`.
Test fixture `bootstrap_setup()` mints the bootstrap AS cap with
all four `CapRights` so widening-check tests can derive narrower
caps from it. Returns a 5-tuple `(table, cap_handle, as_handle,
mmu, arena)` reused across most cap-wrapper tests.
Verification:
- cargo fmt --all -- --check clean
- cargo host-clippy clean (-D warnings;
`clippy::too_many_arguments`
allow on cap_create_address_space
+ cap_map per the no-ambient-
authority discipline rationale)
- cargo kernel-clippy clean (-D warnings)
- cargo host-test 132/132 kernel (was 121;
+11 cap-wrapper tests).
Workspace total 217 (was 206).
- cargo kernel-build clean
`unsafe` surface: one new `unsafe` block in `cap_create_address_space`
calling `Mmu::create_address_space`. Rides UNSAFE-2026-0023's
existing umbrella scope per the SAFETY comment; no new audit
entry needed (audit-policy.md §1 satisfied with inline
why/invariants/rejected-alternatives summary).
No HAL trait surface change. No BSP file touched. No scheduler
code touched. Cap module (commit 2) untouched.
Refs: ADR-0028, T-018
Audit: UNSAFE-2026-0023
[adr-0028]: https://github.com/cemililik/Tyrne/blob/main/docs/decisions/0028-address-space-data-structure.md
[t-018]: https://github.com/cemililik/Tyrne/blob/main/docs/analysis/tasks/phase-b/T-018-address-space-kernel-object.md
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…in yield_now + ipc_*_and_yield + start
Per [ADR-0028 §Simulation row 3][adr-0028] + [T-018 §Approach
commit 4][t-018]. Threads an `impl FnOnce(AddressSpaceHandle)`
closure parameter through the four scheduler entry points that
perform a context switch (`yield_now`, `ipc_send_and_yield`,
`ipc_recv_and_yield`, `start`); the scheduler invokes the closure
when the outgoing and incoming tasks have distinct
`AddressSpaceHandle`s (or, in `start`'s case, unconditionally for
the first task). v1 BSP passes a no-op `|_| {}` closure at every
call site — commit 5 will wire the real closure that captures the
BSP's `AddressSpaceArena<QemuVirtMmu>` static + `&Mmu` and calls
`Mmu::activate`. The hook is dormant in this commit (all BSP
tasks share the bootstrap AS via `BOOTSTRAP_ADDRESS_SPACE_HANDLE`,
so `address_space_activation_target` returns `None` everywhere).
What this commit adds:
**1. `SlotId::first_slot()` + `BOOTSTRAP_ADDRESS_SPACE_HANDLE` const**
- `SlotId::first_slot()` — new `pub const fn` in `kernel/src/obj/arena.rs`
returning index 0 / generation 0. Names the deterministic first
slot of any arena (the slot consumed by an empty arena's first
`allocate` call). The discipline note in the doc-comment makes
the "alloc first, then name" contract explicit so future BSP
authors can't misorder the bootstrap path.
- `BOOTSTRAP_ADDRESS_SPACE_HANDLE: AddressSpaceHandle` — new
`pub const` in `kernel/src/mm/address_space.rs` constructing the
canonical bootstrap-AS handle via `from_slot(SlotId::first_slot())`.
Used by BSP-side `Task` constructors that need to name the
bootstrap AS before the actual arena allocation runs (commit 5
does the allocation; this commit just names the slot).
**2. `Task` struct extension**
- Added `address_space_handle: AddressSpaceHandle` field to
`kernel/src/obj/task.rs::Task`.
- `Task::new(id)` → `Task::new(id, address_space_handle)`. All
existing v1 callers (4 task.rs tests + 3 BSP `task_a`/`task_b`/
`idle` constructions) updated to pass `BOOTSTRAP_ADDRESS_SPACE_HANDLE`.
- `Task::address_space_handle()` accessor.
- New `address_space_handle_round_trips` test pins the field round-trip.
**3. `Scheduler<C>` parallel array**
- `task_address_space_handles: [Option<AddressSpaceHandle>; TASK_ARENA_CAPACITY]`
— new field on `Scheduler<C>` parallel to `task_states` /
`task_handles`. Indexed by task slot. `None` means "no AS
registered for this slot" (defensive default for slots that
pre-date the threading; the activation hook treats `None` as
"no AS switch").
- `Scheduler::new` initializes the array to `[None; ...]`.
**4. `add_task` + `register_idle` signature changes**
- Both gain `address_space_handle: AddressSpaceHandle` parameter
(positioned between the `TaskHandle` parameter and the entry
`fn() -> !`).
- Both write the AS handle to `task_address_space_handles[idx]`
alongside the existing `task_handles[idx]` write.
- All 11 BSP/test call sites updated.
**5. `yield_now` / `ipc_recv_and_yield` / `ipc_send_and_yield` /
`start` closure parameter + activation hook**
- All four gain `activate_address_space: impl FnOnce(AddressSpaceHandle)`
parameter.
- `yield_now`: after computing `next_idx` (under the momentary
`&mut Scheduler<C>` borrow), reads both tasks' AS handles and
computes whether activation is needed via the helper
`address_space_activation_target`. Drops the `&mut` borrow.
Inside the `IrqGuard` scope, before `cpu.context_switch`, invokes
the closure if activation is needed. The closure body can freely
access BSP-side state (typically a `StaticCell` AS arena) because
no scheduler `&mut` is alive at this point.
- `ipc_recv_and_yield`: same activation hook structure inserted in
the Phase-2 dispatch path (the one with its own
`cpu.context_switch` site). On the Phase-1 early-return or
Phase-2 Deadlock paths, the closure is dropped unused (FnOnce
drop on function exit).
- `ipc_send_and_yield`: threads the closure to the internal
`yield_now` call when `needs_yield` is true; drops it explicitly
on the no-yield (`SendOutcome::Enqueued`) path.
- `start`: activates the first task's AS unconditionally (no
"previous AS" to compare against — the bootstrap stack frame
is being abandoned). Reads `first_as` via a momentary unsafe
block per ADR-0021's discipline.
**6. `address_space_activation_target` helper**
Pure private fn at module level. Maps `(current_as, next_as)` to
`Option<AddressSpaceHandle>`:
- `(Some(c), Some(n)) if c != n` → `Some(n)` (switch needed).
- All other branches → `None` (no switch).
Used by both `yield_now` and `ipc_recv_and_yield`. Pinned by
`address_space_activation_target_pure_function` test (covers all
5 input branches).
**7. New scheduler tests (3 new in `sched::tests`)**
- `yield_now_skips_activation_when_tasks_share_address_space` —
pins the same-AS short-circuit (closure not fired).
- `yield_now_activates_when_tasks_differ_in_address_space` — pins
the differing-AS switch path (closure fired with `next` handle).
- `address_space_activation_target_pure_function` — pins the
helper's branch table.
**8. BSP updates (`bsp-qemu-virt/src/main.rs`)**
- `sched.add_task` calls for handle_a / handle_b: insert
`BOOTSTRAP_ADDRESS_SPACE_HANDLE` between the task handle and
entry fn.
- `register_idle` call: same insertion.
- `yield_now` (idle loop + Task B's explicit yield) + IPC
bridge calls (`ipc_recv_and_yield`, two `ipc_send_and_yield`
call sites) + `start`: append `|_| {}` as the trailing
activation closure. Commit 5 replaces these with the real
callback that captures `&Mmu` + `&AddressSpaceArena<M>`.
Verification:
- cargo fmt --all -- --check clean
- cargo host-clippy clean (-D warnings)
- cargo kernel-clippy clean (-D warnings)
- cargo host-test 136/136 kernel (was 132 at end
of commit 3; +4: 3 activation-hook
tests + 1 Task::address_space_handle
round-trip test). Workspace total
221 (was 217 at end of commit 3).
- cargo kernel-build clean
`unsafe` surface: one new `unsafe` block in `start` (the
`first_as` read after `start_prelude`'s `&mut` borrow drops);
SAFETY comment cites UNSAFE-2026-0014's existing momentary-&mut
umbrella. No new audit-log entries needed — the activation hook
itself is safe Rust calling an FnOnce closure; the BSP-side
closure (commit 5) will earn its own audit consideration.
No HAL trait surface change.
Refs: ADR-0028, T-018
Audit: UNSAFE-2026-0014
[adr-0028]: https://github.com/cemililik/Tyrne/blob/main/docs/decisions/0028-address-space-data-structure.md
[t-018]: https://github.com/cemililik/Tyrne/blob/main/docs/analysis/tasks/phase-b/T-018-address-space-kernel-object.md
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…essSpaceArena StaticCell + activation closure + smoke banner
Per [ADR-0028 §Simulation row 0][adr-0028] + [T-018 §Approach
commit 5][t-018]. Wires the BSP-side address-space infrastructure:
the bootstrap AS is now a live arena slot 0 with a minted authority
cap, the activation hook in commit 4 has a real closure (no
longer no-op), and the smoke trace gains exactly one new banner
line confirming the bring-up.
What this commit adds:
**kernel/src/mm/address_space.rs**
- `pub fn activate_address_space_handle<M: Mmu>(arena, handle, mmu)`
— public BSP-facing scheduler-side activation helper. Looks up
`handle` in `arena` and invokes `Mmu::activate` on the inner
BSP-specific value. Stale-handle behaviour: silent no-op (panic
inside the activation hook would compound the failure since
the hook runs inside an `IrqGuard` scope). The BSP's
`activate_address_space` fn wraps a call to this helper.
- `get_address_space` re-exported through `kernel/src/mm/mod.rs`
so BSP-side code can reach it directly when needed (the
activation helper uses it internally; the BSP previously had
no other consumer).
**bsp-qemu-virt/src/mmu.rs**
- `QemuVirtAddressSpace::from_existing_root(root: PhysFrame) -> Self`
— new safe inherent constructor for the bootstrap-AS case. Per
[ADR-0028 §Simulation row 0][adr-0028]: `mmu_bootstrap` already
activated the L0/L1/L2 frames via `MSR TTBR0_EL1` before any
kernel-side `AddressSpace<QemuVirtMmu>` value exists. This
constructor wraps the live root without going through the
`unsafe Mmu::create_address_space` trait method, whose contract
requires the root to be zero-filled (true for post-PMM-alloc
frames, false for the live bootstrap root). The body is just
`Self { root }` — same as `Mmu::create_address_space`'s body —
but the name + safety boundary make the bootstrap-path's
intent explicit.
**bsp-qemu-virt/src/main.rs**
- `extern "C" { static __boot_pt_l0: [u64; 512]; }` — linker
symbol re-exposed (already exposed in `mmu_bootstrap.rs` but
needs to be reachable from `kernel_entry`'s init block).
Resolves to the bootstrap L0 frame's PA (identity-mapped per
ADR-0027 so PA == VA).
- Four new `StaticCell` statics:
- `MMU: StaticCell<QemuVirtMmu>` — the long-lived Mmu instance
the activation closure dereferences. Zero-sized type;
StaticCell storage is bookkeeping only.
- `AS_ARENA: StaticCell<AddressSpaceArena<QemuVirtMmu>>` — the
address-space arena. Slot 0 holds the bootstrap AS after
`kernel_entry` init.
- `BOOTSTRAP_AS_CAP: StaticCell<CapHandle>` — the bootstrap AS
authority cap. Gated `#[allow(dead_code)]` for v1 (the demo
doesn't create a second AS); B5+ userspace `cap_create_address_space`
callers use it as `parent_cap_handle`.
- `BOOTSTRAP_AS_TABLE: StaticCell<CapabilityTable>` — the
kernel-init cap table holding the bootstrap AS cap. Same
`#[allow(dead_code)]` rationale.
- `fn activate_address_space(handle: AddressSpaceHandle)` — the
scheduler activation callback. Passed by name (no closure) to
`yield_now` / `ipc_send_and_yield` / `ipc_recv_and_yield` /
`start` at all 7 call sites. Body: SAFETY-commented StaticCell
derefs (UNSAFE-2026-0010 umbrella) + delegate to
`tyrne_kernel::mm::activate_address_space_handle`. v1 demo
never fires this function (all tasks share `BOOTSTRAP_ADDRESS_SPACE_HANDLE`
so `address_space_activation_target` short-circuits); the
wiring is in place so B5+ multi-AS userspace tasks slot in
additively.
- New AS-arena init block in `kernel_entry`, between the PMM
banner and GIC init. Order (per ADR-0028 §Simulation row 0):
1. `MMU.write(QemuVirtMmu::new())` — install the Mmu.
2. `AS_ARENA.write(AddressSpaceArena::new())` — install the arena.
3. Read `__boot_pt_l0`'s PA via `addr_of!`. Wrap as
`PhysFrame::from_aligned(...)` with an `.expect()` documenting
the linker.ld 4 KiB-alignment contract.
4. `QemuVirtAddressSpace::from_existing_root(l0_root)` — wrap
the live root (BSP-side safe constructor).
5. `AddressSpace::wrap_bootstrap(inner)` — kernel-side wrap
with metadata.
6. `create_address_space(arena, address_space)` — alloc arena
slot 0. The `.expect()` documents the empty-arena cannot-fail
contract.
7. `BOOTSTRAP_AS_TABLE.write(CapabilityTable::new())` + mint
the bootstrap AS cap via `insert_root` with all four
`CapRights` (kernel-init holds full authority). `.expect()`
documents the empty-table cannot-fail contract.
8. `BOOTSTRAP_AS_CAP.write(...)` — store the cap handle.
9. Print the new banner: `tyrne: address-space-arena ready
(1 / 8 slots used; bootstrap AS root = 0x<pa>)`.
- All 7 scheduler call sites updated from `|_| {}` to
`activate_address_space`: idle_entry's yield_now; task_a's
ipc_send_and_yield + ipc_recv_and_yield; task_b's
ipc_recv_and_yield + ipc_send_and_yield + yield_now;
kernel_entry's `start` call.
Verification:
- cargo fmt --all -- --check clean
- cargo host-clippy clean (-D warnings)
- cargo kernel-clippy clean (-D warnings)
- cargo host-test 136/136 kernel (unchanged from
commit 4 — this commit is BSP-
only + 1 helper addition in
kernel/mm/address_space.rs).
Workspace total 221.
- cargo kernel-build clean
- QEMU smoke (debug build) banner trace verified:
```
tyrne: hello from kernel_main
tyrne: mmu activated
tyrne: pmm initialized (32602 frames
available; 166 reserved)
tyrne: address-space-arena ready
(1 / 8 slots used; bootstrap
AS root = 0x40092000)
tyrne: timer ready (62500000 Hz,
resolution 16 ns)
tyrne: starting cooperative scheduler
tyrne: task B — waiting for IPC
tyrne: task A -- sending IPC
tyrne: task B — received IPC ...
tyrne: task A — received reply ...
tyrne: all tasks complete
tyrne: boot-to-end elapsed = ...
```
One new line `tyrne: address-space-
arena ready (...)` inserted between
PMM banner and timer banner per
ADR-0028 §Simulation row 0. Full
demo runs to `tyrne: all tasks
complete`; no new fault classes.
The PMM-reserved-frame count delta (166 vs T-017's 164; 32602
vs 32604 free) reflects this commit's `.bss` growth from the new
`AS_ARENA` static (~600 bytes for the 8-slot arena + the four
new StaticCells); the kernel-image-section growth rounds up
two additional frames at `__stack_top_aligned`. Expected.
`unsafe` surface: eight new `unsafe` blocks in `kernel_entry`'s
AS-init block + one in `activate_address_space`. All ride
existing UNSAFE-2026-0010 (StaticCell pattern) + UNSAFE-2026-0014
(momentary `&` / `&mut`) umbrellas. No new audit-log entries
needed.
Refs: ADR-0028, T-018
Audit: UNSAFE-2026-0010, UNSAFE-2026-0014
[adr-0028]: https://github.com/cemililik/Tyrne/blob/main/docs/decisions/0028-address-space-data-structure.md
[t-018]: https://github.com/cemililik/Tyrne/blob/main/docs/analysis/tasks/phase-b/T-018-address-space-kernel-object.md
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…closure-trio next
Closes the T-018 / ADR-0028 / B3 §§2-7 implementation arc. Updates
the audit log + architecture chapters + roadmap status to reflect
the post-merge reality.
What this commit updates:
**docs/audits/unsafe-log.md** — UNSAFE-2026-0014 gains a 2026-05-11
T-018 / ADR-0028 Amendment scope-extending the umbrella to:
- The activation-on-context-switch hook in `yield_now` /
`ipc_recv_and_yield` / `start` (the momentary
`task_address_space_handles[next_idx]` read in `start` lives
inside an unsafe block that ends before the activate /
context_switch sequence).
- The BSP-side `activate_address_space` closure in
`bsp-qemu-virt/src/main.rs` (dereferences `AS_ARENA` + `MMU`
StaticCells inside the scheduler's IrqGuard scope; ride
UNSAFE-2026-0010 for the static-cell pattern + UNSAFE-2026-0014
for the momentary `&` discipline).
Zero new audit-log entries — the AS-object layer is safe Rust over
the existing MMU + PMM + cap surfaces. The cap-gated wrappers'
`unsafe { mmu.create_address_space(root) }` call rides
UNSAFE-2026-0023's existing scope.
**UNSAFE-2026-0025 / 0026 `Pending QEMU smoke verification` notes
stay** — the v1 cooperative IPC demo doesn't call `cap_map` /
`cap_create_address_space` at runtime (all tasks share
`BOOTSTRAP_ADDRESS_SPACE_HANDLE`; activation hook short-circuits).
Host tests + Miri pin the paths; the notes lift via Amendment
when the first B5+ task with a per-task `AddressSpace` arms a
real call.
**docs/architecture/memory-management.md** — new §"Address-space
objects" section covering: `AddressSpace<M>` shape; arena layout;
`CapKind::AddressSpace` variant; cap-gated wrapper surface
(`cap_create_address_space` / `cap_map` / `cap_unmap`); bootstrap-AS
wrap special case (no `Mmu::create_address_space` call on the
live root); activation-on-context-switch hook; `Task` integration
(parallel array in scheduler + `address_space_handle` field on
`Task`); audit-log surface (zero new entries; UNSAFE-2026-0014
umbrella extension); smoke trace banner; forward-compat to
ADR-0033 (high-half migration).
**docs/architecture/boot.md** — Stage-3 sequence diagram extended
with the new arena-init step between PMM init and GIC init.
Prose description of `kernel_entry` updated to name the address-
space arena init step.
**docs/analysis/tasks/phase-b/T-018-address-space-kernel-object.md**
— Status flipped Draft → Done. Review-history row appended with
the closure summary: 6 bisectable commits, 221/221 host tests
(was 200 at PR #27 merge; +21 across 4 sites), QEMU smoke
trace + banner verbatim, pending-verification adjudication
explaining why UNSAFE-2026-0025 / 0026 notes stay, and pointers
to the memory-management.md + boot.md doc updates.
**docs/roadmap/current.md** — Active milestone updated: B3
implementation-complete 2026-05-11; T-018 Done; headline numbers
+ smoke banner + audit-log adjudication recorded; closure trio
is the next review trigger.
**docs/roadmap/phases/phase-b.md** — B3 §§2-7 flipped to ✅ with
per-item commit pointers (commit 1-2 → §3 kernel object; commit
3 → §4 map/unmap + §5 TLB-invalidate-on-unmap discharge; commit
4-5 → §6 activation; host tests across the chain → §7). T-018
added to "Tasks under B3" with `Done` status.
Verification:
- cargo fmt --all -- --check clean
- cargo host-clippy clean (-D warnings)
- cargo test 221/221 workspace
- cargo kernel-build clean
This commit is docs-only; no code touched.
Refs: ADR-0028, T-018
Audit: UNSAFE-2026-0014 (scope-extension Amendment)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ⓘ You've reached your Qodo monthly free-tier limit. Reviews pause until next month — upgrade your plan to continue now, or link your paid account if you already have one. |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @cemililik, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (17)
📝 WalkthroughWalkthroughThis PR completes task T-018 by introducing a typed, generic ChangesAddress-Space Kernel Object & Scheduler Activation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements the AddressSpace kernel object and its supporting infrastructure, including a dedicated arena and capability-gated wrappers for management. It integrates address space activation into the scheduler's context-switch paths and provides BSP-specific wiring for the bootstrap address space. Feedback highlights a resource leak in cap_create_address_space where physical frames are leaked during error recovery and suggests a simplification for the bootstrap root physical address calculation in the BSP entry point.
Comment on lines
+518
to
+527
| match table.insert_root(cap) { | ||
| Ok(cap_handle) => Ok(cap_handle), | ||
| Err(e) => { | ||
| // Best-effort rollback of the arena slot; ignore the | ||
| // unlikely double-free path (the slot was just allocated | ||
| // and we hold the only handle to it). | ||
| let _ = destroy_address_space(arena, handle); | ||
| Err(AddressSpaceError::CapError(e)) | ||
| } | ||
| } |
There was a problem hiding this comment.
In this error path, the arena slot is rolled back, but the physical frame allocated for the root page table is leaked. While noted as a v1 limitation, this is a resource leak that should be addressed for kernel stability. A similar leak occurs if create_address_space fails on line 513. To fix this, the FrameProvider trait could be extended with a free_frame method, and you would need to call it in these error paths to release the frame.
Collaborator
Author
There was a problem hiding this comment.
Addressed in commit 2ed16fc via your suggested preflight-checks approach (round-2 had deferred to "extend FrameProvider with free_frame" — a HAL trait surface change ADR-0028 explicitly avoids).
- Added
Arena::is_full()helper inkernel/src/obj/arena.rsmirroring the existingCapabilityTable::is_full(). cap_create_address_spacestep 3 now preflights botharena.is_full()andtable.is_full()beforepmm.alloc_frame().- Under v1 single-core cooperative semantics the preflight + commit sequence is atomic, so the post-PMM steps 6 + 7 cannot fail with capacity errors. The PMM-leak paths are structurally unreachable in all BSPs.
- The rollback arm in step 7 is retained for type honesty + forward-defensive coverage.
- New test
cap_create_address_space_rejects_missing_deriveasserts PMM stays untouched on preflight-rejected paths.
Comment on lines
+871
to
+883
| let (bootstrap_as_handle, bootstrap_root_pa) = unsafe { | ||
| let arena = (*AS_ARENA.0.get()).assume_init_mut(); | ||
| let inner = mmu::QemuVirtAddressSpace::from_existing_root(l0_root); | ||
| let address_space = tyrne_kernel::mm::AddressSpace::wrap_bootstrap(inner); | ||
| let handle = tyrne_kernel::mm::create_address_space(arena, address_space) | ||
| .expect("bootstrap AS allocation in empty arena cannot fail"); | ||
| let mmu = (*MMU.0.get()).assume_init_ref(); | ||
| let root_pa = tyrne_kernel::mm::get_address_space(arena, handle) | ||
| .expect("just-allocated handle resolves") | ||
| .root_frame(mmu) | ||
| .as_usize(); | ||
| (handle, root_pa) | ||
| }; |
There was a problem hiding this comment.
The calculation of bootstrap_root_pa is more complex than necessary. It's retrieved by inserting the address space into the arena and then reading it back out, but the physical address is already available in l0_root. You can simplify this by getting the address directly from l0_root, making the code more efficient and easier to read.
let bootstrap_root_pa = l0_root.as_usize();
// Wrap the already-live root + publish in arena slot 0.
// SAFETY: AS_ARENA was just written above.
// Audit: UNSAFE-2026-0010 (StaticCell pattern) + UNSAFE-2026-0014
// (momentary &mut to the just-initialised arena).
let bootstrap_as_handle = unsafe {
let arena = (*AS_ARENA.0.get()).assume_init_mut();
let inner = mmu::QemuVirtAddressSpace::from_existing_root(l0_root);
let address_space = tyrne_kernel::mm::AddressSpace::wrap_bootstrap(inner);
tyrne_kernel::mm::create_address_space(arena, address_space)
.expect("bootstrap AS allocation in empty arena cannot fail")
};
Collaborator
Author
There was a problem hiding this comment.
Addressed in commit 2cc1678 exactly as you suggested. The round-trip through arena → get_address_space → root_frame(mmu) is gone; bootstrap_root_pa = l0_root.as_usize() directly. Smoke verified — banner output byte-stable (bootstrap AS root = 0x40092000).
External review on PR #28 surfaced two Minor findings; both valid, both fixed: 1. **Axis 4 — `start`'s activation closure fires outside the `IrqGuard` scope** (`kernel/src/sched/mod.rs:669-678` pre-fix). `yield_now` and `ipc_recv_and_yield` both fire the activation closure **after** their `let _guard = IrqGuard::new(cpu);` construction (i.e., with interrupts masked). `start` had the discipline inverted — closure fired before the guard was constructed. At v1's `start` call site DAIF.I is already unmasked by `kernel_entry` (`bsp-qemu-virt/src/main.rs`'s GIC init + DAIF unmask runs before `start()`), so an IRQ taken between `activate` and `cpu.context_switch` would run the handler under the freshly-installed `TTBR0_EL1`. Harmless in v1 (every task shares the bootstrap AS so the new TTBR's kernel mappings are identical), but a forward-defensive correctness gate for B5+ multi-AS userspace where a per-task AS without the kernel mapping would translation-fault any IRQ taken in this window. Fix: moved `let _guard = IrqGuard::new(cpu);` above the `if let Some(target) = first_as { activate_address_space(target); }` block so the activation fires under the guard. The `first_as` read happens before the guard (the read is from the dropped-borrow window after `start_prelude` returns; no `&mut Scheduler<C>` is alive at this point — same discipline as before, just hoisted earlier in the function body). The activation closure body itself now runs under the IrqGuard scope, matching `yield_now` / `ipc_recv_and_yield`. Inline doc-comment expanded to name the IRQ-window forward-defensive rationale + ADR-0028 §Simulation row 3 reference + the v1-harmless / B5+-load-bearing distinction. 2. **Axis 9 — T-018 checkbox state contradicts `Status: Done`** (`docs/analysis/tasks/phase-b/T-018-address-space-kernel-object.md`). The task file's `Status: Done` (line 5) and review-history `Draft → Done` row (line 177) disagreed with the 60 unchecked `- [ ]` boxes in §Acceptance criteria / §Definition of done — including items that demonstrably landed (`AddressSpace<M>` struct, `cap_*` wrappers, activation hook, BSP wiring, all 17 AddressSpace host tests + 3 activation- hook tests, doc updates, verification gates). Fix: 55 boxes flipped to `[x]` for items that landed. 5 boxes stay `[ ]` (legitimate deferrals consistent with the T-018 review-history's adjudication): - Line 76: Optional second-AS smoke fixture (deferred — host tests + Miri instead per the review-history's "verified by host tests + Miri, runtime exercise deferred to B5+" branch). - Line 91: `activation_hook_no_alloc` test — the no-alloc property is structurally guaranteed (the hook calls `get_address_space` which is a read-only arena accessor; no PMM-touching code path exists) but a dedicated PMM-call-count test was not written. Marked unchecked for honesty. - Lines 95, 96, 133: UNSAFE-2026-0025 / UNSAFE-2026-0026 `Pending QEMU smoke verification` Amendments — the v1 cooperative IPC demo doesn't call `cap_map` / `cap_create_address_space`, so the runtime exerciser conditions are not met; the Amendments lift when the first B5+ task with a per-task `AddressSpace` arms a real call. Consistent with `current.md` + `phase-b.md` "Pending notes stay" wording. Also ran `cargo +nightly miri test -p tyrne-kernel --lib mm::address_space::tests` this session — 17/17 pass under Stacked Borrows. The §Acceptance-criteria miri box (line 111) and §Definition-of-done miri box (line 131) are therefore flipped to `[x]`. Verification: - cargo fmt --all -- --check clean - cargo host-clippy clean (-D warnings) - cargo kernel-clippy clean (-D warnings) - cargo host-test 221/221 workspace (unchanged — fix-up is no-op semantically for v1 single-AS demo; doc box flips don't affect tests). - cargo +nightly miri test (mm::address_space::tests) 17/17 (Stacked Borrows clean) - cargo kernel-build clean Refs: ADR-0028, T-018 Audit: UNSAFE-2026-0014 (existing umbrella scope, unchanged) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ith reason) Two PR-review findings from gemini-code-assist on PR #28: **Applied — Finding 2 (Medium): `bootstrap_root_pa` round-trip is over-complex.** `bsp-qemu-virt/src/main.rs:868-883` (pre-fix) re-read the just-allocated `AddressSpace<QemuVirtMmu>` from the arena and called `root_frame(mmu)` on it to recover the PA for the banner. But `l0_root` (the `PhysFrame` constructed from `__boot_pt_l0`'s linker symbol on line 865) is exactly the same frame the AS wraps via `wrap_bootstrap(inner)`; the `AddressSpace::root_frame` accessor returns `Mmu::address_space_root(inner)` which on `QemuVirtMmu` is `inner.root` which is `l0_root`. The round-trip is pinned by host test `wrap_bootstrap_returns_address_space_with_root` in `kernel/src/mm/address_space.rs::tests`. Fix: read the PA directly from `l0_root.as_usize()` before the arena allocation; drop the post-allocation arena-lookup + root_frame(mmu) chain + the no-longer-needed `mmu` variable inside the unsafe block. The unsafe block shrinks to the bare arena `&mut` + create_address_space call. Banner output is byte-stable (`0x40092000` per the smoke run); behavioural equivalence verified by QEMU smoke through `tyrne: all tasks complete`. **Skipped with reason — Finding 1 (High): PMM frame leak in cap_create_address_space rollback paths (line 513 + 527).** Both rollback paths (the `ArenaFull` at step 5 + the cap-mint failure at step 6) leak the just-allocated PMM root frame. This is **already documented** as a v1 limitation in commit 3's body + the function's own doc-comment at `kernel/src/mm/address_space.rs:427-435`. The fix gemini suggests — extending `FrameProvider` trait with a `free_frame` method — would be a HAL trait surface change. ADR-0028 §Positive consequences explicitly claims "**zero HAL trait surface change**" as a load-bearing property of T-018, and revising the HAL trait warrants its own ADR. Three reasons for the deferral: 1. **Structural unreachability in v1.** The `AddressSpaceArena` capacity is 8 and `CapabilityTable` capacity is 64; the v1 demo allocates exactly one AS (the bootstrap). Both leak paths are unreachable under v1's sized BSPs. 2. **HAL stability post-T-016.** The `Mmu` trait surface was stabilised by T-016 + ADR-0027's `MapperFlush` discipline. Adding `FrameProvider::free_frame` reopens a trait that has been stable for two PRs. ADR-0028 made the explicit trade-off: accept the v1 leak (documented) in exchange for HAL stability. 3. **Natural home is the B4+ `cap_revoke(AddressSpaceCap)` destroy path.** When the destroy path lands, it must walk the page-table tree freeing every L3 / L2 / L1 / L0 frame back to PMM — which means PMM must accept a free. At that point a HAL ADR (call it ADR-0036 or similar) adds `FrameProvider::free_frame` and the cap_create rollback paths gain proper rollback in lockstep. The finding is **valid in spirit** — a permanent leak is a real cost — but the fix is **out of scope for T-018**. T-018's goal was to land the AddressSpace kernel object without expanding the HAL surface; commit 3's doc-comment already names the limitation explicitly and the existing v1 BSP sizes make the unreachable claim sound. Verification: - cargo fmt --all -- --check clean - cargo host-clippy clean (-D warnings) - cargo kernel-clippy clean (-D warnings) - cargo host-test 221/221 workspace (unchanged) - cargo kernel-build clean - QEMU smoke full demo through `tyrne: all tasks complete`; banner `bootstrap AS root = 0x40092000` byte-stable. Refs: ADR-0028, T-018 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Self-review pass on the T-018 implementation surface surfaced
three Minor findings; all three valid, all three fixed.
**Finding 1 — Broken doc-comment structure around `activate_address_space`**
`bsp-qemu-virt/src/main.rs`: when commit 5 inserted
`fn activate_address_space` immediately before `fn idle_entry`,
the doc-comment structure mangled. `idle_entry`'s main prose
(lines 333-358 — "Kernel idle task — runs when no application
task is ready. Per [ADR-0022] / [ADR-0026] ...") flowed without
a blank separator into `activate_address_space`'s new
doc-comment block; Rust merged the two into one combined
doc-comment attached to `activate_address_space`. The
`[ADR-0022]` / `[ADR-0026]` link-reference definitions
(originally trailing `idle_entry`'s prose) ended up as the
only doc-comment lines on `fn idle_entry` itself — semantically
empty for the reader.
Fix: relocated `fn activate_address_space` (+ its doc-comment
+ SAFETY block) from immediately-before-`fn idle_entry` to a
new section between the AddressSpace `StaticCell` block and
the IPC infrastructure section. Conceptually `activate_address_space`
belongs in the AS-infrastructure neighbourhood (it
dereferences `AS_ARENA` + `MMU` which live there); structurally
the move restores `idle_entry`'s doc-comment + link-references
to a contiguous block. `cargo doc` now renders both functions
with their intended prose.
**Finding 2 — Stale "commit 6 lifts Pending note" claims**
`kernel/src/mm/address_space.rs`: commit 5's `cap_create_address_space`
SAFETY block (line 508) and `cap_map` doc-comment (line 541)
both claimed that T-018 commit 6's audit-log Amendment lifts
UNSAFE-2026-0023 / 0025 / 0026's `Pending QEMU smoke verification`
notes. Commit 6 explicitly did NOT lift these notes (the v1
demo doesn't call `cap_map` / `cap_create_address_space` at
runtime; T-018 review-history's adjudication says the notes
stay until B5+ first userspace AS creation).
Fix: rewrote both sites to honestly describe the future-lift
condition. `cap_map`'s doc-comment now says "**Will be** the
first post-bootstrap runtime exerciser ... when a real caller
arms it" + names the v1 host-tests + Miri as the current
evidence base. `cap_create_address_space`'s SAFETY block now
points at the trait-level unsafe declaration rather than
claiming an audit-log Amendment that didn't happen (see
Finding 3).
**Finding 3 — Wrong audit-entry attribution in `cap_create_address_space`**
`kernel/src/mm/address_space.rs:506-509`: the SAFETY block for
`unsafe { mmu.create_address_space(root) }` cited
"Audit: UNSAFE-2026-0023 (existing umbrella scope)". But
UNSAFE-2026-0023's actual scope is `MSR TTBR0_EL1` writes in
`QemuVirtMmu::activate` (plus the 2026-05-08 Amendment
extending to the `mmu_bootstrap` MSR block) — not
`Mmu::create_address_space` calls. The BSP impl body for
`QemuVirtMmu::create_address_space` is a trivial struct wrap
(`QemuVirtAddressSpace { root }`) with no MMIO, no
system-register write, no memory write.
The unsafe at the call site is purely the HAL trait
declaration: `Mmu::create_address_space` is `unsafe fn` per
ADR-0009 to force every caller through an audit-disciplined
site, but the BSP impl body has nothing requiring its own
audit-log entry.
Fix: rewrote the SAFETY block to:
- State explicitly that the unsafe lives at the HAL trait
declaration (ADR-0009), not in any specific audit-log
entry's operation field.
- Note that for the `QemuVirtMmu` impl the body is trivial;
future BSP impls performing real work (e.g., pre-populating
kernel-half mappings) would introduce their own audit entry.
- Document the three invariants the call site upholds:
page-alignment (statically by `PhysFrame`), zero-fill
(UNSAFE-2026-0026 PMM contract), exclusive ownership
(stack-local `PhysFrame` value moves into the arena slot).
Also fixed the corresponding step-4 description in the
function's doc-comment (line 420-425) + removed the stale
`[UNSAFE-2026-0023]` link-reference at line 447.
Verification:
- cargo fmt --all -- --check clean
- cargo host-clippy clean (-D warnings)
- cargo kernel-clippy clean (-D warnings)
- cargo host-test 221/221 workspace (unchanged
— all fixes are doc-only +
one fn-relocation; no code
semantic changes)
- cargo kernel-build clean
- QEMU smoke full demo through
`tyrne: all tasks complete`;
banner byte-stable
(`bootstrap AS root = 0x40092000`).
No new audit-log entries. No HAL trait surface change. No
behavioural changes — `activate_address_space` semantics are
identical post-relocation; the doc-comment + SAFETY-block
rewrites preserve every invariant statement.
Refs: ADR-0028, T-018
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@bsp-qemu-virt/src/mmu.rs`:
- Around line 85-106: The constructor QemuVirtAddressSpace::from_existing_root
is currently a pub safe fn but lets callers create an address space from any
PhysFrame whereas subsequent methods (map/unmap) rely on the frame being a valid
live VMSAv8 root and perform unsafe dereferences; change the API to restore the
safety boundary by either marking the constructor pub unsafe fn with a clear
safety contract that the provided root must be a valid, currently-live L0
translation table (document this and reference Mmu::create_address_space's
safety contract), or narrow visibility to pub(crate) if it is only used for
bootstrap, and update callsites accordingly so only trusted/bootstrap code can
create instances.
In `@docs/analysis/tasks/phase-b/T-018-address-space-kernel-object.md`:
- Line 5: The task T-018 currently shows "Status: Done" and "All Acceptance
criteria checked" while some audit-amendment checklist items (the unchecked
"audit-amendment" acceptance criteria) remain; either set the task status to "In
Progress" and uncheck "All Acceptance criteria checked", or remove the pending
audit-amendment checklist lines from T-018 and create a new task for those
items, then update T-018’s acceptance criteria to reflect only completed gates
and ensure the "audit-amendment" items are moved or marked appropriately.
In `@docs/audits/unsafe-log.md`:
- Around line 246-250: The amendment header and location bullets use relative
commit labels ("commit-5 BSP-wiring", "commit 5 / commit 4") instead of the
file's required date + commit SHA format; update the amendment header line
starting "Amendment (2026-05-11, T-018 / ADR-0028, commit-5 BSP-wiring)" to
include the actual commit SHA(s) for the BSP wiring commit and ensure the header
matches the existing date+SHA format, and likewise replace "commit 5 / commit 4"
in the location bullet with the exact SHAs for those commits; follow the audit
tracking format defined in docs/standards/ when making these edits and keep the
surrounding text (references to
yield_now/ipc_recv_and_yield/start/activate_address_space) unchanged so the
unique symbols remain locatable.
In `@kernel/src/mm/address_space.rs`:
- Around line 589-650: cap_map and cap_unmap currently resolve the address-space
cap but never enforce per-operation rights, so a cap with CapRights::empty() can
still map/unmap; before calling mmu.map / mmu.unmap, inspect the resolved cap's
rights (from the value returned by resolve_address_space_cap) and return
AddressSpaceError::CapError if the cap lacks the required right (check
CapRights::MAP for cap_map and CapRights::UNMAP for cap_unmap). Update cap_map
and cap_unmap to perform this rights check using the same cap value used to
obtain the AddressSpaceHandle (and do not proceed to get_address_space_mut or
call mmu.* if the right is missing). Ensure you reference the resolved cap,
cap_handle, resolve_address_space_cap, cap_map, cap_unmap, and the
AddressSpaceError::CapError/CapRights symbols in your change.
- Around line 486-545: The code currently allocates a physical frame via
pmm.alloc_frame() into local variable root before doing arena slot creation
(create_address_space(arena, AddressSpace::from_mmu_address_space(inner))) and
cap-table insertion (table.insert_root), so failures there leak the frame; fix
by either performing preflight capacity checks on the arena/cap table (e.g.,
call an arena.has_free_slot() and table.has_capacity() helper) before calling
pmm.alloc_frame(), or add a proper rollback that returns the frame to the PMM on
any later error (in the Err(e) arm after table.insert_root fails call the PMM
free/dealloc API to return root, then call destroy_address_space(arena, handle)
as currently done); update code paths around pmm.alloc_frame,
Mmu::create_address_space, create_address_space, table.insert_root and
destroy_address_space to ensure the root frame is always freed on error.
- Around line 356-364: The function activate_address_space_handle currently
silently does nothing when get_address_space(arena, handle) returns None which
can leave the previous MMU context active; change this to fail-closed by
propagating or signaling an error instead of a no-op: modify
activate_address_space_handle to return a Result (or panic) on missing handle
and ensure callers handle the error, or explicitly panic inside
activate_address_space_handle when get_address_space returns None; update call
sites accordingly so Mmu::activate is never skipped silently and include the
handle value in the error/panic message to aid debugging.
- Around line 481-484: The current no-widening check only ensures new_rights ⊆
parent_cap.rights() but doesn't require explicit authority to create an address
space; update the guard in the block that presently checks
parent_cap.rights().contains(new_rights) to also require that
parent_cap.rights() contains CapRights::DERIVE (or a dedicated create right)
before allowing allocation/creation (returning an
AddressSpaceError::CapError(CapError::WidenedRights) or a more specific error if
DERIVE is missing); reference parent_cap, new_rights, and the
AddressSpaceError::CapError/CAPError::WidenedRights handling when adding this
additional check and error path so PMM state cannot be touched without the
DERIVE authority.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9af84837-5775-44b1-a93f-6fda76de9212
📒 Files selected for processing (14)
bsp-qemu-virt/src/main.rsbsp-qemu-virt/src/mmu.rsdocs/analysis/tasks/phase-b/T-018-address-space-kernel-object.mddocs/architecture/boot.mddocs/architecture/memory-management.mddocs/audits/unsafe-log.mddocs/roadmap/current.mddocs/roadmap/phases/phase-b.mdkernel/src/cap/mod.rskernel/src/mm/address_space.rskernel/src/mm/mod.rskernel/src/obj/arena.rskernel/src/obj/task.rskernel/src/sched/mod.rs
Comment on lines
+589
to
+650
| pub fn cap_map<M: Mmu>( | ||
| table: &CapabilityTable, | ||
| cap_handle: CapHandle, | ||
| mmu: &M, | ||
| pmm: &mut dyn FrameProvider, | ||
| arena: &mut AddressSpaceArena<M>, | ||
| va: VirtAddr, | ||
| pa: PhysFrame, | ||
| flags: MappingFlags, | ||
| ) -> Result<(), AddressSpaceError> { | ||
| let handle = resolve_address_space_cap(table, cap_handle)?; | ||
| let address_space = | ||
| get_address_space_mut(arena, handle).ok_or(AddressSpaceError::StaleHandle)?; | ||
| let token = mmu | ||
| .map(address_space.inner_mut(), va, pa, flags, pmm) | ||
| .map_err(AddressSpaceError::MmuMapError)?; | ||
| token.flush(mmu); | ||
| Ok(()) | ||
| } | ||
|
|
||
| /// Capability-gated mapping removal. | ||
| /// | ||
| /// Per [ADR-0028 §Simulation row 2][adr-0028]. Mirrors [`cap_map`] | ||
| /// inversely: resolve the cap, `&mut AddressSpace<M>` from the | ||
| /// arena, call [`Mmu::unmap`], discharge the flush token, return | ||
| /// the orphaned [`PhysFrame`] for caller-side handling (typically | ||
| /// PMM `free_frame` once `cap_revoke(MemoryRegionCap)` lands in | ||
| /// B4+; v1's T-018 tests just verify the return value matches the | ||
| /// originally-mapped PA). | ||
| /// | ||
| /// The intermediate L1/L2/L3 frames that become orphaned when the | ||
| /// last L3 page in a subtree is unmapped are deferred to the per-AS | ||
| /// destroy path (B4+); T-018 wires the per-page unmap discipline | ||
| /// only. | ||
| /// | ||
| /// # Errors | ||
| /// | ||
| /// - [`CapError(_)`] — cap lookup / kind validation. | ||
| /// - [`StaleHandle`] — the cap's [`AddressSpaceHandle`] no longer | ||
| /// names a live arena slot. | ||
| /// - [`MmuUnmapError(MmuError)`] — pass-through from `Mmu::unmap` | ||
| /// (`NotMapped`, `MisalignedAddress`, ...). | ||
| /// | ||
| /// [adr-0028]: https://github.com/cemililik/Tyrne/blob/main/docs/decisions/0028-address-space-data-structure.md | ||
| /// [`CapError(_)`]: AddressSpaceError::CapError | ||
| /// [`StaleHandle`]: AddressSpaceError::StaleHandle | ||
| /// [`MmuUnmapError(MmuError)`]: AddressSpaceError::MmuUnmapError | ||
| pub fn cap_unmap<M: Mmu>( | ||
| table: &CapabilityTable, | ||
| cap_handle: CapHandle, | ||
| mmu: &M, | ||
| arena: &mut AddressSpaceArena<M>, | ||
| va: VirtAddr, | ||
| ) -> Result<PhysFrame, AddressSpaceError> { | ||
| let handle = resolve_address_space_cap(table, cap_handle)?; | ||
| let address_space = | ||
| get_address_space_mut(arena, handle).ok_or(AddressSpaceError::StaleHandle)?; | ||
| let (token, pa) = mmu | ||
| .unmap(address_space.inner_mut(), va) | ||
| .map_err(AddressSpaceError::MmuUnmapError)?; | ||
| token.flush(mmu); | ||
| Ok(pa) |
There was a problem hiding this comment.
Address-space rights narrowing is ineffective for cap_map / cap_unmap.
Both wrappers resolve the cap kind and then mutate the page tables without checking any operation-specific right. In practice, a capability minted with CapRights::empty() can still map and unmap pages, so transferring a “narrowed” AS cap does not actually reduce authority. Please add and enforce explicit map/unmap rights before calling into the MMU.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@kernel/src/mm/address_space.rs` around lines 589 - 650, cap_map and cap_unmap
currently resolve the address-space cap but never enforce per-operation rights,
so a cap with CapRights::empty() can still map/unmap; before calling mmu.map /
mmu.unmap, inspect the resolved cap's rights (from the value returned by
resolve_address_space_cap) and return AddressSpaceError::CapError if the cap
lacks the required right (check CapRights::MAP for cap_map and CapRights::UNMAP
for cap_unmap). Update cap_map and cap_unmap to perform this rights check using
the same cap value used to obtain the AddressSpaceHandle (and do not proceed to
get_address_space_mut or call mmu.* if the right is missing). Ensure you
reference the resolved cap, cap_handle, resolve_address_space_cap, cap_map,
cap_unmap, and the AddressSpaceError::CapError/CapRights symbols in your change.
Collaborator
Author
There was a problem hiding this comment.
Skipped with reason. Adding CapRights::MAP / CapRights::UNMAP variants is out of scope for T-018:
- Adds new
CapRightsvariants — requires ADR coverage (the rights set is settled by ADR-0014; extending it warrants an ADR §Revision notes amendment or successor ADR). - Requires
CapRights::KNOWN_BITSmask update + new variant tests + ADR-0014 documentation. - ADR-0028 §Forward-flag explicitly defers per-operation rights to B4+
MemoryRegionCapwork: "v1 adopts the simplest v1 shape and forward-flags the Phase-C Untyped story."
The correct home for CapRights::MAP / UNMAP is a separate ADR landing alongside MemoryRegionCap. T-018's current behavior is the documented v1 simplification — strengthened in this round by the DERIVE check applied at step 2a (commit 2ed16fc). The DERIVE check is the v1 authority gate; per-operation rights replace it in B4+.
The narrowing-is-ineffective observation IS correct under the v1 simplification; the fix is the rights-set extension ADR, not a T-018 patch.
There was a problem hiding this comment.
@cemililik — understood, and the rationale is sound. The observation that narrowing is ineffective under v1 remains correct, but the fix belongs to the rights-set extension ADR alongside MemoryRegionCap, not here. The DERIVE check at step 2a in commit 2ed16fc is the documented v1 authority gate, and ADR-0028 §Forward-flag explicitly marks per-operation rights as B4+ scope. I'll withdraw the suggestion for this PR.
Would you like me to open a follow-up GitHub issue to track the CapRights::MAP / CapRights::UNMAP extension (new CapRights variants, KNOWN_BITS update, ADR-0014 §Revision amendment, and enforcement in cap_map / cap_unmap) so it doesn't get lost before the B4+ MemoryRegionCap ADR lands?
✏️ Learnings added
Learnt from: cemililik
Repo: cemililik/Tyrne PR: 28
File: kernel/src/mm/address_space.rs:675-736
Timestamp: 2026-05-11T18:00:56.941Z
Learning: In `kernel/src/mm/address_space.rs` (Rust, Tyrne kernel), `cap_map` and `cap_unmap` intentionally do not enforce per-operation rights (e.g., `CapRights::MAP` / `CapRights::UNMAP`). This is a documented v1 simplification per ADR-0028 §Forward-flag: the v1 authority gate is the DERIVE/rights-widening check on the parent cap (step 2a). Per-operation `CapRights` variants require an ADR-0014 §Revision amendment (updating `CapRights::KNOWN_BITS`, adding tests, updating docs) and are deferred to the B4+ `MemoryRegionCap` ADR. Do not flag the absence of per-operation rights enforcement in these wrappers as a bug for T-018 scope.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
External review surfaced 7 findings. Per-finding triage:
**Applied:**
1. **`from_existing_root` safety boundary (Major)** —
`bsp-qemu-virt/src/mmu.rs:103`. Previously `pub fn`, letting
any caller construct a `QemuVirtAddressSpace` from an arbitrary
`PhysFrame`. Downstream `Mmu::map`/`unmap` perform `volatile`
reads + writes through the descriptor chain rooted at this
frame — passing a non-VMSAv8-L0 frame produces UB at the
page-table walker.
Fix: change to `pub unsafe fn` with a documented `# Safety`
contract that the root must be a currently-live VMSAv8 L0
translation table (correctly-encoded descriptors, kernel-half
mappings populated). Distinct from `Mmu::create_address_space`'s
contract (which requires zero-fill, not live mappings) — both
are caller-side preconditions the type system can't enforce.
Updated the single v1 call site in `kernel_entry` to add a
SAFETY comment justifying the contract is met (root derived
from `__boot_pt_l0` which `mmu_bootstrap` populated + activated
via `TTBR0_EL1` before this block runs).
2. **PMM-leak via preflight capacity checks (Minor)** —
`kernel/src/mm/address_space.rs::cap_create_address_space`.
Round-2 deferred the leak fix to a future ADR (extend
`FrameProvider` with `free_frame`); the reviewer's alternative
— preflight checks on arena + cap-table capacity before PMM
alloc — works without HAL surface change.
Added `Arena::is_full()` helper (`kernel/src/obj/arena.rs:184`,
`pub const fn`; mirrors `CapabilityTable::is_full()`'s existing
shape). `cap_create_address_space` now does two preflight
checks at step 3, **before** `pmm.alloc_frame()`:
- `arena.is_full()` → `Err(ArenaFull)` (no PMM mutation yet)
- `table.is_full()` → `Err(CapError(CapsExhausted))` (ditto)
Under v1's single-core cooperative model the preflight + commit
sequence is atomic (no peer modifies arena/table between
them), so the post-PMM steps 6 + 7 cannot fail with capacity
errors. The PMM-leak paths are now structurally unreachable in
**all** BSPs (not just v1's sized arenas). The rollback arm in
step 7's `match table.insert_root(cap)` is retained for type
honesty + forward-defensive coverage but should not fire.
3. **`activate_address_space_handle` fail-soft trade-off (Minor)** —
`kernel/src/mm/address_space.rs:356`. Previous behaviour was
silent no-op on stale handle. Reviewer wanted fail-closed
(Result or panic).
Compromise applied: added `debug_assert!(false, ...)` in the
else branch with handle interpolation for dev-time diagnostics.
Release-build behaviour stays as silent no-op — panic'ing
from inside the activation hook's `IrqGuard` scope (mandatory
per `yield_now` / `ipc_recv_and_yield` / `start`'s discipline)
would leave the kernel in a silent-halt state because the
panic handler may not reach the console under
interrupts-disabled discipline. Doc-comment expanded with
explicit fail-soft rationale + forward-flag that v1's
stale-handle case is structurally unreachable.
Not changed to Result-returning API (bigger BSP-side surface
change; v1's scheduler/start call sites would need
error-propagation paths that have no useful action besides
the same silent-or-panic choice).
4. **DERIVE rights check in `cap_create_address_space` (Minor)** —
`kernel/src/mm/address_space.rs:486`. Previous code only
checked no-widening (new_rights ⊆ parent.rights); didn't
require parent to hold any specific authority. Mirrors
`CapabilityTable::cap_derive`'s discipline.
Added: `if !parent_cap.rights().contains(CapRights::DERIVE)
{ return Err(CapError(InsufficientRights)); }` as step 2a
(before the no-widening step 2b). v1 kernel-init holds the
bootstrap AS cap with all four rights, so the check is
structurally satisfied; forward-defensive for narrowed AS
caps in B5+ Phase-C `Untyped` discipline.
Updated `cap_create_address_space_rejects_widened_rights`
test fixture: parent now has `CapRights::DERIVE` only (not
`CapRights::empty()`); attempting to widen to DUPLICATE
triggers the no-widening branch (step 2b), as intended,
instead of being blocked at the new DERIVE check.
Added `cap_create_address_space_rejects_missing_derive`
test pinning the new DERIVE-check branch. Asserts PMM is
untouched (preflight discipline holds).
5. **T-018 Status: Done vs unchecked-boxes contradiction
(Minor)** —
`docs/analysis/tasks/phase-b/T-018-address-space-kernel-object.md:128`.
Previously `[x] All Acceptance criteria checked.` but 5 boxes
remain `[ ]`. The 5 unchecked items are intentional deferrals
per the Review-history adjudication (second-AS smoke fixture,
`activation_hook_no_alloc` dedicated test, UNSAFE-2026-0025
+ UNSAFE-2026-0026 lift Amendments at 3 reference sites).
Reworded line 128 to: "All Acceptance criteria checked **or
explicitly deferred per Review history**" with the 5 items
enumerated inline. Honest representation of the actual state;
Status stays `Done` because T-018's implementation work IS
complete — only the deferred audit Amendments + optional
smoke fixture remain, and those are explicitly tracked.
6. **Audit-log Amendment SHA format (Nit)** —
`docs/audits/unsafe-log.md:246, 248`. My T-018 Amendment used
"commit-5 BSP-wiring" / "commit 5 / commit 4" instead of the
file's existing format (date + task + commit SHA in
backticks, per the 2026-05-06 / 2026-05-07 Amendments at
lines 218, 225, 236, 241).
Fixed: replaced "commit-5 BSP-wiring" with `commit `0d16ea4`
+ commit `1b0f1d9`` (the actual scheduler-hook + BSP-wiring
commit SHAs); same fix at the "Additional locations" bullet.
**Skipped with reason:**
A. **Per-operation rights (`CapRights::MAP` / `UNMAP`) (Minor) —
reviewer's `cap_map`/`cap_unmap` rights-check finding.**
Out of scope for T-018. Adding new `CapRights` variants
requires:
- ADR coverage (the rights set is settled by ADR-0014;
extending it warrants a new ADR or §Revision-notes
amendment).
- `KNOWN_BITS` mask update + tests.
- Documentation in ADR-0014 or successor.
ADR-0028 §Forward-flag explicitly defers per-operation
rights discipline to B4+ `MemoryRegionCap` work: "v1
adopts the simplest v1 shape and forward-flags the
Phase-C `Untyped` story." The current v1 behaviour
("any AS cap with DERIVE grants map/unmap authority")
is the documented v1 simplification. The correct home
for `CapRights::MAP` / `UNMAP` is a separate ADR landing
alongside the `MemoryRegionCap` introduction.
Verification:
- cargo fmt --all -- --check clean
- cargo host-clippy clean (-D warnings)
- cargo kernel-clippy clean (-D warnings)
- cargo host-test 222/222 workspace
(137 kernel; was 136 — +1
cap_create_address_space_rejects_missing_derive)
- cargo kernel-build clean
- QEMU smoke full demo through
`tyrne: all tasks complete`;
banner byte-stable.
Refs: ADR-0028, T-018, ADR-0014 (cap rights discipline)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…asks, and documentation - Added `propose-standard-change` skill for proposing changes to existing standards. - Introduced `start-task` skill for creating new roadmap tasks with structured documentation. - Created `supersede-adr` skill to manage the supersession of existing ADRs. - Implemented `sync-adr-index` skill to rebuild the ADR index from existing files. - Added `update-glossary` skill for adding or updating glossary entries. - Introduced `write-adr` skill for drafting new Architecture Decision Records. - Created `write-architecture-doc` skill for documenting subsystems and components. - Added `write-guide` skill for creating task-oriented guides for contributors and users.
…ction fix(decisions): enhance clarity on ASID isolation and TLB flushing in address-space data structure feat(capabilities): add depth checking for capability derivation to prevent resource leaks fix(scheduler): ensure proper handling of idle task scenarios to prevent self-context-switching
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
.agents/skills/perform-code-review/SKILL.md (1)
18-21: ⚡ Quick winAvoid duplicating the security-sensitive subsystems list.
Line 20 embeds a list of security-sensitive subsystems that is presumably also defined in
security-review.md. If that authoritative list changes, this copy will drift out of sync, creating a maintenance burden and potential confusion.Consider rephrasing to: "Does the change touch any of the subsystems listed under security-review.md — Scope?" and removing the embedded list.
♻️ Proposed refactor
- - Does the change touch any of the subsystems listed under [security-review.md — Scope](../../../docs/standards/security-review.md)? - - Capabilities, IPC, syscalls, memory management, scheduler, boot, cryptography, authentication, `unsafe` regions, security-sensitive dependencies. - - If **yes**, this skill alone is not sufficient — also run [perform-security-review](../perform-security-review/SKILL.md) as a separate pass. + - Does the change touch any of the subsystems listed under [security-review.md — Scope](../../../docs/standards/security-review.md)? (Capabilities, IPC, syscalls, memory management, scheduler, boot, cryptography, authentication, `unsafe` regions, or security-sensitive dependencies.) + - If **yes**, this skill alone is not sufficient — also run [perform-security-review](../perform-security-review/SKILL.md) as a separate pass.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.agents/skills/perform-code-review/SKILL.md around lines 18 - 21, The embedded copy of security-sensitive subsystems under the "2. **Identify risk class.**" section should be removed to avoid duplication; change the bulleted subpoint to a single sentence that references the canonical list in security-review.md (e.g., "Does the change touch any of the subsystems listed under [security-review.md — Scope](../../../docs/standards/security-review.md)?") and delete the following indented list text so the file no longer mirrors the authoritative list; update only the content under the "Identify risk class." heading and leave the rest intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.agents/skills/add-bsp/SKILL.md:
- Around line 36-46: The fenced code blocks in SKILL.md that start with the
directory tree "bsp-<target>/" and the other two unlabeled fences (the assembly
snippet containing "boot.s" / "main.rs" lines and the shell snippet around lines
179) need explicit language identifiers to satisfy markdown lint MD040; update
the three fences to use appropriate info strings (for example use "text" for the
directory tree block, "asm" for the assembly/boot.s snippet, and "sh" for the
shell/command snippet) so each triple-backtick fence becomes ```text, ```asm,
and ```sh respectively.
In @.agents/skills/conduct-approval-review/SKILL.md:
- Line 11: The artefact path example in SKILL.md uses
docs/analysis/reviews/<type>/YYYY-MM-DD-*.md which conflicts with the repo's
canonical "*-reviews" directory naming; update that example to use the canonical
pattern (e.g. docs/analysis/<type>-reviews/YYYY-MM-DD-*.md) so the
approval-review input selector will match real files, and verify there are no
other examples in SKILL.md still using docs/analysis/reviews/<type>/... .
In @.agents/skills/conduct-review/SKILL.md:
- Line 3: Update the review artifact path template in SKILL.md so it matches the
repository's actual directory names (use the "*-reviews" pattern instead of
"reviews/<type>"). Specifically, replace occurrences of
"docs/analysis/reviews/<type>/" in the skill description and in the master-plan
link examples with the corresponding "docs/analysis/<type>-reviews/" form (e.g.,
"code-reviews", "business-reviews"); ensure all instances in the file (the
description string and the master-plan link lines) are updated so outputs and
README links point to the real directories.
In @.agents/skills/propose-standard-change/SKILL.md:
- Line 43: Update the stale repository path in the downstream search checklist
entry that currently reads "Skills (`.Codex/skills/`)" to reflect the current
convention ".agents/skills/"; locate the string "Skills (`.Codex/skills/`)" in
SKILL.md and replace it (and any other occurrences of `.Codex/skills/`) with
".agents/skills/" so contributors are pointed to the correct folder.
In `@docs/analysis/tasks/phase-b/T-018-address-space-kernel-object.md`:
- Around line 62-63: The doc still says “No TLB flush at activate” but the
code/ADR now perform an explicit global TLB invalidate on address-space
switches; update the bullet for T-018 to reflect that activation now issues a
global TLB flush (replace the “No TLB flush at activate” line with a statement
that activation performs a global invalidate on AS switch), remove guidance that
callers must rely on map-before-activate for stale-entry avoidance, and
reference the relevant policy symbols (TCR_EL1.AS, the activation hook, T-018
tests and ADR-0033) so the text aligns with the implemented behavior.
In `@docs/audits/unsafe-log.md`:
- Around line 248-249: The audit list omitted
kernel/src/sched/mod.rs::ipc_send_and_yield which is part of T-018; update the
paragraph that enumerates activation-hook sites to include ipc_send_and_yield
(or add an explicit sentence explaining why ipc_send_and_yield is intentionally
excluded), and ensure any referenced helper like address_space_activation_target
or the closure returned by address_space_activation_target is mentioned so the
amendment fully enumerates all introduced activation-hook sites for
traceability.
---
Nitpick comments:
In @.agents/skills/perform-code-review/SKILL.md:
- Around line 18-21: The embedded copy of security-sensitive subsystems under
the "2. **Identify risk class.**" section should be removed to avoid
duplication; change the bulleted subpoint to a single sentence that references
the canonical list in security-review.md (e.g., "Does the change touch any of
the subsystems listed under [security-review.md —
Scope](../../../docs/standards/security-review.md)?") and delete the following
indented list text so the file no longer mirrors the authoritative list; update
only the content under the "Identify risk class." heading and leave the rest
intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 780ac08f-4c69-49e9-b0b9-6c1ab01f4d9b
📒 Files selected for processing (25)
.agents/skills/add-bsp/SKILL.md.agents/skills/add-dependency/SKILL.md.agents/skills/conduct-approval-review/SKILL.md.agents/skills/conduct-review/SKILL.md.agents/skills/justify-unsafe/SKILL.md.agents/skills/perform-code-review/SKILL.md.agents/skills/perform-security-review/SKILL.md.agents/skills/propose-standard-change/SKILL.md.agents/skills/start-task/SKILL.md.agents/skills/supersede-adr/SKILL.md.agents/skills/sync-adr-index/SKILL.md.agents/skills/update-glossary/SKILL.md.agents/skills/write-adr/SKILL.md.agents/skills/write-architecture-doc/SKILL.md.agents/skills/write-guide/SKILL.mdbsp-qemu-virt/src/main.rsbsp-qemu-virt/src/mmu.rsdocs/analysis/tasks/phase-b/T-018-address-space-kernel-object.mddocs/audits/unsafe-log.mddocs/decisions/0028-address-space-data-structure.mdhal/src/mmu/mod.rskernel/src/cap/table.rskernel/src/mm/address_space.rskernel/src/obj/arena.rskernel/src/sched/mod.rs
✅ Files skipped from review due to trivial changes (7)
- .agents/skills/justify-unsafe/SKILL.md
- .agents/skills/update-glossary/SKILL.md
- .agents/skills/supersede-adr/SKILL.md
- .agents/skills/start-task/SKILL.md
- .agents/skills/write-guide/SKILL.md
- .agents/skills/add-dependency/SKILL.md
- .agents/skills/sync-adr-index/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (3)
- kernel/src/obj/arena.rs
- kernel/src/mm/address_space.rs
- bsp-qemu-virt/src/main.rs
8b9f52e (6 valid, 1 skipped) CodeRabbit posted a fresh batch of inline findings on the latest two commits (`0d4e62c` skills + `8b9f52e` round-3 follow-up). Per-finding triage: **Applied:** 1. **add-bsp/SKILL.md MD040 fence labels (Nit/Quick win)** — 3 unlabeled fenced code blocks at lines 36 (directory tree), 157 (smoke trace), and 179 (commit message). Added `text` identifier to each. CodeRabbit also flagged L83 (asm) as unlabeled but it's already `asm`; left untouched. 2. **conduct-approval-review review-path drift (Minor)** — L11 used `docs/analysis/reviews/<type>/YYYY-MM-DD-*.md` which would substitute to `docs/analysis/reviews/business/` etc., but the actual directory is `business-reviews`. CodeRabbit's proposed canonical (`docs/analysis/<type>-reviews/`) is itself wrong — there's no such top-level dir; the real path is `docs/analysis/reviews/<type>-reviews/`. Added `-reviews` suffix to the placeholder. 3. **conduct-review review-path drift (Minor)** — same drift as F2, applied at 5 sites (L3, L36, L40, L49, L58). 4. **propose-standard-change stale path (Minor)** — L43 referenced `.Codex/skills/` (old layout from prior project name); current layout is `.agents/skills/`. Replaced. 5. **T-018 task "No TLB flush at activate" drift (Major)** — `docs/analysis/tasks/phase-b/T-018-address-space-kernel-object.md` L62 said the activation hook deliberately does NOT invalidate the TLB, but the implementation in `bsp-qemu-virt/src/mmu.rs:: QemuVirtMmu::activate` issues `TLBI VMALLE1` (landed strictly more conservative than the original ADR-0028 advisory text). Rewrote the bullet to describe the actual 5-step activate sequence + TCR_EL1.AS=0 motivation. Also updated the related L60 sentence which previously said only "DSB ISH + ISB". 6. **unsafe-log activation-hook enumeration incomplete (Minor)** — The 2026-05-11 Amendment on UNSAFE-2026-0014 listed `yield_now`, `ipc_recv_and_yield`, `start`, and the BSP closure — but omitted `ipc_send_and_yield` (which threads the `activate_address_space` closure into its delegated `yield_now` call) and the pure `address_space_activation_target` helper. Added both for traceability; the activation itself still fires inside `yield_now` so no safety-argument change. **Skipped (1):** 7. **perform-code-review duplicated subsystems list (Nit)** — CodeRabbit suggested removing the embedded list of security-sensitive subsystems and referencing `security-review.md` only. The embedded list is informative for first-time readers and the reference is already adjacent; drift risk is low because both files are reviewed when scope changes. Skipped per the minimal-change directive. No code changes; doc/skill/audit-log consistency only. `cargo fmt --all --check` clean (no Rust delta); final sweep shows zero residual `.Codex/skills/` or `docs/analysis/reviews/<type>/` references in `.agents/` or `docs/`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/analysis/tasks/phase-b/T-018-address-space-kernel-object.md`:
- Line 28: The markdown link target on the checked item currently points to
".claude/skills/write-adr/SKILL.md"; update that link target to use
".agents/skills/write-adr/SKILL.md" so it references the repository's actual
skills directory; locate the checkbox line in T-018-address-space-kernel-object
(the line containing "ADR-0028 Accepted" and the backticked `write-adr` skill)
and replace the ".claude/skills/..." segment with ".agents/skills/..." to
correct the path.
- Around line 110-111: The checklist expectation mismatches the recorded test
count: replace the `~212` expectation with the actual recorded `221` (and update
the parenthetical to reflect +21 new AddressSpace tests instead of +~12) so the
two entries (`~212` vs `221/221`) are consistent; edit the checklist line that
reads "`cargo host-test` passes — expected ~212 (current 200 + ~12 new
AddressSpace tests from the list above)`" to use `221` and `+21` to match the
recorded `221/221`.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d185e1a1-a03c-47aa-9e38-aa3fe8426a5c
📒 Files selected for processing (6)
.agents/skills/add-bsp/SKILL.md.agents/skills/conduct-approval-review/SKILL.md.agents/skills/conduct-review/SKILL.md.agents/skills/propose-standard-change/SKILL.mddocs/analysis/tasks/phase-b/T-018-address-space-kernel-object.mddocs/audits/unsafe-log.md
✅ Files skipped from review due to trivial changes (3)
- .agents/skills/conduct-review/SKILL.md
- docs/audits/unsafe-log.md
- .agents/skills/add-bsp/SKILL.md
…(2 valid) CodeRabbit posted two inline findings on the T-018 task md after the previous round merged. Both verified against current state: 1. **L28 stale `.claude/skills/` link** — the `write-adr` skill reference pointed at the old `.claude/skills/` layout; the current layout is `.agents/skills/` (same drift as PR #28 round-4 F4, but in a different file CodeRabbit hadn't scanned then). Updated link target. 2. **L110 test-count expectation drift** — the AC line forecast `~212 (current 200 + ~12 new AddressSpace tests)` from the planning phase, but the [Review history](#review-history) row at landing recorded `221/221` with a `+21` breakdown (17 AddressSpace + 3 scheduler activation-hook + 1 `Task::address_space_handle` round-trip). Rewrote the AC line to match the landed count + breakdown so the two parts of the task md no longer contradict. Residual drift not in CodeRabbit's scope (9 other `.claude/skills/` references across TEMPLATE.md, analysis README files, and prior task mds T-001/T-006/T-015/T-016/T-017) is left untouched per the minimal-change directive; it can be swept in a separate docs-only commit if desired. No code changes. `cargo fmt --all --check` clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The prior commit `0d4e62c` added skill files under `.agents/skills/`
but left `.claude/skills/` in place as a parallel duplicate (identical
content, 16 entries). CLAUDE.md, AGENTS.md, the migration's own README,
memory notes, and ~14 doc cross-references still pointed at the old
location. This commit completes the migration:
**Updates (live references):**
- `CLAUDE.md` — table row + Skills section prose + index link
now point to `.agents/skills/`. The "Anthropic skill convention"
sentence reworded: skills follow the Anthropic skill-file *shape*
(per-skill directory + `SKILL.md` with YAML frontmatter) under
`.agents/` to keep the library agent-neutral; Claude Code and
other agent runners can both read them.
- `AGENTS.md` — top-level pointer updated.
- `.agents/skills/README.md` — new, adapted from the deleted
`.claude/skills/README.md` with all paths and the
"agent-discovery" note updated.
- Live doc cross-references (12 files): `docs/analysis/README.md`,
`docs/analysis/reviews/README.md`,
`docs/analysis/reviews/code-reviews/README.md`,
`docs/analysis/reviews/code-reviews/master-plan.md`,
`docs/analysis/reviews/code-reviews/2026-05-06-full-tree-comprehensive-review-plan.md`,
`docs/analysis/tasks/{README,TEMPLATE}.md`, and per-task files
T-001, T-006, T-009, T-015, T-016, T-017.
**Deletes:** `git rm -r .claude/skills/` removes all 16 entries + the
old README — the duplicate is gone.
**Intentionally untouched** (point-in-time review snapshots — historical
records describing repo state at the time of the review; rewriting
them retroactively would falsify the artefacts):
- `docs/analysis/reviews/code-reviews/2026-05-06-full-tree-comprehensive.md`
- `docs/analysis/reviews/code-reviews/2026-05-06-full-tree/track-{h,i}-*.md`
- `docs/analysis/reviews/code-reviews/2026-05-07-pr-12-to-17-multi-axis-review/track-{b,e,f,g}-*.md`
Their `.claude/skills/...` links will not resolve after this commit;
that is the correct trade-off — the snapshots' truthfulness is the
load-bearing property, not their link integrity.
**Why `.agents/` over `.claude/`:** the user chose the agent-neutral
prefix so the skill library serves both Claude Code (via configured
skill paths) and other agent runners uniformly. Trade-off accepted:
Claude Code's default `.claude/skills/` auto-discovery path is no
longer used; the project's tooling resolves skills through `.agents/`
explicitly. No behavioural break observed in this session — the same
slugs (`write-adr`, `conduct-review`, etc.) resolve correctly.
No code changes. `cargo fmt --all --check` clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
8 tasks
cemililik
added a commit
that referenced
this pull request
May 14, 2026
Bot review-round on PR #29 (CodeRabbit + gemini-code-assist; sourcery rate-limited). All six findings are valid drift in the closure-trio docs + `current.md` and were verified against empirical per-module test counts via `cargo test -p tyrne-kernel --lib -- <module>:: --list` at HEAD `3ec94b0` and at B2 closure `b0035ce`. **Empirical truth — per-module test deltas (B2 closure → HEAD):** | Module | B2 | HEAD | Δ | |---|---|---|---| | `mm::pmm::tests` | 0 | 15 | +15 | | `mm::address_space::tests` | 0 | 20 | +20 | | `sched::tests` | 22 | 27 | +5 | | `obj::task::tests` | 3 | 4 | +1 | | **Total kernel-host** | **25** | **66** | **+41** | Categorised by origin: 15 PMM (T-017) + 22 T-018-commits-1-3 (18 AS + 3 sched + 1 task) + 4 review-round (2 AS + 2 sched) = 41. **Six findings applied:** 1. **CodeRabbit @ current.md:7 — "three review rounds" → "five".** The lead phrase contradicted the enumeration of round-1..round-3 + round-4-follow-on + round-5-follow-on in the same sentence. 2. **CodeRabbit @ current.md:55 + gemini @ current.md:55 — bootstrap AS root `0x40092000` → `0x4008d000`.** Cross-doc drift: the closure-trio artefacts all cite `0x4008d000` (live smoke trace at HEAD `6334881`); current.md L50 + L55 had the older value from commit `daee78f` (pre-PR-#28-merge; the address shifted because review-round commits altered `.text`/`.bss` layout, moving `__boot_pt_l0`). Both L50 (Active task bullet, daee78f-inherited) and L55 (Last completed tasks bullet, my closure-trio commit) now match the live value. 3. **gemini @ business-reviews/2026-05-14-B3-closure.md:65 — `sched::tests` (5 new) text vs 6-item list mismatch.** The list incorrectly attributed the `Task::address_space_handle_round_trips` test to `sched::tests`; it actually lives in `obj::task::tests`. Rewrote the per-test breakdown paragraph to list each module empirically (PMM +15, AS +20, sched +5, task +1) with sum = 41 and origin-category cross-check; added the missing `obj::task::tests (+1, 3 → 4)` bullet. 4. **gemini @ perf-reviews/2026-05-14-B3-closure.md:73 — per-crate categorisation inconsistent with business retro.** Perf used "+14 PMM + 22 AS + 5 review-round = 41"; business retro used different sub-totals; both differed from empirical reality (+15 PMM, +20 AS-module, +4 review-round). Rewrote perf's Test count section to match the business retro's per-module empirical layout + added an origin-category sum + cross-link to the business retro per-test breakdown. 5. **gemini @ current.md:7 — "5 from the review-round fix-ups" inconsistent with other reports.** The "5th" entry (cited as "the existing test-discriminant cleanup") was a *phantom*: the only existing-test modification in the review-round arc was the T-007 Deadlock test gaining an endpoint-state assertion (a *modified* test, not a *new* one). Rewrote L7's test breakdown as "18 AS + 3 sched + 1 task + 4 review-round = 26", which matches the PR #27 → PR #28 delta exactly. 6. **gemini @ current.md:55 — same bootstrap AS root drift as CodeRabbit #2 (consolidated above).** **Business retro audit-note paragraph updated** to record the correction trail: the earlier draft mis-counted PMM as +14 (actual +15) and the review-round arc as +5 or +6 (actual +4); the closure-trio re-read pass on 2026-05-14 corrected both. No drift between the headline (41) and the breakdown after the correction. Security review unchanged — it doesn't cite per-module test counts, only the workspace-wide 226 total which is correct. No code changes; docs-only sweep. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
5 tasks
8 tasks
This was referenced May 31, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes B3 §§2-7 — the address-space abstraction implementation arc opened by PR #27 (ADR-0028 Accepted; T-018 Draft). 6 bisectable commits land the kernel-side
AddressSpace<M>kernel object + capability-gatedMmu::map/unmapwrappers + activation-on-context-switch hook + BSP wiring. Workspace tests grow 200 → 221 (+21); smoke trace gains exactly one new banner line.Six commits (bisectable)
`12caaae` — `feat(kernel): T-018 commit 1` — `kernel/src/mm/address_space.rs` data-structure landing: `AddressSpace` + `AddressSpaceHandle` + `AddressSpaceArena` + `AddressSpaceError` (commit-1 variants: `ArenaFull`, `StaleHandle`) + 4 free functions + 6 host tests. Reuses `crate::obj::arena::Arena<T, N>` per ADR-0016. No `unsafe`. No HAL surface change.
`fd17bd7` — `feat(kernel): T-018 commit 2` — `kernel/src/cap/mod.rs` cap-variant landing: `CapKind::AddressSpace` + `CapObject::AddressSpace(AddressSpaceHandle)` + `CapError::WrongKind` (new variant on `#[non_exhaustive]` enum) + `CapObject::kind()` arm + doc updates. Pure additive.
`dce49b3` — `feat(kernel): T-018 commit 3` — cap-gated wrapper surface: `cap_create_address_space` (6-step with no-widening rights check + arena/cap-mint rollback discipline) + `cap_map` + `cap_unmap` + `resolve_address_space_cap` helper + `AddressSpace::from_mmu_address_space` constructor + extended `AddressSpaceError` (`OutOfFrames`, `CapError(CapError)`, `MmuMapError(MmuError)`, `MmuUnmapError(MmuError)`) + 11 host tests with FakeMmu + VecFrameProvider. One `unsafe` block calling `Mmu::create_address_space` — rides UNSAFE-2026-0023's existing scope.
`0d16ea4` — `feat(kernel,bsp): T-018 commit 4` — activation-on-context-switch hook: `yield_now` / `ipc_send_and_yield` / `ipc_recv_and_yield` / `start` gain an `impl FnOnce(AddressSpaceHandle)` closure parameter; `Scheduler` gains a `task_address_space_handles` parallel array; `Task::new(id, address_space_handle)` + `Task.address_space_handle()` accessor; `add_task` / `register_idle` gain the AS-handle parameter. `SlotId::first_slot()` const fn + `BOOTSTRAP_ADDRESS_SPACE_HANDLE` const. `address_space_activation_target` pure helper. 3 new scheduler tests pinning fire/short-circuit paths + helper branch table. BSP passes no-op `|_| {}` closures at this stage; commit 5 wires the real callback.
`1b0f1d9` — `feat(bsp-qemu-virt,kernel): T-018 commit 5` — BSP wiring: `MMU` / `AS_ARENA` / `BOOTSTRAP_AS_CAP` / `BOOTSTRAP_AS_TABLE` `StaticCell`s + `QemuVirtAddressSpace::from_existing_root` safe constructor (BSP-side companion to `Mmu::create_address_space` for the bootstrap-AS case, per ADR-0028 §Simulation row 0). New `activate_address_space` callback function passed by name to all 7 scheduler call sites. `kernel_entry` AS-arena init block between PMM banner and GIC init. New `tyrne_kernel::mm::activate_address_space_handle` public helper. QEMU smoke verified: full demo through `tyrne: all tasks complete`; new banner line lands in the expected position.
`daee78f` — `docs: T-018 Done` — closure docs: UNSAFE-2026-0014 umbrella scope-extension Amendment for the activation hook + BSP-side closure; `memory-management.md` §"Address-space objects" section; `boot.md` Stage-3 sequence-diagram extension; T-018 `Draft → Done`; current.md + phase-b.md status updates.
Smoke trace (debug build)
```
tyrne: hello from kernel_main
tyrne: mmu activated
tyrne: pmm initialized (32602 frames available; 166 reserved)
tyrne: address-space-arena ready (1 / 8 slots used; bootstrap AS root = 0x40092000)
tyrne: timer ready (62500000 Hz, resolution 16 ns)
tyrne: starting cooperative scheduler
tyrne: task B — waiting for IPC
tyrne: task A -- sending IPC
tyrne: task B — received IPC (label=0xaaaa); replying
tyrne: task A — received reply (label=0xbbbb); done
tyrne: all tasks complete
tyrne: boot-to-end elapsed = 25754000 ns
```
Exactly one new line vs the T-017 smoke trace (`tyrne: address-space-arena ready ...`) inserted in the expected position per ADR-0028 §Simulation row 0. `1 / 8` confirms slot 0 holds the bootstrap AS. The PMM-reserved-frame delta (166 vs T-017's 164; 32602 vs 32604 free) reflects this PR's `.bss` growth from the four new `StaticCell`s (~600 bytes for the 8-slot AS arena + the MMU + bootstrap cap + cap-table cells).
Audit-log adjudication
Zero new entries. The AS-object layer is safe Rust over the existing MMU + PMM + cap surfaces:
UNSAFE-2026-0025 / 0026 `Pending QEMU smoke verification` notes stay. The v1 cooperative IPC demo doesn't call `cap_map` / `cap_create_address_space` at runtime — all tasks share `BOOTSTRAP_ADDRESS_SPACE_HANDLE`, so `address_space_activation_target` short-circuits. Host tests + Miri pin the paths; the `Pending` notes lift via Amendment when the first B5+ task with a per-task `AddressSpace` arms a real call. This is honest record-keeping per the discipline of every prior `Pending` note in the audit log (UNSAFE-2026-0019/0020/0021's IRQ-dispatch path is the precedent).
Headline metrics
Closes B3 §§2-7
Per phase-b.md §B3 §§2-7 all flipped to ✅ in commit 6. B3 implementation-complete 2026-05-11; closure trio (business + security + performance) is the next review trigger.
Test plan
Refs: ADR-0028, T-018
Audit: UNSAFE-2026-0010, UNSAFE-2026-0014, UNSAFE-2026-0023
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Changes / Bug Fixes
Documentation
Tests