From c14fc9af2a1faace53718008b83f93f0aca72cdb Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 27 Sep 2026 20:08:49 +0200 Subject: [PATCH 01/17] issues: the kernel still creates threads (track) The owner's ruling: the kernel creates no thread but the per-CPU idle loop, and the ruling holds only once the whole kernel-thread machinery is deleted. The track names its exit condition, the spawn sites left once K1 lands, and stages K1-K6 with what unblocks each. Co-Authored-By: Claude Opus 5.5 --- .../the-kernel-still-creates-threads.md | 45 +++++++++++++++++++ kernel/src/drivers/xhci/usbd.rs | 40 ----------------- 2 files changed, 45 insertions(+), 40 deletions(-) create mode 100644 issues/kernel/the-kernel-still-creates-threads.md delete mode 100644 kernel/src/drivers/xhci/usbd.rs 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..60f226622de --- /dev/null +++ b/issues/kernel/the-kernel-still-creates-threads.md @@ -0,0 +1,45 @@ +--- +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` once K1 has +landed: + +``` +kernel/src/iod.rs:15: let _ = kthread::spawn(NAME, body, 0); +kernel/src/log/console.rs:66: let sched = kthread::spawn(NAME, body, 0); +kernel/src/log/nested.rs:46: kthread::spawn("lognest", body, 0); +kernel/src/log/storm.rs:49: kthread::spawn("logstorm", body, thread as u64); +``` + +**Stages:** + +- **K1:** delete `usbd`; its body only parks. +- **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 move to a userland test + program whose threads emit through the syscall path, or are deleted if the log + gate does not need kernel-context producers. Blocked on nothing. +- **K4:** `klogd` goes; console output moves to logd under + `issues/kernel/every-driver-is-still-in-the-kernel.md`. The inline drain at + boot and at panic stays. Blocked on that track moving the console. +- **K5:** #536 deletes `iod`, with the kernel's write-back queue. Lands with + #536. +- **K6:** delete the machinery named above. Blocked on K2–K5. 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()); - } -} From f63ac32d7f8d4bea0598f94a00daacd5b56850f3 Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 27 Sep 2026 20:08:50 +0200 Subject: [PATCH 02/17] Kernel: delete usbd, and the kernel-thread panic policy with it (K1) usbd's body only parked. It goes with its start() call, its `usbd-panic` actuator, and every test list that named it. It was also the only thread that ever walked `OnPanic::Recover`. The one other claimant, iod, was never measured recovering, and #536 deletes iod. So the policy goes rather than moving its actuator to iod: `OnPanic`, the row's `recoverable` word, `panic_recovers_here`, and the Release/Acquire pair that published the word before the identity. A kernel thread's panic now halts the machine, because `percpu::in_syscall` compares the running task's identity with the one that entered the syscall, and a kernel thread never enters one. The panic handler asks only that. A row is now one word. Every reader learns the id it compares through something ordered after the publish: the table lock held across it, or the run queue the task is dispatched from. Relaxed is enough for that. klogd_panic_halts's verdict was the ready marker's absence. A recovered klogd takes the console with it, so that verdict held with kernel-thread panics made recoverable (EXIT=0 on the mutated kernel). The verdict is now the line only halt_all_cpus writes, awaited as an event, and the same mutation reds it (EXIT=1). Co-Authored-By: Claude Opus 5.5 --- kernel/src/actuator.rs | 3 - kernel/src/drivers/xhci/mod.rs | 1 - kernel/src/iod.rs | 5 +- kernel/src/log/console.rs | 8 +-- kernel/src/log/nested.rs | 5 +- kernel/src/log/storm.rs | 5 +- kernel/src/main.rs | 7 +-- kernel/src/quiesce.rs | 4 +- kernel/src/sched/dump.rs | 2 +- kernel/src/sched/kthread.rs | 87 +++++++------------------- tests/toyos.rs | 109 +++++++-------------------------- 11 files changed, 57 insertions(+), 179 deletions(-) diff --git a/kernel/src/actuator.rs b/kernel/src/actuator.rs index 20fe2123af2..7c660782ff8 100644 --- a/kernel/src/actuator.rs +++ b/kernel/src/actuator.rs @@ -458,9 +458,6 @@ actuators! { /// Panic inside `klogd` on its first instruction. klogd_panic = "klogd-panic"; - /// Panic inside `usbd` on its first instruction. - usbd_panic = "usbd-panic"; - /// Stop the boot dead in phase 3, interrupts off, before any log drain. pre_idle_wedge = "pre-idle-wedge"; 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/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..36f2e18e957 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); 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..4b06bbbb95b 100644 --- a/kernel/src/main.rs +++ b/kernel/src/main.rs @@ -178,9 +178,7 @@ 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 { + if percpu::in_syscall() { 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(); @@ -666,8 +664,7 @@ pub(crate) unsafe extern "C" fn kernel_main(kernel_args: &KernelArgs) -> ! { // 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(); + // After klogd so its spawn log has a drainer. iod::start(); smp::set_ready(); diff --git a/kernel/src/quiesce.rs b/kernel/src/quiesce.rs index 234fa2911fe..5d11e9eea7d 100644 --- a/kernel/src/quiesce.rs +++ b/kernel/src/quiesce.rs @@ -25,8 +25,8 @@ //! 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: `klogd` and `iod` +//! are in the process table like anything else, and //! [`crate::sched::kthread::is_kernel_task`] is what tells them apart. //! //! # What the stop waits on diff --git a/kernel/src/sched/dump.rs b/kernel/src/sched/dump.rs index 10cdc57dab1..3a9a85273ce 100644 --- a/kernel/src/sched/dump.rs +++ b/kernel/src/sched/dump.rs @@ -705,7 +705,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..9262f837a31 100644 --- a/kernel/src/sched/kthread.rs +++ b/kernel/src/sched/kthread.rs @@ -1,8 +1,9 @@ //! 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. A panic inside one +//! halts the machine: `percpu::in_syscall` is never true for a task that makes +//! no syscall. use alloc::string::String; use alloc::sync::Arc; @@ -19,37 +20,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 +49,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 +74,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, @@ -148,7 +111,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 +128,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/tests/toyos.rs b/tests/toyos.rs index 1e413c4835a..b9bc34cd103 100644 --- a/tests/toyos.rs +++ b/tests/toyos.rs @@ -1228,12 +1228,10 @@ 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. + // The `klogd-panic` actuator boot, split off so the spawn half is per-PR. ("klogd_panic_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 @@ -10594,20 +10592,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(); @@ -13389,8 +13383,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, @@ -13400,12 +13393,7 @@ 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. + // **A kernel thread's panic halts the machine.** // // The marker is a line of the crash *report* rather than `PANIC:` // itself, because `boot_log` stops at the marker and the name is @@ -13429,53 +13417,18 @@ fn run_machine_test( // `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() - }, + // The verdict is the line only `halt_all_cpus` writes. Never the + // ready marker's absence: a recovered `klogd` takes the console + // down with it, so the machine that recovered says nothing either. + // A liveness bound: the line follows the report in the same panic. + let armed = format!( + "panic: rebooting in {} s unless a key is pressed", + toyos_tco::PANIC_BOUND_MS / 1_000 ); - 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"); + const HALTED: Duration = Duration::from_secs(3); + dead.push(&qemu.drain_until(HALTED, |line| line.contains(&armed))); + dead.must_say(&armed)?; + eprintln!(" [klogd] a kernel thread's panic halted the machine rather than recovering"); Ok(()) } "hash_seed_precedes_every_map" => { @@ -17147,30 +17100,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(()) From 5a7964dd759e5e78e56235cdcba64d126a784287 Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 27 Sep 2026 20:10:32 +0200 Subject: [PATCH 03/17] CLAUDE.md: the kernel creates no thread but the per-CPU idle loop The owner's ruling, placed where every agent reads it before it knows which subsystem it is in. Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CLAUDE.md b/CLAUDE.md index 3b300578402..7ce3fb62416 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -36,7 +36,7 @@ A subdirectory `CLAUDE.md` loads when a file in that subtree is `Read`, and not > A snapshot, deliberately shallow — always read the code. -**Kernel** — minimal; new additions are discussed and justified. Resource management, scheduling, process lifecycle, filesystem, device arbitration. 2 MB pages, demand paging, PIE binaries, full SMP. +**Kernel** — minimal; new additions are discussed and justified. Resource management, scheduling, process lifecycle, filesystem, device arbitration. 2 MB pages, demand paging, PIE binaries, full SMP. **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. **Userspace daemons** — compositor, netd, soundd, sshd, logd. Each claims a device or capability from the kernel and serves its function; crash one and the kernel is fine. From 020a2ef2eea0567e872d351712388ef44546d970 Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 27 Sep 2026 20:27:30 +0200 Subject: [PATCH 04/17] CLAUDE.md: no kernel threads is a principle, not a snapshot fact Four kernel threads still exist; the rule is what new work follows, so it sits among the principles the tree does not yet meet. Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 7ce3fb62416..f27fd71f755 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. @@ -36,8 +37,7 @@ A subdirectory `CLAUDE.md` loads when a file in that subtree is `Read`, and not > A snapshot, deliberately shallow — always read the code. -**Kernel** — minimal; new additions are discussed and justified. Resource management, scheduling, process lifecycle, filesystem, device arbitration. 2 MB pages, demand paging, PIE binaries, full SMP. **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. - +**Kernel** — minimal; new additions are discussed and justified. Resource management, scheduling, process lifecycle, filesystem, device arbitration. 2 MB pages, demand paging, PIE binaries, full SMP. **Userspace daemons** — compositor, netd, soundd, sshd, logd. Each claims a device or capability from the kernel and serves its function; crash one and the kernel is fine. **The log is a userland file.** `/system/bin/logd` reads records on a cursor and owns `/log`; the kernel keeps the record ring, the console and the panel, and writes no file. `SYS_FSYNC` reaches the device's cache flush because logd's durability claim rests on it. From 653008e9aec9893a35aae4a2b234655590b93820 Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 27 Sep 2026 20:46:15 +0200 Subject: [PATCH 05/17] Kernel: a Ring 0 fault outside a syscall halts; QEMU's reset judges a kernel thread's death Answers the first review of #553. Faults and panics now ask one question. `blame` took "a thread is current" as its input, so a Ring 0 page fault on a user address was the process's whenever any tid was current: a kernel thread's null dereference poisoned `klogd` and the machine carried on silent, and an interrupt handler's null dereference killed whichever user thread it landed on. `blame` now takes `percpu::in_syscall()`, the input the panic handler already reads, so a Ring 0 fault outside a syscall is the kernel's and halts, and one inside a syscall stays the process's. The audit behind it: every user-memory access in the kernel goes through the direct map (`user_ptr::window`, the futex word, the loader's `KernelSlice`s, the inbox and shm pages, the crash dump's hand walk); the only Ring 0 dereference of a user-half address is `SYS_DEBUG`'s staged `NULL_READ`, inside a syscall. Three defects the audit found are filed rather than fixed here: issues/panic-path/the-syscall-bracket-outlives-a-migrated-syscall.md, issues/isolation/interrupt-entry-keeps-a-ring-3-ac-flag.md and issues/isolation/a-ring-0-page-fault-demand-pages-user-memory.md. `klogd-fault` reads address zero on klogd's first instruction. `klogd_panic_halts` and the new `klogd_fault_halts` share `power::klogd_death_resets`: boot on `panicked()`'s guest with `panic-reboot-fast`, and require QEMU's own `guest-reset` inside the bound, the way `panic_reboots` does, which now shares `resets_inside_the_bound` with them. The two-boot price of `klogd_panic_halts` is dropped from tests/test-durations; the name is unmeasured on a runner until the next profile. Review REMOVEs applied: the kthread module-doc and publish comments, main.rs's spawn-order comment, the MACHINE_TESTS row comment, the false arm-line claim; the track loses K1, the pasted grep, K3's userland option and K4's pointer, and K4/K5 are rewritten as the orchestrator ruled; the stale usbd prose in three issue files is deleted. Co-Authored-By: Claude Opus 5.5 --- ...g-0-page-fault-demand-pages-user-memory.md | 23 ++++++++ .../interrupt-entry-keeps-a-ring-3-ac-flag.md | 25 +++++++++ .../every-wait-in-this-kernel-is-a-spin.md | 4 -- ...-capability-end-state-is-twelve-answers.md | 54 ------------------ ...-small-interrupts-post-and-threads-wait.md | 5 +- .../the-kernel-still-creates-threads.md | 23 ++------ ...all-bracket-outlives-a-migrated-syscall.md | 28 ++++++++++ kernel/src/actuator.rs | 3 + kernel/src/arch/x86_64/idt/exceptions.rs | 2 +- kernel/src/log/console.rs | 6 ++ kernel/src/main.rs | 1 - kernel/src/sched/kthread.rs | 5 +- tests/common/power.rs | 29 +++++++++- tests/test-durations | 1 - tests/toyos.rs | 55 +++++-------------- toyos-userbound/src/fault.rs | 39 +++++++------ 16 files changed, 161 insertions(+), 142 deletions(-) create mode 100644 issues/isolation/a-ring-0-page-fault-demand-pages-user-memory.md create mode 100644 issues/isolation/interrupt-entry-keeps-a-ring-3-ac-flag.md create mode 100644 issues/panic-path/the-syscall-bracket-outlives-a-migrated-syscall.md diff --git a/issues/isolation/a-ring-0-page-fault-demand-pages-user-memory.md b/issues/isolation/a-ring-0-page-fault-demand-pages-user-memory.md new file mode 100644 index 00000000000..76d6793734d --- /dev/null +++ b/issues/isolation/a-ring-0-page-fault-demand-pages-user-memory.md @@ -0,0 +1,23 @@ +--- +status: open +kind: defect +opened: 2026-09-27 +--- + +# A Ring 0 page fault demand-pages user memory + +`page_fault_handler` (`kernel/src/arch/x86_64/idt/exceptions.rs`) resolves a +not-present fault through `process::handle_page_fault` for a Ring 3 frame or +for any Ring 0 frame with a thread current. No kernel path takes a Ring 0 fault +on a user address on purpose: `user_ptr::translate_user` demand-pages +explicitly and every copy goes through the direct map. So the Ring 0 arm only +ever serves a kernel bug, and it serves it by mapping a page into whichever +process is current — an interrupt handler's stray access fills a bystander's +address space. SMAP then faults the retry, but `cr4::SMAP` is optional in the +declaration (`kernel/src/arch/x86_64/control_regs.rs`), and on a CPU without it +the access succeeds silently. + +**Evidence:** read from the code; no test stages it. + +**Exit condition:** a Ring 0 not-present fault on a user address is never +resolved and goes to `blame`, gated by a guest test. 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..560f060f176 --- /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 instead of faulting into `blame`. + +**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/every-wait-in-this-kernel-is-a-spin.md b/issues/kernel/every-wait-in-this-kernel-is-a-spin.md index 511185188d1..4e7f0d34e3d 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. 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 index 60f226622de..f9e353060f4 100644 --- a/issues/kernel/the-kernel-still-creates-threads.md +++ b/issues/kernel/the-kernel-still-creates-threads.md @@ -17,29 +17,18 @@ 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` once K1 has -landed: - -``` -kernel/src/iod.rs:15: let _ = kthread::spawn(NAME, body, 0); -kernel/src/log/console.rs:66: let sched = kthread::spawn(NAME, body, 0); -kernel/src/log/nested.rs:46: kthread::spawn("lognest", body, 0); -kernel/src/log/storm.rs:49: kthread::spawn("logstorm", body, thread as u64); -``` +**Evidence:** `git grep -n 'kthread::spawn(' -- kernel/src`. **Stages:** -- **K1:** delete `usbd`; its body only parks. - **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 move to a userland test - program whose threads emit through the syscall path, or are deleted if the log +- **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; console output moves to logd under - `issues/kernel/every-driver-is-still-in-the-kernel.md`. The inline drain at - boot and at panic stays. Blocked on that track moving the console. -- **K5:** #536 deletes `iod`, with the kernel's write-back queue. Lands with - #536. +- **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/the-syscall-bracket-outlives-a-migrated-syscall.md b/issues/panic-path/the-syscall-bracket-outlives-a-migrated-syscall.md new file mode 100644 index 00000000000..d84aaf4e3c5 --- /dev/null +++ b/issues/panic-path/the-syscall-bracket-outlives-a-migrated-syscall.md @@ -0,0 +1,28 @@ +--- +status: open +kind: defect +opened: 2026-09-27 +--- + +# The syscall bracket outlives a migrated syscall + +`percpu::in_syscall` is the one answer to "is this Ring 0 context on a +process's behalf": the panic handler recovers on it, and `blame` makes a Ring 0 +user-address fault the process's on it. It 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 (`kernel/src/arch/x86_64/syscall.rs`'s +`syscall_handler` is its one caller). `Hw::switch` in +`kernel/src/arch/x86_64/hw.rs` moves the current identity and never the +bracket. + +So a syscall that parks on CPU A and finishes on CPU B leaves A's word naming +its thread. When that thread next runs in Ring 3 on A before any other syscall +enters there, an interrupt handler's panic or user-address fault on A reads +`in_syscall()` true: the thread is poisoned and the machine carries on, where +the kernel bug should have halted it. + +**Evidence:** read from the code; no test stages it. + +**Exit condition:** the bracket names a thread only while that thread is inside +a syscall on this CPU, and a guest test stages a migrated syscall followed by an +interrupt-context panic on the first CPU and sees the machine halt. diff --git a/kernel/src/actuator.rs b/kernel/src/actuator.rs index 7c660782ff8..5d751cdd1d2 100644 --- a/kernel/src/actuator.rs +++ b/kernel/src/actuator.rs @@ -458,6 +458,9 @@ actuators! { /// Panic inside `klogd` on its first instruction. klogd_panic = "klogd-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/x86_64/idt/exceptions.rs b/kernel/src/arch/x86_64/idt/exceptions.rs index 7c095df69c0..e50e3f8c14f 100644 --- a/kernel/src/arch/x86_64/idt/exceptions.rs +++ b/kernel/src/arch/x86_64/idt/exceptions.rs @@ -115,7 +115,7 @@ impl ExceptionContext<'_> { /// 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()) + blame(self.ring(), self.frame.rip, self.faulted(), percpu::in_syscall()) } } diff --git a/kernel/src/log/console.rs b/kernel/src/log/console.rs index 36f2e18e957..8b49f3bff0d 100644 --- a/kernel/src/log/console.rs +++ b/kernel/src/log/console.rs @@ -422,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/main.rs b/kernel/src/main.rs index 4b06bbbb95b..27c7b19aade 100644 --- a/kernel/src/main.rs +++ b/kernel/src/main.rs @@ -664,7 +664,6 @@ pub(crate) unsafe extern "C" fn kernel_main(kernel_args: &KernelArgs) -> ! { // 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 its spawn log has a drainer. iod::start(); smp::set_ready(); diff --git a/kernel/src/sched/kthread.rs b/kernel/src/sched/kthread.rs index 9262f837a31..ada0f30246a 100644 --- a/kernel/src/sched/kthread.rs +++ b/kernel/src/sched/kthread.rs @@ -1,9 +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. A panic inside one -//! halts the machine: `percpu::in_syscall` is never true for a task that makes -//! no syscall. +//! and a Ring 0 loop reaches none. [`ROWS`] holds every one. use alloc::string::String; use alloc::sync::Arc; @@ -110,7 +108,6 @@ pub fn spawn(name: &str, body: extern "C" fn(u64) -> !, arg: u64) -> ThreadSched ) }); 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)); // The kernel address space, named so one declaration decides every task's `cr3`. let (sched, _dst) = scheduler::enqueue_new( diff --git a/tests/common/power.rs b/tests/common/power.rs index 198952e0448..03c663b1f0a 100644 --- a/tests/common/power.rs +++ b/tests/common/power.rs @@ -885,7 +885,13 @@ 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 budget = resets_inside_the_bound(&mut qemu)?; + eprintln!(" [power] the panicked guest reset itself inside {budget:?} of: {}", line.trim()); + Ok(()) +} +/// QEMU's own `guest-reset`, inside the fast bound plus what a reset costs. +fn resets_inside_the_bound(qemu: &mut QemuInstance) -> Result { let budget = qemu.budget(Duration::from_secs(PANIC_FAST_SECS) + RESET_ALLOWANCE); let mut stop = qemu::QmpShutdown::open(qemu.qmp_socket(), budget); let reason = stop.reason(); @@ -895,8 +901,29 @@ pub fn panic_reboots( let drain = serial::Serial::named("panic reboot drain", tail.as_str()); drain.must_say(PANIC_REBOOTING)?; + Ok(budget) +} - eprintln!(" [power] the panicked guest reset itself inside {budget:?} of: {}", line.trim()); +/// `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, because a +/// recovered `klogd` takes the console down with it. +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, ..panicked() }; + let mut qemu = QemuInstance::boot_with_options(test_config, c_bins, rust_bins, options); + let boot = serial::Serial::boot(&qemu); + for want in said { + boot.must_say(want)?; + } + boot.must_say(&panic_armed())?; + let budget = resets_inside_the_bound(&mut qemu)?; + eprintln!(" [klogd] {kernel_params:?}: QEMU reset the machine inside {budget:?}"); Ok(()) } diff --git a/tests/test-durations b/tests/test-durations index 86acf871b21..12a081bd5b2 100644 --- a/tests/test-durations +++ b/tests/test-durations @@ -254,7 +254,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 diff --git a/tests/toyos.rs b/tests/toyos.rs index b9bc34cd103..c392a3e5083 100644 --- a/tests/toyos.rs +++ b/tests/toyos.rs @@ -1231,8 +1231,8 @@ const MACHINE_TESTS: &[(&str, Sched, Tier)] = &[ // The spawn half alone: one headless boot whose verdict is kernel log // lines. ("klogd_hosted", Sched::Parallel, Tier::Fast), - // The `klogd-panic` actuator boot, split off so the spawn half is per-PR. ("klogd_panic_halts", Sched::Parallel, Tier::Nightly), + ("klogd_fault_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 @@ -13392,45 +13392,20 @@ fn run_machine_test( ); klogd_hosted(&serial::Serial::boot(&qemu)) } - "klogd_panic_halts" => { - // **A kernel thread's panic halts the machine.** - // - // 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 is the line only `halt_all_cpus` writes. Never the - // ready marker's absence: a recovered `klogd` takes the console - // down with it, so the machine that recovered says nothing either. - // A liveness bound: the line follows the report in the same panic. - let armed = format!( - "panic: rebooting in {} s unless a key is pressed", - toyos_tco::PANIC_BOUND_MS / 1_000 - ); - const HALTED: Duration = Duration::from_secs(3); - dead.push(&qemu.drain_until(HALTED, |line| line.contains(&armed))); - dead.must_say(&armed)?; - eprintln!(" [klogd] a kernel thread's panic halted the machine rather than recovering"); - Ok(()) - } + "klogd_panic_halts" => power::klogd_death_resets( + test_config, + c_bins, + rust_bins, + &["klogd-panic", "panic-reboot-fast"], + &["PANIC:", "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"], + ), "hash_seed_precedes_every_map" => { // `kernel/src/hasher.rs`'s `UNSEEDED`, as a prefix: the wrong seed // the compiler cannot reach, because the container works. Its other diff --git a/toyos-userbound/src/fault.rs b/toyos-userbound/src/fault.rs index 94191e7c323..2c3c09714c0 100644 --- a/toyos-userbound/src/fault.rs +++ b/toyos-userbound/src/fault.rs @@ -78,10 +78,10 @@ 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. + /// Ring 0 code, inside the current thread's syscall, 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. @@ -90,8 +90,10 @@ pub enum Blame { /// 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. +/// `rip` is the faulting frame's instruction pointer and `in_syscall` is +/// whether this CPU is inside the current thread's syscall, the question the +/// panic handler asks: a kernel thread or an interrupt handler faults on no +/// process's behalf, whichever thread is current. /// /// **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 @@ -99,13 +101,13 @@ pub enum Blame { /// 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 { +pub fn blame(ring: Ring, rip: u64, faulted: Faulted, in_syscall: 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) => + if in_syscall && !is_user_addr(rip) && is_user_addr(addr) => { Blame::ProcessThroughKernel } @@ -141,9 +143,9 @@ mod tests { /// 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. + /// It must be the kernel's, 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!( @@ -156,8 +158,6 @@ mod tests { 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, @@ -199,10 +199,17 @@ mod tests { ), Blame::ProcessThroughKernel, ); - // No thread to attribute it to, so there is nothing to kill but the - // machine's illusion that it is well. + } + + /// The same null dereference with no syscall open: a kernel thread's, or an + /// interrupt handler's on top of whichever thread is current. On no + /// process's behalf, so the machine halts and no bystander is killed. + #[test] + fn a_ring_0_fault_outside_a_syscall_is_the_kernels() { + let k = Ring::of_cs(KERNEL_CS); + assert_eq!(blame(k, KERNEL_RIP, Faulted::Address(0), false), Blame::Kernel); assert_eq!( - blame(Ring::of_cs(KERNEL_CS), KERNEL_RIP, Faulted::Address(0), false), + blame(k, KERNEL_RIP, Faulted::Address(crate::span::USER_TOP - 1), false), Blame::Kernel, ); } From bb8c1e9b15867207a117652173715cd105aaccb7 Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 27 Sep 2026 20:49:56 +0200 Subject: [PATCH 06/17] tests: QEMU's stop reason is klogd_*_halts' verdict, not the guest's arm line The first cut waited for the arm line as its ready marker, so a machine that recovered went red on the guest's silence before QEMU was asked. The boot now stops at a line both a halting and a recovering kernel write (`PANIC: panicked at`, `#PF UNHANDLED: cr2=0x0`), and the verdict is `QmpShutdown`'s `guest-reset` inside the bound; the report's lines and the arm line are read from the boot log plus the drain after it. Co-Authored-By: Claude Opus 5.5 --- tests/common/power.rs | 35 +++++++++++++++++++++++------------ tests/toyos.rs | 4 +++- 2 files changed, 26 insertions(+), 13 deletions(-) diff --git a/tests/common/power.rs b/tests/common/power.rs index 03c663b1f0a..3103e25d908 100644 --- a/tests/common/power.rs +++ b/tests/common/power.rs @@ -885,44 +885,55 @@ 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 budget = resets_inside_the_bound(&mut qemu)?; + let (budget, _) = resets_inside_the_bound(&mut qemu, PANICKED_AND_STAYED_UP)?; eprintln!(" [power] the panicked guest reset itself inside {budget:?} of: {}", line.trim()); Ok(()) } -/// QEMU's own `guest-reset`, inside the fast bound plus what a reset costs. -fn resets_inside_the_bound(qemu: &mut QemuInstance) -> Result { +/// QEMU's own `guest-reset`, inside the fast bound plus what a reset costs, +/// 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, + never: &str, +) -> Result<(Duration, String), String> { let budget = qemu.budget(Duration::from_secs(PANIC_FAST_SECS) + RESET_ALLOWANCE); let mut stop = qemu::QmpShutdown::open(qemu.qmp_socket(), budget); 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) + Ok((budget, tail)) } /// `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, because a -/// recovered `klogd` takes the console down with it. +/// what dies. The verdict is QEMU's reset, never the guest's word: `marker` is +/// a line both a halting and a recovering kernel write, and a recovered `klogd` +/// takes the console down with it. pub fn klogd_death_resets( test_config: &Path, c_bins: &[(String, Vec)], rust_bins: &[(String, Vec)], kernel_params: &'static [&'static str], + marker: &'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, ..panicked() }; + let options = BootOptions { kernel_params, ready_marker: marker, ..panicked() }; let mut qemu = QemuInstance::boot_with_options(test_config, c_bins, rust_bins, options); - let boot = serial::Serial::boot(&qemu); + let mut dead = serial::Serial::boot(&qemu); + let (budget, tail) = resets_inside_the_bound( + &mut qemu, + "QEMU never reported stopping: klogd died and the machine carried on without it", + )?; + dead.push(&tail); for want in said { - boot.must_say(want)?; + dead.must_say(want)?; } - boot.must_say(&panic_armed())?; - let budget = resets_inside_the_bound(&mut qemu)?; + dead.must_say(&panic_armed())?; eprintln!(" [klogd] {kernel_params:?}: QEMU reset the machine inside {budget:?}"); Ok(()) } diff --git a/tests/toyos.rs b/tests/toyos.rs index c392a3e5083..a14c06344ec 100644 --- a/tests/toyos.rs +++ b/tests/toyos.rs @@ -13397,13 +13397,15 @@ fn run_machine_test( c_bins, rust_bins, &["klogd-panic", "panic-reboot-fast"], - &["PANIC:", "klogd-panic: the console drainer died", "Process: klogd"], + "PANIC: panicked at", + &["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"], + "#PF UNHANDLED: cr2=0x0", &["KERNEL PANIC: read unmapped address at 0x0", "console::body"], ), "hash_seed_precedes_every_map" => { From fbd31a1b43bf2e6792b9b38f5c8c67c0a1ab83c4 Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 27 Sep 2026 20:55:06 +0200 Subject: [PATCH 07/17] tests: klogd_*_halts stop at klogd's spawn line, the last one both arms write Under the old blame input a kernel thread's fault is recovered and nothing after klogd's spawn line reaches the wire: the fault's own `#PF UNHANDLED` record is committed and never drained. So a report line as the boot's marker still let the guest's silence decide. The marker is now `kthread: klogd pid=`, written before klogd runs, and QEMU's stop reason is the only verdict on halt versus carry on. Co-Authored-By: Claude Opus 5.5 --- tests/common/power.rs | 10 +++++----- tests/toyos.rs | 2 -- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/tests/common/power.rs b/tests/common/power.rs index 3103e25d908..90ec29e8cf3 100644 --- a/tests/common/power.rs +++ b/tests/common/power.rs @@ -910,19 +910,19 @@ fn resets_inside_the_bound( } /// `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: `marker` is -/// a line both a halting and a recovering kernel write, and a recovered `klogd` -/// takes the console down with it. +/// what dies. The verdict is QEMU's reset, never the guest's word: a recovered +/// `klogd` takes the console down with it, so the boot stops at klogd's spawn +/// line, the last one a halting and a recovering kernel both write. pub fn klogd_death_resets( test_config: &Path, c_bins: &[(String, Vec)], rust_bins: &[(String, Vec)], kernel_params: &'static [&'static str], - marker: &'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: marker, ..panicked() }; + 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 mut dead = serial::Serial::boot(&qemu); let (budget, tail) = resets_inside_the_bound( diff --git a/tests/toyos.rs b/tests/toyos.rs index a14c06344ec..1c83edd7a1a 100644 --- a/tests/toyos.rs +++ b/tests/toyos.rs @@ -13397,7 +13397,6 @@ fn run_machine_test( c_bins, rust_bins, &["klogd-panic", "panic-reboot-fast"], - "PANIC: panicked at", &["klogd-panic: the console drainer died", "Process: klogd"], ), "klogd_fault_halts" => power::klogd_death_resets( @@ -13405,7 +13404,6 @@ fn run_machine_test( c_bins, rust_bins, &["klogd-fault", "panic-reboot-fast"], - "#PF UNHANDLED: cr2=0x0", &["KERNEL PANIC: read unmapped address at 0x0", "console::body"], ), "hash_seed_precedes_every_map" => { From 7eb6b51880e7af93984b1840c932004705ddcec7 Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 27 Sep 2026 21:05:08 +0200 Subject: [PATCH 08/17] tests: klogd's spawn line is not the last line of a recovered boot Under M1 the recovered machine goes on writing; the spawn line is only one both arms write. Co-Authored-By: Claude Opus 5.5 --- tests/common/power.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/common/power.rs b/tests/common/power.rs index 90ec29e8cf3..bd948db0da7 100644 --- a/tests/common/power.rs +++ b/tests/common/power.rs @@ -912,7 +912,7 @@ fn resets_inside_the_bound( /// `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: a recovered /// `klogd` takes the console down with it, so the boot stops at klogd's spawn -/// line, the last one a halting and a recovering kernel both write. +/// line, which a halting and a recovering kernel both write. pub fn klogd_death_resets( test_config: &Path, c_bins: &[(String, Vec)], From 894a85e05188719959d8b2b61ef83bc41daa7c9e Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 27 Sep 2026 21:39:21 +0200 Subject: [PATCH 09/17] Kernel: every kernel panic halts, and panic recovery is deleted The owner's ruling: a kernel panic means a kernel invariant broke, and the kernel does not keep running on state it can no longer trust. A panic inside a syscall halts like any other; a Ring 0 fault is a kernel bug and halts; a Ring 3 fault still kills only its process. "The kernel never crashes from userland" is kept by refusing bad input at the boundary, not by surviving panics. This reverses the rule that a recoverable panic ends only the offending process. Deleted: - the panic handler's `in_syscall()` branch, `try_recover_from_panic` (x86 and the aarch64 stub) and `recover_or_halt`; - `sched/poison.rs`, the per-CPU poison bank, `poison_tid`, `schedule_no_return`, `process::PoisonWake`/`zombify_poisoned`, `toyos_proclife::poison`, `Watch::thread`, the `poison-overwrite` feature and its loom model and CI red; the idle loop's `reap_poisoned` is now `reap_finished` and only collects published exits; - `panic_console::discard_capture`, `CaptureAccess::discard`, `CaptureLatch::owned_by` and the loom model of a discard; - `toyos_userbound::{blame, Blame, Faulted}`: whose fault a trap is is now its ring alone, and `fatal_exception` kills a Ring 3 fault's process and halts on anything else; - Ring 0 demand paging: `page_fault_handler` resolves a not-present fault only for a Ring 3 frame, so a Ring 0 fault halts before anything is mapped into the current process; `handle_page_fault`'s kernel-thread refusal went with it; - `SYS_DEBUG` action 2's one-shot arming, which existed to refuse a second call into a lock a recovered panic stranded; - the aarch64 `percpu::in_syscall` stub. The x86 one stays: the crash report reads it to print the syscall a death happened inside. Tests: - `panic_recovery` keeps its Ring 3 arm only (a user segfault kills its process and the system lives), and leaves `ACTUATOR_TESTS`. - `syscall_panic_halts`, `syscall_fault_halts`, `lock_across_switch_halts` and `heap_over_ceiling_halts` each drive one `SYS_DEBUG` death on its own boot and take the verdict from QEMU's `guest-reset` through QMP, through `power::syscall_death_resets`, which shares `klogd_death_resets`'s tail. - `heap_ceiling_recovery` is `heap_ceiling_bounds`: its over-ceiling arm and the heap-still-works arm after it are gone. - `screen_recoverable_untouched` and `screen_survived_panic_not_blamed` are deleted: there is no survived panic to paint or not paint. Issues: the stale-bracket, Ring-0-demand-paging, stranded-PROCESS_TABLE and discard-refusal defects are gone with the code they were about. What is left of the stale bracket is a crash report naming a finished syscall, filed narrowly; `idle_stack_guard`'s inert "the read succeeded" arm is filed. Co-Authored-By: Claude Opus 5.5 --- ...eturned-arm-reads-a-line-nothing-writes.md | 20 ++ ...g-0-page-fault-demand-pages-user-memory.md | 23 -- ...hing-charges-kernel-memory-to-a-process.md | 14 +- ...t-can-name-a-syscall-that-already-ended.md | 26 ++ .../panic-holding-process-table-hangs.md | 30 -- ...-refusal-branch-is-exercised-by-nothing.md | 34 --- ...all-bracket-outlives-a-migrated-syscall.md | 28 -- kernel-loom/Cargo.toml | 10 - kernel-loom/src/lib.rs | 3 - kernel-loom/tests/panic_capture.rs | 49 +--- kernel-loom/tests/poison_set.rs | 66 ----- kernel-loom/tests/reap_gate.rs | 15 +- kernel/Cargo.toml | 3 - kernel/src/arch/aarch64/percpu.rs | 4 - kernel/src/arch/aarch64/trap.rs | 4 - kernel/src/arch/x86_64/idt/exceptions.rs | 97 ++---- kernel/src/arch/x86_64/idt/mod.rs | 2 - kernel/src/arch/x86_64/percpu.rs | 1 - kernel/src/arch/x86_64/syscall.rs | 2 - kernel/src/drivers/panic_console/access.rs | 18 +- kernel/src/drivers/panic_console/latch.rs | 6 +- kernel/src/drivers/panic_console/mod.rs | 18 +- kernel/src/main.rs | 10 - kernel/src/process.rs | 36 +-- kernel/src/sched/driver.rs | 4 +- kernel/src/sched/dump.rs | 3 +- kernel/src/sched/mod.rs | 1 - kernel/src/sched/poison.rs | 69 ----- kernel/src/scheduler.rs | 87 +----- kernel/src/syscall/debug.rs | 6 +- kernel/src/syscall/dispatch.rs | 7 +- src/build.rs | 3 +- src/ci.rs | 3 - tests/common/power.rs | 50 +++- tests/common/qemu.rs | 13 +- tests/common/screen.rs | 3 +- tests/common/serial.rs | 8 +- tests/test-durations | 3 - .../toyos-rust-tests/src/bin/heap_ceiling.rs | 66 +---- .../src/bin/panic_recovery.rs | 57 +--- .../src/bin/test_panic_child.rs | 18 +- tests/toyos.rs | 277 ++++-------------- toyos-proclife/src/interleave.rs | 2 +- toyos-proclife/src/lib.rs | 17 -- toyos-proclife/src/model.rs | 6 - toyos-proclife/src/poison.rs | 129 -------- toyos-proclife/src/teardown.rs | 9 +- toyos-sched/src/hw.rs | 3 +- toyos-sched/src/task.rs | 8 +- toyos-userbound/src/fault.rs | 229 +-------------- toyos-userbound/src/lib.rs | 12 +- 51 files changed, 240 insertions(+), 1372 deletions(-) create mode 100644 issues/build/idle-stack-guards-returned-arm-reads-a-line-nothing-writes.md delete mode 100644 issues/isolation/a-ring-0-page-fault-demand-pages-user-memory.md create mode 100644 issues/panic-path/a-crash-report-can-name-a-syscall-that-already-ended.md delete mode 100644 issues/panic-path/panic-holding-process-table-hangs.md delete mode 100644 issues/panic-path/the-discards-refusal-branch-is-exercised-by-nothing.md delete mode 100644 issues/panic-path/the-syscall-bracket-outlives-a-migrated-syscall.md delete mode 100644 kernel-loom/tests/poison_set.rs delete mode 100644 kernel/src/sched/poison.rs delete mode 100644 toyos-proclife/src/poison.rs 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/a-ring-0-page-fault-demand-pages-user-memory.md b/issues/isolation/a-ring-0-page-fault-demand-pages-user-memory.md deleted file mode 100644 index 76d6793734d..00000000000 --- a/issues/isolation/a-ring-0-page-fault-demand-pages-user-memory.md +++ /dev/null @@ -1,23 +0,0 @@ ---- -status: open -kind: defect -opened: 2026-09-27 ---- - -# A Ring 0 page fault demand-pages user memory - -`page_fault_handler` (`kernel/src/arch/x86_64/idt/exceptions.rs`) resolves a -not-present fault through `process::handle_page_fault` for a Ring 3 frame or -for any Ring 0 frame with a thread current. No kernel path takes a Ring 0 fault -on a user address on purpose: `user_ptr::translate_user` demand-pages -explicitly and every copy goes through the direct map. So the Ring 0 arm only -ever serves a kernel bug, and it serves it by mapping a page into whichever -process is current — an interrupt handler's stray access fills a bystander's -address space. SMAP then faults the retry, but `cr4::SMAP` is optional in the -declaration (`kernel/src/arch/x86_64/control_regs.rs`), and on a CPU without it -the access succeeds silently. - -**Evidence:** read from the code; no test stages it. - -**Exit condition:** a Ring 0 not-present fault on a user address is never -resolved and goes to `blame`, gated by a guest test. 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/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..e74e313baa0 --- /dev/null +++ b/issues/panic-path/a-crash-report-can-name-a-syscall-that-already-ended.md @@ -0,0 +1,26 @@ +--- +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. + +So a syscall that parks on CPU A and finishes on CPU B leaves A's word naming +its 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. + +**Evidence:** read from the code; no test stages it. + +**Exit condition:** the bracket names a thread only while that thread is inside +a syscall on this CPU, and a guest test stages a migrated syscall followed by an +interrupt-context death on the first CPU whose report carries no `Syscall:` +line. 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-syscall-bracket-outlives-a-migrated-syscall.md b/issues/panic-path/the-syscall-bracket-outlives-a-migrated-syscall.md deleted file mode 100644 index d84aaf4e3c5..00000000000 --- a/issues/panic-path/the-syscall-bracket-outlives-a-migrated-syscall.md +++ /dev/null @@ -1,28 +0,0 @@ ---- -status: open -kind: defect -opened: 2026-09-27 ---- - -# The syscall bracket outlives a migrated syscall - -`percpu::in_syscall` is the one answer to "is this Ring 0 context on a -process's behalf": the panic handler recovers on it, and `blame` makes a Ring 0 -user-address fault the process's on it. It 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 (`kernel/src/arch/x86_64/syscall.rs`'s -`syscall_handler` is its one caller). `Hw::switch` in -`kernel/src/arch/x86_64/hw.rs` moves the current identity and never the -bracket. - -So a syscall that parks on CPU A and finishes on CPU B leaves A's word naming -its thread. When that thread next runs in Ring 3 on A before any other syscall -enters there, an interrupt handler's panic or user-address fault on A reads -`in_syscall()` true: the thread is poisoned and the machine carries on, where -the kernel bug should have halted it. - -**Evidence:** read from the code; no test stages it. - -**Exit condition:** the bracket names a thread only while that thread is inside -a syscall on this CPU, and a guest test stages a migrated syscall followed by an -interrupt-context panic on the first CPU and sees the machine halt. 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..aae85b1516f 100644 --- a/kernel-loom/tests/reap_gate.rs +++ b/kernel-loom/tests/reap_gate.rs @@ -26,7 +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, +//! unpublished exit*, which is the defect stated exactly. Verified 2026-08-17, //! both ways round. use kernel_loom::reap_gate::ReapGate; @@ -68,24 +68,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 +99,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/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/idt/exceptions.rs b/kernel/src/arch/x86_64/idt/exceptions.rs index e50e3f8c14f..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::in_syscall()) - } } // 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..9903b558d0d 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 @@ -556,19 +555,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/main.rs b/kernel/src/main.rs index 27c7b19aade..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,13 +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(); } - if percpu::in_syscall() { - 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(); } @@ -649,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(); } diff --git a/kernel/src/process.rs b/kernel/src/process.rs index 072cba886a7..74ffc9899c4 100644 --- a/kernel/src/process.rs +++ b/kernel/src/process.rs @@ -32,7 +32,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::(); @@ -710,35 +710,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() @@ -1281,11 +1254,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/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 3a9a85273ce..cca1edf3cd5 100644 --- a/kernel/src/sched/dump.rs +++ b/kernel/src/sched/dump.rs @@ -384,8 +384,7 @@ 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. + // Not an `IrqGuard`: this must unconditionally set IF on exit. crate::arch::cpu::disable_interrupts(); while crate::arch::cpu::counter() < until { core::hint::spin_loop(); 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..1731f3d63ea 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, @@ -537,15 +537,10 @@ pub(crate) fn syscall_dispatch(num: u64, a1: u64, a2: u64, a3: u64, a4: u64) -> // 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 } 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 a65b75e2709..07160e7b400 100644 --- a/src/build.rs +++ b/src/build.rs @@ -2642,7 +2642,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 @@ -2653,7 +2653,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 69f0c30ff37..d913627e84a 100644 --- a/src/ci.rs +++ b/src/ci.rs @@ -245,9 +245,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/power.rs b/tests/common/power.rs index bd948db0da7..000e17dfacc 100644 --- a/tests/common/power.rs +++ b/tests/common/power.rs @@ -924,18 +924,54 @@ pub fn klogd_death_resets( 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 mut dead = serial::Serial::boot(&qemu); - let (budget, tail) = resets_inside_the_bound( - &mut qemu, - "QEMU never reported stopping: klogd died and the machine carried on without it", - )?; + let dead = serial::Serial::boot(&qemu); + let never = "QEMU never reported stopping: klogd died and the machine carried on without it"; + died_and_reset(&mut qemu, 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"], + kernel_features: toyos_build::build::TEST_KERNEL, + 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); + 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, 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, + mut dead: serial::Serial, + never: &str, + said: &[&str], +) -> Result { + let (budget, tail) = resets_inside_the_bound(qemu, never)?; dead.push(&tail); for want in said { dead.must_say(want)?; } dead.must_say(&panic_armed())?; - eprintln!(" [klogd] {kernel_params:?}: QEMU reset the machine inside {budget:?}"); - Ok(()) + eprintln!(" [power] {:?}: QEMU reset the machine inside {budget:?}", said.first()); + 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 c6f140a0a1e..dfcf0d70f7e 100644 --- a/tests/common/qemu.rs +++ b/tests/common/qemu.rs @@ -672,15 +672,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 @@ -1011,9 +1002,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 12a081bd5b2..aaab24f68a3 100644 --- a/tests/test-durations +++ b/tests/test-durations @@ -212,7 +212,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 @@ -362,8 +361,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/heap_ceiling.rs b/tests/toyos-rust-tests/src/bin/heap_ceiling.rs index bd8c0b5c39f..b261f290a14 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,9 +109,8 @@ 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 +/// `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. @@ -136,36 +123,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 index 61029a93327..dc7692d12b2 100644 --- a/tests/toyos-rust-tests/src/bin/panic_recovery.rs +++ b/tests/toyos-rust-tests/src/bin/panic_recovery.rs @@ -1,62 +1,11 @@ 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") @@ -66,14 +15,14 @@ fn test_user_segfault() { println!(" PASS: user segfault killed process (exit={})", status.code().unwrap_or(-1)); } -/// System still works after all three fault types. +/// System still works after the segfault. fn test_system_alive() { let output = Command::new("/system/bin/echo") .arg("still alive") .output() - .expect("failed to run echo after recoveries"); + .expect("failed to run echo after the segfault"); 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"); + println!(" PASS: system alive after a user segfault"); } 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..d7b7df02bed 100644 --- a/tests/toyos-rust-tests/src/bin/test_panic_child.rs +++ b/tests/toyos-rust-tests/src/bin/test_panic_child.rs @@ -1,14 +1,10 @@ //! 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. -//! -//! 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. +//! 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. use toyos_abi::syscall::SyscallError; @@ -16,7 +12,7 @@ fn main() { let action: u64 = std::env::args() .nth(1) .and_then(|s| s.parse().ok()) - .unwrap_or(0); + .expect("usage: test_panic_child "); let rc = toyos_abi::syscall::debug(action); if rc == SyscallError::InvalidArgument.to_u64() { eprintln!( @@ -24,7 +20,7 @@ fn main() { 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 1c83edd7a1a..95ace79a21a 100644 --- a/tests/toyos.rs +++ b/tests/toyos.rs @@ -152,10 +152,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 @@ -325,8 +321,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. @@ -598,11 +593,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), @@ -1233,6 +1223,10 @@ const MACHINE_TESTS: &[(&str, Sched, Tier)] = &[ ("klogd_hosted", Sched::Parallel, Tier::Fast), ("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 @@ -1557,7 +1551,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), @@ -1666,7 +1660,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"]), @@ -1753,6 +1747,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"]), @@ -1796,8 +1794,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 @@ -3102,18 +3098,14 @@ fn check_rust_result(result: &TestResult) -> bool { } } -/// Checks both exit code and serial diagnostics for panic recovery. +/// Checks both exit code and the segfault's crash report. 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"), + ("Registers:", "expected register dump from the fault"), ("SEGFAULT tid=", "expected SEGFAULT header"), ("deliberate_null_deref", "expected deliberate_null_deref in segfault backtrace"), ("+0x", "expected symbolized backtraces"), @@ -3126,10 +3118,6 @@ fn check_panic_recovery(result: &TestResult) -> bool { 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) } @@ -3205,11 +3193,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:"; @@ -6852,143 +6838,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}")), } } @@ -13406,6 +13255,35 @@ fn run_machine_test( &["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, + &["KERNEL PANIC: read unmapped address at 0x0", 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)?; + if name == "lock_across_switch_halts" { + check_tripwire_attribution(&said)?; + } + Ok(()) + } "hash_seed_precedes_every_map" => { // `kernel/src/hasher.rs`'s `UNSEEDED`, as a prefix: the wrong seed // the compiler cannot reach, because the container works. Its other @@ -13715,57 +13593,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" => { @@ -19561,10 +19401,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, @@ -19597,14 +19434,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-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/src/hw.rs b/toyos-sched/src/hw.rs index 4603bfc9f6d..780fd41a743 100644 --- a/toyos-sched/src/hw.rs +++ b/toyos-sched/src/hw.rs @@ -104,8 +104,7 @@ pub trait Machine: Kicker + 'static { /// 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 + /// `sti;hlt` is one atom — and an RAII guard restores /// the caller's flags instead. fn irq_guard(&self) -> Self::IrqGuard; diff --git a/toyos-sched/src/task.rs b/toyos-sched/src/task.rs index 642f5af5439..73ff5b4efe9 100644 --- a/toyos-sched/src/task.rs +++ b/toyos-sched/src/task.rs @@ -273,10 +273,7 @@ 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, // The two-phase wait handshake. @@ -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 2c3c09714c0..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,77 +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, inside the current thread's syscall, 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 `in_syscall` is -/// whether this CPU is inside the current thread's syscall, the question the -/// panic handler asks: a kernel thread or an interrupt handler faults on no -/// process's behalf, whichever thread is current. -/// -/// **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, in_syscall: bool) -> Blame { - if ring.is_user() { - return Blame::Process; - } - match faulted { - Faulted::Address(addr) - if in_syscall && !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; @@ -140,127 +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, 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, - ); - 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, - ); - } - - /// The same null dereference with no syscall open: a kernel thread's, or an - /// interrupt handler's on top of whichever thread is current. On no - /// process's behalf, so the machine halts and no bystander is killed. - #[test] - fn a_ring_0_fault_outside_a_syscall_is_the_kernels() { - let k = Ring::of_cs(KERNEL_CS); - assert_eq!(blame(k, KERNEL_RIP, Faulted::Address(0), false), Blame::Kernel); - assert_eq!( - blame(k, KERNEL_RIP, Faulted::Address(crate::span::USER_TOP - 1), 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, From 6897bff95657dd67371cd0d1109c1132025d395b Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 27 Sep 2026 21:49:21 +0200 Subject: [PATCH 10/17] tests: the syscall-death boots take their kernel from the parameter; the wedge issue loses its recovery paragraph `panic-reboot-fast` already selects the test kernel, which carries `test-actuators`, and the harness refuses a boot that also names the build. The wedge arms' DEADLOCK panic in `logd`'s fsync now halts like any other; `usb_reset_records_the_phase_it_cut` stays green on this tree, so only the paragraph describing the recovery composition goes. Co-Authored-By: Claude Opus 5.5 --- ...iver-panics-every-other-cpu-that-wants-its-lock.md | 11 +---------- tests/common/power.rs | 3 +-- 2 files changed, 2 insertions(+), 12 deletions(-) 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/tests/common/power.rs b/tests/common/power.rs index 000e17dfacc..56185dce397 100644 --- a/tests/common/power.rs +++ b/tests/common/power.rs @@ -942,7 +942,6 @@ pub fn syscall_death_resets( ) -> Result { let options = BootOptions { kernel_params: &["panic-reboot-fast"], - kernel_features: toyos_build::build::TEST_KERNEL, ready_marker: qemu::DEFAULT_READY, ..panicked() }; @@ -970,7 +969,7 @@ fn died_and_reset( dead.must_say(want)?; } dead.must_say(&panic_armed())?; - eprintln!(" [power] {:?}: QEMU reset the machine inside {budget:?}", said.first()); + eprintln!(" [power] {said:?}: QEMU reset the machine inside {budget:?}"); Ok(dead.text().to_string()) } From af5737cbadd8f7e101d9c4c9403d147687488be3 Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 27 Sep 2026 22:09:12 +0200 Subject: [PATCH 11/17] issues: a sysroot cloned during a toolchain rebuild never gets its cargo Found while measuring this branch's Ring-3 red arm: the run published a sysroot without `cargo`, and every harness run from the worktree has panicked on the C corpus since. Co-Authored-By: Claude Opus 5.5 --- ...-toolchain-rebuild-never-gets-its-cargo.md | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 issues/build/a-sysroot-cloned-during-a-toolchain-rebuild-never-gets-its-cargo.md 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..93b70b59dae --- /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` held +`rustc` and `rustdoc` and no `cargo` from 22:00 until 22:08 on 2026-09-27, +while `/Users/jan/Dev/jan/toyos` ran `toyos-build --build-only`; the link came +back 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. From 73e00d78ec789cd03d4ef445565827414f8686a0 Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 27 Sep 2026 22:10:03 +0200 Subject: [PATCH 12/17] Point the citations of the deleted `recover_or_halt` at `fatal_exception` A Ring 3 fault's `kill_process(-1)` now sits in `fatal_exception` itself. The sysroot issue says only what was read, not when the primary's build ran. Co-Authored-By: Claude Opus 5.5 --- ...ned-during-a-toolchain-rebuild-never-gets-its-cargo.md | 8 ++++---- issues/kernel/deferred-release-outlives-its-syscall.md | 4 ++-- issues/kernel/every-wait-in-this-kernel-is-a-spin.md | 6 +++--- tests/toyos.rs | 2 +- 4 files changed, 10 insertions(+), 10 deletions(-) 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 index 93b70b59dae..9594732d9ff 100644 --- 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 @@ -6,10 +6,10 @@ 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` held -`rustc` and `rustdoc` and no `cargo` from 22:00 until 22:08 on 2026-09-27, -while `/Users/jan/Dev/jan/toyos` ran `toyos-build --build-only`; the link came -back at 22:08. At 22:03:59 a `cargo test --test toyos-build` in the +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 diff --git a/issues/kernel/deferred-release-outlives-its-syscall.md b/issues/kernel/deferred-release-outlives-its-syscall.md index 343e88157ec..4d819388d52 100644 --- a/issues/kernel/deferred-release-outlives-its-syscall.md +++ b/issues/kernel/deferred-release-outlives-its-syscall.md @@ -237,8 +237,8 @@ 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 +`File` nor `close_all` — that one is also called from `fatal_exception`'s +Ring 3 arm (`arch/idt/exceptions.rs:348`), which has no syscall to return through. The track carries this as **wall 4**, with the three shapes the owner has to 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 4e7f0d34e3d..3243960c396 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 @@ -157,8 +157,8 @@ both before any lock conversion; the order is forced, not preferred. `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 + path a `try_lock` with no answer for its failure: `fatal_exception`'s + Ring 3 arm reaches `process::exit` from a CPU exception (`arch/idt/exceptions.rs:348`), and nothing on that path mints a `Parkable`. What *does* have to convert is `Lock`, and wall 5 is why. @@ -381,7 +381,7 @@ means everywhere, not only here. Three shapes: 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 + `fatal_exception`'s Ring 3 arm, which has no syscall to return through; 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/tests/toyos.rs b/tests/toyos.rs index 4f60c4893f8..0012796692e 100644 --- a/tests/toyos.rs +++ b/tests/toyos.rs @@ -2993,7 +2993,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 From f9700b900bae2c97ed4cac8f7aae281b8246d3da Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 27 Sep 2026 22:29:34 +0200 Subject: [PATCH 13/17] CLAUDE.md: the Kernel and Userspace daemons paragraphs are two paragraphs again Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CLAUDE.md b/CLAUDE.md index f3bfd751f1d..ff922a95f2a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -38,6 +38,7 @@ A subdirectory `CLAUDE.md` loads when a file in that subtree is `Read`, and not > A snapshot, deliberately shallow — always read the code. **Kernel** — minimal; new additions are discussed and justified. Resource management, scheduling, process lifecycle, filesystem, device arbitration. 2 MB pages, demand paging, PIE binaries, full SMP. + **Userspace daemons** — compositor, netd, soundd, sshd, logd. Each claims a device or capability from the kernel and serves its function; crash one and the kernel is fine. **The log is a userland file.** `/system/bin/logd` reads records on a cursor and owns `/log`; the kernel keeps the record ring, the console and the panel, and writes no file. `SYS_FSYNC` reaches the device's cache flush because logd's durability claim rests on it. From 69e8dff55b4eb8f6e3d6bd3c89ea0a05f614d487 Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 27 Sep 2026 23:15:31 +0200 Subject: [PATCH 14/17] Panic path: reset without waiting on the console wire `syscall_panic_halts` went red at f9700b90: the guest printed "returning this machine to firmware" and QEMU reported no reset for the rest of the harness's wait. The QMP connection was up (a late connect to an exited QEMU fails loudly instead), and the 20 s drain after it ran to its end, so QEMU was still running. The mechanism, read from the code and that run's serial: - `klogd` was stopped after one 16-byte UART burst of a line (`[kernel 0.573 cp`). The halt IPI is a fixed vector, so a CPU stops only with interrupts on, and the one console lock held with interrupts on is the wire. - `reboot_now` reset through `acpi::reboot`, whose `serial::flush_final` spins `PANIC_LOCK_SPIN_LIMIT` (100,000,000 `try_lock`+`pause`) on that wire. The limit is a count, not a time bound. `reboot_now` now resets through `acpi::reset_now`: the log is already drained by then, and nothing on the panic path waits on a lock a stopped CPU may hold. The count itself is filed: `panic-path/the-panic-lock-spin-limit-is-a-count-its-comment-calls-a-second.md`. The syscall-death rows also subscribe to QMP before they write the command, as `machine_reboot` does (`watch_the_bound`). QMP delivers no event emitted before its client connected. Co-Authored-By: Claude Opus 5.5 --- ...t-is-a-count-its-comment-calls-a-second.md | 32 +++++++++++++++++++ kernel/src/drivers/xhci/stop.rs | 3 +- kernel/src/panic_reboot.rs | 14 ++++---- tests/common/power.rs | 32 ++++++++++++------- .../src/bin/panic_recovery.rs | 28 ---------------- .../src/bin/segfault_child.rs | 11 ------- 6 files changed, 60 insertions(+), 60 deletions(-) create mode 100644 issues/panic-path/the-panic-lock-spin-limit-is-a-count-its-comment-calls-a-second.md delete mode 100644 tests/toyos-rust-tests/src/bin/panic_recovery.rs delete mode 100644 tests/toyos-rust-tests/src/bin/segfault_child.rs 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..39805f40fe7 --- /dev/null +++ b/issues/panic-path/the-panic-lock-spin-limit-is-a-count-its-comment-calls-a-second.md @@ -0,0 +1,32 @@ +--- +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, and nobody has +measured that on any machine this kernel boots. + +It stopped a machine from resetting once. In the orchestrator's +`cargo test --test toyos-build -- --nightly syscall_panic_halts` at `f9700b90`, +EXIT=1, the guest printed `returning this machine to firmware` and QEMU reported +no reset for the rest of the harness's wait. The log prints a 2.96x liveness +width, so that wait was 25 s x 2.96 plus the 20 s drain after it, about 94 s. +The serial shows `klogd` stopped after one 16-byte burst of a line +(`[kernel 0.573 cp`). The halt IPI is a fixed vector, so a CPU stops only with +interrupts on, and the one console lock held with interrupts on is the wire. +The panic path's reset then went through `acpi::reboot`, whose `flush_final` +spun this count on that wire. The panic path now resets through +`acpi::reset_now` and does not wait on the wire. Both waits above still take +the count. + +**Evidence:** the run above, and 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/src/drivers/xhci/stop.rs b/kernel/src/drivers/xhci/stop.rs index b76afe0e41a..889c5b7fd47 100644 --- a/kernel/src/drivers/xhci/stop.rs +++ b/kernel/src/drivers/xhci/stop.rs @@ -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 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/tests/common/power.rs b/tests/common/power.rs index 56185dce397..9b481909542 100644 --- a/tests/common/power.rs +++ b/tests/common/power.rs @@ -885,20 +885,27 @@ 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 (budget, _) = resets_inside_the_bound(&mut qemu, PANICKED_AND_STAYED_UP)?; + 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 own `guest-reset`, inside the fast bound plus what a reset costs, -/// and the serial the guest wrote after its boot log; `never` is what a guest -/// that did not stop means to the caller. +/// 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); + (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 budget = qemu.budget(Duration::from_secs(PANIC_FAST_SECS) + RESET_ALLOWANCE); - let mut stop = qemu::QmpShutdown::open(qemu.qmp_socket(), budget); 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); @@ -910,9 +917,7 @@ fn resets_inside_the_bound( } /// `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: a recovered -/// `klogd` takes the console down with it, so the boot stops at klogd's spawn -/// line, which a halting and a recovering kernel both write. +/// 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)], @@ -925,8 +930,9 @@ pub fn klogd_death_resets( 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, dead, never, said).map(drop) + died_and_reset(&mut qemu, watch, dead, never, said).map(drop) } /// `SYS_DEBUG` `action` ends the kernel inside its caller's syscall, and the @@ -947,23 +953,25 @@ pub fn syscall_death_resets( }; 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, dead, never, said) + 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, never)?; + let (budget, tail) = resets_inside_the_bound(qemu, watch, never)?; dead.push(&tail); for want in said { dead.must_say(want)?; 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 dc7692d12b2..00000000000 --- a/tests/toyos-rust-tests/src/bin/panic_recovery.rs +++ /dev/null @@ -1,28 +0,0 @@ -use std::process::Command; - -fn main() { - test_user_segfault(); - test_system_alive(); - println!("all panic recovery tests passed"); -} - -/// 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 the segfault. -fn test_system_alive() { - let output = Command::new("/system/bin/echo") - .arg("still alive") - .output() - .expect("failed to run echo after the segfault"); - assert!(output.status.success()); - let stdout = String::from_utf8_lossy(&output.stdout); - assert_eq!(stdout.trim(), "still alive"); - println!(" PASS: system alive after a user segfault"); -} 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(); -} From c645c04551cc6a85d67621dbf5dd6723be0fb4bc Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 27 Sep 2026 23:15:44 +0200 Subject: [PATCH 15/17] syscall_fault_halts reads a live window; panic_recovery folds into fault_gates - **The demand-paging row.** `SYS_DEBUG` `NULL_READ` is now a Ring 0 read of the address its caller passes, recorded first as `SYS_DEBUG: a Ring 0 read of ` (a record, because the panic path drains records and a program's own line is still in userland when the machine ends). `test_panic_child` maps 4 MiB it never touches and passes base + 2 MiB. `syscall_fault_halts` requires `KERNEL PANIC: read unmapped address at `: a kernel that demand-paged the window for Ring 0 re-executes into SMAP's protection fault and says `protection violation` instead. This row replaces the read of 0. - **`panic_recovery` is folded into `fault_gates`**: a `pf` arm in `fault_gate_child` whose report `check_fault_gates` requires. Deleted: `panic_recovery.rs`, `check_panic_recovery`, its timeout and duration rows, and `segfault_child`, which nothing else ran. - **`(Ready(_), Dead)` is not a legal edge.** The only production edge into `Dead` is `Running -> Dead`. - **`deaf_window` holds an `IrqGuard`**; the unconditional `sti` had no reason left. - **`in_syscall` stays.** The stack-top frame cannot tell a syscall's from a Ring 3 interrupt's without reading words userland chose. The issue gains the other migration direction and `process::handle_fault`. - The review's REMOVE lines are deleted, as are the `fatal_exception` citations in the two wall-4 issues; the recovering-`klogd` sentence went with the previous commit. Co-Authored-By: Claude Opus 5.5 --- .../interrupt-entry-keeps-a-ring-3-ac-flag.md | 2 +- .../deferred-release-outlives-its-syscall.md | 4 +- .../every-wait-in-this-kernel-is-a-spin.md | 9 +-- ...t-can-name-a-syscall-that-already-ended.md | 37 ++++++++--- kernel-loom/tests/reap_gate.rs | 3 +- kernel/src/drivers/panic_console/mod.rs | 4 +- kernel/src/quiesce.rs | 3 +- kernel/src/sched/dump.rs | 5 +- kernel/src/syscall/dispatch.rs | 12 +++- tests/test-durations | 1 - .../src/bin/disk_backtrace_child.rs | 8 +-- .../src/bin/fault_gate_child.rs | 8 +++ tests/toyos-rust-tests/src/bin/fault_gates.rs | 1 + .../toyos-rust-tests/src/bin/heap_ceiling.rs | 4 +- .../src/bin/process_lifecycle.rs | 4 +- .../src/bin/test_panic_child.rs | 23 ++++++- tests/toyos.rs | 63 ++++++++----------- toyos-abi/src/syscall.rs | 4 +- toyos-sched/src/hw.rs | 6 +- toyos-sched/src/task.rs | 2 +- 20 files changed, 114 insertions(+), 89 deletions(-) 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 index 560f060f176..8137e1bd6b7 100644 --- a/issues/isolation/interrupt-entry-keeps-a-ring-3-ac-flag.md +++ b/issues/isolation/interrupt-entry-keeps-a-ring-3-ac-flag.md @@ -17,7 +17,7 @@ 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 instead of faulting into `blame`. +writes it silently. **Evidence:** read from the code and the SDM; no test stages it. diff --git a/issues/kernel/deferred-release-outlives-its-syscall.md b/issues/kernel/deferred-release-outlives-its-syscall.md index 4d819388d52..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 `fatal_exception`'s -Ring 3 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 3243960c396..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 @@ -156,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: `fatal_exception`'s - Ring 3 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.** @@ -380,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 - `fatal_exception`'s Ring 3 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/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 index e74e313baa0..9047cc7ac4f 100644 --- 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 @@ -11,16 +11,35 @@ The crash report prints `Syscall:` and a user backtrace only while 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. +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`). -So a syscall that parks on CPU A and finishes on CPU B leaves A's word naming -its 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. +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 bracket names a thread only while that thread is inside -a syscall on this CPU, and a guest test stages a migrated syscall followed by an -interrupt-context death on the first CPU whose report carries no `Syscall:` -line. +**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/kernel-loom/tests/reap_gate.rs b/kernel-loom/tests/reap_gate.rs index aae85b1516f..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 -//! unpublished exit*, 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}; diff --git a/kernel/src/drivers/panic_console/mod.rs b/kernel/src/drivers/panic_console/mod.rs index 9903b558d0d..8ab04c87f44 100644 --- a/kernel/src/drivers/panic_console/mod.rs +++ b/kernel/src/drivers/panic_console/mod.rs @@ -258,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,7 +289,7 @@ static CLAIMED_AT: core::sync::atomic::AtomicU64 = core::sync::atomic::AtomicU64 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. +/// idle loop. #[cfg(feature = "boot-actuators")] pub fn probe_due() -> bool { if !crate::actuator::metal_panic_probe() { diff --git a/kernel/src/quiesce.rs b/kernel/src/quiesce.rs index 5d11e9eea7d..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` and `iod` -//! 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/dump.rs b/kernel/src/sched/dump.rs index cca1edf3cd5..81e23cffb06 100644 --- a/kernel/src/sched/dump.rs +++ b/kernel/src/sched/dump.rs @@ -384,12 +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. - 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; diff --git a/kernel/src/syscall/dispatch.rs b/kernel/src/syscall/dispatch.rs index 1731f3d63ea..0aa2db888ed 100644 --- a/kernel/src/syscall/dispatch.rs +++ b/kernel/src/syscall/dispatch.rs @@ -533,9 +533,15 @@ 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 => { let _held = LOCK_ACROSS_SWITCH.lock(); crate::scheduler::yield_now(); diff --git a/tests/test-durations b/tests/test-durations index 7fdc48c8abe..4af45c58707 100644 --- a/tests/test-durations +++ b/tests/test-durations @@ -311,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 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 b261f290a14..d524e957657 100644 --- a/tests/toyos-rust-tests/src/bin/heap_ceiling.rs +++ b/tests/toyos-rust-tests/src/bin/heap_ceiling.rs @@ -111,9 +111,7 @@ fn at_ceiling_is_servable() { /// /// `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. +/// 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!( 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/test_panic_child.rs b/tests/toyos-rust-tests/src/bin/test_panic_child.rs index d7b7df02bed..513641cd0aa 100644 --- a/tests/toyos-rust-tests/src/bin/test_panic_child.rs +++ b/tests/toyos-rust-tests/src/bin/test_panic_child.rs @@ -5,15 +5,34 @@ //! `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. +//! +//! `NULL_READ` is given an address inside a live anonymous region this process +//! never touched: a page demand paging fills for Ring 3 and must not for Ring 0. + +use toyos_abi::syscall::{self, debug_action, MmapFlags, MmapProt, SyscallError}; -use toyos_abi::syscall::SyscallError; +const PAGE_2M: usize = 2 << 20; fn main() { let action: u64 = std::env::args() .nth(1) .and_then(|s| s.parse().ok()) .expect("usage: test_panic_child "); - let rc = toyos_abi::syscall::debug(action); + let rc = if action == debug_action::NULL_READ { + // SAFETY: a fresh anonymous region this process never dereferences. + let base = unsafe { + syscall::mmap( + core::ptr::null_mut(), + 2 * PAGE_2M, + MmapProt::READ | MmapProt::WRITE, + MmapFlags::ANONYMOUS | MmapFlags::PRIVATE, + ) + }; + assert!(!base.is_null(), "mmap of {} bytes failed", 2 * PAGE_2M); + syscall::debug_with(action, base as u64 + PAGE_2M as u64) + } else { + syscall::debug(action) + }; if rc == SyscallError::InvalidArgument.to_u64() { eprintln!( "ERROR: SYS_DEBUG {action} answered InvalidArgument — this kernel carries no \ diff --git a/tests/toyos.rs b/tests/toyos.rs index 0012796692e..cf9d6b662b4 100644 --- a/tests/toyos.rs +++ b/tests/toyos.rs @@ -252,7 +252,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. @@ -3094,34 +3093,9 @@ fn check_rust_result(result: &TestResult) -> bool { } } -/// Checks both exit code and the segfault's crash report. -fn check_panic_recovery(result: &TestResult) -> bool { - if !check_rust_result(result) { - return false; - } - - let checks: &[(&str, &str)] = &[ - ("Registers:", "expected register dump from the 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; - } - } - 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; @@ -3211,6 +3185,25 @@ 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:?}"))?; + 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 @@ -3289,6 +3282,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; @@ -3465,7 +3460,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, @@ -13319,10 +13313,7 @@ fn run_machine_test( &["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, - &["KERNEL PANIC: read unmapped address at 0x0", syscall, "User backtrace:"], - ), + "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. @@ -13332,10 +13323,11 @@ fn run_machine_test( other => unreachable!("{other} is not a syscall-death row"), }; let said = power::syscall_death_resets(test_config, c_bins, rust_bins, action, said)?; - if name == "lock_across_switch_halts" { - check_tripwire_attribution(&said)?; + match name { + "lock_across_switch_halts" => check_tripwire_attribution(&said), + "syscall_fault_halts" => check_ring0_read_unmapped(&said), + _ => Ok(()), } - Ok(()) } "hash_seed_precedes_every_map" => { // `kernel/src/hasher.rs`'s `UNSEEDED`, as a prefix: the wrong seed @@ -18287,7 +18279,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), 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-sched/src/hw.rs b/toyos-sched/src/hw.rs index 780fd41a743..8211ef984c1 100644 --- a/toyos-sched/src/hw.rs +++ b/toyos-sched/src/hw.rs @@ -101,11 +101,7 @@ pub trait Machine: Kicker + 'static { /// 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 — and an RAII guard restores - /// the caller's flags instead. + /// Has no caller in either world. fn irq_guard(&self) -> Self::IrqGuard; /// Enable interrupts and halt, atomically — on x86 the `sti;hlt` pair and diff --git a/toyos-sched/src/task.rs b/toyos-sched/src/task.rs index 73ff5b4efe9..419de23ae15 100644 --- a/toyos-sched/src/task.rs +++ b/toyos-sched/src/task.rs @@ -275,7 +275,7 @@ fn legal(from: TaskState, to: TaskState) -> bool { (Running(_), Dead) => true, // 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)) From 355e0d64275073fb94c82e414b79098b110de20c Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 27 Sep 2026 23:53:50 +0200 Subject: [PATCH 16/17] syscall_fault_halts reads an untouched .bss window; its red carries the capture - **The window was never demand-paged.** An anonymous `mmap` is mapped when it is made (`sys_mmap` allocates its pages and maps them with `alloc_and_map`/`map_range`, and the region is `RegionKind::Mapped`), so base + 2 MiB of one was present, and the Ring 0 read took SMAP's fault on a present page: `read protection violation`, which the row refused. The kernel's report was right; the test's region was wrong. Demand-paged (`RegionKind::Anonymous`) regions are the loader's: a segment's pages past its file bytes. `test_panic_child` now reads the 2 MiB-aligned window inside a 4 MiB `static mut` it never touches, so the fault is not-present and must say `unmapped`, and a kernel that fills it for Ring 0 re-executes into SMAP's `protection violation`. - **A syscall-death check's error carries the capture.** These guests put the 16550 on stdio, so no `uart-*.log` exists for the red run to keep, and the check's bare message was all that was left. The harness-wide gap is filed: `issues/build/a-guest-with-no-virtio-keeps-no-serial-when-its-run-reds.md`. Co-Authored-By: Claude Opus 5.5 --- ...irtio-keeps-no-serial-when-its-run-reds.md | 23 ++++++++++++++++++ .../src/bin/test_panic_child.rs | 24 ++++++++----------- tests/toyos.rs | 3 +++ 3 files changed, 36 insertions(+), 14 deletions(-) create mode 100644 issues/build/a-guest-with-no-virtio-keeps-no-serial-when-its-run-reds.md 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/tests/toyos-rust-tests/src/bin/test_panic_child.rs b/tests/toyos-rust-tests/src/bin/test_panic_child.rs index 513641cd0aa..7fd8e325e51 100644 --- a/tests/toyos-rust-tests/src/bin/test_panic_child.rs +++ b/tests/toyos-rust-tests/src/bin/test_panic_child.rs @@ -6,30 +6,26 @@ //! needs `test-actuators` and asked for nothing, which is a harness mistake and //! not a kernel that failed to stop. //! -//! `NULL_READ` is given an address inside a live anonymous region this process -//! never touched: a page demand paging fills for Ring 3 and must not for Ring 0. +//! `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::{self, debug_action, MmapFlags, MmapProt, 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()) .expect("usage: test_panic_child "); let rc = if action == debug_action::NULL_READ { - // SAFETY: a fresh anonymous region this process never dereferences. - let base = unsafe { - syscall::mmap( - core::ptr::null_mut(), - 2 * PAGE_2M, - MmapProt::READ | MmapProt::WRITE, - MmapFlags::ANONYMOUS | MmapFlags::PRIVATE, - ) - }; - assert!(!base.is_null(), "mmap of {} bytes failed", 2 * PAGE_2M); - syscall::debug_with(action, base as u64 + PAGE_2M as u64) + let window = (&raw const UNTOUCHED as usize).next_multiple_of(PAGE_2M); + syscall::debug_with(action, window as u64) } else { syscall::debug(action) }; diff --git a/tests/toyos.rs b/tests/toyos.rs index cf9d6b662b4..3d8ab2fb762 100644 --- a/tests/toyos.rs +++ b/tests/toyos.rs @@ -13323,11 +13323,14 @@ fn run_machine_test( 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 From 37d98262622297c2f916a98b983f2456dbb44b8e Mon Sep 17 00:00:00 2001 From: japabu Date: Mon, 28 Sep 2026 01:04:07 +0200 Subject: [PATCH 17/17] Round 5 review fixes: dead IrqGuard trait items go, the null-read check refuses address 0, and three stale comments the review named are cut MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Machine::irq_guard`/`type IrqGuard` have no caller in either world — `deaf_window` calls the concrete `crate::arch::IrqGuard::close()` directly and never went through the trait. Deleted from the trait and its five impls (kernel x86_64 and aarch64, the sim, the unit-test double, and the loom model), the last two of which the review's own list missed and clippy's aarch64 kernel invocation and the loom build caught in their place. `check_ring0_read_unmapped` accepted address 0, so a `test_panic_child` regression to `syscall::debug(action)` for every action — dropping the demand-paged window argument — would read null and stay green. It now refuses `addr == 0` by name. Three comments the review found false once panic recovery left: `xhci/stop.rs` no longer says its summary is written from `acpi::reboot` or that `before_reset` has exactly two callers (`reset_now` is the only path now); `panic_console`'s `probe_due` doc drops "called from the idle loop", which only narrated the caller; the spin-limit issue drops the disproven "nobody has measured that" claim and the `f9700b90` paragraph the wire-held measurement already refuted, along with the `tests/common/metal.rs` and `tests/toyos.rs` comments citing the deleted `panic_recovery` test. Filed `issues/panic-path/a-syscall-panic-reset-the-guest-announced-and-qemu-never-saw.md` for the `f9700b90` red the spin-limit issue used to (wrongly) explain. Co-Authored-By: Claude Opus 5.5 --- ...-the-guest-announced-and-qemu-never-saw.md | 53 +++++++++++++++++++ ...t-is-a-count-its-comment-calls-a-second.md | 18 +------ kernel/src/arch/aarch64/hw.rs | 6 --- kernel/src/arch/x86_64/hw.rs | 6 --- kernel/src/drivers/panic_console/mod.rs | 3 +- kernel/src/drivers/xhci/stop.rs | 9 ++-- tests/common/metal.rs | 4 +- tests/toyos.rs | 10 ++-- toyos-sched/loom/tests/loom_retire.rs | 2 - toyos-sched/sim/src/hw_impl.rs | 7 --- toyos-sched/src/cpu.rs | 2 - toyos-sched/src/hw.rs | 9 +--- 12 files changed, 66 insertions(+), 63 deletions(-) create mode 100644 issues/panic-path/a-syscall-panic-reset-the-guest-announced-and-qemu-never-saw.md 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/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 index 39805f40fe7..ed33d6b0bd7 100644 --- 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 @@ -10,23 +10,9 @@ opened: 2026-09-27 `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, and nobody has -measured that on any machine this kernel boots. +whatever that many `pause`s cost on the CPU running them. -It stopped a machine from resetting once. In the orchestrator's -`cargo test --test toyos-build -- --nightly syscall_panic_halts` at `f9700b90`, -EXIT=1, the guest printed `returning this machine to firmware` and QEMU reported -no reset for the rest of the harness's wait. The log prints a 2.96x liveness -width, so that wait was 25 s x 2.96 plus the 20 s drain after it, about 94 s. -The serial shows `klogd` stopped after one 16-byte burst of a line -(`[kernel 0.573 cp`). The halt IPI is a fixed vector, so a CPU stops only with -interrupts on, and the one console lock held with interrupts on is the wire. -The panic path's reset then went through `acpi::reboot`, whose `flush_final` -spun this count on that wire. The panic path now resets through -`acpi::reset_now` and does not wait on the wire. Both waits above still take -the count. - -**Evidence:** the run above, and the code. +**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/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/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/drivers/panic_console/mod.rs b/kernel/src/drivers/panic_console/mod.rs index 8ab04c87f44..12c37d40e58 100644 --- a/kernel/src/drivers/panic_console/mod.rs +++ b/kernel/src/drivers/panic_console/mod.rs @@ -288,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. +/// Whether the `metal-panic-probe` boot should panic now. #[cfg(feature = "boot-actuators")] pub fn probe_due() -> bool { if !crate::actuator::metal_panic_probe() { diff --git a/kernel/src/drivers/xhci/stop.rs b/kernel/src/drivers/xhci/stop.rs index 889c5b7fd47..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); @@ -635,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/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/toyos.rs b/tests/toyos.rs index 3d8ab2fb762..4306ca410fd 100644 --- a/tests/toyos.rs +++ b/tests/toyos.rs @@ -3137,13 +3137,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(); @@ -3197,6 +3190,9 @@ fn check_ring0_read_unmapped(serial: &str) -> Result<(), String> { .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}")); 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 8211ef984c1..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,14 +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. - 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. ///