From 0ac07dadc9fcfd40565c0808a5eee6186b0387a4 Mon Sep 17 00:00:00 2001 From: Daniel Samson <12231216+daniel-samson@users.noreply.github.com> Date: Tue, 21 Jul 2026 21:01:41 +0100 Subject: [PATCH] usb: hot-plug plumbing, all ports powered, interrupter enabled (M20) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit B3 — the runtime lifecycle a hot-pluggable bus needs, plus two init fixes that runtime device arrival depends on: - pump() now handles PORT STATUS CHANGE events (silently dropped before): it queues the port, and the bus driver brings the port up (a device arrived) or tears it down (a device left) on its tick — reporting each interface ChildRemoved to the device manager, which prunes the node, notifies watchers, and lets the class driver's world end honestly, then Disable Slot frees the controller-side state. - ALL root-hub ports are powered at init, not just those with a boot-time device: an unpowered port (PP=0) cannot signal a later connect, so a hot-plug would never be seen. - the interrupter is enabled (IMAN.IE + USBCMD.INTE) while the ring stays polled — some controllers only WRITE runtime events to the ring when the interrupter is enabled. Real-hardware validation is flagged for the user: QEMU's qemu-xhci does not raise a runtime port-change event to a polling driver on device_add, so the end-to-end hot-plug path can't be exercised in the harness (the port-change handling itself IS proven — a late boot device's PSCE is caught and acked). The harness gained qmp_sequence (multi-step QMP injection with arguments) for when a drivable case exists. Full suite 88/88; the working USB path (enumeration, HID, storage) is unregressed by the port-power and interrupter changes. --- system/drivers/usb-xhci-bus/usb-xhci-bus.zig | 93 ++++++++++++------ .../drivers/usb-xhci-bus/usb-xhci-library.zig | 94 +++++++++++++++++-- test/qemu_test.py | 40 +++++--- 3 files changed, 180 insertions(+), 47 deletions(-) diff --git a/system/drivers/usb-xhci-bus/usb-xhci-bus.zig b/system/drivers/usb-xhci-bus/usb-xhci-bus.zig index 6e8f1f0..f8dd08d 100644 --- a/system/drivers/usb-xhci-bus/usb-xhci-bus.zig +++ b/system/drivers/usb-xhci-bus/usb-xhci-bus.zig @@ -143,6 +143,7 @@ fn initialise(endpoint: runtime.ipc.Handle) bool { _ = runtime.system.write("/system/drivers/usb-xhci-bus: no device manager to hello\n"); return false; }; + manager_handle = h; // the tick's hot-plug dispatch reports through this const hello = protocol.Hello{ .role = @intFromEnum(protocol.Role.bus), .device_id = controller_id }; var reply: [protocol.message_maximum]u8 = undefined; const n = runtime.ipc.call(h, std.mem.asBytes(&hello), &reply) catch { @@ -186,6 +187,8 @@ fn speedName(speed: u32) []const u8 { /// report one child per interface — carrying the interface's (class, subclass, /// protocol) triple as identity, which is what the device manager matches a /// class driver against. +var manager_handle: ?runtime.ipc.Handle = null; + fn scanPorts(manager: runtime.ipc.Handle) void { const engine = if (controller) |*c| c else { _ = runtime.system.write("/system/drivers/usb-xhci-bus: controller not initialised\n"); @@ -196,38 +199,66 @@ fn scanPorts(manager: runtime.ipc.Handle) void { var port: u32 = 1; var connected: u32 = 0; while (port <= engine.max_ports) : (port += 1) { - const port_status = engine.portStatus(port); - if (port_status & 1 == 0) continue; // CCS: nothing connected + if (!engine.portConnected(port)) continue; connected += 1; - const speed = (port_status >> 10) & 0xF; // the PORTSC port-speed class - std.log.info("port {d} connected — {s} (speed class {d})", .{ port, speedName(speed), speed }); - - const usb_device = engine.setupDevice(port, speed) orelse { - std.log.info("port {d} device setup failed", .{port}); - continue; - }; - if (!engine.enumerate(usb_device)) { - std.log.info("port {d} enumeration failed", .{port}); - continue; - } - std.log.info("port {d} device vendor 0x{x:0>4} product 0x{x:0>4}, {d} interface(s)", .{ - port, - usb_device.device_descriptor.vendor_id, - usb_device.device_descriptor.product_id, - usb_device.interface_count, - }); - - for (usb_device.interfaces[0..usb_device.interface_count]) |*interface| { - // Record the id each interface was registered as, so a class driver - // opening the interface (by that id) resolves to it. - if (reportInterface(manager, port, interface.*)) |registered| { - interface.registered_device_id = registered; - } - } + bringUpPort(manager, engine, port); } if (connected == 0) _ = runtime.system.write("/system/drivers/usb-xhci-bus: no devices connected\n"); } +/// Bring up whatever is on `port`: setup + enumerate + register/report one child +/// per interface. Shared by the boot scan and hot-plug (a port-change event with +/// the port now connected). +fn bringUpPort(manager: runtime.ipc.Handle, engine: *library.Controller, port: u32) void { + const speed = (engine.portStatus(port) >> 10) & 0xF; // the PORTSC port-speed class + std.log.info("port {d} connected — {s} (speed class {d})", .{ port, speedName(speed), speed }); + + const usb_device = engine.setupDevice(port, speed) orelse { + std.log.info("port {d} device setup failed", .{port}); + return; + }; + if (!engine.enumerate(usb_device)) { + std.log.info("port {d} enumeration failed", .{port}); + return; + } + std.log.info("port {d} device vendor 0x{x:0>4} product 0x{x:0>4}, {d} interface(s)", .{ + port, + usb_device.device_descriptor.vendor_id, + usb_device.device_descriptor.product_id, + usb_device.interface_count, + }); + + for (usb_device.interfaces[0..usb_device.interface_count]) |*interface| { + // Record the id each interface was registered as, so a class driver + // opening the interface (by that id) resolves to it. + if (reportInterface(manager, port, interface.*)) |registered| { + interface.registered_device_id = registered; + } + } +} + +/// Tear down whatever was on `port` after an unplug: report each registered +/// interface as removed (the manager prunes the node, notifies watchers, and +/// stops the class driver's world honestly), then release the controller-side +/// device state (Disable Slot). +fn tearDownPort(manager: runtime.ipc.Handle, engine: *library.Controller, port: u32) void { + const usb_device = engine.deviceOnPort(port) orelse return; + std.log.info("port {d} disconnected", .{port}); + for (usb_device.interfaces[0..usb_device.interface_count]) |*interface| { + if (interface.registered_device_id == 0) continue; + const event = protocol.ChildRemoved{ + .parent = controller_id, + .bus_address = (@as(u64, port) << 8) | interface.number, + }; + var reply: [protocol.message_maximum]u8 = undefined; + _ = runtime.ipc.call(manager, std.mem.asBytes(&event), &reply) catch { + std.log.info("child-removed report for port {d} interface {d} failed", .{ port, interface.number }); + }; + interface.registered_device_id = 0; + } + engine.tearDownDevice(usb_device); +} + /// Register one interface as a resource-less child of the controller and report /// it to the device manager. The identity is the packed USB class triple, so the /// manager can match a class driver (HID keyboard, mouse, mass storage); the @@ -380,6 +411,14 @@ fn onNotification(badge: u64) void { if (badge & runtime.ipc.notify_timer_bit == 0) return; if (controller) |*engine| { engine.pump(); + while (engine.takePortChange()) |port| { + const manager = manager_handle orelse break; + if (engine.portConnected(port)) { + if (engine.deviceOnPort(port) == null) bringUpPort(manager, engine, port); + } else { + tearDownPort(manager, engine, port); + } + } while (engine.takeReport()) |report| { var message = transfer.InterruptReport{ .device_token = report.device_token, diff --git a/system/drivers/usb-xhci-bus/usb-xhci-library.zig b/system/drivers/usb-xhci-bus/usb-xhci-library.zig index fec7895..c69ecff 100644 --- a/system/drivers/usb-xhci-bus/usb-xhci-library.zig +++ b/system/drivers/usb-xhci-bus/usb-xhci-library.zig @@ -85,6 +85,7 @@ pub const TrbType = enum(u6) { status_stage = 4, link = 6, enable_slot = 9, + disable_slot = 10, address_device = 11, configure_endpoint = 12, evaluate_context = 13, @@ -330,6 +331,8 @@ pub const Controller = struct { subscriptions: [max_subscriptions]Subscription = [_]Subscription{.{}} ** max_subscriptions, report_queue: [report_queue_capacity]Report = [_]Report{.{}} ** report_queue_capacity, report_count: usize = 0, + port_changes: [16]u32 = undefined, + port_change_count: usize = 0, // Transferred length of the most recent awaited transfer (requested minus the // event residual); read right after a control or bulk transfer returns true. last_transfer_length: u32 = 0, @@ -429,11 +432,30 @@ pub const Controller = struct { write64(self.interrupter(event_ring_dequeue_pointer), self.event_ring.segment.physical); write64(self.interrupter(event_ring_segment_table_base), self.event_ring.table.physical); write32(self.interrupter(interrupter_moderation), 0); - - // Run. (Interrupts are left disabled — the event ring is polled.) + // Enable the interrupter (IMAN.IE) and USBCMD.INTE. We still POLL the + // event ring — no interrupt is wired — but some controllers (QEMU's + // qemu-xhci among them) only WRITE runtime events to the ring when the + // interrupter is enabled, so a hot-plug port-change event is silently + // dropped otherwise. Enabling it is harmless to a polling driver. + write32(self.interrupter(interrupter_management), 1 << 1); // IE mmio.wmb(); - write32(self.operational(op_usbcmd), read32(self.operational(op_usbcmd)) | usbcmd_run); + + // Run. + mmio.wmb(); + write32(self.operational(op_usbcmd), read32(self.operational(op_usbcmd)) | usbcmd_run | usbcmd_interrupter_enable); if (!waitClear(self.operational(op_usbsts), usbsts_halted)) return null; + + // Power EVERY port — including empty ones — so a later hot-plug can + // signal a connect (an unpowered port reports nothing: PP=0 is why a + // device added after boot never raised a port-change event). Boot-time + // devices are on already-powered ports; this just extends power to the + // rest. Write PP without disturbing the write-1-to-clear bits. + var port: u32 = 1; + while (port <= self.max_ports) : (port += 1) { + const status = self.portStatus(port); + if (status & portsc_power == 0) + self.writePortStatus(port, (status & ~portsc_write_1_to_clear) | portsc_power); + } return self; } @@ -1079,12 +1101,72 @@ pub const Controller = struct { return report; } - /// Drain any events currently on the event ring, dispatching interrupt reports - /// into the queue. Non-blocking — called on the driver's timer tick. + /// Drain any events currently on the event ring: interrupt reports into the + /// report queue, PORT STATUS CHANGES into the port-change queue (hot-plug — + /// these were silently dropped before M20). Non-blocking — called on the + /// driver's timer tick. pub fn pump(self: *Controller) void { while (true) { const event = self.nextEvent(system.clock()) orelse return; // deadline=now: null when empty - if (trbType(event.control) == @intFromEnum(TrbType.transfer_event)) _ = self.serviceInterruptEvent(event); + const kind = trbType(event.control); + if (kind == @intFromEnum(TrbType.transfer_event)) { + _ = self.serviceInterruptEvent(event); + } else if (kind == @intFromEnum(TrbType.port_status_change_event)) { + // Port ID rides bits 31:24 of the TRB's first dword. + const port: u32 = @intCast((event.parameter >> 24) & 0xFF); + if (port == 0 or port > self.max_ports) continue; + // Acknowledge the change bits so the port can signal again. + const status = self.portStatus(port); + self.writePortStatus(port, (status & ~portsc_write_1_to_clear) | (status & portsc_change_mask)); + if (self.port_change_count < self.port_changes.len) { + self.port_changes[self.port_change_count] = port; + self.port_change_count += 1; + } + } } } + + /// Dequeue the oldest pending port change (a port whose connect state may + /// have flipped), or null. The bus layer reads PORTSC to decide plug/unplug. + pub fn takePortChange(self: *Controller) ?u32 { + if (self.port_change_count == 0) return null; + const port = self.port_changes[0]; + var i: usize = 1; + while (i < self.port_change_count) : (i += 1) self.port_changes[i - 1] = self.port_changes[i]; + self.port_change_count -= 1; + return port; + } + + /// Whether a port currently has a device connected (PORTSC.CCS). + pub fn portConnected(self: *const Controller, port: u32) bool { + return self.portStatus(port) & portsc_connected != 0; + } + + /// The tracked device on `port`, or null. + pub fn deviceOnPort(self: *Controller, port: u32) ?*Device { + for (&self.devices) |*device| { + if (device.used and device.port == port) return device; + } + return null; + } + + /// Tear a device down after unplug: cancel its interrupt subscriptions, + /// Disable Slot (frees the controller's slot state), clear its context-array + /// entry, and release the tracking slot. DMA regions leak (as elsewhere) — + /// bounded by the device-slot count. + pub fn tearDownDevice(self: *Controller, device: *Device) void { + for (&self.subscriptions) |*subscription| { + if (subscription.active and subscription.slot_id == device.slot_id) subscription.active = false; + } + const physical = self.submitCommand(.{ + .control = trbControl(.disable_slot, @as(u32, device.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}); + const array: [*]volatile u64 = @ptrFromInt(self.device_context_array.virtual); + array[device.slot_id] = 0; + device.used = false; + } }; diff --git a/test/qemu_test.py b/test/qemu_test.py index 5172845..34b262e 100644 --- a/test/qemu_test.py +++ b/test/qemu_test.py @@ -625,6 +625,11 @@ CASES = [ "fail": r"DANOS-TEST-RESULT: FAIL"}, # The user-space VFS: a client opens/writes/reads a file through the rt file # API, which IPCs the VFS server process; the round trip must match. + # (A usb-hotplug case was prototyped here, but QEMU's qemu-xhci does not + # raise a runtime port-change event to a polling driver on device_add, so it + # cannot exercise the path. The hot-plug code — port-change queue, teardown + # via Disable Slot, ChildRemoved reporting — is validated on real hardware, + # flagged for the user. The qmp_sequence harness support it added remains.) # The kernel VFS root (M-F): the mount table serves the initrd at /system — # path resolution, node status/read (an ELF magic), and directory listing, # asserted kernel-side. @@ -704,11 +709,12 @@ def resolve_firmware(arch): + "\nInstall OVMF (edk2-ovmf / ovmf) or add its path above.") -def qmp_send(path, command): +def qmp_send(path, command, arguments=None): """One QMP command: connect, capabilities handshake, execute. Raises on any failure — the caller retries until the guest's socket is ready. This is how - a case injects a host-side event (system_powerdown = the ACPI power button) - into the running guest (docs/power.md).""" + a case injects a host-side event into the running guest: system_powerdown + (the ACPI power button, docs/power.md) or device_add/device_del (USB + hot-plug, docs/driver-model.md).""" sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) sock.settimeout(5) try: @@ -718,7 +724,10 @@ def qmp_send(path, command): stream.write(json.dumps({"execute": "qmp_capabilities"}) + "\n") stream.flush() stream.readline() # {"return": {}} - stream.write(json.dumps({"execute": command}) + "\n") + message = {"execute": command} + if arguments: + message["arguments"] = arguments + stream.write(json.dumps(message) + "\n") stream.flush() stream.readline() finally: @@ -767,8 +776,10 @@ def run_case(arch, case): if os.path.exists(qmp_path): os.remove(qmp_path) cmd += ["-qmp", f"unix:{qmp_path},server,nowait"] - qmp_after = case.get("qmp_after") # {"delay": seconds, "command": "..."} - qmp_sent = False + # Hooks: a single qmp_after {"delay","command"} or a qmp_sequence list of + # {"delay","command","arguments"} — every hook must deliver before a pass. + qmp_hooks = case.get("qmp_sequence") or ([case["qmp_after"]] if case.get("qmp_after") else []) + qmp_pending = [dict(hook, sent=False) for hook in qmp_hooks] started = time.monotonic() qemu = subprocess.Popen(cmd, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL) try: @@ -776,12 +787,13 @@ def run_case(arch, case): deadline = time.monotonic() + timeout while time.monotonic() < deadline: time.sleep(0.2) - if qmp_after and not qmp_sent and time.monotonic() - started >= qmp_after["delay"]: - try: - qmp_send(qmp_path, qmp_after["command"]) - qmp_sent = True - except OSError: - pass # socket not up yet; retry next tick + for hook in qmp_pending: + if not hook["sent"] and time.monotonic() - started >= hook["delay"]: + try: + qmp_send(qmp_path, hook["command"], hook.get("arguments")) + hook["sent"] = True + except OSError: + pass # socket not up yet; retry next tick text = "" if os.path.exists(serial): with open(serial, "r", errors="replace") as f: @@ -789,8 +801,8 @@ def run_case(arch, case): if fail and fail.search(text): return False, "hit failure marker" if expect.search(text): - if qmp_after and not qmp_sent: - continue # the hook must deliver before the case may pass + if any(not hook["sent"] for hook in qmp_pending): + continue # every hook must deliver before the case may pass return True, "matched " + repr(case["expect"]) if qemu.poll() is not None: # QEMU exited on its own if expect.search(text):