From c3db963164429155326dedc1dab33a61f314aedd Mon Sep 17 00:00:00 2001 From: Robert Allan James Date: Sat, 5 Sep 2026 20:45:20 -0400 Subject: [PATCH] =?UTF-8?q?FABRIC-3.md=20=C2=A7VII:=20xHCI/BOT=20single-de?= =?UTF-8?q?vice=20architecture=20=E2=80=94=20plan=20only,=20halted?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Documents the full scope of the driver's single-device-at-a-time state (connect/enumerate, control-transfer, and BOT state machines), the persistent-vs-in-flight distinction that bounds the fix, the concurrency target, and a numbered punch list. No driver code has been touched — implementation is explicitly held pending go-ahead. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_018EjXFo7mPXjUMjfJeuUUz4 --- FABRIC-3.md | 150 ++++++++++++++++++ .../zuse-thumb-ident.img} | Bin 2 files changed, 150 insertions(+) rename disk/{zuse.img => thumbdrives/zuse-thumb-ident.img} (100%) diff --git a/FABRIC-3.md b/FABRIC-3.md index ff0ea45..9f3d94e 100644 --- a/FABRIC-3.md +++ b/FABRIC-3.md @@ -964,3 +964,153 @@ puts a concrete, purchasable board under that already-named milestone. [Puzhi — PZ7020-StarLite](https://www.en.puzhi.com/Product/AMD-FPGA-Development-Board/Zynq-7000-SoC/PZ7020-StarLite), [Xilinx/AMD — Zynq-7000 SoC Data Sheet (DS190)](https://www.mouser.com/datasheet/2/903/ds190-Zynq-7000-Overview-1595492.pdf), [PCBSync — XC7Z010 vs XC7Z020 comparison](https://pcbsync.com/xilinx-xc7z010/). + +## VII. xHCI/BOT driver: single-device architecture must go multi-device — PLANNED, NOT YET BUILT + +**Status 2026-09-05: design + punch list only. No code has been touched. Halted pending +Captain Bob's explicit "okay" to begin implementation** — this section exists to write the +decision down before building, per this project's own standing methodology +([[feedback_fabric_series_effective_methodology]]). + +### VII.1 — How this surfaced + +While scoping the identity-thumbdrive minting workflow (8 real WIREBIND/Zuse identities to +be minted from blank images in `disk/thumbdrives/`), the plan called for hot-swapping a +single USB slot via QMP because the kernel's xHCI/BOT stack tracks exactly one attached mass- +storage device at a time. Bob interrupted before that script ran: *"Wait! We can only have +one usb slot. is that a QEMU limitation?"* It is not — confirmed live via `qemu-xhci`'s own +multi-port root hub and this driver's own `port_slot_id[XHCI_MAX_TRACKED_PORTS]` (32 tracked +ports, `include/starkernel/xhci.h:515`), which is already multi-port-aware. Everything +*downstream* of port/slot correlation is a deliberate single-device singleton, dating to +Milestones 2e–2h. Bob's ruling: *"We MUST fix that! It was a deliberate choice at that time +and is no longer true, much in the same way a build flag for console."* — same class of +correction as the headless-console change (§ pending its own FABRIC writeup): a simplification +that was right when made, and is stale now that the project needs it to not be true. + +### VII.2 — Exact scope of the singleton (full inventory, `include/starkernel/xhci_driver.h`) + +`port_slot_id[]` is the only field in `xhci_dev_t` that is already an array. Every field +downstream of a successful Address Device is a single reused scalar, each with an explicit +"single-device scope" comment written at the time it was added: + +**Connect / enumerate state machine:** +- `connect_state`, `pending_connect_port_id`, `pending_connect_slot_id` +- `input_ctx`, `device_ctx`, `ep0_ring` — "only ever addresses one device at a time" + +**Control-transfer / descriptor state machine:** +- `transfer_purpose`, `pending_transfer_slot_id` +- `device_descriptor[18]`, `config_descriptor[128]` — "reused, not per-slot" +- `bulk_in_ep_addr`/`bulk_out_ep_addr`, `bulk_in_ring`/`bulk_out_ring`, endpoint max-packet + fields + +**BOT (Bulk-Only Transport / SCSI) state machine:** +- `bot_cbw`/`bot_csw`, `bot_data_buf[1024]`, `bot_last_tag` +- `bot_cmd_kind`, `bot_last_status`, `bot_expected_data_len` +- `bot_read10_*`/`bot_write10_*` staging fields +- `bot_tur_retries`, `bot_tur_chain_target` +- `bot_cap_last_lba`/`bot_cap_block_size` +- Full stall-recovery sub-state-machine (G.1/§F.14): "this driver runs exactly one bulk + transfer at a time... there is no concurrency to serialize" +- `bot_msc_attach_pending`/`bot_msc_attach_slot_id`/`bot_msc_attached`/`bot_msc_detach_pending` + — the flags `repl.c`'s own `static blkio_dev_t usb_blk_dev` (single-device scope, matching + the xHCI driver's own precedent, per its comment at `repl.c:244`) depends on. + +Also confirmed: this driver runs on **all three architectures**, not just amd64 — `qemu-xhci` +is instantiated for amd64, aarch64, and riscv64 alike (`Makefile.starkernel` lines 819, 903, +993), and `xhci_find_and_map()`/`xhci_bringup()` are called unconditionally from +`kernel_main.c` (no `ARCH_*` guard). Any fix here is a three-architecture change, same +acceptance bar as everything else in this document. + +### VII.3 — The distinction that makes this tractable: persistent vs. in-flight state + +Not everything above needs to become concurrent. Two different categories: + +1. **Per-slot persistent state — must become per-slot (arrays), full stop.** Endpoint + addresses/max-packet, bulk rings, device/config descriptors, the `blkio_dev_t` (and its + `usb_blk_dev` analogue in `repl.c`), and the MSC attach flags. If these stay singular, a + second device attaching while a first is already open **corrupts the first's live state** — + this is the actual bug being fixed, not a cosmetic one. + +2. **Per-transfer in-flight state — stays single, gains a slot selector.** `connect_state`, + `transfer_purpose`, `pending_transfer_slot_id`, `bot_cbw`/`bot_csw`, `bot_cmd_kind`, the + stall-recovery sub-state-machine. Nothing in any real use case (present or planned) reads + two drives at the literal same instant — the driver stays "one transfer in flight at a + time," it just needs to know *which slot* that one transfer belongs to, instead of + assuming there is only ever one possible answer. + +This is the difference between a bounded, scoped refactor (make category 1 arrays, thread a +slot index through category 2) and a full concurrent rewrite of the driver's core state +machine. Only category 1 needs to change shape. + +### VII.4 — Concurrency bound + +Decided 2026-09-05 (Bob): **a small N, matching a real xHCI root hub port count**, not a +hardcoded 2. `XHCI_MAX_TRACKED_PORTS` is already 32 (port/slot correlation layer) — the new +per-slot arrays (category 1 above) get their own bound, `XHCI_MAX_CONCURRENT_MSC` (proposed +default **4**), independent of and smaller than the 32-port tracking table, since concurrent +*mass-storage devices actually open at once* is a much smaller number than *ports the +hardware can electrically report*. Exact value is part of the punch list below (§VII.6 item +2), not fixed yet. + +### VII.5 — Bob's live suggestion: consolidate into one file, pass state explicitly + +Raised mid-scoping: *"maybe refactor the driver into one single driver file and calling from +the dependency? Simplify the code."* Worth noting precisely what's already true and what +isn't, before this becomes a punch-list item: + +- **Already true:** nearly every command-builder function (`xhci_cmd_*`, `xhci_bot_*`, + `xhci_ep0_*`) already takes an explicit `xhci_dev_t *dev` parameter — this is already + dependency-injection-shaped at the call-site level, not a global-variable design. +- **Not true — the actual singleton:** `xhci_poll_events()` itself takes **no** arguments and + reads a module-static `xhci_dev_t*` via `xhci_get_dev()`; `repl.c`'s `sk_repl_idle()` and + `kernel_main.c`'s bring-up call site are the only two places that hold a real handle, and + everything else (the entire event-driven dispatch chain) resolves the device through that + static rather than being handed it. That static-singleton *retrieval* pattern is the one + thing genuinely worth removing as part of this — moving from "one implicit global device" + to "one explicit registry of up to `XHCI_MAX_CONCURRENT_MSC` devices, looked up by slot ID, + passed explicitly" is the natural per-slot-array design from §VII.3 *and* answers Bob's + "calling from the dependency" framing at the same time — no separate mechanism needed for + both goals. +- **File consolidation (`xhci_driver.h` + `src/starkernel/usb/xhci.c` → one file):** a + separate, smaller question from the state-shape fix above, genuinely optional. Current split + is ordinary header/implementation separation, already followed throughout `src/starkernel/` + (nothing else in the tree merges its public API header and its `.c` file). Recommend keeping + the split and judging "simpler" by whether the *singleton* is gone, not by file count — but + this is Bob's call, not a default to override. Tracked as punch-list item 6 (optional). + +### VII.6 — Punch list (numbered = proposed execution order; nothing here has been started) + +1. Introduce `XHCI_MAX_CONCURRENT_MSC` (proposed 4) and convert every category-1 field + (§VII.3) in `xhci_dev_t` from a scalar to a `[XHCI_MAX_CONCURRENT_MSC]` array indexed by a + new small "MSC slot index" (not the raw xHCI hardware slot ID, which can be sparse/large) — + endpoint addresses/max-packet, bulk rings, device/config descriptors, MSC attach flags. +2. Add the index-allocation/lookup pair (`xhci_msc_slot_for(slot_id)` / + `xhci_msc_slot_alloc(slot_id)` or equivalent) that maps an xHCI hardware slot ID to an MSC + slot index, replacing the current implicit "there is only one" assumption everywhere a + category-1 field is touched. +3. Thread an explicit slot selector through the category-2 in-flight fields (§VII.3) so the + existing one-transfer-at-a-time state machine knows *which* device's transfer is in flight, + without itself becoming concurrent. +4. Replace `xhci_get_dev()`'s single-static-pointer retrieval with the explicit per-slot + registry from item 1/2, updating the two real call sites (`kernel_main.c` bring-up, + `repl.c`'s `sk_repl_idle()`) and `xhci_poll_events()`'s own internal dispatch to look up by + slot instead of assuming the one device. +5. Update `repl.c`'s `static blkio_dev_t usb_blk_dev` (currently single-device scope, + `repl.c:244`) to the same `[XHCI_MAX_CONCURRENT_MSC]` shape, and update + `capsule_wirebind.c`/`capsule_zuse_boot.c`'s attach call sites accordingly — this is the + layer the actual identity-login (WIREBIND/Zuse) flow depends on, and the reason this fix + exists at all. +6. *(Optional, Bob's call per §VII.5, not required for correctness)* — consolidate + `xhci_driver.h` + `xhci.c` into one file, if the per-slot-registry change above doesn't + already read as "simple enough" on its own. +7. Three-architecture acceptance (`clean qemu`, amd64 → aarch64 → riscv64, sequential, + foreground) confirming: (a) existing single-device behavior is unchanged when only one + device is ever attached (no regression), (b) two devices attached simultaneously (QMP + `device_add` twice against `xhci0.0`/`xhci0.1` without an intervening `device_del`) both + enumerate, both open as BOT/SCSI devices, and neither corrupts the other's descriptors/ + rings/BOT state. +8. Only after item 7 passes: resume the identity-thumbdrive minting workflow — either via true + simultaneous multi-device attach (Zuse's drive + a target drive both present at once) or the + original QMP hot-swap sequencing (still valid, just no longer the only option). + +**Halt point:** implementation does not begin until Bob says so. diff --git a/disk/zuse.img b/disk/thumbdrives/zuse-thumb-ident.img similarity index 100% rename from disk/zuse.img rename to disk/thumbdrives/zuse-thumb-ident.img