From d26262bf5665ea3d8c68f3cf0b661c568ebfe4c5 Mon Sep 17 00:00:00 2001 From: Daniel Samson <12231216+daniel-samson@users.noreply.github.com> Date: Mon, 13 Jul 2026 02:22:19 +0100 Subject: [PATCH] pci-bus registers and reports what it scans (M19.2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Each function is registered under the bridge with the config-space slice and BARs sized by the same all-ones probe the kernel uses — byte-for-byte equal descriptors, so the idempotent register returns the kernel's existing node ids during coexistence instead of duplicating the tree. The bridge gained the 16-bit io_port aperture that functions' I/O BARs need to pass containment. Reports carry the registered device_id, and the pci-scan scenario drills a forced restart: kill the enumerator after its reports, watch the respawn re-scan, and assert the broker's PCI node count never grew. The usb-restart test trigger is pinned to the xHCI reporter (pci-bus racing it to two reports used to steal the kill). Harness hardening: failing cases preserve their serial logs; the heavy scenarios run at 150s. --- docs/m19-m20-plan.md | 13 ++- system/devices/acpi.zig | 3 + system/drivers/pci-bus/pci-bus.zig | 103 ++++++++++++++++++ system/kernel/tests.zig | 40 ++++++- .../device-manager/device-manager.zig | 33 +++++- test/qemu_test.py | 16 ++- 6 files changed, 188 insertions(+), 20 deletions(-) diff --git a/docs/m19-m20-plan.md b/docs/m19-m20-plan.md index 95340c0..56e4d76 100644 --- a/docs/m19-m20-plan.md +++ b/docs/m19-m20-plan.md @@ -89,12 +89,13 @@ branch is green; keep branches; push everything. manager matches pci_host_bridge → pci-bus per device with the full protocol contract; `pci-scan` builds its expected marker from the kernel's own count — equivalence on the first run; suite 55/55). -- [ ] **M19.2** — register + report: each function registered under the bridge - (config-space slice + BARs, `pci_class` in the descriptor), reported with - `child_added { device_id, identity = class triple }`. Manager mirrors; - spawn-from-reports stays **off**. Scenario extends `pci-scan`: - registered ids resolve, no duplicates after a forced driver restart - (idempotence proven end to end). +- [x] **M19.2** — register + report (BAR probe mirrored byte-for-byte from the + kernel's addBars so dedupe returns the kernel's node ids during + coexistence; the bridge gained the io_port aperture I/O BARs need; + reports carry the registered device_id; pci-scan drills a forced restart + and asserts the PCI node count never grows — plus harness hardening: a + failing case now preserves its serial as -failed-serial.log, and + the heavy scenarios run at 150s; suite 55/55). - [ ] **M19.3** — the flip: kernel `enumeratePci` call removed (bridge node stays); manager matches PCI drivers from reports. One commit. The existing xHCI scenarios (`driver-restart`, `usb-report`, `device-list`) diff --git a/system/devices/acpi.zig b/system/devices/acpi.zig index 0e37c47..8aa495d 100644 --- a/system/devices/acpi.zig +++ b/system/devices/acpi.zig @@ -553,6 +553,9 @@ fn parseMcfg(device_tree: *DeviceTree, hal: Hal, header: *const SystemDescriptor _ = bridge.addResource(.memory, alloc.base_address, bus_count << 20); _ = bridge.addResource(.bus_range, alloc.start_bus, bus_count); addBridgeApertures(bridge); + // The bridge decodes the whole 16-bit I/O space toward its bus — the + // window functions' I/O BARs must register-contain within (M19.2). + _ = bridge.addResource(.io_port, 0, 1 << 16); try enumeratePci(device_tree, bridge, hal, alloc.*); } diff --git a/system/drivers/pci-bus/pci-bus.zig b/system/drivers/pci-bus/pci-bus.zig index 4a24277..d54aa3c 100644 --- a/system/drivers/pci-bus/pci-bus.zig +++ b/system/drivers/pci-bus/pci-bus.zig @@ -22,8 +22,10 @@ fn writeLine(comptime fmt: []const u8, arguments: anytype) void { var bridge_id: u64 = protocol.no_device; var ecam_base: usize = 0; +var ecam_physical: u64 = 0; var start_bus: u64 = 0; var bus_count: u64 = 0; +var manager_handle: runtime.ipc.Handle = 0; /// One aligned 32-bit read from a function's configuration space. fn configRead(bus: u64, dev: u64, function: u64, offset: u64) u32 { @@ -32,6 +34,25 @@ fn configRead(bus: u64, dev: u64, function: u64, offset: u64) u32 { return register.*; } +fn configWrite(bus: u64, dev: u64, function: u64, offset: u64, value: u32) void { + const address = ecam_base + (((bus - start_bus) << 20) | (dev << 15) | (function << 12) | offset); + const register: *volatile u32 = @ptrFromInt(address); + register.* = value; +} + +fn configRead16(bus: u64, dev: u64, function: u64, offset: u64) u16 { + const word = configRead(bus, dev, function, offset & ~@as(u64, 3)); + return @truncate(word >> @intCast((offset & 3) * 8)); +} + +fn configWrite16(bus: u64, dev: u64, function: u64, offset: u64, value: u16) void { + const aligned = offset & ~@as(u64, 3); + const shift: u5 = @intCast((offset & 3) * 8); + const word = configRead(bus, dev, function, aligned); + const mask = @as(u32, 0xFFFF) << shift; + configWrite(bus, dev, function, aligned, (word & ~mask) | (@as(u32, value) << shift)); +} + /// Claim the bridge, map the ECAM, hello the manager, then scan. fn initialise(endpoint: runtime.ipc.Handle) bool { _ = endpoint; @@ -64,6 +85,7 @@ fn initialise(endpoint: runtime.ipc.Handle) bool { }; start_bus = bus_range.start; bus_count = bus_range.len; + ecam_physical = descriptor.resources[0].start; ecam_base = device.mmioMap(bridge_id, 0) orelse { _ = runtime.system.write("pci-bus: ECAM mmio_map failed\n"); return false; @@ -90,6 +112,7 @@ fn initialise(endpoint: runtime.ipc.Handle) bool { _ = runtime.system.write("pci-bus: hello refused\n"); return false; } + manager_handle = h; scan(); return true; @@ -115,12 +138,92 @@ fn scan() void { const class_revision = configRead(bus, dev, function, 0x08); found += 1; writeLine("pci-bus: {d}:{d}.{d} class 0x{x:0>6}\n", .{ bus, dev, function, class_revision >> 8 }); + registerAndReport(bus, dev, function, class_revision >> 8); } } } writeLine("pci-bus: {d} functions found\n", .{found}); } +/// Register one function under the bridge and report it to the manager. The +/// descriptor mirrors the kernel's own recording byte for byte — config slice +/// as resource 0, then the sized BARs — so during coexistence the idempotent +/// device_register (M19.0) returns the kernel's existing node id rather than +/// growing a duplicate, and the report carries the id drivers already use. +fn registerAndReport(bus: u64, dev: u64, function: u64, class_triple: u32) void { + var descriptor = std.mem.zeroes(device.DeviceDescriptor); + descriptor.class = @intFromEnum(device.DeviceClass.pci_device); + descriptor.pci_class = class_triple; + descriptor.resources[0] = .{ + .kind = @intFromEnum(device.ResourceKind.memory), + .start = ecam_physical + (((bus - start_bus) << 20) | (dev << 15) | (function << 12)), + .len = 4096, + }; + descriptor.resource_count = 1; + + // The standard BAR-sizing probe, exactly as the kernel does it: decode off, + // write all-ones, read the writable mask back, restore. Header type 0 only. + const header_type = (configRead(bus, dev, function, 0x0C) >> 16) & 0x7F; + if (header_type == 0) { + const command = configRead16(bus, dev, function, 0x04); + configWrite16(bus, dev, function, 0x04, command & ~@as(u16, 0b11)); + var i: u64 = 0; + while (i < 6) : (i += 1) { + if (descriptor.resource_count >= 8) break; + const off = 0x10 + i * 4; + const original = configRead(bus, dev, function, off); + if (original == 0) continue; + const slot: usize = @intCast(descriptor.resource_count); + if (original & 1 != 0) { + configWrite(bus, dev, function, off, 0xFFFF_FFFF); + const readback = configRead(bus, dev, function, off); + configWrite(bus, dev, function, off, original); + const mask = readback & 0xFFFF_FFFC; + const size: u32 = if (mask == 0) 0 else (~mask +% 1) & 0xFFFF; + descriptor.resources[slot] = .{ .kind = @intFromEnum(device.ResourceKind.io_port), .start = original & 0xFFFF_FFFC, .len = size }; + descriptor.resource_count += 1; + } else if ((original >> 1) & 0x3 == 2) { + const original_high = configRead(bus, dev, function, off + 4); + configWrite(bus, dev, function, off, 0xFFFF_FFFF); + configWrite(bus, dev, function, off + 4, 0xFFFF_FFFF); + const lo = configRead(bus, dev, function, off); + const hi = configRead(bus, dev, function, off + 4); + configWrite(bus, dev, function, off, original); + configWrite(bus, dev, function, off + 4, original_high); + const readback = (@as(u64, hi) << 32) | (lo & 0xFFFF_FFF0); + const size: u64 = if (readback == 0) 0 else ~readback +% 1; + descriptor.resources[slot] = .{ .kind = @intFromEnum(device.ResourceKind.memory), .start = (@as(u64, original_high) << 32) | (original & 0xFFFF_FFF0), .len = size }; + descriptor.resource_count += 1; + i += 1; // consumed the high half + } else { + configWrite(bus, dev, function, off, 0xFFFF_FFFF); + const readback = configRead(bus, dev, function, off); + configWrite(bus, dev, function, off, original); + const mask = readback & 0xFFFF_FFF0; + const size: u32 = if (mask == 0) 0 else ~mask +% 1; + descriptor.resources[slot] = .{ .kind = @intFromEnum(device.ResourceKind.memory), .start = original & 0xFFFF_FFF0, .len = size }; + descriptor.resource_count += 1; + } + } + configWrite16(bus, dev, function, 0x04, command); + } + + const registered = device.register(bridge_id, &descriptor) orelse { + writeLine("pci-bus: register refused for {d}:{d}.{d}\n", .{ bus, dev, function }); + return; + }; + const report = protocol.ChildAdded{ + .parent = bridge_id, + .bus_address = (bus << 8) | (dev << 3) | function, + .identity = class_triple, + .device_id = registered, + }; + var reply: [protocol.message_maximum]u8 = undefined; + _ = runtime.ipc.call(manager_handle, std.mem.asBytes(&report), &reply) catch { + writeLine("pci-bus: child report for {d}:{d}.{d} failed\n", .{ bus, dev, function }); + }; +} + fn onMessage(message: []const u8, reply: []u8, sender: u32, capability: ?runtime.ipc.Handle) usize { _ = message; _ = reply; diff --git a/system/kernel/tests.zig b/system/kernel/tests.zig index 8b09a80..2a3b497 100644 --- a/system/kernel/tests.zig +++ b/system/kernel/tests.zig @@ -1836,13 +1836,14 @@ fn pciScanTest(boot_information: *const BootInformation) void { while (i < rd.count) : (i += 1) { const item = rd.entry(i) orelse continue; if (!eql(item.name, "device-manager")) continue; - manager = process.spawnProcessSupervised(item.blob, 4, &.{"device-manager"}, scheduler.currentId(), null) catch 0; + manager = process.spawnProcessSupervised(item.blob, 4, &.{ "device-manager", "test-pci-restart" }, scheduler.currentId(), null) catch 0; break; } - check("device-manager spawned", manager != 0); + check("device-manager spawned (test-pci-restart mode)", manager != 0); + // First scan: the ring-3 count equals the kernel's. scheduler.setPriority(1); - const deadline = architecture.millis() + 15000; + var deadline = architecture.millis() + 15000; var seen = false; while (architecture.millis() < deadline and !seen) { if (process.write_len >= marker.len and eql(process.write_buffer[0..marker.len], marker)) seen = true; @@ -1850,6 +1851,39 @@ fn pciScanTest(boot_information: *const BootInformation) void { } scheduler.setPriority(4); check("the ring-3 scan found exactly the kernel's function count", seen); + + // The restart drill: the manager kills pci-bus after its reports; the + // respawn re-claims, re-scans, and re-registers. + const restart_marker = "device-manager: restarting pci-bus"; + scheduler.setPriority(1); + deadline = architecture.millis() + 15000; + var restarted = false; + while (architecture.millis() < deadline and !restarted) { + if (process.write_len >= restart_marker.len and eql(process.write_buffer[0..restart_marker.len], restart_marker)) restarted = true; + scheduler.yield(); + } + scheduler.setPriority(4); + check("the manager restarted pci-bus", restarted); + + scheduler.setPriority(1); + deadline = architecture.millis() + 15000; + seen = false; + while (architecture.millis() < deadline and !seen) { + if (process.write_len >= marker.len and eql(process.write_buffer[0..marker.len], marker)) seen = true; + scheduler.yield(); + } + scheduler.setPriority(4); + check("the respawned scan reported the same count", seen); + + // No duplicates: the registrations deduped against the kernel's own nodes + // on the first pass, and against themselves on the second. + var after: [64]device_abi.DeviceDescriptor = undefined; + const m = @min(devices_broker.enumerate(&after), after.len); + var after_count: u32 = 0; + for (after[0..m]) |d| { + if (d.class == @intFromEnum(device_abi.DeviceClass.pci_device)) after_count += 1; + } + check("no duplicate PCI nodes after register + restart + re-register", after_count == kernel_count); result(); } diff --git a/system/services/device-manager/device-manager.zig b/system/services/device-manager/device-manager.zig index a579bbe..178cef0 100644 --- a/system/services/device-manager/device-manager.zig +++ b/system/services/device-manager/device-manager.zig @@ -108,6 +108,7 @@ var manager_endpoint: runtime.ipc.Handle = 0; var test_restart_mode = false; var test_usb_restart_mode = false; var test_usb_killed = false; +var test_pci_restart_mode = false; var test_kill_pid: u32 = 0; var test_kill_due_ns: u64 = 0; @@ -400,13 +401,32 @@ fn onChildAdded(message: []const u8, reply: []u8, sender: u32) usize { } const report_reply = protocol.ReportReply{ .status = status }; @memcpy(reply[0..@sizeOf(protocol.ReportReply)], std.mem.asBytes(&report_reply)); + if (test_pci_restart_mode and !test_usb_killed) { + if (driverByProcess(sender)) |driver| { + if (std.mem.eql(u8, driver.name(), "pci-bus") and childCountOf(sender) >= 3) { + // The pci restart drill: kill the enumerator after it has + // reported; the respawn must re-register without duplicates + // (M19.0 idempotence, proven end to end by pci-scan). + test_usb_killed = true; + test_kill_pid = sender; + test_kill_due_ns = system.clock() + 1_000_000_000; + _ = system.timerOnce(manager_endpoint, 1100); + } + } + } if (test_usb_restart_mode and !test_usb_killed and childCountOf(sender) >= 2) { - // Delayed, not immediate: the device-list scenario's subscriber needs a - // window to enumerate and subscribe before the events start. - test_usb_killed = true; - test_kill_pid = sender; - test_kill_due_ns = system.clock() + 2_000_000_000; - _ = system.timerOnce(manager_endpoint, 2100); + // Only the xHCI reporter is the drill's victim — pci-bus also reports + // now, and whichever finishes second must not trigger the kill. + if (driverByProcess(sender)) |driver| { + if (std.mem.eql(u8, driver.name(), "usb-xhci-bus")) { + // Delayed, not immediate: the device-list scenario's subscriber + // needs a window to enumerate and subscribe before the events. + test_usb_killed = true; + test_kill_pid = sender; + test_kill_due_ns = system.clock() + 2_000_000_000; + _ = system.timerOnce(manager_endpoint, 2100); + } + } } return @sizeOf(protocol.ReportReply); } @@ -476,6 +496,7 @@ pub fn main(init: runtime.process.Init) void { if (init.arguments.get(1)) |mode| { test_restart_mode = std.mem.eql(u8, mode, "test-restart"); test_usb_restart_mode = std.mem.eql(u8, mode, "test-usb-restart"); + test_pci_restart_mode = std.mem.eql(u8, mode, "test-pci-restart"); } runtime.service.run(protocol.message_maximum, .{ .service = .device_manager, diff --git a/test/qemu_test.py b/test/qemu_test.py index 9d3a0a4..17771d7 100644 --- a/test/qemu_test.py +++ b/test/qemu_test.py @@ -168,7 +168,7 @@ CASES = [ # Stress the big kernel lock across cores; heavier, so a longer timeout. {"name": "smp-stress", "smp": 4, - "timeout": 90, + "timeout": 150, "expect": r"DANOS-TEST-RESULT: PASS", "fail": r"DANOS-TEST-RESULT: FAIL"}, # Retry: a forced first-wake failure must still bring every core online. @@ -263,7 +263,7 @@ CASES = [ # death, and the respawned driver re-reports (docs/device-manager.md). {"name": "usb-report", "smp": 4, - "timeout": 90, + "timeout": 150, "qemu_extra": ["-device", "qemu-xhci,id=xhci", "-device", "usb-kbd,bus=xhci.0", "-device", "usb-mouse,bus=xhci.0"], @@ -286,11 +286,11 @@ CASES = [ # the reporter's test-kill produces (docs/device-manager.md). {"name": "device-list", "smp": 4, - "timeout": 90, + "timeout": 150, "qemu_extra": ["-device", "qemu-xhci,id=xhci", "-device", "usb-kbd,bus=xhci.0", "-device", "usb-mouse,bus=xhci.0"], - "expect": r"device-list: 2 devices[\s\S]*" + "expect": r"device-list: \d+ devices[\s\S]*" r"device-list: subscribed[\s\S]*" r"device-manager: test mode: killing the reporter[\s\S]*" r"device-list: removed \(device[\s\S]*" @@ -301,7 +301,7 @@ CASES = [ # each time), and hits the crash-loop cap (docs/device-manager.md). {"name": "driver-restart", "smp": 4, - "timeout": 90, + "timeout": 150, "qemu_extra": ["-device", "qemu-xhci,id=xhci", "-device", "usb-kbd,bus=xhci.0", "-device", "usb-mouse,bus=xhci.0"], @@ -479,6 +479,12 @@ def main(): for case in selected: print(f" {case['name']:<12} ... ", end="", flush=True) ok, detail = run_case(arch, case) + if not ok: + # Keep the evidence: serial.log is otherwise overwritten by the + # next case, and an intermittent failure's log is unrecoverable. + source = os.path.join(WORK, "serial.log") + if os.path.exists(source): + shutil.copy(source, os.path.join(WORK, f"{case['name']}-failed-serial.log")) print(("PASS" if ok else "FAIL") + f" ({detail})") if not ok: failures += 1