From 1e41371ca7e06cf21a62cbc26490471b1998a5f0 Mon Sep 17 00:00:00 2001 From: japabu Date: Wed, 30 Sep 2026 23:38:42 +0200 Subject: [PATCH 1/7] AArch64 stage 5: PSCI resets and powers the machine off, behind one seam Stage 5 of issues/kernel/toyos-runs-on-arm64.md owed SYSTEM_RESET, SYSTEM_OFF and CPU_OFF behind a reset and power-off seam that takes x86-64's reset register and PM1a out of drivers/acpi.rs, and the stop shown on eight CPUs ending in that power-off. Stage 4's remaining items wait on stage 6 (the HVF run), on metal, or on an ABI brief, so this is the first unblocked step whose exit is not met. The seam: kernel/src/power.rs keeps what every reset owes whatever the machine (the console's last drain, the xHCI stop before the register, the reboot refused without a reset); arch::power is the register write. x86-64's is the FADT's reset register and S5 through PM1a, moved as they were, both decoded in one place before the IDT loads; the PM1a decode used to run after gpt::init. Its I/O-port-space checks were dead on x86 and every arch::pio user was that code, so both pio modules go. AArch64's is PSCI (Arm DEN0022): SYSTEM_RESET, which asks no coordination of the other cores (5.12.2), so a wedge may call it; and SYSTEM_OFF, whose caller places every core in a known state first (5.10.1), by the specification's own recipe (5.10.3): an SGI asks every other CPU in the roster to stop its timer and call CPU_OFF, and the caller waits, bounded, for AFFINITY_INFO to answer each off before it calls SYSTEM_OFF. A CPU still on past the budget is named on the UART raw, since the console's last drain is behind the call. The conduit psci::init found is kept in one word, which smp::start and the reset both read. toyos-gicv3 gains the inverse of packed_affinity, which is how PSCI names a CPU. Tests: virt_smp and virt_el1_smp's case now ends with the job `shutdown`, and both judge the power-off: the stop's record names eight CPUs, `Shutting down.` is the last word, QEMU stops for guest-shutdown, and QEMU's own `arm_psci_call` trace (BootOptions::psci_trace) has every CPU but one call CPU_OFF once before the remaining one calls SYSTEM_OFF once, through SMC at EL2 and through HVC at EL1. virt_reboot (tests/virtrebootcase) judges one SYSTEM_RESET and guest-reset. machine_shutdown is x86-64's power-off, which no test judged by QEMU's stop reason before this moved it. toyos-checks' psci_power_off_judge refuses each way a trace falls short. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_016t9wjdQkB8SH7bmfUoiy6L --- issues/kernel/toyos-runs-on-arm64.md | 6 +- kernel/src/arch/aarch64/boot.rs | 10 +- kernel/src/arch/aarch64/irqchip.rs | 18 ++- kernel/src/arch/aarch64/mod.rs | 2 +- kernel/src/arch/aarch64/pio.rs | 15 -- kernel/src/arch/aarch64/power.rs | 66 ++++++++ kernel/src/arch/aarch64/psci.rs | 90 +++++++++-- kernel/src/arch/aarch64/smp.rs | 4 +- kernel/src/arch/aarch64/trap.rs | 1 + kernel/src/arch/x86_64/boot.rs | 2 +- kernel/src/arch/x86_64/mod.rs | 2 +- kernel/src/arch/x86_64/pio.rs | 7 - kernel/src/arch/x86_64/power.rs | 165 ++++++++++++++++++++ kernel/src/deadline.rs | 2 +- kernel/src/drivers/acpi.rs | 220 +-------------------------- kernel/src/drivers/serial.rs | 2 +- kernel/src/hardlockup/mod.rs | 4 +- kernel/src/main.rs | 2 +- kernel/src/panic_reboot.rs | 13 +- kernel/src/power.rs | 59 +++++++ kernel/src/syscall/machine.rs | 14 +- src/build.rs | 1 + tests/checks.rs | 28 ++++ tests/common/power.rs | 37 ++++- tests/common/qemu.rs | 13 ++ tests/toyos.rs | 177 ++++++++++++++++++--- tests/virtrebootcase/system.toml | 18 +++ tests/virtsmpcase/system.toml | 6 +- toyos-gicv3/src/lib.rs | 6 + toyos-gicv3/tests/gicv3.rs | 10 +- 30 files changed, 686 insertions(+), 314 deletions(-) delete mode 100644 kernel/src/arch/aarch64/pio.rs create mode 100644 kernel/src/arch/aarch64/power.rs delete mode 100644 kernel/src/arch/x86_64/pio.rs create mode 100644 kernel/src/arch/x86_64/power.rs create mode 100644 kernel/src/power.rs create mode 100644 tests/virtrebootcase/system.toml diff --git a/issues/kernel/toyos-runs-on-arm64.md b/issues/kernel/toyos-runs-on-arm64.md index 123db6c3868..08adf2589b0 100644 --- a/issues/kernel/toyos-runs-on-arm64.md +++ b/issues/kernel/toyos-runs-on-arm64.md @@ -293,11 +293,7 @@ Each stage names its exit; "measured" means a number from a run. `toyos_elf::RelocError::TlsDescriptor` for a library). **Every CPU starts, ahead of small-kernel stage 6 by the owner's word, as stage 4 did:** it ports no device interrupt, so nothing of the relay. - Owed before the exit holds: - `SYSTEM_RESET`, `SYSTEM_OFF` and `CPU_OFF` behind a reset and power-off - seam that takes x86-64's reset register and PM1a out of - `drivers/acpi.rs`, and the stop shown on eight CPUs ending in that - power-off; the TLS-descriptor resolver; + Owed before the exit holds: the TLS-descriptor resolver; `issues/kernel/the-crash-evidence-records-x86-fault-registers.md`; the blocked-task dump's probe of a CPU that ignored its kick (`sched/dump.rs`'s `probe_silent`), which reaches `irqchip::send_nmi`'s diff --git a/kernel/src/arch/aarch64/boot.rs b/kernel/src/arch/aarch64/boot.rs index bdd84b1aa01..f21745c1f1c 100644 --- a/kernel/src/arch/aarch64/boot.rs +++ b/kernel/src/arch/aarch64/boot.rs @@ -309,19 +309,21 @@ pub fn reserved() -> Region { } /// What the boot learns bringing interrupts up and hands later steps: the -/// other CPUs the MADT names, and how to start them. +/// other CPUs the MADT names. pub struct Platform { gic: super::irqchip::Gic, - psci: Option, } /// Interrupt delivery: this CPU's per-CPU block, the GIC and the timer's /// interrupt, and interrupts unmasked. The syscall gate is the vectors' own. +/// Then PSCI, which starts the other CPUs and resets and powers off the +/// machine. pub fn interrupts(rsdp_addr: u64) -> Platform { super::percpu::init_bsp(); let gic = super::irqchip::init(rsdp_addr); super::cpu::enable_interrupts(); - Platform { gic, psci: super::psci::init(rsdp_addr) } + super::psci::init(rsdp_addr); + Platform { gic } } /// The clock: the generic timer's count, at the rate firmware states in @@ -349,7 +351,7 @@ pub fn platform_devices(_rsdp_addr: u64) {} /// Every other CPU, running. pub fn start_other_cpus(platform: &Platform, _args: &KernelArgs) { - super::smp::start(&platform.gic, platform.psci); + super::smp::start(&platform.gic); } /// The interrupt-controller selftests an actuator asks for. diff --git a/kernel/src/arch/aarch64/irqchip.rs b/kernel/src/arch/aarch64/irqchip.rs index eb7f49b1468..5917f868de9 100644 --- a/kernel/src/arch/aarch64/irqchip.rs +++ b/kernel/src/arch/aarch64/irqchip.rs @@ -40,6 +40,8 @@ pub(super) enum Intid { Kick = 0, /// Stops a CPU for good: [`stop_other_cpus`]'s. Halt, + /// Turns a CPU off for the machine's power-off: [`off_all_but_self`]'s. + Off, /// `crate::log::nested`'s delivery, which [`send_self`] raises. LogNest, /// What `irq-storm` floods this CPU with. @@ -50,6 +52,7 @@ pub(super) enum Intid { pub(super) const SGI_KICK: u32 = Intid::Kick as u32; pub(super) const SGI_HALT: u32 = Intid::Halt as u32; +pub(super) const SGI_OFF: u32 = Intid::Off as u32; #[cfg(feature = "boot-actuators")] pub(super) const SGI_STORM: u32 = Intid::Storm as u32; @@ -215,7 +218,7 @@ pub fn init_cpu(frame: u64) { redistributor.write_u32(GICR_ICFGR1, edge); stop_timer_hardware(); - let enabled = 1 << SGI_KICK | 1 << SGI_HALT | 1 << timer; + let enabled = 1 << SGI_KICK | 1 << SGI_HALT | 1 << SGI_OFF | 1 << timer; #[cfg(feature = "boot-actuators")] let enabled = enabled | 1 << Intid::LogNest as u32 | 1 << SGI_STORM; redistributor.write_u32(GICR_ISENABLER0, enabled); @@ -294,13 +297,22 @@ pub fn kick_cpu(cpu: u32) { sgi(SGI_KICK, crate::smp::hardware_id(cpu)); } +pub fn kick_all_but_self() { + all_but_self(SGI_KICK); +} + +/// Raise the power-off's SGI on every other CPU the roster holds. +pub(super) fn off_all_but_self() { + all_but_self(SGI_OFF); +} + // Each by name, not with `IRM`'s broadcast: that reaches an AP which echoed too // late and halted with its SGIs enabled, where a kick left pending wakes the // masked `wfi` at once, for good. -pub fn kick_all_but_self() { +fn all_but_self(intid: u32) { let me = percpu::cpu_id(); for cpu in (0..crate::smp::cpu_count()).filter(|&cpu| cpu != me) { - kick_cpu(cpu); + sgi(intid, crate::smp::hardware_id(cpu)); } } diff --git a/kernel/src/arch/aarch64/mod.rs b/kernel/src/arch/aarch64/mod.rs index 8df6a4c33f2..32d29deafdb 100644 --- a/kernel/src/arch/aarch64/mod.rs +++ b/kernel/src/arch/aarch64/mod.rs @@ -26,8 +26,8 @@ pub mod irqchip; pub mod keyboard_controller; pub mod paging; pub mod percpu; -pub mod pio; pub mod pmu; +pub mod power; pub mod psci; pub mod rtc; pub mod smp; diff --git a/kernel/src/arch/aarch64/pio.rs b/kernel/src/arch/aarch64/pio.rs deleted file mode 100644 index b2886833d47..00000000000 --- a/kernel/src/arch/aarch64/pio.rs +++ /dev/null @@ -1,15 +0,0 @@ -//! The I/O port space: AArch64 has none. - -/// Whether this architecture has an I/O port space at all. Firmware tables -/// that name a port are only honoured where it does. -pub const EXISTS: bool = false; - -/// Never called: every caller checks [`EXISTS`] first. -pub unsafe fn outb(_port: u16, _value: u8) { - unreachable!("AArch64 has no I/O port space") -} - -/// Never called: every caller checks [`EXISTS`] first. -pub unsafe fn outw(_port: u16, _value: u16) { - unreachable!("AArch64 has no I/O port space") -} diff --git a/kernel/src/arch/aarch64/power.rs b/kernel/src/arch/aarch64/power.rs new file mode 100644 index 00000000000..ce8261acde4 --- /dev/null +++ b/kernel/src/arch/aarch64/power.rs @@ -0,0 +1,66 @@ +//! Reset and power-off: PSCI's `SYSTEM_RESET` and `SYSTEM_OFF` (Arm DEN0022 +//! §5.12, §5.10), through the conduit [`super::psci`] kept. A machine without +//! PSCI has neither, and holds where either is asked for. + +use super::{cpu, irqchip, percpu, psci}; +use crate::drivers::serial; +use crate::time::{Budget, Deadline, Duration}; + +/// How long the other CPUs get to be off by PSCI's answer before this one +/// powers the machine off anyway. +const OTHERS_OFF: Budget = Budget::of( + Duration::from_millis(100), + "the machine powers off with the CPUs PSCI still answers on, each named on the console", +); + +pub fn can_reset() -> bool { + psci::conduit().is_some() +} + +/// `SYSTEM_RESET`, which asks nothing of the other CPUs (DEN0022 §5.12.2): a +/// wedge calls this, so it takes no lock and makes no other call. +pub fn reset() -> ! { + if let Some(psci) = psci::conduit() { + psci.system_reset(); + } + cpu::halt() +} + +/// `SYSTEM_OFF`, once every other CPU the roster holds has turned itself off +/// with `CPU_OFF` and PSCI answers it off: the caller puts every core in a +/// known state first, and this is DEN0022 §5.10.3's own way to. +pub fn off() -> ! { + cpu::disable_interrupts(); + let Some(psci) = psci::conduit() else { cpu::halt() }; + irqchip::off_all_but_self(); + let deadline = Deadline::at(crate::clock::now() + OTHERS_OFF.duration()); + let me = percpu::cpu_id(); + for cpu in (0..crate::smp::cpu_count()).filter(|&cpu| cpu != me) { + let mpidr = toyos_gicv3::unpacked_affinity(crate::smp::hardware_id(cpu)); + while !matches!(psci.affinity_info(mpidr), Ok(psci::Affinity::Off)) { + if deadline.reached(crate::clock::now()) { + // Raw: the console's last drain is behind this call. + serial::panic_raw(b"power: cpu"); + serial::panic_raw_dec(u64::from(cpu)); + serial::panic_raw(b" is not off by PSCI's answer inside the budget; SYSTEM_OFF regardless\n"); + break; + } + core::hint::spin_loop(); + } + } + psci.system_off(); + cpu::halt() +} + +/// This CPU's part in [`off`], on the SGI that asked for it: its timer +/// stopped, since a core `CPU_OFF` turned off may take no interrupt (DEN0022 +/// §5.5.2), then `CPU_OFF`. A refusal leaves it halted, and [`off`] names it +/// still on. +pub(super) fn cpu_off() -> ! { + irqchip::end(irqchip::SGI_OFF); + irqchip::stop_timer(); + if let Some(psci) = psci::conduit() { + psci.cpu_off(); + } + cpu::halt() +} diff --git a/kernel/src/arch/aarch64/psci.rs b/kernel/src/arch/aarch64/psci.rs index e4df148df5e..4348e497022 100644 --- a/kernel/src/arch/aarch64/psci.rs +++ b/kernel/src/arch/aarch64/psci.rs @@ -3,27 +3,48 @@ //! where a hypervisor stands in for it. Every call is SMCCC's (Arm DEN0028D): //! the function in `x0`, its arguments in `x1`–`x3`, the answer in `x0`. +use core::sync::atomic::{AtomicU8, Ordering::Relaxed}; + use toyos_acpi::Psci; use crate::drivers::acpi::direct_phys; use crate::log; -/// The function IDs of `PSCI_VERSION` and the SMC64 `CPU_ON`, as PSCI -/// numbers them from 0.2 on. +/// The function IDs this kernel calls, as PSCI numbers them from 0.2 on +/// (DEN0022 §5.1), each the SMC64 one where there is one. const PSCI_VERSION: u32 = 0x8400_0000; +const CPU_OFF: u32 = 0x8400_0002; const CPU_ON: u32 = 0xC400_0003; +const AFFINITY_INFO: u32 = 0xC400_0004; +const SYSTEM_OFF: u32 = 0x8400_0008; +const SYSTEM_RESET: u32 = 0x8400_0009; -/// `CPU_ON`'s `target_cpu`: `MPIDR_EL1`'s affinity fields where they stand, -/// Aff3 in bits 39:32, every other bit zero. +/// `CPU_ON`'s `target_cpu` and `AFFINITY_INFO`'s `target_affinity`: +/// `MPIDR_EL1`'s affinity fields where they stand, Aff3 in bits 39:32, every +/// other bit zero. const TARGET_CPU: u64 = 0xFF_00FF_FFFF; /// How this machine reaches PSCI. #[derive(Clone, Copy)] +#[repr(u8)] pub enum Conduit { - Smc, + Smc = 1, Hvc, } +/// The conduit [`init`] found, zero for none: one word, because a reset on a +/// wedged machine reads it and may take no lock. Written on the boot CPU +/// before any other starts. +static CONDUIT: AtomicU8 = AtomicU8::new(0); + +/// What `AFFINITY_INFO` answers of one CPU (DEN0022 §5.7.1). +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub enum Affinity { + On, + Off, + OnPending, +} + /// A PSCI call's refusal, as the specification numbers its return codes. #[derive(Clone, Copy, Debug)] pub enum Error { @@ -97,6 +118,34 @@ impl Conduit { } } + /// Turn this CPU off: an answer is a refusal, since `CPU_OFF` does not + /// return when it succeeds. + pub fn cpu_off(self) -> Error { + Error::of(self.call(CPU_OFF, 0, 0, 0)) + } + + /// Whether the CPU whose `MPIDR_EL1` is `mpidr` is on, off, or on its way + /// on: affinity level 0, the CPU itself. + pub fn affinity_info(self, mpidr: u64) -> Result { + match self.call(AFFINITY_INFO, mpidr & TARGET_CPU, 0, 0) { + 0 => Ok(Affinity::On), + 1 => Ok(Affinity::Off), + 2 => Ok(Affinity::OnPending), + code => Err(Error::of(code)), + } + } + + /// Power the machine off. Neither this nor [`Conduit::system_reset`] + /// returns (DEN0022 §5.1.9, §5.1.11), except where firmware breaks that. + pub fn system_off(self) { + self.call(SYSTEM_OFF, 0, 0, 0); + } + + /// Reset the machine cold. + pub fn system_reset(self) { + self.call(SYSTEM_RESET, 0, 0, 0); + } + fn name(self) -> &'static str { match self { Self::Smc => "SMC", @@ -105,14 +154,15 @@ impl Conduit { } } -/// PSCI as the FADT names it, asked for its version: `None` where the machine -/// offers none this kernel can call, which the log says. -pub fn init(rsdp_addr: u64) -> Option { +/// PSCI as the FADT names it, asked for its version and kept for +/// [`conduit`]; none where the machine offers none this kernel can call, +/// which the log says. +pub fn init(rsdp_addr: u64) { let fadt = match toyos_acpi::find_table(direct_phys(), rsdp_addr, b"FACP", toyos_acpi::SDT_HEADER_LEN) { Ok(fadt) => fadt, Err(e) => { log!("PSCI: none, because the FADT is unusable: {e:?}"); - return None; + return; } }; let conduit = match toyos_acpi::psci(&fadt) { @@ -120,27 +170,37 @@ pub fn init(rsdp_addr: u64) -> Option { Psci::Hvc => Conduit::Hvc, Psci::Absent => { log!("PSCI: none: the FADT's ARM_BOOT_ARCH does not set PSCI_COMPLIANT"); - return None; + return; } Psci::Undefined { revision, minor } => { log!("PSCI: none: the FADT is version {revision}.{minor}, and ARM_BOOT_ARCH begins at 5.1"); - return None; + return; } Psci::Short => { log!("PSCI: none: the FADT ends before ARM_BOOT_ARCH and the minor version after it"); - return None; + return; } }; let version = conduit.call(PSCI_VERSION, 0, 0, 0); if version < 0 { log!("PSCI: PSCI_VERSION through {} answered {:?}; none used", conduit.name(), Error::of(version)); - return None; + return; } let (major, minor) = (version >> 16, version & 0xFFFF); if major == 0 && minor < 2 { log!("PSCI: {major}.{minor} through {}, which numbers no SMC64 CPU_ON; none used", conduit.name()); - return None; + return; } log!("PSCI: {major}.{minor} through {}", conduit.name()); - Some(conduit) + CONDUIT.store(conduit as u8, Relaxed); +} + +/// The conduit [`init`] kept, or `None` on a machine without PSCI. +pub fn conduit() -> Option { + match CONDUIT.load(Relaxed) { + 0 => None, + 1 => Some(Conduit::Smc), + 2 => Some(Conduit::Hvc), + other => unreachable!("PSCI: the kept conduit reads {other}, which `init` never stores"), + } } diff --git a/kernel/src/arch/aarch64/smp.rs b/kernel/src/arch/aarch64/smp.rs index 0f294097f43..a8441abf4a9 100644 --- a/kernel/src/arch/aarch64/smp.rs +++ b/kernel/src/arch/aarch64/smp.rs @@ -30,11 +30,11 @@ pub struct ApStart { /// Start every other CPU `gic` names, in the MADT's order, until one does not /// echo within [`AP_START`] or the roster is full. -pub fn start(gic: &irqchip::Gic, psci: Option) { +pub fn start(gic: &irqchip::Gic) { let me = cpu::hardware_id(); ROSTER.set_bsp(me); let others = gic.cpus.iter().filter(|gicc| packed_affinity(gicc.mpidr) != me); - let Some(psci) = psci else { + let Some(psci) = psci::conduit() else { log!("SMP: no PSCI to start the other {} CPUs with; the boot CPU runs alone", others.count()); return; }; diff --git a/kernel/src/arch/aarch64/trap.rs b/kernel/src/arch/aarch64/trap.rs index d85e2ca15a8..0999dd3725e 100644 --- a/kernel/src/arch/aarch64/trap.rs +++ b/kernel/src/arch/aarch64/trap.rs @@ -166,6 +166,7 @@ fn irq(from_el0: bool) { // Never ended: the running priority it keeps is every interrupt's // own, so the interface signals this CPU nothing and the halt stays. irqchip::SGI_HALT => cpu::halt(), + irqchip::SGI_OFF => super::power::cpu_off(), irqchip::SGI_KICK => { percpu::irq_took(Source::Timer); irqchip::end(intid); diff --git a/kernel/src/arch/x86_64/boot.rs b/kernel/src/arch/x86_64/boot.rs index 341a7edd204..da62778e7a0 100644 --- a/kernel/src/arch/x86_64/boot.rs +++ b/kernel/src/arch/x86_64/boot.rs @@ -87,7 +87,7 @@ pub fn interrupts(rsdp_addr: u64) -> Platform { // Off the same tables as the MADT, and before the IDT below makes a panic // reportable: a panic that can be reported but not ended leaves the machine // holding its panel for a hand that may not be in the room. - acpi::init_reset(rsdp_addr); + super::power::init(rsdp_addr); apic::init(); percpu::init_bsp(apic::id()); ioapic::init(&madt); diff --git a/kernel/src/arch/x86_64/mod.rs b/kernel/src/arch/x86_64/mod.rs index 1e565f53f79..a56ace6f588 100644 --- a/kernel/src/arch/x86_64/mod.rs +++ b/kernel/src/arch/x86_64/mod.rs @@ -30,8 +30,8 @@ pub mod nmi_gate; pub mod paging; pub mod pat; pub mod percpu; -pub mod pio; pub mod pmu; +pub mod power; pub mod rtc; pub mod smp; pub mod switch; diff --git a/kernel/src/arch/x86_64/pio.rs b/kernel/src/arch/x86_64/pio.rs deleted file mode 100644 index 5a29539bbb2..00000000000 --- a/kernel/src/arch/x86_64/pio.rs +++ /dev/null @@ -1,7 +0,0 @@ -//! The I/O port space: x86-64 has one, reached by `in` and `out`. - -/// Whether this architecture has an I/O port space at all. Firmware tables -/// that name a port are only honoured where it does. -pub const EXISTS: bool = true; - -pub use super::cpu::{outb, outw}; diff --git a/kernel/src/arch/x86_64/power.rs b/kernel/src/arch/x86_64/power.rs new file mode 100644 index 00000000000..7a65115441e --- /dev/null +++ b/kernel/src/arch/x86_64/power.rs @@ -0,0 +1,165 @@ +//! Reset and power-off through the FADT: its reset register, and S5 soft-off +//! through the PM1a control block with the `SLP_TYPa` the DSDT's `\_S5_` +//! package names. +//! +//! All input is firmware-supplied and untrusted: a table that does not decode +//! is a machine with no reboot or no soft-off, said by name, never a panic. + +use core::mem::size_of; +use core::sync::atomic::{AtomicU16, AtomicU8, Ordering}; + +use toyos_acpi::{Reset, Table, TableError, SDT_HEADER_LEN, SDT_REVISION}; + +use super::cpu; +use crate::drivers::acpi::direct_phys; +use crate::log; + +const SLP_EN: u16 = 1 << 13; + +static PM1A_CNT_PORT: AtomicU16 = AtomicU16::new(0); +static SLP_TYPA: AtomicU16 = AtomicU16::new(0); + +static RESET_PORT: AtomicU16 = AtomicU16::new(0); +static RESET_VALUE: AtomicU8 = AtomicU8::new(0); + +/// Record the FADT's reset register and its S5 soft-off, or say by name why +/// this machine has either not. +/// +/// Before `percpu::init_bsp` loads the IDT: from then on every panic can be +/// reported, and a panic that can be reported but not ended is a machine that +/// still needs a hand. Walking these tables inside the panic handler instead is +/// refused — a table walk on a machine that has already failed once is how a +/// panic becomes a triple fault. +pub fn init(rsdp_addr: u64) { + init_reset(rsdp_addr); + init_off(rsdp_addr); +} + +fn init_reset(rsdp_addr: u64) { + let fadt = match toyos_acpi::find_table(direct_phys(), rsdp_addr, b"FACP", toyos_acpi::FADT_FOR_RESET) { + Ok(table) => table, + Err(e) => { + log!("ACPI: FADT unusable: {e:?} — no reboot, a panic will hold the panel"); + return; + } + }; + match toyos_acpi::reset_register(&fadt) { + Reset::Port { port, value } => { + RESET_PORT.store(port, Ordering::Relaxed); + RESET_VALUE.store(value, Ordering::Relaxed); + log!("ACPI: reset register SystemIO {port:#x} <- {value:#04x}"); + } + other => log!("ACPI: no reset register this kernel writes ({other:?}) — no reboot"), + } +} + +// A machine without these tables keeps booting without soft-off rather than panicking. +fn init_off(rsdp_addr: u64) { + const FADT_FOR_POWER: usize = toyos_acpi::FADT_PM1A_CNT_BLK + size_of::(); + const FADT_FOR_X_DSDT: usize = toyos_acpi::FADT_X_DSDT + size_of::(); + + let fadt = match toyos_acpi::find_table(direct_phys(), rsdp_addr, b"FACP", FADT_FOR_POWER) { + Ok(table) => table, + Err(e) => { + log!("ACPI: FADT unusable: {e:?} — no soft-off, shutdown will halt instead"); + return; + } + }; + + let Some(pm1a) = fadt.u32_at(toyos_acpi::FADT_PM1A_CNT_BLK) else { + log!("ACPI: FADT has no PM1a control block — no soft-off"); + return; + }; + let pm1a = pm1a as u16; + + // Prefer X_DSDT over DSDT; a revision claiming 2.0 doesn't prove the field is present, so the length is checked rather than trusting the revision alone. + let dsdt_addr = toyos_acpi::dsdt_address(&fadt); + if dsdt_addr == 0 { + log!( + "ACPI: FADT names no DSDT (rev {:?}, needs {FADT_FOR_X_DSDT} bytes for X_DSDT) — no soft-off", + fadt.byte(SDT_REVISION) + ); + return; + } + + let dsdt = match Table::open(direct_phys(), dsdt_addr, b"DSDT", SDT_HEADER_LEN) { + Ok(table) => table, + Err(TableError::Unmapped { .. }) => { + log!("ACPI: FADT points the DSDT at {dsdt_addr:#x}, which is not an address — no soft-off"); + return; + } + Err(e) => { + log!("ACPI: DSDT at {dsdt_addr:#x} unusable: {e:?} — no soft-off"); + return; + } + }; + + let Some(slp_typ) = find_s5_slp_typ(&dsdt) else { + log!("ACPI: no \\_S5_ package in the DSDT — no soft-off"); + return; + }; + + PM1A_CNT_PORT.store(pm1a, Ordering::Relaxed); + SLP_TYPA.store(slp_typ, Ordering::Relaxed); + log!("ACPI: PM1a={pm1a:#x} SLP_TYPa={slp_typ}"); +} + +pub fn can_reset() -> bool { + RESET_PORT.load(Ordering::Relaxed) != 0 +} + +/// Write the reset register and nothing else: no lock, nothing but the port +/// the FADT named. A machine with no reset register halts. +// No fallback: 0xCF9, the keyboard controller and anything else are written only where a table named them. +pub fn reset() -> ! { + let port = RESET_PORT.load(Ordering::Relaxed); + if port != 0 { + // SAFETY: the port is non-zero only where `init_reset` decoded an 8-bit System I/O register, and the value is that register's. + unsafe { cpu::outb(port, RESET_VALUE.load(Ordering::Relaxed)) }; + } + cpu::halt() +} + +/// Enter S5, or halt on a machine whose tables named no soft-off. +pub fn off() -> ! { + let pm1a = PM1A_CNT_PORT.load(Ordering::Relaxed); + if pm1a != 0 { + let val = (SLP_TYPA.load(Ordering::Relaxed) << 10) | SLP_EN; + // SAFETY: pm1a and slp_typ come only from the validated FADT parse via PM1A_CNT_PORT/SLP_TYPA, and the zero check above confirms that parse happened. + unsafe { cpu::outw(pm1a, val) }; + } + cpu::halt() +} + +/// Scan DSDT AML bytecode for the \_S5_ package and extract SLP_TYPa. +// Bounded by the table's declared length via [`Table::byte`]. +fn find_s5_slp_typ(dsdt: &Table

) -> Option { + let s5 = b"_S5_"; + let len = dsdt.len(); + + for i in SDT_HEADER_LEN..len.saturating_sub(7) { + if (0..4).any(|j| dsdt.byte(i + j) != Some(s5[j])) { + continue; + } + if dsdt.byte(i + 4) != Some(0x12) { + continue; + } + + let pkg_lead = dsdt.byte(i + 5)?; + let pkg_len_bytes = match (pkg_lead >> 6) & 0x03 { + 0 => 1usize, + n => (n + 1) as usize, + }; + + // Skip: "_S5_"(4) + PackageOp(1) + PkgLength + NumElements(1) + let val_off = i + 4 + 1 + pkg_len_bytes + 1; + let byte = dsdt.byte(val_off)?; + return Some(if byte == 0x0A { + dsdt.byte(val_off + 1)? as u16 + } else { + byte as u16 + }); + } + + None +} diff --git a/kernel/src/deadline.rs b/kernel/src/deadline.rs index 4c4bd89dece..8fdc1d07159 100644 --- a/kernel/src/deadline.rs +++ b/kernel/src/deadline.rs @@ -221,7 +221,7 @@ fn expire() -> ! { // The seal first, because the USB stop `reset_now` makes before it writes // the register is bounded but not instant, and this record is the // diagnostic the whole mechanism exists for. - crate::drivers::acpi::reset_now() + crate::power::reset_now() } /// What a sealed record opens with, and so what the next boot's loader prints diff --git a/kernel/src/drivers/acpi.rs b/kernel/src/drivers/acpi.rs index 7ea329f7195..8d5dde043ef 100644 --- a/kernel/src/drivers/acpi.rs +++ b/kernel/src/drivers/acpi.rs @@ -3,31 +3,17 @@ //! The decode is `toyos-acpi`, pure and host-tested against QEMU's own tables //! and a crafted corpus. What stays here is everything that touches the //! machine: the direct-map reader the crate decodes through, the log lines a -//! machine owner reads a refusal off, the `Vec`s the crate cannot allocate, and -//! the PM1 port writes that turn a validated FADT into a soft-off. +//! machine owner reads a refusal off, and the `Vec`s the crate cannot allocate. //! //! All input is firmware-supplied and untrusted: no panic on any input path, //! every failure is a [`TableError`] and the caller decides what it means. -//! -//! **No reset this kernel performs leaves a USB device mid-command.** -//! [`reset_now`] and [`shutdown`] are the only two places this kernel writes a -//! register that ends the machine, and each stops every xHCI controller -//! ([`stop::before_reset`]) before it does — which is what makes that a property -//! of the reset rather than of whoever asked for one, and what a third caller -//! gets without knowing it is owed. Resets this kernel does not perform — a TCO -//! or firmware watchdog, a triple fault, power loss — are outside it and always -//! will be. use alloc::vec::Vec; use core::mem::size_of; use core::ptr::{read_unaligned, read_volatile}; -use core::sync::atomic::{AtomicU16, AtomicU8, Ordering}; -use crate::drivers::xhci::stop; use crate::log; use crate::DirectMap; -use toyos_acpi::{ - Century, MadtEntry, Mapped, Memory, Reset, CMOS_RAM, MADT_ENTRIES, SDT_HEADER_LEN, SDT_REVISION, -}; +use toyos_acpi::{Century, MadtEntry, Mapped, Memory, CMOS_RAM, MADT_ENTRIES, SDT_HEADER_LEN, SDT_REVISION}; pub use toyos_acpi::{IoApicEntry, SourceOverride, TableError}; @@ -61,11 +47,7 @@ pub fn direct_phys() -> DirectPhys { pub struct Table(toyos_acpi::Table); impl Table { - pub fn open(base: u64, signature: &[u8; 4], needed: usize) -> Result { - toyos_acpi::Table::open(direct_phys(), base, signature, needed).map(Table) - } - - /// The declared length, already bounded by [`Table::open`]. + /// The declared length, already bounded by [`find_table`]. pub fn len(&self) -> usize { self.0.len() } @@ -132,14 +114,6 @@ pub fn inventory(rsdp_addr: u64) { ); } -const SLP_EN: u16 = 1 << 13; - -static PM1A_CNT_PORT: AtomicU16 = AtomicU16::new(0); -static SLP_TYPA: AtomicU16 = AtomicU16::new(0); - -static RESET_PORT: AtomicU16 = AtomicU16::new(0); -static RESET_VALUE: AtomicU8 = AtomicU8::new(0); - /// Log a refusal with the reason, and hand the caller `None`. // Never a panic: a machine owner needs to see the reason, not have the kernel die on a firmware defect. fn refuse(what: &str, error: TableError) -> Option { @@ -165,64 +139,6 @@ pub fn find_ecam_base(rsdp_addr: u64) -> Option<(u64, u16)> { Some((base, segment)) } -/// Parse FADT and DSDT to prepare for ACPI shutdown. -// A machine without them keeps booting without soft-off rather than panicking. -// The reset register is not decoded here: it is `init_reset`, which runs before -// the boot has anything a panic could report on. -pub fn init_power(rsdp_addr: u64) { - const FADT_FOR_POWER: usize = toyos_acpi::FADT_PM1A_CNT_BLK + size_of::(); - const FADT_FOR_X_DSDT: usize = toyos_acpi::FADT_X_DSDT + size_of::(); - - let fadt = match find_table(rsdp_addr, b"FACP", FADT_FOR_POWER) { - Ok(table) => table, - Err(e) => { - log!("ACPI: FADT unusable: {e:?} — no soft-off, shutdown will halt instead"); - return; - } - }; - - let Some(pm1a) = fadt.field::(toyos_acpi::FADT_PM1A_CNT_BLK) else { - log!("ACPI: FADT has no PM1a control block — no soft-off"); - return; - }; - let pm1a = pm1a as u16; - if pm1a != 0 && !crate::arch::pio::EXISTS { - log!("ACPI: FADT puts PM1a control at port {pm1a:#x}, and this machine has no I/O port space — no soft-off"); - return; - } - - // Prefer X_DSDT over DSDT; a revision claiming 2.0 doesn't prove the field is present, so the length is checked rather than trusting the revision alone. - let dsdt_addr = toyos_acpi::dsdt_address(&fadt.0); - if dsdt_addr == 0 { - log!( - "ACPI: FADT names no DSDT (rev {:?}, needs {FADT_FOR_X_DSDT} bytes for X_DSDT) — no soft-off", - fadt.field::(SDT_REVISION) - ); - return; - } - - let dsdt = match Table::open(dsdt_addr, b"DSDT", SDT_HEADER_LEN) { - Ok(table) => table, - Err(TableError::Unmapped { .. }) => { - log!("ACPI: FADT points the DSDT at {dsdt_addr:#x}, which is not an address — no soft-off"); - return; - } - Err(e) => { - log!("ACPI: DSDT at {dsdt_addr:#x} unusable: {e:?} — no soft-off"); - return; - } - }; - - let Some(slp_typ) = find_s5_slp_typ(&dsdt) else { - log!("ACPI: no \\_S5_ package in the DSDT — no soft-off"); - return; - }; - - PM1A_CNT_PORT.store(pm1a, Ordering::Relaxed); - SLP_TYPA.store(slp_typ, Ordering::Relaxed); - log!("ACPI: PM1a={pm1a:#x} SLP_TYPa={slp_typ}"); -} - /// FADT revision and the IA-PC boot architecture flags. // `Err` is not "absent" and must not be treated as one by the caller. pub fn iapc_boot_arch(rsdp_addr: u64) -> Result<(u8, u16), TableError> { @@ -256,103 +172,6 @@ pub fn rtc_century_register(rsdp_addr: u64) -> Result, TableError> { } } -/// Record the FADT's reset register, or say by name why this machine has none. -/// -/// The one decode of it, and it runs before `percpu::init_bsp` rather than with -/// the rest of the power tables: from the moment that function loads the IDT, -/// every panic can be reported, and a panic that can be reported but not ended -/// is a machine that still needs a hand. Walking these tables inside the panic -/// handler instead is refused — a table walk on a machine that has already -/// failed once is how a panic becomes a triple fault. -pub fn init_reset(rsdp_addr: u64) { - let fadt = match find_table(rsdp_addr, b"FACP", toyos_acpi::FADT_FOR_RESET) { - Ok(table) => table, - Err(e) => { - log!("ACPI: FADT unusable: {e:?} — no reboot, a panic will hold the panel"); - return; - } - }; - match toyos_acpi::reset_register(&fadt.0) { - Reset::Port { port, .. } if !crate::arch::pio::EXISTS => { - log!("ACPI: the reset register is SystemIO {port:#x}, and this machine has no I/O port space — no reboot"); - } - Reset::Port { port, value } => { - RESET_PORT.store(port, Ordering::Relaxed); - RESET_VALUE.store(value, Ordering::Relaxed); - log!("ACPI: reset register SystemIO {port:#x} <- {value:#04x}"); - } - other => log!("ACPI: no reset register this kernel writes ({other:?}) — no reboot"), - } -} - -pub fn can_reboot() -> bool { - RESET_PORT.load(Ordering::Relaxed) != 0 -} - -/// Return the machine to firmware through the FADT's reset register. -// No fallback: 0xCF9, the keyboard controller and anything else are written only where a table named them. -pub fn reboot() -> ! { - crate::drivers::serial::flush_final(); - // Kernel-internal, so a bug rather than a machine quiesced and then left halted quietly. - assert!( - RESET_PORT.load(Ordering::Relaxed) != 0, - "reboot: no reset register, and the caller did not ask can_reboot() first" - ); - reset_now() -} - -/// Write the reset register and nothing else. -/// -/// **[`reboot`] is not reachable from a wedge**, which is why this exists -/// beside it: that path opens with `serial::flush_final`, and a `BackendGuard` -/// masks interrupts for its whole life — so a CPU stuck inside one holds what -/// the call would wait for, on exactly the boots `crate::deadline` exists for. -/// This takes no lock and touches nothing but the port the FADT named. -/// -/// A machine with no reset register halts here rather than returning: the -/// caller has already sealed why, and holding is what such a machine has always -/// done. -pub fn reset_now() -> ! { - // **Here and not at either caller.** `stop::before_reset` is registers and - // nothing else — written for the panic path, so it takes no lock and - // allocates nothing, which is the only kind of call a wedge may make — and - // it *appends* its account to whatever this boot already sealed, so a - // `WEDGED` page carries what the reset did to USB the way a `PANIC` one - // does. A caller seals first and calls this second: the record is the - // diagnostic the seal exists for and may not be lost to a stop that does - // not return. - stop::before_reset(); - let port = RESET_PORT.load(Ordering::Relaxed); - if port == 0 { - crate::arch::cpu::halt(); - } - // SAFETY: the port is non-zero only where `init_reset` decoded an 8-bit System I/O register, and the value is that register's. - unsafe { crate::arch::pio::outb(port, RESET_VALUE.load(Ordering::Relaxed)) }; - - crate::arch::cpu::halt(); -} - -/// Trigger ACPI S5 (soft-off) shutdown. -pub fn shutdown() -> ! { - // Last chance: nothing drains the log ring after this point. - crate::drivers::serial::flush_final(); - // S5 takes VBUS with it on a machine whose ports are not always-on and - // takes nothing on one whose are, so the devices are handed back here for - // the same reason as at a reboot. - stop::before_reset(); - - let pm1a = PM1A_CNT_PORT.load(Ordering::Relaxed); - let slp_typ = SLP_TYPA.load(Ordering::Relaxed); - - if pm1a != 0 { - let val = (slp_typ << 10) | SLP_EN; - // SAFETY: pm1a and slp_typ come only from the validated FADT parse via PM1A_CNT_PORT/SLP_TYPA, and the zero check above confirms that parse happened. - unsafe { crate::arch::pio::outw(pm1a, val) }; - } - - crate::arch::cpu::halt(); -} - /// Given the RSDP address, parse XSDT -> HPET table -> return HPET MMIO base address. pub fn find_hpet_base(rsdp_addr: u64) -> Option { let base = match toyos_acpi::hpet_base(direct_phys(), rsdp_addr) { @@ -405,36 +224,3 @@ pub fn parse_madt(rsdp_addr: u64) -> Option { log!("ACPI: MADT cpus={:?}", apic_ids); Some(MadtInfo { apic_ids, io_apics, source_overrides }) } - -/// Scan DSDT AML bytecode for the \_S5_ package and extract SLP_TYPa. -// Bounded by the table's declared length via [`Table::field`]. -fn find_s5_slp_typ(dsdt: &Table) -> Option { - let s5 = b"_S5_"; - let len = dsdt.len(); - - for i in SDT_HEADER_LEN..len.saturating_sub(7) { - if (0..4).any(|j| dsdt.field::(i + j) != Some(s5[j])) { - continue; - } - if dsdt.field::(i + 4) != Some(0x12) { - continue; - } - - let pkg_lead = dsdt.field::(i + 5)?; - let pkg_len_bytes = match (pkg_lead >> 6) & 0x03 { - 0 => 1usize, - n => (n + 1) as usize, - }; - - // Skip: "_S5_"(4) + PackageOp(1) + PkgLength + NumElements(1) - let val_off = i + 4 + 1 + pkg_len_bytes + 1; - let byte = dsdt.field::(val_off)?; - return Some(if byte == 0x0A { - dsdt.field::(val_off + 1)? as u16 - } else { - byte as u16 - }); - } - - None -} diff --git a/kernel/src/drivers/serial.rs b/kernel/src/drivers/serial.rs index 10a9acd2f5f..813b2224a29 100644 --- a/kernel/src/drivers/serial.rs +++ b/kernel/src/drivers/serial.rs @@ -161,7 +161,7 @@ pub unsafe fn panic_flush() { } /// Drains the ring before the machine powers off, so the tail of a shutdown -/// is not lost to `acpi::shutdown()` cutting power with logs still queued. +/// is not lost to `power::shutdown()` cutting power with logs still queued. /// /// Bounded on the wire like `panic_flush`, but never bypasses: every CPU is /// still live here, and reading the ring unsynchronized is only safe once diff --git a/kernel/src/hardlockup/mod.rs b/kernel/src/hardlockup/mod.rs index 66f02bb5e9d..425fd2e1b77 100644 --- a/kernel/src/hardlockup/mod.rs +++ b/kernel/src/hardlockup/mod.rs @@ -28,7 +28,7 @@ //! stuck: it seals a `WEDGED` record naming itself, its `pc` and `sp` from the //! NMI frame, the lock it is spinning on if `Lock::lock` recorded one, a line //! for every other CPU, and the tail of the log ring — then writes the reset -//! register through `acpi::reset_now`. +//! register through `power::reset_now`. //! //! # The discipline this file is written under //! @@ -284,7 +284,7 @@ fn locked_up(me: usize, pc: u64, sp: u64, now: u64) -> ! { // The seal first, because the USB stop `reset_now` makes before it writes // the register is bounded but not instant, and this record is the // diagnostic the whole mechanism exists for. - crate::drivers::acpi::reset_now() + crate::power::reset_now() } /// What this CPU was waiting for before a nested acquisition took the slot, so diff --git a/kernel/src/main.rs b/kernel/src/main.rs index e91c58580f8..8c968345d63 100644 --- a/kernel/src/main.rs +++ b/kernel/src/main.rs @@ -36,6 +36,7 @@ mod hardlockup; mod mm; mod panic; mod panic_reboot; +mod power; mod keyboard; mod mouse; @@ -458,7 +459,6 @@ pub(crate) unsafe extern "C" fn kernel_main(kernel_args: &KernelArgs) -> ! { arch::watchdog::init(&pci_devices); file_cache::init(); gpt::init(kernel_args); - acpi::init_power(kernel_args.rsdp_addr); boot_phase!("peripherals ready", t_periph); diff --git a/kernel/src/panic_reboot.rs b/kernel/src/panic_reboot.rs index b36141ae16f..9a071579bbb 100644 --- a/kernel/src/panic_reboot.rs +++ b/kernel/src/panic_reboot.rs @@ -11,14 +11,9 @@ //! both phases then compare the same counter against the same unit. A machine //! that states no frequency and has no calibrated clock cannot time anything, //! and the arm line says so instead of resetting on a guess. -//! -//! The reset is the FADT's; a machine whose reset register this kernel could -//! not decode is refused by name and holds, with no fallback. That register is -//! decoded before `percpu::init_bsp` loads the IDT, so every panic that can -//! reach this path at all has one to write. use crate::arch::cpu; -use crate::drivers::{acpi, serial}; +use crate::drivers::serial; use crate::time::{Budget, Duration}; /// The shipped bound. @@ -118,7 +113,7 @@ pub fn arm(on_the_record: bool) -> Bound { // The clock and the reset register are two separate ways to have no // reboot, and a line naming one when the other is what failed answers // whoever reads the panel with nothing. - match (deadline(budget), acpi::can_reboot()) { + match (deadline(budget), crate::power::can_reboot()) { (Some((cycles, source)), true) => { if on_the_record { alert!( @@ -163,7 +158,7 @@ pub fn reboot_now() -> ! { b"\npanic: no key inside the bound, so nobody is here: returning this machine to \ firmware\n", ); - // Not `acpi::reboot`: its flush waits on the console wire, and a CPU this + // Not `power::reboot`: its flush waits on the console wire, and a CPU this // panic stopped may be holding it. - acpi::reset_now() + crate::power::reset_now() } diff --git a/kernel/src/power.rs b/kernel/src/power.rs new file mode 100644 index 00000000000..0b39a9d641c --- /dev/null +++ b/kernel/src/power.rs @@ -0,0 +1,59 @@ +//! The machine's two ends this kernel performs, a reset and a power-off, each +//! the architecture's own (`arch::power`). +//! +//! **No reset this kernel performs leaves a USB device mid-command.** +//! [`reset_now`] and [`shutdown`] are the only two places this kernel ends the +//! machine, and each stops every xHCI controller ([`stop::before_reset`]) +//! before it does — which is what makes that a property of the reset rather +//! than of whoever asked for one, and what a third caller gets without knowing +//! it is owed. Resets this kernel does not perform — a TCO or firmware +//! watchdog, a triple fault, power loss — are outside it and always will be. + +use crate::drivers::{serial, xhci::stop}; + +/// Whether this machine has a reset this kernel can perform. +pub fn can_reboot() -> bool { + crate::arch::power::can_reset() +} + +/// Return the machine to firmware. +pub fn reboot() -> ! { + serial::flush_final(); + // Kernel-internal, so a bug rather than a machine quiesced and then left halted quietly. + assert!(can_reboot(), "reboot: no reset, and the caller did not ask can_reboot() first"); + reset_now() +} + +/// Reset the machine and do nothing else. +/// +/// **[`reboot`] is not reachable from a wedge**, which is why this exists +/// beside it: that path opens with `serial::flush_final`, and a `BackendGuard` +/// masks interrupts for its whole life — so a CPU stuck inside one holds what +/// the call would wait for, on exactly the boots `crate::deadline` exists for. +/// This takes no lock. +/// +/// A machine with no reset halts here rather than returning: the caller has +/// already sealed why, and holding is what such a machine has always done. +pub fn reset_now() -> ! { + // **Here and not at either caller.** `stop::before_reset` is registers and + // nothing else — written for the panic path, so it takes no lock and + // allocates nothing, which is the only kind of call a wedge may make — and + // it *appends* its account to whatever this boot already sealed, so a + // `WEDGED` page carries what the reset did to USB the way a `PANIC` one + // does. A caller seals first and calls this second: the record is the + // diagnostic the seal exists for and may not be lost to a stop that does + // not return. + stop::before_reset(); + crate::arch::power::reset() +} + +/// Power the machine off, or halt on one that offers no power-off. +pub fn shutdown() -> ! { + // Last chance: nothing drains the log ring after this point. + serial::flush_final(); + // A power-off takes VBUS with it on a machine whose ports are not + // always-on and takes nothing on one whose are, so the devices are handed + // back here for the same reason as at a reboot. + stop::before_reset(); + crate::arch::power::off() +} diff --git a/kernel/src/syscall/machine.rs b/kernel/src/syscall/machine.rs index 2c1a2e6ea93..a613ec4cd67 100644 --- a/kernel/src/syscall/machine.rs +++ b/kernel/src/syscall/machine.rs @@ -7,7 +7,7 @@ use alloc::vec::Vec; -use crate::drivers::acpi; +use crate::power; use crate::user_ptr::{SyscallContext, UserBytesMut}; use crate::UserAddr; use crate::{log, process}; @@ -142,7 +142,7 @@ fn quiesce(last: &str) -> Result<(), SyscallError> { // volumes still have bytes to take and this takes the controller away from // them; and after everything else here, // because nothing may run between it and the register stop - // `acpi::reboot`/`acpi::shutdown` do — which every reset this kernel + // `power::reboot`/`power::shutdown` do — which every reset this kernel // performs goes through. It is bounded, and the reset follows either way. crate::drivers::xhci::seal_shut(); Ok(()) @@ -170,11 +170,11 @@ pub(super) fn sys_shutdown(syscap: RawHandle) -> u64 { if let Err(e) = quiesce("Shutting down.") { return e.to_u64(); } - acpi::shutdown(); + power::shutdown(); } /// Returns the machine to firmware; requires a `SysCap` carrying [`Rights::POWER`]. Returns only when refused. -// The register is demanded before anything is torn down: a machine whose FADT names none is left running, not synced, stopped and still on. +// The reset is demanded before anything is torn down: a machine without one is left running, not synced, stopped and still on. pub(super) fn sys_reboot(syscap: RawHandle) -> u64 { if let Err(e) = demand_syscap(syscap, Rights::POWER) { return e.refuse(); @@ -182,14 +182,14 @@ pub(super) fn sys_reboot(syscap: RawHandle) -> u64 { if refused_once() { return SyscallError::NotSupported.to_u64(); } - if !acpi::can_reboot() { - log!("reboot: this machine's FADT names no reset register — refused"); + if !power::can_reboot() { + log!("reboot: this machine has no reset this kernel performs — refused"); return SyscallError::NotSupported.to_u64(); } if let Err(e) = quiesce("Rebooting.") { return e.to_u64(); } - acpi::reboot(); + power::reboot(); } /// The most live threads `SYS_SYSINFO` will describe; kept under `mm::MAX_HEAP_ALLOC` so an unbounded thread count cannot trip the allocator's fail-fast assert. diff --git a/src/build.rs b/src/build.rs index 15d9f5386ea..b69012db8f7 100644 --- a/src/build.rs +++ b/src/build.rs @@ -3166,6 +3166,7 @@ mod tests { "tests/updatecase/system.toml", "tests/virtjobcase/system.toml", "tests/virtpaniccase/system.toml", + "tests/virtrebootcase/system.toml", "tests/virtsmpcase/system.toml", ]; diff --git a/tests/checks.rs b/tests/checks.rs index 0bfca6e3cd7..f6d82be9a98 100644 --- a/tests/checks.rs +++ b/tests/checks.rs @@ -941,4 +941,32 @@ mod checks { .expect("03_struct's expectation is staged"); assert_eq!(staged.trim_end(), got); } + + /// `arm_psci_call` as QEMU 11.1.1's `trace-events` formats it, with and + /// without the log backend's `pid@time:` head, and every way a power-off's + /// trace falls short of PSCI's recipe refused. + #[test] + fn psci_power_off_judge() { + let line = |head: &str, function: u64, cpu: u64| { + format!("{head}arm_psci_call PSCI Call x0=0x{function:016x} x1=0x0000000000000000 x2=0x0000000000000000 x3=0x0000000000000000 cpuid=0x{cpu:x}") + }; + let text = [line("", PSCI_CPU_OFF, 1), line("4242@1759272000.123456:", PSCI_SYSTEM_OFF, 0)].join("\n"); + assert_eq!(psci_calls(&text), Ok(vec![(PSCI_CPU_OFF, 1), (PSCI_SYSTEM_OFF, 0)])); + assert!(psci_calls("arm_psci_call PSCI Call x0=?").is_err()); + + let (off, system_off) = (PSCI_CPU_OFF, PSCI_SYSTEM_OFF); + let affinity_info = 0xC400_0004; + assert_eq!(psci_powered_off(&[(off, 1), (affinity_info, 3), (off, 2), (off, 0), (system_off, 3)], 4), Ok(3)); + for short in [ + vec![(off, 1), (off, 0), (system_off, 3)], + vec![(off, 1), (off, 0), (system_off, 3), (off, 2)], + vec![(off, 1), (off, 1), (off, 2), (off, 0), (system_off, 3)], + vec![(off, 1), (off, 2), (off, 0)], + vec![(off, 1), (off, 2), (off, 0), (system_off, 3), (system_off, 3)], + vec![(off, 1), (off, 2), (off, 0), (PSCI_SYSTEM_RESET, 3), (system_off, 3)], + vec![(off, 1), (off, 2), (off, 3), (system_off, 3)], + ] { + assert!(psci_powered_off(&short, 4).is_err(), "{short:x?}"); + } + } } diff --git a/tests/common/power.rs b/tests/common/power.rs index 03421a895ed..efe4f8d85c3 100644 --- a/tests/common/power.rs +++ b/tests/common/power.rs @@ -72,6 +72,41 @@ pub fn machine_reboot( Ok(()) } +/// The machine powers off when a process holding `POWER` asks it to: ACPI S5 +/// through the PM1a control block the FADT names, with the DSDT's `\_S5_`. +pub fn machine_shutdown( + test_config: &Path, + c_bins: &[(String, Vec)], + rust_bins: &[(String, Vec)], +) -> Result<(), String> { + let options = BootOptions { qmp: true, ..Default::default() }; + let mut qemu = QemuInstance::boot_with_options(test_config, c_bins, rust_bins, options); + + let boot = serial::Serial::boot(&qemu); + boot.must_be_clean()?; + // q35's ICH9 power-management block, and its `\_S5_`. + boot.must_say("ACPI: PM1a=0x604 SLP_TYPa=0")?; + + let mut stop = qemu::QmpShutdown::open(qemu.qmp_socket(), qemu.budget(WAIT)); + + writeln!(qemu.stdin_mut(), "run shutdown").expect("write to QEMU stdin"); + qemu.flush_stdin(); + let reason = stop.reason(); + let tail = qemu.drain_serial(WAIT); + + let drain = serial::Serial::named("shutdown drain", tail.as_str()); + drain.must_be_clean()?; + drain.must_say("Shutting down.")?; + match reason.as_deref() { + Some("guest-shutdown") => {} + Some(seen) => return Err(format!("QEMU stopped this guest for {seen:?}, not a power-off\n{tail}")), + None => return Err(format!("QEMU never reported stopping: the guest asked to power off and stayed up\n{tail}")), + } + + eprintln!(" [power] QEMU stopped the guest for guest-shutdown"); + Ok(()) +} + /// A boot with no host on the console runs its manifest's jobs and ends /// itself, and the loader's own account of it is on the stick beside `logd`'s. pub fn metal_job_reboot( @@ -2357,7 +2392,7 @@ const STICK_ENUMERATED: &str = "usb-storage: 1 device(s)"; /// Every way this kernel resets a machine, and the account each one leaves. /// -/// `acpi::reboot` and `acpi::shutdown` are the two resets this kernel performs, +/// `power::reboot` and `power::shutdown` are the two resets this kernel performs, /// and three different things reach them: a job list's last `reboot`, the test /// runner's own job deadline, and the panic console's bound. Each arm is a /// chained boot, so the pass after the reset can be read. diff --git a/tests/common/qemu.rs b/tests/common/qemu.rs index 7feb27c1729..33071ada5f2 100644 --- a/tests/common/qemu.rs +++ b/tests/common/qemu.rs @@ -2209,6 +2209,10 @@ pub struct BootOptions { /// controller start into this file: the device's own account /// of what reached it, which no line a driver prints can be. pub nvme_trace: Option, + /// Have QEMU record every PSCI call a vCPU makes into this file, with the + /// calling CPU's affinity: the firmware side's own account of what the + /// kernel asked of it. + pub psci_trace: Option, } /// Where the guest sees the host under QEMU's user-mode networking, and where @@ -2289,6 +2293,7 @@ impl Default for BootOptions { wire_dump: None, userland_nvme: None, nvme_trace: None, + psci_trace: None, } } } @@ -4363,6 +4368,14 @@ fn qemu_command( } qemu.arg("-D").arg(trace); } + if let Some(trace) = &options.psci_trace { + assert!( + options.profile.arch() == Arch::Aarch64 && options.profile.accel() == Accel::Tcg, + "a PSCI trace is TCG's `arm_psci_call`, and this profile's PSCI is not QEMU's TCG" + ); + assert!(options.nvme_trace.is_none(), "QEMU has one `-D` log, and the NVMe trace holds it"); + qemu.arg("-trace").arg("arm_psci_call").arg("-D").arg(trace); + } // The mass-storage devices beside the boot stick, and the only ones a test // may write to: the boot stick is on the same bus and carries the image the diff --git a/tests/toyos.rs b/tests/toyos.rs index f031b536ad4..900ab62919e 100644 --- a/tests/toyos.rs +++ b/tests/toyos.rs @@ -522,6 +522,7 @@ const SCREEN_TESTS: &[(&str, Sched, qemu::Profile)] = &[ ("virt_el1_smp", Sched::Parallel, qemu::Profile::VirtTcg), ("virt_failed_ap_leaves_no_hole", Sched::Parallel, qemu::Profile::VirtEl2), ("virt_fatal_halts_the_others_first", Sched::Parallel, qemu::Profile::VirtEl2), + ("virt_reboot", Sched::Parallel, qemu::Profile::VirtEl2), ]; /// What `screen_console_shell` types, and what it then looks for on its own. @@ -894,6 +895,7 @@ const MACHINE_TESTS: &[(&str, Sched)] = &[ // One boot of its own, because it ends the machine. Every verdict is a // kernel line or the stop reason QEMU reported; no clock is in either. ("machine_reboot", Sched::Parallel), + ("machine_shutdown", Sched::Parallel), // Its own boot: every verdict is a console line, QEMU's stop reason or a record off the image. ("metal_job_reboot", Sched::Parallel), // Its own boot, and the one whose numbers are the T14's: what it judges @@ -3557,7 +3559,7 @@ fn virt_copyout(arch: toyos_build::arch::Arch) -> &'static [u8] { fn virt_job(profile: qemu::Profile, job: &str, said: &str) -> Result<(), String> { let config = compile::repo_root().join("tests/virtjobcase/system.toml"); let case = config.parent().expect("system.toml has a directory"); - let qemu = QemuInstance::boot_with_options( + let mut qemu = QemuInstance::boot_with_options( case, &[], &[], @@ -3570,15 +3572,15 @@ fn virt_job(profile: qemu::Profile, job: &str, said: &str) -> Result<(), String> ..Default::default() }, ); - judge_virt_job(qemu, job, said).map(drop) + judge_virt_job(&mut qemu, job, said).map(drop) } /// Wait for `job`'s end on a guest booted with it, and judge it: it ends with /// exit 0, having said `said`. Answers everything the PL011 carried. -fn judge_virt_job(mut qemu: QemuInstance, job: &str, said: &str) -> Result { +fn judge_virt_job(qemu: &mut QemuInstance, job: &str, said: &str) -> Result { let end = format!("===TEST_END {job} "); let mut rest = String::new(); - let waited = await_marker(&mut qemu, &mut rest, &end, &format!("the job {job} to end")); + let waited = await_marker(qemu, &mut rest, &end, &format!("the job {job} to end")); let serial = format!("{}\n{rest}", qemu.boot_log()); if let Err(why) = waited { return Err(format!("{why}\nserial:\n{serial}")); @@ -3605,21 +3607,15 @@ const UNMAP_TOUCH_SAID: &str = /// The CPUs `virt_smp` boots. const VIRT_CPUS: u32 = 8; -/// Boot `tests/virtsmpcase` under `profile` on `cpus` CPUs, with `params` armed. -fn boot_virt_smp(profile: qemu::Profile, cpus: u32, params: &'static [&'static str]) -> QemuInstance { +/// Boot `tests/virtsmpcase` as `options` say. +fn boot_virt_smp(options: BootOptions) -> QemuInstance { let config = compile::repo_root().join("tests/virtsmpcase/system.toml"); let case = config.parent().expect("system.toml has a directory"); QemuInstance::boot_with_options( case, &[], &[], - BootOptions { - profile, - smp: cpus, - kernel_params: params, - ready_marker: "control registers: SCTLR_EL1=", - ..Default::default() - }, + BootOptions { ready_marker: "control registers: SCTLR_EL1=", ..options }, ) } @@ -3627,9 +3623,22 @@ fn boot_virt_smp(profile: qemu::Profile, cpus: u32, params: &'static [&'static s /// firmware enters every CPU at EL`el` and whose FADT names PSCI's `conduit`: /// each CPU is started by `CPU_ON`, holds the control-register declaration as /// entered there and joins the scheduler, and the case's job `unmap_touch` -/// ends with exit 0. +/// ends with exit 0. Then its job `shutdown` stops the machine and powers it +/// off, every other CPU turned off first ([`psci_powered_off`]). fn virt_smp(profile: qemu::Profile, conduit: &str, el: u32) -> Result<(), String> { - let serial = judge_virt_job(boot_virt_smp(profile, VIRT_CPUS, &[]), "unmap_touch", UNMAP_TOUCH_SAID)?; + let trace = common::lane::dir().join(format!("virt_smp-{conduit}.psci")); + let _ = fs::remove_file(&trace); + let mut qemu = boot_virt_smp(BootOptions { + profile, + smp: VIRT_CPUS, + qmp: true, + psci_trace: Some(trace.clone()), + ..Default::default() + }); + // Before the job list can reach its `shutdown`: QMP delivers no event + // emitted before its client connected. + let mut stop = qemu::QmpShutdown::open(qemu.qmp_socket(), qemu.budget(qemu::GUEST_QUIET)); + let serial = judge_virt_job(&mut qemu, "unmap_touch", UNMAP_TOUCH_SAID)?; let psci = serial.lines().find(|l| l.contains("PSCI: ")).unwrap_or_default(); if !psci.contains(&format!(" through {conduit}")) { return Err(format!("PSCI is not said to be reached through {conduit}: {psci:?}\nserial:\n{serial}")); @@ -3651,17 +3660,108 @@ fn virt_smp(profile: qemu::Profile, conduit: &str, el: u32) -> Result<(), String } } eprintln!(" [virt] {VIRT_CPUS} CPUs entered at EL{el}, started through {conduit}, and scheduling"); + let (console, calls) = ended_through_psci(&mut qemu, &mut stop, serial, SHUTTING_DOWN, "guest-shutdown", &trace)?; + let record = console + .lines() + .find_map(toyos_quiesce::Record::parse) + .ok_or_else(|| format!("the kernel wrote no stop record\n{console}"))?; + if record.cpus != VIRT_CPUS { + return Err(format!("the stop ran across {} CPUs, not {VIRT_CPUS}: {record}\n{console}", record.cpus)); + } + let last = psci_powered_off(&calls, VIRT_CPUS)?; + eprintln!(" [virt] {record}; every CPU but {last:#x} called CPU_OFF, then {last:#x} SYSTEM_OFF"); Ok(()) } +/// The kernel's last word when it powers the machine off. +const SHUTTING_DOWN: &str = "Shutting down."; + +/// PSCI function IDs, as `arm_psci_call` prints `x0` (Arm DEN0022 §5.1). +const PSCI_CPU_OFF: u64 = 0x8400_0002; +const PSCI_SYSTEM_OFF: u64 = 0x8400_0008; +const PSCI_SYSTEM_RESET: u64 = 0x8400_0009; + +/// Wait for the boot's `last` word on a guest whose job list asked for its +/// end, then for QEMU to stop for `reason` and exit; `console` is what it has +/// said so far. Nothing may follow the last word. Answers the whole console +/// and every PSCI call QEMU traced into `trace`, in order. +fn ended_through_psci( + qemu: &mut QemuInstance, + stop: &mut qemu::QmpShutdown, + mut console: String, + last: &str, + reason: &str, + trace: &Path, +) -> Result<(String, Vec<(u64, u64)>), String> { + await_marker(qemu, &mut console, last, "the boot's last word")?; + let stopped = stop.reason(); + let by = qemu.budget(qemu::GUEST_QUIET); + console.push_str(&qemu.await_exit(by)?); + if stopped.as_deref() != Some(reason) { + return Err(format!("QEMU stopped this guest for {stopped:?}, not {reason:?}\n{console}")); + } + let lines: Vec<&str> = console.lines().collect(); + let at = lines.iter().position(|l| l.contains(last)).expect("awaited above"); + let after: Vec<&str> = lines[at + 1..].iter().copied().filter(|l| !l.trim().is_empty()).collect(); + if !after.is_empty() { + return Err(format!("{} line(s) after the boot's last word:\n {}", after.len(), after.join("\n "))); + } + let traced = fs::read_to_string(trace).map_err(|e| format!("read the PSCI trace: {e}"))?; + Ok((console, psci_calls(&traced)?)) +} + +/// Every call QEMU's `arm_psci_call` trace `text` records, in order: the +/// function (`x0`) and the affinity of the CPU that made it (`cpuid`). +fn psci_calls(text: &str) -> Result, String> { + let hex = |line: &str, key: &str| -> Option { + let digits: String = + line.split(key).nth(1)?.strip_prefix("0x")?.chars().take_while(char::is_ascii_hexdigit).collect(); + u64::from_str_radix(&digits, 16).ok() + }; + text.lines() + .filter(|l| l.contains("arm_psci_call")) + .map(|l| match (hex(l, " x0="), hex(l, " cpuid=")) { + (Some(function), Some(cpu)) => Ok((function, cpu)), + _ => Err(format!("a PSCI trace line this reader does not know: {l:?}")), + }) + .collect() +} + +/// A power-off by PSCI's own recipe (DEN0022 §5.10.3), read off QEMU's trace +/// of a machine of `cpus` CPUs whose affinities are `0..cpus`: every CPU but +/// one called `CPU_OFF`, each once, and then the one left called `SYSTEM_OFF`, +/// once. Answers that CPU. +fn psci_powered_off(calls: &[(u64, u64)], cpus: u32) -> Result { + let offs: Vec = (0..calls.len()).filter(|&i| calls[i].0 == PSCI_SYSTEM_OFF).collect(); + let [at] = offs[..] else { + return Err(format!("QEMU traced {} SYSTEM_OFF calls, not one: {calls:x?}", offs.len())); + }; + if calls.iter().any(|&(function, _)| function == PSCI_SYSTEM_RESET) { + return Err(format!("a SYSTEM_RESET beside the power-off: {calls:x?}")); + } + let last = calls[at].1; + let mut turned_off: Vec = + calls[..at].iter().filter(|&&(function, _)| function == PSCI_CPU_OFF).map(|&(_, cpu)| cpu).collect(); + turned_off.sort_unstable(); + let others: Vec = (0..u64::from(cpus)).filter(|&cpu| cpu != last).collect(); + if turned_off != others || calls[at..].iter().any(|&(function, _)| function == PSCI_CPU_OFF) { + return Err(format!( + "before {last:#x}'s SYSTEM_OFF, CPU_OFF came from {turned_off:x?}, not from each of {others:x?} \ + once and nothing after: {calls:x?}" + )); + } + Ok(last) +} + /// `smp_failed_ap_leaves_no_hole` on AArch64: `smp-skip-ap` keeps `CPU_ON` /// from the CPU that would be cpu2 of four, and the bring-up stops there, so /// cpu2's id goes to no CPU behind it. The case's job, which the scheduler /// places across the CPUs that came up, ends with exit 0. fn virt_failed_ap_leaves_no_hole(profile: qemu::Profile) -> Result<(), String> { const CPUS: u32 = 4; - let qemu = boot_virt_smp(profile, CPUS, &["smp-skip-ap"]); - let serial = judge_virt_job(qemu, "unmap_touch", UNMAP_TOUCH_SAID)?; + let mut qemu = + boot_virt_smp(BootOptions { profile, smp: CPUS, kernel_params: &["smp-skip-ap"], ..Default::default() }); + let serial = judge_virt_job(&mut qemu, "unmap_touch", UNMAP_TOUCH_SAID)?; // The premise, not just a small machine: cpu1 came up and cpu2 did not. for premise in ["SMP: cpu1 mpidr=0x1 online", "SMP: cpu2 mpidr=0x2 did not echo within"] { if !serial.contains(premise) { @@ -3683,6 +3783,45 @@ fn virt_failed_ap_leaves_no_hole(profile: qemu::Profile) -> Result<(), String> { Ok(()) } +/// `tests/virtrebootcase`'s one job asks for a reboot: the boot's last word is +/// `Rebooting.`, QEMU stops for `guest-reset`, and its trace of PSCI holds one +/// `SYSTEM_RESET` and neither `CPU_OFF` nor `SYSTEM_OFF`. +fn virt_reboot(profile: qemu::Profile) -> Result<(), String> { + let config = compile::repo_root().join("tests/virtrebootcase/system.toml"); + let case = config.parent().expect("system.toml has a directory"); + let trace = common::lane::dir().join("virt_reboot.psci"); + let _ = fs::remove_file(&trace); + let mut qemu = QemuInstance::boot_with_options( + case, + &[], + &[], + BootOptions { + profile, + qmp: true, + psci_trace: Some(trace.clone()), + ready_marker: "control registers: SCTLR_EL1=", + ..Default::default() + }, + ); + let mut stop = qemu::QmpShutdown::open(qemu.qmp_socket(), qemu.budget(qemu::GUEST_QUIET)); + let console = qemu.boot_log().to_string(); + let (console, calls) = ended_through_psci(&mut qemu, &mut stop, console, bootlog::REBOOTING, "guest-reset", &trace)?; + // The runner's own deadline also reboots, and is not the job asking. + if console.contains(bootlog::JOB_DEADLINE_SAID) { + return Err(format!("the reboot was the job list's deadline, not its job\n{console}")); + } + let asked: Vec = calls + .iter() + .map(|&(function, _)| function) + .filter(|&function| [PSCI_CPU_OFF, PSCI_SYSTEM_OFF, PSCI_SYSTEM_RESET].contains(&function)) + .collect(); + if asked != [PSCI_SYSTEM_RESET] { + return Err(format!("QEMU traced {asked:x?} of CPU_OFF, SYSTEM_OFF and SYSTEM_RESET, not one SYSTEM_RESET")); + } + eprintln!(" [virt] Rebooting., then one SYSTEM_RESET, and QEMU stopped for guest-reset"); + Ok(()) +} + /// [`panic_halts_the_others_first`] on AArch64: `tests/virtpaniccase` runs /// `test_rs_panic_halts_first` as its one job on [`STOP_CPUS`] CPUs, and the /// halt SGI stops every CPU but the one going fatal. @@ -5262,6 +5401,7 @@ fn run_screen_test( "virt_el1_smp" => virt_smp(profile, "HVC", 1), "virt_failed_ap_leaves_no_hole" => virt_failed_ap_leaves_no_hole(profile), "virt_fatal_halts_the_others_first" => virt_fatal_halts_the_others_first(profile), + "virt_reboot" => virt_reboot(profile), "screen_late_panic" => { // The ordinary fatal panic, which no userland process can produce: // crash_report, capture, panic_flush, halt_all_cpus, render. The @@ -9960,6 +10100,7 @@ fn run_machine_test( // Body in `tests/common/gpt.rs`, same reason. "boot_partition_identity" => common::gpt::boot_partition_identity(test_config, c_bins, rust_bins), "machine_reboot" => power::machine_reboot(test_config, c_bins, rust_bins), + "machine_shutdown" => power::machine_shutdown(test_config, c_bins, rust_bins), "metal_job_reboot" => power::metal_job_reboot(test_config, c_bins, rust_bins), "metal_device_probe" => devices::metal_device_probe(test_config, c_bins, rust_bins), "job_deadline_reboots" => power::job_deadline_reboots(test_config, c_bins, rust_bins), @@ -11156,7 +11297,7 @@ fn run_machine_test( // on the small one. // // The kernel's own shutdown lines are observable now: the ring - // is drained in `acpi::shutdown()` before it cuts the power. + // is drained in `power::shutdown()` before it cuts the power. // Asserted below, because "how far did the sync get" is the only // diagnostic a shutdown failure has, and on a machine with no // serial it is the only channel there is. diff --git a/tests/virtrebootcase/system.toml b/tests/virtrebootcase/system.toml new file mode 100644 index 00000000000..47714eeaff1 --- /dev/null +++ b/tests/virtrebootcase/system.toml @@ -0,0 +1,18 @@ +# The one job returns the machine to firmware. +[boot] +start = ["logd", "test-runner"] + +[programs.logd] +service = true +syscap = ["logread"] + +[programs.test-runner] +receives = ["power"] +# Four minutes from boot: the job runs on emulated CPUs, and a job past the +# bound reboots the guest with the job named. +args = ["--bound-ms=240000", "reboot"] + +[programs.toybox] + +[symlinks] +"bin/reboot" = "/system/bin/toybox" diff --git a/tests/virtsmpcase/system.toml b/tests/virtsmpcase/system.toml index e7dfb0bc46d..246a4e8b081 100644 --- a/tests/virtsmpcase/system.toml +++ b/tests/virtsmpcase/system.toml @@ -1,5 +1,6 @@ # Every CPU the machine names, and the one job whose premise is more than one -# CPU: a page unmapped beside a thread that is still reading it. +# CPU: a page unmapped beside a thread that is still reading it. Then the +# machine powers off, every CPU but one turned off first. [boot] start = ["logd", "test-runner"] @@ -11,9 +12,10 @@ syscap = ["logread"] receives = ["power"] # Four minutes from boot: every job here runs on emulated CPUs, and a job past # the bound reboots the guest with the job named. -args = ["--bound-ms=240000", "unmap_touch"] +args = ["--bound-ms=240000", "unmap_touch", "shutdown"] [programs.toybox] [symlinks] "bin/unmap_touch" = "/system/bin/toybox" +"bin/shutdown" = "/system/bin/toybox" diff --git a/toyos-gicv3/src/lib.rs b/toyos-gicv3/src/lib.rs index 43bec42c2d4..a1dcb92d08a 100644 --- a/toyos-gicv3/src/lib.rs +++ b/toyos-gicv3/src/lib.rs @@ -22,6 +22,12 @@ pub const fn packed_affinity(mpidr: u64) -> u32 { ((mpidr & 0xFF_FFFF) | ((mpidr >> 8) & 0xFF00_0000)) as u32 } +/// [`packed_affinity`] undone: the four fields where `MPIDR_EL1` holds them, +/// every other bit zero, which is how PSCI names a CPU. +pub const fn unpacked_affinity(packed: u32) -> u64 { + (packed & 0xFF_FFFF) as u64 | ((packed >> 24) as u64) << 32 +} + /// `ICC_SGI1R_EL1` raising SGI `intid` on the one CPU whose packed affinity is /// `target`: `Aff3`, `Aff2` and `Aff1` name its cluster, `RS` the range of /// sixteen `Aff0` values it falls in, and the target list its one bit there. diff --git a/toyos-gicv3/tests/gicv3.rs b/toyos-gicv3/tests/gicv3.rs index 1348e21b600..373cc9c12b9 100644 --- a/toyos-gicv3/tests/gicv3.rs +++ b/toyos-gicv3/tests/gicv3.rs @@ -1,4 +1,4 @@ -use toyos_gicv3::{find_redistributor, packed_affinity, sgi1r, sgi1r_others, FRAME}; +use toyos_gicv3::{find_redistributor, packed_affinity, sgi1r, sgi1r_others, unpacked_affinity, FRAME}; #[test] fn affinity_drops_mpidr_flags_between_aff2_and_aff3() { @@ -7,6 +7,14 @@ fn affinity_drops_mpidr_flags_between_aff2_and_aff3() { assert_eq!(packed_affinity(mpidr), 0x1234_5678); } +#[test] +fn unpacking_puts_each_field_back_and_no_flag() { + let mpidr = 0x12 << 32 | 1 << 31 | 1 << 30 | 1 << 24 | 0x34_5678; + assert_eq!(unpacked_affinity(packed_affinity(mpidr)), 0x12_0034_5678); + // QEMU `virt`'s seventeenth CPU, the first of its second cluster. + assert_eq!(unpacked_affinity(0x100), 0x100); +} + #[test] fn sgi_names_aff0_by_range_and_bit() { // Aff0 0x13 is range 1, bit 3. From bb355a4e3004bbaa791b66dcb4ed34b4a7c6dea4 Mon Sep 17 00:00:00 2001 From: japabu Date: Thu, 1 Oct 2026 01:44:44 +0200 Subject: [PATCH 2/7] AArch64 power seam, review round 1: a CPU PSCI leaves on is named, and every refusal says its code The review of 647 at d1c216d9c upheld two BLOCKERs. This commit answers them and fixes the NOTEs. B1: m3, which takes any AFFINITY_INFO answer for OFF, stayed green. In every arm the other CPUs' CPU_OFF reached QEMU before the caller's SYSTEM_OFF, so no arm had a CPU that PSCI still answered ON at the power-off. virt_off_names_the_cpus_left_on is the review's P1 arm. Its actuator, power-off-spares-the-last-two, makes cpu6 and cpu7 halt on SGI_OFF without calling CPU_OFF. The console must then name each of them that is not the powering CPU, after the last word and in roster order, and nothing else. QEMU's trace must show every other CPU calling CPU_OFF before the one SYSTEM_OFF. The review's patch set kernel_features: ACTUATOR_KERNEL beside kernel_params. The harness's kernel_of refuses that pair, since a parameter already selects the test kernel, so the arm sets kernel_params only. The patch also left the new actuator dead on x86-64, whose kernel builds -Dwarnings. The x86 boot-actuators check failed on it with "function `power_off_spares_the_last_two` is never used". Both PSCI actuators now carry #[allow(dead_code)], with the reason at the site. actuator.rs is generic code, so a target_arch cfg may not go there. m3 is reshaped to fit the new match: an ON answer is taken for OFF. B2: the timer stop before CPU_OFF has no arm that can fail, and none is in reach. The gap is issues/kernel/nothing-fails-without-the-timer-stop-before-cpu-off.md. It records the owner and this exit: an AArch64 metal target where SGI_OFF lands on a CPU whose timer is armed. The owner's ruling that no ARM hardware is a target stands against that exit, and the issue says so. Its evidence is the deletion mutation's guest run, which is the orchestrator's. NOTEs: - x86-64: init_reset stays before percpu::init_bsp. init_off moves after it, so a fault in the DSDT checksum or the \_S5_ scan is reported. - AArch64: cpu_off no longer ends SGI_OFF. SGI_HALT is never ended either, so a refused CPU_OFF leaves the CPU halted at the SGI's running priority, with no pending kick to wake it. - Every PSCI refusal on the way out prints its code raw: SYSTEM_RESET or SYSTEM_OFF returning, a refused CPU_OFF, and AFFINITY_INFO refusing, which is now named at once instead of waited out. psci::Error::code undoes Error::of. - can_reset's no-PSCI arm is virt_reboot_refused_without_psci. With the actuator psci-withheld, psci::init keeps no conduit. The job reboot is then refused by name before anything is torn down and ends with exit 1, and the boot never says Rebooting. - The panic arm line promises "unless a key is pressed" only where the architecture's keyboard_controller::PANIC_KEYS says the panic path reads one. The same holds for reboot_now's "no key inside the bound". The held line and Bound::Held say "no reset", which is true on a machine without PSCI. panic_before_peripherals_reboots now refuses the new held text by a named constant. - find_s5_slp_typ moves into toyos-acpi as s5_slp_typ, answering S5. It refuses an \_S5_ value SLP_TYP's three bits cannot hold (S5::Wide). Its host tests are crafted AML in corpus.rs: - ZeroOp, OneOp and a BytePrefix constant; - \_S4_'s package ahead of \_S5_'s; - a two-byte PkgLength; - a wide value and a WordPrefix; - no package, a method named \_S5_, and a package cut by the table's end; - a package past the declared length. No fixture DSDT exists to test against: fixtures/qemu-11.1.1/SOURCE says that boot's DSDT "is not here". Two host mutations are red: PkgLength read as one byte, and the width refusal removed. - x86-64 refuses a PM1a_CNT_BLK past the 16-bit port space by name, and a zero one as no PM1a control block, where the old code narrowed the address to u16. - machine_reboot and machine_shutdown are one function, machine_stops. It takes the command, the decode line, the last word and the stop reason. REMOVEs: the list of PSCI's callers in aarch64 boot.rs, the init_power story in tests/common/power.rs and the clause citing it, and the track's drivers/acpi.rs:325,345 port-I/O citation. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_016t9wjdQkB8SH7bmfUoiy6L --- ...s-without-the-timer-stop-before-cpu-off.md | 46 ++++++ issues/kernel/toyos-runs-on-arm64.md | 1 - kernel/src/actuator.rs | 13 ++ kernel/src/arch/aarch64/boot.rs | 2 - .../src/arch/aarch64/keyboard_controller.rs | 3 + kernel/src/arch/aarch64/power.rs | 69 ++++++--- kernel/src/arch/aarch64/psci.rs | 31 ++++- kernel/src/arch/x86_64/boot.rs | 3 +- kernel/src/arch/x86_64/i8042/mod.rs | 3 + kernel/src/arch/x86_64/power.rs | 74 +++------- kernel/src/panic_reboot.rs | 30 ++-- tests/checks.rs | 9 +- tests/common/power.rs | 87 ++++-------- tests/toyos.rs | 131 ++++++++++++++++-- toyos-acpi/src/dsdt.rs | 48 +++++++ toyos-acpi/src/lib.rs | 2 + toyos-acpi/tests/corpus.rs | 69 ++++++++- 17 files changed, 454 insertions(+), 167 deletions(-) create mode 100644 issues/kernel/nothing-fails-without-the-timer-stop-before-cpu-off.md create mode 100644 toyos-acpi/src/dsdt.rs diff --git a/issues/kernel/nothing-fails-without-the-timer-stop-before-cpu-off.md b/issues/kernel/nothing-fails-without-the-timer-stop-before-cpu-off.md new file mode 100644 index 00000000000..532a312be72 --- /dev/null +++ b/issues/kernel/nothing-fails-without-the-timer-stop-before-cpu-off.md @@ -0,0 +1,46 @@ +--- +status: open +kind: defect +opened: 2026-10-01 +--- + +# Nothing fails without the timer stop before `CPU_OFF` + +`kernel/src/arch/aarch64/power.rs`'s `cpu_off` stops the CPU's timer before +it calls `CPU_OFF`. The reason is DEN0022 §5.5.2: an interrupt that reaches a +core `CPU_OFF` turned off is an erroneous state. That section is where the +requirement comes from. It is not evidence that this kernel meets it, and no +test can fail if the stop is deleted: + +- QEMU's PSCI is the only provider any guest reaches, and it delivers nothing + to a vCPU it holds off. +- A second provider, TF-A under QEMU, needs a C cross-toolchain, which is not + a declared host tool. +- No AArch64 metal target exists. By the owner's ruling in + `issues/kernel/toyos-runs-on-arm64.md`, no ARM hardware is a target. +- `SGI_OFF` lands on idle CPUs. An idle CPU with no parked deadline has + already stopped its one-shot (`toyos-sched/src/timer.rs:43`), so deleting + the stop changes nothing even on metal in that case. + +The stop serves a wider claim: no interrupt reaches a core `CPU_OFF` turned +off. That claim also covers a kick from a CPU that is not off yet, and the +stop does nothing about kicks. + +**Evidence**: the deletion mutation, a guest run at PR #647's head: + +```diff +--- a/kernel/src/arch/aarch64/power.rs ++++ b/kernel/src/arch/aarch64/power.rs +@@ pub(super) fn cpu_off() -> ! { +- irqchip::stop_timer(); +``` + +`cargo test --test toyos-build -- virt_`, run by the orchestrator as the job +`armnext-r2-t0-no-timer-stop`. The guest runs are the orchestrator's, and its +exit code is recorded here once measured. + +**Exit**: an AArch64 metal target where `SGI_OFF` lands on a CPU whose timer +is armed, with the deletion mutation red there. + +Owner: `kernel/src/arch/aarch64/power.rs`'s `cpu_off`, under +`issues/kernel/toyos-runs-on-arm64.md`. diff --git a/issues/kernel/toyos-runs-on-arm64.md b/issues/kernel/toyos-runs-on-arm64.md index 08adf2589b0..31acef12142 100644 --- a/issues/kernel/toyos-runs-on-arm64.md +++ b/issues/kernel/toyos-runs-on-arm64.md @@ -95,7 +95,6 @@ bound). Heaviest: `arch::cpu` (57 references), `arch::percpu` (28), - **Port I/O** (`cpu::inb/outb/inw/outw`): `rtc.rs:203-204`, `drivers/serial.rs` (16 sites), `drivers/i8042/mod.rs` (12 sites), - `drivers/acpi.rs:325,345` (reset and PM1a soft-off), `drivers/watchdog.rs:84-158`, `bootloader/src/watchdog.rs:195,204`. - **APIC / IOAPIC / MSI.** `hw.rs:1-5`: "Everything here is x2APIC, TSC or a single instruction." The MSI doorbell `0xFEE0_0000` is hardcoded three diff --git a/kernel/src/actuator.rs b/kernel/src/actuator.rs index 48c7404e73f..151d284b757 100644 --- a/kernel/src/actuator.rs +++ b/kernel/src/actuator.rs @@ -169,6 +169,19 @@ actuators! { /// `log_after_a_refused_stop`. power_refused_once = "power-refused-once"; + /// The roster's last two CPUs take the power-off's SGI and halt without + /// `CPU_OFF`, so PSCI answers them on for the whole budget. Judged by + /// `virt_off_names_the_cpus_left_on`. + // PSCI is AArch64's alone, so x86-64 builds an accessor it never reads. + #[allow(dead_code)] + power_off_spares_the_last_two = "power-off-spares-the-last-two"; + + /// `psci::init` keeps no conduit, so this kernel has no reset: a machine + /// without PSCI. Judged by `virt_reboot_refused_without_psci`. + // PSCI is AArch64's alone, so x86-64 builds an accessor it never reads. + #[allow(dead_code)] + psci_withheld = "psci-withheld"; + /// Hold a typed copy into user memory whose destination carries /// `user_ptr::remap_race`'s mark between its translation and its store, /// until its own process has mapped memory again: a sibling's `munmap` diff --git a/kernel/src/arch/aarch64/boot.rs b/kernel/src/arch/aarch64/boot.rs index f21745c1f1c..ee212d5d98f 100644 --- a/kernel/src/arch/aarch64/boot.rs +++ b/kernel/src/arch/aarch64/boot.rs @@ -316,8 +316,6 @@ pub struct Platform { /// Interrupt delivery: this CPU's per-CPU block, the GIC and the timer's /// interrupt, and interrupts unmasked. The syscall gate is the vectors' own. -/// Then PSCI, which starts the other CPUs and resets and powers off the -/// machine. pub fn interrupts(rsdp_addr: u64) -> Platform { super::percpu::init_bsp(); let gic = super::irqchip::init(rsdp_addr); diff --git a/kernel/src/arch/aarch64/keyboard_controller.rs b/kernel/src/arch/aarch64/keyboard_controller.rs index ef6462509b8..e04e19a2e3c 100644 --- a/kernel/src/arch/aarch64/keyboard_controller.rs +++ b/kernel/src/arch/aarch64/keyboard_controller.rs @@ -1,6 +1,9 @@ //! The platform's own keyboard controller. An Arm machine has none: its //! keyboards are USB, and the panic panel's key poll reads nothing here. +/// Whether the panic path reads a key here. +pub const PANIC_KEYS: bool = false; + /// No byte ever waits. pub fn poll_byte() -> Option<(u8, bool)> { None diff --git a/kernel/src/arch/aarch64/power.rs b/kernel/src/arch/aarch64/power.rs index ce8261acde4..98ebd6e4d12 100644 --- a/kernel/src/arch/aarch64/power.rs +++ b/kernel/src/arch/aarch64/power.rs @@ -1,6 +1,9 @@ //! Reset and power-off: PSCI's `SYSTEM_RESET` and `SYSTEM_OFF` (Arm DEN0022 //! §5.12, §5.10), through the conduit [`super::psci`] kept. A machine without //! PSCI has neither, and holds where either is asked for. +//! +//! Every line here goes to the UART raw: it takes no lock, and nothing drains +//! the log ring after any of these calls. use super::{cpu, irqchip, percpu, psci}; use crate::drivers::serial; @@ -18,10 +21,13 @@ pub fn can_reset() -> bool { } /// `SYSTEM_RESET`, which asks nothing of the other CPUs (DEN0022 §5.12.2): a -/// wedge calls this, so it takes no lock and makes no other call. +/// wedge calls this, so it takes no lock. pub fn reset() -> ! { if let Some(psci) = psci::conduit() { - psci.system_reset(); + let refusal = psci.system_reset(); + serial::panic_raw(b"power: SYSTEM_RESET answered "); + say_code(refusal); + serial::panic_raw(b"; holding\n"); } cpu::halt() } @@ -37,30 +43,61 @@ pub fn off() -> ! { let me = percpu::cpu_id(); for cpu in (0..crate::smp::cpu_count()).filter(|&cpu| cpu != me) { let mpidr = toyos_gicv3::unpacked_affinity(crate::smp::hardware_id(cpu)); - while !matches!(psci.affinity_info(mpidr), Ok(psci::Affinity::Off)) { - if deadline.reached(crate::clock::now()) { - // Raw: the console's last drain is behind this call. - serial::panic_raw(b"power: cpu"); - serial::panic_raw_dec(u64::from(cpu)); - serial::panic_raw(b" is not off by PSCI's answer inside the budget; SYSTEM_OFF regardless\n"); - break; + loop { + match psci.affinity_info(mpidr) { + Ok(psci::Affinity::Off) => break, + Ok(_) if !deadline.reached(crate::clock::now()) => core::hint::spin_loop(), + Ok(_) => { + serial::panic_raw(b"power: cpu"); + serial::panic_raw_dec(u64::from(cpu)); + serial::panic_raw(b" is not off by PSCI's answer inside the budget; SYSTEM_OFF regardless\n"); + break; + } + Err(refusal) => { + serial::panic_raw(b"power: cpu"); + serial::panic_raw_dec(u64::from(cpu)); + serial::panic_raw(b"'s AFFINITY_INFO answered "); + say_code(refusal); + serial::panic_raw(b"; SYSTEM_OFF regardless\n"); + break; + } } - core::hint::spin_loop(); } } - psci.system_off(); + let refusal = psci.system_off(); + serial::panic_raw(b"power: SYSTEM_OFF answered "); + say_code(refusal); + serial::panic_raw(b"; holding\n"); cpu::halt() } /// This CPU's part in [`off`], on the SGI that asked for it: its timer /// stopped, since a core `CPU_OFF` turned off may take no interrupt (DEN0022 -/// §5.5.2), then `CPU_OFF`. A refusal leaves it halted, and [`off`] names it -/// still on. +/// §5.5.2), then `CPU_OFF`. The SGI is never ended, as `SGI_HALT` is not, so +/// a refusal leaves this CPU halted with nothing signalled to it, and [`off`] +/// names it still on. pub(super) fn cpu_off() -> ! { - irqchip::end(irqchip::SGI_OFF); irqchip::stop_timer(); - if let Some(psci) = psci::conduit() { - psci.cpu_off(); + if crate::actuator::power_off_spares_the_last_two() && percpu::cpu_id() + 2 >= crate::smp::cpu_count() { + cpu::halt() } + let Some(psci) = psci::conduit() else { + unreachable!("power: SGI_OFF is raised only by `off`, which holds a conduit") + }; + let refusal = psci.cpu_off(); + serial::panic_raw(b"power: cpu"); + serial::panic_raw_dec(u64::from(percpu::cpu_id())); + serial::panic_raw(b"'s CPU_OFF answered "); + say_code(refusal); + serial::panic_raw(b"\n"); cpu::halt() } + +/// The code PSCI refused a call with, signed. +fn say_code(refusal: psci::Error) { + let code = refusal.code(); + if code < 0 { + serial::panic_raw(b"-"); + } + serial::panic_raw_dec(u64::from(code.unsigned_abs())); +} diff --git a/kernel/src/arch/aarch64/psci.rs b/kernel/src/arch/aarch64/psci.rs index 4348e497022..d70537d7807 100644 --- a/kernel/src/arch/aarch64/psci.rs +++ b/kernel/src/arch/aarch64/psci.rs @@ -75,6 +75,22 @@ impl Error { other => Self::Unnamed(other), } } + + /// The code PSCI answered with: [`Error::of`] undone. + pub fn code(self) -> i32 { + match self { + Self::NotSupported => -1, + Self::InvalidParameters => -2, + Self::Denied => -3, + Self::AlreadyOn => -4, + Self::OnPending => -5, + Self::InternalFailure => -6, + Self::NotPresent => -7, + Self::Disabled => -8, + Self::InvalidAddress => -9, + Self::Unnamed(code) => code, + } + } } impl Conduit { @@ -136,14 +152,15 @@ impl Conduit { } /// Power the machine off. Neither this nor [`Conduit::system_reset`] - /// returns (DEN0022 §5.1.9, §5.1.11), except where firmware breaks that. - pub fn system_off(self) { - self.call(SYSTEM_OFF, 0, 0, 0); + /// returns (DEN0022 §5.1.9, §5.1.11), so an answer is firmware breaking + /// that. + pub fn system_off(self) -> Error { + Error::of(self.call(SYSTEM_OFF, 0, 0, 0)) } /// Reset the machine cold. - pub fn system_reset(self) { - self.call(SYSTEM_RESET, 0, 0, 0); + pub fn system_reset(self) -> Error { + Error::of(self.call(SYSTEM_RESET, 0, 0, 0)) } fn name(self) -> &'static str { @@ -191,6 +208,10 @@ pub fn init(rsdp_addr: u64) { log!("PSCI: {major}.{minor} through {}, which numbers no SMC64 CPU_ON; none used", conduit.name()); return; } + if crate::actuator::psci_withheld() { + log!("PSCI: {major}.{minor} through {}, withheld: this kernel keeps no conduit", conduit.name()); + return; + } log!("PSCI: {major}.{minor} through {}", conduit.name()); CONDUIT.store(conduit as u8, Relaxed); } diff --git a/kernel/src/arch/x86_64/boot.rs b/kernel/src/arch/x86_64/boot.rs index da62778e7a0..a606ea3f475 100644 --- a/kernel/src/arch/x86_64/boot.rs +++ b/kernel/src/arch/x86_64/boot.rs @@ -87,9 +87,10 @@ pub fn interrupts(rsdp_addr: u64) -> Platform { // Off the same tables as the MADT, and before the IDT below makes a panic // reportable: a panic that can be reported but not ended leaves the machine // holding its panel for a hand that may not be in the room. - super::power::init(rsdp_addr); + super::power::init_reset(rsdp_addr); apic::init(); percpu::init_bsp(apic::id()); + super::power::init_off(rsdp_addr); ioapic::init(&madt); idt::enable_interrupts(); super::syscall::init(); diff --git a/kernel/src/arch/x86_64/i8042/mod.rs b/kernel/src/arch/x86_64/i8042/mod.rs index 44abd8fc377..3048b09a413 100644 --- a/kernel/src/arch/x86_64/i8042/mod.rs +++ b/kernel/src/arch/x86_64/i8042/mod.rs @@ -1379,6 +1379,9 @@ pub fn init(rsdp_addr: u64) { } } +/// Whether the panic path reads a key here: [`poll_byte`] is its poll. +pub const PANIC_KEYS: bool = true; + /// One byte from the controller if it has one; never waits. Only legal once /// every CPU is halted — port 0x60's sole reader is otherwise the ISR. pub fn poll_byte() -> Option<(u8, bool)> { diff --git a/kernel/src/arch/x86_64/power.rs b/kernel/src/arch/x86_64/power.rs index 7a65115441e..2df8b54dd80 100644 --- a/kernel/src/arch/x86_64/power.rs +++ b/kernel/src/arch/x86_64/power.rs @@ -8,7 +8,7 @@ use core::mem::size_of; use core::sync::atomic::{AtomicU16, AtomicU8, Ordering}; -use toyos_acpi::{Reset, Table, TableError, SDT_HEADER_LEN, SDT_REVISION}; +use toyos_acpi::{Reset, Table, TableError, S5, SDT_HEADER_LEN, SDT_REVISION}; use super::cpu; use crate::drivers::acpi::direct_phys; @@ -17,25 +17,19 @@ use crate::log; const SLP_EN: u16 = 1 << 13; static PM1A_CNT_PORT: AtomicU16 = AtomicU16::new(0); -static SLP_TYPA: AtomicU16 = AtomicU16::new(0); +static SLP_TYPA: AtomicU8 = AtomicU8::new(0); static RESET_PORT: AtomicU16 = AtomicU16::new(0); static RESET_VALUE: AtomicU8 = AtomicU8::new(0); -/// Record the FADT's reset register and its S5 soft-off, or say by name why -/// this machine has either not. +/// Record the FADT's reset register, or say by name why this machine has none. /// /// Before `percpu::init_bsp` loads the IDT: from then on every panic can be /// reported, and a panic that can be reported but not ended is a machine that /// still needs a hand. Walking these tables inside the panic handler instead is /// refused — a table walk on a machine that has already failed once is how a /// panic becomes a triple fault. -pub fn init(rsdp_addr: u64) { - init_reset(rsdp_addr); - init_off(rsdp_addr); -} - -fn init_reset(rsdp_addr: u64) { +pub fn init_reset(rsdp_addr: u64) { let fadt = match toyos_acpi::find_table(direct_phys(), rsdp_addr, b"FACP", toyos_acpi::FADT_FOR_RESET) { Ok(table) => table, Err(e) => { @@ -53,8 +47,9 @@ fn init_reset(rsdp_addr: u64) { } } -// A machine without these tables keeps booting without soft-off rather than panicking. -fn init_off(rsdp_addr: u64) { +/// Record S5 soft-off, or say by name why this machine has none; it keeps +/// booting either way. After the IDT, so a fault in the DSDT walk is reported. +pub fn init_off(rsdp_addr: u64) { const FADT_FOR_POWER: usize = toyos_acpi::FADT_PM1A_CNT_BLK + size_of::(); const FADT_FOR_X_DSDT: usize = toyos_acpi::FADT_X_DSDT + size_of::(); @@ -66,11 +61,14 @@ fn init_off(rsdp_addr: u64) { } }; - let Some(pm1a) = fadt.u32_at(toyos_acpi::FADT_PM1A_CNT_BLK) else { + let Some(block) = fadt.u32_at(toyos_acpi::FADT_PM1A_CNT_BLK).filter(|&block| block != 0) else { log!("ACPI: FADT has no PM1a control block — no soft-off"); return; }; - let pm1a = pm1a as u16; + let Ok(pm1a) = u16::try_from(block) else { + log!("ACPI: FADT puts the PM1a control block at {block:#x}, past the 16-bit port space — no soft-off"); + return; + }; // Prefer X_DSDT over DSDT; a revision claiming 2.0 doesn't prove the field is present, so the length is checked rather than trusting the revision alone. let dsdt_addr = toyos_acpi::dsdt_address(&fadt); @@ -94,9 +92,16 @@ fn init_off(rsdp_addr: u64) { } }; - let Some(slp_typ) = find_s5_slp_typ(&dsdt) else { - log!("ACPI: no \\_S5_ package in the DSDT — no soft-off"); - return; + let slp_typ = match toyos_acpi::s5_slp_typ(&dsdt) { + S5::SlpTyp(slp_typ) => slp_typ, + S5::Absent => { + log!("ACPI: no \\_S5_ package in the DSDT — no soft-off"); + return; + } + S5::Wide(byte) => { + log!("ACPI: the DSDT's \\_S5_ names SLP_TYPa {byte:#x}, wider than its three bits — no soft-off"); + return; + } }; PM1A_CNT_PORT.store(pm1a, Ordering::Relaxed); @@ -124,42 +129,9 @@ pub fn reset() -> ! { pub fn off() -> ! { let pm1a = PM1A_CNT_PORT.load(Ordering::Relaxed); if pm1a != 0 { - let val = (SLP_TYPA.load(Ordering::Relaxed) << 10) | SLP_EN; + let val = (u16::from(SLP_TYPA.load(Ordering::Relaxed)) << 10) | SLP_EN; // SAFETY: pm1a and slp_typ come only from the validated FADT parse via PM1A_CNT_PORT/SLP_TYPA, and the zero check above confirms that parse happened. unsafe { cpu::outw(pm1a, val) }; } cpu::halt() } - -/// Scan DSDT AML bytecode for the \_S5_ package and extract SLP_TYPa. -// Bounded by the table's declared length via [`Table::byte`]. -fn find_s5_slp_typ(dsdt: &Table

) -> Option { - let s5 = b"_S5_"; - let len = dsdt.len(); - - for i in SDT_HEADER_LEN..len.saturating_sub(7) { - if (0..4).any(|j| dsdt.byte(i + j) != Some(s5[j])) { - continue; - } - if dsdt.byte(i + 4) != Some(0x12) { - continue; - } - - let pkg_lead = dsdt.byte(i + 5)?; - let pkg_len_bytes = match (pkg_lead >> 6) & 0x03 { - 0 => 1usize, - n => (n + 1) as usize, - }; - - // Skip: "_S5_"(4) + PackageOp(1) + PkgLength + NumElements(1) - let val_off = i + 4 + 1 + pkg_len_bytes + 1; - let byte = dsdt.byte(val_off)?; - return Some(if byte == 0x0A { - dsdt.byte(val_off + 1)? as u16 - } else { - byte as u16 - }); - } - - None -} diff --git a/kernel/src/panic_reboot.rs b/kernel/src/panic_reboot.rs index 9a071579bbb..655c0062124 100644 --- a/kernel/src/panic_reboot.rs +++ b/kernel/src/panic_reboot.rs @@ -12,7 +12,7 @@ //! that states no frequency and has no calibrated clock cannot time anything, //! and the arm line says so instead of resetting on a guess. -use crate::arch::cpu; +use crate::arch::{cpu, keyboard_controller}; use crate::drivers::serial; use crate::time::{Budget, Duration}; @@ -37,7 +37,7 @@ pub enum Bound { /// Reset the machine at this `cpu::counter` reading. At(u64), /// Hold the panel: somebody is reading it, or nothing here could time a - /// wait, or this machine has no reset register to write. + /// wait, or this machine has no reset this kernel performs. Held, } @@ -110,30 +110,31 @@ pub fn arm(on_the_record: bool) -> Bound { // ASCII only, here and in every line below: the panel's font renders // anything outside 0x20..=0x7E as a dot. let secs = budget.duration().millis() / 1_000; - // The clock and the reset register are two separate ways to have no - // reboot, and a line naming one when the other is what failed answers - // whoever reads the panel with nothing. + // A key retires the bound only where the panic path reads one. + let unless = if keyboard_controller::PANIC_KEYS { " unless a key is pressed" } else { "" }; match (deadline(budget), crate::power::can_reboot()) { (Some((cycles, source)), true) => { if on_the_record { alert!( - "{ARMED} in {secs} s unless a key is pressed, timed by {}", + "{ARMED} in {secs} s{unless}, timed by {}", source.named() ); } else { - serial::panic_raw(b"panic: rebooting unless a key is pressed\n"); + serial::panic_raw(b"panic: rebooting"); + serial::panic_raw(unless.as_bytes()); + serial::panic_raw(b"\n"); } Bound::At(cycles) } (Some((_, source)), false) => { if on_the_record { alert!( - "{HELD}, timed by {}: this kernel has decoded no reset register to hand \ - the machine back to firmware with", + "{HELD}, timed by {}: this kernel has no reset to hand the machine back to \ + firmware with", source.named() ); } else { - serial::panic_raw(b"panic: holding this panel: no reset register\n"); + serial::panic_raw(b"panic: holding this panel: no reset\n"); } Bound::Held } @@ -154,10 +155,11 @@ pub fn arm(on_the_record: bool) -> Bound { /// Return the machine to firmware. The second of this path's two lines, and it /// goes out raw: the log has already been flushed and drained by here. pub fn reboot_now() -> ! { - serial::panic_raw( - b"\npanic: no key inside the bound, so nobody is here: returning this machine to \ - firmware\n", - ); + serial::panic_raw(if keyboard_controller::PANIC_KEYS { + b"\npanic: no key inside the bound, so nobody is here: returning this machine to firmware\n" + } else { + b"\npanic: the bound is over: returning this machine to firmware\n" + }); // Not `power::reboot`: its flush waits on the console wire, and a CPU this // panic stopped may be holding it. crate::power::reset_now() diff --git a/tests/checks.rs b/tests/checks.rs index 92865569e7a..926982363e5 100644 --- a/tests/checks.rs +++ b/tests/checks.rs @@ -951,7 +951,8 @@ mod checks { /// `arm_psci_call` as QEMU 11.1.1's `trace-events` formats it, with and /// without the log backend's `pid@time:` head, and every way a power-off's - /// trace falls short of PSCI's recipe refused. + /// trace falls short of PSCI's recipe refused, a CPU left on calling + /// `CPU_OFF` among them. #[test] fn psci_power_off_judge() { let line = |head: &str, function: u64, cpu: u64| { @@ -963,7 +964,8 @@ mod checks { let (off, system_off) = (PSCI_CPU_OFF, PSCI_SYSTEM_OFF); let affinity_info = 0xC400_0004; - assert_eq!(psci_powered_off(&[(off, 1), (affinity_info, 3), (off, 2), (off, 0), (system_off, 3)], 4), Ok(3)); + assert_eq!(psci_powered_off(&[(off, 1), (affinity_info, 3), (off, 2), (off, 0), (system_off, 3)], 4, &[]), Ok(3)); + assert_eq!(psci_powered_off(&[(off, 1), (off, 0), (system_off, 3)], 4, &[2]), Ok(3)); for short in [ vec![(off, 1), (off, 0), (system_off, 3)], vec![(off, 1), (off, 0), (system_off, 3), (off, 2)], @@ -973,7 +975,8 @@ mod checks { vec![(off, 1), (off, 2), (off, 0), (PSCI_SYSTEM_RESET, 3), (system_off, 3)], vec![(off, 1), (off, 2), (off, 3), (system_off, 3)], ] { - assert!(psci_powered_off(&short, 4).is_err(), "{short:x?}"); + assert!(psci_powered_off(&short, 4, &[]).is_err(), "{short:x?}"); } + assert!(psci_powered_off(&[(off, 1), (off, 2), (off, 0), (system_off, 3)], 4, &[2]).is_err()); } } diff --git a/tests/common/power.rs b/tests/common/power.rs index 54546dc206f..0de3070d688 100644 --- a/tests/common/power.rs +++ b/tests/common/power.rs @@ -40,70 +40,47 @@ fn returned_to_firmware(reason: Option, never: &str, tail: &str) -> Resu } } -/// The machine returns to firmware when a process holding `POWER` asks it to. -pub fn machine_reboot( - test_config: &Path, - c_bins: &[(String, Vec)], - rust_bins: &[(String, Vec)], -) -> Result<(), String> { - let options = BootOptions { qmp: true, ..Default::default() }; - let mut qemu = QemuInstance::boot_with_options(test_config, c_bins, rust_bins, options); - - let boot = serial::Serial::boot(&qemu); - boot.must_be_clean()?; - // A decode this kernel got wrong, never one it bypassed: a kernel writing - // 0xcf9 without reading the FADT satisfies this and the stop reason both. - boot.must_say("ACPI: reset register SystemIO 0xcf9 <- 0x0f")?; - - let mut stop = qemu::QmpShutdown::open(qemu.qmp_socket(), qemu.budget(WAIT)); - - writeln!(qemu.stdin_mut(), "run reboot").expect("write to QEMU stdin"); - qemu.flush_stdin(); - let reason = stop.reason(); - // Ends when QEMU exits and the reader disconnects, so a guest that came back to firmware pays none of this. - let tail = qemu.drain_serial(WAIT); - - let drain = serial::Serial::named("reboot drain", tail.as_str()); - drain.must_be_clean()?; - drain.must_say(REBOOTING)?; - returned_to_firmware(reason, ASKED_AND_STAYED_UP, &tail)?; - - eprintln!(" [power] QEMU stopped the guest for guest-reset"); - Ok(()) -} - -/// The machine powers off when a process holding `POWER` asks it to: ACPI S5 -/// through the PM1a control block the FADT names, with the DSDT's `\_S5_`. -pub fn machine_shutdown( +/// The machine ends when a process holding `POWER` types `command`: the boot +/// said `decoded`, the drain says `last`, and QEMU stops for `reason`. +/// +/// `decoded` carries a value this kernel read out of the firmware's tables, +/// so a decode it got wrong fails here and one it bypassed cannot pass: a +/// kernel writing 0xcf9 without reading the FADT satisfies the stop reason +/// alone. +pub fn machine_stops( test_config: &Path, c_bins: &[(String, Vec)], rust_bins: &[(String, Vec)], + command: &str, + decoded: &str, + last: &str, + reason: &str, ) -> Result<(), String> { let options = BootOptions { qmp: true, ..Default::default() }; let mut qemu = QemuInstance::boot_with_options(test_config, c_bins, rust_bins, options); let boot = serial::Serial::boot(&qemu); boot.must_be_clean()?; - // q35's ICH9 power-management block, and its `\_S5_`. - boot.must_say("ACPI: PM1a=0x604 SLP_TYPa=0")?; + boot.must_say(decoded)?; let mut stop = qemu::QmpShutdown::open(qemu.qmp_socket(), qemu.budget(WAIT)); - writeln!(qemu.stdin_mut(), "run shutdown").expect("write to QEMU stdin"); + writeln!(qemu.stdin_mut(), "run {command}").expect("write to QEMU stdin"); qemu.flush_stdin(); - let reason = stop.reason(); + let stopped = stop.reason(); + // Ends when QEMU exits and the reader disconnects, so a guest that stopped pays none of this. let tail = qemu.drain_serial(WAIT); - let drain = serial::Serial::named("shutdown drain", tail.as_str()); + let drain = serial::Serial::named(&format!("{command} drain"), tail.as_str()); drain.must_be_clean()?; - drain.must_say("Shutting down.")?; - match reason.as_deref() { - Some("guest-shutdown") => {} - Some(seen) => return Err(format!("QEMU stopped this guest for {seen:?}, not a power-off\n{tail}")), - None => return Err(format!("QEMU never reported stopping: the guest asked to power off and stayed up\n{tail}")), + drain.must_say(last)?; + match stopped.as_deref() { + Some(seen) if seen == reason => {} + Some(seen) => return Err(format!("QEMU stopped this guest for {seen:?}, not {reason:?}\n{tail}")), + None => return Err(format!("QEMU never reported stopping: the guest asked to {command} and stayed up\n{tail}")), } - eprintln!(" [power] QEMU stopped the guest for guest-shutdown"); + eprintln!(" [power] {command}: QEMU stopped the guest for {reason}"); Ok(()) } @@ -897,12 +874,6 @@ fn died_and_reset( /// A panic inside `percpu::init_bsp`, one statement after it loads the IDT, /// finds a reset register already decoded. /// -/// That is the window the owner's T14 stops in and the earliest point a panic is -/// reportable at all. The FADT's reset register used to be decoded at -/// `acpi::init_power`, hundreds of statements later, so a panic here said it had -/// "decoded no reset register to hand the machine back to firmware with" and -/// held the panel for a hand. -/// /// **The reset itself is not asserted here, and cannot be on this guest**: the /// bound is carried in TSC cycles, and before `clock::init` those come from /// CPUID leaves 15H/16H, which QEMU's model answers with zeros. So this guest @@ -924,14 +895,13 @@ pub fn panic_before_peripherals_reboots( let qemu = QemuInstance::boot_with_options(test_config, c_bins, rust_bins, options); let boot = serial::Serial::boot(&qemu); - // Ordering is the whole assertion: this line is what `init_power` used to - // print long after the panic below. + // Ordering is the whole assertion. let decoded = boot.must_say("ACPI: reset register SystemIO")?.to_string(); boot.must_say("EARLY PANIC: panicked at")?; let held = boot.must_say(PANIC_HELD_HEAD)?.to_string(); - // The one thing this branch removed. A guest reaching the other held branch - // for the other reason must not be read as this one passing. - boot.must_not_say("decoded no reset register")?; + // A guest reaching the other held branch for the other reason must not be + // read as this one passing. + boot.must_not_say(NO_RESET_HELD)?; if !held.contains("states no counter frequency") { return Err(format!( "the panel held for a reason this guest was not expected to reach\n{held}" @@ -948,6 +918,9 @@ pub fn panic_before_peripherals_reboots( /// [`panic_reboot::arm`]: kernel/src/panic_reboot.rs const PANIC_HELD_HEAD: &str = "panic: holding this panel"; +/// The held line's reason on a machine with no reset. +const NO_RESET_HELD: &str = "this kernel has no reset to hand the machine back to firmware with"; + /// The ceiling on the reset past the bound: the flush the reset path makes /// before it writes the register, and the host seeing QEMU's event. Scaled by /// [`QemuInstance::budget`] at the call site. diff --git a/tests/toyos.rs b/tests/toyos.rs index a421d7161bb..8b86caeafc0 100644 --- a/tests/toyos.rs +++ b/tests/toyos.rs @@ -523,6 +523,8 @@ const SCREEN_TESTS: &[(&str, Sched, qemu::Profile)] = &[ ("virt_failed_ap_leaves_no_hole", Sched::Parallel, qemu::Profile::VirtEl2), ("virt_fatal_halts_the_others_first", Sched::Parallel, qemu::Profile::VirtEl2), ("virt_reboot", Sched::Parallel, qemu::Profile::VirtEl2), + ("virt_off_names_the_cpus_left_on", Sched::Parallel, qemu::Profile::VirtEl2), + ("virt_reboot_refused_without_psci", Sched::Parallel, qemu::Profile::VirtEl2), ]; /// What `screen_console_shell` types, and what it then looks for on its own. @@ -3646,7 +3648,8 @@ fn virt_smp(profile: qemu::Profile, conduit: &str, el: u32) -> Result<(), String } } eprintln!(" [virt] {VIRT_CPUS} CPUs entered at EL{el}, started through {conduit}, and scheduling"); - let (console, calls) = ended_through_psci(&mut qemu, &mut stop, serial, SHUTTING_DOWN, "guest-shutdown", &trace)?; + let (console, calls) = + ended_through_psci(&mut qemu, &mut stop, serial, SHUTTING_DOWN, "guest-shutdown", &trace, |_| Vec::new())?; let record = console .lines() .find_map(toyos_quiesce::Record::parse) @@ -3654,7 +3657,7 @@ fn virt_smp(profile: qemu::Profile, conduit: &str, el: u32) -> Result<(), String if record.cpus != VIRT_CPUS { return Err(format!("the stop ran across {} CPUs, not {VIRT_CPUS}: {record}\n{console}", record.cpus)); } - let last = psci_powered_off(&calls, VIRT_CPUS)?; + let last = psci_powered_off(&calls, VIRT_CPUS, &[])?; eprintln!(" [virt] {record}; every CPU but {last:#x} called CPU_OFF, then {last:#x} SYSTEM_OFF"); Ok(()) } @@ -3669,8 +3672,9 @@ const PSCI_SYSTEM_RESET: u64 = 0x8400_0009; /// Wait for the boot's `last` word on a guest whose job list asked for its /// end, then for QEMU to stop for `reason` and exit; `console` is what it has -/// said so far. Nothing may follow the last word. Answers the whole console -/// and every PSCI call QEMU traced into `trace`, in order. +/// said so far. What follows the last word is `said_after` of the calls, line +/// for line, and nothing else. Answers the whole console and every PSCI call +/// QEMU traced into `trace`, in order. fn ended_through_psci( qemu: &mut QemuInstance, stop: &mut qemu::QmpShutdown, @@ -3678,6 +3682,7 @@ fn ended_through_psci( last: &str, reason: &str, trace: &Path, + said_after: impl FnOnce(&[(u64, u64)]) -> Vec, ) -> Result<(String, Vec<(u64, u64)>), String> { await_marker(qemu, &mut console, last, "the boot's last word")?; let stopped = stop.reason(); @@ -3689,11 +3694,17 @@ fn ended_through_psci( let lines: Vec<&str> = console.lines().collect(); let at = lines.iter().position(|l| l.contains(last)).expect("awaited above"); let after: Vec<&str> = lines[at + 1..].iter().copied().filter(|l| !l.trim().is_empty()).collect(); - if !after.is_empty() { - return Err(format!("{} line(s) after the boot's last word:\n {}", after.len(), after.join("\n "))); - } let traced = fs::read_to_string(trace).map_err(|e| format!("read the PSCI trace: {e}"))?; - Ok((console, psci_calls(&traced)?)) + let calls = psci_calls(&traced)?; + let want = said_after(&calls); + if !after.iter().map(|l| l.trim_end()).eq(want.iter().map(String::as_str)) { + return Err(format!( + "{} line(s) after the boot's last word, not {want:?}:\n {}", + after.len(), + after.join("\n ") + )); + } + Ok((console, calls)) } /// Every call QEMU's `arm_psci_call` trace `text` records, in order: the @@ -3715,9 +3726,9 @@ fn psci_calls(text: &str) -> Result, String> { /// A power-off by PSCI's own recipe (DEN0022 §5.10.3), read off QEMU's trace /// of a machine of `cpus` CPUs whose affinities are `0..cpus`: every CPU but -/// one called `CPU_OFF`, each once, and then the one left called `SYSTEM_OFF`, -/// once. Answers that CPU. -fn psci_powered_off(calls: &[(u64, u64)], cpus: u32) -> Result { +/// one and those `left_on` called `CPU_OFF`, each once, and then the one left +/// called `SYSTEM_OFF`, once. Answers that CPU. +fn psci_powered_off(calls: &[(u64, u64)], cpus: u32, left_on: &[u64]) -> Result { let offs: Vec = (0..calls.len()).filter(|&i| calls[i].0 == PSCI_SYSTEM_OFF).collect(); let [at] = offs[..] else { return Err(format!("QEMU traced {} SYSTEM_OFF calls, not one: {calls:x?}", offs.len())); @@ -3729,7 +3740,7 @@ fn psci_powered_off(calls: &[(u64, u64)], cpus: u32) -> Result { let mut turned_off: Vec = calls[..at].iter().filter(|&&(function, _)| function == PSCI_CPU_OFF).map(|&(_, cpu)| cpu).collect(); turned_off.sort_unstable(); - let others: Vec = (0..u64::from(cpus)).filter(|&cpu| cpu != last).collect(); + let others: Vec = (0..u64::from(cpus)).filter(|&cpu| cpu != last && !left_on.contains(&cpu)).collect(); if turned_off != others || calls[at..].iter().any(|&(function, _)| function == PSCI_CPU_OFF) { return Err(format!( "before {last:#x}'s SYSTEM_OFF, CPU_OFF came from {turned_off:x?}, not from each of {others:x?} \ @@ -3791,7 +3802,8 @@ fn virt_reboot(profile: qemu::Profile) -> Result<(), String> { ); let mut stop = qemu::QmpShutdown::open(qemu.qmp_socket(), qemu.budget(qemu::GUEST_QUIET)); let console = qemu.boot_log().to_string(); - let (console, calls) = ended_through_psci(&mut qemu, &mut stop, console, bootlog::REBOOTING, "guest-reset", &trace)?; + let (console, calls) = + ended_through_psci(&mut qemu, &mut stop, console, bootlog::REBOOTING, "guest-reset", &trace, |_| Vec::new())?; // The runner's own deadline also reboots, and is not the job asking. if console.contains(bootlog::JOB_DEADLINE_SAID) { return Err(format!("the reboot was the job list's deadline, not its job\n{console}")); @@ -3808,6 +3820,76 @@ fn virt_reboot(profile: qemu::Profile) -> Result<(), String> { Ok(()) } +/// `power-off-spares-the-last-two`: cpu6 and cpu7 halt on the power-off's SGI +/// without `CPU_OFF`, so `AFFINITY_INFO` answers each on to the end of the +/// budget. Each of them but the one powering off is named after the last +/// word, in roster order, and nothing else is; every other CPU is off before +/// the one `SYSTEM_OFF`. +fn virt_off_names_the_cpus_left_on(profile: qemu::Profile) -> Result<(), String> { + let trace = common::lane::dir().join("virt_off_left_on.psci"); + let _ = fs::remove_file(&trace); + let mut qemu = boot_virt_smp(BootOptions { + profile, + smp: VIRT_CPUS, + qmp: true, + kernel_params: &["power-off-spares-the-last-two"], + psci_trace: Some(trace.clone()), + ..Default::default() + }); + let mut stop = qemu::QmpShutdown::open(qemu.qmp_socket(), qemu.budget(qemu::GUEST_QUIET)); + let serial = judge_virt_job(&mut qemu, "unmap_touch", UNMAP_TOUCH_SAID)?; + let spared = |calls: &[(u64, u64)]| -> Vec { + let last = calls.iter().find(|&&(function, _)| function == PSCI_SYSTEM_OFF).map(|&(_, cpu)| cpu); + (u64::from(VIRT_CPUS) - 2..u64::from(VIRT_CPUS)).filter(|&cpu| Some(cpu) != last).collect() + }; + let named = |calls: &[(u64, u64)]| -> Vec { + spared(calls) + .iter() + .map(|cpu| format!("power: cpu{cpu} is not off by PSCI's answer inside the budget; SYSTEM_OFF regardless")) + .collect() + }; + let (_, calls) = ended_through_psci(&mut qemu, &mut stop, serial, SHUTTING_DOWN, "guest-shutdown", &trace, named)?; + let left_on = spared(&calls); + let last = psci_powered_off(&calls, VIRT_CPUS, &left_on)?; + eprintln!(" [virt] {left_on:?} left on and named; the rest CPU_OFF, then {last:#x} SYSTEM_OFF"); + Ok(()) +} + +/// What the kernel says refusing a reboot on a machine with no reset. +const REBOOT_REFUSED: &str = "reboot: this machine has no reset this kernel performs — refused"; + +/// `psci-withheld` leaves this kernel no PSCI and so no reset: +/// `tests/virtrebootcase`'s job `reboot` is refused by name before anything is +/// torn down, ends with exit 1, and the boot never says `Rebooting.`. +fn virt_reboot_refused_without_psci(profile: qemu::Profile) -> Result<(), String> { + let config = compile::repo_root().join("tests/virtrebootcase/system.toml"); + let case = config.parent().expect("system.toml has a directory"); + let mut qemu = QemuInstance::boot_with_options( + case, + &[], + &[], + BootOptions { + profile, + kernel_params: &["psci-withheld"], + ready_marker: "control registers: SCTLR_EL1=", + ..Default::default() + }, + ); + let mut rest = String::new(); + await_marker(&mut qemu, &mut rest, "===TEST_END reboot ", "the job reboot to end")?; + let console = format!("{}\n{rest}", qemu.boot_log()); + for said in [REBOOT_REFUSED, "===TEST_END reboot exit=1==="] { + if !console.contains(said) { + return Err(format!("{said:?} not on the PL011\n{console}")); + } + } + if console.contains(bootlog::REBOOTING) { + return Err(format!("a machine with no reset began one\n{console}")); + } + eprintln!(" [virt] {REBOOT_REFUSED}, and the job ended exit 1"); + Ok(()) +} + /// [`panic_halts_the_others_first`] on AArch64: `tests/virtpaniccase` runs /// `test_rs_panic_halts_first` as its one job on [`STOP_CPUS`] CPUs, and the /// halt SGI stops every CPU but the one going fatal. @@ -5388,6 +5470,8 @@ fn run_screen_test( "virt_failed_ap_leaves_no_hole" => virt_failed_ap_leaves_no_hole(profile), "virt_fatal_halts_the_others_first" => virt_fatal_halts_the_others_first(profile), "virt_reboot" => virt_reboot(profile), + "virt_off_names_the_cpus_left_on" => virt_off_names_the_cpus_left_on(profile), + "virt_reboot_refused_without_psci" => virt_reboot_refused_without_psci(profile), "screen_late_panic" => { // The ordinary fatal panic, which no userland process can produce: // crash_report, capture, panic_flush, halt_all_cpus, render. The @@ -10085,8 +10169,25 @@ fn run_machine_test( "pkg_install_gbae" => pkg::pkg_install_gbae(test_config, c_bins, rust_bins), // Body in `tests/common/gpt.rs`, same reason. "boot_partition_identity" => common::gpt::boot_partition_identity(test_config, c_bins, rust_bins), - "machine_reboot" => power::machine_reboot(test_config, c_bins, rust_bins), - "machine_shutdown" => power::machine_shutdown(test_config, c_bins, rust_bins), + "machine_reboot" => power::machine_stops( + test_config, + c_bins, + rust_bins, + "reboot", + "ACPI: reset register SystemIO 0xcf9 <- 0x0f", + bootlog::REBOOTING, + "guest-reset", + ), + // q35's ICH9 power-management block, and its `\_S5_`. + "machine_shutdown" => power::machine_stops( + test_config, + c_bins, + rust_bins, + "shutdown", + "ACPI: PM1a=0x604 SLP_TYPa=0", + SHUTTING_DOWN, + "guest-shutdown", + ), "metal_job_reboot" => power::metal_job_reboot(test_config, c_bins, rust_bins), "metal_device_probe" => devices::metal_device_probe(test_config, c_bins, rust_bins), "job_deadline_reboots" => power::job_deadline_reboots(test_config, c_bins, rust_bins), diff --git a/toyos-acpi/src/dsdt.rs b/toyos-acpi/src/dsdt.rs new file mode 100644 index 00000000000..5ac3506317f --- /dev/null +++ b/toyos-acpi/src/dsdt.rs @@ -0,0 +1,48 @@ +//! The DSDT (signature `DSDT`), read for one value by a byte scan, not an AML +//! interpreter: the first element of its `\_S5_` package (ACPI 6.5, "\_Sx"). + +use crate::{Phys, Table, SDT_HEADER_LEN}; + +/// AML's `PackageOp` and `BytePrefix` (ACPI 6.5, "AML Grammar Definition"). +const PACKAGE_OP: u8 = 0x12; +const BYTE_PREFIX: u8 = 0x0A; + +/// The widest `SLP_TYPx` the PM1 control register holds: three bits, 12:10. +const SLP_TYP_MAX: u8 = 7; + +/// What a DSDT says to write to `SLP_TYPa` for S5 soft-off. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub enum S5 { + SlpTyp(u8), + /// No `_S5_` followed by `PackageOp` whose first element lies inside the + /// table. + Absent, + /// A first element wider than `SLP_TYPx`, which shifted into place would + /// set `SLP_EN` and reserved bits. + Wide(u8), +} + +/// The first element of the first `\_S5_` package in `dsdt`, read inside the +/// table's declared length: a `BytePrefix` constant, or the element's own +/// byte, which is `ZeroOp` and `OneOp`'s value. +pub fn s5_slp_typ(dsdt: &Table

) -> S5 { + for at in SDT_HEADER_LEN..dsdt.len() { + if (0..4).any(|i| dsdt.byte(at + i) != Some(b"_S5_"[i])) || dsdt.byte(at + 4) != Some(PACKAGE_OP) { + continue; + } + // `PkgLength`'s lead byte counts the bytes after it in bits 7:6 + // ("Package Length Encoding"); `NumElements` follows it. + let Some(lead) = dsdt.byte(at + 5) else { return S5::Absent }; + let element = at + 5 + 1 + usize::from(lead >> 6) + 1; + let value = match dsdt.byte(element) { + Some(BYTE_PREFIX) => dsdt.byte(element + 1), + byte => byte, + }; + return match value { + Some(value) if value <= SLP_TYP_MAX => S5::SlpTyp(value), + Some(value) => S5::Wide(value), + None => S5::Absent, + }; + } + S5::Absent +} diff --git a/toyos-acpi/src/lib.rs b/toyos-acpi/src/lib.rs index 9e237850066..99a40edff32 100644 --- a/toyos-acpi/src/lib.rs +++ b/toyos-acpi/src/lib.rs @@ -14,6 +14,7 @@ #![no_std] #![forbid(unsafe_code)] +mod dsdt; mod fadt; mod gtdt; mod madt; @@ -22,6 +23,7 @@ mod spcr; use toyos_bootmap::DirectMapEnd; +pub use dsdt::{s5_slp_typ, S5}; pub use fadt::{ century_of, dsdt_address, iapc_boot_arch, psci, reset_register, rtc_century, Century, Psci, Reset, CMOS_RAM, FADT_FOR_RESET, FADT_PM1A_CNT_BLK, FADT_X_DSDT, diff --git a/toyos-acpi/tests/corpus.rs b/toyos-acpi/tests/corpus.rs index 231856db8be..aee8599391e 100644 --- a/toyos-acpi/tests/corpus.rs +++ b/toyos-acpi/tests/corpus.rs @@ -12,8 +12,8 @@ use common::{declare_len, entry, madt, rsdp, sdt, xsdt, Machine}; use toyos_abi::boot::RootBridgeWindow; use toyos_acpi::{ dsdt_address, ecam_base, find_table, hpet_base, iapc_boot_arch, madt_entries, memory_windows, - psci, reset_register, rtc_century, Century, MadtEntry, MadtHalt, Phys, Psci, Reset, Table, - TableError, MADT_ENTRIES, MAX_TABLE_LEN, + psci, reset_register, rtc_century, s5_slp_typ, Century, MadtEntry, MadtHalt, Phys, Psci, Reset, + Table, TableError, MADT_ENTRIES, MAX_TABLE_LEN, S5, }; const RSDP_AT: u64 = 0x1_0000; @@ -615,3 +615,68 @@ fn no_single_byte_mutation_of_a_firmwares_descriptor_list_panics_or_runs_away() assert_eq!(mutations, 2 * 186 * 255); assert!(refused > 0 && refused < mutations, "{refused} of {mutations} refused"); } + +/// `dsdt` opened as the kernel opens one, and its `\_S5_` read. +fn s5_of(dsdt: &[u8]) -> S5 { + let regions: &[(u64, &[u8])] = &[(TABLE_AT, dsdt)]; + let dsdt = Table::open(Machine { regions }, TABLE_AT, b"DSDT", 36).expect("DSDT"); + s5_slp_typ(&dsdt) +} + +/// `Name (_S5_, Package (4) { first, ... })`: `NameOp`, the name, +/// `PackageOp`, a one-byte `PkgLength`, `NumElements`, then `first` and three +/// `ZeroOp`s. +fn s5_package(first: &[u8]) -> Vec { + let mut aml = vec![0x08, b'_', b'S', b'5', b'_', 0x12, (2 + first.len() + 3) as u8, 0x04]; + aml.extend_from_slice(first); + aml.extend_from_slice(&[0x00; 3]); + aml +} + +/// `ZeroOp`, `OneOp` and a `BytePrefix` constant are each the value they +/// encode, and `\_S4_`'s package before it is not `\_S5_`'s. +#[test] +fn s5s_first_element_is_read_whichever_constant_encodes_it() { + let s4 = [0x08, b'_', b'S', b'4', b'_', 0x12, 0x08, 0x04, 0x0A, 0x06, 0x0A, 0x06, 0x00, 0x00]; + for (first, want) in [(&[0x00][..], 0), (&[0x01], 1), (&[0x0A, 0x07], 7)] { + let aml = [&s4[..], &s5_package(first)].concat(); + assert_eq!(s5_of(&sdt(b"DSDT", 2, &aml)), S5::SlpTyp(want), "{first:x?}"); + } +} + +/// A `PkgLength` whose lead byte says one more follows: the element is past +/// both, where reading it one byte early would answer `NumElements`, 4. +#[test] +fn a_two_byte_package_length_is_stepped_over() { + let aml = [0x08, b'_', b'S', b'5', b'_', 0x12, 0x48, 0x00, 0x04, 0x0A, 0x05, 0x00, 0x00, 0x00]; + assert_eq!(s5_of(&sdt(b"DSDT", 2, &aml)), S5::SlpTyp(5)); +} + +/// Three bits hold `SLP_TYPx`: a wider constant, and an encoding this scan +/// does not read as a byte (`WordPrefix`), are refused with the byte, never +/// shifted into `SLP_EN`. +#[test] +fn an_s5_value_wider_than_slp_typ_is_refused_with_it() { + assert_eq!(s5_of(&sdt(b"DSDT", 2, &s5_package(&[0x0A, 0x08]))), S5::Wide(8)); + assert_eq!(s5_of(&sdt(b"DSDT", 2, &s5_package(&[0x0B, 0x05, 0x00]))), S5::Wide(0x0B)); +} + +/// No `_S5_`, an `_S5_` that is not a package, and a package the table ends +/// inside are all no soft-off. +#[test] +fn an_s5_package_that_is_not_there_to_its_first_element_is_absent() { + assert_eq!(s5_of(&sdt(b"DSDT", 2, &[0u8; 16])), S5::Absent); + let method = [0x14, 0x07, b'_', b'S', b'5', b'_', 0x00, 0xA3]; + assert_eq!(s5_of(&sdt(b"DSDT", 2, &method)), S5::Absent); + let cut = &s5_package(&[0x0A, 0x05])[..9]; + assert_eq!(s5_of(&sdt(b"DSDT", 2, cut)), S5::Absent); +} + +/// The scan stops at the declared length: a package in bytes past it, which +/// the machine holds and the table does not, is not read. +#[test] +fn an_s5_package_past_the_declared_length_is_not_read() { + let mut dsdt = sdt(b"DSDT", 2, &s5_package(&[0x01])); + declare_len(&mut dsdt, 36); + assert_eq!(s5_of(&dsdt), S5::Absent); +} From 94dea677c9166bbd39eaf5fddeddee0d5cdcdf7d Mon Sep 17 00:00:00 2001 From: japabu Date: Fri, 2 Oct 2026 18:34:18 +0200 Subject: [PATCH 3/7] The T14 reads its own soft-off decode, and the corpus stops saying the crate reads no DSDT `machine_soft_off_decoded` is a metal row on the boot `machine_reboot` already rides: the kernel log off the T14's stick carries the PM1a control block's port and the `SLP_TYPa` its `\_S5_` names, or none of `init_off`'s refusals would have let it. This branch moved that decode after the IDT and into `toyos_acpi::s5_slp_typ`, and added two refusals of firmware values, a PM1a block past the port space and an `\_S5_` value wider than three bits. The T14's values have been through neither. QEMU's `machine_shutdown` judges q35's, and the write: a T14 that powered itself off never answers the metal loop again, so the power-off itself has no row. `toyos-acpi/tests/corpus.rs` still said that nothing in the crate reads what is inside a DSDT and that `find_s5_slp_typ` stays in the kernel, under a test named for two things the crate does not do. `s5_slp_typ` has been the crate's since bb355a4e3 and five corpus cases cover it. The statement and its half of the test go; both assertions it held are made elsewhere in the file (`hpet_base` answering `Absent` at two sites, a DSDT opening in `s5_of`). The test keeps the half that is true, under its name. The timer-stop issue is `kind: tooling`: it is an instrument that does not exist, and nothing in the kernel is broken. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_013UDZQ6fSKw14e4w2TKTRfm --- ...ls-without-the-timer-stop-before-cpu-off.md | 2 +- tests/common/power.rs | 8 ++++++++ tests/toyos.rs | 9 ++++++++- toyos-acpi/tests/corpus.rs | 18 ++++-------------- 4 files changed, 21 insertions(+), 16 deletions(-) diff --git a/issues/kernel/nothing-fails-without-the-timer-stop-before-cpu-off.md b/issues/kernel/nothing-fails-without-the-timer-stop-before-cpu-off.md index 532a312be72..6df14e10422 100644 --- a/issues/kernel/nothing-fails-without-the-timer-stop-before-cpu-off.md +++ b/issues/kernel/nothing-fails-without-the-timer-stop-before-cpu-off.md @@ -1,6 +1,6 @@ --- status: open -kind: defect +kind: tooling opened: 2026-10-01 --- diff --git a/tests/common/power.rs b/tests/common/power.rs index 4eb66f19e1b..27008fa3fea 100644 --- a/tests/common/power.rs +++ b/tests/common/power.rs @@ -63,6 +63,14 @@ pub fn machine_shutdown(test_config: &Path) -> Result<(), String> { Ok(()) } +/// The kernel decoded S5 soft-off out of this machine's FADT and DSDT. Every +/// other branch of `arch::power::init_off` says `no soft-off` and not this. +pub fn soft_off_decoded(kernel: &serial::Serial) -> Result<(), String> { + let line = kernel.must_say(SOFT_OFF_DECODED)?; + eprintln!(" [power] {}", line.trim()); + Ok(()) +} + /// The kernel's read-back above its own arm, in /// `kernel/src/arch/x86_64/watchdog.rs`: whole clauses, one per branch. const ARMED_ON_ARRIVAL: &str = "so the bootloader had already armed the timer"; diff --git a/tests/toyos.rs b/tests/toyos.rs index 837bd207223..c9d3f6ccf79 100644 --- a/tests/toyos.rs +++ b/tests/toyos.rs @@ -172,7 +172,7 @@ const MACHINE_TESTS: &[&str] = &[ "nested_nmi_is_loud", // The power-off itself: the metal loop reaches the T14 over `ssh` and has // no way to turn it back on, so only a machine QEMU reports stopping can - // be asked. + // be asked. `machine_soft_off_decoded` reads the T14's own decode. "machine_shutdown", ]; @@ -542,6 +542,13 @@ const METAL: &[(&str, metal::Metal)] = &[ }, }, ), + ( + // This machine's own PM1a block and `\_S5_`, which are not q35's. The + // write is `machine_shutdown`'s under QEMU: a T14 that powered itself + // off never answers the loop again. + "machine_soft_off_decoded", + metal::Metal { arms: JOBCASE, judge: |b| power::soft_off_decoded(&b[0].kernel()) }, + ), // ---- one image: tests/proctreecase ---- ( // A parent's end takes its children down. The guest carries every diff --git a/toyos-acpi/tests/corpus.rs b/toyos-acpi/tests/corpus.rs index aee8599391e..1b4ad4ec520 100644 --- a/toyos-acpi/tests/corpus.rs +++ b/toyos-acpi/tests/corpus.rs @@ -430,22 +430,12 @@ fn no_single_byte_mutation_of_a_real_table_panics_or_runs_away() { assert!(halts[1] > 0, "no resealed mutation halted a walk, so that arm is untested here"); } -/// **Stated as tests, so extending the decoder reds the statement.** Nothing in -/// this crate reads what is *inside* a DSDT — `find_s5_slp_typ` scans AML and -/// stays in the kernel — and the XSDT walk deliberately returns the first -/// signature match's verdict rather than trying a second table of the same name. +/// **Stated as a test, so extending the decoder reds the statement.** The XSDT +/// walk deliberately returns the first signature match's verdict rather than +/// trying a second table of the same name. #[test] -fn the_two_things_the_corpus_does_not_cover_are_the_two_the_crate_does_not_do() { +fn the_xsdt_walk_answers_with_its_first_match_and_tries_no_second() { let head = rsdp(XSDT_AT, 2, 36); - let dsdt = sdt(b"DSDT", 2, &[0u8; 8]); - let root = xsdt(&[TABLE_AT]); - let regions: &[(u64, &[u8])] = &[(RSDP_AT, &head), (XSDT_AT, &root), (TABLE_AT, &dsdt)]; - let m = Machine { regions }; - // A DSDT opens like any other table; its contents are nothing this crate - // reads, so no case here covers `\_S5_`. - assert!(Table::open(m, TABLE_AT, b"DSDT", 36).is_ok()); - assert_eq!(hpet_base(m, RSDP_AT).err(), Some(TableError::Absent)); - let mut broken = sdt(b"HPET", 1, &[0u8; 20]); broken[9] = broken[9].wrapping_add(1); let good = sdt(b"HPET", 1, &[0u8; 20]); From 2a548d8349dbf2f6e4cc8663accc35e581adcd95 Mon Sep 17 00:00:00 2001 From: japabu Date: Fri, 2 Oct 2026 18:43:06 +0200 Subject: [PATCH 4/7] issues: the timer stop's deletion measured, and the AArch64 panic reset nothing judges The timer-stop issue owed its evidence a run. With the stop deleted from `cpu_off` at 94dea677c, `cargo test --test toyos-build -- virt_` exits 0, 19 of 19, so the issue now says what was measured and where the patch and the run are. The review of d1c216d9c noted that an AArch64 panic now resets after its bound and nothing judges it. bb355a4e3 made the two lines it named true. The reset itself was never measured: one boot now has, `test-late-panic` on `virt` waited to QEMU's stop, and it ends 60.0 s after the arm line in one SYSTEM_RESET and a `guest-reset`. A standing row costs 60 s of a 45 s suite, the actuator that shortened the bound is gone since #660, and AArch64 has no metal, so the gap is an issue with that measurement as its evidence and the row as its exit. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_013UDZQ6fSKw14e4w2TKTRfm --- ...s-without-the-timer-stop-before-cpu-off.md | 10 +++-- ...at-an-aarch64-panic-resets-at-its-bound.md | 40 +++++++++++++++++++ 2 files changed, 46 insertions(+), 4 deletions(-) create mode 100644 issues/panic-path/nothing-judges-that-an-aarch64-panic-resets-at-its-bound.md diff --git a/issues/kernel/nothing-fails-without-the-timer-stop-before-cpu-off.md b/issues/kernel/nothing-fails-without-the-timer-stop-before-cpu-off.md index 6df14e10422..c066f9d10f4 100644 --- a/issues/kernel/nothing-fails-without-the-timer-stop-before-cpu-off.md +++ b/issues/kernel/nothing-fails-without-the-timer-stop-before-cpu-off.md @@ -26,7 +26,8 @@ The stop serves a wider claim: no interrupt reaches a core `CPU_OFF` turned off. That claim also covers a kick from a CPU that is not off yet, and the stop does nothing about kicks. -**Evidence**: the deletion mutation, a guest run at PR #647's head: +**Evidence**: the deletion mutation, applied at `94dea677c` and reversed +after the run: ```diff --- a/kernel/src/arch/aarch64/power.rs @@ -35,9 +36,10 @@ stop does nothing about kicks. - irqchip::stop_timer(); ``` -`cargo test --test toyos-build -- virt_`, run by the orchestrator as the job -`armnext-r2-t0-no-timer-stop`. The guest runs are the orchestrator's, and its -exit code is recorded here once measured. +`cargo test --test toyos-build -- virt_` exited 0 with it applied: `19 +passed, 19 total`, the power-off of eight CPUs in `virt_smp`, `virt_el1_smp` +and `virt_off_names_the_cpus_left_on` among them. The patch and the run are +https://github.com/ToyOSOrg/ToyOS/pull/647#issuecomment-5956924013. **Exit**: an AArch64 metal target where `SGI_OFF` lands on a CPU whose timer is armed, with the deletion mutation red there. diff --git a/issues/panic-path/nothing-judges-that-an-aarch64-panic-resets-at-its-bound.md b/issues/panic-path/nothing-judges-that-an-aarch64-panic-resets-at-its-bound.md new file mode 100644 index 00000000000..cb3a20224d4 --- /dev/null +++ b/issues/panic-path/nothing-judges-that-an-aarch64-panic-resets-at-its-bound.md @@ -0,0 +1,40 @@ +--- +status: open +kind: tooling +opened: 2026-10-02 +--- + +# Nothing judges that an AArch64 panic resets the machine at its bound + +Since PSCI gave AArch64 a reset, a panic there after `psci::init` arms +`panic_reboot`'s bound, where it used to hold the panel, and at the bound's +end `reboot_now` calls `SYSTEM_RESET`. No test waits for that reset. +`virt_fatal_halts_the_others_first` reads the arm line in either of its two +words and ends there; `virt_reboot` judges `SYSTEM_RESET` from the syscall, +not from a panic. + +A test is one boot and costs the shipped bound, `toyos_tco::PANIC_BOUND_MS`, +60 s of every suite run, on a suite that takes 45 s. The actuator that +shortened the bound for a judge, `panic-reboot-fast`, went with the x86-64 +tests that used it, and AArch64 has no metal row to move this to. + +**Evidence**: the measurement, once, at `94dea677c`, as a test added by a +patch and removed after the run: `test-late-panic` on `virt` at EL2, waited to +QEMU's stop. `cargo test --test toyos-build -- virt_scout` exited 0: + +``` +[kernel 1.557 cpu0] panic: rebooting in 60 s, timed by the calibrated clock +[scout] QEMU's stop reason Some("guest-reset"), 60.0s after the ready marker was read +[scout] said after the ready marker: Ok("\npanic: the bound is over: returning this machine to firmware\n") +[scout] CPU_OFF, SYSTEM_OFF and SYSTEM_RESET calls traced: [(84000009, 0)] +``` + +The patch is `s0-scout-panic-resets.patch` in +https://github.com/ToyOSOrg/ToyOS/pull/647#issuecomment-5956924013. + +**Exit**: that patch's test as a `virt_` row, red when `reboot_now` halts +instead of resetting, once one boot can shorten the bound, or on the owner's +word that the row is worth its 60 s. + +Owner: `kernel/src/panic_reboot.rs`, under +`issues/kernel/toyos-runs-on-arm64.md`. From 17e636323b02a85557c5fecb1cfe00255c89e18e Mon Sep 17 00:00:00 2001 From: japabu Date: Fri, 2 Oct 2026 18:47:15 +0200 Subject: [PATCH 5/7] toyos-acpi: `s5_slp_typ` is held to QEMU's own DSDT The review of d1c216d9c asked for the S5 decode to be host-tested on QEMU's DSDT, and bb355a4e3 answered that no fixture held one: capturing it takes a boot, which was not an implementer's to run then. It is now. `fixtures/qemu-11.1.1/dsdt.bin` is the 8500 bytes a `Profile::Headless` guest of QEMU 11.1.1 held at 0x7f77a000, read by the monitor's `pmemsave` in a test added by a patch and removed after its run, so no byte came through the decoder. It is not the boot the other eight files came off: that one's firmware put the tables 0x400000 higher. Read beside them, this boot's MADT, HPET, DMAR and WAET are those files byte for byte, and its RSDP, XSDT, FADT and MCFG differ in address bytes and checksums alone, which `SOURCE` now says. `SOURCE` also loses its claim that nothing in the crate decodes a DSDT. `the_dsdt_names_the_sleep_type_that_powers_qemu_off` opens it at its own address and reads `S5::SlpTyp(0)`, the value that boot's kernel logged as `ACPI: PM1a=0x604 SLP_TYPa=0` and `machine_shutdown` powers QEMU off with. `\_S4_`'s package sits 14 bytes ahead of `\_S5_`'s there and names 2, so a scan that took the wrong package answers 2. `src/licence.rs` gains the file's row, as the other eight have one. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_013UDZQ6fSKw14e4w2TKTRfm --- src/licence.rs | 6 ++++++ toyos-acpi/fixtures/qemu-11.1.1/SOURCE | 9 ++++++++- toyos-acpi/fixtures/qemu-11.1.1/dsdt.bin | Bin 0 -> 8500 bytes toyos-acpi/tests/fixtures.rs | 16 ++++++++++++++-- 4 files changed, 28 insertions(+), 3 deletions(-) create mode 100644 toyos-acpi/fixtures/qemu-11.1.1/dsdt.bin diff --git a/src/licence.rs b/src/licence.rs index c126de32d6b..c82373c771f 100644 --- a/src/licence.rs +++ b/src/licence.rs @@ -228,6 +228,12 @@ pub const COMMITTED_FILES: &[(&str, &str, &str, Terms)] = &[ "ours: QEMU's own DMAR, captured by the commit that added toyos-acpi", Terms::Spdx("MIT OR Apache-2.0"), ), + ( + "toyos-acpi/fixtures/qemu-11.1.1/dsdt.bin", + "d183acae4048280f9c9cd8353045544d7d84319b304b7f65d4d0895e8b8ac06b", + "ours: QEMU's own DSDT, captured by the commit that reads `\\_S5_` out of it", + Terms::Spdx("MIT OR Apache-2.0"), + ), ( "toyos-acpi/fixtures/qemu-11.1.1/facp.bin", "410716dfb169eaba296ed3c336028843b3cf9fce6ca7c179bb242199cfec9d1a", diff --git a/toyos-acpi/fixtures/qemu-11.1.1/SOURCE b/toyos-acpi/fixtures/qemu-11.1.1/SOURCE index 0bb135df3ab..adb3ca02e1e 100644 --- a/toyos-acpi/fixtures/qemu-11.1.1/SOURCE +++ b/toyos-acpi/fixtures/qemu-11.1.1/SOURCE @@ -15,4 +15,11 @@ right. Each table sums to zero. The addresses are `tests/fixtures.rs`'s `REGIONS`, so the XSDT's own entries point at them and the walk under test is the real one. The DSDT this boot named -(0x7fb7a000, 8500 bytes) is not here: nothing in this crate decodes one. +(0x7fb7a000, 8500 bytes) is not here. + +dsdt.bin is the DSDT of a later boot of the same QEMU and profile, read off +guest physical memory with the monitor's `pmemsave`: 8500 bytes at 0x7f77a000, +summing to zero. That boot's firmware put every table 0x400000 lower and the +ECAM elsewhere, so its RSDP, XSDT, FADT and MCFG differ from the files here in +address bytes and checksums and in nothing else, and its MADT, HPET, DMAR and +WAET are these byte for byte. Its kernel logged `ACPI: PM1a=0x604 SLP_TYPa=0`. diff --git a/toyos-acpi/fixtures/qemu-11.1.1/dsdt.bin b/toyos-acpi/fixtures/qemu-11.1.1/dsdt.bin new file mode 100644 index 0000000000000000000000000000000000000000..89f5c8e3720f35516ce969abe3afbba52df3812b GIT binary patch literal 8500 zcmb7JO>Y~=8J^*Xv|28u6-8OL?AUBR&=x5Y%62}`B0$Jpep#YSky4yu4R9q{t>gsA zB8d)8U_e%Y#PO*P>!3%)Q2s)%HPA!bn{N%!L)$|yxwJr0#HX;&JF{cWkQ5N=##tQ1|!h!Mr7M`;x5i~JLPV$-$C(n{rcdg;G{L*;$YR606dxv@cwS|M_bZ>il#&(0|-!6T3;oO^F zUi+~0=#9Vr>fSqc3V@6FweWi>qC@y`XrB)c+%I}N{5H|WwS|M3FM8s#Y;);K6jIAq zj%=b-3y1II7tc+Plf3=_F_2!TcAp1CL+(%fC9WeAK)@_igjcAHC*bcrgB8 z$4(rEk$vZG$QT;V^={hZQ7)ZeSr%dx-eqh(T9+4t{n!0=$rH-6t>sqHD=sjoZ+E6I z*ylc*W6Yr;r@w#n`~82*Q|8`c?(*zQG36nN$3wPvB!@ZT;3yu9Qsviu7zuW}_3JV#N# zA8ivNy2W=Guop~n?Ad}&ew0R-{SW8(EuO3CPsgN808WPn#B7@>}Z2#B+9b8A;+v)t%g@+d8dnC z9&J=FcA_Hy%E@?V5%M@88HbsnLvGi~Th3#NjmB~x`Cp{{ex&^acU3OZ)$M5yZ=H*}60 zI>$|&2z8y7q0=&STBc5fy3Pqh=Y*ki!qkaS*J&F%Z9}JR>O`pP%osW|hR%$s6QQot zF?2eHPRG=VP}i9?bY=~mSyLxMUFW2sbJEZ`Y3f9%>zp!lP8m9x{QiBQ*>Gj!$*ojFq{LS5&Kp>xL2Ib-TXsO!udI`f9kyr~nRu5;GV zIcw;gHFYA?b)GVGo-%ZvGIb)nk81HIYZ~1sS}~D^R%J!w4w8~sS}~D^Ne7Y z;+61>V3y;>?u=x5MRKR6x6;Iw1No()u3dc2$d_6fg)5o zVW1K#m@rTcN(L&Sq9z$ALZuT1DzSnI1J$5ppb{#WWS|I@P8g`f3MLFxgOY(ts9=(T zB2@DU1C?08gn?>MGEfN>Ofpb}N+%3dVg(ZhszJ#>B~&oUKoKgPFi?pVOc2Z#2?Lc_!GwWoP%=;n6-+Wvgi0q2RAL1a2C6~H zKqXW#$v_b*oiI>|6-*eY1|DxEM;i4{y3s0Jkil~BPX14XEG!ayZf zFkzq?lnhit1(OUEq0$Khl~}=qfof1PPze=GGEjs{Ck#|#1rr9ULCHWRR4~av5h|T9 zP>B^x7^ns%1C>z0Bm+gLbizO-Rxn|p8k7uFLIsly6rs`y14X16C?efJ5$XntP%}`C z2?N!bWS|<83{+#nKs6=|RAZ8XYD_XvjR^zQm@rU{Nd~Gh$v`zG3{+#nKs6>AsKz7% z)tE3)L~3?npolPIVlc@-5n>utNK7@hFi=ElY+;~?)Yy`NB2;5b28s}0MdrJ(PMc`oKV|#C#9oiFzb|!zhvEsESV7u+&^rW5ne2h(qj|;ch zJ!-SlI5wh>x9urLXGqj82K!6n@cEMZHsegmFESQNccxF-GoMegi6k6~VGU+jP1!A$ zpXzp2OYD%k&}y%g^3z?v(P6jg+};x{Q}l_05MEovbKOq6!|!d!eW3Ex9#=09ROctC zUe>CYMfEcJr&llMM^`Tg`%Tg=zs}&hRIJ@joS?j?mG?w>Z=k$)OnEOZ?+=K#@lEUp zkG-#z_eFVspuB%fc|R^+87!YVLHUYSz9Pz32Fh2CDPM`pR|m_dPf)(9m9L8O)q(QW zW6D?K^0mS8V<#wI)5_OG`PxAF+A-y8arydS`SBB!uWRM&qI`XzeEpd6b(Bxhax_@p zmF3mqfmZ`O?(%b`*_Iy@k34?pxsKBfKF0R6nGWv@L+NDO$aF)`WzI~8mx!TsvTbC# zq33MIOoum%p>(osWV)f}CU2(0>&8$z*)}rW&~q?rro%hPP&(N*GTqR9eacLS7n7lM zvTbC#q5E{sOoz9Xp>(p%Oy@Q&xgk)o>aEJEy8SqoJlX~b_Uf(GCc~TeGqk3y& zjX-a+UTdvTfA7QS-Q8ao-rxG*z1{aWw?1G2_gYJf@k5if?Hd2a`NrYSEvUn?j<5LN zIJOhq`3VIjR`6PtW$rbWJKSqG-{SZdU!+5ov!v%WeSGiF5i;p@DoX{x{nBb7*!SMJ zE_Mgd2idP!z(SEK%e|F!{+zd3{bl07?rpPAJ*eBEQ%Ab!E@JNPU$kK z`?L=**o^uS?TqvWFOY!9p2GM4p({&^hhe^%CQ&CV#0OI$!^r&kEAw8*TegmoVR_4y zYq><v$ zZNgPjau<%+8*G#2eiOi8VYHl~Ax3fR+#yjJiMFtjMHN&cJNW~%pm)0)qo=Al;a%;l zI>$`a=sRwxml*VDXT)$uc4DxVpf?8_F+C%D7ku(MJtDL|FxnTwvtb?56M_e-Sz4XW z*cQX54|<`_PhzujHA7<9_Rh`VlS+ynDe^oe3 z;yjJB*vDe*zhd7}T&LIiexF|J=`X>I9mzdxy8T^VVWDy5KmGn6zL!@XP2PJEyO?pd zxD7wFWA~?+tbQj1|T>wnu~g zE2&n!)fS8LKoShnh=qAL*CXeV!%xEpetI3{JD0k-u@ub`&M$X!R?1G;_>hR`6}mzq z++Q3`L$Lo+^_nMrJG)Y8uHo*vNa%qcyJv!^I;+N;VuJ(rBnncdAD%7s!Dc?UjTb+K%g`L~nvS zKYuw%{UIb><@A3U{)b#C)4`*IZ!y^KN`=m=bg0pxZUy_77t20+Ew}ZPaB&hJ+wEz4 zw#GviGLHN52D^dZXA@%jpKo4Wd&-)^YnaBe?DVs={T=LI@~$=)RPHn$C3r558Wx6* zJ3jd&$*kvGhKUUJUuds-Pg7?w8)W>hovmTDj~KU}Wt?DBooXo_?F7TPP(wLxNTR`Y N=t;!8O9k8Y{|CxA{b~RJ literal 0 HcmV?d00001 diff --git a/toyos-acpi/tests/fixtures.rs b/toyos-acpi/tests/fixtures.rs index b4a769e455a..608ccc12113 100644 --- a/toyos-acpi/tests/fixtures.rs +++ b/toyos-acpi/tests/fixtures.rs @@ -7,8 +7,9 @@ use common::Machine; use toyos_abi::boot::RootBridgeWindow; use toyos_acpi::{ century_of, dsdt_address, ecam_base, find_table, hpet_base, iapc_boot_arch, madt_entries, - memory_windows, psci, reset_register, rtc_century, Century, IoApicEntry, MadtEntry, Psci, - Reset, SourceOverride, TableError, FADT_PM1A_CNT_BLK, MADT_ENTRIES, + memory_windows, psci, reset_register, rtc_century, s5_slp_typ, Century, IoApicEntry, + MadtEntry, Psci, Reset, SourceOverride, Table, TableError, FADT_PM1A_CNT_BLK, MADT_ENTRIES, + S5, }; /// Where each table sat in that guest's physical memory. The XSDT's entries @@ -108,6 +109,17 @@ fn the_fadt_names_the_power_block_and_the_dsdt() { assert_eq!(dsdt_address(&fadt), 0x7fb7_a000); } +/// `ACPI: PM1a=0x604 SLP_TYPa=0`, off the DSDT of the boot that logged it +/// (`fixtures/qemu-11.1.1/SOURCE`): `\_S5_`'s package, found past `\_S4_`'s, +/// whose first element is 2. +#[test] +fn the_dsdt_names_the_sleep_type_that_powers_qemu_off() { + const DSDT: u64 = 0x7f77_a000; + let regions: &[(u64, &[u8])] = &[(DSDT, include_bytes!("../fixtures/qemu-11.1.1/dsdt.bin"))]; + let dsdt = Table::open(Machine { regions }, DSDT, b"DSDT", 36).expect("DSDT"); + assert_eq!(s5_slp_typ(&dsdt), S5::SlpTyp(0)); +} + /// `ACPI: reset register SystemIO 0xcf9 <- 0x0f`, against QEMU 11.1.1's own /// source: `hw/i386/acpi-build.c:222-226` publishes `AML_AS_SYSTEM_IO`, /// `bit_width` 8, `ICH9_RST_CNT_IOPORT` (`include/hw/southbridge/ich9.h:122`, From f4f6a453d0c2528b89c7771d3ffa36069738bbb9 Mon Sep 17 00:00:00 2001 From: japabu Date: Fri, 2 Oct 2026 19:51:39 +0200 Subject: [PATCH 6/7] The power-off waits DEAF_CPU's span for PSCI's answer, asked once a millisecond `off` gave every other CPU 100 ms to take `SGI_OFF` and be off by `AFFINITY_INFO`, on a private `Budget` whose number was a choice. A vCPU its host serves late is slow and not dead: #670 moved `time::AP_START` off the same 100 ms on a recorded red. The wait now ends at `time::DEAF_CPU`'s span, the bound the tree already has for a CPU that does not take an interrupt, and declares nothing of its own. Its end stays a degraded answer and no panic: what keeps a CPU on may be firmware's refusal of `CPU_OFF`, and the machine was asked to end. The poll was a bare spin of firmware calls. At 100 ms `virt_off_names_the_cpus_left_on`'s trace was 6199064 bytes, 50812 calls; at five seconds that loop is fifty times it. `AFFINITY_INFO` is now asked again once per `ASK_AGAIN`, a 1 ms `Cadence`, as the xHCI port poll is paced: the same row's trace is 610488 bytes, 5004 calls, and the row says its count. Review of 17e636323, the rest: - `psci::Error::code` wrote `Error::of`'s table a second time for callers that print the number. `cpu_off`, `system_off`, `system_reset` and `affinity_info`'s refusal answer the `i32` the call returned. - `psci::init` runs first in `interrupts`, as x86-64's `init_reset` does, so a panic in the per-CPU block or the GIC's bring-up has a reset. - `virt_fatal_halts_the_others_first` is held to the armed line as a machine with a reset and no panic key says it, `panic: rebooting in 60 s, timed by`, where it took the held line too. The issue that records what stays unheld says so. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_013UDZQ6fSKw14e4w2TKTRfm --- ...at-an-aarch64-panic-resets-at-its-bound.md | 6 +-- kernel/src/arch/aarch64/boot.rs | 3 +- kernel/src/arch/aarch64/power.rs | 30 ++++++++----- kernel/src/arch/aarch64/psci.rs | 42 ++++++------------- tests/toyos.rs | 15 +++---- 5 files changed, 45 insertions(+), 51 deletions(-) diff --git a/issues/panic-path/nothing-judges-that-an-aarch64-panic-resets-at-its-bound.md b/issues/panic-path/nothing-judges-that-an-aarch64-panic-resets-at-its-bound.md index cb3a20224d4..e67d0090ce8 100644 --- a/issues/panic-path/nothing-judges-that-an-aarch64-panic-resets-at-its-bound.md +++ b/issues/panic-path/nothing-judges-that-an-aarch64-panic-resets-at-its-bound.md @@ -9,9 +9,9 @@ opened: 2026-10-02 Since PSCI gave AArch64 a reset, a panic there after `psci::init` arms `panic_reboot`'s bound, where it used to hold the panel, and at the bound's end `reboot_now` calls `SYSTEM_RESET`. No test waits for that reset. -`virt_fatal_halts_the_others_first` reads the arm line in either of its two -words and ends there; `virt_reboot` judges `SYSTEM_RESET` from the syscall, -not from a panic. +`virt_fatal_halts_the_others_first` holds the arm line, `panic: rebooting in +60 s, timed by`, and ends there; `virt_reboot` judges `SYSTEM_RESET` from the +syscall, not from a panic. A test is one boot and costs the shipped bound, `toyos_tco::PANIC_BOUND_MS`, 60 s of every suite run, on a suite that takes 45 s. The actuator that diff --git a/kernel/src/arch/aarch64/boot.rs b/kernel/src/arch/aarch64/boot.rs index b4eccf580f2..afdceef985e 100644 --- a/kernel/src/arch/aarch64/boot.rs +++ b/kernel/src/arch/aarch64/boot.rs @@ -336,10 +336,11 @@ pub struct Platform { /// Interrupt delivery: this CPU's per-CPU block, the GIC and the timer's /// interrupt, and interrupts unmasked. The syscall gate is the vectors' own. pub fn interrupts(rsdp_addr: u64) -> Platform { + // First, so a panic in anything below has a reset to end its bound with. + super::psci::init(rsdp_addr); super::percpu::init_bsp(); let gic = super::irqchip::init(rsdp_addr); super::cpu::enable_interrupts(); - super::psci::init(rsdp_addr); Platform { gic } } diff --git a/kernel/src/arch/aarch64/power.rs b/kernel/src/arch/aarch64/power.rs index 9bfeb6cf3e8..91144dcae50 100644 --- a/kernel/src/arch/aarch64/power.rs +++ b/kernel/src/arch/aarch64/power.rs @@ -9,13 +9,13 @@ use super::{cpu, irqchip, percpu, psci}; use crate::drivers::serial; -use crate::time::{Budget, Deadline, Duration}; +use crate::time::{Cadence, Deadline, Duration, DEAF_CPU}; -/// How long the other CPUs get to be off by PSCI's answer before this one -/// powers the machine off anyway. -const OTHERS_OFF: Budget = Budget::of( - Duration::from_millis(100), - "the machine powers off with the CPUs PSCI still answers on, each named on the console", +/// How often [`off`] asks again of a CPU PSCI still answers on: PSCI says a +/// CPU is off only to one that asks. +const ASK_AGAIN: Cadence = Cadence::every( + Duration::from_millis(1), + "one call into firmware per period, and a power-off at most one period after the last CPU is off", ); pub fn can_reset() -> bool { @@ -37,19 +37,28 @@ pub fn reset() -> ! { /// `SYSTEM_OFF`, once every other CPU the roster holds has turned itself off /// with `CPU_OFF` and PSCI answers it off: the caller puts every core in a -/// known state first, and this is DEN0022 §5.10.3's own way to. +/// known state first, and this is DEN0022 §5.10.3's own way to. The budget +/// for all of them is [`DEAF_CPU`]'s span from the SGI: a CPU PSCI still +/// answers on at its end is named, and the machine powers off regardless. pub fn off() -> ! { cpu::disable_interrupts(); let Some(psci) = psci::conduit() else { cpu::halt() }; irqchip::off_all_but_self(); - let deadline = Deadline::at(crate::clock::now() + OTHERS_OFF.duration()); + // The tripwire's span without its panic: what keeps a CPU on may be + // firmware's refusal, and firmware's word never panics this kernel. + let deadline = Deadline::at(crate::clock::now() + Duration::from_nanos(DEAF_CPU.nanos())); let me = percpu::cpu_id(); for cpu in (0..crate::smp::cpu_count()).filter(|&cpu| cpu != me) { let mpidr = toyos_gicv3::unpacked_affinity(crate::smp::hardware_id(cpu)); loop { match psci.affinity_info(mpidr) { Ok(psci::Affinity::Off) => break, - Ok(_) if !deadline.reached(crate::clock::now()) => core::hint::spin_loop(), + Ok(_) if !deadline.reached(crate::clock::now()) => { + let again = Deadline::at(crate::clock::now() + ASK_AGAIN.duration()); + while !again.reached(crate::clock::now()) { + core::hint::spin_loop(); + } + } Ok(_) => { let mut uart = serial::panic_registers(); uart.write(b"power: cpu"); @@ -103,8 +112,7 @@ pub(super) fn cpu_off() -> ! { } /// The code PSCI refused a call with, signed. -fn say_code(uart: &mut serial::PanicUart, refusal: psci::Error) { - let code = refusal.code(); +fn say_code(uart: &mut serial::PanicUart, code: i32) { if code < 0 { uart.write(b"-"); } diff --git a/kernel/src/arch/aarch64/psci.rs b/kernel/src/arch/aarch64/psci.rs index d70537d7807..44f17d0b5b5 100644 --- a/kernel/src/arch/aarch64/psci.rs +++ b/kernel/src/arch/aarch64/psci.rs @@ -75,22 +75,6 @@ impl Error { other => Self::Unnamed(other), } } - - /// The code PSCI answered with: [`Error::of`] undone. - pub fn code(self) -> i32 { - match self { - Self::NotSupported => -1, - Self::InvalidParameters => -2, - Self::Denied => -3, - Self::AlreadyOn => -4, - Self::OnPending => -5, - Self::InternalFailure => -6, - Self::NotPresent => -7, - Self::Disabled => -8, - Self::InvalidAddress => -9, - Self::Unnamed(code) => code, - } - } } impl Conduit { @@ -134,33 +118,33 @@ impl Conduit { } } - /// Turn this CPU off: an answer is a refusal, since `CPU_OFF` does not - /// return when it succeeds. - pub fn cpu_off(self) -> Error { - Error::of(self.call(CPU_OFF, 0, 0, 0)) + /// Turn this CPU off: an answer is the code of a refusal, since `CPU_OFF` + /// does not return when it succeeds. + pub fn cpu_off(self) -> i32 { + self.call(CPU_OFF, 0, 0, 0) } /// Whether the CPU whose `MPIDR_EL1` is `mpidr` is on, off, or on its way - /// on: affinity level 0, the CPU itself. - pub fn affinity_info(self, mpidr: u64) -> Result { + /// on: affinity level 0, the CPU itself. `Err` is the code of a refusal. + pub fn affinity_info(self, mpidr: u64) -> Result { match self.call(AFFINITY_INFO, mpidr & TARGET_CPU, 0, 0) { 0 => Ok(Affinity::On), 1 => Ok(Affinity::Off), 2 => Ok(Affinity::OnPending), - code => Err(Error::of(code)), + code => Err(code), } } /// Power the machine off. Neither this nor [`Conduit::system_reset`] - /// returns (DEN0022 §5.1.9, §5.1.11), so an answer is firmware breaking - /// that. - pub fn system_off(self) -> Error { - Error::of(self.call(SYSTEM_OFF, 0, 0, 0)) + /// returns (DEN0022 §5.1.9, §5.1.11), so an answer is the code firmware + /// broke that with. + pub fn system_off(self) -> i32 { + self.call(SYSTEM_OFF, 0, 0, 0) } /// Reset the machine cold. - pub fn system_reset(self) -> Error { - Error::of(self.call(SYSTEM_RESET, 0, 0, 0)) + pub fn system_reset(self) -> i32 { + self.call(SYSTEM_RESET, 0, 0, 0) } fn name(self) -> &'static str { diff --git a/tests/toyos.rs b/tests/toyos.rs index c9d3f6ccf79..1a54f5f951f 100644 --- a/tests/toyos.rs +++ b/tests/toyos.rs @@ -1633,7 +1633,10 @@ fn virt_off_names_the_cpus_left_on(profile: qemu::Profile) -> Result<(), String> let (_, calls) = ended_through_psci(&mut qemu, &mut stop, serial, power::SHUTTING_DOWN, "guest-shutdown", &trace, named)?; let left_on = spared(&calls); let last = psci_powered_off(&calls, VIRT_CPUS, &left_on)?; - eprintln!(" [virt] {left_on:?} left on and named; the rest CPU_OFF, then {last:#x} SYSTEM_OFF"); + eprintln!( + " [virt] {left_on:?} left on and named; the rest CPU_OFF, then {last:#x} SYSTEM_OFF; {} PSCI call(s) traced", + calls.len() + ); Ok(()) } @@ -3527,16 +3530,14 @@ fn record_cpu(line: &str) -> Option { } /// The CPU that went fatal, once the console carries its line past -/// `stop_other_cpus` — `panic_reboot::arm`'s, in either of its two words. +/// `stop_other_cpus`: `panic_reboot::arm`'s, as a machine with a reset whose +/// panic path reads no key says it. fn fatal_past_the_stop(console: &str) -> Option { - const PAST_THE_STOP: [&str; 2] = ["panic: rebooting in", "panic: holding this panel"]; + let armed = format!("panic: rebooting in {} s, timed by", toyos_tco::PANIC_BOUND_MS / 1_000); let lines: Vec<&str> = console.lines().collect(); let nonce = lines.iter().position(|l| l.contains(FATAL_HALT_NONCE))?; let fatal = record_cpu(lines[nonce])?; - lines[nonce..] - .iter() - .any(|l| record_cpu(l) == Some(fatal) && PAST_THE_STOP.iter().any(|word| l.contains(word))) - .then_some(fatal) + lines[nonce..].iter().any(|l| record_cpu(l) == Some(fatal) && l.contains(&armed)).then_some(fatal) } /// What a run has established, as it establishes it. From 190edc726c6b1586c38b099a4be5ec2d7cf6252e Mon Sep 17 00:00:00 2001 From: japabu Date: Fri, 2 Oct 2026 20:35:26 +0200 Subject: [PATCH 7/7] Review of de4e37b96: three trace removals and one comment sentence go `virt_smp`, `virt_reboot` and `virt_off_names_the_cpus_left_on` each removed their PSCI trace before the boot that writes it, and read no status. The file cannot be there: `lane::dir()` is under the directory `Run::begin` makes new for every process, and each trace name has one row. `the_others_halt_first`'s doc said the fatal CPU holds its panel. The only machine that row boots arms the reset, and a panic that holds is its red. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_013UDZQ6fSKw14e4w2TKTRfm --- tests/toyos.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/tests/toyos.rs b/tests/toyos.rs index c7b031ec1a0..a2c5526625f 100644 --- a/tests/toyos.rs +++ b/tests/toyos.rs @@ -1407,7 +1407,6 @@ fn boot_virt_smp(options: BootOptions) -> QemuInstance { /// off, every other CPU turned off first ([`psci_powered_off`]). fn virt_smp(profile: qemu::Profile, conduit: &str, el: u32) -> Result<(), String> { let trace = common::lane::dir().join(format!("virt_smp-{conduit}.psci")); - let _ = fs::remove_file(&trace); let mut qemu = boot_virt_smp(BootOptions { profile, smp: VIRT_CPUS, @@ -1569,7 +1568,6 @@ fn virt_reboot(profile: qemu::Profile) -> Result<(), String> { let config = compile::repo_root().join("tests/virtrebootcase/system.toml"); let case = config.parent().expect("system.toml has a directory"); let trace = common::lane::dir().join("virt_reboot.psci"); - let _ = fs::remove_file(&trace); let mut qemu = QemuInstance::boot_with_options( case, &[], @@ -1609,7 +1607,6 @@ fn virt_reboot(profile: qemu::Profile) -> Result<(), String> { /// the one `SYSTEM_OFF`. fn virt_off_names_the_cpus_left_on(profile: qemu::Profile) -> Result<(), String> { let trace = common::lane::dir().join("virt_off_left_on.psci"); - let _ = fs::remove_file(&trace); let mut qemu = boot_virt_smp(BootOptions { profile, smp: VIRT_CPUS, @@ -3477,8 +3474,7 @@ const STOP_CPUS: u32 = 4; /// **QEMU is the judge, and no clock is in it.** Once the fatal path of the /// `test_rs_panic_halts_first` that `qemu` runs has said its line past the /// stop, `panic_reboot::arm`'s, every vCPU but the one that went fatal must be -/// one [`qemu::stopped_cpus`] calls halted. The fatal one holds its panel, so -/// the machine is still there to ask. +/// one [`qemu::stopped_cpus`] calls halted. fn the_others_halt_first(mut qemu: QemuInstance, arch: toyos_build::arch::Arch) -> Result<(), String> { let cpus = STOP_CPUS as usize; let mut console = String::new();