Skip to content

Development #11 — B1 closure trio + T-012 Done promotion + post-merge cleanup#11

Merged
cemililik merged 5 commits into
mainfrom
development
May 6, 2026
Merged

Development #11 — B1 closure trio + T-012 Done promotion + post-merge cleanup#11
cemililik merged 5 commits into
mainfrom
development

Conversation

@cemililik
Copy link
Copy Markdown
Collaborator

@cemililik cemililik commented Apr 29, 2026

Summary

  • B1 milestone implementation-complete. T-012 promoted In Review → Done 2026-04-28; T-013 already Done 2026-04-27 via PR Development #9 — T-011 + T-008 + T-013 (B0 hygiene close + B1 EL drop) #9. Milestone-level "Done" still gated on maintainer-side QEMU smoke + Miri pass — those are pre-closure work-items tracked in the audit-log Pending QEMU smoke verification status notes on UNSAFE-2026-0019 / 0020 / 0021.
  • B1 closure review trio landed. Business retrospective + consolidated security pass + performance baseline re-run, all dated 2026-04-28. Verdict trio: Approve (security, with two forward-flagged non-blocking items only relevant when a future caller exists), Baseline recorded (performance, no proposal made; three concerns flagged for future hypothesis-driven cycles), B2-prep pivot (business retro's Pathfinder unblocks ADR-0027 drafting in parallel with maintainer-side smoke run).
  • Post-merge cleanup landed. Independent approval-review on the trio returned Approve with 1 Medium + 7 Low + 2 Nit findings — all applied as prose / numerical-accuracy / framing fixes; substantive verdicts unchanged. Adjacent fix: docs/architecture/exceptions.md §"Idle's wfi activation" prose drift corrected (the section still claimed idle stays in spin_loop until T-012 lands; T-012 has shipped and idle_entry uses cpu.wait_for_interrupt()). Final cleanup: repo-wide umbrix-to-phase-a.mdtyrne-to-phase-a.md rename closes the pre-existing N2 finding from the project's 2026-04-22 Umbrix → Tyrne rename.

This is a documentation-only PR — no kernel code, BSP code, or HAL code changed since PR #10. All trio + cleanup work happens in docs/.

What landed (commit-by-commit)

Commit Theme Notes
ffefb76 docs(roadmap) T-012 Done sweep — Status flipped to Done; tasks/phase-b README + phase-b.md B1 sub-item 5 + acceptance criteria + new "Status: B1 implementation complete 2026-04-28" line; current.md active-phase / active-milestone / last-completed-tasks / active-decisions all pivoted; architecture/README.md + exceptions.md status banners flipped to Done; top-level README phase-status sentence freshened.
9678d6c docs(roadmap,analysis) B1 closure review trio + roadmap pivot to B2 prep. Three new artefacts (security-reviews/2026-04-28-B1-closure.md, performance-optimization-reviews/2026-04-28-B1-closure.md, business-reviews/2026-04-28-B1-closure.md); three review-type README index updates; current.md pivot from "B1 closure trio is the next work-item" to "B2 prep — ADR-0027 drafting unblocked".
a5d4f9b docs(analysis,architecture) Approval-review findings on the trio — 1 Medium (canonisation-threshold three-way reconciliation in business retro), 7 Low (cancel_deadline race forward-flag in security; kernel zero-unsafe grep description correction; GIC ITARGETSR offset bound 0xC00 → 0xBFC; irq_entry+panic_entry ~400 → ~244 bytes objdump-verified; T-013/T-012 .text contribution two-thirds → 58 %; trampoline mov sp, x0 typo → mov x0, sp with restore 16 → 15; cost-of-arc commit-count 2+1+1+1+1+2+1=9 categorisation), 2 Nit (zero/two diagram framing; stale-link drift framing). Plus an adjacent finding: docs/architecture/exceptions.md §"Idle's wfi activation" prose drift fix (T-012 has shipped; section was still framing WFI as a future change).
10e3351 docs(refs) Repo-wide stale-link rename. git mv × 2 (docs/analysis/reviews/{code,security}-reviews/2026-04-21-umbrix-to-phase-a.md…tyrne-to-phase-a.md) + 3 stale link-form references fixed (B0 closure security review × 2 + B1 closure security review × 1). The empirical reality differed from the original framing — ~20 references already used the new tyrne- name (broken until rename), only 3 still used the old umbrix- name (working until rename); the rename cleans both directions. Closes the B1 retro's Repo-wide umbrix → tyrne link sweep Adjustments item.

Reviews included in this PR

Three artefacts under docs/analysis/reviews/:

  • B1 closure consolidated security review — eight-axis adversarial pass. Verdict Approve (no Yüksek findings). Two forward-flagged non-blocking items (arm_deadline CVAL+CTL race + cancel_deadline CTL+GIC race), both only relevant when a future caller exists. Three threat-model shifts identified (IRQ delivery as structural property; expanded unsafe surface with proportionally expanded discipline; IRQ-handler aliasing as unified second-axis rule). Three positive review-surface observations (architecture-doc-as-security-multiplier confirmed second time; review-fix-round cadence catching 19+2 findings pre-merge; explicit Pending QEMU smoke verification audit-log status notes as forward-discipline pattern).
  • B1 closure performance baseline — re-baseline against A6 baseline. Verdict Baseline recorded (no proposal made; per master-plan §Pre-flight). New Metric 6: IRQ delivery cost (~51 instructions per timer-IRQ ack-and-ignore round trip — 30 trampoline + 1 bl + 20 handler body + ~25 cycles microcode; not exercised in v1 because the IPC demo arms no deadline). Section deltas vs A6: .text 13.6 → 21.4 KiB (+57 %; T-013 + T-012 contribute ~58 % of the delta), .bss 17.5 → 21.7 KiB (+24 %; ~94 % from T-007's 4 KiB idle stack — T-012's .bss cost is ~32 bytes). Three concerns flagged for future hypothesis-driven cycles: IPC round-trip wall-clock measurement (now infrastructure-unblocked via T-009's now_ns()), stack high-water-mark probes, TrapFrame slimming for ack-and-ignore handlers.
  • B1 closure retrospective — five-section retro. Cost-of-arc: B1 ~7 commits/day, between Phase A's 15/day and B0's 8/day. Two design patterns empirically validated: design-first arc (exceptions.md landed before code; only two diagram/prose mismatches caught in PR Development #10 — T-012 (B1 IRQ infrastructure) + B0 closure retros #10's two rounds — no structural design-vs-implementation disagreement) and explicit Pending QEMU smoke verification audit-log status notes (cross-arc reproducibility threshold for canonisation as unsafe-policy.md §3 sub-rule). ADR-0025 §Rule 2 calibration ongoing: planned-vs-unplanned Amendment distinction proposed for the 2026-05-04 ADR-0024 first-week measurement.

Documentation surface changes

What this PR does NOT do

  • No new ADRs. ADR-0021's 2026-04-28 Amendment was landed by PR Development #10 — T-012 (B1 IRQ infrastructure) + B0 closure retros #10; this PR doesn't touch ADR bodies. ADR-0027 (kernel virtual memory layout) is the natural next ADR but was deliberately not drafted here — its drafting is the B2-prep work scheduled to run in parallel with the maintainer-side QEMU smoke + Miri runs.
  • No new audit-log entries. UNSAFE-2026-0019 / 0020 / 0021 + UNSAFE-2026-0014 commit-28c5ce9 Amendment all landed in PR Development #10 — T-012 (B1 IRQ infrastructure) + B0 closure retros #10. The Pending QEMU smoke verification status notes on the three new entries are explicitly not lifted in this PR — they remain until the maintainer runs the smoke trace and appends an Amendment recording the verification result. The introducing-commit-boundary discipline (UNSAFE-2026-0017's "Discipline note for future readers" paragraph) means audit-log bodies are append-only after their introducing commit.
  • No new unsafe surface. The four commits in this PR are 100 % documentation; zero bsp-qemu-virt/, kernel/, hal/, test-hal/, Cargo.{toml,lock} changes.
  • No B2 implementation work. B2 (MMU activation, kernel-half mapping) opens with ADR-0027 drafting; this PR pivots current.md to flag the unblock and the design-first hard-requirement, but the ADR itself is the next session's call.

DoD status

Documentation-only PR; the kernel/build gates apply trivially because no code changed:

Gate Status
cargo fmt --all -- --check ✅ clean
cargo host-clippy ✅ clean (-D warnings)
cargo kernel-clippy ✅ clean
cargo kernel-build ✅ clean
cargo host-test ✅ 149 / 149 (25 hal + 90 kernel + 34 test-hal) — same as PR #10 post-5c9cf06
cargo +nightly miri test (host) ⏳ deferred to CI / R6 GitHub Actions skeleton — same disclaimer T-013 + T-012 used; pre-B1-closure work-item
QEMU smoke (default + -machine virtualization=on) ⏳ deferred to maintainer-side verification — pre-B1-closure work-item; lifts UNSAFE-2026-0019 / 0020 / 0021 status notes via append-only Amendment

Test plan

  • CI: fmt, host-clippy, kernel-clippy, kernel-build, host-test, Miri pass on the R6 workflow (no code changed; this should be a no-op delta from PR Development #10 — T-012 (B1 IRQ infrastructure) + B0 closure retros #10's CI run).
  • Cross-reference graph: post-merge, grep -rE "tyrne-to-phase-a\.md|umbrix-to-phase-a" --include="*.md" should show 32 link-form references all resolving to existing files; the only umbrix-to-phase-a mentions are the two prose descriptions in the B1 closure retro.
  • Independent approval-review of this PR's four commits, focusing on (a) the B1 closure trio's substantive findings, (b) the rename's repo-wide consistency, (c) the exceptions.md idle prose's alignment with shipped code.
  • Maintainer-side QEMU smoke (pre-B1-closure work-item, separate from this PR's merge): default config (EL1 entry) + -machine virtualization=on (EL2 entry); reproduce the A6 + T-009 instrumentation trace post-T-012; lifts the three audit-log Pending QEMU smoke verification status notes via append-only Amendment after the smoke completes.
  • Maintainer-side Miri (pre-B1-closure work-item): cargo +nightly miri test --workspace --exclude tyrne-bsp-qemu-virt.

What this PR enables

Once merged + the maintainer-side smoke + Miri runs land, B1 milestone closes formally and B2 (MMU activation) can begin. The B2 design-first hard-requirement adopted by the B1 retro means the next implementation arc opens with an ADR (ADR-0027) + an architecture doc (docs/architecture/memory-management.md), then code — same shape as T-012's design-first arc.

🤖 Generated with Claude Code

Summary by Sourcery

Update roadmap and architecture documentation to mark T-012 and B1 implementation as Done, record the B1 closure review trio, and clean up stale references and audit-log status notes.

New Features:

  • Add B1 closure business, security, and performance review artifacts, including a new IRQ delivery performance baseline metric.
  • Document B1 implementation completion in the roadmap and top-level README, and outline B2 (MMU) preparation and design-first expectations.

Enhancements:

  • Clarify the exceptions architecture doc to reflect shipped idle WFI behaviour, completed T-012 work, and remaining maintainer-side verification items.
  • Refine roadmap phase-B details, including task statuses, active decisions, next tasks/review triggers, and expanded unsafe-audit status context.
  • Index the new B1 closure reviews in the business, security, and performance review READMEs for easier navigation.

Documentation:

  • Rename Phase A review artefact filenames and fix all remaining Umbrix→Tyrne links to restore a consistent, non-broken cross-reference graph across the docs.

Summary by CodeRabbit

  • Documentation
    • Marked Phase B1 milestones (exception/IRQ/timer/idle work) as Done and updated task status entries.
    • Added B1 closure artifacts: business retrospective, consolidated security review, and performance baseline.
    • Updated architecture and roadmap indices to reflect B1 completion and advance preparation for Phase B2.
    • Noted pending maintainer verification items (QEMU smoke and Miri) as follow-up actions.

cemililik and others added 4 commits April 29, 2026 03:00
T-012 promoted `In Review → Done` via PR #10 merge to `main`. The
substantive implementation arc — three commits + one doc-sweep + two
review-fix sweeps — is now closed; the next work-item is the B1
closure review trio (business + consolidated security + performance-
baseline), not a new implementation task.

Files updated:

1. `docs/analysis/tasks/phase-b/T-012-exception-and-irq-infrastructure.md` —
   Status field flipped to `Done`. Two new review-history rows: one
   summarising the two PR #10 review-fix sweeps (round-1 `d820a88`
   11/12 findings + round-2 `5c9cf06` 8 findings + 2 nitpicks; what
   they covered code-side and doc-side), and the `In Review → Done`
   promotion row recording the PR #10 merge with merge-SHA back-fill
   pending (same precedent as `6d2f725` for T-012 step-3 SHA). The
   row makes the two maintainer-side DoD gates explicit: `cargo
   +nightly miri test` + QEMU smoke verification of the deliberate-
   deadline path stay pinned for the maintainer's CI / hardware run,
   and the audit-log entries UNSAFE-2026-0019 / 0020 / 0021 keep
   their `Pending QEMU smoke verification` status notes intact until
   the maintainer lifts them via append-only Amendment after running
   the smoke (this is *not* a merge-time mechanical update; it
   tracks an actual QEMU run).

2. `docs/analysis/tasks/phase-b/README.md` — T-012 row status `In
   Review (2026-04-28)` → `Done (2026-04-28)`.

3. `docs/roadmap/phases/phase-b.md` — B1 sub-item 5 (T-012) flipped
   from "In Review 2026-04-28" to "Done 2026-04-28 via PR #10 merge"
   with summary of the implementation + review arc; B1 acceptance-
   criteria T-012 line flipped from "delivered (pending verification)"
   to "delivered (Done 2026-04-28)" with the QEMU-smoke + Miri
   maintainer-side disclaimer preserved. New **Status** line added
   below the criteria: "B1 implementation complete 2026-04-28 with
   PR #10's merge to main"; B1 closure review trio explicitly named
   as pre-closure work (mirrors B0's "Status: B0 closed 2026-04-27"
   convention).

4. `docs/roadmap/current.md` — substantial sweep:
   - Active phase / milestone / task lines: B1 implementation
     complete; T-012 Done; no active task (next is B1 closure review
     trio, not a new implementation task).
   - "In review" + "In progress" both now `none`.
   - Last-completed-tasks line restructured around T-012 with
     headline numbers (149 host tests, 148 → 149 via the sub-tick
     rounding test; three new audit entries; two review-fix sweeps
     closing 19 findings + 2 nitpicks).
   - Active-decisions block updated: ADR-0010 IRQ-delivery half is
     live (no longer "deferred to T-012"); ADR-0021 references the
     2026-04-28 Amendment; ADR-0022 first rider's *Sub-rider* is now
     closed by T-012's WFI activation.
   - Next-task-to-open: explicit "none until B1 closure review
     lands"; B2 (MMU activation) is the next implementation arc
     after closure.
   - Next-review-trigger: B1 closure trio explicitly named with
     deep-links to the three review folders; QEMU smoke + Miri
     surfaced as pre-closure work-items.
   - `unsafe` audit-status note updated to 2026-04-28 / post-PR-#10:
     adds UNSAFE-2026-0019 / 0020 / 0021 with their pending-smoke
     status notes; UNSAFE-2026-0014 second Amendment (commit
     `28c5ce9`, IRQ-handler future-site) recorded.

5. `docs/architecture/README.md` — `exceptions.md` row status:
   "T-012 In Review 2026-04-28; pending maintainer-side QEMU smoke
   verification" → "T-012 Done 2026-04-28 via PR #10 merge; ...
   maintainer-side QEMU smoke verification of the deliberate-
   deadline path remains pre-B1-closure work".

6. `docs/architecture/exceptions.md` — top-of-doc Status banner +
   bottom-of-implementation-map paragraph both updated to "Done
   2026-04-28 via PR #10 merge"; mentions the two review-fix
   sweeps explicitly so a reader chasing commits has the full
   chain (`a043079` → `b4ed68c` → `28c5ce9` → `d820a88` →
   `5c9cf06`); QEMU smoke + Miri framed as pre-B1-closure work.

7. `README.md` (top-level) — phase-status sentence freshened:
   B0 closed 2026-04-27, B1 implementation complete 2026-04-28
   (kernel runs at EL1 unconditionally, 16-entry vector table,
   GIC v2 driver, generic-timer IRQ wiring, idle WFI), B2 MMU
   activation next.

Audit-log entries (UNSAFE-2026-0019 / 0020 / 0021) intentionally
not edited in this commit. Their `Pending QEMU smoke verification`
status notes are factual claims about an unobserved verification
step; they stay until the maintainer actually runs the smoke and
appends an Amendment recording the result. Lifting them at merge
time would be a body-edit on an append-only log.

Gates (no code changed in this commit; documentation only):
cargo fmt / kernel-build clean; host-test 149 / 149 (25 hal +
90 kernel + 34 test-hal) — same as `5c9cf06`.

Refs: T-012, ADR-0010 (deferred halves now live), ADR-0021
(2026-04-28 Amendment), ADR-0022 (first rider Sub-rider closed),
ADR-0024 (B1 EL drop), `docs/roadmap/current.md` next-review-trigger
(B1 closure trio)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…prep

Lands the three review artifacts that close out B1 implementation
(T-013 + T-012) at the milestone level — business retrospective,
consolidated security pass, performance baseline re-run — and pivots
the roadmap from "B1 closure review trio is the next work-item" to
"B2 prep — ADR-0027 drafting is unblocked" per the business retro's
Pathfinder output.

Three new artifacts:

1. `docs/analysis/reviews/security-reviews/2026-04-28-B1-closure.md`
   — eight-axis adversarial pass over the post-PR-#10 surface. Covers
   the new GIC v2 MMIO surface (UNSAFE-2026-0019), the EL1 vector
   table install + asm trampolines (UNSAFE-2026-0020), the EL1
   virtual generic-timer compare-register writes (UNSAFE-2026-0021),
   and the UNSAFE-2026-0014 commit-`28c5ce9` Amendment naming
   `irq_entry` as a future site of the same momentary-`&mut` pattern.
   Verdict: Approve. No Yüksek findings. One forward-flagged
   non-blocking item (`arm_deadline` CVAL+CTL race only relevant
   when a future caller exists). Three threat-model shifts
   identified (IRQ delivery as structural property; expanded unsafe
   surface with proportionally expanded discipline; IRQ-handler
   aliasing as a unified second-axis rule). Three positive review-
   surface observations (design-first as security multiplier, two-
   round review-fix cadence catching 19+2 findings pre-merge,
   audit-log Pending-QEMU-smoke status notes as explicit
   verification-pending markers).

2. `docs/analysis/reviews/performance-optimization-reviews/`
   `2026-04-28-B1-closure.md` — per-section size delta vs A6
   baseline; `.text` 13.6 → 21.4 KiB (+57%; T-012 ~5.3 KiB of the
   ~8 KiB delta — vector table 2 KiB, gic.rs ~1.6 KiB, exceptions
   ~0.4 KiB, Timer arm/cancel ~0.3 KiB, hal::timer ~0.5 KiB), `.bss`
   17.5 → 21.7 KiB (+24% almost entirely from T-007's 4 KiB idle
   stack; T-012's `.bss` cost is ~32 bytes for the GIC StaticCell).
   New Metric 6: IRQ delivery cost (~52 instructions per timer-IRQ
   round trip — 31 trampoline + 20 handler body + ~25 cycles
   microcode; not exercised in v1 because the IPC demo arms no
   deadline). Three concerns flagged for future hypothesis-driven
   cycles (IPC round-trip wall-clock measurement now infrastructure-
   unblocked via T-009's `now_ns()`; stack high-water-mark probes;
   `TrapFrame` slimming for ack-and-ignore handlers). Verdict:
   baseline recorded; no proposal made.

3. `docs/analysis/reviews/business-reviews/2026-04-28-B1-closure.md`
   — five-section retrospective (chronicler / plan-diff / learning
   / adjuster / pathfinder). Cost-of-arc: B1 ran ~7 commits/day
   between Phase A's 15/day and B0's 8/day; meta-process cost paid
   forward, review-fix-round cadence is now load-bearing (not
   free). Two new design patterns empirically validated: (a)
   design-first arc — `exceptions.md` landed before code in commit
   `c366200`, zero "doc disagrees with code" findings during
   PR #10's two rounds (with two minor diagram/prose mismatches
   round-1+2 caught); (b) explicit `Pending QEMU smoke verification`
   audit-log status notes as a forward-discipline pattern (three
   data points; recommend canonising as `unsafe-policy.md` §3 sub-
   rule once a third arc reproduces it). Pathfinder pivots to B2
   prep — ADR-0027 drafting unblocked, parallelizable with
   maintainer-side QEMU smoke + Miri.

Index updates:

4. `docs/analysis/reviews/business-reviews/README.md` — new row for
   the B1 closure retro.
5. `docs/analysis/reviews/security-reviews/README.md` — new row for
   the B1 closure security pass.
6. `docs/analysis/reviews/performance-optimization-reviews/`
   `README.md` — new row for the B1 closure baseline; the
   "blocked-on-timer-init" disclaimer prose updated to reflect that
   T-009 + T-012 have lit up the measurement primitive.

Roadmap pivot:

7. `docs/roadmap/current.md` —
   - Active-phase headline: B1 closure review trio "landed
     2026-04-28; milestone closure waits on maintainer-side QEMU
     smoke + Miri pass" (was "is the next work-item").
   - Active-task block: pivots from "none — next is the B1 closure
     review trio" to "none — next implementation work is B2 prep —
     drafting ADR-0027". By ADR-0025 §Rule 1 the implementation
     T-NNN gets opened in the same commit that lands ADR-0027's
     *Dependency chain* section; current.md does not pre-allocate
     the number.
   - Last-reviews list: the three B1 closure artifacts added at
     the top.
   - Next-task-to-open: B2 prep with the design-first pattern
     applied — `docs/architecture/memory-management.md` lands
     alongside (ideally before) the implementation, per the B1
     retro's Adjustments item that adopts design-first as a B2
     hard-requirement.
   - Next-review-trigger: B2 closure (whenever T-014 / whichever
     number lands), with the 2026-05-04 ADR-0024 rider-rate
     measurement and the maintainer-side QEMU smoke + Miri runs
     remaining as separate-but-tracked open work-items.

Methodology notes for the artifacts:

- The security review explicitly cross-references the [B0 closure
  security review's §2](docs/analysis/reviews/security-reviews/
  2026-04-27-B0-closure.md) which named this artifact as the venue
  for T-012's GIC MMIO surface coverage — closing that forward-
  reference cleanly.
- The performance baseline notes the A6 80-KiB-flat-binary number
  is not directly comparable to the new 24.1-KiB measurement
  because the linker LOAD segment shape is different; the section-
  level breakdown is the canonical comparison and is byte-accurate
  for both readings. Recommendation: drop the flat-binary line
  from future baseline reports unless an audit step recomputes
  the A6 number under the current linker script.
- The business retro's planned-vs-unplanned-Amendment distinction
  for ADR-0025 §Rule 2 is a recommended rule clarification, not a
  rule change; defer to the maintainer at the 2026-05-04 first-
  week measurement of ADR-0024.

Gates (no code changed in this commit; documentation only):
cargo fmt / kernel-build / kernel-clippy / host-clippy clean;
host-test 149 / 149 (25 hal + 90 kernel + 34 test-hal) — same
as `5c9cf06` post-PR-#10-round-2.

Refs: T-012 (Done 2026-04-28), T-013 (Done 2026-04-27), ADR-0010
(deferred halves now live), ADR-0021 (2026-04-28 Amendment),
ADR-0022 (first rider Sub-rider closed), ADR-0024 (B1 EL drop),
ADR-0025 (§Rule 1 + §Rule 2 calibration ongoing), UNSAFE-2026-0019
/ 0020 / 0021 (Pending QEMU smoke verification status notes
maintained).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…sure trio + exceptions.md idle prose drift

Two parallel inputs land here as one cohesive doc-only commit:

(A) Independent approval-review on commit 9678d6c (the B1 closure
    trio) returned Approve verdict with 1 Medium + 7 Low + 2 Nit
    findings. The Approve verdict means the originating commit's
    substantive conclusions stand; the findings are prose / numerical
    accuracy / framing improvements.

(B) A separate inline-comment finding flagged that
    `docs/architecture/exceptions.md` §"Idle's wfi activation" still
    contained the pre-T-012 framing ("Until T-012's full set lands,
    idle stays in the spin_loop form") even though `idle_entry`'s
    body uses `cpu.wait_for_interrupt()` since commit `b4ed68c`. The
    fix here aligns the prose with the shipped code.

(A) — review-trio fixes:

`docs/analysis/reviews/business-reviews/2026-04-28-B1-closure.md`:
- (M1) "Codify verification-pending status notes" Adjustments item
  reconciled with Appendix item #2 — the canonisation criterion is
  now consistently "the next IRQ-related arc reproduces the
  pattern", not "three same-arc entries". UNSAFE-2026-0019 / 0020 /
  0021 share one arc and one shape; they are *one* cross-arc data
  point. The §"Audit-log Pending QEMU smoke verification status
  notes" Learning section, the Adjustments item, and the Appendix
  item now all carry the same threshold language.
- (L6) Cost-of-arc commit-count breakdown corrected from
  "3 implementation + 1 design-first + 1 doc-sweep + 2 review-fix +
  1 Done = 8 (claimed 9)" to a 9-commit categorization that actually
  sums correctly: 2 implementation (a043079, b4ed68c) + 1 design-
  first doc (c366200) + 1 doc-sweep (28c5ce9 — T-012 step 6+7) +
  1 SHA-backfill (6d2f725) + 1 review-fix-prep (67ff867) + 2 PR-
  review-round (d820a88, 5c9cf06) + 1 Done-sweep (ffefb76) = 9.
  Commit `28c5ce9` is now classified as the doc-sweep role rather
  than double-counted as both step 6+7 and the documentation-sweep
  commit.
- (N1) "Result: zero 'doc and code disagree' findings" softened to
  "only two diagram/prose mismatches caught — no structural
  'doc disagrees with code' findings"; the original phrasing self-
  contradicted the immediately-following "Two diagram/prose
  corrections" paragraph.
- (N2) Stale-link drift section qualified: "stale-link drift
  suppressed" → "stale-link drift suppressed for new T-012 changes",
  with explicit acknowledgement that pre-existing repo drift
  (umbrix → tyrne rename leftovers) is a separate concern outside
  PR #10's scope. New Adjustments item ("Repo-wide
  umbrix-to-phase-a.md → tyrne-to-phase-a.md link sweep") opens the
  separate `docs(refs):` commit that handles the pre-existing
  drift.

`docs/analysis/reviews/security-reviews/2026-04-28-B1-closure.md`:
- (L1) New §3 forward-flagged item: cancel_deadline ordering race
  window between Step 1 (MSR CNTV_CTL_EL0=0b10 source-mask) and
  Step 2 (gic.disable(TIMER_IRQ)). Symmetric to the arm_deadline
  CVAL+CTL race that the original review flagged; the original
  review missed this symmetric case. v1 still benign (no caller).
- (L4) "Kernel crate is still zero-unsafe" claim corrected. The
  `grep -r "unsafe " kernel/src/` description was misleading — the
  raw-pointer scheduler bridge `unsafe fn` bodies contain 15 inner
  `unsafe { ... }` blocks (mandatory under
  `#![deny(unsafe_op_in_unsafe_fn)]`). The substantive sentence
  ("T-012 added zero kernel-crate unsafe blocks; the new surface
  is BSP-internal") is preserved; only the misleading claim about
  the absence of `unsafe { ... }` blocks is rewritten.
- (L8) GIC ITARGETSR loop-bound math precise now: last byte_idx is
  1020 (loop condition `byte_idx < irq_count` exits at 1024); max
  offset = 0x800 + 0x3FC = 0xBFC. The original "0xC00" was the
  one-past-end byte. Conclusion ("well within the 4 KiB window")
  unchanged.
- Verdict block: "One forward-flagged item" → "Two forward-flagged
  items" reflecting (L1)'s new entry; the cross-references in the
  README index and the business retro's Adjustments are kept
  consistent.

`docs/analysis/reviews/performance-optimization-reviews/`
`2026-04-28-B1-closure.md`:
- (L7) §Metric 1 decomposition table:
  `exceptions.rs::irq_entry + panic_entry ~400 bytes` →
  `~244 bytes (irq_entry 116 B + panic_entry 128 B per
  rust-objdump)`. Verified by disassembling the release ELF.
- (L5) "Roughly two-thirds of the A6→B1 .text delta is T-012 alone"
  → "Roughly 58 % of the A6→B1 .text delta is T-013 + T-012
  specifically" with explicit T-013 / T-012 / B0 split. The
  arithmetic now uses the corrected 244-byte exceptions.rs figure;
  total estimate sums to ~7,810 bytes (~2 % residual to LLVM
  optimization shifts; matches empirical +7,968 within rounding).
- (L2) §Metric 6 IRQ delivery cost trampoline line-items corrected.
  The `mov sp, x0` typo (which would have corrupted SP) → `mov x0,
  sp` (correct: writes the frame pointer to x0 for `bl irq_entry`).
  Save phase enumeration now verified line-by-line against
  vectors.s:115-130 (15 instructions: sub sp + 10 stp + mrs elr +
  mrs spsr + 1 stp + mov x0,sp). Restore phase corrected from "16"
  to 15 (the original "11 ldp" double-counted the elr/spsr ldp
  pair). New approximate dispatch cost: 51 instructions + ~25
  cycles of microcode (down from the original 52). The §Hotspot
  observation (~60 % trampoline) becomes ~59 % (30/51).

`docs/analysis/reviews/security-reviews/README.md`:
- Index row's verdict description updated to match the artifact's
  two forward-flagged items.

Substantive verdicts unchanged: security Approve, performance
Baseline-recorded, business B2-prep pivot all stand. The findings
are consistency / accuracy / framing improvements, not corrections
to the underlying conclusions.

(B) — exceptions.md idle prose drift:

`docs/architecture/exceptions.md` §"Idle's wfi activation"
restructured. The original "from spin_loop → to WFI" historical
comparison framed the WFI form as a future change ("Until T-012's
full set lands, idle stays in the spin_loop form"), which was
accurate when the doc was drafted design-first in commit `c366200`
but is now stale — `b4ed68c` flipped idle_entry to WFI.

The new prose:
- Leads with "Idle's body uses WFI as the default wake strategy —
  the form ADR-0022's Decision outcome originally specified, now
  shipped" + the WFI-form code block.
- Records the historical flip as past tense: "T-012's commit
  `b4ed68c` flipped `idle_entry`'s body from the interim
  core::hint::spin_loop() shape ... to the wait_for_interrupt()
  shape above. This closes ADR-0022 first rider's Sub-rider gate."
- Names the wake source explicitly (timer IRQ; arm_deadline
  callers from B5+ time_sleep_until or B-late preemption tick).
- Notes that v1's IPC demo path makes WFI structurally unreachable
  (both application tasks tail-`spin_loop`-yield permanently); the
  shipped change is observable as a code-shape change, not a
  behavioural change in the v1 trace.
- Flags remaining T-012 items (maintainer-side QEMU smoke + Miri)
  as separate from this section's scope, with cross-link to the
  B1 closure retro's Adjustments and to the audit-log
  Pending-QEMU-smoke status notes.

The spin_loop code block is removed because preserving an obsolete
pre-shipping form serves no current reader; the historical
transition is captured in the prose plus the cross-reference to
commit `b4ed68c`.

Audit-log entries (UNSAFE-2026-0019 / 0020 / 0021) and
ADR-0021 §Revision notes Amendment intentionally not edited — the
findings here are prose/numerical fixes in review and architecture
artifacts, not in append-only audit or ADR bodies. Introducing-
commit-boundary discipline preserved.

Pre-existing N2 (`umbrix-to-phase-a.md` → `tyrne-to-phase-a.md`
repo-wide rename) deliberately out of scope for this commit; will
land in a separate `docs(refs):` commit so the scope is clear.

Gates (no code changed in this commit; documentation only):
cargo fmt / kernel-build / host-clippy / kernel-clippy clean;
host-test 149 / 149 (25 hal + 90 kernel + 34 test-hal) — same
as `9678d6c` post-trio.

Refs: T-012 (Done 2026-04-28; commit b4ed68c flipped idle to
WFI), ADR-0021 (2026-04-28 Amendment unchanged), ADR-0022 (first
rider Sub-rider closure paragraph unchanged), ADR-0025 (§Rule 2
planned-vs-unplanned-Amendment distinction observation
unchanged), UNSAFE-2026-0019 / 0020 / 0021 (Pending QEMU smoke
verification status notes maintained, untouched).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Closes the pre-existing N2 finding the B1-closure approval-review
surfaced: the 2026-04-22 Umbrix → Tyrne project rename had moved
most filename references to the new shape but left two artefact
files at the old name. ~20 references across the repo already used
the new `tyrne-to-phase-a.md` name (and were therefore broken until
this commit lands the rename); only three cross-references still
pointed at the old `umbrix-to-phase-a.md` name (and were therefore
working until this commit). The fix renames the files and edits the
three stale link forms so both directions are clean.

Renames (`git mv`):

1. `docs/analysis/reviews/code-reviews/2026-04-21-umbrix-to-phase-a.md`
   → `docs/analysis/reviews/code-reviews/2026-04-21-tyrne-to-phase-a.md`
2. `docs/analysis/reviews/security-reviews/2026-04-21-umbrix-to-phase-a.md`
   → `docs/analysis/reviews/security-reviews/2026-04-21-tyrne-to-phase-a.md`

Reference fixes (link form, was umbrix-, now tyrne-):

3. `docs/analysis/reviews/security-reviews/2026-04-27-B0-closure.md`
   line 3 — `last reviewed by [\`2026-04-21-tyrne-to-phase-a.md\`]
   (2026-04-21-tyrne-to-phase-a.md)`. Inside the `Change:` field;
   was the change-range marker for the B0 closure review's
   pre-period.
4. `docs/analysis/reviews/security-reviews/2026-04-27-B0-closure.md`
   line 115 — `[Phase A → exit security review (2026-04-21)]
   (2026-04-21-tyrne-to-phase-a.md)`. Inside the §References block.
5. `docs/analysis/reviews/security-reviews/2026-04-28-B1-closure.md`
   line 141 — same `[Phase A → exit security review (2026-04-21)]`
   reference, B1 closure version.

Why edit committed review artifacts: this is a stale-link fix, not
a substantive change to either review's findings or verdict; it
matches the precedent set by `8800b0b` (`fix(analysis): apply
approval-review findings on B0 closure reviews`), which edited
committed review artifacts post-merge for similar accuracy fixes.
The unsafe-policy.md §3 introducing-commit-boundary discipline
applies to audit-log entries (and was preserved here — `unsafe-log.md`
already used the new `tyrne-` name and was untouched), not to
review artifacts.

Reference forms NOT edited (deliberate — these are descriptive prose
about the rename concern itself, not broken links):

- `docs/analysis/reviews/business-reviews/2026-04-28-B1-closure.md`
  line 139 (§What we learned, file-rename / stale-link drift
  paragraph) — describes the pre-existing drift's empirical reality.
- `docs/analysis/reviews/business-reviews/2026-04-28-B1-closure.md`
  line 152 (Adjustments item, now ✅ Done) — describes the rename
  action this commit performed.

The B1 closure business retro has also been updated as part of this
commit:

- The §What we learned `Stale-link drift` paragraph rewritten to
  reflect the empirical reality (two files at old name; ~20
  references at new name; three at old name) rather than the
  original framing's broader-scope assumption ("references across
  current.md, README.md, multiple task files, audit log, technical-
  analysis docs"). Cross-link to the rename commit (this commit)
  added.
- The Adjustments item flipped from `[ ]` to `[x]` ✅ Done
  2026-04-28 with the closure paragraph documenting the empirical-
  reality difference, the fix, and the result (zero broken
  cross-references repo-wide; B2 opens on a clean cross-reference
  graph). Marking the item done in the same commit that closes it
  keeps the retro consistent with the actual state.

The retro change is part of this commit (not the previous one)
because the Adjustments-item closure is *caused by* the rename;
splitting them would leave the previous commit's retro saying "[ ]
not done" while this commit completes the action.

Verification (after rename):

- `grep -roE "tyrne-to-phase-a\.md" --include="*.md"` returns 32
  link-form references; all resolve to the now-existing files.
- `grep -rE "umbrix-to-phase-a" --include="*.md"` returns 2 prose
  mentions (the descriptive paragraphs in the B1 closure retro);
  zero broken links.
- Both renamed files exist at their new paths and are byte-
  identical to their pre-rename contents (`git mv` is a pure
  rename without content modification).

Gates (no code changed; pure documentation rename + 4 prose
edits): cargo fmt / kernel-build / host-clippy / kernel-clippy
clean; host-test 149 / 149 unchanged.

Refs: B1 closure approval-review pre-existing N2 finding;
rename action originally scheduled in the [B1 closure
retrospective's Adjustments](docs/analysis/reviews/business-reviews/2026-04-28-B1-closure.md)
post-`a5d4f9b`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@qodo-code-review
Copy link
Copy Markdown

ⓘ 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
Copy link
Copy Markdown

sourcery-ai Bot commented Apr 29, 2026

Reviewer's Guide

Documentation-only PR that (1) promotes T-012 and the B1 milestone to implementation-complete across roadmap, task, and architecture docs; (2) adds the B1 closure review trio (security, performance baseline, business retrospective) and wires them into indexes; and (3) performs targeted cleanup including correcting exceptions/idle WFI prose and fixing the Umbrix→Tyrne review filename/link drift.

Sequence diagram for T_012 lifecycle and B1 closure documentation

sequenceDiagram
  actor Maintainer
  actor DevAgent
  participant Repo
  participant UnsafeLog
  participant RoadmapDocs
  participant ReviewDocs

  DevAgent->>Repo: create Task_T_012 (Draft)
  DevAgent->>Repo: add ArchDoc_Exceptions_md (design_first)
  DevAgent->>Repo: implement T_012 commits
  DevAgent->>Repo: add Unsafe_2026_0019_0021 entries
  DevAgent->>UnsafeLog: record new unsafe entries

  DevAgent->>Repo: open PR_10 (code + docs)
  Maintainer->>Repo: perform review_round_1
  Maintainer-->>DevAgent: review findings (safety, docs, tests)
  DevAgent->>Repo: commit review_round_1_fixes

  Maintainer->>Repo: perform review_round_2
  Maintainer-->>DevAgent: additional findings
  DevAgent->>Repo: commit review_round_2_fixes

  Maintainer->>Repo: merge PR_10 to main
  DevAgent->>RoadmapDocs: flip T_012 status In_Review -> Done
  DevAgent->>RoadmapDocs: mark B1 implementation complete
  DevAgent->>ReviewDocs: add B1_business_retro
  DevAgent->>ReviewDocs: add B1_security_review
  DevAgent->>ReviewDocs: add B1_performance_baseline

  DevAgent->>UnsafeLog: add Pending_QEMU_smoke_verification
  DevAgent->>RoadmapDocs: note maintainer_side QEMU + Miri pending

  Maintainer->>Repo: later run QEMU_smoke + Miri (outside this PR)
  Maintainer->>UnsafeLog: append Amendments lifting pending_status
  Maintainer->>RoadmapDocs: mark B1 milestone formally closed
Loading

Class diagram for B1/T_012 documentation relationships

classDiagram

class Milestone_B1 {
  status: implementation_complete
  opened: 2026_04_21
  implementation_complete: 2026_04_28
}

class Task_T_012 {
  phase: B
  milestone: B1
  status: Done
  created: 2026_04_27
}

class Task_T_013 {
  phase: B
  milestone: B1
  status: Done
  promoted: 2026_04_27
}

class ADR_0010_TimerTrait {
  status: Accepted
}

class ADR_0021_IPC_Bridge {
  status: Accepted
  amendment_2026_04_28: irq_handler_aliasing_discipline
}

class ADR_0022_Idle_Deadlock {
  status: Accepted
  first_rider_sub_rider: closed
}

class ADR_0024_EL_Drop {
  status: Accepted
}

class Review_B1_Security_2026_04_28 {
  type: security
  verdict: Approve
}

class Review_B1_Perf_2026_04_28 {
  type: performance_baseline
  verdict: Baseline_recorded
}

class Review_B1_Business_2026_04_28 {
  type: business_retro
}

class Unsafe_2026_0019 {
  scope: GIC_v2_MMIO
  status: Active
  pending_qemu_smoke: true
}

class Unsafe_2026_0020 {
  scope: EL1_vector_table_trampolines
  status: Active
  pending_qemu_smoke: true
}

class Unsafe_2026_0021 {
  scope: Generic_timer_CVAL_CTL
  status: Active
  pending_qemu_smoke: true
}

class Unsafe_2026_0014 {
  scope: Scheduler_momentary_mut_pattern
  amendment_28c5ce9: irq_entry_future_site
}

class ArchDoc_Exceptions_md {
  status: Accepted
  describes: exception_and_irq_infrastructure
}

class Roadmap_Current_md {
  active_phase: B
  active_milestone: B1
}

class Roadmap_PhaseB_md {
  B1_status: implementation_complete
}

class Reviews_Index_Security {
}

class Reviews_Index_Perf {
}

class Reviews_Index_Business {
}

Milestone_B1 "1" -- "2" Task_T_012 : includes
Milestone_B1 "1" -- "1" Task_T_013 : includes

Task_T_012 --> ADR_0010_TimerTrait : closes_deferred_halves
Task_T_012 --> ADR_0021_IPC_Bridge : adds_amendment
Task_T_012 --> ADR_0022_Idle_Deadlock : closes_first_rider
Task_T_013 --> ADR_0024_EL_Drop : implements

Task_T_012 --> Unsafe_2026_0019 : introduces
Task_T_012 --> Unsafe_2026_0020 : introduces
Task_T_012 --> Unsafe_2026_0021 : introduces
Task_T_012 --> Unsafe_2026_0014 : adds_amendment

Task_T_012 --> ArchDoc_Exceptions_md : implements_design
ArchDoc_Exceptions_md --> ADR_0010_TimerTrait : references
ArchDoc_Exceptions_md --> ADR_0021_IPC_Bridge : references
ArchDoc_Exceptions_md --> ADR_0022_Idle_Deadlock : references
ArchDoc_Exceptions_md --> ADR_0024_EL_Drop : references

Milestone_B1 --> Review_B1_Security_2026_04_28 : closure_review
Milestone_B1 --> Review_B1_Perf_2026_04_28 : closure_review
Milestone_B1 --> Review_B1_Business_2026_04_28 : closure_review

Review_B1_Security_2026_04_28 --> Unsafe_2026_0019 : audits
Review_B1_Security_2026_04_28 --> Unsafe_2026_0020 : audits
Review_B1_Security_2026_04_28 --> Unsafe_2026_0021 : audits
Review_B1_Security_2026_04_28 --> Unsafe_2026_0014 : audits

Review_B1_Perf_2026_04_28 --> Task_T_012 : measures_cost
Review_B1_Perf_2026_04_28 --> Task_T_013 : measures_cost
Review_B1_Business_2026_04_28 --> Milestone_B1 : analyzes_arc

Roadmap_Current_md --> Milestone_B1 : summarizes_status
Roadmap_PhaseB_md --> Milestone_B1 : defines_acceptance

Reviews_Index_Security --> Review_B1_Security_2026_04_28 : lists
Reviews_Index_Perf --> Review_B1_Perf_2026_04_28 : lists
Reviews_Index_Business --> Review_B1_Business_2026_04_28 : lists
Loading

Flow diagram for B1 implementation completion and closure gating

flowchart TD

  A_B1_Started["Phase B Milestone B1 opened<br/>T-013 EL_drop, T-012 exception_infrastructure planned"]
  B_T013_Done["T-013 Done 2026-04-27<br/>EL drop to EL1 implemented"]
  C_T012_DesignFirst["T-012 design-first<br/>exceptions_md drafted"]
  D_T012_Impl_Commits["T-012 implementation commits<br/>GIC + vectors + timer + idle WFI"]
  E_T012_Review_Fixes["PR #10 review rounds<br/>design + safety fixes<br/>host tests 149/149"]
  F_T012_Done["T-012 status: Done 2026-04-28<br/>roadmap and task docs updated"]
  G_B1_Closure_Trio["B1 closure review trio added<br/>security + performance + business"]
  H_B1_Impl_Complete["Milestone B1 implementation complete<br/>(but not fully closed)"]
  I_Pending_QEMU_Miri["Maintainer-side work pending<br/>QEMU smoke for IRQ path<br/>Miri run on workspace"]
  J_Unsafe_Status_Notes["UNSAFE-2026-0019/0020/0021<br/>carry Pending_QEMU_smoke_verification<br/>status notes"]
  K_B1_Formal_Closure["B1 milestone formally closed<br/>(after maintainer verifies and amends log)"]
  L_B2_Prep_ADR_0027["B2 prep: ADR-0027 drafting<br/>(memory layout) can start in parallel"]

  A_B1_Started --> B_T013_Done
  B_T013_Done --> C_T012_DesignFirst
  C_T012_DesignFirst --> D_T012_Impl_Commits
  D_T012_Impl_Commits --> E_T012_Review_Fixes
  E_T012_Review_Fixes --> F_T012_Done
  F_T012_Done --> G_B1_Closure_Trio
  G_B1_Closure_Trio --> H_B1_Impl_Complete
  H_B1_Impl_Complete --> J_Unsafe_Status_Notes
  H_B1_Impl_Complete --> I_Pending_QEMU_Miri
  I_Pending_QEMU_Miri --> K_B1_Formal_Closure
  J_Unsafe_Status_Notes --> K_B1_Formal_Closure
  H_B1_Impl_Complete --> L_B2_Prep_ADR_0027
Loading

File-Level Changes

Change Details Files
Promote T-012 from In Review to Done and mark B1 implementation as complete across roadmap and task docs, including updated status narratives and next-work pointers.
  • Update docs/roadmap/current.md to reflect B1 implementation complete, no active task, B2-prep as next work, refreshed active decisions, last completed tasks, next review trigger, and unsafe-audit status
  • Flip T-012 status and dates from In Review to Done in the phase-b milestone breakdown and acceptance criteria, adding an explicit "B1 implementation complete" status line
  • Mark T-012 as Done in the phase-b tasks index and extend its task file review-history with review-fix sweeps and Done promotion rows
docs/roadmap/current.md
docs/roadmap/phases/phase-b.md
docs/analysis/tasks/phase-b/README.md
docs/analysis/tasks/phase-b/T-012-exception-and-irq-infrastructure.md
Update architecture documentation for the exceptions/IRQ path to match shipped behaviour and final T-012 status, especially idle's WFI activation and implementation mapping.
  • Flip exceptions.md status banner from T-012 In Review to T-012 Done via PR Development #10 — T-012 (B1 IRQ infrastructure) + B0 closure retros #10 with explicit mention of review-fix sweeps and pending maintainer-side smoke/Miri
  • Rewrite the Idle's wfi activation section to describe WFI as the shipped default, capture the historical spin_loop→WFI flip, and note remaining pre-closure work-items
  • Clarify that T-012 has landed as Done in the implementation-map section while still calling out pending smoke/Miri work
docs/architecture/exceptions.md
docs/architecture/README.md
Add the B1 closure review trio (security, performance, business) and index entries, capturing the milestone-level verdicts and forward-looking concerns.
  • Introduce a B1 closure performance baseline document with detailed metric breakdowns, concerns, and references
  • Introduce a B1 closure business retrospective covering cost-of-arc, ADR governance, audit-log patterns, and B2-prep recommendations
  • Introduce a B1 closure consolidated security review for T-012 and new unsafe entries, with verdict and forward flags
  • Wire the three new reviews into their respective README indexes and into roadmap/current.md last-reviews list
docs/analysis/reviews/performance-optimization-reviews/2026-04-28-B1-closure.md
docs/analysis/reviews/business-reviews/2026-04-28-B1-closure.md
docs/analysis/reviews/security-reviews/2026-04-28-B1-closure.md
docs/analysis/reviews/performance-optimization-reviews/README.md
docs/analysis/reviews/business-reviews/README.md
docs/analysis/reviews/security-reviews/README.md
docs/roadmap/current.md
Perform post-merge documentation cleanup around naming, links, and prose drift, including the Umbrix→Tyrne review rename and idle/WFI text corrections.
  • Rename the Phase-A-exit code/security review artifacts from umbrix-to-phase-a.md to tyrne-to-phase-a.md and update the few remaining stale links to match the majority usage
  • Adjust B0 closure security review text to reference the new tyrne-to-phase-a filenames
  • Fix idle WFI activation prose drift in exceptions.md so it no longer describes WFI as future work and clarifies its v1 observability
  • Refresh the top-level README status paragraph to describe Phase B B0 closure and B1 implementation completion, pointing toward B2
docs/analysis/reviews/code-reviews/2026-04-21-tyrne-to-phase-a.md
docs/analysis/reviews/security-reviews/2026-04-21-tyrne-to-phase-a.md
docs/analysis/reviews/security-reviews/2026-04-27-B0-closure.md
docs/architecture/exceptions.md
README.md

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 29, 2026

📝 Walkthrough

Walkthrough

Documentation updates marking Phase B1 (T-012 exception/IRQ infrastructure and T-013) as complete: status changes across README/architecture/roadmap, new B1 closure retrospectives for business, security, and performance, index updates, and preparatory notes toward B2 (MMU activation and downstream work).

Changes

Cohort / File(s) Summary
Top-level Status & Roadmap
README.md, docs/roadmap/current.md, docs/roadmap/phases/phase-b.md
Promotes B1/T-012 to Done (PR #10 referenced), updates milestone/status text, moves next-review trigger to B2 (MMU arc), and records pending maintainer-side QEMU smoke / miri gates.
Architecture Index & Doc
docs/architecture/README.md, docs/architecture/exceptions.md
Marks exceptions.md as Done, adds PR #10 merge context, updates idle wfi narrative (ADR-0022 closed) and notes deferred QEMU/Miri verification.
Task List
docs/analysis/tasks/phase-b/README.md, docs/analysis/tasks/phase-b/T-012-exception-and-irq-infrastructure.md
Updates T-012 status from In Review → Done, adjusts acceptance checklist and review-history entries to record PR #10 merge and remaining maintainer verification items.
Business Review
docs/analysis/reviews/business-reviews/2026-04-28-B1-closure.md, docs/analysis/reviews/business-reviews/README.md
Adds B1 closure retrospective detailing completed tasks (T-013, T-012), ADR amendments, audit-log entries, test deltas, delivered architecture docs, and index entry for 2026-04-28.
Security Review
docs/analysis/reviews/security-reviews/2026-04-28-B1-closure.md, docs/analysis/reviews/security-reviews/README.md, docs/analysis/reviews/security-reviews/2026-04-27-B0-closure.md
Adds B1 consolidated security closure with trust-boundary analysis (GICv2 MMIO, EL1 vector table, IRQ trampoline, timer wiring), documents UNSAFE audit items and race windows, marks verdict Approve, updates index and corrects B0 closure link target.
Performance Baseline
docs/analysis/reviews/performance-optimization-reviews/2026-04-28-B1-closure.md, docs/analysis/reviews/performance-optimization-reviews/README.md
Introduces B1 baseline artifact: ELF/section sizing, contributors to .text/.bss growth, IRQ delivery cost metric, boot-time estimates, hotspot analysis, and index update with new metric.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 I thumped my foot — B1's done at last,

EL1 and IRQs hopped past,
Papers filed and baselines laid,
QEMU snooze and Miri stayed,
Onward we bound toward B2's vast.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: B1 closure documentation, T-012 promotion to Done, and repository cleanup—all of which are the primary focus across the changeset.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch development

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.

❤️ Share
Review rate limit: 0/1 reviews remaining, refill in 60 minutes.

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've left some high level feedback:

  • The repeated explanations of the Pending QEMU smoke verification pattern across current.md, phase-b.md, the reviews, and the T-012 task file could be factored into a single canonical explanation (e.g., in unsafe-policy.md or a short standard) and referenced from those sites to reduce duplication and drift risk.
  • The narrative sections around B1 status and scope (e.g., current.md, phase-b.md, B1 closure retro, security/perf reviews) repeat similar timeline and cost-of-arc details; consider tightening or cross-linking instead of restating full context in each file to keep future milestone updates simpler and less error-prone.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The repeated explanations of the `Pending QEMU smoke verification` pattern across `current.md`, `phase-b.md`, the reviews, and the T-012 task file could be factored into a single canonical explanation (e.g., in `unsafe-policy.md` or a short standard) and referenced from those sites to reduce duplication and drift risk.
- The narrative sections around B1 status and scope (e.g., `current.md`, `phase-b.md`, B1 closure retro, security/perf reviews) repeat similar timeline and cost-of-arc details; consider tightening or cross-linking instead of restating full context in each file to keep future milestone updates simpler and less error-prone.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request completes the implementation of Phase B1, which includes exception infrastructure and interrupt delivery for the microkernel. Key changes involve the installation of an EL1 exception vector table, a GIC v2 driver, and generic-timer IRQ wiring. The documentation has been updated to reflect the 'Done' status of milestone B1, and several performance and security review artifacts have been added. I have provided feedback on improving the conciseness of the README, correcting a calculation error in the B1 closure retrospective, clarifying the status of the file rename, and improving the evergreen nature of the task documentation.

Comment thread README.md Outdated
> A capability-based microkernel for high-assurance devices, written in Rust.

**Status:** Pre-alpha. Phase A complete (2026-04-21) — the kernel boots on QEMU `virt` aarch64 and runs a two-task IPC demo end-to-end with capability-gated message transfer. Phase B underway: scheduler hardening, idle task, monotonic time source via the ARM Generic Timer, and the path toward EL drop / MMU / userspace. See [`docs/roadmap/current.md`](docs/roadmap/current.md) for the active task.
**Status:** Pre-alpha. Phase A complete (2026-04-21) — the kernel boots on QEMU `virt` aarch64 and runs a two-task IPC demo end-to-end with capability-gated message transfer. Phase B B0 (Phase-A exit hygiene — scheduler hardening, idle task, monotonic time source via the ARM Generic Timer) closed 2026-04-27. Phase B B1 (drop to EL1 + exception infrastructure) implementation complete 2026-04-28: kernel runs at EL1 unconditionally with a 16-entry vector table at `VBAR_EL1`, a GIC v2 driver, generic-timer IRQ wiring, and idle's `wfi` activation. Path forward: B2 MMU activation, then per-task address spaces, task loader, syscall boundary, first userspace "hello". See [`docs/roadmap/current.md`](docs/roadmap/current.md) for the active task.
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The status description is very detailed and lengthy for the main README.md. A more concise summary that directs readers to docs/roadmap/current.md for details would improve readability for newcomers to the project.

Suggested change
**Status:** Pre-alpha. Phase A complete (2026-04-21) — the kernel boots on QEMU `virt` aarch64 and runs a two-task IPC demo end-to-end with capability-gated message transfer. Phase B B0 (Phase-A exit hygiene — scheduler hardening, idle task, monotonic time source via the ARM Generic Timer) closed 2026-04-27. Phase B B1 (drop to EL1 + exception infrastructure) implementation complete 2026-04-28: kernel runs at EL1 unconditionally with a 16-entry vector table at `VBAR_EL1`, a GIC v2 driver, generic-timer IRQ wiring, and idle's `wfi` activation. Path forward: B2 MMU activation, then per-task address spaces, task loader, syscall boundary, first userspace "hello". See [`docs/roadmap/current.md`](docs/roadmap/current.md) for the active task.
**Status:** Pre-alpha. Phase A (IPC demo) and Phase B milestones B0 (hygiene) and B1 (exception infrastructure) are implementation-complete. The kernel now runs at EL1 with a functional IRQ path. Next up is B2 (MMU activation). See [`docs/roadmap/current.md`](docs/roadmap/current.md) for details.


Commit `c366200` landed `docs/architecture/exceptions.md` *before* the implementation commits `a043079` / `b4ed68c`. The doc covered the vector-table layout, the IRQ dispatch flow (Mermaid sequence), the GIC v2 driver shape, the generic-timer IRQ wiring, the idle WFI activation closure, *and* the IRQ-handler interaction with the raw-pointer scheduler bridge. The implementation map at the doc's end listed each of T-012's seven Approach steps with status flags (🔵 next, 🟡 follows, 🟢 in-progress / partially done by this doc) so the next implementation commit knew exactly where to land.

**Result: only two diagram/prose mismatches caught during PR #10's two rounds — no structural "doc disagrees with code" findings.** The Mermaid diagram in the doc was updated in PR #10 round-1 to reflect the shipped ack-and-ignore behaviour (the original draft showed `sched::on_timer_irq` which was deliberately deferred); round-2 caught a remaining prose mismatch at lines 160-167 that round-1 missed. Two diagram/prose corrections out of 19 + 2 + 2 = 23 total findings — both narrow scope, both about post-design behaviour drift rather than design-vs-implementation disagreement. **Lesson: design-first reduces but does not eliminate doc-vs-code drift; the doc still needs a post-implementation reconciliation pass.** Recommendation: continue design-first for B2's MMU activation, but bake "post-implementation doc-vs-code reconciliation" into the AC rather than relying on the review-fix-rounds to catch it.
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

There appears to be a calculation error in the total number of findings. The text states 19 + 2 + 2 = 23 total findings, but other documents in this PR and the breakdown on line 90 of this file (11/12 findings + 8 findings + 2 nitpicks) suggest a total of 21 findings (19 findings + 2 nitpicks). Please correct the total for consistency and accuracy.

Suggested change
**Result: only two diagram/prose mismatches caught during PR #10's two rounds — no structural "doc disagrees with code" findings.** The Mermaid diagram in the doc was updated in PR #10 round-1 to reflect the shipped ack-and-ignore behaviour (the original draft showed `sched::on_timer_irq` which was deliberately deferred); round-2 caught a remaining prose mismatch at lines 160-167 that round-1 missed. Two diagram/prose corrections out of 19 + 2 + 2 = 23 total findings — both narrow scope, both about post-design behaviour drift rather than design-vs-implementation disagreement. **Lesson: design-first reduces but does not eliminate doc-vs-code drift; the doc still needs a post-implementation reconciliation pass.** Recommendation: continue design-first for B2's MMU activation, but bake "post-implementation doc-vs-code reconciliation" into the AC rather than relying on the review-fix-rounds to catch it.
**Result: only two diagram/prose mismatches caught during PR #10's two rounds — no structural "doc disagrees with code" findings.** The Mermaid diagram in the doc was updated in PR #10 round-1 to reflect the shipped ack-and-ignore behaviour (the original draft showed `sched::on_timer_irq` which was deliberately deferred); round-2 caught a remaining prose mismatch at lines 160-167 that round-1 missed. Two diagram/prose corrections out of 21 total findings (19 findings + 2 nitpicks) — both narrow scope, both about post-design behaviour drift rather than design-vs-implementation disagreement. **Lesson: design-first reduces but does not eliminate doc-vs-code drift; the doc still needs a post-implementation reconciliation pass.** Recommendation: continue design-first for B2's MMU activation, but bake "post-implementation doc-vs-code reconciliation" into the AC rather than relying on the review-fix-rounds to catch it.


### File renames and stale-link drift — third confirmation

The T-009 mini-retro flagged file-rename / cross-reference drift as a pattern. The B0 retro confirmed it as a pattern (T-008's review needed a second round to catch four broken cross-references). T-012 produced **zero broken cross-references in its own changes** during PR #10's two rounds. Why? The post-fix sweep DoD rule (codified during B0) plus design-first (the `exceptions.md` doc was authoritative from commit-1, so subsequent commits couldn't drift). **Pattern confirmed: design-first + post-fix sweep DoD + review-fix-round cadence = stale-link drift suppressed for new T-012 changes.** Pre-existing repo-wide drift — the 2026-04-22 Umbrix → Tyrne rename had moved most filename references to the new shape but left two artefact files (`docs/analysis/reviews/{code,security}-reviews/2026-04-21-umbrix-to-phase-a.md`) at the old name, surfaced by the B1-closure approval-review's N2 finding — was a separate concern outside PR #10's scope; a follow-up `docs(refs):` commit (the immediate next commit on the `development` branch after this retro lands) renames both files to the new shape and fixes the three remaining link-form references that still point at the old name (the other ~20 references already use the new name and resolve correctly post-rename). The R6 CI follow-up (automated link check) is still desirable, both as a regression gate for new changes and as a backstop against future rename drift.
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This paragraph states that the umbrix-to-phase-a.md rename will be handled in a future commit. However, this pull request already includes the rename commit (10e3351). The documentation should be updated to reflect that this change is part of the current PR to avoid confusion. The Adjustments section on line 152 correctly marks this as done, but this paragraph contradicts it.

Suggested change
The T-009 mini-retro flagged file-rename / cross-reference drift as a pattern. The B0 retro confirmed it as a pattern (T-008's review needed a second round to catch four broken cross-references). T-012 produced **zero broken cross-references in its own changes** during PR #10's two rounds. Why? The post-fix sweep DoD rule (codified during B0) plus design-first (the `exceptions.md` doc was authoritative from commit-1, so subsequent commits couldn't drift). **Pattern confirmed: design-first + post-fix sweep DoD + review-fix-round cadence = stale-link drift suppressed for new T-012 changes.** Pre-existing repo-wide drift — the 2026-04-22 Umbrix → Tyrne rename had moved most filename references to the new shape but left two artefact files (`docs/analysis/reviews/{code,security}-reviews/2026-04-21-umbrix-to-phase-a.md`) at the old name, surfaced by the B1-closure approval-review's N2 finding — was a separate concern outside PR #10's scope; a follow-up `docs(refs):` commit (the immediate next commit on the `development` branch after this retro lands) renames both files to the new shape and fixes the three remaining link-form references that still point at the old name (the other ~20 references already use the new name and resolve correctly post-rename). The R6 CI follow-up (automated link check) is still desirable, both as a regression gate for new changes and as a backstop against future rename drift.
The T-009 mini-retro flagged file-rename / cross-reference drift as a pattern. The B0 retro confirmed it as a pattern (T-008's review needed a second round to catch four broken cross-references). T-012 produced **zero broken cross-references in its own changes** during PR #10's two rounds. Why? The post-fix sweep DoD rule (codified during B0) plus design-first (the `exceptions.md` doc was authoritative from commit-1, so subsequent commits couldn't drift). **Pattern confirmed: design-first + post-fix sweep DoD + review-fix-round cadence = stale-link drift suppressed for new T-012 changes.** Pre-existing repo-wide drift from the 2026-04-22 Umbrix → Tyrne rename was also fixed in this PR. Two artefact files (`docs/analysis/reviews/{code,security}-reviews/2026-04-21-umbrix-to-phase-a.md`) were renamed and three remaining stale link-form references were fixed. The R6 CI follow-up (automated link check) is still desirable, both as a regression gate for new changes and as a backstop against future rename drift.

| 2026-04-28 | @cemililik (+ Claude Opus 4.7 agent) | Promoted `Draft → In Progress` with a *design-first* arc. The B0 closure security review §8 ("Architecture docs are a security multiplier") tightened T-012's Documentation AC to require `docs/architecture/exceptions.md` as a hard deliverable; this entry records that doc landing **ahead of** the code, not as a follow-up. The doc covers: 16-entry vector table layout at `VBAR_EL1` (Mermaid 4×4 grid; only the Current-EL-SP_ELx IRQ entry fires in v1, others trampoline to a panic-class handler); IRQ dispatch flow (trampoline → register-frame save → Rust handler → ack via `IrqController` → dispatch → EOI → eret; Mermaid sequence diagram); GIC v2 driver shape (distributor `0x0800_0000` + CPU interface `0x0801_0000`; per-register table; init sequence; per-method MMIO mapping); generic-timer IRQ wiring (PPI 27, `CNTV_CVAL_EL0` / `CNTV_CTL_EL0` programming, handler hook); idle WFI activation closure; IRQ-handler interaction with the raw-pointer scheduler bridge (likely an ADR-0021 Amendment when the handler is wired). Implementation map at the end of the doc shows the seven Approach steps with status flags (🔵 next, 🟡 follows, 🟢 in-progress / partially done by this doc) so the next implementation commit knows exactly where to land. **No code in this commit** — `bsp-qemu-virt` and `kernel/src/sched/mod.rs` untouched; `bsp-qemu-virt/src/gic.rs` not yet created; boot.s not yet extended; the `unimplemented!()` panics in `<QemuVirtCpu as Timer>` still in place. T-012 stays In Progress; the next session brings up the code against this design. `docs/architecture/README.md` index gains an `exceptions.md` row marked Design status. ADR-0024 (post-T-013) status note in the README also corrected (was "EL drop pending T-013"; T-013 has been Done since 2026-04-27). |
| 2026-04-28 | @cemililik (+ Claude Opus 4.7 agent) | Promoted `In Progress → In Review`. Three implementation commits land in order: (1) commit `a043079` — GIC v2 driver (`bsp-qemu-virt/src/gic.rs`), 16-entry vector table at `tyrne_vectors` (`bsp-qemu-virt/src/vectors.s`), `MSR VBAR_EL1` install + `MSR DAIFClr, #0x2` unmask in `kernel_entry`; UNSAFE-2026-0019 (GIC MMIO) + UNSAFE-2026-0020 (vector table + trampolines) appended. (2) commit `b4ed68c` — `<QemuVirtCpu as Timer>::arm_deadline` / `cancel_deadline` real implementations; `irq_entry` dispatch on PPI 27 (mask `CNTV_CTL_EL0` + EOI; ack-and-ignore for v1); `idle_entry` body switched from `core::hint::spin_loop()` to `cpu.wait_for_interrupt()`; `tyrne_hal::timer::ns_to_ticks` helper + 5 host tests (148/148 host suite green); UNSAFE-2026-0021 (CNTV_CTL/CVAL writes) appended. (3) commit `28c5ce9` — documentation sweep: ADR-0021 §Revision notes Amendment for IRQ-handler aliasing discipline (extends ADR-0021's no-`&mut`-across-switch rule to the IRQ frame; v1's ack-and-ignore body vacuously satisfies the discipline; future scheduler-touching IRQ arcs follow Option A's momentary-`&mut` pattern); UNSAFE-2026-0014 Amendment naming `irq_entry` as a future site of the same pattern; ADR-0010 §Revision notes recording that the deferred `arm_deadline` / `cancel_deadline` halves are now live; ADR-0022 first rider's *Sub-rider* gains a closure paragraph naming this task and commit; `docs/architecture/README.md` `exceptions.md` row promoted from `Design` to live status; phase-b §B1 README + `docs/roadmap/current.md` updated. QEMU smoke + Miri pass deferred to maintainer-side verification (R6 CI skeleton pinned for Miri; QEMU trace reproduces in maintainer's environment, not in this dev container — same disclaimer T-013 used). All four DoD gates that depend on this dev environment pass; the two deferred items are flagged on the DoD checklist with rationale. |
| 2026-04-28 | @cemililik (+ Claude Opus 4.7 agent) | Two review-fix sweeps before merge. (a) PR #10 review-round-1 (commit `d820a88`) applied 11 of 12 findings: `irq_entry` / `panic_entry` promoted to `unsafe extern "C" fn` so caller-side preconditions are visible at the type level; `QemuVirtGic::enable` / `disable` now assert `irq.0 < GIC_MAX_IRQ` (1020) per ARM IHI 0048B §4.3.2; `tyrne_hal::timer::ns_to_ticks` switched from floor to ceiling division (via `u128::div_ceil`) so `Timer::arm_deadline`'s "reaches or exceeds `deadline_ns`" contract holds at non-divisor frequencies; saturation test extended with two over-boundary cases; nine doc fixes (Mermaid diagram + saved-register prose in `exceptions.md` matching shipped ack-and-ignore behaviour, ADR-0021 Amendment-already-exists rephrasing, B0-closure retro arithmetic, phase-b T-012 row label). (b) PR #10 review-round-2 (commit `5c9cf06`) applied 8 findings + 2 nitpicks: both `irq_entry` inline SAFETY blocks expanded to the full `unsafe-policy.md` §1 triplet (invariants + rejected alternatives + audit ref); `exceptions.md` ADR-0021 amendment language and date corrected (2026-04-27 → 2026-04-28) at three sites + sched::on_timer_irq prose at lines 160-167 rewritten to match shipped ack-and-ignore body; ADR-0021 §Revision notes Amendment cited signature updated `extern "C" fn` → `unsafe extern "C" fn` with explanatory parenthetical; `current.md` "Next review trigger" intermediate-gate clause about WFI activation removed (already landed in `b4ed68c`); compile-time `const _: () = assert!(size_of::<TrapFrame>() == 192)` guard added; `ns_to_ticks_rounds_up_on_subtick` test (freq=3, ns=333_333_334 → 2) added so a future floor-rounding regression fails the host suite. Host count grew 148 → 149 (+1 sub-tick rounding test). Gates remain green across both rounds. |
| 2026-04-28 | @cemililik (+ independent review agent) | Promoted `In Review → Done` via PR #10 merge to `main` (merge commit SHA pending back-fill in a follow-up `docs(audits)`-style commit, per the precedent set by `6d2f725` for the T-012 step-3 SHA). At merge: 149 / 149 host tests, kernel-build / kernel-clippy / host-clippy / fmt all clean. Three implementation commits + one documentation sweep + two review-round commits land as one cohesive `In Review` arc. **Two DoD gates remain explicitly maintainer-side and untouched by this promotion** — `cargo +nightly miri test` and the QEMU smoke run that exercises the deliberate-deadline path; both are pinned for the maintainer's CI / hardware-side runs (R6 GitHub Actions skeleton). UNSAFE-2026-0019 / 0020 / 0021 audit-log entries keep their `Pending QEMU smoke verification at the maintainer's first opportunity` status notes intact; those notes will be lifted to past-tense via append-only Amendments only after the maintainer has actually run the smoke, not as part of the merge promotion. T-012 closes B1's exception-infrastructure half: ADR-0010 deferred halves are live (`arm_deadline` / `cancel_deadline` real on `QemuVirtCpu`), ADR-0022 first rider's *Sub-rider* is closed (idle's WFI activation), ADR-0021 carries the IRQ-handler aliasing discipline as the 2026-04-28 Amendment. Next review trigger is the **B1 closure** business + security + performance review trio (per `docs/roadmap/current.md`'s next-review-trigger entry). |
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The note about the merge commit SHA being back-filled later is a process detail that can be confusing and makes the document feel dated immediately after merge. This pattern appears in other documents in this PR as well. Consider rephrasing to be more evergreen, for example by stating that the promotion to "Done" happens with the merge of the PR that includes these changes.

Suggested change
| 2026-04-28 | @cemililik (+ independent review agent) | Promoted `In Review → Done` via PR #10 merge to `main` (merge commit SHA pending back-fill in a follow-up `docs(audits)`-style commit, per the precedent set by `6d2f725` for the T-012 step-3 SHA). At merge: 149 / 149 host tests, kernel-build / kernel-clippy / host-clippy / fmt all clean. Three implementation commits + one documentation sweep + two review-round commits land as one cohesive `In Review` arc. **Two DoD gates remain explicitly maintainer-side and untouched by this promotion** — `cargo +nightly miri test` and the QEMU smoke run that exercises the deliberate-deadline path; both are pinned for the maintainer's CI / hardware-side runs (R6 GitHub Actions skeleton). UNSAFE-2026-0019 / 0020 / 0021 audit-log entries keep their `Pending QEMU smoke verification at the maintainer's first opportunity` status notes intact; those notes will be lifted to past-tense via append-only Amendments only after the maintainer has actually run the smoke, not as part of the merge promotion. T-012 closes B1's exception-infrastructure half: ADR-0010 deferred halves are live (`arm_deadline` / `cancel_deadline` real on `QemuVirtCpu`), ADR-0022 first rider's *Sub-rider* is closed (idle's WFI activation), ADR-0021 carries the IRQ-handler aliasing discipline as the 2026-04-28 Amendment. Next review trigger is the **B1 closure** business + security + performance review trio (per `docs/roadmap/current.md`'s next-review-trigger entry). |
| 2026-04-28 | @cemililik (+ independent review agent) | Promoted `In Review → Done` with the merge of PR #10 to `main`. At merge: 149 / 149 host tests, kernel-build / kernel-clippy / host-clippy / fmt all clean. Three implementation commits + one documentation sweep + two review-round commits land as one cohesive `In Review` arc. **Two DoD gates remain explicitly maintainer-side and untouched by this promotion** — `cargo +nightly miri test` and the QEMU smoke run that exercises the deliberate-deadline path; both are pinned for the maintainer's CI / hardware-side runs (R6 GitHub Actions skeleton). UNSAFE-2026-0019 / 0020 / 0021 audit-log entries keep their `Pending QEMU smoke verification at the maintainer's first opportunity` status notes intact; those notes will be lifted to past-tense via append-only Amendments only after the maintainer has actually run the smoke, not as part of the merge promotion. T-012 closes B1's exception-infrastructure half: ADR-0010 deferred halves are live (`arm_deadline` / `cancel_deadline` real on `QemuVirtCpu`), ADR-0022 first rider's *Sub-rider* is closed (idle's WFI activation), ADR-0021 carries the IRQ-handler aliasing discipline as the 2026-04-28 Amendment. Next review trigger is the **B1 closure** business + security + performance review trio (per `docs/roadmap/current.md`'s next-review-trigger entry). |

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🧹 Nitpick comments (1)
README.md (1)

5-5: Consider splitting the status line for readability.

This is clear but very dense; a short bullet list (Phase A / B0 / B1 / next) would also eliminate the repeated sentence-start pattern flagged by LanguageTool.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@README.md` at line 5, Split the dense "Status:" single-line entry into a
short bullet list to improve readability: create separate bullets for "Phase A",
"Phase B B0", "Phase B B1", and "Next" (or "Path forward"), moving the detailed
notes about kernel boot, QEMU virt aarch64 IPC demo, the scheduler/idle/timer
work, and the EL1/vector/GIC/timer/idle details into their respective bullets
and add a final bullet linking to docs/roadmap/current.md; keep the "Status:"
label and dates but avoid repeating sentence starts so each bullet is a concise
summary of the phase and its outcome.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@docs/analysis/reviews/performance-optimization-reviews/2026-04-28-B1-closure.md`:
- Line 177: Update the summary text to match the linked security review by
changing "one forward-flagged non-blocking item" to "two forward-flagged
non-blocking items" and include both item names exactly as recorded in "B1
closure security review (2026-04-28)"; ensure the existing "arm_deadline
CVAL+CTL race" entry remains and add the other forward-flagged item from that
security review so the count and list are fully synchronized.

In `@docs/analysis/reviews/security-reviews/2026-04-28-B1-closure.md`:
- Around line 68-71: The doc's claims that irq_entry is "straight-line" and
"loop-free" are incorrect — the actual implementation in irq_entry (see
bsp-qemu-virt's irq_entry handler) uses a loop to acknowledge pending IRQs via
repeated GICC_IAR reads; update the text to remove absolutes about being
straight-line/loop-free/constant-time and instead state that irq_entry may
iterate to drain pending IRQs, describe that the loop is bounded by the number
of pending IRQs returned by GICC_IAR, and soften any timing/real-time guarantees
(e.g., change "constant-time" to "bounded by pending-interrupt count" and remove
claims that rely on being strictly straight-line). Ensure you reference
irq_entry and the GICC_IAR-based acknowledge loop in the revised wording.
- Line 120: Replace the non-English verdict word "Yüksek" with "High" in the
verdict text to comply with the English-only guideline; locate the verdict
string "Yüksek" in the document's verdict section and update that token to
"High" without altering surrounding markdown or formatting so the rest of the
review text remains unchanged.

In `@docs/analysis/reviews/security-reviews/README.md`:
- Line 36: Update the verdict text string "Approve — no Yüksek findings;" to use
an English term (e.g., "Approve — no High findings;" or "Approve — no
high-severity findings;") so the README entry uses only English; locate and
replace that exact phrase in the README line containing the table entry (the
cell starting "Approve — no Yüksek findings;") and ensure the rest of the
punctuation/format remains unchanged.

In `@docs/analysis/tasks/phase-b/T-012-exception-and-irq-infrastructure.md`:
- Line 5: Update the checklist item that currently reads "Task status updated to
`In Review`" in the T-012 document so it matches the actual state `Done` (e.g.,
change the checklist text to "Task status updated to `Done`"); ensure the
document’s status lines for T-012 (the header/status and the checklist entry)
are consistent and reflect the new `Done` state.

In `@docs/roadmap/phases/phase-b.md`:
- Line 96: Update the B1 status sentence that currently reads "B1 closure review
trio is pending" to reflect that the closure trio has landed on 2026-04-28;
change the paragraph around "Status: B1 implementation complete 2026-04-28" /
"B1 closure review trio is pending" to state the trio is complete (landed via
PRs noted) and leave only "maintainer-side QEMU smoke + Miri pass" as pending.
Ensure you modify the exact phrase "B1 closure review trio" and the surrounding
sentence so the only remaining pending items are the maintainer QEMU smoke and
Miri passes.

---

Nitpick comments:
In `@README.md`:
- Line 5: Split the dense "Status:" single-line entry into a short bullet list
to improve readability: create separate bullets for "Phase A", "Phase B B0",
"Phase B B1", and "Next" (or "Path forward"), moving the detailed notes about
kernel boot, QEMU virt aarch64 IPC demo, the scheduler/idle/timer work, and the
EL1/vector/GIC/timer/idle details into their respective bullets and add a final
bullet linking to docs/roadmap/current.md; keep the "Status:" label and dates
but avoid repeating sentence starts so each bullet is a concise summary of the
phase and its outcome.
🪄 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: 5d6959f0-75a0-4a85-907d-e3e9adedbb70

📥 Commits

Reviewing files that changed from the base of the PR and between 7b42bbe and 10e3351.

📒 Files selected for processing (16)
  • README.md
  • docs/analysis/reviews/business-reviews/2026-04-28-B1-closure.md
  • docs/analysis/reviews/business-reviews/README.md
  • docs/analysis/reviews/code-reviews/2026-04-21-tyrne-to-phase-a.md
  • docs/analysis/reviews/performance-optimization-reviews/2026-04-28-B1-closure.md
  • docs/analysis/reviews/performance-optimization-reviews/README.md
  • docs/analysis/reviews/security-reviews/2026-04-21-tyrne-to-phase-a.md
  • docs/analysis/reviews/security-reviews/2026-04-27-B0-closure.md
  • docs/analysis/reviews/security-reviews/2026-04-28-B1-closure.md
  • docs/analysis/reviews/security-reviews/README.md
  • docs/analysis/tasks/phase-b/README.md
  • docs/analysis/tasks/phase-b/T-012-exception-and-irq-infrastructure.md
  • docs/architecture/README.md
  • docs/architecture/exceptions.md
  • docs/roadmap/current.md
  • docs/roadmap/phases/phase-b.md

Comment thread docs/analysis/reviews/performance-optimization-reviews/2026-04-28-B1-closure.md Outdated
Comment on lines +68 to +71
- **`irq_entry`'s ack-and-ignore body is constant-time.** OK — straight-line: read GICC_IAR (1 volatile read), branch on spurious (1 cmp + 1 b), branch on IRQ ID (1 cmp + 1 b), `MSR CNTV_CTL_EL0` (1 instruction), `gic.end_of_interrupt` (1 volatile write), return. ~10 instructions of dispatch logic plus the 31 instructions of trampoline save/restore — bounded, predictable, no allocation, no scheduler touch. Matches the [`docs/architecture/exceptions.md`](../../../architecture/exceptions.md) §Invariants → "ISR minimality" claim.
- **No new heap allocation.** OK — T-012 added zero `alloc::*` imports, zero `Box::new`, zero `Vec::new`. Kernel remains heap-free per ADR-0001. The 192-byte `TrapFrame` is on the kernel stack (allocated by `sub sp, sp, #192`); not a heap allocation.
- **No unbounded loops in ISR.** OK — `irq_entry`'s body has no loop. The dispatch is straight-line.
- **No new kernel panic on a *hot* path.** OK — the new panic in `irq_entry` is the structurally-unreachable "unhandled IRQ" guard; the GIC range-assertion panics in `enable` / `disable` are also defensive against kernel-internal regressions, not user input. No new userspace-reachable panic.
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Correct ISR characterization: irq_entry is not straight-line and not loop-free.

This section says dispatch is constant-time/straight-line and that there are no loops in ISR, but bsp-qemu-virt/src/exceptions.rs shows irq_entry iterates in a loop while acknowledging pending IRQs. Please revise these claims to avoid overstating timing bounds.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/analysis/reviews/security-reviews/2026-04-28-B1-closure.md` around lines
68 - 71, The doc's claims that irq_entry is "straight-line" and "loop-free" are
incorrect — the actual implementation in irq_entry (see bsp-qemu-virt's
irq_entry handler) uses a loop to acknowledge pending IRQs via repeated GICC_IAR
reads; update the text to remove absolutes about being
straight-line/loop-free/constant-time and instead state that irq_entry may
iterate to drain pending IRQs, describe that the loop is bounded by the number
of pending IRQs returned by GICC_IAR, and soften any timing/real-time guarantees
(e.g., change "constant-time" to "bounded by pending-interrupt count" and remove
claims that rely on being strictly straight-line). Ensure you reference
irq_entry and the GICC_IAR-based acknowledge loop in the revised wording.

Comment thread docs/analysis/reviews/security-reviews/2026-04-28-B1-closure.md Outdated
Comment thread docs/analysis/reviews/security-reviews/README.md Outdated
Comment thread docs/roadmap/phases/phase-b.md Outdated
Approval review on PR #11 returned a punch-list: 5 inline comments
+ 1 nitpick + 3 PR-level comments + 2 overall comments. This commit
applies 9 of the 10 actionable findings; 1 finding (security review
§4 irq_entry "loop" claim) was verified false against the source
and is rejected with explanation below. The 2 overall comments
(pattern-deduplication, narrative-tightening) are noted as known
follow-up concerns but are out-of-scope for this commit.

Findings applied:

(F1) `docs/analysis/reviews/performance-optimization-reviews/`
     `2026-04-28-B1-closure.md` — security cross-reference line:
     "one forward-flagged non-blocking item" → "two forward-flagged
     non-blocking items" with both names (`arm_deadline` CVAL+CTL
     race + `cancel_deadline` CTL+GIC race). Synchronises with the
     security review (which gained the second item in commit
     `a5d4f9b`).

(F3) `docs/analysis/reviews/security-reviews/2026-04-28-B1-closure.md`
     §Verdict — "no Yüksek findings" → "no high-severity findings".
     Brings the B1 trio's verdict prose under CLAUDE.md §3 (English
     in repository) without disrupting the established
     conduct-approval-review SKILL template's Turkish severity-label
     vocabulary (which the user explicitly approved earlier as
     "established pattern"). Scope kept narrow: this commit
     translates only the B1 trio's instances; the B0 closure
     security review's index entry retains "Yüksek" as a
     pre-existing committed artifact (unsafe-policy.md §3 Amendment
     discipline applies to audit-log entries, not review artifacts,
     but a broader sweep is a separate scope question and not
     trigger-driven by this review round).

(F4) `docs/analysis/reviews/security-reviews/README.md` line 36 —
     same Yüksek → high-severity translation in the index entry's
     Verdict cell.

(F5) `docs/analysis/tasks/phase-b/T-012-exception-and-irq-`
     `infrastructure.md` Definition-of-done checklist line 82 —
     "Task status updated to `In Review`" → "Task status updated to
     `Done`". The Status field at line 5 was already `Done` from
     ffefb76; the DoD checklist line was a stale residual from the
     `In Review` promotion. Now consistent.

(F6) `docs/roadmap/phases/phase-b.md` B1 milestone status block —
     "B1 closure review trio is pending" → "B1 closure review trio
     landed 2026-04-28 via PR #11" with deep-links to the three
     artefacts. Only QEMU smoke + Miri remain pending. The B1
     milestone-level `Done` flip happens after the maintainer-side
     verification.

(N1) `README.md` Status: line — split from a dense single-paragraph
     summary into a four-bullet status list (Phase A / Phase B B0 /
     Phase B B1 / Path forward) plus a final pointer to
     `docs/roadmap/current.md`. Each bullet is a concise summary;
     dates retained; "Status:" label retained.

(P1) `docs/analysis/reviews/business-reviews/2026-04-28-B1-closure.md`
     §"Design-first arc" — math `19 + 2 + 2 = 23 total findings` →
     `21 total review items (19 substantive findings — 11 round-1 +
     8 round-2 — plus 2 round-2 nitpicks; the round-1 nitpick
     disposition is folded into the 11 applied)`. The `19+2+2`
     arithmetic was wrong (sums to 23, but the breakdown didn't
     match either round's commit-message numbers). The new text
     gives a clear decomposition matching `d820a88` ("apply 11 of
     12 findings") and `5c9cf06` ("8 findings + 2 nitpicks") commit
     messages.

(P2) `docs/analysis/reviews/business-reviews/2026-04-28-B1-closure.md`
     §"File renames and stale-link drift" — paragraph that said the
     `umbrix → tyrne` rename would land in "a follow-up `docs(refs):`
     commit (the immediate next commit after this retro lands)" was
     stale: the rename DID land in commit `10e3351`, which is one
     of PR #11's four commits — the same PR that lands this retro.
     Updated to "the `docs(refs):` commit `10e3351` (one of the
     four commits in PR #11, the same PR that lands this retro)"
     with deep-link. The Adjustments item already correctly says
     ✅ Done; this paragraph now matches.

(P3) Merge-commit-SHA back-fill across 3 documents. PR #10's merge
     into `main` is commit `7b42bbe` (verified via
     `git log --oneline 9a66e8b..origin/main`); the documents that
     said "merge commit SHA pending back-fill in a follow-up
     `docs(audits)`-style commit" are now updated to reference
     `7b42bbe` directly with a clickable PR #10 link. Affected
     files:
     - `docs/analysis/tasks/phase-b/T-012-exception-and-irq-`
       `infrastructure.md` (review-history Done promotion row).
     - `docs/analysis/reviews/security-reviews/`
       `2026-04-28-B1-closure.md` (Change frontmatter).
     - `docs/roadmap/current.md` (Last-completed-tasks line; also
       added merge-commit `9a66e8b` cross-reference for T-013/PR
       #9 to match PR #10's back-fill shape).
     The earlier framing was process-detail noise (`docs(audits)-`
     style follow-up commit) that made each document feel dated
     immediately after merge; the new framing names the merge
     commit directly and is evergreen.

Finding rejected:

(F2) Security review §4 "Kernel-mode discipline" — claim that
     `irq_entry` is "straight-line / loop-free / constant-time".
     The reviewer asserted this is incorrect because "irq_entry
     uses a loop to acknowledge pending IRQs via repeated
     GICC_IAR reads".

     Verified against the source ([`bsp-qemu-virt/src/exceptions.rs`
     `irq_entry`](docs/architecture/exceptions.md) lines ~145-193):
     `irq_entry`'s body is straight-line. There is no `loop`, no
     `while`, no `for`. The dispatch shape is:
       1. `let gic = ...` (single static deref)
       2. `let Some(irq) = gic.acknowledge() else { return };`
          (single GICC_IAR read; spurious → early return)
       3. `if irq.0 == TIMER_IRQ_ID { ... return; }`
          (single dispatch arm; mask + EOI + return)
       4. `panic!("unhandled IRQ")` (structurally unreachable in v1)

     `gic.acknowledge()` returns `Option<IrqNumber>` — a single
     IRQ ID, not an iterator. The function does NOT drain pending
     IRQs in a loop; the GICv2 architecture actually delivers one
     IRQ per acknowledge cycle. To process additional pending
     IRQs, the CPU re-takes the exception after `eret` (if more
     are pending and DAIF.I is unmasked), which goes through the
     trampoline + `irq_entry` again — but each `irq_entry`
     invocation handles exactly one IRQ.

     The "constant-time / straight-line" claim is therefore
     correct and remains in the security review unchanged. The
     reviewer's mental model may have come from a typical
     edge-triggered GIC handler that DOES loop on IAR until
     spurious — but Tyrne's choice (per [docs/architecture/
     exceptions.md](docs/architecture/exceptions.md) §"Dispatch
     flow") is one-IRQ-per-entry; the documentation accurately
     reflects the implementation.

Overall comments not addressed (noted as known concerns):

- **`Pending QEMU smoke verification` pattern repetition.** The
  pattern appears in `current.md`, `phase-b.md`, the three review
  artefacts, and the T-012 task file. Factoring into a single
  canonical explanation (in `unsafe-policy.md` or a short standard)
  is the right shape; per the [B1 closure retro Adjustments item +
  Appendix #2](docs/analysis/reviews/business-reviews/2026-04-28-B1-closure.md),
  the canonisation trigger is "the next IRQ-related arc reproduces
  the pattern" — cross-arc reproducibility threshold. Not actionable
  in this commit.
- **B1 narrative repetition across files.** Real concern but
  addressing it via cross-linking would substantially restructure
  multiple committed review artefacts (which are append-only-style
  per the project convention). Better handled at the next milestone
  retro's plan-diff section, when the retro can decide whether to
  pivot to a cross-link-heavy style going forward. Recorded here
  for visibility.

Gates (no code changed; documentation only): cargo fmt /
kernel-build / kernel-clippy / host-clippy clean; host-test 149 /
149 unchanged.

Refs: PR #11 approval review (the conversation-driven punch-list);
T-012 (Done 2026-04-28; merge commit `7b42bbe`); ADR-0021 (2026-04-28
Amendment unchanged); ADR-0022 (first rider Sub-rider closed,
unchanged); CLAUDE.md §3 (English-in-repository rule applied to
verdict prose).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@cemililik
Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 29, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (1)
docs/analysis/reviews/security-reviews/2026-04-28-B1-closure.md (1)

68-71: ⚠️ Potential issue | 🟠 Major

Unaddressed past review finding: IRQ dispatch characterization needs correction.

A previous review identified that lines 68-71 incorrectly characterize irq_entry as "straight-line" and "loop-free," but the actual implementation uses a loop to acknowledge pending IRQs via repeated GICC_IAR reads. This finding was not marked as addressed.

Please revise to accurately reflect the implementation: remove absolute claims about being straight-line/loop-free/constant-time and instead describe that irq_entry may iterate to drain pending IRQs, with the loop bounded by the number of pending interrupts returned by GICC_IAR.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/analysis/reviews/security-reviews/2026-04-28-B1-closure.md` around lines
68 - 71, Update the review text about irq_entry to remove the incorrect
"straight-line", "loop-free", and "constant-time" assertions and instead state
that irq_entry may iterate reading GICC_IAR to drain pending interrupts;
describe that the dispatch loop is bounded by the number of pending IRQs
returned by GICC_IAR and thus remains predictable, heap-free, and non-allocating
(preserving ISR minimality), and adjust any references to the invariants/ISR
minimality claim to reflect this bounded-loop behavior of irq_entry.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@docs/analysis/reviews/security-reviews/2026-04-28-B1-closure.md`:
- Around line 68-71: Update the review text about irq_entry to remove the
incorrect "straight-line", "loop-free", and "constant-time" assertions and
instead state that irq_entry may iterate reading GICC_IAR to drain pending
interrupts; describe that the dispatch loop is bounded by the number of pending
IRQs returned by GICC_IAR and thus remains predictable, heap-free, and
non-allocating (preserving ISR minimality), and adjust any references to the
invariants/ISR minimality claim to reflect this bounded-loop behavior of
irq_entry.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 208f690f-ddb3-4a88-a081-5ed4c58a7967

📥 Commits

Reviewing files that changed from the base of the PR and between 10e3351 and 214052d.

📒 Files selected for processing (8)
  • README.md
  • docs/analysis/reviews/business-reviews/2026-04-28-B1-closure.md
  • docs/analysis/reviews/performance-optimization-reviews/2026-04-28-B1-closure.md
  • docs/analysis/reviews/security-reviews/2026-04-28-B1-closure.md
  • docs/analysis/reviews/security-reviews/README.md
  • docs/analysis/tasks/phase-b/T-012-exception-and-irq-infrastructure.md
  • docs/roadmap/current.md
  • docs/roadmap/phases/phase-b.md
✅ Files skipped from review due to trivial changes (1)
  • docs/analysis/reviews/business-reviews/2026-04-28-B1-closure.md
🚧 Files skipped from review as they are similar to previous changes (2)
  • docs/analysis/reviews/security-reviews/README.md
  • docs/roadmap/phases/phase-b.md

@cemililik cemililik merged commit 181cc87 into main May 6, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant