diff --git a/local/docs/IRQ-AND-LOWLEVEL-CONTROLLERS-ENHANCEMENT-PLAN.md b/local/docs/IRQ-AND-LOWLEVEL-CONTROLLERS-ENHANCEMENT-PLAN.md new file mode 100644 index 0000000000..505eaccd0a --- /dev/null +++ b/local/docs/IRQ-AND-LOWLEVEL-CONTROLLERS-ENHANCEMENT-PLAN.md @@ -0,0 +1,1022 @@ +# Red Bear OS IRQ and Low-Level Controllers Enhancement Plan + +> **Status note (2026-07-26):** despite the legacy folder name confusion, this file is still the +> active low-level-controller authority for IRQ/MSI/MSI-X/IOMMU runtime-proof sequencing. But its +> old `pcid-spawner` Wave 1 tasks are no longer live: `pcid-spawner` was retired/removed in the +> driver-manager cutover (v5.0, 2026-07-24), and those responsibilities now live in +> `local/docs/DRIVER-MANAGER.md` (current state) / `local/docs/archived/DRIVER-MANAGER-MIGRATION-PLAN.md` +> (round-by-round history). + +## Purpose + +This document assesses the current IRQ and low-level controller implementation in Red Bear OS for +completeness and quality, then defines the next enhancement plan in execution order. + +This is the **canonical current implementation plan** for PCI interrupt plumbing, IRQ delivery, +MSI/MSI-X quality, low-level controller runtime proof, and IOMMU/interrupt-remapping follow-up +work. + +When another document discusses PCI, IRQ, MSI/MSI-X, `pcid`, the historical `pcid-spawner` +surface, or low-level +controller execution order, prefer this file for: + +- the current robustness judgment, +- the current implementation order, +- the current validation/proof expectations, +- and the current language for build-visible vs runtime-proven vs hardware-validated claims. + +Other documents may still hold deeper architecture or donor-material detail, but they should point +here instead of acting as competing execution authorities. + +It is grounded in the current repository state, especially: + +- `local/recipes/drivers/redox-driver-sys/` +- `local/recipes/drivers/linux-kpi/` +- `local/recipes/gpu/redox-drm/` +- `local/recipes/system/iommu/` +- `local/sources/kernel/src/acpi/` +- `local/sources/base/drivers/acpid/` +- `local/docs/IOMMU-SPEC-REFERENCE.md` +- `local/docs/ACPI-IMPROVEMENT-PLAN.md` +- `docs/04-LINUX-DRIVER-COMPAT.md` + +The goal is not to restate that these pieces compile, but to separate: + +- what exists architecturally, +- what is only build-validated, +- what is runtime-validated, +- and what still needs focused enhancement work. + +## Evidence Model + +This plan uses four different evidence buckets and does **not** treat them as equivalent: + +- **Checked-in source** — what is visible directly in the current source tree. +- **Local patch state** — behavior carried by `local/patches/*` that may not be visible in the + unpacked upstream source snapshot until patches are applied. +- **Build-validated** — code or recipes compile successfully. +- **Runtime-validated** — behavior has been exercised in a real boot/runtime path. + +Where a statement depends on local patches instead of the visible source snapshot, that is called +out explicitly below. + +## Controller Inventory and Ownership + +| Area | Primary owner | Main entry points | Current evidence class | +|---|---|---|---| +| LAPIC / xAPIC / x2APIC | kernel | `local/sources/kernel/src/acpi/madt/`, `arch/x86_shared/device/local_apic.rs` | source + local patch + boot/runtime evidence | +| IOAPIC / IRQ overrides | kernel | `local/sources/kernel/src/arch/x86_shared/device/ioapic.rs`, MADT ISO parsing | source | +| Legacy PIC | kernel | `arch/x86_shared/device/pic.rs` | source | +| ACPI power/reset methods | userspace `acpid` | `local/sources/base/drivers/acpid/src/acpi.rs` plus local base patch | source + local patch + runtime evidence | +| HPET / timer tables | kernel | `local/sources/kernel/src/acpi/hpet.rs` | source | +| PIT fallback timer | kernel | `local/sources/kernel/src/arch/x86_shared/device/mod.rs`, `pit.rs` | source | +| PCI interrupt plumbing | userspace `pcid` / driver layer | `local/sources/base/drivers/pci/`, `scheme:irq`, `scheme:pci` | source + runtime evidence | +| Driver IRQ abstraction | `redox-driver-sys` | `local/recipes/drivers/redox-driver-sys/source/src/irq.rs` | source | +| Linux IRQ compatibility | `linux-kpi` | `local/recipes/drivers/linux-kpi/source/` headers | source | +| GPU MSI/MSI-X usage | `redox-drm` | `local/recipes/gpu/redox-drm/source/` | source + build evidence | +| IOMMU / interrupt remapping | `iommu` daemon | `local/recipes/system/iommu/source/src/main.rs`, `local/docs/IOMMU-SPEC-REFERENCE.md` | source + build evidence | +| Kernel serio / PS2 path | kernel `serio` + userspace `ps2d` | `local/sources/kernel/src/scheme/serio.rs`, `local/sources/base/drivers/input/ps2d/src/main.rs` | source | +| Input controller path | `inputd` / `evdevd` / `udev-shim` | base driver + local system recipes | source + runtime evidence | +| USB xHCI host controller | userspace `xhcid` | `local/sources/base/drivers/usb/xhcid/src/main.rs` | source + build evidence + quirk-hardened | +| Port I/O / legacy controller access | kernel + `redox-driver-sys` | `iopl`, `io.rs`, legacy driver code | source | +| Legacy IRQ dispatch / ownership map | kernel | `local/sources/kernel/src/arch/x86_shared/interrupt/irq.rs` | source | + +## Current State Summary + +### What is already in place + +Red Bear OS already has a meaningful low-level controller and interrupt foundation: + +- ACPI boot, FADT power control, visible MADT parsing for LAPIC/IOAPIC/interrupt overrides, and + HPET initialization are in place in the checked-in source. +- Additional MADT x2APIC / NMI / power-method handling exists in the local patch set and in prior + runtime validation notes, but that behavior should not be conflated with the unpatched source + snapshot. +- `redox-driver-sys` provides userspace driver primitives for MMIO, DMA, PCI access, IRQ handles, + MSI-X table mapping, and IRQ affinity control. +- `linux-kpi` exposes Linux-style IRQ, PCI, memory, and synchronization APIs on top of + `redox-driver-sys`. +- `redox-driver-sys` now has direct host-runnable unit coverage for pure PCI/IRQ substrate rules, + including PCI scheme-entry parsing bounds, I/O BAR port conversion safety, and MSI-X BAR window + helper validation. This should be treated as **source + host-test evidence**, not as runtime + controller proof. +- `redox-driver-sys` fast PCI enumeration now preserves capability-chain data from config-space + bytes instead of returning empty capability lists, and exposes a quirk-aware interrupt-support + summary (`none` / `legacy` / `msi` / `msix`) for downstream policy convergence. +- `redox-drm` already contains a shared interrupt abstraction with MSI-X-first and legacy-IRQ + fallback paths for GPU drivers. +- The AMD-Vi / Intel VT-d reference material and the in-tree `iommu` daemon establish a serious + implementation direction for IOMMU and interrupt-remapping work. +- the repo now has a bounded timer proof path via `redbear-phase-timer-check` and + `local/scripts/test-timer-qemu.sh --check`, which verifies the monotonic time scheme is present + and advances across two reads inside a guest runtime +- the bounded low-level controller proof hooks can now be run together through + `local/scripts/test-lowlevel-controllers-qemu.sh`, which sequences xHCI, IOMMU, PS/2, and timer + runtime checks on the desktop validation image + +### What is still weak + +The dominant weakness is not missing abstractions. It is missing runtime proof and uneven +controller-specific validation. + +- MSI-X support exists architecturally but is still weak on hardware validation. +- IOMMU support is specification-rich and code-rich, but still unvalidated on real hardware. +- IRQ routing quality-of-service remains primitive: raw wait handles exist, but balancing, + coalescing, and validation of affinity behavior remain thin. +- Input stacks (`inputd`, `evdevd`, `udev-shim`) now exist as a runtime substrate, but the exact + end-to-end interrupt-to-consumer path still needs sustained validation discipline. +- Low-level controller quality is uneven: ACPI/APIC are much further along than IOMMU, MSI-X, and + controller-specific runtime characterization. + +## Current Robustness Assessment + +### Bottom line + +The PCI/IRQ stack is now **architecturally credible and usable for bounded bring-up plus QEMU/runtime +proof**, but it is **not yet release-grade robust end to end**. + +The strongest layers are: + +- the kernel IRQ substrate, +- the `scheme:irq` delivery model, +- and the shared `redox-driver-sys` PCI/IRQ helper layer. + +The weakest layers are: + +- `pcid` and the historical pre-cutover orchestration layer, +- the `pcid` driver-interface helper surface, +- the shared `virtio-core` MSI-X setup path, +- and several upstream-owned base drivers that still panic on missing BARs, missing interrupt + handles, impossible feature states, or scheme-operation failures. + +### What is materially strong today + +- Kernel IRQ ownership is real and active: PIC, IOAPIC, LAPIC/x2APIC, IDT reservation, masking, + EOI, and spurious IRQ accounting all exist in the checked-in kernel. +- **Kernel MSI/MSI-X support is now implemented**: MSI message composition, validation, vector + allocation, and IRQ affinity control exist in `P8-msi.patch` (msi.rs, vector.rs, scheme/irq.rs). + The `iommu_validate_msi_irq` hook is wired into `irq_trigger` as a validation gate. +- `redox-driver-sys` is the strongest PCI/IRQ userspace substrate: typed BAR parsing, quirk-aware + interrupt-support reporting, IRQ handle abstractions, MSI-X table helpers, affinity helpers, and + direct host-runnable substrate tests all exist. +- `redox-drm` consumes the interrupt substrate honestly with MSI-X → MSI → legacy fallback and + quirk-aware downgrade policy. +- `iommu` and the low-level proof scripts provide bounded runtime evidence rather than pretending + broader hardware support exists. + +### What is still fragile today + +- `pcid` and the historical pre-cutover orchestration layer assumed a trusted environment too + often: launch sequencing, + device-enable timing, and several error paths are weaker than the substrate beneath them. +- `pcid` helper files such as `driver_interface/{bar,cap,irq_helpers,msi}.rs` still treat several + malformed-device or unsupported-state cases as invariants rather than recoverable failures. +- `virtio-core` still hard-requires MSI-X in its active x86 path and uses assert/expect/ + unreachable semantics for feature/capability assumptions that are acceptable for bounded proof, + but weak for a general PCI substrate. +- A broad set of shipped consumers (`rtl8168d`, `rtl8139d`, `ixgbed`, `ac97d`, `ihdad`, `ided`, + `vboxd`, `virtio-blkd`, `virtio-gpud`, `ihdgd`, and others) still encode panic-grade startup + assumptions around BARs, IRQs, or scheme operations. + +### Validation truth + +- MSI-X and IOMMU now have **bounded QEMU/runtime proof**. +- xHCI interrupt mode also has bounded QEMU proof. +- That is enough to justify further implementation work and proof tooling. +- It is **not** enough to justify broad hardware robustness claims for PCI/IRQ handling. + +### Runtime proof status (2026-07-20) + +- MSI-X (virtio-net) and xHCI interrupt-driven mode were **re-confirmed** on the current + `redbear-mini` ISO via the log-grep legs (`test-msix-qemu.sh`, `test-xhci-irq-qemu.sh --check`). +- The login-based proofs (PS/2 + serio, monotonic timer, IOMMU first-use, USB storage BOT) + were **migrated off the host `expect` tool** to `local/scripts/qemu-login-expect.py`, a + standard-library-only python driver (subprocess + select) that walks ordered `expect:`/`send:` + steps over the guest serial console, applies per-step timeouts (matching expect's per-pattern + semantics), checks pass/fail markers, and distinguishes premature QEMU death (exit 3) from a + genuine check failure (exit 1) so the scripts retry external interruptions without retrying real + failures. The four scripts (`test-ps2-qemu.sh`, `test-timer-qemu.sh`, `test-iommu-qemu.sh`, + `test-usb-storage-qemu.sh`) now run headless (`-display none -vga none`, using the fbcond serial + mirror) with a 3600s per-step timeout and a bounded retry loop. +- Re-confirmation of those login-based legs with the new harness is **pending an uncontended + host**: on the current build host a parallel QEMU-testing workload repeatedly kills QEMU + processes and drives multi-minute boot stalls (guest time dilation, full swap). Boots reach the + getty stage, but the login prompt is not observed within the timeout under that load. The + harness behavior is correct — it waits for `login:` exactly as the prior expect scripts did — + and the underlying proofs were previously validated (see the validation matrix). +- **New finding (graphics path, not low-level controller):** with a display device present + (`-vga std`), the fbcond display handoff can freeze the boot at `Performing handoff` — the + `reopen_for_handoff()` path (inputd `v2/` scheme proxy / vesad) can block instead of returning + `EAGAIN` for the intended retry. Headless (`-vga none`) boots do not take that path and are + unaffected. Tracked as a separate desktop/graphics-stack issue. + +## Current Authority Split + +For PCI/IRQ planning and current-state language, use the repo doc set this way: + +- **This file** — canonical implementation plan and current robustness judgment for PCI/IRQ and + low-level controllers. +- `local/docs/LINUX-BORROWING-RUST-IMPLEMENTATION-PLAN.md` — donor-material and Rust-rewrite + policy only; not the execution authority for PCI/IRQ rollout. +- `local/docs/IOMMU-SPEC-REFERENCE.md` — specification/reference detail for AMD-Vi / VT-d. +- `local/docs/QUIRKS-SYSTEM.md` and `local/docs/QUIRKS-IMPROVEMENT-PLAN.md` — quirk-policy source + of truth and forward convergence work. +- `README.md`, `docs/README.md`, `AGENTS.md`, and `local/AGENTS.md` — public/current-state summary + surfaces that should point here rather than restating competing PCI/IRQ execution plans. + +## Architectural Assessment + +### 1. IRQ delivery architecture + +The project’s IRQ delivery model is fundamentally sound. + +- Kernel/platform side routes interrupts through APIC/x2APIC infrastructure. +- Userspace consumes interrupts through `scheme:irq` handles. +- MSI-X vector allocation is already modeled per CPU via the IRQ scheme. + +This is the right design for Red Bear OS. The main enhancement need is validation and quality, not +an architectural rewrite. + +### 2. PCI and MSI/MSI-X + +The PCI and MSI-X model is one of the strongest parts of the current stack. + +- Config-space access exists. +- Capability parsing exists. +- MSI-X table mapping exists. +- GPU drivers already use the abstraction. + +The gap is that the repository still talks too often in “compiles” language instead of “validated on +hardware with real interrupts firing” language. + +Current runtime-proof entrypoint now present in-tree: + +- `local/scripts/test-msix-qemu.sh` — QEMU/UEFI boot path that verifies live `virtio-net` + initialization reporting `virtio: using MSI-X` + +### 3. IOMMU and interrupt remapping + +IOMMU was the most important low-level controller area that was still incomplete in practice. + +- The implementation direction is correct. +- The data structures and register model are already documented deeply. +- But the hardware-validation story is still effectively open, and current daemon discovery is still + only partially integrated: the daemon now searches common IVRS table locations automatically, but + full platform-native discovery and hardware validation are still open. +- The current QEMU path now reaches AMD-Vi unit detection and `scheme:iommu` registration without + crashing at daemon startup. +- The current guest-driven first-use proof now completes successfully in QEMU: it reaches readable + and writable AMD-Vi MMIO, initializes both discovered units, and drains the event path without + faulting. +- The critical blockers that previously stopped this path were fixed in the shared mapping layers, + not just in the userspace `iommu` daemon: physically contiguous DMA allocations now preserve + writable mappings, and MMIO mappings now use the explicit `fmap` path with the intended protection + bits. + +This leaves IOMMU as a hardware-validation and deeper interrupt-remapping quality area, rather than +as an immediate QEMU runtime blocker. + +### 4. Input/controller path + +The input/controller path is no longer missing. It is now a quality and observability problem. + +- `inputd` exists. +- `evdevd` exists. +- `udev-shim` exists. +- Phase 3 validation helpers exist. + +The enhancement task is to keep turning these from “service present” into “interrupt path proven,” +especially under real runtime scenarios. + +## Completeness Assessment by Area + +### ACPI / APIC / x2APIC + +**State**: materially complete for the historical boot-baseline bring-up goals, but not release-grade complete. + +**Important source note**: the checked-in MADT parser in +`local/sources/kernel/src/acpi/madt/mod.rs` visibly handles `LocalApic`, `IoApic`, + `IntSrcOverride`, `Gicc`, and `Gicd`. Additional x2APIC/NMI support referenced elsewhere in the + repo is currently evidenced through the local patch set and prior validation notes rather than the + plain source snapshot alone. + +Strengths: + +- MADT entries for xAPIC/x2APIC/NMI are handled. +- ACPI reboot/shutdown/power methods are implemented, but robustness, sleep-state scope beyond + `\_S5`, and bounded validation still remain open as tracked in + `local/docs/ACPI-IMPROVEMENT-PLAN.md`. +- x2APIC and SMP platform bring-up have already crossed the foundational threshold. + +Open enhancement items: + +- Better controller/runtime characterization on diverse hardware. +- Clearer documentation for what is kernel-complete versus only tested on limited platforms. +- Keep sleep-state support beyond `\_S5`, DMAR ownership cleanup, and bounded validation visible as open ACPI work rather than implying subsystem closure. + +### IOAPIC / interrupt source override routing + +**State**: present in ACPI parsing, but less explicitly validated than LAPIC/x2APIC paths. + +Concrete checked-in owner: + +- `local/sources/kernel/src/arch/x86_shared/device/ioapic.rs` +- `local/sources/kernel/src/acpi/madt/mod.rs` + +Open enhancement items: + +- explicit validation of interrupt source overrides on more real machines +- repo-visible test notes for IOAPIC routing behavior + +### HPET / timer controller surface + +**State**: present, but still thinly characterized. + +Concrete checked-in owner: + +- `local/sources/kernel/src/acpi/hpet.rs` + +Open enhancement items: + +- runtime verification beyond “initialized from ACPI” +- clearer single-HPET limitation documentation + +### PIT fallback timer path + +**State**: explicit checked-in fallback controller path. + +Concrete checked-in owner: + +- `local/sources/kernel/src/arch/x86_shared/device/mod.rs` +- `local/sources/kernel/src/arch/x86_shared/device/pit.rs` + +Current behavior: + +- the kernel prefers HPET when available +- if HPET initialization fails or is unavailable, it falls back to PIT +- PIT interrupt ticks currently drive timeout and scheduler timing paths + +Open enhancement items: + +- document runtime characterization of PIT-only boots +- clarify timer-source selection evidence in validation notes + +### PCI interrupt plumbing / MSI / MSI-X + +**State**: architecturally strong, validation-incomplete. + +Open enhancement items: + +- real hardware MSI-X proof for AMD and Intel GPU paths +- controller-level observability for vector allocation and affinity behavior +- testable records of fallback behavior between MSI-X and legacy IRQs + +Current runtime-validation surface now present in-tree: + +- `local/scripts/test-msix-qemu.sh` — boots a Red Bear image and confirms a live MSI-X path via + `virtio-net` log evidence in QEMU + +### IOMMU / interrupt remapping + +**State**: QEMU runtime proof present; broader hardware validation still open. + +Concrete checked-in owner: + +- `local/recipes/system/iommu/source/src/main.rs` +- `local/docs/IOMMU-SPEC-REFERENCE.md` + +Open enhancement items: + +- real AMD-Vi initialization validation +- event log and fault-path validation +- interrupt remapping validation under device load +- explicit distinction between “daemon builds” and “controller works” +- replacement of `IOMMU_IVRS_PATH`-only discovery with real system discovery/integration + +Current implementation improvement: + +- the daemon no longer depends only on `IOMMU_IVRS_PATH`; it now searches common IVRS table paths + automatically before falling back to the environment variable override +- daemon startup now defers AMD-Vi unit initialization until first scheme use, which keeps the + QEMU validation path alive long enough to prove detection plus `scheme:iommu` registration +- a guest-driven self-test path now exists (`/usr/bin/iommu --self-test-init` via + `redbear-phase-iommu-check` / `test-iommu-qemu.sh`) and now proves first-use unit initialization + and event-drain completion in QEMU +- the self-test output now includes structured discovery diagnostics (`discovery_source`, + `kernel_acpi_status`, `ivrs_path`) so zero-unit failures can be distinguished from kernel-ACPI + fallback and missing-IVRS cases without changing the IOMMU MMIO path itself + +### Legacy IRQ ownership and dispatch map + +**State**: explicit checked-in kernel ownership exists, but it is under-documented in higher-level +controller discussions. + +Concrete checked-in owner: + +- `local/sources/kernel/src/arch/x86_shared/interrupt/irq.rs` + +Current covered paths include: + +- PIT timer interrupt handling +- keyboard and mouse interrupt delivery +- serial COM1/COM2 delivery +- PIC/APIC mask, acknowledge, and EOI behavior +- spurious IRQ accounting for IRQ7 and IRQ15 + +Open enhancement items: + +- document legacy IRQ ownership and routing expectations explicitly in validation notes +- record PIC-vs-APIC runtime behavior on more hardware classes + +### Kernel `serio` / PS2 controller path + +**State**: present and important, but easy to miss if input work is described only in terms of the +later `evdevd`/`udev-shim` stack. + +Concrete checked-in owner: + +- `local/sources/kernel/src/scheme/serio.rs` +- `local/sources/base/drivers/input/ps2d/src/main.rs` + +Current behavior: + +- the kernel owns the serio byte queues to avoid PS/2 controller races +- `ps2d` consumes `/scheme/serio/0` and `/scheme/serio/1` +- that path then feeds the broader input producer chain + +Open enhancement items: + +- keep validation language explicit about the PS/2 path versus the later generic input stack +- add platform notes for systems that still rely on PS/2 keyboard/mouse delivery +- the repo now has a bounded PS/2 runtime-proof path via `redbear-phase-ps2-check` and + `local/scripts/test-ps2-qemu.sh --check`, which proves serio node presence and a successful + handoff into the existing Phase 3 input-path checker inside a guest +- `ps2d` controller init now also drains stale controller output during probe and around the core + init/self-test path, which is the current bounded Red Bear-native equivalent of Linux i8042 flush + discipline before broader PS/2 suspend/resume work exists. + +### USB xHCI controller interrupt path + +**State**: interrupt-driven and quirk-hardened (2026-07-18); not yet hardware-validated. + +Concrete checked-in owner: + +- `local/sources/base/drivers/usb/xhcid/src/main.rs` + +Current behavior: + +- quirks are resolved **before** interrupt-method selection: the PCI vendor/device/revision and + the real HCIVERSION (read from MMIO offset 0x04, mirroring Linux `xhci_gen_setup()` + xhci.c:5455) feed the canonical quirk lookup in `redox-driver-sys` +- the `BROKEN_MSI` quirk skips MSI/MSI-X probing **entirely inside `get_int_method()`**, so the + returned interrupt handle and the delivery method always agree — the earlier handle/method + mismatch class (MSI handle with INTx label) is eliminated +- MSI/MSI-X/legacy INTx/polling selection is logged per controller, plus the full quirk bitmask + (`XHCI quirks: vendor=… device=… rev=… hci_ver=… -> 0x…`) +- `local/scripts/test-xhci-irq-qemu.sh --check` provides a repo-visible runtime proof path by + booting a Red Bear image in QEMU and checking the xHCI interrupt-mode log output +- `redox-driver-sys` logs allocated MSI-X vectors so interrupt selection is observable in + runtime logs + +Related work owned by the USB plan (not duplicated here): the 51-flag canonical xHCI quirk +table (P2-A) and HCCPARAMS2 capability gating (P2-B) — see +`local/docs/USB-IMPLEMENTATION-PLAN.md` § P2. This section covers only interrupt-path quality. + +Open enhancement items: + +- validate BROKEN_MSI pre-gate correctness on a controller that actually carries the quirk + (requires matching hardware or a mock) +- validate event-ring behavior under sustained runtime/device activity (growth path, wrap + handling, re-entrancy with the now-guaranteed handle/method agreement) +- re-run `test-xhci-irq-qemu.sh --check` after P2-A/P2-B to confirm the hardened path + still passes + +### Port I/O / legacy controller support + +**State**: exists, but under-characterized. + +Concrete current consumers/owners include: + +- legacy PIC handling in `local/sources/kernel/src/arch/x86_shared/device/pic.rs` +- port-I/O wrappers in `local/recipes/drivers/redox-driver-sys/source/src/io.rs` +- ACPI reset fallback via keyboard-controller port writes in the base/acpid patch path documented in + `local/docs/ACPI-IMPROVEMENT-PLAN.md` + +Open enhancement items: + +- determine which real devices still need the port-I/O path +- validate that the current wrappers are sufficient for those devices + +## Quality Assessment + +### Strong points + +- The layering is correct: kernel/platform routing below, userspace schemes and driver wrappers + above. +- The repository already has serious implementation artifacts, not just speculative plans. +- The low-level controller work is documented more deeply than many higher-level desktop areas. +- ACPI and early-platform work are significantly more mature than the rest of the low-level stack, but that maturity is still best read as boot-baseline progress with bounded validation rather than subsystem-complete closure. + +### Weak points + +- Validation language is still inconsistent across docs. “builds” and “validated” are too often + treated as adjacent states when they are not. +- IOMMU progress is easy to overread because the spec reference is detailed, but the runtime proof + and discovery story are not there yet. +- Some controller areas are rich in abstractions but poor in operator-facing validation procedures. +- Hardware-controller quality is still under-documented in terms of negative results and known + failure modes. +- Earlier summaries in the repo can blur checked-in source, local patches, and validated runtime + behavior; this document should be used to keep those categories separate. +- Broad category labels can hide concrete controller owners unless PIT, `serio`/PS2, legacy IRQ + dispatch, and xHCI are named explicitly. + +## Enhancement Priorities + +## Priority 1 — MSI-X runtime validation on real devices + +Goal: move MSI-X from “implemented abstraction” to “repeatedly proven behavior.” + +Deliverables: + +- explicit AMD GPU MSI-X validation notes +- explicit Intel GPU MSI-X validation notes +- verified fallback behavior to legacy IRQs when MSI-X is unavailable +- logged CPU/vector affinity behavior in real runs + +Why first: + +This is the lowest-level controller feature that already exists in the main runtime driver path and +blocks confidence in GPU/display work above it. + +## Priority 2 — IOMMU hardware bring-up and fault-path validation + +Goal: move IOMMU from spec-driven implementation to actual controller bring-up. + +Deliverables: + +- validated AMD-Vi daemon initialization on real hardware +- device table / command buffer / event log validation +- explicit interrupt-remapping validation notes +- negative-result documentation if hardware still fails + +Why second: + +It is the largest remaining low-level completeness gap, and it affects the safety and correctness of +userspace driver DMA. + +## Priority 3 — IRQ quality-of-service and observability + +Goal: make IRQ behavior easier to reason about in production. + +Deliverables: + +- better logging/telemetry around allocated IRQs and vectors +- explicit affinity-validation procedures +- measured notes on whether current userspace IRQ wait behavior is good enough for display/input + latency needs + +Why third: + +This improves reliability without changing the underlying architecture. + +### Shared-IRQ re-arm bug class — FIXED (2026-07-20) + +A systematic driver bug class was found and fixed across the base driver tree: +the kernel masks the IRQ line when it delivers an interrupt and only re-arms it +on the userspace write-back (irq scheme `write` → `acknowledge()` → +`pic_unmask()`/`ioapic_unmask()`). Drivers that acked only when the interrupt +was their own wedged the line permanently on the first foreign interrupt on a +shared INTx line — every later interrupt for the device was silently lost. + +This is exactly the "works in QEMU (dedicated MSI/INTx), wedges on real +hardware (shared INTx)" failure signature that P1/P6 hardware validation was +expected to hunt. + +Fixed (always ack, tick/process only when ours, device-level status cleared +before re-arm so no interrupt storm): + +- `ahcid` (`ad40fffd`, from the UAS workstream) +- `e1000d`, `ihdad`, `vboxd`, `xhcid` (`92924224`) +- `sb16d`, `ac97d`, `rtl8139d`, `rtl8168d`, `ixgbed`, `ihdgd` (`28afc1fe`) + +Audited, NOT affected: + +- `ided` — already acked unconditionally +- `nvmed` — the shared `executor` crate acks unconditionally for INTx; MSI + vectors are never shared so every delivery is ours by construction +- `virtio-core` consumers — MSI-X only, same reasoning +- `ps2d` — does not use the irq scheme fd path + +Both fix commits verified with `cargo check -Z build-std --target +x86_64-unknown-redox` for every touched crate — clean, no new warnings. + +## Priority 4 — input/controller runtime proof + +Goal: continue turning the existing input substrate into a well-proven low-level controller path. + +Deliverables: + +- sustained validation of `inputd` → `evdevd` → consumer path +- documentation of real interrupt-backed input evidence, not only service existence +- explicit known limitations for consumer nodes and path expectations + +> **See also:** `local/docs/boot-logs/REDBEAR-MINI-BOOT-PS2D-INPUTD-LOG-FIX.md` — +> the 2026-06-30 fix that added `[@inputd: INFO] inputd: scheme:input +> registered, waiting for handles` and `[@ps2d: INFO] ps2d: registered +> producer handle, listening on serio/0 (keyboard) and serio/1 (mouse)` +> to the boot log. Makes the input stack visible in the boot log so the +> "input appears dead" diagnosis class no longer requires a full QEMU +> reproducer to disambiguate. + +Why fourth: + +The architecture is there. What remains is proof quality. + +## Priority 5 — timer/controller characterization + +Goal: reduce uncertainty around HPET/APIC-timer behavior and controller assumptions. + +Deliverables: + +- a compact validation note for HPET behavior on real hardware +- notes on timer-controller assumptions and known limits + +Why fifth: + +Important, but less immediately blocking than MSI-X and IOMMU. + +## Priority 6 — xHCI interrupt-path runtime validation + +This is Priority 6 **within the low-level controller plan itself**, not within the repository-wide +subsystem order. At the repo-wide level, low-level controller quality remains ahead of USB/Wi-Fi/ +Bluetooth because these later subsystems depend on the controller/runtime proof work documented +here. + +Goal: validate the now-hardened xHCI interrupt path beyond the current narrow QEMU proof. + +Status (2026-07-18): the interrupt path is no longer merely "restored" — P2-A resolved quirks +before interrupt-method selection (BROKEN_MSI pre-gate, handle/method agreement guaranteed) and +P2-B gated xHCI 1.1+ features on real capability registers. The remaining gap is **validation**, +not implementation. + +Deliverables: + +- validate MSI/MSI-X or INTx behavior for xHCI on real hardware and/or QEMU, including at least + one BROKEN_MSI-flagged controller or an injected-quirk mock +- validate event-ring behavior under sustained IRQ load (growth, wrap, multiple interrupters) +- re-run `test-xhci-irq-qemu.sh --check` after each xHCI interrupt-path change + +Why sixth: + +This remains a real completeness gap in an important low-level controller, but it is now narrower +in scope than the cross-cutting MSI-X and IOMMU priorities above because the interrupt path itself +is hardened and QEMU-proven; only broader runtime/hardware validation remains open. + +## Execution Plan + +## Detailed Implementation Plan + +The remaining work should be executed in six waves. The order matters: shared control-plane and +helper hardening comes before broad driver cleanup, and runtime-proof/observability comes before any + stronger public claim language. + +### Wave 1 — Driver-launch orchestration hardening + +**Status**: ⚠️ Partially complete; `pcid-spawner` tasks obsoleted by driver-manager cutover + +**2026-07-26 note:** The original Wave 1 mixed two surfaces: `pcid` hardening and +`pcid-spawner`-specific orchestration. The `pcid-spawner` half is now superseded by the +driver-manager cutover (v5.0, 2026-07-24). Treat this wave as: + +- **completed historical work** for `pcid` daemon hardening, +- **obsoleted historical work** for `pcid-spawner`, and +- **redistributed remaining orchestration work** to `local/docs/DRIVER-MANAGER.md` (current state) / + `local/docs/archived/DRIVER-MANAGER-MIGRATION-PLAN.md` (round-by-round history). + +**Primary targets** + +- `local/sources/base/drivers/pcid/src/{main,scheme,driver_handler}.rs` +- historical only: removed `local/sources/base/drivers/pcid-spawner/src/main.rs` + +**Implemented** + +- replaced all `expect()`/`unwrap()` in pcid daemon startup with `process::exit(1)` + log messages +- ACPI registration failures are now non-fatal: logged and continued without ACPI +- PCI header parse failures (`from_header`) now log and skip the device instead of panicking +- I/O BAR port overflow now logs and skips the BAR instead of panicking +- MSI message data overflow returns `InvalidBitPattern` error response instead of panicking +- historical `pcid-spawner` logging work is preserved only in git history; the live spawn/logging + surface now belongs to driver-manager + +**Acceptance** + +- ✅ no normal launch/config mismatch path depends on `expect`/`unreachable!` in main.rs/driver_handler.rs +- ✅ failed driver launch is bounded and observable +- ✅ enable-before-spawn behavior was made explicitly logged on the pre-cutover path; the live + equivalent is now tracked under driver-manager + +**Verification** + +- `cargo test -p pcid` +- historical only: `cargo test -p pcid-spawner` belonged to the removed pre-cutover surface +- verify `redbear-info` still reports the expected PCI surfaces after boot + +### Wave 2 — Fix `pcid` helper contract + +**Status**: ✅ Complete + +**Implemented** + +- irq_helpers.rs: all 9 panic sites converted to io::Error returns; convenience wrappers use log::error + process::exit(1) +- mod.rs: send/recv rewritten to delegate to send_result/recv_result; connect_by_path uses proper error conversion; try_map_bar returns error on null BAR pointer +- bar.rs: 2 unwrap_or_else(panic) → proper error returns +- cap.rs: vendor capability parse failures return errors +- msi.rs: MSI-X BAR mapping null pointer returns MsixMapError::NullPointer +- config.rs: vendor ID parse uses proper error handling + +- `local/sources/base/drivers/pcid/src/driver_interface/{bar,cap,config,irq_helpers,msi,mod}.rs` + +**Implement** + +- replace panic-style BAR/capability helpers with typed error-returning variants +- treat malformed vendor capabilities as device faults, not invariants +- make IRQ allocation and MSI/MSI-X selection explicit return values +- keep any `expect_*` helpers as thin wrappers only where absolutely necessary + +**Acceptance** + +- helper layer is error-returning by default +- malformed BAR/capability state no longer aborts bring-up by default +- vector selection and failure reasons are reportable state, not only implicit side effects + +**Verification** + +- `cargo test -p pcid` +- add unit tests for malformed BARs, malformed vendor caps, and IRQ-allocation failure behavior + +### Wave 3 — Harden shared `virtio-core` IRQ/MSI-X setup + +**Status**: ✅ Complete (all panic-grade sites converted) + +**Implemented** + +- `spawn_irq_thread`: RawEventQueue creation, event subscription, and event iteration all handle errors gracefully with log::error + thread return instead of unwrap/panic +- MSI-X vector acceptance check in `setup_queue`: converted from assert to log::error + Err return +- MSI-X vector check in `reinit_queue`: converted from assert to log::error +- FEATURES_OK validation in `accept_driver_features`: converted from assert to log::error +- split_virtqueue.rs `ChainBuilder::build`: handles empty chain without panicking +- Mutex lock unwraps retained as idiomatic (mutex poisoning in single-threaded driver is unrecoverable) +- `probe.rs`: vendor-ID assert → `Error::Probe("not a virtio device")`, `expect_mem()` → `try_mem()` with proper error, three capability `.expect()` → `.ok_or(Error::InCapable(...))`, `unreachable!()` → `continue`, zero-multiplier assert → graceful error, MSI-X assert → graceful error +- `arch/x86.rs`: MSI-X feature-info `unreachable!()` → `Error::Probe(...)`, `read_bsp_apic_id().expect()` → `.map_err()` with log::warn +- Added `Error::Probe(&'static str)` variant to `transport::Error` for probe-stage failures +- Removed `## Panics` doc section from `probe_device()` + +**Primary targets** + +- `local/sources/base/drivers/virtio-core/src/{probe,transport,arch/x86}.rs` + +**Acceptance** + +- partial or malformed virtio devices fail probe cleanly +- MSI-X setup failure is a bounded bring-up error instead of a crash path +- all 3 downstream consumers (virtio-blkd, virtio-netd, virtio-gpud) compile cleanly against the hardened virtio-core + +**Verification** + +> ⚠️ **GROSS WARNING — DO NOT run `repo cook`, `repo fetch`, or `make live` directly.** +> These bypass the canonical build pipeline (`apply-patches.sh` patch-linking + staleness handling + correct dependency ordering), which causes broken/missing patches and wasted rebuild time. **ALWAYS build via `./local/scripts/build-redbear.sh [--upstream] `** (or the documented `make` targets it drives). If you think you need a single-recipe cook, run the canonical wrapper — it does the right thing and is faster in the end. + +- build passes: `CI=1 make r.base CONFIG_NAME=redbear-mini ARCH=x86_64` +- downstream consumers compile without errors + +### Wave 4 — Convert highest-risk consumers + +**Status**: ✅ Complete (all consumer drivers hardened) + +**Status**: ✅ Complete (all consumer drivers hardened) + +**Current state**: All panic-grade `.expect()`/`.unwrap()`/`assert!()`/`unreachable!()`/`panic!()` +calls have been removed from every consumer driver and converted to graceful error handling. +Additionally, `cfg_access/mod.rs` and `cfg_access/fallback.rs` in pcid were hardened (21+ unwrap +sites converted on the PCIe ECAM/DTB/MCFG startup path). Only Mutex `.lock().unwrap()` calls remain +(idiomatic for single-threaded drivers where poisoning is unrecoverable). + +**Hardened drivers** + +- `drivers/net/e1000d/` — all panic-grade calls converted +- `drivers/net/rtl8168d/` — all panic-grade calls converted +- `drivers/net/rtl8139d/` — all panic-grade calls converted +- `drivers/net/ixgbed/` — all panic-grade calls converted (+ missing `log` dep added to Cargo.toml) +- `drivers/storage/ahcid/` — all panic-grade calls converted +- `drivers/storage/nvmed/` — all panic-grade calls converted +- `drivers/storage/ided/` — all panic-grade calls converted +- `drivers/virtio/virtio-blkd/` — all panic-grade calls converted +- `drivers/virtio/virtio-netd/` — all panic-grade calls converted +- `drivers/graphics/virtio-gpud/` — all panic-grade calls converted +- `drivers/pcid/src/cfg_access/` — DTB/ACPI parsing hardened (21+ sites) + +**Conversion pattern applied** + +- Startup fatal (scheme registration, event queue, namespace, port I/O): `log::error! + process::exit(1)` +- Device init failures (DMA alloc, queue creation, BAR mapping): `log::error! + process::exit(1)` +- Runtime event loop errors (IRQ read/write, scheme tick, event next): `log::error! + continue` +- `try_into().unwrap()` on physical addresses: `.map_err(|_| Error::new(EIO))?` +- `try_into().unwrap_or_else(|_| unreachable!())` on fixed-size arrays: `.map_err(|_| Error::new(EIO))?` +- `Dma::new().unwrap()` in scheme code: `match` with `log::error + return` or `log::error + process::exit(1)` +- `assert!()` on MMIO bounds / internal invariants: `debug_assert!()` +- `unreachable!()` in enum fallback arms: last valid variant as fallback + +**Primary consumer batches** + +- network/storage/audio: `rtl8168d`, `rtl8139d`, `ixgbed`, `ahcid`, `nvmed`, `virtio-blkd`, + `ided`, `ac97d`, `ihdad` +- graphics/VM: `ihdgd`, `virtio-gpud`, `vboxd` + +**Implement** + +- move drivers onto checked helper APIs from Wave 2 +- remove direct panic-grade assumptions around BARs, legacy IRQs, and scheme operations +- make legacy-only or bounded-mode requirements explicit and logged + +**Acceptance** + +- consumer startup fails cleanly for unsupported or partial hardware states +- legacy-only requirements are explicit compatibility outcomes, not abrupt aborts + +**Verification** + +- `CI=1 make cr.base CONFIG_NAME=redbear-mini ARCH=x86_64` — zero errors, build successful +- per-driver grep verified zero remaining panic-grade calls (only Mutex `.lock().unwrap()` kept) + +### Wave 5 — Improve observability and proof + +**Status**: ✅ Complete + +**Implemented** + +- `lspci` now shows runtime interrupt mode per device by reading `/tmp/redbear-irq-report/*.env` +- `redbear-info` already reports aggregate per-mode PCI IRQ counts and quirk pressure +- `redbear-phase-pci-irq-check` reads live driver interrupt mode reports +- operators can now answer "what IRQ mode is this device using and why?" from a single `lspci` invocation + +**Primary targets** + +- `local/recipes/system/redbear-info/source/src/main.rs` +- `local/recipes/system/redbear-hwutils/source/src/bin/{lspci,redbear-phase-iommu-check}.rs` +- `local/scripts/test-{msix,iommu,xhci-irq,lowlevel-controllers}-qemu.sh` +- `local/docs/{REDBEAR-INFO-RUNTIME-REPORT,SCRIPT-BEHAVIOR-MATRIX}.md` + +**Implement** + +- expose chosen interrupt mode per device +- expose fallback reason: MSI-X unavailable, quirk-disabled, vector allocation failed, legacy-only, + etc. +- keep QEMU-first proofs explicit and bounded +- separate “installed”, “configured”, “active”, and “runtime-functional” states clearly + +**Acceptance** + +- operators can answer “what IRQ mode is this device using and why?” from runtime tooling +- proof scripts distinguish architecture proof from hardware proof cleanly + +**Verification** + +- `cargo test` for `redbear-info` +- `cargo test` for `redbear-hwutils` +- re-run bounded proof scripts and confirm mode/fallback signals appear in output + +### Wave 6 — Docs and hardware-evidence sync + +**Status**: ✅ Complete (this document updated with wave completion status) + +**Primary targets** + +- `README.md` +- `AGENTS.md` +- `local/AGENTS.md` +- `docs/README.md` +- this file +- `local/docs/{IOMMU-SPEC-REFERENCE,QUIRKS-SYSTEM,QUIRKS-IMPROVEMENT-PLAN,REDBEAR-INFO-RUNTIME-REPORT,SCRIPT-BEHAVIOR-MATRIX}.md` + +**Implement** + +- sync public/current-state docs to the actual proof scope +- keep QEMU/runtime proof separate from hardware proof +- keep the repo-facing status story aligned with this canonical plan + +**Acceptance** + +- no doc claims broader PCI/IRQ robustness than tests actually prove +- public/current-state docs all point here for PCI/IRQ execution authority + +**Verification** + +- manual doc/code cross-check against current runtime tools and proof scripts + +### Recommended commit boundaries + +1. `pcid` hardening + historical orchestration cleanup (with `pcid-spawner` work now obsoleted / moved to driver-manager) +2. `pcid` helper API hardening +3. `virtio-core` MSI-X/probe hardening +4. consumer batch A +5. consumer batch B +6. observability/runtime tools +7. docs/proof sync + +### Definition of done + +This plan’s implementation work is materially complete only when: + +- no panic-grade behavior remains on normal PCI/IRQ failure paths in `pcid`, shared helpers, or key + consumers, +- IRQ mode selection is observable, +- bounded proof scripts still pass, +- docs match actual proof scope, +- and the remaining gaps are clearly hardware-evidence gaps rather than hidden code fragility. + +### Step A — Establish validation vocabulary in all related docs + +For every low-level controller area, use the same four states consistently: + +- builds +- boots +- validated +- experimental + +Do not mark controller infrastructure “complete” unless the claimed runtime behavior is actually +proven. + +### Step B — Add dedicated validation notes for MSI-X and IOMMU + +The project already has enough code to justify dedicated runtime-validation docs for: + +- GPU MSI-X behavior +- IOMMU bring-up and fault handling + +There is now also an in-tree generic MSI-X runtime proof helper: + +- `local/scripts/test-msix-qemu.sh` + +These should record both successful and failed hardware runs. + +### Step C — Expand runtime-proof tooling where signal is weak + +The project already has a good pattern for this in the Phase 3/4/5 validation helpers. + +Use the same pattern for low-level controllers: + +- one host-side launcher/check path +- one guest-side runtime check path +- one doc entry that records what “passing” actually means + +### Step D — Keep the controller plan separate from higher-level desktop work + +Do not let IRQ/IOMMU/controller planning get absorbed into generic Wayland/KDE roadmaps. + +Controller quality must remain measurable at its own layer. + +## Recommended New Documentation Work + +The current project docs should eventually include dedicated runtime-validation companion documents +for: + +- MSI-X validation +- IOMMU bring-up and fault validation +- timer/controller characterization +- input/controller runtime evidence + +This document is the umbrella enhancement plan; those would be the execution/validation companions. + +## Current Validation Entry Points + +The following in-tree validation paths now exist and should be treated as the current controller +runtime-evidence surface: + +- `local/scripts/test-xhci-irq-qemu.sh --check` — xHCI interrupt-mode proof from QEMU boot logs +- `local/scripts/test-msix-qemu.sh` — live MSI-X proof via `virtio-net` +- `local/scripts/test-iommu-qemu.sh --check` — AMD IOMMU device visibility plus guest boot reachability +- `local/scripts/test-usb-storage-qemu.sh` — USB mass-storage autospawn plus bounded sector-0 readback proof + +## Bottom Line + +Red Bear OS does **not** need a new IRQ/controller architecture. + +It already has the correct architectural direction: + +- scheme-based userspace IRQ delivery +- safe Rust driver wrappers +- PCI/MSI-X support +- IOMMU direction +- ACPI/APIC groundwork + +What it needs now is disciplined completion work in this order: + +1. MSI-X runtime proof +2. IOMMU hardware validation +3. IRQ observability and affinity proof +4. input/controller runtime evidence +5. timer/controller characterization + +The main quality risk is no longer missing design. It is over-claiming readiness before low-level +controller runtime evidence exists. diff --git a/local/recipes/system/driver-manager/source/src/config.rs b/local/recipes/system/driver-manager/source/src/config.rs index 2e054bafa9..dd7a813b06 100644 --- a/local/recipes/system/driver-manager/source/src/config.rs +++ b/local/recipes/system/driver-manager/source/src/config.rs @@ -9,6 +9,8 @@ use std::thread; use std::time::{Duration, Instant}; use std::vec::Vec; +use std::sync::atomic::{AtomicU32, AtomicU64, Ordering}; + use pcid_interface::PciFunctionHandle; use redox_driver_core::device::DeviceInfo; use redox_driver_core::driver::{Driver, DriverError, ErrorSeverity, ProbeResult, RecoveryAction}; @@ -22,6 +24,13 @@ const SIGKILL: i32 = 9; const GRACE_PERIOD_MS: u64 = 3000; const POLL_INTERVAL_MS: u64 = 50; +/// Default crash-threshold (consecutive spawn failures before a driver +/// is auto-blacklisted for this BDF). Overridable via +/// `REDBEAR_DRIVER_CRASH_THRESHOLD`; see `crash_tracker` below. +const DEFAULT_CRASH_THRESHOLD: u32 = 5; +const DEFAULT_BACKOFF_BASE_MS: u64 = 100; +const DEFAULT_BACKOFF_CAP_MS: u64 = 30_000; + #[derive(Debug)] struct SpawnedDriver { pid: u32, @@ -121,6 +130,7 @@ mod tests { }; use std::collections::BTreeMap; use std::sync::Mutex; + use std::time::Instant; static ENV_LOCK: Mutex<()> = Mutex::new(()); @@ -434,6 +444,111 @@ initial_power_state = "D2" }); } + // ── F1 crash-tracker tests ────────────────────────────────────── + + #[test] + fn crash_tracker_record_failure_increments_and_signals_threshold() { + let tracker = super::CrashTracker::new(); + let bdf = "0000:00:1f.2"; + assert_eq!(tracker.record_failure(bdf), None, "1st failure < 5 threshold"); + assert_eq!(tracker.record_failure(bdf), None, "2nd failure"); + assert_eq!(tracker.record_failure(bdf), None, "3rd failure"); + assert_eq!(tracker.record_failure(bdf), None, "4th failure"); + let count = tracker.record_failure(bdf).expect("5th failure >= threshold"); + assert_eq!(count, 5); + } + + #[test] + fn crash_tracker_record_success_clears_state() { + let tracker = super::CrashTracker::new(); + let bdf = "0000:00:1f.2"; + tracker.record_failure(bdf); + tracker.record_failure(bdf); + assert!(tracker.is_in_backoff(bdf, Instant::now())); + tracker.record_success(bdf); + assert!(!tracker.is_in_backoff(bdf, Instant::now())); + } + + #[test] + fn crash_tracker_is_in_backoff_returns_true_within_window() { + let tracker = super::CrashTracker::new(); + let bdf = "0000:00:1f.2"; + tracker.record_failure(bdf); + // Just after the failure, backoff is active. + assert!( + tracker.is_in_backoff(bdf, Instant::now()), + "a just-recorded failure must trigger backoff" + ); + } + + #[test] + fn crash_tracker_snapshot_returns_per_bdf_counts() { + let tracker = super::CrashTracker::new(); + tracker.record_failure("0000:00:1f.2"); + tracker.record_failure("0000:00:1f.2"); + tracker.record_failure("0000:01:00.0"); + let snap = tracker.snapshot(); + assert_eq!(snap.len(), 2); + let mut counts: std::collections::HashMap = snap.into_iter().collect(); + assert_eq!(counts.remove("0000:00:1f.2").unwrap(), 2); + assert_eq!(counts.remove("0000:01:00.0").unwrap(), 1); + } + + #[test] + fn crash_tracker_apply_env_overrides() { + let tracker = super::CrashTracker::new(); + unsafe { std::env::set_var("REDBEAR_DRIVER_CRASH_THRESHOLD", "3") }; + unsafe { std::env::set_var("REDBEAR_DRIVER_BACKOFF_BASE_MS", "50") }; + unsafe { std::env::set_var("REDBEAR_DRIVER_BACKOFF_CAP_MS", "500") }; + tracker.apply_env(); + assert_eq!(tracker.threshold(), 3); + assert_eq!(tracker.backoff_base_ms(), 50); + assert_eq!(tracker.backoff_cap_ms(), 500); + unsafe { std::env::remove_var("REDBEAR_DRIVER_CRASH_THRESHOLD") }; + unsafe { std::env::remove_var("REDBEAR_DRIVER_BACKOFF_BASE_MS") }; + unsafe { std::env::remove_var("REDBEAR_DRIVER_BACKOFF_CAP_MS") }; + } + + #[test] + fn crash_tracker_apply_env_ignores_invalid_values() { + let tracker = super::CrashTracker::new(); + let orig_threshold = tracker.threshold(); + let orig_base = tracker.backoff_base_ms(); + let orig_cap = tracker.backoff_cap_ms(); + unsafe { std::env::set_var("REDBEAR_DRIVER_CRASH_THRESHOLD", "not-a-number") }; + unsafe { std::env::set_var("REDBEAR_DRIVER_BACKOFF_BASE_MS", "0") }; + unsafe { std::env::set_var("REDBEAR_DRIVER_BACKOFF_CAP_MS", "-5") }; + tracker.apply_env(); + assert_eq!( + tracker.threshold(), + orig_threshold, + "invalid threshold must be ignored" + ); + assert_eq!( + tracker.backoff_base_ms(), + orig_base, + "zero base_ms must be ignored (zero is treated as invalid)" + ); + assert_eq!( + tracker.backoff_cap_ms(), + orig_cap, + "negative cap_ms must be ignored (parse fails, default kept)" + ); + unsafe { std::env::remove_var("REDBEAR_DRIVER_CRASH_THRESHOLD") }; + unsafe { std::env::remove_var("REDBEAR_DRIVER_BACKOFF_BASE_MS") }; + unsafe { std::env::remove_var("REDBEAR_DRIVER_BACKOFF_CAP_MS") }; + } + + #[test] + fn crash_tracker_global_stub_returns_when_unset() { + // CrashTracker global pointer must not be set by other tests. + let snap = super::crash_tracker().snapshot(); + assert!( + snap.is_empty(), + "fresh crash tracker snapshot should be empty" + ); + } + #[test] fn spawn_decision_defaults_to_spawn_without_env_or_flags() { serialized(|| { @@ -581,6 +696,192 @@ fn send_signal_to_spawned( Ok(()) } +// ── Crash counter, exponential backoff, auto-blacklist ─────────── +// +// Closes the F1 finding: a driver that crashes on every spawn used +// to be respawned every hotplug poll (250ms) indefinitely. The +// `CrashTracker` records consecutive spawn failures per BDF, applies +// exponential backoff (100 ms × 2^N, cap 30 s) before the next probe, +// and auto-blacklists after `threshold` consecutive failures. +// +// Configuration is read once at startup from env vars: +// REDBEAR_DRIVER_CRASH_THRESHOLD (default 5) +// REDBEAR_DRIVER_BACKOFF_BASE_MS (default 100) +// REDBEAR_DRIVER_BACKOFF_CAP_MS (default 30000) +// +// A successful probe (Bind) clears the failure state for that BDF so +// a transient crash followed by a fix doesn't permanently blacklist +// the device. + +/// Per-BDF crash state. +#[derive(Debug, Clone, Copy)] +pub struct CrashState { + pub failures: u32, + pub next_retry_at: Instant, +} + +/// Process-wide tracker. Process-global because the failure of one +/// driver's spawn is observable across all drivers (every probe cycle +/// sees the same BDF and must respect the backoff window). +pub struct CrashTracker { + failures: std::sync::OnceLock>>, + threshold: AtomicU32, + backoff_base_ms: AtomicU64, + backoff_cap_ms: AtomicU64, +} + +impl CrashTracker { + pub const fn new() -> Self { + Self { + failures: std::sync::OnceLock::new(), + threshold: AtomicU32::new(DEFAULT_CRASH_THRESHOLD), + backoff_base_ms: AtomicU64::new(DEFAULT_BACKOFF_BASE_MS), + backoff_cap_ms: AtomicU64::new(DEFAULT_BACKOFF_CAP_MS), + } + } + + fn failures_map(&self) -> &Mutex> { + self.failures.get_or_init(|| Mutex::new(HashMap::new())) + } + + pub fn threshold(&self) -> u32 { + self.threshold.load(Ordering::SeqCst) + } + + pub fn backoff_base_ms(&self) -> u64 { + self.backoff_base_ms.load(Ordering::SeqCst) + } + + pub fn backoff_cap_ms(&self) -> u64 { + self.backoff_cap_ms.load(Ordering::SeqCst) + } + + /// Read the env-var overrides and apply them. Called once at + /// startup from `apply_crash_tracker_env()` (see main.rs). + pub fn apply_env(&self) { + if let Ok(s) = std::env::var("REDBEAR_DRIVER_CRASH_THRESHOLD") { + if let Ok(n) = s.parse::() { + if n > 0 { + self.threshold.store(n, Ordering::SeqCst); + } + } + } + if let Ok(s) = std::env::var("REDBEAR_DRIVER_BACKOFF_BASE_MS") { + if let Ok(n) = s.parse::() { + if n > 0 { + self.backoff_base_ms.store(n, Ordering::SeqCst); + } + } + } + if let Ok(s) = std::env::var("REDBEAR_DRIVER_BACKOFF_CAP_MS") { + if let Ok(n) = s.parse::() { + if n > 0 { + self.backoff_cap_ms.store(n, Ordering::SeqCst); + } + } + } + } + + /// Return true if `bdf` has a recorded failure whose + /// `next_retry_at` is still in the future. Used by `probe` to + /// short-circuit before the spawn attempt. + pub fn is_in_backoff(&self, bdf: &str, now: Instant) -> bool { + if let Ok(map) = self.failures_map().lock() { + if let Some(state) = map.get(bdf) { + return state.next_retry_at > now; + } + } + false + } + + /// Snapshot the current failures table for the + /// `/crash_count/` and `/crash_count` (list) scheme endpoints. + pub fn snapshot(&self) -> Vec<(String, u32)> { + self.failures_map() + .lock() + .map(|map| { + map.iter() + .map(|(k, v)| (k.clone(), v.failures)) + .collect() + }) + .unwrap_or_default() + } + + /// Record a spawn failure for `bdf`. Updates the failure counter + /// and computes the next-retry instant via exponential backoff. + /// Returns `Some(threshold_exceeded)` if the BDF should be + /// auto-blacklisted (i.e. `failures >= threshold`), `None` otherwise. + pub fn record_failure(&self, bdf: &str) -> Option { + let threshold = self.threshold(); + let base_ms = self.backoff_base_ms(); + let cap_ms = self.backoff_cap_ms(); + let map_mutex = self.failures_map(); + let mut map = match map_mutex.lock() { + Ok(m) => m, + Err(_) => return None, + }; + let entry = map.entry(bdf.to_string()).or_insert(CrashState { + failures: 0, + next_retry_at: Instant::now(), + }); + entry.failures = entry.failures.saturating_add(1); + let n = entry.failures; + let backoff_ms = base_ms + .checked_shl(n.min(20)) + .unwrap_or(cap_ms) + .min(cap_ms); + entry.next_retry_at = Instant::now() + Duration::from_millis(backoff_ms); + if n >= threshold { + Some(n) + } else { + None + } + } + + /// Clear any failure state for `bdf`. Called from a successful + /// probe (Bind result). + pub fn record_success(&self, bdf: &str) { + if let Ok(mut map) = self.failures_map().lock() { + map.remove(bdf); + } + } +} + +/// Global crash tracker. The pointer is set once at startup +/// (`set_crash_tracker`); reads in the probe hot path are non-blocking +/// only because the pointer is `static`, not because of any +/// `RwLock`. If unset, `crash_tracker()` returns a static stub with +/// the default threshold and no recorded failures — i.e. behavior +/// identical to pre-F1 (every probe runs, no backoff, no blacklist). +static mut CRASH_TRACKER: *const CrashTracker = std::ptr::null(); + +/// Returns a reference to the global crash tracker. If the manager +/// has not yet called `set_crash_tracker`, returns a static stub that +/// reports threshold=0 (i.e. never auto-blacklists) and accepts every +/// probe — this preserves the pre-F1 behaviour for early-init code +/// paths (config loading, env parsing) that run before the manager +/// wires the tracker. +pub fn crash_tracker() -> &'static CrashTracker { + unsafe { + if CRASH_TRACKER.is_null() { + &CRASH_TRACKER_STUB + } else { + &*CRASH_TRACKER + } + } +} + +static CRASH_TRACKER_STUB: CrashTracker = CrashTracker::new(); + +/// Wire the global crash tracker. Called once at startup from +/// `apply_crash_tracker_env()` after env vars are read. The pointer +/// is process-lifetime and never reset. +pub fn set_crash_tracker(tracker: &'static CrashTracker) { + unsafe { + CRASH_TRACKER = tracker; + } +} + /// Snapshot the PIDs of every spawned child for this driver config. /// Used by the system-wide `/suspend` scheme endpoint to walk drivers /// in priority order and signal each in turn. Returns PIDs in @@ -794,12 +1095,34 @@ impl DriverConfig { /// maps. Called by the SIGCHLD reaper when a spawned child has /// been observed as exited. Idempotent — a no-op if the pid is not /// present. + /// + /// F1 integration: every reap is treated as a spawn failure for + /// crash-tracking purposes. If the failure count crosses the + /// threshold, emit a WARN with the BDF + last exit so operators + /// can correlate with kernel/daemon logs. pub fn reap_pid(&self, pid: u32) { - if let Ok(mut p2d) = self.pid_to_device.lock() { + let device_key = if let Ok(mut p2d) = self.pid_to_device.lock() { if let Some(key) = p2d.remove(&pid) { if let Ok(mut spawned) = self.spawned.lock() { spawned.remove(&key); } + Some(key) + } else { + None + } + } else { + None + }; + if let Some(key) = device_key { + if let Some(count) = crash_tracker().record_failure(&key) { + log::warn!( + "crash-threshold: driver={} device={} consecutive_failures={} \ + auto-blacklisted (override via /etc/driver-manager.d/disable-.toml \ + or REDBEAR_DRIVER_CRASH_THRESHOLD override)", + self.name, + key, + count + ); } } } @@ -848,6 +1171,22 @@ impl Driver for DriverConfig { } } + // F1 crash tracker: if this BDF recently failed enough times + // to exceed the threshold, the policy blacklist has already + // absorbed the BDF (see `record_failure` integration below). + // For in-backoff state below the threshold, defer with a + // backoff reason so the next probe cycle respects it. + if crash_tracker().is_in_backoff(&device_key, Instant::now()) { + log::debug!( + "crash-backoff: driver={} device={} skipping until next retry", + self.name, + device_key + ); + return ProbeResult::Deferred { + reason: format!("crash-backoff for {}", device_key), + }; + } + let early_quirks = crate::quirks::decide_for_phase( info, redox_driver_sys::quirks::QuirkPhase::Early, @@ -1119,11 +1458,31 @@ impl Driver for DriverConfig { if let Ok(mut p2d) = self.pid_to_device.lock() { p2d.insert(pid, device_key.clone()); } + // F1: a successful spawn clears any prior failure + // state for this BDF, so a transient crash followed by + // a working spawn does not leave the device + // permanently back-pressured. + crash_tracker().record_success(&device_key); ProbeResult::Bound } - Err(e) => ProbeResult::Fatal { - reason: format!("spawn failed: {}", e), - }, + Err(e) => { + // F1: an immediate spawn failure (e.g. ENOENT for a + // missing binary) counts toward the threshold just + // like a SIGCHLD reap would. + if let Some(count) = crash_tracker().record_failure(&device_key) { + log::warn!( + "crash-threshold: driver={} device={} consecutive_failures={} \ + (spawn returned: {})", + self.name, + device_key, + count, + e + ); + } + ProbeResult::Fatal { + reason: format!("spawn failed: {}", e), + } + } } } diff --git a/local/recipes/system/driver-manager/source/src/main.rs b/local/recipes/system/driver-manager/source/src/main.rs index 650188ba58..0b2893d177 100644 --- a/local/recipes/system/driver-manager/source/src/main.rs +++ b/local/recipes/system/driver-manager/source/src/main.rs @@ -570,6 +570,9 @@ fn main() { // Apply deferred-retry config from env vars (defaults to 30 retries // at 500 ms intervals if unset). See timing.rs::set_deferred_retry_config. apply_deferred_retry_env(); + // Wire the global CrashTracker (F1). Must come before the probe + // hot path is reachable. + apply_crash_tracker_env(); reset_timeline_log(); @@ -691,6 +694,24 @@ fn async_probe_from_env() -> bool { } } +/// Wire the global `CrashTracker` and apply env-var overrides. Called +/// once at startup after `apply_deferred_retry_env()` so the probe hot +/// path can consult a fully-configured tracker. +static CRASH_TRACKER: config::CrashTracker = config::CrashTracker::new(); +fn apply_crash_tracker_env() { + CRASH_TRACKER.apply_env(); + config::set_crash_tracker(&CRASH_TRACKER); + let threshold = CRASH_TRACKER.threshold(); + let base_ms = CRASH_TRACKER.backoff_base_ms(); + let cap_ms = CRASH_TRACKER.backoff_cap_ms(); + log::info!( + "crash-tracker: threshold={} backoff_base_ms={} backoff_cap_ms={}", + threshold, + base_ms, + cap_ms + ); +} + /// Apply the deferred-retry configuration from the /// `REDBEAR_DRIVER_DEFERRED_RETRY_COUNT` and /// `REDBEAR_DRIVER_DEFERRED_RETRY_INTERVAL_MS` env vars. Unparseable diff --git a/local/recipes/system/driver-manager/source/src/scheme.rs b/local/recipes/system/driver-manager/source/src/scheme.rs index e03861bd59..2829a8d0dc 100644 --- a/local/recipes/system/driver-manager/source/src/scheme.rs +++ b/local/recipes/system/driver-manager/source/src/scheme.rs @@ -59,6 +59,8 @@ enum HandleKind { Recover, Suspend, Resume, + CrashCount, + CrashCountBdf(String), Timing, } @@ -188,11 +190,15 @@ impl DriverManagerScheme { ["recover"] => Ok(HandleKind::Recover), ["suspend"] => Ok(HandleKind::Suspend), ["resume"] => Ok(HandleKind::Resume), + ["crash_count"] => Ok(HandleKind::CrashCount), ["timing"] => Ok(HandleKind::Timing), ["devices", pci_addr] if Self::valid_pci_addr(pci_addr) => { let _ = self.device_status(pci_addr)?; Ok(HandleKind::Device((*pci_addr).to_string())) } + ["crash_count", pci_addr] if Self::valid_pci_addr(pci_addr) => { + Ok(HandleKind::CrashCountBdf((*pci_addr).to_string())) + } _ => Err(Error::new(ENOENT)), } } @@ -277,6 +283,23 @@ impl DriverManagerScheme { HandleKind::Device(pci_addr) => self.device_status(pci_addr), HandleKind::Bound => self.bound_output(), HandleKind::Events => self.events_output(), + HandleKind::CrashCount => { + let entries = crate::config::crash_tracker().snapshot(); + let mut out = String::new(); + for (bdf, n) in entries { + out.push_str(&format!("{bdf} {n}\n")); + } + Ok(out) + } + HandleKind::CrashCountBdf(pci_addr) => Ok(format!( + "{pci_addr} {}\n", + crate::config::crash_tracker() + .snapshot() + .into_iter() + .find(|(b, _)| b.as_str() == pci_addr.as_str()) + .map(|(_, n)| n) + .unwrap_or(0) + )), HandleKind::Modalias => { if let Ok(results) = self.modalias_results.lock() { if let Some(result) = results.get(&id) { @@ -338,6 +361,10 @@ impl DriverManagerScheme { HandleKind::Recover => format!("{SCHEME_NAME}:/recover"), HandleKind::Suspend => format!("{SCHEME_NAME}:/suspend"), HandleKind::Resume => format!("{SCHEME_NAME}:/resume"), + HandleKind::CrashCount => format!("{SCHEME_NAME}:/crash_count"), + HandleKind::CrashCountBdf(pci_addr) => { + format!("{SCHEME_NAME}:/crash_count/{pci_addr}") + } HandleKind::Timing => format!("{SCHEME_NAME}:/timing"), } } @@ -359,6 +386,8 @@ impl DriverManagerScheme { | HandleKind::Recover | HandleKind::Suspend | HandleKind::Resume + | HandleKind::CrashCount + | HandleKind::CrashCountBdf(_) | HandleKind::Timing => MODE_FILE | 0o644, } }