From 0aeeffb486a0a898a136b22ee3f2e61271eaa5ee Mon Sep 17 00:00:00 2001 From: japabu Date: Sat, 10 Oct 2026 13:35:23 +0200 Subject: [PATCH 01/12] acpiserver reads the battery and the AC adapter through the machine's AML and its embedded controller MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The load keeps the namespace whose DSDT loaded, with its host. After it the server finds every PNP0C09, PNP0C0A and ACPI0003 device by its _HID, asks each controller's _GLK, runs its _REG(3, 1), reads each present battery's _BIX (or _BIF) once and its _BST and each adapter's _PSR every 10 s between SCIs, never inside a query drain, and says a line where a battery's whole percent or state, or the adapter's, moved. The host answers the EmbeddedControl space through RD_EC and WR_EC (ACPI 6.5 §12.3.1, §12.3.2), which the transaction state machine gains beside QR_EC; every other write AML asks for is still denied. A controller whose _GLK is 1 has no battery read: the server takes no lock around a transaction yet. The decoders hold §10.2.2's packages by place: 0xFFFFFFFF is unknown and never a number, a rate in mA is power only at the present voltage, and a state that says charging and discharging at once is refused. Model, serial, type and OEM strings are never taken out of the package. The battery_read metal row rides the held testcases boot and prints the readings for comparison with Linux's /sys/class/power_supply on the T14. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017cSFvbD35xJ2kGANVdm23C --- ...dded-controller-without-the-global-lock.md | 5 +- ...ine-in-acpi-mode-and-interprets-its-aml.md | 15 +- tests/toyos.rs | 56 ++ userland/acpiserver-api/src/lib.rs | 13 + userland/acpiserver/src/aml.rs | 104 ++- userland/acpiserver/src/battery.rs | 874 ++++++++++++++++++ userland/acpiserver/src/ec.rs | 234 ++++- userland/acpiserver/src/host.rs | 137 ++- userland/acpiserver/src/main.rs | 134 ++- 9 files changed, 1442 insertions(+), 130 deletions(-) create mode 100644 userland/acpiserver/src/battery.rs diff --git a/issues/the-acpi-server-talks-to-the-embedded-controller-without-the-global-lock.md b/issues/the-acpi-server-talks-to-the-embedded-controller-without-the-global-lock.md index 745b4b3e90b..b4f5dbe57c8 100644 --- a/issues/the-acpi-server-talks-to-the-embedded-controller-without-the-global-lock.md +++ b/issues/the-acpi-server-talks-to-the-embedded-controller-without-the-global-lock.md @@ -9,8 +9,9 @@ opened: 2026-10-04 A machine's AML names whether its firmware also reaches the embedded controller from SMM: the controller device's `_GLK` returns 1, and then every transaction is taken under the FACS global lock (ACPI 6.5 §6.5.7, §5.2.10.1). -`/system/bin/acpiserver` interprets no AML, so it cannot evaluate `_GLK`, and -it takes no lock: on such a machine a query of the server's and one of the +`/system/bin/acpiserver` evaluates `_GLK` only to read no battery where it is +1 (`userland/acpiserver/src/battery.rs`), and takes no lock around its own +queries: on such a machine a query of the server's and one of the firmware's can interleave on the same two ports. On the T14 no table names a `_GLK` at all: a byte search of its DSDT and every diff --git a/issues/toyos-runs-the-machine-in-acpi-mode-and-interprets-its-aml.md b/issues/toyos-runs-the-machine-in-acpi-mode-and-interprets-its-aml.md index 4e10af18689..4477647eb62 100644 --- a/issues/toyos-runs-the-machine-in-acpi-mode-and-interprets-its-aml.md +++ b/issues/toyos-runs-the-machine-in-acpi-mode-and-interprets-its-aml.md @@ -258,6 +258,16 @@ otherwise pay to find again: What the server's load of the tables, on the T14 and through the kernel's mediated access, leaves open: +- **The controller's `_REG(3, 1)` runs on a namespace no `_INI` has run in.** + The battery's read (`userland/acpiserver/src/battery.rs`) tells each + controller device its space is there and then reads the battery, and runs + no `_INI`: the T14's controller `_INI` calls the firmware through + `SMI_CMD`, which the host does not write. The scouts' dry run read that + the T14's `_REG` takes another branch before its `_INI` than after it, + reading and writing controller offset 0x03 where after it touches the + controller not at all; Linux runs it after. Owner: this stage. **Exit**: + the slice that runs the `_STA` and `_INI` walk runs it before `_REG`. + - **The server waits for no release of the Global Lock** (the orchestrator's ruling, not the owner's). A take that finds the firmware holding the lock leaves it the request, as ACPI 6.5 §5.2.10.1 has it, and is then denied by @@ -325,8 +335,9 @@ mediated access, leaves open: What the firmware call, which the kernel makes for the server where its AML stores a byte to `SMI_CMD`, leaves open. The server's AML makes none yet: its -host denies every write AML asks for and passes none to the kernel -(`userland/acpiserver/src/host.rs`). +host writes only the embedded controller's space, through the controller's +own ports, and denies every other write AML asks for, passing none to the +kernel (`userland/acpiserver/src/host.rs`). - **What a call does there is the firmware's**, and the kernel bounds who, when, where, which byte and how often: diff --git a/tests/toyos.rs b/tests/toyos.rs index fe5c4acf6c6..f2df876c75d 100644 --- a/tests/toyos.rs +++ b/tests/toyos.rs @@ -563,6 +563,17 @@ const METAL: &[(&str, metal::Metal)] = &[ judge: |b| acpi_tables_on_metal(b[0]), }, ), + ( + // The T14's battery and AC adapter as the server read them through + // its AML and its embedded controller: `battery_on_metal` says what + // is held, and prints the readings for Linux's on the same machine. + // The same boot as `acpi_server_events`, held past several polls. + "battery_read", + metal::Metal { + arms: TESTCASES_HELD, + judge: |b| battery_on_metal(b[0]), + }, + ), ( // The server killed: the kernel writes `ACPI_DISABLE` as its claim goes, // and `SCI_EN` reads clear after it. @@ -5145,6 +5156,51 @@ fn acpi_tables_on_metal(back: &metal::Readback) -> Result<(), String> { Ok(()) } +/// What the T14's tables name, by a census of them read outside the tree: +/// one control-method battery, one AC adapter, one embedded controller. +const T14_POWER_SOURCES: &str = "acpiserver: power sources: 1 of 1 control-method batteries present, 1 AC adapter(s), 1 embedded controller device(s) told their space is there; the Global Lock taken "; + +/// The T14's battery as the server read it: the one battery its tables name +/// found present once the controller's `_REG` ran, its information read, +/// and at least one reading of it with the adapter's state; every method +/// answered, and none of them took the Global Lock, which the controller's +/// own fields carry no rule for. The lines are printed whole: their numbers +/// are what Linux's `/sys/class/power_supply` reads on the same machine are +/// compared against, by whoever holds both. +fn battery_on_metal(back: &metal::Readback) -> Result<(), String> { + back.job_passed("test_rs_acpi_hold")?; + let log = back.log(); + let lines: Vec<&str> = log + .text() + .lines() + .filter(|l| toyos_logstream::program_line(l).is_some_and(|said| said.tag == "acpiserver")) + .collect(); + if let Some(fired) = lines.iter().find(|l| l.contains("panicked")) { + return Err(format!("the server died: {fired}")); + } + let refused: Vec<&&str> = lines.iter().filter(|l| l.contains("acpiserver: refused") || l.contains(" refused: ")).collect(); + if !refused.is_empty() { + return Err(format!("the server refused something of this machine's AML: {refused:#?}")); + } + let found = log.must_say(T14_POWER_SOURCES)?; + let takes = number_between(found, "; the Global Lock taken ", " times on the way")?; + if takes != 0 { + return Err(format!("the battery's methods took the Global Lock {takes} times: {found}")); + } + let info = log.must_say(&format!("acpiserver: {}battery 1 of 1 from ", acpiserver_api::BATTERY_INFO))?; + let read: Vec<&&str> = lines.iter().filter(|l| l.contains(acpiserver_api::BATTERY_READ)).collect(); + if read.is_empty() { + return Err("the server said no reading of the battery".into()); + } + if let Some(unknown) = read.iter().find(|l| l.contains("unknown") || !(l.ends_with("; AC online") || l.ends_with("; AC offline"))) { + return Err(format!("a reading with a value unknown: {unknown}")); + } + for line in [found, info].into_iter().chain(read.iter().map(|l| **l)) { + eprintln!(" [battery] {}", line.trim()); + } + Ok(()) +} + /// The server's death on the T14: the kernel put the machine in ACPI mode for /// the job's claim, and when the killed server's claim went it wrote /// `ACPI_DISABLE` (the FADT's 0xf1) and read `SCI_EN` clear: the firmware has diff --git a/userland/acpiserver-api/src/lib.rs b/userland/acpiserver-api/src/lib.rs index ebad342baed..dec8a30fc73 100644 --- a/userland/acpiserver-api/src/lib.rs +++ b/userland/acpiserver-api/src/lib.rs @@ -7,3 +7,16 @@ /// count interval where a count moved: after its count of SCIs, and before /// each query number with how often it was taken. pub const QUERIES_COUNTED: &str = "embedded controller queries taken: "; + +/// A present battery's static information, once after the tables' load: +/// `battery of `, then its numbers. +pub const BATTERY_INFO: &str = "battery information: "; + +/// A battery's reading, on its first and wherever its whole percent or state +/// or the adapters' moved: `battery of : %`, its +/// remaining and full capacity, its state, rate, voltage and the power it +/// moves in mW, then the adapters' state. +pub const BATTERY_READ: &str = "battery read: "; + +/// No battery is read on this machine, and why. +pub const NO_BATTERY: &str = "no battery: "; diff --git a/userland/acpiserver/src/aml.rs b/userland/acpiserver/src/aml.rs index 9d6e2f57814..1d65cd866c8 100644 --- a/userland/acpiserver/src/aml.rs +++ b/userland/acpiserver/src/aml.rs @@ -21,8 +21,9 @@ //! where none did**, whatever kept it — tables unread, a DSDT refused, //! an `\_S5` that is no package of two integers, a value the register does //! not hold — and that is said once, loudly ([`NO_S5_HANDED`]); the server -//! goes on serving. Nothing is evaluated after it yet, so the namespace is -//! not kept: no embedded-controller query is served. +//! goes on serving. **A namespace whose DSDT loaded is kept** ([`Aml`]), +//! with the host that answered its load, for what the server evaluates in it +//! after ([`crate::battery`]); no embedded-controller query's method is run. //! //! A table's line says its place, whether it is the DSDT or an SSDT, and a //! refusal by its kind; what the firmware chose — a name, an offset, an @@ -33,9 +34,15 @@ use std::time::Instant; use toyos_acpi::TableError; use toyos_aml::{Error, Host, Interpreter, Value}; -use crate::host::{Firmware, Kernel, Refusal, Stopping, OWN}; +use crate::host::{Controller, Firmware, Kernel, Refusal, Stopping, OWN}; use crate::tables::Tables; +/// A machine's namespace, and the host every evaluation in it is answered by. +pub struct Aml<'k, K, C> { + pub interpreter: Interpreter, + pub host: Firmware<'k, K, C>, +} + /// An embedded-controller query, taken off the controller, run once the /// drain that took it has ended: a query's method may itself talk to the /// controller. @@ -56,7 +63,7 @@ pub struct Loaded { } /// What a refused evaluation or load is called in a line anyone may quote. -fn kind(why: &Error) -> String { +pub fn kind(why: &Error) -> String { match why { Error::Malformed { why, .. } => format!("malformed AML: {why}"), Error::NotFound(_) => "a name that does not resolve".into(), @@ -97,8 +104,17 @@ pub const NO_S5_HANDED: &str = "acpiserver: no \\_S5 was handed to the kernel, which powers off only on one an ACPI server supplied and refuses a shutdown without one"; /// Load the machine's definition blocks from the RSDP at `rsdp`, and -/// evaluate `\_S5`. -pub fn load(kernel: &K, rsdp: u64) -> Loaded { +/// evaluate `\_S5`; the namespace, where its DSDT loaded, is kept with `ec`, +/// the controller the machine's row names. +pub fn load<'k, K: Kernel, C: Controller>(kernel: &'k K, ec: Option, rsdp: u64) -> (Loaded, Option>) { + let mut host = Firmware::new(kernel, ec); + let mut interpreter = Interpreter::new(); + let loaded = load_into(kernel, &mut interpreter, &mut host, rsdp); + let kept = loaded.blocks.first().is_some_and(|dsdt| dsdt.is_ok()) && !host.stopping; + (loaded, kept.then_some(Aml { interpreter, host })) +} + +fn load_into(kernel: &K, interpreter: &mut Interpreter, host: &mut Firmware<'_, K, C>, rsdp: u64) -> Loaded { let began = Instant::now(); let mut loaded = Loaded::default(); let tables = Tables::new(kernel); @@ -124,14 +140,12 @@ pub fn load(kernel: &K, rsdp: u64) -> Loaded { return loaded; } - let mut host = Firmware::new(kernel); - let mut interpreter = Interpreter::new(); let count = blocks.len(); for (place, block) in blocks.iter().enumerate() { let name = if place == 0 { "DSDT" } else { "SSDT" }; let place = place + 1; let done = match block { - Ok(table) => interpreter.load(&mut host, table).map_err(|why| { + Ok(table) => interpreter.load(host, table).map_err(|why| { println!("{OWN}table {place} was refused {why:x?}"); kind(&why) }), @@ -184,7 +198,7 @@ pub fn load(kernel: &K, rsdp: u64) -> Loaded { ); if loaded.blocks.first().is_some_and(|dsdt| dsdt.is_ok()) { - match s5(&mut interpreter, &mut host) { + match s5(interpreter, host) { Ok((a, b)) => { println!("acpiserver: \\_S5 evaluated: SLP_TYPa={a} SLP_TYPb={b}"); loaded.s5 = Some((a, b)); @@ -233,16 +247,21 @@ fn s5(interpreter: &mut Interpreter, host: &mut dyn Host) -> Result<(u64, u64), } #[cfg(test)] -mod tests { +pub mod tests { use super::*; + use crate::ec::tests::Emulated; use crate::host::tests::Scripted; use crate::host::{Take, HELD}; + fn load_only(kernel: &K, rsdp: u64) -> Loaded { + load(kernel, None::, rsdp).0 + } + /// QEMU's own tables where its guest held them /// (`toyos-acpi/fixtures/qemu-11.1.1/SOURCE`), as ACPI reclaim memory. - const RSDP: u64 = 0x7fb7_e014; + pub const RSDP: u64 = 0x7fb7_e014; const DSDT: u64 = 0x7fb7_a000; - const QEMU: &[(u64, &[u8])] = &[ + pub const QEMU: &[(u64, &[u8])] = &[ (RSDP, include_bytes!("../../../toyos-acpi/fixtures/qemu-11.1.1/rsdp.bin")), (0x7fb7_d0e8, include_bytes!("../../../toyos-acpi/fixtures/qemu-11.1.1/xsdt.bin")), (0x7fb7_9000, include_bytes!("../../../toyos-acpi/fixtures/qemu-11.1.1/facp.bin")), @@ -254,7 +273,7 @@ mod tests { (DSDT, include_bytes!("../../../toyos-acpi/fixtures/qemu-11.1.1/dsdt.bin")), ]; - fn machine(tables: &[(u64, &[u8])]) -> Scripted { + pub fn machine(tables: &[(u64, &[u8])]) -> Scripted { Scripted { memory: tables.iter().map(|&(at, bytes)| (at, 9, bytes.to_vec())).collect(), ..Default::default() } } @@ -264,7 +283,7 @@ mod tests { #[test] fn qemus_tables_load_and_s5_is_what_its_kernel_decoded() { let kernel = machine(QEMU); - assert_eq!(load(&kernel, RSDP), Loaded { blocks: vec![Ok(())], s5: Some((0, 0)), handed: true }); + assert_eq!(load_only(&kernel, RSDP), Loaded { blocks: vec![Ok(())], s5: Some((0, 0)), handed: true }); assert!(kernel.asked.borrow().iter().all(|access| access.write == 0 && access.space == 0)); assert!(!kernel.held.get()); assert_eq!(kernel.handed.get(), Some(0)); @@ -273,7 +292,7 @@ mod tests { // The builders below are `toyos-aml`'s test encodings of §20.2, the few a // definition block here needs. - fn sealed(signature: &[u8; 4], body: &[u8]) -> Vec { + pub fn sealed(signature: &[u8; 4], body: &[u8]) -> Vec { let mut table = vec![0u8; 36]; table[..4].copy_from_slice(signature); table[4..8].copy_from_slice(&(36 + body.len() as u32).to_le_bytes()); @@ -298,7 +317,7 @@ mod tests { } /// A machine whose XSDT lists a FADT naming `dsdt` and then `ssdts`. - fn crafted(dsdt: &[u8], ssdts: &[&[u8]]) -> Scripted { + pub fn crafted(dsdt: &[u8], ssdts: &[&[u8]]) -> Scripted { const AT: u64 = 0x10_0000; let at = |n: usize| AT + 0x1_0000 * n as u64; let mut fadt_body = vec![0u8; 116 - 36]; @@ -324,7 +343,7 @@ mod tests { Scripted { memory, ..Default::default() } } - const CRAFTED_RSDP: u64 = 0x10_0000 - 0x1000; + pub const CRAFTED_RSDP: u64 = 0x10_0000 - 0x1000; #[test] fn a_refused_ssdt_is_one_block_refused_and_the_rest_load() { @@ -335,7 +354,7 @@ mod tests { let good = sealed(b"SSDT", &name(b"CCCC", 4)); let kernel = crafted(&dsdt, &[&collides, &unsummed, &good]); assert_eq!( - load(&kernel, CRAFTED_RSDP), + load_only(&kernel, CRAFTED_RSDP), Loaded { blocks: vec![Ok(()), Err("a name defined twice".into()), Err("its bytes do not sum to zero".into()), Ok(())], s5: Some((5, 7)), @@ -350,21 +369,40 @@ mod tests { let dsdt = sealed(b"DSDT", &[0x08, b'A']); let ssdt = sealed(b"SSDT", &name(b"CCCC", 4)); let kernel = crafted(&dsdt, &[&ssdt, &ssdt]); - let loaded = load(&kernel, CRAFTED_RSDP); + let loaded = load_only(&kernel, CRAFTED_RSDP); assert_eq!(loaded.blocks.len(), 1, "an SSDT was loaded onto no DSDT: {loaded:?}"); assert!(matches!(&loaded.blocks[0], Err(kind) if kind.starts_with("malformed AML: ")), "{loaded:?}"); assert_eq!(loaded.s5, None); } + /// The namespace is kept where the DSDT loaded, an SSDT refused or not, + /// and nowhere else: not where the DSDT was refused, the tables were + /// not read, or the machine stopped during the load. + #[test] + fn a_namespace_is_kept_only_where_its_dsdt_loaded() { + let kept = |kernel: &Scripted, rsdp| load(kernel, None::, rsdp).1.is_some(); + let dsdt = sealed(b"DSDT", &cat(&[&name(b"AAAA", 1), &s5_package(5, 7)])); + let collides = sealed(b"SSDT", &name(b"AAAA", 2)); + assert!(kept(&crafted(&dsdt, &[&collides]), CRAFTED_RSDP)); + assert!(kept(&machine(QEMU), RSDP)); + assert!(!kept(&crafted(&sealed(b"DSDT", &[0x08, b'A']), &[]), CRAFTED_RSDP)); + assert!(!kept(&machine(QEMU), 0x1000)); + for answered in [3, 40] { + let kernel = machine(QEMU); + kernel.stops_after.set(Some(answered)); + assert!(!kept(&kernel, RSDP), "after {answered} reads"); + } + } + #[test] fn tables_the_kernel_will_not_read_are_refused_by_its_name_for_it() { // No RSDP where the claim says: RAM, to the stand-in. - assert_eq!(load(&machine(QEMU), 0x1000), Loaded::default()); + assert_eq!(load_only(&machine(QEMU), 0x1000), Loaded::default()); // The DSDT's own bytes are not the firmware's to read. let kernel = machine(&QEMU[..QEMU.len() - 1]); let ram = "its bytes could not be read: a SystemMemory read the kernel refused UsableMemory, in memory of type 7"; - assert_eq!(load(&kernel, RSDP), Loaded { blocks: vec![Err(ram.into())], s5: None, handed: false }); + assert_eq!(load_only(&kernel, RSDP), Loaded { blocks: vec![Err(ram.into())], s5: None, handed: false }); } /// The shape the first load on the real machine had: its RSDP and XSDT @@ -381,17 +419,17 @@ mod tests { let listed = kernel.memory.split_off(2); kernel.kept = listed.iter().map(|(at, _, bytes)| (*at, at + bytes.len() as u64, 5)).collect(); let kept = "its bytes could not be read: a SystemMemory read the kernel refused MemoryType, in memory of type 5"; - assert_eq!(load(&kernel, CRAFTED_RSDP), Loaded { blocks: vec![Err(kept.into())], s5: None, handed: false }); + assert_eq!(load_only(&kernel, CRAFTED_RSDP), Loaded { blocks: vec![Err(kept.into())], s5: None, handed: false }); } #[test] fn an_s5_that_is_no_package_of_two_integers_is_refused_by_kind() { let absent = sealed(b"DSDT", &name(b"AAAA", 1)); - assert_eq!(load(&crafted(&absent, &[]), CRAFTED_RSDP), Loaded { blocks: vec![Ok(())], s5: None, handed: false }); + assert_eq!(load_only(&crafted(&absent, &[]), CRAFTED_RSDP), Loaded { blocks: vec![Ok(())], s5: None, handed: false }); let integer = sealed(b"DSDT", &name(b"_S5_", 5)); - assert_eq!(load(&crafted(&integer, &[]), CRAFTED_RSDP), Loaded { blocks: vec![Ok(())], s5: None, handed: false }); + assert_eq!(load_only(&crafted(&integer, &[]), CRAFTED_RSDP), Loaded { blocks: vec![Ok(())], s5: None, handed: false }); let short = sealed(b"DSDT", &cat(&[&[0x08], b"_S5_", &[0x12, 0x04, 0x01, 0x0A, 0x05]])); - assert_eq!(load(&crafted(&short, &[]), CRAFTED_RSDP), Loaded { blocks: vec![Ok(())], s5: None, handed: false }); + assert_eq!(load_only(&crafted(&short, &[]), CRAFTED_RSDP), Loaded { blocks: vec![Ok(())], s5: None, handed: false }); } /// `SLP_TYPa` is the package's first element and the only one handed @@ -401,20 +439,20 @@ mod tests { #[test] fn only_an_s5_the_kernel_kept_is_a_power_off() { let kernel = crafted(&sealed(b"DSDT", &s5_package(7, 9)), &[]); - assert_eq!(load(&kernel, CRAFTED_RSDP), Loaded { blocks: vec![Ok(())], s5: Some((7, 9)), handed: true }); + assert_eq!(load_only(&kernel, CRAFTED_RSDP), Loaded { blocks: vec![Ok(())], s5: Some((7, 9)), handed: true }); assert_eq!(kernel.handed.get(), Some(7)); let kernel = crafted(&sealed(b"DSDT", &s5_package(8, 0)), &[]); - assert_eq!(load(&kernel, CRAFTED_RSDP), Loaded { blocks: vec![Ok(())], s5: Some((8, 0)), handed: false }); + assert_eq!(load_only(&kernel, CRAFTED_RSDP), Loaded { blocks: vec![Ok(())], s5: Some((8, 0)), handed: false }); assert_eq!(kernel.handed.get(), None); for dsdt in [sealed(b"DSDT", &name(b"AAAA", 1)), sealed(b"DSDT", &[0x08, b'A'])] { let kernel = crafted(&dsdt, &[]); - assert!(!load(&kernel, CRAFTED_RSDP).handed); + assert!(!load_only(&kernel, CRAFTED_RSDP).handed); assert_eq!(kernel.handed.get(), None); } let kernel = machine(QEMU); - assert!(!load(&kernel, 0x1000).handed); + assert!(!load_only(&kernel, 0x1000).handed); assert_eq!(kernel.handed.get(), None); } @@ -423,7 +461,7 @@ mod tests { for answered in [0, 3, 40] { let kernel = machine(QEMU); kernel.stops_after.set(Some(answered)); - assert_eq!(load(&kernel, RSDP), Loaded::default(), "after {answered} reads"); + assert_eq!(load_only(&kernel, RSDP), Loaded::default(), "after {answered} reads"); } } @@ -483,13 +521,13 @@ mod tests { let field = toyos_abi::acpi::Access::read(toyos_abi::acpi::Space::SystemMemory, NVS, toyos_abi::acpi::Width::Byte); let kernel = with_nvs(); - assert_eq!(load(&kernel, CRAFTED_RSDP), Loaded { blocks: vec![Ok(())], s5: Some((5, 0)), handed: true }); + assert_eq!(load_only(&kernel, CRAFTED_RSDP), Loaded { blocks: vec![Ok(())], s5: Some((5, 0)), handed: true }); assert!(!kernel.held.get(), "the load ended holding the Global Lock"); assert_eq!(kernel.asked.borrow().last(), Some(&field)); let kernel = with_nvs(); kernel.takes.borrow_mut().push_back(Take::Pending); - let loaded = load(&kernel, CRAFTED_RSDP); + let loaded = load_only(&kernel, CRAFTED_RSDP); assert_eq!(loaded, Loaded { blocks: vec![Err(format!("denied by this server: {HELD}"))], s5: None, handed: false }); assert_ne!(kernel.asked.borrow().last(), Some(&field), "the field was read without the lock"); } diff --git a/userland/acpiserver/src/battery.rs b/userland/acpiserver/src/battery.rs new file mode 100644 index 00000000000..bf24bdc7234 --- /dev/null +++ b/userland/acpiserver/src/battery.rs @@ -0,0 +1,874 @@ +//! The machine's batteries and AC adapters, as its AML reports them (ACPI 6.5 +//! §10.2, §10.3), read in the namespace the load kept ([`crate::aml::Aml`]). +//! +//! **Once, after the load** ([`Power::find`]). Where the machine's row names +//! an embedded controller, each controller device (`PNP0C09`) is asked its +//! `_GLK` (§6.5.7) and then told by its `_REG` that its space is there +//! (§6.5.4, `_REG(3, 1)`), before anything reads that space: firmware +//! commonly chooses between the controller and another path by what its +//! `_REG` stored. A controller whose `_GLK` asks for the Global Lock has no +//! battery read at all: this server takes the lock around no transaction +//! yet. Then every device whose `_HID` names a control-method battery +//! (`PNP0C0A`) or an AC adapter (`ACPI0003`) is found by a walk, a battery +//! is kept where its `_STA` says one is present (§6.3.7, bit 4; a device +//! with no `_STA` has none), and its static information is read once, from +//! `_BIX` where it has one and `_BIF` where it has not (§10.2.2). +//! +//! **Every [`POLL`]** each battery's `_BST` and each adapter's `_PSR` are +//! evaluated ([`Power::read`]), and a line is said on the first reading and +//! wherever a battery's whole percent or state, or the adapter's, moved since +//! the line before: a draining battery says a line a percent, never one a +//! poll. +//! +//! **What a line says is numbers.** The model, serial number, type and OEM +//! strings of `_BIX` and `_BIF` are never taken out of the package, and a +//! device's path is the firmware's choice, said only under [`OWN`]. A +//! refused evaluation is said under [`REFUSED`] the first time and counted +//! after; its battery or adapter reads as unknown until one answers. + +use std::fmt; +use std::time::Duration; + +use toyos_aml::{Error, Kind, Value}; + +use crate::aml::{kind, Aml}; +use crate::host::{Controller, Kernel, OWN}; +use crate::ledger::Ledger; + +/// How often each battery and adapter is read. +pub const POLL: Duration = Duration::from_secs(10); + +/// What opens the line of an evaluation this module asked for and the +/// interpreter or the host refused. +pub const REFUSED: &str = "acpiserver: a method read for the power sources refused: "; + +/// §5.6.7 / §6.1.5 identifiers, compared in their text form. +const CONTROLLER: &str = "PNP0C09"; +const BATTERY: &str = "PNP0C0A"; +const ADAPTER: &str = "ACPI0003"; + +/// §10.2.2: a capacity, rate or voltage the battery does not know. +const UNKNOWN: u64 = 0xFFFF_FFFF; + +/// `_BST`'s Battery State bits (§10.2.2, "_BST"). +const DISCHARGING: u64 = 1 << 0; +const CHARGING: u64 = 1 << 1; +const CRITICAL: u64 = 1 << 2; +const LIMITING: u64 = 1 << 3; + +/// `_STA`'s battery-present bit (§6.3.7). +const BATTERY_PRESENT: u64 = 1 << 4; + +/// What `_BIF` and `_BIX` say a battery's capacities and rates are counted +/// in (their Power Unit). +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub enum Unit { + /// Capacities in mWh, rates in mW. + Power, + /// Capacities in mAh, rates in mA. + Current, +} + +impl Unit { + fn capacity(self) -> &'static str { + match self { + Unit::Power => "mWh", + Unit::Current => "mAh", + } + } + + fn rate(self) -> &'static str { + match self { + Unit::Power => "mW", + Unit::Current => "mA", + } + } +} + +/// A battery's static information, the numbers of `_BIX` or `_BIF`. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub struct Info { + /// Which of the two it came from. + pub from: &'static str, + pub unit: Unit, + pub design: Option, + pub full: Option, + /// mV. + pub design_voltage: Option, + /// `_BIX`'s alone. + pub cycles: Option, +} + +/// One `_BST`. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub struct Status { + /// The Battery State bits as the firmware returned them. + pub state: u64, + /// In the battery's [`Unit`]'s rate. + pub rate: Option, + /// In its capacity. + pub remaining: Option, + /// mV. + pub voltage: Option, +} + +/// One element of a package of DWORDs: an Integer no wider than 32 bits, +/// [`UNKNOWN`] read as `None`. +fn dword(elements: &[Value], at: usize, what: &str) -> Result, String> { + match elements.get(at) { + Some(&Value::Integer(UNKNOWN)) => Ok(None), + Some(&Value::Integer(value)) => u32::try_from(value).map(Some).map_err(|_| format!("its {what} is wider than a DWORD")), + Some(_) => Err(format!("its {what} is no Integer")), + None => Err(format!("it has no {what}")), + } +} + +fn package<'v>(value: &'v Value, least: usize) -> Result<&'v [Value], String> { + match value { + Value::Package(elements) if elements.len() >= least => Ok(elements), + Value::Package(elements) => Err(format!("a package of {} elements, where §10.2.2 has at least {least}", elements.len())), + _ => Err("no package".into()), + } +} + +fn unit(elements: &[Value], at: usize) -> Result { + match elements.get(at) { + Some(Value::Integer(0)) => Ok(Unit::Power), + Some(Value::Integer(1)) => Ok(Unit::Current), + Some(Value::Integer(other)) => Err(format!("its Power Unit is {other}, which §10.2.2 does not define")), + _ => Err("its Power Unit is no Integer".into()), + } +} + +/// `_BIF`: Power Unit, Design Capacity, Last Full Charge Capacity, Battery +/// Technology, Design Voltage, and eight more this server reads nothing of. +pub fn bif(value: &Value) -> Result { + let elements = package(value, 13)?; + Ok(Info { + from: "_BIF", + unit: unit(elements, 0)?, + design: dword(elements, 1, "Design Capacity")?, + full: dword(elements, 2, "Last Full Charge Capacity")?, + design_voltage: dword(elements, 4, "Design Voltage")?, + cycles: None, + }) +} + +/// `_BIX`: `_BIF`'s numbers behind a Revision, with the Cycle Count after +/// them. Revision 0 has 20 elements and revision 1 adds a 21st; the fields +/// read here are at the same places in both. +pub fn bix(value: &Value) -> Result { + let elements = package(value, 20)?; + Ok(Info { + from: "_BIX", + unit: unit(elements, 1)?, + design: dword(elements, 2, "Design Capacity")?, + full: dword(elements, 3, "Last Full Charge Capacity")?, + design_voltage: dword(elements, 5, "Design Voltage")?, + cycles: dword(elements, 8, "Cycle Count")?, + }) +} + +/// `_BST`: Battery State, Present Rate, Remaining Capacity, Present Voltage. +pub fn bst(value: &Value) -> Result { + let elements = package(value, 4)?; + let state = match elements[0] { + Value::Integer(state) if state & (CHARGING | DISCHARGING) == CHARGING | DISCHARGING => { + return Err("its state says charging and discharging at once, which §10.2.2 forbids".into()); + } + Value::Integer(state) => state, + _ => return Err("its Battery State is no Integer".into()), + }; + Ok(Status { + state, + rate: dword(elements, 1, "Battery Present Rate")?, + remaining: dword(elements, 2, "Battery Remaining Capacity")?, + voltage: dword(elements, 3, "Battery Present Voltage")?, + }) +} + +/// The charge in whole percent of the last full charge, rounded down. +pub fn percent(info: &Info, status: &Status) -> Option { + let full = info.full.filter(|&full| full > 0)?; + Some(u64::from(status.remaining?) * 100 / u64::from(full)) +} + +/// The power the battery moves, in mW: its rate where that is power, and +/// the rate times its present voltage where that is current. +pub fn milliwatts(info: &Info, status: &Status) -> Option { + let rate = u64::from(status.rate?); + match info.unit { + Unit::Power => Some(rate), + Unit::Current => Some(rate * u64::from(status.voltage?) / 1000), + } +} + +/// A battery state's bits by name. +fn state(bits: u64) -> String { + let mut said: Vec = [(DISCHARGING, "discharging"), (CHARGING, "charging"), (CRITICAL, "critical"), (LIMITING, "charge limiting")] + .iter() + .filter(|&&(bit, _)| bits & bit != 0) + .map(|&(_, name)| name.into()) + .collect(); + let reserved = bits & !(DISCHARGING | CHARGING | CRITICAL | LIMITING); + if reserved != 0 { + said.push(format!("reserved bits {reserved:#x}")); + } + if said.is_empty() { "neither charging nor discharging".into() } else { said.join(", ") } +} + +struct Known(Option, &'static str); + +impl fmt::Display for Known { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self.0 { + Some(value) => write!(f, "{value} {}", self.1), + None => f.write_str("unknown"), + } + } +} + +/// The information line's numbers. +pub fn said_info(info: &Info) -> String { + let unit = info.unit.capacity(); + format!( + "from {}: design {}, last full {}, design voltage {}, {}", + info.from, + Known(info.design, unit), + Known(info.full, unit), + Known(info.design_voltage, "mV"), + match info.cycles { + Some(cycles) => format!("{cycles} cycles"), + None => "cycles unknown".into(), + } + ) +} + +/// A reading line's numbers. +pub fn said_status(info: &Info, status: &Status) -> String { + format!( + "{}, {} of {}, {}, rate {}, {}, moving {}", + percent(info, status).map_or("charge unknown".into(), |percent| format!("{percent}%")), + Known(status.remaining, info.unit.capacity()), + Known(info.full, info.unit.capacity()), + state(status.state), + Known(status.rate, info.unit.rate()), + Known(status.voltage, "mV"), + milliwatts(info, status).map_or("unknown mW".into(), |mw| format!("{mw} mW")), + ) +} + +/// An `_HID`'s text: a string as it is, an integer as the EISA ID it +/// compresses (§6.1.5): three letters of five bits each and four hex digits, +/// stored in the byte order of their text. +fn hid(value: &Value) -> Option { + match value { + Value::String(text) => String::from_utf8(text.clone()).ok(), + &Value::Integer(id) => { + let id = u32::try_from(id).ok()?.swap_bytes(); + if id >> 31 != 0 { + return None; + } + let letter = |at: u32| char::from(b'@' + ((id >> at) & 0x1F) as u8); + Some(format!("{}{}{}{:04X}", letter(26), letter(21), letter(16), id & 0xFFFF)) + } + _ => None, + } +} + +struct Battery { + path: String, + info: Info, + /// What its last line said: whole percent and state. + said: Option<(Option, u64)>, +} + +/// The machine's power sources, found once and read every [`POLL`]. +pub struct Power { + batteries: Vec, + adapters: Vec, + /// What the last line said of the adapters. + ac_said: Option>, + refused: Ledger, + /// The last evaluation found the machine stopping. + stopping: bool, +} + +impl Power { + /// An evaluation of `path` with `args`: `Ok(None)` where nothing is + /// named there. + fn evaluate(&mut self, aml: &mut Aml<'_, K, C>, path: &str, args: &[Value]) -> Result, Error> { + let answer = match aml.interpreter.evaluate(&mut aml.host, path, args) { + Err(Error::NotFound(named)) if named == path => Ok(None), + answer => answer.map(Some), + }; + self.stopping = aml.host.stopping; + answer + } + + /// A refusal of `method` of the device at `device`, said the first time; + /// on a stopping machine every evaluation is refused, and none is said. + fn refused(&mut self, device: &str, method: &str, why: &str) { + if self.stopping { + return; + } + let what = format!("{method}: {why}"); + if self.refused.see(&what) { + println!("{REFUSED}{what}"); + println!("{OWN}that was {device}.{method}"); + } + } + + /// Find the machine's batteries and adapters in `aml`, telling its + /// controller its space is there first, and read each battery's + /// information; `None` where there is nothing to read. + pub fn find(aml: &mut Aml<'_, K, C>, controller: bool) -> Option { + let mut power = Power { batteries: Vec::new(), adapters: Vec::new(), ac_said: None, refused: Ledger::default(), stopping: false }; + let takes = aml.host.takes; + let hids: Vec = match aml.interpreter.walk() { + Ok(mut walk) => { + let mut found = Vec::new(); + while let Some(entry) = walk.next() { + if entry.name == *b"_HID" && entry.kind != Kind::Device { + found.push(walk.path().to_owned()); + } + } + found + } + Err(why) => { + println!("acpiserver: {}the namespace's walk was refused: {}", acpiserver_api::NO_BATTERY, kind(&why)); + return None; + } + }; + let mut controllers = Vec::new(); + let mut batteries = Vec::new(); + for path in hids { + let device = path.strip_suffix("._HID").expect("the walk's path of a `_HID` ends in it").to_owned(); + let id = match power.evaluate(aml, &path, &[]) { + Ok(value) => value.as_ref().and_then(hid), + Err(why) => { + power.refused(&device, "_HID", &kind(&why)); + None + } + }; + match id.as_deref() { + Some(CONTROLLER) => controllers.push(device), + Some(BATTERY) => batteries.push(device), + Some(ADAPTER) => power.adapters.push(device), + _ => {} + } + } + if power.stopping { + return None; + } + + if controller { + for device in &controllers { + match power.evaluate(aml, &format!("{device}._GLK"), &[]) { + Ok(None | Some(Value::Integer(0))) => {} + Ok(Some(_)) => { + println!( + "acpiserver: {}the embedded controller's _GLK asks for the Global Lock around its transactions, which this server takes around none yet", + acpiserver_api::NO_BATTERY + ); + println!("{OWN}that was {device}._GLK"); + return None; + } + Err(why) => power.refused(device, "_GLK", &kind(&why)), + } + // §6.5.4: Arg0 the EmbeddedControl space, Arg1 1 for "connect". + if let Err(why) = power.evaluate(aml, &format!("{device}._REG"), &[Value::Integer(3), Value::Integer(1)]) { + power.refused(device, "_REG", &kind(&why)); + } + } + } + + let count = batteries.len(); + for device in batteries { + let sta = match power.evaluate(aml, &format!("{device}._STA"), &[]) { + Ok(None) => 0x0F, + Ok(Some(Value::Integer(sta))) => sta, + Ok(Some(_)) => { + power.refused(&device, "_STA", "no Integer"); + continue; + } + Err(why) => { + power.refused(&device, "_STA", &kind(&why)); + continue; + } + }; + if sta & BATTERY_PRESENT == 0 { + println!("{OWN}{device} says no battery is present: _STA {sta:#x}"); + continue; + } + let info = match power.evaluate(aml, &format!("{device}._BIX"), &[]) { + Ok(None) => match power.evaluate(aml, &format!("{device}._BIF"), &[]) { + Ok(None) => Err(("_BIF", "the device has neither _BIX nor _BIF".to_owned())), + Ok(Some(value)) => bif(&value).map_err(|why| ("_BIF", why)), + Err(why) => Err(("_BIF", kind(&why))), + }, + Ok(Some(value)) => bix(&value).map_err(|why| ("_BIX", why)), + Err(why) => Err(("_BIX", kind(&why))), + }; + match info { + Ok(info) => power.batteries.push(Battery { path: device, info, said: None }), + Err((method, why)) => power.refused(&device, method, &why), + } + } + if power.stopping { + return None; + } + + let present = power.batteries.len(); + for (n, battery) in power.batteries.iter().enumerate() { + println!("acpiserver: {}battery {} of {present} {}", acpiserver_api::BATTERY_INFO, n + 1, said_info(&battery.info)); + println!("{OWN}battery {} of {present} is {}", n + 1, battery.path); + } + if present > 0 { + power.read(aml); + } + let found = format!( + "{present} of {count} control-method batteries present, {} AC adapter(s), {} embedded controller device(s) told their space is there; the Global Lock taken {} times on the way", + power.adapters.len(), + if controller { controllers.len() } else { 0 }, + aml.host.takes - takes, + ); + if present == 0 { + println!("acpiserver: {}{found}", acpiserver_api::NO_BATTERY); + return None; + } + println!("acpiserver: power sources: {found}, the first reading's included; each read every {} s", POLL.as_secs()); + Some(power) + } + + /// Read every battery and adapter, saying a line of each that moved. + pub fn read(&mut self, aml: &mut Aml<'_, K, C>) { + let mut ac = None; + for at in 0..self.adapters.len() { + let device = self.adapters[at].clone(); + match self.evaluate(aml, &format!("{device}._PSR"), &[]) { + Ok(Some(Value::Integer(online))) => ac = Some(ac.unwrap_or(false) || online != 0), + Ok(Some(_)) => self.refused(&device, "_PSR", "no Integer"), + Ok(None) => self.refused(&device, "_PSR", "the adapter has no _PSR"), + Err(_) if self.stopping => return, + Err(why) => self.refused(&device, "_PSR", &kind(&why)), + } + } + let ac_moved = self.ac_said != Some(ac); + self.ac_said = Some(ac); + let ac = match ac { + Some(true) => "AC online", + Some(false) => "AC offline", + None if self.adapters.is_empty() => "no AC adapter", + None => "AC unknown", + }; + let count = self.batteries.len(); + for n in 0..count { + let device = self.batteries[n].path.clone(); + let status = match self.evaluate(aml, &format!("{device}._BST"), &[]) { + Ok(Some(value)) => bst(&value), + Ok(None) => Err("the battery has no _BST".into()), + Err(_) if self.stopping => return, + Err(why) => Err(kind(&why)), + }; + let status = match status { + Ok(status) => status, + Err(why) => { + self.refused(&device, "_BST", &why); + continue; + } + }; + let battery = &mut self.batteries[n]; + let now = (percent(&battery.info, &status), status.state); + if battery.said != Some(now) || ac_moved { + battery.said = Some(now); + println!("acpiserver: {}battery {} of {count}: {}; {ac}", acpiserver_api::BATTERY_READ, n + 1, said_status(&battery.info, &status)); + } + } + } +} + +/// The interpreter's own test encodings of §20.2, which the namespace +/// tests below assemble a machine's tables from. +#[cfg(test)] +#[path = "../aml/tests/common/mod.rs"] +mod common; + +#[cfg(test)] +mod tests { + use super::*; + use crate::aml::tests::{crafted, machine, sealed, CRAFTED_RSDP, QEMU, RSDP}; + use crate::ec::tests::Emulated; + use crate::host::tests::Scripted; + + use super::common; + use common::{device, field, if_, index, int, lequal, method, op_region, package, ret, scope, skip, store, string, unit as field_unit}; + + /// The controller's space as the fixture's fields lay it out. + const STATE: u8 = 0x38; + const AC: u8 = 0x46; + const PAGE: u8 = 0x81; + const RATE: u8 = 0xA0; + const VOLTAGE: u8 = 0xA8; + + /// A laptop's namespace as §10.2.2, §10.3 and §6.5.4 have one: an + /// embedded controller whose `_REG` records that its space is there; a + /// battery below it whose `_STA` says it is present only once `_REG` has + /// run, whose `_BIX` is constant and whose `_BST` selects a page of the + /// controller's space with a write and then reads it; and an adapter whose + /// `_PSR` reads the controller. `glk` is the controller's `_GLK`, where + /// it has one. + fn laptop(glk: Option) -> Vec { + let fields = [ + skip(8 * usize::from(STATE)), + field_unit("BSTA", 8), + skip(8 * usize::from(AC - STATE - 1)), + field_unit("ACST", 8), + skip(8 * usize::from(PAGE - AC - 1)), + field_unit("PAGE", 8), + skip(8 * usize::from(RATE - PAGE - 1)), + field_unit("RATE", 16), + field_unit("REMN", 16), + skip(8 * usize::from(VOLTAGE - RATE - 4)), + field_unit("VOLT", 16), + ]; + let mut bix: Vec> = [1, 0, 57000, 50000, 1, 11520, 5700, 570, 312, 95000, 0xFFFF_FFFF, 0xFFFF_FFFF, 1000, 1000, 100, 100] + .map(int) + .to_vec(); + bix.extend(["MODEL", "SERIAL", "LION", "OEM"].map(string)); + bix.push(int(1)); + let bst = [ + store(b"NVSB", &[0x60]), + store(&int(1), b"PAGE"), + store(b"BSTA", &index(b"BSTP", &int(0), &[0])), + store(b"RATE", &index(b"BSTP", &int(1), &[0])), + store(b"REMN", &index(b"BSTP", &int(2), &[0])), + store(b"VOLT", &index(b"BSTP", &int(3), &[0])), + ret(b"BSTP"), + ] + .concat(); + let battery = device( + "BAT0", + &[ + common::def_name("_HID", &int(0x0A0C_D041)), + method("_STA", 0, &[if_(b"ECOK", &ret(&int(0x1F))), ret(&int(0x0F))].concat()), + method("_BIX", 0, &ret(&package(&bix))), + common::def_name("BSTP", &package(&[int(0), int(0), int(0), int(0)])), + method("_BST", 0, &bst), + ] + .concat(), + ); + let controller = device( + "EC0", + &[ + common::def_name("_HID", &int(0x090C_D041)), + op_region("NVSR", 0x00, &int(NVS), &int(1)), + field("NVSR", 0x01, &[field_unit("NVSB", 8)]), + op_region("ECOR", 0x03, &int(0), &int(0x100)), + field("ECOR", 0x01, &fields), + common::def_name("ECOK", &int(0)), + method("_REG", 2, &if_(&lequal(&[0x68], &int(3)), &store(&[0x69], b"ECOK"))), + glk.map_or(Vec::new(), |glk| common::def_name("_GLK", &int(glk))), + battery, + ] + .concat(), + ); + let adapter = device("AC", &[common::def_name("_HID", &string("ACPI0003")), method("_PSR", 0, &ret(&common::name("\\_SB.EC0.ACST")))].concat()); + sealed(b"DSDT", &scope("\\_SB", &[controller, adapter].concat())) + } + + /// What the fixture's controller holds: discharging at 6210 mW with + /// 43511 of 50000 mWh left at 12312 mV, off AC. + fn space() -> [u8; 256] { + let mut space = [0; 256]; + space[usize::from(STATE)] = 1; + let at = usize::from(RATE); + space[at..at + 2].copy_from_slice(&6210u16.to_le_bytes()); + space[at + 2..at + 4].copy_from_slice(&43511u16.to_le_bytes()); + let at = usize::from(VOLTAGE); + space[at..at + 2].copy_from_slice(&12312u16.to_le_bytes()); + space + } + + /// A byte of firmware memory `_BST` reads first, as real ones read their + /// NVS. + const NVS: u64 = 0x7700_0000; + + fn laptop_machine(glk: Option) -> Scripted { + let mut kernel = crafted(&laptop(glk), &[]); + kernel.memory.push((NVS, 10, vec![0])); + kernel + } + + fn loaded(kernel: &Scripted, ec: Option) -> Aml<'_, Scripted, Emulated> { + let (_, aml) = crate::aml::load(kernel, ec, CRAFTED_RSDP); + aml.expect("the fixture's DSDT loads") + } + + fn done(aml: &Aml<'_, Scripted, Emulated>) -> Vec<(u8, u8, u8)> { + aml.host.ec.as_ref().expect("a controller").done.clone() + } + + const RD: u8 = 0x80; + const WR: u8 = 0x81; + + /// The controller is told its space is there before the battery is asked + /// whether it is present, which here it says only after; its information + /// is read once and the battery read with it, through the controller's + /// transactions alone: the adapter's byte, then the page `_BST` selects, + /// written before its fields are read, a byte at a time. + #[test] + fn a_laptops_battery_is_found_after_reg_and_read_through_the_controller() { + let kernel = laptop_machine(None); + let mut aml = loaded(&kernel, Some(Emulated::new(space()))); + let power = Power::find(&mut aml, true).expect("a battery"); + assert_eq!(power.adapters.len(), 1); + assert_eq!(power.batteries.len(), 1); + assert_eq!(power.batteries[0].path, "\\_SB_.EC0_.BAT0"); + assert_eq!( + power.batteries[0].info, + Info { from: "_BIX", unit: Unit::Power, design: Some(57000), full: Some(50000), design_voltage: Some(11520), cycles: Some(312) } + ); + assert_eq!( + done(&aml), + [ + (RD, AC, 0), + (WR, PAGE, 1), + (RD, STATE, 0), + (RD, RATE, 0), + (RD, RATE + 1, 0), + (RD, RATE + 2, 0), + (RD, RATE + 3, 0), + (RD, VOLTAGE, 0), + (RD, VOLTAGE + 1, 0), + ] + ); + assert_eq!(power.batteries[0].said, Some((Some(87), DISCHARGING))); + assert_eq!(power.ac_said, Some(Some(false))); + assert!(power.refused.is_empty() && aml.host.refused.is_empty()); + assert!(kernel.asked.borrow().iter().all(|access| access.write == 0), "the kernel was asked for a write"); + } + + /// A line is said only where what it says moved: the percent, the state + /// or the adapter. + #[test] + fn a_reading_is_kept_until_its_percent_its_state_or_the_adapter_moves() { + let kernel = laptop_machine(None); + let mut aml = loaded(&kernel, Some(Emulated::new(space()))); + let mut power = Power::find(&mut aml, true).expect("a battery"); + let poke = |aml: &mut Aml<'_, Scripted, Emulated>, at: u8, bytes: &[u8]| { + let ec = aml.host.ec.as_mut().expect("a controller"); + ec.space[usize::from(at)..usize::from(at) + bytes.len()].copy_from_slice(bytes); + }; + power.read(&mut aml); + assert_eq!((power.batteries[0].said, power.ac_said), (Some((Some(87), DISCHARGING)), Some(Some(false)))); + poke(&mut aml, RATE, &7000u16.to_le_bytes()); + poke(&mut aml, RATE + 2, &43600u16.to_le_bytes()); + power.read(&mut aml); + assert_eq!(power.batteries[0].said, Some((Some(87), DISCHARGING)), "87% discharging at another rate is no new line"); + poke(&mut aml, RATE + 2, &43000u16.to_le_bytes()); + power.read(&mut aml); + assert_eq!(power.batteries[0].said, Some((Some(86), DISCHARGING))); + poke(&mut aml, AC, &[1]); + power.read(&mut aml); + assert_eq!(power.ac_said, Some(Some(true))); + poke(&mut aml, STATE, &[CHARGING as u8]); + power.read(&mut aml); + assert_eq!(power.batteries[0].said, Some((Some(86), CHARGING))); + } + + /// No controller in the machine's row: `_REG` is not run, the battery + /// says it is absent, and the controller's space is never reached. + #[test] + fn a_row_with_no_controller_runs_no_reg_and_reads_no_battery() { + let kernel = laptop_machine(None); + let mut aml = loaded(&kernel, None); + assert!(Power::find(&mut aml, false).is_none()); + let ecok = aml.interpreter.evaluate(&mut aml.host, "\\_SB_.EC0_.ECOK", &[]); + assert_eq!(ecok, Ok(Value::Integer(0)), "_REG ran for a controller the row does not name"); + assert!(aml.host.refused.is_empty()); + } + + /// §6.5.7: a controller whose `_GLK` is 1 wants the Global Lock around + /// each transaction, which this server does not take: no battery is + /// read, and the controller is not even told its space is there. + #[test] + fn a_controller_that_wants_the_global_lock_has_no_battery_read() { + for (glk, read) in [(Some(1), false), (Some(0), true), (None, true)] { + let kernel = laptop_machine(glk); + let mut aml = loaded(&kernel, Some(Emulated::new(space()))); + assert_eq!(Power::find(&mut aml, true).is_some(), read, "_GLK {glk:?}"); + let ecok = aml.interpreter.evaluate(&mut aml.host, "\\_SB_.EC0_.ECOK", &[]); + assert_eq!(ecok, Ok(Value::Integer(u64::from(read))), "_GLK {glk:?}"); + assert_eq!(done(&aml).is_empty(), !read, "_GLK {glk:?}: {:x?}", done(&aml)); + } + } + + /// QEMU's q35 has neither a battery nor an adapter, and its row no + /// controller. + #[test] + fn qemus_namespace_has_no_battery() { + let kernel = machine(QEMU); + let (_, aml) = crate::aml::load(&kernel, None::, RSDP); + let mut aml = aml.expect("QEMU's DSDT loads"); + assert!(Power::find(&mut aml, false).is_none()); + assert!(aml.host.refused.is_empty()); + } + + /// A `_BST` that is no package of four is said once and counted, and the + /// battery's last line stands. + #[test] + fn a_refused_reading_is_said_once_and_counted() { + let kernel = laptop_machine(None); + let mut aml = loaded(&kernel, Some(Emulated::new(space()))); + let mut power = Power::find(&mut aml, true).expect("a battery"); + power.read(&mut aml); + let said = power.batteries[0].said; + aml.host.ec.as_mut().expect("a controller").space[usize::from(STATE)] = (CHARGING | DISCHARGING) as u8; + for _ in 0..3 { + power.read(&mut aml); + } + assert_eq!(power.batteries[0].said, said); + assert_eq!(power.refused.counts(), "_BST: its state says charging and discharging at once, which §10.2.2 forbids x3"); + } + + /// A machine that stops while `_BST` reads its firmware memory ends the + /// read with nothing refused, and its battery's last line stands. + #[test] + fn a_machine_that_stops_mid_read_refuses_nothing() { + let kernel = laptop_machine(None); + let mut aml = loaded(&kernel, Some(Emulated::new(space()))); + let mut power = Power::find(&mut aml, true).expect("a battery"); + power.read(&mut aml); + let said = power.batteries[0].said; + kernel.stops_after.set(Some(0)); + power.read(&mut aml); + assert!(aml.host.stopping, "the read reached the kernel"); + assert_eq!(power.batteries[0].said, said); + assert!(power.refused.is_empty(), "{}", power.refused.counts()); + } + + fn integers(values: &[u64]) -> Vec { + values.iter().map(|&value| Value::Integer(value)).collect() + } + + /// A `_BIF` of §10.2.2's 13 elements: the numbers given, then the + /// granularities and the four strings. + fn bif_of(numbers: [u64; 9]) -> Value { + let mut elements = integers(&numbers); + elements.extend(["MODEL", "SERIAL", "LION", "OEM"].map(|s| Value::String(s.as_bytes().to_vec()))); + Value::Package(elements) + } + + /// A `_BIX` of `len` elements, revision first: Power Unit, Design + /// Capacity, Last Full Charge Capacity, Technology, Design Voltage, + /// warning and low capacities, Cycle Count, then the rest. + fn bix_of(revision: u64, numbers: [u64; 8], len: usize) -> Value { + let mut elements = integers(&[revision]); + elements.extend(integers(&numbers)); + elements.extend(integers(&[95000, 0xFFFF_FFFF, 0xFFFF_FFFF, 1000, 1000, 100, 100])); + elements.extend(["MODEL", "SERIAL", "LION", "OEM"].map(|s| Value::String(s.as_bytes().to_vec()))); + if len == 21 { + elements.push(Value::Integer(1)); + } + assert_eq!(elements.len(), len); + Value::Package(elements) + } + + #[test] + fn a_bif_is_read_by_its_place_in_the_package() { + let info = bif(&bif_of([0, 57000, 51230, 1, 11520, 2850, 570, 100, 100])).expect("a _BIF"); + assert_eq!( + info, + Info { from: "_BIF", unit: Unit::Power, design: Some(57000), full: Some(51230), design_voltage: Some(11520), cycles: None } + ); + assert_eq!(said_info(&info), "from _BIF: design 57000 mWh, last full 51230 mWh, design voltage 11520 mV, cycles unknown"); + } + + #[test] + fn a_bix_of_either_revision_is_read_at_the_same_places() { + for (revision, len) in [(0, 20), (1, 21)] { + let info = bix(&bix_of(revision, [1, 4000, 3600, 1, 15400, 400, 200, 312], len)).expect("a _BIX"); + assert_eq!( + info, + Info { from: "_BIX", unit: Unit::Current, design: Some(4000), full: Some(3600), design_voltage: Some(15400), cycles: Some(312) } + ); + assert_eq!(said_info(&info), "from _BIX: design 4000 mAh, last full 3600 mAh, design voltage 15400 mV, 312 cycles"); + } + } + + /// §10.2.2: 0xFFFFFFFF is a value the battery does not know, and is said + /// so, never as a number. + #[test] + fn an_unknown_dword_is_unknown_and_never_a_number() { + let info = bix(&bix_of(1, [0, UNKNOWN, UNKNOWN, 1, UNKNOWN, 0, 0, UNKNOWN], 21)).expect("a _BIX"); + assert_eq!((info.design, info.full, info.design_voltage, info.cycles), (None, None, None, None)); + assert_eq!(said_info(&info), "from _BIX: design unknown, last full unknown, design voltage unknown, cycles unknown"); + let status = bst(&Value::Package(integers(&[DISCHARGING, UNKNOWN, UNKNOWN, UNKNOWN]))).expect("a _BST"); + assert_eq!(said_status(&info, &status), "charge unknown, unknown of unknown, discharging, rate unknown, unknown, moving unknown mW"); + } + + #[test] + fn a_bst_reads_its_four_dwords_and_names_its_state() { + let info = bif(&bif_of([0, 57000, 50000, 1, 11520, 0, 0, 1, 1])).expect("a _BIF"); + let status = bst(&Value::Package(integers(&[DISCHARGING, 6210, 43511, 12312]))).expect("a _BST"); + assert_eq!(status, Status { state: DISCHARGING, rate: Some(6210), remaining: Some(43511), voltage: Some(12312) }); + assert_eq!(percent(&info, &status), Some(87), "rounded down"); + assert_eq!(milliwatts(&info, &status), Some(6210), "a rate in mW is the power"); + assert_eq!(said_status(&info, &status), "87%, 43511 mWh of 50000 mWh, discharging, rate 6210 mW, 12312 mV, moving 6210 mW"); + + let charging = bst(&Value::Package(integers(&[CHARGING | CRITICAL | LIMITING | 0x30, 0, 50000, 13000]))).expect("a _BST"); + assert_eq!(state(charging.state), "charging, critical, charge limiting, reserved bits 0x30"); + assert_eq!(state(0), "neither charging nor discharging"); + assert_eq!(percent(&info, &charging), Some(100)); + } + + /// A rate in mA moves the rate times the present voltage: mA × mV / 1000 + /// is mW. With no voltage known there is no power, and no design voltage + /// stands in for it. + #[test] + fn a_current_is_power_only_at_its_present_voltage() { + let info = bix(&bix_of(1, [1, 4000, 3600, 1, 15400, 0, 0, 1], 21)).expect("a _BIX"); + let status = bst(&Value::Package(integers(&[DISCHARGING, 543, 1800, 13672]))).expect("a _BST"); + assert_eq!(milliwatts(&info, &status), Some(543 * 13672 / 1000)); + assert_eq!(percent(&info, &status), Some(50)); + assert_eq!(said_status(&info, &status), "50%, 1800 mAh of 3600 mAh, discharging, rate 543 mA, 13672 mV, moving 7423 mW"); + let unknown_voltage = Status { voltage: None, ..status }; + assert_eq!(milliwatts(&info, &unknown_voltage), None); + } + + #[test] + fn a_full_charge_of_zero_is_no_percent() { + let info = bif(&bif_of([0, 57000, 0, 1, 11520, 0, 0, 1, 1])).expect("a _BIF"); + let status = bst(&Value::Package(integers(&[0, 0, 100, 12000]))).expect("a _BST"); + assert_eq!(percent(&info, &status), None); + } + + #[test] + fn a_package_that_is_not_what_10_2_2_says_is_refused_by_name() { + assert_eq!(bst(&Value::Package(integers(&[CHARGING | DISCHARGING, 0, 0, 0]))), Err("its state says charging and discharging at once, which §10.2.2 forbids".into())); + assert_eq!(bst(&Value::Package(integers(&[0, 0, 0]))), Err("a package of 3 elements, where §10.2.2 has at least 4".into())); + assert_eq!(bst(&Value::Integer(0)), Err("no package".into())); + assert_eq!(bst(&Value::Package(integers(&[0, 1 << 32, 0, 0]))), Err("its Battery Present Rate is wider than a DWORD".into())); + let mut stringy = integers(&[0, 0, 0]); + stringy.push(Value::String(b"12000".to_vec())); + assert_eq!(bst(&Value::Package(stringy)), Err("its Battery Present Voltage is no Integer".into())); + assert_eq!(bif(&bif_of([2, 0, 0, 0, 0, 0, 0, 0, 0])), Err("its Power Unit is 2, which §10.2.2 does not define".into())); + assert_eq!(bix(&bix_of(0, [0; 8], 20)).map(|info| info.unit), Ok(Unit::Power)); + let Value::Package(mut short) = bix_of(0, [0; 8], 20) else { unreachable!() }; + short.pop(); + assert_eq!(bix(&Value::Package(short)), Err("a package of 19 elements, where §10.2.2 has at least 20".into())); + } + + /// §6.1.5's compressed EISA ID, as `EISAID ("PNP0C0A")` compiles it, and + /// a string `_HID` as it is. + #[test] + fn an_hid_is_read_as_its_text_whether_compressed_or_a_string() { + assert_eq!(hid(&Value::Integer(0x0A0C_D041)).as_deref(), Some("PNP0C0A")); + assert_eq!(hid(&Value::Integer(0x090C_D041)).as_deref(), Some("PNP0C09")); + assert_eq!(hid(&Value::String(b"ACPI0003".to_vec())).as_deref(), Some("ACPI0003")); + assert_eq!(hid(&Value::Integer(0x8000_0000_u32.swap_bytes().into())), None, "the reserved top bit set"); + assert_eq!(hid(&Value::Integer(1 << 32)), None); + assert_eq!(hid(&Value::Buffer(b"PNP0C0A".to_vec())), None); + } +} diff --git a/userland/acpiserver/src/ec.rs b/userland/acpiserver/src/ec.rs index 3ce447d1725..cdcd368cc71 100644 --- a/userland/acpiserver/src/ec.rs +++ b/userland/acpiserver/src/ec.rs @@ -3,10 +3,11 @@ //! answers what its host does next, so how long a host waits, and how, is the //! host's. //! -//! A transaction is a command byte written to the command register and the one -//! byte read back, the write once the controller's input buffer is empty and -//! the read once its output buffer is full. Stage 1 asks only for queries; -//! reads and writes of the controller's space join when AML reaches it. +//! A transaction is a command byte written to the command register, then the +//! bytes its command takes written to the data register, each once the +//! controller's input buffer is empty; then a read of the one byte it answers +//! once its output buffer is full, or, where it answers none, a wait until +//! the controller has taken the last byte written. /// The status register's bits (Table 12.3). pub const OBF: u8 = 1 << 0; @@ -14,8 +15,12 @@ pub const IBF: u8 = 1 << 1; /// An event is waiting to be queried. pub const SCI_EVT: u8 = 1 << 5; +/// Table 12.6: read a byte of the controller's space (§12.3.1). +const RD_EC: u8 = 0x80; +/// Table 12.6: write a byte of the controller's space (§12.3.2). +const WR_EC: u8 = 0x81; /// Table 12.6: query, which answers the number of the event waiting, or 0 -/// for none. +/// for none (§12.3.5). const QR_EC: u8 = 0x84; /// What the host does next. @@ -24,9 +29,10 @@ pub enum Do { /// Read the status register again, until it says otherwise. Wait(Wait), WriteCommand(u8), + WriteData(u8), /// Read the data register and hand the byte to [`Transaction::read`]. ReadData, - /// Finished, with the byte read back. + /// Finished, with the byte read back, or the byte written. Done(u8), } @@ -39,34 +45,65 @@ pub enum Wait { #[derive(Clone, Copy, PartialEq, Eq, Debug)] enum Phase { - Command, + /// Writing the byte at this index of `bytes`. + Send(usize), Answer, + /// The last byte is written, and the controller has not taken it yet. + Settle, Done(u8), } /// One transaction, from its command to its answer. #[derive(Debug)] pub struct Transaction { - command: u8, + /// The command, then what it takes. + bytes: [u8; 3], + len: usize, + answers: bool, phase: Phase, } impl Transaction { + fn new(sent: &[u8], answers: bool) -> Self { + let mut bytes = [0; 3]; + bytes[..sent.len()].copy_from_slice(sent); + Self { bytes, len: sent.len(), answers, phase: Phase::Send(0) } + } + /// Ask for the waiting event's number. pub fn query() -> Self { - Self { command: QR_EC, phase: Phase::Command } + Self::new(&[QR_EC], true) + } + + /// The byte at `address` of the controller's space. + pub fn read_at(address: u8) -> Self { + Self::new(&[RD_EC, address], true) + } + + /// `value` to `address` of the controller's space. + pub fn write_at(address: u8, value: u8) -> Self { + Self::new(&[WR_EC, address, value], false) } /// What to do, given what the status register reads now. pub fn step(&mut self, status: u8) -> Do { match self.phase { - Phase::Command if status & IBF != 0 => Do::Wait(Wait::InputEmpty), - Phase::Command => { - self.phase = Phase::Answer; - Do::WriteCommand(self.command) + Phase::Send(_) | Phase::Settle if status & IBF != 0 => Do::Wait(Wait::InputEmpty), + Phase::Send(i) => { + self.phase = match i + 1 { + next if next < self.len => Phase::Send(next), + _ if self.answers => Phase::Answer, + _ => Phase::Settle, + }; + if i == 0 { Do::WriteCommand(self.bytes[0]) } else { Do::WriteData(self.bytes[i]) } } Phase::Answer if status & OBF == 0 => Do::Wait(Wait::OutputFull), Phase::Answer => Do::ReadData, + Phase::Settle => { + let written = self.bytes[self.len - 1]; + self.phase = Phase::Done(written); + Do::Done(written) + } Phase::Done(byte) => Do::Done(byte), } } @@ -79,11 +116,11 @@ impl Transaction { } #[cfg(test)] -mod tests { +pub mod tests { use super::*; - /// A controller as Table 12.3 has it: the host's write fills the input - /// buffer, the controller takes it and later fills the output buffer. + /// A host that runs `tx` against a controller saying `statuses` in turn + /// and answering `answer` to a read of its data register. fn run(mut tx: Transaction, statuses: &[u8], answer: u8) -> (Vec, u8) { let mut said = Vec::new(); let mut statuses = statuses.iter().copied(); @@ -94,11 +131,105 @@ mod tests { match next { Do::ReadData => tx.read(answer), Do::Done(byte) => return (said, byte), - Do::Wait(_) | Do::WriteCommand(_) => {} + Do::Wait(_) | Do::WriteCommand(_) | Do::WriteData(_) => {} } } } + /// A controller as §12.2 and §12.3 have it: 256 bytes of space; a byte + /// written fills the input buffer until the controller takes it, which + /// it does `slow` status reads later; a command's answer fills the output + /// buffer as it takes the command's last byte. It refuses, by panic, + /// a write while its input buffer is full, a read of its data register + /// while its output buffer is empty, and a data byte no command asked for. + #[derive(Debug)] + pub struct Emulated { + pub space: [u8; 256], + pub slow: u32, + pending: u32, + input: bool, + output: Option, + /// The command taken, and the bytes it has taken after it. + taken: Vec, + pub queries: Vec, + /// Every command taken whole: `(command, address, value)`. + pub done: Vec<(u8, u8, u8)>, + } + + impl Emulated { + pub fn new(space: [u8; 256]) -> Self { + Emulated { space, slow: 2, pending: 0, input: false, output: None, taken: Vec::new(), queries: Vec::new(), done: Vec::new() } + } + + pub fn status(&mut self) -> u8 { + if self.input { + if self.pending == 0 { + self.input = false; + self.take(); + } else { + self.pending -= 1; + } + } + u8::from(self.output.is_some()) * OBF | u8::from(self.input) * IBF + } + + fn take(&mut self) { + let wants = match self.taken[0] { + RD_EC => 2, + WR_EC => 3, + QR_EC => 1, + other => panic!("the controller was sent command {other:#04x}"), + }; + if self.taken.len() < wants { + return; + } + let (command, address, value) = (self.taken[0], self.taken.get(1).copied().unwrap_or(0), self.taken.get(2).copied().unwrap_or(0)); + match command { + RD_EC => self.output = Some(self.space[usize::from(address)]), + WR_EC => self.space[usize::from(address)] = value, + _ => self.output = Some(self.queries.pop().unwrap_or(0)), + } + self.done.push((command, address, value)); + self.taken.clear(); + } + + fn written(&mut self, byte: u8) { + assert!(!self.input, "a byte was written while the controller's input buffer was full"); + self.input = true; + self.pending = self.slow; + self.taken.push(byte); + } + + pub fn command(&mut self, byte: u8) { + assert!(self.taken.is_empty(), "a command was written inside another's transaction"); + self.written(byte); + } + + pub fn data(&mut self, byte: u8) { + assert!(!self.taken.is_empty(), "a data byte was written that no command asked for"); + self.written(byte); + } + + pub fn read(&mut self) -> u8 { + self.output.take().expect("the data register was read with the output buffer empty") + } + + /// One transaction, driven as the server's host drives it. + pub fn transact(&mut self, mut tx: Transaction) -> u8 { + for _ in 0..1000 { + let status = self.status(); + match tx.step(status) { + Do::Wait(_) => {} + Do::WriteCommand(byte) => self.command(byte), + Do::WriteData(byte) => self.data(byte), + Do::ReadData => tx.read(self.read()), + Do::Done(byte) => return byte, + } + } + panic!("the transaction never ended: {tx:?} against {self:?}"); + } + } + #[test] fn a_query_writes_its_command_once_the_input_is_empty_and_reads_once_the_output_is_full() { let (said, byte) = run(Transaction::query(), &[IBF, IBF, 0, IBF, 0, OBF, 0], 0x1c); @@ -117,9 +248,78 @@ mod tests { assert_eq!(byte, 0x1c); } + /// §12.3.1: RD_EC, then the address once the command is taken, then the + /// byte once the output buffer is full. + #[test] + fn a_read_writes_its_address_only_once_the_command_is_taken() { + let (said, byte) = run(Transaction::read_at(0xA8), &[IBF, 0, IBF, IBF, 0, 0, OBF, 0], 0x5A); + assert_eq!( + said, + [ + Do::Wait(Wait::InputEmpty), + Do::WriteCommand(0x80), + Do::Wait(Wait::InputEmpty), + Do::Wait(Wait::InputEmpty), + Do::WriteData(0xA8), + Do::Wait(Wait::OutputFull), + Do::ReadData, + Do::Done(0x5A), + ] + ); + assert_eq!(byte, 0x5A); + } + + /// §12.3.2: WR_EC, the address, the value, each once the input buffer is + /// empty, and done only once the controller has taken the value. + #[test] + fn a_write_ends_once_the_controller_has_taken_its_value() { + let (said, byte) = run(Transaction::write_at(0x81, 0x01), &[0, IBF, 0, 0, IBF, IBF, OBF], 0); + assert_eq!( + said, + [ + Do::WriteCommand(0x81), + Do::Wait(Wait::InputEmpty), + Do::WriteData(0x81), + Do::WriteData(0x01), + Do::Wait(Wait::InputEmpty), + Do::Wait(Wait::InputEmpty), + Do::Done(0x01), + ], + "a full output buffer is no answer a write waits for" + ); + assert_eq!(byte, 0x01); + } + + #[test] + fn every_transaction_runs_against_a_controller_that_takes_each_byte_late() { + let mut space = [0; 256]; + space[0xA0] = 0x34; + for slow in [0, 1, 5] { + let mut ec = Emulated::new(space); + ec.slow = slow; + assert_eq!(ec.transact(Transaction::read_at(0xA0)), 0x34); + assert_eq!(ec.transact(Transaction::write_at(0x81, 0x02)), 0x02); + assert_eq!(ec.space[0x81], 0x02); + ec.queries.push(0x1C); + assert_eq!(ec.transact(Transaction::query()), 0x1C); + assert_eq!(ec.transact(Transaction::query()), 0, "no event waiting"); + assert_eq!(ec.done, [(0x80, 0xA0, 0), (0x81, 0x81, 0x02), (0x84, 0, 0), (0x84, 0, 0)]); + } + } + #[test] #[should_panic(expected = "a byte read that nothing asked for")] fn a_byte_before_the_command_is_refused() { Transaction::query().read(0); } + + #[test] + #[should_panic(expected = "a byte read that nothing asked for")] + fn a_write_reads_no_byte() { + let mut tx = Transaction::write_at(0, 0); + for _ in 0..3 { + tx.step(0); + } + tx.read(0); + } } diff --git a/userland/acpiserver/src/host.rs b/userland/acpiserver/src/host.rs index a76ddab746a..0174bf437f6 100644 --- a/userland/acpiserver/src/host.rs +++ b/userland/acpiserver/src/host.rs @@ -3,13 +3,15 @@ //! mediated access ([`toyos_abi::acpi`]), and [`Kernel`] is that access as //! this server asks for it, so a host test answers in the kernel's place. //! -//! **Nothing is written.** Loading a machine's tables writes nothing, and -//! this server evaluates nothing that does yet: a write in any space is -//! denied by name, and so is every access to the embedded controller's -//! space, which no transaction serves for AML yet. SystemCMOS never arrives: -//! the interpreter refuses that space itself. A read of memory, of a port or -//! of a function's configuration space is the kernel's to make or refuse, and -//! a refusal it names is denied under that name. +//! **Nothing is written but the embedded controller's space.** A write to +//! memory, a port or a function's configuration space is denied by name. The +//! controller's space is read and written a byte at a time through its own +//! transactions ([`crate::ec`]), on the ports this server holds +//! ([`Controller`]), and denied by name on a machine whose row names no +//! controller. SystemCMOS never arrives: the interpreter refuses that space +//! itself. A read of memory, of a port or of a function's configuration space +//! is the kernel's to make or refuse, and a refusal it names is denied under +//! that name. //! //! **The Global Lock is the kernel's to exchange** (ACPI 6.5 §5.2.10.1). A //! take that finds the firmware holding it is denied by name ([`HELD`]) and @@ -31,6 +33,7 @@ use std::time::{Duration, Instant}; use toyos_abi::acpi::{pci_address, Access, Refused, Space, Width, UNLISTED}; use toyos_aml::{Address, Denied, Host}; +use crate::ec::Transaction; use crate::ledger::Ledger; /// What opens a line that carries an address, a PCI function or a name the @@ -73,6 +76,12 @@ pub trait Kernel { fn s5(&self, slp_typ_a: u64) -> Result; } +/// The embedded controller, as this server drives it: one transaction run to +/// its end, or the server ended loudly where the controller stops answering. +pub trait Controller { + fn transact(&mut self, tx: Transaction) -> u8; +} + /// Why a read was not made. #[derive(Clone, Copy, PartialEq, Eq, Debug)] pub enum Refusal { @@ -143,8 +152,10 @@ impl Pages { } /// The interpreter's host on one machine, and what it was asked. -pub struct Firmware<'k, K> { +pub struct Firmware<'k, K, C> { kernel: &'k K, + /// The controller the kernel's row names, if it names one. + pub ec: Option, /// The zero of [`Host::timer`]. began: Instant, /// Reads made, by [`Space`]. @@ -154,22 +165,28 @@ pub struct Firmware<'k, K> { pub takes: u64, pub contended: u64, pub notifies: u64, + /// Bytes of the controller's space read and written. + pub ec_reads: u64, + pub ec_writes: u64, pub refused: Ledger, notified: Ledger, /// The kernel answered that the machine is stopping. pub stopping: bool, } -impl<'k, K: Kernel> Firmware<'k, K> { - pub fn new(kernel: &'k K) -> Self { +impl<'k, K: Kernel, C: Controller> Firmware<'k, K, C> { + pub fn new(kernel: &'k K, ec: Option) -> Self { Firmware { kernel, + ec, began: Instant::now(), reads: [0; 3], pages: Pages::default(), takes: 0, contended: 0, notifies: 0, + ec_reads: 0, + ec_writes: 0, refused: Ledger::default(), notified: Ledger::default(), stopping: false, @@ -190,6 +207,15 @@ impl<'k, K: Kernel> Firmware<'k, K> { self.stopping = true; Denied(Refusal::Stopping.to_string()) } + + /// The machine's controller, or the denial of an access to its space + /// where its row names none. + fn controller(&mut self, what: &str, at: Address) -> Result<&mut C, Denied> { + if self.ec.is_none() { + return Err(self.deny(format!("{what} {NO_CONTROLLER}"), format_args!("{at:x?}"))); + } + Ok(self.ec.as_mut().expect("just seen")) + } } fn wide(width: toyos_aml::Access) -> Width { @@ -201,15 +227,25 @@ fn wide(width: toyos_aml::Access) -> Width { } } -const NO_CONTROLLER: &str = "EmbeddedControl: this server runs no controller transaction for AML yet"; +const NO_CONTROLLER: &str = "EmbeddedControl: the machine's ACPI row names no embedded controller"; -impl Host for Firmware<'_, K> { +/// The one width the interpreter reaches the controller's space at. +fn byte(width: toyos_aml::Access) { + assert_eq!(width, toyos_aml::Access::Byte, "the interpreter reaches EmbeddedControl a byte at a time (Table 19.34)"); +} + +impl Host for Firmware<'_, K, C> { fn read(&mut self, at: Address, width: toyos_aml::Access) -> Result { let (space, address) = match at { Address::Memory(address) => (Space::SystemMemory, address), Address::Io(port) => (Space::SystemIo, u64::from(port)), Address::PciConfig { segment, bus, device, function, offset } => (Space::PciConfig, pci_address(segment, bus, device, function, offset)), - Address::EmbeddedControl(_) => return Err(self.deny(format!("a read of {NO_CONTROLLER}"), format_args!("{at:x?}"))), + Address::EmbeddedControl(address) => { + byte(width); + let read = self.controller("a read of", at)?.transact(Transaction::read_at(address)); + self.ec_reads += 1; + return Ok(u64::from(read)); + } }; match read(self.kernel, space, address, wide(width)) { Ok((value, memory_type)) => { @@ -226,12 +262,18 @@ impl Host for Firmware<'_, K> { fn write(&mut self, at: Address, width: toyos_aml::Access, value: u64) -> Result<(), Denied> { let what = match at { - Address::Memory(_) => "a write to SystemMemory: this server writes nothing for AML yet".into(), - Address::Io(_) => "a write to SystemIO: this server writes nothing for AML yet".into(), - Address::PciConfig { .. } => "a write to PCI_Config: this server writes nothing for AML yet".into(), - Address::EmbeddedControl(_) => format!("a write to {NO_CONTROLLER}"), + Address::Memory(_) => "SystemMemory", + Address::Io(_) => "SystemIO", + Address::PciConfig { .. } => "PCI_Config", + Address::EmbeddedControl(address) => { + byte(width); + let value = u8::try_from(value).expect("a byte access writes a byte"); + self.controller("a write to", at)?.transact(Transaction::write_at(address, value)); + self.ec_writes += 1; + return Ok(()); + } }; - Err(self.deny(what, format_args!("{width:?} {value:#x} to {at:x?}"))) + Err(self.deny(format!("a write to {what}: this server writes nothing there for AML yet"), format_args!("{width:?} {value:#x} to {at:x?}"))) } fn sleep(&mut self, ms: u64) { @@ -283,6 +325,7 @@ pub mod tests { use std::collections::VecDeque; use super::*; + use crate::ec::tests::Emulated; /// A kernel that answers reads from bytes at addresses, each range with a /// memory type, refuses `kept` ranges as the real one refuses memory of a @@ -388,7 +431,7 @@ pub mod tests { #[test] fn a_read_is_the_kernels_in_each_space_and_counted_by_space_and_page() { let kernel = machine(); - let mut host = Firmware::new(&kernel); + let mut host = Firmware::new(&kernel, None::); assert_eq!(host.read(Address::Memory(NVS), toyos_aml::Access::QWord), Ok(0x8877_6655_4433_2211)); assert_eq!(host.read(Address::Memory(NVS + 2), toyos_aml::Access::Word), Ok(0x4433)); assert_eq!(host.read(Address::Memory(NVS + 0x2000), toyos_aml::Access::Byte), Ok(0xAB)); @@ -422,7 +465,7 @@ pub mod tests { #[test] fn a_read_the_kernel_refuses_is_denied_under_the_kernels_name_and_counted() { let kernel = machine(); - let mut host = Firmware::new(&kernel); + let mut host = Firmware::new(&kernel, None::); let ram = "a SystemMemory read the kernel refused UsableMemory, in memory of type 7"; for _ in 0..3 { assert_eq!(host.read(Address::Memory(0x10_0000), toyos_aml::Access::DWord), Err(Denied(ram.into()))); @@ -439,26 +482,56 @@ pub mod tests { assert_eq!(host.pages.by_type(), "none"); } + impl Controller for Emulated { + fn transact(&mut self, tx: Transaction) -> u8 { + Emulated::transact(self, tx) + } + } + #[test] - fn nothing_is_written_and_the_controllers_space_is_not_reached() { + fn nothing_is_written_but_the_controllers_space_and_that_only_where_the_row_names_one() { let kernel = machine(); - let mut host = Firmware::new(&kernel); let function = Address::PciConfig { segment: 0, bus: 0, device: 0x1F, function: 3, offset: 0x40 }; - for (at, space) in [(Address::Memory(NVS), "SystemMemory"), (Address::Io(0xB2), "SystemIO"), (function, "PCI_Config")] { - let denied = host.write(at, toyos_aml::Access::Byte, 0).expect_err("a write was made"); - assert_eq!(denied.0, format!("a write to {space}: this server writes nothing for AML yet")); - } let controller = Address::EmbeddedControl(0x38); - assert_eq!(host.write(controller, toyos_aml::Access::Byte, 1), Err(Denied(format!("a write to {NO_CONTROLLER}")))); - assert_eq!(host.read(controller, toyos_aml::Access::Byte), Err(Denied(format!("a read of {NO_CONTROLLER}")))); + for ec in [None, Some(Emulated::new([0; 256]))] { + let mut host = Firmware::new(&kernel, ec); + for (at, space) in [(Address::Memory(NVS), "SystemMemory"), (Address::Io(0xB2), "SystemIO"), (function, "PCI_Config")] { + let denied = host.write(at, toyos_aml::Access::Byte, 0).expect_err("a write was made"); + assert_eq!(denied.0, format!("a write to {space}: this server writes nothing there for AML yet")); + } + if host.ec.is_none() { + assert_eq!(host.write(controller, toyos_aml::Access::Byte, 1), Err(Denied(format!("a write to {NO_CONTROLLER}")))); + assert_eq!(host.read(controller, toyos_aml::Access::Byte), Err(Denied(format!("a read of {NO_CONTROLLER}")))); + assert_eq!((host.ec_reads, host.ec_writes), (0, 0)); + } + } assert!(kernel.asked.borrow().is_empty(), "the kernel was asked for an access this server denies itself"); + } + + /// The controller's space is its own transactions', one a byte: a read is + /// RD_EC of its address, a write WR_EC of its address and value, and + /// neither reaches the kernel. + #[test] + fn the_controllers_space_is_read_and_written_through_its_transactions() { + let kernel = machine(); + let mut space = [0; 256]; + space[0xA8] = 0x5A; + let mut host = Firmware::new(&kernel, Some(Emulated::new(space))); + assert_eq!(host.read(Address::EmbeddedControl(0xA8), toyos_aml::Access::Byte), Ok(0x5A)); + assert_eq!(host.write(Address::EmbeddedControl(0x81), toyos_aml::Access::Byte, 0x01), Ok(())); + assert_eq!(host.read(Address::EmbeddedControl(0x81), toyos_aml::Access::Byte), Ok(0x01)); + let ec = host.ec.as_ref().expect("a controller"); + assert_eq!(ec.done, [(0x80, 0xA8, 0), (0x81, 0x81, 0x01), (0x80, 0x81, 0)]); + assert_eq!((host.ec_reads, host.ec_writes), (2, 1)); assert_eq!(host.reads, [0, 0, 0]); + assert!(host.refused.is_empty()); + assert!(kernel.asked.borrow().is_empty(), "the controller's space reached the kernel"); } #[test] fn the_lock_is_taken_and_given_back_through_the_kernel() { let kernel = machine(); - let mut host = Firmware::new(&kernel); + let mut host = Firmware::new(&kernel, None::); assert_eq!(host.global_lock(true), Ok(())); assert!(kernel.held.get()); assert_eq!(host.global_lock(false), Ok(())); @@ -471,7 +544,7 @@ pub mod tests { fn a_lock_the_firmware_holds_is_denied_by_name_and_counted() { let kernel = machine(); kernel.takes.borrow_mut().extend([Take::Pending, Take::Pending, Take::Taken]); - let mut host = Firmware::new(&kernel); + let mut host = Firmware::new(&kernel, None::); for _ in 0..2 { assert_eq!(host.global_lock(true), Err(Denied(HELD.into()))); assert!(!kernel.held.get(), "a lock the firmware holds was reported taken"); @@ -488,7 +561,7 @@ pub mod tests { fn a_lock_the_kernel_cannot_take_is_denied_and_never_reported_held() { let kernel = machine(); kernel.takes.borrow_mut().push_back(Take::Unusable); - let mut host = Firmware::new(&kernel); + let mut host = Firmware::new(&kernel, None::); assert_eq!( host.global_lock(true), Err(Denied("the Global Lock: the kernel exchanges no lock word in this machine's FACS".into())) @@ -500,7 +573,7 @@ pub mod tests { fn a_stopping_machine_denies_everything_and_is_no_refusal_of_the_firmwares() { let kernel = machine(); kernel.stops_after.set(Some(1)); - let mut host = Firmware::new(&kernel); + let mut host = Firmware::new(&kernel, None::); assert_eq!(host.read(Address::Memory(NVS), toyos_aml::Access::Byte), Ok(0x11)); assert!(!host.stopping); let stopping = Denied(Refusal::Stopping.to_string()); diff --git a/userland/acpiserver/src/main.rs b/userland/acpiserver/src/main.rs index 958f777578c..29a2626c3bf 100644 --- a/userland/acpiserver/src/main.rs +++ b/userland/acpiserver/src/main.rs @@ -29,10 +29,16 @@ //! drain than [`QUERIES`], and [`sci::EMPTY_SCIS`] SCIs in a row that carried //! nothing are each a panic naming the registers. //! +//! **After the load the machine's batteries and AC adapters are found** in +//! the namespace it kept, and read every [`battery::POLL`] between SCIs +//! ([`battery`]): the controller's space is reached only there, never inside +//! a drain. +//! //! The log carries each query number the first time it is taken and a count //! of every one at [`COUNTS`] intervals, never a line per event. mod aml; +mod battery; mod ec; mod host; mod ledger; @@ -51,7 +57,9 @@ use toyos_abi::acpi::{Access, AcpiInfo, Block, FIXED_POWER_BUTTON}; use toyos_abi::syscall::{DeviceType, SyscallError}; use ec::{Do, Transaction, Wait}; -use host::{Answer, Kernel, Stopping, Take}; +use aml::Aml; +use battery::Power; +use host::{Answer, Controller, Kernel, Stopping, Take}; use sci::{Event, Served, Unserved, Unstopped, PM1_STATUS, PWRBTN}; /// The most a controller is waited for at one step of a transaction: one that @@ -62,10 +70,15 @@ const QUERIES: usize = 32; /// How often the log is told the queries' counts, where they moved. const COUNTS: Duration = Duration::from_secs(30); -struct Server { - dev: AcpiDev, +struct Server<'a> { + dev: &'a AcpiDev, info: AcpiInfo, served: Served, + /// The controller the row names, if it names one. + ec: Option, + /// The namespace the load kept, and the power sources found in it. + aml: Option, Ports>>, + power: Option, /// Taken off the controller by the drain, run after it. queued: VecDeque, /// Every query number taken, and how often. @@ -88,10 +101,14 @@ fn main() { power_button: info.flags & FIXED_POWER_BUTTON != 0, ec_gpe: info.has_ec().then_some(info.ec_gpe), }; + let claim = Claim(&dev); let mut server = Server { - dev, + dev: &dev, info, served, + ec: info.has_ec().then_some(Ports { command: info.ec_command.port, data: info.ec_data.port }), + aml: None, + power: None, queued: VecDeque::new(), counts: BTreeMap::new(), counted: 0, @@ -101,10 +118,60 @@ fn main() { power_off: false, }; server.arm(); - server.power_off = aml::load(&Claim(&server.dev), server.info.rsdp).handed; + let (loaded, kept) = aml::load(&claim, server.ec, server.info.rsdp); + server.power_off = loaded.handed; + server.aml = kept; + if let Some(aml) = &mut server.aml { + server.power = Power::find(aml, server.ec.is_some()); + } server.serve(); } +/// The embedded controller's two ports, which this server's row holds. +#[derive(Clone, Copy)] +struct Ports { + command: u16, + data: u16, +} + +impl Controller for Ports { + /// One transaction, waiting on the controller at each step for at most + /// [`EC_STEP`]. + fn transact(&mut self, mut tx: Transaction) -> u8 { + let mut waiting: Option<(Wait, Instant)> = None; + loop { + let status = in8(self.command); + match tx.step(status) { + Do::Wait(wait) => { + let since = match waiting { + Some((was, since)) if was == wait => since, + _ => Instant::now(), + }; + assert!( + since.elapsed() < EC_STEP, + "acpiserver: the embedded controller kept {wait:?} unmet for {EC_STEP:?} (EC_SC {status:#04x})" + ); + waiting = Some((wait, since)); + std::thread::yield_now(); + } + Do::WriteCommand(byte) => { + waiting = None; + out8(self.command, byte); + } + Do::WriteData(byte) => { + waiting = None; + out8(self.data, byte); + } + Do::ReadData => { + waiting = None; + tx.read(in8(self.data)); + } + Do::Done(byte) => return byte, + } + } + } +} + /// The claim as the tables' fetch and their AML ask it for what lies outside /// its own ports. struct Claim<'a>(&'a AcpiDev); @@ -151,7 +218,7 @@ fn bytes(block: Block) -> impl Iterator { (0..block.len / 2).map(move |i| (block.port + i, block.enable() + i)) } -impl Server { +impl Server<'_> { fn arm(&mut self) { let pm1 = self.info.pm1_event; out16(pm1.enable(), 0); @@ -185,15 +252,26 @@ impl Server { fn serve(&mut self) -> ! { let poller = Poller::new(1); let mut next_count = Instant::now() + COUNTS; + let mut next_read = Instant::now() + battery::POLL; // A watch answers once: one still registered from a wait that timed // out is not registered again. let mut watching = false; loop { + if let (Some(power), Some(aml)) = (&mut self.power, &mut self.aml) + && Instant::now() >= next_read + { + power.read(aml); + next_read = Instant::now() + battery::POLL; + if aml.host.stopping { + self.power = None; + } + } if !watching { - poller.watch(&self.dev, READABLE, 0); + poller.watch(self.dev, READABLE, 0); watching = true; } - let wait = next_count.saturating_duration_since(Instant::now()); + let until = if self.power.is_some() { next_count.min(next_read) } else { next_count }; + let wait = until.saturating_duration_since(Instant::now()); poller.wait(1, wait.as_nanos() as u64, |_| watching = false); if Instant::now() >= next_count { self.log_counts(); @@ -263,9 +341,10 @@ impl Server { /// Take every query the controller has waiting off it, queued for after. fn drain(&mut self) { + let mut ec = self.ec.expect("a drain runs only where the row names a controller"); let mut taken = 0; - while in8(self.info.ec_command.port) & ec::SCI_EVT != 0 { - let q = self.transact(Transaction::query()); + while in8(ec.command) & ec::SCI_EVT != 0 { + let q = ec.transact(Transaction::query()); if q == 0 { break; } @@ -273,7 +352,7 @@ impl Server { assert!( taken <= QUERIES, "acpiserver: the embedded controller had more than {QUERIES} queries waiting at once (EC_SC {:#04x})", - in8(self.info.ec_command.port) + in8(ec.command) ); self.queued.push_back(q); } @@ -291,39 +370,6 @@ impl Server { } } - /// One transaction, waiting on the controller at each step for at most - /// [`EC_STEP`]. - fn transact(&self, mut tx: Transaction) -> u8 { - let (command, data) = (self.info.ec_command.port, self.info.ec_data.port); - let mut waiting: Option<(Wait, Instant)> = None; - loop { - let status = in8(command); - match tx.step(status) { - Do::Wait(wait) => { - let since = match waiting { - Some((was, since)) if was == wait => since, - _ => Instant::now(), - }; - assert!( - since.elapsed() < EC_STEP, - "acpiserver: the embedded controller kept {wait:?} unmet for {EC_STEP:?} (EC_SC {status:#04x})" - ); - waiting = Some((wait, since)); - std::thread::yield_now(); - } - Do::WriteCommand(byte) => { - waiting = None; - out8(command, byte); - } - Do::ReadData => { - waiting = None; - tx.read(in8(data)); - } - Do::Done(byte) => return byte, - } - } - } - fn log_counts(&mut self) { if self.counted == self.logged { return; From 6465a1d30c781f33798db152d563bbaaef6c1fa7 Mon Sep 17 00:00:00 2001 From: japabu Date: Sat, 10 Oct 2026 14:00:11 +0200 Subject: [PATCH 02/12] A battery's read returns the lines it says, so a line said every poll reds a test Mutation m10 (every reading said on every poll) stayed green: the tests read only what the last line said. read now hands its lines to its caller, which says them, and the test holds each poll's lines. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017cSFvbD35xJ2kGANVdm23C --- userland/acpiserver/src/battery.rs | 46 ++++++++++++++++++------------ userland/acpiserver/src/main.rs | 4 ++- 2 files changed, 31 insertions(+), 19 deletions(-) diff --git a/userland/acpiserver/src/battery.rs b/userland/acpiserver/src/battery.rs index bf24bdc7234..14879371f3c 100644 --- a/userland/acpiserver/src/battery.rs +++ b/userland/acpiserver/src/battery.rs @@ -425,7 +425,9 @@ impl Power { println!("{OWN}battery {} of {present} is {}", n + 1, battery.path); } if present > 0 { - power.read(aml); + for line in power.read(aml) { + println!("{line}"); + } } let found = format!( "{present} of {count} control-method batteries present, {} AC adapter(s), {} embedded controller device(s) told their space is there; the Global Lock taken {} times on the way", @@ -441,8 +443,10 @@ impl Power { Some(power) } - /// Read every battery and adapter, saying a line of each that moved. - pub fn read(&mut self, aml: &mut Aml<'_, K, C>) { + /// Read every battery and adapter: the line of each battery whose + /// reading moved, for its caller to say. + pub fn read(&mut self, aml: &mut Aml<'_, K, C>) -> Vec { + let mut lines = Vec::new(); let mut ac = None; for at in 0..self.adapters.len() { let device = self.adapters[at].clone(); @@ -450,7 +454,7 @@ impl Power { Ok(Some(Value::Integer(online))) => ac = Some(ac.unwrap_or(false) || online != 0), Ok(Some(_)) => self.refused(&device, "_PSR", "no Integer"), Ok(None) => self.refused(&device, "_PSR", "the adapter has no _PSR"), - Err(_) if self.stopping => return, + Err(_) if self.stopping => return lines, Err(why) => self.refused(&device, "_PSR", &kind(&why)), } } @@ -468,7 +472,7 @@ impl Power { let status = match self.evaluate(aml, &format!("{device}._BST"), &[]) { Ok(Some(value)) => bst(&value), Ok(None) => Err("the battery has no _BST".into()), - Err(_) if self.stopping => return, + Err(_) if self.stopping => return lines, Err(why) => Err(kind(&why)), }; let status = match status { @@ -482,9 +486,10 @@ impl Power { let now = (percent(&battery.info, &status), status.state); if battery.said != Some(now) || ac_moved { battery.said = Some(now); - println!("acpiserver: {}battery {} of {count}: {}; {ac}", acpiserver_api::BATTERY_READ, n + 1, said_status(&battery.info, &status)); + lines.push(format!("acpiserver: {}battery {} of {count}: {}; {ac}", acpiserver_api::BATTERY_READ, n + 1, said_status(&battery.info, &status))); } } + lines } } @@ -650,9 +655,9 @@ mod tests { } /// A line is said only where what it says moved: the percent, the state - /// or the adapter. + /// or the adapter; find said the first. #[test] - fn a_reading_is_kept_until_its_percent_its_state_or_the_adapter_moves() { + fn a_reading_is_said_only_where_its_percent_its_state_or_the_adapter_moves() { let kernel = laptop_machine(None); let mut aml = loaded(&kernel, Some(Emulated::new(space()))); let mut power = Power::find(&mut aml, true).expect("a battery"); @@ -660,21 +665,26 @@ mod tests { let ec = aml.host.ec.as_mut().expect("a controller"); ec.space[usize::from(at)..usize::from(at) + bytes.len()].copy_from_slice(bytes); }; - power.read(&mut aml); - assert_eq!((power.batteries[0].said, power.ac_said), (Some((Some(87), DISCHARGING)), Some(Some(false)))); + assert_eq!(power.read(&mut aml), Vec::::new(), "nothing moved since find's reading"); poke(&mut aml, RATE, &7000u16.to_le_bytes()); poke(&mut aml, RATE + 2, &43600u16.to_le_bytes()); - power.read(&mut aml); - assert_eq!(power.batteries[0].said, Some((Some(87), DISCHARGING)), "87% discharging at another rate is no new line"); + assert_eq!(power.read(&mut aml), Vec::::new(), "87% discharging at another rate is no new line"); poke(&mut aml, RATE + 2, &43000u16.to_le_bytes()); - power.read(&mut aml); - assert_eq!(power.batteries[0].said, Some((Some(86), DISCHARGING))); + assert_eq!( + power.read(&mut aml), + ["acpiserver: battery read: battery 1 of 1: 86%, 43000 mWh of 50000 mWh, discharging, rate 7000 mW, 12312 mV, moving 7000 mW; AC offline"] + ); poke(&mut aml, AC, &[1]); - power.read(&mut aml); - assert_eq!(power.ac_said, Some(Some(true))); + assert_eq!( + power.read(&mut aml), + ["acpiserver: battery read: battery 1 of 1: 86%, 43000 mWh of 50000 mWh, discharging, rate 7000 mW, 12312 mV, moving 7000 mW; AC online"] + ); poke(&mut aml, STATE, &[CHARGING as u8]); - power.read(&mut aml); - assert_eq!(power.batteries[0].said, Some((Some(86), CHARGING))); + assert_eq!( + power.read(&mut aml), + ["acpiserver: battery read: battery 1 of 1: 86%, 43000 mWh of 50000 mWh, charging, rate 7000 mW, 12312 mV, moving 7000 mW; AC online"] + ); + assert_eq!(power.read(&mut aml), Vec::::new()); } /// No controller in the machine's row: `_REG` is not run, the battery diff --git a/userland/acpiserver/src/main.rs b/userland/acpiserver/src/main.rs index 29a2626c3bf..9248b38d6a6 100644 --- a/userland/acpiserver/src/main.rs +++ b/userland/acpiserver/src/main.rs @@ -260,7 +260,9 @@ impl Server<'_> { if let (Some(power), Some(aml)) = (&mut self.power, &mut self.aml) && Instant::now() >= next_read { - power.read(aml); + for line in power.read(aml) { + println!("{line}"); + } next_read = Instant::now() + battery::POLL; if aml.host.stopping { self.power = None; From e73c65d31f510c14116932f823561e09eaf46632 Mon Sep 17 00:00:00 2001 From: japabu Date: Sat, 10 Oct 2026 14:21:41 +0200 Subject: [PATCH 03/12] A take of the Global Lock the firmware holds waits for its release, and an Acquire of \_GL waits as long as its timeout says MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The T14's battery methods take the Global Lock through Lock-rule memory fields: the orchestrator's T14 reading at 6465a1d30 counted 3 takes on the way to the first reading, and every value matched Linux on the same machine. The battery_read row redded on any take, as designed, because a take that found the firmware holding the lock was denied rather than waited out. ACPI 6.5 §5.2.10.1: a take that finds the lock owned sets the pending bit (the kernel's compare-and-exchange, toyos_acpi::acquire, held to the specification's own sequence by toyos-acpi/tests/facs.rs), and the firmware answers its release with GBL_STS (PM1 status bit 5, Table 4.13), raising the SCI where GBL_EN is set (Table 4.14). The OS's own release of a lock the firmware asked for writes GBL_RLS (PM1 control bit 2), which the kernel does. - acpiserver's host takes, and on Pending waits on the SCI for GBL_STS (sci::await_release), then takes again, until taken or RELEASE (1 s, this server's bound, no measurement) passes, which panics loudly. While it waits GBL_EN is the only enable set, since the SCI is level and anything else latched would raise it again on every acknowledgement; each enable is put back after, so what latched meanwhile is served by the loop after. - toyos-aml's Host splits global_lock into global_take(within) and global_release. An Acquire of \_GL passes its TimeoutValue (§19.6.2), 0xFFFF as no bound, and one that times out returns True holding nothing; a Lock field's take has no bound of the AML's. - The power-sources line says the takes, the gives back, and how many takes found the firmware holding the lock; battery_read reds unless the takes are more than none and as many were given back. - The track's compromise "the server waits for no release" goes: its exit was the wait coming back with a test that reaches its port sequence, which sci.rs's emulated PM1/GPE0 block is. The controller _GLK issue says the lock is there to take, and stays open: nothing takes it around a controller transaction. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017cSFvbD35xJ2kGANVdm23C --- ...dded-controller-without-the-global-lock.md | 6 +- ...ine-in-acpi-mode-and-interprets-its-aml.md | 11 -- tests/toyos.rs | 10 +- userland/acpiserver/aml/src/exec.rs | 38 ++-- userland/acpiserver/aml/src/field.rs | 2 +- userland/acpiserver/aml/src/lib.rs | 14 +- userland/acpiserver/aml/tests/common/mod.rs | 19 +- userland/acpiserver/aml/tests/evaluate.rs | 29 ++- userland/acpiserver/aml/tests/regions.rs | 4 +- userland/acpiserver/src/aml.rs | 16 +- userland/acpiserver/src/battery.rs | 6 +- userland/acpiserver/src/host.rs | 140 +++++++++++---- userland/acpiserver/src/main.rs | 84 ++++++++- userland/acpiserver/src/sci.rs | 166 +++++++++++++++++- 14 files changed, 448 insertions(+), 97 deletions(-) diff --git a/issues/the-acpi-server-talks-to-the-embedded-controller-without-the-global-lock.md b/issues/the-acpi-server-talks-to-the-embedded-controller-without-the-global-lock.md index b4f5dbe57c8..0e5229c332e 100644 --- a/issues/the-acpi-server-talks-to-the-embedded-controller-without-the-global-lock.md +++ b/issues/the-acpi-server-talks-to-the-embedded-controller-without-the-global-lock.md @@ -12,7 +12,11 @@ transaction is taken under the FACS global lock (ACPI 6.5 §6.5.7, §5.2.10.1). `/system/bin/acpiserver` evaluates `_GLK` only to read no battery where it is 1 (`userland/acpiserver/src/battery.rs`), and takes no lock around its own queries: on such a machine a query of the server's and one of the -firmware's can interleave on the same two ports. +firmware's can interleave on the same two ports. The lock itself is there to +take: the interpreter's host takes it for a Lock field or an Acquire of +`\_GL`, waiting out the firmware's hold on its `GBL_STS` +(`userland/acpiserver/src/host.rs`); nothing takes it around a controller +transaction. On the T14 no table names a `_GLK` at all: a byte search of its DSDT and every SSDT for the name finds none (the tables captured before its wipe, read diff --git a/issues/toyos-runs-the-machine-in-acpi-mode-and-interprets-its-aml.md b/issues/toyos-runs-the-machine-in-acpi-mode-and-interprets-its-aml.md index 4477647eb62..8c7c7ed05e7 100644 --- a/issues/toyos-runs-the-machine-in-acpi-mode-and-interprets-its-aml.md +++ b/issues/toyos-runs-the-machine-in-acpi-mode-and-interprets-its-aml.md @@ -268,17 +268,6 @@ mediated access, leaves open: controller not at all; Linux runs it after. Owner: this stage. **Exit**: the slice that runs the `_STA` and `_INI` walk runs it before `_REG`. -- **The server waits for no release of the Global Lock** (the orchestrator's - ruling, not the owner's). A take that finds the firmware holding the lock - leaves it the request, as ACPI 6.5 §5.2.10.1 has it, and is then denied by - name and counted in the server's ledger; the access under it is not made, - and the table or method that asked is refused. The wait that section - describes, for the SCI the firmware raises with `GBL_STS`, is not in the - tree: no tier reached it, and the T14's load took the lock 241 times and - found the firmware holding it in none. Owner: this stage. **Exit**: a - machine's log carries the denial, `the Global Lock: the firmware holds it`, - which the `acpi_tables_loaded` row reds on as on every refusal; the wait - comes back with the test that reaches its port sequence. - **A press during the load waits for it, and a power-off asked during it is refused.** The server arms the power button and then loads the tables before it serves an SCI, so a press in that time diff --git a/tests/toyos.rs b/tests/toyos.rs index 64bc51bbaa0..b082f8a40ce 100644 --- a/tests/toyos.rs +++ b/tests/toyos.rs @@ -5335,8 +5335,9 @@ const T14_POWER_SOURCES: &str = "acpiserver: power sources: 1 of 1 control-metho /// The T14's battery as the server read it: the one battery its tables name /// found present once the controller's `_REG` ran, its information read, /// and at least one reading of it with the adapter's state; every method -/// answered, and none of them took the Global Lock, which the controller's -/// own fields carry no rule for. The lines are printed whole: their numbers +/// answered, and the Global Lock, which the methods take through Lock-rule +/// memory fields, taken and given back as often, the takes that found the +/// firmware holding it included. The lines are printed whole: their numbers /// are what Linux's `/sys/class/power_supply` reads on the same machine are /// compared against, by whoever holds both. fn battery_on_metal(back: &metal::Readback) -> Result<(), String> { @@ -5356,8 +5357,9 @@ fn battery_on_metal(back: &metal::Readback) -> Result<(), String> { } let found = log.must_say(T14_POWER_SOURCES)?; let takes = number_between(found, "; the Global Lock taken ", " times on the way")?; - if takes != 0 { - return Err(format!("the battery's methods took the Global Lock {takes} times: {found}")); + let given_back = number_between(found, " times on the way and given back ", " times, the firmware found")?; + if takes == 0 || given_back != takes { + return Err(format!("the battery's methods took the Global Lock {takes} times and gave it back {given_back}: {found}")); } let info = log.must_say(&format!("acpiserver: {}battery 1 of 1 from ", acpiserver_api::BATTERY_INFO))?; let read: Vec<&&str> = lines.iter().filter(|l| l.contains(acpiserver_api::BATTERY_READ)).collect(); diff --git a/userland/acpiserver/aml/src/exec.rs b/userland/acpiserver/aml/src/exec.rs index 20143bcb625..762dbd5718e 100644 --- a/userland/acpiserver/aml/src/exec.rs +++ b/userland/acpiserver/aml/src/exec.rs @@ -250,7 +250,7 @@ impl<'a> Machine<'a> { let mut released = Ok(()); if self.global > 0 { self.global = 0; - released = self.host.global_lock(false).map_err(|d| Error::Host(d.0)); + released = self.host.global_release().map_err(|d| Error::Host(d.0)); } let r = r?; released?; @@ -260,18 +260,24 @@ impl<'a> Machine<'a> { Ok(r) } - pub(crate) fn take_global(&mut self) -> Result<(), Error> { - if self.global == 0 { - self.host.global_lock(true).map_err(|d| Error::Host(d.0))?; + /// Holds the Global Lock once more, taking it from the host where this + /// evaluation does not hold it yet: `Ok(false)` is a take `within` a + /// bound that timed out, and nothing held. + pub(crate) fn take_global(&mut self, within: Option) -> Result { + if self.global == 0 && !self.host.global_take(within).map_err(|d| Error::Host(d.0))? { + if within.is_none() { + return Err(Error::Host("a take of the Global Lock with no bound came back untaken".into())); + } + return Ok(false); } self.global += 1; - Ok(()) + Ok(true) } pub(crate) fn drop_global(&mut self) -> Result<(), Error> { self.global = self.global.checked_sub(1).expect("the Global Lock is given back only by who took it"); if self.global == 0 { - self.host.global_lock(false).map_err(|d| Error::Host(d.0))?; + self.host.global_release().map_err(|d| Error::Host(d.0))?; } Ok(()) } @@ -874,18 +880,20 @@ impl<'a> Machine<'a> { Ok(Flow::Next) } - fn acquire(&mut self, m: &Kept) -> Result<(), Error> { + /// `false` where `\_GL`'s firmware side held it past `within` + /// milliseconds, and nothing was acquired. + fn acquire(&mut self, m: &Kept, within: Option) -> Result { // §19.6.88: an Acquire's SyncLevel is equal to or above the current one. if m.held.get() == 0 && self.levels.last().is_some_and(|&l| m.sync < l) { return Err(Error::Rule("Acquire of a Mutex below the current SyncLevel (§19.6.88)")); } - if m.global && m.held.get() == 0 { - self.take_global()?; + if m.global && m.held.get() == 0 && !self.take_global(within)? { + return Ok(false); } m.held.set(m.held.get() + 1); self.held.push(m.clone()); self.levels.push(m.sync); - Ok(()) + Ok(true) } fn release(&mut self, m: &Kept) -> Result<(), Error> { @@ -1487,15 +1495,15 @@ impl<'a> Machine<'a> { fn acquire_op(&mut self, f: &mut Frame, c: &mut Cursor<'_>) -> Result { let t = self.super_name(f, c)?; - c.word()?; + // §19.6.2: 0xFFFF waits with no bound. + let within = Some(c.word()?).filter(|&ms| ms != 0xFFFF); let id = self.node_of(f, &t)?; let Object::Mutex(m) = self.node_object(id)? else { return Err(Error::Type("Acquire of an object that is not a Mutex (§19.6.2)")); }; - // One invocation runs at a time, so no Mutex is owned by another: - // every Acquire is granted, and none times out. - self.acquire(&m)?; - Ok(Object::Int(0)) + // One invocation runs at a time, so no Mutex is owned by another + // invocation: only `\_GL`'s firmware side can time an Acquire out. + Ok(Object::Int(if self.acquire(&m, within)? { 0 } else { self.w.ones() })) } fn wait_op(&mut self, f: &mut Frame, c: &mut Cursor<'_>) -> Result { diff --git a/userland/acpiserver/aml/src/field.rs b/userland/acpiserver/aml/src/field.rs index 28353ce1ec2..9aef5d90791 100644 --- a/userland/acpiserver/aml/src/field.rs +++ b/userland/acpiserver/aml/src/field.rs @@ -407,7 +407,7 @@ impl Machine<'_> { /// Runs `body` holding the Global Lock where `lock` says (§19.6.47). fn locked(&mut self, lock: bool, body: impl FnOnce(&mut Self) -> Result) -> Result { if lock { - self.take_global()?; + self.take_global(None)?; } let r = body(self); if lock { diff --git a/userland/acpiserver/aml/src/lib.rs b/userland/acpiserver/aml/src/lib.rs index f75947a350f..9a7a0cfd034 100644 --- a/userland/acpiserver/aml/src/lib.rs +++ b/userland/acpiserver/aml/src/lib.rs @@ -21,7 +21,8 @@ //! SystemIO, PCI_Config and EmbeddedControl space; an access in any other //! space is refused as [`Error::Unsupported`], as are `Load`, `LoadTable` //! and `DataTableRegion`. Only one invocation runs at a time, so a Mutex is -//! never contended and an Event is never signalled by anyone else. +//! never contended and an Event is never signalled by anyone else; `\_GL`'s +//! other owner is the firmware, whose side the [`Host`] waits out. //! //! The predefined objects are the operating system's (§5.7), answered as //! Windows answers them, by the owner's rulings ("Like Windows, not Linux"; @@ -181,9 +182,14 @@ pub trait Host { fn timer(&mut self) -> u64; /// Notify (§19.6.94) of the object at this absolute path. fn notify(&mut self, object: &str, value: u64); - /// Takes (`true`) or gives back (`false`) the firmware's Global Lock - /// (§5.2.10.1), around a Lock field's access and `\_GL`'s ownership. - fn global_lock(&mut self, take: bool) -> Result<(), Denied>; + /// Takes the firmware's Global Lock (§5.2.10.1), for a Lock field's + /// access or `\_GL`'s first Acquire, waiting where the firmware holds it + /// for its release at most `within` milliseconds, an Acquire's + /// TimeoutValue below 0xFFFF (§19.6.2); `None` is no bound of the AML's, + /// and is never answered `Ok(false)`, a take that timed out. + fn global_take(&mut self, within: Option) -> Result; + /// Gives back the Global Lock a take took. + fn global_release(&mut self) -> Result<(), Denied>; } /// An object as the caller receives or passes it. diff --git a/userland/acpiserver/aml/tests/common/mod.rs b/userland/acpiserver/aml/tests/common/mod.rs index fae63efcb5b..460080b1f89 100644 --- a/userland/acpiserver/aml/tests/common/mod.rs +++ b/userland/acpiserver/aml/tests/common/mod.rs @@ -296,7 +296,8 @@ pub enum Event { Sleep(u64), Stall(u64), Notify(String, u64), - GlobalLock(bool), + GlobalTake(Option), + GlobalRelease, } /// A machine of bytes: every address space a sparse map, every request @@ -306,6 +307,8 @@ pub struct Machine { pub bytes: BTreeMap<(u8, u64), u8>, pub log: Vec, pub refuse: bool, + /// The firmware holds the Global Lock past every bound a take names. + pub firmware_holds: bool, } fn key(a: Address) -> (u8, u64) { @@ -389,8 +392,13 @@ impl Host for Machine { self.log.push(Event::Notify(object.into(), value)); } - fn global_lock(&mut self, take: bool) -> Result<(), Denied> { - self.log.push(Event::GlobalLock(take)); + fn global_take(&mut self, within: Option) -> Result { + self.log.push(Event::GlobalTake(within)); + Ok(!self.firmware_holds) + } + + fn global_release(&mut self) -> Result<(), Denied> { + self.log.push(Event::GlobalRelease); Ok(()) } } @@ -465,7 +473,10 @@ impl Host for Sink { 0 } fn notify(&mut self, _: &str, _: u64) {} - fn global_lock(&mut self, _: bool) -> Result<(), Denied> { + fn global_take(&mut self, _: Option) -> Result { + Ok(true) + } + fn global_release(&mut self) -> Result<(), Denied> { Ok(()) } } diff --git a/userland/acpiserver/aml/tests/evaluate.rs b/userland/acpiserver/aml/tests/evaluate.rs index c36740a0e4b..90b5b37ace1 100644 --- a/userland/acpiserver/aml/tests/evaluate.rs +++ b/userland/acpiserver/aml/tests/evaluate.rs @@ -405,7 +405,34 @@ fn mutexes_follow_sync_levels_and_are_released_by_the_end() { assert_eq!(ip.evaluate(&mut m, "\\GOOD", &[]), i(1)); m.log.clear(); ip.evaluate(&mut m, "\\GL", &[]).unwrap(); - assert_eq!(m.log, vec![Event::GlobalLock(true), Event::GlobalLock(false)]); + assert_eq!(m.log, vec![Event::GlobalTake(None), Event::GlobalRelease]); +} + +/// §19.6.2: an Acquire of `\_GL` waits for the firmware's side at most its +/// TimeoutValue in milliseconds, 0xFFFF naming no bound; one that times out +/// returns True and holds nothing, and one this evaluation already holds asks +/// the host for nothing. +#[test] +fn an_acquire_of_the_global_lock_waits_as_its_timeout_says() { + let acquire = |ms: u16| cat(&[&[0x5B, 0x23], &name("\\_GL"), &ms.to_le_bytes()]); + let release = cat(&[&[0x5B, 0x27], &name("\\_GL")]); + let (mut ip, mut m) = loaded(&cat(&[ + &method("SOON", 0, &cat(&[&store(&acquire(5), &local(0)), &release, &ret(&local(0))])), + &method("LATE", 0, &ret(&acquire(5))), + &method("EVER", 0, &cat(&[&store(&acquire(0xFFFF), &local(0)), &store(&acquire(0), &local(1)), &release, &release, &ret(&add(&local(0), &local(1), ZERO))])), + ])); + assert_eq!(ip.evaluate(&mut m, "\\EVER", &[]), i(0)); + assert_eq!(m.log, vec![Event::GlobalTake(None), Event::GlobalRelease], "the second Acquire asked the host again"); + m.log.clear(); + assert_eq!(ip.evaluate(&mut m, "\\SOON", &[]), i(0)); + assert_eq!(m.log, vec![Event::GlobalTake(Some(5)), Event::GlobalRelease]); + m.firmware_holds = true; + m.log.clear(); + assert_eq!(ip.evaluate(&mut m, "\\LATE", &[]), i(ONES), "an Acquire that timed out is True (§19.6.2)"); + assert_eq!(m.log, vec![Event::GlobalTake(Some(5))], "a lock the take did not get was given back"); + m.log.clear(); + assert!(matches!(ip.evaluate(&mut m, "\\EVER", &[]), Err(Error::Host(_))), "an unbounded take came back untaken and the method ran on"); + assert_eq!(m.log, vec![Event::GlobalTake(None)]); } #[test] diff --git a/userland/acpiserver/aml/tests/regions.rs b/userland/acpiserver/aml/tests/regions.rs index cbc0d7b298b..a56e59807df 100644 --- a/userland/acpiserver/aml/tests/regions.rs +++ b/userland/acpiserver/aml/tests/regions.rs @@ -361,10 +361,10 @@ fn a_lock_field_holds_the_global_lock_across_its_access() { assert_eq!( m.log, vec![ - Event::GlobalLock(true), + Event::GlobalTake(None), Event::Read(mem(0x1000), Access::Byte), Event::Read(mem(0x1001), Access::Byte), - Event::GlobalLock(false), + Event::GlobalRelease, ] ); } diff --git a/userland/acpiserver/src/aml.rs b/userland/acpiserver/src/aml.rs index 1d65cd866c8..5c0260a989d 100644 --- a/userland/acpiserver/src/aml.rs +++ b/userland/acpiserver/src/aml.rs @@ -251,7 +251,7 @@ pub mod tests { use super::*; use crate::ec::tests::Emulated; use crate::host::tests::Scripted; - use crate::host::{Take, HELD}; + use crate::host::Take; fn load_only(kernel: &K, rsdp: u64) -> Loaded { load(kernel, None::, rsdp).0 @@ -488,10 +488,10 @@ pub mod tests { } /// A Lock field's access takes the Global Lock through the kernel and - /// gives it back; one the firmware holds is the load's refusal, by name, - /// and the field is not read without it. + /// gives it back; one the firmware holds is waited for, and the load goes + /// on once the take after its release has it. #[test] - fn a_lock_field_read_at_load_takes_the_lock_and_a_held_lock_refuses_the_table() { + fn a_lock_field_read_at_load_takes_the_lock_and_waits_out_the_firmwares_hold() { const NVS: u64 = 0x7700_0000; // OperationRegion (REGN, SystemMemory, 0x77000000, 4); Field (REGN, // ByteAcc, Lock, Preserve) { FLDA, 8 }; Name (COPY, 0); Store (FLDA, COPY). @@ -527,8 +527,10 @@ pub mod tests { let kernel = with_nvs(); kernel.takes.borrow_mut().push_back(Take::Pending); - let loaded = load_only(&kernel, CRAFTED_RSDP); - assert_eq!(loaded, Loaded { blocks: vec![Err(format!("denied by this server: {HELD}"))], s5: None, handed: false }); - assert_ne!(kernel.asked.borrow().last(), Some(&field), "the field was read without the lock"); + kernel.releases.borrow_mut().push_back(true); + assert_eq!(load_only(&kernel, CRAFTED_RSDP), Loaded { blocks: vec![Ok(())], s5: Some((5, 0)), handed: true }); + assert!(!kernel.held.get(), "the load ended holding the Global Lock"); + assert_eq!(kernel.waited.borrow().len(), 1, "the firmware's hold was not waited out"); + assert_eq!(kernel.asked.borrow().last(), Some(&field)); } } diff --git a/userland/acpiserver/src/battery.rs b/userland/acpiserver/src/battery.rs index 14879371f3c..9cbe157cf18 100644 --- a/userland/acpiserver/src/battery.rs +++ b/userland/acpiserver/src/battery.rs @@ -324,7 +324,7 @@ impl Power { /// information; `None` where there is nothing to read. pub fn find(aml: &mut Aml<'_, K, C>, controller: bool) -> Option { let mut power = Power { batteries: Vec::new(), adapters: Vec::new(), ac_said: None, refused: Ledger::default(), stopping: false }; - let takes = aml.host.takes; + let (takes, contended, given_back) = (aml.host.takes, aml.host.contended, aml.host.given_back); let hids: Vec = match aml.interpreter.walk() { Ok(mut walk) => { let mut found = Vec::new(); @@ -430,10 +430,12 @@ impl Power { } } let found = format!( - "{present} of {count} control-method batteries present, {} AC adapter(s), {} embedded controller device(s) told their space is there; the Global Lock taken {} times on the way", + "{present} of {count} control-method batteries present, {} AC adapter(s), {} embedded controller device(s) told their space is there; the Global Lock taken {} times on the way and given back {} times, the firmware found holding it by {} of the takes", power.adapters.len(), if controller { controllers.len() } else { 0 }, aml.host.takes - takes, + aml.host.given_back - given_back, + aml.host.contended - contended, ); if present == 0 { println!("acpiserver: {}{found}", acpiserver_api::NO_BATTERY); diff --git a/userland/acpiserver/src/host.rs b/userland/acpiserver/src/host.rs index 0174bf437f6..36196b5ff32 100644 --- a/userland/acpiserver/src/host.rs +++ b/userland/acpiserver/src/host.rs @@ -13,9 +13,12 @@ //! is the kernel's to make or refuse, and a refusal it names is denied under //! that name. //! -//! **The Global Lock is the kernel's to exchange** (ACPI 6.5 §5.2.10.1). A -//! take that finds the firmware holding it is denied by name ([`HELD`]) and -//! counted: this server waits for no release of the firmware's yet. +//! **The Global Lock's word is the kernel's to exchange** (ACPI 6.5 +//! §5.2.10.1). A take that finds the firmware holding it leaves the firmware +//! the pending bit, waits for the release that bit asks for +//! ([`Kernel::released`]) and takes again, and is counted; one the firmware +//! holds past [`RELEASE`] ends this server loudly, and one an Acquire bounds +//! below that comes back untaken at its bound. //! //! **The power-off's sleep type is the one thing handed to the kernel** //! ([`Kernel::s5`]): the kernel writes the register, and reads no AML. @@ -40,8 +43,10 @@ use crate::ledger::Ledger; /// firmware chose: this machine's own, and quoted in no record. pub const OWN: &str = "acpiserver: (this machine's own, quoted in no record) "; -/// The denial of a take that found the firmware holding the Global Lock. -pub const HELD: &str = "the Global Lock: the firmware holds it, and this server waits for no release yet"; +/// The most the firmware may hold the Global Lock after a take left it the +/// pending bit: this server's bound, and no measurement, over a lock SMM +/// holds for one handler's run. +pub const RELEASE: Duration = Duration::from_secs(1); /// The kernel does nothing more for this claim: the machine is stopping. #[derive(Clone, Copy, PartialEq, Eq, Debug)] @@ -70,6 +75,10 @@ pub trait Kernel { fn access(&self, access: Access) -> Result; fn lock_take(&self) -> Result; fn lock_release(&self) -> Result<(), Stopping>; + /// Waits until `until` for the firmware to give back a Global Lock a + /// take left it the pending bit in ([`crate::sci::await_release`]): + /// `false` is the deadline. + fn released(&self, until: Instant) -> bool; /// Hand the kernel `\_S5`'s `SLP_TYPa` for its power-off, which it takes /// once under a claim: `false` where it is wider than the register's /// field, and the kernel kept nothing. @@ -161,9 +170,11 @@ pub struct Firmware<'k, K, C> { /// Reads made, by [`Space`]. pub reads: [u64; 3], pub pages: Pages, - /// Takes of the Global Lock, and how many found the firmware holding it. + /// Takes of the Global Lock, how many found the firmware holding it, + /// and gives back. pub takes: u64, pub contended: u64, + pub given_back: u64, pub notifies: u64, /// Bytes of the controller's space read and written. pub ec_reads: u64, @@ -184,6 +195,7 @@ impl<'k, K: Kernel, C: Controller> Firmware<'k, K, C> { pages: Pages::default(), takes: 0, contended: 0, + given_back: 0, notifies: 0, ec_reads: 0, ec_writes: 0, @@ -300,23 +312,41 @@ impl Host for Firmware<'_, K, C> { } } - fn global_lock(&mut self, take: bool) -> Result<(), Denied> { - if !take { - return self.kernel.lock_release().map_err(|Stopping| self.stopped()); - } + fn global_take(&mut self, within: Option) -> Result { self.takes += 1; - match self.kernel.lock_take() { - Err(Stopping) => Err(self.stopped()), - Ok(Take::Taken) => Ok(()), - Ok(Take::Unusable) => { - Err(self.deny("the Global Lock: the kernel exchanges no lock word in this machine's FACS".into(), format_args!("a take"))) + let asked = Instant::now(); + let bound = asked + RELEASE; + let until = within.map_or(bound, |ms| bound.min(asked + Duration::from_millis(ms.into()))); + let mut contended = false; + loop { + match self.kernel.lock_take() { + Err(Stopping) => return Err(self.stopped()), + Ok(Take::Taken) => return Ok(true), + Ok(Take::Unusable) => { + let what = "the Global Lock: the kernel exchanges no lock word in this machine's FACS"; + return Err(self.deny(what.into(), format_args!("a take"))); + } + Ok(Take::Pending) => {} } - Ok(Take::Pending) => { + if !contended { + contended = true; self.contended += 1; - Err(self.deny(HELD.into(), format_args!("a take the firmware was left the request for"))) + } + if !self.kernel.released(until) { + assert!( + until < bound, + "acpiserver: the firmware held the Global Lock {RELEASE:?} after a take left it the pending bit, and raised no GBL_STS" + ); + return Ok(false); } } } + + fn global_release(&mut self) -> Result<(), Denied> { + self.kernel.lock_release().map_err(|Stopping| self.stopped())?; + self.given_back += 1; + Ok(()) + } } #[cfg(test)] @@ -342,6 +372,11 @@ pub mod tests { /// What each take answers, in turn; `Taken` once it runs out. pub takes: RefCell>, pub held: Cell, + /// What each wait for the firmware's release answers, in turn; the + /// deadline once it runs out. Each deadline asked, after the moment + /// the scripted kernel was made. + pub releases: RefCell>, + pub waited: RefCell>, /// Accesses and lock exchanges answered before the machine stops. pub stops_after: Cell>, /// The `SLP_TYPa` it was handed and kept. @@ -405,6 +440,12 @@ pub mod tests { Ok(()) } + fn released(&self, until: Instant) -> bool { + assert!(!self.held.get(), "a release waited for under a lock already held"); + self.waited.borrow_mut().push(until); + self.releases.borrow_mut().pop_front().unwrap_or(false) + } + fn s5(&self, slp_typ_a: u64) -> Result { self.stopping()?; assert!(!self.held.get(), "the sleep type was handed over under the Global Lock"); @@ -532,29 +573,59 @@ pub mod tests { fn the_lock_is_taken_and_given_back_through_the_kernel() { let kernel = machine(); let mut host = Firmware::new(&kernel, None::); - assert_eq!(host.global_lock(true), Ok(())); + assert_eq!(host.global_take(None), Ok(true)); assert!(kernel.held.get()); - assert_eq!(host.global_lock(false), Ok(())); + assert_eq!(host.global_release(), Ok(())); assert!(!kernel.held.get()); - assert_eq!((host.takes, host.contended), (1, 0)); + assert_eq!((host.takes, host.contended, host.given_back), (1, 0, 1)); + assert!(kernel.waited.borrow().is_empty(), "a take the kernel answered taken waited"); assert!(host.refused.is_empty()); } + /// §5.2.10.1: a take that finds the firmware holding the lock leaves it + /// the pending bit and waits for the release that asks for; each release + /// is followed by a take again, which may find the firmware holding it + /// again. #[test] - fn a_lock_the_firmware_holds_is_denied_by_name_and_counted() { + fn a_lock_the_firmware_holds_is_waited_for_and_taken_after_its_release() { let kernel = machine(); kernel.takes.borrow_mut().extend([Take::Pending, Take::Pending, Take::Taken]); + kernel.releases.borrow_mut().extend([true, true]); let mut host = Firmware::new(&kernel, None::); - for _ in 0..2 { - assert_eq!(host.global_lock(true), Err(Denied(HELD.into()))); - assert!(!kernel.held.get(), "a lock the firmware holds was reported taken"); - } - assert_eq!(kernel.takes.borrow().len(), 1, "a denied take asked the kernel again"); - assert_eq!(host.refused.counts(), format!("{HELD} x2")); - // The firmware let go: the next take is the kernel's answer again. - assert_eq!(host.global_lock(true), Ok(())); - assert_eq!(host.global_lock(false), Ok(())); - assert_eq!((host.takes, host.contended), (3, 2)); + let asked = Instant::now(); + assert_eq!(host.global_take(None), Ok(true)); + assert!(kernel.held.get()); + assert!(kernel.takes.borrow().is_empty() && kernel.releases.borrow().is_empty()); + let waited = kernel.waited.borrow(); + assert_eq!(waited.len(), 2); + assert!(waited.iter().all(|&until| until == waited[0] && until >= asked + RELEASE), "{waited:?}: not one deadline RELEASE after the take"); + assert_eq!(host.global_release(), Ok(())); + assert_eq!((host.takes, host.contended, host.given_back), (1, 1, 1)); + assert!(host.refused.is_empty()); + } + + /// §19.6.2: an Acquire's TimeoutValue bounds the wait, and one that meets + /// it comes back untaken, holding nothing. + #[test] + fn an_acquires_own_bound_ends_the_wait_untaken() { + let kernel = machine(); + kernel.takes.borrow_mut().push_back(Take::Pending); + let mut host = Firmware::new(&kernel, None::); + let asked = Instant::now(); + assert_eq!(host.global_take(Some(5)), Ok(false)); + assert!(!kernel.held.get()); + let until = kernel.waited.borrow()[0]; + assert!(until >= asked + Duration::from_millis(5) && until < asked + RELEASE, "{until:?} after {asked:?}"); + assert_eq!((host.takes, host.contended, host.given_back), (1, 1, 0)); + } + + #[test] + #[should_panic(expected = "acpiserver: the firmware held the Global Lock 1s after a take left it the pending bit, and raised no GBL_STS")] + fn a_firmware_that_holds_the_lock_past_the_bound_ends_the_server() { + let kernel = machine(); + kernel.takes.borrow_mut().push_back(Take::Pending); + let mut host = Firmware::new(&kernel, None::); + let _ = host.global_take(None); } #[test] @@ -563,10 +634,11 @@ pub mod tests { kernel.takes.borrow_mut().push_back(Take::Unusable); let mut host = Firmware::new(&kernel, None::); assert_eq!( - host.global_lock(true), + host.global_take(None), Err(Denied("the Global Lock: the kernel exchanges no lock word in this machine's FACS".into())) ); assert_eq!(host.contended, 0); + assert!(kernel.waited.borrow().is_empty()); } #[test] @@ -579,8 +651,8 @@ pub mod tests { let stopping = Denied(Refusal::Stopping.to_string()); assert_eq!(host.read(Address::Memory(NVS), toyos_aml::Access::Byte), Err(stopping.clone())); assert!(host.stopping); - assert_eq!(host.global_lock(true), Err(stopping.clone())); - assert_eq!(host.global_lock(false), Err(stopping)); + assert_eq!(host.global_take(None), Err(stopping.clone())); + assert_eq!(host.global_release(), Err(stopping)); assert!(host.refused.is_empty(), "the stop was counted as something this machine's firmware was refused"); } } diff --git a/userland/acpiserver/src/main.rs b/userland/acpiserver/src/main.rs index 9248b38d6a6..d07b3c6c892 100644 --- a/userland/acpiserver/src/main.rs +++ b/userland/acpiserver/src/main.rs @@ -27,7 +27,10 @@ //! [`aml::query`] says, after the drain that took them. An event this server //! never enabled, a controller that does not answer, more queries in one //! drain than [`QUERIES`], and [`sci::EMPTY_SCIS`] SCIs in a row that carried -//! nothing are each a panic naming the registers. +//! nothing are each a panic naming the registers. Inside an evaluation, a +//! take of the Global Lock the firmware holds waits on the SCI for its +//! release alone ([`sci::await_release`]), and what else latched meanwhile is +//! served after. //! //! **After the load the machine's batteries and AC adapters are found** in //! the namespace it kept, and read every [`battery::POLL`] between SCIs @@ -60,7 +63,7 @@ use ec::{Do, Transaction, Wait}; use aml::Aml; use battery::Power; use host::{Answer, Controller, Kernel, Stopping, Take}; -use sci::{Event, Served, Unserved, Unstopped, PM1_STATUS, PWRBTN}; +use sci::{Enables, Event, Fixed, Served, Unserved, Unstopped, PM1_STATUS, PWRBTN}; /// The most a controller is waited for at one step of a transaction: one that /// has not moved in this will not. @@ -101,7 +104,7 @@ fn main() { power_button: info.flags & FIXED_POWER_BUTTON != 0, ec_gpe: info.has_ec().then_some(info.ec_gpe), }; - let claim = Claim(&dev); + let claim = Claim { dev: &dev, info }; let mut server = Server { dev: &dev, info, @@ -173,8 +176,12 @@ impl Controller for Ports { } /// The claim as the tables' fetch and their AML ask it for what lies outside -/// its own ports. -struct Claim<'a>(&'a AcpiDev); +/// its own ports, and as the wait for the firmware's release of the Global +/// Lock reads its event blocks and its SCI. +struct Claim<'a> { + dev: &'a AcpiDev, + info: AcpiInfo, +} impl Claim<'_> { /// The kernel's answer; a stopping machine's is the caller's to carry, @@ -191,28 +198,87 @@ impl Claim<'_> { impl Kernel for Claim<'_> { fn access(&self, access: Access) -> Result { - Self::answered("a mediated access", self.0.access(access)).map(|(made, memory_type)| Answer { made, memory_type }) + Self::answered("a mediated access", self.dev.access(access)).map(|(made, memory_type)| Answer { made, memory_type }) } fn lock_take(&self) -> Result { - match self.0.lock_take() { + match self.dev.lock_take() { Err(SyscallError::NotSupported) => Ok(Take::Unusable), answer => Self::answered("a take of the Global Lock", answer).map(|taken| if taken { Take::Taken } else { Take::Pending }), } } fn lock_release(&self) -> Result<(), Stopping> { - Self::answered("the Global Lock's release", self.0.lock_release()) + Self::answered("the Global Lock's release", self.dev.lock_release()) + } + + fn released(&self, until: Instant) -> bool { + sci::await_release(&mut Waiting { claim: self, poller: Poller::new(1), watching: false }, until) } fn s5(&self, slp_typ_a: u64) -> Result { - match self.0.s5(slp_typ_a) { + match self.dev.s5(slp_typ_a) { Err(SyscallError::InvalidArgument) => Ok(false), answer => Self::answered("the power-off's sleep type", answer).map(|()| true), } } } +/// The claim's event blocks and SCI for one wait for the firmware's release. +struct Waiting<'c, 'a> { + claim: &'c Claim<'a>, + poller: Poller, + /// A watch answers once: one still registered from a wait that timed out + /// is not registered again. + watching: bool, +} + +impl Fixed for Waiting<'_, '_> { + fn pm1_status(&mut self) -> u16 { + in16(self.claim.info.pm1_event.port) + } + + fn pm1_clear(&mut self, bits: u16) { + out16(self.claim.info.pm1_event.port, bits); + } + + fn enables(&mut self) -> Enables { + Enables { + pm1: in16(self.claim.info.pm1_event.enable()), + gpe0: bytes(self.claim.info.gpe0).map(|(_, enable)| in8(enable)).collect(), + } + } + + fn enable(&mut self, enables: &Enables) { + out16(self.claim.info.pm1_event.enable(), enables.pm1); + for ((_, enable), &byte) in bytes(self.claim.info.gpe0).zip(&enables.gpe0) { + out8(enable, byte); + } + } + + fn sci(&mut self, until: Instant) -> bool { + let left = until.saturating_duration_since(Instant::now()); + if left.is_zero() { + return false; + } + if !self.watching { + self.poller.watch(self.claim.dev, READABLE, 0); + self.watching = true; + } + let mut answered = false; + self.poller.wait(1, left.as_nanos() as u64, |_| answered = true); + self.watching &= !answered; + match self.claim.dev.irq() { + Ok(_) | Err(SyscallError::WouldBlock) => true, + Err(other) => panic!("acpiserver: the claim's record answered {other:?} while the Global Lock was waited for"), + } + } + + fn ack(&mut self) { + self.claim.dev.ack().expect("acpiserver: the claim's acknowledgement"); + } +} + /// Each byte of a status-and-enable block: its status port and its enable port. fn bytes(block: Block) -> impl Iterator { (0..block.len / 2).map(move |i| (block.port + i, block.enable() + i)) diff --git a/userland/acpiserver/src/sci.rs b/userland/acpiserver/src/sci.rs index c1d0a30cf14..0b2eda845ff 100644 --- a/userland/acpiserver/src/sci.rs +++ b/userland/acpiserver/src/sci.rs @@ -1,7 +1,10 @@ //! What one SCI carried: every event whose status and enable bits are both //! set, read off the PM1 event block and the GPE0 block, and refused by name -//! where it is one this server never enabled; and what a press is whose -//! power-off came back refused ([`unstopped`]). +//! where it is one this server never enabled; what a press is whose +//! power-off came back refused ([`unstopped`]); and the wait for the +//! firmware's release of the Global Lock ([`await_release`]). + +use std::time::Instant; use toyos::power::Refused; use toyos_abi::syscall::SyscallError; @@ -9,6 +12,11 @@ use toyos_abi::syscall::SyscallError; /// PM1 status and enable (ACPI 6.5 Tables 4.13, 4.14): the power button. pub const PWRBTN: u16 = 1 << 8; +/// PM1 status and enable (Tables 4.13, 4.14): `GBL_STS`, which the firmware +/// sets when it gives back a Global Lock it found its pending bit set in +/// (§5.2.10.1), and `GBL_EN`, which lets that raise the SCI. +pub const GBL: u16 = 1 << 5; + /// PM1 status bits a write of one clears (Table 4.13): timer, bus master, /// global lock release, power button, sleep button, RTC, PCIe wake and wake. pub const PM1_STATUS: u16 = 1 << 0 | 1 << 4 | 1 << 5 | 1 << 8 | 1 << 9 | 1 << 10 | 1 << 14 | 1 << 15; @@ -60,6 +68,56 @@ pub fn events(served: &Served, pm1: (u16, u16), gpe0: &[(u8, u8)]) -> Result, +} + +/// The fixed hardware and the SCI, as the wait for the firmware's release +/// drives them. +pub trait Fixed { + fn pm1_status(&mut self) -> u16; + /// Write `bits` to PM1 status, which clears each one set (Table 4.13). + fn pm1_clear(&mut self, bits: u16); + fn enables(&mut self) -> Enables; + fn enable(&mut self, enables: &Enables); + /// Waits for the SCI until `until` and takes its record: `false` is the + /// deadline, and a wake with no record is `true`. + fn sci(&mut self, until: Instant) -> bool; + /// Unmasks the SCI. + fn ack(&mut self); +} + +/// Waits until `until` for `GBL_STS`, after a take of the Global Lock left +/// the firmware the pending bit (§5.2.10.1): `true` where it was set, and is +/// now clear. While it waits `GBL_EN` is the one enable set, so no other +/// event raises the SCI it waits on, whose line is level; each enable is put +/// back as it was, so an event latched meanwhile raises the SCI again for +/// whoever serves it. A `GBL_STS` left from an earlier release ends the wait +/// too, which costs the caller one take more and loses no release. +pub fn await_release(fixed: &mut impl Fixed, until: Instant) -> bool { + let kept = fixed.enables(); + fixed.enable(&Enables { pm1: GBL, gpe0: vec![0; kept.gpe0.len()] }); + let released = loop { + let set = fixed.pm1_status() & GBL != 0; + if set { + fixed.pm1_clear(GBL); + } + // Acknowledged with nothing but `GBL_STS` enabled, and that clear. + fixed.ack(); + if set { + break true; + } + if !fixed.sci(until) { + break false; + } + }; + fixed.enable(&kept); + released +} + /// SCIs in a row that carried no event this server serves, past which the /// line is a storm and not a machine with something to say. pub const EMPTY_SCIS: u32 = 64; @@ -128,6 +186,110 @@ mod tests { ); } + /// What the firmware or a device sets, at one wait for the SCI. + #[derive(Clone, Copy)] + enum Set { + Pm1(u16), + /// A GPE0 status byte, and its bits. + Gpe(usize, u8), + } + + /// A PM1 event block and a GPE0 block, whose status bits a write of one + /// clears and whose SCI is level: asserted while any status bit and its + /// enable are both set (§4.8.3.1), so an acknowledgement then is one the + /// line answers at once. A wait for the SCI lets the script set one thing + /// after another until the line is raised; past the script the wait + /// meets its deadline. + struct Block { + status: u16, + gpe0: Vec, + enables: Enables, + script: Vec, + sets: usize, + acks: usize, + } + + impl Block { + fn new(script: &[Set]) -> Self { + Block { + status: 0, + gpe0: vec![0; 3], + enables: Enables { pm1: PWRBTN, gpe0: vec![0x40, 0, 0x01] }, + script: script.to_vec(), + sets: 0, + acks: 0, + } + } + } + + impl Fixed for Block { + fn pm1_status(&mut self) -> u16 { + self.status + } + fn pm1_clear(&mut self, bits: u16) { + self.status &= !bits; + } + fn enables(&mut self) -> Enables { + self.enables.clone() + } + fn enable(&mut self, enables: &Enables) { + self.enables = enables.clone(); + } + fn sci(&mut self, _: Instant) -> bool { + while let Some(&set) = self.script.get(self.sets) { + match set { + Set::Pm1(bits) => self.status |= bits, + Set::Gpe(byte, bits) => self.gpe0[byte] |= bits, + } + self.sets += 1; + if self.raised() { + return true; + } + } + false + } + fn ack(&mut self) { + self.acks += 1; + assert!(!self.raised(), "acknowledged with PM1 status {:#06x} or GPE0 status {:x?} raising the line", self.status, self.gpe0); + } + } + + impl Block { + fn raised(&self) -> bool { + self.status & self.enables.pm1 != 0 || self.gpe0.iter().zip(&self.enables.gpe0).any(|(status, enable)| status & enable != 0) + } + } + + #[test] + fn the_wait_ends_on_the_firmwares_release_and_leaves_every_other_event_latched_and_enabled() { + // A press and the controller's GPE come while the firmware holds the + // lock; then it lets go. + let mut block = Block::new(&[Set::Pm1(PWRBTN), Set::Gpe(0, 0x40), Set::Pm1(GBL)]); + let kept = block.enables.clone(); + assert!(await_release(&mut block, Instant::now())); + assert_eq!(block.status, PWRBTN, "GBL_STS left set, or an event the wait does not serve cleared"); + assert_eq!(block.gpe0, [0x40, 0, 0], "the controller's GPE cleared by the wait"); + assert_eq!(block.enables, kept, "an enable was not put back"); + assert_eq!((block.sets, block.acks), (3, 2), "the wait woke for an event it had disabled"); + } + + #[test] + fn a_release_already_latched_ends_the_wait_without_one() { + let mut block = Block::new(&[]); + block.status = GBL; + assert!(await_release(&mut block, Instant::now())); + assert_eq!((block.status, block.sets, block.acks), (0, 0, 1)); + } + + #[test] + fn a_firmware_that_never_lets_go_meets_the_deadline_with_its_enables_back() { + let mut block = Block::new(&[Set::Pm1(PWRBTN)]); + let kept = block.enables.clone(); + assert!(!await_release(&mut block, Instant::now())); + assert_eq!(block.enables, kept); + assert_eq!(block.status, PWRBTN); + } + #[test] fn only_a_press_on_a_machine_handed_no_sleep_type_is_dropped() { let no_sleep_type = Refused::Kernel(SyscallError::NotSupported); From 0ce2eec3d1682beb15d63260783ff252d561a42a Mon Sep 17 00:00:00 2001 From: japabu Date: Sat, 10 Oct 2026 15:20:29 +0200 Subject: [PATCH 04/12] An Acquire of \_GL waits its own TimeoutValue however long, and a press latched before the wait is served after it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An Acquire whose TimeoutValue was 1000 ms or more had its wait clamped to RELEASE and then asserted the clamp away, ending the server where §19.6.2 says the Acquire returns True. RELEASE now bounds only the take the AML set no bound on, and the two takes are two methods of the interpreter's Host: global_take returns Result<(), Denied>, so an unbounded take cannot come back untaken, and global_take_within(ms) returns the bool. A timed-out Acquire is charged as a wait against the 10 s one evaluation may spend asleep, so a loop of them is bounded too. The claim owns one Poller and its watch state, shared by serve and the wait for the firmware's release, instead of a Poller made and dropped per wait. acpi_press_across_a_lock_wait boots tests/acpicase on the test kernel with an SSDT QEMU adds (-acpitable, a new BootOptions field): it sleeps, then Acquires \_GL within 2 s and Notifies where that timed out. acpi_mediated's new held arm stages the firmware's side of the lock owned and hands the claim to acpiserver. The harness presses once the server armed, reads PM1 and the lock word over QEMU's monitor (the press latched, the take not yet made), and then needs the power-off after the Acquire's timeout. A press inside the wait is one QEMU drops: it sets PWRBTN_STS only under PWRBTN_EN. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017cSFvbD35xJ2kGANVdm23C --- tests/common/power.rs | 138 ++++++++++++++++++ tests/common/qemu.rs | 52 ++++++- .../toyos-rust-tests/src/bin/acpi_mediated.rs | 29 +++- tests/toyos.rs | 10 +- userland/acpiserver/aml/src/exec.rs | 14 +- userland/acpiserver/aml/src/lib.rs | 15 +- userland/acpiserver/aml/tests/common/mod.rs | 14 +- userland/acpiserver/aml/tests/evaluate.rs | 12 +- userland/acpiserver/src/host.rs | 110 +++++++------- userland/acpiserver/src/main.rs | 75 +++++----- 10 files changed, 354 insertions(+), 115 deletions(-) diff --git a/tests/common/power.rs b/tests/common/power.rs index 7ffbbea66a0..6eaa2ad3172 100644 --- a/tests/common/power.rs +++ b/tests/common/power.rs @@ -760,3 +760,141 @@ pub fn acpi_power_button(test_config: &Path) -> Result<(), String> { eprintln!(" [power] the press: {ACPI_PRESSED}"); Ok(()) } + +/// How long [`ssdt_acquiring_the_global_lock`] sleeps as it loads, in ms, +/// before its Acquire: the window the press is sent in, wide beside a host's +/// answer to a line. +const PRESS_WINDOW_MS: u16 = 5000; + +/// The Acquire's TimeoutValue, in ms; the two together are within the +/// 10 s one evaluation may spend asleep. +const ACQUIRE_MS: u16 = 2000; + +/// What the server says of the table's Notify, which runs only where the +/// Acquire before it timed out. +const ACQUIRE_TIMED_OUT: &str = "Notify(\\_SB_.GLW_, 0x80) for the first time"; + +/// What it says of the load's takes: one, and it found the firmware holding +/// the lock. +const ONE_TAKE_CONTENDED: &str = "; took the Global Lock 1 times, 1 of them from the firmware;"; + +/// PM1 status and enable (ACPI 6.5 Tables 4.13, 4.14): `PWRBTN_STS` and +/// `PWRBTN_EN`. +const PWRBTN: u16 = 1 << 8; + +/// The FACS's lock word's owned bit (Table 5.11), with the pending bit a +/// take that finds it owned sets clear. +const OWNED: u32 = 2; + +/// An SSDT (ACPI 6.5 §5.2.11.2) whose definition block declares the device +/// `\_SB.GLW` and, as it loads, sleeps `window` ms, then Acquires `\_GL` +/// within `ms`, giving it back where that took it and Notifying `\_SB.GLW` +/// with 0x80 where it timed out: +/// +/// ```text +/// Device (\_SB.GLW) {} +/// Sleep (window) +/// If (Acquire (\_GL, ms)) { Notify (\_SB.GLW, 0x80) } Else { Release (\_GL) } +/// ``` +fn ssdt_acquiring_the_global_lock(window: u16, ms: u16) -> Vec { + const GL: &[u8] = b"\\_GL_"; + // `\` and a DualNamePrefix (§20.2.2). + const GLW: &[u8] = b"\\\x2E_SB_GLW_"; + // A PkgLength of one byte (§20.2.4): itself and what follows, under 64. + let package = |op: &[u8], body: &[u8]| [op, &[u8::try_from(body.len() + 1).expect("a short package")], body].concat(); + let sleep = [&[0x5B, 0x22, 0x0B], &window.to_le_bytes()[..]].concat(); + let acquire = [&[0x5B, 0x23], GL, &ms.to_le_bytes()].concat(); + let notify = [&[0x86], GLW, &[0x0A, 0x80]].concat(); + let release = [&[0x5B, 0x27], GL].concat(); + let aml = [ + package(&[0x5B, 0x82], GLW), + sleep, + package(&[0xA0], &[acquire, notify].concat()), + package(&[0xA1], &release), + ] + .concat(); + let length = u32::try_from(36 + aml.len()).expect("a short table"); + let mut table = [b"SSDT".as_slice(), &length.to_le_bytes(), &[2, 0], b"TOYOS ", b"GLWAIT ", &1u32.to_le_bytes(), b"TOYS", &1u32.to_le_bytes(), &aml].concat(); + // The byte at 9 makes the whole table sum to zero. + table[9] = table.iter().fold(0u8, |sum, &b| sum.wrapping_sub(b)); + table +} + +/// The number after `after` on a line, in hex. +fn hex_after(line: &str, after: &str) -> Result { + line.split_once(after) + .map(|(_, rest)| rest.trim_start_matches("0x")) + .and_then(|rest| u64::from_str_radix(&rest[..rest.find(|c: char| !c.is_ascii_hexdigit()).unwrap_or(rest.len())], 16).ok()) + .ok_or_else(|| format!("no number after {after:?} on {line:?}")) +} + +/// A press latched before the server's wait for the firmware's release of +/// the Global Lock is served once the wait ends at its Acquire's timeout: the +/// wait clears every enable but `GBL_EN`, takes the SCI the press raised, +/// and puts every enable back after, so the press's status raises the SCI +/// again for `serve`. `tests/acpicase` with the test kernel's actuator for +/// the firmware's side of the lock, which `acpi_mediated`'s `held` arm stages +/// owned before it hands the server the claim; the table QEMU adds sleeps +/// [`PRESS_WINDOW_MS`] as it loads, then Acquires the lock within +/// [`ACQUIRE_MS`]. +/// +/// The press is sent once the server has armed, and QEMU's monitor reads +/// right after it `PWRBTN_STS` latched under `PWRBTN_EN` and the lock word +/// owned with no pending bit: the server had not yet taken, so its wait came +/// after the press. A press inside the wait is QEMU's to drop, which sets +/// `PWRBTN_STS` only under `PWRBTN_EN`. Then the press must power the machine +/// off, after the Acquire said it timed out. +pub fn acpi_press_across_a_lock_wait(probe: (String, Vec)) -> Result<(), String> { + let case = super::compile::repo_root().join("tests/acpicase"); + let mut qemu = QemuInstance::boot_with_options( + &case, + &[], + &[], + BootOptions { + // The test kernel, for the Global Lock's actuator, as `acpi_mediated_access` boots it. + kernel_params: &["i8042-withheld"], + ready_marker: "acpi: the Global Lock is the FACS's at ", + extra_root_files: vec![ + probe, + // The arm's name is the file's: the probe asks whether it is there. + ("share/acpi_mediated_held".to_string(), b"held\n".to_vec()), + ], + acpi_tables: vec![ssdt_acquiring_the_global_lock(PRESS_WINDOW_MS, ACQUIRE_MS)], + qmp: true, + ..Default::default() + }, + ); + let boot = serial::Serial::named("the boot", qemu.boot_log().to_string()); + let pm1 = hex_after(boot.must_say("acpi: the ACPI row: PM1a events ")?, "PM1a events ")?; + let pm1 = u16::try_from(pm1).map_err(|_| format!("a PM1a event block at {pm1:#x}"))?; + let lock_word = hex_after(boot.must_say("acpi: the Global Lock is the FACS's at ")?, "the FACS's at ")?; + // Opened before the press: QMP delivers no event emitted before its + // client connected. + let mut stop = qemu::QmpShutdown::open(qemu.qmp_socket(), qemu.budget(qemu::GUEST_QUIET)); + let mut console = format!("{}\n", qemu.boot_log()); + qemu::await_marker(&mut qemu, &mut console, ACPI_ARMED, "the ACPI server arming")?; + stop.power_button(); + // Table 4.12: PM1 enable follows PM1 status in the block. + let (status, enable, word) = (stop.port_word(pm1), stop.port_word(pm1 + 2), stop.memory_word(lock_word)); + if status & enable & PWRBTN == 0 || word != OWNED { + return Err(format!( + "right after the press PM1 read status {status:#06x} under enable {enable:#06x} and the lock word {word:#x}, where a \ + press latched before the server's take reads PWRBTN_STS under PWRBTN_EN ({PWRBTN:#06x}) and the word owned with no \ + pending bit ({OWNED:#x})" + )); + } + let pressed_at = console.len(); + ended(&mut qemu, &mut stop, &mut console, SHUTTING_DOWN, "guest-shutdown")?; + let after = serial::Serial::named("the press", console[pressed_at..].to_string()); + after.must_say(ONE_TAKE_CONTENDED)?; + after.must_say_after(ACQUIRE_TIMED_OUT, ACPI_PRESSED)?; + after.must_be_clean()?; + let whole = serial::Serial::named("the boot and the press", console); + if acpi_tables_loaded(&whole, &whole, Q35_S5_SUPPLIED)? < 2 { + return Err(format!("the server loaded no table beside the DSDT, so not the one QEMU was handed:\n{}", whole.text())); + } + eprintln!(" [power] PM1 status {status:#06x} under enable {enable:#06x} and the lock word {word:#x} after the press"); + eprintln!(" [power] {}", whole.must_say(ONE_TAKE_CONTENDED)?.trim()); + eprintln!(" [power] {}", whole.must_say_after(ACQUIRE_TIMED_OUT, ACPI_PRESSED)?.trim()); + Ok(()) +} diff --git a/tests/common/qemu.rs b/tests/common/qemu.rs index 448a000089d..a19c28e22f2 100644 --- a/tests/common/qemu.rs +++ b/tests/common/qemu.rs @@ -1049,6 +1049,10 @@ pub struct BootOptions { /// update's ROOT of this many bytes: a machine that updates itself. /// `None` for every guest whose subject is not the update. pub second_slot: Option, + /// Whole ACPI tables, header and checksum included, that QEMU lists + /// beside its own (`-acpitable`): firmware AML a guest's own tables do + /// not carry. q35's alone. + pub acpi_tables: Vec>, } impl BootOptions { @@ -1087,6 +1091,7 @@ impl Default for BootOptions { extra_root_files: Vec::new(), psci_trace: None, second_slot: None, + acpi_tables: Vec::new(), } } } @@ -1369,6 +1374,9 @@ impl QemuInstance { }; let sockets = Sockets::new(&options); + for (i, table) in options.acpi_tables.iter().enumerate() { + fs::write(acpi_table(&sockets.dir, i), table).expect("[qemu] write an added ACPI table"); + } let screendump = test_dir.join(format!("screen-{seq}.ppm")); // Per-instance, not a fixed /tmp path: a screen test waits on this @@ -1928,6 +1936,14 @@ impl Qmp { self.await_reply("\"return\""); } + /// Run `command` in the human monitor and return what it printed. + fn human(&mut self, command: &str) -> String { + self.execute_capturing(&format!( + "{{\"execute\":\"human-monitor-command\",\"arguments\":\ + {{\"command-line\":\"{command}\"}}}}" + )) + } + /// `execute`, keeping what the command answered with. Only the human /// monitor answers with anything; every other command here returns `{}`. fn execute_capturing(&mut self, command: &str) -> String { @@ -1987,6 +2003,22 @@ impl QmpShutdown { self.0.execute("{\"execute\":\"system_powerdown\"}"); } + /// A 16-bit read of the guest's I/O `port`, by the human monitor on the + /// one connection QEMU serves; asked before the stop, whose event a + /// command's reply would otherwise pass over, as [`Self::memory_word`]. + pub fn port_word(&mut self, port: u16) -> u16 { + let said = self.0.human(&format!("i /h {port:#x}")); + let value = monitor_value(&said, "= ").and_then(|value| u16::try_from(value).ok()); + value.unwrap_or_else(|| panic!("qmp: the monitor's read of port {port:#x} said {said:?}")) + } + + /// A 32-bit read of the guest's physical memory at `phys`. + pub fn memory_word(&mut self, phys: u64) -> u32 { + let said = self.0.human(&format!("xp /1wx {phys:#x}")); + let value = monitor_value(&said, ": ").and_then(|value| u32::try_from(value).ok()); + value.unwrap_or_else(|| panic!("qmp: the monitor's read of {phys:#x} said {said:?}")) + } + /// The `reason` the `SHUTDOWN` event names — `guest-reset`, /// `guest-shutdown`, `host-signal` — or `None` if the guest never stopped. pub fn reason(&mut self) -> Option { @@ -2005,6 +2037,12 @@ impl QmpShutdown { } } +/// The hex value after the last `after` on what the human monitor said. +fn monitor_value(said: &str, after: &str) -> Option { + let (_, value) = said.rsplit_once(after)?; + u64::from_str_radix(value.trim().trim_start_matches("0x"), 16).ok() +} + /// The `reason` of the `SHUTDOWN` event in what QMP has sent so far. fn shutdown_reason(bytes: &[u8]) -> Option { let text = String::from_utf8_lossy(bytes); @@ -2026,10 +2064,7 @@ impl QmpMonitor { /// Run `command` in the human monitor and return what it printed. pub fn human(&mut self, command: &str) -> String { - self.0.execute_capturing(&format!( - "{{\"execute\":\"human-monitor-command\",\"arguments\":\ - {{\"command-line\":\"{command}\"}}}}" - )) + self.0.human(command) } } @@ -2330,6 +2365,10 @@ fn qemu_command( if options.gdb_stub { qemu.arg("-s"); } + assert!(options.acpi_tables.is_empty() || arch == Arch::X86_64, "an added ACPI table is q35's"); + for i in 0..options.acpi_tables.len() { + qemu.arg("-acpitable").arg(format!("file={}", acpi_table(socket_dir, i).display())); + } if let Some(socket) = qmp_socket { qemu.arg("-qmp") .arg(format!("unix:{},server,nowait", socket.display())); @@ -2361,6 +2400,11 @@ impl Sockets { } } +/// The `i`th of [`BootOptions::acpi_tables`], as a file in a boot's own `dir`. +fn acpi_table(dir: &Path, i: usize) -> PathBuf { + dir.join(format!("acpi-{i}.aml")) +} + /// The QMP socket `options` asks for, named in `dir`. fn qmp_socket(dir: &Path, options: &BootOptions) -> Option { options.qmp.then(|| dir.join("qmp.sock")) diff --git a/tests/toyos-rust-tests/src/bin/acpi_mediated.rs b/tests/toyos-rust-tests/src/bin/acpi_mediated.rs index 0ed9c5bfa9a..68b2fcb7fc8 100644 --- a/tests/toyos-rust-tests/src/bin/acpi_mediated.rs +++ b/tests/toyos-rust-tests/src/bin/acpi_mediated.rs @@ -42,12 +42,18 @@ //! [`OUTLIVED`] has a child supply this guest's own sleep type and exit, and //! the parent, which supplied none, ask for the power-off after the child's //! claim and one more have been released. +//! +//! **A press is served across the server's wait for the firmware's release** +//! ([`held`]), on a boot whose ROOT carries [`HELD`] and whose firmware lists +//! a table that takes the Global Lock at its load: the firmware's side of the +//! lock is staged owned before `/system/bin/acpiserver` is handed the claim. +//! The press, the wait's timeout and the power-off are the harness's to read. use std::os::toyos::process::CommandExt; use std::process::{Command, Stdio}; use std::time::{Duration, Instant}; -use toyos::endow::{Endowments, SYSCAP_LABEL}; +use toyos::endow::{Endowments, DEV_PREFIX, SYSCAP_LABEL}; use toyos::syscap::SysCap; use toyos::{AsHandle, Device}; use toyos_abi::acpi::{pci_address, Access, AcpiInfo, Refused, Space, Width, UNLISTED}; @@ -65,6 +71,9 @@ const CLAIM_LABEL: &str = "acpi-claim"; /// binary's name and takes no argument. const OUTLIVED: &str = "/system/share/acpi_mediated_outlived"; +/// The file whose presence on ROOT makes this boot [`held`]'s. +const HELD: &str = "/system/share/acpi_mediated_held"; + /// `EFI_MEMORY_TYPE`s: what the kernel hands out as RAM, and ACPI's two. const USABLE: [u8; 5] = [1, 2, 3, 4, 7]; const ACPI_RECLAIM: u8 = 9; @@ -112,6 +121,7 @@ impl Holder { fn main() { match std::env::args().nth(1).as_deref() { None if std::fs::exists(OUTLIVED).expect("acpi: ask ROOT for this boot's arm") => outlived(), + None if std::fs::exists(HELD).expect("acpi: ask ROOT for this boot's arm") => held(), None => probe(), Some("keeper") => keeper(), Some("supplier") => supplier(), @@ -629,3 +639,20 @@ fn outlived() { let refused = toyos::power::stop(toyos::power::Stop::Shutdown); panic!("acpi: the power-off was refused: {refused:?}"); } + +/// What [`held`] says once the server has the claim, with the firmware's +/// side of the lock owned. +const STAGED: &str = "acpi: the firmware's side of the Global Lock staged owned, and the server handed the claim"; + +fn held() { + let cap: SysCap = Endowments::get().take(SYSCAP_LABEL).expect("test-runner endows a device-minting capability"); + let claim = claim(&cap); + assert_eq!(syscall::debug_with(debug_action::ACPI_FIRMWARE_LOCK, debug_action::FIRMWARE_OWNS), 0, "acpi: the lock was not free before the server ran"); + let mut server = Command::new("/system/bin/acpiserver") + .endow(&format!("{DEV_PREFIX}{}", DeviceType::Acpi.class_name()), claim.into_raw().0) + .spawn() + .expect("acpi: spawn the server"); + println!("{STAGED}"); + let ended = server.wait().expect("acpi: wait for the server"); + panic!("acpi: the server ended {ended:?}, where a press powers the machine off"); +} diff --git a/tests/toyos.rs b/tests/toyos.rs index d31744eff5d..4f169115f5f 100644 --- a/tests/toyos.rs +++ b/tests/toyos.rs @@ -179,7 +179,8 @@ const RUST_SKIP: &[&str] = &[ // It claims the fixed hardware itself, which needs a boot that starts no // server, and stages the firmware's side of the Global Lock and finds the // i8042's row another claim's, which need the test kernel and its - // `i8042-withheld`: `acpi_mediated_access` runs it on tests/acpicase. + // `i8042-withheld`: `acpi_mediated_access` and `acpi_press_across_a_lock_wait` + // run it on tests/acpicase. "acpi_mediated", // It powers the machine off: `machine_shutdown_short_stop` runs it. "stop_short", @@ -356,6 +357,12 @@ const MACHINE_TESTS: &[&str] = &[ // by stopping. No host test reaches either, and the T14 is never asked to // power off. "acpi_supply_outlives_holder", + // A press while acpiserver waits for the firmware's release of the Global + // Lock: the firmware's hold is the test kernel's actuator on the FACS's + // word, the table that takes the lock is one QEMU adds, and the press is + // QEMU's on demand. The T14's firmware holds the lock when it will, and + // nothing presses its button but a hand. + "acpi_press_across_a_lock_wait", // The power-off after a stop that left a thread running, in ACPI mode: it // ends the machine, so only one QEMU reports stopping can be asked, and // the T14 hands over in legacy mode, where no holder means no quieting. @@ -3702,6 +3709,7 @@ fn run_machine_test(name: &str, test_config: &Path) -> Result<(), String> { "acpi_mediated_access" => acpi_mediated_access(2), "acpi_lock_given_back_on_one_cpu" => acpi_mediated_access(1), "acpi_supply_outlives_holder" => acpi_supply_outlives_holder(), + "acpi_press_across_a_lock_wait" => power::acpi_press_across_a_lock_wait(suite_bin(toyos_build::arch::Arch::X86_64, "acpi_mediated")), "machine_shutdown_short_stop" => power::machine_shutdown_short_stop(test_config), "machine_shutdown_wire_held" => power::machine_shutdown_wire_held(test_config, false), "machine_shutdown_wire_kept" => power::machine_shutdown_wire_held(test_config, true), diff --git a/userland/acpiserver/aml/src/exec.rs b/userland/acpiserver/aml/src/exec.rs index 762dbd5718e..e1e67695a15 100644 --- a/userland/acpiserver/aml/src/exec.rs +++ b/userland/acpiserver/aml/src/exec.rs @@ -262,13 +262,17 @@ impl<'a> Machine<'a> { /// Holds the Global Lock once more, taking it from the host where this /// evaluation does not hold it yet: `Ok(false)` is a take `within` a - /// bound that timed out, and nothing held. + /// bound that timed out, charged as a wait, and nothing held. pub(crate) fn take_global(&mut self, within: Option) -> Result { - if self.global == 0 && !self.host.global_take(within).map_err(|d| Error::Host(d.0))? { - if within.is_none() { - return Err(Error::Host("a take of the Global Lock with no bound came back untaken".into())); + if self.global == 0 { + match within { + None => self.host.global_take().map_err(|d| Error::Host(d.0))?, + Some(ms) if !self.host.global_take_within(ms).map_err(|d| Error::Host(d.0))? => { + self.wait(u64::from(ms) * 1000)?; + return Ok(false); + } + Some(_) => {} } - return Ok(false); } self.global += 1; Ok(true) diff --git a/userland/acpiserver/aml/src/lib.rs b/userland/acpiserver/aml/src/lib.rs index 9a7a0cfd034..69fc21fab7c 100644 --- a/userland/acpiserver/aml/src/lib.rs +++ b/userland/acpiserver/aml/src/lib.rs @@ -81,7 +81,8 @@ pub const MAX_LIVE: usize = 16 << 20; /// The bytes of work one step stands for: a step for every this many bytes /// an operation makes, copies, compares or walks. pub(crate) const WORK_PER_STEP: usize = 64; -/// The time one evaluation may ask to Sleep, Stall and Wait, together, in µs. +/// The time one evaluation may ask to Sleep, Stall and Wait, and wait out +/// in Acquires of `\_GL` that timed out, together, in µs. pub(crate) const MAX_WAIT_US: u64 = 10_000_000; /// What the Revision opcode answers (§19.6.119): this interpreter's revision. pub(crate) const REVISION: u64 = 1; @@ -183,11 +184,13 @@ pub trait Host { /// Notify (§19.6.94) of the object at this absolute path. fn notify(&mut self, object: &str, value: u64); /// Takes the firmware's Global Lock (§5.2.10.1), for a Lock field's - /// access or `\_GL`'s first Acquire, waiting where the firmware holds it - /// for its release at most `within` milliseconds, an Acquire's - /// TimeoutValue below 0xFFFF (§19.6.2); `None` is no bound of the AML's, - /// and is never answered `Ok(false)`, a take that timed out. - fn global_take(&mut self, within: Option) -> Result; + /// access or an Acquire of `\_GL` with no TimeoutValue (0xFFFF, + /// §19.6.2), waiting where the firmware holds it for its release. + fn global_take(&mut self) -> Result<(), Denied>; + /// Takes the Global Lock for an Acquire of `\_GL` whose TimeoutValue is + /// `ms`, waiting where the firmware holds it at most that long: `false` + /// is the timeout, and nothing taken. + fn global_take_within(&mut self, ms: u16) -> Result; /// Gives back the Global Lock a take took. fn global_release(&mut self) -> Result<(), Denied>; } diff --git a/userland/acpiserver/aml/tests/common/mod.rs b/userland/acpiserver/aml/tests/common/mod.rs index 460080b1f89..c9e8e54da0d 100644 --- a/userland/acpiserver/aml/tests/common/mod.rs +++ b/userland/acpiserver/aml/tests/common/mod.rs @@ -392,8 +392,13 @@ impl Host for Machine { self.log.push(Event::Notify(object.into(), value)); } - fn global_take(&mut self, within: Option) -> Result { - self.log.push(Event::GlobalTake(within)); + fn global_take(&mut self) -> Result<(), Denied> { + self.log.push(Event::GlobalTake(None)); + Ok(()) + } + + fn global_take_within(&mut self, ms: u16) -> Result { + self.log.push(Event::GlobalTake(Some(ms))); Ok(!self.firmware_holds) } @@ -473,7 +478,10 @@ impl Host for Sink { 0 } fn notify(&mut self, _: &str, _: u64) {} - fn global_take(&mut self, _: Option) -> Result { + fn global_take(&mut self) -> Result<(), Denied> { + Ok(()) + } + fn global_take_within(&mut self, _: u16) -> Result { Ok(true) } fn global_release(&mut self) -> Result<(), Denied> { diff --git a/userland/acpiserver/aml/tests/evaluate.rs b/userland/acpiserver/aml/tests/evaluate.rs index 90b5b37ace1..99f0a7a286a 100644 --- a/userland/acpiserver/aml/tests/evaluate.rs +++ b/userland/acpiserver/aml/tests/evaluate.rs @@ -410,8 +410,8 @@ fn mutexes_follow_sync_levels_and_are_released_by_the_end() { /// §19.6.2: an Acquire of `\_GL` waits for the firmware's side at most its /// TimeoutValue in milliseconds, 0xFFFF naming no bound; one that times out -/// returns True and holds nothing, and one this evaluation already holds asks -/// the host for nothing. +/// returns True, holds nothing and is charged as a wait, and one this +/// evaluation already holds asks the host for nothing. #[test] fn an_acquire_of_the_global_lock_waits_as_its_timeout_says() { let acquire = |ms: u16| cat(&[&[0x5B, 0x23], &name("\\_GL"), &ms.to_le_bytes()]); @@ -419,6 +419,7 @@ fn an_acquire_of_the_global_lock_waits_as_its_timeout_says() { let (mut ip, mut m) = loaded(&cat(&[ &method("SOON", 0, &cat(&[&store(&acquire(5), &local(0)), &release, &ret(&local(0))])), &method("LATE", 0, &ret(&acquire(5))), + &method("LONG", 0, &cat(&[&store(&acquire(6000), &local(0)), &ret(&acquire(6000))])), &method("EVER", 0, &cat(&[&store(&acquire(0xFFFF), &local(0)), &store(&acquire(0), &local(1)), &release, &release, &ret(&add(&local(0), &local(1), ZERO))])), ])); assert_eq!(ip.evaluate(&mut m, "\\EVER", &[]), i(0)); @@ -426,13 +427,14 @@ fn an_acquire_of_the_global_lock_waits_as_its_timeout_says() { m.log.clear(); assert_eq!(ip.evaluate(&mut m, "\\SOON", &[]), i(0)); assert_eq!(m.log, vec![Event::GlobalTake(Some(5)), Event::GlobalRelease]); + assert_eq!(ip.usage().waited_us, 0, "an Acquire that took the lock was charged a wait"); m.firmware_holds = true; m.log.clear(); assert_eq!(ip.evaluate(&mut m, "\\LATE", &[]), i(ONES), "an Acquire that timed out is True (§19.6.2)"); assert_eq!(m.log, vec![Event::GlobalTake(Some(5))], "a lock the take did not get was given back"); - m.log.clear(); - assert!(matches!(ip.evaluate(&mut m, "\\EVER", &[]), Err(Error::Host(_))), "an unbounded take came back untaken and the method ran on"); - assert_eq!(m.log, vec![Event::GlobalTake(None)]); + assert_eq!(ip.usage().waited_us, 5000); + assert_eq!(ip.evaluate(&mut m, "\\LONG", &[]), Err(Error::Bound("more time asleep than one evaluation may spend"))); + assert_eq!(ip.usage().waited_us, 12_000_000); } #[test] diff --git a/userland/acpiserver/src/host.rs b/userland/acpiserver/src/host.rs index 36196b5ff32..af5ab36dc63 100644 --- a/userland/acpiserver/src/host.rs +++ b/userland/acpiserver/src/host.rs @@ -16,9 +16,10 @@ //! **The Global Lock's word is the kernel's to exchange** (ACPI 6.5 //! §5.2.10.1). A take that finds the firmware holding it leaves the firmware //! the pending bit, waits for the release that bit asks for -//! ([`Kernel::released`]) and takes again, and is counted; one the firmware -//! holds past [`RELEASE`] ends this server loudly, and one an Acquire bounds -//! below that comes back untaken at its bound. +//! ([`Kernel::released`]) and takes again, and is counted. A take the AML +//! bounds, an Acquire's, comes back untaken at its bound; one it does not +//! ends this server loudly where the firmware holds the lock past +//! [`RELEASE`]. //! //! **The power-off's sleep type is the one thing handed to the kernel** //! ([`Kernel::s5`]): the kernel writes the register, and reads no AML. @@ -43,9 +44,9 @@ use crate::ledger::Ledger; /// firmware chose: this machine's own, and quoted in no record. pub const OWN: &str = "acpiserver: (this machine's own, quoted in no record) "; -/// The most the firmware may hold the Global Lock after a take left it the -/// pending bit: this server's bound, and no measurement, over a lock SMM -/// holds for one handler's run. +/// The most the firmware may hold the Global Lock after a take the AML set no +/// bound on left it the pending bit: this server's bound, and no measurement, +/// over a lock SMM holds for one handler's run. pub const RELEASE: Duration = Duration::from_secs(1); /// The kernel does nothing more for this claim: the machine is stopping. @@ -228,6 +229,32 @@ impl<'k, K: Kernel, C: Controller> Firmware<'k, K, C> { } Ok(self.ec.as_mut().expect("just seen")) } + + /// A take of the Global Lock, waiting for the firmware's release until + /// `until` wherever it finds the firmware holding it: `false` is the + /// deadline, and nothing taken. + fn take(&mut self, until: Instant) -> Result { + self.takes += 1; + let mut contended = false; + loop { + match self.kernel.lock_take() { + Err(Stopping) => return Err(self.stopped()), + Ok(Take::Taken) => return Ok(true), + Ok(Take::Unusable) => { + let what = "the Global Lock: the kernel exchanges no lock word in this machine's FACS"; + return Err(self.deny(what.into(), format_args!("a take"))); + } + Ok(Take::Pending) => {} + } + if !contended { + contended = true; + self.contended += 1; + } + if !self.kernel.released(until) { + return Ok(false); + } + } + } } fn wide(width: toyos_aml::Access) -> Width { @@ -312,34 +339,15 @@ impl Host for Firmware<'_, K, C> { } } - fn global_take(&mut self, within: Option) -> Result { - self.takes += 1; - let asked = Instant::now(); - let bound = asked + RELEASE; - let until = within.map_or(bound, |ms| bound.min(asked + Duration::from_millis(ms.into()))); - let mut contended = false; - loop { - match self.kernel.lock_take() { - Err(Stopping) => return Err(self.stopped()), - Ok(Take::Taken) => return Ok(true), - Ok(Take::Unusable) => { - let what = "the Global Lock: the kernel exchanges no lock word in this machine's FACS"; - return Err(self.deny(what.into(), format_args!("a take"))); - } - Ok(Take::Pending) => {} - } - if !contended { - contended = true; - self.contended += 1; - } - if !self.kernel.released(until) { - assert!( - until < bound, - "acpiserver: the firmware held the Global Lock {RELEASE:?} after a take left it the pending bit, and raised no GBL_STS" - ); - return Ok(false); - } + fn global_take(&mut self) -> Result<(), Denied> { + if self.take(Instant::now() + RELEASE)? { + return Ok(()); } + panic!("acpiserver: the firmware held the Global Lock {RELEASE:?} after a take left it the pending bit, and raised no GBL_STS"); + } + + fn global_take_within(&mut self, ms: u16) -> Result { + self.take(Instant::now() + Duration::from_millis(ms.into())) } fn global_release(&mut self) -> Result<(), Denied> { @@ -573,7 +581,7 @@ pub mod tests { fn the_lock_is_taken_and_given_back_through_the_kernel() { let kernel = machine(); let mut host = Firmware::new(&kernel, None::); - assert_eq!(host.global_take(None), Ok(true)); + assert_eq!(host.global_take(), Ok(())); assert!(kernel.held.get()); assert_eq!(host.global_release(), Ok(())); assert!(!kernel.held.get()); @@ -593,7 +601,7 @@ pub mod tests { kernel.releases.borrow_mut().extend([true, true]); let mut host = Firmware::new(&kernel, None::); let asked = Instant::now(); - assert_eq!(host.global_take(None), Ok(true)); + assert_eq!(host.global_take(), Ok(())); assert!(kernel.held.get()); assert!(kernel.takes.borrow().is_empty() && kernel.releases.borrow().is_empty()); let waited = kernel.waited.borrow(); @@ -604,19 +612,23 @@ pub mod tests { assert!(host.refused.is_empty()); } - /// §19.6.2: an Acquire's TimeoutValue bounds the wait, and one that meets - /// it comes back untaken, holding nothing. + /// §19.6.2: an Acquire's TimeoutValue bounds the wait, however far past + /// [`RELEASE`], and one that meets it comes back untaken, holding nothing. #[test] fn an_acquires_own_bound_ends_the_wait_untaken() { - let kernel = machine(); - kernel.takes.borrow_mut().push_back(Take::Pending); - let mut host = Firmware::new(&kernel, None::); - let asked = Instant::now(); - assert_eq!(host.global_take(Some(5)), Ok(false)); - assert!(!kernel.held.get()); - let until = kernel.waited.borrow()[0]; - assert!(until >= asked + Duration::from_millis(5) && until < asked + RELEASE, "{until:?} after {asked:?}"); - assert_eq!((host.takes, host.contended, host.given_back), (1, 1, 0)); + for ms in [5, 1000, 0xFFFE] { + let kernel = machine(); + kernel.takes.borrow_mut().push_back(Take::Pending); + let mut host = Firmware::new(&kernel, None::); + let asked = Instant::now(); + assert_eq!(host.global_take_within(ms), Ok(false)); + let returned = Instant::now(); + assert!(!kernel.held.get()); + let until = kernel.waited.borrow()[0]; + let bound = Duration::from_millis(ms.into()); + assert!(until >= asked + bound && until <= returned + bound, "{until:?} is not {bound:?} after the take at {asked:?}"); + assert_eq!((host.takes, host.contended, host.given_back), (1, 1, 0)); + } } #[test] @@ -625,7 +637,7 @@ pub mod tests { let kernel = machine(); kernel.takes.borrow_mut().push_back(Take::Pending); let mut host = Firmware::new(&kernel, None::); - let _ = host.global_take(None); + let _ = host.global_take(); } #[test] @@ -634,7 +646,7 @@ pub mod tests { kernel.takes.borrow_mut().push_back(Take::Unusable); let mut host = Firmware::new(&kernel, None::); assert_eq!( - host.global_take(None), + host.global_take(), Err(Denied("the Global Lock: the kernel exchanges no lock word in this machine's FACS".into())) ); assert_eq!(host.contended, 0); @@ -651,7 +663,7 @@ pub mod tests { let stopping = Denied(Refusal::Stopping.to_string()); assert_eq!(host.read(Address::Memory(NVS), toyos_aml::Access::Byte), Err(stopping.clone())); assert!(host.stopping); - assert_eq!(host.global_take(None), Err(stopping.clone())); + assert_eq!(host.global_take(), Err(stopping.clone())); assert_eq!(host.global_release(), Err(stopping)); assert!(host.refused.is_empty(), "the stop was counted as something this machine's firmware was refused"); } diff --git a/userland/acpiserver/src/main.rs b/userland/acpiserver/src/main.rs index d07b3c6c892..6e5b4f696f5 100644 --- a/userland/acpiserver/src/main.rs +++ b/userland/acpiserver/src/main.rs @@ -48,6 +48,7 @@ mod ledger; mod sci; mod tables; +use std::cell::Cell; use std::collections::{BTreeMap, VecDeque}; use std::time::{Duration, Instant}; @@ -74,7 +75,7 @@ const QUERIES: usize = 32; const COUNTS: Duration = Duration::from_secs(30); struct Server<'a> { - dev: &'a AcpiDev, + claim: &'a Claim<'a>, info: AcpiInfo, served: Served, /// The controller the row names, if it names one. @@ -104,9 +105,9 @@ fn main() { power_button: info.flags & FIXED_POWER_BUTTON != 0, ec_gpe: info.has_ec().then_some(info.ec_gpe), }; - let claim = Claim { dev: &dev, info }; + let claim = Claim { dev: &dev, info, poller: Poller::new(1), watching: Cell::new(false) }; let mut server = Server { - dev: &dev, + claim: &claim, info, served, ec: info.has_ec().then_some(Ports { command: info.ec_command.port, data: info.ec_data.port }), @@ -177,13 +178,27 @@ impl Controller for Ports { /// The claim as the tables' fetch and their AML ask it for what lies outside /// its own ports, and as the wait for the firmware's release of the Global -/// Lock reads its event blocks and its SCI. +/// Lock reads its event blocks and its SCI; and the one poller its SCI is +/// waited on with, between SCIs and inside that wait. struct Claim<'a> { dev: &'a AcpiDev, info: AcpiInfo, + poller: Poller, + /// A watch answers once: one still registered from a wait that timed out + /// is not registered again. + watching: Cell, } impl Claim<'_> { + /// Waits until `until` for the claim's record to be readable. + fn readable(&self, until: Instant) { + if !self.watching.replace(true) { + self.poller.watch(self.dev, READABLE, 0); + } + let wait = until.saturating_duration_since(Instant::now()); + self.poller.wait(1, wait.as_nanos() as u64, |_| self.watching.set(false)); + } + /// The kernel's answer; a stopping machine's is the caller's to carry, /// and any other refusal of a call this server formed is this server's /// defect. @@ -213,7 +228,7 @@ impl Kernel for Claim<'_> { } fn released(&self, until: Instant) -> bool { - sci::await_release(&mut Waiting { claim: self, poller: Poller::new(1), watching: false }, until) + sci::await_release(&mut Waiting(self), until) } fn s5(&self, slp_typ_a: u64) -> Result { @@ -225,33 +240,27 @@ impl Kernel for Claim<'_> { } /// The claim's event blocks and SCI for one wait for the firmware's release. -struct Waiting<'c, 'a> { - claim: &'c Claim<'a>, - poller: Poller, - /// A watch answers once: one still registered from a wait that timed out - /// is not registered again. - watching: bool, -} +struct Waiting<'c, 'a>(&'c Claim<'a>); impl Fixed for Waiting<'_, '_> { fn pm1_status(&mut self) -> u16 { - in16(self.claim.info.pm1_event.port) + in16(self.0.info.pm1_event.port) } fn pm1_clear(&mut self, bits: u16) { - out16(self.claim.info.pm1_event.port, bits); + out16(self.0.info.pm1_event.port, bits); } fn enables(&mut self) -> Enables { Enables { - pm1: in16(self.claim.info.pm1_event.enable()), - gpe0: bytes(self.claim.info.gpe0).map(|(_, enable)| in8(enable)).collect(), + pm1: in16(self.0.info.pm1_event.enable()), + gpe0: bytes(self.0.info.gpe0).map(|(_, enable)| in8(enable)).collect(), } } fn enable(&mut self, enables: &Enables) { - out16(self.claim.info.pm1_event.enable(), enables.pm1); - for ((_, enable), &byte) in bytes(self.claim.info.gpe0).zip(&enables.gpe0) { + out16(self.0.info.pm1_event.enable(), enables.pm1); + for ((_, enable), &byte) in bytes(self.0.info.gpe0).zip(&enables.gpe0) { out8(enable, byte); } } @@ -261,21 +270,15 @@ impl Fixed for Waiting<'_, '_> { if left.is_zero() { return false; } - if !self.watching { - self.poller.watch(self.claim.dev, READABLE, 0); - self.watching = true; - } - let mut answered = false; - self.poller.wait(1, left.as_nanos() as u64, |_| answered = true); - self.watching &= !answered; - match self.claim.dev.irq() { + self.0.readable(until); + match self.0.dev.irq() { Ok(_) | Err(SyscallError::WouldBlock) => true, Err(other) => panic!("acpiserver: the claim's record answered {other:?} while the Global Lock was waited for"), } } fn ack(&mut self) { - self.claim.dev.ack().expect("acpiserver: the claim's acknowledgement"); + self.0.dev.ack().expect("acpiserver: the claim's acknowledgement"); } } @@ -304,7 +307,7 @@ impl Server<'_> { self.drain(); self.run_queued(); } - self.dev.ack().expect("acpiserver: the claim's acknowledgement"); + self.claim.dev.ack().expect("acpiserver: the claim's acknowledgement"); println!( "acpiserver: armed: power button {}, embedded controller {}", if self.served.power_button { "served" } else { "not the fixed one, so not served" }, @@ -316,12 +319,8 @@ impl Server<'_> { } fn serve(&mut self) -> ! { - let poller = Poller::new(1); let mut next_count = Instant::now() + COUNTS; let mut next_read = Instant::now() + battery::POLL; - // A watch answers once: one still registered from a wait that timed - // out is not registered again. - let mut watching = false; loop { if let (Some(power), Some(aml)) = (&mut self.power, &mut self.aml) && Instant::now() >= next_read @@ -334,24 +333,18 @@ impl Server<'_> { self.power = None; } } - if !watching { - poller.watch(self.dev, READABLE, 0); - watching = true; - } - let until = if self.power.is_some() { next_count.min(next_read) } else { next_count }; - let wait = until.saturating_duration_since(Instant::now()); - poller.wait(1, wait.as_nanos() as u64, |_| watching = false); + self.claim.readable(if self.power.is_some() { next_count.min(next_read) } else { next_count }); if Instant::now() >= next_count { self.log_counts(); next_count = Instant::now() + COUNTS; } - match self.dev.irq() { + match self.claim.dev.irq() { Ok(record) => self.scis += u64::from(record.count), Err(SyscallError::WouldBlock) => continue, Err(other) => panic!("acpiserver: the claim's record answered {other:?}"), } self.take(); - self.dev.ack().expect("acpiserver: the claim's acknowledgement"); + self.claim.dev.ack().expect("acpiserver: the claim's acknowledgement"); } } From 0f1b0596686074b397b753c49cb4bfb2ba9680f1 Mon Sep 17 00:00:00 2001 From: japabu Date: Sat, 10 Oct 2026 15:23:37 +0200 Subject: [PATCH 05/12] The track records the 1 s bound on an unbounded take of the Global Lock, with its owner and exit RELEASE is this server's guess: no hold of the lock has been seen, the T14's battery read found the firmware holding it by 0 of its 3 takes, and Linux's ff_gbl_lock read 0 there at 2 minutes of uptime. The exit is a measured hold. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017cSFvbD35xJ2kGANVdm23C --- ...machine-in-acpi-mode-and-interprets-its-aml.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/issues/toyos-runs-the-machine-in-acpi-mode-and-interprets-its-aml.md b/issues/toyos-runs-the-machine-in-acpi-mode-and-interprets-its-aml.md index 8c7c7ed05e7..6e24d2f62e0 100644 --- a/issues/toyos-runs-the-machine-in-acpi-mode-and-interprets-its-aml.md +++ b/issues/toyos-runs-the-machine-in-acpi-mode-and-interprets-its-aml.md @@ -268,6 +268,21 @@ mediated access, leaves open: controller not at all; Linux runs it after. Owner: this stage. **Exit**: the slice that runs the `_STA` and `_INI` walk runs it before `_REG`. +- **A take of the Global Lock that the AML sets no bound on ends the server + where the firmware holds the lock past 1 s** (`RELEASE`, + `userland/acpiserver/src/host.rs`). The bound is this server's guess, not + a measurement. The firmware is meant to hold the lock for the run of one + SMI handler, and no hold has been seen. The T14's battery read took the + lock 3 times, and the firmware held it for none of them. Linux's + `ff_gbl_lock` counter there read 0 at 2 minutes of uptime, and a reading + after a longer session is still owed. A Lock field and an `Acquire(\_GL, + 0xFFFF)` take with no bound. An Acquire with a TimeoutValue waits that + long, and the time is charged to the evaluation's 10 s. Owner: this + stage. **Exit**: a hold measured on the T14, either by a contended take + in the server's own count or by `ff_gbl_lock` after a long Linux session. + That measurement sets the bound, or replaces it with no bound if the + firmware never holds the lock. + - **A press during the load waits for it, and a power-off asked during it is refused.** The server arms the power button and then loads the tables before it serves an SCI, so a press in that time From 0d6cb71e50fcfbad6a3e5253f451407f16c79480 Mon Sep 17 00:00:00 2001 From: japabu Date: Sat, 10 Oct 2026 16:09:33 +0200 Subject: [PATCH 06/12] The wait for the firmware's release clears an enable only once its status has latched The wait set `GBL_EN` alone for its whole length, so a power button press or a controller GPE inside it rested on the specification's model, where a status latches under a clear enable (ACPI 6.5 Table 4.13) and the enable only gates the SCI. QEMU's ICH9 latches `PWRBTN_STS` only under `PWRBTN_EN` (measured in PR #838, comment 6098023980), so there a press inside the wait was dropped. The wait now sets `GBL_EN` beside every enable kept and, at each SCI, clears the enable of each status that has latched, leaving the status for whoever serves it; the level line falls, and every enable is put back after. No model of the hardware is assumed. The host test's emulated block now latches a PM1 status only under its enable, the stricter model, and counts one wake per event. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017cSFvbD35xJ2kGANVdm23C --- userland/acpiserver/src/main.rs | 15 +++++---- userland/acpiserver/src/sci.rs | 58 +++++++++++++++++++-------------- 2 files changed, 43 insertions(+), 30 deletions(-) diff --git a/userland/acpiserver/src/main.rs b/userland/acpiserver/src/main.rs index 6e5b4f696f5..b63bf378451 100644 --- a/userland/acpiserver/src/main.rs +++ b/userland/acpiserver/src/main.rs @@ -64,7 +64,7 @@ use ec::{Do, Transaction, Wait}; use aml::Aml; use battery::Power; use host::{Answer, Controller, Kernel, Stopping, Take}; -use sci::{Enables, Event, Fixed, Served, Unserved, Unstopped, PM1_STATUS, PWRBTN}; +use sci::{Bits, Event, Fixed, Served, Unserved, Unstopped, PM1_STATUS, PWRBTN}; /// The most a controller is waited for at one step of a transaction: one that /// has not moved in this will not. @@ -243,22 +243,25 @@ impl Kernel for Claim<'_> { struct Waiting<'c, 'a>(&'c Claim<'a>); impl Fixed for Waiting<'_, '_> { - fn pm1_status(&mut self) -> u16 { - in16(self.0.info.pm1_event.port) + fn status(&mut self) -> Bits { + Bits { + pm1: in16(self.0.info.pm1_event.port), + gpe0: bytes(self.0.info.gpe0).map(|(status, _)| in8(status)).collect(), + } } fn pm1_clear(&mut self, bits: u16) { out16(self.0.info.pm1_event.port, bits); } - fn enables(&mut self) -> Enables { - Enables { + fn enables(&mut self) -> Bits { + Bits { pm1: in16(self.0.info.pm1_event.enable()), gpe0: bytes(self.0.info.gpe0).map(|(_, enable)| in8(enable)).collect(), } } - fn enable(&mut self, enables: &Enables) { + fn enable(&mut self, enables: &Bits) { out16(self.0.info.pm1_event.enable(), enables.pm1); for ((_, enable), &byte) in bytes(self.0.info.gpe0).zip(&enables.gpe0) { out8(enable, byte); diff --git a/userland/acpiserver/src/sci.rs b/userland/acpiserver/src/sci.rs index 0b2eda845ff..5b228cf183a 100644 --- a/userland/acpiserver/src/sci.rs +++ b/userland/acpiserver/src/sci.rs @@ -68,9 +68,9 @@ pub fn events(served: &Served, pm1: (u16, u16), gpe0: &[(u8, u8)]) -> Result, } @@ -78,11 +78,11 @@ pub struct Enables { /// The fixed hardware and the SCI, as the wait for the firmware's release /// drives them. pub trait Fixed { - fn pm1_status(&mut self) -> u16; + fn status(&mut self) -> Bits; /// Write `bits` to PM1 status, which clears each one set (Table 4.13). fn pm1_clear(&mut self, bits: u16); - fn enables(&mut self) -> Enables; - fn enable(&mut self, enables: &Enables); + fn enables(&mut self) -> Bits; + fn enable(&mut self, enables: &Bits); /// Waits for the SCI until `until` and takes its record: `false` is the /// deadline, and a wake with no record is `true`. fn sci(&mut self, until: Instant) -> bool; @@ -92,20 +92,29 @@ pub trait Fixed { /// Waits until `until` for `GBL_STS`, after a take of the Global Lock left /// the firmware the pending bit (§5.2.10.1): `true` where it was set, and is -/// now clear. While it waits `GBL_EN` is the one enable set, so no other -/// event raises the SCI it waits on, whose line is level; each enable is put -/// back as it was, so an event latched meanwhile raises the SCI again for -/// whoever serves it. A `GBL_STS` left from an earlier release ends the wait -/// too, which costs the caller one take more and loses no release. +/// now clear. `GBL_EN` is set beside every enable kept; an event that +/// latches meanwhile keeps its status for whoever serves it, and loses its +/// enable until the wait ends, so the level SCI it raised falls. An enable +/// is cleared only once its status has latched, so the wait holds whether a +/// status latches under a clear enable (Table 4.13) or, as QEMU's ICH9 power +/// button, only under a set one. A `GBL_STS` left from an earlier release +/// ends the wait too, which costs the caller one take more and loses no +/// release. pub fn await_release(fixed: &mut impl Fixed, until: Instant) -> bool { let kept = fixed.enables(); - fixed.enable(&Enables { pm1: GBL, gpe0: vec![0; kept.gpe0.len()] }); + let mut enabled = Bits { pm1: kept.pm1 | GBL, gpe0: kept.gpe0.clone() }; let released = loop { - let set = fixed.pm1_status() & GBL != 0; + let status = fixed.status(); + let set = status.pm1 & GBL != 0; if set { fixed.pm1_clear(GBL); } - // Acknowledged with nothing but `GBL_STS` enabled, and that clear. + enabled.pm1 &= !status.pm1; + for (enable, status) in enabled.gpe0.iter_mut().zip(&status.gpe0) { + *enable &= !status; + } + fixed.enable(&enabled); + // Acknowledged with every status latched disabled, and `GBL_STS` clear. fixed.ack(); if set { break true; @@ -197,13 +206,14 @@ mod tests { /// A PM1 event block and a GPE0 block, whose status bits a write of one /// clears and whose SCI is level: asserted while any status bit and its /// enable are both set (§4.8.3.1), so an acknowledgement then is one the - /// line answers at once. A wait for the SCI lets the script set one thing - /// after another until the line is raised; past the script the wait - /// meets its deadline. + /// line answers at once. A PM1 status latches only under its enable, as + /// QEMU's ICH9 power button does. A wait for the SCI lets the script set + /// one thing after another until the line is raised; past the script the + /// wait meets its deadline. struct Block { status: u16, gpe0: Vec, - enables: Enables, + enables: Bits, script: Vec, sets: usize, acks: usize, @@ -214,7 +224,7 @@ mod tests { Block { status: 0, gpe0: vec![0; 3], - enables: Enables { pm1: PWRBTN, gpe0: vec![0x40, 0, 0x01] }, + enables: Bits { pm1: PWRBTN, gpe0: vec![0x40, 0, 0x01] }, script: script.to_vec(), sets: 0, acks: 0, @@ -223,22 +233,22 @@ mod tests { } impl Fixed for Block { - fn pm1_status(&mut self) -> u16 { - self.status + fn status(&mut self) -> Bits { + Bits { pm1: self.status, gpe0: self.gpe0.clone() } } fn pm1_clear(&mut self, bits: u16) { self.status &= !bits; } - fn enables(&mut self) -> Enables { + fn enables(&mut self) -> Bits { self.enables.clone() } - fn enable(&mut self, enables: &Enables) { + fn enable(&mut self, enables: &Bits) { self.enables = enables.clone(); } fn sci(&mut self, _: Instant) -> bool { while let Some(&set) = self.script.get(self.sets) { match set { - Set::Pm1(bits) => self.status |= bits, + Set::Pm1(bits) => self.status |= bits & self.enables.pm1, Set::Gpe(byte, bits) => self.gpe0[byte] |= bits, } self.sets += 1; @@ -270,7 +280,7 @@ mod tests { assert_eq!(block.status, PWRBTN, "GBL_STS left set, or an event the wait does not serve cleared"); assert_eq!(block.gpe0, [0x40, 0, 0], "the controller's GPE cleared by the wait"); assert_eq!(block.enables, kept, "an enable was not put back"); - assert_eq!((block.sets, block.acks), (3, 2), "the wait woke for an event it had disabled"); + assert_eq!((block.sets, block.acks), (3, 4), "the wait woke other than once for each event"); } #[test] From e74eac3b89ba17c4ce59f677d84b6e7e6dbec728 Mon Sep 17 00:00:00 2001 From: japabu Date: Sat, 10 Oct 2026 16:09:33 +0200 Subject: [PATCH 07/12] An Acquire of \_GL waits no longer than its evaluation has left to wait A timed-out Acquire was charged its TimeoutValue after the take had waited it, so one evaluation could sleep its 10 s and then wait 65.5 s more, all of it with the server serving no SCI. The take is now bounded by min(TimeoutValue, what is left), and a TimeoutValue past what is left is refused once that has run out; with nothing left the take is one attempt. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017cSFvbD35xJ2kGANVdm23C --- ...machine-in-acpi-mode-and-interprets-its-aml.md | 3 ++- userland/acpiserver/aml/src/exec.rs | 15 ++++++++++----- userland/acpiserver/aml/tests/evaluate.rs | 9 ++++++++- 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/issues/toyos-runs-the-machine-in-acpi-mode-and-interprets-its-aml.md b/issues/toyos-runs-the-machine-in-acpi-mode-and-interprets-its-aml.md index 6e24d2f62e0..88d009711f6 100644 --- a/issues/toyos-runs-the-machine-in-acpi-mode-and-interprets-its-aml.md +++ b/issues/toyos-runs-the-machine-in-acpi-mode-and-interprets-its-aml.md @@ -277,7 +277,8 @@ mediated access, leaves open: `ff_gbl_lock` counter there read 0 at 2 minutes of uptime, and a reading after a longer session is still owed. A Lock field and an `Acquire(\_GL, 0xFFFF)` take with no bound. An Acquire with a TimeoutValue waits that - long, and the time is charged to the evaluation's 10 s. Owner: this + long, at most what the evaluation has left of its 10 s, and the + TimeoutValue is charged to it. Owner: this stage. **Exit**: a hold measured on the T14, either by a contended take in the server's own count or by `ff_gbl_lock` after a long Linux session. That measurement sets the bound, or replaces it with no bound if the diff --git a/userland/acpiserver/aml/src/exec.rs b/userland/acpiserver/aml/src/exec.rs index e1e67695a15..7c8f5c9dfb8 100644 --- a/userland/acpiserver/aml/src/exec.rs +++ b/userland/acpiserver/aml/src/exec.rs @@ -262,16 +262,21 @@ impl<'a> Machine<'a> { /// Holds the Global Lock once more, taking it from the host where this /// evaluation does not hold it yet: `Ok(false)` is a take `within` a - /// bound that timed out, charged as a wait, and nothing held. + /// bound that timed out, charged as a wait, and nothing held. The take + /// waits no longer than the evaluation has left to wait, so a bound past + /// that is refused once what is left has run out. pub(crate) fn take_global(&mut self, within: Option) -> Result { if self.global == 0 { match within { None => self.host.global_take().map_err(|d| Error::Host(d.0))?, - Some(ms) if !self.host.global_take_within(ms).map_err(|d| Error::Host(d.0))? => { - self.wait(u64::from(ms) * 1000)?; - return Ok(false); + Some(ms) => { + let left = (MAX_WAIT_US - self.waited_us) / 1000; + let bound = u16::try_from(left).map_or(ms, |left| ms.min(left)); + if !self.host.global_take_within(bound).map_err(|d| Error::Host(d.0))? { + self.wait(u64::from(ms) * 1000)?; + return Ok(false); + } } - Some(_) => {} } } self.global += 1; diff --git a/userland/acpiserver/aml/tests/evaluate.rs b/userland/acpiserver/aml/tests/evaluate.rs index 99f0a7a286a..a0a7b3677b3 100644 --- a/userland/acpiserver/aml/tests/evaluate.rs +++ b/userland/acpiserver/aml/tests/evaluate.rs @@ -411,7 +411,8 @@ fn mutexes_follow_sync_levels_and_are_released_by_the_end() { /// §19.6.2: an Acquire of `\_GL` waits for the firmware's side at most its /// TimeoutValue in milliseconds, 0xFFFF naming no bound; one that times out /// returns True, holds nothing and is charged as a wait, and one this -/// evaluation already holds asks the host for nothing. +/// evaluation already holds asks the host for nothing. The host waits no +/// longer than the evaluation has left to wait. #[test] fn an_acquire_of_the_global_lock_waits_as_its_timeout_says() { let acquire = |ms: u16| cat(&[&[0x5B, 0x23], &name("\\_GL"), &ms.to_le_bytes()]); @@ -420,6 +421,7 @@ fn an_acquire_of_the_global_lock_waits_as_its_timeout_says() { &method("SOON", 0, &cat(&[&store(&acquire(5), &local(0)), &release, &ret(&local(0))])), &method("LATE", 0, &ret(&acquire(5))), &method("LONG", 0, &cat(&[&store(&acquire(6000), &local(0)), &ret(&acquire(6000))])), + &method("SPNT", 0, &cat(&[&[0x5B, 0x22], &int(10_000), &ret(&acquire(5))])), &method("EVER", 0, &cat(&[&store(&acquire(0xFFFF), &local(0)), &store(&acquire(0), &local(1)), &release, &release, &ret(&add(&local(0), &local(1), ZERO))])), ])); assert_eq!(ip.evaluate(&mut m, "\\EVER", &[]), i(0)); @@ -433,8 +435,13 @@ fn an_acquire_of_the_global_lock_waits_as_its_timeout_says() { assert_eq!(ip.evaluate(&mut m, "\\LATE", &[]), i(ONES), "an Acquire that timed out is True (§19.6.2)"); assert_eq!(m.log, vec![Event::GlobalTake(Some(5))], "a lock the take did not get was given back"); assert_eq!(ip.usage().waited_us, 5000); + m.log.clear(); assert_eq!(ip.evaluate(&mut m, "\\LONG", &[]), Err(Error::Bound("more time asleep than one evaluation may spend"))); + assert_eq!(m.log, vec![Event::GlobalTake(Some(6000)), Event::GlobalTake(Some(4000))], "a take waited past what was left"); assert_eq!(ip.usage().waited_us, 12_000_000); + m.log.clear(); + assert_eq!(ip.evaluate(&mut m, "\\SPNT", &[]), Err(Error::Bound("more time asleep than one evaluation may spend"))); + assert_eq!(m.log, vec![Event::Sleep(10_000), Event::GlobalTake(Some(0))], "a take with nothing left waited"); } #[test] From d89ef2f84c244d85825b351ae91136ef0b0a2ed5 Mon Sep 17 00:00:00 2001 From: japabu Date: Sat, 10 Oct 2026 16:09:34 +0200 Subject: [PATCH 08/12] The table in acpi_press_across_a_lock_wait waits for the press, not 5 s The SSDT slept 5 s before its Acquire, and the harness's press and its monitor reads had to land inside that window: a slow host made the test red. The table now reads PWRBTN_STS through a SystemIO field over q35's PM1a event block and polls it with Sleep (1) until the press latches, bounded by the evaluation's 10 s, which refuses the table loudly. The order press-then-take is the table's own, so the monitor reads of PM1 and the FACS word, and the harness's port_word and memory_word, go. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017cSFvbD35xJ2kGANVdm23C --- tests/common/power.rs | 89 +++++++++++++++++++------------------------ tests/common/qemu.rs | 35 ++--------------- 2 files changed, 44 insertions(+), 80 deletions(-) diff --git a/tests/common/power.rs b/tests/common/power.rs index 6eaa2ad3172..28223c3569c 100644 --- a/tests/common/power.rs +++ b/tests/common/power.rs @@ -761,13 +761,14 @@ pub fn acpi_power_button(test_config: &Path) -> Result<(), String> { Ok(()) } -/// How long [`ssdt_acquiring_the_global_lock`] sleeps as it loads, in ms, -/// before its Acquire: the window the press is sent in, wide beside a host's -/// answer to a line. -const PRESS_WINDOW_MS: u16 = 5000; - -/// The Acquire's TimeoutValue, in ms; the two together are within the -/// 10 s one evaluation may spend asleep. +/// The PM1a event block QEMU's q35 firmware sets, whose status +/// [`ssdt_acquiring_the_global_lock`] reads: the table is built before the +/// boot, which then says the block it found. +const Q35_PM1A_EVENTS: u16 = 0x600; + +/// The Acquire's TimeoutValue, in ms; the wait for the press before it has +/// the rest of the 10 s one evaluation may spend asleep, past which the +/// load refuses the table. const ACQUIRE_MS: u16 = 2000; /// What the server says of the table's Notify, which runs only where the @@ -778,37 +779,41 @@ const ACQUIRE_TIMED_OUT: &str = "Notify(\\_SB_.GLW_, 0x80) for the first time"; /// the lock. const ONE_TAKE_CONTENDED: &str = "; took the Global Lock 1 times, 1 of them from the firmware;"; -/// PM1 status and enable (ACPI 6.5 Tables 4.13, 4.14): `PWRBTN_STS` and -/// `PWRBTN_EN`. -const PWRBTN: u16 = 1 << 8; - -/// The FACS's lock word's owned bit (Table 5.11), with the pending bit a -/// take that finds it owned sets clear. -const OWNED: u32 = 2; - /// An SSDT (ACPI 6.5 §5.2.11.2) whose definition block declares the device -/// `\_SB.GLW` and, as it loads, sleeps `window` ms, then Acquires `\_GL` -/// within `ms`, giving it back where that took it and Notifying `\_SB.GLW` -/// with 0x80 where it timed out: +/// `\_SB.GLW` and, as it loads, waits for `PWRBTN_STS` (Table 4.13) in the +/// PM1a event block at `pm1`, then Acquires `\_GL` within `ms`, giving it +/// back where that took it and Notifying `\_SB.GLW` with 0x80 where it timed +/// out: /// /// ```text /// Device (\_SB.GLW) {} -/// Sleep (window) +/// OperationRegion (PM1S, SystemIO, pm1, 2) +/// Field (PM1S, WordAcc, NoLock, Preserve) { Offset (1), PBST, 1 } +/// While (!PBST) { Sleep (1) } /// If (Acquire (\_GL, ms)) { Notify (\_SB.GLW, 0x80) } Else { Release (\_GL) } /// ``` -fn ssdt_acquiring_the_global_lock(window: u16, ms: u16) -> Vec { +fn ssdt_acquiring_the_global_lock(pm1: u16, ms: u16) -> Vec { const GL: &[u8] = b"\\_GL_"; // `\` and a DualNamePrefix (§20.2.2). const GLW: &[u8] = b"\\\x2E_SB_GLW_"; + const PM1S: &[u8] = b"PM1S"; + const PBST: &[u8] = b"PBST"; // A PkgLength of one byte (§20.2.4): itself and what follows, under 64. let package = |op: &[u8], body: &[u8]| [op, &[u8::try_from(body.len() + 1).expect("a short package")], body].concat(); - let sleep = [&[0x5B, 0x22, 0x0B], &window.to_le_bytes()[..]].concat(); + // SystemIO (1) at a WordConst, of a ByteConst's length. + let region = [&[0x5B, 0x80], PM1S, &[0x01, 0x0B], &pm1.to_le_bytes(), &[0x0A, 0x02]].concat(); + // WordAcc (2); a ReservedField (0) of 8 bits, then PBST of 1. + let field = package(&[0x5B, 0x81], &[PM1S, &[0x02, 0x00, 0x08], PBST, &[0x01]].concat()); + // LNot (0x92) of PBST; Sleep of One (0x01). + let wait = package(&[0xA2], &[&[0x92], PBST, &[0x5B, 0x22, 0x01]].concat()); let acquire = [&[0x5B, 0x23], GL, &ms.to_le_bytes()].concat(); let notify = [&[0x86], GLW, &[0x0A, 0x80]].concat(); let release = [&[0x5B, 0x27], GL].concat(); let aml = [ package(&[0x5B, 0x82], GLW), - sleep, + region, + field, + wait, package(&[0xA0], &[acquire, notify].concat()), package(&[0xA1], &release), ] @@ -830,20 +835,15 @@ fn hex_after(line: &str, after: &str) -> Result { /// A press latched before the server's wait for the firmware's release of /// the Global Lock is served once the wait ends at its Acquire's timeout: the -/// wait clears every enable but `GBL_EN`, takes the SCI the press raised, -/// and puts every enable back after, so the press's status raises the SCI -/// again for `serve`. `tests/acpicase` with the test kernel's actuator for -/// the firmware's side of the lock, which `acpi_mediated`'s `held` arm stages -/// owned before it hands the server the claim; the table QEMU adds sleeps -/// [`PRESS_WINDOW_MS`] as it loads, then Acquires the lock within -/// [`ACQUIRE_MS`]. -/// -/// The press is sent once the server has armed, and QEMU's monitor reads -/// right after it `PWRBTN_STS` latched under `PWRBTN_EN` and the lock word -/// owned with no pending bit: the server had not yet taken, so its wait came -/// after the press. A press inside the wait is QEMU's to drop, which sets -/// `PWRBTN_STS` only under `PWRBTN_EN`. Then the press must power the machine -/// off, after the Acquire said it timed out. +/// wait clears the press's enable once it finds its status latched, takes +/// the SCI the press raised, and puts every enable back after, so the press's +/// status raises the SCI again for `serve`. `tests/acpicase` with the test +/// kernel's actuator for the firmware's side of the lock, which +/// `acpi_mediated`'s `held` arm stages owned before it hands the server the +/// claim; the table QEMU adds waits as it loads for the press, sent once the +/// server has armed, and only then Acquires the lock within [`ACQUIRE_MS`]. +/// Then the press must power the machine off, after the Acquire said it timed +/// out. pub fn acpi_press_across_a_lock_wait(probe: (String, Vec)) -> Result<(), String> { let case = super::compile::repo_root().join("tests/acpicase"); let mut qemu = QemuInstance::boot_with_options( @@ -859,30 +859,22 @@ pub fn acpi_press_across_a_lock_wait(probe: (String, Vec)) -> Result<(), Str // The arm's name is the file's: the probe asks whether it is there. ("share/acpi_mediated_held".to_string(), b"held\n".to_vec()), ], - acpi_tables: vec![ssdt_acquiring_the_global_lock(PRESS_WINDOW_MS, ACQUIRE_MS)], + acpi_tables: vec![ssdt_acquiring_the_global_lock(Q35_PM1A_EVENTS, ACQUIRE_MS)], qmp: true, ..Default::default() }, ); let boot = serial::Serial::named("the boot", qemu.boot_log().to_string()); let pm1 = hex_after(boot.must_say("acpi: the ACPI row: PM1a events ")?, "PM1a events ")?; - let pm1 = u16::try_from(pm1).map_err(|_| format!("a PM1a event block at {pm1:#x}"))?; - let lock_word = hex_after(boot.must_say("acpi: the Global Lock is the FACS's at ")?, "the FACS's at ")?; + if pm1 != u64::from(Q35_PM1A_EVENTS) { + return Err(format!("the PM1a event block is at {pm1:#x}, where the table QEMU was handed reads {Q35_PM1A_EVENTS:#x}")); + } // Opened before the press: QMP delivers no event emitted before its // client connected. let mut stop = qemu::QmpShutdown::open(qemu.qmp_socket(), qemu.budget(qemu::GUEST_QUIET)); let mut console = format!("{}\n", qemu.boot_log()); qemu::await_marker(&mut qemu, &mut console, ACPI_ARMED, "the ACPI server arming")?; stop.power_button(); - // Table 4.12: PM1 enable follows PM1 status in the block. - let (status, enable, word) = (stop.port_word(pm1), stop.port_word(pm1 + 2), stop.memory_word(lock_word)); - if status & enable & PWRBTN == 0 || word != OWNED { - return Err(format!( - "right after the press PM1 read status {status:#06x} under enable {enable:#06x} and the lock word {word:#x}, where a \ - press latched before the server's take reads PWRBTN_STS under PWRBTN_EN ({PWRBTN:#06x}) and the word owned with no \ - pending bit ({OWNED:#x})" - )); - } let pressed_at = console.len(); ended(&mut qemu, &mut stop, &mut console, SHUTTING_DOWN, "guest-shutdown")?; let after = serial::Serial::named("the press", console[pressed_at..].to_string()); @@ -893,7 +885,6 @@ pub fn acpi_press_across_a_lock_wait(probe: (String, Vec)) -> Result<(), Str if acpi_tables_loaded(&whole, &whole, Q35_S5_SUPPLIED)? < 2 { return Err(format!("the server loaded no table beside the DSDT, so not the one QEMU was handed:\n{}", whole.text())); } - eprintln!(" [power] PM1 status {status:#06x} under enable {enable:#06x} and the lock word {word:#x} after the press"); eprintln!(" [power] {}", whole.must_say(ONE_TAKE_CONTENDED)?.trim()); eprintln!(" [power] {}", whole.must_say_after(ACQUIRE_TIMED_OUT, ACPI_PRESSED)?.trim()); Ok(()) diff --git a/tests/common/qemu.rs b/tests/common/qemu.rs index a19c28e22f2..919b1e89f6d 100644 --- a/tests/common/qemu.rs +++ b/tests/common/qemu.rs @@ -1936,14 +1936,6 @@ impl Qmp { self.await_reply("\"return\""); } - /// Run `command` in the human monitor and return what it printed. - fn human(&mut self, command: &str) -> String { - self.execute_capturing(&format!( - "{{\"execute\":\"human-monitor-command\",\"arguments\":\ - {{\"command-line\":\"{command}\"}}}}" - )) - } - /// `execute`, keeping what the command answered with. Only the human /// monitor answers with anything; every other command here returns `{}`. fn execute_capturing(&mut self, command: &str) -> String { @@ -2003,22 +1995,6 @@ impl QmpShutdown { self.0.execute("{\"execute\":\"system_powerdown\"}"); } - /// A 16-bit read of the guest's I/O `port`, by the human monitor on the - /// one connection QEMU serves; asked before the stop, whose event a - /// command's reply would otherwise pass over, as [`Self::memory_word`]. - pub fn port_word(&mut self, port: u16) -> u16 { - let said = self.0.human(&format!("i /h {port:#x}")); - let value = monitor_value(&said, "= ").and_then(|value| u16::try_from(value).ok()); - value.unwrap_or_else(|| panic!("qmp: the monitor's read of port {port:#x} said {said:?}")) - } - - /// A 32-bit read of the guest's physical memory at `phys`. - pub fn memory_word(&mut self, phys: u64) -> u32 { - let said = self.0.human(&format!("xp /1wx {phys:#x}")); - let value = monitor_value(&said, ": ").and_then(|value| u32::try_from(value).ok()); - value.unwrap_or_else(|| panic!("qmp: the monitor's read of {phys:#x} said {said:?}")) - } - /// The `reason` the `SHUTDOWN` event names — `guest-reset`, /// `guest-shutdown`, `host-signal` — or `None` if the guest never stopped. pub fn reason(&mut self) -> Option { @@ -2037,12 +2013,6 @@ impl QmpShutdown { } } -/// The hex value after the last `after` on what the human monitor said. -fn monitor_value(said: &str, after: &str) -> Option { - let (_, value) = said.rsplit_once(after)?; - u64::from_str_radix(value.trim().trim_start_matches("0x"), 16).ok() -} - /// The `reason` of the `SHUTDOWN` event in what QMP has sent so far. fn shutdown_reason(bytes: &[u8]) -> Option { let text = String::from_utf8_lossy(bytes); @@ -2064,7 +2034,10 @@ impl QmpMonitor { /// Run `command` in the human monitor and return what it printed. pub fn human(&mut self, command: &str) -> String { - self.0.human(command) + self.0.execute_capturing(&format!( + "{{\"execute\":\"human-monitor-command\",\"arguments\":\ + {{\"command-line\":\"{command}\"}}}}" + )) } } From b91c58a942f64dfb9977ec0e1e4149e5dd48bac1 Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 11 Oct 2026 07:40:38 +0200 Subject: [PATCH 09/12] A controller port refused to the drain detaches it from AML too, under a host test The server test now refuses the controller's ports both ways, to AML at the battery's read and to the server's own drain, and holds each to the same end: unserved, detached from AML, its GPE disabled, and asked for by neither again. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017cSFvbD35xJ2kGANVdm23C --- userland/acpiserver/src/main.rs | 67 +++++++++++++++++++-------------- 1 file changed, 38 insertions(+), 29 deletions(-) diff --git a/userland/acpiserver/src/main.rs b/userland/acpiserver/src/main.rs index b937e240365..8fdd907ce05 100644 --- a/userland/acpiserver/src/main.rs +++ b/userland/acpiserver/src/main.rs @@ -658,37 +658,46 @@ mod tests { } /// The controller is attached to AML as it is armed, its `_REG` run and - /// the battery found through it; a port of it the kernel then refuses to - /// AML, at the battery's next read, ends the controller as one refused to - /// the server's own drain does: detached, its GPE disabled, and neither - /// AML nor the drain asks the kernel for it again. + /// the battery found through it. A port of it the kernel then refuses, + /// to AML at the battery's next read or to the server's own drain, ends + /// the controller for both: detached from AML, unserved, its GPE + /// disabled, and neither AML nor the drain asks the kernel for it again. #[test] - fn a_controller_port_the_kernel_refuses_to_aml_ends_the_controller() { - let kernel = crate::battery::tests::laptop_machine(None); - let info = AcpiInfo { rsdp: CRAFTED_RSDP, pm1_event: Block::NONE, gpe0: GPE0, flags: FIXED_POWER_BUTTON, reserved: [0; 3] }; - let mut server = Server::new(&kernel, Bytes::default(), info); - for (_, enable) in bytes(GPE0) { - server.ports.out8(enable, 0); + fn a_controller_port_the_kernel_refuses_to_aml_or_the_drain_ends_it_for_both() { + for to_aml in [true, false] { + let kernel = crate::battery::tests::laptop_machine(None); + let info = AcpiInfo { rsdp: CRAFTED_RSDP, pm1_event: Block::NONE, gpe0: GPE0, flags: FIXED_POWER_BUTTON, reserved: [0; 3] }; + let mut server = Server::new(&kernel, Bytes::default(), info); + for (_, enable) in bytes(GPE0) { + server.ports.out8(enable, 0); + } + server.aml = load(&kernel, CRAFTED_RSDP).1; + let (batteries, adapters) = server.arm_controller(); + assert_eq!(server.served.ec_gpe, Some(3)); + assert!(server.aml.as_ref().is_some_and(|aml| aml.host.ec.is_some()), "the served controller is not attached to AML"); + server.find_power(batteries, adapters); + assert!(server.power.is_some(), "no battery read through the attached controller"); + assert_eq!(server.ec.as_ref().map(|ec| ec.gpe), Some(3)); + + // The kernel keeps the controller's ports from here on. + *kernel.controller.borrow_mut() = None; + if to_aml { + server.read_power(); + } else { + server.drain(); + } + assert_eq!((server.ec.is_some(), server.served.ec_gpe), (false, None), "refused to AML {to_aml}: the controller is still served"); + assert!( + server.aml.as_ref().is_some_and(|aml| aml.host.ec.is_none() && aml.host.ended.is_none()), + "refused to AML {to_aml}: the controller is still attached to AML" + ); + assert!(bytes(GPE0).all(|(_, enable)| server.ports.in8(enable) == 0), "refused to AML {to_aml}: the controller's GPE is left enabled"); + let asked = kernel.asked.borrow().len(); + server.read_power(); + server.drain(); + let ports = kernel.asked.borrow().iter().skip(asked).filter(|access| access.space == Space::SystemIo as u8).count(); + assert_eq!(ports, 0, "refused to AML {to_aml}: the controller was asked again"); } - server.aml = load(&kernel, CRAFTED_RSDP).1; - let (batteries, adapters) = server.arm_controller(); - assert_eq!(server.served.ec_gpe, Some(3)); - assert!(server.aml.as_ref().is_some_and(|aml| aml.host.ec.is_some()), "the served controller is not attached to AML"); - server.find_power(batteries, adapters); - assert!(server.power.is_some(), "no battery read through the attached controller"); - assert_eq!(server.ec.as_ref().map(|ec| ec.gpe), Some(3)); - - // The kernel keeps the controller's ports from here on. - *kernel.controller.borrow_mut() = None; - server.read_power(); - assert_eq!((server.ec.is_some(), server.served.ec_gpe), (false, None), "a controller refused to AML is still served"); - assert!(server.aml.as_ref().is_some_and(|aml| aml.host.ec.is_none() && aml.host.ended.is_none())); - assert!(bytes(GPE0).all(|(_, enable)| server.ports.in8(enable) == 0), "the controller's GPE is left enabled"); - let asked = kernel.asked.borrow().len(); - server.read_power(); - server.drain(); - let ports = kernel.asked.borrow().iter().skip(asked).filter(|access| access.space == Space::SystemIo as u8).count(); - assert_eq!(ports, 0, "a controller refused to AML was asked again"); } /// The owner's ruling: a machine whose power button is a control method From d2c7267fc0ba123bf9e05c046b900271614486dc Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 11 Oct 2026 07:49:54 +0200 Subject: [PATCH 10/12] The battery fixture's _REG writes the controller, as the T14's does The T14's `_REG` reads and writes controller offset 0x03 (the scouts' dry run, recorded in the track). The fixture's now writes offset 0x03 too, so the controller must be attached to AML before `_REG` runs: one attached after it has `_REG`'s write denied, which the battery test reads in the controller's transactions and the host's refusals. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017cSFvbD35xJ2kGANVdm23C --- userland/acpiserver/src/battery.rs | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/userland/acpiserver/src/battery.rs b/userland/acpiserver/src/battery.rs index 40d4e2b8972..ece7a699db6 100644 --- a/userland/acpiserver/src/battery.rs +++ b/userland/acpiserver/src/battery.rs @@ -427,6 +427,7 @@ pub mod tests { use common::{device, field, if_, index, int, lequal, method, op_region, package, ret, scope, skip, store, string, unit as field_unit}; /// The controller's space as the fixture's fields lay it out. + const CONNECTED: u8 = 0x03; const STATE: u8 = 0x38; const AC: u8 = 0x46; const PAGE: u8 = 0x81; @@ -435,14 +436,17 @@ pub mod tests { /// A laptop's namespace as §10.2.2, §10.3, §6.5.4 and §12.11 have one: an /// embedded controller at [`EC_DATA`] and [`EC_COMMAND`] whose `_REG` - /// records that its space is there; a battery below it whose `_STA` says + /// records that its space is there, in itself and, as the T14's writes + /// offset 0x03, in the controller; a battery below it whose `_STA` says /// it is present only once `_REG` has run, whose `_BIX` is constant and /// whose `_BST` selects a page of the controller's space with a write and /// then reads it; and an adapter whose `_PSR` reads the controller. /// `glk` is the controller's `_GLK`, where it has one. fn laptop(glk: Option) -> Vec { let fields = [ - skip(8 * usize::from(STATE)), + skip(8 * usize::from(CONNECTED)), + field_unit("REGS", 8), + skip(8 * usize::from(STATE - CONNECTED - 1)), field_unit("BSTA", 8), skip(8 * usize::from(AC - STATE - 1)), field_unit("ACST", 8), @@ -491,7 +495,7 @@ pub mod tests { op_region("ECOR", 0x03, &int(0), &int(0x100)), field("ECOR", 0x01, &fields), common::def_name("ECOK", &int(0)), - method("_REG", 2, &if_(&lequal(&[0x68], &int(3)), &store(&[0x69], b"ECOK"))), + method("_REG", 2, &if_(&lequal(&[0x68], &int(3)), &[store(&[0x69], b"ECOK"), store(&[0x69], b"REGS")].concat())), glk.map_or(Vec::new(), |glk| common::def_name("_GLK", &int(glk))), battery, ] @@ -555,8 +559,9 @@ pub mod tests { const RD: u8 = 0x80; const WR: u8 = 0x81; - /// The controller is told its space is there before the battery is asked - /// whether it is present, which here it says only after; its information + /// The controller is attached before its `_REG` runs, which writes it, + /// and told its space is there before the battery is asked whether it + /// is present, which here it says only after; its information /// is read once and the battery read with it, through the controller's /// transactions alone, each through the kernel: the adapter's byte, then /// the page `_BST` selects, written before its fields are read, a byte at @@ -576,6 +581,7 @@ pub mod tests { assert_eq!( done(&kernel), [ + (WR, CONNECTED, 1), (RD, AC, 0), (WR, PAGE, 1), (RD, STATE, 0), From cce064f995f87841c150ca270b32110b96441833 Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 11 Oct 2026 08:12:42 +0200 Subject: [PATCH 11/12] battery_read demands no Global Lock take of the battery's methods, and a host test pins what the census counts The T14 run at d2c7267fc read the power-sources census as "the Global Lock taken 0 times on the way and given back 0", and the row redded on the 0. At 60077a173 the same line read 3. The rework onto #845's controller path moved the namespace's _HID walk, the controller's _GLK and its _REG out of Power::find and ahead of it, into devices::find and attach, so find's window now spans the battery's _STA and _BIX and the first _PSR and _BST alone. Those took the lock none of the times; the 3 were the walk's, _GLK's or _REG's. The row's premise, that the battery's methods take the lock through Lock-rule memory fields, was a misattribution the wider window hid. The row now holds only that every take was given back. The lock word's real exchange on the T14 is acpi_tables_loaded's, whose load takes it 241 times. Power keeps what find counted (Locked), and a host test pins the window: in the fixture both _REG and _BST read an NVS byte under a Lock-rule field, _BST's take finds the firmware holding the lock, and the census counts _BST's take alone, contended. The track's sentence on the 3 takes says where they were taken. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017cSFvbD35xJ2kGANVdm23C --- ...ine-in-acpi-mode-and-interprets-its-aml.md | 8 ++- tests/toyos.rs | 13 ++-- userland/acpiserver/src/battery.rs | 62 +++++++++++++++---- 3 files changed, 62 insertions(+), 21 deletions(-) diff --git a/issues/toyos-runs-the-machine-in-acpi-mode-and-interprets-its-aml.md b/issues/toyos-runs-the-machine-in-acpi-mode-and-interprets-its-aml.md index 3347ac77029..107b943f96d 100644 --- a/issues/toyos-runs-the-machine-in-acpi-mode-and-interprets-its-aml.md +++ b/issues/toyos-runs-the-machine-in-acpi-mode-and-interprets-its-aml.md @@ -277,9 +277,11 @@ mediated access, leaves open: where the firmware holds the lock past 1 s** (`RELEASE`, `userland/acpiserver/src/host.rs`). The bound is this server's guess, not a measurement. The firmware is meant to hold the lock for the run of one - SMI handler, and no hold has been seen. The T14's battery read took the - lock 3 times, and the firmware held it for none of them. Linux's - `ff_gbl_lock` counter there read 0 at 2 minutes of uptime, and a reading + SMI handler, and no hold has been seen. On the T14 the load takes the + lock 241 times, the namespace's `_HID` walk, the controller's `_GLK` and + `_REG` and the battery's first read together 3 times, and the battery's + and adapter's methods alone none; the firmware held it for none of them. + Linux's `ff_gbl_lock` counter there read 0 at 2 minutes of uptime, and a reading after a longer session is still owed. A Lock field and an `Acquire(\_GL, 0xFFFF)` take with no bound. An Acquire with a TimeoutValue waits that long, at most what the evaluation has left of its 10 s, and the diff --git a/tests/toyos.rs b/tests/toyos.rs index 9c6480c2bb9..7e3d9e8ec73 100644 --- a/tests/toyos.rs +++ b/tests/toyos.rs @@ -5590,11 +5590,12 @@ const T14_POWER_SOURCES: &str = "acpiserver: power sources: 1 of 1 control-metho /// The T14's battery as the server read it: the one battery its tables name /// found present once the controller's `_REG` ran, its information read, /// and at least one reading of it with the adapter's state; every method -/// answered, and the Global Lock, which the methods take through Lock-rule -/// memory fields, taken and given back as often, the takes that found the -/// firmware holding it included. The lines are printed whole: their numbers -/// are what Linux's `/sys/class/power_supply` reads on the same machine are -/// compared against, by whoever holds both. +/// answered, and every take of the Global Lock they made given back. No take +/// is demanded: the census counts the battery's and adapter's own methods, +/// which on this machine need not take the lock, and the real lock word's +/// exchange is `acpi_tables_on_metal`'s to hold. The lines are printed +/// whole: their numbers are what Linux's `/sys/class/power_supply` reads on +/// the same machine are compared against, by whoever holds both. fn battery_on_metal(back: &metal::Readback) -> Result<(), String> { back.job_passed("test_rs_acpi_hold")?; let log = back.log(); @@ -5613,7 +5614,7 @@ fn battery_on_metal(back: &metal::Readback) -> Result<(), String> { let found = log.must_say(T14_POWER_SOURCES)?; let takes = number_between(found, "; the Global Lock taken ", " times on the way")?; let given_back = number_between(found, " times on the way and given back ", " times, the firmware found")?; - if takes == 0 || given_back != takes { + if given_back != takes { return Err(format!("the battery's methods took the Global Lock {takes} times and gave it back {given_back}: {found}")); } let info = log.must_say(&format!("acpiserver: {}battery 1 of 1 from ", acpiserver_api::BATTERY_INFO))?; diff --git a/userland/acpiserver/src/battery.rs b/userland/acpiserver/src/battery.rs index ece7a699db6..66fd90112e2 100644 --- a/userland/acpiserver/src/battery.rs +++ b/userland/acpiserver/src/battery.rs @@ -9,6 +9,11 @@ //! no `_STA` has none), and its static information is read once, from //! `_BIX` where it has one and `_BIF` where it has not (§10.2.2). //! +//! The census line [`Power::find`] says counts the Global Lock's takes of +//! the batteries' and adapters' own methods alone ([`Locked`]): the +//! controller's `_REG` and the walk that named the devices ran before it, +//! and are not theirs. +//! //! **Every [`POLL`]** each battery's `_BST` and each adapter's `_PSR` are //! evaluated ([`Power::read`]), and a line is said on the first reading and //! wherever a battery's whole percent or state, or the adapter's, moved since @@ -255,6 +260,16 @@ struct Battery { said: Option<(Option, u64)>, } +/// The Global Lock as the host counted it across [`Power::find`]'s +/// evaluations: the takes, the gives back, and the takes that found the +/// firmware holding it. +#[derive(Clone, Copy, Default, PartialEq, Eq, Debug)] +struct Locked { + taken: u64, + given_back: u64, + contended: u64, +} + /// The machine's power sources, found once and read every [`POLL`]. pub struct Power { batteries: Vec, @@ -264,6 +279,8 @@ pub struct Power { refused: Ledger, /// The last evaluation found the machine stopping. stopping: bool, + /// What `find`'s evaluations took of the Global Lock. + locked: Locked, } impl Power { @@ -294,7 +311,7 @@ impl Power { /// reading, with `adapters`' (the devices [`crate::devices::find`] named /// by path); `None` where there is no battery to read. pub fn find(aml: &mut Aml<'_, K>, batteries: Vec, adapters: Vec) -> Option { - let mut power = Power { batteries: Vec::new(), adapters, ac_said: None, refused: Ledger::default(), stopping: false }; + let mut power = Power { batteries: Vec::new(), adapters, ac_said: None, refused: Ledger::default(), stopping: false, locked: Locked::default() }; let (takes, contended, given_back) = (aml.host.takes, aml.host.contended, aml.host.given_back); let count = batteries.len(); for device in batteries { @@ -342,13 +359,14 @@ impl Power { println!("{line}"); } } + power.locked = Locked { taken: aml.host.takes - takes, given_back: aml.host.given_back - given_back, contended: aml.host.contended - contended }; let found = format!( "{present} of {count} control-method batteries present, {} AC adapter(s), with {} embedded controller attached; the Global Lock taken {} times on the way and given back {} times, the firmware found holding it by {} of the takes", power.adapters.len(), if aml.host.ec.is_some() { "an" } else { "no" }, - aml.host.takes - takes, - aml.host.given_back - given_back, - aml.host.contended - contended, + power.locked.taken, + power.locked.given_back, + power.locked.contended, ); if present == 0 { println!("acpiserver: {}{found}", acpiserver_api::NO_BATTERY); @@ -422,6 +440,7 @@ pub mod tests { use crate::devices::{self, Found}; use crate::ec::tests::Emulated; use crate::host::tests::{Scripted, EC_COMMAND, EC_DATA}; + use crate::host::Take; use super::common; use common::{device, field, if_, index, int, lequal, method, op_region, package, ret, scope, skip, store, string, unit as field_unit}; @@ -437,10 +456,12 @@ pub mod tests { /// A laptop's namespace as §10.2.2, §10.3, §6.5.4 and §12.11 have one: an /// embedded controller at [`EC_DATA`] and [`EC_COMMAND`] whose `_REG` /// records that its space is there, in itself and, as the T14's writes - /// offset 0x03, in the controller; a battery below it whose `_STA` says - /// it is present only once `_REG` has run, whose `_BIX` is constant and - /// whose `_BST` selects a page of the controller's space with a write and - /// then reads it; and an adapter whose `_PSR` reads the controller. + /// offset 0x03, in the controller, and reads a byte of firmware memory + /// under the Global Lock; a battery below it whose `_STA` says it is + /// present only once `_REG` has run, whose `_BIX` is constant and whose + /// `_BST` reads that byte too, then selects a page of the controller's + /// space with a write and reads it; and an adapter whose `_PSR` reads + /// the controller. /// `glk` is the controller's `_GLK`, where it has one. fn laptop(glk: Option) -> Vec { let fields = [ @@ -491,11 +512,12 @@ pub mod tests { crs(&[EC_DATA as u16, EC_COMMAND as u16]), byte(b"_GPE", 3), op_region("NVSR", 0x00, &int(NVS), &int(1)), - field("NVSR", 0x01, &[field_unit("NVSB", 8)]), + // ByteAcc, Lock (§20.2.5.2). + field("NVSR", 0x11, &[field_unit("NVSB", 8)]), op_region("ECOR", 0x03, &int(0), &int(0x100)), field("ECOR", 0x01, &fields), common::def_name("ECOK", &int(0)), - method("_REG", 2, &if_(&lequal(&[0x68], &int(3)), &[store(&[0x69], b"ECOK"), store(&[0x69], b"REGS")].concat())), + method("_REG", 2, &if_(&lequal(&[0x68], &int(3)), &[store(b"NVSB", &[0x60]), store(&[0x69], b"ECOK"), store(&[0x69], b"REGS")].concat())), glk.map_or(Vec::new(), |glk| common::def_name("_GLK", &int(glk))), battery, ] @@ -518,8 +540,8 @@ pub mod tests { space } - /// A byte of firmware memory `_BST` reads first, as real ones read their - /// NVS. + /// A byte of firmware memory `_REG` and `_BST` read first under the + /// Global Lock, as real ones read their NVS. const NVS: u64 = 0x7700_0000; /// The fixture's machine, its controller answering at its two ports. @@ -600,6 +622,22 @@ pub mod tests { assert!(asked.iter().all(|access| access.write == 0 || [EC_DATA, EC_COMMAND].contains(&access.address)), "the kernel was asked for a write but the controller's"); } + /// The census counts the Global Lock's takes of the battery's and + /// adapter's methods alone: `_BST`'s, which finds the firmware holding + /// it and waits out its release, and not the controller's `_REG`'s, + /// made as it was attached. + #[test] + fn the_census_counts_the_global_lock_of_the_power_sources_methods_alone() { + let kernel = laptop_machine(None); + *kernel.takes.borrow_mut() = [Take::Taken, Take::Pending, Take::Taken].into(); + *kernel.releases.borrow_mut() = [true].into(); + let (aml, power) = found(&kernel); + let power = power.expect("a battery"); + assert_eq!(power.locked, Locked { taken: 1, given_back: 1, contended: 1 }); + assert_eq!((aml.host.takes, aml.host.given_back, aml.host.contended), (2, 2, 1), "the controller's _REG took no lock of its own"); + assert!(kernel.takes.borrow().is_empty() && kernel.releases.borrow().is_empty() && !kernel.held.get()); + } + /// A line is said only where what it says moved: the percent, the state /// or the adapter; find said the first. #[test] From 788521a48a57de054a81391de2a9fb79b2dd988c Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 11 Oct 2026 08:32:46 +0200 Subject: [PATCH 12/12] The census is a line find answers, a query waiting at arm runs on the attached controller, and the T14 row holds no lock count MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review of cce064f99: - The arm's order (attach and `_REG` before the GPE enable and the drain) could not fail under any test: the battery tests attach directly, and the refusal test's machine has a fixed button, where no query's method runs. A control-method-button variant of the battery fixture now has query 0x28 waiting on its controller as it is armed, and a `_Q28` that reads the controller's space. The new test asserts the controller saw `_REG`'s write, then the drain's query, then the method's read, with nothing denied. The emulated controller now raises SCI_EVT while a query waits (ACPI 6.5 §12.2.1), which the drain reads before it queries. - `Power::locked` and `Locked` shipped only for a test. `Power::find` now answers its census line for its caller to say, as `read` answers its reading lines, and the census test reads the counts off that line. - `battery_on_metal` no longer holds `given_back == takes`: an evaluation gives back the lock it holds as it ends, and on the T14 the clause read 0 against 0. The two differ where an Acquire timed out or the machine stopped mid-release, neither a defect, and where the kernel exchanges no lock word, which the row already reds on as a refusal. - `NO_BATTERY` had one reader, so its text is inline in `battery.rs`. - The track's opening says what the server evaluates now, and the `RELEASE` exit says which count the server logs. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017cSFvbD35xJ2kGANVdm23C --- ...ine-in-acpi-mode-and-interprets-its-aml.md | 9 +- tests/toyos.rs | 13 +-- userland/acpiserver-api/src/lib.rs | 3 - userland/acpiserver/src/battery.rs | 91 ++++++++++--------- userland/acpiserver/src/ec.rs | 5 +- userland/acpiserver/src/main.rs | 29 +++++- 6 files changed, 92 insertions(+), 58 deletions(-) diff --git a/issues/toyos-runs-the-machine-in-acpi-mode-and-interprets-its-aml.md b/issues/toyos-runs-the-machine-in-acpi-mode-and-interprets-its-aml.md index 107b943f96d..923a27c4d98 100644 --- a/issues/toyos-runs-the-machine-in-acpi-mode-and-interprets-its-aml.md +++ b/issues/toyos-runs-the-machine-in-acpi-mode-and-interprets-its-aml.md @@ -12,7 +12,11 @@ every CPU every 2.2 s The ACPI server loads a machine's DSDT and SSDTs, evaluates `\_S5` and hands the kernel its sleep type, which the kernel powers the machine off with and -has from nowhere else; it evaluates nothing else of their AML yet. What else +has from nowhere else. Beyond that it evaluates only the devices it serves: +the walk's `_HID`, `_STA`, `_CRS` and `_GPE`, the embedded controller's +`_GLK` and `_REG`, its query methods where the power button is a control +method device, and the batteries' and AC adapters' `_STA`, `_BIX` or +`_BIF`, `_BST` and `_PSR`. What else ToyOS takes from them it takes another way: the loader asks UEFI for the root bridges' windows `_CRS` would name (`bootloader/src/rootbridge.rs`). Nothing reads `_CST`, which names a CPU's C-states. @@ -287,7 +291,8 @@ mediated access, leaves open: long, at most what the evaluation has left of its 10 s, and the TimeoutValue is charged to it. Owner: this stage. **Exit**: a hold measured on the T14, either by a contended take - in the server's own count or by `ff_gbl_lock` after a long Linux session. + in the count the server logs, which covers the load and the power + sources' census alone, or by `ff_gbl_lock` after a long Linux session. That measurement sets the bound, or replaces it with no bound if the firmware never holds the lock. diff --git a/tests/toyos.rs b/tests/toyos.rs index 7e3d9e8ec73..802563d3940 100644 --- a/tests/toyos.rs +++ b/tests/toyos.rs @@ -5590,10 +5590,10 @@ const T14_POWER_SOURCES: &str = "acpiserver: power sources: 1 of 1 control-metho /// The T14's battery as the server read it: the one battery its tables name /// found present once the controller's `_REG` ran, its information read, /// and at least one reading of it with the adapter's state; every method -/// answered, and every take of the Global Lock they made given back. No take -/// is demanded: the census counts the battery's and adapter's own methods, -/// which on this machine need not take the lock, and the real lock word's -/// exchange is `acpi_tables_on_metal`'s to hold. The lines are printed +/// answered. The census's Global Lock counts are printed and held to +/// nothing: an evaluation gives back the lock it still holds as it ends +/// (`finish`, `userland/acpiserver/aml/src/exec.rs`), and the real lock +/// word's exchange is `acpi_tables_on_metal`'s to hold. The lines are printed /// whole: their numbers are what Linux's `/sys/class/power_supply` reads on /// the same machine are compared against, by whoever holds both. fn battery_on_metal(back: &metal::Readback) -> Result<(), String> { @@ -5612,11 +5612,6 @@ fn battery_on_metal(back: &metal::Readback) -> Result<(), String> { return Err(format!("the server refused something of this machine's AML: {refused:#?}")); } let found = log.must_say(T14_POWER_SOURCES)?; - let takes = number_between(found, "; the Global Lock taken ", " times on the way")?; - let given_back = number_between(found, " times on the way and given back ", " times, the firmware found")?; - if given_back != takes { - return Err(format!("the battery's methods took the Global Lock {takes} times and gave it back {given_back}: {found}")); - } let info = log.must_say(&format!("acpiserver: {}battery 1 of 1 from ", acpiserver_api::BATTERY_INFO))?; let read: Vec<&&str> = lines.iter().filter(|l| l.contains(acpiserver_api::BATTERY_READ)).collect(); if read.is_empty() { diff --git a/userland/acpiserver-api/src/lib.rs b/userland/acpiserver-api/src/lib.rs index d37b5c05a00..fb09b38c7e0 100644 --- a/userland/acpiserver-api/src/lib.rs +++ b/userland/acpiserver-api/src/lib.rs @@ -23,6 +23,3 @@ pub const BATTERY_INFO: &str = "battery information: "; /// remaining and full capacity, its state, rate, voltage and the power it /// moves in mW, then the adapters' state. pub const BATTERY_READ: &str = "battery read: "; - -/// No battery is read on this machine, and why. -pub const NO_BATTERY: &str = "no battery: "; diff --git a/userland/acpiserver/src/battery.rs b/userland/acpiserver/src/battery.rs index 66fd90112e2..c1552048b6f 100644 --- a/userland/acpiserver/src/battery.rs +++ b/userland/acpiserver/src/battery.rs @@ -9,10 +9,9 @@ //! no `_STA` has none), and its static information is read once, from //! `_BIX` where it has one and `_BIF` where it has not (§10.2.2). //! -//! The census line [`Power::find`] says counts the Global Lock's takes of -//! the batteries' and adapters' own methods alone ([`Locked`]): the -//! controller's `_REG` and the walk that named the devices ran before it, -//! and are not theirs. +//! The census line [`Power::find`] answers counts the Global Lock's takes of +//! the batteries' and adapters' own methods alone: the controller's `_REG` +//! and the walk that named the devices ran before it, and are not theirs. //! //! **Every [`POLL`]** each battery's `_BST` and each adapter's `_PSR` are //! evaluated ([`Power::read`]), and a line is said on the first reading and @@ -260,16 +259,6 @@ struct Battery { said: Option<(Option, u64)>, } -/// The Global Lock as the host counted it across [`Power::find`]'s -/// evaluations: the takes, the gives back, and the takes that found the -/// firmware holding it. -#[derive(Clone, Copy, Default, PartialEq, Eq, Debug)] -struct Locked { - taken: u64, - given_back: u64, - contended: u64, -} - /// The machine's power sources, found once and read every [`POLL`]. pub struct Power { batteries: Vec, @@ -279,8 +268,6 @@ pub struct Power { refused: Ledger, /// The last evaluation found the machine stopping. stopping: bool, - /// What `find`'s evaluations took of the Global Lock. - locked: Locked, } impl Power { @@ -309,9 +296,10 @@ impl Power { /// Keep each of `batteries` present, read its information and first /// reading, with `adapters`' (the devices [`crate::devices::find`] named - /// by path); `None` where there is no battery to read. - pub fn find(aml: &mut Aml<'_, K>, batteries: Vec, adapters: Vec) -> Option { - let mut power = Power { batteries: Vec::new(), adapters, ac_said: None, refused: Ledger::default(), stopping: false, locked: Locked::default() }; + /// by path); `None` where there is no battery to read, and none on a + /// stopping machine. The census line beside it is its caller's to say. + pub fn find(aml: &mut Aml<'_, K>, batteries: Vec, adapters: Vec) -> (Option, Option) { + let mut power = Power { batteries: Vec::new(), adapters, ac_said: None, refused: Ledger::default(), stopping: false }; let (takes, contended, given_back) = (aml.host.takes, aml.host.contended, aml.host.given_back); let count = batteries.len(); for device in batteries { @@ -346,7 +334,7 @@ impl Power { } } if power.stopping { - return None; + return (None, None); } let present = power.batteries.len(); @@ -359,21 +347,19 @@ impl Power { println!("{line}"); } } - power.locked = Locked { taken: aml.host.takes - takes, given_back: aml.host.given_back - given_back, contended: aml.host.contended - contended }; let found = format!( "{present} of {count} control-method batteries present, {} AC adapter(s), with {} embedded controller attached; the Global Lock taken {} times on the way and given back {} times, the firmware found holding it by {} of the takes", power.adapters.len(), if aml.host.ec.is_some() { "an" } else { "no" }, - power.locked.taken, - power.locked.given_back, - power.locked.contended, + aml.host.takes - takes, + aml.host.given_back - given_back, + aml.host.contended - contended, ); if present == 0 { - println!("acpiserver: {}{found}", acpiserver_api::NO_BATTERY); - return None; + return (None, Some(format!("acpiserver: no battery: {found}"))); } - println!("acpiserver: power sources: {found}, the first reading's included; each read every {} s", POLL.as_secs()); - Some(power) + let line = format!("acpiserver: power sources: {found}, the first reading's included; each read every {} s", POLL.as_secs()); + (Some(power), Some(line)) } /// Read every battery and adapter: the line of each battery whose @@ -446,9 +432,9 @@ pub mod tests { use common::{device, field, if_, index, int, lequal, method, op_region, package, ret, scope, skip, store, string, unit as field_unit}; /// The controller's space as the fixture's fields lay it out. - const CONNECTED: u8 = 0x03; + pub const CONNECTED: u8 = 0x03; const STATE: u8 = 0x38; - const AC: u8 = 0x46; + pub const AC: u8 = 0x46; const PAGE: u8 = 0x81; const RATE: u8 = 0xA0; const VOLTAGE: u8 = 0xA8; @@ -462,8 +448,10 @@ pub mod tests { /// `_BST` reads that byte too, then selects a page of the controller's /// space with a write and reads it; and an adapter whose `_PSR` reads /// the controller. - /// `glk` is the controller's `_GLK`, where it has one. - fn laptop(glk: Option) -> Vec { + /// `glk` is the controller's `_GLK`, where it has one; with a `button` + /// the machine's power button is a control method device (§4.8.2.2.1.2) + /// and the controller's query 0x28's method reads the adapter's byte. + fn laptop(glk: Option, button: bool) -> Vec { let fields = [ skip(8 * usize::from(CONNECTED)), field_unit("REGS", 8), @@ -519,12 +507,14 @@ pub mod tests { common::def_name("ECOK", &int(0)), method("_REG", 2, &if_(&lequal(&[0x68], &int(3)), &[store(b"NVSB", &[0x60]), store(&[0x69], b"ECOK"), store(&[0x69], b"REGS")].concat())), glk.map_or(Vec::new(), |glk| common::def_name("_GLK", &int(glk))), + if button { method("_Q28", 0, &store(b"ACST", &common::local(0))) } else { Vec::new() }, battery, ] .concat(), ); let adapter = device("AC", &[common::def_name("_HID", &string("ACPI0003")), method("_PSR", 0, &ret(&common::name("\\_SB.EC0.ACST")))].concat()); - sealed(b"DSDT", &scope("\\_SB", &[controller, adapter].concat())) + let button = if button { device("PWRB", &common::def_name("_HID", &int(0x0C0C_D041))) } else { Vec::new() }; + sealed(b"DSDT", &scope("\\_SB", &[controller, adapter, button].concat())) } /// What the fixture's controller holds: discharging at 6210 mW with @@ -546,7 +536,19 @@ pub mod tests { /// The fixture's machine, its controller answering at its two ports. pub fn laptop_machine(glk: Option) -> Scripted { - let mut kernel = crafted(&laptop(glk), &[]); + machine(laptop(glk, false)) + } + + /// The fixture's machine with a control-method power button, and query + /// 0x28 waiting on its controller. + pub fn button_laptop_machine() -> Scripted { + let kernel = machine(laptop(None, true)); + kernel.controller.borrow_mut().as_mut().expect("a controller").queries.push(0x28); + kernel + } + + fn machine(dsdt: Vec) -> Scripted { + let mut kernel = crafted(&dsdt, &[]); kernel.memory.push((NVS, 10, vec![0])); *kernel.controller.borrow_mut() = Some(Emulated::new(space())); kernel @@ -563,11 +565,16 @@ pub mod tests { } fn found(kernel: &Scripted) -> (Aml<'_, Scripted>, Option) { - let (mut aml, found) = armed(kernel); - let power = Power::find(&mut aml, found.batteries, found.adapters); + let (aml, power, _) = census(kernel); (aml, power) } + fn census(kernel: &Scripted) -> (Aml<'_, Scripted>, Option, Option) { + let (mut aml, found) = armed(kernel); + let (power, census) = Power::find(&mut aml, found.batteries, found.adapters); + (aml, power, census) + } + fn done(kernel: &Scripted) -> Vec<(u8, u8, u8)> { kernel.controller.borrow().as_ref().expect("a controller").done.clone() } @@ -578,8 +585,8 @@ pub mod tests { ec.space[usize::from(at)..usize::from(at) + bytes.len()].copy_from_slice(bytes); } - const RD: u8 = 0x80; - const WR: u8 = 0x81; + pub const RD: u8 = 0x80; + pub const WR: u8 = 0x81; /// The controller is attached before its `_REG` runs, which writes it, /// and told its space is there before the battery is asked whether it @@ -631,9 +638,11 @@ pub mod tests { let kernel = laptop_machine(None); *kernel.takes.borrow_mut() = [Take::Taken, Take::Pending, Take::Taken].into(); *kernel.releases.borrow_mut() = [true].into(); - let (aml, power) = found(&kernel); - let power = power.expect("a battery"); - assert_eq!(power.locked, Locked { taken: 1, given_back: 1, contended: 1 }); + let (aml, power, census) = census(&kernel); + assert!(power.is_some(), "a battery"); + let census = census.expect("a census"); + let counted = "the Global Lock taken 1 times on the way and given back 1 times, the firmware found holding it by 1 of the takes,"; + assert!(census.contains(counted), "{census}"); assert_eq!((aml.host.takes, aml.host.given_back, aml.host.contended), (2, 2, 1), "the controller's _REG took no lock of its own"); assert!(kernel.takes.borrow().is_empty() && kernel.releases.borrow().is_empty() && !kernel.held.get()); } diff --git a/userland/acpiserver/src/ec.rs b/userland/acpiserver/src/ec.rs index cdcd368cc71..d84019968e8 100644 --- a/userland/acpiserver/src/ec.rs +++ b/userland/acpiserver/src/ec.rs @@ -136,7 +136,8 @@ pub mod tests { } } - /// A controller as §12.2 and §12.3 have it: 256 bytes of space; a byte + /// A controller as §12.2 and §12.3 have it: 256 bytes of space, and + /// SCI_EVT set while one of its `queries` waits; a byte /// written fills the input buffer until the controller takes it, which /// it does `slow` status reads later; a command's answer fills the output /// buffer as it takes the command's last byte. It refuses, by panic, @@ -170,7 +171,7 @@ pub mod tests { self.pending -= 1; } } - u8::from(self.output.is_some()) * OBF | u8::from(self.input) * IBF + u8::from(self.output.is_some()) * OBF | u8::from(self.input) * IBF | u8::from(!self.queries.is_empty()) * SCI_EVT } fn take(&mut self) { diff --git a/userland/acpiserver/src/main.rs b/userland/acpiserver/src/main.rs index 8fdd907ce05..4860aeecf0a 100644 --- a/userland/acpiserver/src/main.rs +++ b/userland/acpiserver/src/main.rs @@ -356,7 +356,11 @@ impl<'a, K: Kernel, P: Ports> Server<'a, K, P> { /// once the controller is armed ([`Power::find`]). fn find_power(&mut self, batteries: Vec, adapters: Vec) { let Some(aml) = &mut self.aml else { return }; - self.power = Power::find(aml, batteries, adapters); + let census; + (self.power, census) = Power::find(aml, batteries, adapters); + if let Some(census) = census { + println!("{census}"); + } self.ended(); } @@ -700,6 +704,29 @@ mod tests { } } + /// A query waiting as the controller is armed, on a machine whose power + /// button is a control method device, is drained and its method run only + /// once the controller is attached to AML and its `_REG` run: the + /// method's read of the controller's space reaches the controller, and + /// is not denied as one no controller is attached for. + #[test] + fn a_query_waiting_at_arm_runs_its_method_on_the_attached_controller() { + use crate::battery::tests::{button_laptop_machine, AC, CONNECTED, RD, WR}; + let kernel = button_laptop_machine(); + let info = AcpiInfo { rsdp: CRAFTED_RSDP, pm1_event: Block::NONE, gpe0: GPE0, flags: 0, reserved: [0; 3] }; + let mut server = Server::new(&kernel, Bytes::default(), info); + for (_, enable) in bytes(GPE0) { + server.ports.out8(enable, 0); + } + server.aml = load(&kernel, CRAFTED_RSDP).1; + server.arm_controller(); + assert_eq!(server.counts, BTreeMap::from([(0x28, 1)]), "the query waiting was not drained"); + let refused = &server.aml.as_ref().expect("the namespace").host.refused; + assert!(refused.is_empty(), "{}", refused.counts()); + let done = kernel.controller.borrow().as_ref().expect("a controller").done.clone(); + assert_eq!(done, [(WR, CONNECTED, 1), (0x84, 0, 0), (RD, AC, 0)], "_REG's write, the drain's query, the method's read"); + } + /// The owner's ruling: a machine whose power button is a control method /// device and whose namespace was not kept stays in ACPI mode with the /// button dead. The server serves neither the button nor the controller,