diff --git a/docs/bounds-track-plan.md b/docs/bounds-track-plan.md index e709c81..3c387f7 100644 --- a/docs/bounds-track-plan.md +++ b/docs/bounds-track-plan.md @@ -14,7 +14,7 @@ next one starts.* |---|---|---| | L1 | Reclamation: a dead task's registrations die with its claims | **stopped — the step was wrong; see open question 4** | | L2 | Bounds build check + allowlist; declare what we have already touched | **done** — `zig build bounds`, 273 allowlisted, 5 declared | -| L3 | xHCI: slot count from `HCSPARAMS1.MaxSlots`, not 8 | not started | +| 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 | not started | | L5 | USB: interfaces from the descriptor, and the misattributed-endpoint bug | not started | | L6 | xHCI: a failed `allocateDevice` stops leaking an enabled slot | not started | diff --git a/system/drivers/usb-xhci-bus/usb-xhci-bus.zig b/system/drivers/usb-xhci-bus/usb-xhci-bus.zig index 7a1d8b0..be7a9b3 100644 --- a/system/drivers/usb-xhci-bus/usb-xhci-bus.zig +++ b/system/drivers/usb-xhci-bus/usb-xhci-bus.zig @@ -228,8 +228,13 @@ fn initialise(endpoint: ipc.Handle) bool { _ = logging.write("/system/drivers/usb-xhci-bus: controller reset/bring-up failed\n"); return false; }; - std.log.info("controller running ({d} slots, {d}-byte contexts)", .{ + // Both numbers, because for a long time they disagreed silently: the controller + // reported its real slot count and the driver tracked a fixed 8 of them, so a + // ninth device — trivially reachable behind a hub — simply did not exist. They + // must now be equal, and the suite asserts it. + std.log.info("controller running ({d} slots, tracking {d}, {d}-byte contexts)", .{ controller.?.max_slots, + controller.?.devices.len, controller.?.context_size, }); // The proof of life: a No-Op command round-trips the command ring, the event diff --git a/system/drivers/usb-xhci-bus/usb-xhci-library.zig b/system/drivers/usb-xhci-bus/usb-xhci-library.zig index fb0862d..d0cf396 100644 --- a/system/drivers/usb-xhci-bus/usb-xhci-library.zig +++ b/system/drivers/usb-xhci-bus/usb-xhci-library.zig @@ -328,9 +328,6 @@ pub const Report = struct { data: [64]u8 = [_]u8{0} ** 64, }; -// How many addressed devices this driver tracks at once. QEMU presents a handful -// (a keyboard, a mouse, a storage stick); a fuller machine would grow this. -const max_devices = 8; const max_subscriptions = 8; const report_queue_capacity = 16; @@ -449,7 +446,14 @@ pub const Controller = struct { device_context_array: memory.DmaRegion, command_ring: ProducerRing, event_ring: EventRing, - devices: [max_devices]Device = [_]Device{.{}} ** max_devices, + /// One entry per device slot the **controller** says it has (HCSPARAMS1.MaxSlots, + /// 1..255), allocated at bring-up. This used to be a fixed 8 with the comment "QEMU + /// presents a handful; a fuller machine would grow this" — which is the shape the + /// bounds rule exists to stop, since the controller has always reported the real + /// number and `op_config` below is already programmed with it. A desktop's keyboard, + /// mouse, webcam, headset, hub and two sticks reach 8 without trying, and everything + /// past it vanished (behind a hub, without even a log line). + devices: []Device, subscriptions: [max_subscriptions]Subscription = [_]Subscription{.{}} ** max_subscriptions, report_queue: [report_queue_capacity]Report = [_]Report{.{}} ** report_queue_capacity, report_count: usize = 0, @@ -582,8 +586,16 @@ pub const Controller = struct { .device_context_array = undefined, .command_ring = undefined, .event_ring = undefined, + .devices = &.{}, }; + // One tracking slot per slot the controller reports. A controller that claims + // no slots cannot address anything, so treat that as a dead controller rather + // than allocating nothing and failing mysteriously later. + if (self.max_slots == 0) return null; + self.devices = memory.allocator().alloc(Device, self.max_slots) catch return null; + for (self.devices) |*device| device.* = .{}; + // Wait for the controller to report ready, then halt it if it is running. if (!waitClear(self.operational(op_usbsts), usbsts_controller_not_ready)) return null; if (read32(self.operational(op_usbcmd)) & usbcmd_run != 0) { @@ -790,7 +802,7 @@ pub const Controller = struct { } fn allocateDevice(self: *Controller) ?*Device { - for (&self.devices) |*device| { + for (self.devices) |*device| { if (!device.used) return device; } return null; @@ -1011,7 +1023,7 @@ pub const Controller = struct { /// The next pending (hub, downstream-port) change to service, or null. Clears /// the returned port's bit. Called on the bus tick. pub fn takeHubChange(self: *Controller) ?struct { hub: *Device, port: u16 } { - for (&self.devices) |*device| { + for (self.devices) |*device| { if (!device.used or !device.is_hub or device.hub_change_mask == 0) continue; const bit: u5 = @intCast(@ctz(device.hub_change_mask)); device.hub_change_mask &= ~(@as(u32, 1) << bit); @@ -1084,7 +1096,7 @@ pub const Controller = struct { } pub fn deviceOnHubPort(self: *Controller, hub: *Device, port: u16) ?*Device { - for (&self.devices) |*device| { + for (self.devices) |*device| { if (device.used and device.parent_slot == hub.slot_id and device.parent_port == port) return device; } return null; @@ -1364,7 +1376,7 @@ pub const Controller = struct { /// Find the tracked device and interface an assigned device id belongs to. pub fn findInterface(self: *Controller, device_id: u64) ?struct { device: *Device, interface: *InterfaceInfo } { - for (&self.devices) |*device| { + for (self.devices) |*device| { if (!device.used) continue; for (device.interfaces[0..device.interface_count]) |*interface| { if (interface.registered_device_id == device_id) return .{ .device = device, .interface = interface }; @@ -1633,7 +1645,7 @@ pub const Controller = struct { /// The tracked device on `port`, or null. pub fn deviceOnPort(self: *Controller, port: u32) ?*Device { - for (&self.devices) |*device| { + for (self.devices) |*device| { if (device.used and device.port == port) return device; } return null; @@ -1642,7 +1654,7 @@ pub const Controller = struct { /// The next used device whose parent hub is `hub_slot` and slot id > `after` /// (for recursive teardown when a hub itself disconnects), or null. pub fn nextChildOf(self: *Controller, hub_slot: u8, after: u8) ?*Device { - for (&self.devices) |*device| { + for (self.devices) |*device| { if (device.used and device.parent_slot == hub_slot and device.slot_id > after) return device; } return null; diff --git a/test/qemu_test.py b/test/qemu_test.py index 8bcccc5..a46907f 100644 --- a/test/qemu_test.py +++ b/test/qemu_test.py @@ -625,11 +625,18 @@ CASES = [ # manager spawn the USB keyboard driver, which opens its device over the # transfer protocol, asks for boot protocol, subscribes to its interrupt # endpoint, and comes up — proof the class-driver <-> controller path works. + # + # It also pins the slot count to the hardware's answer. The backreference is the + # assertion: the driver must track exactly as many device slots as the controller + # reports in HCSPARAMS1.MaxSlots. It tracked a fixed 8 while QEMU's xHCI reports + # 64, so seven eighths of the controller was invisible and a device behind a hub + # past the eighth vanished without a log line. \1 fails the moment they diverge. {"name": "usb-hid", "smp": 4, "timeout": 150, # usb-kbd/usb-mouse ride the default boot xHCI bus (see qemu_args). - "expect": r"(?=[\s\S]*usb-hid-keyboard: ok)(?=[\s\S]*usb-hid-mouse: ok)", + "expect": r"(?=[\s\S]*usb-xhci-bus: controller running \((\d+) slots, tracking \1,)" + r"(?=[\s\S]*usb-hid-keyboard: ok)(?=[\s\S]*usb-hid-mouse: ok)", "fail": r"DANOS-TEST-RESULT: FAIL"}, # Keyboard echo: inject a known phrase via QMP send-key; the usb-hid-keyboard # driver decodes it and echoes each character to the log (the simple diff --git a/tools/bounds-allowlist.txt b/tools/bounds-allowlist.txt index 542e2c6..dd38ac1 100644 --- a/tools/bounds-allowlist.txt +++ b/tools/bounds-allowlist.txt @@ -83,7 +83,6 @@ system/drivers/usb-xhci-bus/usb-xhci-library.zig:data system/drivers/usb-xhci-bus/usb-xhci-library.zig:descriptor system/drivers/usb-xhci-bus/usb-xhci-library.zig:head system/drivers/usb-xhci-bus/usb-xhci-library.zig:header -system/drivers/usb-xhci-bus/usb-xhci-library.zig:max_devices system/drivers/usb-xhci-bus/usb-xhci-library.zig:max_endpoints_per_interface system/drivers/usb-xhci-bus/usb-xhci-library.zig:max_interfaces system/drivers/usb-xhci-bus/usb-xhci-library.zig:max_subscriptions