diff --git a/CHANGELOG.md b/CHANGELOG.md index 4951071a..15da6df5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,6 +48,19 @@ hashes are given where a change is easiest to understand by reading the commit. (reset values, visible `GFIFOLEVEL` only for the last 32 slots, no FIFO threshold interrupt), are now pinned by tests and recorded in `docs/Indy/rex3.md`. +### Monitor and CI + +- **`machine-start` is idempotent and acknowledges the transition, and + `machine-status` reports guest state and monitor readiness.** The monitor + command now prints `idle -> starting -> running` (or `running (no change)`), + `machine-stop` acknowledges symmetrically, and `machine-status [json]` gives + a caller one place to ask whether the guest is `idle`, `starting`, `running` + or `halted` and whether the monitor has bound its port. The CI socket gains + the same `status` command, `iris-ci status [--wait-ready SECS]` polls it + instead of sleeping, and `--ci` creates the control socket only after + `Machine::start` has bound the monitor, so a successful connection is + itself a readiness signal. Tests cover a command sent before the monitor is + ready, duplicate starts, start after halt/reset, and invalid transitions. ### NAT gateway diff --git a/Cargo.toml b/Cargo.toml index 138c184b..cd4290f5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -343,6 +343,12 @@ windows-sys = { version = "0.61", features = ["Win32_Foundation", "Win32_System_ [target.'cfg(not(windows))'.dependencies] libc = "0.2" +[dev-dependencies] +# Integration tests (`tests/`) are separate crates and do not inherit the +# package's normal dependencies; serde_json is needed to parse the monitor's +# and CI socket's JSON replies. +serde_json = "1.0" + [profile.release] lto = "fat" codegen-units = 1 diff --git a/HACKING.md b/HACKING.md index bd6d46dc..0c29c035 100644 --- a/HACKING.md +++ b/HACKING.md @@ -53,11 +53,15 @@ The front ends do not use `Machine` directly. `main.rs`, the window iris-gui hold a `Box` from `workstation::build` (`src/workstation.rs`). The trait's required methods are what every machine has (start and stop, the screen, the keyboard and mouse, the serial console, -the monitor); its provided methods are optional facilities (snapshots, CHD -folding, the scratch volume, disc changes, the NAT controls) that default to -"not supported on this machine". `Machine` implements all of it. A machine -built around a different chipset implements the same trait and gets the -window, iris-ci, iris-gui and the monitor's `reset`/`save`/`load` with it. +the monitor, the stored lifecycle state); its provided methods are either +shared logic every machine wants the same way (`status`, and the idempotent +`start_guest`/`stop_guest` behind `machine-start`/`machine-stop`, see +`src/machine_state.rs`) or optional facilities (snapshots, CHD folding, the +scratch volume, disc changes, the NAT controls) that default to "not +supported on this machine". `Machine` implements all of it. A machine built +around a different chipset implements the same trait and gets the window, +iris-ci, iris-gui and the monitor's `reset`/`save`/`load`/`machine-status` +with it. The monitor's system controller, the machine-events thread and the CI server reach the machine through one shared `MachineSlot` per machine, which the diff --git a/STORAGE.md b/STORAGE.md index 9c94a15e..63d72c7b 100644 --- a/STORAGE.md +++ b/STORAGE.md @@ -116,6 +116,7 @@ debugging the wrapper itself. ./target/release/iris --ci # In another terminal: drive it +./target/release/iris-ci status --wait-ready 60 # guest state + monitor readiness ./target/release/iris-ci boot # PROM menu → IRIS console login (one cmd) ./target/release/iris-ci login # send root + dismiss vt100 prompt + wait # ./target/release/iris-ci run 'ls /' # send shell command, get stdout + exit code @@ -134,6 +135,24 @@ Run `iris-ci --help` for the full list, or `iris-ci --help` for any subcommand. Every operation has a typed clap arg — no JSON quoting, no hand-managed timeouts. +### Waiting for a fresh machine + +The control socket is created only after `Machine::start` has bound the monitor, +so a successful connection (or `iris-ci ping`) also means the monitor is +listening. A harness that launches iris should poll, not sleep: +`iris-ci status --wait-ready 60` retries until `monitor_ready` is true and then +prints the guest state (`idle`, `starting`, `running`, `halted`) and the +monitor address. `--json` emits the same fields as one object. A `--ci` machine +sits in `starting` with the CPU parked until `iris-ci start` (or the monitor's +`machine-start`), whose reply acknowledges the transition; repeating it is a +no-op. See [debug.md](debug.md#machine) for `machine-status` on the monitor +side. + +`script FILE` (also spelled `batch FILE`) runs one command per line through a +single control-socket connection, so a multi-step scenario costs one connect +rather than one per line. It prints per-step status and stops at the first +failure; `--keep-going` runs the remaining steps and still exits non-zero. + For automation that doesn't want to depend on `iris-ci`, the underlying socket protocol is newline-delimited JSON; `cmd` and `args` per request, `{ok, data, error}` per response. See `src/ci.rs` for the dispatch table. diff --git a/clanker/INDEX.md b/clanker/INDEX.md index d9ea5f60..4b6cfd6c 100644 --- a/clanker/INDEX.md +++ b/clanker/INDEX.md @@ -35,6 +35,7 @@ CPU architecture: CP0, TLB and its fast paths, caches, FPU, LL/SC, CPU models. B | [instr_stats_insmap_generalization.md](cpu/instr_stats_insmap_generalization.md) | design | instr_stats generalized into a decode+exec instruction map (insmap) | | [intrfast-self-overwrite-investigation.md](cpu/intrfast-self-overwrite-investigation.md) | open | intrfast (0x08016xxx) gets overwritten with data mid-boot — investigation in progress | | [movci-is-cp1-by-behaviour-not-encoding.md](cpu/movci-is-cp1-by-behaviour-not-encoding.md) | rule | MOVF/MOVT are CP1 accesses with an integer encoding | +| [mp-llsc-is-a-host-compare-exchange.md](cpu/mp-llsc-is-a-host-compare-exchange.md) | design | MP LL/SC: SC is a host compare-exchange on the data, not a line-ownership tracker | | [nanotlb-asid-mutation-flush.md](cpu/nanotlb-asid-mutation-flush.md) | rule | nanotlb must be flushed on ASID mutation, not just on privilege transition | | [nutlb-erl-guard-defeats-permission-test.md](cpu/nutlb-erl-guard-defeats-permission-test.md) | history | nutlb: the ERL guard bit defeats the permission test entirely | | [r5000-taglo-is-the-r4400-pstate.md](cpu/r5000-taglo-is-the-r4400-pstate.md) | rule | R5000 L1 TagLo uses the R4400 PState encoding | diff --git a/clanker/cpu/mp-llsc-is-a-host-compare-exchange.md b/clanker/cpu/mp-llsc-is-a-host-compare-exchange.md new file mode 100644 index 00000000..a6aafa6b --- /dev/null +++ b/clanker/cpu/mp-llsc-is-a-host-compare-exchange.md @@ -0,0 +1,114 @@ +# MP LL/SC: SC is a host compare-exchange on the data, not a line-ownership tracker + +Status: design decision, 2026-10-23. Nothing implemented yet; this records the +reasoning so the multiprocessor (Octane) work does not re-derive it. Today +`exec_sc`/`exec_scd` in `src/cpu/mips_exec.rs` test the LLbit and then call +`write_data`, which is correct for one CPU and racy for two. + +## The decision + +When IRIS runs more than one CPU thread: + +- `LL`/`LLD` record the physical address **and the value loaded**, and set the + LLbit as today. +- `SC`/`SCD` succeed iff the LLbit is set **and** a host atomic + `compare_exchange` of the word at that physical address, from the LL'd value + to the new value, succeeds. The store happens only inside the CAS. +- Plain stores from other CPUs, and DMA, stay what they are now: aligned host + stores of exactly their own width. They need no tracking and no locking. +- Exceptions and `ERET` keep clearing the LLbit (see + `sc-succeeds-on-llbit-alone.md`); that part does not change. +- `LL`/`SC` stay `Classify::Excluded` in jitv2, so the CAS lives in one place, + the interpreter. + +This is what QEMU does for SMP MIPS, ARM and PowerPC guests. + +## Why not track cache-line ownership + +The tempting design is one atomic byte per guest cache line, every CPU access +ORs its bit in, `LL` resets the byte to the issuing CPU, `SC` checks that no +other bit appeared. It was considered and rejected for four reasons: + +1. **It is the global lock in disguise.** A locked RMW on every guest load and + store is a full fence per access on x86, and the tracking bytes false-share: + 64 guest lines' bytes sit in one host line, so two CPUs working in unrelated + 8 KB regions bounce the same host line on every access. It also puts a + store on the load path, which the inline-memory JIT and tcache fast paths + cannot absorb. +2. **Reads must not be tracked.** The architecture requires `SC` to fail only + when another processor or coherent I/O agent *stores* into the block. + Remote loads downgrade the line to shared and are allowed to leave the + LLbit alone. Tracking them only adds spurious failures. +3. **`LL` resetting the byte clobbers other CPUs' reservations.** Two CPUs + with `LL` outstanding on different words of one line (adjacent lock words, + which IRIX has) each cancel the other; both `SC`s fail, both retry, and + they livelock with no store ever happening. +4. **Check-then-store is not atomic.** Another CPU's store lands between the + check and the store and two `SC`s both report success. Checking the byte + again *after* the store does not fix it: a failed `SC` must leave memory + untouched, and by then the store has landed. Concretely: A checks (clean), + B stores, A stores 1 into a lock word, A re-checks, sees B, reports + failure. Memory holds A's 1, nobody owns the lock, everyone spins forever. + Restoring the old value destroys B's store instead. + +Validate-before-and-after works for operations whose effect can be discarded +(seqlock readers). A store cannot be discarded. The only correct form of the +tracking design is a per-line lock that every plain store from every CPU must +take, which is strictly more expensive than the CAS and buys only exact ABA. + +## Why the CAS is enough + +Hardware makes "nobody intervened" and "the store happens" one event by holding +the line exclusive across both. The host CAS is the same mechanism one level +down: the host core takes the host cache line exclusive for the duration of the +instruction, and every other emulated CPU's plain store to that word is +serialised against it by the host's own coherence protocol. The guest word is a +single aligned host `u32`/`u64` thanks to the `^3` byte and `rotate_left(32)` +qword layout in `src/dev/mem.rs`, so there is nothing to tear. + +The one deviation from hardware is **ABA**: if the word changed and changed +back between `LL` and `SC`, the CAS succeeds where hardware would fail. +Spurious *failure* is architecturally allowed (software must loop), so spurious +*success* is the only thing that can go wrong, and ABA is all of it. Every +primitive in the IRIX kernel's `atomic_ops.h` is value-based (add, compare-and- +swap, test-and-set, bit set/clear, the `k_flags` bitlock), so none of them can +observe it. Nothing in IRIX pops a lock-free stack with `LL`/`SC`. + +Width interactions are consistent with hardware: a byte store by another CPU +into a different byte of the LL'd word fails the CAS, and hardware fails that +`SC` too because the byte store is a store into the block. + +## Prerequisites that matter more than the CAS itself + +These are the real MP work; the CAS is a dozen lines. + +- **The cache model must be data-less.** `mips_cache_v2` keeps real L1D and L2 + line contents; with two CPUs that is two private copies of RAM. An MP build + runs tags-only with every access going to shared RAM. Under tcache the L2 + data is already bypassed; L1D must follow. +- **`write64_masked` is a read-modify-write** of a host qword and will lose + bytes when two CPUs hit one qword. It must become per-byte stores or a CAS + loop. `write8`/`write16`/`write32`/`write64` are already exact-width single + stores; keep them that way and audit device DMA paths for merges. +- **RAM backing must be 8-byte aligned** for `SCD`'s CAS and for torn-free + `write64`. `Vec` guarantees 4; allocators give 16 in practice; make it + explicit. +- **Memory ordering per host.** x86 TSO is stronger than MIPS, so guest `SYNC` + can be a no-op there. On aarch64 `SYNC` needs a real fence and the `SC` CAS + needs acquire-release. Pick the `Ordering` per host arch from day one. +- **Cross-CPU code invalidation.** The per-page generation counters in + `Memory` are atomic already, but a compiled page running on CPU B must see + CPU A's write before B's `CACHE` flush completes. IRIX flushes explicitly + after writing code, so a check at the flush point on the executing CPU is + enough, but it has to be cross-thread. +- **Count vs. the HEART timer.** CPU threads drift relative to each other; + per-CPU Count must stay monotonic and roughly in step + (`cpu-count-is-wallclock-derived.md`). + +## Instrument + +`--features llstats` (`ll stats`, see `sc-succeeds-on-llbit-alone.md`) already +reports per-address `sc_fail` runs. Under MP it is the first thing to look at +when a lock appears stuck: a `max_run` in the hundreds on one address means the +CAS is failing where it should not, or a plain store is reaching the word +through a path that is not width-exact. diff --git a/debug.md b/debug.md index e9f6b538..8a25ea46 100644 --- a/debug.md +++ b/debug.md @@ -81,11 +81,38 @@ accepted in every build but do nothing without the feature. | Command | Description | |---|---| -| `machine-start` / `machine-stop` | Start / stop the CPU and all peripherals | +| `machine-start` | Start the CPU and all peripherals. Idempotent: prints the transition it performed, e.g. `machine-start: halted -> starting -> running`, or `machine-start: running (no change)` | +| `machine-stop` | Stop the CPU and all peripherals. Idempotent: prints `machine-stop: running -> halted` | +| `machine-status [json]` | Guest state (`idle`, `starting`, `running`, `halted`) and whether the monitor is accepting commands; `json` emits one object | | `reset` | Reset all hardware to power-on state | | `save ` / `load ` | Save / load a snapshot under `saves//` (see [STORAGE.md](STORAGE.md#snapshots-and-rollback)) | | `testdev` | Bare-metal test device status (`cpu-tests/`, `bench/`) | +The machine lifecycle is one value, and every surface reports it the same way: +`idle` (not started; the monitor is not listening), `starting` (the monitor +accepts commands but the CPU is parked), `running` (the CPU is executing), and +`halted` (stopped by `machine-stop`, `reset` or a guest power-off). `running` is +read from the CPU's own running flag, so the `start`/`stop` execution commands +and breakpoints are reflected without a separate bookkeeping step. + +A fresh `--ci` machine is **ready for commands while still `starting`**: the +monitor is up and the peripherals are running, but the CPU stays parked until +`machine-start` (or `iris-ci start`). Do not sleep before sending it — poll +instead. Either retry the monitor TCP connect until it answers, or use the CI +socket, which iris creates only after the monitor has bound: + +```sh +iris-ci --socket /tmp/iris.sock status --wait-ready 60 +tools/mon.py "machine-start" +``` + +`machine-status` is available as soon as a monitor connection is accepted; the +`json` form is one object with `state`, `cpu_running`, `monitor_ready` and +`monitor_addr`. `iris-ci status` prints the same fields from the same source. +If the monitor could not bind its port (another iris is using it), the CI +socket still answers but reports `monitor_ready` false, so a harness can tell a +bind failure from a guest that never started. + ### Execution control | Command | Description | diff --git a/src/ci.rs b/src/ci.rs index 2c1962ed..c7c6c50b 100644 --- a/src/ci.rs +++ b/src/ci.rs @@ -285,6 +285,7 @@ fn dispatch(server: &CiServer, req: &Request) -> Response { "ping" => Response::ok(), "quit" => cmd_quit(server, &req.args), "start" => cmd_start(server), + "status" => cmd_status(server), "save" => cmd_save(server, &req.args), "restore" => cmd_restore(server, &req.args), "rollback" => cmd_rollback(server), @@ -422,8 +423,22 @@ fn cmd_quit(server: &CiServer, args: &Value) -> Response { } fn cmd_start(server: &CiServer) -> Response { - match server.with_machine(|m| m.cpu_start()) { - Ok(()) => Response::ok(), + // Same path as the monitor's `machine-start`: idempotent, and it brings + // peripherals back before releasing the CPU after a stop. A machine that + // has not finished `Machine::start` reports `idle` and the harness should + // poll `status` first; the CI socket is created after that startup step, + // so a connected client normally cannot see `idle`. + match server.with_machine_result(|m| m.start_guest()) { + Ok(_) => Response::ok(), + Err(e) => Response::err(e), + } +} + +/// Guest state and monitor readiness, from the same `Workstation::status` the +/// monitor's `machine-status` prints. +fn cmd_status(server: &CiServer) -> Response { + match server.with_machine(|m| m.status()) { + Ok(status) => Response::data(status.to_json()), Err(e) => Response::err(e), } } diff --git a/src/ip32_machine.rs b/src/ip32_machine.rs index e6456e1d..a5b2220f 100644 --- a/src/ip32_machine.rs +++ b/src/ip32_machine.rs @@ -141,6 +141,10 @@ pub struct O2Machine { disks: Vec, /// The flash file, recorded in a snapshot's manifest. flash_path: Option, + /// Where the machine is in its start/stop lifecycle (see + /// `Workstation::lifecycle`); never `Running`, which `status` reads from + /// the CPU. + lifecycle: crate::machine_state::GuestState, } /// What an O2 snapshot's manifest names as its machine. @@ -342,6 +346,7 @@ impl O2Machine { controller_slot: None, boot, paused: Arc::new(AtomicBool::new(false)), + lifecycle: crate::machine_state::GuestState::Idle, disks, flash_path: o2.flash.clone(), }) @@ -434,6 +439,11 @@ impl O2Machine { } pub fn start(&mut self) { + // Publish "starting" before the monitor binds, so a caller polling + // readiness can tell "not started" from "startup under way". + if let Ok(starting) = self.lifecycle.transition(crate::machine_state::GuestState::Starting) { + self.lifecycle = starting; + } self.running.store(true, Ordering::SeqCst); self.monitor.clone().start_server(format!("127.0.0.1:{}", self.monitor_port)); let d = self.display.clone(); @@ -521,6 +531,10 @@ impl O2Machine { eprintln!("iris: O2 stopped with rendering engine writes still queued"); } self.bus.save_flash(); + // A machine that was never started has nothing to halt. + if self.lifecycle != crate::machine_state::GuestState::Idle { + self.lifecycle = crate::machine_state::GuestState::Halted; + } } /// Let the service thread look after the devices again. @@ -762,6 +776,8 @@ impl Workstation for O2Machine { self.controller_up.store(true, Ordering::SeqCst); } fn monitor_addr(&self) -> Option { self.monitor.bound_addr() } + fn lifecycle(&self) -> crate::machine_state::GuestState { self.lifecycle } + fn set_lifecycle(&mut self, state: crate::machine_state::GuestState) { self.lifecycle = state; } fn get_display(&self) -> Option> { Some(O2Machine::get_display(self)) } fn get_ps2(&self) -> Arc { O2Machine::get_ps2(self) } diff --git a/src/iris_ci_main.rs b/src/iris_ci_main.rs index f2c9740f..437b670c 100644 --- a/src/iris_ci_main.rs +++ b/src/iris_ci_main.rs @@ -63,7 +63,15 @@ struct Cli { enum Cmd { /// Liveness check — returns "ok" if the socket is reachable. Ping, - /// Start the CPU thread (no-op if already running). + /// Show the guest lifecycle state and whether the monitor is accepting + /// commands. `--wait-ready SECS` polls until it is. + Status { + /// Poll until the monitor has bound its port, up to SECS seconds + /// (0 = one query). Exits non-zero on timeout. + #[arg(long, default_value_t = 0, value_name = "SECS")] + wait_ready: u64, + }, + /// Start the CPU and peripherals (no-op if already running). Start, /// Cleanly shut down iris. `--sync-chd [ID|all]` folds pending CHD diffs /// back into their bases first (default: all disks). @@ -268,6 +276,7 @@ fn main() { fn dispatch(opts: &Opts, cmd: Cmd) -> Result<()> { match cmd { Cmd::Ping => simple(opts, "ping", json!({}), "ok"), + Cmd::Status { wait_ready } => cmd_status(opts, wait_ready), Cmd::Start => simple(opts, "start", json!({}), "started"), Cmd::Quit { sync_chd } => match sync_chd { // A fold recompresses the whole base, so it can far outrun the 300s default. @@ -475,6 +484,66 @@ fn print_response(opts: &Opts, ok_msg: &str, data: &Value) { } } +/// `iris-ci status`: guest lifecycle state and monitor readiness from the same +/// `Machine::status` the monitor's `machine-status` prints. With +/// `--wait-ready`, keep polling (including through "socket not up yet") until +/// the monitor has bound its port, then stop — a readiness poll a harness can +/// use instead of a fixed sleep. +fn cmd_status(opts: &Opts, wait_ready: u64) -> Result<()> { + let deadline = Instant::now() + Duration::from_secs(wait_ready); + loop { + match send(opts, "status", json!({})) { + Ok(data) => { + let ready = data.get("monitor_ready").and_then(Value::as_bool).unwrap_or(false); + if ready { + print_status(opts, &data); + return Ok(()); + } + if wait_ready == 0 { + // A one-shot query reports whatever the machine says; the + // caller decides whether "not ready yet" matters. + print_status(opts, &data); + return Ok(()); + } + if Instant::now() >= deadline { + print_status(opts, &data); + let state = data.get("state").and_then(Value::as_str).unwrap_or("unknown"); + return Err(Error::Local(format!( + "monitor not ready after {wait_ready}s (state {state}); see iris stderr for a bind failure" + ))); + } + } + Err(Error::Connection(_)) if wait_ready > 0 && Instant::now() < deadline => { + // The CI socket itself may not exist yet: iris creates it only + // after the monitor has bound. + } + Err(e) => return Err(e), + } + std::thread::sleep(Duration::from_millis(50)); + } +} + +fn print_status(opts: &Opts, data: &Value) { + if opts.json { + println!("{}", serde_json::to_string_pretty(data).unwrap_or_else(|_| data.to_string())); + return; + } + if opts.quiet { return; } + let state = data.get("state").and_then(Value::as_str).unwrap_or("unknown"); + let cpu = if data.get("cpu_running").and_then(Value::as_bool).unwrap_or(false) { + "running" + } else { + "stopped" + }; + let monitor = match data.get("monitor_addr").and_then(Value::as_str) { + Some(addr) => format!("ready ({addr})"), + None => "not ready".to_string(), + }; + println!("state {state}"); + println!("cpu {cpu}"); + println!("monitor {monitor}"); +} + fn cmd_list(opts: &Opts) -> Result<()> { let data = send(opts, "list", json!({}))?; if opts.json { println!("{}", serde_json::to_string_pretty(&data).unwrap_or_default()); return Ok(()); } diff --git a/src/lib.rs b/src/lib.rs index 69023f1f..2ce83c6f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -177,6 +177,7 @@ pub mod ip32; pub mod ip32_handover; pub mod ip32_screen; pub mod ip32_machine; +pub mod machine_state; pub mod platform; pub mod physical; pub mod monitor; diff --git a/src/machine.rs b/src/machine.rs index 56e43bef..596721c2 100644 --- a/src/machine.rs +++ b/src/machine.rs @@ -7,6 +7,7 @@ use std::sync::mpsc; use std::thread; use crate::config::{GraphicsBoard, MachineConfig, MachineProfile, NetworkConfig}; +use crate::machine_state::GuestState; use crate::traits::{BusDevice, Device, Resettable, Saveable, MachineEvent}; use crate::locks::LockMonitor; use crate::dev::eeprom_93c56::Eeprom93c56; @@ -113,6 +114,11 @@ pub struct Machine { /// instead of freed memory. controller_slot: Option, controller_registered: bool, + /// Where the machine is in its start/stop lifecycle. Never `Running`: + /// `status` derives that from the CPU's own running flag, so an + /// execution-control command cannot leave it stale. See + /// [`crate::machine_state`]. + lifecycle: GuestState, } /// In-memory snapshot of the just-restored guest state. Populated at the end @@ -991,6 +997,7 @@ impl Machine { fullhouse: !guinness, controller_slot: None, controller_registered: false, + lifecycle: GuestState::Idle, } } @@ -1026,6 +1033,13 @@ impl Machine { } pub fn start(&mut self) { + // Publish "starting" before the monitor binds, so a caller that polls + // readiness can tell "not started" (Idle) from "startup under way". + // Halted -> Starting covers the GUI's Stop then Start; a stored + // Running state is impossible (status derives it from the CPU). + if let Ok(starting) = self.lifecycle.transition(GuestState::Starting) { + self.lifecycle = starting; + } // Start peripherals self.mc.start(); self.hpc3.start(); @@ -1085,6 +1099,11 @@ impl Machine { self.hpc3.stop(); self.mc.stop(); if let Some(u64) = &self._phys.ultra64 { u64.stop(); } + // Idle -> Halted is not a lifecycle transition: a machine that was + // never started has nothing to halt, and the monitor is not up to ask. + if self.lifecycle != GuestState::Idle { + self.lifecycle = GuestState::Halted; + } } /// The address this Machine's own monitor is serving on, or None when it @@ -2070,6 +2089,8 @@ impl Workstation for Machine { fn machine_slot(&mut self) -> MachineSlot { Machine::machine_slot(self) } fn register_system_controller(&mut self) { Machine::register_system_controller(self) } fn monitor_addr(&self) -> Option { Machine::monitor_addr(self) } + fn lifecycle(&self) -> GuestState { self.lifecycle } + fn set_lifecycle(&mut self, state: GuestState) { self.lifecycle = state; } fn get_display(&self) -> Option> { Machine::get_display(self) } fn get_display_head1(&self) -> Option> { Machine::get_display_head1(self) } @@ -2129,6 +2150,36 @@ mod controller_lifetime_tests { .unwrap() } + /// #102: `machine-start`'s acknowledgement names the transition it + /// performed, from a never-started machine through `Starting` to + /// `Running`. A second start is a no-op, and a start after `machine-stop` + /// takes the same route from `Halted`. The process-level test in + /// `tests/monitor_ready.rs` covers the monitor command and readiness. + #[test] + fn start_guest_acknowledges_the_lifecycle_transition() { + let mut machine = build_machine(); + assert_eq!(machine.status().state, GuestState::Idle); + assert!(!machine.status().monitor_ready(), "no monitor before Machine::start"); + + let ack = machine.start_guest().expect("start from idle"); + assert_eq!(ack.text(), "idle -> starting -> running"); + assert_eq!(machine.status().state, GuestState::Running); + assert!(machine.status().cpu_running); + + // Duplicate start: no second start and no error. + let ack = machine.start_guest().expect("duplicate start"); + assert!(!ack.changed()); + assert_eq!(ack.text(), "running (no change)"); + + let ack = machine.stop_guest(); + assert_eq!(ack.text(), "running -> halted"); + assert_eq!(machine.status().state, GuestState::Halted); + + let ack = machine.start_guest().expect("start after halt"); + assert_eq!(ack.text(), "halted -> starting -> running"); + machine.stop(); + } + /// iris-gui drops a Machine on Stop while the monitor keeps its /// SystemController. Every command through that controller afterwards used /// to dereference freed memory (SIGSEGV in `save_snapshot`). diff --git a/src/machine_state.rs b/src/machine_state.rs new file mode 100644 index 00000000..3695f33c --- /dev/null +++ b/src/machine_state.rs @@ -0,0 +1,332 @@ +//! Guest lifecycle state and monitor readiness. +//! +//! One value answers "what is the guest doing, and can I talk to it?" for the +//! monitor's `machine-start` acknowledgement, `machine-status`, and the CI +//! socket's `status` command. Keeping the state and its transitions here means +//! those three surfaces cannot report different things. + +use std::fmt; +use std::net::SocketAddr; + +/// Where the guest is in its start/stop lifecycle. +/// +/// `Running` is derived from the CPU's own running flag, not stored: an +/// execution-control command (`start`, `stop`, a breakpoint) changes it without +/// going through a machine-level transition, and reporting a stale copy would +/// be worse than storing nothing at all. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum GuestState { + /// The machine has not been started: its peripherals are down and the + /// monitor is not accepting commands. + Idle, + /// The machine is up — peripherals started and the monitor accepting + /// commands — but the CPU is parked, waiting for `machine-start` or + /// `start`. This is the state a `--ci` machine sits in until its harness + /// releases the CPU. + Starting, + /// The CPU is executing. + Running, + /// The CPU was stopped by `machine-stop`, `reset`, or a guest power-off. + /// The monitor still accepts commands. + Halted, +} + +impl GuestState { + pub fn as_str(self) -> &'static str { + match self { + GuestState::Idle => "idle", + GuestState::Starting => "starting", + GuestState::Running => "running", + GuestState::Halted => "halted", + } + } + + /// Move to `to`, refusing transitions the lifecycle does not have. + /// + /// Identical states are allowed so callers can apply an operation + /// idempotently. The machine-level path is `Idle`/`Halted` -> `Starting` + /// -> `Running`; reaching `Running` directly from `Idle` or `Halted` is a + /// bug (it would claim the guest ran without a start), and a running guest + /// cannot move back to `Starting` without being halted first. + pub fn transition(self, to: GuestState) -> Result { + use GuestState::*; + let allowed = self == to + || matches!( + (self, to), + (Idle, Starting) + | (Halted, Starting) + | (Starting, Running) + | (Starting, Halted) + | (Running, Halted) + ); + if allowed { + Ok(to) + } else { + Err(TransitionError { from: self, to }) + } + } +} + +impl fmt::Display for GuestState { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(self.as_str()) + } +} + +/// A refused [`GuestState::transition`]. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub struct TransitionError { + pub from: GuestState, + pub to: GuestState, +} + +impl fmt::Display for TransitionError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!( + f, + "invalid guest state transition: {} -> {}", + self.from.as_str(), + self.to.as_str() + ) + } +} + +impl std::error::Error for TransitionError {} + +/// What a machine-level start or stop did, for the command that asked. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub struct TransitionAck { + pub from: GuestState, + /// An intermediate state the operation passed through, when it is worth + /// showing. A start from `Idle` or `Halted` goes through `Starting`; a + /// start from an already-`Starting` machine does not. + pub via: Option, + pub to: GuestState, +} + +impl TransitionAck { + pub fn changed(&self) -> bool { + self.from != self.to + } + + /// One line for a monitor client: `halted -> starting -> running`, or + /// `running (no change)` when the operation was a no-op. + pub fn text(&self) -> String { + if !self.changed() { + format!("{} (no change)", self.to.as_str()) + } else if let Some(via) = self.via { + format!("{} -> {} -> {}", self.from.as_str(), via.as_str(), self.to.as_str()) + } else { + format!("{} -> {}", self.from.as_str(), self.to.as_str()) + } + } + + /// The same information for a JSON client. + pub fn to_json(&self) -> serde_json::Value { + serde_json::json!({ + "from": self.from.as_str(), + "via": self.via.map(GuestState::as_str), + "to": self.to.as_str(), + "changed": self.changed(), + }) + } +} + +/// A snapshot of the guest's state plus whether the monitor is accepting +/// commands, produced only by `Workstation::status`. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub struct MachineStatus { + pub state: GuestState, + /// True while the CPU thread is executing. Always `state == Running`. + pub cpu_running: bool, + /// The address the monitor bound, or `None` until it is listening (or + /// when it could not bind and is disabled for this run). + pub monitor_addr: Option, +} + +impl MachineStatus { + pub fn monitor_ready(&self) -> bool { + self.monitor_addr.is_some() + } + + /// Three lines a person can read and a shell can grep. + pub fn text(&self) -> String { + let monitor = match self.monitor_addr { + Some(addr) => format!("ready ({addr})"), + None => "not ready".to_string(), + }; + format!( + "state: {}\ncpu: {}\nmonitor: {}", + self.state.as_str(), + if self.cpu_running { "running" } else { "stopped" }, + monitor + ) + } + + pub fn to_json(&self) -> serde_json::Value { + serde_json::json!({ + "state": self.state.as_str(), + "cpu_running": self.cpu_running, + "monitor_ready": self.monitor_ready(), + "monitor_addr": self.monitor_addr.map(|a| a.to_string()), + }) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn addr() -> SocketAddr { + "127.0.0.1:8888".parse().unwrap() + } + + /// The normal start path names all three states in the acknowledgement. + #[test] + fn start_from_idle_goes_through_starting_to_running() { + let starting = GuestState::Idle.transition(GuestState::Starting).unwrap(); + let running = starting.transition(GuestState::Running).unwrap(); + assert_eq!(running, GuestState::Running); + + let ack = TransitionAck { + from: GuestState::Idle, + via: Some(GuestState::Starting), + to: GuestState::Running, + }; + assert_eq!(ack.text(), "idle -> starting -> running"); + assert!(ack.changed()); + } + + /// A second start on a running guest is a no-op, not an error and not a + /// second `Starting` transition. + #[test] + fn duplicate_start_while_running_is_a_no_op() { + let state = GuestState::Running.transition(GuestState::Running).unwrap(); + assert_eq!(state, GuestState::Running); + + let ack = TransitionAck { + from: GuestState::Running, + via: None, + to: GuestState::Running, + }; + assert!(!ack.changed()); + assert_eq!(ack.text(), "running (no change)"); + } + + /// Both a halt and a reset leave the machine at `Halted`, and a start from + /// there is the same `halted -> starting -> running` path as an idle + /// start. + #[test] + fn start_after_halt_and_after_reset() { + let halted = GuestState::Running.transition(GuestState::Halted).unwrap(); + assert_eq!(halted, GuestState::Halted); + assert_eq!( + halted.transition(GuestState::Starting).unwrap().transition(GuestState::Running).unwrap(), + GuestState::Running + ); + + // A halted machine stopped again stays halted. + assert_eq!(halted.transition(GuestState::Halted).unwrap(), GuestState::Halted); + let ack = TransitionAck { + from: GuestState::Halted, + via: Some(GuestState::Starting), + to: GuestState::Running, + }; + assert_eq!(ack.text(), "halted -> starting -> running"); + } + + #[test] + fn start_from_an_already_started_machine_has_no_via() { + let ack = TransitionAck { + from: GuestState::Starting, + via: None, + to: GuestState::Running, + }; + assert_eq!(ack.text(), "starting -> running"); + } + + /// Reaching `Running` without passing through `Starting`, or rewinding a + /// running guest, is refused and names both ends. + #[test] + fn invalid_transitions_are_refused() { + for (from, to) in [ + (GuestState::Idle, GuestState::Running), + (GuestState::Idle, GuestState::Halted), + (GuestState::Halted, GuestState::Running), + (GuestState::Running, GuestState::Starting), + (GuestState::Running, GuestState::Idle), + ] { + let err = from.transition(to).unwrap_err(); + assert_eq!(err.from, from); + assert_eq!(err.to, to); + assert_eq!( + err.to_string(), + format!("invalid guest state transition: {} -> {}", from.as_str(), to.as_str()) + ); + } + } + + #[test] + fn status_reports_running_exactly_when_the_cpu_runs() { + let running = MachineStatus { + state: GuestState::Running, + cpu_running: true, + monitor_addr: Some(addr()), + }; + assert!(running.monitor_ready()); + assert!(running.text().contains("state: running")); + assert!(running.text().contains("monitor: ready (127.0.0.1:8888)")); + + let idle = MachineStatus { + state: GuestState::Idle, + cpu_running: false, + monitor_addr: None, + }; + assert!(!idle.monitor_ready()); + assert!(idle.text().contains("monitor: not ready")); + } + + #[test] + fn json_carries_state_and_monitor_readiness() { + let json = MachineStatus { + state: GuestState::Starting, + cpu_running: false, + monitor_addr: Some(addr()), + } + .to_json(); + assert_eq!(json["state"], "starting"); + assert_eq!(json["cpu_running"], false); + assert_eq!(json["monitor_ready"], true); + assert_eq!(json["monitor_addr"], "127.0.0.1:8888"); + + let json = MachineStatus { + state: GuestState::Idle, + cpu_running: false, + monitor_addr: None, + } + .to_json(); + assert_eq!(json["monitor_ready"], false); + assert!(json["monitor_addr"].is_null()); + } + + #[test] + fn ack_json_matches_ack_text() { + let ack = TransitionAck { + from: GuestState::Halted, + via: Some(GuestState::Starting), + to: GuestState::Running, + }; + let json = ack.to_json(); + assert_eq!(json["from"], "halted"); + assert_eq!(json["via"], "starting"); + assert_eq!(json["to"], "running"); + assert_eq!(json["changed"], true); + + let ack = TransitionAck { + from: GuestState::Running, + via: None, + to: GuestState::Running, + }; + assert_eq!(ack.to_json()["changed"], false); + } +} diff --git a/src/main.rs b/src/main.rs index 417f01e1..100e244d 100644 --- a/src/main.rs +++ b/src/main.rs @@ -82,20 +82,6 @@ fn main() { } } - // CI control socket: started after Machine::new so it can hand out the - // machine pointer + CiSerialBackend to command handlers. - let _ci_server = if ci_enabled { - match iris::ci::start_server(&mut *machine, &ci_socket_path) { - Ok(s) => Some(s), - Err(e) => { - eprintln!("iris: failed to start CI server: {}", e); - std::process::exit(1); - } - } - } else { - None - }; - // DIAG: optionally enable verbose logging from startup via IRIS_DEBUG_LOG. // IRIS_DEBUG_LOG="mc,mips" enables those modules. "all" enables everything. // Output is broadcast to a stderr sink so jit-diag.sh's tee captures it inline. @@ -131,6 +117,24 @@ fn main() { start_perfetto(perfetto); machine.start(); + + // CI control socket: started after the machine so a successful connection + // means the monitor has already bound its port. That makes "the CI socket + // answers" a reliable readiness signal for a harness — no fixed sleep — + // and keeps CI commands from racing the half-started Machine. The CPU + // itself stays parked in `--ci` mode (see Machine::start). + let _ci_server = if ci_enabled { + match iris::ci::start_server(&mut *machine, &ci_socket_path) { + Ok(s) => Some(s), + Err(e) => { + eprintln!("iris: failed to start CI server: {}", e); + std::process::exit(1); + } + } + } else { + None + }; + // The terminal console talks to this process's own monitor only. Without // one (its port taken by another iris), there is no console: attached to // the other iris's monitor, it would exit along with that iris. diff --git a/src/workstation.rs b/src/workstation.rs index 69b61357..6f1b02da 100644 --- a/src/workstation.rs +++ b/src/workstation.rs @@ -29,6 +29,7 @@ use crate::dev::z85c30::CiSerialBackend; use crate::gfx_display::GfxDisplay; use crate::hptimer::TimerManager; use crate::machine::Machine; +use crate::machine_state::{GuestState, MachineStatus, TransitionAck}; use crate::monitor::Monitor; use crate::net::NatControl; use crate::traits::{Device, MachineEvent}; @@ -74,6 +75,74 @@ pub trait Workstation: Send { /// bind its port or has not started. fn monitor_addr(&self) -> Option; + /// Where the machine is in its start/stop lifecycle, as it stores it. + /// Never `Running`: [`Workstation::status`] derives that from the CPU's + /// own running flag, so an execution-control command cannot leave it + /// stale. `start` moves it to `Starting` before the monitor binds and + /// `stop` to `Halted` (unless it was never started). See + /// [`crate::machine_state`]. + fn lifecycle(&self) -> GuestState; + fn set_lifecycle(&mut self, state: GuestState); + + /// The guest's current state and whether the monitor is accepting + /// commands. Every surface that reports state — `machine-status`, the + /// `machine-start` acknowledgement, and the CI socket's `status` — goes + /// through here, so they cannot disagree. + fn status(&self) -> MachineStatus { + let cpu_running = self.cpu_is_running(); + MachineStatus { + // Running is read from the CPU, not stored: `start`/`stop` and + // breakpoints change the CPU's flag without a machine-level + // transition, and a stale lifecycle copy would be a second, + // wrong answer. + state: if cpu_running { GuestState::Running } else { self.lifecycle() }, + cpu_running, + monitor_addr: self.monitor_addr(), + } + } + + /// Start the CPU and peripherals on behalf of `machine-start` and the CI + /// socket's `start`. + /// + /// Idempotent: a running machine is left alone and the acknowledgement says + /// so. A machine that is `Starting` only needs the CPU released; an `Idle` + /// or `Halted` one also restarts its peripherals. The returned + /// [`TransitionAck`] is what the monitor prints. + fn start_guest(&mut self) -> Result { + let from = self.status().state; + if from == GuestState::Running { + return Ok(TransitionAck { from, via: None, to: from }); + } + + let via = if from == GuestState::Starting { + None + } else { + let starting = self + .lifecycle() + .transition(GuestState::Starting) + .map_err(|e| e.to_string())?; + self.set_lifecycle(starting); + Some(GuestState::Starting) + }; + // Peripherals are idempotently startable; do it even from `Starting` + // in case a previous stop left one of them down. + self.restart_peripherals(); + self.cpu_start(); + let to = self.status().state; + Ok(TransitionAck { from, via, to }) + } + + /// Stop the CPU and peripherals on behalf of `machine-stop`. + /// + /// Idempotent: stopping a halted machine changes nothing. Returns the + /// transition for the monitor's acknowledgement. + fn stop_guest(&mut self) -> TransitionAck { + let from = self.status().state; + self.stop(); + let to = self.status().state; + TransitionAck { from, via: None, to } + } + // ---- host interfaces ---- /// The display of the first graphics head, or None when headless. @@ -351,7 +420,7 @@ pub struct SystemController { } impl SystemController { - fn with_machine Result<(), String>>(&self, f: F) -> Result<(), String> { + fn with_machine(&self, f: impl FnOnce(&mut dyn Workstation) -> Result) -> Result { with_machine_slot(&self.machine, f)? } } @@ -365,8 +434,9 @@ impl Device for SystemController { fn register_commands(&self) -> Vec<(String, String)> { vec![ - ("machine-stop".to_string(), "Stop CPU and all peripherals".to_string()), - ("machine-start".to_string(), "Start CPU and all peripherals".to_string()), + ("machine-start".to_string(), "Start CPU and all peripherals (idempotent; prints the state transition)".to_string()), + ("machine-stop".to_string(), "Stop CPU and all peripherals (idempotent; prints the state transition)".to_string()), + ("machine-status".to_string(), "machine-status [json] — Guest state (idle/starting/running/halted) and monitor readiness".to_string()), ("reset".to_string(), "Reset all hardware to power-on state".to_string()), ("save".to_string(), "save — Save snapshot to saves//".to_string()), ("load".to_string(), "load — Load snapshot from saves//".to_string()), @@ -378,16 +448,28 @@ impl Device for SystemController { fn execute_command(&self, cmd: &str, args: &[&str], mut writer: Box) -> Result<(), String> { match cmd { "machine-stop" => { - let _ = writeln!(writer, "Stopping machine..."); - self.with_machine(|m| { m.stop(); Ok(()) }) + let ack = self.with_machine(|m| Ok(m.stop_guest()))?; + let _ = writeln!(writer, "machine-stop: {}", ack.text()); + Ok(()) } "machine-start" => { - let _ = writeln!(writer, "Starting machine..."); - self.with_machine(|m| { - m.restart_peripherals(); - m.cpu_start(); - Ok(()) - }) + let ack = self.with_machine(|m| m.start_guest())?; + let _ = writeln!(writer, "machine-start: {}", ack.text()); + Ok(()) + } + "machine-status" => { + let json = match args { + [] => false, + ["json"] => true, + _ => return Err("Usage: machine-status [json]".to_string()), + }; + let status = self.with_machine(|m| Ok(m.status()))?; + if json { + let _ = writeln!(writer, "{}", status.to_json()); + } else { + let _ = writeln!(writer, "{}", status.text()); + } + Ok(()) } "reset" => { let _ = writeln!(writer, "Resetting machine..."); diff --git a/tests/monitor_ready.rs b/tests/monitor_ready.rs new file mode 100644 index 00000000..e51a4628 --- /dev/null +++ b/tests/monitor_ready.rs @@ -0,0 +1,272 @@ +//! End-to-end readiness and state reporting (#102). +//! +//! Boots the real `iris` binary in `--ci` mode with no disk, then drives the +//! monitor over TCP: +//! +//! * a command sent before the monitor binds is refused (the client must poll, +//! not sleep), and the same poll then observes `starting` and `monitor_ready`; +//! * `machine-start` acknowledges `starting -> running` and is a no-op the +//! second time; +//! * `machine-stop` halts, and `machine-start` after the halt and after a +//! `reset` acknowledges `halted -> starting -> running`; +//! * `machine-status` text and JSON agree with each acknowledgement. +//! +//! The state-machine unit tests live in `src/machine_state.rs`; this is the +//! proof that the command surface, the TCP monitor and `--ci` startup order all +//! report the same thing. + +#![cfg(unix)] + +use std::io::{BufRead, BufReader, Read, Write}; +use std::net::TcpStream; +use std::os::unix::net::UnixStream; +use std::path::{Path, PathBuf}; +use std::process::{Child, Command, Stdio}; +use std::time::{Duration, Instant}; + +const STARTUP_TIMEOUT: Duration = Duration::from_secs(20); + +/// Kills iris if a test assertion fires before the clean `quit`, so a failure +/// cannot leave an orphan holding this test's ports. +struct ChildGuard(Child); + +impl Drop for ChildGuard { + fn drop(&mut self) { + if self.0.try_wait().ok().flatten().is_none() { + let _ = self.0.kill(); + let _ = self.0.wait(); + } + } +} + +fn free_ports(n: usize) -> Vec { + let listeners: Vec<_> = (0..n) + .map(|_| std::net::TcpListener::bind("127.0.0.1:0").expect("bind ephemeral port")) + .collect(); + listeners.iter().map(|l| l.local_addr().unwrap().port()).collect() +} + +fn temp_dir(name: &str) -> PathBuf { + let dir = std::env::temp_dir().join(format!( + "iris-monitor-ready-{name}-{}", + std::process::id() + )); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).expect("create test directory"); + dir +} + +fn write_config(dir: &Path, monitor: u16, serial_a: u16, serial_b: u16, ci_socket: &Path) { + let text = format!( + "headless = true\n\ + no_audio = true\n\ + monitor_port = {monitor}\n\ + serial_port_a = {serial_a}\n\ + serial_port_b = {serial_b}\n\ + ci_socket = \"{}\"\n\ + \n\ + # No disk: the embedded PROM is enough for this test, and the default\n\ + # scsi1.raw would otherwise abort startup when it is absent.\n\ + scsi = {{}}\n", + ci_socket.display() + ); + std::fs::write(dir.join("iris.toml"), text).expect("write test config"); +} + +/// Retry the connect (the readiness poll a harness should use) and drain the +/// monitor's banner up to its first prompt. +fn connect_polling(port: u16, deadline: Instant) -> TcpStream { + loop { + match TcpStream::connect(("127.0.0.1", port)) { + Ok(mut stream) => { + stream.set_read_timeout(Some(Duration::from_millis(100))).unwrap(); + read_until_prompt(&mut stream, deadline); + return stream; + } + Err(_) => { + assert!(Instant::now() < deadline, "monitor on port {port} never accepted a connection"); + std::thread::sleep(Duration::from_millis(20)); + } + } + } +} + +/// One CI control-socket request, retried until the socket appears (it is +/// created after the monitor binds). +fn ci_command(socket: &Path, cmd: &str, deadline: Instant) -> serde_json::Value { + loop { + if let Ok(mut stream) = UnixStream::connect(socket) { + stream.set_read_timeout(Some(Duration::from_secs(5))).unwrap(); + let request = format!("{{\"cmd\":\"{cmd}\",\"args\":{{}}}}\n"); + stream.write_all(request.as_bytes()).unwrap(); + stream.flush().unwrap(); + let mut line = String::new(); + BufReader::new(stream).read_line(&mut line).unwrap(); + let response: serde_json::Value = + serde_json::from_str(line.trim()).expect("CI response parses"); + assert_eq!(response["ok"], true, "CI {cmd}: {response}"); + return response["data"].clone(); + } + assert!( + Instant::now() < deadline, + "CI socket {} never appeared", + socket.display() + ); + std::thread::sleep(Duration::from_millis(20)); + } +} + +fn read_until_prompt(stream: &mut TcpStream, deadline: Instant) -> Vec { + let mut out = Vec::new(); + let mut buf = [0u8; 4096]; + loop { + match stream.read(&mut buf) { + Ok(0) => panic!("monitor closed the connection: {:?}", String::from_utf8_lossy(&out)), + Ok(n) => { + out.extend_from_slice(&buf[..n]); + // The monitor's prompt is a line of its own: responses end in + // CRLF followed by "> ". Matching any "> " would stop on the + // arrow in "halted -> starting -> running". + if out.ends_with(b"\r\n> ") { + return out; + } + } + Err(e) + if e.kind() == std::io::ErrorKind::WouldBlock + || e.kind() == std::io::ErrorKind::TimedOut => + { + assert!( + Instant::now() < deadline, + "timed out waiting for the monitor prompt; got {:?}", + String::from_utf8_lossy(&out) + ); + } + Err(e) => panic!("monitor read failed: {e}"), + } + } +} + +fn command(stream: &mut TcpStream, cmd: &str) -> String { + stream.write_all(cmd.as_bytes()).unwrap(); + stream.write_all(b"\r\n").unwrap(); + stream.flush().unwrap(); + let bytes = read_until_prompt(stream, Instant::now() + Duration::from_secs(10)); + let text = String::from_utf8_lossy(&bytes).replace("\r\n", "\n"); + // Drop the trailing prompt the monitor writes after every command. + text.trim_end_matches("> ").trim().to_string() +} + +fn status_json(reply: &str) -> serde_json::Value { + let start = reply.find('{').unwrap_or_else(|| panic!("no JSON in {reply:?}")); + let end = reply.rfind('}').unwrap_or_else(|| panic!("no JSON in {reply:?}")); + serde_json::from_str(&reply[start..=end]).expect("machine-status JSON parses") +} + +fn wait_for_exit(child: &mut Child, deadline: Instant) -> std::process::ExitStatus { + loop { + match child.try_wait().expect("wait for iris") { + Some(status) => return status, + None if Instant::now() < deadline => std::thread::sleep(Duration::from_millis(20)), + None => { + let _ = child.kill(); + let _ = child.wait(); + panic!("iris did not exit after quit"); + } + } + } +} + +#[test] +fn monitor_start_is_acknowledged_and_idempotent() { + let dir = temp_dir("commands"); + let ports = free_ports(3); + let (monitor_port, serial_a, serial_b) = (ports[0], ports[1], ports[2]); + let ci_socket = dir.join("iris.sock"); + write_config(&dir, monitor_port, serial_a, serial_b, &ci_socket); + + // Nothing is listening yet. This is the case the old harness hid with a + // fixed sleep: the first connect fails, so the caller must retry rather + // than assume the command was delivered. + assert!( + TcpStream::connect(("127.0.0.1", monitor_port)).is_err(), + "port {monitor_port} was already in use; retry the test" + ); + + let stderr = std::fs::File::create(dir.join("stderr.log")).unwrap(); + let mut child = ChildGuard( + Command::new(env!("CARGO_BIN_EXE_iris")) + .current_dir(&dir) + .arg("--config") + .arg(dir.join("iris.toml")) + .arg("--ci") + .arg("--ci-socket") + .arg(&ci_socket) + .arg("--monitor-port") + .arg(monitor_port.to_string()) + .arg("--serial-port-a") + .arg(serial_a.to_string()) + .arg("--serial-port-b") + .arg(serial_b.to_string()) + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::from(stderr)) + .spawn() + .expect("spawn iris"), + ); + + let mut stream = connect_polling(monitor_port, Instant::now() + STARTUP_TIMEOUT); + + let status = status_json(&command(&mut stream, "machine-status json")); + assert_eq!(status["state"], "starting", "CI mode parks the CPU until machine-start"); + assert_eq!(status["cpu_running"], false); + assert_eq!(status["monitor_ready"], true); + assert_eq!( + status["monitor_addr"], + format!("127.0.0.1:{monitor_port}"), + "the status names the monitor's own address" + ); + + // The CI socket is created after the monitor binds, so a successful answer + // is itself proof of readiness; both surfaces must report the same state. + let ci = ci_command(&ci_socket, "status", Instant::now() + STARTUP_TIMEOUT); + assert_eq!(ci, status, "CI status and machine-status disagree"); + + let text = command(&mut stream, "machine-status"); + assert!(text.contains("state: starting"), "{text}"); + assert!(text.contains("monitor: ready (127.0.0.1:"), "{text}"); + + // First start: the acknowledgement names the whole transition. + let ack = command(&mut stream, "machine-start"); + assert_eq!(ack, "machine-start: starting -> running"); + let status = status_json(&command(&mut stream, "machine-status json")); + assert_eq!(status["state"], "running"); + assert_eq!(status["cpu_running"], true); + + // Duplicate start: no second start, no error. + let ack = command(&mut stream, "machine-start"); + assert_eq!(ack, "machine-start: running (no change)"); + + // Start after halt. + assert_eq!(command(&mut stream, "machine-stop"), "machine-stop: running -> halted"); + let status = status_json(&command(&mut stream, "machine-status json")); + assert_eq!(status["state"], "halted"); + assert_eq!(status["cpu_running"], false); + assert_eq!(status["monitor_ready"], true, "a halted machine keeps its monitor"); + assert_eq!(command(&mut stream, "machine-start"), "machine-start: halted -> starting -> running"); + + // Start after reset: reset leaves the CPU stopped and the monitor alive. + let reset = command(&mut stream, "reset"); + assert!(reset.contains("Resetting machine..."), "{reset}"); + let status = status_json(&command(&mut stream, "machine-status json")); + assert_eq!(status["state"], "halted", "reset leaves the machine halted"); + assert_eq!(command(&mut stream, "machine-start"), "machine-start: halted -> starting -> running"); + + // Shut down cleanly through the CI socket; there is no guest disk here, so + // stopping the machine first is the whole safety story. + assert_eq!(command(&mut stream, "machine-stop"), "machine-stop: running -> halted"); + drop(stream); + ci_command(&ci_socket, "quit", Instant::now() + Duration::from_secs(10)); + let status = wait_for_exit(&mut child.0, Instant::now() + Duration::from_secs(10)); + assert!(status.success(), "iris exited with {status}"); + let _ = std::fs::remove_dir_all(&dir); +}