FABRIC-3.md: aarch64/riscv64 boot-path code audit pass (§IV.3/§V.3 item 7)
Reviewed arch/aarch64/{apic,arch,timer,interrupts}.c and
arch/riscv64/{apic,plic,arch,interrupts,timer}.c for the same class of
QEMU-virt-vs-real-hardware assumption §III item 6's amd64 audit looked for.
Report only, per this project's "identify, don't fix unless asked" rule --
no source files changed.
Findings, aarch64:
- Severe, confirmed live: apic.c's GICD/GICC base addresses are hardcoded
QEMU-virt constants, self-documented as a deliberate exception because
QEMU's aarch64 firmware never forwards a DTB. That premise no longer
holds -- the native Pi 5 boot path (item 2) receives a real DTB and
calls the same, unmodified kernel_main(), whose M4 sequence calls
apic_init(boot_info) unconditionally (kernel_main.c:413); apic_init()
still ignores boot_info entirely. Real BCM2712 GIC-400 is at
0x10_7fff9000, confirmed against bcm2712.dtsi -- a different region of
the address space entirely from the hardcoded 0x08000000. With the MMU
off at this point in boot, this blocks reaching ok> on real hardware as
the code stands.
- Doc-only correction: interrupts.c's own comment claims VBAR_EL2 is never
installed ("a known gap"). Checked against isr.S and found stale -- the
actual implementation already branches on aarch64_current_el() and
installs vbar_el2/vbar_el1 correctly. No functional gap; the comment
describes one the code already closed.
- arch_cold_reset() hardcodes the PSCI conduit to HVC, a QEMU-specific
workaround (QEMU's AAVMF has no genuine EL3). The Pi 5's real ATF means
EL3 firmware exists there, making SMC the conventional real-hardware
conduit -- no runtime detection exists.
Findings, riscv64:
- plic.c's QEMU-virt-hardcoded PLIC_BASE is already tracked (§V.3 item 3);
this pass confirms rather than rediscovers it.
- arch_early_init()'s satp clear is justified entirely by behavior
observed under QEMU's EDK2 firmware; the native boot path (U-Boot+
OpenSBI, no EDK2) doesn't share that observation, though the action is
likely still safe since OpenSBI's handoff conventionally leaves satp=0
already. Lowest-severity finding in the set.
- Clean: the SBI timer path and arch_cold_reset()'s SBI SRST call are both
genuinely hardware-independent -- named as the portable pattern amd64's
i8042-pulse reset and aarch64's hardcoded-HVC PSCI call both lack.
Doc-only change. Three-arch QEMU acceptance (amd64/aarch64/riscv64, in
order) run to confirm non-regression only.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019YcT3H2PQeyujrzjqS3Var
This commit is contained in:
co-authored by
Claude Sonnet 5
parent
6a6ff9353f
commit
0f798256a0
+92
-10
@@ -538,13 +538,63 @@ blocker for free.
|
|||||||
currently builds — item 1's separate-image build target (linking at `0x80000`) is still
|
currently builds — item 1's separate-image build target (linking at `0x80000`) is still
|
||||||
outstanding future work, same gap that item's own text already flagged.
|
outstanding future work, same gap that item's own text already flagged.
|
||||||
|
|
||||||
|
7. **Code audit pass — DONE, 2026-09-04.** Reviewed `arch/aarch64/apic.c`, `arch.c`,
|
||||||
|
`timer.c`, `interrupts.c` for the same class of QEMU-virt-vs-real-hardware assumption
|
||||||
|
§III item 6's amd64 audit looked for. Report only, per this project's own "identify,
|
||||||
|
don't fix unless asked" rule — nothing below was changed.
|
||||||
|
|
||||||
|
**Finding — severe, confirmed live, blocks reaching `ok>` on real hardware as the code
|
||||||
|
stands today**: `apic.c`'s own file header already self-documents `GICD_BASE_PA`/
|
||||||
|
`GICC_BASE_PA` (`0x08000000`/`0x08010000`) as "QEMU-virt-machine constants... a
|
||||||
|
deliberate, recorded exception," reasoned correctly at the time it was written: no DTB
|
||||||
|
was ever available to discover them from, because QEMU's own aarch64 UEFI firmware
|
||||||
|
doesn't forward one. That premise no longer holds — the native boot path (item 2, DONE)
|
||||||
|
receives a real DTB directly and calls the same, unmodified `kernel_main()` M4 sequence,
|
||||||
|
which calls `apic_init(boot_info)` unconditionally (confirmed: `kernel_main.c:413`, no
|
||||||
|
arch- or boot-path gating) — `apic_init()` itself still ignores `boot_info` entirely
|
||||||
|
(`(void)boot_info;`) and always programs the QEMU addresses. The real BCM2712 GIC-400 is
|
||||||
|
at `0x10_7fff9000` (distributor) / `0x10_7fffa000` (CPU interface 0) — confirmed directly
|
||||||
|
against `bcm2712.dtsi`'s own `gicv2` node (`compatible = "arm,gic-400"`), not recalled —
|
||||||
|
a completely different region of the physical address space from QEMU's. With the MMU off
|
||||||
|
at this point in boot (per the standard aarch64 boot protocol and this kernel's own
|
||||||
|
`arch_mmu_init()` stub), a 32-bit MMIO write to an address BCM2712 doesn't decode as GIC
|
||||||
|
registers is a data abort, not a silent wrong-value write — this blocks item 10 below
|
||||||
|
(`ok>`) as the code stands, on the same critical path as item 1's still-missing `0x80000`
|
||||||
|
build target. Not fixed this pass — `fdt_find_node_by_compatible(dtb, "arm,gic-400")` →
|
||||||
|
`fdt_find_prop_in_node(..., "reg", ...)` is the existing primitive that would supply the
|
||||||
|
real base once `apic_init()` is updated to use it when a DTB is present, falling back to
|
||||||
|
the QEMU constants when it (still correctly) isn't.
|
||||||
|
**Finding — doc-only, not a functional gap**: `arch_interrupts_init()`'s own doc comment
|
||||||
|
in `interrupts.c` claims "`VBAR_EL1` is written unconditionally regardless of the
|
||||||
|
detected level... if EL2, this is a known gap" — checked against `isr.S`
|
||||||
|
(`aarch64_install_vectors`, lines 124–146) and found **stale**: the actual implementation
|
||||||
|
already branches on `aarch64_current_el()` and writes `vbar_el2`/`vbar_el1` correctly,
|
||||||
|
with `el2_mode_flag` threading the same answer through the IRQ trampoline for
|
||||||
|
`ELR_EL1`/`SPSR_EL1` vs `ELR_EL2`/`SPSR_EL2` selection. The comment describes a gap the
|
||||||
|
code already closed; left as-is per report-only policy, but flagged so a future reader
|
||||||
|
doesn't take the stale comment at face value. Which EL the Pi 5's ATF hands the kernel
|
||||||
|
off at under the native path is itself unconfirmed (the `atf@0` reserved-memory region
|
||||||
|
found during item 2's work only establishes that EL3 firmware exists, not which EL the
|
||||||
|
kernel lands at) — moot for this specific gap now that both paths are actually wired, but
|
||||||
|
worth knowing for other EL-dependent code.
|
||||||
|
**Finding — same defect class as amd64's `arch_cold_reset()` finding, on the QEMU side of
|
||||||
|
this arch specifically**: `arch/aarch64/arch.c`'s `arch_cold_reset()` issues PSCI
|
||||||
|
`SYSTEM_RESET` via `HVC` — a fix already documented in-file as a QEMU-specific
|
||||||
|
workaround, because QEMU's AAVMF firmware has no genuine EL3/TrustZone secure monitor to
|
||||||
|
answer an `SMC`. The Pi 5's real ATF (confirmed present via the `atf@0` reservation) is
|
||||||
|
exactly the kind of genuine EL3 firmware PSCI's `SMC` conduit assumes — the conventional
|
||||||
|
real-hardware call, not `HVC`. No runtime conduit detection exists; this is hardcoded to
|
||||||
|
the QEMU-only fix with no fallback. Riscv64's equivalent (`arch/riscv64/arch.c`, SBI SRST)
|
||||||
|
is the portable pattern this and amd64's i8042-pulse version both lack — worth naming as
|
||||||
|
the standard to bring the other two toward, not fixed here.
|
||||||
|
|
||||||
**Hardware-dependent, after 2026-09-17 (not started until then):**
|
**Hardware-dependent, after 2026-09-17 (not started until then):**
|
||||||
7. Build the boot media (SD card: `config.txt`, `bcm2712-rpi-5-b.dtb`, kernel image).
|
8. Build the boot media (SD card: `config.txt`, `bcm2712-rpi-5-b.dtb`, kernel image).
|
||||||
8. Connect HDMI + keyboard (observation decision above).
|
9. Connect HDMI + keyboard (observation decision above).
|
||||||
9. Boot; confirm `ok>`/`zuse)ok>` reached.
|
10. Boot; confirm `ok>`/`zuse)ok>` reached.
|
||||||
10. Mint a Zuse identity on real media, confirm re-attach — the `v2.4.0` gate's own
|
11. Mint a Zuse identity on real media, confirm re-attach — the `v2.4.0` gate's own
|
||||||
requirement, same shape as amd64's.
|
requirement, same shape as amd64's.
|
||||||
11. Update this section with results before moving to riscv64's own hardware-dependent steps.
|
12. Update this section with results before moving to riscv64's own hardware-dependent steps.
|
||||||
|
|
||||||
## V. riscv64 — Milk-V Mars
|
## V. riscv64 — Milk-V Mars
|
||||||
|
|
||||||
@@ -654,14 +704,46 @@ it doesn't. This is a real punch-list item, not a hypothetical.
|
|||||||
tooling's actual invocation once building the first real image, not just cited from
|
tooling's actual invocation once building the first real image, not just cited from
|
||||||
VisionFive 2 research.
|
VisionFive 2 research.
|
||||||
|
|
||||||
|
7. **Code audit pass — DONE, 2026-09-04.** Reviewed `arch/riscv64/apic.c`, `plic.c`,
|
||||||
|
`arch.c`, `interrupts.c`, `timer.c` for the same class of QEMU-virt-vs-real-hardware
|
||||||
|
assumption §III item 6's amd64 audit and item 7 above's aarch64 audit looked for. Report
|
||||||
|
only — nothing below was changed.
|
||||||
|
|
||||||
|
**Clean, and the most portable pattern of all three architectures**: the SBI timer path
|
||||||
|
(`apic.c`) is genuinely hardware-independent — it probes for the TIME extension at
|
||||||
|
runtime and reports loudly rather than assuming it's present, so it depends on nothing
|
||||||
|
but OpenSBI itself, present on both QEMU and (per §V.1's own research) the Mars's real
|
||||||
|
U-Boot+OpenSBI chain. `arch_cold_reset()` (`arch.c`) uses the SBI SRST extension — a real
|
||||||
|
standards-defined mechanism, not a board-specific hack — making it the one of the three
|
||||||
|
architectures' cold-reset implementations that does **not** need a finding here (contrast
|
||||||
|
amd64's i8042-pulse hack and aarch64's HVC-hardcoded PSCI call, both flagged above/in
|
||||||
|
§III). `timer.c` is confirmed DTB-first as this section's own preamble already stated:
|
||||||
|
`timebase-frequency` is read via `fdt_prop_u32()` with a *named* QEMU-only fallback
|
||||||
|
(`RISCV_TIMEBASE_HZ_FALLBACK`) used only when the DTB is absent or invalid.
|
||||||
|
**Finding — already tracked, this pass confirms rather than discovers it**: `plic.c`'s
|
||||||
|
`PLIC_BASE`/`PLIC_CONTEXT_S` hardcoded-to-QEMU-virt situation is exactly item 3 above,
|
||||||
|
already flagged as "a real punch-list item, not a hypothetical" before this audit ran.
|
||||||
|
Nothing new to add beyond confirming the file's own header comment is accurate and the
|
||||||
|
risk is real, not overstated.
|
||||||
|
**Finding — minor, reasoning doesn't transfer to the native path, action likely still
|
||||||
|
safe**: `arch_early_init()`'s explicit `satp` clear (Bare-mode switch) is justified in
|
||||||
|
its own comment entirely by behavior *observed under QEMU's EDK2 RISC-V firmware*
|
||||||
|
(confirmed `satp.MODE=10`/Sv57 live, kernel identity-mapped within it). Under the native
|
||||||
|
boot path (U-Boot+OpenSBI, no UEFI/EDK2 at all per §V.1) that specific observation cannot
|
||||||
|
apply — OpenSBI's S-mode handoff conventionally already leaves `satp=0` (Bare mode), most
|
||||||
|
likely making this a no-op there rather than a hazard, but the *stated justification* for
|
||||||
|
the switch (identity-mapped, safe to clear) was derived from a firmware stack the native
|
||||||
|
path doesn't use, and hasn't been re-confirmed for OpenSBI's actual handoff state. Lowest
|
||||||
|
severity of any finding across both audits — flagged for completeness, not urgency.
|
||||||
|
|
||||||
**Hardware-dependent, after 2026-09-17:**
|
**Hardware-dependent, after 2026-09-17:**
|
||||||
7. Build and flash the boot image to QSPI flash (or attempt UART XMODEM recovery boot if QSPI
|
8. Build and flash the boot image to QSPI flash (or attempt UART XMODEM recovery boot if QSPI
|
||||||
flashing isn't set up yet — both are real supported paths per §V.1).
|
flashing isn't set up yet — both are real supported paths per §V.1).
|
||||||
8. Connect HDMI + keyboard.
|
9. Connect HDMI + keyboard.
|
||||||
9. Boot; confirm `ok>`/`zuse)ok>` reached.
|
10. Boot; confirm `ok>`/`zuse)ok>` reached.
|
||||||
10. Mint a Zuse identity on real media, confirm re-attach — the `v2.5.0` gate's own
|
11. Mint a Zuse identity on real media, confirm re-attach — the `v2.5.0` gate's own
|
||||||
requirement.
|
requirement.
|
||||||
11. Update this section with results.
|
12. Update this section with results.
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
|
|||||||
@@ -1,5 +1,5 @@
|
|||||||
# Capsule Block Manifest — Auto-generated
|
# Capsule Block Manifest — Auto-generated
|
||||||
<!-- Generated by mkcapsule --manifest 2026-09-05T03:13:16Z -->
|
<!-- Generated by mkcapsule --manifest 2026-09-05T03:29:42Z -->
|
||||||
<!-- DO NOT EDIT — re-run mkcapsule --manifest to refresh. -->
|
<!-- DO NOT EDIT — re-run mkcapsule --manifest to refresh. -->
|
||||||
<!-- Hand-written justifications and immutability notes live -->
|
<!-- Hand-written justifications and immutability notes live -->
|
||||||
<!-- in MANIFEST.md alongside this auto-generated index. -->
|
<!-- in MANIFEST.md alongside this auto-generated index. -->
|
||||||
|
|||||||
Binary file not shown.
File diff suppressed because it is too large
Load Diff
File diff suppressed because it is too large
Load Diff
File diff suppressed because it is too large
Load Diff
Reference in New Issue
Block a user