From 23857ec9d7d5a99cbbc7528c871655714028fbde Mon Sep 17 00:00:00 2001 From: japabu Date: Thu, 8 Oct 2026 13:29:12 +0200 Subject: [PATCH 1/4] Harness: metal boots are taken whole by name, a stalled wait carries what the guest said, and a guest test boots the console image The suite takes every word that is nobody's value as a filter, and a word `boot:` under `--metal` takes that boot whole: its shared members as the whole profile chunks them, and every row with an arm on it. `metal::select` is the one selection, after chunking, so `shared-2` is a name; a word that takes nothing is refused by name in both suites. The line that said the shared boots' members are not filtered by name was false and is gone with the two per-table filters it stood beside. `await_guest`'s stall puts the last forty lines the guest said during the wait under its sentence, with the count of what was cut, through the one `window` the never-announced verdict already cut by. `console_image_boots` boots `console/system.toml`'s image on the metal-sim shape with no display and waits for `console: ready`. Nothing is typed at its shell: its only input is the i8042, and issues/the-console-loses-typed-keystrokes-under-host-load.md is open. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01RvnWQFcMuGqTHYhvSnTe8A --- src/testargs.rs | 72 ++++++++++++++++++++++------------- tests/checks.rs | 22 +++++++---- tests/checks/metal.rs | 50 ++++++++++++++++++++++++ tests/checks/qemu.rs | 14 +++++++ tests/common/metal.rs | 70 ++++++++++++++++++++++++++++------ tests/common/qemu.rs | 34 +++++++++++------ tests/toyos.rs | 88 ++++++++++++++++++++++--------------------- 7 files changed, 250 insertions(+), 100 deletions(-) diff --git a/src/testargs.rs b/src/testargs.rs index 898767a8433..8f346b2ce81 100644 --- a/src/testargs.rs +++ b/src/testargs.rs @@ -1,9 +1,10 @@ //! The suite's command line, checked against the flags it actually has. //! -//! `tests/toyos.rs` takes the first word that is nobody's value as the run's -//! filter, so a flag this table does not declare would hand its own value to -//! that filter and report a one-test run as a pass. The table is the harness's -//! whole vocabulary, and [`SUITE`] is the only way to read a word off its argv. +//! `tests/toyos.rs` takes every word that is nobody's value as one of the +//! run's filters, so a flag this table does not declare would hand its own +//! value to them and report a one-test run as a pass. The table is the +//! harness's whole vocabulary, and [`SUITE`] is the only way to read a word +//! off its argv. use crate::flags::declare_flags; use std::path::PathBuf; @@ -23,14 +24,22 @@ declare_flags!(pub SUITE = { pub METAL_READBACK = "--metal-readback", Next; }); -/// The run's filter and `--metal`'s mode, both decided by [`parse`]: an unknown -/// flag refuses the line before either is read. +/// How a filter word names one of the metal profile's boots whole, ahead of +/// the boot's name as `--metal --list` prints it. +pub const BOOT: &str = "boot:"; + +/// The run's filters and `--metal`'s mode, all decided by [`parse`]: an unknown +/// flag refuses the line before any is read. pub struct Parsed<'a> { - pub filter: Option<&'a str>, + /// A run takes every name any of these is part of, and with none, and no + /// `boots`, every name there is. + pub filters: Vec<&'a str>, + /// The [`BOOT`] words, without it: boots `--metal` takes whole. + pub boots: Vec<&'a str>, pub metal: Option, } -/// Validate the harness's argv and return the run's filter and metal mode. +/// Validate the harness's argv and return the run's filters and metal mode. /// /// `Err` is a refusal to print and exit on. It is asked before the sysroot lock /// and before anything is compiled, so a stale command line costs a message @@ -49,20 +58,17 @@ pub fn parse(args: &[String]) -> Result, String> { return Err(refusal); } - let mut filter: Option<&str> = None; - for word in line.positionals { - if let Some(first) = filter { - return Err(format!( - "{first:?} and {word:?}: the suite takes one filter, and the second word \ - would have been dropped in silence.\n\ - A filter is a substring, so `{first}` and `{word}` are one run only if one \ - substring matches both." - )); - } - filter = Some(word); - } + let (boots, filters): (Vec<&str>, Vec<&str>) = + line.positionals.into_iter().partition(|word| word.starts_with(BOOT)); + let boots: Vec<&str> = boots.into_iter().map(|word| &word[BOOT.len()..]).collect(); let has = |want| SUITE.present(args, want); + if let (Some(boot), false) = (boots.first(), has(&METAL)) { + return Err(format!( + "{BOOT}{boot} names a boot of the metal profile, and a run without --metal has \ + none, so it would be dropped in silence; add --metal" + )); + } if has(&JOBS) && has(&JOBS_SHORT) { return Err( "--jobs and -j are two spellings of one width, and the run would read one of \ @@ -87,12 +93,12 @@ pub fn parse(args: &[String]) -> Result, String> { )); } } - if let Some(word) = filter { + for word in filters.iter().chain(&boots) { if word.trim().is_empty() { return Err("--metal with an empty filter: name a registration or drop the word" .to_string()); } - if let Some(flag) = SUITE.0.iter().find(|f| f.name.trim_start_matches('-') == word) { + if let Some(flag) = SUITE.0.iter().find(|f| f.name.trim_start_matches('-') == *word) { return Err(format!( "{word:?} beside --metal is {}'s name without its dashes, and would be \ read as a filter that selects whatever contains it", @@ -125,7 +131,7 @@ pub fn parse(args: &[String]) -> Result, String> { } }); - Ok(Parsed { filter, metal }) + Ok(Parsed { filters, boots, metal }) } /// What `--metal`'s own flags resolve a run to. @@ -147,8 +153,9 @@ mod tests { args.iter().map(ToString::to_string).collect() } + /// The run's filters, as one text. fn parse_owned(args: &[&str]) -> Result, String> { - parse(&owned(args)).map(|p| p.filter.map(ToString::to_string)) + parse(&owned(args)).map(|p| (!p.filters.is_empty()).then(|| p.filters.join(" "))) } fn metal_owned(args: &[&str]) -> Result, String> { @@ -186,9 +193,20 @@ mod tests { } #[test] - fn two_filters_are_refused_because_only_one_would_run() { - let refusal = parse_owned(&["futex", "dlopen"]).unwrap_err(); - assert!(refusal.contains("\"futex\"") && refusal.contains("\"dlopen\""), "{refusal}"); + fn every_word_that_is_nobodys_value_is_a_filter() { + assert_eq!(parse_owned(&["futex", "--jobs", "4", "dlopen"]).unwrap().as_deref(), Some("futex dlopen")); + } + + /// A boot word is `--metal`'s alone, and never one of the filters. + #[test] + fn a_boot_word_names_a_metal_boot_and_nothing_else() { + let line = owned(&["--metal", "boot:shared-2", "control_regs", "boot:ccorpus"]); + let parsed = parse(&line).unwrap(); + assert_eq!((parsed.filters, parsed.boots), (vec!["control_regs"], vec!["shared-2", "ccorpus"])); + let refusal = parse_owned(&["boot:shared"]).unwrap_err(); + assert!(refusal.contains("add --metal"), "{refusal}"); + let refusal = metal_owned(&["--metal", "boot:"]).unwrap_err(); + assert!(refusal.contains("empty filter"), "{refusal}"); } /// Every `None` here is a default the run then takes in silence: `--jobs` diff --git a/tests/checks.rs b/tests/checks.rs index 19efd357599..47234a677b9 100644 --- a/tests/checks.rs +++ b/tests/checks.rs @@ -781,8 +781,8 @@ mod checks { /// architecture. #[test] fn a_run_selects_by_filter() -> Result<(), String> { - let taken = |filter: Option<&str>| -> BTreeSet { - let (machine, screen) = select(filter); + let taken = |filters: &[&str]| -> BTreeSet { + let (machine, screen) = select(filters); machine .iter() .map(|n| n.to_string()) @@ -791,12 +791,13 @@ mod checks { }; let names = |of: &[&str]| -> BTreeSet { of.iter().map(|n| n.to_string()).collect() }; let every: BTreeSet = declared().map(String::from).collect(); - let cases = [ - (None, every), - (Some("virt_el2"), names(&["virt_el2_drop"])), - (Some("el2_drop"), names(&["virt_el2_drop"])), - (Some("nested_nmi"), names(&["nested_nmi_is_loud"])), - (Some("no_such_test"), BTreeSet::new()), + let cases: [(&[&str], _); 6] = [ + (&[], every), + (&["virt_el2"], names(&["virt_el2_drop"])), + (&["el2_drop"], names(&["virt_el2_drop"])), + (&["nested_nmi"], names(&["nested_nmi_is_loud"])), + (&["el2_drop", "nested_nmi"], names(&["virt_el2_drop", "nested_nmi_is_loud"])), + (&["no_such_test"], BTreeSet::new()), ]; for (filter, want) in cases { let got = taken(filter); @@ -910,6 +911,11 @@ mod checks { metal_checks::a_failing_shared_member_fails_its_boot(); } + #[test] + fn metal_words_take_rows_members_and_whole_boots() { + metal_checks::words_take_rows_members_and_whole_boots(); + } + #[test] fn metal_cleared_page_owes_no_panel() { metal_checks::a_cleared_page_owes_no_panel(); diff --git a/tests/checks/metal.rs b/tests/checks/metal.rs index 2bbd38de727..1a90b554af5 100644 --- a/tests/checks/metal.rs +++ b/tests/checks/metal.rs @@ -327,6 +327,56 @@ pub fn a_failing_shared_member_fails_its_boot() { ); } +/// **What a run's words take**: no word the whole profile; a name word every +/// row and member it is part of, chunked as they come; a boot word that boot +/// as the whole profile chunks it, with every row that rides it; and a word +/// that takes nothing is refused. +pub fn words_take_rows_members_and_whole_boots() { + static ROWS: [(&str, Metal); 2] = [ + ("alpha_row", Metal { arms: &[metal::once("own", "tests/testcases", &[], &[])], judge: |_| Ok(()) }), + ("beta_row", Metal { arms: &[metal::once("shared-2", "tests/testcases", &[], &[])], judge: |_| Ok(()) }), + ]; + let boot = |boot: &str, jobs: &[&str]| metal::SharedBoot { + boot: boot.to_string(), + config: "tests/testcases", + params: &[], + features: &[], + members: const { std::num::NonZeroUsize::new(2).expect("a chunk holds a member") }, + jobs: jobs.iter().map(ToString::to_string).collect(), + files: Vec::new(), + links: Vec::new(), + }; + let profile = [boot("shared", &["test_rs_a1", "test_rs_a2", "test_rs_b1"]), boot("ccorpus", &["c1"])]; + let taken = |names: &[&str], boots: &[&str]| { + metal::select(names, boots, &ROWS, &profile).map(|(rows, shared)| { + let rows: Vec<&str> = rows.iter().map(|(name, _)| *name).collect(); + let shared: Vec = + shared.iter().map(|boot| format!("{}={}", boot.boot, boot.jobs.join("+"))).collect(); + (rows.join(","), shared.join(",")) + }) + }; + let took = |rows: &str, shared: &str| Ok::<_, String>((rows.to_string(), shared.to_string())); + assert_eq!( + taken(&[], &[]), + took("alpha_row,beta_row", "shared=test_rs_a1+test_rs_a2,shared-2=test_rs_b1,ccorpus=c1") + ); + assert_eq!(taken(&["b1"], &[]), took("", "shared=test_rs_b1")); + assert_eq!(taken(&["alpha", "c1"], &[]), took("alpha_row", "ccorpus=c1")); + assert_eq!(taken(&[], &["shared"]), took("", "shared=test_rs_a1+test_rs_a2")); + assert_eq!(taken(&[], &["shared-2"]), took("beta_row", "shared-2=test_rs_b1")); + assert_eq!(taken(&["alpha"], &["ccorpus", "shared-2"]), took("alpha_row,beta_row", "shared-2=test_rs_b1,ccorpus=c1")); + assert_eq!(taken(&[], &["own"]), took("alpha_row", "")); + let refused: [(&[&str], &[&str], &str); 3] = [ + (&["a1", "nope"], &[], "\"nope\" is part of no"), + (&["rs_a1"], &[], "\"rs_a1\" is part of no"), + (&[], &["shared", "shared-3"], "boot:shared-3 names no boot"), + ]; + for (names, boots, refusal) in refused { + let said = taken(names, boots).expect_err("a word that takes nothing was accepted"); + assert!(said.contains(refusal), "{said}"); + } +} + /// **A page the pass after the reset cleared owes no panel**: `foreignrecord`'s /// census went with its record, so the boot is green and records its /// `complete_ms` alone. The same record cleared by the pass before the handoff diff --git a/tests/checks/qemu.rs b/tests/checks/qemu.rs index 0096b197cee..5299b29a37e 100644 --- a/tests/checks/qemu.rs +++ b/tests/checks/qemu.rs @@ -288,6 +288,20 @@ pub fn ceiling_self_check() -> Result<(), String> { )); } + // 6. **The window is the tail, and it says what it cut.** A stalled wait's + // verdict and the one above are both this, over a capture of any length. + let long: String = (1..=45).map(|n| format!("line {n}\n")).collect(); + let cut = window(&long); + if cut.lines().next() != Some("(the last 40 of the 45 lines in it)") + || cut.lines().nth(1) != Some("line 6") + || cut.lines().last() != Some("line 45") + { + return Err(format!("45 lines were not cut to their last 40 under the count:\n{cut}")); + } + if window("one\ntwo\n") != "one\ntwo" { + return Err(format!("a capture that fits was cut or counted: {:?}", window("one\ntwo\n"))); + } + eprintln!( " [ceiling] the panic, the stall, the slow test and the healthy run, each named apart \ from the other three; the panic's verdict carries the kernel's own {} lines, a test \ diff --git a/tests/common/metal.rs b/tests/common/metal.rs index 08752b18c62..1217c55b1de 100644 --- a/tests/common/metal.rs +++ b/tests/common/metal.rs @@ -166,6 +166,62 @@ fn sized(shared: &[SharedBoot]) -> Vec { out } +/// The rows and the shared boots a run takes. +pub type Taken = (Vec<(&'static str, &'static Metal)>, Vec); + +/// What a run's words take of the profile: every row and shared member a +/// `names` word is part of, chunked as they come, and every boot a `boots` +/// word names, whole — its shared members as the whole profile chunks them, +/// and every row with an arm on it, which brings that row's other boots. No +/// word at all takes everything. +/// +/// **A word that takes nothing is refused**: it would be dropped in silence. +/// A boot taken whole and a member taken by name that would ride a boot of +/// the same name are two images under one name, which [`batches`] refuses. +pub fn select( + names: &[&str], + boots: &[&str], + rows: &'static [(&'static str, Metal)], + shared: &[SharedBoot], +) -> Result { + // A shared Rust member is filtered by the name its source file has. + fn bare(job: &str) -> &str { + job.strip_prefix("test_rs_").unwrap_or(job) + } + let all = names.is_empty() && boots.is_empty(); + let jobs = || shared.iter().flat_map(|boot| &boot.jobs).map(|job| bare(job)); + if let Some(dead) = names.iter().find(|word| { + !rows.iter().any(|(name, _)| name.contains(**word)) && !jobs().any(|job| job.contains(**word)) + }) { + return Err(format!("{dead:?} is part of no metal registration's name and no shared member's")); + } + let whole = sized(shared); + let arms = || rows.iter().flat_map(|(_, decl)| decl.arms).map(|arm| arm.boot); + let known: BTreeSet<&str> = whole.iter().map(|boot| boot.boot.as_str()).chain(arms()).collect(); + if let Some(dead) = boots.iter().find(|boot| !known.contains(**boot)) { + return Err(format!("boot:{dead} names no boot of the metal profile, whose boots are {known:?}")); + } + + let named = |name: &str| all || names.iter().any(|word| name.contains(word)); + let mut taken: Vec = + whole.iter().filter(|boot| boots.contains(&boot.boot.as_str())).cloned().collect(); + let by_name: Vec = shared + .iter() + .cloned() + .map(|mut boot| { + boot.jobs.retain(|job| named(bare(job))); + boot + }) + .collect(); + taken.extend(sized(&by_name)); + let rows = rows + .iter() + .filter(|(name, decl)| named(name) || decl.arms.iter().any(|arm| boots.contains(&arm.boot))) + .map(|(name, decl)| (*name, decl)) + .collect(); + Ok((rows, taken)) +} + /// How a registration runs on the T14. pub struct Metal { pub arms: &'static [Arm], @@ -568,10 +624,6 @@ fn batches( // First, so a registration naming a shared boot rides it rather than // minting a second one under the same name with a different list. for boot in shared { - // A boot nothing selected is a boot nothing has to flash. - if boot.jobs.is_empty() { - continue; - } let was = out.insert( boot.boot.clone(), Batch { @@ -890,7 +942,8 @@ pub enum Verdict { Staged, } -/// The whole metal profile: batch, build, drive, judge, report. +/// The whole metal profile over what [`select`] took of it: batch, build, +/// drive, judge, report. // Each argument is one of the suite's own flags or tables, passed through // once; a struct holding them would be a second name for the command line. #[allow(clippy::too_many_arguments)] @@ -902,8 +955,6 @@ pub fn run( quiet: bool, ) -> Verdict { let root = super::compile::repo_root(); - let shared = sized(shared); - let shared = shared.as_slice(); let batches = match batches(tests, shared) { Ok(batches) => batches, Err(why) => { @@ -918,7 +969,7 @@ pub fn run( shared.iter().map(|b| b.jobs.len()).sum::(), batches.len(), ); - if runs.is_empty() && shared.iter().all(|b| b.jobs.is_empty()) { + if runs.is_empty() && shared.is_empty() { eprintln!("[metal] nothing to run"); return Verdict::Red; } @@ -1123,9 +1174,6 @@ pub fn judge_readbacks( } let mut members = 0usize; for boot in shared { - if boot.jobs.is_empty() { - continue; - } eprintln!("\n[metal] {}: {} member(s)", boot.boot, boot.jobs.len()); let back = readbacks.get(&boot.boot).expect("every shared boot was batched"); let mut ran = 0usize; diff --git a/tests/common/qemu.rs b/tests/common/qemu.rs index 256fa764615..4177def44bf 100644 --- a/tests/common/qemu.rs +++ b/tests/common/qemu.rs @@ -399,9 +399,21 @@ pub const DIED_SAYING: &str = "--- what the kernel said as it died ---"; pub const NEVER_ANNOUNCED: &str = "--- the guest never announced this test; the window it was given ---"; -/// How many of that window's lines the verdict carries. +/// How many of a window's lines a verdict carries. const WINDOW_LINES: usize = 40; +/// The last [`WINDOW_LINES`] lines of `said`, under the count of what was cut. +pub fn window(said: &str) -> String { + let lines: Vec<&str> = said.lines().collect(); + let kept = lines.len().min(WINDOW_LINES); + let head = if lines.len() > kept { + format!("(the last {kept} of the {} lines in it)\n", lines.len()) + } else { + String::new() + }; + format!("{head}{}", lines[lines.len() - kept..].join("\n")) +} + /// Why a wait ended badly, carrying the guest's own account of it. /// /// **A newtype, because what this closes is an omission and an omission cannot @@ -448,15 +460,7 @@ impl WaitVerdict { if started || verdict.0.contains(DIED_SAYING) || before.trim().is_empty() { return verdict; } - let lines: Vec<&str> = before.lines().collect(); - let kept = lines.len().min(WINDOW_LINES); - let head = if lines.len() > kept { - format!("(the last {kept} of the {} lines in it)\n", lines.len()) - } else { - String::new() - }; - let tail = lines[lines.len() - kept..].join("\n"); - Self(format!("{}\n{NEVER_ANNOUNCED}\n{head}{tail}", verdict.0)) + Self(format!("{}\n{NEVER_ANNOUNCED}\n{}", verdict.0, window(before))) } } @@ -544,7 +548,7 @@ pub fn ceiling_verdict( /// the wait ends when the guest goes quiet or wedges, so a guest with a twelfth /// of the machine costs the run wall clock and never a verdict — and when it /// does end early the message says so in the words the classifier reads -/// ([`STALLED`]). +/// ([`STALLED`]), above the [`window`] of what the guest said during the wait. /// /// `doing` is what the guest was asked to do, in the caller's own words. The /// caller keeps its assertion; what this owns is the difference between "it did @@ -583,7 +587,13 @@ pub fn await_guest( ) .to_string()); } - Err(format!("{STALLED} waiting for {doing} — {}", live.why())) + // Under the sentence, which stays the line a summary quotes: a wait that + // ended on nothing has no other account of what the guest did instead. + Err(format!( + "{STALLED} waiting for {doing} — {}\n--- what the guest said while it was waited on ---\n{}", + live.why(), + window(since) + )) } /// [`await_guest`] for the common case: one marker anywhere in the capture. diff --git a/tests/toyos.rs b/tests/toyos.rs index 0e30d5c6acd..cc34a785477 100644 --- a/tests/toyos.rs +++ b/tests/toyos.rs @@ -283,6 +283,9 @@ const MACHINE_TESTS: &[&str] = &[ // function is its I219, and a kernel that dies on this takes the bench's // machine down with it. "bar_map_again", + // `console/system.toml`'s image, which runs no job and hands no machine + // back: a metal boot of it ends with a hand on the power button. + "console_image_boots", ]; /// **The metal profile**: which registrations run on the ThinkPad T14, what @@ -1049,7 +1052,7 @@ const SELFTESTS: &[metal::Arm] = &[metal::once( /// The boots every discovered Rust binary rides on the T14: the shipping /// kernel's, and [`ACTUATOR_TESTS`] on the kernel that carries `SYS_DEBUG`. -fn shared_metal(keep: impl Fn(&str) -> bool) -> Vec { +fn shared_metal() -> Vec { let (debug, shipping): (Vec, Vec) = discover_rust_tests() .into_iter() .partition(|name| ACTUATOR_TESTS.contains(&name.as_str())); @@ -1060,11 +1063,7 @@ fn shared_metal(keep: impl Fn(&str) -> bool) -> Vec { params: &[], features: &[], members: const { metal::members_fitting(toyos_tco::RUST_MEMBER_MS) }, - jobs: shipping - .iter() - .filter(|n| keep(n)) - .map(|n| format!("test_rs_{n}")) - .collect(), + jobs: shipping.iter().map(|n| format!("test_rs_{n}")).collect(), files: Vec::new(), links: Vec::new(), }, @@ -1079,7 +1078,7 @@ fn shared_metal(keep: impl Fn(&str) -> bool) -> Vec { params: &[], features: toyos_build::build::TEST_KERNEL, members: const { std::num::NonZeroUsize::new(18).expect("a chunk holds a member") }, - jobs: debug.iter().filter(|n| keep(n)).map(|n| format!("test_rs_{n}")).collect(), + jobs: debug.iter().map(|n| format!("test_rs_{n}")).collect(), files: Vec::new(), links: Vec::new(), }, @@ -1113,18 +1112,12 @@ const LATENCYCASE: &[metal::Arm] = &[metal::once( /// host reading the stick can say which of a hundred and nineteen failed. A /// job list of a hundred and nineteen `ccheck`s would leave one name and a /// hundred and nineteen records told apart only by position. -fn c_corpus_metal( - c_bins: &[(String, Vec)], - keep: impl Fn(&str) -> bool, -) -> metal::SharedBoot { +fn c_corpus_metal(c_bins: &[(String, Vec)]) -> metal::SharedBoot { let mut jobs = Vec::new(); let mut files = Vec::new(); let mut links = Vec::new(); let mut seen: BTreeMap = BTreeMap::new(); for (case, data) in c_bins { - if !keep(case) { - continue; - } // A case with no committed expectation is one nothing could judge, and // shipping it would be a job that passes by comparing nothing. let Some(expected) = c_expectation(case) else { continue }; @@ -1768,6 +1761,23 @@ fn acpi_mediated_access() -> Result<(), String> { Ok(()) } +/// Boot the image `--console-boot` builds, on the machine shape with a panel +/// for `/system/bin/console` to claim, and wait for what that image owes at +/// boot: the console up on its panel, having read the log and the keyboard. +/// Nothing is typed at its shell, whose only input is the i8042: +/// `issues/the-console-loses-typed-keystrokes-under-host-load.md`. +fn console_image_boots() -> Result<(), String> { + let mut qemu = QemuInstance::boot_with_options( + &compile::repo_root().join("console"), + &[], + &[], + BootOptions { profile: qemu::Profile::Metal, ready_marker: bootlog::COMPLETE, ..Default::default() }, + ); + let mut console = format!("{}\n", qemu.boot_log()); + await_marker(&mut qemu, &mut console, "console: ready ", "the console to take its panel")?; + serial::Serial::named("the console image's boot", console).must_be_clean() +} + /// A `mask-windows` boot's windows: `common::irqcensus::windows`'s verdict, /// with `cpus` CPUs reporting. fn mask_windows(capture: &str, cpus: u32) -> Result<(), String> { @@ -2805,6 +2815,7 @@ fn run_machine_test(name: &str, test_config: &Path) -> Result<(), String> { "acpi_mediated_access" => acpi_mediated_access(), "machine_shutdown_short_stop" => power::machine_shutdown_short_stop(test_config), "bar_map_again" => bar_map_again(test_config), + "console_image_boots" => console_image_boots(), other => Err(format!("unknown machine test {other}")), } } @@ -4744,19 +4755,19 @@ fn build_tasks( machine.chain(screen).collect() } -/// Whether a run takes `name`: its filter matches it. -fn kept(filter: Option<&str>, name: &str) -> bool { - filter.is_none_or(|f| name.contains(f)) +/// Whether a run takes `name`: it has no filter, or one that matches it. +fn kept(filters: &[&str], name: &str) -> bool { + filters.is_empty() || filters.iter().any(|f| name.contains(f)) } /// The machine tests and the screen tests a run boots. type Selection = (Vec<&'static str>, Vec<(&'static str, qemu::Profile)>); /// Every declared test a run [`kept`]. -fn select(filter: Option<&str>) -> Selection { +fn select(filters: &[&str]) -> Selection { ( - MACHINE_TESTS.iter().filter(|n| kept(filter, n)).copied().collect(), - SCREEN_TESTS.iter().filter(|(n, _)| kept(filter, n)).copied().collect(), + MACHINE_TESTS.iter().filter(|n| kept(filters, n)).copied().collect(), + SCREEN_TESTS.iter().filter(|(n, _)| kept(filters, n)).copied().collect(), ) } @@ -4873,9 +4884,7 @@ fn main() { std::process::exit(1); } }; - let filter = parsed.filter; - // The one selection every entry point below takes, the metal's included. - let keep = |name: &str| kept(filter, name); + let filters = parsed.filters.as_slice(); let debug_mode = SUITE.present(&args, &testargs::DEBUG); let list_mode = SUITE.present(&args, &testargs::LIST); @@ -4914,22 +4923,15 @@ fn main() { if let Some(mode) = parsed.metal { let (c_bins, rust_bins) = build_shared_bins(); - let selected: Vec<(&str, &'static metal::Metal)> = METAL - .iter() - .filter(|(name, _)| keep(name)) - .map(|(name, decl)| (*name, decl)) - .collect(); - // The shared boots carry names no registration holds, so an empty - // selection is only a dead filter when they are empty too — which - // `metal::run` says for itself. - if selected.is_empty() && filter.is_some() { - eprintln!( - "[toyos] no metal registration matches filter {filter:?}; the shared boots' \ - members are not filtered by name" - ); - } - let mut boots = shared_metal(keep); - boots.push(c_corpus_metal(&c_bins, keep)); + let mut boots = shared_metal(); + boots.push(c_corpus_metal(&c_bins)); + let (selected, boots) = match metal::select(filters, &parsed.boots, METAL, &boots) { + Ok(selection) => selection, + Err(refusal) => { + eprintln!("[toyos] {refusal}"); + run.exit(1); + } + }; // Three statuses for the three things this can establish, as the // ordinary suite has: green, red, and "measured nothing" — a run that @@ -4957,10 +4959,12 @@ fn main() { return; } - let (machine_to_run, screen_to_run) = select(filter); + let (machine_to_run, screen_to_run) = select(filters); - if screen_to_run.is_empty() && machine_to_run.is_empty() { - eprintln!("No test matches filter {filter:?}"); + // A filter that takes nothing would be dropped in silence beside one that + // takes something. + if let Some(dead) = filters.iter().find(|f| !declared().any(|name| name.contains(**f))) { + eprintln!("No test matches filter {dead:?}"); run.exit(1); } From 795a74859bc31fb0f73877ad17be26467d8ffa90 Mon Sep 17 00:00:00 2001 From: japabu Date: Thu, 8 Oct 2026 13:36:50 +0200 Subject: [PATCH 2/4] The corpus check calls c_corpus_metal as it is now, and the last drain's bounded count on a held wire is filed `tests/checks.rs` still passed the filter `c_corpus_metal` no longer takes, so `toyos-checks` did not compile at 23857ec9d. issues/the-power-offs-last-drain-gives-up-on-a-wire-whose-holder-cannot-run.md files what the review of #767 read in `serial::flush_final`: read from the code, not run. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01RvnWQFcMuGqTHYhvSnTe8A --- ...es-up-on-a-wire-whose-holder-cannot-run.md | 54 +++++++++++++++++++ tests/checks.rs | 2 +- 2 files changed, 55 insertions(+), 1 deletion(-) create mode 100644 issues/the-power-offs-last-drain-gives-up-on-a-wire-whose-holder-cannot-run.md diff --git a/issues/the-power-offs-last-drain-gives-up-on-a-wire-whose-holder-cannot-run.md b/issues/the-power-offs-last-drain-gives-up-on-a-wire-whose-holder-cannot-run.md new file mode 100644 index 00000000000..91c62ce54a1 --- /dev/null +++ b/issues/the-power-offs-last-drain-gives-up-on-a-wire-whose-holder-cannot-run.md @@ -0,0 +1,54 @@ +--- +status: open +kind: defect +opened: 2026-10-08 +--- + +# The power-off's last drain counts iterations on a wire whose holder may be unable to run, and then powers off without the tail + +Read from the code at `d78350c71`, not run. Named by the review of pull +request #767 (its comment 6058801505); every line below was read again here. + +`serial::flush_final` (`kernel/src/drivers/serial.rs:263`) is the last drain +before `power::shutdown` and `power::reboot` end the machine +(`kernel/src/power.rs:21`, `:53`). It asks `try_wire` up to +`PANIC_LOCK_SPIN_LIMIT` times (`serial.rs:131`, `serial_lock::within`) and, if +the wire never came free, appends one line to the black box and returns: the +machine powers off with its last records undrained. + +The wire is a `SleepLock` its holder keeps "with interrupts on and preemption +allowed" (`serial.rs:277`), and `klogd` takes it once per pass +(`kernel/src/log/console.rs`, `body`: `serial::wire(&parkable)`). On the way +to the power-off, `quiesce` (`kernel/src/syscall/machine.rs`) logs the boot's +last word and calls `console::drain_inline` (`:112`), which declines a held +wire (`console.rs:90`: `let Some(wire) = serial::try_wire() else { return }`), +and then `xhci::seal_shut` (`:132`), which, where it gets the controller lock, +forgets the guard: "Preemption stays disabled with it" +(`kernel/src/drivers/xhci/mod.rs:1561`). + +So on one CPU, a `klogd` descheduled inside its hold when the stop's thread +passes `drain_inline` is never run again: the stop's thread has preemption off +from the seal to the power-off, `flush_final`'s count is a wait on a holder +that cannot run, and it expires. `Shutting down.` and every line logged after +it are then on no console. On more than one CPU the holder runs elsewhere and +lets go. + +**Not established**: how `klogd` comes to be descheduled inside its hold at +that point. `drain_for_the_stop` (`machine.rs:95`) parks on the wire and so +finds it free once, but the two censuses, the stop's record and the last +word are logged after it and before `drain_inline`, and a record's commit +wakes `klogd`. No run shows the loss. `machine_shutdown_short_stop` +waits on `Shutting down.` on one CPU through this drain, so its history on +`main` is the base rate. + +## Owner + +The orchestrator. + +## Exit condition + +`flush_final` waits on the wire's release where its holder can still run, or +the stop keeps the holder from being left inside its hold; and a one-CPU guest +test, whose actuator leaves `klogd` descheduled inside its hold of the wire as +the stop's thread passes `drain_inline`, reads `Shutting down.` on the console. +That test is red on a kernel whose last drain is today's. diff --git a/tests/checks.rs b/tests/checks.rs index 47234a677b9..cc7e62f99a2 100644 --- a/tests/checks.rs +++ b/tests/checks.rs @@ -1135,7 +1135,7 @@ mod checks { #[test] fn the_c_corpus_stages_the_expectation_the_host_compares() { let got = "12\n34\n12\n34\n56\n78\n~fred()"; - let boot = c_corpus_metal(&[("03_struct".to_string(), Vec::new())], |_| true); + let boot = c_corpus_metal(&[("03_struct".to_string(), Vec::new())]); let staged = boot .files .iter() From 6e3cb894f0ef7f9e7bcfbcb99377fc6a30bca0f0 Mon Sep 17 00:00:00 2001 From: japabu Date: Thu, 8 Oct 2026 13:51:03 +0200 Subject: [PATCH 3/4] Answer review round 1: the window's check and the dead guard go, and the filed reading turns on the seal - tests/checks/qemu.rs: step 6 of ceiling_self_check is deleted. It tested that `window` keeps the tail under its count, which a reader of the eight moved lines checks, and it held the tail's wording a second time. `window` has no caller outside its module again and is private. - tests/common/metal.rs: `run`'s "nothing to run" guard is deleted: `select` refuses every word that takes nothing and no word takes the whole profile, so nothing reaches it. `select`'s doc no longer promises a refusal of a boot taken whole beside a member that lands on its label: `batches` refuses it, naming the label and neither word. - src/testargs.rs: `parse`'s header said every refusal costs only a message. Under --metal a word that takes nothing is refused by `select`, after the shared binaries are built; the header says so. - The filed reading: preemption goes off where `seal_shut` takes the controller lock, not at `drain_inline`, and nothing keeps klogd from running between the two. The claim, the unknown and the exit's actuator are restated on the seal, and four citations are corrected to the lines they quote. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01RvnWQFcMuGqTHYhvSnTe8A --- ...es-up-on-a-wire-whose-holder-cannot-run.md | 62 +++++++++++-------- src/testargs.rs | 7 ++- tests/checks/qemu.rs | 14 ----- tests/common/metal.rs | 6 -- tests/common/qemu.rs | 2 +- 5 files changed, 41 insertions(+), 50 deletions(-) diff --git a/issues/the-power-offs-last-drain-gives-up-on-a-wire-whose-holder-cannot-run.md b/issues/the-power-offs-last-drain-gives-up-on-a-wire-whose-holder-cannot-run.md index 91c62ce54a1..e812387367d 100644 --- a/issues/the-power-offs-last-drain-gives-up-on-a-wire-whose-holder-cannot-run.md +++ b/issues/the-power-offs-last-drain-gives-up-on-a-wire-whose-holder-cannot-run.md @@ -12,32 +12,39 @@ request #767 (its comment 6058801505); every line below was read again here. `serial::flush_final` (`kernel/src/drivers/serial.rs:263`) is the last drain before `power::shutdown` and `power::reboot` end the machine (`kernel/src/power.rs:21`, `:53`). It asks `try_wire` up to -`PANIC_LOCK_SPIN_LIMIT` times (`serial.rs:131`, `serial_lock::within`) and, if -the wire never came free, appends one line to the black box and returns: the -machine powers off with its last records undrained. - -The wire is a `SleepLock` its holder keeps "with interrupts on and preemption -allowed" (`serial.rs:277`), and `klogd` takes it once per pass -(`kernel/src/log/console.rs`, `body`: `serial::wire(&parkable)`). On the way -to the power-off, `quiesce` (`kernel/src/syscall/machine.rs`) logs the boot's -last word and calls `console::drain_inline` (`:112`), which declines a held -wire (`console.rs:90`: `let Some(wire) = serial::try_wire() else { return }`), -and then `xhci::seal_shut` (`:132`), which, where it gets the controller lock, -forgets the guard: "Preemption stays disabled with it" -(`kernel/src/drivers/xhci/mod.rs:1561`). - -So on one CPU, a `klogd` descheduled inside its hold when the stop's thread -passes `drain_inline` is never run again: the stop's thread has preemption off -from the seal to the power-off, `flush_final`'s count is a wait on a holder -that cannot run, and it expires. `Shutting down.` and every line logged after -it are then on no console. On more than one CPU the holder runs elsewhere and -lets go. +`PANIC_LOCK_SPIN_LIMIT` times (`serial.rs:131`, `:264`, `serial_lock::within`) +and, if the wire never came free, appends one line to the black box and +returns: the machine powers off with its last records undrained. + +The wire is a `SleepLock` "held with interrupts on and preemption allowed" +(`serial.rs:278-279`), and `klogd` takes it once per pass +(`kernel/src/log/console.rs:417`, `body`: `serial::wire(&parkable)`). On the +way to the power-off, `quiesce` (`kernel/src/syscall/machine.rs:69`) logs the +boot's last word (`:109`) and calls `console::drain_inline` (`:112`), which +declines a held wire (`console.rs:94`: +`let Some(wire) = serial::try_wire() else { return }`). Its last call is +`xhci::seal_shut` (`machine.rs:132`), which, where it gets the controller +lock, forgets the guard: "Preemption stays disabled with it" +(`kernel/src/drivers/xhci/mod.rs:1550`; the `forget` is `:1565`). + +The seal is the point this turns on. Up to it nothing read here takes +preemption from the stop's thread (it parks at `:94`), so a `klogd` holding +the wire when `drain_inline` declined can still run, finish its pass and let +go: nothing between `:112` and `:132` prevents that. From a seal that was +taken to the power-off the stop's thread has preemption off. So on one CPU, +a `klogd` descheduled inside its hold when `seal_shut` takes the controller +lock is never run again: `flush_final`'s count is then a wait on a +holder that cannot run, and it expires. Whatever that hold had not yet put on +the wire, and every record committed after it, is on no console. On more than +one CPU the holder runs elsewhere and lets go. Where the seal is refused +(`seal_shut`'s `None` arm) no guard is forgotten, and this reading does not +apply. **Not established**: how `klogd` comes to be descheduled inside its hold at -that point. `drain_for_the_stop` (`machine.rs:95`) parks on the wire and so +the seal. `drain_for_the_stop` (`machine.rs:94`) parks on the wire and so finds it free once, but the two censuses, the stop's record and the last -word are logged after it and before `drain_inline`, and a record's commit -wakes `klogd`. No run shows the loss. `machine_shutdown_short_stop` +word are logged after it (`:96`-`:109`) and before the seal, and a record's +commit wakes `klogd`. No run shows the loss. `machine_shutdown_short_stop` waits on `Shutting down.` on one CPU through this drain, so its history on `main` is the base rate. @@ -48,7 +55,8 @@ The orchestrator. ## Exit condition `flush_final` waits on the wire's release where its holder can still run, or -the stop keeps the holder from being left inside its hold; and a one-CPU guest -test, whose actuator leaves `klogd` descheduled inside its hold of the wire as -the stop's thread passes `drain_inline`, reads `Shutting down.` on the console. -That test is red on a kernel whose last drain is today's. +the stop keeps the holder from being left inside its hold at the seal; and a +one-CPU guest test, whose actuator leaves `klogd` descheduled inside its hold +of the wire as the stop's thread takes the seal in `seal_shut`, reads +`Shutting down.` on the console. That test is red on a kernel whose last +drain is today's. diff --git a/src/testargs.rs b/src/testargs.rs index 8f346b2ce81..e1903aec0b8 100644 --- a/src/testargs.rs +++ b/src/testargs.rs @@ -42,8 +42,11 @@ pub struct Parsed<'a> { /// Validate the harness's argv and return the run's filters and metal mode. /// /// `Err` is a refusal to print and exit on. It is asked before the sysroot lock -/// and before anything is compiled, so a stale command line costs a message -/// rather than a queue behind it. +/// and before anything is compiled, so a line this refuses costs a message +/// rather than a queue behind it. Not every dead word is refused here: under +/// `--metal`, whether a filter or a [`BOOT`] word takes anything is known only +/// against the profile's built members, so that refusal comes after the shared +/// binaries' build. pub fn parse(args: &[String]) -> Result, String> { let line = SUITE.walk(args); if let Some(word) = line.unknown { diff --git a/tests/checks/qemu.rs b/tests/checks/qemu.rs index 5299b29a37e..0096b197cee 100644 --- a/tests/checks/qemu.rs +++ b/tests/checks/qemu.rs @@ -288,20 +288,6 @@ pub fn ceiling_self_check() -> Result<(), String> { )); } - // 6. **The window is the tail, and it says what it cut.** A stalled wait's - // verdict and the one above are both this, over a capture of any length. - let long: String = (1..=45).map(|n| format!("line {n}\n")).collect(); - let cut = window(&long); - if cut.lines().next() != Some("(the last 40 of the 45 lines in it)") - || cut.lines().nth(1) != Some("line 6") - || cut.lines().last() != Some("line 45") - { - return Err(format!("45 lines were not cut to their last 40 under the count:\n{cut}")); - } - if window("one\ntwo\n") != "one\ntwo" { - return Err(format!("a capture that fits was cut or counted: {:?}", window("one\ntwo\n"))); - } - eprintln!( " [ceiling] the panic, the stall, the slow test and the healthy run, each named apart \ from the other three; the panic's verdict carries the kernel's own {} lines, a test \ diff --git a/tests/common/metal.rs b/tests/common/metal.rs index 1217c55b1de..c4862013199 100644 --- a/tests/common/metal.rs +++ b/tests/common/metal.rs @@ -176,8 +176,6 @@ pub type Taken = (Vec<(&'static str, &'static Metal)>, Vec); /// word at all takes everything. /// /// **A word that takes nothing is refused**: it would be dropped in silence. -/// A boot taken whole and a member taken by name that would ride a boot of -/// the same name are two images under one name, which [`batches`] refuses. pub fn select( names: &[&str], boots: &[&str], @@ -969,10 +967,6 @@ pub fn run( shared.iter().map(|b| b.jobs.len()).sum::(), batches.len(), ); - if runs.is_empty() && shared.is_empty() { - eprintln!("[metal] nothing to run"); - return Verdict::Red; - } let (dir, offline): (PathBuf, bool) = match &mode { MetalMode::List => { diff --git a/tests/common/qemu.rs b/tests/common/qemu.rs index 4177def44bf..cb7a278d3c8 100644 --- a/tests/common/qemu.rs +++ b/tests/common/qemu.rs @@ -403,7 +403,7 @@ pub const NEVER_ANNOUNCED: &str = const WINDOW_LINES: usize = 40; /// The last [`WINDOW_LINES`] lines of `said`, under the count of what was cut. -pub fn window(said: &str) -> String { +fn window(said: &str) -> String { let lines: Vec<&str> = said.lines().collect(); let kept = lines.len().min(WINDOW_LINES); let head = if lines.len() > kept { From e9185c18f840c6ad588b10684c0bbaba9ddfaf28 Mon Sep 17 00:00:00 2001 From: japabu Date: Thu, 8 Oct 2026 14:43:25 +0200 Subject: [PATCH 4/4] acpi_mediated_access's wait leaves its stall to the harness #767 gave this wait a capture of its own, from the boot log's end. That is where await_guest's window starts, so with the harness-wide tail a stalled wait here said its lines twice. The local one goes; the wait is a bare `?` again. acpi_lock_given_back_on_one_cpu shares the function. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01RvnWQFcMuGqTHYhvSnTe8A --- tests/toyos.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/toyos.rs b/tests/toyos.rs index e0f319f3c88..c400212a8bb 100644 --- a/tests/toyos.rs +++ b/tests/toyos.rs @@ -1755,9 +1755,7 @@ fn acpi_mediated_access(cpus: u32) -> Result<(), String> { let ended = format!("===TEST_END {JOB} "); await_guest(&mut qemu, &mut console, "the probe's power-off to give the lock back", |said| { said.contains(GIVEN_BACK_AT_THE_STOP) || said.contains(&ended) - }) - // The probe's whole run: a wait that ends on neither line has no other account of it. - .map_err(|why| format!("{why}\nsince its boot the guest said:\n{}", &console[qemu.boot_log().len()..]))?; + })?; let said = serial::Serial::named("the probe's boot", console); said.must_be_clean()?; said.must_say(isa::WITHHELD)?;