daniel/danos
1
0
Fork 0
Code Issues 4 Pull Requests Actions Packages Projects 1 Releases Wiki Activity

The flip: PCI enumeration leaves the kernel (M19.3)

Browse Source 
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.
This commit is contained in:
Daniel Samson
2026-07-13 02:54:50 +01:00
parent d26262bf56
commit af2c766f42
7 changed files with 135 additions and 185 deletions
Download Patch File Download Diff File Expand all files Collapse all files
system/kernel/process.zig
+22 -3
View File
@@ -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);
}
system/kernel/tests.zig
+51 -26
View File
@@ -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(&registered), 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();