Artemis Milestone 2d: xHCI Event Ring servicing, polled not interrupt-driven
Implements Event Ring TRB parsing and ERDP dequeue-pointer update
(xhci_poll_events(), src/starkernel/usb/xhci.c), called from
sk_repl_idle()'s existing ~1s idle cadence rather than a per-arch
interrupt handler.
A first attempt wired real interrupt delivery (PCI->IOAPIC GSI routing,
a dedicated isr_stub34/vector 0x22, GIC/PLIC routing mirroring
virtio_input.c). Checked live via QMP query-pci before trusting it: the
amd64 PIRQ swizzle formula predicted GSI 16 for the xHCI controller at
PCI slot 4; the real QEMU-assigned IRQ was 10, and embedded ICH9
functions contradicted the same formula too. Reverted all of it back to
the exact committed baseline rather than chasing chipset PIRQ routing
further, and reframed around Section U item 6's own design intent
("interrupt-driven, coarse cadence, cheap early-exit... quick check
blocks... done") via sk_repl_idle() instead -- USB insertion is a
human-timescale event, not a hot path.
Added -device qemu-xhci to all three QEMU launch targets (required for
any of this to be testable). Verified end to end via genuine post-boot
hotplug (QMP device_add/device_del usb-storage): all three architectures
detect a live attach within seconds. A false-alarm heartbeat "freeze"
found mid-verification traced to querying the wrong counter
(vm->heartbeat.tick_count, which only advances during word execution,
not the kernel's real ISR-driven heartbeat_ticks()) -- confirmed via a
temporary diagnostic word, captured and reverted.
Full writeup, including the discarded interrupt-routing attempt and the
false-alarm investigation, in FABRIC-2.md's Milestone 2c/2d entries.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HZ8kNoTuP63pbQtro4qvrm
This commit is contained in:
co-authored by
Claude Sonnet 5
parent
c2f1d94c97
commit
2b16daba16
+85
-17
@@ -3116,27 +3116,94 @@ kernel compilation passes), and a live amd64 boot still reaches POST 1012/0/0 an
|
||||
unaffected (expected — nothing calls `xhci_find_and_map()` yet, so this is purely additive
|
||||
until 2c wires it in).
|
||||
|
||||
**2c. Controller bring-up**
|
||||
- [ ] Read Capability Registers to learn controller parameters (max device slots, max ports,
|
||||
**2c. Controller bring-up ✅ DONE 2026-08-22**
|
||||
- [x] Read Capability Registers to learn controller parameters (max device slots, max ports,
|
||||
max interrupters — needed to size later allocations)
|
||||
- [ ] Perform xHCI controller reset sequence
|
||||
- [ ] Allocate and program the Device Context Base Address Array (DCBAA)
|
||||
- [ ] Allocate and program the Command Ring, write its base address to the Operational
|
||||
- [x] Perform xHCI controller reset sequence
|
||||
- [x] Allocate and program the Device Context Base Address Array (DCBAA)
|
||||
- [x] Allocate and program the Command Ring, write its base address to the Operational
|
||||
Register `CRCR`
|
||||
- [ ] Allocate and program at least one Event Ring (segment table + ring buffer), wire it to
|
||||
- [x] Allocate and program at least one Event Ring (segment table + ring buffer), wire it to
|
||||
Interrupter 0
|
||||
- [ ] Set the `RUN/STOP` bit to start the controller
|
||||
- [ ] Confirm controller reaches a running state (poll a status register, don't assume)
|
||||
- [x] Set the `RUN/STOP` bit to start the controller
|
||||
- [x] Confirm controller reaches a running state (poll a status register, don't assume)
|
||||
|
||||
**2d. Interrupt/event handling**
|
||||
- [ ] Wire an interrupt handler for the xHCI controller's IRQ line (via existing per-arch
|
||||
interrupt infrastructure, `starkernel/arch/*/interrupts.c` — same place the timer ISR
|
||||
already hooks in, per the heartbeat work this session)
|
||||
- [ ] Implement Event Ring TRB (Transfer Request Block) parsing — at minimum, Port Status
|
||||
Change events (hotplug) and Command Completion events, to start
|
||||
- [ ] Implement Event Ring dequeue-pointer update / interrupt-clear sequence so the
|
||||
`xhci_bringup()` (`src/starkernel/usb/xhci.c`) was already written, uncommitted, at the start of
|
||||
this session — but referenced an `XHCI_WAIT_FOR` macro that was never defined anywhere in the
|
||||
tree, breaking the build (`implicit declaration of function`, plus the `xhci_wait_bit()` helper
|
||||
meant to back it sitting unused under `-Werror=unused-function`). Fixed by wiring all four wait
|
||||
sites directly to `xhci_wait_bit(reg, mask, want_set, max_ticks)` with the correct
|
||||
register/bit/polarity per site, rather than inventing the missing macro. Verified via
|
||||
`make -f Makefile.starkernel ARCH=amd64` compiling clean end to end. Committed `c2f1d94` (same
|
||||
commit also flipped `g_doe_log_enabled`'s default off — the per-tick `[HADES][DOE]` export was
|
||||
flooding every acceptance boot log for no reason during ordinary verification; `HB-ON` still
|
||||
re-enables it for a real DoE campaign).
|
||||
|
||||
**2d. Interrupt/event handling ✅ DONE 2026-08-22 — polled, not interrupt-driven**
|
||||
- [x] ~~Wire an interrupt handler for the xHCI controller's IRQ line~~ — attempted, reverted;
|
||||
see below for why this became a poll instead
|
||||
- [x] Implement Event Ring TRB (Transfer Request Block) parsing — Port Status Change and
|
||||
Command Completion events logged (full handling is 2e/2g's scope), Transfer events logged
|
||||
too (2g's scope)
|
||||
- [x] Implement Event Ring dequeue-pointer update / interrupt-clear sequence so the
|
||||
controller keeps delivering new events
|
||||
|
||||
**First attempt (interrupt-driven) built, then reverted after a live check proved the amd64
|
||||
routing formula wrong.** Wired PCI→IOAPIC GSI routing (new `ioapic_route_pci_irq()`), a
|
||||
dedicated `isr_stub34`/vector `0x22` (mirroring `isr_stub33`'s own fix for the identical
|
||||
misrouted-to-spurious bug), and arch-guarded routing mirroring `virtio_input.c`'s riscv64/PLIC
|
||||
and aarch64/GIC pattern. Before trusting any of it, checked live via QMP `query-pci`: the
|
||||
formula used for amd64 (`GSI = 16 + ((slot + pin - 1) % 4)`, the commonly-cited Q35/ICH9
|
||||
default) predicted GSI 16 for the xHCI controller at PCI slot 4; the real, QEMU-reported IRQ
|
||||
was **10**. Embedded ICH9 functions (SATA/SMBus at slot 31) contradicted the same formula too,
|
||||
ruling out a simple one-line fix — deriving the real PIRQ routing would need reading the ICH9
|
||||
LPC's actual PIRQ control registers (config offset 0x60+), a genuinely deep chipset detour nobody
|
||||
asked for.
|
||||
|
||||
**Reframed instead of chasing the formula further.** Section U item 6 (Captain Bob's own words)
|
||||
specifies the trigger as *"interrupt-driven, coarse cadence, cheap early-exit… quick check
|
||||
blocks… done… ignore what we can"* and names `sk_repl_idle()` — confirmed empty and ready
|
||||
(Section V) — as its home. USB insertion is a human-timescale event, not a hot path; polling
|
||||
costs nothing meaningful here. Reverted all interrupt-routing code (the IOAPIC PCI-routing
|
||||
functions, `isr_stub34`, the three `interrupts.c` dispatch hooks) back to exactly the committed
|
||||
baseline (`git diff` against those files is empty), keeping only the already-correct Event Ring
|
||||
TRB-parsing body — renamed `xhci_isr()` → `xhci_poll_events()`, called from `sk_repl_idle()` at
|
||||
its existing ~1s cadence (`SK_IDLE_BEAT_INTERVAL`). `xhci_bringup()` never enables
|
||||
`USBCMD.INTE`/`IMAN.IE`, so the IMAN/USBSTS RW1C ack-writes are correctly absent from the poll
|
||||
path — nothing ever latches them.
|
||||
|
||||
**A real ack-sequence bug caught before it shipped, in the discarded interrupt path.**
|
||||
`xhci_isr()`'s first draft never cleared `IMAN.IP`/`USBSTS.EINT` — with the line programmed
|
||||
level-triggered, the controller would have re-asserted immediately on servicing, a wedge (the
|
||||
same class of bug `virtio_input.c`'s own header comment already documents: "skipping this leaves
|
||||
the condition latched — storm or hang, not a subtle bug"). Moot once the interrupt path was
|
||||
dropped, but the discipline that caught it (advisor review before committing) is worth recording.
|
||||
|
||||
**A real, reproducible false alarm caught and corrected during verification — not folded into
|
||||
this item's own scope.** `HEARTBEAT-TICKS@` was queried at the idle `ok>` prompt on
|
||||
aarch64/riscv64 to check whether the poll fires at all, showed a static, byte-identical value
|
||||
(77) across independent fresh boots, and was initially treated as a stalled hardware timer — a
|
||||
serious-looking regression. Root cause: `HEARTBEAT-TICKS@` reads `vm->heartbeat.tick_count`, a
|
||||
**per-VM software counter** (Loop #7's own model, incremented only during FORTH word execution)
|
||||
— not the kernel's real ISR-driven `heartbeat_ticks()`. It correctly stops advancing once the
|
||||
REPL idles and nothing executes; this is expected behavior, not a bug. Confirmed directly with a
|
||||
temporary diagnostic word (`KHB-TICKS@`, added, used, reverted — probe-capture-revert
|
||||
convention) exposing the real kernel counter: it advanced 3830→7099 in ~20s of idle on riscv64,
|
||||
proving the ISR and `sk_repl_idle()`'s gate are both healthy. The actual reason no Port Status
|
||||
Change event had appeared in the original aarch64/riscv64 tests was simpler: the USB device had
|
||||
been attached *before* boot in those runs, so the controller's reset during bring-up produced no
|
||||
fresh state-change transition to detect — not a timer problem at all. A genuine post-boot hotplug
|
||||
(attach after reaching `ok>`) produced an immediate, clean event on both.
|
||||
|
||||
**Verified end to end, all three architectures, genuine post-boot hotplug via QMP
|
||||
`device_add usb-storage`/`device_del`:** amd64, aarch64, and riscv64 all print
|
||||
`xhci: port status change event` within seconds of a live attach, and amd64 additionally
|
||||
confirmed the reverse (detach) and a second attach both individually detected. `-device
|
||||
qemu-xhci,id=xhci0[,addr=0x4]` added to all three QEMU launch targets (`Makefile.starkernel`) —
|
||||
required for anything past this point to be testable at all, including future milestones.
|
||||
Final three-arch acceptance (probe-free, `clean qemu`): all three boot to `ok>` cleanly,
|
||||
`logs/20260822-091923/amd64/`, `logs/20260822-092035/aarch64/`, `logs/20260822-092229/riscv64/`.
|
||||
|
||||
**2e. Hotplug detection (the actual trigger for everything in Section U)**
|
||||
- [ ] On a Port Status Change event, read the corresponding Port Register to determine
|
||||
connect vs. disconnect
|
||||
@@ -3290,8 +3357,9 @@ for whenever this resumes: UEFI-only boot path, no legacy BIOS/MBR support, no G
|
||||
other bootloader in the chain** — `starkernel_loader.efi` is meant to be the entire boot path,
|
||||
generic and minimal, not one stage in a longer one.
|
||||
|
||||
- [ ] Confirm which physical machine will receive the first real-hardware boot test (CPU
|
||||
arch — amd64 assumed given the SanDisk drives on hand, but confirm)
|
||||
- [x] Confirm which physical machine will receive the first real-hardware boot test (CPU
|
||||
arch — amd64 assumed given the SanDisk drives on hand, but confirm) — **Beelink SER5**,
|
||||
named 2026-08-22 during Milestone 2d work. amd64, matching the SanDisk-drive assumption.
|
||||
- [ ] Build a fresh `starkernel.iso` via `make -f Makefile.starkernel ARCH=amd64 clean` +
|
||||
the ISO-build step (already runs as part of `qemu` target — confirm it can be invoked
|
||||
standalone without also launching QEMU, or just let the `qemu` target build it and
|
||||
|
||||
Reference in New Issue
Block a user