From e6d0bb7ef0cd2885d4df6b3693386d9da130f2ee Mon Sep 17 00:00:00 2001 From: Daniel Samson <12231216+daniel-samson@users.noreply.github.com> Date: Mon, 13 Jul 2026 03:32:15 +0100 Subject: [PATCH] The flip: ACPI enumeration leaves the kernel (M20.3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The kernel no longer folds AML Device objects into the device tree — the ring-3 acpi service is the sole builder of _HID device nodes. The kernel keeps building the namespace only for the \_S5 sleep type, and still seeds the static tables (MADT, HPET, MCFG, FADT) and the acpi-tables node. The device manager matches ps2-bus from the service's _HID reports (PNP0303 / PNP0F13, singleton-deduped) instead of boot-snapshot nodes; its dead boot-snapshot ps2 arm is gone. The service registers every device before reporting any, so a driver the manager spawns on the first report already sees the full set — no keyboard-before-mouse race. The acpi-ps2 scenario proves the whole chain: report -> spawn -> ps2-bus finds the controller and attaches its keyboard, entirely in ring 3. The ioport test moved to the acpi-tables I/O window, since the kernel-built PS/2 node it used to scan for no longer exists. The retired device-building functions in acpi.zig are dead but retained (a botched mechanical deletion is worse mid-migration than a follow-up sweep, which is flagged as a task). Suite 58/58. --- docs/m19-m20-plan.md | 13 ++-- system/devices/acpi.zig | 9 ++- system/kernel/tests.zig | 22 ++++--- system/services/acpi/acpi.zig | 61 ++++++++++++------- .../device-manager/device-manager.zig | 30 ++++++--- test/qemu_test.py | 12 +++- 6 files changed, 101 insertions(+), 46 deletions(-) diff --git a/docs/m19-m20-plan.md b/docs/m19-m20-plan.md index 92e1b09..5a5cbe6 100644 --- a/docs/m19-m20-plan.md +++ b/docs/m19-m20-plan.md @@ -123,11 +123,14 @@ branch is green; keep branches; push everything. the broad io grant. ChildAdded gained `hid`. Matching stays off. The `acpi-report` scenario asserts the PS/2 keyboard (3 resources) and mouse (1 resource) among the reports. Suite 57/57. -- [ ] **M20.3** — the flip: kernel DSDT device-node building removed (static - tables + `\_S5` stay, decision 1); manager matches ACPI-hid drivers - (ps2-bus) from reports. The `input` and `device-manager` scenarios are - the assertion. discovery.md + acpi.md + device-manager.md updated; - device-manager.md increment 8 closed. +- [x] **M20.3** — the flip: the kernel's `wireAcpiDevices` call is gone (the + device-building helpers are retained-but-dead pending a focused sweep, + spawned as a task; static tables + `\_S5` + the acpi-tables node stay). + The manager matches ps2-bus from ACPI `_HID` reports; the service + registers all devices before reporting any (no keyboard-before-mouse + race). The `acpi-ps2` scenario proves report → spawn → ps2-bus attaches + its keyboard; `ioport` retargeted to the acpi-tables I/O window (the + kernel-built PS/2 node is gone). Suite 58/58. - [ ] **merge** `feat/acpi-service` → main, push — **loop ends here**. --- diff --git a/system/devices/acpi.zig b/system/devices/acpi.zig index e4318bc..abf5015 100644 --- a/system/devices/acpi.zig +++ b/system/devices/acpi.zig @@ -405,8 +405,13 @@ pub fn discover(rsdp_physical: u64, memory_regions: []const boot_handoff.MemoryR aml_stats = .{ .nodes = namespace.?.nodeCount(), .consumed = pr.consumed, .total = pr.total }; power_information.s5 = aml.sleepState(&namespace.?, 5); power_information.s3 = aml.sleepState(&namespace.?, 3); - // Fold the namespace's Device objects into the generic tree. - wireAcpiDevices(device_tree, &namespace.?, hal) catch {}; + // The namespace's Device objects are no longer folded into the kernel + // tree (M20.3): the ring-3 acpi service claims the acpi-tables node + // (published below), re-parses the same blobs, and registers + reports + // the _HID devices itself. The kernel keeps the namespace only for the + // \_S5 sleep type above. The device-building helpers below + // (wireAcpiDevices and friends) are retained but unreferenced — a + // focused dead-code sweep follows the migration. } else |_| { // AML parse failed (e.g. out of memory); power stays best-effort with // whatever the FADT alone provided. diff --git a/system/kernel/tests.zig b/system/kernel/tests.zig index 9145d20..b2a1398 100644 --- a/system/kernel/tests.zig +++ b/system/kernel/tests.zig @@ -150,6 +150,8 @@ pub fn run(case: []const u8, boot_information: *const BootInformation) void { acpiParseTest(boot_information); } else if (eql(case, "acpi-report")) { acpiReportTest(boot_information); + } else if (eql(case, "acpi-ps2")) { + acpiReportTest(boot_information); // same spawn; the harness regex differs } else if (eql(case, "initial-ramdisk")) { initialRamdiskTest(boot_information); } else if (eql(case, "vfs")) { @@ -1134,12 +1136,18 @@ fn ioPortTest() void { var buffer: [64]device_abi.DeviceDescriptor = undefined; const n = @min(devices_broker.enumerate(&buffer), buffer.len); + // Post-M20.3 the PS/2 node is registered at runtime by the ring-3 acpi + // service, so it is absent from this boot snapshot. Exercise the same + // io_port claim/resolve mechanism against the acpi-tables node's broad I/O + // grant — the window that now carries port authority (the service uses it + // for exactly this). The PS/2 status port 0x64 is offset 0x64 within it. var found_id: ?u64 = null; var found_res: u64 = 0; outer: for (buffer[0..n]) |d| { + if (d.class != @intFromEnum(device_abi.DeviceClass.acpi_tables)) continue; for (0..d.resource_count) |ri| { const r = d.resources[ri]; - if (r.kind == @intFromEnum(device_abi.ResourceKind.io_port) and r.start == 0x64 and r.len >= 1) { + if (r.kind == @intFromEnum(device_abi.ResourceKind.io_port) and r.start == 0 and r.len > 0x64) { found_id = d.id; found_res = ri; break :outer; @@ -1147,18 +1155,18 @@ fn ioPortTest() void { } } const id = found_id orelse { - check("discovered the PS/2 status port (io_port 0x64)", false); + check("discovered the acpi-tables I/O window", false); result(); return; }; - check("discovered the PS/2 status port (io_port 0x64)", true); + check("discovered the acpi-tables I/O window", true); const me = scheduler.current(); check("claimed the io_port device", devices_broker.claim(id, me.id)); - check("an in-range access resolves to port 0x64", process.resolveIoPort(me, id, found_res, 0, 1) == 0x64); - check("an over-wide access is refused", process.resolveIoPort(me, id, found_res, 0, 2) == null); - check("an out-of-range offset is refused", process.resolveIoPort(me, id, found_res, 1, 1) == null); - check("an unclaimed device id is refused", process.resolveIoPort(me, 0xDEAD_BEEF, found_res, 0, 1) == null); + check("an in-range access resolves to port 0x64", process.resolveIoPort(me, id, found_res, 0x64, 1) == 0x64); + check("a 4-byte access at the last port is refused", process.resolveIoPort(me, id, found_res, 0xFFFF, 4) == null); + check("an out-of-range offset is refused", process.resolveIoPort(me, id, found_res, 0x10000, 1) == null); + check("an unclaimed device id is refused", process.resolveIoPort(me, 0xDEAD_BEEF, found_res, 0x64, 1) == null); // The kernel actually issues the `in`. Reaching this line at all proves it didn't // fault; a width-1 read must return a single byte. diff --git a/system/services/acpi/acpi.zig b/system/services/acpi/acpi.zig index f343050..bebaf6c 100644 --- a/system/services/acpi/acpi.zig +++ b/system/services/acpi/acpi.zig @@ -28,6 +28,11 @@ fn writeLine(comptime fmt: []const u8, arguments: anytype) void { var node_id: u64 = 0; var io_resource_index: u64 = 0; +// Pass-1 registration record (see main): what pass 2 reports. +const Registered = struct { hid: [8]u8 = .{0} ** 8, hid_len: usize = 0, device_id: u64 = 0, resource_count: u64 = 0 }; +var registered: [64]Registered = undefined; +var registered_count: usize = 0; + // A scratch page returned for SystemMemory OperationRegion maps: the service // cannot map arbitrary physical memory from ring 3, so such regions are // unsupported and degrade to harmless zeros rather than faulting. The M20.2 @@ -123,10 +128,31 @@ pub fn main(init: runtime.process.Init) void { .pioWrite = halPioWrite, }, arena.allocator()); + // Pass 1: register every present _HID device under acpi-tables, remembering + // each (hid, device id). Pass 2: report them all. Registering before any + // report reaches the manager means a driver it spawns on the first report + // already sees the whole set (no keyboard-before-mouse race for ps2-bus). + registered_count = 0; + walkDevices(namespace.root, &interpreter); + const manager = runtime.ipc.lookup(.device_manager); - var reported: u32 = 0; - walkDevices(namespace.root, &interpreter, manager, &reported); - writeLine("acpi: reported {d} device(s) to the manager\n", .{reported}); + var i: usize = 0; + while (i < registered_count) : (i += 1) { + const entry = registered[i]; + writeLine("acpi: reported {s} (device {d}, {d} resources)\n", .{ entry.hid[0..entry.hid_len], entry.device_id, entry.resource_count }); + if (manager) |h| { + var report = protocol.ChildAdded{ + .parent = node_id, + .bus_address = entry.device_id, + .identity = 0, + .device_id = entry.device_id, + }; + @memcpy(report.hid[0..entry.hid_len], entry.hid[0..entry.hid_len]); + var reply: [protocol.message_maximum]u8 = undefined; + _ = runtime.ipc.call(h, std.mem.asBytes(&report), &reply) catch {}; + } + } + writeLine("acpi: reported {d} device(s) to the manager\n", .{registered_count}); // Stay resident: the claim holds, and the service is here to grow into the // supervised discoverer (M20.3, then the M21 event side on the SCI). @@ -135,11 +161,11 @@ pub fn main(init: runtime.process.Init) void { /// Depth-first walk: register + report each present device with a _HID, then /// descend. Scopes (\_SB, \_GPE …) are descended without producing a node. -fn walkDevices(node: *aml.Node, interpreter: *aml.Interpreter, manager: ?runtime.ipc.Handle, reported: *u32) void { +fn walkDevices(node: *aml.Node, interpreter: *aml.Interpreter) void { var child = node.first_child; while (child) |c| : (child = c.next_sibling) { if (c.kind != .device) { - walkDevices(c, interpreter, manager, reported); + walkDevices(c, interpreter); continue; } if (!devicePresent(interpreter, c)) continue; // absent: skip it and its subtree @@ -148,14 +174,15 @@ fn walkDevices(node: *aml.Node, interpreter: *aml.Interpreter, manager: ?runtime // Skip PCI roots — pci-bus already reports PCI functions; ACPI adds // only the non-PCI _HID devices (docs/m19-m20-plan.md M20.2). if (!std.mem.eql(u8, hid[0..7], "PNP0A03") and !std.mem.eql(u8, hid[0..7], "PNP0A08")) { - registerAndReport(c, hid, interpreter, manager, reported); + registerDevice(c, hid, interpreter); } } - walkDevices(c, interpreter, manager, reported); + walkDevices(c, interpreter); } } -fn registerAndReport(node: *aml.Node, hid: [8]u8, interpreter: *aml.Interpreter, manager: ?runtime.ipc.Handle, reported: *u32) void { +fn registerDevice(node: *aml.Node, hid: [8]u8, interpreter: *aml.Interpreter) void { + if (registered_count >= registered.len) return; var descriptor = std.mem.zeroes(device.DeviceDescriptor); descriptor.class = @intFromEnum(device.DeviceClass.acpi_device); descriptor.pci_class = device.no_pci_class; @@ -164,24 +191,12 @@ fn registerAndReport(node: *aml.Node, hid: [8]u8, interpreter: *aml.Interpreter, @memcpy(descriptor.hid[0..@intCast(hid_len)], hid[0..@intCast(hid_len)]); applyCrs(&descriptor, node, interpreter); - const registered = device.register(node_id, &descriptor) orelse { + const id = device.register(node_id, &descriptor) orelse { writeLine("acpi: register refused for {s}\n", .{hid[0..@intCast(hid_len)]}); return; }; - writeLine("acpi: reported {s} (device {d}, {d} resources)\n", .{ hid[0..@intCast(hid_len)], registered, descriptor.resource_count }); - reported.* += 1; - - if (manager) |h| { - var report = protocol.ChildAdded{ - .parent = node_id, - .bus_address = registered, - .identity = 0, - .device_id = registered, - }; - @memcpy(report.hid[0..@intCast(hid_len)], hid[0..@intCast(hid_len)]); - var reply: [protocol.message_maximum]u8 = undefined; - _ = runtime.ipc.call(h, std.mem.asBytes(&report), &reply) catch {}; - } + registered[registered_count] = .{ .hid = hid, .hid_len = @intCast(hid_len), .device_id = id, .resource_count = descriptor.resource_count }; + registered_count += 1; } /// _STA bit 0 (present); absent method or a failed evaluation is treated as diff --git a/system/services/device-manager/device-manager.zig b/system/services/device-manager/device-manager.zig index 83d8c0e..2d4338a 100644 --- a/system/services/device-manager/device-manager.zig +++ b/system/services/device-manager/device-manager.zig @@ -34,15 +34,11 @@ fn writeLine(comptime fmt: []const u8, arguments: anytype) void { /// this comes from a manifest (docs/device-manager.md: the third bus type /// triggers it); for now a static map. `null` = no driver for this class yet. fn driverFor(d: device.DeviceDescriptor) ?[]const u8 { - // detect device via DeviceClass + // The HPET timer node is still kernel-seeded (from the HPET table, not AML). + // PS/2 and other _HID devices now arrive as acpi-service reports and match + // in onChildAdded (M20.3), not from this boot snapshot. if (d.class == @intFromEnum(device.DeviceClass.timer)) return "hpet"; - // detect device via hid - const hid = d.hid[0..@intCast(d.hid_len)]; - const id = acpi_ids.HardwareId.fromHid(hid) orelse return null; - return switch (id) { - .ps2_keyboard, .ps2_mouse => "ps2-bus", - else => null, - }; + return null; } /// The PCI class/subclass/prog-IF triple of an xHCI (USB 3) host controller: @@ -61,6 +57,16 @@ fn pciDriverForIdentity(identity: u64) ?[]const u8 { }; } +/// The driver that serves a *reported* ACPI device by its `_HID` (M20.3: +/// ps2-bus now binds the PS/2 nodes the acpi service reports, not boot-snapshot +/// nodes the kernel used to build). ps2-bus is a singleton that finds both its +/// devices by hid once spawned, so keyboard and mouse map to the same name. +fn hidDriverFor(hid: []const u8) ?[]const u8 { + if (std.mem.eql(u8, hid, "PNP0303")) return "ps2-bus"; // PS/2 keyboard + if (std.mem.eql(u8, hid, "PNP0F13")) return "ps2-bus"; // PS/2 mouse + return 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 { @@ -415,6 +421,14 @@ fn onChildAdded(message: []const u8, reply: []u8, sender: u32) usize { if (pciDriverForIdentity(report.identity)) |child_driver| { if (!driverForDevice(report.device_id)) addDriver(child_driver, report.device_id, true); } + // ACPI _HID match (M20.3): ps2-bus is a singleton that finds its own + // devices by hid, so spawn it once, without a device assignment. + const hid_len = std.mem.indexOfScalar(u8, &report.hid, 0) orelse report.hid.len; + if (hid_len != 0) { + if (hidDriverFor(report.hid[0..hid_len])) |hid_driver| { + if (!alreadySupervised(hid_driver)) addDriver(hid_driver, protocol.no_device, false); + } + } } } else { status = -1; diff --git a/test/qemu_test.py b/test/qemu_test.py index 20761ad..bd30af4 100644 --- a/test/qemu_test.py +++ b/test/qemu_test.py @@ -281,12 +281,22 @@ CASES = [ "timeout": 60, "expect": r"acpi-parse: ok", "fail": r"acpi-parse: mismatch|DANOS-TEST-RESULT: FAIL"}, + # M20.3: the flip — ps2-bus now comes up from the acpi service's report, not + # a kernel-built node. Ordered: report -> spawn -> the driver attaches its + # keyboard, proving discovery runs entirely in ring 3 (docs/m19-m20-plan.md). + {"name": "acpi-ps2", + "smp": 4, + "timeout": 150, + "expect": r"acpi: reported PNP0303[\s\S]*" + r"device-manager: spawned ps2-bus[\s\S]*" + r"ps2-bus: keyboard driver attached", + "fail": r"DANOS-TEST-RESULT: FAIL"}, # M20.2: the acpi service evaluates _CRS/_STA in ring 3 and registers + # reports its _HID devices — the two PS/2 nodes must appear with resources # (keyboard: io 0x60/0x64 + IRQ = 3; mouse: IRQ = 1) (docs/m19-m20-plan.md). {"name": "acpi-report", "smp": 4, - "timeout": 60, + "timeout": 150, "qemu_extra": ["-device", "qemu-xhci,id=xhci", "-device", "usb-kbd,bus=xhci.0", "-device", "usb-mouse,bus=xhci.0"],