FABRIC.md: item 4.5a complete -- full ISR-shared global audit, all three architectures

Read every interrupt/exception vector handler on amd64, aarch64, and
riscv64, and every global or static variable each one touches directly or
through a called function, checked against its volatile declaration.

One confirmed hazard, matching what 4.5 already reported: TimeTrustState.
ticks is written in ISR context on all three architectures and read by
mainline (including a busy-wait) without being volatile.

Everything else checked out for one of three reasons, each verified by
reading the actual read/write sites rather than assumed: already correctly
volatile (g_sk_fault_word, g_spurious_count, g_plic_claim_count, the
heartbeat.c top/bottom-half handoff, i8042.c's ring buffer, virtio_input.c's
diagnostic counters); write-once during init then single-context for the
rest of boot, so never actually concurrent (each arch's timer-calibration
state, virtio_input.c's device-routing globals); or ISR-reachable only on
the fatal exception path, which halts the core permanently afterward so
there's no return to mainline to race with (console/framebuffer state).

Full findings recorded in FABRIC.md as 4.5a's inventory. No code changed --
investigation only, per the item's own scope.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Robert Allan James
2026-08-11 10:04:31 -04:00
co-authored by Claude Sonnet 5
parent de69b3827d
commit 8f4348ec18
+47 -1
View File
@@ -5393,7 +5393,7 @@ document and committing that amendment as its own item.*
scoping only, per Captain Bob's explicit instruction 2026-08-11.
*Refs:* discovered via 4.4g; independent of Console work.
- [ ] **4.5a — Full ISR/interrupt-context global-state audit, all three architectures.**
- [x] **4.5a — Full ISR/interrupt-context global-state audit, all three architectures.**
Investigation only, no code change. For each architecture, enumerate every vector actually
wired to a handler (amd64: IDT/APIC vectors in `arch/amd64/interrupts.c`; aarch64: GIC
vectors in `arch/aarch64/interrupts.c`; riscv64: PLIC/trap causes in
@@ -5408,6 +5408,52 @@ document and committing that amendment as its own item.*
file:line, or an explicit "none found" recorded per architecture.
*Refs:* 4.5.
> **Done 2026-08-11.** Every vector handler on all three architectures read; every global
> or `static` each one touches (directly or through a called function) checked against its
> declaration. One confirmed hazard, matching what 4.5 already reported; everything else
> checked out, for reasons recorded below rather than left as a bare "it's fine."
>
> **Confirmed hazard (1):** `TimeTrustState.ticks` (`g_heartbeat.ticks`, `timer.h:90`) —
> written directly in the timer ISR on all three architectures (`heartbeat_tick()`,
> `heartbeat.c:163`, called from amd64 `interrupts.c:342`, aarch64 `interrupts.c:89`,
> riscv64 `interrupts.c:80`) and read directly by mainline via `heartbeat_ticks()`
> (`heartbeat.c:200-202`), including the busy-wait at `kernel_main.c:880`. Genuinely
> concurrent ISR-write / mainline-read of non-`volatile` state — the real bug 4.6b exists
> to fix.
>
> **Already correctly `volatile` (no action needed):** `g_sk_fault_word` (all three
> `interrupts.c`), `g_spurious_count` (amd64 `interrupts.c:59`), `g_plic_claim_count`
> (riscv64 `interrupts.c:47`), `g_pending_counter`/`g_pending_valid`/`g_adaptive_period_ns`
> (`heartbeat.c:57-63`, the documented top/bottom-half handoff), i8042.c's `ring[]`/
> `ring_head`/`ring_tail`/`g_i8042_isr_count`, virtio_input.c's `g_diag_head`/`g_diag_tail`/
> `g_virtio_input_isr_count`.
>
> **Not `volatile`, but not a hazard — write-once-during-init, then single-context for the
> rest of boot, so no concurrent access ever occurs:** each architecture's timer-calibration
> state (amd64 `apic.c`: `timer_initial_count`, `s_apic_hz`; aarch64 `apic.c`: `s_timer_ppi`,
> `s_timer_period_tsc`, `s_counter_hz_apic`; riscv64 `apic.c`: `s_timer_period_tsc`,
> `s_time_hz`, `s_sbi_time_ok`, `s_next_deadline`) is written once by `apic_timer_init()`
> before `arch_enable_interrupts()` is ever called, then touched only by each arch's
> ISR-context `*_timer_rearm()` for the rest of boot — verified by grepping every read/write
> site of each symbol, not assumed from the pattern looking familiar. Same reasoning for
> virtio_input.c's `g_vinput_ready`/`g_virtio_input_plic_source`/`g_virtio_input_gic_intid`:
> all three are written exactly once inside `virtio_input_find_keyboard()` (called from
> `kernel_main.c` during M7.pre PCI init, before interrupts are enabled) and read-only by
> the ISR afterward.
>
> **Special case, no `volatile` needed by design:** `console_puts()`/`console_println()`
> and the framebuffer/VT100 state they touch (`g_vt`, `g_active_vm_name`, `g_line_start`,
> `g_fb`) are reachable from ISR context only on the fatal exception/fault path
> (`isr_common_handler()`'s default case, `aarch64_exception_handler()`,
> `riscv64_exception_handler()`) — and every one of those paths halts the core permanently
> afterward (`while(1) arch_halt();` / `for(;;) wfi`/`wfe`). There is no return to mainline
> after a fault handler touches console state, so no concurrent-access hazard exists despite
> the state not being `volatile`. The hot-path IRQs (timer tick, keyboard, virtio-input)
> never call console functions at all.
>
> No other ISR-reachable global state was found beyond what's listed above, on any of the
> three architectures.
- [ ] **4.5b — Fix every hazard 4.5a found.** Depends on 4.5a. Code fix, no optimization
flags touched yet. `TimeTrustState.ticks` is the one already-confirmed instance — mark it
`volatile`, or fold it into the existing `g_pending_counter`/`g_pending_valid`-style