usb: the controller says how many device slots it has
max_devices was 8, with the comment "QEMU presents a handful; a fuller machine would grow this" — a number chosen against the test rig, waiting for a real machine, which is the pattern docs/bounds-track-plan.md exists to stop. The driver already knew the true figure. It reads HCSPARAMS1.MaxSlots at bring-up and writes it straight into op_config, so every slot the controller offers has always been *enabled*; only the array tracking them was 8. QEMU's xHCI reports 64, so seven eighths of the controller was live and invisible, and the ninth device — a keyboard, mouse, webcam, headset, hub and two sticks reach that without trying — disappeared on a hub-attached path that logs nothing at all. The array becomes a slice allocated from max_slots at bring-up. A controller claiming zero slots cannot address anything, so that is now a dead controller rather than an empty allocation failing mysteriously later. The Device Context Base Address Array is a page, 511 usable entries, so it already covered the 255-slot maximum. The bring-up line reports both numbers, and usb-hid asserts they are equal with a backreference rather than a magic number, so the test cannot drift from the hardware. Pinning tracking back to 8 fails it: "64 slots, tracking 8". Suite 115/115.
This commit is contained in:
Daniel Samson
@@ -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 |
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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;
|
||||
|
||||
+8
-1
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user