usb: a slot the controller granted is always handed back

Both device-setup paths issued a successful Enable Slot and then returned
null if allocateDevice failed, without disabling it. A slot the driver
forgets is one the controller never reissues, so each attempt lost one
permanently for the boot. The hub path did it with no log line at all.

Both now release the slot through a shared disableSlot, extracted from
tearDownDevice, and the hub path warns like the root-port path does.

tearDownDevice also now frees the interface list. That allocation arrived
with the previous commit, so an unplug would have leaked it — found while
reading the teardown path for this fix rather than by a test.

No regression test, and it is recorded as open question 5 rather than
implied. After the slot count became the controller's own figure, reaching
this path needs more devices than the controller has slots: QEMU offers four
against sixty-four. What was verified is that the new path RUNS correctly —
pinning tracking to 2 with four devices attached produced "port 6 setup: no
free device slot", the first two devices enumerated normally, and no Disable
Slot error or timeout appeared, which is how disableSlot reports failure.

Suite 116/116.
This commit is contained in:
Daniel Samson
2026-08-08 12:16:42 +01:00
parent 729b40ece7
commit 5d55217212
2 changed files with 30 additions and 8 deletions
+6 -1
View File
@@ -17,7 +17,12 @@ next one starts.*
| L3 | xHCI: slot count from `HCSPARAMS1.MaxSlots`, not 8 | **done** — QEMU reports 64; the driver tracked 8 | | L3 | xHCI: slot count from `HCSPARAMS1.MaxSlots`, not 8 | **done** — QEMU reports 64; the driver tracked 8 |
| L4 | USB: configuration descriptor sized by `wTotalLength`, not 512 | **done** — QEMU tops out at 211 bytes, so the case catches the class, not the original trigger | | L4 | USB: configuration descriptor sized by `wTotalLength`, not 512 | **done** — QEMU tops out at 211 bytes, so the case catches the class, not the original trigger |
| L5 | USB: interfaces from the descriptor, and the misattributed-endpoint bug | **done** — fix is by construction; no direct test, see open question 5 | | L5 | USB: interfaces from the descriptor, and the misattributed-endpoint bug | **done** — fix is by construction; no direct test, see open question 5 |
| L6 | xHCI: a failed `allocateDevice` stops leaking an enabled slot | not started | | L6 | xHCI: a failed `allocateDevice` stops leaking an enabled slot | **done** — path forced and verified; no regression test, see open question 5 |
**Run complete.** L1 stopped (the step was wrong), L2–L6 landed. Suite 115 → 116.
Allowlist 278 → 269. Two steps ship without a permanent regression test, both because
QEMU's USB devices are too small to reach the paths — see open question 5, which is the
audit's own lesson recurring: the test rig is smaller than a real machine.
**Suite:** 115/115 at the start of the run. **Suite:** 115/115 at the start of the run.
**Branch:** `claude/bounds-track`. **Branch:** `claude/bounds-track`.
@@ -943,7 +943,8 @@ pub const Controller = struct {
return null; return null;
}; };
const device = self.allocateDevice() orelse { const device = self.allocateDevice() orelse {
std.log.info("port {d} setup: no free device slot", .{port}); std.log.warn("port {d} setup: no free device slot", .{port});
self.disableSlot(slot_id); // the controller already gave it to us
return null; return null;
}; };
device.* = .{ device.* = .{
@@ -1145,7 +1146,11 @@ pub const Controller = struct {
std.log.info("hub slot {d} port {d}: Enable Slot failed", .{ hub.slot_id, port }); std.log.info("hub slot {d} port {d}: Enable Slot failed", .{ hub.slot_id, port });
return null; return null;
}; };
const device = self.allocateDevice() orelse return null; const device = self.allocateDevice() orelse {
std.log.warn("hub slot {d} port {d}: no free device slot", .{ hub.slot_id, port });
self.disableSlot(slot_id); // the controller already gave it to us
return null;
};
const child_speed = mapHubPortSpeed(speed); const child_speed = mapHubPortSpeed(speed);
device.* = .{ device.* = .{
.used = true, .used = true,
@@ -1747,15 +1752,27 @@ pub const Controller = struct {
for (&self.subscriptions) |*subscription| { for (&self.subscriptions) |*subscription| {
if (subscription.active and subscription.slot_id == device.slot_id) subscription.active = false; if (subscription.active and subscription.slot_id == device.slot_id) subscription.active = false;
} }
self.disableSlot(device.slot_id);
device.freeInterfaces();
device.used = false;
}
/// Hand a slot back to the controller and clear its context-array entry.
///
/// Every path that has issued a successful Enable Slot owes this, including the
/// ones that then fail to bring the device up. A slot the driver forgets is one
/// the controller never reissues, so the loss is permanent for the boot: the two
/// setup paths used to return null straight after a failed `allocateDevice`,
/// leaking a slot per attempt — and the hub path did it without even a log line.
fn disableSlot(self: *Controller, slot_id: u8) void {
const physical = self.submitCommand(.{ const physical = self.submitCommand(.{
.control = trbControl(.disable_slot, @as(u32, device.slot_id) << 24), .control = trbControl(.disable_slot, @as(u32, slot_id) << 24),
}); });
if (self.awaitCommand(physical)) |code| { if (self.awaitCommand(physical)) |code| {
if (code != @intFromEnum(CompletionCode.success)) if (code != @intFromEnum(CompletionCode.success))
std.log.info("slot {d}: Disable Slot completion code {d}", .{ device.slot_id, code }); std.log.info("slot {d}: Disable Slot completion code {d}", .{ slot_id, code });
} else std.log.info("slot {d}: Disable Slot timed out", .{device.slot_id}); } else std.log.info("slot {d}: Disable Slot timed out", .{slot_id});
const array: [*]volatile u64 = @ptrFromInt(self.device_context_array.virtual); const array: [*]volatile u64 = @ptrFromInt(self.device_context_array.virtual);
array[device.slot_id] = 0; array[slot_id] = 0;
device.used = false;
} }
}; };