From 5d55217212adc5eea7af1d7262aea02658b0b093 Mon Sep 17 00:00:00 2001 From: Daniel Samson <12231216+daniel-samson@users.noreply.github.com> Date: Sat, 8 Aug 2026 12:16:42 +0100 Subject: [PATCH] usb: a slot the controller granted is always handed back MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- docs/bounds-track-plan.md | 7 ++++- .../drivers/usb-xhci-bus/usb-xhci-library.zig | 31 ++++++++++++++----- 2 files changed, 30 insertions(+), 8 deletions(-) diff --git a/docs/bounds-track-plan.md b/docs/bounds-track-plan.md index 9c69f37..90d34c5 100644 --- a/docs/bounds-track-plan.md +++ b/docs/bounds-track-plan.md @@ -17,7 +17,12 @@ next one starts.* | 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 | | 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. **Branch:** `claude/bounds-track`. diff --git a/system/drivers/usb-xhci-bus/usb-xhci-library.zig b/system/drivers/usb-xhci-bus/usb-xhci-library.zig index 7f3a9dc..9d0186b 100644 --- a/system/drivers/usb-xhci-bus/usb-xhci-library.zig +++ b/system/drivers/usb-xhci-bus/usb-xhci-library.zig @@ -943,7 +943,8 @@ pub const Controller = struct { return null; }; 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; }; device.* = .{ @@ -1145,7 +1146,11 @@ pub const Controller = struct { std.log.info("hub slot {d} port {d}: Enable Slot failed", .{ hub.slot_id, port }); 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); device.* = .{ .used = true, @@ -1747,15 +1752,27 @@ pub const Controller = struct { for (&self.subscriptions) |*subscription| { 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(.{ - .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 (code != @intFromEnum(CompletionCode.success)) - std.log.info("slot {d}: Disable Slot completion code {d}", .{ device.slot_id, code }); - } else std.log.info("slot {d}: Disable Slot timed out", .{device.slot_id}); + std.log.info("slot {d}: Disable Slot completion code {d}", .{ slot_id, code }); + } else std.log.info("slot {d}: Disable Slot timed out", .{slot_id}); const array: [*]volatile u64 = @ptrFromInt(self.device_context_array.virtual); - array[device.slot_id] = 0; - device.used = false; + array[slot_id] = 0; } };