Doc-fix sweep — close 7 Track-E blockers + non-blocker doc drift (comprehensive review follow-up) - #13
Merged
Merged
Conversation
Address all seven blocker-class items from Track E of the 2026-05-06 comprehensive code review. Each is a factual statement that fell out of sync with the code at HEAD; none requires a code change. Fixes land docs-vs-code consistency at HEAD `9cbf578`. ## docs/architecture/overview.md (Track-E §Blocker #1) - BSP table row for `bsp-qemu-virt` corrected: GICv3 → GICv2. The QEMU `virt` machine defaults to GICv2; the BSP ships a v2-only driver per ADR-0011 + UNSAFE-2026-0019. GICv3 requires `-machine gic-version=3` and is out of scope for v1. ## docs/architecture/hal.md (Track-E §Blockers #2, #3, #4) - Mermaid box label: `BIrq["GICv3 / GIC-400 impl"]` → `"GICv2 / GIC-400 impl"`. There is no GICv3 impl in tree; the bsp-pi4 row at line 198 already correctly notes GIC-400 (a v2 subset). - BSP table interrupt-controller row: GICv3 → GICv2. - Timer subsection: the "`arm_deadline` / `cancel_deadline` are `unimplemented!()` in QEMU virt's BSP today" assertion is now stale — T-012 (Done 2026-04-28) implemented both bodies under UNSAFE-2026-0021. Subsection rewritten to state the real status, cite ADR-0010 §Revision notes, and note the smoke-verification gap (no v1 caller arms a deadline; `Pending QEMU smoke` notation on UNSAFE-2026-0021 records this). ## docs/architecture/scheduler.md (Track-E §Blocker #5) - Two passages still framed `wait_for_interrupt` activation as a *future* T-012 deliverable. T-012 landed 2026-04-28; the BSP's `idle_entry` body uses `cpu.wait_for_interrupt(); yield_now(...)` per UNSAFE-2026-0019/0020/0021. Both passages updated to state the current shape, cite the audit entries, and reference ADR-0022 §Revision notes 2026-04-28's first-rider Sub-rider closure. ## docs/architecture/security-model.md (Track-E §Blocker #6) - *Open questions* §"Early IRQ masking in BSP reset vectors" closed by T-013 (Done 2026-04-27, ADR-0024, UNSAFE-2026-0017): `boot.s` begins `_start` with `msr daifset, #0xf` as the literal first instruction, and the BSP boot checklist §1a now records the rule. Bullet rewritten as a closure-rider with citations. ## docs/glossary.md (Track-E §Blocker #7) - CDT entry's live Markdown link to `decisions/0023-...md` (a file that does not exist — ADR-0023 is reserved-empty / accept-deferred per Phase B0 closure) was a 404 every other reference avoided. Replaced with prose only ("see ADR-0023 (accept-deferred per Phase B0 closure; no file at HEAD, tracked in `phase-b.md` ADR ledger and the B0 closure security review)") so a Markdown reader no longer follows the link to a missing file. Refs: comprehensive-review-2026-05-06, ADR-0010, ADR-0011, ADR-0022, ADR-0024, ADR-0026 Audit: UNSAFE-2026-0017, UNSAFE-2026-0019, UNSAFE-2026-0020, UNSAFE-2026-0021 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Close three non-blocker drift items from Track E of the 2026-05-06 comprehensive code review. Each was a Phase-A-era framing that no longer matches the post-B1 tree. ## README.md Repository-layout tree previously listed only `docs/` + root-level files and ended with "Source code layout … will be added after the architecture phase." Phase A and B0/B1 are closed; the Rust workspace contains four crates totalling ~9 590 LOC. Replaced with the actual top-level layout (kernel/, hal/, test-hal/, bsp-qemu-virt/, tools/, plus the docs/ subdirectories — audits, analysis, roadmap), with crate-name annotations and pointers to ADR-0006, the two-task-demo guide, and current.md. ## CONTRIBUTING.md "There is no source code to extend or refactor meaningfully yet" is no longer accurate. Reframed to describe the actual contribution expectation: source PRs welcome but should align with the active milestone in current.md to avoid premature rewrites; the kernel boots end-to-end on QEMU virt today. ## SECURITY.md "There is no runnable kernel yet" is stale (kernel boots end-to-end on QEMU and runs the two-task IPC demo). The "(planned, Phase 2)" qualifier on the threat-model document is also stale — `docs/architecture/security-model.md` is Accepted at HEAD. Sentence rewritten to acknowledge current state while preserving the "pre-alpha; no production guarantees" framing. Refs: comprehensive-review-2026-05-06, ADR-0006 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Closes the remaining Track-E + Track-H + Track-I non-blocker doc
drift items the comprehensive code review at HEAD `9cbf578`
identified. All single-edit clarifications; no policy change.
## docs/standards/README.md
Standards index missed `bsp-boot-checklist.md`. The file exists, is
in active use (referenced from `unsafe-policy.md §5a`,
`security-model.md §Open questions`, T-013 review-history), and is
genuinely a `Accepted` standard — only the index row was missing.
Added.
## docs/decisions/README.md
ADR index jumped 0022 → 0024 with no row for ADR-0023, while
`phase-b.md`'s ADR ledger and prose elsewhere reference ADR-0023 as
accept-deferred. The two indices disagreed on whether a
deferred-without-file ADR appears as a row. Added an ADR-0023 row
in the index (Status: Deferred, no file link) so a future reader
scanning the README does not miss the deferred slot.
## docs/decisions/template.md
The template's status enum implicitly listed only Proposed /
Accepted / Deprecated / Superseded by NNNN. ADR-0018 and ADR-0023
both use a real `Deferred` state introduced by ADR-0025-era
governance amendments. Added a comment block after the frontmatter
documenting the five recognised status values, including Deferred.
## docs/architecture/overview.md
The "(final form documented in `hal.md`, planned)" parenthetical on
the HAL trait surface is stale — hal.md is Accepted and shipped.
Replaced "planned" with "Accepted" so `overview.md`'s framing
matches `hal.md`'s actual status.
## docs/guides/two-task-demo.md
Expected-output table previously omitted two production lines
emitted by `bsp-qemu-virt::main.rs` post-T-009: `tyrne: timer
ready (...)` and `tyrne: boot-to-end elapsed = ... ns`. Added both
to the trace block with a note that the values are
QEMU-default-dependent. Execution-trace prose now mentions idle's
fallback-slot registration per ADR-0026, replacing the obsolete
"a dedicated `wfe`-based idle path is Phase B work" line — that
work landed in T-012, and T-014 / ADR-0026 then moved idle out of
the FIFO into a fallback slot. Per-line table gains rows for the
timer-ready and boot-to-end-elapsed lines.
## docs/standards/infrastructure.md
Two doc-vs-state mismatches the comprehensive review surfaced:
1. **§Continuous integration → Required gates** listed `cargo audit`
/ `cargo vet check` / "QEMU smoke" as merge-blockers, but
`.github/workflows/ci.yml` does not run any of the three
(Cargo.lock has zero external entries, so the supply-chain
gates would be no-ops; the QEMU smoke is maintainer-launched).
Each gate now carries an inline parenthetical noting the
conditional ("currently dormant: zero external dependencies"
/ "maintainer-launched only; no `qemu-smoke` CI job yet").
2. **§Configuration files** previously listed `supply-chain/config.toml`,
`supply-chain/audits.toml`, and `.github/dependabot.yml` as if
present; none exist at HEAD. Split the table into "Present at
HEAD" and "Planned (when first extern dep lands)" sub-headings,
added the trigger pointer to `add-dependency` skill, and added
a sentence noting that the lint set is canonical at
`code-style.md §Lints` so the two standards stay obviously in
sync. The Present table also picks up a one-line note that
`miri` is added on-demand by the CI job, not declared in
`rust-toolchain.toml`'s components array.
Refs: comprehensive-review-2026-05-06, ADR-0018, ADR-0023, ADR-0025,
ADR-0026
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ⓘ You've reached your Qodo monthly free-tier limit. Reviews pause until next month — upgrade your plan to continue now, or link your paid account if you already have one. |
Reviewer's GuideDoc-only PR that brings architecture, glossary, standards, guides, and root docs back in sync with the current kernel/ HAL/ BSP implementation after T-009/T-012/T-013/B0/B1, mainly correcting GIC version, timer/idle behavior, DAIF masking status, ADR indices/status, and top-level project status and layout. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Rate limit exceeded
To continue reviewing without waiting, purchase usage credits 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 (4)
📝 WalkthroughWalkthroughDocumentation updates across multiple files to reflect current project status (end-to-end boot on QEMU aarch64 with two-task IPC demo), refined architectural details (GICv2-based HAL, WFI-idle scheduler), new decision records, glossary expansions, and expanded execution guides. ChangesDocumentation Updates
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="docs/standards/infrastructure.md" line_range="72" />
<code_context>
-- `cargo audit` — fails on known advisories. `cargo-audit` database is updated weekly in CI.
-- `cargo vet check` — fails if any dependency is not audited.
+- QEMU smoke — kernel boots under `qemu-system-aarch64 -machine virt` and reaches the success marker. *(As of 2026-05: maintainer-launched only; no `qemu-smoke` CI job yet — tracked as a B2-or-later roadmap follow-up.)*
+- `cargo audit` — fails on known advisories. `cargo-audit` database is updated weekly in CI. *(Conditional — currently dormant: `Cargo.lock` carries zero external dependencies, so the gate would be a no-op. The job is wired in once the first extern dep lands per [add-dependency](../../.claude/skills/add-dependency/SKILL.md).)*
+- `cargo vet check` — fails if any dependency is not audited. *(Same conditional — see `cargo audit` above.)*
</code_context>
<issue_to_address>
**suggestion (typo):** Consider spelling out “extern dep” as “external dependency” for clarity.
In this sentence, the shorthand stands out against the otherwise consistent use of “dependency/dependencies.” Using “the first external dependency” here would keep terminology consistent and be clearer, especially for new readers.
```suggestion
- `cargo audit` — fails on known advisories. `cargo-audit` database is updated weekly in CI. *(Conditional — currently dormant: `Cargo.lock` carries zero external dependencies, so the gate would be a no-op. The job is wired in once the first external dependency lands per [add-dependency](../../.claude/skills/add-dependency/SKILL.md).)*
```
</issue_to_address>
### Comment 2
<location path="docs/standards/infrastructure.md" line_range="156" />
<code_context>
| `.cargo/config.toml` | Target triples, linker flags per target. |
+| `.github/workflows/*.yml` | CI pipelines. Active jobs at HEAD: `lint-and-host-test`, `kernel-build`, `miri`, `coverage`. |
+
+### Planned (when first extern dep lands)
+
+| File | Purpose |
</code_context>
<issue_to_address>
**suggestion (typo):** Clarify “extern dep” in the heading by expanding it to “external dependency”.
Prefer the full phrase “external dependency” (e.g., “when the first external dependency lands”) to avoid confusing readers who may not recognize the abbreviation.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| | `.cargo/config.toml` | Target triples, linker flags per target. | | ||
| | `.github/workflows/*.yml` | CI pipelines. Active jobs at HEAD: `lint-and-host-test`, `kernel-build`, `miri`, `coverage`. | | ||
|
|
||
| ### Planned (when first extern dep lands) |
There was a problem hiding this comment.
suggestion (typo): Clarify “extern dep” in the heading by expanding it to “external dependency”.
Prefer the full phrase “external dependency” (e.g., “when the first external dependency lands”) to avoid confusing readers who may not recognize the abbreviation.
![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 updates the project documentation to reflect the completion of Phase A and B0/B1 milestones, specifically the successful end-to-end kernel boot on QEMU virt. Key changes include updating the workspace layout in the README, refining HAL and scheduler documentation to reflect the implementation of timer IRQs and WFI-based idle tasks, and closing security model items related to early IRQ masking. Feedback focuses on improving documentation maintainability by removing hardcoded commit hashes and line numbers, and correcting a description of task behavior in the cooperative scheduler to ensure it accurately reflects the need for yielding to avoid starvation.
| - [ADR-0019: Scheduler shape](../decisions/0019-scheduler-shape.md) — cooperative single-core FIFO; ready queue is a fixed-capacity bounded `SchedQueue` indexed against a per-task arena. Capacity is `TASK_ARENA_CAPACITY` (currently 16). | ||
| - [ADR-0020: `ContextSwitch` trait and `Cpu` v2](../decisions/0020-cpu-trait-v2-context-switch.md) — register save/restore is a HAL primitive; the scheduler holds the per-task context array and never inspects its contents. | ||
| - [ADR-0022: Idle task and typed scheduler deadlock error](../decisions/0022-idle-task-and-typed-scheduler-deadlock.md) — the BSP registers an idle task at boot so the FIFO is never structurally empty; `SchedError::Deadlock` survives as a defensive return for preemption / SMP / a misconfigured BSP. ADR-0022's first rider clarifies that the idle task's body uses `core::hint::spin_loop` until [T-012](../analysis/tasks/phase-b/T-012-exception-and-irq-infrastructure.md) wires the timer IRQ. | ||
| - [ADR-0022: Idle task and typed scheduler deadlock error](../decisions/0022-idle-task-and-typed-scheduler-deadlock.md) — the BSP registers an idle task at boot so the FIFO is never structurally empty; `SchedError::Deadlock` survives as a defensive return for preemption / SMP / a misconfigured BSP. The idle task's body uses `cpu.wait_for_interrupt()` followed by `yield_now`; the time-source half (T-009) and IRQ-delivery half (T-012) of ADR-0010 are both live, so WFI is the production form. The interim `core::hint::spin_loop()` shape ADR-0022's first rider introduced was retired by T-012 (commit `b4ed68c`); see ADR-0022 §Revision notes 2026-04-28 for the closure of the first rider's *Sub-rider* under [UNSAFE-2026-0019](../audits/unsafe-log.md) / [UNSAFE-2026-0020](../audits/unsafe-log.md) / [UNSAFE-2026-0021](../audits/unsafe-log.md). |
There was a problem hiding this comment.
Hardcoding commit hashes in architectural documentation creates a maintenance burden and can lead to broken or misleading links if the repository history is rewritten (e.g., via rebase). It is generally safer to refer to stable identifiers like Task IDs (T-012) or ADRs.
Suggested change
| - [ADR-0022: Idle task and typed scheduler deadlock error](../decisions/0022-idle-task-and-typed-scheduler-deadlock.md) — the BSP registers an idle task at boot so the FIFO is never structurally empty; `SchedError::Deadlock` survives as a defensive return for preemption / SMP / a misconfigured BSP. The idle task's body uses `cpu.wait_for_interrupt()` followed by `yield_now`; the time-source half (T-009) and IRQ-delivery half (T-012) of ADR-0010 are both live, so WFI is the production form. The interim `core::hint::spin_loop()` shape ADR-0022's first rider introduced was retired by T-012 (commit `b4ed68c`); see ADR-0022 §Revision notes 2026-04-28 for the closure of the first rider's *Sub-rider* under [UNSAFE-2026-0019](../audits/unsafe-log.md) / [UNSAFE-2026-0020](../audits/unsafe-log.md) / [UNSAFE-2026-0021](../audits/unsafe-log.md). | |
| - [ADR-0022: Idle task and typed scheduler deadlock error](../decisions/0022-idle-task-and-typed-scheduler-deadlock.md) — the BSP registers an idle task at boot so the FIFO is never structurally empty; `SchedError::Deadlock` survives as a defensive return for preemption / SMP / a misconfigured BSP. The idle task's body uses `cpu.wait_for_interrupt()` followed by `yield_now`; the time-source half (T-009) and IRQ-delivery half (T-012) of ADR-0010 are both live, so WFI is the production form. The interim `core::hint::spin_loop()` shape ADR-0022's first rider introduced was retired by T-012; see ADR-0022 §Revision notes 2026-04-28 for the closure of the first rider's *Sub-rider* under [UNSAFE-2026-0019](../audits/unsafe-log.md) / [UNSAFE-2026-0020](../audits/unsafe-log.md) / [UNSAFE-2026-0021](../audits/unsafe-log.md). |
| ### Idle task and structural non-emptiness | ||
|
|
||
| ADR-0022 mandates that the BSP register a single, lowest-priority idle task at boot. Its presence makes the FIFO ready queue structurally non-empty for the lifetime of the kernel. The idle task's body in v1 is `core::hint::spin_loop()` followed by `yield_now`, not `wait_for_interrupt`, because no IRQ source is configured before [T-012](../analysis/tasks/phase-b/T-012-exception-and-irq-infrastructure.md). When T-012 lands, the body switches to `cpu.wait_for_interrupt(); yield_now(...)` and the scheduler's behaviour is unchanged from the outside. | ||
| ADR-0022 mandates that the BSP register a single, lowest-priority idle task at boot. Its presence makes the FIFO ready queue structurally non-empty for the lifetime of the kernel. The idle task's body is `cpu.wait_for_interrupt()` followed by `yield_now` — the production form ADR-0022's *Decision outcome* originally specified. T-012 (commit `b4ed68c`, Done 2026-04-28) closed ADR-0022's first rider's *Sub-rider* by landing both halves of the wake-source precondition (T-009's `CNTVCT_EL0` time source and T-012's GIC v2 + `VBAR_EL1` IRQ delivery), so WFI is now safe to issue under v1; the interim `core::hint::spin_loop()` shape the first rider introduced is retired. Audit citations: [UNSAFE-2026-0019](../audits/unsafe-log.md) (GIC MMIO), [UNSAFE-2026-0020](../audits/unsafe-log.md) (vector table), [UNSAFE-2026-0021](../audits/unsafe-log.md) (timer compare-register writes). See [`docs/architecture/exceptions.md` §"Idle's `wfi` activation"](exceptions.md) for the cross-cutting picture. |
There was a problem hiding this comment.
As noted previously, hardcoding commit hashes in documentation is prone to becoming stale. Consider removing the specific hash and relying on the Task ID (T-012) and date for reference.
Suggested change
| ADR-0022 mandates that the BSP register a single, lowest-priority idle task at boot. Its presence makes the FIFO ready queue structurally non-empty for the lifetime of the kernel. The idle task's body is `cpu.wait_for_interrupt()` followed by `yield_now` — the production form ADR-0022's *Decision outcome* originally specified. T-012 (commit `b4ed68c`, Done 2026-04-28) closed ADR-0022's first rider's *Sub-rider* by landing both halves of the wake-source precondition (T-009's `CNTVCT_EL0` time source and T-012's GIC v2 + `VBAR_EL1` IRQ delivery), so WFI is now safe to issue under v1; the interim `core::hint::spin_loop()` shape the first rider introduced is retired. Audit citations: [UNSAFE-2026-0019](../audits/unsafe-log.md) (GIC MMIO), [UNSAFE-2026-0020](../audits/unsafe-log.md) (vector table), [UNSAFE-2026-0021](../audits/unsafe-log.md) (timer compare-register writes). See [`docs/architecture/exceptions.md` §"Idle's `wfi` activation"](exceptions.md) for the cross-cutting picture. | |
| ADR-0022 mandates that the BSP register a single, lowest-priority idle task at boot. Its presence makes the FIFO ready queue structurally non-empty for the lifetime of the kernel. The idle task's body is `cpu.wait_for_interrupt()` followed by `yield_now` — the production form ADR-0022's *Decision outcome* originally specified. T-012 (Done 2026-04-28) closed ADR-0022's first rider's *Sub-rider* by landing both halves of the wake-source precondition (T-009's `CNTVCT_EL0` time source and T-012's GIC v2 + `VBAR_EL1` IRQ delivery), so WFI is now safe to issue under v1; the interim `core::hint::spin_loop()` shape the first rider introduced is retired. Audit citations: [UNSAFE-2026-0019](../audits/unsafe-log.md) (GIC MMIO), [UNSAFE-2026-0020](../audits/unsafe-log.md) (vector table), [UNSAFE-2026-0021](../audits/unsafe-log.md) (timer compare-register writes). See [`docs/architecture/exceptions.md` §"Idle's `wfi` activation"](exceptions.md) for the cross-cutting picture. |
| - **Concrete bounds** for the quotas under *Bounded kernel resources*: numeric defaults for each, per-target tuning policy, and how upgrades change them without invalidating running systems. | ||
| - **Cross-table capability derivation tree (CDT).** Whether IPC-transferred capabilities should retain a parent-child link to the sender's entry so that the sender can revoke the copy post-transfer, and — if so — how per-task-table CDT storage scales. seL4's answer is a whole-system CDT; Phase B needs to decide before the first multi-task system uses transfer as a revoke-retained grant. See the v1 qualification on *Revocation is transitive* above. | ||
| - **Early IRQ masking in BSP reset vectors.** v1's `boot.s` on QEMU `virt` does not explicitly `msr daifset, #0xf` before stack / BSS setup — the reset state happens to leave interrupts effectively masked (no IRQ source is configured and the GIC is untouched), but this is a per-platform accident rather than a guarantee. Future BSPs should adopt "mask DAIF first, everything else after" as a standard reset-vector prologue; add the instruction to the [BSP boot checklist](../standards/bsp-boot-checklist.md) and audit the existing checklist at that time. | ||
| - **Early IRQ masking in BSP reset vectors.** ✅ **Closed by [T-013](../analysis/tasks/phase-b/T-013-el-drop-to-el1.md) (Done 2026-04-27, ADR-0024, [UNSAFE-2026-0017](../audits/unsafe-log.md)).** `bsp-qemu-virt/src/boot.s:84` now begins `_start` with `msr daifset, #0xf` as the **literal first instruction** before stack/BSS setup, and the [BSP boot checklist §1a](../standards/bsp-boot-checklist.md) records "mask DAIF first" as a standard reset-vector prologue every future BSP must observe. The previous "per-platform accident" framing is retired; DAIF masking is now a structural property of every Tyrne reset vector, with the audit trail captured under UNSAFE-2026-0017's first Amendment block. Future BSPs (`bsp-pi4`, etc.) inherit the rule via the boot checklist. |
There was a problem hiding this comment.
Hardcoding line numbers in documentation is prone to drift. Since the instruction is at the beginning of the _start symbol, it is more robust to refer to the symbol itself rather than a specific line number in boot.s.
Suggested change
| - **Early IRQ masking in BSP reset vectors.** ✅ **Closed by [T-013](../analysis/tasks/phase-b/T-013-el-drop-to-el1.md) (Done 2026-04-27, ADR-0024, [UNSAFE-2026-0017](../audits/unsafe-log.md)).** `bsp-qemu-virt/src/boot.s:84` now begins `_start` with `msr daifset, #0xf` as the **literal first instruction** before stack/BSS setup, and the [BSP boot checklist §1a](../standards/bsp-boot-checklist.md) records "mask DAIF first" as a standard reset-vector prologue every future BSP must observe. The previous "per-platform accident" framing is retired; DAIF masking is now a structural property of every Tyrne reset vector, with the audit trail captured under UNSAFE-2026-0017's first Amendment block. Future BSPs (`bsp-pi4`, etc.) inherit the rule via the boot checklist. | |
| - **Early IRQ masking in BSP reset vectors.** ✅ **Closed by [T-013](../analysis/tasks/phase-b/T-013-el-drop-to-el1.md) (Done 2026-04-27, ADR-0024, [UNSAFE-2026-0017](../audits/unsafe-log.md)).** The `_start` symbol in `bsp-qemu-virt/src/boot.s` now begins with `msr daifset, #0xf` as the **literal first instruction** before stack/BSS setup, and the [BSP boot checklist §1a](../standards/bsp-boot-checklist.md) records "mask DAIF first" as a standard reset-vector prologue every future BSP must observe. The previous "per-platform accident" framing is retired; DAIF masking is now a structural property of every Tyrne reset vector, with the audit trail captured under UNSAFE-2026-0017's first Amendment block. Future BSPs (`bsp-pi4`, etc.) inherit the rule via the boot checklist. |
| ``` | ||
|
|
||
| After printing "all tasks complete", Task A enters a `core::hint::spin_loop()`. The kernel halts in this state; QEMU continues running but produces no further output. The specific instruction the hint lowers to is up to the compiler and is not load-bearing — a dedicated `wfe`-based idle path is Phase B work (see ADR-0019 open questions). Terminate with **Ctrl-A x** (QEMU monitor quit). | ||
| The frequency in `timer ready` (`62500000 Hz`) and the elapsed-time figure on the last line are QEMU-default values; on real hardware the frequency varies (see [ADR-0010 §References](../decisions/0010-timer-trait.md)) and the elapsed time depends on the host's QEMU performance — single-digit-millisecond order is typical on modern laptops. After printing "all tasks complete" and the boot-to-end timing line, Task A enters a `core::hint::spin_loop()`. Idle is registered separately via [`register_idle`](../../kernel/src/sched/mod.rs) per [ADR-0026](../decisions/0026-idle-dispatch-fallback.md), so once both application tasks are stuck in their spin tails the dispatcher's fallback resolves to idle, which issues `wait_for_interrupt` (`WFI`); QEMU continues running but produces no further serial output. Terminate with **Ctrl-A x** (QEMU monitor quit). |
There was a problem hiding this comment.
In a cooperative scheduler, a task entering a core::hint::spin_loop() without yielding will starve the system, preventing the dispatcher from ever reaching the idle task. The description of the fallback resolving to idle implies that the application tasks must yield in their "spin tails". Consider updating the description to clarify that these tasks yield.
Suggested change
| The frequency in `timer ready` (`62500000 Hz`) and the elapsed-time figure on the last line are QEMU-default values; on real hardware the frequency varies (see [ADR-0010 §References](../decisions/0010-timer-trait.md)) and the elapsed time depends on the host's QEMU performance — single-digit-millisecond order is typical on modern laptops. After printing "all tasks complete" and the boot-to-end timing line, Task A enters a `core::hint::spin_loop()`. Idle is registered separately via [`register_idle`](../../kernel/src/sched/mod.rs) per [ADR-0026](../decisions/0026-idle-dispatch-fallback.md), so once both application tasks are stuck in their spin tails the dispatcher's fallback resolves to idle, which issues `wait_for_interrupt` (`WFI`); QEMU continues running but produces no further serial output. Terminate with **Ctrl-A x** (QEMU monitor quit). | |
| The frequency in `timer ready` (`62500000 Hz`) and the elapsed-time figure on the last line are QEMU-default values; on real hardware the frequency varies (see [ADR-0010 §References](../decisions/0010-timer-trait.md)) and the elapsed time depends on the host's QEMU performance — single-digit-millisecond order is typical on modern laptops. After printing "all tasks complete" and the boot-to-end timing line, Task A enters a loop that calls `yield_now()`. Idle is registered separately via [`register_idle`](../../kernel/src/sched/mod.rs) per [ADR-0026](../decisions/0026-idle-dispatch-fallback.md), so once both application tasks are stuck in their yielding spin tails the dispatcher's fallback resolves to idle, which issues `wait_for_interrupt` (`WFI`); QEMU continues running but produces no further serial output. Terminate with **Ctrl-A x** (QEMU monitor quit). |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/architecture/scheduler.md`:
- Line 11: Update the ADR-0022 passage so it describes the idle task as a
separate fallback consulted only when the ready FIFO is empty (per ADR-0026's
model `ready.dequeue().or(s.idle)`), not as making the FIFO structurally
non-empty; rephrase sentences that currently state the BSP registers an idle
task "so the FIFO is never structurally empty" to instead state the BSP
registers an idle fallback slot used when the ready queue is empty, and apply
the same rewording to the other occurrence noted (line 73); keep references to
`SchedError::Deadlock`, `cpu.wait_for_interrupt()`, `yield_now`, and the retired
`core::hint::spin_loop()` comment intact but ensure they no longer imply FIFO
residency.
🪄 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: 71369e3d-50c5-48fc-8f62-dac0e74b8da5
📒 Files selected for processing (13)
CONTRIBUTING.mdREADME.mdSECURITY.mddocs/architecture/hal.mddocs/architecture/overview.mddocs/architecture/scheduler.mddocs/architecture/security-model.mddocs/decisions/README.mddocs/decisions/template.mddocs/glossary.mddocs/guides/two-task-demo.mddocs/standards/README.mddocs/standards/infrastructure.md
…etation) Address inline + bot comments on PR #13. Each finding verified against current code; 7 applied, 1 applied with corrected interpretation (the suggested fix did not match the source). ## Applied 1. **scheduler.md line 11 + line 73** — Idle is a fallback slot, not a FIFO resident. (User inline comment + coderabbitai-bot + gemini-code-assist-bot.) Both passages now describe idle per ADR-0026: "lives in a dedicated `Scheduler::idle: Option<TaskHandle>` fallback slot — not in the FIFO ready queue — and the dispatcher consults it only when the ready queue is empty (`ready.dequeue().or(s.idle)`)". The "structurally non-empty FIFO" framing was inherited from ADR-0022 Option A and contradicted the same file's later §Revision notes that already records the ADR-0026 supersession. Both passages also drop the hardcoded commit `b4ed68c` per gemini's suggestion (commit hashes drift when history is rewritten; T-012 + Done date is the stable identifier). 2. **security-model.md line 330** — Replace `boot.s:84` line ref with `_start` symbol reference. (gemini-code-assist-bot.) Line numbers in `boot.s` drift on every asm edit; the `_start` symbol is stable and the rule is "literal first instruction of `_start`", so the reference now reads "The `_start` symbol in `bsp-qemu-virt/src/boot.s` now begins with `msr daifset, #0xf` as the **literal first instruction**". 3. **two-task-demo.md line 47** — Honestly describe what happens after "all tasks complete". (gemini-code-assist-bot, with **corrected interpretation**.) gemini's suggested fix said "Task A enters a loop that calls `yield_now()`" — but that does not match the actual source. `bsp-qemu-virt/src/main.rs:486` ends task_a with `loop { core::hint::spin_loop(); }` (no `yield_now`). Under a cooperative scheduler this means Task A holds the CPU forever and idle is **structurally never dispatched** in this demo. Doc text updated to match: "Task A enters `loop { core::hint::spin_loop() }`. The cooperative scheduler does not preempt; Task A never yields out of this loop, so the CPU stays parked there indefinitely … idle is structurally unreached and the WFI path stays cold." Idle's fallback-slot wiring is still correctly described per ADR-0026 — it is reachable in principle, just not exercised by this demo's tail behaviour. 4. **infrastructure.md line 72 + line 156** — Spell out "extern dep" as "external dependency". (sourcery-ai-bot ×2 + matches user's inline message.) Two occurrences in the file (line 72's `cargo audit` conditional note and line 156's "Planned" sub-heading); both replaced via a `replace_all` substitution. The body of the Planned section already said "external dependency" / "external dependencies" consistently; the abbreviation was an outlier. ## Verification `cargo fmt --check` clean (no source changes); doc-only PR. Diff stat: 4 files, +6/-6. Refs: PR #13, comprehensive-review-2026-05-06, ADR-0026 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
6 tasks
cemililik
added a commit
that referenced
this pull request
May 7, 2026
…2 prep activated Closes B1 — Drop to EL1 + exception infrastructure. The fresh closure trio replaces the 2026-04-28 trio's load-bearing role: that trio approved B1 implementation-complete based on host-test + miri + paper-review evidence; the maintainer-side QEMU smoke (still `Pending QEMU smoke verification` on UNSAFE-2026-0019/0020/0021 at the time) had not run. When the maintainer ran it on 2026-05-06, the smoke surfaced an idle-dispatch regression. T-014 fixed the regression; the comprehensive multi-agent code review (also 2026-05-06) generated α/β/γ doc-polish PRs; today's trio records what B1 actually is once smoke-verified end-to-end. ## Three new review artefacts - docs/analysis/reviews/business-reviews/2026-05-07-B1-closure.md — Period 2026-04-28 → 2026-05-07. What landed (T-014 + ADR-0026 + PRs #12 / #13 / #14 / #15); what changed in the plan (B1 reopen → T-014 fix → fresh closure; B2 prep reactivated; ADR-0026 repurposed); what we learned (smoke is the project's only end-to- end liveness oracle; ADR analysis must simulate, not just argue; comprehensive review's blind spot was "did you actually run the program?"; bot-driven review-rounds are productive when findings are factual-mechanical, less so when stylistic). Adjustments include "no closure-trio without recorded smoke", write-adr skill simulation-table check, comprehensive-review Track K — Live execution. - docs/analysis/reviews/security-reviews/2026-05-07-B1-closure.md — Eight axes, all OK. ADR-0026 / T-014 introduce no new attack surface, capability widening, memory-safety hazard, or threat- model shift. UNSAFE-2026-0014 third Amendment for register_idle; UNSAFE-2026-0019/0020 partial-verification + post-T-014 smoke Amendments; UNSAFE-2026-0021 no-verification Amendment. Eight inherited forward-flagged items unchanged at original severity. Verdict: Approve. - docs/analysis/reviews/performance-optimization-reviews/2026-05-07-B1-closure.md — Re-baseline. Net footprint-neutral vs 2026-04-28: .text 21,792 bytes (-116), .rodata 2,928 (+144), .bss 22,256 (+8). The +144 .rodata is panic-message clarity strings; the +8 .bss is idle: Option<TaskHandle>. Smoke 5.5–6.5 ms boot-to-end, zero events. 11 P-numbered proposals from Track D remain queued (P3 partially landed by γ; P1 / P10 / P4 highest-ROI near-term). No proposals to merge this cycle. Verdict: Merge. ## Status flips + index updates - T-014 In Review → Done. T-014 user-story file's review-history gains row 4 recording the maintainer's independent verification and the closure trio's landing. - docs/analysis/tasks/phase-b/README.md — T-014 row to Done. - docs/roadmap/phases/phase-b.md — sub-breakdown item 7 (T-014) flipped to Done; B1 status block rewritten ("B1 closed 2026-05-07") with citations to the three new review artefacts. - docs/roadmap/current.md — top callout rewritten to record B1 truly closed (2026-05-07); active phase remains B; active milestone advances to B2 (MMU activation); active task cleared (B2 prep / ADR-0027 drafting opens next per ADR-0025 §Rule 1); audit status footnote gains the 2026-05-07 update. - The three review-folder README index tables (business / security / performance) gain 2026-05-07-B1-closure rows. ## Verification recap - cargo fmt --check, cargo host-clippy -D warnings, cargo kernel-clippy -D warnings, cargo kernel-build — all clean. - cargo host-test 25 + 93 + 34 = 152/152. - cargo +nightly miri test 152/152 clean. - QEMU smoke at HEAD e9fa019 reproduces the full demo trace + the boot-to-end elapsed = ... line; -d int,unimp,guest_errors empty for the entire ~5.8 ms run. ## What stays open for δ + B2 prep - δ — write ADR-0023 placeholder file with Status: Deferred body (the README index gained the row in α; the file body is δ's job). - δ — endpoint rollback / ipc_cancel_recv ADR before B2 lands the first userspace destroy path (Track A non-blocker; SchedError::Deadlock rollback leaves endpoint in RecvWaiting). - B2 prep — ADR-0027 (kernel virtual memory layout) drafting + docs/architecture/memory-management.md design-first. The ADR's Dependency chain opens T-015 in the same commit per ADR-0025 §Rule 1. Refs: ADR-0026, ADR-0022, ADR-0025, T-014, B1 closure trio Audit: UNSAFE-2026-0014, UNSAFE-2026-0019, UNSAFE-2026-0020, UNSAFE-2026-0021 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
4 tasks
cemililik
added a commit
that referenced
this pull request
May 7, 2026
Closes δ.1 of the post-B1-closure follow-up arc. ADR-0023's slot was recorded as "accept-deferred" at the B0 closure (2026-04-27), but the file did not exist; the README index gained a no-link row in α (PR #13), and the glossary's CDT entry pointed at the missing path. This commit lands the placeholder body so: - The in-tree references that mention ADR-0023 (glossary, phase-b.md ledger, decisions/README.md index, the B0 closure security review prose) resolve to a citable artefact instead of a 404. - The deferral conditions are recorded at the slot itself rather than scattered across review prose; future agents recognise when the trigger fires (multi-task server arc; first userspace driver with sub-cap delegation; threat-model escalation). - The four-option sketch (whole-system CDT, per-table + back-pointer, explicit revoke-token, indefinite defer to userspace) gives a future ADR author a starting analysis instead of a blank page. Status remains `Deferred`. The body is a placeholder, not a load- bearing decision; when a trigger fires, the body gets rewritten in place with a Status flip from `Deferred` to `Proposed` (then `Accepted`). The append-only rule that protects original Accepted bodies does not apply to Deferred placeholders. ## Files changed - docs/decisions/0023-cross-table-capability-revocation-policy.md (new) — placeholder body with deferral conditions, options sketch, references. - docs/decisions/README.md — index row gains the file link (was a prose-only no-link row from α). - docs/glossary.md — CDT entry's "no file at HEAD" qualification removed; clean link to the placeholder. Refs: ADR-0023, ADR-0014, ADR-0017, B0 closure security review 2026-04-27 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes the doc-state-vs-code drift that the 2026-05-06 comprehensive multi-agent code review surfaced as Track E: Request changes — 7 blocker-class items + ~10 non-blocker drift items. None of the items requires a code change; all are factual statements that fell out of sync with the code at HEAD as Phase A → B0 → B1 progressed and individual edits were missed in the doc sweep that should have followed each landing.
This is the α in the post-B1 fix-arc plan (α = Track E doc fixes; β = bulk URL +
Yükseksweep; γ = code-side small polish; then B1 closure trio; then B2 prep). β / γ follow in their own PRs.Commits
d2a4ab1docs(architecture,glossary)— closes the 7 Track-E blockers:overview.mdBSP table: GICv3 → GICv2 (with a one-line clarifier on-machine gic-version=3).hal.mdMermaid box:BIrq["GICv3 / GIC-400 impl"]→"GICv2 / GIC-400 impl".hal.mdBSP table interrupt-controller row: GICv3 → GICv2.hal.mdTimer subsection:unimplemented!()framing replaced with the post-T-012 reality + ADR-0010 §Revision notes citation + UNSAFE-2026-0021 cross-reference.scheduler.mdtwo passages: idle bodyspin_loopframing →wait_for_interruptreality post-T-012 + UNSAFE-2026-0019/0020/0021 citations.security-model.mdOpen question on DAIF mask: closed by T-013 / ADR-0024 / UNSAFE-2026-0017 (boot.s now masks DAIF as the literal first instruction).glossary.mdADR-0023 dead-link replaced with prose ("see ADR-0023 (accept-deferred per Phase B0 closure)").16ba195docs(root)— refresh root docs for post-B1 reality:README.mdrepository-layout tree addskernel/,hal/,test-hal/,bsp-qemu-virt/,tools/+ the docs/ subdirectories with crate-name annotations; the "Source code layout will be added after the architecture phase" closing sentence is replaced with a real status-of-the-tree note.CONTRIBUTING.md"no source code to extend or refactor meaningfully yet" → realistic guidance pointing readers atcurrent.mdfor active milestone scope.SECURITY.md"no runnable kernel yet" + "(planned, Phase 2)" threat-model qualifiers replaced with the actual status (kernel boots end-to-end on QEMU; security-model.md is Accepted).f710881docs(standards,decisions,guides)— non-blocker doc-drift cluster:standards/README.mdadds the missingbsp-boot-checklist.mdrow.decisions/README.mdadds the missing ADR-0023 row (Status: Deferred, no file link) so the index no longer jumps 0022 → 0024.decisions/template.mdadds a comment block enumerating the five recognised status values (incl. Deferred).architecture/overview.md"(final form documented inhal.md, planned)" → "Accepted".guides/two-task-demo.mdexpected-output table gains the post-T-009timer readyandboot-to-end elapsedlines + an idle/fallback-slot mention; per-line table gains rows for both new lines.standards/infrastructure.md§CI Required gates now annotates each as conditional or maintainer-launched; §Configuration files split into "Present at HEAD" + "Planned (when first extern dep lands)" with acode-style.md §Lintsback-pointer for sync.Diff stat
No code changes. No source compilation impact.
Test plan
d2a4ab1and confirms the new prose matches the actual source-of-truth file (bsp-qemu-virt/src/gic.rsfor GIC version,bsp-qemu-virt/src/main.rs:276for idle WFI,bsp-qemu-virt/src/boot.s:84for the DAIF mask, etc.).cargo fmt --checkclean (verified — no Rust changes; doc-only PR).cargo host-test,cargo +nightly miri test,cargo kernel-build,cargo host-clippy,cargo kernel-clippyall expected to remain green (no source changes).decisions/README.md's new ADR-0023 row matchesphase-b.md's ADR ledger (both should now agree on Deferred state).Out of scope (deferred to subsequent PRs)
Yükseksweep (J-NB1, J-NB2): 60+ stalecemililik/TyrneOSrustdoc/manifest URLs across 27 files →cemililik/Tyrne; 7Yüksek→Highin committed English docs. Mechanical, single sweep commit each.Aarch64TaskContextsize guard,SchedQueue<0>const_assert,CAP_TABLE_CAPACITYconst-block migration,ContextSwitch +Send+Sync, three obj-test lint-allow blocks, irq_entry redundant fence +_framedoc + kernel_entry defensive loop, vectors.s ↔ TrapFrame size-assert cross-reference comment. ~9 files, kernel/host-test/miri rerun required.The 7 doc-drift blockers + non-blocker cluster in this PR are the single largest doc-side debt the comprehensive review identified. Code-side findings (Tier 4 small polish) and bulk URL/Yüksek sweep (Tier 3) are explicitly out of scope here to keep this PR reviewable doc-only.
🤖 Generated with Claude Code
Summary by Sourcery
Align documentation with current post-B1 kernel reality and architecture decisions without changing code.
New Features:
Enhancements:
Summary by CodeRabbit