From c1318c58a39c9976d75fd6345cbc00d58954b70c Mon Sep 17 00:00:00 2001 From: japabu Date: Sat, 10 Oct 2026 17:40:02 +0200 Subject: [PATCH 1/2] Delete virtio_sound_counts: its count is set by host timing The owner's ruling: if the host is under load, no guest test may be red; timings are valid only on metal; this test is invalid. Under host load the test went red: soundserver submitted 62 periods for a client that filled 64, after two late wakes of up to 17 ms. The assertion `submitted >= filled` compares two numbers that both depend on scheduling. One is the periods soundserver mixed as silence before the client delivered and on wakes it found the client's ring empty. The other is the periods still in the client's ring when it closed, which the ramp-out does not play. Neither is a property of the code. Deleted: the guest binary, its `RUST_SKIP` and `MACHINE_TESTS` entries, its `run_machine_test` arm and its harness function. The device shape stays, because `iommu_virtio_platform` uses it too. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017cSFvbD35xJ2kGANVdm23C --- .../src/bin/virtio_sound_counts.rs | 134 ------------------ tests/toyos.rs | 72 ---------- 2 files changed, 206 deletions(-) delete mode 100644 tests/toyos-rust-tests/src/bin/virtio_sound_counts.rs diff --git a/tests/toyos-rust-tests/src/bin/virtio_sound_counts.rs b/tests/toyos-rust-tests/src/bin/virtio_sound_counts.rs deleted file mode 100644 index d0072d7d52e..00000000000 --- a/tests/toyos-rust-tests/src/bin/virtio_sound_counts.rs +++ /dev/null @@ -1,134 +0,0 @@ -//! One stream through soundserver's own virtio-sound driver, judged by what -//! soundserver counted and in what state it left the device — never by how long -//! anything took. -//! -//! soundserver suspends only once every period it submitted has come back from -//! the device, so a stream that ends with soundserver suspended is one whose -//! every submitted period completed; and the periods it submitted cover at -//! least the periods this client filled. With no client its wait has no -//! timeout, so the suspend is reached only through the claim's interrupts: a -//! transmit queue with no vector leaves it `running`. - -use std::sync::mpsc; -use std::time::{Duration, Instant}; - -use cpal::traits::{DeviceTrait, HostTrait, StreamTrait}; -use toyos_inspect::{Value, SOUND}; - -/// Periods of tone the client fills: a few laps of the eight-period pipeline. -const PERIODS: u64 = 64; - -/// A hang ceiling on each wait here, none of which is more than a second of -/// audio away. -const WITHIN: Duration = Duration::from_secs(30); - -const FREQ_HZ: f64 = 440.0; -const AMPLITUDE: f64 = 16000.0; - -/// What soundserver's `inspect` answer says of its device and stream. -struct Sound { - device: String, - state: String, - clients: u64, - submitted: u64, - period_frames: u64, - rate: u64, -} - -fn sound() -> Sound { - let answer = inspect::ask(SOUND).unwrap_or_else(|why| panic!("soundserver's inspect answer: {why}")); - let text = |path: &str| match answer.get(path) { - Some(Value::Text(text)) => text.clone(), - other => panic!("soundserver's snapshot has no text at {path}: {other:?}"), - }; - let number = |path: &str| match answer.get(path) { - Some(&Value::U64(n)) => n, - other => panic!("soundserver's snapshot has no number at {path}: {other:?}"), - }; - Sound { - device: text("sound.device"), - state: text("sound.stream.state"), - clients: number("sound.stream.clients"), - submitted: number("sound.periods.submitted"), - period_frames: number("sound.period_frames"), - rate: number("sound.rate_hz"), - } -} - -fn main() { - let before = sound(); - assert_eq!(before.device, "virtio-sound", "soundserver drives no virtio-sound device"); - assert_eq!( - (before.state.as_str(), before.clients), - ("suspended", 0), - "soundserver had a stream before this client's: no count here is this client's alone" - ); - - let filled = play(PERIODS * before.period_frames); - - // soundserver says nothing to a client that left; its answer is where the - // suspend is read, and the device playing its tail out wakes it once a - // period. - let deadline = Instant::now() + WITHIN; - let after = loop { - let now = sound(); - if now.state == "suspended" && now.clients == 0 { - break now; - } - assert!( - Instant::now() < deadline, - "soundserver's stream still reads `{}` with {} client(s) {WITHIN:?} after its only client \ - closed: a submitted period never came back", - now.state, - now.clients - ); - std::thread::sleep(Duration::from_nanos(1_000_000_000 * now.period_frames / now.rate)); - }; - - let submitted = after.submitted - before.submitted; - let covered = filled / before.period_frames; - assert!( - submitted >= covered, - "soundserver submitted {submitted} period(s) for a client that filled {covered}" - ); - println!( - "virtio_sound_counts: {submitted} period(s) submitted for {covered} filled, every one \ - completed before the stream stopped" - ); -} - -/// Play a tone until `frames` have been filled, and close the stream: the -/// frames filled. -fn play(frames: u64) -> u64 { - let host = cpal::default_host(); - let device = host.default_output_device().expect("no audio output device"); - let config = device.default_output_config().expect("no audio config"); - let sample_rate = config.sample_rate() as f64; - let channels = config.channels() as usize; - let (done, filled) = mpsc::channel(); - let mut n: u64 = 0; - let stream = device - .build_output_stream( - config.into(), - move |data: &mut [i16], _: &cpal::OutputCallbackInfo| { - for frame in data.chunks_exact_mut(channels) { - let phase = 2.0 * std::f64::consts::PI * FREQ_HZ * n as f64 / sample_rate; - frame.fill((AMPLITUDE * phase.sin()) as i16); - n += 1; - } - if n >= frames { - // The receiver is gone once one has been taken. - let _ = done.send(n); - } - }, - |err| panic!("the stream reported an error: {err}"), - None, - ) - .expect("failed to build audio stream"); - stream.play().expect("failed to play"); - let filled = filled - .recv_timeout(WITHIN) - .unwrap_or_else(|why| panic!("{frames} frames were not filled within {WITHIN:?}: {why}")); - drop(stream); - filled -} diff --git a/tests/toyos.rs b/tests/toyos.rs index b41d46a5359..13d6ae93c3b 100644 --- a/tests/toyos.rs +++ b/tests/toyos.rs @@ -186,10 +186,6 @@ const RUST_SKIP: &[&str] = &[ // It claims QEMU's virtio NIC, which the T14 has none of: `bar_map_again` // runs it. "bar_map_again", - // It plays through soundserver's virtio-sound driver, which the T14 has no - // device for, and its counts are judged beside soundserver's lines: - // `virtio_sound_counts` runs it. - "virtio_sound_counts", // It claims the NVMe controller a boot off that disk starts no server // for, and reads the inventory: `block_grants_reach_their_partitions` // runs it on tests/blockgrantcase. @@ -384,10 +380,6 @@ 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", - // A stream through soundserver's own virtio-sound driver, counted and - // ordered: the device exists only on a QEMU machine — the T14's sound is - // HDA — and its audio goes nowhere, since no guest test plays any. - "virtio_sound_counts", // `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", @@ -3758,7 +3750,6 @@ fn run_machine_test(name: &str, test_config: &Path) -> Result<(), String> { "machine_shutdown_wire_kept" => power::machine_shutdown_wire_held(test_config, true), "machine_shutdown_wire_at_the_seal" => power::machine_shutdown_wire_at_the_seal(test_config), "bar_map_again" => bar_map_again(test_config), - "virtio_sound_counts" => virtio_sound_counts(test_config), "console_image_boots" => console_image_boots(), "nvme_disk_keeps_log_and_home" => nvme_disk_keeps_log_and_home(test_config), "block_grants_reach_their_partitions" => block_grants_reach_their_partitions(), @@ -4236,69 +4227,6 @@ fn bar_map_again(test_config: &Path) -> Result<(), String> { Ok(()) } -/// soundserver brings the virtio-sound function up itself, behind the unit, and -/// plays one stream through it to the end: the job's counts (`inspect`'s), and -/// soundserver's lines in their order — the stream started once after the -/// client connected, and stopped once, before soundserver suspended, with no -/// refusal on the way. -fn virtio_sound_counts(test_config: &Path) -> Result<(), String> { - const JOB: &str = "virtio_sound_counts"; - /// soundserver's bring-up, which it says before any client is accepted. - const BROUGHT_UP: [&str; 3] = [ - "virtio-sound: configured stream 0: 44100Hz 2ch s16le", - "soundserver: ready, 8 buffers, 44100Hz 2ch, 512 bytes/period, 128 frames/period", - "soundserver: suspended", - ]; - /// The stream's lines, in the order they are owed, each once. - const IN_ORDER: [&str; 6] = [ - " connected (id=", - "soundserver: resumed", - "virtio-sound: stream 0 started", - " removed (", - "virtio-sound: stream 0 stopped", - "soundserver: suspended", - ]; - let bin = qemu::build_toyos_bin(qemu::SUITE_ARCH, &compile::repo_root().join("tests/toyos-rust-tests"), JOB); - let mut qemu = - QemuInstance::boot_with_options(test_config, &[], &[(JOB.to_string(), bin)], BootOptions::default()); - let mut console = qemu.boot_log().to_string(); - await_guest(&mut qemu, &mut console, "soundserver's bring-up", |c| BROUGHT_UP.iter().all(|l| c.contains(l))) - .map_err(|e| format!("{e}\n{console}"))?; - let boot = serial::Serial::named("the boot", console); - boot.must_be_clean()?; - boot.must_say("soundserver: VirtIO: PCI ")?; - boot.must_say("access_platform=y")?; - boot.must_not_say(audio::NULL_SINK)?; - - let result = qemu.run_test(&format!("test_rs_{JOB}"), Duration::from_secs(120)); - if let Some(why) = &result.error { - return Err(format!("{why}\nthe job said:\n{}", result.stdout)); - } - if result.exit_code != Some(0) { - return Err(format!("the job ended {:?}:\n{}", result.exit_code, result.stdout)); - } - let said = &result.stdout; - let Some(counted) = said.lines().find(|l| l.contains("virtio_sound_counts: ")) else { - return Err(format!("the job never said what it counted:\n{said}")); - }; - if said.contains("cannot be driven on") { - return Err(format!("soundserver refused its device mid-stream:\n{said}")); - } - let mut from = 0; - for line in IN_ORDER { - let times = said.matches(line).count(); - let Some(at) = said[from..].find(line) else { - return Err(format!("`{line}` is not after the line before it in the window:\n{said}")); - }; - if times != 1 { - return Err(format!("`{line}` said {times} times in one stream's window:\n{said}")); - } - from += at + line.len(); - } - eprintln!(" [sound] {}", counted.trim()); - Ok(()) -} - /// Every device delivery is still cpu0's, and no CPU took a shootdown IPI the /// issuer did not count. /// From e03a5a00d3f35135378b6f0011a627890c959a61 Mon Sep 17 00:00:00 2001 From: japabu Date: Sat, 10 Oct 2026 18:11:52 +0200 Subject: [PATCH 2/2] soundserver's virtio-sound stream is played by no guest test: file it Deleting virtio_sound_counts for its host-timed count also took away the three things it asserted that host timing does not set: the order of the stream's lines, suspension only after every submitted period completes, and suspension reached through the claim's interrupts. The issue records that gap with an owner and an exit a QEMU test asserting only order and completion can meet. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017cSFvbD35xJ2kGANVdm23C --- ...sound-stream-is-played-by-no-guest-test.md | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 issues/soundservers-virtio-sound-stream-is-played-by-no-guest-test.md diff --git a/issues/soundservers-virtio-sound-stream-is-played-by-no-guest-test.md b/issues/soundservers-virtio-sound-stream-is-played-by-no-guest-test.md new file mode 100644 index 00000000000..90c559ba7d9 --- /dev/null +++ b/issues/soundservers-virtio-sound-stream-is-played-by-no-guest-test.md @@ -0,0 +1,30 @@ +--- +status: open +kind: tooling +opened: 2026-10-10 +--- + +# soundserver's virtio-sound stream is played by no guest test + +No guest test plays a stream through soundserver's own virtio-sound driver +(`userland/soundserver/src/virtio.rs`, driven by the device loop in +`userland/soundserver/src/mix.rs`). `iommu_virtio_platform` reaches its +bring-up and stops there; `toyos-virtio-sound`'s host tests reach the driver +crate, not the interrupt-driven loop. Three behaviours of that path go unseen: + +- the order of its lines: `virtio-sound: stream 0 started` once, after the + client connects, then `virtio-sound: stream 0 stopped` once, then + `soundserver: suspended`; +- the suspend comes only once every submitted period has completed + (`unplayed == 0` in the suspend block of `mix.rs`); +- the suspend is reached through the claim's interrupts: with no client the + loop's wait has no timeout, so a transmit queue with no vector leaves the + stream `running`. + +`virtio_sound_counts` asserted all three, beside a count of submitted against +filled periods that host scheduling sets; it was deleted whole for the count. + +Owner: the orchestrator, which dispatches it. Exit: a QEMU guest test plays a +stream through soundserver's virtio-sound driver and asserts only the three +behaviours above, by order and completion, never a count or a time beyond a +hang ceiling; and the test reds when the transmit queue's vector is withheld.