docs(closure): Phase B closure — B6 step 7 (guide, perf baseline, security review, retrospective)#43
Merged
Conversation
docs/guides/first-userspace.md: what the first real EL0 task demonstrates, how to run it, the annotated +0x400 round-trip trace, the build pipeline, the wire-up, and the three gates + high-half isolation that make the boundary safe. Refs: B6 closure Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…tive (legs 2+4) Perf baseline (2026-06-01-B6-closure): boot-to-end band + footprint + test-count vs B5, with a same-host control proving the first EL0 task adds no measurable boot-to-end cost (delta sub-floor). Per-op micro-measurements (EL0 round-trip / IPC / ctx-switch) deferred to T-029 (a focused feature-gated CNTVCT instrumentation task) per the no-rushing discipline. Phase B retrospective (2026-06-01-phase-b-closure): the B0-B6 arc, headline numbers, what-changed, what-we-learned (gate-by-gate EL0 discipline; adversarial+Miri; the same-host perf control is now load-bearing), prior-Adjustments closure, and carry-forwards to Phase C. Refs: B6 closure Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…admap (legs 3/4 + close) Closes the Phase B closure quartet (step 7 of milestone B6): - Consolidated Phase-B seam security review (B0–B6 integrated surface): 5 lenses / 29 agents with per-finding skeptic verification across the eight master-plan axes. Approve, 0 confirmed exploitable defects (17 findings: 12 refuted, 5 nits). The seam review's job: catch a defect the per-task reviews could miss at the boundaries between subsystems — none found. Phase-gates that the 'panic-free dispatcher' property is syscall-input-scoped (distinct from EL0 execution-time faults, the deferred K3-4 item) and records the preemption/SMP + loader-trust forward-flags. - dispatch::dispatch doc: scope 'panic-free' to syscall input explicitly; note that a non-SVC EL0 sync fault is a denial-of-self routed to the panic handler in v1 (K3-4 / Phase E), not handled here. Doc-only; kernel binary byte-identical. - Roadmap: phase-b.md step 7 marked DONE with links to the four closure artifacts (guide, perf baseline, both security reviews, retrospective) + the T-029 micro-bench deferral; §'Phase B closure' marked CLOSED and Phase C active; current.md Phase-B-closed capstone banner. Gates: fmt clean, kernel-clippy -D warnings clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Reviewer's GuideDocumentation-only Phase B closure PR that adds a first-userspace guide, perf baseline, consolidated security review, and Phase B retrospective, updates roadmap banners to mark Phase B as closed and Phase C active, and clarifies via a doc-comment that the syscall dispatcher’s panic-freedom is scoped to syscall inputs (not general EL0 faults). Sequence diagram for EL0 syscall vs non-SVC fault handling (panic scope clarification)sequenceDiagram
actor EL0Task
participant SyscallEntry
participant SyscallDispatcher
participant Scheduler
participant KernelPanic
alt SVC_syscall
EL0Task->>SyscallEntry: SVC #n
SyscallEntry->>SyscallDispatcher: dispatch(ctx, args)
SyscallDispatcher-->>SyscallEntry: SyscallEffect / SyscallError
SyscallEntry->>Scheduler: task_exit_current (on Terminate)
Scheduler-->>EL0Task: task terminated
else non_SVC_EL0_fault
EL0Task-)KernelPanic: [non-SVC sync fault routes to panic handler]
KernelPanic-->>EL0Task: task killed (denial-of-self)
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
✅ Files skipped from review due to trivial changes (5)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR adds Phase B6 closure artifacts (business, perf, security reviews), a first-userspace guide, a T-029 perf micro-bench task, roadmap updates marking Phase B closed and Phase C active, and clarifies the syscall dispatch panic-free documentation. ChangesPhase B6 Closure and Phase C Activation
Sequence Diagram(s)sequenceDiagram
participant Boot as Boot/BSP
participant Kernel as EL1 Kernel
participant Task as EL0 Task
participant Console as Console
Boot->>Kernel: load hello.bin & seed capabilities
Kernel->>Task: create task, set TTBR, dispatch
Task->>Kernel: SVC (console_write)
Kernel->>Console: write bytes
Console-->>Kernel: ack
Kernel-->>Task: return from SVC
Task->>Kernel: SVC (task_exit)
Kernel->>Kernel: cleanup / schedule next
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The expanded
dispatchdoc comment now mixes behavioural guarantees with a lot of phase/planning context; consider tightening it to the essential behavioural contract and linking to the security review for historical details instead of embedding that text here. - The Phase B closure summary is repeated at length in both
phase-b.mdandcurrent.md; you might want to centralize the detailed narrative in one place and keep the other as a shorter pointer to reduce future maintenance overhead. - The long, dense blockquote banner in
current.mdpacks many concepts into a single paragraph; consider splitting it into shorter paragraphs or a small bullet list to improve scan-ability for readers tracking the current phase status.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The expanded `dispatch` doc comment now mixes behavioural guarantees with a lot of phase/planning context; consider tightening it to the essential behavioural contract and linking to the security review for historical details instead of embedding that text here.
- The Phase B closure summary is repeated at length in both `phase-b.md` and `current.md`; you might want to centralize the detailed narrative in one place and keep the other as a shorter pointer to reduce future maintenance overhead.
- The long, dense blockquote banner in `current.md` packs many concepts into a single paragraph; consider splitting it into shorter paragraphs or a small bullet list to improve scan-ability for readers tracking the current phase status.
## Individual Comments
### Comment 1
<location path="docs/guides/first-userspace.md" line_range="80-81" />
<code_context>
+
+## What the program does
+
+[`userland/hello/src/main.rs`](../../userland/hello/src/main.rs) is a
+`#![no_std] #![no_main]` aarch64 program whose entire body is:
+
+```rust
</code_context>
<issue_to_address>
**nitpick (typo):** Consider capitalizing “AArch64” for consistency with typical architecture naming and the rest of the docs.
```suggestion
[`userland/hello/src/main.rs`](../../userland/hello/src/main.rs) is a
`#![no_std] #![no_main]` AArch64 program whose entire body is:
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![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 formally closes Phase B of the Tyrne microkernel development, introducing comprehensive retrospective documentation, performance baselines, security reviews, and guides for running the first EL0 userspace task. It also clarifies in the syscall dispatcher documentation that its panic-free guarantees are scoped to syscall inputs rather than execution-time faults. The review feedback identifies two broken relative links in the newly added documentation files (2026-06-01-phase-b-closure.md and T-029-perf-microbench.md) that need to be corrected.
| # Security review 2026-06-01 — Phase B closure (consolidated, B0–B6) | ||
|
|
||
| - **Subject:** the **integrated** Phase B kernel surface, now that a real EL0 userspace task is attacker-observable (B6). Each Phase B task was individually security-reviewed (T-022 high-half = Approve; T-025/T-026 adversarial passes; [T-028 EL0-boundary](2026-06-01-T-028-el0-userspace-wireup.md) = Approve, 0 confirmed exploitable). This consolidated pass is the **seam review**: does the *combination* of the Phase B subsystems — capability system, IPC, MMU/AddressSpace/PMM, the high-half regime, the syscall boundary, and the EL0 transition — introduce a defect a per-task review could miss at the boundaries between tasks? | ||
| - **Reviewer:** @cemililik (+ Claude Opus 4.8 (1M context) agent, adversarial across the eight axes of the [master plan](master-plan.md) via a 5-lens / 29-agent workflow with per-finding skeptic verification). Performed **2026-06-01** at the B6-closure HEAD. |
There was a problem hiding this comment.
The relative link to master-plan.md is broken because master-plan.md is located in the performance-optimization-reviews directory, not in security-reviews. It should be updated to ../performance-optimization-reviews/master-plan.md.
Suggested change
| - **Reviewer:** @cemililik (+ Claude Opus 4.8 (1M context) agent, adversarial across the eight axes of the [master plan](master-plan.md) via a 5-lens / 29-agent workflow with per-finding skeptic verification). Performed **2026-06-01** at the B6-closure HEAD. | |
| - **Reviewer:** @cemililik (+ Claude Opus 4.8 (1M context) agent, adversarial across the eight axes of the [master plan](../performance-optimization-reviews/master-plan.md) via a 5-lens / 29-agent workflow with per-finding skeptic verification). Performed **2026-06-01** at the B6-closure HEAD. |
| - **Status:** Draft (opened in the B6-closure commit; the [B6 closure perf review](../../reviews/performance-optimization-reviews/2026-06-01-B6-closure.md) defers the per-op micro-measurements here) | ||
| - **Created:** 2026-06-01 | ||
| - **Author:** @cemililik (+ Claude Opus 4.8 agent) | ||
| - **Dependencies:** the timer ([ADR-0010](../../../decisions/0010-timer-abstraction.md) — `Cpu::now_ns` / `CNTVCT_EL0`); the scheduler (`yield_now` / `context_switch`), IPC (`ipc_send`/`ipc_recv`), and the EL0 syscall path ([T-028](T-028-el0-userspace-wireup.md)). |
There was a problem hiding this comment.
The relative link to 0010-timer-abstraction.md is broken because the ADR file is named 0010-timer-trait.md (as referenced in current.md). It should be updated to 0010-timer-trait.md.
Suggested change
| - **Dependencies:** the timer ([ADR-0010](../../../decisions/0010-timer-abstraction.md) — `Cpu::now_ns` / `CNTVCT_EL0`); the scheduler (`yield_now` / `context_switch`), IPC (`ipc_send`/`ipc_recv`), and the EL0 syscall path ([T-028](T-028-el0-userspace-wireup.md)). | |
| - **Dependencies:** the timer ([ADR-0010](../../../decisions/0010-timer-trait.md) — `Cpu::now_ns` / `CNTVCT_EL0`); the scheduler (`yield_now` / `context_switch`), IPC (`ipc_send`/`ipc_recv`), and the EL0 syscall path ([T-028](T-028-el0-userspace-wireup.md)). |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/reviews/security-reviews/2026-06-01-phase-b-closure.md`:
- Line 13: The table row in the docs contains inline code with unescaped pipe
characters (e.g. USER|EXECUTE, USER|WRITE) which break markdown table parsing;
edit the table cell text to escape the pipes (for example USER\|EXECUTE and
USER\|WRITE) or rewrite the phrases to avoid using '|' so the Mmu::translate,
TTBR0, UXN/PXN, W^X and task_exit_current references stay intact and the row
renders as a single cell.
In `@docs/roadmap/current.md`:
- Around line 7-9: The roadmap has conflicting "current focus" banners — both
"PHASE B CLOSED: B6 Done" / "PHASE B" entries and "Phase C is now active."
remain; reconcile by keeping a single authoritative current-state banner (e.g.,
retain the "PHASE B CLOSED" paragraph that ends with "Phase C is now active.")
and remove or update downstream entries such as the "FIRST USERSPACE TASK RUNS"
banner and any B6/Phase B "Active phase/milestone/task" lines so they no longer
claim Phase B is current; search for the exact strings "PHASE B CLOSED", "Phase
C is now active.", and "FIRST USERSPACE TASK RUNS" to locate and edit the
conflicting sections.
In `@docs/roadmap/phases/phase-b.md`:
- Around line 299-300: Update the closure sentence for Phase B to explicitly
state that per-operation IPC/context-switch micro-measurements were
intentionally deferred to T-029 so that acceptance expectations align; amend the
paragraph that currently reads "Phase B delivered..." / "Phase C is now active"
to append a brief qualifier referencing T-029 (and optionally K3-4/T-029
carry-forward) clarifying that those fine-grained perf micro-benchmarks are
postponed to the T-029 perf micro-bench, ensuring the closure note reconciles
deferred perf acceptance text.
🪄 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: c0184445-bb2c-40c4-a470-86166a33c889
📒 Files selected for processing (8)
docs/analysis/reviews/business-reviews/2026-06-01-phase-b-closure.mddocs/analysis/reviews/performance-optimization-reviews/2026-06-01-B6-closure.mddocs/analysis/reviews/security-reviews/2026-06-01-phase-b-closure.mddocs/analysis/tasks/phase-b/T-029-perf-microbench.mddocs/guides/first-userspace.mddocs/roadmap/current.mddocs/roadmap/phases/phase-b.mdkernel/src/syscall/dispatch.rs
…rrent.md, tighten dispatch doc Standing methodology (verify each finding vs current code; fix valid, skip invalid with reason; minimal): VALID — fixed: - F1 GFM table pipes: escape USER\|EXECUTE / USER\|WRITE inside the 'Memory safety' table cell of the consolidated Phase-B security review (unescaped '|' inside inline code still breaks GFM table parsing). - F2 current.md banner conflict: the status block (Active phase/milestone/ task + last-completed/working-branch/in-review) was stale from T-024 and contradicted the new 'PHASE B CLOSED / Phase C active' banner. Reconciled: Active phase -> C; Active milestone/task -> none (B6 + T-022..T-028 Done); In review -> PR #43; Working branch -> b6-closure; Last milestone -> B6; Last task -> T-028. Historical detail preserved (demoted, not deleted); B6 future-tense ('must close gates before a real EL0 task runs') -> past. - F3 phase-b.md §closure: explicit qualifier that per-op EL0/IPC/ctx-switch micro-measurements are intentionally deferred to T-029 and are NOT a Phase B acceptance gate. - F4 dispatch doc: trimmed the phase/date planning prose, kept the syscall-input-scoped boundary + the denial-of-self clarification + the greppable K3-4 anchor (behavioural contract, not a planning log). Doc-only; kernel binary byte-identical. - aarch64 -> AArch64 in first-userspace.md L81 (repo prose convention; the doc's other prose uses already say AArch64; L103 stays the target triple). - F9 T-029 timer ADR link: 0010-timer-abstraction.md -> 0010-timer-trait.md (the only broken relative link across all 7 closure docs; verified by an exhaustive link sweep). SKIPPED — not valid / declined with reason: - master-plan.md link in the security review: NOT broken — the file exists in security-reviews/ where the link points (the reviewer confused it with the perf-dir master-plan). Verified on disk. - Overall: 'centralize the closure narrative' / 'split the dense banner' — declined: current.md's convention is self-contained dated superseding banners; the bold sub-labels (Guide/Performance/Security/Business) already give scan anchors. Centralizing would break the append-only banner model. Gates: fmt clean, kernel-clippy -D warnings clean, link sweep 0 broken. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
Thanks — addressed in Fixed
Skipped (with reason)
Gates after the round: |
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.
Phase B closure — milestone B6 step 7 (the closure quartet)
This PR files the Phase B closure quartet (step 7 of milestone B6), closing Phase B. All B6 functional steps (1–6, through the merged T-028 EL0 wire-up) were already on
main; this is the documentation/retrospective closure + one doc-only kernel clarification.What's in here
1. First-userspace guide —
docs/guides/first-userspace.md(c8792cf)What B6 demonstrates, how to run it, an annotated
+0x400EL0→EL1 trace, the build pipeline, the wire-up, and the three gates + high-half isolation table.2. Perf baseline —
2026-06-01-B6-closure.md(bcb1f13)Boot-to-end band p10/p50/p90 = 13.143 / 15.507 / 18.469 ms, with a same-host control (rebuilt the pre-EL0 commit
9551a33back-to-back) proving running the first EL0 task adds no measurable boot-to-end cost (the EL0 round-trip is sub-floor; the raw B6<B5 drop is session drift, the control confirms it). Footprint +2.7 KiBtext(incl. the embedded image) / +13.6 KiB.bss(RAM). Per-op EL0/IPC/ctx-switch micro-measurements deferred to T-029 (Draft, opened here) — they need feature-gatedCNTVCTinstrumentation deserving its own gate + audit pass, not a closure-tail addition.3. Security review (consolidated seam) —
2026-06-01-phase-b-closure.md(eac69d2)The integrated B0–B6 surface reviewed now that a real EL0 task is attacker-observable: 5 lenses / 29 agents with per-finding skeptic verification across the eight master-plan axes. Approve, 0 confirmed exploitable defects (17 findings: 12 refuted, 5 nits). Phase-gates that the "panic-free dispatcher" property is syscall-input-scoped (distinct from EL0 execution-time faults — K3-4) and records the preemption/SMP + loader-trust forward-flags. The matching
dispatch::dispatchdoc clarification is doc-only (kernel binary byte-identical).4. Phase B retrospective —
2026-06-01-phase-b-closure.md(bcb1f13)The B0–B6 arc, headline numbers (366 host tests, Miri 0 UB), what-changed/what-we-learned (gate-by-gate EL0 discipline; adversarial+Miri; the same-host perf control is now load-bearing), and the Phase C carry-forwards.
+ Roadmap:
phase-b.mdstep 7 DONE + §closure CLOSED / Phase C active;current.mdPhase-B-closed capstone banner.Carry-forwards to Phase C (all confirmed non-exploitable in v1)
SVCfault containment (K3-4, Phase E) — currently a denial-of-self panic.ipc_sendunreachable!, IRQ-under-task-TTBR0).Validation
cargo fmt --all --checkcleancargo kernel-clippy(-D warnings) clean🤖 Generated with Claude Code
Summary by Sourcery
Document Phase B closure with first-userspace guidance, performance baseline, security seam review, and retrospective, and clarify syscall panic-freedom scope while updating roadmaps to mark Phase B as complete and Phase C as active.
Enhancements:
Documentation:
Summary by CodeRabbit
Documentation
Chores