From a2a05d0b3d6240da53bbf13ab0e54ca376dcede0 Mon Sep 17 00:00:00 2001 From: Daniel Samson <12231216+daniel-samson@users.noreply.github.com> Date: Mon, 13 Jul 2026 01:59:32 +0100 Subject: [PATCH 1/4] Discovery-migration prerequisites (M19.0) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The host bridge now carries MMIO apertures derived from the boot memory map's gaps below 4 GiB (largest three, sort-merged; a single after-the- last-region hole dies on OVMF's flash at the top) plus one aperture above the described space — so a user-space device_register of PCI functions with BAR resources can pass containment. The discovery test asserts every PCI memory resource lies inside a bridge window and names any escapee. device_register is idempotent on exact (parent, class, identity, resources) match — a restarted registering bus cannot duplicate its children; proven directly against the broker in the bus test. ChildAdded gains device_id so a report can carry the registered kernel id a matched driver needs as its assignment. --- docs/m17-m18-plan.md | 4 ++ docs/m19-m20-plan.md | 8 ++- system/devices/acpi.zig | 66 ++++++++++++++++++- system/devices/platform.zig | 3 +- system/kernel/devices-broker.zig | 20 ++++++ system/kernel/tests.zig | 57 ++++++++++++++++ .../device-manager-protocol.zig | 7 +- .../device-manager/device-manager.zig | 9 ++- 8 files changed, 165 insertions(+), 9 deletions(-) diff --git a/docs/m17-m18-plan.md b/docs/m17-m18-plan.md index 0348bf8..8518c9f 100644 --- a/docs/m17-m18-plan.md +++ b/docs/m17-m18-plan.md @@ -1,5 +1,9 @@ # M17–M18 execution plan: process lifecycle + device manager +**Archived — completed 2026-07-13** (every item checked; suite ended 54/54). +Kept as the record of how M17–M18 landed; the successor is +[m19-m20-plan.md](m19-m20-plan.md). + The operational plan for building [process-lifecycle.md](process-lifecycle.md) (M17) and [device-manager.md](device-manager.md) increments 5–7 (M18). Design is settled in those documents; this file is the build order — one phase at a time, diff --git a/docs/m19-m20-plan.md b/docs/m19-m20-plan.md index d1b2b66..47fa5a5 100644 --- a/docs/m19-m20-plan.md +++ b/docs/m19-m20-plan.md @@ -79,9 +79,11 @@ branch is green; keep branches; push everything. ## Status -- [ ] **M19.0** — prerequisites on `feat/pci-bus`: bridge MMIO apertures from - the memory-map holes; `device_register` idempotence (+ kernel unit - checks); `ChildAdded.device_id`; archive note on m17-m18-plan.md. +- [x] **M19.0** — prerequisites (bridge apertures from the memory map's + *gaps* — the single-hole rule died on OVMF's flash at the top of 4 GiB, + caught by the new every-BAR-contained assert in `discovery`; idempotent + `device_register` proven in `bus`; `ChildAdded.device_id`; + m17-m18-plan.md archived; suite 54/54). - [ ] **M19.1** — pci-bus driver, scan only: claim the host bridge, map the ECAM window, walk bus/device/function headers, log what it finds. Scenario `pci-scan`: the kernel test compares the driver's reported count diff --git a/system/devices/acpi.zig b/system/devices/acpi.zig index 2f68c15..0e37c47 100644 --- a/system/devices/acpi.zig +++ b/system/devices/acpi.zig @@ -384,8 +384,9 @@ const PciHeader = extern struct { /// Discover hardware from the ACPI tables rooted at `rsdp_physical` and populate /// `device_tree`. `hal` provides MMIO mapping (for PCIe ECAM) and port I/O. Also parses the /// FADT and the AML sleep-state (`_Sx`) packages into `power_information` for the power service. -pub fn discover(rsdp_physical: u64, device_tree: *DeviceTree, hal: Hal) !void { +pub fn discover(rsdp_physical: u64, memory_regions: []const boot_handoff.MemoryRegion, device_tree: *DeviceTree, hal: Hal) !void { if (rsdp_physical == 0) return error.NoRsdp; + boot_memory_regions = memory_regions; // Start clean so a re-run doesn't accumulate stale state. power_information = .{}; @@ -551,11 +552,74 @@ fn parseMcfg(device_tree: *DeviceTree, hal: Hal, header: *const SystemDescriptor // ECAM window: 1 MiB of configuration space per bus. _ = bridge.addResource(.memory, alloc.base_address, bus_count << 20); _ = bridge.addResource(.bus_range, alloc.start_bus, bus_count); + addBridgeApertures(bridge); try enumeratePci(device_tree, bridge, hal, alloc.*); } } +/// The boot memory map, stored at discover() entry for the aperture derivation +/// below (and, in M20, for the acpi-tables node's containment windows). +var boot_memory_regions: []const boot_handoff.MemoryRegion = &.{}; + +/// The bridge's MMIO apertures, derived from the boot memory map's holes +/// (docs/m19-m20-plan.md decision 2): registered PCI functions carry BAR +/// resources, and `device_register` containment demands the bridge own windows +/// that cover them. Everything the firmware described is "not hole"; the low +/// aperture runs from the end of the described space below 4 GiB up to the +/// I/O-APIC region, the high one from 4 GiB (or the end of RAM above it) to +/// the 46-bit line. Coarse, mechanical, and AML-free — available at boot no +/// matter what later moved to user space. +fn addBridgeApertures(bridge: *device_model.Device) void { + // Below 4 GiB the described regions are sparse (RAM low, firmware flash + // and tables high), so the holes are the *gaps between* them — a single + // "after the last region" rule dies on OVMF's flash at the very top. + // Sort-merge the described ranges, then keep the three largest gaps + // (resource slots are bounded at 8 per device; ECAM + bus range + 3 + the + // high aperture fits). Above 4 GiB one aperture runs from the end of the + // described space to the 46-bit line. + const Range = struct { base: u64, end: u64 }; + var below: [64]Range = undefined; + var below_count: usize = 0; + var high_end: u64 = 1 << 32; + for (boot_memory_regions) |region| { + const end = region.base + region.pages * 4096; + if (end > high_end) high_end = end; + if (region.base >= (1 << 32) or below_count == below.len) continue; + below[below_count] = .{ .base = region.base, .end = @min(end, 1 << 32) }; + below_count += 1; + } + // Insertion sort by base (the map is small and this runs once at boot). + for (1..below_count) |i| { + const key = below[i]; + var j = i; + while (j > 0 and below[j - 1].base > key.base) : (j -= 1) below[j] = below[j - 1]; + below[j] = key; + } + // Walk the sorted ranges, collecting inter-region gaps of at least 1 MiB. + var gaps: [3]Range = .{Range{ .base = 0, .end = 0 }} ** 3; + var cursor: u64 = 0; + var index: usize = 0; + while (index <= below_count) : (index += 1) { + const gap_end = if (index == below_count) (1 << 32) else below[index].base; + if (gap_end > cursor and gap_end - cursor >= (1 << 20)) { + // Keep the three largest, replacing the smallest kept so far. + var smallest: usize = 0; + for (gaps, 0..) |gap, gi| { + if (gap.end - gap.base < gaps[smallest].end - gaps[smallest].base) smallest = gi; + } + if (gap_end - cursor > gaps[smallest].end - gaps[smallest].base) { + gaps[smallest] = .{ .base = cursor, .end = gap_end }; + } + } + if (index < below_count and below[index].end > cursor) cursor = below[index].end; + } + for (gaps) |gap| { + if (gap.end > gap.base) _ = bridge.addResource(.memory, gap.base, gap.end - gap.base); + } + _ = bridge.addResource(.memory, high_end, (@as(u64, 1) << 46) - high_end); +} + /// Brute-force scan the ECAM window's bus range for present PCI functions. No /// bridge recursion yet: on the ECAM path the host bridge decodes every bus in /// the window, so scanning the declared range finds everything QEMU exposes. diff --git a/system/devices/platform.zig b/system/devices/platform.zig index ee78cff..98ce62f 100644 --- a/system/devices/platform.zig +++ b/system/devices/platform.zig @@ -72,7 +72,8 @@ pub fn discover( var device_tree = try DeviceTree.init(allocator); if (boot_information.acpi_rsdp != 0) { - try acpi.discover(boot_information.acpi_rsdp, &device_tree, hal); + const memory_regions = @as([*]const boot_handoff.MemoryRegion, @ptrFromInt(boot_handoff.physicalToVirtual(boot_information.memory_map.regions)))[0..boot_information.memory_map.len]; + try acpi.discover(boot_information.acpi_rsdp, memory_regions, &device_tree, hal); } else { // No ACPI RSDP. A device-tree boot would parse its blob here; today that // path is a stub, so this reports the machine described itself no way we diff --git a/system/kernel/devices-broker.zig b/system/kernel/devices-broker.zig index 2147188..8460c4e 100644 --- a/system/kernel/devices-broker.zig +++ b/system/kernel/devices-broker.zig @@ -180,6 +180,26 @@ pub fn register(parent_id: u64, owner: u32, descriptor: *const device_abi.Device if (!ok) return error.NotContained; } + // Idempotent on exact match (docs/m19-m20-plan.md decision 3): a restarted + // registering bus re-registers what it rediscovers, and the table has no + // unregister — an identical (class, identity, resources) child under the + // same parent returns the existing id instead of appending a duplicate. + for (devices[0..count]) |*existing| { + if (existing.parent != parent_id) continue; + if (existing.class != descriptor.class) continue; + if (existing.pci_class != descriptor.pci_class) continue; + if (existing.hid_len != descriptor.hid_len) continue; + if (!std.mem.eql(u8, existing.hid[0..@intCast(existing.hid_len)], descriptor.hid[0..@intCast(descriptor.hid_len)])) continue; + if (existing.resource_count != descriptor.resource_count) continue; + var same = true; + for (0..@intCast(descriptor.resource_count)) |i| { + const a = existing.resources[i]; + const b = descriptor.resources[i]; + if (a.kind != b.kind or a.start != b.start or a.len != b.len) same = false; + } + if (same) return existing.id; + } + var d = std.mem.zeroes(device_abi.DeviceDescriptor); d.id = count; d.parent = parent_id; diff --git a/system/kernel/tests.zig b/system/kernel/tests.zig index bf6237b..2477f26 100644 --- a/system/kernel/tests.zig +++ b/system/kernel/tests.zig @@ -283,6 +283,34 @@ fn discoveryTest() void { check("PCI functions were enumerated (MCFG/ECAM)", pci_functions >= 1); check("each PCI function exposes its ECAM config space as resource 0", pci_config_ok); + // M19.0: every PCI memory resource (config slice and BARs alike) must be + // contained in one of its parent bridge's windows — the aperture derivation + // from the memory map is what makes a future user-space device_register of + // these functions pass containment. This is the assert that catches a + // too-coarse hole computation before M19.2 would. + var bars_contained = true; + for (buffer[0..n]) |d| { + if (d.class != @intFromEnum(device_abi.DeviceClass.pci_device)) continue; + if (d.parent >= n) { + bars_contained = false; + continue; + } + const bridge = buffer[@intCast(d.parent)]; + for (d.resources[0..@intCast(d.resource_count)]) |r| { + if (r.kind != @intFromEnum(device_abi.ResourceKind.memory)) continue; + var inside = false; + for (bridge.resources[0..@intCast(bridge.resource_count)]) |w| { + if (w.kind != @intFromEnum(device_abi.ResourceKind.memory)) continue; + if (r.start >= w.start and r.start + r.len <= w.start + w.len) inside = true; + } + if (!inside) { + bars_contained = false; + log(" escaping BAR: 0x{x}+0x{x} on device {d}\n", .{ r.start, r.len, d.id }); + } + } + } + check("every PCI BAR lies inside a bridge aperture (M19.0)", bars_contained); + result(); } @@ -2085,6 +2113,35 @@ fn hpetGsi() ?u32 { /// land in the device table with the containment invariant intact. fn busTest(boot_information: *const BootInformation) void { log("DANOS-TEST-BEGIN: bus\n", .{}); + + // M19.0: device_register is idempotent on exact match — a restarted + // registering bus must not duplicate its children. Driven directly against + // the broker: claim an unclaimed node, register the same (class, hid, + // resourceless) child twice, expect one id and one table entry. + { + const me = scheduler.currentId(); + var probe: [1]device_abi.DeviceDescriptor = undefined; + const total = devices_broker.enumerate(&probe); + check("device tree is seeded for the idempotence check", total >= 1); + if (devices_broker.ownerOf(0) == null) { + check("claimed device 0 for the idempotence check", devices_broker.claim(0, me)); + var child = std.mem.zeroes(device_abi.DeviceDescriptor); + child.class = @intFromEnum(device_abi.DeviceClass.unknown); + child.pci_class = device_abi.no_pci_class; + child.hid_len = 4; + child.hid[0..4].* = "idem".*; + const first = devices_broker.register(0, me, &child) catch 0; + check("first register succeeded", first != 0); + const before = devices_broker.enumerate(&probe); + const second = devices_broker.register(0, me, &child) catch 0; + check("re-register returned the same id", second == first); + check("re-register grew nothing", devices_broker.enumerate(&probe) == before); + devices_broker.releaseAllOwnedBy(me); + } else { + check("device 0 unexpectedly claimed before the idempotence check", false); + } + } + if (boot_information.initial_ramdisk_len == 0) { check("bootloader handed over an initial_ramdisk", false); result(); diff --git a/system/services/device-manager/device-manager-protocol.zig b/system/services/device-manager/device-manager-protocol.zig index 2a49db4..bf494a3 100644 --- a/system/services/device-manager/device-manager-protocol.zig +++ b/system/services/device-manager/device-manager-protocol.zig @@ -73,8 +73,13 @@ pub const ChildAdded = extern struct { parent: u64, /// Where on the bus (for USB: the root port number, 1-based). bus_address: u64, - /// Bus-specific identity (for USB: the PORTSC port-speed class). + /// Bus-specific identity (for USB: the PORTSC port-speed class; for PCI: + /// the class triple). identity: u64, + /// The kernel device id this child was `device_register`ed as — what the + /// manager hands a matched driver as its argv assignment — or `no_device` + /// for an unregistered leaf (a USB port before the descriptor track). + device_id: u64 = no_device, }; pub const child_added_size = @sizeOf(ChildAdded); diff --git a/system/services/device-manager/device-manager.zig b/system/services/device-manager/device-manager.zig index b59f2a1..262f026 100644 --- a/system/services/device-manager/device-manager.zig +++ b/system/services/device-manager/device-manager.zig @@ -136,6 +136,8 @@ const Child = struct { parent: u64 = 0, bus_address: u64 = 0, identity: u64 = 0, + // The kernel device id (registered by the reporter), or protocol.no_device. + device_id: u64 = 0, reporter: u32 = 0, // the reporting driver instance's process id }; @@ -145,18 +147,19 @@ var children: [maximum_children]Child = .{Child{}} ** maximum_children; /// Record (or refresh) a reported child. Refreshing matters: a restarted bus /// driver re-reports what it rediscovers, and the same (parent, port) must not /// duplicate. -fn addChild(parent: u64, bus_address: u64, identity: u64, reporter: u32) bool { +fn addChild(parent: u64, bus_address: u64, identity: u64, device_id: u64, reporter: u32) bool { var free: ?*Child = null; for (&children) |*child| { if (child.used and child.parent == parent and child.bus_address == bus_address) { child.identity = identity; + child.device_id = device_id; child.reporter = reporter; return true; } if (!child.used and free == null) free = child; } const slot = free orelse return false; - slot.* = .{ .used = true, .parent = parent, .bus_address = bus_address, .identity = identity, .reporter = reporter }; + slot.* = .{ .used = true, .parent = parent, .bus_address = bus_address, .identity = identity, .device_id = device_id, .reporter = reporter }; return true; } @@ -382,7 +385,7 @@ fn onChildAdded(message: []const u8, reply: []u8, sender: u32) usize { const report = std.mem.bytesToValue(protocol.ChildAdded, message[0..protocol.child_added_size]); var status: i32 = 0; if (driverByProcess(sender)) |driver| { - if (!addChild(report.parent, report.bus_address, report.identity, sender)) status = -1; + if (!addChild(report.parent, report.bus_address, report.identity, report.device_id, sender)) status = -1; writeLine("device-manager: child added (device {d} port {d}, identity {d}) by {s}\n", .{ report.parent, report.bus_address, report.identity, driver.name() }); if (status == 0) publishEvent(message[0..protocol.child_added_size]); } else { From 10b89c06ffbac7ecb03ed9a529a1e674afc98d08 Mon Sep 17 00:00:00 2001 From: Daniel Samson <12231216+daniel-samson@users.noreply.github.com> Date: Mon, 13 Jul 2026 02:05:32 +0100 Subject: [PATCH 2/4] The PCI scan from ring 3: pci-bus walks the ECAM it mapped (M19.1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The manager matches the pci_host_bridge node and spawns pci-bus with the bridge id as its assignment — hello, supervision, restart, all the M18 contract for free. The driver claims the bridge, maps the ECAM window (resource 0) through the ordinary mmio grant, and repeats the kernel's brute-force bus/device/function walk from user space. The pci-scan scenario builds its expected marker from the kernel's own function count, so the two enumerations must agree exactly — the equivalence that licenses retiring the kernel walk in M19.3. --- build.zig | 3 + docs/m19-m20-plan.md | 9 +- system/drivers/pci-bus/pci-bus.zig | 147 ++++++++++++++++++ system/kernel/tests.zig | 60 +++++++ .../device-manager/device-manager.zig | 7 + test/qemu_test.py | 7 + 6 files changed, 229 insertions(+), 4 deletions(-) create mode 100644 system/drivers/pci-bus/pci-bus.zig diff --git a/build.zig b/build.zig index b163acc..3dbc40b 100644 --- a/build.zig +++ b/build.zig @@ -339,6 +339,7 @@ pub fn build(b: *std.Build) void { const ps2_keyboard_exe = addUserBinary(b, kernel_target, runtime_module, posix_module, mmio_module, xkeyboard_config_module, acpi_ids_module, "ps2-keyboard", "system/drivers/ps2-bus/keyboard.zig"); const ps2_mouse_exe = addUserBinary(b, kernel_target, runtime_module, posix_module, mmio_module, xkeyboard_config_module, acpi_ids_module, "ps2-mouse", "system/drivers/ps2-bus/mouse.zig"); const usb_xhci_bus_exe = addUserBinary(b, kernel_target, runtime_module, posix_module, mmio_module, xkeyboard_config_module, acpi_ids_module, "usb-xhci-bus", "system/drivers/usb-xhci-bus/usb-xhci-bus.zig"); + const pci_bus_exe = addUserBinary(b, kernel_target, runtime_module, posix_module, mmio_module, xkeyboard_config_module, acpi_ids_module, "pci-bus", "system/drivers/pci-bus/pci-bus.zig"); // A test fixture, not a real driver: hellos to the device manager, then faults — // what the driver-restart scenario drives the crash-loop cap with. const crash_test_exe = addUserBinary(b, kernel_target, runtime_module, posix_module, mmio_module, xkeyboard_config_module, acpi_ids_module, "crash-test", "system/services/crash-test/crash-test.zig"); @@ -388,6 +389,8 @@ pub fn build(b: *std.Build) void { mk_run.addFileArg(ps2_mouse_exe.getEmittedBin()); mk_run.addArg("usb-xhci-bus"); mk_run.addFileArg(usb_xhci_bus_exe.getEmittedBin()); + mk_run.addArg("pci-bus"); + mk_run.addFileArg(pci_bus_exe.getEmittedBin()); mk_run.addArg("crash-test"); mk_run.addFileArg(crash_test_exe.getEmittedBin()); mk_run.addArg("device-list"); diff --git a/docs/m19-m20-plan.md b/docs/m19-m20-plan.md index 47fa5a5..95340c0 100644 --- a/docs/m19-m20-plan.md +++ b/docs/m19-m20-plan.md @@ -84,10 +84,11 @@ branch is green; keep branches; push everything. caught by the new every-BAR-contained assert in `discovery`; idempotent `device_register` proven in `bus`; `ChildAdded.device_id`; m17-m18-plan.md archived; suite 54/54). -- [ ] **M19.1** — pci-bus driver, scan only: claim the host bridge, map the - ECAM window, walk bus/device/function headers, log what it finds. - Scenario `pci-scan`: the kernel test compares the driver's reported count - against the broker table's `pci_device` count — equivalence, per class. +- [x] **M19.1** — pci-bus driver, scan only (claims the bridge, maps ECAM + through its grant, brute-force walk with the multifunction rule; the + 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; diff --git a/system/drivers/pci-bus/pci-bus.zig b/system/drivers/pci-bus/pci-bus.zig new file mode 100644 index 0000000..4a24277 --- /dev/null +++ b/system/drivers/pci-bus/pci-bus.zig @@ -0,0 +1,147 @@ +//! /system/drivers/pci-bus — the PCI bus driver: enumeration moved out of ring 0 +//! (docs/m19-m20-plan.md, M19). The device manager matches the `pci_host_bridge` +//! node and spawns one instance per bridge, the bridge's device id as argv[1] — +//! the same per-device contract as usb-xhci-bus. +//! +//! M19.1 (this increment): claim the bridge, map its ECAM window (resource 0; +//! the bus range and the MMIO apertures follow it), walk every +//! bus/device/function config header, and log what the walk finds — ending +//! with "pci-bus: N functions found", which the `pci-scan` scenario compares +//! against the kernel's own enumeration. Registration and reports (M19.2), and +//! the kernel walk's retirement (M19.3), build on this proven-equivalent scan. + +const std = @import("std"); +const runtime = @import("runtime"); +const protocol = runtime.device_manager_protocol; +const device = runtime.device; + +fn writeLine(comptime fmt: []const u8, arguments: anytype) void { + var line: [128]u8 = undefined; + _ = runtime.system.write(std.fmt.bufPrint(&line, fmt, arguments) catch return); +} + +var bridge_id: u64 = protocol.no_device; +var ecam_base: usize = 0; +var start_bus: u64 = 0; +var bus_count: u64 = 0; + +/// One aligned 32-bit read from a function's configuration space. +fn configRead(bus: u64, dev: u64, function: u64, offset: u64) u32 { + const address = ecam_base + (((bus - start_bus) << 20) | (dev << 15) | (function << 12) | offset); + const register: *volatile u32 = @ptrFromInt(address); + return register.*; +} + +/// Claim the bridge, map the ECAM, hello the manager, then scan. +fn initialise(endpoint: runtime.ipc.Handle) bool { + _ = endpoint; + if (!device.claim(bridge_id)) { + writeLine("pci-bus: unable to claim bridge device {d}\n", .{bridge_id}); + return false; + } + const buffer = runtime.allocator().alloc(device.DeviceDescriptor, 64) catch { + _ = runtime.system.write("pci-bus: out of memory\n"); + return false; + }; + const total = device.enumerate(buffer); + const descriptor = for (buffer[0..@min(total, buffer.len)]) |d| { + if (d.id == bridge_id) break d; + } else { + writeLine("pci-bus: device {d} not in the device tree\n", .{bridge_id}); + return false; + }; + // Resource 0 is the ECAM window (1 MiB of config space per bus); the bus + // range rides beside it. The MMIO apertures (M19.0) come after both. + if (descriptor.resource_count < 2 or descriptor.resources[0].kind != @intFromEnum(device.ResourceKind.memory)) { + _ = runtime.system.write("pci-bus: bridge has no ECAM window\n"); + return false; + } + const bus_range = for (descriptor.resources[0..@intCast(descriptor.resource_count)]) |resource| { + if (resource.kind == @intFromEnum(device.ResourceKind.bus_range)) break resource; + } else { + _ = runtime.system.write("pci-bus: bridge has no bus range\n"); + return false; + }; + start_bus = bus_range.start; + bus_count = bus_range.len; + ecam_base = device.mmioMap(bridge_id, 0) orelse { + _ = runtime.system.write("pci-bus: ECAM mmio_map failed\n"); + return false; + }; + + // The handshake, then the scan (reports join in M19.2). + var manager: ?runtime.ipc.Handle = null; + var tries: u32 = 0; + while (manager == null and tries < 100) : (tries += 1) { + manager = runtime.ipc.lookup(.device_manager); + if (manager == null) runtime.system.sleep(20); + } + const h = manager orelse { + _ = runtime.system.write("pci-bus: no device manager to hello\n"); + return false; + }; + const hello = protocol.Hello{ .role = @intFromEnum(protocol.Role.bus), .device_id = bridge_id }; + var reply: [protocol.message_maximum]u8 = undefined; + const n = runtime.ipc.call(h, std.mem.asBytes(&hello), &reply) catch { + _ = runtime.system.write("pci-bus: hello call failed\n"); + return false; + }; + if (n < protocol.reply_size or std.mem.bytesToValue(protocol.HelloReply, reply[0..protocol.reply_size]).status != 0) { + _ = runtime.system.write("pci-bus: hello refused\n"); + return false; + } + + scan(); + return true; +} + +/// The brute-force walk the kernel does today, from ring 3: every bus in the +/// range, 32 devices, 8 functions; vendor id FFFFh means nothing decodes there, +/// and only multifunction devices get their functions 1..7 probed. +fn scan() void { + var found: u32 = 0; + var bus: u64 = start_bus; + while (bus < start_bus + bus_count) : (bus += 1) { + var dev: u64 = 0; + while (dev < 32) : (dev += 1) { + const first = configRead(bus, dev, 0, 0); + if (first & 0xFFFF == 0xFFFF) continue; + const multifunction = (configRead(bus, dev, 0, 0x0C) >> 16) & 0x80 != 0; + var function: u64 = 0; + while (function < 8) : (function += 1) { + if (function != 0 and !multifunction) break; + const vendor_device = configRead(bus, dev, function, 0); + if (vendor_device & 0xFFFF == 0xFFFF) continue; + 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 }); + } + } + } + writeLine("pci-bus: {d} functions found\n", .{found}); +} + +fn onMessage(message: []const u8, reply: []u8, sender: u32, capability: ?runtime.ipc.Handle) usize { + _ = message; + _ = reply; + _ = sender; + _ = capability; + return 0; +} + +pub fn main(init: runtime.process.Init) void { + const argument = init.arguments.get(1) orelse return; // bare (ramdisk sweep): stay silent + bridge_id = std.fmt.parseInt(u64, argument, 10) catch { + writeLine("pci-bus: malformed bridge device id '{s}'\n", .{argument}); + return; + }; + runtime.service.run(protocol.message_maximum, .{ + .init = initialise, + .on_message = onMessage, + }); +} + +pub const panic = runtime.panic; +comptime { + _ = &runtime.start._start; // pull the runtime entry shim into the image +} diff --git a/system/kernel/tests.zig b/system/kernel/tests.zig index 2477f26..8b09a80 100644 --- a/system/kernel/tests.zig +++ b/system/kernel/tests.zig @@ -144,6 +144,8 @@ pub fn run(case: []const u8, boot_information: *const BootInformation) void { usbReportTest(boot_information); } else if (eql(case, "device-list")) { deviceListTest(boot_information); + } else if (eql(case, "pci-scan")) { + pciScanTest(boot_information); } else if (eql(case, "initial-ramdisk")) { initialRamdiskTest(boot_information); } else if (eql(case, "vfs")) { @@ -1793,6 +1795,64 @@ fn deviceListTest(boot_information: *const BootInformation) void { result(); } +/// M19.1: the ring-3 PCI scan agrees with the kernel's. The manager spawns +/// pci-bus for the host bridge; the driver walks the same ECAM window through +/// its mmio_map grant and must find exactly the functions the kernel's own +/// enumeration recorded — the equivalence that licenses retiring the kernel +/// walk in M19.3. +fn pciScanTest(boot_information: *const BootInformation) void { + log("DANOS-TEST-BEGIN: pci-scan\n", .{}); + if (boot_information.initial_ramdisk_len == 0) { + check("bootloader handed over an initial_ramdisk", false); + result(); + return; + } + const image = @as([*]const u8, @ptrFromInt(boot_handoff.physicalToVirtual(boot_information.initial_ramdisk_base)))[0..boot_information.initial_ramdisk_len]; + const rd = initial_ramdisk.Reader.init(image) orelse { + check("initial_ramdisk image is valid", false); + result(); + return; + }; + + // What the kernel found: the expected marker is built from its own count. + var buffer: [64]device_abi.DeviceDescriptor = undefined; + const n = @min(devices_broker.enumerate(&buffer), buffer.len); + var kernel_count: u32 = 0; + for (buffer[0..n]) |d| { + if (d.class == @intFromEnum(device_abi.DeviceClass.pci_device)) kernel_count += 1; + } + check("the kernel enumerated PCI functions to compare against", kernel_count >= 1); + var marker_buffer: [48]u8 = undefined; + const marker = std.fmt.bufPrint(&marker_buffer, "pci-bus: {d} functions found", .{kernel_count}) catch { + check("marker formatted", false); + result(); + return; + }; + + process.setInitialRamdisk(image); + process.write_count = 0; + var manager: u32 = 0; + var i: u32 = 0; + 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; + break; + } + check("device-manager spawned", manager != 0); + + scheduler.setPriority(1); + const 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; + scheduler.yield(); + } + scheduler.setPriority(4); + check("the ring-3 scan found exactly the kernel's function count", seen); + result(); +} + /// The whole user-side surface at once: spawn process-test's supervisor role, /// which — entirely from ring 3 — creates an exit endpoint, spawns its two /// children supervised, sees them in process_enumerate, kills them (one blocked, diff --git a/system/services/device-manager/device-manager.zig b/system/services/device-manager/device-manager.zig index 262f026..a579bbe 100644 --- a/system/services/device-manager/device-manager.zig +++ b/system/services/device-manager/device-manager.zig @@ -318,6 +318,13 @@ fn initialise(endpoint: runtime.ipc.Handle) bool { var matched: usize = 0; for (buffer[0..count]) |descriptor| { + if (descriptor.class == @intFromEnum(device.DeviceClass.pci_host_bridge)) { + // The PCI bus driver: enumeration in ring 3 (M19), one instance + // per bridge, the bridge id as its assignment. + matched += 1; + addDriver("pci-bus", descriptor.id, true); + continue; + } if (pciDriverFor(descriptor)) |driver_name| { matched += 1; addDriver(driver_name, descriptor.id, true); diff --git a/test/qemu_test.py b/test/qemu_test.py index 79e168d..9d3a0a4 100644 --- a/test/qemu_test.py +++ b/test/qemu_test.py @@ -274,6 +274,13 @@ CASES = [ r"device-manager: restarting usb-xhci-bus[\s\S]*" r"device-manager: child added", "fail": r"DANOS-TEST-RESULT: FAIL"}, + # M19.1: the ring-3 PCI scan (pci-bus walks the ECAM through its mmio_map + # grant) finds exactly the functions the kernel's own walk recorded. + {"name": "pci-scan", + "smp": 4, + "timeout": 60, + "expect": r"DANOS-TEST-RESULT: PASS", + "fail": r"DANOS-TEST-RESULT: FAIL"}, # M18.3: the application surface — device-list enumerates the tree over IPC, # subscribes (endpoint as capability), and observes the removed/added events # the reporter's test-kill produces (docs/device-manager.md). 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 3/4] 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 From af2c766f420772c6136af09e87a4e7a2ba8e758c Mon Sep 17 00:00:00 2001 From: Daniel Samson <12231216+daniel-samson@users.noreply.github.com> Date: Mon, 13 Jul 2026 02:54:50 +0100 Subject: [PATCH 4/4] The flip: PCI enumeration leaves the kernel (M19.3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit enumeratePci, addBars, pciConfigurationPtr, and the PciHeader struct are deleted; the kernel seeds only the host bridge, and the ring-3 pci-bus driver's reports are the sole source of PCI function nodes. The manager matches PCI drivers from reported identity, deduped by registered device id so a bus restart never double-spawns. The flip did its job by exposing a latent SMP race: ring-3 device_register made the broker table concurrent for the first time, and mmio_map read it lock-free — under load a torn resource length mapped hpet's window wrong (its user fault) and underflowed r.len-1 into a kernel integer-overflow panic. Fixed: the broker read in mmio_map (and claim) runs under the big kernel lock, the arithmetic rejects zero-length and wrapping windows cleanly, and pci-bus no longer registers unimplemented size-0 BARs. driver-restart hammered 6x, suite 55/55. --- docs/discovery.md | 10 ++ docs/m19-m20-plan.md | 14 +- system/devices/acpi.zig | 153 ++---------------- system/drivers/pci-bus/pci-bus.zig | 5 +- system/kernel/process.zig | 25 ++- system/kernel/tests.zig | 77 ++++++--- .../device-manager/device-manager.zig | 36 +++-- 7 files changed, 135 insertions(+), 185 deletions(-) diff --git a/docs/discovery.md b/docs/discovery.md index 86499a6..6dad24b 100644 --- a/docs/discovery.md +++ b/docs/discovery.md @@ -167,3 +167,13 @@ free; discovery on x86 is partly about *finding* what ARM just tells you. - [ipc.md](ipc.md) — the channels that interrupts-as-messages and the device manager will ride on. - [vision.md](vision.md) — why drivers belong in isolated user space at all. + +## Update (M19.3, 2026-07-13): PCI enumeration left the kernel + +The kernel now seeds only the `pci_host_bridge` node (ECAM window, MMIO +apertures derived from the memory map's holes, bus range, and the 16-bit I/O +window). The per-function walk moved to the ring-3 `pci-bus` driver +([device-manager.md](device-manager.md)): it claims the bridge, repeats the +ECAM scan through its mmio grant, and `device_register`s what it finds, which +the device manager mirrors and matches. The ACPI namespace walk follows in M20; +the static tables (MADT, HPET, MCFG, FADT + `\\_S5`) stay kernel-side. diff --git a/docs/m19-m20-plan.md b/docs/m19-m20-plan.md index 56e4d76..881839d 100644 --- a/docs/m19-m20-plan.md +++ b/docs/m19-m20-plan.md @@ -96,11 +96,15 @@ branch is green; keep branches; push everything. 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`) - are the assertion — xhci must come up spawned off a pci-bus report, and - the suite must not be able to tell the difference. discovery.md updated. +- [x] **M19.3** — the flip: kernel `enumeratePci`/`addBars`/`PciHeader` all + deleted (bridge node stays); manager matches PCI drivers from reported + identity, deduped by registered id. Surfaced and fixed a real SMP race the + flip created — ring-3 device_register made the broker table concurrent, so + mmio_map's lock-free read intermittently tore hpet's resource length + (user fault) and overflowed `r.len-1` into a kernel panic; now the broker + read is under the big lock and the arithmetic is guarded, and pci-bus + skips size-0 BARs. discovery.md updated; suite 55/55 (driver-restart + hammered 6×). - [ ] **merge** `feat/pci-bus` → main, push. - [ ] **M20.1** — acpi service, parse only (fills the existing placeholder at system/services/acpi/acpi.zig): kernel publishes `acpi-tables` diff --git a/system/devices/acpi.zig b/system/devices/acpi.zig index 8aa495d..8e008db 100644 --- a/system/devices/acpi.zig +++ b/system/devices/acpi.zig @@ -360,25 +360,6 @@ const Hpet = extern struct { page_protection: u8, }; -// --- PCI configuration-space header (first 64 bytes, common fields) --------- - -const PciHeader = extern struct { - vendor_id: u16 align(1), - device_id: u16 align(1), - command: u16 align(1), - status: u16 align(1), - revision_id: u8, - prog_if: u8, - subclass: u8, - class_code: u8, - cache_line_size: u8, - latency_timer: u8, - /// bit 7 set => multi-function device. - header_type: u8, - bist: u8, - // 0x10 onward (BARs, etc.) depends on header_type; read separately. -}; - // --- Entry point ------------------------------------------------------------ /// Discover hardware from the ACPI tables rooted at `rsdp_physical` and populate @@ -452,7 +433,7 @@ fn handleTable(device_tree: *DeviceTree, hal: Hal, sdt_physical: u64) !void { if (std.mem.eql(u8, &sig, &APIC)) { try parseMadt(device_tree, header); } else if (std.mem.eql(u8, &sig, &MCFG)) { - try parseMcfg(device_tree, hal, header); + try parseMcfg(device_tree, header); } else if (std.mem.eql(u8, &sig, &HPET)) { try parseHpet(device_tree, hal, header); } else if (std.mem.eql(u8, &sig, &FACP)) { @@ -537,7 +518,7 @@ fn parseMadt(device_tree: *DeviceTree, header: *const SystemDescriptorTableHeade } /// MCFG -> a pci_host_bridge per ECAM segment, then a PCI enumeration underneath. -fn parseMcfg(device_tree: *DeviceTree, hal: Hal, header: *const SystemDescriptorTableHeader) !void { +fn parseMcfg(device_tree: *DeviceTree, header: *const SystemDescriptorTableHeader) !void { const total: usize = header.length; const base: [*]const u8 = @ptrCast(header); @@ -557,7 +538,11 @@ fn parseMcfg(device_tree: *DeviceTree, hal: Hal, header: *const SystemDescriptor // 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.*); + // The function walk itself retired to ring 3 (M19.3): the pci-bus + // driver claims this bridge, repeats the scan through its ECAM grant, + // and device_registers what it finds — the kernel seeds only the + // bridge. The scan's equivalence was proven before the hand-off + // (pci-scan), and the walk's history is in git if archaeology calls. } } @@ -587,7 +572,13 @@ fn addBridgeApertures(bridge: *device_model.Device) void { var high_end: u64 = 1 << 32; for (boot_memory_regions) |region| { const end = region.base + region.pages * 4096; - if (end > high_end) high_end = end; + // Above 4 GiB only *usable RAM* blocks the aperture: OVMF describes + // its own 64-bit PCI window as a reserved region and then programs + // BARs inside it — honoring reserved there would exclude the very + // space BARs live in. Below 4 GiB every described region blocks (the + // kernel image, the tables, the ramdisk all live there). Bring-up + // trust: only the bridge's claimant can register into the aperture. + if (region.kind == .usable and end > high_end) high_end = end; if (region.base >= (1 << 32) or below_count == below.len) continue; below[below_count] = .{ .base = region.base, .end = @min(end, 1 << 32) }; below_count += 1; @@ -623,106 +614,6 @@ fn addBridgeApertures(bridge: *device_model.Device) void { _ = bridge.addResource(.memory, high_end, (@as(u64, 1) << 46) - high_end); } -/// Brute-force scan the ECAM window's bus range for present PCI functions. No -/// bridge recursion yet: on the ECAM path the host bridge decodes every bus in -/// the window, so scanning the declared range finds everything QEMU exposes. -fn enumeratePci( - device_tree: *DeviceTree, - bridge: *device_model.Device, - hal: Hal, - alloc: McfgAllocation, -) !void { - var bus: u16 = alloc.start_bus; - while (bus <= alloc.end_bus) : (bus += 1) { - var device: u8 = 0; - while (device < 32) : (device += 1) { - const h0: *align(1) const PciHeader = @ptrCast(pciConfigurationPtr(alloc, hal, @intCast(bus), device, 0)); - if (h0.vendor_id == 0xFFFF) continue; // no function 0 => slot empty - - const funcs: u8 = if (h0.header_type & 0x80 != 0) 8 else 1; - var function: u8 = 0; - while (function < funcs) : (function += 1) { - const configuration = pciConfigurationPtr(alloc, hal, @intCast(bus), device, function); - const h: *align(1) const PciHeader = @ptrCast(configuration); - if (h.vendor_id == 0xFFFF) continue; - - var nb: [24]u8 = undefined; - const nm = std.fmt.bufPrint(&nb, "{s}:{x:0>2}:{x:0>2}.{d}", .{ - bridge.name(), bus, device, function, - }) catch "pcidev"; - const node = try device_tree.addChild(bridge, .pci_device, nm); - // Resource 0 is the function's own 4 KiB ECAM configuration space. A - // claimed PCI driver mmio_maps this to reach its command register, - // BARs, and — the point — its capability list (MSI/MSI-X, PCIe - // extended caps), without any new syscall. Physical address per the - // ECAM formula (same as pciConfigurationPtr). - const config_physical = alloc.base_address + - (@as(u64, @as(u8, @intCast(bus)) - alloc.start_bus) << 20) + - (@as(u64, device) << 15) + (@as(u64, function) << 12); - _ = node.addResource(.memory, config_physical, abi.page_size); - node.ids.pci_vendor = h.vendor_id; - node.ids.pci_device = h.device_id; - node.ids.pci_class = (@as(u24, h.class_code) << 16) | - (@as(u24, h.subclass) << 8) | h.prog_if; - node.ids.pci_bdf = (@as(u16, @intCast(bus)) << 8) | (@as(u16, device) << 3) | function; - - // BARs only exist in header type 0 (normal devices), not bridges. - if (h.header_type & 0x7F == 0) addBars(node, configuration); - } - } - } -} - -/// Record and size the memory/IO windows named by a device's Base Address -/// Registers. Sizing is the standard probe: disable decode, write all-ones, read -/// back the writable (address) bits, restore. `size = ~mask + 1`. -fn addBars(node: *device_model.Device, configuration: [*]align(1) u8) void { - // Stop the device decoding its BARs while we transiently write all-ones. - const command = rd(u16, configuration, 0x04); - wr(u16, configuration, 0x04, command & ~@as(u16, 0b11)); - - var i: usize = 0; - while (i < 6) : (i += 1) { - const off = 0x10 + i * 4; - const orig = rd(u32, configuration, off); - if (orig == 0) continue; - - if (orig & 1 != 0) { - // I/O-space BAR (16-bit address space on x86). - wr(u32, configuration, off, 0xFFFF_FFFF); - const readback = rd(u32, configuration, off); - wr(u32, configuration, off, orig); - const mask = readback & 0xFFFF_FFFC; - const size: u32 = if (mask == 0) 0 else (~mask +% 1) & 0xFFFF; - _ = node.addResource(.io_port, orig & 0xFFFF_FFFC, size); - } else if ((orig >> 1) & 0x3 == 2) { - // 64-bit memory BAR: this BAR pair spans two configuration slots. - const orig_hi = rd(u32, configuration, off + 4); - wr(u32, configuration, off, 0xFFFF_FFFF); - wr(u32, configuration, off + 4, 0xFFFF_FFFF); - const lo = rd(u32, configuration, off); - const hi = rd(u32, configuration, off + 4); - wr(u32, configuration, off, orig); - wr(u32, configuration, off + 4, orig_hi); - const readback = (@as(u64, hi) << 32) | (lo & 0xFFFF_FFF0); - const size: u64 = if (readback == 0) 0 else ~readback +% 1; - const address = (@as(u64, orig_hi) << 32) | (orig & 0xFFFF_FFF0); - _ = node.addResource(.memory, address, size); - i += 1; // consumed the high half - } else { - // 32-bit memory BAR. - wr(u32, configuration, off, 0xFFFF_FFFF); - const readback = rd(u32, configuration, off); - wr(u32, configuration, off, orig); - const mask = readback & 0xFFFF_FFF0; - const size: u32 = if (mask == 0) 0 else ~mask +% 1; - _ = node.addResource(.memory, orig & 0xFFFF_FFF0, size); - } - } - - wr(u16, configuration, 0x04, command); // restore decode -} - /// HPET -> a timer node with its register block as an MMIO resource, plus the GSI /// its comparators can raise. /// @@ -1253,16 +1144,6 @@ fn readCntRegister(base: [*]align(1) const u8, len: usize, xoff: usize, legacy_o /// The mapped configuration space of one PCI function (its 4 KiB ECAM page). Mapped /// writable so BAR sizing can probe it; reads and writes both go through here. -fn pciConfigurationPtr(alloc: McfgAllocation, hal: Hal, bus: u8, device: u8, function: u8) [*]align(1) u8 { - const physical = alloc.base_address + - (@as(u64, bus - alloc.start_bus) << 20) + - (@as(u64, device) << 15) + - (@as(u64, function) << 12); - // Map the configuration page (writable, for BAR sizing) and use the virtual - // address the HAL hands back. - return @ptrFromInt(hal.mapMmio(physical, abi.page_size, true)); -} - /// Read a little-endian integer at `off` from a (possibly unaligned) byte pointer. /// x86 is little-endian and native, so an unaligned load suffices. fn rd(comptime T: type, bytes: [*]align(1) const u8, off: usize) T { @@ -1270,12 +1151,6 @@ fn rd(comptime T: type, bytes: [*]align(1) const u8, off: usize) T { return p.*; } -/// Write a little-endian integer at `off` through a (possibly unaligned) pointer. -fn wr(comptime T: type, bytes: [*]align(1) u8, off: usize, value: T) void { - const p: *align(1) T = @ptrCast(bytes + off); - p.* = value; -} - // --- tests ------------------------------------------------------------------ test "eisaIdToStr decodes a packed EISA id" { diff --git a/system/drivers/pci-bus/pci-bus.zig b/system/drivers/pci-bus/pci-bus.zig index d54aa3c..2c19c59 100644 --- a/system/drivers/pci-bus/pci-bus.zig +++ b/system/drivers/pci-bus/pci-bus.zig @@ -180,6 +180,7 @@ fn registerAndReport(bus: u64, dev: u64, function: u64, class_triple: u32) void configWrite(bus, dev, function, off, original); const mask = readback & 0xFFFF_FFFC; const size: u32 = if (mask == 0) 0 else (~mask +% 1) & 0xFFFF; + if (size == 0) continue; // unimplemented BAR — nothing to register 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) { @@ -192,15 +193,17 @@ fn registerAndReport(bus: u64, dev: u64, function: u64, class_triple: u32) void 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; + i += 1; // consumed the high half regardless + if (size == 0) continue; 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; + if (size == 0) continue; descriptor.resources[slot] = .{ .kind = @intFromEnum(device.ResourceKind.memory), .start = original & 0xFFFF_FFF0, .len = size }; descriptor.resource_count += 1; } diff --git a/system/kernel/process.zig b/system/kernel/process.zig index d795ec4..a724862 100644 --- a/system/kernel/process.zig +++ b/system/kernel/process.zig @@ -298,6 +298,8 @@ fn systemDeviceEnumerate(state: *architecture.CpuState) void { /// device_claim(id) -> 0/-1: take exclusive ownership of a device for this process. fn systemDeviceClaim(state: *architecture.CpuState) void { + const claim_flags = sync.enter(); + defer sync.leave(claim_flags); if (devices_broker.claim(architecture.systemCallArg(state, 0), scheduler.current().id)) architecture.setSystemCallResult(state, 0) else @@ -312,10 +314,22 @@ fn systemMmioMap(state: *architecture.CpuState) void { const resource_index = architecture.systemCallArg(state, 1); const t = scheduler.current(); if (t.aspace == 0) return fail(state); - const owner = devices_broker.ownerOf(device_id) orelse return fail(state); - if (owner != t.id) return fail(state); // not claimed by this process - const r = devices_broker.resourceOf(device_id, resource_index) orelse return fail(state); + // Read the broker table under the lock: ring-3 device_register (M19) now + // mutates it concurrently on other cores, so a lock-free read here could + // see a torn resource (and a torn length used to panic the arithmetic + // below on integer overflow). + const r = blk: { + const flags = sync.enter(); + defer sync.leave(flags); + const owner = devices_broker.ownerOf(device_id) orelse return fail(state); + if (owner != t.id) return fail(state); // not claimed by this process + break :blk devices_broker.resourceOf(device_id, resource_index) orelse return fail(state); + }; if (r.kind != @intFromEnum(device_abi.ResourceKind.memory)) return fail(state); + // A zero-length or wrapping window is not mappable — fail cleanly rather + // than underflow `r.len - 1`. + if (r.len == 0) return fail(state); + if (@addWithOverflow(r.start, r.len)[1] != 0) return fail(state); if (t.device_map_next == 0) t.device_map_next = device_arena_base; const first = r.start & ~@as(u64, page_size - 1); @@ -451,6 +465,11 @@ fn systemDeviceRegister(state: *architecture.CpuState) void { var descriptor: device_abi.DeviceDescriptor = undefined; if (!ipc.copyFromUser(t.aspace, descriptor_ptr, std.mem.asBytes(&descriptor))) return fail(state); + // Under the big kernel lock: the broker's table is also mutated by the + // death sweep (releaseAllOwnedBy) and read by enumerate on other cores — + // ring-3 registration (M19) made those genuinely concurrent. + const flags = sync.enter(); + defer sync.leave(flags); const id = devices_broker.register(parent_id, t.id, &descriptor) catch return fail(state); architecture.setSystemCallResult(state, id); } diff --git a/system/kernel/tests.zig b/system/kernel/tests.zig index 2a3b497..47a6596 100644 --- a/system/kernel/tests.zig +++ b/system/kernel/tests.zig @@ -270,20 +270,28 @@ fn discoveryTest() void { // M15: every PCI function now carries its own 4 KiB ECAM configuration space as // resource 0 — the window a driver mmio_maps to walk its capability list (MSI etc). + // M19.3: the kernel seeds only the bridge; functions arrive by the ring-3 + // scan (proven equivalent in pci-scan before the walk retired). var buffer: [64]device_abi.DeviceDescriptor = undefined; const n = @min(devices_broker.enumerate(&buffer), buffer.len); - var pci_functions: u32 = 0; - var pci_config_ok = true; + var bridges: u32 = 0; + var bridge_shape_ok = false; for (buffer[0..n]) |d| { - if (d.class != @intFromEnum(device_abi.DeviceClass.pci_device)) continue; - pci_functions += 1; - const has_config = d.resource_count >= 1 and - d.resources[0].kind == @intFromEnum(device_abi.ResourceKind.memory) and - d.resources[0].len == abi.page_size; - if (!has_config) pci_config_ok = false; + if (d.class != @intFromEnum(device_abi.DeviceClass.pci_host_bridge)) continue; + bridges += 1; + var has_bus_range = false; + var has_io = false; + var memory_windows: u32 = 0; + for (d.resources[0..@intCast(d.resource_count)]) |resource| { + if (resource.kind == @intFromEnum(device_abi.ResourceKind.bus_range)) has_bus_range = true; + if (resource.kind == @intFromEnum(device_abi.ResourceKind.io_port)) has_io = true; + if (resource.kind == @intFromEnum(device_abi.ResourceKind.memory)) memory_windows += 1; + } + // ECAM plus at least one MMIO aperture, the bus range, the I/O window. + if (has_bus_range and has_io and memory_windows >= 2) bridge_shape_ok = true; } - check("PCI functions were enumerated (MCFG/ECAM)", pci_functions >= 1); - check("each PCI function exposes its ECAM config space as resource 0", pci_config_ok); + check("a PCI host bridge was seeded (MCFG)", bridges >= 1); + check("the bridge carries ECAM, apertures, bus range, and the I/O window", bridge_shape_ok); // M19.0: every PCI memory resource (config slice and BARs alike) must be // contained in one of its parent bridge's windows — the aperture derivation @@ -1814,20 +1822,16 @@ fn pciScanTest(boot_information: *const BootInformation) void { return; }; - // What the kernel found: the expected marker is built from its own count. + // Post-flip (M19.3) ground truth: the kernel no longer enumerates PCI + // functions, so equivalence inverts — the broker's function count after + // the scan must equal what the driver itself reported finding. var buffer: [64]device_abi.DeviceDescriptor = undefined; const n = @min(devices_broker.enumerate(&buffer), buffer.len); - var kernel_count: u32 = 0; + var boot_pci: u32 = 0; for (buffer[0..n]) |d| { - if (d.class == @intFromEnum(device_abi.DeviceClass.pci_device)) kernel_count += 1; + if (d.class == @intFromEnum(device_abi.DeviceClass.pci_device)) boot_pci += 1; } - check("the kernel enumerated PCI functions to compare against", kernel_count >= 1); - var marker_buffer: [48]u8 = undefined; - const marker = std.fmt.bufPrint(&marker_buffer, "pci-bus: {d} functions found", .{kernel_count}) catch { - check("marker formatted", false); - result(); - return; - }; + check("the kernel seeded no PCI functions (the walk retired)", boot_pci == 0); process.setInitialRamdisk(image); process.write_count = 0; @@ -1841,16 +1845,35 @@ fn pciScanTest(boot_information: *const BootInformation) void { } check("device-manager spawned (test-pci-restart mode)", manager != 0); - // First scan: the ring-3 count equals the kernel's. + // First scan: wait for the driver's count line and parse the number. + const count_prefix = "pci-bus: "; + const count_suffix = " functions found"; + var reported: u32 = 0; scheduler.setPriority(1); 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; + while (architecture.millis() < deadline and reported == 0) { + if (process.write_len > count_prefix.len + count_suffix.len and eql(process.write_buffer[0..count_prefix.len], count_prefix)) { + const line = process.write_buffer[0..process.write_len]; + const digits_end = std.mem.indexOf(u8, line, count_suffix) orelse { + scheduler.yield(); + continue; + }; + reported = std.fmt.parseInt(u32, line[count_prefix.len..digits_end], 10) catch 0; + } scheduler.yield(); } scheduler.setPriority(4); - check("the ring-3 scan found exactly the kernel's function count", seen); + check("the ring-3 scan reported a function count", reported >= 1); + + // Every reported function was registered: the broker holds exactly them. + var registered: [64]device_abi.DeviceDescriptor = undefined; + const r = @min(devices_broker.enumerate(®istered), registered.len); + var registered_pci: u32 = 0; + for (registered[0..r]) |d| { + if (d.class == @intFromEnum(device_abi.DeviceClass.pci_device)) registered_pci += 1; + } + check("the broker holds exactly the reported functions", registered_pci == reported); + const kernel_count = reported; // the no-duplicate check below reuses it // The restart drill: the manager kills pci-bus after its reports; the // respawn re-claims, re-scans, and re-registers. @@ -1865,9 +1888,11 @@ fn pciScanTest(boot_information: *const BootInformation) void { scheduler.setPriority(4); check("the manager restarted pci-bus", restarted); + var marker_buffer: [48]u8 = undefined; + const marker = std.fmt.bufPrint(&marker_buffer, "pci-bus: {d} functions found", .{reported}) catch ""; scheduler.setPriority(1); deadline = architecture.millis() + 15000; - seen = false; + 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; scheduler.yield(); diff --git a/system/services/device-manager/device-manager.zig b/system/services/device-manager/device-manager.zig index 178cef0..ed2b4cb 100644 --- a/system/services/device-manager/device-manager.zig +++ b/system/services/device-manager/device-manager.zig @@ -50,17 +50,26 @@ fn driverFor(d: device.DeviceDescriptor) ?[]const u8 { /// pci-class.zig decodes. const xhci_pci_class: u64 = 0x0C_03_30; -/// The bus driver that serves a PCI function, or null. A machine can carry -/// several identical controllers — one driver instance per device, the id as -/// argv[1]. These drivers speak the protocol: a hello is expected. -fn pciDriverFor(d: device.DeviceDescriptor) ?[]const u8 { - if (d.class != @intFromEnum(device.DeviceClass.pci_device)) return null; - return switch (d.pci_class) { +/// The driver that serves a *reported* PCI function (M19.3: matching moved +/// from the boot snapshot to the bus reports), or null. A machine can carry +/// several identical controllers — one driver instance per reported device, +/// its registered id as argv[1]. +fn pciDriverForIdentity(identity: u64) ?[]const u8 { + return switch (identity) { xhci_pci_class => "usb-xhci-bus", else => null, }; } +/// Whether some driver entry already serves registered device `device_id` — +/// a re-report after a bus restart must not spawn a second instance. +fn driverForDevice(device_id: u64) bool { + for (&drivers) |*driver| { + if (driver.used and driver.device_id == device_id) return true; + } + return false; +} + // --- supervision ------------------------------------------------------------- /// How long a protocol driver has to hello after its spawn. @@ -326,11 +335,8 @@ fn initialise(endpoint: runtime.ipc.Handle) bool { addDriver("pci-bus", descriptor.id, true); continue; } - if (pciDriverFor(descriptor)) |driver_name| { - matched += 1; - addDriver(driver_name, descriptor.id, true); - continue; - } + // PCI functions no longer appear in the boot snapshot (M19.3): the + // pci-bus driver reports them, and onChildAdded matches from reports. const driver_name = driverFor(descriptor) orelse continue; matched += 1; // Skip a singleton that is already alive (the initial-ramdisk sweep test @@ -396,6 +402,14 @@ fn onChildAdded(message: []const u8, reply: []u8, sender: u32) usize { if (!addChild(report.parent, report.bus_address, report.identity, report.device_id, sender)) status = -1; writeLine("device-manager: child added (device {d} port {d}, identity {d}) by {s}\n", .{ report.parent, report.bus_address, report.identity, driver.name() }); if (status == 0) publishEvent(message[0..protocol.child_added_size]); + // Matching from reports (M19.3): a registered child whose identity + // names a driver gets one, once — re-reports after a bus restart + // dedupe on the registered id, exactly like the registrations do. + if (status == 0 and report.device_id != protocol.no_device) { + if (pciDriverForIdentity(report.identity)) |child_driver| { + if (!driverForDevice(report.device_id)) addDriver(child_driver, report.device_id, true); + } + } } else { status = -1; }