xHCI: drive Port Reset on port reuse; WIREBIND: kill the console VM too (FABRIC-3.md §XI.5)
Build / build-amd64-iso (push) Canceled after 0s
Build / build-aarch64-iso (push) Canceled after 0s
Build / build-riscv64-img (push) Canceled after 0s

Two independent bugs that together caused a reliable hotplug wedge:
reusing an xHCI port for a second identity right after an unclean
detach of a first would leave no further hotplug events reaching the
guest at all.

Bug 1 (xhci.c/xhci_driver.h): the xHCI driver never drove PORTSC.PR --
a known, named gap since Milestone 2e (the code's own comment flagged
it, PORTSC_PR/PRC were defined but never referenced). A port's first
connect each boot reads PED already set, so skipping the reset
happened to work; a second device on the same port after a prior
disconnect reads PED clear, and Address Device reliably failed without
an explicit reset cycle. New XHCI_CONN_AWAIT_PORT_RESET state drives
PR and waits for PED to read set before proceeding to Enable Slot.

Bug 2 (capsule_wirebind.c): capsule_wirebind_eject()/unclean_detach()
compared g_repl_active_vm against the *user* VM's pointer
(g_wirebind_attached_vm_id tracks that one, not the console VM) --
never equal, since USE/g_repl_active_vm always points at the console
VM. The guard never fired and the console VM was never killed at all,
only orphaned -- paired to a dead user VM but still the REPL's active
session. New wirebind_teardown_console() helper resolves and tears
down the console VM by its own tracked bare username.

Verified live on amd64: the exact repro (identity 01 on port 2,
unclean detach, identity 02 on the same port immediately after) --
previously wedged with "xhci: address device failed" and no further
hotplug activity; now attaches cleanly and fast, both VMs' KILL
messages appear, console is immediately interactive on the new
identity. Three-arch clean qemu acceptance passed, all clean on the
first attempt.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014Ec88YKxxhZGG1RNnune78
This commit is contained in:
Robert Allan James
2026-09-09 22:11:09 -04:00
co-authored by Claude Sonnet 5
parent 9ea5580ace
commit b301317902
14 changed files with 36161 additions and 38 deletions
+39 -8
View File
@@ -262,6 +262,43 @@ static int wirebind_resolve_attached(VMRegistryEntry *out) {
return 0;
}
/* FABRIC-3.md, 2026-09-09: shared by capsule_wirebind_eject() and
* capsule_wirebind_unclean_detach() -- tears down the *console* VM
* (registered under the bare username, e.g. "00" -- capsule_console_
* birth() in capsule_wirebind_try_attach() above) paired with the user
* VM ("<username>~user") those two functions already kill.
*
* Real bug found live, 2026-09-09: both functions used to compare
* sk_repl_get_active_vm() against entry.vm_ptr, where `entry` comes
* from wirebind_resolve_attached() -- which resolves g_wirebind_
* attached_vm_id, the *user* VM's id (see that field's own doc comment:
* "the user VM, not the console VM"). But USE/g_repl_active_vm always
* points at the *console* VM, never the user VM -- so that comparison
* was never true, the dangling-pointer guard never fired, and the
* console VM was never killed at all here, just silently orphaned
* (paired to a now-dead user VM, but still live and still whatever the
* active REPL session was). Confirmed live: reusing an xHCI port for a
* second identity right after an unclean detach of a first left the
* console stuck showing the first identity's own dead-paired prompt.
*
* Uses g_wirebind_attached_username (the bare username, captured at
* attach time) rather than deriving it from entry.name (which is
* already the "<user>~user" registry name) -- see capsule_wirebind_
* try_attach()'s own comment on why the two names are tracked
* separately. Called before either caller clears that field. */
static void wirebind_teardown_console(void) {
if (g_wirebind_attached_username[0] == '\0') return;
VMRegistryEntry console_entry;
if (capsule_vm_find_by_name(g_wirebind_attached_username, &console_entry) != 0 ||
console_entry.state != VM_STATE_LIVE) {
return;
}
if (sk_repl_get_active_vm() == (VM *)console_entry.vm_ptr) {
sk_repl_set_active_vm((VM *)0);
}
capsule_vm_kill(console_entry.name);
}
int capsule_wirebind_eject(void) {
VMRegistryEntry entry;
if (wirebind_resolve_attached(&entry) != 0) {
@@ -273,10 +310,7 @@ int capsule_wirebind_eject(void) {
* remove (§F.10 decision 1). */
blk_vm_flush_all((VM *)entry.vm_ptr);
if (sk_repl_get_active_vm() == (VM *)entry.vm_ptr) {
sk_repl_set_active_vm((VM *)0);
}
wirebind_teardown_console();
capsule_vm_kill(entry.name);
g_wirebind_attached_valid = 0;
g_wirebind_attached_dev = (struct blkio_dev *) 0;
@@ -306,10 +340,7 @@ void capsule_wirebind_unclean_detach(struct blkio_dev *dev) {
/* Device is already gone (§F.10 decision 2) -- no flush attempted;
* data since the last flush is lost, matching real-OS unclean-removal
* semantics. */
if (sk_repl_get_active_vm() == (VM *)entry.vm_ptr) {
sk_repl_set_active_vm((VM *)0);
}
wirebind_teardown_console();
capsule_vm_kill(entry.name);
g_wirebind_attached_valid = 0;
g_wirebind_attached_dev = (struct blkio_dev *) 0;
+81 -21
View File
@@ -1358,27 +1358,82 @@ static void xhci_handle_port_connected(xhci_dev_t *dev, uint32_t port_id, uint32
{
log_message(LOG_DEBUG, "xhci: port status change -- device connected");
/* Milestone 2e prep: Address Device requires the port in Default
* state. USB3 links train and enable themselves; USB2 needs software
* to drive PORTSC.PR and wait for PRC/PED before the device will
* respond to addressing -- not yet known which this driver's ports
* need, so log raw PORTSC and PED rather than assume. */
* state (PED set). USB3 links train and enable themselves; USB2
* needs software to drive PORTSC.PR and wait for PRC/PED before the
* device will respond to addressing. */
xhci_log_hex32("xhci: portsc=", portsc);
log_message(LOG_DEBUG, (portsc & XHCI_PORTSC_PED)
? "xhci: port enabled (PED set)"
: "xhci: port not yet enabled (PED clear)");
if (port_id > XHCI_MAX_TRACKED_PORTS) {
log_message(LOG_WARN, "xhci: port beyond tracked range -- enable slot skipped");
return;
}
/* FABRIC-3.md, 2026-09-09: a reset this driver itself kicked off
* (below) has landed as a fresh port-status-change event -- CCS is
* still set (device never left), so this is the *same* connect
* continuing, not a new one. Proceed to Enable Slot now that PED
* reads set; if it still doesn't, the reset is still in progress
* (real hardware/QEMU can take more than one event to settle it) --
* just wait for the next port-status-change event, same as every
* other outstanding-command wait in this driver (no explicit
* timeout, matching this file's existing convention). */
if (dev->connect_state == XHCI_CONN_AWAIT_PORT_RESET &&
dev->pending_connect_port_id == port_id) {
if (portsc & XHCI_PORTSC_PED) {
log_message(LOG_DEBUG, "xhci: port reset complete (PED set)");
dev->pending_connect_speed = XHCI_PORTSC_SPEED(portsc);
dev->connect_state = XHCI_CONN_AWAIT_ENABLE_SLOT;
xhci_cmd_enable_slot(dev);
} else {
log_message(LOG_DEBUG, "xhci: port reset still in progress (PED clear)");
}
return;
}
/* Only one Enable Slot in flight at a time (real xHCI Command Ring
* semantics, one outstanding command) -- if another connect's slot
* request is still outstanding, this one is queued (FABRIC-3.md §VII
* item 3, 2026-09-05 -- used to be dropped here) rather than lost. */
if (port_id > XHCI_MAX_TRACKED_PORTS) {
log_message(LOG_WARN, "xhci: port beyond tracked range -- enable slot skipped");
} else if (dev->connect_state == XHCI_CONN_IDLE) {
if (dev->connect_state != XHCI_CONN_IDLE) {
xhci_queue_pending_event(dev, port_id, 1, portsc, 0);
return;
}
if (portsc & XHCI_PORTSC_PED) {
/* Common case: the port already reads enabled -- either this is
* the very first connect on this port this boot (QEMU/most real
* USB3 root hubs present it already-enabled), or a USB3 link
* that trained/enabled itself. Go straight to Enable Slot, same
* as this driver has always done for that case. */
log_message(LOG_DEBUG, "xhci: port enabled (PED set)");
dev->pending_connect_port_id = port_id;
dev->pending_connect_speed = XHCI_PORTSC_SPEED(portsc);
dev->connect_state = XHCI_CONN_AWAIT_ENABLE_SLOT;
xhci_cmd_enable_slot(dev);
} else {
xhci_queue_pending_event(dev, port_id, 1, portsc, 0);
/* FABRIC-3.md, 2026-09-09: closes the "reused-port hotplug
* wedge" this function's own comment used to flag as an open
* question ("not yet known which this driver's ports need").
* Confirmed live: a second device attached to a port whose
* previous occupant just disconnected reads PED clear here, and
* without a reset cycle Address Device reliably fails
* ("xhci: address device failed") -- the port needs an explicit
* Port Reset before it will respond to addressing again, exactly
* as USB2 devices always have. Same preserve-PP-only write
* discipline xhci_poll_events()'s own CSC ack already uses;
* PORTSC.PR is RW1S and self-clears in hardware once the reset
* completes (PRC set, observed above via the PED check on the
* event this produces), so no matching "clear" write is needed. */
log_message(LOG_DEBUG, "xhci: port not yet enabled (PED clear) -- driving port reset");
dev->pending_connect_port_id = port_id;
dev->connect_state = XHCI_CONN_AWAIT_PORT_RESET;
xhci_port_regs_t *port = xhci_port_regs(dev, port_id);
if (port) {
port->portsc = (portsc & XHCI_PORTSC_PP) | XHCI_PORTSC_PR;
} else {
dev->connect_state = XHCI_CONN_IDLE;
dev->pending_connect_port_id = 0;
}
}
}
@@ -1441,10 +1496,10 @@ static void xhci_scan_ports_for_already_connected(xhci_dev_t *dev)
if (!(portsc & XHCI_PORTSC_CCS)) continue;
xhci_handle_port_connected(dev, port_id, portsc);
/* Acknowledge CSC the same way xhci_poll_events() does, in case
* the controller latched it during reset -- harmless if it was
* never set. */
port->portsc = (portsc & XHCI_PORTSC_PP) | XHCI_PORTSC_CSC;
/* Acknowledge CSC and PRC the same way xhci_poll_events() does, in
* case the controller latched either during reset -- harmless if
* neither was actually set. */
port->portsc = (portsc & XHCI_PORTSC_PP) | XHCI_PORTSC_CSC | XHCI_PORTSC_PRC;
}
}
@@ -1634,12 +1689,17 @@ void xhci_poll_events(void)
}
}
}
/* Acknowledge only CSC (RW1CS): preserve PP, write 0 for
* PED/PR (writing 1 there disables the port / starts a new
* reset -- not intended here) and for every other _C bit
* (writing 0 leaves them untouched, not cleared) -- the
* same discipline this driver already applies to ERDP.EHB. */
port->portsc = (portsc & XHCI_PORTSC_PP) | XHCI_PORTSC_CSC;
/* Acknowledge CSC and PRC (both RW1C): preserve PP, write 0
* for PED/PR (writing 1 there disables the port / starts a
* new reset -- not intended here) and for every other _C
* bit (writing 0 leaves them untouched, not cleared) -- the
* same discipline this driver already applies to ERDP.EHB.
* PRC added FABRIC-3.md 2026-09-09 alongside
* xhci_handle_port_connected()'s new port-reset drive --
* unconditional like CSC always was (RW1C: writing 1 to an
* already-clear bit is a no-op, harmless when this event
* wasn't reset-related). */
port->portsc = (portsc & XHCI_PORTSC_PP) | XHCI_PORTSC_CSC | XHCI_PORTSC_PRC;
break;
}
case XHCI_TRB_TYPE_COMMAND_COMPLETION_EVT: {