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