docs: ADR-0035 + T-017 — B3 prep: Physical Memory Manager (bitmap allocator) - #25
Merged
Merged
Conversation
…uisite — bitmap allocator) + open T-017
B3 prep arc opens. ADR-0035 is the load-bearing design ADR for the
PMM that B3's address-space abstraction (B3 §3 Map/unmap operations
+ B3 §2 AddressSpace kernel object) consumes. Lands at status
Proposed; the careful re-read + Accept follow as separate commits
per write-adr skill §10.
What's settled (ADR-0035):
- **Bitmap allocator with hint pointer.** One bit per frame; 4 KiB
metadata for QEMU virt's 32,768 frames (128 MiB / 4 KiB). Linear
scan from hint with rewind-on-free; O(N) worst-case, O(1) typical
under v1's no-fragmentation pressure.
- **Reservation list at init.** `Pmm::new(extent, reserved)` is
one-shot; walks the reserved-range list and marks every covered
frame's bit. v1 reservations: kernel image + .boot_pt + boot
stack. The single-bit Reserved-vs-Allocated collapse forces the
discrimination into the kernel code that calls `Pmm::new`
(no caller ever holds a `PhysFrame` covering a Reserved range);
trade-off recorded in §Negative consequences.
- **`FrameProvider` impl.** ADR-0009's trait surface unchanged;
the PMM is the first real impl outside the host-test
`VecFrameProvider`.
- **Forward-portable to high-half kernel.** Bitmap addressing is
regime-independent; the future ADR-0033 placeholder relocates
the bitmap base address but does not require a `PA → VA`
translation discipline at the algorithm level. Rejected
alternatives (in-frame linked free-list — Option C) would have
baked in the identity-only assumption.
- **Zero new `unsafe` audit entries.** Bitmap operations are safe
Rust; the frame-zeroing site joins UNSAFE-2026-0001's umbrella
via a future Amendment (lands with T-017's implementation).
§Considered options surveyed: A bitmap (chosen), B external free-
list (32× metadata; rejected), C in-frame linked free-list
(regime-dependent; rejected), D buddy allocator (overkill for v1's
single-size; rejected).
§Simulation table (5 rows): walks init / alloc / free / exhaustion
/ stats state transitions through the chosen Option A shape, with
the Reserved-vs-Allocated collapse called out as the critical
correctness subtlety in row 2.
§Dependency chain: opens [T-017 — Physical Memory Manager (PMM)
bring-up](../analysis/tasks/phase-b/T-017-physical-memory-manager.md)
in this commit per ADR-0025 §Rule 1 (forward-reference contract).
T-017 is a single bundled task (mirrors T-016 shape) covering:
new `kernel/src/mm/pmm.rs` module + `bsp-qemu-virt::main.rs`
PMM publication wiring + 8 host tests + UNSAFE-2026-0001 Amendment
+ docs/architecture/memory-management.md + boot.md cross-links.
Smoke trace gains exactly one new line (`tyrne: pmm initialized
(N frames available; M reserved)`).
Cross-references propagated:
- docs/decisions/README.md — ADR index gains the ADR-0035 row at
status Proposed.
- docs/roadmap/phases/phase-b.md — B3 §Status block flips to "B3
prep active 2026-05-09"; sub-breakdown extended to seat ADR-0035
ahead of ADR-0028 (address-space data structure); ADR ledger
gains the 0035 row.
- docs/roadmap/current.md — Active milestone flips to B3
("Address space abstraction"); Active task points at T-017 Draft.
- docs/analysis/tasks/phase-b/README.md — T-016 status flipped to
Done; T-017 row added.
- docs/analysis/tasks/phase-b/T-016-mmu-activation.md §Informs —
bidirectional link to T-017 (forward-reference now grounded
per ADR-0025 §Rule 1).
- docs/architecture/memory-management.md §"Frame allocation
discipline" — references ADR-0035 + T-017 with the post-T-017
state framing ("when T-017 lands, the discipline stays
unchanged").
Verification:
- cargo fmt --check clean
- cargo test 185/185 host (unchanged — no code
touched, ADRs + tasks are docs)
- cargo kernel-build clean
Refs: ADR-0035, T-017
Careful-re-read pass per write-adr skill §10 on Propose commit 7a6cc27. Surfaced + closed three substantive drafting issues before flipping Status to Accepted: 1. **§Context L13 — empty `[ADR-0028](#)` anchor.** ADR-0028 is reserved per ADR-0027 §Context but has no file today; the broken `#` link would have rendered as a clickable-but-dead reference. Fixed: replaced with plain-text "The ADR-0028 slot is reserved... no file today, opens with the second B3 ADR". 2. **§Decision drivers L35 vs §Decision outcome L58 contradiction.** L35 had said "if the PMM can zero via safe `core::slice::fill` ... no `unsafe` is involved"; L58 had said the zeroing IS `unsafe` via `core::ptr::write_bytes`. Materialising a `&mut [u8; 4096]` slice from a raw pointer is itself an `unsafe` step (`core::slice::from_raw_parts_mut`), so the L35 claim was false. Fixed: rewrote L35 to acknowledge that raw `write_bytes` is the more honest expression and that the slice-construction "alternative" is not actually safe-Rust. 3. **§Simulation row 2 — muddled "undefined-vs-error" wording.** The row had said the bitmap "either tracks two bitmaps or accepts that 'double free of a Reserved frame is undefined' (we choose the latter — `PmmError::DoubleFree` reports)" — defined-as-error and undefined-behavior at once. The row also relied on a mechanism (consulting the bitmap bit alone) that cannot detect Reserved-vs-Allocated since the bitmap collapses them. Fixed: rewrote row 2 to commit explicitly to the defensive validation pattern — the PMM caches a small list of reserved ranges (`MAX_RESERVED_RANGES = 8`) and `free_frame` does an O(R) scan to reject Reserved-frame-PA inputs with `PmmError::DoubleFree` before mutating the bitmap. The fix propagated into T-017's `Pmm` struct fields list (added `reserved_ranges` field) + `Pmm::new` signature (now returns `Result<Self, PmmError>` to handle `TooManyReservedRanges`) + `free_frame` description + 2 new host tests (`new_rejects_too_many_reserved_ranges` + `free_frame_rejects_pa_in_reserved_range`). Cross-references propagated: - docs/decisions/README.md — ADR-0035 row flipped Proposed → Accepted. - docs/roadmap/phases/phase-b.md — B3 §Status text + sub-breakdown item 1 + ADR ledger 0035 row all reflect Accepted state. - docs/roadmap/current.md — Active milestone line records ADR-0035 Accepted; T-017 still Draft (moves to In Progress on PR-merge per the established convention). T-017's expected host-test count rises from ~193 to ~195 (8 → 10 new PMM tests; the +2 are the row-2-defensive-validation guards). Verification: - cargo fmt --check clean - cargo test 185/185 host (unchanged — docs only) - cargo kernel-build clean Refs: ADR-0035, T-017
ⓘ You've reached your Qodo monthly free-tier limit. Reviews pause until next month — upgrade your plan to continue now, or link your paid account if you already have one. |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @cemililik, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
|
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 (8)
✨ 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 |
![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 introduces the design and implementation plan for the Physical Memory Manager (PMM) as a prerequisite for the B3 milestone. It adds ADR-0035, which selects a bitmap allocator with a hint pointer for tracking physical frames, and T-017, a bundled task covering the PMM module, BSP wiring, and host tests. Feedback focuses on strengthening the PMM's robustness by suggesting explicit validation for page alignment and range containment in the constructor and free_frame logic to prevent potential out-of-bounds access or calculation errors.
| - `allocated_count: usize` — cached count of frames in `Allocated` state. | ||
| - [ ] **`PMM_BITMAP_BYTES` const** parameterised over the BSP's frame count. For QEMU virt's 128 MiB / 4 KiB = 32 768 frames, `PMM_BITMAP_BYTES = 4096`. The kernel crate exposes the type as `Pmm<const N: usize>` with `N = PMM_BITMAP_BYTES`; the BSP provides the const at instantiation time. | ||
| - [ ] **`MAX_RESERVED_RANGES` const** = 8 (kernel-crate constant; same across BSPs). Caps the size of the `reserved_ranges` array. v1 uses 3 entries; the headroom accommodates future BSP layouts without unbounded metadata. `Pmm::new` returns `Err(PmmError::TooManyReservedRanges)` if the caller passes more. | ||
| - [ ] **`Pmm::new(extent, reserved)`** constructor — `pub fn new(extent: PhysFrameRange, reserved: &[PhysFrameRange]) -> Result<Self, PmmError>`. Validates `reserved.len() <= MAX_RESERVED_RANGES` (returns `Err(TooManyReservedRanges)` otherwise); walks the reserved-range list and sets every covered frame's bit to 1 (Reserved); copies the list into the `reserved_ranges` array for `free_frame`'s defensive validation; sets `hint` to the first frame *not* in any reserved range; computes `free_count` / `reserved_count` / `allocated_count` initial values. **Safe Rust**; no `unsafe`. Per [ADR-0035 §Simulation §Step 0](../../../decisions/0035-physical-memory-manager.md#simulation). |
There was a problem hiding this comment.
The Pmm::new constructor should explicitly validate that the extent is page-aligned (both start and end) and that all reserved ranges are within the managed extent. If these invariants are violated, it should return Err(PmmError::MisalignedAddress) or Err(PmmError::OutOfRange) respectively to prevent index calculation errors or out-of-bounds bitmap access during initialization.
| - [ ] **`MAX_RESERVED_RANGES` const** = 8 (kernel-crate constant; same across BSPs). Caps the size of the `reserved_ranges` array. v1 uses 3 entries; the headroom accommodates future BSP layouts without unbounded metadata. `Pmm::new` returns `Err(PmmError::TooManyReservedRanges)` if the caller passes more. | ||
| - [ ] **`Pmm::new(extent, reserved)`** constructor — `pub fn new(extent: PhysFrameRange, reserved: &[PhysFrameRange]) -> Result<Self, PmmError>`. Validates `reserved.len() <= MAX_RESERVED_RANGES` (returns `Err(TooManyReservedRanges)` otherwise); walks the reserved-range list and sets every covered frame's bit to 1 (Reserved); copies the list into the `reserved_ranges` array for `free_frame`'s defensive validation; sets `hint` to the first frame *not* in any reserved range; computes `free_count` / `reserved_count` / `allocated_count` initial values. **Safe Rust**; no `unsafe`. Per [ADR-0035 §Simulation §Step 0](../../../decisions/0035-physical-memory-manager.md#simulation). | ||
| - [ ] **`Pmm::alloc_frame() -> Option<PhysFrame>`** — scans bitmap from `hint` forward for a 0 bit; on hit, sets the bit, zero-fills the 4 KiB frame contents via `core::ptr::write_bytes`, advances `hint`, decrements `free_count`, increments `allocated_count`, returns `Some(PhysFrame)`. On miss (forward + wrap scan both empty), returns `None`. Per [ADR-0035 §Simulation §Steps 1, 3](../../../decisions/0035-physical-memory-manager.md#simulation). | ||
| - [ ] **`Pmm::free_frame(frame: PhysFrame) -> Result<(), PmmError>`** — computes frame index from PA; defensively rejects via O(R) scan of `reserved_ranges` if the frame falls in any reserved range (returns `Err(PmmError::DoubleFree)` without mutation); reads the bit — if `0` (already Free), returns `Err(PmmError::DoubleFree)`; if `1` (Allocated), clears the bit; rewinds `hint = min(hint, i)`; increments `free_count`; decrements `allocated_count`. Per [ADR-0035 §Simulation §Step 2](../../../decisions/0035-physical-memory-manager.md#simulation). The Reserved-vs-Allocated bitmap-collapse plus the explicit reserved-range check is the v1 trade-off: 128 bytes of cached range metadata buys defensive `free_frame(reserved_pa)` rejection without doubling the bitmap. |
There was a problem hiding this comment.
The free_frame logic should include an explicit check that the provided PhysFrame PA is within the managed extent. If it is outside the range, the function should return Err(PmmError::OutOfRange) before attempting to compute the bitmap index or perform the reserved-range scan.
Doc-only fixup commit on the B3-prep PR. Reviewer Approve-with-nits verdict; all 10 actionable findings applied. No merge-blocker; cargo gates unchanged from main (185/185 host; clippy / fmt / kernel-build clean). Applied (Minor): 1. **Finding 3.1 — §Simulation row 3 wrap-scan rationale.** The "two-pass scan ensures correctness even under hint-past-last- free" claim describes a state that cannot occur in v1 (the unconditional rewind in row 2 keeps `hint ≤ lowest-free-index`). Reworded row 3 to honestly frame the wrap as forward-compat scaffolding for the future SMP extension where per-core caches may leave the global hint stale. 2. **Finding 4.1 — UNSAFE-2026-0001 umbrella adjudication-deferred.** The reviewer correctly noted that UNSAFE-2026-0001's actual scope (PL011 MMIO base blessing) is semantically distant from PMM frame-zeroing (raw write_bytes over normal-cached RAM). Added an explicit "adjudication-deferred to T-017's security review" soft-flag in §Dependency chain step 5, propagated to §Decision drivers + §Decision outcome + Option A pros + T-017 §Approach commit 2. The Amendment-vs-new-entry call is now honestly framed as a borderline judgment for T-017's review; either outcome is bounded. 3. **Finding 8.1 — repurposed T-017 test #10.** The original `free_frame_rejects_pa_in_reserved_range` overlapped with test #4 (which already pinned `free_frame(reserved)` → `DoubleFree`). Repurposed to `free_frame_reserved_check_iterates_only_populated_slots` to pin the populated-slots-only contract on the defensive scan (genuinely O(populated-entries), not O(R)). Combined coverage is now: test #4 = "reserved-PA correctly rejected"; test #10 = "non-reserved-PA correctly accepted under partially-populated array". 4. **Finding 8.2 — `MAX_RESERVED_RANGES` per-BSP const generic.** The hardcoded `8` was unjustified and risked TooManyReservedRanges on future BSPs whose typical aarch64 boot-stack reservations (DTB + ATF + ACPI + initrd + framebuffer) could land 7–9 ranges. Promoted to a per-BSP const generic `R` consistent with the existing `PMM_BITMAP_BYTES` `N` parameterisation: the kernel crate exposes `Pmm<const N: usize, const R: usize>`; v1's `bsp-qemu-virt` picks `R = 8`; future BSPs pick what they need. The `expect("...")` BSP panic is structurally unreachable when `R` is sized per the BSP's static reservation list. 5. **Finding 10.1 — Option A regime-independence qualifier.** The bare "regime-independent (high-half-portable)" claim in §Pros didn't carry the qualifier from §Decision outcome / §Positive ("metadata addressing"). Added the same qualifier to §Pros and noted that the frame-zeroing step's `*mut u8` from PA is identity-mapping-dependent today — a `phys_to_virt` helper is needed post-high-half (but the same helper would be needed for the rejected Option C, so Option A's regime-independence advantage at the algorithm-and-metadata level still stands). Applied (Nit): 6. **Finding 3.2 — §Simulation row 0 `.bss` zero-fill prose.** The "bitmap zero-initialised by `.bss` zero-fill" precondition was technically true but not load-bearing — `Pmm::new` constructs a fully-initialised value. Reworded row 0 to acknowledge this: "the zero-init is precondition-only, not load-bearing". 7. **Finding 5.1 — Option B u32/u16 indices math.** The 128 KiB metadata claim assumed `u32` indices; `u16` halves to 64 KiB for v1's 32 K frames. Tightened the math with a parenthetical ("64 KiB with `u16` since 32 K fits in 15 bits — but `u32` is forward-portable to BSPs with > 64 K frames such as Pi 4 with 1 M frames at 4 GiB"). Qualitative judgment unchanged (still 16–32× the bitmap). 8. **Finding 8.3 — §Approach commit 1 test set.** Moved `new_rejects_too_many_reserved_ranges` from "implicit later commit" to commit 1 alongside `new_marks_reserved_ranges_and_initialises_counters` (both pin `Pmm::new`, so they belong in the constructor commit). 9. **Finding 9 — UNSAFE-2026-0001 umbrella name consistency.** The ADR's prose drifted between "kernel-static-buffer raw-pointer write to identity-mapped memory" (L35) and "MMIO + kernel-static- buffer raw-pointer" (L58). Picked the former as the canonical phrasing and aligned both surfaces. 10. **Finding 12.1 — T-016 §"Why one bundled task" disambiguation.** The line "instead of T-016 + T-017 + T-018?" was written when those numbers were hypothetical sub-task splits of T-016 itself. Post-T-017 (PMM) + T-018 (AddressSpace, future) numbering, the line risked confusion. Added a parenthetical clarifying it's about a hypothetical 3-way split of T-016, not the actual post-T-016 numbering. Bonus — Finding 12.2 (kernel-internal framing) — slight cosmetic qualifier added: "kernel-internal at runtime" + a parenthetical clarifying that the BSP touches the PMM only at construction time (linker-symbol → Pmm::new → StaticCell publication) and never afterwards. Verification: - cargo fmt --check clean - cargo test 185/185 host (unchanged — docs only) - cargo kernel-build clean Files touched: - docs/decisions/0035-physical-memory-manager.md (+30 / -20) - docs/analysis/tasks/phase-b/T-017-physical-memory-manager.md (+18 / -10) - docs/analysis/tasks/phase-b/T-016-mmu-activation.md (+1 / -1) This commit completes the PR #25 review-round; all reviewer-flagged findings closed. The Accept commit (2de67f1) carrying ADR-0035 → Accepted stays as the load-bearing flip; this fixup commit is the honesty / consistency polish pass on top. Refs: ADR-0035, T-017
…convention)
Reviewer round-2 produced 3 Minor findings on the post-c70ed84
state. Verified each against current code; 2 applied, 1 skipped
with reason.
Applied:
1. **Axis 3 — §Simulation row 2 O(R) notation drift.** The ADR
used "O(R)" informally to mean "O(reserved-range count)"; T-017
formalises `R` as the per-BSP const generic for the
`[Option<PhysFrameRange>; R]` array capacity (`Pmm<const N: usize,
const R: usize>`). The symbol overlap risked a reader interpreting
"O(R)" as O(array capacity) rather than O(populated entries).
Reworded row 2 to "O(number of reserved ranges) — small
constant for v1's BSP (3 ranges); T-017 formalises this as
iterating only the `Some(_)` slots of the cached
`[Option<PhysFrameRange>; R]` array (where `R` is T-017's
per-BSP const generic for the array capacity, distinct from
the iteration cost)". The two distinct uses of `R` (capacity
vs iteration cost) are now explicit.
2. **Axis 7 — Broken `[ADR-0027 §"Frame allocation discipline"]`
link.** Verified: ADR-0027 has sections Context / Decision
drivers / (a/b/c) Decision outcome / Why D / Simulation /
Dependency chain / Consequences / etc. — but NO "Frame
allocation discipline" section. That section exists only in
`docs/architecture/memory-management.md`. The bare-path link
would have landed the reader at ADR-0027's top, not the
intended discussion. Reworded the §Context sentence to cite
ADR-0027 generically (without a §-anchor pretense) while
keeping the memory-management.md `#frame-allocation-discipline`
link, with a parenthetical noting that the architecture
chapter holds the canonical section and ADR-0027 references
it from §Decision drivers + §Decision outcome.
Skipped:
3. **Axis 7 — Missing anchors on 3 ADR-0025 § links** (suggested
`#rule-1` × 2 + `#revision-notes` × 1). Two reasons:
(a) **Project convention is bare links to ADR-0025 § sections.**
Verified across the codebase: 5+ existing references
(T-008, T-011, T-012, T-013, T-014, T-015, T-016) all use
`[ADR-0025 §Rule 1](path/to/0025-adr-governance-amendments.md)`
or `[ADR-0025 §Revision notes](...)` without anchors. My
3 new references match the convention. Adding anchors to
3 of N references creates inconsistency rather than
resolving it; a project-wide sweep would be a separate
hygiene task.
(b) **`#rule-1` short form does not match GitHub's
auto-anchor for the actual heading.** The heading is
`### Rule 1 — Forward-reference contract: every "future
task" claim is grounded`; GitHub's slugifier (lowercase +
strip-punctuation + space-to-hyphen + collapse-consecutive)
produces a long anchor like
`#rule-1--forward-reference-contract-every-future-task-claim-is-grounded`,
not `#rule-1`. A bare `#rule-1` link could resolve in
some renderers (e.g., kramdown with explicit
attribute-list `{#rule-1}` syntax — which Tyrne's docs
don't use) but not in GitHub's default renderer. The
reviewer's specific suggestion would therefore land the
reader at the file's top regardless. (`#revision-notes`
IS a valid GitHub auto-anchor for `## Revision notes`,
but applying it in isolation would deepen the
inconsistency rather than resolve it.)
Tracked as a project-wide convention question: if the
maintainer prefers anchored links, a separate hygiene PR can
sweep all ~10+ ADR-0025 § references uniformly (and the same
pattern for any other multi-section ADRs). Out of scope for
PR #25.
Verification:
- cargo fmt --check clean
- cargo test 185/185 host (unchanged — docs only)
- cargo kernel-build clean
Files touched:
- docs/decisions/0035-physical-memory-manager.md (+2 / −2)
Refs: ADR-0035, T-017
…ndings
Reviewer round-3 (gemini-code-assist; sourcery / coderabbit / qodo
all rate-limited so no substantive feedback) produced 2 medium-
priority inline findings. Both verified valid; both applied. Each
strengthens defensive validation in PMM-API entry points without
adding new error variants (OutOfRange + MisalignedAddress already
in PmmError enum).
Applied:
1. **Finding A — `Pmm::new` should validate `extent` is page-aligned
and `reserved` ranges fit within `extent`.** Without these
validations the row-1 `from_aligned` unwrap depends on an
informal "BSP-init contract"; the row-1 unwrap promise can be
violated by a misbehaving BSP-side caller passing an unaligned
extent (resulting in off-by-one bitmap-index math) or a reserved
range that straddles / exceeds the extent (resulting in writes
past the bitmap's `[u8; N]` bound). Added three fail-fast
validations to `Pmm::new`'s spec — all check before any bitmap
mutation, so partial-mutation states are structurally
impossible:
(i) extent.start + extent.end page-aligned → MisalignedAddress
(ii) every reserved range ⊆ [extent.start, extent.end) → OutOfRange
(iii) reserved.len() ≤ R → TooManyReservedRanges (already present)
Propagated to ADR-0035 §Simulation row 0 + T-017 Pmm::new
acceptance criterion + 2 new host tests
(`new_rejects_unaligned_extent`,
`new_rejects_reserved_range_outside_extent`).
2. **Finding B — `free_frame` should validate PA is within `extent`
before computing bitmap index.** Without this an attacker /
misbehaving caller passing an unrelated PhysFrame (kernel-static
buffer's PA, MMIO base, frame from a different PMM instance)
could underflow `pa - extent.start` or produce an out-of-bounds
bitmap index → UB on subsequent bitmap access. Added
extent-bounds check at the FRONT of free_frame's logic
(precedes index arithmetic + reserved-range scan + bit read).
Propagated to ADR-0035 §Simulation row 2 + T-017 free_frame
acceptance criterion + 1 new host test
(`free_frame_rejects_pa_outside_extent`, distinct from the
reserved-range-rejection test which covers PAs *inside* extent).
Test count rises 10 → 13. Total expected post-T-017
implementation: 195 → 198. T-017 §Approach commit-1 absorbs the
2 new Pmm::new tests; commit-2 absorbs the 1 free_frame test
(both consistent with the existing test-distribution discipline
where tests pinning a method belong in that method's
introducing commit).
Both findings strengthen the existing §Simulation discipline
(row 1's correctness was implicit; now structural). Row 1's
`from_aligned` unwrap is **provably-correct** post-fix (extent.start
page-aligned + i × 4096 preserve-aligned).
Verification:
- cargo fmt --check clean
- cargo test 185/185 host (unchanged — docs only)
Files touched:
- docs/decisions/0035-physical-memory-manager.md (rows 0 + 2)
- docs/analysis/tasks/phase-b/T-017-physical-memory-manager.md
(Pmm::new + free_frame acceptance criteria + 3 new tests +
expected test count + §Approach commit-1/commit-2 lists)
Refs: ADR-0035, T-017
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
B3 prep arc opens. Lands ADR-0035 — Physical Memory Manager (B3 prerequisite — bitmap allocator) at status `Accepted` (Propose + careful-re-read separate-commit pair per `write-adr` skill §10) and opens T-017 — Physical Memory Manager at `Draft` per ADR-0025 §Rule 1 (forward-reference contract). Implementation commit chain follows post-merge.
Two commits
What ADR-0035 settles
Bitmap allocator with hint pointer. Single bit per frame; 4 KiB metadata for QEMU virt's 32 768 frames (128 MiB / 4 KiB). Linear scan from hint pointer with rewind-on-free; O(N) worst-case, O(1) amortised typical under v1's no-fragmentation pressure. Lowest metadata footprint of any considered option.
Reservation list at init. `Pmm::new(extent, reserved) -> Result<Self, PmmError>` is one-shot at boot; walks the BSP-provided reservation list (kernel image + `.boot_pt` + boot stack for v1) and marks every covered frame's bit as set. The PMM caches the reservation-range list (max 8 entries, fixed-size array) so `free_frame` can defensively reject attempted-frees of Reserved-range PAs with `PmmError::DoubleFree`. The bitmap collapses Reserved + Allocated into a single bit; the cached list provides the per-frame discrimination `free_frame` needs.
Forward-portable to high-half kernel. Bitmap addressing is regime-independent; the future ADR-0033 placeholder (high-half migration) relocates the bitmap base address but does not require a `PA → VA` translation discipline at the algorithm level. Rejected alternatives — Option C (in-frame linked free-list) — would have baked in the identity-only assumption.
Zero new `unsafe` audit entries. Bitmap operations are safe Rust; the frame-zeroing site joins UNSAFE-2026-0001's existing umbrella ("kernel-static-buffer raw-pointer write to identity-mapped memory") via Amendment in T-017. Audit-log surface stays bounded at the project's "minimum required" pace.
§Simulation table (5 rows; the load-bearing design checklist)
§Considered options
A — bitmap with hint pointer (chosen). B — external free-list (32× metadata, rejected). C — in-frame linked free-list (regime-dependent, rejected). D — buddy allocator (overkill for v1 single-size, rejected).
What's next (T-017 implementation chain, post-merge)
T-017 is a single bundled task; planned 4 bisectable commits per its §Approach:
Test plan
Refs: ADR-0035, T-017