Development #10 — T-012 (B1 IRQ infrastructure) + B0 closure retros

[cemililik]

Summary

• B1 substantially landed. T-012 — exception vector table at VBAR_EL1, GIC v2 driver for QEMU virt, generic-timer IRQ wiring, and idle's WFI activation — promoted to In Review. Pairs with T-013's EL drop (already on main from PR Development #9 — T-011 + T-008 + T-013 (B0 hygiene close + B1 EL drop) #9) to give Phase B its full kernel-takes-an-interrupt foundation.
• B0 closure retros. Business + security retros for the B0-via-PR-Development #9 — T-011 + T-008 + T-013 (B0 hygiene close + B1 EL drop) #9 milestone, plus the four post-merge Adjustment items applied (post-fix-sweep DoD rule, conduct-approval-review skill canonised, T-012 design-first DoD update, security-review consolidated artefact).
• Aliasing discipline extended. ADR-0021's no-&mut-across-cpu.context_switch rule extends to the IRQ-handler frame via a §Revision notes Amendment; v1's irq_entry is ack-and-ignore and vacuously satisfies the discipline; the rule is in writing before the first scheduler-touching IRQ handler lands.

What landed (commit-by-commit)

Commit	Theme	Notes
b60978d	docs(roadmap)	Close B0 + promote T-008 / T-011 / T-013 to Done.
f9c145c	docs	B0 closure business + security reviews + 4 retro Adjustment items.
8800b0b	fix(analysis)	Apply approval-review findings on B0 closure reviews (Orta #1: Sign-off → Threat-model impact + separate Verdict; Düşük #1: Appendix sourcing line).
c366200	docs(arch)	T-012 design-first — docs/architecture/exceptions.md lands ahead of code (per B0 sec-review §8).
a043079	feat(bsp)	T-012 step 1+2+3 — 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.
b4ed68c	feat(bsp,hal)	T-012 step 4+5 — <QemuVirtCpu as Timer>::arm_deadline / cancel_deadline real impls; irq_entry ack-and-ignore on PPI 27; idle_entry body uses cpu.wait_for_interrupt(); tyrne_hal::timer::ns_to_ticks helper + 5 host tests (148/148 host suite green).
28c5ce9	docs	T-012 step 6+7 — ADR-0021 §Revision notes Amendment (IRQ-handler aliasing discipline) + UNSAFE-2026-0014 Amendment (irq_entry future site) + ADR-0010 §Revision notes (arm_deadline / cancel_deadline live) + ADR-0022 first rider Sub-rider closure + T-012 task file → In Review with all AC/DoD checkboxes flipped.
6d2f725	docs	Back-fill <TBD-commit-3> → 28c5ce9 SHA across 8 files (matches the UNSAFE-2026-0011 SHA back-fill pattern).
67ff867	docs	Review-fix sweep — Düşük #1 (idle_entry doc-comment was stale; rewrote to describe current WFI behaviour) + Düşük #2 (ADR-0021 Amendment header date 2026-04-27 → 2026-04-28).

New audit-log entries

Three new entries; one Amendment on an existing entry. All append-only per unsafe-policy.md §3.

• UNSAFE-2026-0019 — QemuVirtGic MMIO surface (distributor + CPU interface; init sequence + IrqController trait impl). Introduced a043079.
• UNSAFE-2026-0020 — EL1 exception vector table install + asm trampolines (MSR VBAR_EL1, vectors.s 16-entry table, IRQ trampoline saving 192-byte TrapFrame, panic-class trampolines for unhandled vectors). Introduced a043079.
• UNSAFE-2026-0021 — EL1 virtual generic-timer compare-register writes (CNTV_CVAL_EL0, CNTV_CTL_EL0 enable / mask). Introduced b4ed68c.
• UNSAFE-2026-0014 Amendment — names irq_entry as a future site of the same momentary-&mut pattern; v1 has no live citation yet (irq_entry is ack-and-ignore). Introduced 28c5ce9.

ADR amendments (no new ADRs)

• ADR-0010 §Revision notes — arm_deadline / cancel_deadline deferred halves are now live on QemuVirtCpu; pointer to UNSAFE-2026-0021. Trait surface unchanged.
• ADR-0021 §Revision notes — Amendment extending the no-&mut-across-context-switch rule to the IRQ-handler frame. Three numbered future-call-site rules; rejected alternatives recorded.
• ADR-0022 §Revision notes — first-rider Sub-rider closure paragraph: T-009 (time source) ✅ + T-012 (IRQ delivery) ✅ ⇒ idle_entry switches from core::hint::spin_loop() + yield to cpu.wait_for_interrupt() + yield. ADR's chosen option matches shipped code from this commit forward.

Architecture documentation

• docs/architecture/exceptions.md — new (~262 lines) — vector table layout (Mermaid 4×4 grid), IRQ dispatch flow (Mermaid sequence), GIC v2 driver shape (per-register table + init sequence), generic-timer IRQ wiring, idle WFI activation, IRQ-handler aliasing discipline. Implementation map with per-step status flags.
• docs/architecture/README.md — exceptions.md row promoted from Design to Accepted (v0.0.1).

Reviews included in this PR

• B0 closure business retro — Phase A vs B0 cost-of-arc (~30 commits, 2 days vs ~50 commits, 6 days; 109 → 143 host tests); ADR-0025 §Rule 1 + §Rule 2 first empirical run; audit-log Amendment discipline codified; 8 Adjustment items (4 done, 4 pending — one of which, T-012 promotion, is closed by this PR).
• B0 closure consolidated security review — 8-axis adversarial review of UNSAFE-2026-0006..0018 + 4 Amendments. Verdict: Approve. No Yüksek / Orta findings. Two threat-model shifts named (ADR-0024 EL1 structural property; ADR-0021 aliasing model with Stacked-Borrows compliance).
• T-009 mini-retro — already on main; cited by the B0 retro as the agenda-loader.

New skill

• .claude/skills/conduct-approval-review/SKILL.md — distinct from perform-code-review / perform-security-review / conduct-review. 9-step procedure for the "you've already done the work, now confirm it's coherent and ready" review pattern. Three validation runs cited as the canonisation justification.

DoD status

Gate	Status
cargo fmt --all -- --check	✅ clean
cargo host-clippy	✅ clean (-D warnings)
cargo kernel-clippy	✅ clean
cargo kernel-build	✅ clean
cargo host-test	✅ 148 / 148 (24 hal + 90 kernel + 34 test-hal; was 143 pre-T-012)
cargo +nightly miri test (host)	⏳ deferred to CI (R6 GitHub Actions skeleton already pinned for Miri)
QEMU smoke (default + -machine virtualization=on)	⏳ deferred to maintainer-side verification — same disclaimer T-013 used

Test plan

• CI: fmt, host-clippy, kernel-clippy, kernel-build, host-test, Miri pass on the R6 workflow.
• QEMU smoke (default config): kernel boots; A6 cooperative-IPC trace reproduces (eight lines + T-009 timer banner + T-009 boot-to-end measurement). The kernel can take an interrupt but none fires today (no caller arms a deadline in v1 IPC demo).
• QEMU smoke (-machine virtualization=on, EL2 entry): kernel boots; T-013's EL drop still works post-T-012; identical post-post_eret trace.
• (Optional) Deliberate-deadline test path: a debug task body arms arm_deadline(now + 100ms); the IRQ fires; irq_entry masks the timer + EOIs; kernel returns to interrupted code. (No caller in v1 demo; this is the first real exercise of the IRQ path.)
• Independent review pass of T-012's three implementation commits + the documentation sweep, with the T-012-arc review prompt (the one I drafted in chat) as the briefing.

What this PR does NOT do

• No sched::on_timer_irq hook. v1's IPC demo arms no deadlines; irq_entry is ack-and-ignore by design. The hook is added at the same time as the first scheduler-touching IRQ arc (preemption / time_sleep_until / IPI cross-CPU wake). The ADR-0021 §Revision-notes Amendment lands the discipline now so the implementation conforms when the hook is wired.
• No FakeIrqController in tyrne-test-hal. Reasoned-deferred — no host-testable kernel logic touches the GIC trait surface in v1 (the tyrne_hal::timer::ns_to_ticks math is the new host-testable surface, covered by 5 new tests). Added when there's something to test against it.
• No splitting of T-012 into T-012a / T-012b. The retro's cost-of-arc anticipation (1–5 commits) was met at 3 commits; the bundled-by-default approach landed cleanly.
• No B2 / MMU work. That decision (when to draft ADR-0027) is now unblocked by T-012's In Review status — left as the next session's call.

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Exception/interrupt framework with EL1 vector table, IRQ delivery, and GICv2 support
  • Real generic-timer deadline arming/canceling; idle now sleeps on interrupts (WFI)

• Documentation

  • New architecture, audits/unsafe-log, security & business review artifacts, and roadmap/task/decision updates
  • New "conduct-approval-review" skill and tightened code-review standards

• Tests

  • Added timer conversion unit tests

[sourcery-ai]

Sorry @cemililik, your pull request is larger than the review limit of 150000 diff characters

[coderabbitai]

📝 Walkthrough

Walkthrough

Installs AArch64 EL1 exception/IRQ infrastructure for the QEMU virt BSP: vector table and trampolines, GICv2 driver and init, timer arm/cancel wiring, BSP startup changes (VBAR_EL1, DAIF unmask, WFI idle), plus extensive documentation, audits, and task/status updates.

Changes

Cohort / File(s)	Summary
Skills & Standards \.claude/skills/README.md, \.claude/skills/conduct-approval-review/SKILL.md, docs/standards/code-review.md	Added conduct-approval-review skill and a code-review checklist requiring fixed-string grep verification after doc-relevant token changes.
Exception & IRQ Core bsp-qemu-virt/src/vectors.s, bsp-qemu-virt/src/exceptions.rs, bsp-qemu-virt/src/gic.rs	Added EL1 vector table and trampolines, TrapFrame, panic/irq entry points, and a QEMU-virt GICv2 MMIO driver with init, enable/disable, acknowledge, and EOI semantics (spurious handling included).
Timer & CPU bsp-qemu-virt/src/cpu.rs, hal/src/timer.rs	Implemented QemuVirtCpu deadline arm/cancel (CNTV_CVAL/CNTV_CTL_EL0 + GIC PPI enable/disable) and added ns_to_ticks (const, saturating, ceiling) with tests.
BSP Init & Idle bsp-qemu-virt/src/main.rs	Installs tyrne_vectors via VBAR_EL1, initializes GIC with MMIO bases, clears DAIF.I, switches idle loop to wait_for_interrupt() then yield_now, and introduces a global GIC handle.
Build & Linker bsp-qemu-virt/build.rs, bsp-qemu-virt/linker.ld	Added src/vectors.s to cargo rerun-if-changed; ensured .text.vectors is placed/KEPT in .text with 2048-byte alignment.
Docs — Architecture & Tasks docs/architecture/exceptions.md, docs/architecture/README.md, docs/analysis/tasks/phase-b/*, docs/analysis/reviews/*, docs/roadmap/*, docs/roadmap/phases/phase-b.md	New exceptions architecture doc; updated architecture index, task statuses (T-008/T-011/T-013 → Done, T-012 → In Review), B0 closure retrospectives, and roadmap/phase docs reflecting PR merges and test counts.
Docs — Audits & Decisions docs/audits/unsafe-log.md, docs/decisions/0010-timer-trait.md, docs/decisions/0021-raw-pointer-scheduler-ipc-bridge.md, docs/decisions/0022-idle-task-and-typed-scheduler-deadlock.md	Appended UNSAFE amendments for IRQ/timer/GIC/vector-table; updated ADRs for timer trait semantics, IRQ aliasing discipline (no live &mut across irq entry), and idle WFI activation.
Miscellaneous docs/analysis/reviews/business-reviews/*.md, docs/analysis/reviews/security-reviews/*.md, \.claude/skills/*	Added B0 closure business/security reviews and related index entries; small README/index updates.

Sequence Diagram(s)

sequenceDiagram
    participant CPU as CPU (kernel)
    participant VT as Vector Table
    participant Trampoline as IRQ Trampoline (asm)
    participant IRQEntry as irq_entry() (Rust)
    participant GIC as QemuVirtGic
    participant Timer as Generic Timer
    participant Scheduler as Scheduler (timer hook)

    Timer->>CPU: PPI 27 fires (deadline reached)
    CPU->>VT: Exception taken to EL1 (IRQ slot)
    VT->>Trampoline: branch to irq trampoline
    Trampoline->>Trampoline: save registers + ELR/SPSR
    Trampoline->>IRQEntry: call irq_entry(frame)
    IRQEntry->>GIC: acknowledge() (read GICC_IAR)
    GIC-->>IRQEntry: returns IRQ id (27 or spurious)
    alt Timer IRQ (valid)
        IRQEntry->>Timer: mask CNTV_CTL_EL0
        IRQEntry->>GIC: end_of_interrupt(27) (write GICC_EOIR)
        IRQEntry->>Scheduler: invoke timer hook
    else Spurious
        IRQEntry->>GIC: end_of_interrupt(spurious)
    end
    IRQEntry-->>Trampoline: return from irq_entry
    Trampoline->>CPU: restore registers
    Trampoline->>CPU: eret (resume)

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

• Development #8 — Continues the timer/HAL work; closely related to ns_to_ticks and the QemuVirtCpu timer implementation.
• Development #6 — Related BSP/idle-loop and scheduler-bridge changes touching similar idle behavior and kernel entry sites.

Poem

🐇 I nibbled vectors, neat and aligned,
GICs hum softly, interrupts find,
Timers tick, deadlines no longer dread,
Trampolines save, then leap overhead,
Hooray — the kernel wakes from its bed!

🚥 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 accurately describes the main changes: T-012 IRQ infrastructure work and B0 closure retrospectives. It is specific, clear, and reflects the primary deliverables.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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 docstrings

• Create stacked PR
• Commit on current branch

🧪 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

• X
• Mastodon
• Reddit
• LinkedIn

_{Review rate limit: 0/1 reviews remaining, refill in 60 minutes.}

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

[gemini-code-assist]

Code Review

This pull request implements the exception infrastructure and interrupt delivery for the QEMU virt BSP, including a GIC v2 driver, an aarch64 exception vector table, and Rust handlers for IRQs and panics. The kernel now configures VBAR_EL1 at boot, enables the generic timer's deadline functionality, and updates the idle task to use the WFI instruction. Feedback primarily addressed inaccuracies in the exceptions.md architecture documentation, where Mermaid diagrams and descriptive text incorrectly described the register saving/restoring logic and included a scheduler hook not yet present in the implementation.

[gemini-code-assist]

The Mermaid sequence diagram indicates that the IRQ trampoline saves registers x0..x29. However, the implementation in bsp-qemu-virt/src/vectors.s (lines 116-125) only saves x0..x18 and x30 (LR), skipping the callee-saved registers x19..x29. The diagram should be updated to accurately reflect the saved register set.

Suggested change

	Tramp->>Frame: stp x0..x29; mrs ELR_EL1, SPSR_EL1
	Tramp->>Frame: stp x0..x18, x30; mrs ELR_EL1, SPSR_EL1

[gemini-code-assist]

The Mermaid diagram shows a call to sched::on_timer_irq(&Frame). However, the current implementation of irq_entry in bsp-qemu-virt/src/exceptions.rs (lines 156-183) does not yet include this hook, as noted in the source comments. The diagram should match the actual implementation state for v1, or clearly mark this as a future extension.

Suggested change

	Handler->>Sched: sched::on_timer_irq(&Frame)
	alt timer IRQ (id 27)
	Handler->>Handler: mask timer + EOI (v1)

[gemini-code-assist]

Similar to the save sequence, the restore sequence in the Mermaid diagram mentions ldp x0..x29, which does not match the implementation in vectors.s (lines 133-145).

Suggested change

	Tramp->>Frame: ldp x0..x29; msr ELR_EL1, SPSR_EL1
	Tramp->>Frame: ldp x0..x18, x30; msr ELR_EL1, SPSR_EL1

[gemini-code-assist]

The text states that the trampoline saves "enough of x19..x29 that the Rust handler can use callee-saved registers freely". This is inaccurate; the trampoline in vectors.s does not save any registers in the x19..x29 range. Instead, it relies on the Rust compiler's adherence to the AAPCS64 ABI, where the callee (irq_entry) is responsible for preserving those registers if it uses them.

Suggested change

	The trampoline saves the AAPCS64-caller-saved registers (`x0..x18`) plus enough of `x19..x29` that the Rust handler can use callee-saved registers freely; `ELR_EL1` and `SPSR_EL1` are saved so the eret restores the interrupted PSTATE exactly. The exact saved set is fixed by the trampoline's asm and exposed to Rust as a `#[repr(C)]` struct passed by `&mut`.
	The trampoline saves the AAPCS64-caller-saved registers (`x0..x18`) plus `x30` (LR); `ELR_EL1` and `SPSR_EL1` are saved so the eret restores the interrupted PSTATE exactly. Callee-saved registers (`x19..x29`) are preserved by the Rust handler per the C ABI. The exact saved set is fixed by the trampoline's asm and exposed to Rust as a `#[repr(C)]` struct passed by pointer.

[coderabbitai]

Actionable comments posted: 12

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

docs/analysis/tasks/phase-b/T-012-exception-and-irq-infrastructure.md (1)

8-10: ⚠️ Potential issue | 🟡 Minor

Dependency status here needs to be updated.

T-009 is still described as In Review, but the other roadmap/closure docs in this PR already mark it Done on 2026-04-27. Leaving it stale makes T-012’s gating ambiguous.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@docs/analysis/tasks/phase-b/T-012-exception-and-irq-infrastructure.md` around
lines 8 - 10, Update the dependency status text in T-012 to reflect that T-009
is Done (not "In Review")—replace the T-009 status string in the Dependencies
paragraph (reference symbol T-009) with "Done" and, if known, add the completion
date (2026-04-27) for clarity; also verify and update the ADR-0024 mention only
if ADR-0024 has actually been Accepted so the gating sentence about VBAR_EL1
installation is accurate (referenced symbols: T-009, ADR-0024, T-012).

🧹 Nitpick comments (1)

hal/src/timer.rs (1)

431-438: The “saturates” test does not currently exercise the saturation branch.

ns=u64::MAX, freq=1_000_000_000 evaluates to exactly u64::MAX, so this checks the boundary value, not overflow-to-saturation behavior. Add one case that forces ticks > u64::MAX pre-cast (e.g., very high frequency) to validate the branch directly.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@hal/src/timer.rs` around lines 431 - 438, The test
ns_to_ticks_saturates_at_u64_max currently only hits the exact boundary; add an
additional assertion that forces the internal u128 calculation to exceed
u64::MAX so the saturation branch in ns_to_ticks is exercised (for example set
high_freq to a value larger than 1_000_000_000 such as 1_000_000_001 or u64::MAX
and assert ns_to_ticks(huge_ns, high_freq) == u64::MAX); update the test body of
ns_to_ticks_saturates_at_u64_max to include this extra case referencing
ns_to_ticks and the existing huge_ns/high_freq variables.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.claude/skills/conduct-approval-review/SKILL.md:
- Around line 93-114: The fenced code block in the SKILL.md example is missing a
language tag which triggers markdownlint; update the opening fence from ``` to
include a language (e.g., ```md or ```text or ```markdown) so the block is
annotated—modify the fenced block in the SKILL.md content (the triple-backtick
block under the "Verdict" / "Verification audit" section) to include one of
those language tags.
- Line 30: Replace the non-English strings in SKILL.md with English equivalents:
translate the Turkish quote "kabul edilen bulguların tamamı düzeltildi mi?" to
an English sentence (e.g., "Have all accepted findings been fixed?") and change
the severity labels `Yüksek` / `Orta` / `Düşük` to `High` / `Medium` / `Low`;
update every occurrence noted in the diff (including the quoted sentence and the
labels around the lines referenced, e.g., the quoted review example and the
severity list) so the whole document remains English-only.

In `@bsp-qemu-virt/src/exceptions.rs`:
- Around line 137-138: The assembly entry functions are currently declared as
safe extern "C" functions; change both irq_entry and panic_entry to be unsafe
extern "C" fn to reflect their caller-side safety preconditions and prevent
accidental Rust calls. Update the function signatures for irq_entry and
panic_entry (keeping #[no_mangle] and extern "C") to: declare them with the
unsafe qualifier (e.g., #[no_mangle] pub unsafe extern "C" fn irq_entry(_frame:
*mut TrapFrame) { ... } and similarly for panic_entry) so the type system
enforces the unsafe contract at call sites.

In `@bsp-qemu-virt/src/gic.rs`:
- Around line 308-338: The enable() and disable() methods compute distributor
MMIO offsets from irq.0 and then perform unsafe writes without validating the
IRQ, which can overflow/saturate the offset math; add a validation step at the
start of both enable() and disable() (before computing reg_offset or calling
write_distributor) that ensures IrqNumber is in the allowed range (e.g. less
than the maximum IRQs handled by the distributor / that (n/32).saturating_mul(4)
+ base stays within the GIC window), and reject/abort on invalid values
(panic/debug_assert/return an Err depending on your API choice) so
write_distributor is never invoked with an out-of-range offset; reference the
enable/disable methods, IrqNumber, write_distributor, and the
GICD_ISENABLER_BASE / GICD_ICENABLER_BASE constants when adding the checks.

In `@docs/analysis/reviews/business-reviews/2026-04-27-B0-closure.md`:
- Around line 31-44: The header "Five `unsafe` regions introduced or amended
during B0" is inconsistent with the table listing UNSAFE-2026-0012 through
UNSAFE-2026-0018 (seven entries); update the scope label to "Seven `unsafe`
regions" to match the table, or if the intent was to list only five, remove or
collapse two of the entries (for example drop UNSAFE-2026-0012 and
UNSAFE-2026-0018) and adjust any accompanying text accordingly; ensure
references to UNSAFE-2026-0012, UNSAFE-2026-0013, UNSAFE-2026-0014,
UNSAFE-2026-0015, UNSAFE-2026-0016, UNSAFE-2026-0017, and UNSAFE-2026-0018
remain consistent after the change.

In `@docs/analysis/tasks/phase-b/README.md`:
- Line 14: Update the index row for T-012 in
docs/analysis/tasks/phase-b/README.md to match the task file status by changing
the status cell for [T-012](T-012-exception-and-irq-infrastructure.md) from "In
Progress" to "In Review"; ensure the table row for T-012 exactly matches the
status used in T-012-exception-and-irq-infrastructure.md so the index and task
file are synchronized.

In `@docs/architecture/exceptions.md`:
- Around line 79-88: The docs currently show the timer IRQ path calling
sched::on_timer_irq, but the shipped implementation (irq_entry) masks
CNTV_CTL_EL0, calls gic.end_of_interrupt / EOIs the interrupt and returns
without borrowing scheduler state; update the sequence diagram and the timer
section to reflect this shipped flow (remove the immediate sched::on_timer_irq
call from the "current" path), and instead document the deferred wake hook as a
future/optional path; reference the irq_entry handler and CNTV_CTL_EL0 masking
behavior and ensure the alt/else branches and end_of_interrupt lines match the
actual gic.end_of_interrupt behavior.
- Around line 66-92: The docs claim the trampoline saves x0..x29, but the actual
assembly in vectors.s saves x0..x18 and x30 plus ELR_EL1 and SPSR_EL1 into the
192-byte frame; update the mermaid diagram (replace "stp x0..x29" with the
correct saved registers: x0..x18 and x30) and change the prose to state that
TrapFrame (the #[repr(C)] struct exposed to Rust) contains x0..x18, x30, ELR_EL1
and SPSR_EL1, while x19..x29 are not pushed by the trampoline but are preserved
by Rust callee conventions (AAPCS64), and reference the trampoline entry
(irq_entry) and TrapFrame struct names so readers can correlate docs with
vectors.s and the Rust handler.

In `@docs/decisions/0021-raw-pointer-scheduler-ipc-bridge.md`:
- Around line 132-134: Reword the paragraph so it no longer claims that the
first scheduler-touching IRQ handler will create the UNSAFE-2026-0014 naming
amendment; instead state that the audits log already records UNSAFE-2026-0014
and that when a future scheduler-touching IRQ handler (irq_entry) is introduced
it should add a follow-up Amendment that records activation of that
already-named site rather than creating a parallel audit entry, retaining the
single-pattern mental model referenced alongside UNSAFE-2026-0020.

In `@docs/roadmap/current.md`:
- Around line 13-14: The roadmap's headline metrics for sched/mod.rs and
workspace coverage conflict with the B0 closure retro; choose the canonical
values recorded in the B0 closure retro (sched/mod.rs 84.16% and workspace
96.37%) and update the milestone summary in current.md to match (replace 83.93%
→ 84.16% and 96.33% → 96.37%), then re-run any small consistency check to ensure
the PR/merge references and test counts in the same paragraph remain unchanged
and accurate (verify the host tests and PR/merge commit text for T-011 and B0
closure notes).

In `@docs/roadmap/phases/phase-b.md`:
- Line 94: The bullet incorrectly states "T-012 closed" while its parenthetical
notes verification is pending; update the line mentioning T-012 so it does not
assert closure—e.g., change "T-012 closed" to "T-012 implemented (pending
verification)" or "T-012 delivered — verification pending" and keep the existing
details about arm_deadline, idle_entry, and ADR-0022 intact so the note reflects
that the implementation is present but final closure awaits maintainer-side QEMU
verification.

In `@hal/src/timer.rs`:
- Around line 192-194: The ns_to_ticks conversion currently floors deadlines by
using simple division, which can arm IRQs early; change the conversion to round
up (ceiling) so any sub-tick ns maps to the next tick. In ns_to_ticks, compute
ticks with a ceiling formula using the existing intermediate (ns as u128 *
frequency_hz as u128) and NANOS_PER_SECOND so that ticks = ceil(intermediate /
NANOS_PER_SECOND), then perform the same overflow check against u64::MAX before
returning; ensure you update the ticks calculation where variables intermediate,
frequency_hz, NANOS_PER_SECOND, and ticks are used.

---

Outside diff comments:
In `@docs/analysis/tasks/phase-b/T-012-exception-and-irq-infrastructure.md`:
- Around line 8-10: Update the dependency status text in T-012 to reflect that
T-009 is Done (not "In Review")—replace the T-009 status string in the
Dependencies paragraph (reference symbol T-009) with "Done" and, if known, add
the completion date (2026-04-27) for clarity; also verify and update the
ADR-0024 mention only if ADR-0024 has actually been Accepted so the gating
sentence about VBAR_EL1 installation is accurate (referenced symbols: T-009,
ADR-0024, T-012).

---

Nitpick comments:
In `@hal/src/timer.rs`:
- Around line 431-438: The test ns_to_ticks_saturates_at_u64_max currently only
hits the exact boundary; add an additional assertion that forces the internal
u128 calculation to exceed u64::MAX so the saturation branch in ns_to_ticks is
exercised (for example set high_freq to a value larger than 1_000_000_000 such
as 1_000_000_001 or u64::MAX and assert ns_to_ticks(huge_ns, high_freq) ==
u64::MAX); update the test body of ns_to_ticks_saturates_at_u64_max to include
this extra case referencing ns_to_ticks and the existing huge_ns/high_freq
variables.

🪄 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: 9f98bf09-ead3-482f-852c-b0ca6ef1c3a9

📥 Commits

Reviewing files that changed from the base of the PR and between 9a66e8b and 67ff867.

📒 Files selected for processing (28)

• .claude/skills/README.md
• .claude/skills/conduct-approval-review/SKILL.md
• bsp-qemu-virt/build.rs
• bsp-qemu-virt/linker.ld
• bsp-qemu-virt/src/cpu.rs
• bsp-qemu-virt/src/exceptions.rs
• bsp-qemu-virt/src/gic.rs
• bsp-qemu-virt/src/main.rs
• bsp-qemu-virt/src/vectors.s
• docs/analysis/reviews/business-reviews/2026-04-27-B0-closure.md
• docs/analysis/reviews/business-reviews/README.md
• docs/analysis/reviews/security-reviews/2026-04-27-B0-closure.md
• docs/analysis/reviews/security-reviews/README.md
• docs/analysis/tasks/phase-b/README.md
• docs/analysis/tasks/phase-b/T-008-architecture-docs.md
• docs/analysis/tasks/phase-b/T-011-missing-tests-bundle.md
• docs/analysis/tasks/phase-b/T-012-exception-and-irq-infrastructure.md
• docs/analysis/tasks/phase-b/T-013-el-drop-to-el1.md
• docs/architecture/README.md
• docs/architecture/exceptions.md
• docs/audits/unsafe-log.md
• docs/decisions/0010-timer-trait.md
• docs/decisions/0021-raw-pointer-scheduler-ipc-bridge.md
• docs/decisions/0022-idle-task-and-typed-scheduler-deadlock.md
• docs/roadmap/current.md
• docs/roadmap/phases/phase-b.md
• docs/standards/code-review.md
• hal/src/timer.rs

[cemililik]

@coderabbitai review

[coderabbitai]

✅ 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.

[coderabbitai]

Actionable comments posted: 4

♻️ Duplicate comments (1)

docs/architecture/exceptions.md (1)

160-165: ⚠️ Potential issue | 🟠 Major

Document the shipped timer IRQ path here as well.

This section still says irq_entry calls sched::on_timer_irq(...), but bsp-qemu-virt/src/exceptions.rs currently masks CNTV_CTL_EL0, EOIs the interrupt, and returns. Keeping the future wake hook in the current flow blurs the v1 guarantee that IRQ entry does not borrow scheduler state yet.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@docs/architecture/exceptions.md` around lines 160 - 165, Update the
documentation to reflect the actual shipped timer IRQ path: describe that
irq_entry (in the QEMU-virt BSP) masks CNTV_CTL_EL0, calls gic.end_of_interrupt
for IrqNumber(27), and returns without invoking sched::on_timer_irq; remove or
revise the sentence claiming irq_entry calls sched::on_timer_irq so the docs
preserve the v1 guarantee that IRQ entry does not borrow scheduler state and
instead note that any wake/scheduling hook happens later outside the IRQ entry
path.

🧹 Nitpick comments (2)

hal/src/timer.rs (1)

417-467: Add a direct sub-tick rounding regression test.

Current ns_to_ticks tests mostly use values where floor and ceil produce the same result. A small remainder-case test would lock in the ceiling policy and prevent silent regressions.

✅ Proposed test addition

 #[test]
 fn ns_to_ticks_one_second_yields_frequency_at_any_freq() {
@@
 }
 
+#[test]
+fn ns_to_ticks_rounds_up_sub_tick_deadlines() {
+    // 62.5 MHz => 16 ns per tick.
+    let freq = 62_500_000;
+    assert_eq!(ns_to_ticks(1, freq), 1);
+    assert_eq!(ns_to_ticks(15, freq), 1);
+    assert_eq!(ns_to_ticks(16, freq), 1);
+    assert_eq!(ns_to_ticks(17, freq), 2);
+}
+
 #[test]
 fn ns_to_ticks_saturates_at_u64_max() {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@hal/src/timer.rs` around lines 417 - 467, Add a unit test (e.g., fn
ns_to_ticks_rounds_up_on_subtick) that asserts ns_to_ticks uses ceiling on
sub-tick remainders: call ns_to_ticks with a frequency and an ns value that is
just 1ns greater than an exact tick boundary (for example freq = 3 and ns =
333_333_334) and assert the result is the ceiling tick count (2); place the test
near the other ns_to_ticks tests and reference ns_to_ticks and the
NANOS_PER_SECOND logic so the rounding policy is locked in.

bsp-qemu-virt/src/exceptions.rs (1)

43-70: Add a compile-time size guard for TrapFrame.

TrapFrame is part of a byte-for-byte ABI with bsp-qemu-virt/src/vectors.s, but right now a future field edit only fails on the first live IRQ. A const size assertion here would turn that drift into a build-time failure.

Suggested guard

 #[repr(C)]
 #[derive(Debug)]
 pub struct TrapFrame {
     /// `x0` and `x1` saved at frame offset 0x00.
     pub x0_x1: [u64; 2],
@@
     /// Padding — keeps the frame at 192 bytes total (16-byte SP-aligned).
     pub _reserved: [u64; 2],
 }
+
+const _: [(); 0xC0] = [(); core::mem::size_of::<TrapFrame>()];

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@bsp-qemu-virt/src/exceptions.rs` around lines 43 - 70, The TrapFrame struct
is part of a byte-for-byte ABI and needs a compile-time size assertion to catch
accidental layout changes; add a const size guard after the TrapFrame definition
that uses core::mem::size_of::<TrapFrame>() (or a const assertion pattern) to
assert its size equals 192 bytes (or the expected byte size matching vectors.s),
so any future edits will fail at compile time if the struct size drifts.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@bsp-qemu-virt/src/exceptions.rs`:
- Around line 139-143: The SAFETY comment for the unsafe block using
(*GIC.0.get()).assume_init_ref() must be expanded to the full triplet: state why
unsafe is needed (to create a stable &QemuVirtGic reference from a static
MaybeUninit in IRQ context where Rust cannot prove initialization), what
invariants are relied on (kernel_entry initialized GIC before unmasking
interrupts, GIC is never moved or reinitialized, no mutable aliases during ISR
so the returned reference remains valid for ISR lifetime), and why safer
alternatives were rejected (e.g., Mutex/RefCell/Option would either require
heap/locking unavailable in early IRQ path or impose unacceptable
overhead/latency for ISR). Do the same for the other unsafe block around lines
163–169 (identify the function/usage that dereferences the same static or uses
assume_init/_mut), ensuring each comment names the symbol (GIC, QemuVirtGic,
kernel_entry, ISR, assume_init_ref) and asserts the exact invariants and
rationale.

In `@docs/architecture/exceptions.md`:
- Around line 15-16: Update every mention of ADR-0021 in this document to
reflect that the IRQ-handler amendment now exists and is dated 2026-04-28:
replace language that describes the amendment as future/open (e.g., "will likely
gain an Amendment", "dated 2026-04-27") with a statement that the amendment is
present and dated 2026-04-28, and ensure any reference to the IRQ-handler
amendment mentions ADR-0021 and the 2026-04-28 amendment date; apply the same
change to the other occurrences noted in the file (the remaining ADR-0021
references).

In `@docs/decisions/0021-raw-pointer-scheduler-ipc-bridge.md`:
- Around line 127-130: The IRQ entry function's signature must be declared
unsafe to preserve the caller-side contract: change the shipped handler back to
pub unsafe extern "C" fn irq_entry(_frame: *mut TrapFrame) (or restore the
unsafe on the existing irq_entry symbol), and ensure any callers/registrations
expect an unsafe extern "C" entry; also add a short comment next to irq_entry
and/or TrapFrame noting that changes touching capabilities/IPC/memory/crypto
must be flagged for explicit security review as described in the ADR.

In `@docs/roadmap/current.md`:
- Line 31: The roadmap's "Next review trigger" paragraph still references an
intermediate gate: "a code review on the GIC + vector-table commit before WFI
activation lands"; remove that clause (or change it to a past-tense note that
the gate is complete) since WFI activation already landed in commit b4ed68c, so
update the "Next review trigger" sentence under the Next review trigger header
to either delete the intermediate gate mention or mark it as completed.

---

Duplicate comments:
In `@docs/architecture/exceptions.md`:
- Around line 160-165: Update the documentation to reflect the actual shipped
timer IRQ path: describe that irq_entry (in the QEMU-virt BSP) masks
CNTV_CTL_EL0, calls gic.end_of_interrupt for IrqNumber(27), and returns without
invoking sched::on_timer_irq; remove or revise the sentence claiming irq_entry
calls sched::on_timer_irq so the docs preserve the v1 guarantee that IRQ entry
does not borrow scheduler state and instead note that any wake/scheduling hook
happens later outside the IRQ entry path.

---

Nitpick comments:
In `@bsp-qemu-virt/src/exceptions.rs`:
- Around line 43-70: The TrapFrame struct is part of a byte-for-byte ABI and
needs a compile-time size assertion to catch accidental layout changes; add a
const size guard after the TrapFrame definition that uses
core::mem::size_of::<TrapFrame>() (or a const assertion pattern) to assert its
size equals 192 bytes (or the expected byte size matching vectors.s), so any
future edits will fail at compile time if the struct size drifts.

In `@hal/src/timer.rs`:
- Around line 417-467: Add a unit test (e.g., fn
ns_to_ticks_rounds_up_on_subtick) that asserts ns_to_ticks uses ceiling on
sub-tick remainders: call ns_to_ticks with a frequency and an ns value that is
just 1ns greater than an exact tick boundary (for example freq = 3 and ns =
333_333_334) and assert the result is the ceiling tick count (2); place the test
near the other ns_to_ticks tests and reference ns_to_ticks and the
NANOS_PER_SECOND logic so the rounding policy is locked in.

🪄 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: 56f32c6c-98f5-48df-8b5f-6e373606778e

📥 Commits

Reviewing files that changed from the base of the PR and between 67ff867 and d820a88.

📒 Files selected for processing (11)

• .claude/skills/conduct-approval-review/SKILL.md
• bsp-qemu-virt/src/exceptions.rs
• bsp-qemu-virt/src/gic.rs
• docs/analysis/reviews/business-reviews/2026-04-27-B0-closure.md
• docs/analysis/tasks/phase-b/README.md
• docs/analysis/tasks/phase-b/T-012-exception-and-irq-infrastructure.md
• docs/architecture/exceptions.md
• docs/decisions/0021-raw-pointer-scheduler-ipc-bridge.md
• docs/roadmap/current.md
• docs/roadmap/phases/phase-b.md
• hal/src/timer.rs

✅ Files skipped from review due to trivial changes (1)

• docs/analysis/reviews/business-reviews/2026-04-27-B0-closure.md

🚧 Files skipped from review as they are similar to previous changes (3)

• docs/analysis/tasks/phase-b/README.md
• docs/analysis/tasks/phase-b/T-012-exception-and-irq-infrastructure.md
• bsp-qemu-virt/src/gic.rs

[cemililik]

@coderabbitai review

[coderabbitai]

✅ 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.

[coderabbitai]

Actionable comments posted: 1

🤖 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/architecture/exceptions.md`:
- Around line 171-195: The documentation still claims idle remains a spin-loop
until T-012, but the code and implementation map show idle_entry now calls
cpu.wait_for_interrupt() (WFI) and the WFI path is shipped; update the prose in
the idle section to reflect that idle now uses the WFI path (idle_entry ->
cpu.wait_for_interrupt()) as the default wake strategy, remove or reword the
stale sentence about staying in spin_loop form, and reference ADR-0022/T-012
appropriately (e.g., note that the first rider/Sub-rider gate from ADR-0022 is
closed and that remaining T-012 items are separate follow-ups) so the text
aligns with the shown code block.

🪄 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: d26f348e-41d6-42a1-ba25-7129ecfc2226

📥 Commits

Reviewing files that changed from the base of the PR and between d820a88 and 5c9cf06.

📒 Files selected for processing (5)

• bsp-qemu-virt/src/exceptions.rs
• docs/architecture/exceptions.md
• docs/decisions/0021-raw-pointer-scheduler-ipc-bridge.md
• docs/roadmap/current.md
• hal/src/timer.rs

✅ Files skipped from review due to trivial changes (1)

• docs/roadmap/current.md

🚧 Files skipped from review as they are similar to previous changes (1)

• hal/src/timer.rs

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Update the idle prose to reflect the shipped wfi path.

This paragraph still says idle remains in the spin-loop form until T-012 lands, but the implementation map marks the WFI activation complete. That makes the sentence read stale next to the new code block.

Proposed fix

- Until T-012's full set lands, idle stays in the spin_loop form.
+ T-012 now activates the `wfi` idle path; the spin-loop form above is shown only as the pre-change baseline.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@docs/architecture/exceptions.md` around lines 171 - 195, The documentation
still claims idle remains a spin-loop until T-012, but the code and
implementation map show idle_entry now calls cpu.wait_for_interrupt() (WFI) and
the WFI path is shipped; update the prose in the idle section to reflect that
idle now uses the WFI path (idle_entry -> cpu.wait_for_interrupt()) as the
default wake strategy, remove or reword the stale sentence about staying in
spin_loop form, and reference ADR-0022/T-012 appropriately (e.g., note that the
first rider/Sub-rider gate from ADR-0022 is closed and that remaining T-012
items are separate follow-ups) so the text aligns with the shown code block.