The AML interpreter is walked in declaration order and says what a call took

[Japabu]

Slice I1 of the next ACPI stage: what the server's later slices ask of the AML interpreter (userland/acpiserver/aml) before any of them runs a method on a machine: Interpreter::walk and Interpreter::usage. No kernel change and no syscall. The server calls neither yet, but load and evaluate changed under it, and it runs both at every boot.

Head 9cf510ce1: the slice's commit 26b40a452, origin/main 1084ddc9a merged (a6cd8521b), origin/main d78350c71 (#761) merged (c20b02f3c, no conflict), review round 1's answer (885d8d327), the track issue's two bullets (bd7b86547) and one line of a test helper that CI's clippy refused (9cf510ce1). The branch carries no commit of another pull request: its diff against main is its own.

It lands before #764 (slice S2), on which nothing in its code depends. Review round 2 sent back a head that had #764 merged in; that merge is out, and the branch was rebuilt from a6cd8521b. Checked at the new head:

• git diff 14bfcea31 bd7b86547 -- userland/acpiserver/aml/src userland/acpiserver/aml/tests/heap.rs userland/acpiserver/aml/tests/hostile.rs userland/acpiserver/aml/tests/walk.rs: empty, 0 bytes. The code round 2 reviewed is the code here.
• git diff origin/main...bd7b86547 --stat: eight files, +703 −29, the track issue and under userland/acpiserver/aml src/exec.rs, src/lib.rs, src/namespace.rs, src/object.rs, tests/heap.rs, tests/hostile.rs, tests/walk.rs. Nothing of Power-off goes through the ACPI server: the claim's holder supplies the sleep type, and the kernel's S5 byte scan is deleted #764's.
• git merge-base --is-ancestor f2a90a291 bd7b86547: exit 1, Power-off goes through the ACPI server: the claim's holder supplies the sleep type, and the kernel's S5 byte scan is deleted #764's merge is no ancestor.

In the track issue the walk's bullet and the wait limit's are this branch's, written onto main's text; the paragraph on the meter carries this branch's reading of the real tables under main's "Owner: the power-off stage", and the two other lines of that owner are untouched. The wait bullet's "Owner: this stage" is the interpreter stage, in whose list it stands. #764 will meet the meter paragraph from the other side: it changes that owner to "this stage" where this branch changed the paragraph's reading, so its next merge of main conflicts there and is resolved by keeping both.

What changed, per decision

• Interpreter::walk: the namespace read from the root down, in declaration order. Each object after the one it is in, siblings in the order their tables declared them, a later table's after an earlier one's.
  • No sequence number a node. Since The AML interpreter's 16 MiB counts the heap it holds, and a refused load gives its memory back #750 the arena is a stack, so its order is creation order: two passes over it make a first-child and a next-sibling link a node. Nothing sorts and nothing recurses over a depth a table chooses.
  • An Alias is not walked. The links leave out a node whose alias is set; its source is met at its own place. An Alias of an ancestor therefore cannot make the walk loop.
  • A predefined scope is Kind::Scope, descended and reported as what it is; the caller decides what is a device. Nothing here evaluates _STA or _INI, or reads §6.5.1.
  • An entry is a depth, a name and a kind; the path is the walk's, one string. Walk::path is the path of the entry last returned, in the text evaluate takes. A caller leaves a subtree out by skipping to the next entry no deeper than its top.
  • The walk is held against the meter while it lasts: eight bytes a node for the links and five a level of the deepest path, each taken before it is allocated and given back when the walk is dropped, a walk refused for its path included. A full interpreter refuses to be walked (walk returns Result) rather than allocate past MAX_LIVE.
• Interpreter::usage: what the last load or evaluation took. Steps, microseconds asked to Sleep, Stall and Wait (the request the limit refused included), and the meter's reading at its end. A refused call has one; a call refused before anything ran reads zero steps and zero time, whatever the call before it took.
• The wait limit stays ten seconds, and no caller names another (review round 1). Interpreter::evaluate_within, Machine::wait_us with its constructor argument, and the pub on MAX_WAIT_US are deleted: nothing called them, the 14 real tables asked for 0 µs, and no reading shows a method refused at ten seconds. A caller's limit also removed the one ceiling on what hostile AML may ask of Host::sleep. It comes back in the slice that carries a reading of a method refused at ten seconds; the track issue's bullet "An evaluation may ask to wait ten seconds, and no caller names another limit" says so, with owner and exit.
• The bridge refusal was already typed (Error::Bridge, on main since the server's load). Nothing to do.

What is left of I1

Stores are not bounded apart from one evaluation's holdings. A method that fills the interpreter still leaves every later evaluation that must hold something refused. The track's bullet "A refused evaluation keeps what it stored" keeps its exit and main's owner, the power-off stage. The caller's wait limit is the second thing left, recorded in the bullet named above.

Measured on the real machine's tables

Run outside the tree by a scratch harness that depends on the crate by path, at 26b40a452: cargo run --release -- <the tables' directory> <the PCI reading taken under Linux> (aarch64 macOS, load averages near 40, no figure a timing). The log is measure.log in the first round's scratchpad; it records no exit code, and runs to its last line. The DSDT's one memory word answered, every PCI function as Linux reads it, everything else zero. Its lines, without the two that carry a local path (cargo's Compiling of the harness and its Running) and the two on the memory word:

table 0: Ok steps 32946 waited_us 0 live 2033626
table 1: Ok steps 508 waited_us 0 live 2073583
table 2: Ok steps 117 waited_us 0 live 2081162
table 3: Ok steps 930 waited_us 0 live 2134568
table 4: Ok steps 111 waited_us 0 live 2144234
table 5: Ok steps 14 waited_us 0 live 2144789
table 6: Ok steps 971 waited_us 0 live 2191971
table 7: Ok steps 323 waited_us 0 live 2205092
table 8: Ok steps 537 waited_us 0 live 2230627
table 9: Ok steps 463 waited_us 0 live 2284058
table 10: Ok steps 2296 waited_us 0 live 2396734
table 11: Ok steps 2 waited_us 0 live 2396734
table 12: Ok steps 180 waited_us 0 live 2411362
table 13: Ok steps 21 waited_us 0 live 2412886
load: steps sum 39419 most 32946 waited_us 0; host reads 346 writes 0 lock takes 202
after load: meter 2412886 heap 1819038 (0.754 of the meter)
walk: 6890 entries, deepest 7, heap while walking 55163, kinds {"Buffer": 110, "BufferField": 50, "Device": 228, "Event": 1, "FieldUnit": 3717, "Integer": 711, "Method": 1670, "Mutex": 14, "OperationRegion": 117, "Package": 195, "PowerResource": 9, "Processor": 24, "Scope": 5, "String": 38, "ThermalZone": 1}
\_S5: Ok usage Usage { steps: 0, waited_us: 0, live: 2412886 }
parents 267, of which declared against name order 217
devices 228; EC devices 1 ; root bridges 1
EC at depth 4 is below a root bridge: true
_INI objects 45; kinds {"Method"}

• 14 of 14 tables load, in 39,419 steps; the DSDT takes 32,946, 3.1% of the step bound. No table asked to wait.
• After the load the meter reads 2,412,886 bytes (14.4% of MAX_LIVE) over 1,819,038 bytes of heap by a counting allocator.
• The walk returns 6,890 entries, 7 deep at most, and held 55,163 bytes while it lasted: 8 × 6,891 nodes with the root, and 5 × 7.
• 217 of 267 parents declare their children against name order.
• The embedded controller's device is below the PCI root bridge. Of initialisation order that is all that was read: 45 _INI, all methods, and under these answers, every read but the one word and the functions' headers zero, a dry run found two of them writing anything, that bridge's and that controller's. What the 45 do under the machine's own answers nothing has read, and the issue's bullet now says only this.

The arena's capacity is still read by nothing: 8,192 slots is its doubling, not a reading, and the issue's exit says so. The sources of the crate have changed since 26b40a452 only by round 1's deletion and by nothing the harness's figures rest on; the harness was not rerun at this head.

Checks of high-risk code (a trust boundary)

The namespace is built from firmware's bytes, so the walk is bounded by what the meter admits and tested against what a table can choose:

case	test
names against sort order, a second table adding to the first's device, a refused table and a method's exit before the slots are reused	siblings_are_read_in_the_order_they_were_declared
an Alias of a device, of a sibling, and of an ancestor (cyclic)	an_alias_is_not_walked
every kind, and a scope with a device child	every_object_is_walked_as_what_it_is
12,000 levels deep, through 120 tables	a_namespace_nested_deep_is_walked_whole
4,000 siblings in reverse name order	a_parent_of_thousands_is_walked_in_their_order
40,026 nodes under a counting allocator: bytes held, given back, and refused when full	a_walk_is_held_against_the_bound_while_it_lasts
12,012 nodes 12,000 deep, the meter left exactly 8 × nodes + 5 × depth bytes and then one less	a_walk_takes_its_links_and_its_deepest_path_from_the_bound (new in round 1)
ten seconds to the microsecond and one more, an evaluation's and a load's	the_wait_limit_is_exact_and_a_loads_too
usage after a load, a step-bound refusal, a wait, a store, a returned value, a refusal before running, a refused load, and a load refused before anything ran	usage_is_each_calls_own_and_a_refusals_too

The byte-exact pair. The new test stores buffers through a method until the interpreter has exactly the walk's bytes left, read from usage().live and MAX_LIVE: it is walked, and holds afterwards what it held before. With one byte less, which is room for the links and for all of the path but a byte, walk() is Err(Bound) and usage().live after the next evaluation is what it was. The reviewer's sizing, room between one link vector's bytes and two, cannot be built at this nesting: one vector is 48,048 bytes and the path 60,000, so a walk charged one vector is refused for its path in all of that range. The pair pins the sum to the byte instead, and m14 is the one-vector patch, red on it.

Independent oracle: for the walk's heap, the allocator's own count of requested bytes, which shares nothing with the meter's arithmetic (heap.rs asserts 8 * nodes + 5 held and zero after). For order, the ACPI specification gives none to hold the walk against: declaration order is the design's choice, and the tests state it.

Mutations, all rerun at bd7b86547, against main's toyos-acpi. Each is a checked patch (git apply --check), applied, its named test run alone (cargo test -p toyos-aml --test <binary> -- --exact <test>), reverted, the tree clean after each. RED is the log's test <name> ... FAILED line, not the exit alone, so a patch that did not build would not count; every log shows Finished and one test run. The thirteen patches are byte for byte round 1's, posted in that round's comment, and each passes git apply --check at this head.

patch	exit	red on
m1 siblings in the map's order, by name	101	siblings_are_read_in_the_order_they_were_declared
m2 an Alias walked	101	an_alias_is_not_walked
m3 the limit exclusive (>=)	101	the_wait_limit_is_exact_and_a_loads_too, hostile.rs:546: the evaluation asking exactly ten seconds is refused
m6 the walk's links not taken from the meter	101	a_walk_is_held_against_the_bound_while_it_lasts
m7 the walk's bytes never given back	101	same
m8 evaluate's usage not reset	101	usage_is_each_calls_own_and_a_refusals_too
m9 usage recorded only for a value	101	same
m10 the meter read before the call	101	same
m11 a scope reported as a device	101	every_object_is_walked_as_what_it_is
m12 the path not charged (the reviewer's patch)	101	a_walk_takes_its_links_and_its_deepest_path_from_the_bound, walk.rs:281: left: Ok(12011), right: Err(Bound(..))
m13 load's usage not reset (the reviewer's NOTE)	101	usage_is_each_calls_own_and_a_refusals_too, hostile.rs:617: left: Usage { steps: 106, waited_us: 7000, live: 3540 }
m14 the links charged as one vector's	101	the new walk test, walk.rs:281, walked where it must be refused
m15 a walk refused for its path keeps its links	101	the new walk test, walk.rs:283: 16,717,217 held against 16,621,121, the 96,096 bytes of 12,012 nodes' links

m4 and m5 are gone with the code they mutated. m4 ignored the caller's limit and m5 gave a load u64::MAX through the constructor argument; both are deleted, and with one constant in Machine::wait a load has no limit of its own to mutate. The load case stays in the wait test.

Gates

gate	head	result
cargo metadata --locked	bd7b86547	EXIT=0
cargo test -p toyos-aml	bd7b86547	EXIT=0. Seven binaries: evaluate 27, heap 19, hostile 24, namespace 18, regions 18, walk 6 passed, the library's own 0; 0 failed in each
cargo test --manifest-path userland/acpiserver/Cargo.toml	bd7b86547	EXIT=0. Compiles acpiserver against the changed crate and main's toyos-acpi; 26 passed, 0 failed
the thirteen mutations	bd7b86547	EXIT=101 each, RED on the test named, tree clean after each
cargo test -p toyos-aml	9cf510ce1	EXIT=0. Run on bd7b86547 with the one line uncommitted; git show 9cf510ce1 is that diff, byte for byte
m12, m14, m15 again	9cf510ce1	EXIT=101 each, RED on a_walk_takes_its_links_and_its_deepest_path_from_the_bound, the test of theirs that calls the helper the line is in; m12 at walk.rs:281, left: Ok(12011), as before. Same tree as the row above, 19 s for both
host (cargo run -- --ci host)	bd7b86547	RED, CI run 37768135128: [ci] Host: 1 of 78 step(s) red. The step is cargo clippy --manifest-path userland/acpiserver/aml/Cargo.toml --all-targets -- $ADOPTED -D warnings, and its one error manual_is_multiple_of at tests/walk.rs:201, left % 200 == 0, raised by CI's clippy (rust 1.99) and not by the development machine's older one
host	9cf510ce1	green, CI run 37770855110, job 113289783595: [ci] Host: 78 step(s), all green. The line is now left.is_multiple_of(200), left a usize, the value unchanged; this run is the first whole read of the crate by CI's clippy, and it is clean
toolchain / build, guest / suite	9cf510ce1	success, the same run (jobs 113289783975, 113291149880): PASS acpi_power_button (3s), test result: ok. 32 passed, 32 total, [ci] Guest: 5 step(s), all green
toolchain / build	bd7b86547	success, same run
guest / suite	bd7b86547	green, job 113281985415 of the same run: 32 passed, 32 total, PASS acpi_power_button. 9cf510ce1 differs from that head in one file, the host test userland/acpiserver/aml/tests/walk.rs; the suite has not run at 9cf510ce1

The three green gates at bd7b86547 and the thirteen mutations were run by the orchestrator from round 2's request script, round 1's with its log directory changed, 37 s in all. The measurements at 14bfcea31 are not carried: that head compiled #764's toyos-acpi.

Unsure of

• Whether the server wants Kind::Method to carry its argument count. evaluate refuses a wrong count by name, so nothing needs it yet.
• Kind::Reference covers both a reference a Name holds and a package element's unresolved name; the second cannot be a named object by my reading of define's callers, and the match is exhaustive rather than trusting that.
• \_S5 evaluates in zero steps on the real tables (a package's value is read without a term).
• The walk's depth pass reuses the sibling vector as scratch before the links are made. It saves four bytes a node and costs a comment.
• The new test's fill assumes a stored buffer is charged its length and a constant; it asserts the room it left, so a change to what a buffer charges reds it by name rather than passing wrongly.

Growth

git diff --shortstat origin/main...9cf510ce1: 8 files, +703 −29, the last commit one line for one. Production src/: +253 −17. Tests: +416. Issue text: +34 −12. Round 1 moved production from a net 248 lines to 236 and tests from 371 to 416. No dependency added.

🤖 Generated with Claude Code

https://claude.ai/code/session_01RvnWQFcMuGqTHYhvSnTe8A

[Japabu]

Mutation patches for the body's table, as run at 26b40a452 (the merge to a6cd8521b changed none of the files they touch; each passes git apply --check there).

The script, with the test command restated as the one workspace runs it:

#!/bin/sh
# Each mutation: checked, applied, the crate's whole suite run, its exit code read, restored.
S=<scratch>
cd <worktree> || exit 1
for p in $S/mut/*.patch; do
  n=$(basename "$p" .patch)
  git apply --check "$p" || { echo "$n: DOES NOT APPLY"; continue; }
  git apply "$p"
  cargo test -p toyos-aml --no-fail-fast > "$S/mut/$n.log" 2>&1
  code=$?
  git apply -R "$p"
  echo "$n: EXIT=$code; tree: $(git status --porcelain | wc -l | tr -d ' ') changed"
done

The runs' exits (mutate.log):

m1-siblings-by-name: EXIT=101; tree: 0 changed
m10-live-before-the-call: EXIT=101; tree: 0 changed
m11-scope-as-device: EXIT=101; tree: 0 changed
m2-alias-walked: EXIT=101; tree: 0 changed
m3-limit-exclusive: EXIT=101; tree: 0 changed
m4-callers-limit-ignored: EXIT=101; tree: 0 changed
m5-load-unlimited: EXIT=101; tree: 0 changed
m6-walk-unmetered: EXIT=101; tree: 0 changed (rerun: the first patch did not build)
m7-walk-kept: EXIT=101; tree: 0 changed
m8-usage-not-reset: EXIT=101; tree: 0 changed
m9-usage-of-a-value-only: EXIT=101; tree: 0 changed

m1-siblings-by-name.patch

diff --git a/userland/acpiserver/aml/src/namespace.rs b/userland/acpiserver/aml/src/namespace.rs
index cf888b4c1..5db2a3717 100644
--- a/userland/acpiserver/aml/src/namespace.rs
+++ b/userland/acpiserver/aml/src/namespace.rs
@@ -294,11 +294,13 @@ impl Namespace {
         walk.path.reserve_exact(path);
         // Newest first, each node goes to the front of its parent's list,
         // which leaves the oldest there.
-        for (i, n) in self.nodes.iter().enumerate().rev() {
-            walk.next[i] = 0;
-            if let (Some(p), None) = (n.parent, n.alias) {
-                walk.next[i] = walk.first[p as usize];
-                walk.first[p as usize] = i as u32;
+        walk.next.fill(0);
+        for (p, n) in self.nodes.iter().enumerate() {
+            for &c in n.children.values().rev() {
+                if self.nodes[c as usize].alias.is_none() {
+                    walk.next[c as usize] = walk.first[p];
+                    walk.first[p] = c;
+                }
             }
         }
         walk.at = walk.first[0];

m2-alias-walked.patch

diff --git a/userland/acpiserver/aml/src/namespace.rs b/userland/acpiserver/aml/src/namespace.rs
index cf888b4c1..853242bd5 100644
--- a/userland/acpiserver/aml/src/namespace.rs
+++ b/userland/acpiserver/aml/src/namespace.rs
@@ -296,7 +296,7 @@ impl Namespace {
         // which leaves the oldest there.
         for (i, n) in self.nodes.iter().enumerate().rev() {
             walk.next[i] = 0;
-            if let (Some(p), None) = (n.parent, n.alias) {
+            if let Some(p) = n.parent {
                 walk.next[i] = walk.first[p as usize];
                 walk.first[p as usize] = i as u32;
             }

m3-limit-exclusive.patch

diff --git a/userland/acpiserver/aml/src/exec.rs b/userland/acpiserver/aml/src/exec.rs
index e7bbe2675..b65edffce 100644
--- a/userland/acpiserver/aml/src/exec.rs
+++ b/userland/acpiserver/aml/src/exec.rs
@@ -280,7 +280,7 @@ impl<'a> Machine<'a> {
 
     fn wait(&mut self, us: u64) -> Result<(), Error> {
         self.waited_us = self.waited_us.saturating_add(us);
-        if self.waited_us > self.wait_us {
+        if self.waited_us >= self.wait_us {
             return Err(Error::Bound("more time asleep than one evaluation may spend"));
         }
         Ok(())

m4-callers-limit-ignored.patch

diff --git a/userland/acpiserver/aml/src/lib.rs b/userland/acpiserver/aml/src/lib.rs
index 76190c046..fc2f58325 100644
--- a/userland/acpiserver/aml/src/lib.rs
+++ b/userland/acpiserver/aml/src/lib.rs
@@ -392,7 +392,8 @@ impl Interpreter {
         let id = self.named(path)?;
         let args = args.iter().map(|a| self.object_of(a, w, 0)).collect::<Result<Vec<_>, _>>()?;
         let meter = self.meter.clone();
-        let mut m = Machine::new(&mut self.ns, host, w, meter.clone(), wait_us);
+        let _ = wait_us;
+        let mut m = Machine::new(&mut self.ns, host, w, meter.clone(), MAX_WAIT_US);
         let mut handed = 0;
         let r = m.evaluate(id, args).and_then(|o| value_of(&mut m, &meter, &mut handed, o, 0));
         // The value is the caller's from here, and no longer this interpreter's.

m5-load-unlimited.patch

diff --git a/userland/acpiserver/aml/src/lib.rs b/userland/acpiserver/aml/src/lib.rs
index 76190c046..e4111a837 100644
--- a/userland/acpiserver/aml/src/lib.rs
+++ b/userland/acpiserver/aml/src/lib.rs
@@ -354,7 +354,7 @@ impl Interpreter {
         let table = self.meter.bytes(bytes)?;
         let (root, made) = (self.ns.root(), self.ns.mark());
         let mut f = Frame::new(root, Vec::new(), table.clone(), 0);
-        let mut m = Machine::new(&mut self.ns, host, w, self.meter.clone(), MAX_WAIT_US);
+        let mut m = Machine::new(&mut self.ns, host, w, self.meter.clone(), u64::MAX);
         let bytes = table.borrow();
         let mut c = stream::Cursor::new(&bytes, toyos_acpi::SDT_HEADER_LEN, bytes.len());
         let r = m.term_list(&mut f, &mut c).and_then(|flow| match flow {

m6-walk-unmetered.patch

diff --git a/userland/acpiserver/aml/src/namespace.rs b/userland/acpiserver/aml/src/namespace.rs
index cf888b4c1..b7bd5fbdd 100644
--- a/userland/acpiserver/aml/src/namespace.rs
+++ b/userland/acpiserver/aml/src/namespace.rs
@@ -273,8 +273,8 @@ impl Namespace {
     pub(crate) fn walk(&self) -> Result<Walk<'_>, Error> {
         let count = self.nodes.len();
         let links = 2 * count * core::mem::size_of::<u32>();
-        self.meter.take(links)?;
-        let mut walk = Walk { ns: self, first: Vec::new(), next: Vec::new(), at: 0, depth: 1, path: String::new(), held: links };
+        let _ = links;
+        let mut walk = Walk { ns: self, first: Vec::new(), next: Vec::new(), at: 0, depth: 1, path: String::new(), held: 0 };
         for links in [&mut walk.first, &mut walk.next] {
             links.reserve_exact(count);
             links.resize(count, 0);

m7-walk-kept.patch

diff --git a/userland/acpiserver/aml/src/namespace.rs b/userland/acpiserver/aml/src/namespace.rs
index cf888b4c1..2da5eb933 100644
--- a/userland/acpiserver/aml/src/namespace.rs
+++ b/userland/acpiserver/aml/src/namespace.rs
@@ -308,7 +308,7 @@ impl Namespace {
 
 impl Drop for Walk<'_> {
     fn drop(&mut self) {
-        self.ns.meter.give(self.held);
+        let _ = self.held;
     }
 }
 

m8-usage-not-reset.patch

diff --git a/userland/acpiserver/aml/src/lib.rs b/userland/acpiserver/aml/src/lib.rs
index 76190c046..d3d030162 100644
--- a/userland/acpiserver/aml/src/lib.rs
+++ b/userland/acpiserver/aml/src/lib.rs
@@ -381,7 +381,6 @@ impl Interpreter {
     /// at most, which the caller names for a method it knows to ask more
     /// than [`MAX_WAIT_US`].
     pub fn evaluate_within(&mut self, host: &mut dyn Host, path: &str, args: &[Value], wait_us: u64) -> Result<Value, Error> {
-        self.last = Usage::default();
         let r = self.evaluate_in(host, path, args, wait_us);
         self.last.live = self.meter.live();
         r

m9-usage-of-a-value-only.patch

diff --git a/userland/acpiserver/aml/src/exec.rs b/userland/acpiserver/aml/src/exec.rs
index e7bbe2675..e685d0cca 100644
--- a/userland/acpiserver/aml/src/exec.rs
+++ b/userland/acpiserver/aml/src/exec.rs
@@ -243,7 +243,9 @@ impl<'a> Machine<'a> {
     /// control method cannot exit while still holding ownership of a Mutex").
     /// What it took goes to `took`, whichever way it ends.
     pub(crate) fn finish<T>(mut self, r: Result<T, Error>, took: &mut Usage) -> Result<T, Error> {
-        (took.steps, took.waited_us) = (self.steps, self.waited_us);
+        if r.is_ok() {
+            (took.steps, took.waited_us) = (self.steps, self.waited_us);
+        }
         let held = !self.held.is_empty();
         for m in self.held.drain(..) {
             m.held.set(0);

m10-live-before-the-call.patch

diff --git a/userland/acpiserver/aml/src/lib.rs b/userland/acpiserver/aml/src/lib.rs
index 76190c046..40ace2918 100644
--- a/userland/acpiserver/aml/src/lib.rs
+++ b/userland/acpiserver/aml/src/lib.rs
@@ -382,8 +382,8 @@ impl Interpreter {
     /// than [`MAX_WAIT_US`].
     pub fn evaluate_within(&mut self, host: &mut dyn Host, path: &str, args: &[Value], wait_us: u64) -> Result<Value, Error> {
         self.last = Usage::default();
-        let r = self.evaluate_in(host, path, args, wait_us);
         self.last.live = self.meter.live();
+        let r = self.evaluate_in(host, path, args, wait_us);
         r
     }
 

m11-scope-as-device.patch

diff --git a/userland/acpiserver/aml/src/lib.rs b/userland/acpiserver/aml/src/lib.rs
index 76190c046..44030829c 100644
--- a/userland/acpiserver/aml/src/lib.rs
+++ b/userland/acpiserver/aml/src/lib.rs
@@ -278,7 +278,7 @@ impl Iterator for Walk<'_> {
             Object::Field(_) => Kind::FieldUnit,
             Object::BufField(_) => Kind::BufferField,
             Object::Ref(_) | Object::Lazy(_) => Kind::Reference,
-            Object::Scope => Kind::Scope,
+            Object::Scope => Kind::Device,
             Object::Device => Kind::Device,
             Object::Processor => Kind::Processor,
             Object::ThermalZone => Kind::ThermalZone,

[Japabu]

Review of a6cd8521b against origin/main 1084ddc9a, round 1. Read only: no cargo, rustc, clippy, QEMU or test was run by this review; every measurement named below is the implementer's log or is owed.

Net lines (git diff --shortstat origin/main...a6cd8521b): 8 files, +665 −37. Production src/: +273 −25. Tests: +371 −0. Issue text: +21 −12.

BLOCKER

• userland/acpiserver/aml/src/lib.rs:383 (evaluate_within), :86 (pub MAX_WAIT_US), src/exec.rs:47 (Machine::wait_us) — a parameter with one value, built on an estimate — the only non-test caller is evaluate, which passes MAX_WAIT_US; no measurement in the body shows a method that asks for more than 10 s (the 14 tables asked for 0 µs, and the body defers the larger limit to "the battery slice's, with its measurement"); an evaluation that asks for exactly 10,000,000 µs is already admitted (TEN0). The limit also bounds only time asked, never work, so a caller's large value removes the one ceiling on what hostile AML may ask of Host::sleep (2^20 steps of it), and that is added here with nothing calling it. Deletion named: evaluate_within, the wait_us field and constructor argument, the pub on MAX_WAIT_US, and the caller's-limit half of the_wait_limit_is_exact_and_the_callers_to_name (the exact-at-10-s and load cases stay; m3 and m5 keep their reds). It comes back in the slice that carries the reading of a method refused at 10 s.
• userland/acpiserver/aml/src/namespace.rs:291-293 — the path's charge has no test that can fail, and the walk's second refusal is reached by none — the body claims "five a level of the deepest path, taken before either is allocated". Mutation: delete self.meter.take(path)?; and walk.held += path;. I expect it green: heap.rs:259 reads the allocator, not the meter; heap.rs:263 holds because take and give vanish together; heap.rs:267 is refused at the links' take (:276), never at the path's. The same hole hides a walk refused at :292 that fails to give its links back (Drop on the ?). It must turn red a test that nests deep (the 12,000 levels of walk.rs are 60,000 bytes of path over 96 KB of links), leaves the meter room for the links and not the path, sized from usage().live and MAX_LIVE, and asserts walk() is Err(Bound) and usage().live after the next evaluation is what it was. With the room set between one link vector's bytes and two, the same test pins the amount the body lists as unpinned. The implementer runs it.
• PR body, "Gates" — cargo run -- --ci host green at the head is absent, and clippy has run nowhere on this branch — owed: the host check on the ready pull request at the head that lands (or the orchestrator's own run of it at that head).
• PR body, "Gates" — the guest tests the change reaches are absent, and the body's reason for owing none is false of the tree — Interpreter::load, evaluate, Machine::new, finish and wait are changed, and the server runs them at every boot (userland/acpiserver/src/aml.rs:193 and its load; tests/toyos.rs:3868 acpi_tables_loaded, and the power-off rows). Owed: guest / suite green at the head that lands.

NOTE

• userland/acpiserver/aml/src/lib.rs:338 — deleting self.last = Usage::default(); in load stays green by my reading: every load a test reads usage after reaches finish, which overwrites steps and time. One assertion in usage_is_each_calls_own_and_a_refusals_too holds it: a second DSDT (Error::Table), then usage() with zero steps and zero time.
• issues/toyos-runs-the-machine-in-acpi-mode-and-interprets-its-aml.md (the hunk at main's 223-234) — conflicts with Power-off goes through the ACPI server: the claim's holder supplies the sleep type, and the kernel's S5 byte scan is deleted #764 (52ef7a10f): git merge-tree a6cd8521b 52ef7a10f reports one content conflict, in this file and nowhere else. Power-off goes through the ACPI server: the claim's holder supplies the sleep type, and the kernel's S5 byte scan is deleted #764 rewrites "Owner: the power-off stage" to "Owner: this stage" in the paragraph this branch rewrites, and in the bullet that holds what is left of I1. aml/tests/namespace.rs does not conflict: this branch does not touch it. Power-off goes through the ACPI server: the claim's holder supplies the sleep type, and the kernel's S5 byte scan is deleted #764 lands first; this branch then merges main, takes Power-off goes through the ACPI server: the claim's holder supplies the sleep type, and the kernel's S5 byte scan is deleted #764's owner in both places, and reruns cargo test -p toyos-aml and the server's tests at the new head, because Power-off goes through the ACPI server: the claim's holder supplies the sleep type, and the kernel's S5 byte scan is deleted #764 changes toyos-acpi, on whose stillness the unrerun mutations rest.
• PR body — the T14 figures the issue now carries (14 of 14, 39,419 steps, 2,412,886 over 1,819,038, 6,891 nodes, 217 of 267) have no command, exit code or log in the body; the log exists (measure.log, at 26b40a452) and agrees with every figure (55,163 = 8 × 6,891 + 5 × 7; 8,192 − 6,891 = 1,301; the kinds sum to 6,890). Its per-table lines carry no name, byte or address and can go in the body as they are, without its two path lines.
• issues/toyos-runs-the-machine-in-acpi-mode-and-interprets-its-aml.md:236 (prose) — "its initialisation does not hang on the order" says more than was read: the dry run answered every read but one word zero, and measure.log counts 45 _INI methods of which two were found writing under those answers.
• PR body (prose) — "The server calls none of the new surface, so no guest test reaches it" is false of the changed load and evaluate paths, as above.

What the brief asked to be held

1. Hostile input. walk: no recursion (two passes over the arena, then a climb in which each node is left once); no cycle is reachable, since a node's parent index is below its own and a node with alias set enters no list (and holds no children: child follows the alias before create attaches). Allocation is 2 × 4 × nodes taken at :276 before reserve_exact, and 5 × deepest taken at :292 before the path's; the path never outgrows it because Seg::new admits only ASCII (so truncate is always on a boundary). A full interpreter is refused at :276, measured (heap.rs:267, m6 left: None). Node count is bounded by the meter and by u32 in push; depth cannot underflow. evaluate_within: zero admits a zero Sleep and Stall and refuses one microsecond, measured; an enormous limit never refuses, waited_us saturates, and steps stay bounded at 2^20 — the limit bounds waiting asked, not work and not time spent. usage: steps is refused at 2^20 + 1 and charge saturates; waited_us saturates.
2. Declaration order. By my reading of ACPI 6.5, §6.5.1 is the section that orders initialisation: a device's _INI runs if _STA says present, and its children are then examined; it names no order among siblings, and §5.3 gives the namespace none. Parent before child is what the walk guarantees; sibling order is the design's choice, as the body says. Arena order is creation order for each case asked: a re-opened Scope and a Name with a longer path append to the parent they name (asserted, walk.rs CCCC, AAAA); definition-block code creates in the order it executes; a method's objects are gone at its exit (exec.rs:396-401) and walk borrows the interpreter, so none is ever walked; Load is refused; across tables the order is the order the caller loaded them in, which is the server's. walk and step never read children: nothing depends on name order.
3. Mutations. All eleven logs are test reds, none a build failure; m6's final log is red at heap.rs:267 with left: None. Each fails on the assertion its patch removes. git diff 26b40a452 a6cd8521b --stat -- userland/acpiserver toyos-acpi is empty; the merge did change toyos-abi/src/syscall.rs, which the crate's build compiles, and the root manifest and .cargo/config.toml; none is read by a patch or a test, all eleven pass git apply --check at the head, and the unmutated suite is green there (merged.exits: aml EXIT=0, 27/19/24/18/18/5). Accepted until Power-off goes through the ACPI server: the claim's holder supplies the sleep type, and the kernel's S5 byte scan is deleted #764 moves toyos-acpi. Decisions with no mutation: the path's charge and the second refusal (BLOCKER); load's reset (NOTE); the Uninit and Lazy arms of the kind match, which a reader checks.
4. Privacy. Tree, both commit messages, the body and the comment hold counts and one position in the device tree; no table byte, object name from the tables, OEM string, address, serial or local path (the comment's script has <scratch> and <worktree>). Test names are synthetic.
5. What is left of I1. Recorded: the bullet "A refused evaluation keeps what it stored" has an owner that exists and an exit a test can fail; Power-off goes through the ACPI server: the claim's holder supplies the sleep type, and the kernel's S5 byte scan is deleted #764 moves that owner. Nothing in the diff becomes unsafe without it: after a method fills the interpreter walk is refused by name and stays refused. For the caller: a Walk borrows the interpreter, so nothing is evaluated while one is held, and only the last entry's path is kept; the init walk must copy what it will evaluate before it drops the walk, and what it copies is outside the meter.

Clean room: the diff and its comments cite the specification alone.

Checks the landing rests on: host, and guest / suite for the changed load and evaluate paths. Neither has run (draft).

SEND BACK

[Japabu]

Review round 1, answered at 14bfcea31. The thirteen mutation patches, each against that head, and the script that ran them (<scratch> is the round's scratch directory, <worktree> the branch's worktree). All thirteen: EXIT=101, RED on the named test, tree clean after.

the script

#!/bin/sh
# Slice I1, review round 1: wt/toyos-amli1 at 14bfcea31 (a6cd8521b, #764's head 52ef7a10f merged,
# and the round's commit). Host tests of one crate and of the server; every log is under $R.
R=<scratch>
cd <worktree> || { echo "worktree EXIT=1"; exit 1; }
: > "$R/exits"
say() { echo "$1" | tee -a "$R/exits"; }
say "head $(git rev-parse HEAD); tree: $(git status --porcelain --ignore-submodules=none | wc -l | tr -d ' ') changed"

cargo metadata --locked --format-version 1 > "$R/metadata.json" 2> "$R/metadata.log"
say "metadata EXIT=$?"
cargo test -p toyos-aml > "$R/aml.log" 2>&1
say "aml EXIT=$?"
cargo test --manifest-path userland/acpiserver/Cargo.toml > "$R/acpiserver.log" 2>&1
say "acpiserver EXIT=$?"

# One mutation: checked, applied, its named test run alone, restored. RED only where the log
# shows that test failing; a patch that does not build is no red.
mutate() {
  n=$1; bin=$2; t=$3; p="$R/mut/$n.patch"
  git apply --check "$p" || { say "$n DOES NOT APPLY EXIT=1"; return; }
  git apply "$p"
  cargo test -p toyos-aml --test "$bin" -- --exact "$t" > "$R/mut/$n.log" 2>&1
  code=$?
  git apply -R "$p"
  if grep -q "^test $t \.\.\. FAILED" "$R/mut/$n.log"; then v=RED
  elif [ $code -eq 0 ]; then v=GREEN
  else v="NO TEST RAN RED (a build failure?)"; fi
  say "$n ($bin::$t) EXIT=$code $v; tree: $(git status --porcelain --ignore-submodules=none | wc -l | tr -d ' ') changed"
}
mutate m1-siblings-by-name walk siblings_are_read_in_the_order_they_were_declared
mutate m2-alias-walked walk an_alias_is_not_walked
mutate m3-limit-exclusive hostile the_wait_limit_is_exact_and_a_loads_too
mutate m6-walk-unmetered heap a_walk_is_held_against_the_bound_while_it_lasts
mutate m7-walk-kept heap a_walk_is_held_against_the_bound_while_it_lasts
mutate m8-usage-not-reset hostile usage_is_each_calls_own_and_a_refusals_too
mutate m9-usage-of-a-value-only hostile usage_is_each_calls_own_and_a_refusals_too
mutate m10-live-before-the-call hostile usage_is_each_calls_own_and_a_refusals_too
mutate m11-scope-as-device walk every_object_is_walked_as_what_it_is
mutate m12-path-not-charged walk a_walk_takes_its_links_and_its_deepest_path_from_the_bound
mutate m13-load-usage-not-reset hostile usage_is_each_calls_own_and_a_refusals_too
mutate m14-links-charged-as-one-vector walk a_walk_takes_its_links_and_its_deepest_path_from_the_bound
mutate m15-links-kept-by-a-walk-refused-for-its-path walk a_walk_takes_its_links_and_its_deepest_path_from_the_bound
say "done; head $(git rev-parse HEAD); tree: $(git status --porcelain --ignore-submodules=none | wc -l | tr -d ' ') changed"

head 14bfcea31153ed837ccb4c3b6ba51c1908bafc72; tree: 0 changed
metadata EXIT=0
aml EXIT=0
acpiserver EXIT=0
m1-siblings-by-name (walk::siblings_are_read_in_the_order_they_were_declared) EXIT=101 RED; tree: 0 changed
m2-alias-walked (walk::an_alias_is_not_walked) EXIT=101 RED; tree: 0 changed
m3-limit-exclusive (hostile::the_wait_limit_is_exact_and_a_loads_too) EXIT=101 RED; tree: 0 changed
m6-walk-unmetered (heap::a_walk_is_held_against_the_bound_while_it_lasts) EXIT=101 RED; tree: 0 changed
m7-walk-kept (heap::a_walk_is_held_against_the_bound_while_it_lasts) EXIT=101 RED; tree: 0 changed
m8-usage-not-reset (hostile::usage_is_each_calls_own_and_a_refusals_too) EXIT=101 RED; tree: 0 changed
m9-usage-of-a-value-only (hostile::usage_is_each_calls_own_and_a_refusals_too) EXIT=101 RED; tree: 0 changed
m10-live-before-the-call (hostile::usage_is_each_calls_own_and_a_refusals_too) EXIT=101 RED; tree: 0 changed
m11-scope-as-device (walk::every_object_is_walked_as_what_it_is) EXIT=101 RED; tree: 0 changed
m12-path-not-charged (walk::a_walk_takes_its_links_and_its_deepest_path_from_the_bound) EXIT=101 RED; tree: 0 changed
m13-load-usage-not-reset (hostile::usage_is_each_calls_own_and_a_refusals_too) EXIT=101 RED; tree: 0 changed
m14-links-charged-as-one-vector (walk::a_walk_takes_its_links_and_its_deepest_path_from_the_bound) EXIT=101 RED; tree: 0 changed
m15-links-kept-by-a-walk-refused-for-its-path (walk::a_walk_takes_its_links_and_its_deepest_path_from_the_bound) EXIT=101 RED; tree: 0 changed
done; head 14bfcea31153ed837ccb4c3b6ba51c1908bafc72; tree: 0 changed

m1-siblings-by-name.patch

diff --git a/userland/acpiserver/aml/src/namespace.rs b/userland/acpiserver/aml/src/namespace.rs
index cf888b4c1..5db2a3717 100644
--- a/userland/acpiserver/aml/src/namespace.rs
+++ b/userland/acpiserver/aml/src/namespace.rs
@@ -294,11 +294,13 @@ impl Namespace {
         walk.path.reserve_exact(path);
         // Newest first, each node goes to the front of its parent's list,
         // which leaves the oldest there.
-        for (i, n) in self.nodes.iter().enumerate().rev() {
-            walk.next[i] = 0;
-            if let (Some(p), None) = (n.parent, n.alias) {
-                walk.next[i] = walk.first[p as usize];
-                walk.first[p as usize] = i as u32;
+        walk.next.fill(0);
+        for (p, n) in self.nodes.iter().enumerate() {
+            for &c in n.children.values().rev() {
+                if self.nodes[c as usize].alias.is_none() {
+                    walk.next[c as usize] = walk.first[p];
+                    walk.first[p] = c;
+                }
             }
         }
         walk.at = walk.first[0];

m2-alias-walked.patch

diff --git a/userland/acpiserver/aml/src/namespace.rs b/userland/acpiserver/aml/src/namespace.rs
index cf888b4c1..853242bd5 100644
--- a/userland/acpiserver/aml/src/namespace.rs
+++ b/userland/acpiserver/aml/src/namespace.rs
@@ -296,7 +296,7 @@ impl Namespace {
         // which leaves the oldest there.
         for (i, n) in self.nodes.iter().enumerate().rev() {
             walk.next[i] = 0;
-            if let (Some(p), None) = (n.parent, n.alias) {
+            if let Some(p) = n.parent {
                 walk.next[i] = walk.first[p as usize];
                 walk.first[p as usize] = i as u32;
             }

m3-limit-exclusive.patch

diff --git a/userland/acpiserver/aml/src/exec.rs b/userland/acpiserver/aml/src/exec.rs
index 20143bcb6..0f0dbf9e6 100644
--- a/userland/acpiserver/aml/src/exec.rs
+++ b/userland/acpiserver/aml/src/exec.rs
@@ -278,7 +278,7 @@ impl<'a> Machine<'a> {
 
     fn wait(&mut self, us: u64) -> Result<(), Error> {
         self.waited_us = self.waited_us.saturating_add(us);
-        if self.waited_us > MAX_WAIT_US {
+        if self.waited_us >= MAX_WAIT_US {
             return Err(Error::Bound("more time asleep than one evaluation may spend"));
         }
         Ok(())

m6-walk-unmetered.patch

diff --git a/userland/acpiserver/aml/src/namespace.rs b/userland/acpiserver/aml/src/namespace.rs
index cf888b4c1..b7bd5fbdd 100644
--- a/userland/acpiserver/aml/src/namespace.rs
+++ b/userland/acpiserver/aml/src/namespace.rs
@@ -273,8 +273,8 @@ impl Namespace {
     pub(crate) fn walk(&self) -> Result<Walk<'_>, Error> {
         let count = self.nodes.len();
         let links = 2 * count * core::mem::size_of::<u32>();
-        self.meter.take(links)?;
-        let mut walk = Walk { ns: self, first: Vec::new(), next: Vec::new(), at: 0, depth: 1, path: String::new(), held: links };
+        let _ = links;
+        let mut walk = Walk { ns: self, first: Vec::new(), next: Vec::new(), at: 0, depth: 1, path: String::new(), held: 0 };
         for links in [&mut walk.first, &mut walk.next] {
             links.reserve_exact(count);
             links.resize(count, 0);

m7-walk-kept.patch

diff --git a/userland/acpiserver/aml/src/namespace.rs b/userland/acpiserver/aml/src/namespace.rs
index cf888b4c1..2da5eb933 100644
--- a/userland/acpiserver/aml/src/namespace.rs
+++ b/userland/acpiserver/aml/src/namespace.rs
@@ -308,7 +308,7 @@ impl Namespace {
 
 impl Drop for Walk<'_> {
     fn drop(&mut self) {
-        self.ns.meter.give(self.held);
+        let _ = self.held;
     }
 }
 

m8-usage-not-reset.patch

diff --git a/userland/acpiserver/aml/src/lib.rs b/userland/acpiserver/aml/src/lib.rs
index f75947a35..ad07e3e4b 100644
--- a/userland/acpiserver/aml/src/lib.rs
+++ b/userland/acpiserver/aml/src/lib.rs
@@ -371,7 +371,6 @@ impl Interpreter {
     /// Evaluates the object at an absolute path, written `\_SB.PCI0._STA`: a
     /// method is invoked with `args`, anything else is its value.
     pub fn evaluate(&mut self, host: &mut dyn Host, path: &str, args: &[Value]) -> Result<Value, Error> {
-        self.last = Usage::default();
         let r = self.evaluate_in(host, path, args);
         self.last.live = self.meter.live();
         r

m9-usage-of-a-value-only.patch

diff --git a/userland/acpiserver/aml/src/exec.rs b/userland/acpiserver/aml/src/exec.rs
index e7bbe2675..e685d0cca 100644
--- a/userland/acpiserver/aml/src/exec.rs
+++ b/userland/acpiserver/aml/src/exec.rs
@@ -243,7 +243,9 @@ impl<'a> Machine<'a> {
     /// control method cannot exit while still holding ownership of a Mutex").
     /// What it took goes to `took`, whichever way it ends.
     pub(crate) fn finish<T>(mut self, r: Result<T, Error>, took: &mut Usage) -> Result<T, Error> {
-        (took.steps, took.waited_us) = (self.steps, self.waited_us);
+        if r.is_ok() {
+            (took.steps, took.waited_us) = (self.steps, self.waited_us);
+        }
         let held = !self.held.is_empty();
         for m in self.held.drain(..) {
             m.held.set(0);

m10-live-before-the-call.patch

diff --git a/userland/acpiserver/aml/src/lib.rs b/userland/acpiserver/aml/src/lib.rs
index f75947a35..3de881460 100644
--- a/userland/acpiserver/aml/src/lib.rs
+++ b/userland/acpiserver/aml/src/lib.rs
@@ -372,8 +372,8 @@ impl Interpreter {
     /// method is invoked with `args`, anything else is its value.
     pub fn evaluate(&mut self, host: &mut dyn Host, path: &str, args: &[Value]) -> Result<Value, Error> {
         self.last = Usage::default();
-        let r = self.evaluate_in(host, path, args);
         self.last.live = self.meter.live();
+        let r = self.evaluate_in(host, path, args);
         r
     }
 

m11-scope-as-device.patch

diff --git a/userland/acpiserver/aml/src/lib.rs b/userland/acpiserver/aml/src/lib.rs
index 76190c046..44030829c 100644
--- a/userland/acpiserver/aml/src/lib.rs
+++ b/userland/acpiserver/aml/src/lib.rs
@@ -278,7 +278,7 @@ impl Iterator for Walk<'_> {
             Object::Field(_) => Kind::FieldUnit,
             Object::BufField(_) => Kind::BufferField,
             Object::Ref(_) | Object::Lazy(_) => Kind::Reference,
-            Object::Scope => Kind::Scope,
+            Object::Scope => Kind::Device,
             Object::Device => Kind::Device,
             Object::Processor => Kind::Processor,
             Object::ThermalZone => Kind::ThermalZone,

m12-path-not-charged.patch

diff --git a/userland/acpiserver/aml/src/namespace.rs b/userland/acpiserver/aml/src/namespace.rs
index cf888b4c1..54fd7f979 100644
--- a/userland/acpiserver/aml/src/namespace.rs
+++ b/userland/acpiserver/aml/src/namespace.rs
@@ -289,8 +289,6 @@ impl Namespace {
             }
         }
         let path = path_len(deepest as usize);
-        self.meter.take(path)?;
-        walk.held += path;
         walk.path.reserve_exact(path);
         // Newest first, each node goes to the front of its parent's list,
         // which leaves the oldest there.

m13-load-usage-not-reset.patch

diff --git a/userland/acpiserver/aml/src/lib.rs b/userland/acpiserver/aml/src/lib.rs
index f75947a35..19cf45dc6 100644
--- a/userland/acpiserver/aml/src/lib.rs
+++ b/userland/acpiserver/aml/src/lib.rs
@@ -333,7 +333,6 @@ impl Interpreter {
     /// Loads a DSDT or SSDT (§5.4.2): the DSDT first, then each SSDT. The
     /// table's header, length and checksum are [`toyos_acpi::Table::open`]'s.
     pub fn load<P: toyos_acpi::Phys>(&mut self, host: &mut dyn Host, table: &toyos_acpi::Table<P>) -> Result<(), Error> {
-        self.last = Usage::default();
         let r = self.load_in(host, table);
         self.last.live = self.meter.live();
         r

m14-links-charged-as-one-vector.patch

diff --git a/userland/acpiserver/aml/src/namespace.rs b/userland/acpiserver/aml/src/namespace.rs
index cf888b4c1..55113af4d 100644
--- a/userland/acpiserver/aml/src/namespace.rs
+++ b/userland/acpiserver/aml/src/namespace.rs
@@ -272,7 +272,7 @@ const fn path_len(depth: usize) -> usize {
 impl Namespace {
     pub(crate) fn walk(&self) -> Result<Walk<'_>, Error> {
         let count = self.nodes.len();
-        let links = 2 * count * core::mem::size_of::<u32>();
+        let links = count * core::mem::size_of::<u32>();
         self.meter.take(links)?;
         let mut walk = Walk { ns: self, first: Vec::new(), next: Vec::new(), at: 0, depth: 1, path: String::new(), held: links };
         for links in [&mut walk.first, &mut walk.next] {

m15-links-kept-by-a-walk-refused-for-its-path.patch

diff --git a/userland/acpiserver/aml/src/namespace.rs b/userland/acpiserver/aml/src/namespace.rs
index cf888b4c1..7756ac081 100644
--- a/userland/acpiserver/aml/src/namespace.rs
+++ b/userland/acpiserver/aml/src/namespace.rs
@@ -274,7 +274,7 @@ impl Namespace {
         let count = self.nodes.len();
         let links = 2 * count * core::mem::size_of::<u32>();
         self.meter.take(links)?;
-        let mut walk = Walk { ns: self, first: Vec::new(), next: Vec::new(), at: 0, depth: 1, path: String::new(), held: links };
+        let mut walk = Walk { ns: self, first: Vec::new(), next: Vec::new(), at: 0, depth: 1, path: String::new(), held: 0 };
         for links in [&mut walk.first, &mut walk.next] {
             links.reserve_exact(count);
             links.resize(count, 0);
@@ -290,7 +290,7 @@ impl Namespace {
         }
         let path = path_len(deepest as usize);
         self.meter.take(path)?;
-        walk.held += path;
+        walk.held = links + path;
         walk.path.reserve_exact(path);
         // Newest first, each node goes to the front of its parent's list,
         // which leaves the oldest there.

[Japabu]

Review of 14bfcea31 against origin/main d78350c71, round 2. Read only: no cargo, QEMU or test was run by this review; every measurement named is the round's log (exits, aml.log, acpiserver.log, mut/*.log, all at 14bfcea31) or is owed.

Net lines, the branch's own (git diff --shortstat origin/wt/toyos-amls2...14bfcea31): 8 files, +703 −29. Production src/: +253 −17. Tests: +416 −0. Issue text: +34 −12. With #764's four commits, against origin/main: 36 files, +1292 −356.

Round 1's BLOCKERs

1. evaluate_within, Machine::wait_us, pub MAX_WAIT_US: CLOSED. git grep at the head finds no evaluate_within and no wait_us outside kernel/src/drivers/virtio*.rs's unrelated wait_used; MAX_WAIT_US is pub(crate) again, as on main, read once, in Machine::wait (exec.rs:281). No constructor argument, field, doc line or test half is left: Machine::new has four arguments at both call sites, the caller's-limit methods and assertions are gone from hostile.rs, and the exact-at-ten-seconds and load cases stay. m3 at this head: EXIT=101, the_wait_limit_is_exact_and_a_loads_too ... FAILED at hostile.rs:546. m4 and m5 have nothing left to mutate: a load and an evaluation read the same constant in the same line. The track's bullet "An evaluation may ask to wait ten seconds, and no caller names another limit" has an owner that exists (the stage "the interpreter", in whose list it stands) and an exit a reading can meet.

2. The path's charge and the walk's second refusal: CLOSED. a_walk_takes_its_links_and_its_deepest_path_from_the_bound, green in aml.log (walk: 6 passed), and red three ways:

   • m12 (my patch): EXIT=101, walk.rs:281, left: Ok(12011).
   • m14 (links charged as one vector's): EXIT=101, walk.rs:281, left: Ok(12011).
   • m15 (a walk refused for its path keeps its links): EXIT=101, walk.rs:283, 16717217 against 16621121, a difference of 96,096 = 8 × 12,012.

   The arithmetic I asked for was wrong, and the implementer's is right. 12,012 nodes are 48,048 bytes a link vector and 96,096 for both; the path is 5 × 12,000 = 60,000. With room R between one vector and two, the correct code refuses at the links, and a walk charged one vector has R − 48,048 < 48,048 < 60,000 left and is refused for its path: both arms Err(Bound), nothing told apart. The byte-exact pair pins more than my sizing would have: with links + path left the walk is admitted and the meter reads afterwards what it read before, so a charge a byte greater, a take without its give or a give without its take is red at walk.rs:275-277; with one byte less the links fit (96,096 ≤ 156,095) and the path does not (60,000 > 59,999), so the refusal reached is the second one, and any smaller charge of either part walks and is red at :281. The sum is held from both sides and the second refusal's give-back by :283. The test's own arithmetic (8 * nodes + 5 * TABLES * LEVELS) shares the node count with the walk and nothing else; leave asserts the room it left.

3. host green at the head: OPEN. The pull request is a draft and the check reads SKIPPED; the body says "no result yet". Clippy has run nowhere on this branch.

4. guest / suite green at the head: OPEN. Same state; the body now owes it by name, for load, evaluate, Machine::new and finish.

Round 1's NOTEs

All five closed: the second-DSDT assertion is at hostile.rs:614-617 and m13 is red on it (EXIT=101, left: Usage { steps: 106, waited_us: 7000, live: 3540 }); #764's head is merged and the one conflict resolved (below); the body carries the harness's command and log lines, says the log records no exit code and that it is of 26b40a452, and the figures rest on nothing the round changed; the issue's bullet says only what the dry run read; the body's false sentence on guest tests is gone.

What changed since a6cd8521b

• The round's commit 14bfcea31 touches four files, aml/src/exec.rs, aml/src/lib.rs, aml/tests/hostile.rs, aml/tests/walk.rs: the deletion, the load assertion and the new test, read whole. No other source moved.
• The merge f2a90a291: git show --remerge-diff has one conflict, in the track issue's meter paragraph, resolved as this branch's reading of the real tables under Power-off goes through the ACPI server: the claim's holder supplies the sleep type, and the kernel's S5 byte scan is deleted #764's "Owner: this stage"; the merge commit also rewrites the walk bullet and adds the wait bullet, which is its message's to say and it does. Outside the issue the merge is mechanical: the files a6cd8521b..f2a90a291 changes are exactly Power-off goes through the ACPI server: the claim's holder supplies the sleep type, and the kernel's S5 byte scan is deleted #764's (git diff --name-only 1084ddc9a 52ef7a10f).
• The thirteen patches pass git apply --check at the head; each log shows Finished, one test run and that test's FAILED line. metadata EXIT=0, aml EXIT=0 (27/19/24/18/18/6, the library's 0), acpiserver EXIT=0 (28 passed).
• Clean room and privacy: unchanged; the round's diff, commit messages, body and comment cite the specification alone and hold counts only.

BLOCKER

• PR body, "Gates" — cargo run -- --ci host green at the head that lands is absent — owed (round 1's, open).
• PR body, "Gates" — guest / suite green at the head that lands is absent — owed for the changed load and evaluate paths (round 1's, open).
• 14bfcea31 itself — its diff against origin/main carries Power-off goes through the ACPI server: the claim's holder supplies the sleep type, and the kernel's S5 byte scan is deleted #764 at 52ef7a10f, 28 files of kernel, ABI, toyos-acpi, toyos-userbound and server outside this slice, at a head Power-off goes through the ACPI server: the claim's holder supplies the sleep type, and the kernel's S5 byte scan is deleted #764's own review has sent to a further round — this head is not to be made ready or queued; either path below closes it.

NOTE

• PR body (prose) — "so a later merge of main is empty there" holds only while Power-off goes through the ACPI server: the claim's holder supplies the sleep type, and the kernel's S5 byte scan is deleted #764's further round leaves the meter paragraph and the two "Owner: this stage" lines as 52ef7a10f has them.

The landing

What in this branch depends on S2: nothing in code. #764 touches toyos-acpi (the dsdt module and FADT_X_DSDT's pub go) and, in the interpreter's crate, aml/tests/namespace.rs alone. This branch's eight files are disjoint from #764's and from #761's but for the track issue. The crate's sources name toyos_acpi::Table, Phys, SDT_REVISION and SDT_HEADER_LEN, none of which #764 moves, and round 1 measured the crate green on main's toyos-acpi at a6cd8521b. The only tie is the issue's text: three "Owner: this stage" lines taken from #764, where main says "the power-off stage", a stage that exists on main until #764 deletes it.

Path B, before #764 (the shorter one). #763 and #764 are both drafts with rounds open, and #764 waits on #763; this branch waits on neither.

1. Rebuild the branch without f2a90a291: a6cd8521b, a merge of origin/main (git merge-tree --write-tree a6cd8521b origin/main exits 0, no conflict), 14bfcea31's four files as they are (none is touched by Power-off goes through the ACPI server: the claim's holder supplies the sleep type, and the kernel's S5 byte scan is deleted #764 or A claim keeps where its BARs are and no object over them: a BAR asked for again after its handle closed no longer panics the kernel, and two answers held at once map apart #761), and the issue's two bullets (the walk's, the wait limit's) rewritten onto main's text, leaving main's "Owner: the power-off stage" where this branch does not own the line. The wait bullet's "Owner: this stage" stands: its list is the interpreter stage's on main too. Force-push; the branch is its implementer's.
2. Rerun the round's script at the new head: cargo metadata --locked, cargo test -p toyos-aml, the server's tests, the thirteen mutations. The measurements at 14bfcea31 do not carry over: the new head compiles main's toyos-acpi and A claim keeps where its BARs are and no object over them: a BAR asked for again after its handle closed no longer panics the kernel, and two answers held at once map apart #761's toyos-abi/src/syscall.rs.
3. The body restated against main: no carried commits, the gates' new head.
4. A round 3 that reads one thing: git diff 14bfcea31 <new> -- userland/acpiserver/aml/src userland/acpiserver/aml/tests/{heap,hostile,walk}.rs empty, the issue hunk, and the rerun's logs.
5. Ready: host and guest / suite green at that head, then the queue.

Its price is paid by #764: its next merge of main meets the conflict in the meter paragraph from the other side, and reruns the interpreter's and the server's tests against the changed crate, which its host does anyway.

Path A, after #764. Once #763 and #764 are on main: merge main (it brings #761, #763 and #764's final head; git merge-tree --write-tree 14bfcea31 origin/main is clean today, and stays clean only if #764 appends to 52ef7a10f and leaves the issue's paragraph alone); rerun the same script at the merged head, because #764's round changes toyos-acpi and the server again and the mutations rest on their stillness; restate the body, whose own diff is then the diff against main; a round 3 on the merge's --remerge-diff; then host and guest / suite on the ready pull request. The same work as B, after two other landings.

The checks the landing rests on, either path: host (clippy included, which has never run here), and guest / suite for Interpreter::load and evaluate, which the server runs at every boot (acpi_tables_loaded, the power-off rows). No guest test is added or changed by this slice; none is owed a tier argument.

SEND BACK

[Japabu]

Review of bd7b86547 against origin/main d78350c71, round 3. Read only: no cargo, QEMU or test was run by this review; every measurement named is the rerun's log (exits, aml.log, acpiserver.log, mut/*.log, all at bd7b86547) or is owed.

Net lines (git diff --shortstat origin/main...bd7b86547): 8 files, +703 −29. Production src/: +253 −17. Tests: +416 −0. Issue text: +34 −12.

Round 2's BLOCKERs

1. The head carried Power-off goes through the ACPI server: the claim's holder supplies the sleep type, and the kernel's S5 byte scan is deleted #764: CLOSED. git merge-base --is-ancestor f2a90a291 bd7b86547 exits 1; git log origin/main..bd7b86547 is five commits, 26b40a452, a6cd8521b, c20b02f3c, 885d8d327, bd7b86547, none another pull request's; git diff origin/main...bd7b86547 --stat is the track issue and seven files under userland/acpiserver/aml. The merge c20b02f3c is mechanical: git merge-tree --write-tree a6cd8521b d78350c71 exits 0 with tree bf4d7a830, which is c20b02f3c^{tree}. 885d8d327 is 14bfcea31 carried: the same four files, +74 −41 in both.
2. host green at the head: OPEN, unmeasured. Run 37768135128 is at bd7b86547 and in_progress; host reads in_progress with no conclusion. Clippy has still run nowhere on this branch.
3. guest / suite green at the head: OPEN, unmeasured. In the same run toolchain / build is in_progress and no guest / suite job exists yet. The only guest entry gh pr checks shows is skipping, from the draft's run 37767915144: it is not a result.

What round 3 was to read

• The code is the code round 2 reviewed. git diff 14bfcea31 bd7b86547 -- userland/acpiserver/aml/src userland/acpiserver/aml/tests/heap.rs userland/acpiserver/aml/tests/hostile.rs userland/acpiserver/aml/tests/walk.rs is 0 bytes. No evaluate_within and no wait_us under userland, issues or .claude at the head.
• The track issue's hunk. One hunk against main: the meter paragraph's reading of the real tables, the walk bullet, the wait bullet. main's "Owner: the power-off stage, which gives the server its memory" is kept, and that stage exists at the head (**Stage: power-off through the server**, line 292); the two other power-off-stage owners (lines 172, 192) are untouched, as is everything Power-off goes through the ACPI server: the claim's holder supplies the sleep type, and the kernel's S5 byte scan is deleted #764 rewrites. The wait bullet's "Owner: this stage" stands at line 253, inside the list under **Stage: the interpreter** (line 137) and before "What the server's load of the tables ... leaves open" (line 258): an owner that exists, and an exit a reading on the T14 and usage can meet. The walk bullet says only what the dry run read and what nothing has. issues/README.md is untouched and the hunk moves no front matter.
• The rerun, at bd7b86547, tree clean before and after. metadata EXIT=0 (metadata.log empty). aml EXIT=0: evaluate 27, heap 19, hostile 24, namespace 18, regions 18, walk 6, the library's 0, no failure in any. acpiserver EXIT=0: 26 passed, compiled against main's toyos-acpi (28 at round 2's head were Power-off goes through the ACPI server: the claim's holder supplies the sleep type, and the kernel's S5 byte scan is deleted #764's two more). Thirteen mutations, each EXIT=101, each log one Finished, one test run, and that test's FAILED line with its assertion: m3 hostile.rs:546 Err(Bound(..)) against Ok(Integer(1)); m12 and m14 walk.rs:281 left: Ok(12011); m15 walk.rs:283 16717217 against 16621121; m13 hostile.rs:617 steps: 106, waited_us: 7000; m8 hostile.rs:605; m9 hostile.rs:585; m10 hostile.rs:596; m6 heap.rs:267; m7 heap.rs:263; m1 walk.rs:37; m2 walk.rs:127; m11 walk.rs:168. No patch refused to apply and the tree read 0 changed after each.
• The body against the head. Its head, commit list, stat, ancestry exit, gate figures and mutation table are the tree's and the logs'. Its host and guest / suite rows say "no result yet", which is true.

BLOCKER

• PR body, "Gates" — cargo run -- --ci host green at bd7b86547 is absent — owed since round 1; the run that answers it is in progress.
• PR body, "Gates" — guest / suite green at bd7b86547 is absent — owed for the changed Interpreter::load, evaluate, Machine::new and finish, which the server runs at every boot; the run that answers it has not started that job.

Nothing else is open, and neither of these is the implementer's to answer: no change to the branch is asked. They close by reading, not by another round.

What the landing rests on

Run 37768135128, or a later ci.yml run, whose headSha is bd7b865475d4a1212193cd696e1c259c006bbf0a. A push moves the head and voids this.

1. host: conclusion success, and in its log the step cargo run -- --ci host ending with exit 0. This is the first clippy over the slice: a lint on userland/acpiserver/aml/src/{lib,namespace,exec,object}.rs or the three test files reds it here and nowhere earlier.
2. toolchain / build: conclusion success. guest runs whatever toolchain concluded, so it is read by itself.
3. guest / suite: conclusion success, not skipped (the draft's skipped guest is the entry gh pr checks shows now), and in its log the step cargo run -- --ci guest ending with exit 0, with acpi_power_button among the tests that ran and passed: it is the guest test whose judge reads the server's load (tests/common/power.rs:619 calls acpi_tables_loaded on the boot's log), so it is where a regression in load or in \_S5's evaluate shows.

All three green at that head: both BLOCKERs are closed by those readings, the body's two "no result yet" rows are replaced by the run's number, jobs and exits, and the merge is armed. Any of them red, cancelled or skipped: SEND BACK, with that job's log.

NOTE

• PR body, "Gates", guest / suite row (prose) — "acpi_tables_loaded and the power-off rows reach them" names a metal row (tests/toyos.rs:483, judged by acpi_tables_on_metal) for the guest suite; the guest test that reaches the load is acpi_power_button (tests/toyos.rs:269, :2804), through the helper of that name.
• PR body, "Gates" — the host and guest / suite rows carry the run, job and exit once read, as above.

LAND AFTER NAMED CHANGES