From 8f4348ec1856b84cab4e2ab28c0d2c4e6119b9f3 Mon Sep 17 00:00:00 2001 From: Robert Allan James Date: Tue, 11 Aug 2026 10:04:31 -0400 Subject: [PATCH] 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 --- FABRIC.md | 48 +++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 47 insertions(+), 1 deletion(-) diff --git a/FABRIC.md b/FABRIC.md index fbcd487..b80397a 100644 --- a/FABRIC.md +++ b/FABRIC.md @@ -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