diff --git a/CLAUDE.md b/CLAUDE.md index 54463cefc43..3b37935c01f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -28,6 +28,7 @@ A subdirectory `CLAUDE.md` loads when a file in that subtree is `Read`, and not - **Zero silent debt.** Dead code is deleted; every abstraction earns its place. A discovered compromise has exactly two legal outcomes: remove it, or record it with ownership, evidence and an exit condition — and it stays a present-state weakness until removed. - **Fail fast, trust nothing.** Panics over silent degradation; exhaustive matches; the unimplemented dies loudly. Input that crossed a trust boundary is never trusted and never panics the kernel — it is refused. - **The kernel never crashes from userland.** A kernel bug crashes loudly; a userland bug never reaches it. +- **No kernel threads.** The kernel creates no thread but the per-CPU idle loop: kernel work runs, bounded, on the thread or interrupt that caused it and is charged to it; long-running work with no owner is a userland server's. - **Rust is first class.** Not POSIX, not C. Unrepresentable is best: prefer compile-time safety over runtime checks over tests. - **Existing Rust just works.** A program that builds for other operating systems builds and runs on ToyOS unchanged; the ecosystem gains ToyOS support through forks carried upstream, never through ToyOS-specific replacement crates. - **Development ergonomics above all.** Iteration speed beats feature count; tooling comes first. diff --git a/issues/build/a-guest-with-no-virtio-keeps-no-serial-when-its-run-reds.md b/issues/build/a-guest-with-no-virtio-keeps-no-serial-when-its-run-reds.md new file mode 100644 index 00000000000..061c37e7882 --- /dev/null +++ b/issues/build/a-guest-with-no-virtio-keeps-no-serial-when-its-run-reds.md @@ -0,0 +1,23 @@ +--- +status: open +kind: tooling +opened: 2026-09-27 +--- + +# A guest with no virtio device keeps no serial when its run reds + +`tests/common/lane.rs`'s `keep_serial` copies a red run's `uart-*.log` files to +`target/red-run-serial`. A profile with no virtio device (`qemu_command`'s +`shape.virtio.present()` false: `Profile::Metal`, and every `power.rs` guest +built on `panicked()`) routes its 16550 to QEMU's stdio instead, so its only +record is the reader thread's memory, and the kept lane directory is empty. The +console stream of a virtio guest is not kept either; only its UART is. + +**Evidence:** `syscall_fault_halts` red at `8e525da4`: the run printed +`this red run's serial logs are kept at target/red-run-serial/toyos-tmp-45425-0`, +and that directory's `lane-0` is empty. The check's own error carried no +capture, so what the kernel said was lost. + +**Exit condition:** every guest's console, whichever device carries it, is +written under its lane as it is read, and a red run keeps it; a red +`panic_reboots` run shows a non-empty kept lane. diff --git a/issues/build/a-sysroot-cloned-during-a-toolchain-rebuild-never-gets-its-cargo.md b/issues/build/a-sysroot-cloned-during-a-toolchain-rebuild-never-gets-its-cargo.md new file mode 100644 index 00000000000..9594732d9ff --- /dev/null +++ b/issues/build/a-sysroot-cloned-during-a-toolchain-rebuild-never-gets-its-cargo.md @@ -0,0 +1,26 @@ +--- +status: open +kind: tooling +opened: 2026-09-27 +--- + +# A sysroot cloned during a toolchain rebuild never gets its cargo + +The primary checkout's `rust/build/aarch64-apple-darwin/stage2/bin` was +recreated at 22:00 on 2026-09-27 and, read while a `toyos-build --build-only` +ran in the primary, held `rustc` and `rustdoc` and no `cargo`; the `cargo` link +appeared at 22:08. At 22:03:59 a `cargo test --test toyos-build` in the +`wt/toyos-nokthread` worktree rebuilt std and published +`rust/build/sysroots/5dc157f7fac727be` cloned from that `bin/`, so it has +`rustc` and `rustdoc` and no `cargo`. The key is found again on every later +run and nothing re-provisions it: every harness run from that worktree since +panics at `tests/toyos.rs:2970` with `the toyos toolchain at +.../sysroots/5dc157f7fac727be/bin is missing cargo` on the C corpus, before any +test runs. + +**Evidence:** the two directory listings and the harness log above, read on +the dev host; not reproduced on purpose. + +**Exit condition:** a sysroot is never published without the provisioned +`cargo`, and a run that finds a published one without it provisions it or +refuses by name at the toolchain step rather than inside the corpus. diff --git a/issues/build/idle-stack-guards-returned-arm-reads-a-line-nothing-writes.md b/issues/build/idle-stack-guards-returned-arm-reads-a-line-nothing-writes.md new file mode 100644 index 00000000000..fde028ae1a6 --- /dev/null +++ b/issues/build/idle-stack-guards-returned-arm-reads-a-line-nothing-writes.md @@ -0,0 +1,20 @@ +--- +status: open +kind: tooling +opened: 2026-09-27 +--- + +# `idle_stack_guard`'s "the read succeeded" arm reads a line nothing writes + +`tests/common/faults.rs`'s `idle_stack_guard` reds with *the page below the idle +stack is still mapped* when its capture contains `debug syscall returned`. The +only guest it drives, `tests/toyos-rust-tests/src/bin/test_panic_child.rs`, +writes `SYS_DEBUG {action} returned {rc:#x}` when the syscall comes back. The +arm can never fire: a guard page that is still mapped reaches the drain's +ceiling and reds instead on the missing `#PF UNHANDLED` line, naming the wrong +cause. + +**Evidence:** read from both sources; no mutation run. + +**Exit condition:** the arm and the child share one constant for the line, and +a mutation that maps the guard page reds with the arm's own message. diff --git a/issues/isolation/interrupt-entry-keeps-a-ring-3-ac-flag.md b/issues/isolation/interrupt-entry-keeps-a-ring-3-ac-flag.md new file mode 100644 index 00000000000..8137e1bd6b7 --- /dev/null +++ b/issues/isolation/interrupt-entry-keeps-a-ring-3-ac-flag.md @@ -0,0 +1,25 @@ +--- +status: open +kind: defect +opened: 2026-09-27 +--- + +# Interrupt entry keeps a Ring 3 thread's AC flag + +SMAP binds a Ring 0 access only while `RFLAGS.AC` is clear, and a Ring 3 thread +can set `AC` with `popf` (`CR0.AM` is clear, so it costs the thread nothing). +The syscall entry clears it through `IA32_FMASK` +(`kernel/src/arch/x86_64/syscall.rs`), but an interrupt or trap gate does not: +Intel SDM Vol. 3A §6.12.1.3 names TF, VM, RF and NT, and IF for an interrupt +gate. `arch::entry::ring3_naked_asm` prepends only `cld`, and +`kernel/src/arch/x86_64/control_regs.rs` says the boot `clac` is the only one +the kernel needs. + +So an interrupt or exception taken from a Ring 3 thread that set `AC` runs its +handler with SMAP off: a kernel bug there that touches a user address reads or +writes it silently. + +**Evidence:** read from the code and the SDM; no test stages it. + +**Exit condition:** every Ring 0 entry from Ring 3 runs with `AC` clear, gated by +a guest program that sets `AC` and a handler that must fault on a user address. diff --git a/issues/kernel/a-deliberate-wedge-inside-a-driver-panics-every-other-cpu-that-wants-its-lock.md b/issues/kernel/a-deliberate-wedge-inside-a-driver-panics-every-other-cpu-that-wants-its-lock.md index 84da94d8cc1..55791c6567f 100644 --- a/issues/kernel/a-deliberate-wedge-inside-a-driver-panics-every-other-cpu-that-wants-its-lock.md +++ b/issues/kernel/a-deliberate-wedge-inside-a-driver-panics-every-other-cpu-that-wants-its-lock.md @@ -25,15 +25,6 @@ disk. On the T14 that is `logd`'s `SYS_FSYNC`, which reaches kernel::arch::syscall::gate::syscall_entry ``` -The panic lands in a syscall, so `percpu::in_syscall` makes it recoverable: -`try_recover_from_panic` ends that thread and returns to the scheduler, where -`deadline::wedge_if_staged` folds the CPU into the wedge. `apic::halt_all_cpus` -is therefore never reached, and neither is `deadline::stand_down` — which is why -the boot deadline and not the panic path's own bound ends the machine. That -composition is correct; what is not is that a boot staged to measure a *device* -silently loses its log writer, and the only account of it is a `PANIC` record in -a ring tail nobody was draining. - Two things are true and neither is decided here: - The detector firing is right in general — a lock held for ever by a CPU that @@ -46,7 +37,7 @@ Two things are true and neither is decided here: Any actuator that stops a CPU inside a driver — today the `usb-wedge-*` arms, which are QEMU registrations and reach no flashed image. It does not change -their verdicts, but it adds a panic, a dead `logd`, and a page of dropped +their verdicts, but it adds a panic and a page of dropped records to every one of them. ## Exit condition diff --git a/issues/kernel/deferred-release-outlives-its-syscall.md b/issues/kernel/deferred-release-outlives-its-syscall.md index 343e88157ec..ad6132de8ef 100644 --- a/issues/kernel/deferred-release-outlives-its-syscall.md +++ b/issues/kernel/deferred-release-outlives-its-syscall.md @@ -237,9 +237,7 @@ So the two constraints meet. **The hook queue may not park, and the row that would want to is not on the hook queue but in a `Drop` that also may not.** Moving `File` to `deferred` swaps one illegal site for another. The second shape above is still right for the `deferred` rows, and by itself it reaches neither -`File` nor `close_all` — that one is also called from `recover_or_halt`'s -`Blame::Process` arm (`arch/idt/exceptions.rs:348`), which has no syscall to -return through. +`File` nor `close_all`. The track carries this as **wall 4**, with the three shapes the owner has to choose between. Nothing here should be built before that choice, because all diff --git a/issues/kernel/every-wait-in-this-kernel-is-a-spin.md b/issues/kernel/every-wait-in-this-kernel-is-a-spin.md index 511185188d1..9af214f1f54 100644 --- a/issues/kernel/every-wait-in-this-kernel-is-a-spin.md +++ b/issues/kernel/every-wait-in-this-kernel-is-a-spin.md @@ -92,10 +92,6 @@ both before any lock conversion; the order is forced, not preferred. - **The sleep lock.** A sleep-lock holder stays preemptible and raises no preempt count, so the baseline assertion keeps meaning exactly "a spinlock is held". -- **`usbd` and `iod` on the existing kernel-thread machinery.** No housekeeping - thread's wait can stop another's, and a panic inside one is recoverable rather - than a halted machine. Three threads, not one, because a stuck USB enumeration - must not stop the log. - **xHCI async, and the four lock conversions.** Inseparable. A CPU never waits for a device: the lock is dropped before the park, and a completion is matched to its asker by identity, never by arrival order. @@ -160,10 +156,7 @@ both before any lock conversion; the order is forced, not preferred. `PROCESS_TABLE.lock()` at `loader/mod.rs:694`. `release_process` and `kill_process` bracket `teardown_resources` between two table acquisitions and hold neither across it. So `PROCESS_TABLE` does not have to convert for the - park to be legal, and converting it anyway would buy the exception-recovery - path a `try_lock` with no answer for its failure: `recover_or_halt`'s - `Blame::Process` arm reaches `process::exit` from a CPU exception - (`arch/idt/exceptions.rs:348`), and nothing on that path mints a `Parkable`. + park to be legal. What *does* have to convert is `Lock`, and wall 5 is why. **Six, and the count above is the xHCI chunk's rather than the machine's.** @@ -384,9 +377,7 @@ means everywhere, not only here. Three shapes: anywhere. 3. **Give the batch an owner** — `deferred-release-outlives-its-syscall`'s own second shape — and make `File` deferred. That buys a parkable release site on - the syscall path and does *not* cover `close_all` reached from - `recover_or_halt`'s `Blame::Process` arm, which has no syscall to return - through; and it needs `drain_zero_handles`'s two scheduler sites to stop + the syscall path, and it needs `drain_zero_handles`'s two scheduler sites to stop running hooks that can park, which is a redesign of that queue rather than a use of it. diff --git a/issues/kernel/nothing-charges-kernel-memory-to-a-process.md b/issues/kernel/nothing-charges-kernel-memory-to-a-process.md index f22057efe0f..7a003252238 100644 --- a/issues/kernel/nothing-charges-kernel-memory-to-a-process.md +++ b/issues/kernel/nothing-charges-kernel-memory-to-a-process.md @@ -20,8 +20,8 @@ is not a value with a destructor" is unrepresentable; the descriptor type and its un-refcounted clone are gone; the file cache installs a real budget and a budget that was never installed is now a loud kernel bug; and the unbounded user-string copy has a ceiling. What survives is the *accounting*, which nothing -has touched, plus two items: panic recovery still runs no teardown, and peak -memory is written by two paths that overwrite each other. +has touched, plus one item: peak memory is written by two paths that overwrite +each other. Blocked on nothing. Two things worth knowing before it is restarted, because both cost a day to discover: @@ -34,9 +34,7 @@ both cost a day to discover: =`, `drop()` and burial in a collection all pass silently. A drop bomb is the state of the art, and `Unmapped` is already exactly such an obligation. -**The terminal state of every unbounded grower is this entry's.** The allocation -failure itself reports cleanly — a failed kernel allocation takes `alloc`'s -no_std default handler and panics with the size, the layer and the call site -named — so what is left at the end of a grower is *who dies*: whichever thread -happened to allocate, not the one that exhausted the heap. That is what charging -fixes, and nothing else does. +**The terminal state of every unbounded grower is a halted machine.** A failed +kernel allocation panics with the size, the layer and the call site named, and +every kernel panic halts, so a grower userland can drive is refused at its bound +or it ends the machine. diff --git a/issues/kernel/the-capability-end-state-is-twelve-answers.md b/issues/kernel/the-capability-end-state-is-twelve-answers.md index 968c16899fa..b8f185fd161 100644 --- a/issues/kernel/the-capability-end-state-is-twelve-answers.md +++ b/issues/kernel/the-capability-end-state-is-twelve-answers.md @@ -349,60 +349,6 @@ Four decisions, in one place, for the owner: Question 12 is not in this set: its track already holds it. -## Kernel-resident workers - -**Kernel-resident workers are a control-flow boundary, not a memory boundary** — -each one is audited periodically, exists only where independent blocking -progress, fault containment of execution flow, or latency isolation requires it, -and a new one needs explicit architectural justification; work moves to -userspace when the IPC/wait machinery makes that an isolation gain rather than -overhead. - -**Census of 2026-08-20.** `sched::kthread` caps a shipping machine at three and -dies naming a fourth (`kernel/src/sched/kthread.rs:50`, `:102`). All three -exist, and all three are started at the end of `kernel_main` -(`kernel/src/main.rs:696`-`703`): - -- **`klogd`** — the machine's only console drainer, "one thread where every idle - CPU used to drain". `OnPanic::Halt`, because "a machine whose only console - drainer has been killed goes silent with nothing left able to say so" - (`kernel/src/log/console.rs:1`, spawn at `:116`). -- **`usbd`** — owns the xHCI port machine so USB work runs in a context of its - own instead of whichever thread trapped: "a stuck USB enumeration must not - stop the log". Spawned on every machine including one with no controller, at - one kernel stack, so the machine has one answer to how many kernel threads it - has. `OnPanic::Recover`, because every loss it causes is visible - (`kernel/src/drivers/xhci/usbd.rs:1`, spawn at `:47`). Its body is one park - today — nothing posts to it yet. -- **`iod`** — owns the deferred write-back queue, because `OpenFileState::drop` - must flush under a lock that is becoming a sleep lock and a `Drop` impl cannot - hold a `Parkable`. `OnPanic::Recover`, because a killed `iod` costs deferred - write-back and both `SYS_FSYNC`'s error path and `/system/bin/logd`'s give-up policy - can see that (`kernel/src/iod.rs:1`, spawn at `:52`). Its body is one park - today — nothing pushes yet. **One `iod` machine-wide is a decision with a - measurement owed** at the 128-core target, recorded at its own site - (`kernel/src/iod.rs:24`). - -Two more exist only on a `boot-actuators` kernel and are stimulus rather than -workers: `lognest`, one thread (`kernel/src/log/nested.rs:68`), and `logstorm`, -one per log shard (`kernel/src/log/storm.rs:115`) — which is why the cap is -`3 + MAX_LOG_SHARDS` on that build and 3 on a shipping one -(`kernel/src/sched/kthread.rs:50`). - -**Verdict:** all three are justified at their site, each by independent blocking -progress or fault containment of execution flow, and each states its panic -policy. None is a memory boundary. Nothing is owed to userspace yet; the sleep -locks the two idle bodies are waiting for are -`issues/kernel/every-wait-in-this-kernel-is-a-spin.md`. - -**PID-backed pseudo-processes for kernel workers** are pragmatic today — a -process-table row is what makes a kernel thread nameable in `ps`, in -`sched::dump` and in a crash report, and every field of it is the empty value -rather than a plausible one (`kernel/src/sched/kthread.rs:291`). The moment that -representation leaks misleading user-process semantics into policy, -observability, lifecycle or APIs, identity/accounting separates from -user-process semantics rather than preserving the abstraction for convenience. - ## The rest of the review's standing rules - **The adversarial handle-lifecycle suite** the review lists — stale-handle diff --git a/issues/kernel/the-kernel-is-small-interrupts-post-and-threads-wait.md b/issues/kernel/the-kernel-is-small-interrupts-post-and-threads-wait.md index e7ba3863a2c..614cc269375 100644 --- a/issues/kernel/the-kernel-is-small-interrupts-post-and-threads-wait.md +++ b/issues/kernel/the-kernel-is-small-interrupts-post-and-threads-wait.md @@ -21,8 +21,7 @@ times: `Source` enum with two hand-written dispatches, and ad-hoc wake paths. - **USB with three concurrency models:** a state machine inside the scheduler pass, disk I/O spinning with interrupts off for up to 4.75 s under one global - lock, and boot discovery calling a blocking bind from a scheduler pass. The - thread reserved to own the controller, `usbd`, only parks. + lock, and boot discovery calling a blocking bind from a scheduler pass. - **Storage done busy-waiting under spinlocks:** one global VFS lock, NVMe with one command outstanding and polled, and a 2 s operation budget "with preemption off" that budgets audio stalls. That budget drags a refusal chain @@ -138,7 +137,7 @@ times: `/log` survives usbd killed mid-batch, the keyboard keeps working while a stick misbehaves, and Ctrl+Alt+D on the machine's own keyboard files the dump with usbd killed. -5. **USB owned by its thread, then by userland**, with discovery and recovery +5. **USB by userland**, with discovery and recovery written once as straight-line code. **Exit**: no interrupts-off window longer than a register access, and keyboard input keeps flowing while a stick misbehaves. diff --git a/issues/kernel/the-kernel-still-creates-threads.md b/issues/kernel/the-kernel-still-creates-threads.md new file mode 100644 index 00000000000..f9e353060f4 --- /dev/null +++ b/issues/kernel/the-kernel-still-creates-threads.md @@ -0,0 +1,34 @@ +--- +status: open +kind: track +opened: 2026-09-27 +--- + +# The kernel still creates threads + +The owner's ruling: the kernel creates no thread but the per-CPU idle loop. +Kernel work runs, bounded, on the thread or interrupt that caused it and is +charged to it; long-running work with no owner is a userland server's. The +ruling holds only if the whole kernel-thread machinery is deleted — +`sched/kthread.rs`, `kthread::spawn`, the row table and every policy a row +carries, `is_kernel_task`/`current_is_kernel_thread`, and every special case that exists +only for kernel threads. Stopping new uses is not enough. + +**Exit condition:** `kernel/src/sched/kthread.rs` does not exist, and no kernel +code creates a schedulable task other than the per-CPU idle loop. + +**Evidence:** `git grep -n 'kthread::spawn(' -- kernel/src`. + +**Stages:** + +- **K2:** the reaper PR #549 introduces becomes last-thread-out: the victim's + last thread tears down its own process on its way out of the kernel, and the + scheduler frees that thread's kernel stack after switching away. Blocked on + #549 landing. +- **K3:** the test-only `logstorm`/`lognest` producers are deleted if the log + gate does not need kernel-context producers. Blocked on nothing. +- **K4:** `klogd` goes: the owner-approved driver-model design moves the + console to logd, and this track owns that move. +- **K5:** `iod` goes with the kernel's write-back queue; met only when #536 + lands with no new `kthread::spawn`. +- **K6:** delete the machinery named above. Blocked on K2–K5. diff --git a/issues/panic-path/a-crash-report-can-name-a-syscall-that-already-ended.md b/issues/panic-path/a-crash-report-can-name-a-syscall-that-already-ended.md new file mode 100644 index 00000000000..9047cc7ac4f --- /dev/null +++ b/issues/panic-path/a-crash-report-can-name-a-syscall-that-already-ended.md @@ -0,0 +1,45 @@ +--- +status: open +kind: defect +opened: 2026-09-27 +--- + +# A crash report can name a syscall that already ended + +The crash report prints `Syscall:` and a user backtrace only while +`percpu::in_syscall()` holds (`kernel/src/arch/x86_64/idt/exceptions.rs`). That +compares this CPU's recorded syscall task with the current task, and only +`leave_syscall` clears the word, on the CPU where the syscall ends. `Hw::switch` +in `kernel/src/arch/x86_64/hw.rs` moves the current identity and never the +bracket. The number, the user `rip` and the `rbp` the user backtrace walks from +are per-CPU copies too (`syscall_entry` in `kernel/src/arch/x86_64/syscall.rs`). + +A syscall that parks on CPU A and finishes on CPU B is wrong both ways: + +- **A keeps naming it.** A's word still names the thread. When that thread next + runs in Ring 3 on A before any other syscall enters there, an interrupt + handler's death on A reports the finished syscall's number, user `rip` and + user backtrace as the context it died in. +- **B forgets another.** `leave_syscall` on B clears whatever B's word named, + even a second thread still parked inside a syscall it entered on B. When that + thread resumes on B and the kernel dies inside its syscall, the report prints + no `Syscall:` line. + +`process::handle_fault` (`kernel/src/process.rs`) prints `percpu::syscall_num()` +with no bracket at all, so a handle fault in a syscall that migrated names the +syscall that last entered on the CPU it faulted on. + +`syscall_entry` already pushes the user `rsp`, the user `rip` and the number at +the top of the thread's own kernel stack, which moves with the thread; reading +them there would delete every per-CPU copy. The frame alone cannot say whether +it is a syscall's: an interrupt from Ring 3 puts `SS` and `CS` in the slots +where a syscall's frame holds the user's `rsp` and `rdi`, and userland chooses +both. + +**Evidence:** read from the code; no test stages it. + +**Exit condition:** the syscall context a crash report or a handle fault prints +is the current thread's, whichever CPU it entered on, and a guest test stages +both migrations: an interrupt-context death on the first CPU whose report +carries no `Syscall:` line, and a death inside a syscall that parked while +another thread's syscall ended on its CPU, whose report carries its own. diff --git a/issues/panic-path/a-syscall-panic-reset-the-guest-announced-and-qemu-never-saw.md b/issues/panic-path/a-syscall-panic-reset-the-guest-announced-and-qemu-never-saw.md new file mode 100644 index 00000000000..345b114846c --- /dev/null +++ b/issues/panic-path/a-syscall-panic-reset-the-guest-announced-and-qemu-never-saw.md @@ -0,0 +1,53 @@ +--- +status: assigned +kind: defect +opened: 2026-09-28 +--- + +# A syscall-panic reset the guest announced and QEMU never saw + +**Owner: the panic path, held by the orchestrator.** + +At `f9700b90`, `cargo test --test toyos-build -- --nightly syscall_panic_halts` +went red once (EXIT=1). The guest's own serial shows the panic, the reboot +countdown and its last line before reset: + +``` +panic: rebooting in 5 s unless a key is pressed, timed by the calibrated clock +panic: no key inside the bound, so nobody is here: returning this machine to firmware +``` + +— the guest announced that it was resetting. The harness reported: + +``` +syscall_panic_halts: QEMU never reported stopping: the kernel died inside a syscall and the machine carried on without its caller +``` + +QEMU's own `SHUTDOWN` event, the independent oracle `power::syscall_death_resets` +reads, never arrived inside the row's wait. The run took 99.8 s at a 2.96x +liveness width (fastest boot 3908 ms against a 1320 ms reference). + +**The one mechanism proposed for this is measured and does not reproduce it.** +`panic-path/the-panic-lock-spin-limit-is-a-count-its-comment-calls-a-second.md` +held the panic path's console wire across the same reset (`wire-held-red.patch`, +`core::mem::forget(serial::try_wire())` before `acpi::reboot`) and got EXIT=0 at +`8e525da4` in 27 s, against 7–8 s for the fixed path at the same head — the held +wire costs about 19–20 s, not the ~95 s this red ran before giving up, and it +still reset inside budget. A held console wire is therefore not what happened +here, and nothing else measured explains it. + +The panic path no longer waits on that wire at all (`reboot_now` now resets +through `acpi::reset_now`, not `acpi::reboot`), so this exact path is gone — +but the mechanism that produced the red is still unknown, and nothing rules +out another one reaching the same symptom: a guest that says it is resetting +and a QEMU that reports nothing. + +**Evidence:** the log above, in full, at +`scratchpad/nokthread-r6/f9700b90-syscall_panic_halts-red.log` on +`wt/toyos-nokthread`. + +**Exit condition:** either this reproduces again with enough captured around +it (a QMP trace, the guest's own reset timestamp, a second independent signal) +to name the real cause, or `syscall_panic_halts` runs clean often enough on the +reset-through-`reset_now` path that the risk is retired — recorded with the +run count that justifies it, not asserted. diff --git a/issues/panic-path/panic-holding-process-table-hangs.md b/issues/panic-path/panic-holding-process-table-hangs.md deleted file mode 100644 index 8a2f753b61e..00000000000 --- a/issues/panic-path/panic-holding-process-table-hangs.md +++ /dev/null @@ -1,30 +0,0 @@ ---- -status: open -kind: defect -opened: 2026-07-30 ---- - -# A panic while holding `PROCESS_TABLE` hangs the panicking CPU - -`try_recover_from_panic` lands in `sched::driver::idle_loop`, whose -`reap_poisoned` takes that lock unconditionally every iteration, and the dead -thread never releases it. Pre-existing and unchanged by the panic-recovery fix; a -`try_lock` could not have saved it either, since a spinlock's `try_lock` fails -for its own holder too. The general shape — locks a dead thread can strand — -belongs to the capability-handles/ownership work. - -**The VFS lock is the same shape**, and it was the one that bit first: a -`read_dir` over 32,769 files panicked inside `vfs::lock()`, and every later -filesystem operation on the machine spun on it. Measured after `889d611` — the -process was killed and the harness still got its end marker, because the test -runner's report path does not touch the VFS. That particular route is bounded -now (`issues/isolation/`), but the class is not: any panic under `vfs::lock()` still strands it, -and the allocator was only the worst instance because every context allocates. - -**`sys_mmap` was another instance.** A `PROT_NONE` mmap of a length near -`u64::MAX` passed the old size guard, then overflowed under both the -process-data lock and the address-space lock. Reverting #543's fix onto -16d2e645 and running `mmap_prot` reproduces it: `DEADLOCK at -src/process.rs:807:25: 500M spins, ticket=19 now=18`, both locks stranded by -the one panic. #543 moves the check ahead of both locks; the class this issue -tracks is unchanged. diff --git a/issues/panic-path/the-discards-refusal-branch-is-exercised-by-nothing.md b/issues/panic-path/the-discards-refusal-branch-is-exercised-by-nothing.md deleted file mode 100644 index 7693d5092c1..00000000000 --- a/issues/panic-path/the-discards-refusal-branch-is-exercised-by-nothing.md +++ /dev/null @@ -1,34 +0,0 @@ ---- -status: open -kind: tooling -opened: 2026-09-03 ---- - -# `discard_capture`'s refusal branch is a comment with no arm behind it, and the judge over it reads presence and not absence - -Two holes left where `screen_survived_panic_not_blamed` closed the discard's -happy path (PR #382). - -`kernel/src/drivers/panic_console/mod.rs:512-517` says a refused discard leaves -the latch owned so a survived panic can still be painted as the cause of death, -and that `CAPTURE_ACCESS` refuses only under a fatal reader. Nothing executes -that branch: reaching it needs a fatal reader live while a recovering CPU -discards, and no test in the tree arranges the two at once. It is the shape -w4-common names — a refusal-reason whose hazard nothing has fired. - -The judge is the second hole. `screen_survived_panic_not_blamed` asserts the -panel **contains** `FATAL_HALT_NONCE`, never that the survived panic's own text -is **absent**. A panel carrying both — a composited paint, or a live tail whose -window still holds the first panic — passes it. The absence assertion is not -one line: `live_tail` renders the whole retained ring, which legitimately holds -the first panic's message on the green arm too, so distinguishing "painted from -the stale snapshot" from "painted live" needs a marker the snapshot cannot -carry rather than a text the ring may. - -Site: `kernel/src/drivers/panic_console/mod.rs:512-523`, `kernel-loom/tests/panic_capture.rs` -(which models the latch and not the wiring), and `tests/toyos.rs`'s -`screen_survived_panic_not_blamed`. - -Exit: an actuator holding a fatal reader across a recovering CPU's discard, and -a second marker written between the two deaths that the frozen snapshot cannot -hold, asserted absent from the panel. diff --git a/issues/panic-path/the-panic-lock-spin-limit-is-a-count-its-comment-calls-a-second.md b/issues/panic-path/the-panic-lock-spin-limit-is-a-count-its-comment-calls-a-second.md new file mode 100644 index 00000000000..ed33d6b0bd7 --- /dev/null +++ b/issues/panic-path/the-panic-lock-spin-limit-is-a-count-its-comment-calls-a-second.md @@ -0,0 +1,18 @@ +--- +status: open +kind: defect +opened: 2026-09-27 +--- + +# The panic lock spin limit is a count its comment calls a second + +`PANIC_LOCK_SPIN_LIMIT` in `kernel/src/drivers/serial.rs` bounds two waits: +`panic_flush`'s for `BackendGuard`, and `flush_final`'s for the console wire. +It is 100,000,000 iterations of a `try_lock` and a `pause`, and its comment +calls that "~1s of spin". Nothing converts it to time, so the wait lasts +whatever that many `pause`s cost on the CPU running them. + +**Evidence:** the code. + +**Exit condition:** both waits are bounded by time against the counter the +panic path already times its bound with, and the comment says what is measured. diff --git a/kernel-loom/Cargo.toml b/kernel-loom/Cargo.toml index a389a4896db..4253a8d93d9 100644 --- a/kernel-loom/Cargo.toml +++ b/kernel-loom/Cargo.toml @@ -69,16 +69,6 @@ lock-acquire-off = [] # Never on by default and never reachable from a kernel build, which declares # the same name only so `cfg` checking knows it. reap-raise-relaxed = [] -# The negative control for the poison bank. It restores the erasing one-slot -# swap `poison_tid` used to make, so a second death on one CPU before its idle -# trip erases the first, and `poison_set.rs` must red: -# -# cargo test --manifest-path kernel-loom/Cargo.toml --features poison-overwrite \ -# --test poison_set -# -# Never on by default and never reachable from a kernel build, which declares -# the same name only so `cfg` checking knows it. -poison-overwrite = [] # The negative control for the shootdown acknowledgement. It makes # `shootdown.rs`'s `serve` read what it owes `Relaxed`, so the flush no longer # postdates the initiator's page-table write, and `tlb_shootdown.rs` must red: diff --git a/kernel-loom/src/lib.rs b/kernel-loom/src/lib.rs index 2f2b84aa8b0..594fd0faa2a 100644 --- a/kernel-loom/src/lib.rs +++ b/kernel-loom/src/lib.rs @@ -196,9 +196,6 @@ pub mod dump_request; #[path = "../../kernel/src/pcidev/record.rs"] pub mod device_irq; -#[path = "../../kernel/src/sched/poison.rs"] -pub mod poison; - /// Durability debt as generations. Pure `core`, so it compiles here unshimmed; /// `tests/durability.rs` drives the kernel's flush protocol over it. #[path = "../../kernel/src/durability.rs"] diff --git a/kernel-loom/tests/panic_capture.rs b/kernel-loom/tests/panic_capture.rs index e274cfc8655..fbfa0690608 100644 --- a/kernel-loom/tests/panic_capture.rs +++ b/kernel-loom/tests/panic_capture.rs @@ -79,7 +79,7 @@ fn the_owner_refreshes_and_a_second_captor_stays_out() { /// A released latch hands the next captor a write ordered after the last owner's. #[test] -fn a_recovered_panic_hands_the_snapshot_to_the_next_captor() { +fn a_released_latch_hands_the_snapshot_to_the_next_captor() { loom::model(|| { let latch = Arc::new(CaptureLatch::new()); let snapshot = Arc::new(UnsafeCell::new(0u32)); @@ -143,53 +143,6 @@ fn a_reader_and_refresh_never_overlap_on_the_snapshot() { }); } -#[test] -fn discard_cannot_admit_a_writer_under_a_fatal_reader() { - loom::model(|| { - let latch = Arc::new(CaptureLatch::new()); - let access = Arc::new(CaptureAccess::new()); - let snapshot = Arc::new(UnsafeCell::new([0u8; 2])); - - assert_eq!(latch.claim(2), Claim::Fresh); - assert!(access.begin_capture()); - snapshot.with_mut(|p| unsafe { *p = [2, 2] }); - access.publish(true); - - let reader = { - let (access, snapshot) = (access.clone(), snapshot.clone()); - thread::spawn(move || { - if access.read() { - snapshot.with(|p| unsafe { *p }) - } else { - [9, 9] - } - }) - }; - let discard = { - let (latch, access) = (latch.clone(), access.clone()); - thread::spawn(move || { - if latch.owned_by(2) && access.discard() { - assert!(latch.release(2)); - } - }) - }; - let next = { - thread::spawn(move || { - if latch.claim(3) == Claim::Fresh && access.begin_capture() { - snapshot.with_mut(|p| unsafe { (*p)[0] = 3 }); - snapshot.with_mut(|p| unsafe { (*p)[1] = 3 }); - access.publish(true); - } - }) - }; - - let read = reader.join().unwrap(); - discard.join().unwrap(); - next.join().unwrap(); - assert!(read == [2, 2] || read == [3, 3] || read == [9, 9]); - }); -} - /// The negative control: the unlatched shape transliterated onto two words, /// collecting a mixed pair so it reds the day loom stops reaching the interleaving. #[test] diff --git a/kernel-loom/tests/poison_set.rs b/kernel-loom/tests/poison_set.rs deleted file mode 100644 index d9c64385791..00000000000 --- a/kernel-loom/tests/poison_set.rs +++ /dev/null @@ -1,66 +0,0 @@ -//! Loom: the panic path's poison bank. -//! -//! `scheduler::POISONED` was one slot per CPU and `poison_tid` swapped into it, -//! so a second death on one CPU before its next idle trip erased the first. -//! The negative case is a cargo feature rather than a comment: -//! -//! ```text -//! cargo test --manifest-path kernel-loom/Cargo.toml --features poison-overwrite \ -//! --test poison_set -//! ``` -//! -//! restores the erasing swap, and [`a_second_death_banks_beside_the_first`] must red. - -use kernel_loom::poison::{PoisonSet, SLOTS}; -use loom::sync::Arc; -use loom::thread; - -/// Two deaths on one CPU before its idle trip: the reaper must be handed both. -#[test] -fn a_second_death_banks_beside_the_first() { - loom::model(|| { - let bank = PoisonSet::new(); - assert!(bank.bank(1), "an empty bank refused the first death"); - assert!(bank.bank(2), "a bank with seven free slots refused the second"); - let mut got = Vec::new(); - bank.drain(|id| got.push(id)); - got.sort_unstable(); - assert_eq!(got, [1, 2], "a banked death was erased"); - }); -} - -/// A death landing while another CPU drains is handed to that drain or the next — exactly once. -#[test] -fn a_death_racing_a_drain_is_never_lost() { - loom::model(|| { - let bank = Arc::new(PoisonSet::new()); - assert!(bank.bank(1)); - let pusher = { - let bank = Arc::clone(&bank); - thread::spawn(move || assert!(bank.bank(2), "a racing death was refused")) - }; - let mut handed = Vec::new(); - bank.drain(|id| handed.push(id)); - pusher.join().unwrap(); - bank.drain(|id| handed.push(id)); - handed.sort_unstable(); - assert_eq!(handed, [1, 2], "a death was lost or handed twice: {handed:?}"); - }); -} - -/// A full bank refuses the overflow and keeps everything it holds. -#[test] -fn past_capacity_the_bank_refuses_loudly() { - loom::model(|| { - let bank = PoisonSet::new(); - for i in 0..SLOTS as u64 { - assert!(bank.bank(i + 1), "slot {i} refused before the bank was full"); - } - assert!(!bank.bank(99), "a full bank claimed to accept a ninth death"); - let mut got = Vec::new(); - bank.drain(|id| got.push(id)); - got.sort_unstable(); - let want: Vec = (1..=SLOTS as u64).collect(); - assert_eq!(got, want, "the refused death displaced a banked one"); - }); -} diff --git a/kernel-loom/tests/reap_gate.rs b/kernel-loom/tests/reap_gate.rs index 64c3a05e4bd..73e126512c5 100644 --- a/kernel-loom/tests/reap_gate.rs +++ b/kernel-loom/tests/reap_gate.rs @@ -26,8 +26,7 @@ //! //! makes `reap_gate.rs`'s `raise` store relaxed and this file must red, at //! [`a_claim_sees_the_enrolled_work`] — *a claimed gate handed the reaper an -//! empty poison slot*, which is the defect stated exactly. Verified 2026-08-17, -//! both ways round. +//! unpublished exit*, which is the defect stated exactly. use kernel_loom::reap_gate::ReapGate; use loom::sync::atomic::{AtomicUsize, Ordering}; @@ -68,24 +67,23 @@ fn a_raise_is_never_dropped() { assert!( claimed || gate.take(), - "the raise was dropped: nobody claimed it and the flag is down, so a poisoned \ - thread's waiter would wait for ever", + "the raise was dropped: nobody claimed it and the flag is down, so a finished \ + process's entry is never collected", ); }); } /// A claimer sees the work the raise was about. /// -/// `poison_tid` writes its slot and *then* raises; `publish_exit` stores -/// `finished` and *then* raises. The reaper reads both with the process table -/// held and nothing else ordering it against the raiser, so the gate's own +/// `publish_exit` stores `finished` and *then* raises. The reaper reads it with +/// the process table held and nothing else ordering it against the raiser, so the gate's own /// release/acquire pair is the whole edge. Weaken `raise` to `Relaxed` and this /// is the model that reds. #[test] fn a_claim_sees_the_enrolled_work() { loom::model(|| { let gate = Arc::new(ReapGate::new()); - // Stands for the poison slot, or for the object's `finished` flag. + // Stands for the object's `finished` flag. let work = Arc::new(AtomicUsize::new(0)); let raiser = { @@ -100,7 +98,7 @@ fn a_claim_sees_the_enrolled_work() { assert_eq!( work.load(Ordering::Relaxed), 1, - "a claimed gate handed the reaper an empty poison slot — the raise did not \ + "a claimed gate handed the reaper an unpublished exit — the raise did not \ carry the work it was raised for", ); } diff --git a/kernel/Cargo.toml b/kernel/Cargo.toml index 70caaf26652..58521c747cc 100644 --- a/kernel/Cargo.toml +++ b/kernel/Cargo.toml @@ -44,9 +44,6 @@ lock-acquire-off = [] # `src/sched/reap_gate.rs`'s `raise` store goes `Relaxed`, and `reap_gate` # reds. reap-raise-relaxed = [] -# `src/sched/poison.rs`'s bank becomes the erasing one-slot swap, and -# `poison_set` reds. -poison-overwrite = [] # `src/shootdown.rs`'s `serve` reads what it owes `Relaxed`, and # `tlb_shootdown` reds. shootdown-serve-relaxed = [] diff --git a/kernel/src/actuator.rs b/kernel/src/actuator.rs index 9be7cfc5912..23b8aa6e8ee 100644 --- a/kernel/src/actuator.rs +++ b/kernel/src/actuator.rs @@ -465,8 +465,8 @@ actuators! { /// Panic inside `klogd` on its first instruction. klogd_panic = "klogd-panic"; - /// Panic inside `usbd` on its first instruction. - usbd_panic = "usbd-panic"; + /// Read address zero inside `klogd` on its first instruction. + klogd_fault = "klogd-fault"; /// Stop the boot dead in phase 3, interrupts off, before any log drain. pre_idle_wedge = "pre-idle-wedge"; diff --git a/kernel/src/arch/aarch64/hw.rs b/kernel/src/arch/aarch64/hw.rs index 472b8b6a4b4..c7f2adf0255 100644 --- a/kernel/src/arch/aarch64/hw.rs +++ b/kernel/src/arch/aarch64/hw.rs @@ -24,8 +24,6 @@ impl Kicker for KernelHw { } impl Machine for KernelHw { - type IrqGuard = crate::arch::IrqGuard; - fn now(&self) -> Nanos { Nanos(crate::clock::nanos_since_boot()) } @@ -38,10 +36,6 @@ impl Machine for KernelHw { owed!("the timer", "stage 4") } - fn irq_guard(&self) -> crate::arch::IrqGuard { - crate::arch::IrqGuard::close() - } - fn halt(&self) { owed!("the interrupt controller", "stage 4") } diff --git a/kernel/src/arch/aarch64/percpu.rs b/kernel/src/arch/aarch64/percpu.rs index d9f26a7df18..065865a0774 100644 --- a/kernel/src/arch/aarch64/percpu.rs +++ b/kernel/src/arch/aarch64/percpu.rs @@ -66,10 +66,6 @@ pub fn idle_stack_high_water() -> usize { owed!("per-CPU state", "stage 4") } -pub fn in_syscall() -> bool { - owed!("per-CPU state", "stage 4") -} - pub fn syscall_num() -> u64 { owed!("per-CPU state", "stage 4") } diff --git a/kernel/src/arch/aarch64/trap.rs b/kernel/src/arch/aarch64/trap.rs index 24683360a39..4747673475d 100644 --- a/kernel/src/arch/aarch64/trap.rs +++ b/kernel/src/arch/aarch64/trap.rs @@ -186,10 +186,6 @@ pub(crate) const fn frame_interrupts_enabled(spsr: u64) -> bool { /// Nothing to report: the vectors run on the stack they interrupted. pub(crate) fn report_fault_stack() {} -pub(crate) fn try_recover_from_panic() -> ! { - owed!("recovering a syscall's panic", "stage 7") -} - pub fn kernel_exit_to_user_check() { owed!("the return to user mode", "stage 7") } diff --git a/kernel/src/arch/x86_64/hw.rs b/kernel/src/arch/x86_64/hw.rs index 902433dee72..2ff7f2e607a 100644 --- a/kernel/src/arch/x86_64/hw.rs +++ b/kernel/src/arch/x86_64/hw.rs @@ -33,8 +33,6 @@ impl Kicker for KernelHw { } impl Machine for KernelHw { - type IrqGuard = crate::arch::IrqGuard; - fn now(&self) -> Nanos { Nanos(crate::clock::nanos_since_boot()) } @@ -50,10 +48,6 @@ impl Machine for KernelHw { apic::stop_timer(); } - fn irq_guard(&self) -> crate::arch::IrqGuard { - crate::arch::IrqGuard::close() - } - fn halt(&self) { // SAFETY: `sti; hlt` is the atomic enable-and-wait pair — a wake landing between the two is not lost. unsafe { asm!("sti; hlt", options(nomem, nostack)); } diff --git a/kernel/src/arch/x86_64/idt/exceptions.rs b/kernel/src/arch/x86_64/idt/exceptions.rs index 7c095df69c0..dce2ffc825a 100644 --- a/kernel/src/arch/x86_64/idt/exceptions.rs +++ b/kernel/src/arch/x86_64/idt/exceptions.rs @@ -1,10 +1,10 @@ use crate::arch::{cpu, percpu}; use crate::syscall; use crate::arch::percpu::CpuFaultState; -use crate::{alert, log, mm, process, scheduler, symbols}; +use crate::{alert, log, mm, process, symbols}; use crate::symbols::kernel_backtrace; -use toyos_userbound::{blame, Blame, Faulted, Ring}; +use toyos_userbound::Ring; use super::{Vector, TrapFrame, PF_PRESENT, PF_WRITE, PF_INSTRUCTION_FETCH}; @@ -103,20 +103,6 @@ impl ExceptionContext<'_> { fn ring(&self) -> Ring { Ring::of_cs(self.frame.cs) } - - /// CR2 is meaningful on a #PF and stale on every other vector. - fn faulted(&self) -> Faulted { - if self.vector() == Vector::PageFault { - Faulted::Address(self.cr2) - } else { - Faulted::Nothing - } - } - - /// Whose fault it was. See `toyos_userbound::fault`. - fn blame(&self) -> Blame { - blame(self.ring(), self.frame.rip, self.faulted(), percpu::current_tid().is_some()) - } } // DESIGN RULE: crash_report and everything it calls must stay panic-free — no @@ -168,10 +154,6 @@ pub(crate) fn crash_report(info: &CrashInfo) { } fn crash_report_exception(ctx: &ExceptionContext) { - // `theirs` (who is blamed) and `ring3` (which report format) are separate - // questions: a syscall fault is the process's fault even though the frame - // is Ring 0, with a kernel `rip` and a kernel-stack `rbp`. - let theirs = ctx.blame() != Blame::Kernel; let ring3 = ctx.ring().is_user(); let tid = percpu::current_tid().unwrap_or(crate::process::Tid(0)); let pid = percpu::current_pid(); @@ -190,7 +172,7 @@ fn crash_report_exception(ctx: &ExceptionContext) { let name = vector_name(ctx.vector()); - if theirs { + if ring3 { match ctx.vector() { Vector::PageFault => log!("SEGFAULT tid={}: {} {} at {:#x}", tid, pf_action, pf_cause, ctx.cr2), Vector::InvalidOpcode => log!("SIGILL tid={}: illegal instruction", tid), @@ -243,7 +225,7 @@ fn crash_report_exception(ctx: &ExceptionContext) { // // Only for kernel faults — a Ring 3 segfault says nothing about which CPU // is on which kernel stack, and would bury the report about the process. - if !theirs { + if !ring3 { crate::hw::report_contexts(ctx.frame.rsp, None); } @@ -278,7 +260,7 @@ fn crash_report_exception(ctx: &ExceptionContext) { } } - if theirs { + if ring3 { let crash_addr = if ctx.vector() == Vector::PageFault { ctx.cr2 } else { 0 }; process::dump_crash_diagnostics(crash_addr, ctx.frame.rip); } @@ -336,40 +318,6 @@ fn crash_report_panic(info: &core::panic::PanicInfo, rbp: u64) { } } -/// Terminate after a fatal fault, by [`Blame`] — its three states are -/// exhaustive; there is no fourth case to write. -pub(crate) fn recover_or_halt(blame: Blame) -> ! { - match blame { - // True user-mode fault — no kernel locks held, safe to use normal exit. - Blame::Process => { - percpu::set_fault_state(CpuFaultState::Normal); - crate::panic::forget(); - syscall::kill_process(-1); - } - // Kernel fault on the thread's behalf — may hold locks, use try_lock path. - Blame::ProcessThroughKernel => try_recover_from_panic(), - Blame::Kernel => crate::panic::halt_all_cpus(), - } -} - -/// Recovers from a panic in syscall context: poisons the faulted thread for -/// the idle loop to reap, then rejoins the scheduler lock-free. -// Never touches the process table: the faulted thread may hold its lock, so -// only the poison set (read by the idle loop) is safe to use here. -pub(crate) fn try_recover_from_panic() -> ! { - if let Some(tid) = percpu::current_tid() { - let pid = percpu::current_pid().unwrap_or(crate::process::Pid(u32::MAX)); - scheduler::poison_tid(scheduler::TaskId(pid, tid)); - } - percpu::set_fault_state(CpuFaultState::Normal); - // Clears this CPU's captured fault, the same evidence - // `panic_console::discard_capture` clears on the panic path: left - // standing, the next DOUBLE PANIC here would misname an already-survived - // crash. - crate::panic::forget(); - scheduler::schedule_no_return(); -} - /// Double fault handler — runs on IST1. Always from kernel. Never returns. pub(super) fn double_fault_handler(frame: &TrapFrame) -> ! { @@ -494,17 +442,14 @@ pub(super) fn page_fault_handler(frame: &TrapFrame) { // Only handle not-present faults — protection violations are always fatal if frame.error_code & PF_PRESENT == 0 { let is_user = Ring::of_cs(frame.cs).is_user(); - if is_user || percpu::current_tid().is_some() { - if process::handle_page_fault(fault_addr, frame.error_code) { - percpu::set_fault_state(percpu::CpuFaultState::Normal); - return; - } - log!("#PF UNHANDLED: cr2={:#x} rip={:#x} err={:#x} user={} tid={:?}", - fault_addr, frame.rip, frame.error_code, is_user, percpu::current_tid()); - } else { - log!("#PF SKIP: cr2={:#x} rip={:#x} err={:#x} (no tid, not user)", - fault_addr, frame.rip, frame.error_code); + // Ring 3 only: a Ring 0 fault is a kernel bug, and it halts below + // before anything is mapped into whichever process is current. + if is_user && process::handle_page_fault(fault_addr, frame.error_code) { + percpu::set_fault_state(percpu::CpuFaultState::Normal); + return; } + log!("#PF UNHANDLED: cr2={:#x} rip={:#x} err={:#x} user={} tid={:?}", + fault_addr, frame.rip, frame.error_code, is_user, percpu::current_tid()); } else { log!("#PF PRESENT: cr2={:#x} rip={:#x} err={:#x} cs={:#x}", fault_addr, frame.rip, frame.error_code, frame.cs); @@ -524,7 +469,6 @@ pub(super) fn exception_handler(frame: &TrapFrame) -> ! { /// Core fatal exception logic. Prints diagnostics, then kills process or halts all CPUs. fn fatal_exception(ctx: &ExceptionContext) -> ! { - let blame = ctx.blame(); let prev = percpu::swap_fault_state(CpuFaultState::Fatal); let recursive = prev == CpuFaultState::Fatal || prev == CpuFaultState::Panic; @@ -549,17 +493,18 @@ fn fatal_exception(ctx: &ExceptionContext) -> ! { ctx.frame.rip, ctx.cr2, ctx.frame.error_code, cpu::read_cr3(), ctx.frame.rsp, tid_raw); } - // Recursive fault: no second report. Even ProcessThroughKernel can't - // survive `try_recover_from_panic`'s rejoin here, so end the process or halt. + // Recursive fault: no second report. if recursive { - if blame != Blame::Kernel { - percpu::set_fault_state(CpuFaultState::Normal); - crate::panic::forget(); - syscall::kill_process(-1); - } crate::panic::halt_all_cpus(); } crash_report(&CrashInfo::Exception(ctx)); - recover_or_halt(blame); + // A Ring 3 fault holds no kernel lock, so the ordinary exit ends its + // process; every Ring 0 fault is the kernel's, whatever thread is current. + if ctx.ring().is_user() { + percpu::set_fault_state(CpuFaultState::Normal); + crate::panic::forget(); + syscall::kill_process(-1); + } + crate::panic::halt_all_cpus(); } diff --git a/kernel/src/arch/x86_64/idt/mod.rs b/kernel/src/arch/x86_64/idt/mod.rs index 1bc2744e6ca..3c7d56d78a2 100644 --- a/kernel/src/arch/x86_64/idt/mod.rs +++ b/kernel/src/arch/x86_64/idt/mod.rs @@ -520,8 +520,6 @@ pub fn enable_interrupts() { cpu::enable_interrupts(); } -pub(crate) use exceptions::try_recover_from_panic; - /// The crash report for a panic, from the frame pointer the panic handler stood on. pub(crate) fn report_panic(message: &core::panic::PanicInfo, frame: u64) { exceptions::crash_report(&exceptions::CrashInfo::Panic { message, rbp: frame }); diff --git a/kernel/src/arch/x86_64/percpu.rs b/kernel/src/arch/x86_64/percpu.rs index e042c7f17a1..f70b189345e 100644 --- a/kernel/src/arch/x86_64/percpu.rs +++ b/kernel/src/arch/x86_64/percpu.rs @@ -759,7 +759,6 @@ pub fn leave_syscall() { } /// Whether the task this CPU is running is inside a syscall right now, comparing identity rather than a flag since the word is per-CPU but the question is per-thread. -/// Errs false on a migrated/resumed syscall, so a panic there halts rather than hiding. pub fn in_syscall() -> bool { let recorded = gs::read_u64::(); recorded != NO_SYSCALL diff --git a/kernel/src/arch/x86_64/syscall.rs b/kernel/src/arch/x86_64/syscall.rs index 9c21bdbe37c..46d448f9202 100644 --- a/kernel/src/arch/x86_64/syscall.rs +++ b/kernel/src/arch/x86_64/syscall.rs @@ -172,8 +172,6 @@ extern "sysv64" fn syscall_entry() { } /// The syscall bracket: the entry's diagnostic stores stay readable only while [`percpu::in_syscall`] is true. -/// -/// Not a guard type: a panic here does not unwind, so the panic handler must find the bracket still open to decide whether to kill the process. extern "sysv64" fn syscall_handler(num: u64, a1: u64, a2: u64, _: u64, a3: u64, a4: u64) -> u64 { #[cfg(feature = "df-witness")] cpu::df_witness("syscall_handler"); diff --git a/kernel/src/drivers/panic_console/access.rs b/kernel/src/drivers/panic_console/access.rs index 0c2b9cc0f0a..7a3faf55a22 100644 --- a/kernel/src/drivers/panic_console/access.rs +++ b/kernel/src/drivers/panic_console/access.rs @@ -2,8 +2,8 @@ //! and `READY` -> `READING` once and for good. //! //! **`READING` is terminal.** Every writer transition here demands `EMPTY` or -//! `READY`, so once a fatal reader has entered, no capture, refresh or discard -//! runs on this snapshot again and the reader's borrow of it cannot be aliased. +//! `READY`, so once a fatal reader has entered, no capture or refresh runs +//! on this snapshot again and the reader's borrow of it cannot be aliased. #[cfg(not(feature = "loom"))] use core::sync::atomic::{AtomicU32, Ordering}; @@ -69,18 +69,4 @@ impl CaptureAccess { }); changed.is_ok() } - - pub fn discard(&self) -> bool { - #[cfg(not(feature = "loom"))] - let changed = self.state.try_update(Ordering::AcqRel, Ordering::Acquire, |state| match state { - EMPTY | READY => Some(EMPTY), - _ => None, - }); - #[cfg(feature = "loom")] - let changed = self.state.fetch_update(Ordering::AcqRel, Ordering::Acquire, |state| match state { - EMPTY | READY => Some(EMPTY), - _ => None, - }); - changed.is_ok() - } } diff --git a/kernel/src/drivers/panic_console/latch.rs b/kernel/src/drivers/panic_console/latch.rs index 82f26b3d03d..fc6902fbbfc 100644 --- a/kernel/src/drivers/panic_console/latch.rs +++ b/kernel/src/drivers/panic_console/latch.rs @@ -1,4 +1,4 @@ -//! The capture latch: one writer CPU owns the panic snapshot until recovery and may re-enter. +//! The capture latch: one writer CPU owns the panic snapshot and may re-enter. //! No `crate::` references: `kernel-loom` compiles this file under `feature = "loom"` to drive the real latch in its tests. #[cfg(not(feature = "loom"))] @@ -46,10 +46,6 @@ impl CaptureLatch { } } - pub fn owned_by(&self, token: u32) -> bool { - self.owner.load(Ordering::Acquire) == token - } - /// Give the snapshot back only when `token` owns it. pub fn release(&self, token: u32) -> bool { self.owner diff --git a/kernel/src/drivers/panic_console/mod.rs b/kernel/src/drivers/panic_console/mod.rs index 71ed2178f64..12c37d40e58 100644 --- a/kernel/src/drivers/panic_console/mod.rs +++ b/kernel/src/drivers/panic_console/mod.rs @@ -3,9 +3,8 @@ //! Renders log records as an 8x16 text grid onto the UEFI GOP framebuffer, //! through `LogRecord`'s `Display`, so no second formatter can drift from //! `logd`. [`capture`] freezes the report before `panic_flush` drains it; -//! [`render`] paints it inside `halt_all_cpus`, before `panic_flush`. A -//! recovered panic must call [`discard_capture`]. virtio-gpu is -//! unsupported: its scanout needs the unbounded-poll wedge this module avoids. +//! [`render`] paints it inside `halt_all_cpus`, before `panic_flush`. virtio-gpu +//! is unsupported: its scanout needs the unbounded-poll wedge this module avoids. //! //! The two holds this module ends a panic in — [`page_forever`] and //! [`hold_the_panel`] — are also where `crate::panic_reboot`'s bound is @@ -259,7 +258,7 @@ static EARLY: AtomicBool = AtomicBool::new(false); static SNAPSHOT: RenderedCell = RenderedCell(UnsafeCell::new(Rendered::EMPTY)); static CAPTURE_ACCESS: access::CaptureAccess = access::CaptureAccess::new(); -/// `SNAPSHOT`'s writer owner until recovery; the owner may refresh and every other CPU is refused. +/// `SNAPSHOT`'s writer owner; the owner may refresh and every other CPU is refused. static CAPTURE: latch::CaptureLatch = latch::CaptureLatch::new(); /// The early branch's token: percpu is not up there, and exactly one CPU exists. @@ -289,8 +288,7 @@ static CLAIMED_AT: core::sync::atomic::AtomicU64 = core::sync::atomic::AtomicU64 #[cfg(feature = "boot-actuators")] const PROBE_DELAY_NS: u64 = 5_000_000_000; -/// Whether the `metal-panic-probe` boot should panic now. Called from the -/// idle loop, whose fall-through skips recovery — the only path that never paints. +/// Whether the `metal-panic-probe` boot should panic now. #[cfg(feature = "boot-actuators")] pub fn probe_due() -> bool { if !crate::actuator::metal_panic_probe() { @@ -556,19 +554,6 @@ fn captor_token() -> u32 { } } -/// Drop the captured report: this panic was survived. Called only on the recovery branch. -/// -/// A refused discard leaves the latch owned and the report standing, so a -/// survived panic can still be painted as the cause of death: `CAPTURE_ACCESS` -/// refuses only under a fatal reader, and admitting a fresh captor beneath that -/// reader's live borrow is the worse of the two. -pub fn discard_capture() { - let token = captor_token(); - if CAPTURE.owned_by(token) && CAPTURE_ACCESS.discard() { - let _ = CAPTURE.release(token); - } -} - /// Re-freeze the captured report so a line written *after* [`capture`] is /// painted; only refreshes a capture that already exists — [`live_tail`] already reads live otherwise. /// Its one caller is `apic::halt_all_cpus` on a machine with no serial diff --git a/kernel/src/drivers/xhci/mod.rs b/kernel/src/drivers/xhci/mod.rs index f5c2efb8bea..0eb02818fc5 100644 --- a/kernel/src/drivers/xhci/mod.rs +++ b/kernel/src/drivers/xhci/mod.rs @@ -2,7 +2,6 @@ mod device; mod hid; mod legacy; pub mod stop; -pub mod usbd; mod wait; use wait::msc; diff --git a/kernel/src/drivers/xhci/stop.rs b/kernel/src/drivers/xhci/stop.rs index b76afe0e41a..03504f6bf87 100644 --- a/kernel/src/drivers/xhci/stop.rs +++ b/kernel/src/drivers/xhci/stop.rs @@ -99,8 +99,8 @@ static POINTS: [Point; SLOTS] = [Point::EMPTY, Point::EMPTY, Point::EMPTY, Point /// What the shutdown's flush pass emptied, for the one summary line. /// /// **Two atomics and not a return value.** The flush runs above the boot's last -/// word, where a record still reaches the file; this summary is written from -/// `acpi::reboot`, below it. Nothing carries a value across that, and a reset +/// word, where a record still reaches the file; this summary is written +/// below it. Nothing carries a value across that, and a reset /// that flushed nothing — every panic — reads the zeros it was born with. static DISKS: AtomicU32 = AtomicU32::new(0); static FLUSHED: AtomicU32 = AtomicU32::new(0); @@ -440,8 +440,7 @@ fn settle_commands(said: &mut dyn fmt::Write) { /// Entered once for the machine's life. /// -/// **A panic inside this path would otherwise reach it a second time**: the -/// panic path ends in `acpi::reboot`, which is the very site that calls this. +/// **A panic inside this path would otherwise reach it a second time.** static STOPPING: AtomicBool = AtomicBool::new(false); /// Record a controller this kernel drives, so a reset can stop it without the @@ -636,9 +635,8 @@ fn stop_all(said: &mut dyn fmt::Write) { /// Hand every USB device back, then let the caller end the machine. /// -/// **Called from `acpi::reboot` and `acpi::shutdown` and nowhere else**, which -/// is what makes "no reset this kernel performs leaves a USB device -/// mid-command" a property of the reset rather than of whoever asked for one. +/// Makes "no reset this kernel performs leaves a USB device mid-command" a +/// property of the reset rather than of whoever asked for one. pub fn before_reset() { if STOPPING.swap(true, Ordering::AcqRel) { return; diff --git a/kernel/src/drivers/xhci/usbd.rs b/kernel/src/drivers/xhci/usbd.rs deleted file mode 100644 index ab36080e598..00000000000 --- a/kernel/src/drivers/xhci/usbd.rs +++ /dev/null @@ -1,40 +0,0 @@ -//! `usbd`: the kernel thread reserved for the xHCI port machine; the body only parks. - -use toyos_sched::task::WaitClass; - -use crate::watch; -use crate::sched::kthread::{self, OnPanic}; -use crate::scheduler; -use crate::time::Deadline; - -/// The name `sched::dump`, `ps` and a crash report use. -const NAME: &str = "usbd"; - -/// Start the thread. Called once, from `kernel_main`, beside `klogd`'s. -pub fn start() { - // Unconditional: gating this on a controller would give the kernel's thread count a second answer. - let _ = kthread::spawn(NAME, body, 0, OnPanic::Recover); -} - -extern "C" fn body(_arg: u64) -> ! { - // Must stay first: `actuator.rs` promises this panics on usbd's first instruction. - #[cfg(feature = "boot-actuators")] - if crate::actuator::usbd_panic() { - panic!("usbd-panic: the device thread died"); - } - - let parkable = scheduler::Parkable::at_entry(); - let handle = crate::sched::driver::current_handle().expect("usbd runs as a task"); - // Armed once for the loop: rearming here would drop a pending wake. - let armed = watch::arm( - handle.watch(), - 0, - WaitClass::Io, - ) - .expect("a kernel thread is a task and can arm"); - loop { - // No deadline: only an interrupt or a port's own deadline should wake this. - // The cancel arm never fires: nothing retires a kernel thread. - let _ = watch::wait(&parkable, &armed, Deadline::never()); - } -} diff --git a/kernel/src/iod.rs b/kernel/src/iod.rs index 600d9e32cc5..263612ff59a 100644 --- a/kernel/src/iod.rs +++ b/kernel/src/iod.rs @@ -3,7 +3,7 @@ use toyos_sched::task::WaitClass; use crate::watch; -use crate::sched::kthread::{self, OnPanic}; +use crate::sched::kthread; use crate::scheduler; use crate::time::Deadline; @@ -12,8 +12,7 @@ const NAME: &str = "iod"; /// Spawns the `iod` kthread; call once, from `kernel_main`. pub fn start() { - // Recoverable: unlike klogd's silent loss, a killed iod's stalled write-back is visible to SYS_FSYNC and logd. - let _ = kthread::spawn(NAME, body, 0, OnPanic::Recover); + let _ = kthread::spawn(NAME, body, 0); } extern "C" fn body(_arg: u64) -> ! { diff --git a/kernel/src/log/console.rs b/kernel/src/log/console.rs index 4e9b1a23451..8b49f3bff0d 100644 --- a/kernel/src/log/console.rs +++ b/kernel/src/log/console.rs @@ -7,10 +7,6 @@ //! It holds the wire ([`serial::wire`]) with interrupts on and //! preemption allowed, and the registers only for one burst at a time. //! -//! `klogd`'s row in `sched::kthread` is [`OnPanic::Halt`]: it is the only -//! console drainer, and its death must not go silent. Records keep committing -//! to their shards regardless of `klogd`; only the live console is lost while -//! it is down. //! [`Drain::Inline`] and [`Drain::Thread`] are phases, not fallbacks: exactly //! one is active, and `Drain::Inline` *is* [`KLOGD`] being null. @@ -25,7 +21,7 @@ use toyos_sched::park::notify; use crate::drivers::serial::{self, BackendGuard, MAX_CONSOLE_LINE}; use crate::hw::HW; use crate::sched::driver::{cpus, irq_off}; -use crate::sched::kthread::{self, OnPanic}; +use crate::sched::kthread; use crate::sleeplock::SleepGuard; use crate::watch; use crate::sched::payload::KShared; @@ -67,7 +63,7 @@ static DRAINED: Published = Published::new(); /// Start the thread. Called once, from `kernel_main`, before the scheduler starts. /// Placement matters: APs spin until the machine is released, so an earlier spawn could not run while the machine has no console. pub fn start() { - let sched = kthread::spawn(NAME, body, 0, OnPanic::Halt); + let sched = kthread::spawn(NAME, body, 0); // Leaked: `klogd` never exits, and a producer reading this pointer under lock may not touch a refcount. let shared: &'static Arc = alloc::boxed::Box::leak(alloc::boxed::Box::new(sched.shared)); KLOGD.store(shared as *const _ as *mut _, Ordering::Release); @@ -426,6 +422,12 @@ extern "C" fn body(_arg: u64) -> ! { if crate::actuator::klogd_panic() { panic!("klogd-panic: the console drainer died"); } + #[cfg(feature = "boot-actuators")] + if crate::actuator::klogd_fault() { + // SAFETY: unsound by design — a staged Ring 0 null read, only on this actuator's boot. + // Volatile: a plain read could be optimized to unreachable, leaving nothing to fault. + unsafe { core::ptr::read_volatile(core::ptr::null::()) }; + } let parkable = scheduler::Parkable::at_entry(); let handle = crate::sched::driver::current_handle().expect("klogd runs as a task"); diff --git a/kernel/src/log/nested.rs b/kernel/src/log/nested.rs index 5e80f015d60..fb5b423bb55 100644 --- a/kernel/src/log/nested.rs +++ b/kernel/src/log/nested.rs @@ -16,7 +16,7 @@ mod armed { use core::sync::atomic::{AtomicBool, Ordering}; use crate::log::shard::SHARD_RECORDS; - use crate::sched::kthread::{self, OnPanic}; + use crate::sched::kthread; /// One-shot for the body-copy injection point, consumed by `mid_body` or, under `log-shared-reservation`, by the outer `inject`. static ARMED: AtomicBool = AtomicBool::new(false); @@ -43,8 +43,7 @@ mod armed { } crate::log!("lognest start records={SHARD_RECORDS}"); // A kernel thread, not the syscall that arms it: `IF` is clear for a whole syscall, so injecting there would never test the guard. - // `Halt`: this thread carries the whole stimulus; surviving its death would answer the gate having injected nothing. - kthread::spawn("lognest", body, 0, OnPanic::Halt); + kthread::spawn("lognest", body, 0); } extern "C" fn body(_arg: u64) -> ! { diff --git a/kernel/src/log/storm.rs b/kernel/src/log/storm.rs index 0b70d0b0062..a237a19de9e 100644 --- a/kernel/src/log/storm.rs +++ b/kernel/src/log/storm.rs @@ -2,7 +2,7 @@ use core::sync::atomic::{AtomicBool, Ordering}; -use crate::sched::kthread::{self, OnPanic}; +use crate::sched::kthread; // Exceeds a shard's capacity, so the drop path under test is reached at every `--smp` count. const STORM_RECORDS: u64 = 1024; @@ -46,8 +46,7 @@ pub fn start_once() { // The reader parses this line to learn the storm's shape. crate::log!("logstorm start threads={threads} records={STORM_RECORDS}"); for thread in 0..threads { - // `Halt`: a panicked storm thread invalidates the gate's conservation law, so continuing would answer over an incomplete storm. - kthread::spawn("logstorm", body, thread as u64, OnPanic::Halt); + kthread::spawn("logstorm", body, thread as u64); } } diff --git a/kernel/src/main.rs b/kernel/src/main.rs index bada52f0578..98f89dccd80 100644 --- a/kernel/src/main.rs +++ b/kernel/src/main.rs @@ -167,7 +167,6 @@ fn panic(info: &core::panic::PanicInfo) -> ! { arch::trap::report_panic(info, cpu::frame_pointer()); - // Captures now: recovery below may re-enter a scheduler this panic left locked, so a later drain isn't guaranteed. drivers::panic_console::capture(); // One record after the snapshot and before the paint: what tells a frozen // report from a live re-read of a ring siblings are still writing to. @@ -178,15 +177,6 @@ fn panic(info: &core::panic::PanicInfo) -> ! { // SAFETY: IF is clear on this CPU and every other one halts before anything else can write the port. unsafe { drivers::serial::panic_flush(); } - // Recoverable only when a syscall is what's panicking, or a kthread says its own row's answer. - let recoverable = sched::kthread::panic_recovers_here().unwrap_or_else(percpu::in_syscall); - if recoverable { - depth.store(0, core::sync::atomic::Ordering::SeqCst); - // Discarded here: a stale capture would blame this panic for the next fatal one. - drivers::panic_console::discard_capture(); - arch::trap::try_recover_from_panic(); - } - panic::halt_all_cpus(); } @@ -651,7 +641,6 @@ pub(crate) unsafe extern "C" fn kernel_main(kernel_args: &KernelArgs) -> ! { boot_phase!("complete", 0); report_power_on(kernel_args, complete_tsc); - // No current task here, so the handler's recovery predicate fails — the one panic no userland process can produce. #[cfg(feature = "boot-actuators")] if actuator::test_late_panic() { late_panic::Nest:: ! { late_panic::Nest>>>>>>>>>::on_screen_console_check(); } - // Same no-current-task window as above: blame is Kernel, so fatal_exception halts the machine. if actuator::test_kernel_fault() { cpu::undefined_instruction(); } // Last thing before enter_idle_loop: nothing can run before it, and a klogd spawned earlier would idle through phases 5-7 with no drainer. log::console::start(); - // After klogd so their own spawn logs have a drainer. - drivers::xhci::usbd::start(); iod::start(); smp::set_ready(); diff --git a/kernel/src/panic_reboot.rs b/kernel/src/panic_reboot.rs index 30ea48abd79..b36141ae16f 100644 --- a/kernel/src/panic_reboot.rs +++ b/kernel/src/panic_reboot.rs @@ -12,12 +12,10 @@ //! that states no frequency and has no calibrated clock cannot time anything, //! and the arm line says so instead of resetting on a guess. //! -//! The reset is the FADT's, through [`acpi::reboot`]; a machine whose reset -//! register this kernel could not decode is refused by name and holds, with no -//! fallback. That register is decoded before `percpu::init_bsp` loads the IDT, -//! so every panic that can reach this path at all has one to write. -//! -//! [`acpi::reboot`]: crate::drivers::acpi::reboot +//! The reset is the FADT's; a machine whose reset register this kernel could +//! not decode is refused by name and holds, with no fallback. That register is +//! decoded before `percpu::init_bsp` loads the IDT, so every panic that can +//! reach this path at all has one to write. use crate::arch::cpu; use crate::drivers::{acpi, serial}; @@ -165,5 +163,7 @@ pub fn reboot_now() -> ! { b"\npanic: no key inside the bound, so nobody is here: returning this machine to \ firmware\n", ); - acpi::reboot() + // Not `acpi::reboot`: its flush waits on the console wire, and a CPU this + // panic stopped may be holding it. + acpi::reset_now() } diff --git a/kernel/src/process.rs b/kernel/src/process.rs index b89fa88ae70..87025eb7667 100644 --- a/kernel/src/process.rs +++ b/kernel/src/process.rs @@ -30,7 +30,7 @@ use toyos_abi::syscall::EndowEntry; /// The lifecycle's decisions; this file only performs them. pub use toyos_proclife::{ThreadLocation, Watch}; -use toyos_proclife::{join, poison, reap, spawn as proclife_spawn, teardown as proclife, Lifecycle, Processes}; +use toyos_proclife::{join, reap, spawn as proclife_spawn, teardown as proclife, Lifecycle, Processes}; /// One `EndowEntry` on the wire; `loader::start` and [`Endowments::encode`] both index by it. pub const ENDOW_ENTRY_LEN: usize = core::mem::size_of::(); @@ -749,35 +749,8 @@ pub fn mark_thread_zombie(table: &mut ProcessTable, pid: Pid, tid: Tid, code: i3 proclife::mark_zombie(table, pid, tid, code); } -/// What a thread that died in panic recovery leaves to be cleaned up; the panic path itself may hold any lock the faulted thread held, so this only records the thread, and the idle loop runs it later. -#[must_use = "a poisoned thread's waiter must be woken"] -pub enum PoisonWake { - /// A child thread died; the pair is the subject `thread_join` arms on (not the process's main thread). - Joiner(Pid, Tid), - /// The main thread died, so the process is over; publish outside the table lock. - Process(Arc), -} - -/// Mark a poisoned thread dead and say what the idle loop must wake for it. `None`: nothing to do — the entry is gone, or another path already owns teardown. -/// Resources are freed with the table entry rather than before it: every release below wants a lock the faulted thread may still hold. -#[must_use = "a poisoned thread's waiter must be woken"] -pub fn zombify_poisoned(table: &mut ProcessTable, pid: Pid, tid: Tid) -> Option { - match poison::zombify_poisoned(table, pid, tid) { - poison::PoisonOutcome::Nothing => None, - poison::PoisonOutcome::Joiner(watch) => { - let (pid, tid) = watch.thread()?; - Some(PoisonWake::Joiner(pid, tid)) - } - poison::PoisonOutcome::Process(pid) => { - let proc = Processes::get(table, pid) - .expect("zombify_poisoned: the entry it just claimed and marked"); - Some(PoisonWake::Process(Arc::clone(&proc.object))) - } - } -} - /// Take every entry whose process has published its exit. -/// Entries come back rather than being dropped here: the caller holds the table lock, and an entry's drop reaches `remove_vruntime` and, for a process whose teardown never ran, the whole of its `ProcessData`. +/// Entries come back rather than being dropped here: the caller holds the table lock, and an entry's drop reaches `remove_vruntime`. #[must_use = "the reaped entries must be dropped outside the table lock"] pub fn reap_finished(table: &mut ProcessTable, _proof: IdleProof) -> Vec { reap::finished_pids(table).into_iter().filter_map(|pid| table.remove(pid)).collect() @@ -1314,11 +1287,6 @@ pub fn handle_page_fault(fault_addr: u64, _error_code: u64) -> bool { if tid == Tid::MAX { return false; } - // A kernel thread's fault is fatal, said explicitly rather than fallen into by accident: demand paging is a user-mapping mechanism only, and the kernel's direct map is complete. - if crate::sched::kthread::current_is_kernel_thread() { - return false; - } - let (data_arc, addr_space) = { let Some(addr_space) = scheduler::current_address_space() else { return false }; let guard = PROCESS_TABLE.lock(); diff --git a/kernel/src/quiesce.rs b/kernel/src/quiesce.rs index 234fa2911fe..42a598524b7 100644 --- a/kernel/src/quiesce.rs +++ b/kernel/src/quiesce.rs @@ -25,8 +25,7 @@ //! whatever lock the thread on that CPU was holding, and `sync_all` is the //! first thing that would wait on it. //! -//! Kernel threads are exempt by identity, not by accident: `klogd`, `iod` and -//! `usbd` are in the process table like anything else, and +//! Kernel threads are exempt by identity, not by accident: //! [`crate::sched::kthread::is_kernel_task`] is what tells them apart. //! //! # What the stop waits on diff --git a/kernel/src/sched/driver.rs b/kernel/src/sched/driver.rs index 98549e52b9a..43233c12614 100644 --- a/kernel/src/sched/driver.rs +++ b/kernel/src/sched/driver.rs @@ -711,14 +711,12 @@ extern "C" fn idle_loop() -> ! { if crate::actuator::syscall_window_nmi() { crate::arch::syscall::window_storm(); } - // Here, not from a syscall: the panic handler recovers, not paints, when a userland - // thread is current, and the idle loop has none. #[cfg(feature = "boot-actuators")] if crate::drivers::panic_console::probe_due() { panic!("metal-panic-probe: a fatal report over a desktop that owns the screen"); } crate::scheduler::log_health(); - crate::scheduler::reap_poisoned(); + crate::scheduler::reap_finished(); // `pass` below covers this too; here as well so a CPU that // halts immediately has still run every hook first. crate::object::drain_zero_handles(); diff --git a/kernel/src/sched/dump.rs b/kernel/src/sched/dump.rs index 10cdc57dab1..81e23cffb06 100644 --- a/kernel/src/sched/dump.rs +++ b/kernel/src/sched/dump.rs @@ -384,13 +384,11 @@ pub(super) fn deaf_window() { // `rdtsc`, not `nanos_since_boot`: the latter calls into // `compiler_builtins`, which would misname where a stuck CPU is. let until = crate::clock::tsc_deadline(DEAF_NS); - // Not an `IrqGuard`: this must unconditionally set IF on exit, and - // panic recovery may already have left IF clear. - crate::arch::cpu::disable_interrupts(); + let deaf = crate::arch::IrqGuard::close(); while crate::arch::cpu::counter() < until { core::hint::spin_loop(); } - crate::arch::cpu::enable_interrupts(); + drop(deaf); // The victim's own log line is what proves the NMI interrupted it // rather than killed it. let deaf_ms = (crate::clock::nanos_since_boot() - began) / 1_000_000; @@ -705,7 +703,7 @@ fn census() -> Census { // Blocked and running threads are already the CPUs' lines; skip them here. let Some(tag) = tag else { return }; // Kernel threads don't count against the budget: `MAX_KERNEL_TASKS` - // bounds them at three, so counting them can't push these lines off the page. + // bounds them, so counting them can't push these lines off the page. if !kernel { printed += 1; if printed > CENSUS_LINES { diff --git a/kernel/src/sched/kthread.rs b/kernel/src/sched/kthread.rs index 2faf66b2ca9..ada0f30246a 100644 --- a/kernel/src/sched/kthread.rs +++ b/kernel/src/sched/kthread.rs @@ -1,8 +1,7 @@ //! Kernel threads: ordinary tasks that name `mm::paging::kernel` as their //! address space, enter through `loader::kernel_start`, and hold a process-table //! entry. One is preempted or stolen only at a preemption point its body reaches, -//! and a Ring 0 loop reaches none. [`ROWS`] holds every one, and -//! [`panic_recovers_here`] says what a panic inside one means. +//! and a Ring 0 loop reaches none. [`ROWS`] holds every one. use alloc::string::String; use alloc::sync::Arc; @@ -19,37 +18,26 @@ use crate::sync::Lock; use super::payload::ThreadSched; -/// `klogd`, `usbd` and `iod`, plus one `log-storm` thread per shard in the actuator build. +/// `klogd` and `iod`, plus one `log-storm` thread per shard in the actuator build. #[cfg(not(feature = "boot-actuators"))] -const MAX_KERNEL_TASKS: usize = 3; +const MAX_KERNEL_TASKS: usize = 2; #[cfg(feature = "boot-actuators")] -const MAX_KERNEL_TASKS: usize = 3 + toyos_abi::log::MAX_LOG_SHARDS; +const MAX_KERNEL_TASKS: usize = 2 + toyos_abi::log::MAX_LOG_SHARDS; /// Collides with no packed id: neither id map issues `u32::MAX`. const NO_TASK: u64 = u64::MAX; -/// Distinct from a published identity, so the identity can be stored last, -/// after the policy word, without another claimant matching the same row. +/// A reserved row whose identity is not yet known; collides with no packed id. const CLAIMING: u64 = u64::MAX - 1; -/// `task` is stored `Release` after `recoverable` and loaded `Acquire` before it, -/// so a row found by identity never answers with an unwritten policy. -struct Row { - task: AtomicU64, - // `recoverable` exists because `percpu::in_syscall()` is never true for a kernel - // thread, which would otherwise make every kernel-thread panic halt the machine by default. - recoverable: AtomicU64, -} - /// A row reserved before the table lock and published before `enqueue_new`. -struct Claim(&'static Row); +struct Claim(&'static AtomicU64); impl Claim { /// Reserve a row, or panic naming the thread. fn take(name: &str) -> Self { for row in &ROWS { if row - .task .compare_exchange(NO_TASK, CLAIMING, Ordering::Relaxed, Ordering::Relaxed) .is_ok() { @@ -59,46 +47,24 @@ impl Claim { panic!("kthread: {name} is the {}th kernel thread and there is room for {MAX_KERNEL_TASKS}", MAX_KERNEL_TASKS + 1); } - /// Payload first, then the identity `Release`. - fn publish(self, id: TaskId, on_panic: OnPanic) { - self.0 - .recoverable - .store(u64::from(on_panic == OnPanic::Recover), Ordering::Relaxed); - self.0.task.store(id.pack(), Ordering::Release); + fn publish(self, id: TaskId) { + self.0.store(id.pack(), Ordering::Relaxed); } } -/// Registered at spawn and never cleared: a dead `Recover` thread's row stays. -static ROWS: [Row; MAX_KERNEL_TASKS] = - [const { Row { task: AtomicU64::new(NO_TASK), recoverable: AtomicU64::new(0) } }; - MAX_KERNEL_TASKS]; - -/// What a kernel thread's panic does. -#[derive(Clone, Copy, PartialEq, Eq)] -pub enum OnPanic { - /// Ask for `Recover` only when the thread's absence is both survivable and visible. - Recover, - /// `Halt` is `klogd`'s answer: it is the machine's only console drainer, - /// so killing it would leave the machine silently mute. - Halt, -} +/// Registered at spawn and never cleared. +static ROWS: [AtomicU64; MAX_KERNEL_TASKS] = [const { AtomicU64::new(NO_TASK) }; MAX_KERNEL_TASKS]; -/// Lock-free and fault-free, so it may run with any lock held or preemption on. -fn current_row() -> Option<&'static Row> { +/// Is the task this CPU is running a kernel thread? Lock-free and fault-free, +/// so it may run with any lock held or preemption on. +pub fn current_is_kernel_thread() -> bool { let (Some(pid), Some(tid)) = ( crate::arch::percpu::current_pid(), crate::arch::percpu::current_tid(), ) else { - return None; + return false; }; - let packed = TaskId(pid, tid).pack(); - // Pairs with `Claim::publish`'s `Release`: a relaxed load could read an unwritten `recoverable`. - ROWS.iter().find(|row| row.task.load(Ordering::Acquire) == packed) -} - -/// Is the task this CPU is running a kernel thread? -pub fn current_is_kernel_thread() -> bool { - current_row().is_some() + is_kernel_task(TaskId(pid, tid)) } /// Is `id` a kernel thread? @@ -106,16 +72,11 @@ pub fn current_is_kernel_thread() -> bool { // stuck, where taking one could hang diagnostics. pub fn is_kernel_task(id: TaskId) -> bool { let packed = id.pack(); - ROWS.iter().any(|row| row.task.load(Ordering::Acquire) == packed) -} - -/// Whether a panic on the running task recovers; `None` unless it is a kernel thread. -pub fn panic_recovers_here() -> Option { - Some(current_row()?.recoverable.load(Ordering::Relaxed) != 0) + ROWS.iter().any(|row| row.load(Ordering::Relaxed) == packed) } /// Start a kernel thread running `body(arg)` on its own kernel stack and return its scheduler faces. -pub fn spawn(name: &str, body: extern "C" fn(u64) -> !, arg: u64, on_panic: OnPanic) -> ThreadSched { +pub fn spawn(name: &str, body: extern "C" fn(u64) -> !, arg: u64) -> ThreadSched { let (stack, entry_rsp) = crate::loader::alloc_kernel_stack( crate::loader::kernel_start, body as usize as u64, @@ -147,8 +108,7 @@ pub fn spawn(name: &str, body: extern "C" fn(u64) -> !, arg: u64, on_panic: OnPa ) }); let tid = table.get(pid).expect("kthread: the entry just inserted is gone").main_tid(); - // Before `enqueue_new`: from that call the task can run and panic. - claim.publish(TaskId(pid, tid), on_panic); + claim.publish(TaskId(pid, tid)); // The kernel address space, named so one declaration decides every task's `cr3`. let (sched, _dst) = scheduler::enqueue_new( TaskId(pid, tid), @@ -165,15 +125,7 @@ pub fn spawn(name: &str, body: extern "C" fn(u64) -> !, arg: u64, on_panic: OnPa .set_sched(sched.clone()); drop(guard); - crate::log!( - "kthread: {name} pid={} tid={} runs in the kernel address space; a panic in it {}", - pid, - tid, - match on_panic { - OnPanic::Halt => "halts the machine", - OnPanic::Recover => "kills the thread", - } - ); + crate::log!("kthread: {name} pid={pid} tid={tid} runs in the kernel address space"); sched } diff --git a/kernel/src/sched/mod.rs b/kernel/src/sched/mod.rs index f56232bde8a..0a7a33898ba 100644 --- a/kernel/src/sched/mod.rs +++ b/kernel/src/sched/mod.rs @@ -7,7 +7,6 @@ pub mod dump; pub mod dump_request; pub mod kthread; pub mod payload; -pub mod poison; pub mod reap_gate; pub mod futex; diff --git a/kernel/src/sched/poison.rs b/kernel/src/sched/poison.rs deleted file mode 100644 index d44e2aa2e40..00000000000 --- a/kernel/src/sched/poison.rs +++ /dev/null @@ -1,69 +0,0 @@ -//! Hand-off bank for threads that died in panic recovery: a second death on one -//! CPU before its next idle trip banks beside the first instead of erasing it. -//! No `crate::` references: `kernel-loom` compiles this file directly under `feature = "loom"`. - -#[cfg(not(feature = "loom"))] -use core::sync::atomic::{AtomicU64, Ordering}; - -#[cfg(feature = "loom")] -use loom::sync::atomic::{AtomicU64, Ordering}; - -/// The vacant value; a packed id collides with it only if a task's pid and tid are both `u32::MAX`. -pub const EMPTY: u64 = u64::MAX; - -/// Deaths one CPU can bank between idle trips; fixed — the panic path may not allocate. -pub const SLOTS: usize = 8; - -/// A fixed bank of packed task ids, written by the panic path and drained by -/// the idle loop. -pub struct PoisonSet { - slots: [AtomicU64; SLOTS], -} - -impl PoisonSet { - /// Must stay `const`: seeded as a `static`, one per CPU. - #[cfg(not(feature = "loom"))] - pub const fn new() -> Self { - Self { slots: [const { AtomicU64::new(EMPTY) }; SLOTS] } - } - - // No `Default`: `Default::default` can't be `const`, unlike the arm above. - #[allow(clippy::new_without_default)] - #[cfg(feature = "loom")] - pub fn new() -> Self { - Self { slots: core::array::from_fn(|_| AtomicU64::new(EMPTY)) } - } - - /// Bank one packed id; `false` means every slot was full and the caller says so. - /// Each claim is one CAS, so a death taken mid-scan costs a slot, never a loss. - #[cfg(not(feature = "poison-overwrite"))] - pub fn bank(&self, packed: u64) -> bool { - for slot in &self.slots { - if slot - .compare_exchange(EMPTY, packed, Ordering::Release, Ordering::Relaxed) - .is_ok() - { - return true; - } - } - false - } - - // Kernel builds never enable `poison-overwrite`: it restores the erasing - // one-slot swap, and `kernel-loom/tests/poison_set.rs` must red under it. - #[cfg(feature = "poison-overwrite")] - pub fn bank(&self, packed: u64) -> bool { - self.slots[0].swap(packed, Ordering::Release); - true - } - - /// Hand every banked id to `f`, each at most once across every drain. - pub fn drain(&self, mut f: impl FnMut(u64)) { - for slot in &self.slots { - let raw = slot.swap(EMPTY, Ordering::Acquire); - if raw != EMPTY { - f(raw); - } - } - } -} diff --git a/kernel/src/scheduler.rs b/kernel/src/scheduler.rs index ec0f4c03c26..39efc41af0c 100644 --- a/kernel/src/scheduler.rs +++ b/kernel/src/scheduler.rs @@ -31,8 +31,6 @@ pub use crate::sched::driver::{ }; pub use crate::sched::MAX_CPUS; -use crate::sched::poison; - /// Panics unless the preempt depth equals `baseline`: a mismatch means a /// spinlock is held across a scheduler entry that switches. #[track_caller] @@ -565,11 +563,7 @@ pub fn retire_task(sched: &ThreadSched) { } } -/// Per-CPU hand-off bank for threads that died in panic recovery: the panic -/// path may hold any lock, so it can only store here. -static POISONED: [poison::PoisonSet; MAX_CPUS] = [const { poison::PoisonSet::new() }; MAX_CPUS]; - -/// Whether [`reap_poisoned`] has anything to do; claimed by whichever idle +/// Whether [`reap_finished`] has anything to do; claimed by whichever idle /// trip takes the work. static REAP_GATE: ReapGate = ReapGate::new(); @@ -579,85 +573,22 @@ pub fn note_reapable() { REAP_GATE.raise(); } -pub fn poison_tid(id: TaskId) { - let cpu = percpu::cpu_id() as usize; - let Some(bank) = POISONED.get(cpu) else { - crate::log!("poison_tid: cpu {cpu} >= MAX_CPUS — {id} will never be reaped"); - return; - }; - let banked = bank.bank(id.pack()); - // After the bank is written, never before: the gate's release is what - // carries it to the CPU that claims the work. - REAP_GATE.raise(); - if !banked { - crate::log!( - "poison_tid: cpu {cpu} banked {} deaths since its last reap — {id}'s waiter is stranded", - poison::SLOTS - ); - } -} - -/// Zombify threads that died in panic recovery, collect finished processes' -/// entries, and wake whoever was joining them. Called from the idle loop, -/// which holds none of the panicking thread's locks. Checked before locking -/// `PROCESS_TABLE` unconditionally: holding it on every idle trip would -/// starve a crash report's `try_lock` of that table. -pub(crate) fn reap_poisoned() { +/// Collect finished processes' entries. Called from the idle loop, and +/// checked before locking `PROCESS_TABLE` unconditionally: holding it on every +/// idle trip would starve a crash report's `try_lock` of that table. +pub(crate) fn reap_finished() { if !REAP_GATE.take() { return; } - let mut wakes: [Option; MAX_CPUS * poison::SLOTS] = - [const { None }; MAX_CPUS * poison::SLOTS]; // Dropped after the guard: an entry's drop reaches `remove_vruntime`. - let reaped; - { + let reaped = { let mut guard = process::PROCESS_TABLE.lock(); let table = guard.as_mut().unwrap(); - // SAFETY: `reap_poisoned`'s one caller, `sched::driver::idle_loop`, + // SAFETY: `reap_finished`'s one caller, `sched::driver::idle_loop`, // runs on the per-CPU idle stack, which is what `IdleProof` requires. - reaped = process::reap_finished(table, unsafe { process::IdleProof::new_unchecked() }); - let mut next = 0; - for bank in POISONED.iter() { - bank.drain(|raw| { - let id = TaskId::unpack(raw); - wakes[next] = process::zombify_poisoned(table, id.0, id.1); - next += 1; - }); - } - } + process::reap_finished(table, unsafe { process::IdleProof::new_unchecked() }) + }; drop(reaped); - for wake in wakes.into_iter().flatten() { - match wake { - process::PoisonWake::Joiner(pid, tid) => { - if let Some(sched) = process::thread_sched(pid, tid) { - sched.handle.watch().post(); - } - } - // -1: nobody asked for this exit, and teardown never ran to account it. - process::PoisonWake::Process(object) => { - let stats = toyos_abi::syscall::ProcessStats { - pid: object.pid().raw(), - ..Default::default() - }; - object.publish_exit(crate::object::process::Exit { code: -1, stats }) - } - } - } -} - -/// The panic path's exit: the faulted thread's context is unusable, so it -/// dies where it stands. No baseline assert: a panicking thread may hold -/// any lock, and asserting would double-panic and lose the report. -pub fn schedule_no_return() -> ! { - if in_schedule_self() { - crate::log!("schedule_no_return: panicked inside a pass, cannot rejoin"); - crate::panic::halt_all_cpus(); - } - if percpu::current_tid().is_none() { - enter_idle_loop(); - } - driver::pass(Dispose::Exit); - unreachable!("schedule_no_return: returned from the exit pass"); } /// Cumulative CPU time; a running thread's live slice is added by the reader. diff --git a/kernel/src/syscall/debug.rs b/kernel/src/syscall/debug.rs index 1770efab59b..600ebbe1e0d 100644 --- a/kernel/src/syscall/debug.rs +++ b/kernel/src/syscall/debug.rs @@ -4,13 +4,9 @@ use toyos_abi::syscall::SyscallError; -/// `SYS_DEBUG` action 2's lock only — once taken it is never released. +/// `SYS_DEBUG` action 2's lock only. pub(super) static LOCK_ACROSS_SWITCH: crate::sync::Lock<()> = crate::sync::Lock::new(()); -/// Flips false after action 2's one trip, refusing a second call into the lock that never releases. -pub(super) static LOCK_ACROSS_SWITCH_ARMED: core::sync::atomic::AtomicBool = - core::sync::atomic::AtomicBool::new(true); - /// Screen-test sync signal — `halt_all_cpus` paints before it flushes serial. pub(super) const FATAL_HALT_NONCE: &str = "SYS_DEBUG: fatal halt 4b1d9e2c"; diff --git a/kernel/src/syscall/dispatch.rs b/kernel/src/syscall/dispatch.rs index b70b3206b39..0aa2db888ed 100644 --- a/kernel/src/syscall/dispatch.rs +++ b/kernel/src/syscall/dispatch.rs @@ -21,7 +21,7 @@ use toyos_untrusted::Untrusted; use super::HANDLE_LEN; #[cfg(feature = "test-actuators")] -use super::debug::{canary, debug_heap_alloc, FATAL_HALT_NONCE, LOCK_ACROSS_SWITCH, LOCK_ACROSS_SWITCH_ARMED}; +use super::debug::{canary, debug_heap_alloc, FATAL_HALT_NONCE, LOCK_ACROSS_SWITCH}; use super::device::{ holds_claim, sys_device_bar_map, sys_device_claim, sys_device_dma_alloc, sys_device_dma_map, sys_device_dma_unmap, @@ -533,19 +533,20 @@ pub(crate) fn syscall_dispatch(num: u64, a1: u64, a2: u64, a3: u64, a4: u64) -> #[cfg(feature = "test-actuators")] SYS_DEBUG => match a1 { DA::PANIC => panic!("SYS_DEBUG: kernel panic triggered by userspace"), - // SAFETY: unsound by design — a staged null read in Ring 0, gated behind test-actuators. - // Volatile: a plain read could be optimized to unreachable, leaving nothing to fault. - DA::NULL_READ => { unsafe { core::ptr::read_volatile(core::ptr::null::()); } 0 } + // A record and not the caller's own line: the panic path drains records, and a + // program's line is still in userland when the read below ends the machine. + DA::NULL_READ => { + log!("SYS_DEBUG: a Ring 0 read of {a2:#x}"); + // SAFETY: unsound by design — a staged Ring 0 read of the caller's address, gated behind test-actuators. + // Volatile: a plain read could be optimized to unreachable, leaving nothing to fault. + unsafe { core::ptr::read_volatile(a2 as *const u64) }; + 0 + } DA::LOCK_ACROSS_SWITCH => { - if !LOCK_ACROSS_SWITCH_ARMED.swap(false, core::sync::atomic::Ordering::Relaxed) { - return SyscallError::InvalidArgument.to_u64(); - } let _held = LOCK_ACROSS_SWITCH.lock(); crate::scheduler::yield_now(); 0 } - // Unlike every other action, this costs the machine, not just the caller's - // process: one call is already a permanent halt. DA::FATAL_HALT => { log!("{}", FATAL_HALT_NONCE); crate::panic::halt_all_cpus(); } DA::DOUBLE_FAULT => { log!("SYS_DEBUG: provoking a double fault"); diff --git a/src/build.rs b/src/build.rs index 675f0e91dad..0e54936f363 100644 --- a/src/build.rs +++ b/src/build.rs @@ -2632,7 +2632,7 @@ mod tests { // other is a miscomputed base address. No suite builds it, so a // full run pays nothing and a boot storm asks for it by name. "heap-tripwire", - // The five below cost no kernel build at all, for + // The four below cost no kernel build at all, for // `wake-fence-off`'s reason: each is declared only so `cfg` // checking knows the name, and turned on only by // `kernel-loom`, one at a time, to relax the single edge its @@ -2643,7 +2643,6 @@ mod tests { // `heap-lockspin`'s other arm: the same visit to the pass path, // for the same span, without the allocator's lock. "pass-spin", - "poison-overwrite", // `wake-fence-off`'s twin, for a poll ring's one-shot answer: // turned on only by `kernel-loom`, to split `inbox/once.rs`'s // exchange and prove `poll_once` reds without it. diff --git a/src/ci.rs b/src/ci.rs index eaee32f5676..eb7700dce9b 100644 --- a/src/ci.rs +++ b/src/ci.rs @@ -246,9 +246,6 @@ pub(crate) const CONTROLS: &[Control] = &[ "a_lost_try_lock_leaves_the_lock_held ... FAILED", "two_writers_never_overlap ... FAILED", ]), - red(KERNEL_LOOM, "poison-overwrite", Some("poison_set"), &[ - "a_second_death_banks_beside_the_first ... FAILED", - ]), red(KERNEL_LOOM, "reap-raise-relaxed", Some("reap_gate"), &[ "a_claim_sees_the_enrolled_work ... FAILED", ]), diff --git a/tests/common/metal.rs b/tests/common/metal.rs index 09db138cd58..346cb502d5c 100644 --- a/tests/common/metal.rs +++ b/tests/common/metal.rs @@ -718,8 +718,8 @@ fn build( // **What a job spawns comes with it, and it is not optional.** A binary that // cannot find its `.so` or its helper child does not fail an assertion — it // fails to *spawn*. Measured on a metal-shaped guest: `std_tls` did not run - // at all and took the rest of the job list with it, and `disk_backtrace`, - // `fault_gates` and `panic_recovery` each panicked on `entity not found` + // at all and took the rest of the job list with it, and `disk_backtrace` + // and `fault_gates` each panicked on `entity not found` // looking for a child nothing had staged. // // **Which helper, though, is read rather than assumed.** Staging all of them diff --git a/tests/common/power.rs b/tests/common/power.rs index 198952e0448..9b481909542 100644 --- a/tests/common/power.rs +++ b/tests/common/power.rs @@ -885,19 +885,100 @@ pub fn panic_reboots( // Not `must_be_clean`: this boot panics on purpose, and the arm line is // what says the panic path — not something else — is holding the machine. let line = boot.must_say(&panic_armed())?.to_string(); + let watch = watch_the_bound(&qemu); + let (budget, _) = resets_inside_the_bound(&mut qemu, watch, PANICKED_AND_STAYED_UP)?; + eprintln!(" [power] the panicked guest reset itself inside {budget:?} of: {}", line.trim()); + Ok(()) +} +/// QEMU's `SHUTDOWN` event, subscribed to for the fast bound plus what a reset +/// costs. Opened before whatever starts the bound: QMP delivers no event +/// emitted before its client connected. +fn watch_the_bound(qemu: &QemuInstance) -> (qemu::QmpShutdown, Duration) { let budget = qemu.budget(Duration::from_secs(PANIC_FAST_SECS) + RESET_ALLOWANCE); - let mut stop = qemu::QmpShutdown::open(qemu.qmp_socket(), budget); + (qemu::QmpShutdown::open(qemu.qmp_socket(), budget), budget) +} + +/// QEMU's own `guest-reset` on `watch`, and the serial the guest wrote after +/// its boot log; `never` is what a guest that did not stop means to the caller. +fn resets_inside_the_bound( + qemu: &mut QemuInstance, + (mut stop, budget): (qemu::QmpShutdown, Duration), + never: &str, +) -> Result<(Duration, String), String> { let reason = stop.reason(); // A guest that came back to firmware pays none of this: `-no-reboot` exits and the reader disconnects. let tail = qemu.drain_serial(WAIT); - returned_to_firmware(reason, PANICKED_AND_STAYED_UP, &tail)?; + returned_to_firmware(reason, never, &tail)?; let drain = serial::Serial::named("panic reboot drain", tail.as_str()); drain.must_say(PANIC_REBOOTING)?; + Ok((budget, tail)) +} - eprintln!(" [power] the panicked guest reset itself inside {budget:?} of: {}", line.trim()); - Ok(()) +/// `kernel_params` kills `klogd` on its first instruction, and the machine is +/// what dies. The verdict is QEMU's reset, never the guest's word. +pub fn klogd_death_resets( + test_config: &Path, + c_bins: &[(String, Vec)], + rust_bins: &[(String, Vec)], + kernel_params: &'static [&'static str], + said: &[&str], +) -> Result<(), String> { + // `panicked()`'s 16550-only guest: the reset's own line goes to the UART raw. + let options = + BootOptions { kernel_params, ready_marker: "kthread: klogd pid=", ..panicked() }; + let mut qemu = QemuInstance::boot_with_options(test_config, c_bins, rust_bins, options); + let dead = serial::Serial::boot(&qemu); + let watch = watch_the_bound(&qemu); + let never = "QEMU never reported stopping: klogd died and the machine carried on without it"; + died_and_reset(&mut qemu, watch, dead, never, said).map(drop) +} + +/// `SYS_DEBUG` `action` ends the kernel inside its caller's syscall, and the +/// machine is what dies, never only the caller. The verdict is QEMU's reset, as +/// [`klogd_death_resets`]'s is; what the guest said is returned for the caller +/// to read further. +pub fn syscall_death_resets( + test_config: &Path, + c_bins: &[(String, Vec)], + rust_bins: &[(String, Vec)], + action: u64, + said: &[&str], +) -> Result { + let options = BootOptions { + kernel_params: &["panic-reboot-fast"], + ready_marker: qemu::DEFAULT_READY, + ..panicked() + }; + let mut qemu = QemuInstance::boot_with_options(test_config, c_bins, rust_bins, options); + let dead = serial::Serial::boot(&qemu); + let watch = watch_the_bound(&qemu); + writeln!(qemu.stdin_mut(), "run test_rs_test_panic_child {action}") + .expect("write to QEMU stdin"); + qemu.flush_stdin(); + let never = "QEMU never reported stopping: the kernel died inside a syscall and the machine \ + carried on without its caller"; + died_and_reset(&mut qemu, watch, dead, never, said) +} + +/// QEMU's reset inside the bound, then what the dead guest said, `said` and the +/// arm line among it. +fn died_and_reset( + qemu: &mut QemuInstance, + watch: (qemu::QmpShutdown, Duration), + mut dead: serial::Serial, + never: &str, + said: &[&str], +) -> Result { + let (budget, tail) = resets_inside_the_bound(qemu, watch, never)?; + dead.push(&tail); + for want in said { + dead.must_say(want)?; + } + dead.must_say(&panic_armed())?; + eprintln!(" [power] {said:?}: QEMU reset the machine inside {budget:?}"); + Ok(dead.text().to_string()) } /// A panic inside `percpu::init_bsp`, one statement after it loads the IDT, diff --git a/tests/common/qemu.rs b/tests/common/qemu.rs index ed205a58079..c9aa24b63a0 100644 --- a/tests/common/qemu.rs +++ b/tests/common/qemu.rs @@ -650,15 +650,6 @@ impl std::fmt::Display for WaitVerdict { /// and its full backtrace four lines above that sentence, on a guest that died /// at 1.450 s of its own uptime. /// -/// A kernel panic does not end the wait *by itself*, and that is deliberate: -/// the same handler recovers a panic taken in syscall context, killing the -/// caller and leaving the machine running, which is exactly what -/// `panic_recovery`, `heap_ceiling` and `screen_recoverable_untouched` assert. -/// Silence is what separates the two, and it is the separation the harness -/// already trusts everywhere else ([`GUEST_QUIET`]): a recovering guest keeps -/// talking — the test's own `===TEST_END` arrives in milliseconds — and a -/// halted one cannot. -/// /// **The wall clock is not the wedge; silence is.** A test's `ceiling` is the /// budgeted wall clock (`budget_smp`-scaled, so it already carries #256's /// `vcpus/cores` oversubscription widening), and until this it ended the wait @@ -983,9 +974,7 @@ pub fn await_guest( doing: &str, done: impl Fn(&str) -> bool, ) -> Result<(), String> { - // Where this wait's own evidence starts. The capture is the caller's and - // outlives every wait on it, so a panic the machine recovered from ten - // probes ago must not be handed to this one as its cause. + // Where this wait's own evidence starts. let from = log.len(); let mut live = guest_liveness(); while !done(log) && live.working(log) { diff --git a/tests/common/screen.rs b/tests/common/screen.rs index 484886ee85d..07c3f65ab76 100644 --- a/tests/common/screen.rs +++ b/tests/common/screen.rs @@ -232,8 +232,7 @@ impl Ppm { self.rows().iter().position(|r| r.contains(needle)) } - /// Whether every pixel matches `other`. The C6b negative test's whole - /// assertion: a recoverable panic must leave the display untouched. + /// Whether every pixel matches `other`. pub fn identical_to(&self, other: &Ppm) -> bool { self.width == other.width && self.height == other.height && self.pixels == other.pixels } diff --git a/tests/common/serial.rs b/tests/common/serial.rs index 84bec400f00..29ee146e44a 100644 --- a/tests/common/serial.rs +++ b/tests/common/serial.rs @@ -36,13 +36,7 @@ use super::qemu::{is_kernel_line, QemuInstance}; #[derive(Clone, Copy, PartialEq, Eq, Debug)] pub enum Died { /// The kernel itself. Every path that writes one of these words ends at - /// `panic::halt_all_cpus` — **unless** the panic handler finds the panic - /// recoverable, which it does for a `panic!` taken in syscall context - /// (`kernel/src/main.rs`: the caller is killed and the machine carries on, - /// which is what `panic_recovery`, `heap_ceiling` and - /// `screen_recoverable_untouched` are about). No line says which of the two - /// happened. So what a caller learns here is "the kernel said it was - /// dying", and the guest going quiet afterwards is what says it meant it. + /// `panic::halt_all_cpus`. Kernel, /// A process the kernel killed: a Ring 3 fault, reported by name in /// `kernel/src/arch/x86_64/idt/exceptions.rs`. The machine is fine — a test whose diff --git a/tests/test-durations b/tests/test-durations index d76f46dec0c..4af45c58707 100644 --- a/tests/test-durations +++ b/tests/test-durations @@ -209,7 +209,6 @@ hash_seed_precedes_every_map 4976 hda_client_stall 19047 hda_tone 13389 hda_two_live_refused 4848 -heap_ceiling_recovery 10371 hierarchy_paths 87 home_backing_revoked 663 home_budget_refusal_retried 18661 @@ -251,7 +250,6 @@ kernel_log_file 13152 keyboard_claim_close_spares_stdin 4580 kill_while_blocked 44 klogd_hosted 5674 -klogd_panic_halts 16658 lan_dhcp_lease 3029 lan_no_lease 32709 lapic_spurious_vector 6829 @@ -313,7 +311,6 @@ page_cache_partition_offset 6984 panic_before_peripherals_reboots 3227 panic_key_holds 42033 panic_reboots 9768 -panic_recovery 130 pci_capability_walk 5775 pci_claim_caps_truncated 2583 pci_inventory 4529 @@ -360,8 +357,6 @@ screen_log_absent 1823 screen_paged_scrollback 7384 screen_pager_keys 16152 screen_panic_muted 4416 -screen_recoverable_untouched 5117 -screen_survived_panic_not_blamed 8477 serial_vocabulary 2 shipped_config_boots 3024 shm_release_reclaims 29 diff --git a/tests/toyos-rust-tests/src/bin/disk_backtrace_child.rs b/tests/toyos-rust-tests/src/bin/disk_backtrace_child.rs index f42162c3dc6..f5f2152f83f 100644 --- a/tests/toyos-rust-tests/src/bin/disk_backtrace_child.rs +++ b/tests/toyos-rust-tests/src/bin/disk_backtrace_child.rs @@ -1,10 +1,8 @@ //! Dereferences null so the kernel prints a SEGFAULT report for it. //! -//! Distinct from `segfault_child` only in the name of the function that faults, -//! and that is the whole point: `disk_backtrace` copies this binary onto a disk -//! and runs it from there, so the report has to name a symbol no *other* boot -//! could have put in the same capture window. `segfault_child` runs from ROOT -//! in the same suite. +//! `disk_backtrace` copies this binary onto a disk and runs it from there, so +//! the report has to name a symbol no *other* boot could have put in the same +//! capture window. #[inline(never)] fn null_deref_run_from_disk() -> u64 { diff --git a/tests/toyos-rust-tests/src/bin/fault_gate_child.rs b/tests/toyos-rust-tests/src/bin/fault_gate_child.rs index 35aadf5fe50..c1c9c10cb6e 100644 --- a/tests/toyos-rust-tests/src/bin/fault_gate_child.rs +++ b/tests/toyos-rust-tests/src/bin/fault_gate_child.rs @@ -18,6 +18,7 @@ fn main() { "mf" => x87_exception(), "xm" => simd_exception(), "ac" => alignment_check(), + "pf" => read_null(), other => panic!("unknown fault kind {other}"), } println!("survived {kind}"); @@ -143,6 +144,13 @@ fn simd_exception() { println!(" MXCSR after 0.0/0.0 with IM unmasked: {after:#010x}"); } +/// #PF (14). A read of address 0, which lies in no region. +#[inline(never)] +fn read_null() { + // SAFETY: none — the fault is the point, and it ends this process. + unsafe { core::ptr::read_volatile(core::ptr::null::()) }; +} + /// #AC (17). RFLAGS.AC plus a misaligned load. CR0.AM gates it as well and is /// clear — firmware leaves it so and nothing sets it — which is what the /// printed readback separates from a `popfq` that did not take. diff --git a/tests/toyos-rust-tests/src/bin/fault_gates.rs b/tests/toyos-rust-tests/src/bin/fault_gates.rs index 5fd6d5db3f8..2d8d885e3fe 100644 --- a/tests/toyos-rust-tests/src/bin/fault_gates.rs +++ b/tests/toyos-rust-tests/src/bin/fault_gates.rs @@ -41,6 +41,7 @@ const ARMS: &[(&str, Expect)] = &[ // `CR0.AM` is declared clear, so `RFLAGS.AC` buys a Ring 3 process nothing // on any machine this kernel runs on — emulated or not. ("ac", Expect::MachineLives), + ("pf", Expect::Killed), ]; fn main() { diff --git a/tests/toyos-rust-tests/src/bin/heap_ceiling.rs b/tests/toyos-rust-tests/src/bin/heap_ceiling.rs index bd8c0b5c39f..d524e957657 100644 --- a/tests/toyos-rust-tests/src/bin/heap_ceiling.rs +++ b/tests/toyos-rust-tests/src/bin/heap_ceiling.rs @@ -1,26 +1,16 @@ -//! The kernel heap's ceiling, and what it costs the machine to cross it. +//! The kernel heap's ceiling. //! //! `KernelPageSource` hands dlmalloc one 2 MiB page and can hand it no more, //! so `mm::MAX_HEAP_ALLOC` is the largest single allocation the kernel heap -//! can serve. Asking for more is a kernel bug and dies loudly — but the check -//! used to live *inside* `KernelAllocator::alloc`'s lock, and the kernel does -//! not unwind, so the heap stayed locked and the CPU that recovered from the -//! panic spun forever on its next allocation or free. Reporting the bug cost -//! the machine. -//! -//! Three cases, because no one of them says it alone: the ceiling is servable, -//! a request the page source cannot back is refused rather than fatal, and one -//! past the ceiling kills its caller and nothing else. - -use std::process::Command; +//! can serve. Asking for more is a kernel bug and halts the machine, which +//! `heap_over_ceiling_halts` asserts on a boot of its own. -// `SYS_DEBUG` actions a `test-actuators` kernel provides. The first three take +// `SYS_DEBUG` actions a `test-actuators` kernel provides. The first two take // one kernel heap allocation each and release it again — at -// `mm::MAX_HEAP_ALLOC`, at `mm::PAGE_2M`, and at `MAX_HEAP_ALLOC` with -// 4096-byte alignment; the last lowers `SYS_SYSINFO`'s thread bound to a count +// `mm::MAX_HEAP_ALLOC`, and at `MAX_HEAP_ALLOC` with 4096-byte alignment; the last lowers `SYS_SYSINFO`'s thread bound to a count // this guest can reach. use toyos_abi::syscall::debug_action::{ - HEAP_AT_CEILING, HEAP_AT_CEILING_PAGE_ALIGNED, HEAP_OVER_CEILING, LOWER_SYSINFO_BOUND, + HEAP_AT_CEILING, HEAP_AT_CEILING_PAGE_ALIGNED, LOWER_SYSINFO_BOUND, }; /// `SyscallError::ResourceExhausted`, as `SyscallError::to_u64` encodes it. @@ -30,8 +20,6 @@ fn main() { at_ceiling_is_servable(); aligned_at_ceiling_is_refused_not_fatal(); sysinfo_refuses_rather_than_allocating_past_the_ceiling(); - over_ceiling_kills_only_the_caller(); - heap_still_works(); println!("all heap ceiling tests passed"); } @@ -106,8 +94,8 @@ fn sysinfo_answers() -> bool { /// `PAGE_2M - 4096` and the 4 KiB is headroom for dlmalloc's own chunk and /// segment bookkeeping — arithmetic that was reasoned about and never run. /// -/// It is also the negative side of the case below it: an assert that simply -/// refused every large allocation would satisfy that one and fail this. +/// It is also the negative side of `heap_over_ceiling_halts`: an assert that +/// simply refused every large allocation would satisfy that one and fail this. fn at_ceiling_is_servable() { let rc = toyos_abi::syscall::debug(HEAP_AT_CEILING); assert_eq!( @@ -121,12 +109,9 @@ fn at_ceiling_is_servable() { /// The same size, page-aligned, is more than the page source can back — and /// that is an error return, not a dead machine. /// -/// This is the case that proves the ceiling and the lock were two defects and -/// not one. `memalign` pads by the alignment before it asks for backing, so -/// this request satisfies `MAX_HEAP_ALLOC` and still reaches the page source -/// asking for 2,162,688 bytes. Measured against the old code: it panicked -/// inside `Dlmalloc::malloc`, with the allocator lock held, and the guest went -/// silent — so no bound at the entry could ever have closed it. +/// `memalign` pads by the alignment before it asks for backing, so this request +/// satisfies `MAX_HEAP_ALLOC` and still reaches the page source +/// asking for 2,162,688 bytes. fn aligned_at_ceiling_is_refused_not_fatal() { let rc = toyos_abi::syscall::debug(HEAP_AT_CEILING_PAGE_ALIGNED); assert_eq!( @@ -136,36 +121,3 @@ fn aligned_at_ceiling_is_refused_not_fatal() { ); println!(" PASS: an allocation the page source cannot back is refused, not fatal"); } - -/// One page over the ceiling: the caller dies, and nothing else does. -fn over_ceiling_kills_only_the_caller() { - let status = Command::new("/system/bin/test_rs_test_panic_child") - .arg(HEAP_OVER_CEILING.to_string()) - .status() - .expect("failed to spawn child"); - assert!( - !status.success(), - "a 2 MiB kernel heap allocation should have panicked the kernel and killed the child" - ); - println!(" PASS: over-ceiling allocation killed the caller (exit={})", - status.code().unwrap_or(-1)); -} - -/// The property the whole test exists for: the CPU that recovered from that -/// panic can still allocate and free. -/// -/// Reaching this line is already most of the evidence — `status()` above only -/// returns once the kernel has reaped the dead child, which takes the idle -/// loop through `reap_poisoned`. A spawn is the loudest confirmation userland -/// can give: process table entry, handle table, ELF load and the whole teardown, -/// all of it kernel heap traffic, and on this guest all of it on the one CPU -/// that recovered. -fn heap_still_works() { - let output = Command::new("/system/bin/echo") - .arg("still alive") - .output() - .expect("failed to run echo after the over-ceiling panic"); - assert!(output.status.success()); - assert_eq!(String::from_utf8_lossy(&output.stdout).trim(), "still alive"); - println!(" PASS: the kernel heap still allocates and frees after recovery"); -} diff --git a/tests/toyos-rust-tests/src/bin/panic_recovery.rs b/tests/toyos-rust-tests/src/bin/panic_recovery.rs deleted file mode 100644 index 61029a93327..00000000000 --- a/tests/toyos-rust-tests/src/bin/panic_recovery.rs +++ /dev/null @@ -1,79 +0,0 @@ -use std::process::Command; - -fn main() { - test_syscall_panic(); - test_syscall_fault(); - test_lock_across_switch(); - test_lock_across_switch_is_one_shot(); - test_user_segfault(); - test_system_alive(); - println!("all panic recovery tests passed"); -} - -/// Kernel panic!() during syscall → process killed, system survives. -fn test_syscall_panic() { - let status = Command::new("/system/bin/test_rs_test_panic_child") - .arg("0") - .status() - .expect("failed to spawn child"); - assert!(!status.success(), "child that triggers kernel panic should be killed"); - println!(" PASS: syscall panic killed process (exit={})", status.code().unwrap_or(-1)); -} - -/// Kernel null-pointer fault during syscall → process killed, system survives. -fn test_syscall_fault() { - let status = Command::new("/system/bin/test_rs_test_panic_child") - .arg("1") - .status() - .expect("failed to spawn child"); - assert!(!status.success(), "child that triggers kernel fault should be killed"); - println!(" PASS: syscall fault killed process (exit={})", status.code().unwrap_or(-1)); -} - -/// A kernel spinlock held across a scheduler entry → the baseline -/// assert fires at the call site instead of the pass parking with the lock on a -/// stack nothing returns to. Without the assert the syscall returns normally and -/// the child exits 0, so this case has teeth in the negative direction too. -fn test_lock_across_switch() { - let status = Command::new("/system/bin/test_rs_test_panic_child") - .arg("2") - .status() - .expect("failed to spawn child"); - assert!(!status.success(), "child that yields under a kernel lock should be killed"); - println!(" PASS: lock-across-switch tripwire killed process (exit={})", status.code().unwrap_or(-1)); -} - -/// The trip above dies holding the kernel lock it took, and the kernel does not -/// unwind, so nothing will ever release it. A second call must be refused: it -/// would otherwise spin `Lock::lock` to its 500M-spin deadline with interrupts -/// masked and preemption disabled, freezing a single-CPU machine for the whole -/// window — from an ungated syscall. Refused means the child survives. -fn test_lock_across_switch_is_one_shot() { - let status = Command::new("/system/bin/test_rs_test_panic_child") - .arg("2") - .status() - .expect("failed to spawn child"); - assert!(status.success(), "second lock-across-switch call must be refused, not honoured"); - println!(" PASS: second lock-across-switch call refused (exit={})", status.code().unwrap_or(-1)); -} - -/// User-mode segfault → process killed, system survives. -fn test_user_segfault() { - let status = Command::new("/system/bin/test_rs_segfault_child") - .status() - .expect("failed to spawn child"); - assert!(!status.success(), "child that segfaults should be killed"); - println!(" PASS: user segfault killed process (exit={})", status.code().unwrap_or(-1)); -} - -/// System still works after all three fault types. -fn test_system_alive() { - let output = Command::new("/system/bin/echo") - .arg("still alive") - .output() - .expect("failed to run echo after recoveries"); - assert!(output.status.success()); - let stdout = String::from_utf8_lossy(&output.stdout); - assert_eq!(stdout.trim(), "still alive"); - println!(" PASS: system alive after panic + fault + segfault recovery"); -} diff --git a/tests/toyos-rust-tests/src/bin/process_lifecycle.rs b/tests/toyos-rust-tests/src/bin/process_lifecycle.rs index 717686fb9a7..aa98fa60e9a 100644 --- a/tests/toyos-rust-tests/src/bin/process_lifecycle.rs +++ b/tests/toyos-rust-tests/src/bin/process_lifecycle.rs @@ -4,8 +4,8 @@ //! **There is no zombie here and no parent.** A pid-keyed wait needed the //! process table to keep a corpse until somebody claimed it, and rules for who //! was allowed to claim one and what happened when nobody did. The exit code -//! lives on the object instead, published once by whichever of exit, kill or -//! panic recovery owns the teardown — so a wait after the fact reads a value, a +//! lives on the object instead, published once by whichever of exit or kill +//! owns the teardown — so a wait after the fact reads a value, a //! wait before it parks and is woken by the publish, and two holders both get //! the answer. //! diff --git a/tests/toyos-rust-tests/src/bin/segfault_child.rs b/tests/toyos-rust-tests/src/bin/segfault_child.rs deleted file mode 100644 index 52db4f0d141..00000000000 --- a/tests/toyos-rust-tests/src/bin/segfault_child.rs +++ /dev/null @@ -1,11 +0,0 @@ -/// This function dereferences null, triggering a page fault that the kernel -/// cannot resolve (address 0 is unmapped). The kernel should print a SEGFAULT -/// with a backtrace that includes this function name. -#[inline(never)] -fn deliberate_null_deref() -> u64 { - unsafe { core::ptr::read_volatile(core::ptr::null::()) } -} - -fn main() { - let _ = deliberate_null_deref(); -} diff --git a/tests/toyos-rust-tests/src/bin/test_panic_child.rs b/tests/toyos-rust-tests/src/bin/test_panic_child.rs index 3200173c82a..7fd8e325e51 100644 --- a/tests/toyos-rust-tests/src/bin/test_panic_child.rs +++ b/tests/toyos-rust-tests/src/bin/test_panic_child.rs @@ -1,30 +1,41 @@ //! Ask the kernel for one `SYS_DEBUG` action, and report what came back. //! -//! Every action this is driven with kills the caller, except action 9 — so -//! reaching the end at all is a finding, and *which* finding is the whole of -//! what it prints. `InvalidArgument` is the kernel saying it has no debug -//! syscall: the boot needs `test-actuators` and asked for nothing, which is a -//! harness mistake and not a kernel that failed to kill anybody. +//! Every action this is driven with ends the machine — so reaching the end at +//! all is a finding, and *which* finding is the whole of what it prints. +//! `InvalidArgument` is the kernel saying it has no debug syscall: the boot +//! needs `test-actuators` and asked for nothing, which is a harness mistake and +//! not a kernel that failed to stop. //! -//! It exits 0 in both cases on purpose. Every caller asserts that the child did -//! *not* succeed, so a return is a red wherever it happens; the message is what -//! tells the two apart. +//! `NULL_READ` is given a 2 MiB window wholly inside [`UNTOUCHED`]: a page +//! demand paging fills for Ring 3 and must not for Ring 0. `.bss` and not an +//! `mmap`, because an anonymous `mmap` is mapped when it is made. -use toyos_abi::syscall::SyscallError; +use toyos_abi::syscall::{self, debug_action, SyscallError}; + +const PAGE_2M: usize = 2 << 20; + +/// `.bss` this process never touches. Twice the window, so one 2 MiB-aligned +/// window lies inside it wherever the loader placed it. +static mut UNTOUCHED: [u8; 2 * PAGE_2M] = [0; 2 * PAGE_2M]; fn main() { let action: u64 = std::env::args() .nth(1) .and_then(|s| s.parse().ok()) - .unwrap_or(0); - let rc = toyos_abi::syscall::debug(action); + .expect("usage: test_panic_child "); + let rc = if action == debug_action::NULL_READ { + let window = (&raw const UNTOUCHED as usize).next_multiple_of(PAGE_2M); + syscall::debug_with(action, window as u64) + } else { + syscall::debug(action) + }; if rc == SyscallError::InvalidArgument.to_u64() { eprintln!( "ERROR: SYS_DEBUG {action} answered InvalidArgument — this kernel carries no \ actuators, so the boot that drives this one needs `test-actuators`" ); } else { - eprintln!("ERROR: SYS_DEBUG {action} returned {rc:#x}, kernel did not kill the process"); + eprintln!("ERROR: SYS_DEBUG {action} returned {rc:#x}, the kernel did not end the machine"); } std::process::exit(0); } diff --git a/tests/toyos.rs b/tests/toyos.rs index 466d0317ab9..5f1c12b03fa 100644 --- a/tests/toyos.rs +++ b/tests/toyos.rs @@ -117,10 +117,6 @@ const SHARED_BLOCK: Sched = Sched::Parallel; /// second of guest time between its members, and what these need is a syscall /// number the other 150 must not have. const ACTUATOR_TESTS: &[&str] = &[ - // Actions 0, 1 and 2: a kernel `panic!`, a null read in kernel context, and - // a spinlock held across a scheduler entry. Each kills the caller and the - // machine has to survive it, which is the whole verdict. - "panic_recovery", // Actions 10 and 11: the address of sixteen bytes of kernel memory and // whether they still hold what the kernel put there. A guest cannot read the // kernel's address space, so without them a kernel that still made the write @@ -221,7 +217,6 @@ const RUST_SKIP: &[&str] = &[ // The C corpus's comparator: a helper reached through one symlink per case, // never a test of its own. `shared_metal` stages every name on this list. "ccheck", - "segfault_child", "disk_backtrace_child", "fault_gate_child", // `gsbase_locked`'s probe child; its #UD must kill the child, not the run. @@ -293,8 +288,7 @@ const RUST_SKIP: &[&str] = &[ // and the runner's bound is the fallback. `lan_swap` rides it. "lan_swap_hold", // Needs SYS_DEBUG, which the shipping kernel has no arm of at all. - // `heap_ceiling_recovery` boots the `test-actuators` kernel on one CPU, - // which is also what makes its claim about *the recovered CPU* precise. + // `heap_ceiling_bounds` boots the `test-actuators` kernel for it. "heap_ceiling", // Fills /tmp to the VFS listing limit, so it needs a boot nothing else // shares — every later `read_dir("/tmp")` in it would be refused. @@ -562,11 +556,6 @@ const SCREEN_TESTS: &[(&str, Sched, Tier)] = &[ // the desktop's next repaint — which only where that wait lands decides, // so it is timer-anchored despite being a screendump-content check. ("screen_blocked_dump", Sched::Parallel, Tier::Nightly), - ("screen_recoverable_untouched", Sched::Parallel, Tier::Fast), - // The other half of the recovery branch: the test above reads the screen - // either side of a survived panic, which holds whether or not the discard - // did anything. - ("screen_survived_panic_not_blamed", Sched::Parallel, Tier::Nightly), ("screen_early_panic", Sched::Parallel, Tier::Fast), ("screen_late_panic", Sched::Parallel, Tier::Fast), ("screen_paged_scrollback", Sched::Parallel, Tier::Nightly), @@ -1192,13 +1181,15 @@ const MACHINE_TESTS: &[(&str, Sched, Tier)] = &[ // offsets it chose itself. ("operation_nesting", Sched::Parallel, Tier::Fast), ("short_sleep_livelock", Sched::Parallel, Tier::Fast), - // The spawn half alone: one headless boot whose verdict is three kernel - // log lines. + // The spawn half alone: one headless boot whose verdict is kernel log + // lines. ("klogd_hosted", Sched::Parallel, Tier::Fast), - // The two actuator boots (`klogd-panic`, `usbd-panic`), split off so the - // spawn half is per-PR again; alone they still price over the ceiling, - // and sit Nightly. ("klogd_panic_halts", Sched::Parallel, Tier::Nightly), + ("klogd_fault_halts", Sched::Parallel, Tier::Nightly), + ("syscall_panic_halts", Sched::Parallel, Tier::Nightly), + ("syscall_fault_halts", Sched::Parallel, Tier::Nightly), + ("lock_across_switch_halts", Sched::Parallel, Tier::Nightly), + ("heap_over_ceiling_halts", Sched::Parallel, Tier::Nightly), // The two dead ends of the panic path, each staged on purpose and read for // what the machine manages to say on its way out. **Two names because one // over two boots measured 12 s twelve-wide on the dev host**, against @@ -1526,7 +1517,7 @@ const MACHINE_TESTS: &[(&str, Sched, Tier)] = &[ // The directory work's FAT arm, `fs_rename_durable`'s oracle shape. ("fs_dirs_durable", Sched::Parallel, Tier::Fast), ("va_exhaustion", Sched::Parallel, Tier::Fast), - ("heap_ceiling_recovery", Sched::Parallel, Tier::Nightly), + ("heap_ceiling_bounds", Sched::Parallel, Tier::Nightly), ("iommu_context_absent", Sched::Parallel, Tier::Fast), ("iommu_empty_domain", Sched::Parallel, Tier::Fast), ("iommu_interrupt_remapping", Sched::Parallel, Tier::Fast), @@ -1629,7 +1620,7 @@ const CARRIES: &[(&str, &[&str])] = &[ ("gsbase_locked", &["test_rs_gsbase_locked"]), ("sched_check_build", &["test_rs_sched_stress"]), ("short_sleep_livelock", &["test_rs_abuse_short_sleep"]), - ("heap_ceiling_recovery", &["test_rs_heap_ceiling"]), + ("heap_ceiling_bounds", &["test_rs_heap_ceiling"]), ("cache_eviction", &["test_rs_cache_eviction"]), ("irq_census_conservation", &["test_rs_std_mmap"]), ("i8042_health_cadence", &["test_rs_i8042_keyboard"]), @@ -1716,6 +1707,10 @@ const CARRIES: &[(&str, &[&str])] = &[ ("kernel_log_file", &["test_rs_writeback_durability"]), ("double_fault_stack", &["test_rs_test_panic_child"]), ("idle_stack_guard", &["test_rs_test_panic_child"]), + ("syscall_panic_halts", &["test_rs_test_panic_child"]), + ("syscall_fault_halts", &["test_rs_test_panic_child"]), + ("lock_across_switch_halts", &["test_rs_test_panic_child"]), + ("heap_over_ceiling_halts", &["test_rs_test_panic_child"]), ("dump_left_pending_is_owed", &["test_rs_dump_stage_load"]), ("dump_nmi_probe", &["test_rs_dump_stage_load"]), ("syscall_window_nmi", &["test_rs_nmi_window_spin"]), @@ -1759,8 +1754,6 @@ const CARRIES: &[(&str, &[&str])] = &[ ("screen_console_panic", &["test_rs_test_panic_child"]), ("screen_fatal_halt", &["test_rs_test_panic_child"]), ("panic_halts_the_others_first", &["test_rs_panic_halts_first"]), - ("screen_recoverable_untouched", &["test_rs_test_panic_child"]), - ("screen_survived_panic_not_blamed", &["test_rs_test_panic_child"]), ]; /// The clients every `tests/metalcase` desktop boot carries: the group shares @@ -2964,7 +2957,7 @@ const MAX_KERNEL_LINES: usize = 60; /// The kernel's own account of a test that died, which `stdout` cannot carry. /// /// **`exit code Some(-1)` is the kernel saying it killed the process** — -/// `recover_or_halt` answers a Ring 3 fault with `kill_process(-1)` — and every +/// `fatal_exception` answers a Ring 3 fault with `kill_process(-1)` — and every /// word of *why* is a `log!`: the vector, `rip`, `cr2`, the resolved symbol. /// `run_test_paced` files kernel lines under `serial` and keeps them out of /// `stdout`, which is right for a test that passed and leaves a killed one with @@ -3065,42 +3058,9 @@ fn check_rust_result(result: &TestResult) -> bool { } } -/// Checks both exit code and serial diagnostics for panic recovery. -fn check_panic_recovery(result: &TestResult) -> bool { - if !check_rust_result(result) { - return false; - } - - let checks: &[(&str, &str)] = &[ - ("PANIC:", "expected PANIC header"), - ("SYS_DEBUG", "expected SYS_DEBUG in panic message"), - ("Syscall: num=92", "expected syscall context in panic report"), - ("User backtrace:", "expected user backtrace in panic report"), - ("Registers:", "expected register dump from kernel fault"), - ("SEGFAULT tid=", "expected SEGFAULT header"), - ("deliberate_null_deref", "expected deliberate_null_deref in segfault backtrace"), - ("+0x", "expected symbolized backtraces"), - ]; - - let mut ok = true; - for (needle, msg) in checks { - if !result.serial.contains(needle) { - eprintln!("FAIL rs::panic_recovery: {msg}\nserial:\n{}", result.serial); - ok = false; - } - } - if let Err(msg) = check_tripwire_attribution(&result.serial) { - eprintln!("FAIL rs::panic_recovery: {msg}\nserial:\n{}", result.serial); - ok = false; - } - ok & check_symbols_were_read("panic_recovery", &result.serial) -} - /// The kernel names the frames of a process it loaded off a **disk**. /// -/// `null_deref_run_from_disk` is this child's alone, so a `contains` over the -/// capture window cannot be satisfied by `segfault_child` running in the same -/// boot. +/// `null_deref_run_from_disk` is this child's alone. fn check_disk_backtrace(result: &TestResult) -> bool { if !check_rust_result(result) { return false; @@ -3142,13 +3102,6 @@ fn check_disk_backtrace(result: &TestResult) -> bool { /// task's own record — so the two reasons left are a CPU inside a scheduler pass /// and a CPU running nothing, and either one in a report is a finding rather /// than weather. -/// -/// The measured before/after on the dev host under a twelve-wide suite, which is -/// what makes that a claim — N = 12 rounds of `fault_gates` + `panic_recovery` -/// an arm, 2026-08-22: 3 of 12 conceded with the table lookup, 0 of 12 without -/// it, and 1 of 12 with the lookup put back on the same base, that third arm -/// being the control that says the first two are about the code and not about -/// the day. fn check_symbols_were_read(test: &str, serial: &str) -> bool { const CONCEDED: &str = " = serial.lines().filter(|l| l.contains(CONCEDED)).collect(); @@ -3168,11 +3121,9 @@ fn check_symbols_were_read(test: &str, serial: &str) -> bool { /// only thing `#[track_caller]` on `assert_baseline` buys. /// /// A whole-buffer `contains("syscall/dispatch.rs")` certifies none of that: the -/// same boot's `test_syscall_panic` panics in that file too, so the needle is -/// already present before the tripwire runs. Scope it instead to the window -/// between this panic's header and its message — `panicked at ` is -/// the only thing in there, and the backtrace that names every frame comes -/// after the message, so it cannot supply the answer either. +/// backtrace names every frame, that file's included. Scope it instead to the +/// window between this panic's header and its message — `panicked at +/// ` is the only thing in there. fn check_tripwire_attribution(serial: &str) -> Result<(), String> { const MSG: &str = "scheduler entered while a lock is held"; const HEADER: &str = "PANIC:"; @@ -3192,6 +3143,28 @@ fn check_tripwire_attribution(serial: &str) -> Result<(), String> { Ok(()) } +/// The kernel's Ring 0 read of the address `test_panic_child` named halted on +/// that address as **unmapped**. A read that demand paging filled for the +/// caller re-executes into SMAP's protection fault instead, so the word is what +/// says nothing was mapped into the current process. +fn check_ring0_read_unmapped(serial: &str) -> Result<(), String> { + const READ_OF: &str = "SYS_DEBUG: a Ring 0 read of "; + let at = serial.find(READ_OF).ok_or("expected the kernel to name the address it read")?; + let named = serial[at + READ_OF.len()..].split_whitespace().next().unwrap_or_default(); + let addr = named + .strip_prefix("0x") + .and_then(|hex| u64::from_str_radix(hex, 16).ok()) + .ok_or_else(|| format!("the address the kernel read is not a number: {named:?}"))?; + if addr == 0 { + return Err("expected the demand-paged window, not the null read".to_string()); + } + let want = format!("KERNEL PANIC: read unmapped address at {addr:#x}"); + if !serial.contains(&want) { + return Err(format!("expected `{want}`: the read did not fault as unmapped at {addr:#x}")); + } + Ok(()) +} + /// A zero CPU delta is the signature of a suspended soundd and equally of one /// wedged with the device running, so the counter the test reads cannot tell /// them apart on its own. The serial can: in a window where no audio client @@ -3270,6 +3243,8 @@ fn check_fault_gates(result: &TestResult) -> bool { "fault_gate_child::divide_by_zero", "expected the faulting function in the #DE backtrace", ), + ("SEGFAULT tid=", "expected a SEGFAULT header for the null read"), + ("fault_gate_child::read_null", "expected the faulting function in the #PF backtrace"), ]; let mut ok = true; @@ -3446,7 +3421,6 @@ fn settle_for(name: &str) -> fn(&mut QemuInstance, &mut TestResult) { /// Select check function by test name convention. fn check_for(name: &str) -> fn(&TestResult) -> bool { match name { - "panic_recovery" => check_panic_recovery, "disk_backtrace" => check_disk_backtrace, "audio_idle_suspend" => check_audio_idle_suspend, "null_sink_client_exits" => check_null_sink_client_exits, @@ -6854,143 +6828,6 @@ fn run_screen_test( ); Ok(()) } - "screen_recoverable_untouched" => { - // The negative of screen_fatal_halt: a panic the kernel recovers - // from must not paint its report over a live display. Action 0 - // panics in syscall context, which the handler recovers from, so it - // never reaches halt_all_cpus. **Every screen across the recovery, - // not two endpoints**: a report painted and then painted over is - // gone by any endpoint — the fatal fill is looked for on each dump - // from the command until well after the child is reaped. - let mut qemu = QemuInstance::boot_with_options( - test_config, - c_bins, - rust_bins, - BootOptions { - profile: qemu::Profile::Gop, - qmp: true, - // Action 0 is a `SYS_DEBUG` arm, and a kernel that ships - // has none: the child would be answered `InvalidArgument` - // and exit 0, which is this test's own red for a reason - // that is not about the screen at all. - kernel_features: ACTUATOR_KERNEL, - ..Default::default() - }, - ); - let before = qemu.screendump(); - let from = qemu.console_stream().mark(); - writeln!(qemu.stdin_mut(), "run test_rs_test_panic_child").map_err(|e| format!("{e}"))?; - qemu.flush_stdin(); - const ENDED: &str = "===TEST_END test_rs_test_panic_child exit="; - // Past the child's end by this much: a paint the recovery made late - // is still looked for. - const AFTER_END: Duration = Duration::from_millis(1500); - let deadline = Instant::now() + qemu.budget(Duration::from_secs(15)); - let mut ended_at: Option = None; - let mut dumps = 0usize; - loop { - let dump = qemu.screendump(); - dumps += 1; - if dump.fill() == FILL_FATAL { - return Err(format!( - "recovering panic painted its report over the display, on dump {dumps} \ - across the recovery\ndecoded screen:\n{}", - dump.text() - )); - } - if ended_at.is_none() && qemu.console_stream().since(from).contains(ENDED) { - ended_at = Some(Instant::now()); - } - if ended_at.is_some_and(|at| at.elapsed() >= AFTER_END) { - break; - } - if Instant::now() >= deadline { - return Err(format!( - "the recoverable panic never completed\nserial:\n{}", - qemu.console_stream().since(from) - )); - } - } - // The premise, not a formality: a child that never panicked leaves - // every dump boot-filled and this test green. - let said = qemu.console_stream().since(from); - if !said.contains("SYS_DEBUG: kernel panic triggered by userspace") { - return Err(format!("no kernel panic in the child's output\nserial:\n{said}")); - } - if said.contains(&format!("{ENDED}0===")) { - return Err("recoverable panic did not kill the child".to_string()); - } - // A screen that was blank to begin with would pass the fill for - // the wrong reason. - let text = before.text(); - print_screen(name, &format!("{dumps} dumps across the recovery, none fatal\n{text}")); - if !text.contains("Boot: complete") { - return Err(format!("nothing on screen to preserve\ndecoded screen:\n{text}")); - } - Ok(()) - } - "screen_survived_panic_not_blamed" => { - // `discard_capture` told from a no-op: `capture` freezes a report on - // every panic and the recovery branch drops it, so two deaths in one - // boot and the panel must name the second. Action 0 panics in - // syscall context, which the handler recovers from. - let mut qemu = QemuInstance::boot_with_options( - test_config, - c_bins, - rust_bins, - BootOptions { - profile: qemu::Profile::Gop, - qmp: true, - kernel_features: ACTUATOR_KERNEL, - ..Default::default() - }, - ); - const USERSPACE_PANIC: &str = "SYS_DEBUG: kernel panic triggered by userspace"; - let survived = qemu.run_test("test_rs_test_panic_child", Duration::from_secs(15)); - if let Some(err) = &survived.error { - return Err(format!("the survivable panic never completed: {err}")); - } - if survived.exit_code == Some(0) { - return Err("the survivable panic did not kill the child".to_string()); - } - if !survived.serial.contains(USERSPACE_PANIC) { - return Err(format!( - "no kernel panic in the child's output, so there is no capture to \ - discard and the rest of this test would pass vacuously\nserial:\n{}", - survived.serial - )); - } - // The machine walked away from it: that is what makes this a second death. - if !qemu.command_until( - "run test_rs_test_panic_child 3", - FATAL_HALT_NONCE, - Duration::from_secs(15), - ) { - return Err(format!( - "{FATAL_HALT_NONCE:?} never reached the console, so the guest did not \ - survive the first panic and there is no second death to read" - )); - } - let dump = qemu.screendump_until(FATAL_HALT_NONCE, Duration::from_secs(30)); - let text = dump.text(); - print_screen(name, &text); - // The nonce is logged after the first panic's snapshot was frozen, - // so a snapshot the discard failed to drop cannot carry it. - if !text.contains(FATAL_HALT_NONCE) { - return Err(format!( - "the panel does not name the fatal halt: the survived panic's frozen \ - report was painted as the cause of death, so `discard_capture` did not \ - drop it\ndecoded screen:\n{text}" - )); - } - if dump.fill() != FILL_FATAL { - return Err(format!( - "the report is on screen but the fill is {:?}, not the fatal {FILL_FATAL:?}", - dump.fill() - )); - } - Ok(()) - } other => Err(format!("unknown screen test {other}")), } } @@ -10594,20 +10431,16 @@ fn blocked_dump() -> Result<(), String> { return Err(format!("no parked task was named by pid and tid:\n{report}")); } - // **All three kernel threads, by name.** They are almost always blocked, so + // **Every kernel thread, by name.** They are almost always blocked, so // the parked lines above carry them as a pid and a tid and nothing else — - // and on a machine that has gone quiet the question is *which* of the three - // is stuck. `sched::dump`'s census tags a kernel thread whatever it is - // doing, which is C6's gate: three kernel threads split the work — - // `klogd` the console drain, `usbd` the xHCI port machine, `iod` the - // write-back queue — precisely so that one of them wedging does not stop - // the other two. A report that cannot tell them apart cannot say which did. + // and on a machine that has gone quiet the question is *which* one is + // stuck. `sched::dump`'s census tags a kernel thread whatever it is doing. // // Matched with the ` cpu=` that follows the name on the census line, because // a bare name appears in every one of these programs' own log lines and // `/system/bin/init` speaks in a program's name before that program runs // (`tests/CLAUDE.md`). - let unnamed: Vec<&str> = ["klogd", "usbd", "iod"] + let unnamed: Vec<&str> = ["klogd", "iod"] .into_iter() .filter(|name| !report.contains(&format!(" {name} cpu="))) .collect(); @@ -13407,8 +13240,7 @@ fn run_machine_test( // trampoline that never issues an `iretq`. It gets a process-table // entry rather than a bare task, and that is what makes it // nameable: without one a crash report would print a pid nothing - // in the machine resolves. What each row *means* when the panic - // really fires is `klogd_panic_halts`' two actuator boots. + // in the machine resolves. let qemu = QemuInstance::boot_with_options( test_config, c_bins, @@ -13417,84 +13249,49 @@ fn run_machine_test( ); klogd_hosted(&serial::Serial::boot(&qemu)) } - "klogd_panic_halts" => { - // **A kernel thread's panic is not recoverable by accident.** - // `syscall_rip` is never cleared, so the ordinary recovery - // predicate reads whatever user thread last ran on that CPU, and - // a kernel task would recover or halt by accident of work - // stealing. The row in `sched::kthread` replaces the accident - // with an answer; these two actuator boots walk both branches. - // - // The marker is a line of the crash *report* rather than `PANIC:` - // itself, because `boot_log` stops at the marker and the name is - // printed after the header — a boot stopped at the header would - // have nothing left to assert the process table against. - let mut qemu = QemuInstance::boot_with_options( - test_config, - c_bins, - rust_bins, - BootOptions { - kernel_params: &["klogd-panic"], - ready_marker: "Process: klogd", - ..Default::default() - }, - ); - let mut dead = serial::Serial::boot(&qemu); - dead.must_say("PANIC:")?; - dead.must_say("klogd-panic: the console drainer died")?; - // The process table answered for a task with no *user* address - // space — since C6 it names the kernel's, which is what let - // `KernelPayload.address_space` stop being an `Option`. - dead.must_say("Process: klogd")?; - - // The verdict. A *recovered* panic kills the thread and lets the - // machine carry on into userland, which announces itself; the - // fatal branch halts every CPU. The window is a liveness margin - // and not a threshold: `klogd` panics as the scheduler starts, and - // the arm this must never become reaches the marker a few hundred - // milliseconds later — so three seconds is a tenfold margin over - // the state it refuses, and it is the whole of this test's fixed - // cost against the Fast ceiling. - const CARRIED_ON: Duration = Duration::from_secs(3); - dead.push(&qemu.drain_serial(CARRIED_ON)); - dead.must_not_say(qemu::DEFAULT_READY)?; - eprintln!(" [klogd] a kernel thread's panic halted the machine rather than recovering"); - - drop(qemu); - - // **The same panic on the other row, and it is the direction - // nothing had ever taken.** Two rows in one table are one row - // until both branches have been walked: before this arm, every - // kernel-thread panic this tree had ever run took `OnPanic::Halt`, - // so `Recover` was a value rather than a path — and the path it - // names goes through `poison_tid`, the idle loop's `reap_poisoned` - // and `zombify_poisoned`, none of which had ever seen a task with - // no user address space. A row that quietly halted the machine - // would make `usbd` and `iod` worse than the thread they were - // split off from. - // - // The verdict is content in the same window and never a timeout: - // the boot returns at the crash report's own line, and what the - // three seconds after it must contain is the ready marker the - // arm above must *not*. - let mut qemu = QemuInstance::boot_with_options( - test_config, - c_bins, - rust_bins, - BootOptions { - kernel_params: &["usbd-panic"], - ready_marker: "Process: usbd", - ..Default::default() - }, - ); - let mut survived = serial::Serial::boot(&qemu); - survived.must_say("PANIC:")?; - survived.must_say("usbd-panic: the device thread died")?; - survived.must_say("Process: usbd")?; - survived.push(&qemu.drain_serial(CARRIED_ON)); - survived.must_say(qemu::DEFAULT_READY)?; - eprintln!(" [usbd] a kernel thread's panic killed the thread and the machine booted"); - Ok(()) + "klogd_panic_halts" => power::klogd_death_resets( + test_config, + c_bins, + rust_bins, + &["klogd-panic", "panic-reboot-fast"], + &["klogd-panic: the console drainer died", "Process: klogd"], + ), + "klogd_fault_halts" => power::klogd_death_resets( + test_config, + c_bins, + rust_bins, + &["klogd-fault", "panic-reboot-fast"], + &["KERNEL PANIC: read unmapped address at 0x0", "console::body"], + ), + "syscall_panic_halts" | "syscall_fault_halts" | "lock_across_switch_halts" + | "heap_over_ceiling_halts" => { + use toyos_abi::syscall::{debug_action as da, SYS_DEBUG}; + let syscall = format!("Syscall: num={SYS_DEBUG}"); + let syscall = syscall.as_str(); + let (action, said): (u64, &[&str]) = match name { + "syscall_panic_halts" => ( + da::PANIC, + &["SYS_DEBUG: kernel panic triggered by userspace", syscall, "User backtrace:"], + ), + // A Ring 0 read of a user address is the kernel's, inside a syscall too. + "syscall_fault_halts" => (da::NULL_READ, &[syscall, "User backtrace:"]), + "lock_across_switch_halts" => (da::LOCK_ACROSS_SWITCH, &[syscall]), + // The message, not `mm/alloc.rs`: it names the ceiling rather + // than the page source's own request. + "heap_over_ceiling_halts" => { + (da::HEAP_OVER_CEILING, &["exceeds MAX_HEAP_ALLOC", syscall]) + } + other => unreachable!("{other} is not a syscall-death row"), + }; + let said = power::syscall_death_resets(test_config, c_bins, rust_bins, action, said)?; + // With the capture: this guest's 16550 is its stdio, so no + // `uart-*.log` keeps what it said. + match name { + "lock_across_switch_halts" => check_tripwire_attribution(&said), + "syscall_fault_halts" => check_ring0_read_unmapped(&said), + _ => Ok(()), + } + .map_err(|e| format!("{e}\n{said}")) } "hash_seed_precedes_every_map" => { // `kernel/src/hasher.rs`'s `UNSEEDED`, as a prefix: the wrong seed @@ -13805,57 +13602,19 @@ fn run_machine_test( eprintln!(" [sleep] {}", result.stdout.lines().last().unwrap_or("").trim()); Ok(()) } - "heap_ceiling_recovery" => { - // A panic inside the kernel allocator's own lock left the heap - // locked for the rest of the boot: the panicking thread never - // unwinds, so `now` never advances, and the CPU that recovered - // spun `Lock::lock` to its 500M-spin deadline on its next `alloc` - // or `free` — then panicked again, forever. The fix moved the - // ceiling check to `KernelAllocator::alloc`, before the lock. - // - // `smp: 1` is what makes the claim precise. The property is that - // *the recovered CPU* survives its next allocation; on a wider - // machine `/system/bin/echo` could run somewhere else and pass without - // touching it. With one CPU there is nowhere else. - // - // The actuator is SYS_DEBUG 5, 6 and 7, and the reason it is not - // an ordinary workload is beside them in `syscall/dispatch.rs`: routes - // past the ceiling do still exist, - // and each of them holds the VFS lock when it dies, so the - // machine wedges either way and the allocator's recovery cannot - // be observed on its own. - let options = BootOptions { - smp: 1, - kernel_features: ACTUATOR_KERNEL, - ..Default::default() - }; + "heap_ceiling_bounds" => { + // Its own boot: `LOWER_SYSINFO_BOUND` stays lowered for the rest of it. + let options = BootOptions { kernel_features: ACTUATOR_KERNEL, ..Default::default() }; let mut qemu = QemuInstance::boot_with_options(test_config, c_bins, rust_bins, options); serial::Serial::boot(&qemu).must_be_clean()?; let result = qemu.run_test("test_rs_heap_ceiling", Duration::from_secs(30)); if let Some(err) = &result.error { - // The wedge's signature. Before the fix this is where the test - // ends: the child's panic strands the allocator, the guest - // stops answering, and `run_test` runs out of window. - return Err(format!( - "the guest stopped answering after the over-ceiling panic: {err}\n\ - serial:\n{}", - result.serial - )); + return Err(format!("heap_ceiling did not finish: {err}\nserial:\n{}", result.serial)); } if !check_rust_result(&result) { return Err(format!("heap_ceiling failed:\n{}", result.stdout)); } - - // The panic must be the one this test asked for, and it must have - // fired where the fix put it. `mm/alloc.rs` appears in the report - // either way — the old assert was in the same file — so the needle - // is the message, which names the ceiling rather than the page - // source's own request. - let serial = serial::Serial::named("test serial", result.serial.as_str()); - serial.must_say("PANIC:")?; - let line = serial.must_say("exceeds MAX_HEAP_ALLOC")?; - eprintln!(" [heap] {}", line.trim()); Ok(()) } "cache_eviction" => { @@ -17177,30 +16936,14 @@ fn operation_nesting_log(log: &str) -> Result<(), String> { Ok(()) } -/// The machine's three kernel threads are hosted, and each claims the panic row -/// its own loss demands. +/// The machine's kernel threads are hosted. /// -/// Text in, a verdict out: all three lines are `log!` records, so the T14's +/// Text in, a verdict out: every line is a `log!` record, so the T14's /// readback and a QEMU boot log are judged by this one predicate. fn klogd_hosted(boot: &serial::Serial) -> Result<(), String> { boot.must_be_clean()?; - let line = boot.must_say("kthread: klogd")?; - if !line.contains("halts the machine") { - return Err(format!("klogd is hosted but claims the wrong panic row: {line:?}")); - } - eprintln!(" [klogd] {}", line.trim()); - - // **The other two threads, and the opposite row.** `usbd` owns the xHCI - // port machine and `iod` the write-back queue, so a stuck USB enumeration - // cannot stop the log. Their panics are *recoverable* and `klogd`'s - // deliberately is not — a killed drainer is the one loss nothing left alive - // can report — and this is the one boot in the suite where all three rows - // are on the wire together. - for name in ["usbd", "iod"] { + for name in ["klogd", "iod"] { let line = boot.must_say(&format!("kthread: {name}"))?; - if !line.contains("kills the thread") { - return Err(format!("{name} is hosted but claims the wrong panic row: {line:?}")); - } eprintln!(" [kthread] {}", line.trim()); } Ok(()) @@ -18500,7 +18243,6 @@ fn build_test_registry( for name in discover_rust_tests(rust_bins) { let timeout = match name.as_str() { - "panic_recovery" => Duration::from_secs(10), // Writes the child's whole image through bcachefs before it can run // it, which is the only thing here that is not a spawn. "disk_backtrace" => Duration::from_secs(15), @@ -19186,10 +18928,7 @@ fn one_vocabulary() -> Result<(), String> { /// What the declaration itself has to be, before any of it means anything. /// Which shared-boot binaries need `SYS_DEBUG`, asked of their source. /// -/// A name reaches the syscall directly, or through a child it spawns — -/// `panic_recovery`'s three actions are all `test_panic_child`'s, and a rule -/// that only read the test's own source would miss the one test in the list -/// whose whole subject is the syscall. +/// A name reaches the syscall directly, or through a child it spawns. fn needs_actuators(sources: &[(String, String)], registry: &[&str]) -> BTreeSet { // The fourth spelling is the argument-taking form: every action that // carries a payload (TLB_ACK_DELAY_ARM, CENSUS_KIND, LOWER_SYSINFO_BOUND, @@ -19222,14 +18961,12 @@ fn needs_actuators(sources: &[(String, String)], registry: &[&str]) -> BTreeSet< /// `SYS_DEBUG`, and the binaries are what is asked. /// /// **What this does not cover, stated because the hole is real:** a machine or -/// screen test that *drives* one of those binaries on a boot of its own. -/// `screen_recoverable_untouched` was the instance — it runs -/// `test_rs_test_panic_child` on a featureless kernel, where action 0 is answered -/// `InvalidArgument` and the child exits 0 — and no static rule here can say -/// which `BootOptions` a `run_test` call belongs to. What answers it instead is -/// the guest: `test_panic_child` names `InvalidArgument` as *this kernel carries -/// no actuators* rather than reporting a kernel that failed to kill anybody, so -/// the red says what is wrong wherever it happens. +/// screen test that *drives* one of those binaries on a boot of its own. No +/// static rule here can say which `BootOptions` a `run_test` call belongs to. +/// What answers it instead is the guest: `test_panic_child` names +/// `InvalidArgument` as *this kernel carries no actuators* rather than reporting +/// a kernel that failed to stop, so the red says what is wrong wherever it +/// happens. /// /// **Both directions are the point.** A binary that gains a `debug()` call and /// no entry would run on the shipping kernel, where the syscall answers diff --git a/toyos-abi/src/syscall.rs b/toyos-abi/src/syscall.rs index 03d9f2d2dff..f471ce741fe 100644 --- a/toyos-abi/src/syscall.rs +++ b/toyos-abi/src/syscall.rs @@ -788,9 +788,9 @@ pub fn debug_with(action: u64, arg: u64) -> u64 { pub mod debug_action { /// Panic the kernel. Kills the caller's machine, deliberately. pub const PANIC: u64 = 0; - /// Read through a null pointer in Ring 0. + /// Read, in Ring 0, the address the argument names. pub const NULL_READ: u64 = 1; - /// Hold a kernel lock across a scheduler entry. Armed once per boot. + /// Hold a kernel lock across a scheduler entry. pub const LOCK_ACROSS_SWITCH: u64 = 2; /// Halt every CPU. pub const FATAL_HALT: u64 = 3; diff --git a/toyos-proclife/src/interleave.rs b/toyos-proclife/src/interleave.rs index 9eca5031a0b..12d532d72c6 100644 --- a/toyos-proclife/src/interleave.rs +++ b/toyos-proclife/src/interleave.rs @@ -44,7 +44,7 @@ pub enum Op { ThreadExit { pid: Pid, tid: Tid, code: i32, pc: u32, post: Option }, /// `sys_thread_join`: collect or arm, then re-check. Join { pid: Pid, target: Tid, waiter: Tid, pc: u32 }, - /// The idle loop's `reap_poisoned`, reap half. + /// The idle loop's `reap_finished`. IdlePass { pc: u32 }, } diff --git a/toyos-proclife/src/lib.rs b/toyos-proclife/src/lib.rs index 4c5a7d38c53..c3f84577531 100644 --- a/toyos-proclife/src/lib.rs +++ b/toyos-proclife/src/lib.rs @@ -52,7 +52,6 @@ extern crate alloc; extern crate std; pub mod join; -pub mod poison; pub mod reap; pub mod spawn; pub mod table; @@ -115,22 +114,6 @@ pub enum Watch { Process(Pid), } -impl Watch { - /// The thread this names, or `None` for a process's own watch. - /// - /// Total rather than a match at the call site: the kernel resolves a - /// [`Watch::Thread`] through `process::thread_sched` and a - /// [`Watch::Process`] through the object, and a caller that can only - /// perform one of the two says so here instead of writing an arm it - /// believes is unreachable. - pub fn thread(self) -> Option<(Pid, Tid)> { - match self { - Self::Thread(pid, tid) => Some((pid, tid)), - Self::Process(_) => None, - } - } -} - /// The code every thread but the main one is marked dead with when a process is /// torn down out from under it. /// diff --git a/toyos-proclife/src/model.rs b/toyos-proclife/src/model.rs index cd22e5a0232..34121218fc0 100644 --- a/toyos-proclife/src/model.rs +++ b/toyos-proclife/src/model.rs @@ -179,12 +179,6 @@ impl World { } } - pub fn forget_thread(&mut self, pid: Pid, tid: Tid) { - if let Some(proc) = self.procs.get_mut(&pid) { - proc.forget_thread(tid); - } - } - /// `watch::wait_until` — a waiter registered on a subject's watch. pub fn arm(&mut self, on: Watch, waiter: (Pid, Tid)) { self.waiters.insert((on, waiter.0, waiter.1)); diff --git a/toyos-proclife/src/poison.rs b/toyos-proclife/src/poison.rs deleted file mode 100644 index 6581ee5f3cd..00000000000 --- a/toyos-proclife/src/poison.rs +++ /dev/null @@ -1,129 +0,0 @@ -//! What a thread that died in panic recovery leaves to be cleaned up. -//! -//! The panic path itself can do none of it — it may hold any lock the faulted -//! thread was holding — so it records the thread in a per-CPU poison slot and -//! the idle loop runs this later, which is the one context that provably holds -//! none of them. -//! -//! **It is a teardown like any other and takes the same claim.** A poisoned -//! main thread ends its process, so it competes with a `SYS_EXIT` on another -//! thread and with a `SYS_PROCESS_KILL` from a handle holder, and exactly one -//! of the three publishes the exit. What makes this path different is only what -//! it *cannot* do: no resources are released, because every release below it -//! wants a lock the faulted thread may still be recorded as holding, so the -//! process's mappings and handles go with the table entry rather than before -//! it. - -use crate::table::{Lifecycle, Processes}; -use crate::teardown; -use crate::{Pid, ThreadLocation, Tid, Watch, TORN_DOWN_THREAD_CODE}; - -/// What must be woken for a poisoned thread, once the table lock is given up. -/// -/// Both wakes are carried out by the caller rather than performed here, because -/// both must happen with that lock released. -#[must_use = "a poisoned thread's waiter must be woken"] -#[derive(Clone, Copy, PartialEq, Eq, Debug)] -pub enum PoisonOutcome { - /// Nothing to do: the entry is gone, or another path already owns this - /// process's teardown and will publish its exit. - Nothing, - /// A child thread died. **The subject names the thread that died**, which - /// is what a `thread_join` arms on — it used to name the process's main - /// thread, because the wake was by name into a shared parking lot and - /// whoever was woken re-checked. - Joiner(Watch), - /// The main thread died, so the process is over. The exit is published on - /// the object — outside the table lock, like every other publish — and - /// whoever holds a handle reads it there. - Process(Pid), -} - -/// Mark a poisoned thread dead and name what must be woken for it. -pub fn zombify_poisoned(table: &mut T, pid: Pid, tid: Tid) -> PoisonOutcome { - let Some(proc) = table.get(pid) else { return PoisonOutcome::Nothing }; - let main_tid = proc.main_tid(); - - if tid != main_tid { - if proc.location(tid).is_none() { - return PoisonOutcome::Nothing; - } - let proc = table.get_mut(pid).expect("the entry answered a line ago"); - if proc.location(tid).is_some_and(|l| !l.is_zombie()) { - proc.set_location(tid, ThreadLocation::Zombie(TORN_DOWN_THREAD_CODE)); - } - return PoisonOutcome::Joiner(Watch::Thread(pid, tid)); - } - - // The same claim every exit and kill takes, for the same reason: exactly - // one path publishes one exit. - if !teardown::claim_teardown(table, pid) { - return PoisonOutcome::Nothing; - } - let proc = table.get_mut(pid).expect("the claim just succeeded on this entry"); - if proc.location(tid).is_none() { - return PoisonOutcome::Nothing; - } - proc.set_location(tid, ThreadLocation::Zombie(TORN_DOWN_THREAD_CODE)); - PoisonOutcome::Process(pid) -} - -#[cfg(test)] -mod tests { - use super::*; - use crate::model::World; - - #[test] - fn a_poisoned_sibling_names_itself_and_not_the_main_thread() { - let mut world = World::new(); - let pid = world.spawn_process(); - let t1 = world.spawn_thread(pid); - assert_eq!(zombify_poisoned(&mut world, pid, t1), PoisonOutcome::Joiner(Watch::Thread(pid, t1))); - assert_eq!( - world.get(pid).unwrap().location(t1), - Some(ThreadLocation::Zombie(TORN_DOWN_THREAD_CODE)), - ); - assert!( - !world.get(pid).unwrap().tearing_down(), - "a sibling's death is not the process's, so it takes no claim", - ); - } - - #[test] - fn a_poisoned_main_thread_takes_the_claim_and_ends_the_process() { - let mut world = World::new(); - let pid = world.spawn_process(); - let main = world.main_tid(pid); - assert_eq!(zombify_poisoned(&mut world, pid, main), PoisonOutcome::Process(pid)); - assert!(world.get(pid).unwrap().tearing_down()); - } - - /// Not under `mutate-claim-teardown-always-wins`: this asserts the very - /// exclusion that control removes, so it would red for the mutation rather - /// than for a law, and the step that reads the arm's verdict lines could - /// not tell the two apart. - #[cfg(not(feature = "mutate-claim-teardown-always-wins"))] - #[test] - fn a_process_another_path_already_claimed_is_left_alone() { - let mut world = World::new(); - let pid = world.spawn_process(); - let main = world.main_tid(pid); - assert!(teardown::claim_teardown(&mut world, pid)); - assert_eq!(zombify_poisoned(&mut world, pid, main), PoisonOutcome::Nothing); - } - - #[test] - fn a_reaped_entry_is_nothing_to_do_rather_than_a_panic() { - let mut world = World::new(); - assert_eq!(zombify_poisoned(&mut world, Pid(6), Tid(0)), PoisonOutcome::Nothing); - } - - #[test] - fn a_sibling_a_join_already_collected_is_nothing_to_do() { - let mut world = World::new(); - let pid = world.spawn_process(); - let t1 = world.spawn_thread(pid); - world.forget_thread(pid, t1); - assert_eq!(zombify_poisoned(&mut world, pid, t1), PoisonOutcome::Nothing); - } -} diff --git a/toyos-proclife/src/teardown.rs b/toyos-proclife/src/teardown.rs index ac9ca688e2c..b4fde75ff08 100644 --- a/toyos-proclife/src/teardown.rs +++ b/toyos-proclife/src/teardown.rs @@ -1,10 +1,9 @@ //! Who ends a process, which threads they must retire, and what a thread's own //! exit is. //! -//! **Exactly one path publishes exactly one exit.** Three of them can arrive at -//! once — a `SYS_EXIT` on the process's own main thread, a `SYS_PROCESS_KILL` -//! from a holder of a `Process` handle, and the idle loop's sweep of threads -//! that died in panic recovery — and the whole arrangement rests on +//! **Exactly one path publishes exactly one exit.** Two of them can arrive at +//! once — a `SYS_EXIT` on the process's own main thread and a `SYS_PROCESS_KILL` +//! from a holder of a `Process` handle — and the whole arrangement rests on //! [`claim_teardown`] answering `true` to one of them. A second publish is an //! assertion failure in `ProcessObject::publish_exit`, by design: it means two //! teardowns claimed one process, and a kernel that tolerated it would free one @@ -26,7 +25,7 @@ use crate::{Pid, ThreadLocation, Tid, Watch, TORN_DOWN_THREAD_CODE}; /// Claim exclusive teardown of a process. /// -/// Exactly one exit/kill/poison path wins; a later caller must simply exit its +/// Exactly one exit or kill path wins; a later caller must simply exit its /// own thread — the claimant's retire sweep handles it like any other thread. /// `false` also covers a process that is not in the table at all, because there /// is nothing left for a second claimant to do either way. diff --git a/toyos-sched/loom/tests/loom_retire.rs b/toyos-sched/loom/tests/loom_retire.rs index d908fd2d87e..e518c8708f7 100644 --- a/toyos-sched/loom/tests/loom_retire.rs +++ b/toyos-sched/loom/tests/loom_retire.rs @@ -336,13 +336,11 @@ impl Kicker for Silent { } impl Machine for Silent { - type IrqGuard = (); fn now(&self) -> Nanos { NOW } fn set_timer(&self, _deadline: Nanos) {} fn stop_timer(&self) {} - fn irq_guard(&self) {} fn halt(&self) {} fn need_resched(&self, _cpu: CpuId) {} fn trace(&self, _ev: TraceEvent) {} diff --git a/toyos-sched/sim/src/hw_impl.rs b/toyos-sched/sim/src/hw_impl.rs index cc93e5900b9..f3b66ff1546 100644 --- a/toyos-sched/sim/src/hw_impl.rs +++ b/toyos-sched/sim/src/hw_impl.rs @@ -123,11 +123,6 @@ impl Kicker for SimHw { } impl Machine for SimHw { - /// A step is atomic in this world, so "IRQs off" is the default rather - /// than a state to enter: delivery steps are simply not enabled while a - /// pass runs. The guard is therefore a witness with nothing to carry. - type IrqGuard = (); - /// The VM threads `now` into every pass as a value, so this is read by /// exactly one caller: the core's check-build pass-cost recorder. Inside /// a pass it therefore reports the pass's modelled cost; outside one it is @@ -153,8 +148,6 @@ impl Machine for SimHw { }); } - fn irq_guard(&self) {} - fn halt(&self) { self.with(|s| { let cpu = Self::cpu(s); diff --git a/toyos-sched/src/cpu.rs b/toyos-sched/src/cpu.rs index 4abb0582bff..f26d485fc57 100644 --- a/toyos-sched/src/cpu.rs +++ b/toyos-sched/src/cpu.rs @@ -2498,13 +2498,11 @@ mod tests { } impl Machine for TestHw { - type IrqGuard = (); fn now(&self) -> Nanos { Nanos::ZERO } fn set_timer(&self, _deadline: Nanos) {} fn stop_timer(&self) {} - fn irq_guard(&self) {} fn halt(&self) {} fn need_resched(&self, cpu: CpuId) { self.state().need_resched.push(cpu); diff --git a/toyos-sched/src/hw.rs b/toyos-sched/src/hw.rs index 4603bfc9f6d..6006cb79154 100644 --- a/toyos-sched/src/hw.rs +++ b/toyos-sched/src/hw.rs @@ -86,8 +86,6 @@ pub trait Kicker: Sync { /// one-shot timer, interrupt gate, halt, resched request, trace sink. Split /// from [`Hw`] so an implementor needs no [`SchedPayload`] to provide it. pub trait Machine: Kicker + 'static { - type IrqGuard; - /// Sampled ONCE per pass by the driver and threaded as a value — the /// core never reads the clock mid-flight. fn now(&self) -> Nanos; @@ -99,19 +97,9 @@ pub trait Machine: Kicker + 'static { fn stop_timer(&self); - /// Kernel: cli/sti RAII. Sim: gates event delivery for this vcpu. - /// - /// Has no caller in either world, and does **not** fit the site it looks - /// like it should — the idle loop's cli / final recheck / sti;hlt: both exits - /// from that recheck must *set* IF unconditionally — the halt exit because - /// `sti;hlt` is one atom, the stay-awake exit because panic recovery - /// enters the idle loop with IF already 0 — and an RAII guard restores - /// the caller's flags instead. - fn irq_guard(&self) -> Self::IrqGuard; - /// Enable interrupts and halt, atomically — on x86 the `sti;hlt` pair and /// its STI shadow, which is why this is one operation and not an - /// [`Self::irq_guard`] drop followed by a halt. A wake that lands in + /// IRQ-guard drop followed by a halt. A wake that lands in /// between would be consumed as an ordinary interrupt and then slept /// through. Returns once an interrupt has been taken. /// diff --git a/toyos-sched/src/task.rs b/toyos-sched/src/task.rs index 642f5af5439..419de23ae15 100644 --- a/toyos-sched/src/task.rs +++ b/toyos-sched/src/task.rs @@ -273,12 +273,9 @@ fn legal(from: TaskState, to: TaskState) -> bool { // Dispositions of the running task; the home CPU never changes here. (Running(a), Ready(b)) | (Running(a), Committing(b, _)) => a == b, (Running(_), Dead) => true, - // Pick and migrate. `Ready → Dead` is not a reap any more: since the - // cancellable kill nothing converts a ready task to a dead one, and - // the edge survives for the *panic* path, where `schedule_no_return` - // buries a context that cannot be resumed. + // Pick and migrate. (Ready(a), Running(b)) => a == b, - (Ready(_), InTransit(_)) | (Ready(_), Dead) => true, + (Ready(_), InTransit(_)) => true, // The two-phase wait handshake. (Committing(a, _), Running(b)) | (Committing(a, _), Blocked(b)) @@ -676,8 +673,7 @@ impl TaskShared { prev & RETIRE_QUEUED == 0 } - /// Mark the task killed without queuing a retire — the panic-recovery - /// path, which abandons the task instead of retiring it. + /// Mark the task killed without queuing a retire. pub fn mark_kill(&self) { self.state.fetch_or(KILL, Ordering::AcqRel); } diff --git a/toyos-userbound/src/fault.rs b/toyos-userbound/src/fault.rs index 94191e7c323..db71059dc3d 100644 --- a/toyos-userbound/src/fault.rs +++ b/toyos-userbound/src/fault.rs @@ -1,43 +1,14 @@ -//! Whose fault a trap was — the one classification the crash path makes, and -//! the only thing deciding whether a process dies or the machine halts. +//! Whose fault a trap was — the one classification the crash path makes. A +//! Ring 3 fault is its process's, and the process dies; any Ring 0 fault is the +//! kernel's, whatever thread or syscall is current, and the machine halts. //! //! **The ring is a fact the frame carries, and this file exists so that nothing -//! guesses it from anything else.** The decision used to be spelled -//! `is_user_mode() || (PageFault && current_tid().is_some() && cr2 < USER_TOP)`, -//! and the second disjunct answers *user* for any low faulting address however -//! Ring 0 the frame was. On 2026-08-17 the kernel fetched an instruction from -//! address zero under `sched_stress` — -//! `#PF UNHANDLED: cr2=0x0 rip=0x0 err=0x10 user=false tid=Some(Tid(0))` — the -//! disjunct held on `cr2 = 0` and a live tid, and the kernel went down the -//! *recover* path for a null jump instead of halting on it. The `KERNEL PANIC` -//! and the crash report that would have named the null call were therefore -//! never written, and the shared boot said nothing for the whole 88 s guard. -//! The defect underneath was a `context_switch` restoring a task another CPU -//! was still standing on; the reports this repair unblocked are what named it, -//! four sightings later. -//! -//! [`Ring`] is the repair. It is opaque and its one constructor takes a code -//! segment selector, so a privilege level cannot be read out of a faulting -//! address by any caller: the confusion is unwritable rather than fixed at one -//! call site. What no type can settle is the genuinely ambiguous case that -//! second disjunct existed for — Ring 0 code dereferencing a pointer that -//! crossed the syscall boundary, which must kill the process and not the -//! machine — so [`blame`] answers that one at runtime, from the two addresses -//! the frame carries rather than from one of them. -//! -//! The one thing this names outside itself is the user/kernel bound, and it -//! names it in [`crate::span`] — the same constant `user_ptr.rs` refuses an -//! address above, not a copy of it. That is what makes the two boundary rows in -//! the table below mean anything. - -use crate::span::is_user_addr; +//! guesses it from anything else**, such as a faulting address. /// The privilege level a trap frame arrived from. /// /// Opaque, and there is one constructor: the frame's own `cs`. A `Ring` is -/// therefore never anything but what the hardware pushed, which is the whole -/// point of the type — the classification below cannot be handed a ring -/// somebody inferred. +/// therefore never anything but what the hardware pushed. #[derive(Clone, Copy, PartialEq, Eq, Debug)] pub struct Ring(bool); @@ -54,75 +25,10 @@ impl Ring { } } -/// The address a trap names, on the one vector that names one. -/// -/// A #PF reports the linear address it could not translate in CR2; every other -/// vector leaves whatever was there last, so reading it would attribute a fault -/// to an address that has nothing to do with it. -#[derive(Clone, Copy, PartialEq, Eq, Debug)] -pub enum Faulted { - /// A page fault, and what CR2 held. - Address(u64), - /// Any other vector: there is no faulted address. - Nothing, -} - -/// Whose fault it was, and therefore what the kernel does next. -/// -/// Three states, where the crash path used to carry two independent `bool`s -/// (`is_user`, `is_ring3`) — of whose four combinations one, "a user fault from -/// a frame that was not Ring 3 and not in a syscall either", meant nothing and -/// was still writable. -#[derive(Clone, Copy, PartialEq, Eq, Debug)] -pub enum Blame { - /// A Ring 3 frame. The process did it, it holds no kernel lock, and the - /// ordinary exit path can end it. - Process, - /// Ring 0 code, running on a thread's behalf, faulting on a *user* address: - /// a pointer that crossed the syscall boundary. Still the process's, but - /// the faulted thread may hold any kernel lock, so it goes out through the - /// poison set rather than through the process table. - ProcessThroughKernel, - /// Ring 0, and nothing about it belongs to a process. The machine halts, - /// after saying so. - Kernel, -} - -/// Who a fault belongs to. -/// -/// `rip` is the faulting frame's instruction pointer and `on_a_thread` is -/// whether a thread is current on this CPU. -/// -/// **A Ring 0 frame whose `rip` is not a kernel address is the kernel's however -/// low the faulted address is.** That is the sighting above: broken control -/// flow, not a bad pointer — the kernel is not *at* an instruction that could -/// be dereferencing anything on a thread's behalf. An instruction-fetch fault -/// needs no separate arm for the same reason: the address it could not fetch is -/// where `rip` now points, so it fails this test by itself. -pub fn blame(ring: Ring, rip: u64, faulted: Faulted, on_a_thread: bool) -> Blame { - if ring.is_user() { - return Blame::Process; - } - match faulted { - Faulted::Address(addr) - if on_a_thread && !is_user_addr(rip) && is_user_addr(addr) => - { - Blame::ProcessThroughKernel - } - _ => Blame::Kernel, - } -} - #[cfg(test)] mod tests { use super::*; - /// A kernel `rip`, the shape every Ring 0 frame in this kernel has: the - /// direct map starts at `0xFFFF_8000_0000_0000` and the crash reports quote - /// addresses like `0xffff80007b519a02`. - const KERNEL_RIP: u64 = 0xFFFF_8000_7B51_9A02; - /// A user `rip`, from a PIE loaded at the base this kernel uses. - const USER_RIP: u64 = 0x0000_0100_0009_7176; /// A Ring 3 code selector, and a Ring 0 one. const USER_CS: u64 = 0x2B; const KERNEL_CS: u64 = 0x08; @@ -138,122 +44,4 @@ mod tests { } assert!(!Ring::of_cs(0).is_user()); } - - /// The sighting: Ring 0 fetching an instruction from address zero. - /// - /// It must be the kernel's whatever `current_tid()` says, because that is - /// the only answer that prints `KERNEL PANIC` and halts. Under the old - /// disjunct it was the process's, and the report never arrived. - #[test] - fn a_ring_0_fault_at_a_low_address_is_the_kernels() { - assert_eq!( - blame(Ring::of_cs(KERNEL_CS), 0, Faulted::Address(0), true), - Blame::Kernel, - ); - // Not a property of zero: any address in the user half, reached from a - // Ring 0 frame that is not executing kernel code, is the same finding. - assert_eq!( - blame(Ring::of_cs(KERNEL_CS), USER_RIP, Faulted::Address(0x1B), true), - Blame::Kernel, - ); - // And with no thread current at all, which is the case the old spelling - // did get right. - assert_eq!( - blame(Ring::of_cs(KERNEL_CS), 0, Faulted::Address(0), false), - Blame::Kernel, - ); - } - - /// The direction a careless fix breaks: a process that dereferences null - /// must still be the process's, and must not take the machine with it. - #[test] - fn a_user_fault_at_a_low_address_is_still_the_processs() { - assert_eq!( - blame(Ring::of_cs(USER_CS), USER_RIP, Faulted::Address(0), true), - Blame::Process, - ); - // A Ring 3 frame is the process's on every vector, with or without a - // faulted address: #UD, #GP and the arithmetic traps all arrive here. - assert_eq!( - blame(Ring::of_cs(USER_CS), USER_RIP, Faulted::Nothing, true), - Blame::Process, - ); - } - - /// The other half of that direction, and the one the old disjunct was - /// written for: `SYS_DEBUG`'s `NULL_READ` reads address zero from *kernel* - /// code inside a syscall, and `panic_recovery` asserts the caller dies and - /// the machine lives. - #[test] - fn a_kernel_dereference_of_a_user_pointer_is_the_processs() { - assert_eq!( - blame(Ring::of_cs(KERNEL_CS), KERNEL_RIP, Faulted::Address(0), true), - Blame::ProcessThroughKernel, - ); - assert_eq!( - blame( - Ring::of_cs(KERNEL_CS), - KERNEL_RIP, - Faulted::Address(crate::span::USER_TOP - 1), - true, - ), - Blame::ProcessThroughKernel, - ); - // No thread to attribute it to, so there is nothing to kill but the - // machine's illusion that it is well. - assert_eq!( - blame(Ring::of_cs(KERNEL_CS), KERNEL_RIP, Faulted::Address(0), false), - Blame::Kernel, - ); - } - - /// A kernel address faulted from kernel code is the kernel's, which is what - /// `idle_stack_guard` asserts by halting: the guard page under a per-CPU - /// idle stack is read on purpose and the machine must stop. - #[test] - fn a_kernel_address_faulted_from_kernel_code_is_the_kernels() { - assert_eq!( - blame( - Ring::of_cs(KERNEL_CS), - KERNEL_RIP, - Faulted::Address(0xFFFF_8000_0102_0FFF), - true, - ), - Blame::Kernel, - ); - } - - /// Every vector that is not a page fault carries no faulted address, so a - /// Ring 0 one is the kernel's — a `#GP` inside a syscall halts rather than - /// killing whichever process happened to be current. - #[test] - fn a_ring_0_trap_with_no_faulted_address_is_the_kernels() { - assert_eq!( - blame(Ring::of_cs(KERNEL_CS), KERNEL_RIP, Faulted::Nothing, true), - Blame::Kernel, - ); - } - - /// The bound itself, at both of the places the classification turns on it. - #[test] - fn the_user_kernel_bound_is_where_both_answers_change() { - use crate::span::USER_TOP; - let k = Ring::of_cs(KERNEL_CS); - // The faulted address, one byte either side. - assert_eq!( - blame(k, KERNEL_RIP, Faulted::Address(USER_TOP - 1), true), - Blame::ProcessThroughKernel, - ); - assert_eq!(blame(k, KERNEL_RIP, Faulted::Address(USER_TOP), true), Blame::Kernel); - // And `rip`, on the same bound and in the other direction: the test is - // "not in the user half", so the highest address userland can name is - // the last `rip` that answers `Kernel`, and the first one past it — the - // start of the non-canonical hole, which no frame can hold — is the - // first that does not. - assert_eq!(blame(k, USER_TOP - 1, Faulted::Address(0), true), Blame::Kernel); - assert_eq!( - blame(k, USER_TOP, Faulted::Address(0), true), - Blame::ProcessThroughKernel, - ); - } } diff --git a/toyos-userbound/src/lib.rs b/toyos-userbound/src/lib.rs index ee82afec724..5729263857d 100644 --- a/toyos-userbound/src/lib.rs +++ b/toyos-userbound/src/lib.rs @@ -4,15 +4,9 @@ //! address userland's, is the object at it aligned for the type being read, and //! does it lie wholly inside one mapping? **Before a placement**: can a length //! userland asked for be placed at all, and where does it go? **After a trap**: -//! which side did the frame come from, and whose fault was it? +//! which side did the frame come from? //! -//! [`span`] answers the first, [`place`] the second and [`fault`] the third — -//! and the third is written in terms of the first. A fault is classified -//! against the same [`USER_TOP`] the accessors refuse an address above, not -//! against a copy of it, and that is why those two are one crate rather than -//! two: a second constant is a second -//! thing to get wrong, and [`blame`]'s whole argument is that the bound it -//! reads is the kernel's own. +//! [`span`] answers the first, [`place`] the second and [`fault`] the third. //! //! Pure. No I/O, no allocation, no `unsafe`, nothing read from a device and //! nothing named outside this crate. The kernel is the only caller — @@ -33,7 +27,7 @@ pub mod fault; pub mod place; pub mod span; -pub use fault::{blame, Blame, Faulted, Ring}; +pub use fault::Ring; pub use place::{PageSpan, Window}; pub use span::{ align_2m_checked, contiguous, in_user_half, is_user_addr, is_user_object, rebase_base, Access,