B2 prep integration bundle (replaces #19, #20, #21) — ADR-0027 + T-016 Draft + P10 harness + path-drift sweep + 2026-05-08 review - #22
Merged
Conversation
Mechanical fix of off-by-one relative-path drift across the prior comprehensive review's 7 affected track files (track-a, -b, -d, -e, -g, -h, -j). The same drift was caught + fixed in the 2026-05-07 PR #18 review files; this commit applies the equivalent sweep to the prior review per the disposition recorded in PR #18's gemini reply. Path-math summary: - track files live at docs/analysis/reviews/code-reviews/2026-05-06-full-tree/ (5 levels deep from repo root). - repo-root targets (kernel/, tools/, bsp-qemu-virt/, hal/, test-hal/, .claude/, .github/, .cargo/, .gitignore, Cargo.toml, Cargo.lock, LICENSE, README.md, AGENTS.md, CLAUDE.md, SECURITY.md, CONTRIBUTING.md, rust-toolchain.toml) need 5 ../. - docs-relative targets (audits/, standards/, decisions/, roadmap/, architecture/, glossary, analysis/, tasks/) need 4 ../. - One additional broken form: `(../../../../docs/<sub>/...)` was a redundant-docs-prefix shape (4 ../ resolves to docs/, then `docs/` prepends docs/docs/...). Collapsed to `(../../../../<sub>/...)`. 180/180 relative paths in the 7 affected track files now resolve via realpath (validated). The 3 remaining files in the same directory (track-c-security, track-f-tests, track-i-integration) were already using the correct convention and are byte-stable. No content changes — purely mechanical path-count adjustments. The review verdicts, findings, severity classifications, and prose are untouched. Refs: PR #18 review-round (gemini-code-assist disposition) — out-of-scope item now closed Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Single-run boot-to-end claims have ~15-30 % run-to-run variance under QEMU TCG (the counter advances on emulated instructions, not wall- clock time, so translation-cache state leaks into the number). The 2026-05-07 multi-axis review's Track D §D2 promoted the queued P10 proposal — a multi-run harness — from "queued" to "load-bearing before B2 ADR-0027 implementation" so future perf-relevant changes land against a tight percentile band rather than a single anecdote. The harness wraps tools/run-qemu.sh in an iteration loop with a portable per-run watchdog (the kernel halts in WFI after the demo; QEMU never exits on its own), parses the kernel's "boot-to-end elapsed = X ns" emission out of each run, and prints min / p10 / p50 / p90 / p99 / max / mean / stddev in both ns and ms. It optionally writes a markdown report under docs/analysis/reports/ using the perf-review master plan's Inputs / Methodology / Metric / Verdict shape. Pure bash + awk; macOS bash 3.2 compatible (matches run-qemu.sh's existing idioms; no GNU `timeout` binary required). A run aborts non-zero if fewer than half of its iterations produced a valid sample — that threshold is treated as environmental rather than a measurement worth aggregating, per the brief. Out of scope: CI integration (maintainer-launched only, like the QEMU smoke), automatic regression detection vs a stored baseline, the kernel-side `cfg(feature = "bench")` IPC microbench (a future P10 extension; v1 is multi-run aggregation of the existing boot- to-end emission). Refs: P10 (2026-05-06 Track D review), 2026-05-07 multi-axis review §D2
Records the first measured boot-to-end band produced by tools/perf-harness.sh and wires the harness into the project's documentation surface. - docs/analysis/reports/perf-baseline-2026-05-08-post-pr-19-pre- adr-0027.md — auto-generated baseline at HEAD `aa7e6c5`; debug build, 20 iterations, 5 s per-run timeout, QEMU TCG. Headline band: p10=3.884 ms / p50=4.642 ms / p90=5.584 ms / p99=6.558 ms, mean 4.711 ms, stddev 0.709 ms. Brackets the prior "~4-6.5 ms typical" anecdote tightly but is now a measured band on this host rather than an order-of-magnitude observation. - docs/standards/infrastructure.md — new "Performance harness" section names tools/perf-harness.sh as the canonical source for boot-to-end timing claims and deprecates single-run anecdotes in PR bodies. - docs/roadmap/current.md — new 2026-05-08 banner above the 2026-05-07 banner records the harness landing and the measured band; old banner preserved as the historical record (matches the append-only update discipline already used in this file). - docs/analysis/reviews/performance-optimization-reviews/ 2026-05-07-B1-closure.md — one-line cross-reference appended at the end of the "Post-T-015 amendment" section pointing at the new baseline report. The existing single-run claims throughout the 2026-05-07 baseline are deliberately preserved (the brief explicitly asked for the historical record to stay intact). The 2026-05-08 banner does not promote any task to In Progress / Done — the harness is tooling, not a roadmap task. B2 prep (ADR-0027 drafting) remains the active thread. Refs: P10 (2026-05-06 Track D review), 2026-05-07 multi-axis review §D2
…(B2 — identity-mapped MMU activation) + open T-016 ADR-0027 Proposed; T-016 (Draft) opens with this commit per ADR-0025 §Rule 1. Companion architecture chapter docs/architecture/memory-management.md lands in the same commit (design-first, mirroring T-008's scheduler.md/ipc.md and T-012's exceptions.md precedents). ADR-0027 commits to: - **Layout**: identity-only mapping in B2; kernel in TTBR0_EL1; TTBR1_EL1 reserved (EPD1=1) for the future high-half ADR-0033 placeholder when B5 surfaces per-task TTBR0_EL1 swap. 4 KiB granule, 48-bit VA, 4-level translation. Four bootstrap page-table frames in a new .boot_pt linker section (16 KiB total, statically reserved, pre-zeroed by the BSS-zero loop). - **Memory typing**: MAIR_EL1 indices 0 (device-nGnRnE) and 1 (normal cached, write-back, write-allocate, inner+outer shareable). Indices 2..7 reserved for future memory types (write-combining, normal- uncached for DMA, device-GRE). - **Mutation discipline**: Mmu::map and Mmu::unmap return a `#[must_use]` MapperFlush typed flush token (Rust ecosystem prior art: x86_64::structures::paging::MapperFlush). Caller must explicitly flush(mmu) or ignore() — converts "did you remember to flush?" from reviewer attention to a unused_must_use lint failure (denied workspace-wide). Additive change to ADR-0009 trait surface, recorded in ADR-0009 §Revision notes rider via T-016. §Simulation table (5 rows) walks the SCTLR_EL1.M=1 transition end-to-end — first ADR to apply the write-adr skill §Simulation discipline forward (rather than retro-extracted as for ADR-0026 / ADR-0032). The "Critical step" annotation on row 3 documents the load-bearing failure mode (any error in steps 1–2 produces a Translation Fault on the first instruction fetch after MMU enable). §Dependency chain enumerates 8 sub-steps; T-016 covers all of them (bundled task, mirrors T-012 shape). T-016 implementation moves Draft → In Progress only after ADR-0027 Accept (separate commit per write-adr §10); lands in roughly six independently-bisectable commits. Other artefacts updated: - docs/decisions/README.md: index gains ADR-0027 row (Proposed 2026-05-08). - docs/decisions/0009-mmu-trait.md: §Revision notes rider records the additive MapperFlush return-type change. - docs/decisions/0012-boot-flow-qemu-virt.md: §Open questions "Boot-time MMU activation" entry resolved (linkbacks ADR-0027 + T-016 + memory-management.md). - docs/architecture/hal.md §Mmu: paragraph on the MapperFlush discipline. - docs/glossary.md: new entries for MAIR and MapperFlush; existing MMU entry extended with the B2 identity-only commitment. - docs/roadmap/current.md: 2026-05-08 banner; active milestone + active task updated. - docs/roadmap/phases/phase-b.md: B2 status block + ADR ledger row + sub-breakdown updated. - docs/analysis/tasks/phase-b/README.md: T-016 row added. Verification: cargo fmt clean, host-clippy clean, kernel-clippy clean, host-test 159/159, kernel-build clean. No code changes; doc-only. Refs: ADR-0027, T-016, ADR-0009, ADR-0012 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Careful re-read pass complete (per `write-adr` skill §10):
- All 8 §Dependency-chain steps grounded in T-016 (file exists at
docs/analysis/tasks/phase-b/T-016-mmu-activation.md; opened with the
Propose commit per ADR-0025 §Rule 1).
- The single forward-reference to a future ADR ("ADR-0033 placeholder
— kernel high-half migration") is named-but-unallocated, mirroring
the established slot-naming pattern in phase-b.md's ADR ledger
(ADR-0028 / 0029 / 0030 / 0031 are similarly named without files).
No T-NNN forward-reference is unanchored.
- §Negative consequences are real costs the project is willing to pay:
(1) deferred high-half migration is mitigated by the named-future-
ADR-0033 forward-flag; (2) MappingFlags::USER unreachable in v1
is mitigated by the host-test for the encoder; (3) single MAIR
attribute per memory class is mitigated by reserved indices 2..7;
(4) 2 MiB block descriptors at L2 in the bootstrap is mitigated by
the BSP-internal scope (the trait surface stays 4 KiB);
(5) MapperFlush ergonomic cost IS the discipline.
- §Simulation table arithmetic verified end-to-end: L2_low[64..72] =
8 blocks for GIC (0x0800_0000..0x0900_0000); L2_low[72] = 1 block
for UART (0x0900_0000); L2_high[0..64] = 64 blocks for RAM
(0x4000_0000..0x4800_0000); TLBI + IC IALLU + DSB + ISB + SCTLR.M=1
+ ISB sequence matches Linux's __primary_switch shape.
T-016 implementation moves Draft → In Progress post-merge.
Refs: ADR-0027
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ence consistency) All 4 gemini-code-assist inline comments on PR #20 flagged the same inconsistency: ADR-0027 / memory-management.md / T-016 §Acceptance criteria described mmu_bootstrap as called "between the timer banner and the GIC initialisation", but T-016 §Design notes (the canonical source) correctly explained that the timer banner ALSO uses MMIO (UART) and must therefore move to AFTER mmu_bootstrap. Canonical kernel_entry order (from T-016 §Design notes, now propagated to all 6 sites): cpu.now_ns() snapshot → mmu_bootstrap() → "tyrne: mmu activated" print → GIC init → timer banner → demo Six places updated: - ADR-0027 §Decision outcome / §Dependency-chain step 4 (was: "between the timer banner and the GIC initialisation"; now: explicitly "before any MMIO-touching step (timer banner and GIC initialisation alike)" + the full kernel_entry order on one line). - memory-management.md §Boot-time MMU activation sequence prose (line 140 — same fix). - memory-management.md §Boot-time activation sequenceDiagram (line 158 — Note over K extended to "continue with GIC init, then timer banner, then demo" so the diagram matches the prose). - T-016 §Acceptance criteria / Boot-time MMU activation step 1 (line 51 — same fix). - T-016 §Acceptance criteria / kernel_entry wired (line 56 — describes the wiring in full). - T-016 §Verification gates / smoke trace (line 86 — corrected from "between the timer banner and the IPC demo's first task line" to the actual post-fix trace order with the timer banner moved AFTER mmu_bootstrap). - T-016 §Design notes (line 137 — the canonical heading title was itself misleading: "Why mmu_bootstrap between the timer banner and the GIC initialisation?" → "Why mmu_bootstrap runs before any MMIO-touching step?", with the implementation-note paragraph unchanged). No code changes; doc-only. Verification: cargo fmt clean, host-test 159/159 (no source changes), kernel-build clean. Refs: PR #20 review-round (gemini-code-assist) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ctor) Both gemini-code-assist inline comments on PR #21 flagged efficiency improvements: 1. **L213 sample extraction (4-pipe → single awk).** Replaced the `printf | grep | head | grep | head` pipeline with one awk invocation. Departures from gemini's literal suggestion: - Used sub/sub stripping rather than `print $4` because the kernel's boot-to-end line has the format "tyrne: boot-to-end elapsed = NUM ns" — `$4` is the equals sign; `$5` is the number. The sub/sub pattern is format-shift-tolerant (any "...= NUM ns..." shape resolves to NUM regardless of preceding fields), avoiding the fragility of a positional field index that would break if a future kernel build added a prefix or moved the line. - Confirmed by 5-iteration sanity run: band 3.973 / 4.559 / 5.642 ms p10/p50/p90 (consistent with the existing baseline). 2. **L309 STATS parsing (8 echo|awk → single while-read loop).** Replaced eight `echo "$STATS" | awk '$1=="key" {print $2}'` invocations with one `while read -r key val; do case "$key" in ... esac done <<EOF $(read_stats) EOF` loop. Bash 3.2 compatible (heredoc keeps the loop in the parent shell so variable assignments persist past the loop end). Saves 7 fork+exec per harness invocation; runs once per harness invocation so the absolute saving is small, but the pattern is cleaner and more maintainable. Verification: cargo fmt clean, host-test 159/159 (no source changes), harness 5-iter sanity run produces consistent stats; refactor is behaviour-preserving. Refs: PR #21 review-round (gemini-code-assist) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…on-2026-05-08-b2-prep-bundle
…08-b2-prep-bundle
…-2026-05-08-b2-prep-bundle # Conflicts: # docs/roadmap/current.md
…us block stale Proposed) This is the integration branch's wrap-up commit, landing on top of the three merged branches (#19 path-drift sweep, #21 P10 harness, #20 ADR-0027). Three additions: 1. **2026-05-08 multi-axis pre-merge review artefacts** — 4 track files + 1 consolidated review document. The review ran while the three PRs were open and verified them axis-by-axis (mechanical sweep verification, ADR-0027 design correctness + §Simulation arithmetic, governance + audit-log forward-flag completeness, perf-harness bash + awk + statistics). Verdict: Approve all three; one same-branch fix on #20 (item below); 3 Track-2 Majors flow forward as small ADR-0027 / T-016 riders for a future hygiene PR; zero Blockers. 2. **README.md index row** for the 2026-05-08 review file. 3. **T3-M1 same-branch fix** — `phase-b.md` §B2 status block (line 111) and §Sub-breakdown step 1 (line 115) were still saying `Proposed 2026-05-08` even though the ADR ledger row at line 257 correctly said `Accepted 2026-05-08`. The Accept commit `bb0a6ba` on PR #20's branch flipped only the ledger row; the §B2 status block was missed in that pass. Now consistent: all three sites (status block, sub-breakdown step 1, ADR ledger row) say `Accepted 2026-05-08`. Note: the gh PR comments review-rounds for #20 (4 gemini findings on timer-banner sequence) and #21 (2 gemini findings on perf-harness refactor efficiency) were applied on each PR's own branch (commits 2946b35 on #20; ef30b5c on #21) before this integration commit; both fixes are merged in by the merge commits above. Verification: cargo fmt clean, host-test 159/159, host-clippy clean (-D warnings), kernel-clippy clean, kernel-build clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @cemililik, your pull request is larger than the review limit of 150000 diff characters
ⓘ 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. |
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
📝 WalkthroughWalkthroughThis PR documents the B2 MMU activation design (ADR-0027), introduces the T-016 implementation task specification, adds a performance harness tool for boot timing measurement with baseline results, conducts multi-track code reviews of the design and implementation scaffolding across three PRs, and updates roadmap, glossary, and standards documentation to reflect the new architecture. All changes are documentation and tooling; no kernel source code modifications. ChangesADR-0027 and MMU Architecture
T-016 Implementation Task
Performance Harness & Baseline
Multi-Track Code Reviews (PRs
Code Review Document Updates & Audit Log
Integration Documentation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request establishes the B2 milestone by introducing ADR-0027 for kernel virtual memory layout, a new memory management architecture document, and the T-016 MMU activation task. It also implements a performance benchmarking harness with an initial baseline report and performs a mechanical relative-path sweep across existing documentation. Review feedback points out internal status inconsistencies in the roadmap, suggests clarifying escape hatches for the new flush-token discipline in the ADR, and recommends improving the benchmarking script's robustness with interrupt handling and clearer statistical reporting.
|
|
||
| Turn on the MMU with an identity map for the kernel image region and its stack. This is the foundation that per-task address spaces will layer atop. | ||
|
|
||
| **Status: B2 prep active — ADR-0027 `Accepted` 2026-05-08 (Propose + careful-re-read separate-commit pair); T-016 (MMU activation) `Draft` 2026-05-08; implementation moves to `In Progress` post-merge.** [ADR-0027](../../decisions/0027-kernel-virtual-memory-layout.md) commits to identity-only mapping in B2 (kernel in `TTBR0_EL1`; `TTBR1_EL1` reserved for future high-half ADR-0033 placeholder), MAIR indices 0/1 for device-nGnRnE / normal-cached, four bootstrap page-table frames in `.boot_pt`, and the typed [`MapperFlush`](../../../hal/src/mmu.rs) flush-token discipline at the `Mmu` trait surface (additive change to `map`/`unmap` return types). Companion [`docs/architecture/memory-management.md`](../../architecture/memory-management.md) lands in the same PR. ADR-0027 is the **first ADR drafted under [`write-adr` skill §Simulation](../../../.claude/skills/write-adr/SKILL.md) forward** (rather than retro-extracted as for ADR-0026 / ADR-0032). Accept landed as a separate commit per `write-adr` §10. T-016 implementation lands in roughly six independently-bisectable commits; smoke trace gains a single new line (`tyrne: mmu activated`) and is otherwise byte-stable. |
There was a problem hiding this comment.
There's an internal inconsistency in this document. While the ADR ledger on line 257 correctly shows ADR-0027 as Accepted, this status block still refers to it as Proposed. Please update this line to Accepted to match the ledger and reflect the current state of the ADR.
Comment on lines
+38
to
+40
| Statistics are computed across the valid samples only. Percentile | ||
| convention is *nearest-rank* (1-indexed; `idx = ceil(p/100 * n)`). | ||
| Stddev is the population formula (`n` divisor) — descriptive. |
There was a problem hiding this comment.
The methodology section explains the percentile convention but omits a key detail for small sample sizes. With N=20, the nearest-rank calculation for p99 (ceil(0.99 * 20) = 20) means it will always equal the maximum value. This could be misleading. For better clarity, please add a note to the methodology explaining this statistical artifact, or consider suppressing the p99 value for N < 100.
Comment on lines
+70
to
+71
| - **`MapperFlush` is a `#[must_use]` newtype carrying a `VirtAddr`** that is consumed by either `flush(mmu: &impl Mmu)` (which executes `mmu.invalidate_tlb_address(va)` on the held address) or `ignore()` (a documented no-op for callers performing bulk operations who will issue a single `invalidate_tlb_all` afterwards). Forgetting to handle the token is a `unused_must_use` lint failure — the project's workspace lint config promotes this to a deny in the kernel crate. | ||
| - The surface is **additive** in the [ADR-0017](0017-ipc-primitive-set.md) sense: the existing `Mmu::activate` / `invalidate_tlb_address` / `invalidate_tlb_all` methods stay byte-stable; only `map` / `unmap` return-types grow. ADR-0009 §Revision notes records the additive change. No callers in v1 use these methods yet (B2 is the first consumer); the API breakage cost is zero callers today. |
There was a problem hiding this comment.
The statement that forgetting to handle the MapperFlush token is a compile error is not entirely accurate. The #[must_use] lint can be bypassed using let _ = ..., mem::forget(...), or ManuallyDrop::new(...). The ADR should clarify whether these are considered valid escape hatches (which would align with precedents like x86_64::structures::paging::MapperFlush) or if this represents a known gap in the token discipline. This clarification is important for developers implementing against this ADR.
| - [ADR-0032 — Endpoint state rollback + `ipc_cancel_recv` primitive](../decisions/0032-endpoint-rollback-and-cancel-recv.md) — `Accepted` (2026-05-07). Adds a recovery primitive that reverses an `Idle → RecvWaiting` transition, called by `ipc_recv_and_yield`'s Phase 2 Deadlock branch so both *scheduler* and *endpoint* state restore to pre-call shape on `SchedError::Deadlock`. Kernel-internal in v1 (no userspace caller); future consumers are the userspace-driven endpoint destroy drain (B2+), multi-waiter wake (ADR-0019 §Open questions), and preemption-rollback (B5+). Implemented by T-015 (Done 2026-05-07). Includes a Phase-2 Deadlock simulation table; ADR-0017 §Revision notes rider records the additive recovery primitive (user-observable surface unchanged). The Accept commit is the first project-side application of [`write-adr` skill](../../.claude/skills/write-adr/SKILL.md) step 10's *careful re-read* discipline as a separate diff from the Propose commit. | ||
| - **Next task to open:** **B2 prep — ADR-0027 (kernel virtual memory layout) drafting.** With T-015 closed today, the open follow-on items from the [B1 closure retro's *Adjustments*](../analysis/reviews/business-reviews/2026-05-07-B1-closure.md) reduce to ADR-0027 itself — the δ items (ADR-0023 placeholder file write + endpoint rollback / `ipc_cancel_recv` ADR) landed in PR #16 / PR #17 respectively. Per ADR-0025 §Rule 1, ADR-0027's *Dependency chain* section opens the implementation task (T-016 or the next free slot) in the same commit as the ADR. **Design-first applies:** [`docs/architecture/memory-management.md`](../architecture/memory-management.md) lands alongside or before the implementation, mirroring T-008's `scheduler.md`/`ipc.md` and T-012's `exceptions.md` precedents. ADR-0027 will be the **first ADR drafted under [`write-adr` skill](../../.claude/skills/write-adr/SKILL.md) §Simulation discipline** (codified after ADR-0026's caught-by-table experience and the [B1 closure retro](../analysis/reviews/business-reviews/2026-05-07-B1-closure.md) Adjustment): the Decision outcome must include a 3–5 row state-machine table walking the worst-case interaction (page-table walk / TTBR switch / mapping handshake) before the Accept can flip. | ||
| - **Next review trigger:** **B2 closure trio.** Produced when the MMU activation arc reaches `In Review` (the implementation task lands). Possible interim triggers: a mini-retro if ADR-0027's drafting surfaces a learning worth capturing mid-arc; a maintainer-initiated review if a non-trivial Track-C/D follow-up gets picked up before B2 implementation starts. Audit-log entries UNSAFE-2026-0019 / 0020 / 0021 retain their `Pending QEMU smoke verification` status notes — full clearance gates on a future task that arms a real `arm_deadline` (B5+ `time_sleep_until` syscall is the natural venue). | ||
| - [ADR-0027 — Kernel virtual memory layout (B2 — identity-mapped MMU activation)](../decisions/0027-kernel-virtual-memory-layout.md) — **`Accepted` (2026-05-08)**. B2 commits to identity-only mapping (kernel in `TTBR0_EL1`; `TTBR1_EL1` reserved with `EPD1=1` for future high-half ADR-0033 placeholder when B5 surfaces per-task `TTBR0_EL1` swap), 4 KiB granule + 48-bit VA + 4-level translation, MAIR indices 0/1 for device-nGnRnE / normal-cached, four bootstrap page-table frames in a new `.boot_pt` section, and a typed [`MapperFlush`](../../hal/src/mmu.rs) flush-token discipline at the `Mmu` trait surface (additive change to `map`/`unmap` return types, recorded in ADR-0009 §Revision notes rider via T-016). Includes a five-row Phase-2 §Simulation table walking the SCTLR.M=1 transition. **First ADR to apply [`write-adr` skill §Simulation](../../.claude/skills/write-adr/SKILL.md) forward** (rather than retro-extracted as for ADR-0026 / ADR-0032). Implementation: T-016 (Draft, opens with the Propose commit). Accept will be a separate commit per `write-adr` §10. |
There was a problem hiding this comment.
This line has a couple of minor inaccuracies that could cause confusion:
- It refers to a "Phase-2 §Simulation table". The term "Phase-2" is an artifact from ADR-0032 and doesn't apply to ADR-0027's simulation. Please remove it for clarity.
- It states that "Accept will be a separate commit..." in the future tense, but the
Acceptedstatus at the start of the line indicates this has already occurred. Please update the tense to reflect this.
Comment on lines
+111
to
+115
| ### Reporting discipline | ||
|
|
||
| - **Cite the band, not a single sample.** When a PR's commentary needs a boot-to-end figure, run the harness and quote the `p10 / p50 / p90` triple plus the iteration count. A solitary `boot-to-end elapsed = X ns` from a single QEMU launch is not a load-bearing measurement; QEMU TCG's translation-cache behaviour gives ~15-30 % run-to-run variance and a single sample can fall anywhere in the band. | ||
| - **Single-run anecdotes from before this harness landed are preserved as historical record.** The 2026-04-21 / 2026-04-28 / 2026-05-07 perf reviews quote single-run figures; those numbers are not retroactively replaced — but every *new* perf claim cites a harness band. | ||
| - **Baseline reports under `docs/analysis/reports/perf-baseline-*.md`** are append-only artefacts. Re-baselines after a perf-relevant change land as fresh reports with a new context slug; old reports stay in place as the historical record. |
There was a problem hiding this comment.
The reporting discipline section is a good addition. To make it more complete, consider adding a brief note about future plans for automated regression detection, such as a --baseline=<file> mode for the performance harness. This would provide readers with better context on the tool's intended evolution.
| # Exits 0 on success (>= 50 % valid runs), 1 on environmental failure, | ||
| # 2 on argument errors. | ||
|
|
||
| set -euo pipefail |
There was a problem hiding this comment.
The script isn't robust against interruptions (e.g., via Ctrl-C), which could leave orphan QEMU and watchdog processes running. To prevent this, please add a trap at the beginning of the script to ensure that child processes are cleaned up properly on exit. This will likely require managing the child PIDs in global variables that the trap handler can access.
…losure status Re-verified all 13 §Follow-up backlog items from the 2026-05-07 PR #12-#17 multi-axis review against the current integration-branch state. All 9 hygiene items + the 1 forward-flagged P10 harness item are now closed; 3 remain forward-flagged on appropriate downstream venues. Two updates to the consolidated review file: 1. **Top-of-file closure-status banner** — readers see the per-item disposition at-a-glance without scrolling to the bottom backlog. 2. **§Follow-up backlog per-item closure annotations** — items 1-9 gain ✅ + closing-PR + closing-commit references; item 11 (P10 harness) gains ✅ + integration-PR reference + measured-baseline numbers; items 10/12/13 keep their forward-flagged status with "status unchanged 2026-05-08" markers. Re-verification at integration-branch HEAD (per item): | # | Item | Closing PR / commit | Verification | |---|---|---|---| | 1 | current.md + perf re-baseline `.text 22,020` | PR #18 / `94a6c0f` | grep "22,020 bytes" current.md → 1 hit | | 2 | cancel_recv_on_recv_complete test | PR #18 / `25854a1` | grep test name in ipc/mod.rs → 1 hit; host-test 159/159 | | 3 | ipc_cancel_recv doc-rider on cap-bearing state | PR #18 / `25854a1` | grep "destroy-drain callers (Phase B2+)" → 1 hit | | 4 | cancel-block SAFETY wording | PR #18 / `25854a1` | grep "caller_table.*shared.*reborrow" → 1 hit | | 5 | UNSAFE-2026-0014 SHA back-fill (c30f4ee, 7a402cb) | PR #18 / `94a6c0f` | grep both SHAs in unsafe-log.md → 2 hits each | | 6 | unsafe-policy.md §3 mechanical-edit exemption | PR #18 / `94a6c0f` | grep "Mechanical-edit exemption" → 1 hit | | 7 | ADR-0026 §Simulation chronology rider | PR #18 / `94a6c0f` | grep "§Simulation rule was retro-extracted" → 1 hit | | 8 | master-plan AC cross-reference | PR #18 / `94a6c0f` | grep "Closure-trio coordination cross-reference" in security + perf master-plans → 1 hit each | | 9 | ADR-0026 §skill-clause reconciliation rider | PR #18 / `94a6c0f` | grep "single-commit Propose+Accept landing reconciliation" → 1 hit | | 11 | P10 wall-clock harness | this integration PR (replaces #19/#20/#21) | tools/perf-harness.sh exists; baseline report exists; band p10=3.884/p50=4.642/p90=5.584 ms | Forward-flagged (status unchanged): - Item 10: RecvWaiting waiter-identity gap — ADR-0030 / ADR-0019 venue - Item 12: cancel-on-cap-bearing-state destroy-drain ADR — first userspace-destroy task venue - Item 13: B5+ preemption-rollback re-validation of ADR-0032 — B5+ preemption ADR venue This commit only touches the consolidated review's annotation; track files preserved as historical artefacts (their per-track verdicts are the snapshot at the moment of the review, not subject to back-edits). The review's per-item findings (Track-A NIT-2 SchedQueue::new doc rename; Track-G MIN-G1/G2/G3; Track-H MIN-1/MIN-2; Track-A MIN-2 ipc_cancel_recv doc-rider; Track-D D1; Track-F §F-1) are all closed in PR #18 + this integration PR. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… 2026-05-08 + 2026-05-07 follow-ups Closes the 12 remaining items flagged by the 2026-05-08 multi-axis review's §Follow-up backlog (3 Track-2 Majors, 1 Track-3 Major fix already closed in `59c08e9`, 6 Track-3 / Track-2 Minors, 2 Track-4 Minors, 2 Track-2 Nits) plus the carry-forward 2026-05-07 Track-H NIT-1. **Track 2 Majors (forward-flagged → ADR-0027 / phase-b ledger riders):** - M1 (escape-hatch doc): ADR-0027 §Decision outcome (c) gains a bullet documenting `mem::forget` / `ManuallyDrop` / `let _ = ...` as deliberate-but-rare escape hatches, mirroring the `x86_64::structures::paging::MapperFlush` precedent. - M2 (MMU-instance binding): ADR-0027 §Decision outcome (c) gains a bullet noting `MapperFlush::flush(self, mmu: &impl Mmu)` accepts any `Mmu`; multi-`Mmu` deployments (B3+ per-task `AddressSpace`, Phase C multi-CPU) will need a stronger token type. Out of scope for v1. - M3 (ADR-0034 placeholder): ADR-0027 §Decision outcome adds an "ADR-0034 (kernel-image section permissions) placeholder" block alongside ADR-0033, and `phase-b.md` ADR ledger gains rows for both ADR-0033 and ADR-0034 with named-but-unallocated discipline. **Track 3 Minors (governance / wording polish):** - m1 + m2 (current.md L52): drop "Phase-2" prefix on "§Simulation table" (the table walks Steps 0–4, not a "Phase 2"); "Accept will be" → "Accept landed as" + actual commit SHA `bb0a6ba`. - m3 (commit-style.md PR-numbering rider): new §"PR-number references in committed artefacts" subsection naming the recurrence (PR #18 + PR #20 each had a one-commit PR-number fix-up) and codifying three acceptable disciplines (defer banner authoring; reference branch slug; or use commit SHA). - n1 (framing alignment): "first to apply Simulation forward" wording in current.md (banner + Active decisions row) and phase-b.md §B2 status block aligned to the precise "first non-recovery-primitive state-machine ADR drafted under §Simulation" phrasing — ADR-0032's Propose did land with a table; the prior framing was technically defensible only under a narrow reading of "retro-extracted". **Track 2 Nits (substance riders in ADR-0027):** - #4 (DSB ISH vs DSB NSH rationale): §Simulation gains a rationale paragraph after the table — `ISH` is forward-compatible with the eventual SMP boot, sub-microsecond cost on single-core, matches Linux aarch64 `arch/arm64/mm/proc.S` for the same reason. - #5 (TCR_EL1.AS wording tighten): line 59 reworded — `AS = 0` selects 8-bit ASID *size*, not "the ASID value is 0" (the value is `TTBR0_EL1.ASID = 0` and is what's "globally used in v1"). - #7 (line 17 §-citation precision) + Track-3 n1: "first non-recovery-primitive state-machine" framing now precise. - #8 (memory-management.md L88 page-table descriptor cosmetic): ASCII bit-field diagram redrawn to match L2 block-descriptor reality (OutputAddress[47:21], not [47:12]); explanatory note added for L1-block / L3-page variants. **Track 4 Minors (perf-harness.sh):** - #1 (Ctrl-C cleanup trap): new `cleanup_in_flight` shell function + `trap '...' EXIT INT TERM` that kills any in-flight QEMU + watchdog PIDs tracked in `CURRENT_CMD_PID` / `CURRENT_WATCHDOG_PID` shell globals. `run_with_timeout` updates the globals at every call so the trap addresses whichever pair is currently in flight; clears them at every clean exit so the trap is a no-op outside iterations. - #2 (p99 small-N reporting hygiene): generated baseline report Methodology section gains a "**Note on p99 at small `n`**" paragraph explaining that under nearest-rank `p99 == max` for `n < 100` and callers should not over-read it as a tail-latency signal until `n >= 100`. **Track 4 Nit #3 (read_stats refactor):** **Already closed by PR #21 review-round commit `ef30b5c`** — the 8 `echo | awk` parses became a single `while read` loop. Verified at HEAD (`grep -c "while read -r key val" tools/perf-harness.sh` → 1 hit). **2026-05-07 Track-H NIT-1 (Pending Amendment closure-path indexing):** UNSAFE-2026-0019 / 0020 / 0021 each gain a 2026-05-08 "closure-path indexed" Amendment naming the canonical clearance trigger (B5 Milestone, ADR-0030 entry-point, deadline-arming syscall) explicitly, so a future reader of `unsafe-log.md` alone has the full picture without leaving the file. No semantic change; co-locates information that was previously distributed across `phase-b.md` cross-references. Verification gates re-run on the integration branch: - `cargo fmt --all -- --check` clean - `cargo host-test` 159/159 (25 + 100 + 34) - `cargo host-clippy` clean (-D warnings) - `cargo kernel-clippy` clean - `cargo kernel-build` clean - `tools/perf-harness.sh --iterations=3` runs end-to-end with the new trap + Methodology note + while-read parsing intact This commit + the prior 2026-05-08 review's recommendations close all follow-ups identified by both 2026-05-07 and 2026-05-08 multi-axis reviews. Forward-flagged items (10/12/13 from 2026-05-07) and any review-round bot input on this integration branch remain the only open items. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Closes the orchestrator review's findings on PR #22 (16 actionable items; one item — PR #19 description "180/180" wording — not closeable post-merge because the gh PR is already closed and its description is immutable from this branch). **3 Majors (fix-on-branch):** - M1 path-drift in 2026-05-08 consolidated review L124: `(../../decisions/...)` → `(../../../decisions/...)`. The 4-deep file needs 3 `..` to reach `docs/decisions/`. Other relative paths in the same file already use the correct depth; this was the lone broken outlier. - M2 `commit-style.md:130` anchor: the new "PR-number references" rider linked to `track-g-process.md#min-g3` for "PR-numbering fix-up precedent", but MIN-G3 is about closure-trio smoke-trace AC asymmetry, not PR-numbering. Dropped the wrong anchor; the link now points at the 2026-05-08 review's §"Cross-PR observations" §1 ("PR-numbering hygiene drift recurrence"), which IS the canonical source of the rider. - M3 158→159 narrative drift in `current.md:15` and `phase-b.md:103`: added "Current state (post-PR-#18 hygiene): 159/159" addendum that preserves the historical PR-#17-merge-moment claim of 158/158 (factually accurate at the time) while disambiguating from the current 159 baseline that PR #18's `25826a1` brought us to. **7 Minors (substance + governance polish):** - `hal.md` §Mmu present-tense overclaim: `MapperFlush` extension is T-016 deliverable, not current `main`; added a "tense hedge" blockquote making the post-T-016 framing explicit and acknowledging that the current trait surface is still pre-`MapperFlush`. - ADR-0027 §(a) MMIO range mismatch: `0x0902_0000` (the UART's actual end-of-MMIO) vs `0x0920_0000` (the 2 MiB-block-aligned end of the bootstrap's identity-map for that block). Reworded to explicitly cover the block-aligned superset; clarified the unmapped slack is harmless under device-nGnRnE. - ADR-0027 §(a) `TCR_EL1.A1` forward-flag: `A1=0` documented (selects which TTBR holds the ASID; v1 keeps `A1=0` because only `TTBR0_EL1` is active); the future high-half ADR (ADR-0033) gets named as the decider for `A1=1` migration. - `track-3-pr-20-governance.md:31` quoted-excerpt path: the embedded `(../../.claude/skills/write-adr/SKILL.md)` link in a quoted ADR-0027 passage was broken from track-3's depth (5-deep). Restructured the quote into a `> blockquote` with link targets stripped; preserves quote fidelity while avoiding broken-link rendering. - Glossary: added entries for `mmu_bootstrap` and `.boot_pt` — identifiers introduced by ADR-0027 / T-016 that future readers will hit and want a 1-line definition for. - `tools/perf-harness.sh` `export LC_ALL=C` at script entry: forces period-decimal + ASCII-digit awk output regardless of host locale. Without this, `LC_ALL=tr_TR.UTF-8` would emit `5,169` (decimal comma) in baseline reports — valid in tr_TR, mis-parsed in en_US. - 2026-05-08 consolidated review §Follow-up backlog: each of the 15 items now carries an inline ✅ + closing-commit-SHA annotation, matching the [2026-05-07 review's `8b6147d`-style closure-status pattern](docs/analysis/reviews/code-reviews/ 2026-05-07-pr-12-to-17-multi-axis-review.md). Item 8 (PR #19 description "180/180" wording) annotated as "not closeable post-merge" with rationale. **5 Nits (refinements; one was already closed):** - ADR-0027 §Simulation Step 1 row: collapsed the awkward `L2_low[64..72]` + `L2_low[72]` notation into the cleaner half-open `L2_low[64..73]` (= 9 indices = 9 entries) and added a footnote clarifying the half-open Rust convention used throughout the ADR. memory-management.md mermaid diagram updated similarly. - ADR-0027 §References framing: tightened "first ADR drafted under §Simulation; this is the second" to "first ADR drafted under the §Simulation rule (recovery-primitive subject); this ADR is the first non-recovery-primitive state-machine ADR drafted under the same rule" — matches the §Context para 2 framing precisely. - `tools/perf-harness.sh:223` (`QEMU_VERSION` extraction): replaced `head -n 1` with `awk 'NR==1 { print; exit }'` to avoid the `head` SIGPIPE that `set -o pipefail` would propagate. Defensive hardening; observable behaviour unchanged. - `tools/perf-harness.sh:209` watchdog kill: added `kill -0` liveness guard before `kill -KILL`. Before: kill on a dead PID returned ESRCH which `2>/dev/null || true` swallowed silently. After: explicit aliveness check makes the intent self-documenting. - `tools/perf-harness.sh:54-62` trap function: added a one-line comment documenting the idempotency property — both globals empty → no-op; stale PIDs → ESRCH suppressed → safe to fire multiple times. - (Already closed) `unsafe-log.md` 2026-05-08 closure-path Amendments cite Track-H NIT-1: verified — all 3 Amendments (UNSAFE-2026-0019/0020/0021) already include the trace-back link to the prior review's NIT-1. Verification: cargo fmt clean, cargo host-test 159/159, host-clippy clean (-D warnings), kernel-clippy clean, kernel-build clean, perf-harness 3-iter sanity run produces consistent stats with `LC_ALL=C` decimal-period output. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/roadmap/current.md (1)
48-48:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReplace non-English token with English-only phrasing.
Line 48 includes
Yüksek, which violates the repository’s English-only documentation rule. Rephrase without non-English text.Proposed edit
-| **#14** [`1cd810d9`](https://github.com/cemililik/Tyrne/pull/14) | Repo-wide `TyrneOS → Tyrne` URL rename (64 URLs) + `Yüksek → High` localization sweep (5 docs); review-round caught `cd TyrneOS` orphan + `TyrneOS repository` typo in SECURITY.md (both fixed in PR). | **Minor (H):** UNSAFE-2026-0016's body was edited in-place by the localization sweep, technically violating the introducing-commit-boundary discipline — fix is a small Amendment or a `unsafe-policy.md §3` exemption for mechanical localization. (Track B also notes the brief mis-attributed the `tools/run-qemu.sh` Bash 3.2 fix to PR `#14` — actual introducing commit is `0f0c97c` on PR `#12`'s branch.) | +| **#14** [`1cd810d9`](https://github.com/cemililik/Tyrne/pull/14) | Repo-wide `TyrneOS → Tyrne` URL rename (64 URLs) + non-English-term cleanup to `High` (5 docs); review-round caught `cd TyrneOS` orphan + `TyrneOS repository` typo in SECURITY.md (both fixed in PR). | **Minor (H):** UNSAFE-2026-0016's body was edited in-place by the localization sweep, technically violating the introducing-commit-boundary discipline — fix is a small Amendment or a `unsafe-policy.md §3` exemption for mechanical localization. (Track B also notes the brief mis-attributed the `tools/run-qemu.sh` Bash 3.2 fix to PR `#14` — actual introducing commit is `0f0c97c` on PR `#12`'s branch.) |As per coding guidelines, "
**/*: Source code, comments, doc-comments, documentation, commit messages, PR descriptions, and issue text must be in English."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/roadmap/current.md` at line 48, The document contains a non-English token "Yüksek" on the ADR-0024 line which violates the repo's English-only rule; replace that token with an English equivalent (e.g., "High" or a context-appropriate English adjective) wherever "Yüksek" appears in the ADR-0024 — EL drop to EL1 policy entry so the sentence remains grammatically correct and preserves meaning; update the surrounding phrasing if needed to keep tense/format consistent with other roadmap entries (refer to the ADR-0024 entry text to match style).docs/architecture/hal.md (1)
273-273:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix the
unsafe-log.mdhyperlink targetAt Line 273, the link text names
docs/audits/unsafe-log.mdbut the href points to../audits/(directory). Please link directly to the file to avoid stale/broken navigation.Suggested patch
-- and security-review approval on the commit that introduces it. -+ and security-review approval on the commit that introduces it. ... -- and security-review approval on the commit that introduces it. -+ and security-review approval on the commit that introduces it.-- and security-review approval on the commit that introduces it. -+ and security-review approval on the commit that introduces it.-- and security-review approval on the commit that introduces it. -+ and security-review approval on the commit that introduces it.-- and security-review approval on the commit that introduces it. -+ and security-review approval on the commit that introduces it.-- and security-review approval on the commit that introduces it. -+ and security-review approval on the commit that introduces it.-The [unsafe-policy.md](../standards/unsafe-policy.md) applies in full: each `unsafe` block has a `SAFETY:` comment, an entry in [`docs/audits/unsafe-log.md`](../audits/) (created with the first `unsafe` block that lands), and security-review approval on the commit that introduces it. +The [unsafe-policy.md](../standards/unsafe-policy.md) applies in full: each `unsafe` block has a `SAFETY:` comment, an entry in [`docs/audits/unsafe-log.md`](../audits/unsafe-log.md) (created with the first `unsafe` block that lands), and security-review approval on the commit that introduces it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/architecture/hal.md` at line 273, The hyperlink in docs/architecture/hal.md currently points to the audits directory instead of the specific file; update the href so the link targets ../audits/unsafe-log.md (i.e., change the reference that currently reads ../audits/ to ../audits/unsafe-log.md) so the text "docs/audits/unsafe-log.md" links directly to the file; locate the sentence that references unsafe-policy.md/unsafe-log and replace the directory link with the explicit file path.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/analysis/reports/perf-baseline-2026-05-08-post-pr-19-pre-adr-0027.md`:
- Around line 59-80: The fenced code block that contains the raw numeric sample
(the triple-backtick block immediately preceding the list of numbers) needs a
language tag to satisfy markdownlint MD040; update the opening fence from ``` to
```text (or another appropriate language like ```txt) so the block reads as a
plaintext code fence and the linter warning is resolved.
In `@docs/analysis/reviews/code-reviews/2026-05-06-full-tree/track-j-hygiene.md`:
- Line 69: The current wording incorrectly suggests that #![deny(clippy::todo)]
forbids TODO/FIXME/HACK comments; update the two occurrences that mention this
(the lines referencing the kernel crate's deny attribute) to split
responsibilities: state that the kernel enforces macro usage hygiene via
#![deny(clippy::todo)] (which catches todo!() macro calls) and separately credit
the absence of TODO/FIXME/HACK comments to the repository grep/comment scan;
ensure both references explicitly mention the macro vs. comment distinction and
keep the original positive tone about lint enforcement and grep-based comment
hygiene.
In
`@docs/analysis/reviews/code-reviews/2026-05-08-pr-19-20-21-multi-axis-review/track-4-pr-21-perf-harness.md`:
- Line 137: The note in the filename/title table wrongly claims the --report
validation at line 101 “forbids” inputs like `2026-05-08-2026-05-09-foo`; update
that sentence to accurately state that the current regex/validation used by
--report (referenced at line 101) does allow `2026-05-08-2026-05-09-foo` (or
rephrase to say it “does not forbid” such inputs), and adjust the explanatory
text for the example row (`2026-05-08-2026-05-09-foo` →
`perf-baseline-2026-05-08-2026-05-09-foo.md`) so it no longer asserts the
incorrect restriction.
In `@docs/architecture/memory-management.md`:
- Line 205: The bracketed text "ADR-0033 placeholder — Kernel high-half
migration" is not a valid Markdown link; update that token to a proper link or
reference-style link so it renders and resolves correctly. Replace "[ADR-0033
placeholder — Kernel high-half migration]" with either an inline link like
"[ADR-0033 — Kernel high-half migration](<URL-or-relative-path-to-ADR-0033>)" or
a reference-style link such as "[ADR-0033 — Kernel high-half
migration][ADR-0033]" and add a matching reference definition ("[ADR-0033]:
<URL-or-relative-path-to-ADR-0033>") elsewhere in the document; ensure the
visible text remains "ADR-0033 — Kernel high-half migration".
In `@docs/decisions/0027-kernel-virtual-memory-layout.md`:
- Around line 92-93: The explanatory paragraph between Step 3 and Step 4 is
breaking the Simulation table; keep the Step 4 row ("MMU on; PC at
identity-mapped PA; caches on; bootstrap mappings live" / "mmu_bootstrap returns
to `kernel_entry`'s caller; rest of kernel proceeds with MMU active") inside the
table and move the intervening explanatory paragraph (the note about
cache/device semantics and MapperFlush) out from between the rows and place it
below the entire table as a standalone paragraph. Ensure the table markup is
contiguous (no stray paragraph nodes) so Step 4 renders as a table row and the
explanatory note appears after the table.
In `@tools/perf-harness.sh`:
- Around line 75-76: The trap currently uses a single handler (trap
'cleanup_in_flight' EXIT INT TERM) so INT/TERM don't terminate the script;
update traps to call cleanup_in_flight and then exit with explicit codes for
signal handlers: keep EXIT triggering cleanup_in_flight only, and add separate
traps for INT and TERM that call cleanup_in_flight followed by exit 130 (for
INT) and exit 143 (for TERM) so the harness stops immediately on Ctrl-C or TERM
while still running cleanup on normal exit.
---
Outside diff comments:
In `@docs/architecture/hal.md`:
- Line 273: The hyperlink in docs/architecture/hal.md currently points to the
audits directory instead of the specific file; update the href so the link
targets ../audits/unsafe-log.md (i.e., change the reference that currently reads
../audits/ to ../audits/unsafe-log.md) so the text "docs/audits/unsafe-log.md"
links directly to the file; locate the sentence that references
unsafe-policy.md/unsafe-log and replace the directory link with the explicit
file path.
In `@docs/roadmap/current.md`:
- Line 48: The document contains a non-English token "Yüksek" on the ADR-0024
line which violates the repo's English-only rule; replace that token with an
English equivalent (e.g., "High" or a context-appropriate English adjective)
wherever "Yüksek" appears in the ADR-0024 — EL drop to EL1 policy entry so the
sentence remains grammatically correct and preserves meaning; update the
surrounding phrasing if needed to keep tense/format consistent with other
roadmap entries (refer to the ADR-0024 entry text to match style).
🪄 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: 279a758e-273c-4d1f-bcfa-a8b32a983b2f
📒 Files selected for processing (31)
docs/analysis/reports/perf-baseline-2026-05-08-post-pr-19-pre-adr-0027.mddocs/analysis/reviews/code-reviews/2026-05-06-full-tree/track-a-kernel.mddocs/analysis/reviews/code-reviews/2026-05-06-full-tree/track-b-hal.mddocs/analysis/reviews/code-reviews/2026-05-06-full-tree/track-d-performance.mddocs/analysis/reviews/code-reviews/2026-05-06-full-tree/track-e-docs.mddocs/analysis/reviews/code-reviews/2026-05-06-full-tree/track-g-bsp.mddocs/analysis/reviews/code-reviews/2026-05-06-full-tree/track-h-infra.mddocs/analysis/reviews/code-reviews/2026-05-06-full-tree/track-j-hygiene.mddocs/analysis/reviews/code-reviews/2026-05-07-pr-12-to-17-multi-axis-review.mddocs/analysis/reviews/code-reviews/2026-05-08-pr-19-20-21-multi-axis-review.mddocs/analysis/reviews/code-reviews/2026-05-08-pr-19-20-21-multi-axis-review/track-1-pr-19-mechanical.mddocs/analysis/reviews/code-reviews/2026-05-08-pr-19-20-21-multi-axis-review/track-2-pr-20-design.mddocs/analysis/reviews/code-reviews/2026-05-08-pr-19-20-21-multi-axis-review/track-3-pr-20-governance.mddocs/analysis/reviews/code-reviews/2026-05-08-pr-19-20-21-multi-axis-review/track-4-pr-21-perf-harness.mddocs/analysis/reviews/code-reviews/README.mddocs/analysis/reviews/performance-optimization-reviews/2026-05-07-B1-closure.mddocs/analysis/tasks/phase-b/README.mddocs/analysis/tasks/phase-b/T-016-mmu-activation.mddocs/architecture/hal.mddocs/architecture/memory-management.mddocs/audits/unsafe-log.mddocs/decisions/0009-mmu-trait.mddocs/decisions/0012-boot-flow-qemu-virt.mddocs/decisions/0027-kernel-virtual-memory-layout.mddocs/decisions/README.mddocs/glossary.mddocs/roadmap/current.mddocs/roadmap/phases/phase-b.mddocs/standards/commit-style.mddocs/standards/infrastructure.mdtools/perf-harness.sh
| - [ADR-0012 — Boot flow and memory layout for `bsp-qemu-virt`](../decisions/0012-boot-flow-qemu-virt.md) — the static image layout this doc inherits. | ||
| - [ADR-0024 — EL drop to EL1 policy](../decisions/0024-el-drop-policy.md) — kernel runs at EL1 when the MMU activates. | ||
| - [ADR-0027 — Kernel virtual memory layout (B2 — identity-mapped MMU activation)](../decisions/0027-kernel-virtual-memory-layout.md) — the load-bearing decision document for this chapter. | ||
| - [ADR-0033 placeholder — Kernel high-half migration] — opens when B5 surfaces the per-task `TTBR0_EL1` swap requirement. |
There was a problem hiding this comment.
Convert the ADR-0033 placeholder into a valid link/reference format
At Line 205, [ADR-0033 placeholder — Kernel high-half migration] is not a valid Markdown link (missing target), so it renders as bracketed text and breaks reference consistency.
Suggested patch
-- [ADR-0033 placeholder — Kernel high-half migration] — opens when B5 surfaces the per-task `TTBR0_EL1` swap requirement.
+- ADR-0033 placeholder — Kernel high-half migration (opens when B5 surfaces the per-task `TTBR0_EL1` swap requirement).📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - [ADR-0033 placeholder — Kernel high-half migration] — opens when B5 surfaces the per-task `TTBR0_EL1` swap requirement. | |
| - ADR-0033 placeholder — Kernel high-half migration (opens when B5 surfaces the per-task `TTBR0_EL1` swap requirement). |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/architecture/memory-management.md` at line 205, The bracketed text
"ADR-0033 placeholder — Kernel high-half migration" is not a valid Markdown
link; update that token to a proper link or reference-style link so it renders
and resolves correctly. Replace "[ADR-0033 placeholder — Kernel high-half
migration]" with either an inline link like "[ADR-0033 — Kernel high-half
migration](<URL-or-relative-path-to-ADR-0033>)" or a reference-style link such
as "[ADR-0033 — Kernel high-half migration][ADR-0033]" and add a matching
reference definition ("[ADR-0033]: <URL-or-relative-path-to-ADR-0033>")
elsewhere in the document; ensure the visible text remains "ADR-0033 — Kernel
high-half migration".
… banner clarify "180/180" The 2026-05-08 review's §Follow-up backlog item 8 was annotated as "not closeable post-merge" because the original PR #19's gh description is immutable on a closed PR. But the substance of the recommendation — clarify the "180/180" metric phrasing — IS closeable via the integration PR #22's description (which is open and editable) plus the roadmap banner that mirrors the same fact in the source tree. Three updates: 1. **Integration PR #22 description** (via `gh pr edit 22`): the "branches integrated" table row for PR #19 changed from "Mechanical sed sweep of 180 broken relative-path links" to "Mechanical sed sweep of broken relative-path links … (~180 link *instances* across 7 affected files; the 2026-05-08 multi-axis review re-counted at 193 instances over 83 unique targets — substance is the same: every broken link resolves post-sweep)". 2. **`docs/roadmap/current.md` 2026-05-08 banner** item 3: same precise-metric phrasing applied. The `180` headline is preserved for continuity with PR #19's commit-style wording, but the parenthetical clarifies the instances-vs-targets ambiguity. 3. **2026-05-08 review §Follow-up backlog item 8**: status flipped from ⏳ "not closeable post-merge" to ✅ "closed by editing the integration PR #22 description plus a parallel addendum in current.md banner item 3", with a note that the original PR #19 gh description remains as-was (closed PR is immutable from this branch) but every reachable downstream artefact now states the precise metric. This is the last open item from the 2026-05-08 multi-axis review — all 16 actionable items now closed in this branch (15 in prior commits, 1 here). Verification: cargo fmt clean. No source-code changes; doc-only. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Verified each inline finding from gemini-code-assist + coderabbitai
against current code. Applied seven, skipped one with reason.
Applied:
1. perf-baseline-2026-05-08-post-pr-19-pre-adr-0027.md raw-samples
fence: untagged ``` → ```text (markdownlint MD040).
2. track-j-hygiene.md L69 + L142: split `#![deny(clippy::todo)]`
*macro* hygiene from `TODO`/`FIXME`/`HACK` *comment* hygiene.
The lint catches the `todo!()` macro only; comment hygiene is
author discipline verified by recursive grep.
3. track-4-pr-21-perf-harness.md L137: corrected the dedup-edge-
case row's claim that `--report` regex `[A-Za-z0-9._-]`
"forbids" `2026-05-08-2026-05-09-foo`. Digits + hyphens both
match the class; strip-one-date is the documented behaviour for
this shape, not a validation gate.
4. memory-management.md §"Future work" + §References:
`[ADR-0033 placeholder — Kernel high-half migration]` rendered as
plain bracketed text (no link target). Re-pointed at ADR-0027
§Decision outcome where the ADR-0033 / ADR-0034 placeholder slots
are described.
5. ADR-0027 Simulation table: explanatory paragraph
"Why DSB ISH rather than DSB NSH" sat between Step 3 and Step 4
table rows, terminating the table early in markdown renderers.
Moved the paragraph below the entire table.
6. tools/perf-harness.sh trap discipline:
trap 'cleanup_in_flight' EXIT INT TERM
ran cleanup but did not terminate the iteration loop on
Ctrl-C / kill — the loop continued to the next iteration.
Split into three traps:
trap 'cleanup_in_flight' EXIT
trap 'cleanup_in_flight; exit 130' INT
trap 'cleanup_in_flight; exit 143' TERM
so EXIT remains idempotent, INT exits with the conventional
128 + SIGINT(2) = 130, TERM exits with 128 + SIGTERM(15) = 143.
7. hal.md L273 audit-log reference: link target was the directory
`../audits/` — corrected to the file `../audits/unsafe-log.md`
so the surrounding "logged in `docs/audits/unsafe-log.md`" prose
actually links to the named file.
Bonus (gemini infrastructure.md:115 future-regression-detection
note): added a one-bullet forward-looking note that regression
detection is currently manual eyeballing, with a sketched future
`--baseline=<file>` mode comparing new `p50` / `p90` against a
prior baseline within a configured tolerance. Out of scope for v1
until enough baselines exist to calibrate the threshold.
Skipped:
8. current.md L48 "Yüksek" → English. Verified false: line 48 is
the ADR-0024 entry, no Yüksek token. Comprehensive grep across
roadmap/standards/decisions/architecture confirms Yüksek
appears only in (a) `unsafe-policy.md` mechanical-edit
exemption as a *quoted example* of legitimate verbatim mirror,
(b) historical review docs describing the past PR #14
localization sweep — both legitimate quotes — and
(c) `docs/analysis/technical-analysis/` files which are
gitignored per `.gitignore` "Local-only technical analyses".
No committed-artefact violation of the English-only rule.
Gates:
- `cargo fmt --all -- --check` clean
- `cargo test -p tyrne-{kernel,hal,test-hal}` 34/34 pass
- `cargo clippy -p tyrne-{kernel,hal,test-hal} --all-targets -- -D warnings` clean
- `cargo build --target aarch64-unknown-none -p tyrne-bsp-qemu-virt` clean
- `bash -n tools/perf-harness.sh` clean
- `tools/perf-harness.sh --help` renders
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Single integration PR for the B2 milestone-opening bundle. Replaces 3 originally-opened PRs (#19, #20, #21 — all closed in favour of this one) so review happens from one place.
The bundle is the entire B2-prep arc: prior-review hygiene cleanup (#19), measurement infrastructure (#21, load-bearing for T-016 regression detection), and the load-bearing design + task open (#20). All three branches are merged in their natural order (#19 → #21 → #20) so the P10 baseline report's filename label (
perf-baseline-2026-05-08-post-pr-19-pre-adr-0027.md) remains literally true. Then the integration commit lands the 2026-05-08 multi-axis pre-merge review artefacts + closes T3-M1 (phase-b.md §B2 status-block staleProposed).Branches integrated
doc-hygiene-2026-05-06-path-drift-sweepp10-wall-clock-bench-harnesstools/perf-harness.sh+ first measured baseline (p10=3.884 / p50=4.642 / p90=5.584 ms over 20 iter) +infrastructure.md§"Performance harness" policyadr-0027-kernel-virtual-memory-layoutdocs/architecture/memory-management.mdcompanion + T-016 (MMU activation) Draft + 8 cross-reference updates (ADR-0009 §Revision rider, ADR-0012 §Open questions resolution, hal.md §Mmu, glossary, README, current.md, phase-b.md)integration-2026-05-08-b2-prep-bundle(this branch)What's in the integration commit (
59c08e9)MapperFlush, MMU-instance binding rationale, ADR-0034 placeholder for kernel-image section permissions).phase-b.md§B2 status block (line 111) and §Sub-breakdown step 1 (line 115) flipped fromProposed 2026-05-08toAccepted 2026-05-08, matching the ADR ledger row at line 257. The Accept commit on PR ADR-0027 — kernel virtual memory layout (B2) + open T-016 #20's branch flipped only the ledger row; the §B2 status block was missed.Bot review-round disposition (gh PR comments — already applied per branch)
mmu_bootstrapsequence inconsistency in 4 different sites. All 4 applied in commit2946b35on the source branch (T-016 §Acceptance criteria + memory-management.md prose + memory-management.md sequence diagram + ADR-0027 §Decision outcome + 2 minor secondary occurrences).ef30b5con the source branch. Departure from gemini's literalprint $4suggestion: the kernel'sboot-to-end elapsed = NUM nsline has the number at$5not$4($4 is the equals sign); usedsub(/.*= /, ""); sub(/ ns.*/, "")for format-shift tolerance instead. Verified by 5-iteration sanity run: band 3.973 / 4.559 / 5.642 ms p10/p50/p90, consistent with existing baseline.Forward-flagged from the 2026-05-08 review (NOT in this PR)
These flow to the next hygiene PR or to T-016 itself. Listed here so they don't get lost.
Track 2 Majors → ADR-0027 / T-016 riders (small additions; out of scope for this integration PR):
mem::forget/ManuallyDrop::new/let _ = ...escape hatches forMapperFlush(matches thex86_64::structures::paging::MapperFlushprecedent).MapperFlush::flush(self, mmu: &impl Mmu)accepts anyMmu, not the minting one — non-issue in v1 single-Mmureality but worth a forward-flag for B3+.phase-b.md's ADR ledger.Track 3/4 Minors → governance / wording polish hygiene PR:
current.md"Phase-2 §Simulation table" copy-paste artefact; T3-m2: "Accept will be a separate commit" future-tense after Accept already landed; T3-m3: PR-numbering hygiene drift recurrence (banner authoring should defer until aftergh pr create).trap '...' EXIT INT TERMfortools/perf-harness.shorphan-process cleanup on Ctrl-C.n < 100(collapses to max under nearest-rank) or note in Methodology.Test plan
cargo fmt --all -- --checkclean post-mergecargo host-clippyclean (-D warnings)cargo kernel-clippycleancargo host-test159/159 (no source changes from any of the 3 branches; perf-harness.sh refactor is behaviour-preserving)cargo +nightly miri test— same 159/159 (test set unchanged)cargo kernel-buildcleanRead order suggestion for review
docs/analysis/reviews/code-reviews/2026-05-08-pr-19-20-21-multi-axis-review.md— verdicts, headline numbers, per-track summaries, follow-up backlog. ~300 lines.docs/decisions/0027-kernel-virtual-memory-layout.md§Decision outcome + §Simulation. The §Simulation table is verified bit-by-bit by Track 2.docs/architecture/memory-management.md— narrative + diagrams + encoding tables.docs/analysis/tasks/phase-b/T-016-mmu-activation.md— six bisectable commits, four audit-log entries, full Acceptance criteria.tools/perf-harness.sh+ baseline report.docs/analysis/reviews/code-reviews/2026-05-06-full-tree/— purely mechanical, content byte-stable.🤖 Generated with Claude Code
Summary by CodeRabbit
Documentation
Developer Tools