FABRIC-3.md: amd64 boot-path code audit pass (§III item 6)
Build / build-amd64-iso (push) Waiting to run
Build / build-aarch64-iso (push) Waiting to run
Build / build-riscv64-img (push) Waiting to run

Reviewed src/starkernel/boot/uefi_loader.c and every file in arch/amd64/
(arch.c, apic.c, ioapic.c, interrupts.c, i8042.c, timer.c) for anything that
assumes SER5-specific hardware rather than standard UEFI/ACPI. Report only,
per this project's "identify, don't fix unless asked" rule -- no source
files changed.

Verified clean: uefi_loader.c's COM1 presence probe, its two-pass ACPI
2.0-then-1.0 GUID preference, the GetMemoryMap/ExitBootServices golden
path, and GOP BltOnly degradation. apic.c/ioapic.c derive LAPIC mode,
APIC timer frequency, and I/O APIC base/GSI at runtime, not from hardcoded
constants.

Four findings recorded (not fixed):
1. HPET base (timer.c) hardcoded at 0xFED00000, never cross-checked against
   the ACPI "HPET" table -- the same bug class this file's own comment
   already documents and fixed for the PM_TMR port.
2. LAPIC base always 0xFEE00000; apic_init()'s own doc comment flags MADT
   relocation as unimplemented. Narrower than that suggests: ioapic.c's
   parser already reads MadtHeader.local_apic_address, just never plumbs
   it to apic.c. MADT type 5 (Local APIC Address Override) isn't parsed
   at all.
3. i8042 PS/2 controller (i8042.c) and the legacy 8259 PIC (interrupts.c)
   are poked unconditionally with no ACPI FADT presence check, unlike this
   same file's own raw_serial_init() COM1 probe.
4. arch_cold_reset() (confirmed live via BYE, mama_forth_words.c:1437)
   pulses the i8042 reset line instead of using the FADT's standards-defined
   RESET_REG/RESET_VALUE, which doesn't depend on i8042 existing at all.

None are certain to bite on the real SER5 -- the conventions assumed hold
on the large majority of PC-compatible x86_64 systems -- but the decision
in FABRIC-3.md §III requires arguing genericity from standards compliance,
not from "it booted," and these are the concrete gaps.

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:
Robert Allan James
2026-09-04 23:15:09 -04:00
co-authored by Claude Sonnet 5
parent ecdb32e0d6
commit 6a6ff9353f
9 changed files with 27633 additions and 6 deletions
+65 -5
View File
@@ -188,11 +188,71 @@ what's still genuinely unknown, not assumed.
4. Connect HDMI + keyboard to the SER5.
5. Boot the SER5 from the flashed stick (firmware boot-order menu as needed — Secure Boot
already disabled, confirmed above, so no signing prompt expected).
6. **Code audit pass** (can happen before or in parallel with 15, doesn't need the hardware
in hand): review the amd64 boot path (`src/starkernel/boot/uefi_loader.c`,
`arch/amd64/*.c`) for anything that assumes SER5-specific hardware rather than standard
UEFI/ACPI — this is what "genericity" actually rests on per the decision above, not the
SER5 boot succeeding alone.
6. **Code audit pass — DONE, 2026-09-04.** Reviewed `src/starkernel/boot/uefi_loader.c` and
every file in `arch/amd64/` (`arch.c`, `apic.c`, `ioapic.c`, `interrupts.c`, `i8042.c`,
`timer.c`) for anything that assumes SER5-specific hardware rather than standard UEFI/ACPI.
Report only, per this project's own "identify, don't fix unless asked" rule — nothing
below was changed.
**Clean (verified standards-correct, not just "worked in QEMU"):**
- `uefi_loader.c`: COM1 presence is probed (scratch-register readback + bounded THRE spin)
before any code trusts it, so a board with no legacy serial decode — the documented SER5
case — degrades to silently dropping bytes rather than hanging. The ACPI table search
does two explicit passes (2.0 GUID first, 1.0 only as fallback) rather than a single
first-match loop, avoiding a real bug class (silently preferring the RSDT-only 1.0
pointer). `GetMemoryMap`/`ExitBootServices` follows the UEFI "golden path" (nothing
between the two calls) with the required `EFI_INVALID_PARAMETER` retry loop. GOP
framebuffer degrades to `PixelBltOnly` cleanly when no linear framebuffer exists.
- `apic.c`: LAPIC presence, x2APIC-vs-xAPIC mode, and the APIC timer's own tick rate are
all determined at runtime (MSR read, empirical TSC-timed calibration) — no hardcoded
bus-frequency assumption anywhere in the timer path.
- `ioapic.c`: I/O APIC base and GSI base come from the ACPI MADT, not a hardcoded address.
- `timer.c`'s PM_TMR_BLK port: already fixed correctly (`fadt_find_pm_tmr_port()`,
2026-08-11 per this file's own comment) — discovered from the FADT, not assumed from a
single machine type's legacy port.
**Findings (real, not fixed — reported per policy):**
1. **HPET base address hardcoded, same bug class as the already-fixed PM_TMR port.**
`timer.c`'s `HPET_PHYS_BASE 0xFED00000` is never cross-checked against the ACPI `"HPET"`
table (confirmed absent — no `"HPET"` signature lookup exists anywhere in the tree,
unlike the FADT/`"FACP"` walk `fadt_find_pm_tmr_port()` already does and could extend to
reach it with the same RSDP→XSDT→table-by-signature walk). 0xFED00000 is the
near-universal legacy convention, not a guaranteed one — this file's own comment already
documents the *identical* failure mode for the PM Timer port and how it was fixed;
the same reasoning was never applied to HPET.
2. **LAPIC relocation: MADT data already parsed one file over, never plumbed through.**
`apic_init()`'s own doc comment self-flags `boot_info` as unused, "reserved for future
use (e.g., reading the ACPI MADT for the physical LAPIC base address)" — always uses
`LAPIC_DEFAULT_PHYS = 0xFEE00000`. Narrower than "unimplemented": `ioapic.c`'s
`MadtHeader.local_apic_address` field is already read into a parsed struct by
`parse_madt()`, just never passed to `apic.c`. `parse_madt()` also only handles MADT
entry types 1 (I/O APIC) and 2 (Interrupt Source Override) — type 5 (Local APIC Address
Override), the entry that matters on a system that actually relocates it, isn't parsed
at all.
3. **i8042 PS/2 controller poked unconditionally, no presence check.** `i8042_init()` and
`i8042_drain_stale()` read/write ports 0x60/0x64 with no probe of any kind — contrast
with this same codebase's own `raw_serial_init()`, which explicitly probes COM1's
scratch register before trusting it exists. The ACPI FADT carries an IA-PC
boot-architecture flags field that reports 8042 presence; it is never consulted. Same
root gap in `pic_disable()` (`interrupts.c`): unconditionally masks the legacy 8259 PIC
at ports 0x21/0xA1 with no corresponding FADT check.
4. **`arch_cold_reset()` uses the legacy i8042 reset-pulse hack instead of the ACPI Reset
Register.** Confirmed live, not dormant: called directly from `BYE`
(`mama_forth_words.c:1437`, Hera-only cold restart). Pulses port 0x64 with 0xFE — the
pre-ACPI convention that assumes the same i8042 controller finding #3 already flags —
instead of the FADT's `RESET_REG`/`RESET_VALUE` fields, which are the actual
standards-defined generic reset mechanism (confirmed absent from the tree — no
`RESET_REG` reference exists anywhere) and don't depend on i8042 existing at all.
Degrades gracefully (falls through to a `cli;hlt` spin loop, doesn't hang) if the
pulse has no effect — the honest risk here is a silent no-op reset on an i8042-less
board, not a hang.
None of these four are certain to actually bite on the real SER5 — the near-universal
conventions they assume (0xFED00000, 0xFEE00000, i8042-at-0x60/0x64) hold on the large
majority of real PC-compatible x86_64 systems including consumer mini-PCs, and only
finding 4 has a confirmed live call path. Recorded here because the decision above
requires arguing genericity from standards compliance, not from "it booted" — these are
the concrete gaps between what the code assumes and what UEFI/ACPI actually guarantees.
7. Capture the boot: confirm POST reaches the same `1012/0/0` result QEMU shows, confirm
`ok>`/`zuse)ok>`, confirm `rng: backend = rdrand` (live entropy, not the QEMU-only
`virtio-rng` path), save the serial transcript (if wired) the same way `logs/` already