Repository navigation
acpiserver reads the battery and AC through AML, on the namespace-found controller, and waits out the firmware's Global Lock - #838
Conversation
… AML and its embedded controller 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017cSFvbD35xJ2kGANVdm23C
… 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017cSFvbD35xJ2kGANVdm23C
|
guest part 1 of 1 ( |
|
ci-host part 1 of 9 ( |
|
ci-host part 2 of 9 ( |
|
ci-host part 3 of 9 ( |
|
ci-host part 4 of 9 ( |
|
ci-host part 5 of 9 ( |
|
ci-host part 6 of 9 ( |
|
ci-host part 7 of 9 ( |
|
ci-host part 8 of 9 ( |
|
ci-host part 9 of 9 ( |
|
guest-pb part 1 of 1 ( |
|
build-only part 1 of 1 ( |
|
metal-stage part 1 of 1 ( |
|
Mutations, each applied with
m10 stayed green on 0aeeffb (the tests read only what the last line said); 6465a1d makes
diff --git a/userland/acpiserver/src/ec.rs b/userland/acpiserver/src/ec.rs
index cdcd368cc..cbb74ac4b 100644
--- a/userland/acpiserver/src/ec.rs
+++ b/userland/acpiserver/src/ec.rs
@@ -88,7 +88,7 @@ impl Transaction {
/// What to do, given what the status register reads now.
pub fn step(&mut self, status: u8) -> Do {
match self.phase {
- Phase::Send(_) | Phase::Settle if status & IBF != 0 => Do::Wait(Wait::InputEmpty),
+ Phase::Send(0) | 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),
diff --git a/userland/acpiserver/src/battery.rs b/userland/acpiserver/src/battery.rs
index 14879371f..95757df5d 100644
--- a/userland/acpiserver/src/battery.rs
+++ b/userland/acpiserver/src/battery.rs
@@ -484,7 +484,7 @@ impl Power {
};
let battery = &mut self.batteries[n];
let now = (percent(&battery.info, &status), status.state);
- if battery.said != Some(now) || ac_moved {
+ if true {
battery.said = Some(now);
lines.push(format!("acpiserver: {}battery {} of {count}: {}; {ac}", acpiserver_api::BATTERY_READ, n + 1, said_status(&battery.info, &status)));
}
diff --git a/userland/acpiserver/src/battery.rs b/userland/acpiserver/src/battery.rs
index 14879371f..2e7fcc994 100644
--- a/userland/acpiserver/src/battery.rs
+++ b/userland/acpiserver/src/battery.rs
@@ -309,9 +309,6 @@ impl Power {
/// 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}");
@@ -454,8 +451,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 lines,
- Err(why) => self.refused(&device, "_PSR", &kind(&why)),
+ Err(why) => self.refused(&device, "_PSR", &kind(&why)),
}
}
let ac_moved = self.ac_said != Some(ac);
@@ -472,8 +468,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 lines,
- Err(why) => Err(kind(&why)),
+ Err(why) => Err(kind(&why)),
};
let status = match status {
Ok(status) => status,
diff --git a/userland/acpiserver/src/ec.rs b/userland/acpiserver/src/ec.rs
index cdcd368cc..797141408 100644
--- a/userland/acpiserver/src/ec.rs
+++ b/userland/acpiserver/src/ec.rs
@@ -93,7 +93,7 @@ impl Transaction {
self.phase = match i + 1 {
next if next < self.len => Phase::Send(next),
_ if self.answers => Phase::Answer,
- _ => Phase::Settle,
+ _ => Phase::Done(self.bytes[self.len - 1]),
};
if i == 0 { Do::WriteCommand(self.bytes[0]) } else { Do::WriteData(self.bytes[i]) }
}
diff --git a/userland/acpiserver/src/host.rs b/userland/acpiserver/src/host.rs
index 0174bf437..c79201328 100644
--- a/userland/acpiserver/src/host.rs
+++ b/userland/acpiserver/src/host.rs
@@ -268,7 +268,7 @@ impl<K: Kernel, C: Controller> Host for Firmware<'_, K, C> {
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));
+ return Err(Denied(format!("a write to {NO_CONTROLLER} {value}")));
self.ec_writes += 1;
return Ok(());
}
diff --git a/userland/acpiserver/src/battery.rs b/userland/acpiserver/src/battery.rs
index bf24bdc72..e047a11a0 100644
--- a/userland/acpiserver/src/battery.rs
+++ b/userland/acpiserver/src/battery.rs
@@ -377,7 +377,7 @@ impl Power {
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)]) {
+ if let Err(why) = power.evaluate(aml, &format!("{device}._REG"), &[Value::Integer(3), Value::Integer(0)]) {
power.refused(device, "_REG", &kind(&why));
}
}
diff --git a/userland/acpiserver/src/battery.rs b/userland/acpiserver/src/battery.rs
index bf24bdc72..ecc1061b6 100644
--- a/userland/acpiserver/src/battery.rs
+++ b/userland/acpiserver/src/battery.rs
@@ -365,8 +365,8 @@ impl Power {
if controller {
for device in &controllers {
match power.evaluate(aml, &format!("{device}._GLK"), &[]) {
- Ok(None | Some(Value::Integer(0))) => {}
- Ok(Some(_)) => {
+ Ok(_) => {}
+ Ok(Some(Value::Buffer(_))) => {
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
diff --git a/userland/acpiserver/src/battery.rs b/userland/acpiserver/src/battery.rs
index bf24bdc72..480bef668 100644
--- a/userland/acpiserver/src/battery.rs
+++ b/userland/acpiserver/src/battery.rs
@@ -165,7 +165,7 @@ pub fn bix(value: &Value) -> Result<Info, String> {
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")?,
+ cycles: dword(elements, 9, "Cycle Count")?,
})
}
diff --git a/userland/acpiserver/src/battery.rs b/userland/acpiserver/src/battery.rs
index bf24bdc72..b47e176a1 100644
--- a/userland/acpiserver/src/battery.rs
+++ b/userland/acpiserver/src/battery.rs
@@ -189,7 +189,7 @@ pub fn bst(value: &Value) -> Result<Status, String> {
/// The charge in whole percent of the last full charge, rounded down.
pub fn percent(info: &Info, status: &Status) -> Option<u64> {
- let full = info.full.filter(|&full| full > 0)?;
+ let full = info.design.filter(|&full| full > 0)?;
Some(u64::from(status.remaining?) * 100 / u64::from(full))
}
diff --git a/userland/acpiserver/src/battery.rs b/userland/acpiserver/src/battery.rs
index bf24bdc72..bc43f3b42 100644
--- a/userland/acpiserver/src/battery.rs
+++ b/userland/acpiserver/src/battery.rs
@@ -199,7 +199,7 @@ pub fn milliwatts(info: &Info, status: &Status) -> Option<u64> {
let rate = u64::from(status.rate?);
match info.unit {
Unit::Power => Some(rate),
- Unit::Current => Some(rate * u64::from(status.voltage?) / 1000),
+ Unit::Current => Some(rate * u64::from(status.voltage.or(info.design_voltage)?) / 1000),
}
}
diff --git a/userland/acpiserver/src/battery.rs b/userland/acpiserver/src/battery.rs
index bf24bdc72..aeb833ff4 100644
--- a/userland/acpiserver/src/battery.rs
+++ b/userland/acpiserver/src/battery.rs
@@ -116,8 +116,7 @@ pub struct Status {
/// [`UNKNOWN`] read as `None`.
fn dword(elements: &[Value], at: usize, what: &str) -> Result<Option<u32>, 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(&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}")),
} |
|
mutation log m1-ec-no-ibf-wait ( mutation log m10-line-every-poll ( mutation log m11-stopping-refused ( mutation log m2-ec-no-settle ( mutation log m3-host-denies-ec-write ( mutation log m4-reg-disconnect ( mutation log m5-glk-ignored ( |
|
before_the_command_is_refused - should panic ... ok failures: ---- battery::tests::a_controller_that_wants_the_global_lock_has_no_battery_read stdout ---- thread 'battery::tests::a_controller_that_wants_the_global_lock_has_no_battery_read' (254950433) panicked at userland/acpiserver/src/battery.rs:710:13: failures: test result: FAILED. 48 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s error: test failed, to rerun pass Compiling acpiserver v0.1.0 (/Users/jan/Dev/jan/toyos-battery/userland/acpiserver) running 49 tests failures: ---- battery::tests::a_bix_of_either_revision_is_read_at_the_same_places stdout ---- thread 'battery::tests::a_bix_of_either_revision_is_read_at_the_same_places' (254950699) panicked at userland/acpiserver/src/battery.rs:802:13: ---- battery::tests::an_unknown_dword_is_unknown_and_never_a_number stdout ---- thread 'battery::tests::an_unknown_dword_is_unknown_and_never_a_number' (254950711) panicked at userland/acpiserver/src/battery.rs:815:9: ---- battery::tests::a_laptops_battery_is_found_after_reg_and_read_through_the_controller stdout ---- thread 'battery::tests::a_laptops_battery_is_found_after_reg_and_read_through_the_controller' (254950704) panicked at userland/acpiserver/src/battery.rs:633:9: failures: test result: FAILED. 46 passed; 3 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s error: test failed, to rerun pass Compiling acpiserver v0.1.0 (/Users/jan/Dev/jan/toyos-battery/userland/acpiserver) running 49 tests failures: ---- battery::tests::a_bst_reads_its_four_dwords_and_names_its_state stdout ---- thread 'battery::tests::a_bst_reads_its_four_dwords_and_names_its_state' (254951061) panicked at userland/acpiserver/src/battery.rs:826:9: ---- battery::tests::a_full_charge_of_zero_is_no_percent stdout ---- thread 'battery::tests::a_full_charge_of_zero_is_no_percent' (254951064) panicked at userland/acpiserver/src/battery.rs:854:9: ---- battery::tests::a_current_is_power_only_at_its_present_voltage stdout ---- thread 'battery::tests::a_current_is_power_only_at_its_present_voltage' (254951063) panicked at userland/acpiserver/src/battery.rs:844:9: ---- battery::tests::a_laptops_battery_is_found_after_reg_and_read_through_the_controller stdout ---- thread 'battery::tests::a_laptops_battery_is_found_after_reg_and_read_through_the_controller' (254951065) panicked at userland/acpiserver/src/battery.rs:651:9: ---- battery::tests::a_reading_is_said_only_where_its_percent_its_state_or_the_adapter_moves stdout ---- thread 'battery::tests::a_reading_is_said_only_where_its_percent_its_state_or_the_adapter_moves' (254951068) panicked at userland/acpiserver/src/battery.rs:673:9: failures: test result: FAILED. 44 passed; 5 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s error: test failed, to rerun pass Compiling acpiserver v0.1.0 (/Users/jan/Dev/jan/toyos-battery/userland/acpiserver) running 49 tests failures: ---- battery::tests::a_current_is_power_only_at_its_present_voltage stdout ---- thread 'battery::tests::a_current_is_power_only_at_its_present_voltage' (254951349) panicked at userland/acpiserver/src/battery.rs:847:9: failures: test result: FAILED. 48 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s error: test failed, to rerun pass Compiling acpiserver v0.1.0 (/Users/jan/Dev/jan/toyos-battery/userland/acpiserver) running 49 tests failures: ---- battery::tests::an_unknown_dword_is_unknown_and_never_a_number stdout ---- thread 'battery::tests::an_unknown_dword_is_unknown_and_never_a_number' (254951635) panicked at userland/acpiserver/src/battery.rs:814:9: failures: test result: FAILED. 48 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s error: test failed, to rerun pass |
|
The out-of-tree T14 check (
[package]
name = "t14check"
version = "0.0.0"
edition = "2024"
publish = false
[workspace]
[dependencies]
toyos-aml = { path = "/Users/jan/Dev/jan/toyos-battery/userland/acpiserver/aml" }
toyos-acpi = { path = "/Users/jan/Dev/jan/toyos-battery/toyos-acpi" }
//! Out of the tree: a machine's DSDT and SSDTs loaded, its embedded
//! controller's `_REG(3, 1)` run, and its batteries' `_STA`, `_BIX`/`_BIF`,
//! `_BST` and its adapters' `_PSR` evaluated, against a host that answers the
//! controller's space from a 256-byte dump (Linux: `ec_sys`,
//! `/sys/kernel/debug/ec/ec0/io`), named memory words from the command line,
//! and zero for every other read. Every access the battery's methods make is
//! printed, and every take of the Global Lock.
//!
//! `t14check <dir of DSDT and SSDT files> <ec dump> [ADDR:BYTES:VALUE]...`
use std::collections::BTreeMap;
use toyos_acpi::{Phys, Table};
use toyos_aml::{Access, Address, Denied, Host, Interpreter, Value};
#[derive(Clone, Copy)]
struct Image<'a>(&'a [u8]);
impl Phys for Image<'_> {
fn readable(self, phys: u64, len: usize) -> bool {
usize::try_from(phys).ok().and_then(|p| p.checked_add(len)).is_some_and(|e| e <= self.0.len())
}
fn byte(self, phys: u64) -> u8 {
self.0[phys as usize]
}
}
struct Machine {
ec: [u8; 256],
mem: BTreeMap<u64, u8>,
log: Vec<String>,
takes: u64,
slept_ms: u64,
}
fn bytes(w: Access) -> u64 {
match w {
Access::Byte => 1,
Access::Word => 2,
Access::DWord => 4,
Access::QWord => 8,
}
}
impl Host for Machine {
fn read(&mut self, at: Address, w: Access) -> Result<u64, Denied> {
let v = match at {
Address::EmbeddedControl(a) => u64::from(self.ec[usize::from(a)]),
Address::Memory(a) => (0..bytes(w)).fold(0, |v, i| v | u64::from(*self.mem.get(&(a + i)).unwrap_or(&0)) << (8 * i)),
Address::Io(_) => 0,
Address::PciConfig { .. } => u64::MAX >> (64 - 8 * bytes(w)),
};
self.log.push(format!("read {at:x?} {w:?} = {v:#x}"));
Ok(v)
}
fn write(&mut self, at: Address, w: Access, v: u64) -> Result<(), Denied> {
self.log.push(format!("write {at:x?} {w:?} {v:#x}"));
match at {
Address::EmbeddedControl(a) => self.ec[usize::from(a)] = v as u8,
Address::Memory(a) => {
for i in 0..bytes(w) {
self.mem.insert(a + i, (v >> (8 * i)) as u8);
}
}
_ => {}
}
Ok(())
}
fn sleep(&mut self, ms: u64) {
self.slept_ms += ms;
}
fn stall(&mut self, _: u64) {}
fn timer(&mut self) -> u64 {
0
}
fn notify(&mut self, o: &str, v: u64) {
self.log.push(format!("notify {o} {v:#x}"));
}
fn global_lock(&mut self, take: bool) -> Result<(), Denied> {
if take {
self.takes += 1;
}
self.log.push(format!("global lock {}", if take { "take" } else { "release" }));
Ok(())
}
}
fn hid(v: &Value) -> Option<String> {
match v {
Value::String(t) => String::from_utf8(t.clone()).ok(),
&Value::Integer(id) => {
let id = u32::try_from(id).ok()?.swap_bytes();
let l = |at: u32| char::from(b'@' + ((id >> at) & 0x1F) as u8);
Some(format!("{}{}{}{:04X}", l(26), l(21), l(16), id & 0xFFFF))
}
_ => None,
}
}
fn main() {
let args: Vec<String> = std::env::args().collect();
let (dir, dump) = (&args[1], &args[2]);
let mut ec = [0u8; 256];
ec.copy_from_slice(&std::fs::read(dump).expect("the dump")[..256]);
let mut mem = BTreeMap::new();
for word in &args[3..] {
let parts: Vec<u64> = word.split(':').map(|p| u64::from_str_radix(p.trim_start_matches("0x"), 16).expect("hex")).collect();
for i in 0..parts[1] {
mem.insert(parts[0] + i, (parts[2] >> (8 * i)) as u8);
}
}
let mut m = Machine { ec, mem, log: Vec::new(), takes: 0, slept_ms: 0 };
let mut names: Vec<_> = std::fs::read_dir(dir).expect("the dir").map(|e| e.expect("entry").path()).collect();
names.sort_by_key(|p| (!p.file_name().unwrap().to_string_lossy().to_uppercase().starts_with("DSDT"), p.clone()));
let mut i = Interpreter::new();
for p in &names {
let t = std::fs::read(p).expect("a table");
let sig: [u8; 4] = t[..4].try_into().unwrap();
if &sig != b"DSDT" && &sig != b"SSDT" {
continue;
}
let table = Table::open(Image(&t), 0, &sig, 0).expect("the table opens");
println!("load {}: {:?}", p.display(), i.load(&mut m, &table).map_err(|e| format!("{e:?}")));
}
m.log.clear();
let mut hids = Vec::new();
{
let mut walk = i.walk().expect("a walk");
while let Some(e) = walk.next() {
if e.name == *b"_HID" {
hids.push(walk.path().strip_suffix("._HID").unwrap().to_owned());
}
}
}
let mut found: BTreeMap<String, Vec<String>> = BTreeMap::new();
for d in hids {
if let Ok(v) = i.evaluate(&mut m, &format!("{d}._HID"), &[]) {
if let Some(h) = hid(&v).filter(|h| ["PNP0C09", "PNP0C0A", "ACPI0003"].contains(&h.as_str())) {
found.entry(h).or_default().push(d);
}
}
}
println!("found {found:?}");
let mut run = |m: &mut Machine, i: &mut Interpreter, path: String, args: &[Value]| {
m.log.clear();
let (takes, slept) = (m.takes, m.slept_ms);
let r = i.evaluate(m, &path, args);
println!("{path}: {r:?}");
println!(" steps {:?}; Global Lock taken {}; slept {} ms", i.usage(), m.takes - takes, m.slept_ms - slept);
for line in &m.log {
println!(" {line}");
}
};
for ec in found.get("PNP0C09").cloned().unwrap_or_default() {
run(&mut m, &mut i, format!("{ec}._GLK"), &[]);
run(&mut m, &mut i, format!("{ec}._REG"), &[Value::Integer(3), Value::Integer(1)]);
}
for bat in found.get("PNP0C0A").cloned().unwrap_or_default() {
for method in ["_STA", "_BIX", "_BIF", "_BST", "_BST"] {
run(&mut m, &mut i, format!("{bat}.{method}"), &[]);
}
}
for ac in found.get("ACPI0003").cloned().unwrap_or_default() {
run(&mut m, &mut i, format!("{ac}._PSR"), &[]);
}
} |
|
T14 reading at The judge ( The values themselves agree with Linux on the same machine. The orchestrator read Linux's
|
…nd an Acquire of \_GL waits as long as its timeout says The T14's battery methods take the Global Lock through Lock-rule memory fields: the orchestrator's T14 reading at 6465a1d 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017cSFvbD35xJ2kGANVdm23C
|
ci-host part 1 of 9 ( |
|
ci-host part 2 of 9 ( |
|
ci-host part 3 of 9 ( |
|
ci-host part 4 of 9 ( |
|
ci-host part 5 of 9 ( |
|
ci-host part 6 of 9 ( |
|
ci-host part 7 of 9 ( |
|
ci-host part 8 of 9 ( |
… attached controller, and the T14 row holds no lock count Review of cce064f: - 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017cSFvbD35xJ2kGANVdm23C
|
Mutations at
o1-attach-after-the-drain.patch diff --git a/userland/acpiserver/src/main.rs b/userland/acpiserver/src/main.rs
index 4860aeecf..52b06698d 100644
--- a/userland/acpiserver/src/main.rs
+++ b/userland/acpiserver/src/main.rs
@@ -333,13 +333,6 @@ impl<'a, K: Kernel, P: Ports> Server<'a, K, P> {
}
Ok(_) => {}
}
- if let Err(why) = attach(aml, &ec) {
- println!("{CONTROLLER_NONE}{why}");
- return sources;
- }
- if aml.host.stopping {
- return sources;
- }
let n = ec.gpe;
let (status, enable) = bytes(self.info.gpe0).nth(usize::from(n / 8)).expect("the controller's GPE was bounded by the block");
self.ports.out8(status, 1 << (n % 8));
@@ -349,6 +342,14 @@ impl<'a, K: Kernel, P: Ports> Server<'a, K, P> {
self.ec = Some(ec);
self.drain();
self.run_queued();
+ let (Some(aml), Some(ec)) = (&mut self.aml, &self.ec) else { return sources };
+ if let Err(why) = attach(aml, ec) {
+ println!("{CONTROLLER_NONE}{why}");
+ return sources;
+ }
+ if aml.host.stopping {
+ return sources;
+ }
sources
}
n1-census-counts-the-arm.patch diff --git a/userland/acpiserver/src/battery.rs b/userland/acpiserver/src/battery.rs
index c1552048b..0366b5c10 100644
--- a/userland/acpiserver/src/battery.rs
+++ b/userland/acpiserver/src/battery.rs
@@ -351,9 +351,9 @@ impl Power {
"{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,
+ aml.host.takes,
+ aml.host.given_back,
+ aml.host.contended,
);
if present == 0 {
return (None, Some(format!("acpiserver: no battery: {found}")));n2-census-before-the-first-reading.patch diff --git a/userland/acpiserver/src/battery.rs b/userland/acpiserver/src/battery.rs
index c1552048b..802828feb 100644
--- a/userland/acpiserver/src/battery.rs
+++ b/userland/acpiserver/src/battery.rs
@@ -342,11 +342,6 @@ impl Power {
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 {
- for line in power.read(aml) {
- println!("{line}");
- }
- }
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(),
@@ -355,6 +350,11 @@ impl Power {
aml.host.given_back - given_back,
aml.host.contended - contended,
);
+ if present > 0 {
+ for line in power.read(aml) {
+ println!("{line}");
+ }
+ }
if present == 0 {
return (None, Some(format!("acpiserver: no battery: {found}")));
}mutate.sh #!/bin/sh
# Each named mutation: a checked patch applied, `cargo test -p acpiserver` run,
# the patch reverted and the tree checked clean. Run from the worktree's root.
S=$1; shift
for name in "$@"; do
p=$S/mut/$name.patch
git apply --check -p1 $p || { echo "$name: does not apply"; exit 2; }
git apply -p1 $p
cargo test -p acpiserver > $S/logs/$name.log 2>&1
echo "$name EXIT=$?"
git apply -R -p1 $p
[ -z "$(git status --porcelain --ignore-submodules=none)" ] || { echo "$name: tree not clean after revert"; exit 3; }
done |
|
Mutation o1 (attach after the drain) applied to |
|
Mutation o1 (attach after the drain) at |
|
Mutation n1 (census counts from the server's start) at |
|
Mutation n2 (census taken before the first reading) at |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
T14 reading at |
|
Review of #838 at 788521a (base bb3344d), round 2. Since cce064f the branch changed 6 files, +92 −58. The body states production as +19 −32. Whole branch: 20 files, +2234 −216. The shrink is accepted, and the growth was accepted last round. Earlier BLOCKERs
Earlier NOTEs. All are closed except one item in the body:
BLOCKER NOTE
LAND |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017cSFvbD35xJ2kGANVdm23C
/system/bin/acpiserverreads the machine's batteries and AC adapters through its own AML interpreter and the embedded controller, and logs them. This is the battery the track's Stage: the interpreter names ("the battery first"; its exit includes "a T14 row reads the battery's state as the interpreter evaluated it beside Linux's reading of the same machine"), inissues/toyos-runs-the-machine-in-acpi-mode-and-interprets-its-aml.md. The machine's AML takes the firmware's Global Lock (the T14's tables 241 times as they load), so a take that finds the firmware holding it waits for the release (ACPI 6.5 §5.2.10.1) instead of being denied.This head, 788521a, answers the review of cce064f (review). The branch is reworked onto #845's design (main at bb3344d). #845 removed the ECDT controller this branch had read the battery through. Main now finds the controller in the namespace (
devices::find), reaches its ports through the kernel's mediatedport_in/port_out(which can answer stopping or refused), and runs query methods. It also denied EC space to AML. A merge could not reconcile the two designs, so the branch was rebuilt on main's.The T14 on this design
battery_read: EXIT=0, 1 passed (orchestrator's judge). The census reads 1 of 1 batteries present, 1 adapter, the controller attached, and the lock taken 0 times. A reading: 99%, 45680 of 45980 mWh, neither charging nor discharging, 12305 mV, AC online.acpi_server_eventsandacpi_tables_loaded: EXIT=0, 2 passed (orchestrator's judge). This answers the review's BLOCKER 1. Both rows read the same boot asbattery_read, which ran_GLKand_REG(3, 1)ahead of the drain and the load's §5.2.10.1 wait. Readings: 14 of 14 tables loaded in 73 ms, the lock taken 241 times and the firmware found holding it by none, 31 SCIs, and query 0x4f taken 14 times, "served by nothing" (the T14's button is the fixed one)._BIXcross-check. cce064f read_BIXas design 50450 mWh, last full 45980 mWh, design voltage 11520 mV and 117 cycles. That matches the old design's direct-port readings exactly, at e73c65d, 0f1b059 and 60077a1 (linked under History). Linux read two of the four on the same machine, at 6465a1d (1):energy_full_design=50450000andenergy_full=45980000µWh. No Linux reading of the design voltage or cycle count is on record. That comparison also has charge 99%, rate 0 and "Not charging", which agree with cce064f's reading.battery_read,acpi_server_events,acpi_tables_loadedandacpi_server_death, run by the orchestrator.The T14 reading at d2c7267, and what it changed
The judge found
battery_readred on "the Global Lock taken 0 times". The rc=1 in that run came from the orchestrator's wrapper misreading the build line, not fromtoyos-metal(1).The row's lock expectation was wrong, and the rework exposed it. At 60077a1 the census read "taken 3 times on the way". There
Power::findran the namespace's_HIDwalk, the controller's_GLKand_REGitself, and then the battery's methods. The rework moved the walk todevices::findand_GLK/_REGtoattach, both ahead ofPower::find. So find's window now holds only the battery's_STAand_BIXand the first_PSRand_BST, and those took the lock 0 times on the T14. The 3 takes were the walk's,_GLK's or_REG's. The T14's AML is not in the tree (toyos-acpi/tests/thinkpad_t14.rsextracts the FACP and APIC only), so this rests on the two runs' counts and on the code that bounds each window.The answer, at its owner:
The row (
battery_on_metal,tests/toyos.rs) prints the census's lock counts and holds them to nothing. At cce064f it also heldgiven_back == takes. This head deletes that clause (the review's NOTE), because no T14 failure can redden it:finish,userland/acpiserver/aml/src/exec.rs);The real lock word's exchange on the T14 is
acpi_tables_loaded's, which requires the load's takes to be nonzero.The census (
battery.rs):Power::findreturns its census line for its caller to print, asreadreturns its reading lines. The numbers are local tofind, and nothing is stored for a test (the review's BLOCKER 3:LockedandPower::lockedare gone). The module header says what the line counts: the batteries' and adapters' own methods, not_REGor the walk.A host test holds that window from the line itself:
the_census_counts_the_global_lock_of_the_power_sources_methods_alone. The fixture's NVS byte is a Lock-rule field (§20.2.5.2) that both_REGand_BSTread. The scripted lock answers_REG's take, then holds_BST's once until a release. The line must read taken 1, given back 1, contended 1, out of the host's 2/2/1. n1 and n2 (below) turn it red.What changed, and why
One transaction path, main's (
ec.rs,host.rs). The pure transaction machine gains RD_EC (0x80) and WR_EC (0x81) beside QR_EC (§12.3.1, §12.3.2). Each byte waits for IBF clear. A read waits for OBF. A write ends only once IBF clears after its value. Main's kernel-routedtransactgains the data-port write. It moves frommain.rsintohost.rs, withport_in,port_outandUnmade, because the drain and AML now both use it. There is noControllertrait and no direct port access: the old branch had both, and the kernel now owns these ports. The host tests run the realtransactagainstec::tests::Emulated, which sits behind the scripted kernel's two controller ports (Scripted::controller).Emulatedraises SCI_EVT while a query waits (§12.2.1), which the drain reads before it queries.The controller is attached to AML after
devices::find(attach,main.rs). Inarm_controllerthe order is:_GLK(§6.5.7) and attach the controller to the host (Firmware::ec) only where_GLKis absent or 0;_REG(3, 1)(§6.5.4);Steps 2–3 come before step 4 for two reasons. The T14's
_REGreads and writes controller offset 0x03 (the track records the scouts' dry run). And a query's method, on a machine whose button is a control method device, reaches the controller's space. Two tests hold the order:a_laptops_battery_is_found_after_reg_and_read_through_the_controller: the battery fixture's_REGwrites offset 0x03 (m9).a_query_waiting_at_arm_runs_its_method_on_the_attached_controller. A control-method-button variant of the fixture has query 0x28 waiting as the controller is armed, and a_Q28that reads the adapter's byte. The controller must see_REG's write, then the drain's query, then the method's read, with nothing denied (o1).A
_GLKthat is nonzero, not an Integer, or refused leaves the space unattached. AML's accesses to it are then denied by name, and the line says why. The server's own queries still run without the lock, as on main (issues/the-acpi-server-talks-to-the-embedded-controller-without-the-global-lock.md, amended).A refused port ends the controller, from either side. Stopping and refused are handled as main handles them:
stopped().Firmware::ended). The server checks it after every evaluation that may reach the controller (the query run, the find and the poll;Server::ended), then calls main'sunserve.unservenow also detaches the controller from AML, so a port refused to the drain stops AML's accesses too.Power sources (
battery.rs,devices.rs).devices::find's walk now also lists PNP0C0A and ACPI0003 devices by path. Their_STAis not asked there, because a battery's_STAdepends on_REG.battery.rskeeps:_STAbit 4 (§6.3.7; with no_STAthe value is 0x0F, which reads as absent);_BIX, else_BIF, read once;_BIXrevisions 0 and 1 are read at the same places, percent is rounded down, mA × present mV / 1000 gives mW, and a state saying both charging and discharging is refused;REFUSED. The "no battery" line's text is inline inbattery.rs, its only writer:NO_BATTERYleftacpiserver-api, which no reader matched on (the review's NOTE);_BSTand_PSRevery 10 s between SCIs, never inside a drain.It drops its own walk,
_HIDdecoder,_GLKand_REG, whichdevices::idandattachnow cover.Power::findruns afterarm_controller(Server::find_power, which prints the census), and the poll runs inserve(Server::read_power). The census line says whether the controller is attached:with an|no embedded controller attached.The Global Lock: the wait, not
HELD. ACPI 6.5 §5.2.10.1: an OS that finds the lock owned sets the pending bit and waits for the firmware's release. The firmware signals the release withGBL_STSand the SCI, and only then does the OS take the lock again. Denying the take is no reading of that section: the access under the lock is never made, and the table or method that asked is refused. Main'sHELDwas recorded in the track as a compromise, exiting when "the wait comes back with the test that reaches its port sequence". This branch brings both back, soHELDgoes and the compromise entry with it. Main's two tests are made to agree:a_lock_field_read_at_load_takes_the_lock_and_a_held_lock_refuses_the_tablebecomes…_waits_out_the_firmwares_hold: one hold is waited out, and the table loads with the field read under the lock.a_lock_the_firmware_holds_is_denied_by_name_and_countedbecomesa_lock_the_firmware_holds_is_waited_for_and_taken_after_its_release, besidean_acquires_own_bound_ends_the_wait_untakenanda_firmware_that_holds_the_lock_past_the_bound_ends_the_server.The rest is unchanged from the reviewed branch:
sci::await_release, on main's row ports throughWaiting;Kernel::released;global_take,global_take_withinandglobal_release, with an Acquire bounded bymin(TimeoutValue, what the evaluation has left);RELEASE(1 s) as a recorded compromise;Claimowns it, becauseserveand the wait both wait on it;servemoves to animplonServer<Claim>.Kept from main unchanged:
FORWARDEDport writes, the Notify-is-a-press handling of control-method buttons,run_query, theCONTROLLER_*lines, thearmline and every test of main's other than the two above. Two of main's assertions change because the code changed: the no-controller denial string now reads "no controller is attached to this server's AML", andFoundliterals take..Found::default().Harness: unchanged from the reviewed branch except the T14 row. Its census constant (
T14_POWER_SOURCES,tests/toyos.rs) readswith an embedded controller attached, and it holds no lock count (above). That covers theacpi_mediatedheldarm,BootOptions::acpi_tablesandacpi_press_across_a_lock_wait.Issues:
RELEASEexit now names which count the server logs: a contended take counted at the load or in the power sources' census, orff_gbl_lock. Afterfindthe server logs no count of contended takes, so a hold during the polls or a query's method is not read (the review's NOTE)._REGcompromise now points atattach.SMI_CMDparagraph (main's version, taken in the merge) now says the host also asks the kernel for the controller's transactions.HELDcompromise is removed (exit met, above), and theRELEASEcompromise stays._GLKissue now says whatattachdoes with_GLK. It stays open: its exit, the lock around each controller transaction where_GLKis 1, is not met.Checks of high-risk code (devices, concurrency primitive, a boundary)
Independent oracles:
_REGand_GLK, §12.2 and §12.3 for the status bits and the transactions, §10.2.2 for the decoders;Mutations at 788521a: each is a checked patch, run under
cargo test -p acpiserver, and reverted with the tree checked clean (patches, script and exits). All three are red (EXIT=101), each on one test only; the other 62 stay green._REGmoved after the drain and the queries' run (the review's patch)a_query_waiting_at_arm_runs_its_method_on_the_attached_controllera read of EmbeddedControl: no controller is attached to this server's AML x1the_census_counts_the_global_lock_of_the_power_sources_methods_alonetaken 2 times … given back 2 times … holding it by 1taken 0 times … given back 0 times … holding it by 0The review predicted o1 would stay green at cce064f, and it did: EXIT=0, 62 passed (log). The new test is what turns it red.
Mutations m1–m9 at d2c7267, run the same way, all red (EXIT=101):
a_controller_port_the_kernel_refuses_detaches_the_controllera_controller_port_the_kernel_refuses_to_aml_or_the_drain_ends_it_for_bothunserveleaves AML attached_GLKignoreda_controller_that_wants_the_global_lock_is_not_attached_and_no_battery_is_readHELDdenial_REG_REGa_laptops_battery_is_found_after_reg_and_read_through_the_controllerm6 is the negative control for the lock: it puts main's side back.
The row's negative control is a recorded real failure: d2c7267's row, which demanded a take, redded on the T14 against this same server's line (judge).
Earlier rounds' controls of the unchanged parts. Mutations at 60077a1 (linked under History) held the wait's port sequence, the enables put back, and the Acquire bound.
acpi_press_across_a_lock_waitruns that port sequence on q35 and is green at this head.Why the guest test needs QEMU
acpi_press_across_a_lock_waitis unchanged from the reviewed branch. It needs three things:-acpitable;The host tests drive
await_releaseagainst an emulated block, but no host test reaches the realWaitingport writes, and a mutation of them stayed green on the host (round 3's m1). The T14's firmware holds the lock only when it chooses, and nothing but a hand presses its button. This round adds no guest test: the arm order's test is a host test.Gates
cargo run -- --ci hostcargo test --test toyos-build(whole guest suite)acpi_power_button,acpi_mediated_access,acpi_press_across_a_lock_waitamong them)cargo run -- --build-onlycargo test -p acpiservercargo test --test toyos-build -- --metal --metal-readback <scratch>/metal battery_read acpi_server_events acpi_tables_loaded acpi_server_deathbattery_readacpi_server_events acpi_tables_loaded, same bootbattery_readcargo test -p acpiserveralso ran inside--ci host(itsuserland/acpiserverstep), and so did the interpreter's own tests (userland/acpiserver/aml). Logs are posted whole. The scratch directory is written<scratch>, this worktree<worktree>, the primary checkout<primary>and the home directory~.Net lines,
git diff --shortstat origin/main...788521a48: 20 files, +2234 −216. This round (cce064f99..788521a48) is +92 −58, of which production is +19 −32 (the part of each source file before its#[cfg(test)]):LockedandNO_BATTERYgo, andfindreturns its census line. d2c7267's production figure was +974 −160. About 50 of those added lines and 50 of the removed aretransact,port_in,port_outandUnmademoving frommain.rstohost.rs.battery.rsis about +400 production, mostly its decoders and lines. The rest is host tests, the harness and issues.What I am unsure of
_GLKand_REGtook the T14's 3 takes at 60077a1 is not separated: no run counted them apart, and the tables are not here to read. The battery's methods took none of them.UNRUN; cce064f took query 0x4f 14 times, served by nothing). The arm order for a query's method is held only by the host test above.scis.Waiting::scitakes the claim's record, andservedoes not see that count. This is unchanged from the reviewed branch, and only the counts line is affected._GLKleaves the space unattached. That is fail-closed, my choice; the reviewed branch attached after a refused_GLK.History (the old design, before #845)
/sys/class/power_supplywas read on the same machine after 6465a1d's boot (1).🤖 Generated with Claude Code
https://claude.ai/code/session_017cSFvbD35xJ2kGANVdm23C