From 81dd1e93184bf3249d25fb71d4855129232a15ff Mon Sep 17 00:00:00 2001 From: Daniel Samson <12231216+daniel-samson@users.noreply.github.com> Date: Sat, 8 Aug 2026 18:25:16 +0100 Subject: [PATCH] pci: the host bridge arrives by delegation too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pci-bus joins usb-xhci-bus in receiving its device from the manager rather than claiming the id it found in argv[1]. Its hello moves ahead of the ECAM mapping, since that is where the bridge now arrives, and its hello was already mandatory so nothing about its failure behaviour changes. isDelegated compared whole strings, which silently missed this driver: the manager records the boot-snapshot match as the bare "pci-bus" and a devices.csv match as the full "/system/drivers/pci-bus". pci-bus was then neither claiming nor delegated and died on "ECAM mmio_map failed". It now matches on the last path component. Reintroducing the whole-string compare breaks usb-xhci-bus instead of pci-bus — the two spellings swap which driver loses — so usb-hid is the case that catches it, not pci-scan. pci-scan asserts the delegation on the initial bring-up AND after the restart drill, with the device id backreferenced so both must name the same device. That is what proves the manager re-takes a device when its driver dies and hands it to the replacement, which is the property the whole supervision design rests on. The remaining three claimants are NOT converted, and the plan records why rather than working around it. ps2-bus and the acpi service never hello at all, which device-manager.md states deliberately ("legacy drivers ... not yet required to hello"), so delegating to them means either promoting them out of legacy or giving the grant a delivery point that is not hello. virtio-gpu hellos best-effort by design — "standalone bring-up has no manager" — and delegation would make it mandatory. Both are decisions, not mechanical steps. Consequence: D6 is blocked, because device_claim cannot be closed off while three claimants still depend on it. D7-D9 are unaffected — they concern what the kernel stores and how its table is sized. Suite 118/118. --- docs/bounds-track-plan.md | 29 ++++++++++++++++++- system/drivers/pci-bus/pci-bus.zig | 20 ++++++++----- .../device-manager/device-manager.zig | 11 +++++-- test/qemu_test.py | 9 ++++-- 4 files changed, 56 insertions(+), 13 deletions(-) diff --git a/docs/bounds-track-plan.md b/docs/bounds-track-plan.md index 8ab7143..25e8952 100644 --- a/docs/bounds-track-plan.md +++ b/docs/bounds-track-plan.md @@ -43,7 +43,7 @@ that cannot safely run in user space.** | D2 | Adversarial case: a process handed nothing is refused, on a held device and a free one | **done** — `device-authority-test`; the claim half joins it at D6 | | D3 | The manager claims the seeded devices at boot, before any driver is spawned | **merged into D4** — see below | | D4 | The manager claims + delegates on `hello`; `usb-xhci-bus` is the first driver converted | **done** — caught an IOMMU regression I introduced; see below | -| D5 | The other four claimants converted: `pci-bus`, `ps2-bus`, `virtio-gpu`, `acpi` | not started | +| D5 | The other four claimants converted: `pci-bus`, `ps2-bus`, `virtio-gpu`, `acpi` | **partial** — `pci-bus` done; the other three need decisions, see below | | D6 | `device_claim` refuses a device the caller was not handed; the hole is closed | not started | | D7 | Zero-resource devices stop being kernel objects — inventory moves to the manager | not started | | D8 | **`maximum_children_per_parent` deleted** — the authorisation it stood in for exists | not started | @@ -80,6 +80,33 @@ They land together, with the manager claiming only for drivers in an explicit anything in D4 — recorded rather than dismissed, because D4 moved the `hello` earlier and so did shift boot timing. Watch it across the remaining steps. +### Open questions raised by D5 — three of the four claimants cannot be converted yet + +Delegation is delivered in `onHello`. That works for a driver that says hello, and +**two of the four do not**. + +6. **`ps2-bus` and the `acpi` service never hello at all**, and that is deliberate: + device-manager.md records "Legacy drivers (e.g. ps2-bus) are supervised and + restarted but **not yet required to hello**", and the `Driver.speaks_protocol` flag + exists to say so. Delegating to them means either making them speak the protocol — + promoting them out of "legacy", which is a change to their documented status — or + giving the grant a second delivery point that is not `hello`. Neither is written + down. A second delivery point would also need its own answer to "when", since the + whole value of `hello` here is that it is the moment the driver is known to be alive + and is a synchronous point to hand something over. + +7. **`virtio-gpu` hellos, but best-effort by design.** Its call is + `_ = device_manager.hello(.device, device_id);` with the comment "Best-effort: + standalone bring-up has no manager." Delegation would make the hello *mandatory* and + move it to the front, so the driver could no longer come up without a manager. No + test exercises standalone today (the `virtio-gpu` case boots the manager stack, and + devices.csv matches it), so this is a documented intent rather than a live path — + but discarding a documented intent is a decision, not a mechanical step. + +Until these are answered, `device_claim` cannot be closed off at D6 for those three, so +**D6 is blocked on question 6 and 7**. D7–D9 are not: they concern what the kernel +stores and how the table is sized, and are independent of which drivers have converted. + ### Settled, so the run does not re-litigate them - **The manager claims, it is not granted.** No binary names in the kernel; the rule is diff --git a/system/drivers/pci-bus/pci-bus.zig b/system/drivers/pci-bus/pci-bus.zig index 5cd81cc..77af176 100644 --- a/system/drivers/pci-bus/pci-bus.zig +++ b/system/drivers/pci-bus/pci-bus.zig @@ -75,11 +75,20 @@ fn configWrite16(bus: u64, dev: u64, function: u64, offset: u64, value: u16) voi configWrite(bus, dev, function, aligned, (word & ~mask) | (@as(u32, value) << shift)); } -/// Claim the bridge, map the ECAM, hello the manager, then scan. +/// Hello the manager (which is where the bridge arrives), map the ECAM, then scan. fn initialise(endpoint: ipc.Handle) bool { _ = endpoint; - device.claim(bridge_id) catch |e| { - std.log.info("unable to claim bridge device {d}: {s}", .{ bridge_id, @errorName(e) }); + // **The handshake first, because it is where the device arrives.** This driver + // used to claim `bridge_id` here — first-come-first-served, so the manager's + // matching was advisory and any process could have claimed the bridge by naming + // the same id. The manager now holds it and transfers it in the hello reply + // (docs/os-development/device-authority.md). `hello` is synchronous, so the + // transfer has completed by the time this returns. + // + // Keep the manager handle to report children through; a supervised bus that + // cannot reach its manager has nothing to serve. + manager_handle = device_manager.hello(.bus, bridge_id) orelse { + std.log.info("no hello with the device manager; bridge {d} not delegated", .{bridge_id}); return false; }; const buffer = memory.allocator().alloc(device.DeviceDescriptor, 64) catch { @@ -113,11 +122,6 @@ fn initialise(endpoint: ipc.Handle) bool { return false; }; - // The handshake (role: bus — we enumerate PCI and report the functions we - // find), then the scan. Keep the manager handle to report children through; - // a supervised bus that cannot reach its manager has nothing to serve. - manager_handle = device_manager.hello(.bus, bridge_id) orelse return false; - scan(); return true; } diff --git a/system/services/device-manager/device-manager.zig b/system/services/device-manager/device-manager.zig index 92ec1e0..f2aff2a 100644 --- a/system/services/device-manager/device-manager.zig +++ b/system/services/device-manager/device-manager.zig @@ -246,12 +246,19 @@ fn alreadySupervised(name: []const u8) bool { /// `usb-xhci-bus` is first because it was the first driver to conform to `hello` /// (device-manager.md, M18.1), so it is the one whose handshake is best proven. const delegated_drivers = [_][]const u8{ - "/system/drivers/usb-xhci-bus", + "usb-xhci-bus", + "pci-bus", }; +/// Matched on the **last path component**, because a driver reaches this table under +/// two different spellings: the boot-snapshot match records the bare `pci-bus`, while a +/// devices.csv match records the full `/system/drivers/pci-bus`. Comparing whole +/// strings silently missed the bare form — pci-bus was left neither claiming nor +/// delegated, and died on `ECAM mmio_map failed`. fn isDelegated(name: []const u8) bool { + const leaf = if (std.mem.lastIndexOfScalar(u8, name, '/')) |slash| name[slash + 1 ..] else name; for (delegated_drivers) |candidate| { - if (std.mem.eql(u8, name, candidate)) return true; + if (std.mem.eql(u8, leaf, candidate)) return true; } return false; } diff --git a/test/qemu_test.py b/test/qemu_test.py index e4b3f13..708f373 100644 --- a/test/qemu_test.py +++ b/test/qemu_test.py @@ -854,10 +854,15 @@ CASES = [ {"name": "pci-scan", "smp": 4, "timeout": 60, - "expect": r"pci-bus: (\d+) functions found[\s\S]*" + # The bridge must arrive by DELEGATION, not by claiming — and again after the + # restart, which is what proves the manager re-takes the device when its driver + # dies and hands it to the replacement. \1 pins it to the same device both times. + "expect": r"device-manager: delegated device (\d+) to \S*pci-bus[\s\S]*" + r"pci-bus: (\d+) functions found[\s\S]*" r"device-manager: test mode: killing the reporter[\s\S]*" r"device-manager: restarting \S*pci-bus[\s\S]*" - r"pci-bus: \1 functions found[\s\S]*" + r"device-manager: delegated device \1 to \S*pci-bus[\s\S]*" + r"pci-bus: \2 functions found[\s\S]*" r"DANOS-TEST-RESULT: PASS", "fail": r"DANOS-TEST-RESULT: FAIL"}, # M18.3: the application surface — device-list enumerates the tree over IPC,