[Japabu]

Test defects the AMD bench's testcases boot showed under #1154, fixed rather than recorded. The T14-only rows that ride the AMD's boot, and the AMD's row on the T14, stay with issues/a-metal-row-names-no-machine-so-another-benchs-boot-carries-it.md.

Problem

• Waits counted from boot. idle_entered, acpi_hold, ps2server_hold and tone_after_dc6 gave up their log wait at a fixed time after boot (JOB_BOUND_MS less a tenth, 54 s), so a job begun late failed before it looked. On the AMD at 938c4a7, idle_entered and acpi_hold started at 59.6 s and 62.8 s and ended within 0ns, over lines the server had written at 6–7 s.
• counters hung on an uncontrolled event. counters_metal began its idle second only once the embedded controller raised its first query. The AMD's controller raises none, so the job waited out 10 s and failed. On the T14 a later boot query could land in the idle second (issues/the-counters-rows-idle-second-can-hold-the-ecs-second-boot-query.md).

Change

• Waits from the job's own start. served_log::START_LINE, 30 s, bounds a wait for a line a service writes during its start: a liveness ceiling, not a timing claim. idle_entered, acpi_hold, ps2server_hold and tone_after_dc6 wait by it.

  • acpi_hold now waits for acpiserver_api::STARTED, not the power sources a machine without a battery never writes.
  • acpi_counted keeps its boot-counted rule (1d0d558). An own-start bound for a count the AMD never writes overran the runner's 60.3 s list bound there in r3, and ended the list inside it. Bounding it from its start needs the runner to hand jobs their remaining bound, which stays with the rows record.
  • The test runner's whole-list deadline counts from boot by design: it bounds the list, not a job.

• The end of the server's start, one spelling. acpiserver writes acpiserver_api::STARTED after served_aml, the last line of its start on every machine. counters_metal waits for it under START_LINE, and ACPI_BOUND is gone.

• The counters row's claim, narrowed (orchestrator rulings). Nothing ToyOS writes on its own account lands in the idle second. idle_second_lines admits, and the row prints, only:

  • acpiserver's first take of a query or GPE (acpiserver_api::FIRST_TAKEN) and the kernel's first ACPI fixed-hardware interrupt, each only where the whole log holds its words once;
  • the clock's TSC refinement report (kernel::clock::TSC_REFINED), written by a scheduler pass and arming no timer;
  • the clock's TSC step report (kernel::clock::TSC_STEP_READ), which the AMD bench's deep idle provokes once a boot (issues/the-amd-benchs-tsc-steps-1-7-ms-forward-once-a-boot.md).

  Every other line is refused, the clock's slewing of a step and acpiserver's periodic count among them.

• One spelling each. acpiserver writes FIRST_TAKEN and STARTED through acpiserver-api. The kernel writes TSC_REFINED and TSC_STEP_READ through kernel::clock. The judge, acpi_events_on_metal, clock_report and the two QEMU clock tests match on those constants.

• Records.

  • Closed: issues/the-counters-rows-idle-second-can-hold-the-ecs-second-boot-query.md and issues/acpi-hold-gives-up-at-a-time-counted-from-boot-whatever-its-runner-was-given.md.
  • issues/acpiserver-lines-are-spelled-again-by-their-readers.md drops the spellings moved into acpiserver-api.

Tests

• Host: checks::the_idle_second_admits_only_the_machines_own_lines runs idle_second_lines over lines in the readbacks' own form.

  • Passing, and reported: the T14's first queries, the first fixed-hardware interrupt, a first GPE, a TSC refinement report and the AMD's step report.
  • Refused: acpiserver's periodic count, a program's line, a first time said twice, and the clock's slewing line.
  • A line outside the second is not judged.

• Controls, each red as intended:

  • a classifier admitting every line;
  • a refinement arm admitting any clock: line;
  • a step arm admitting any clock: line.

• QEMU: a_stepped_tsc_is_slewed_then_demoted now requires the step report, read through kernel::clock::TSC_STEP_READ. a_stalled_hpet_is_left_out reads the refinement report through TSC_REFINED. Both pass at e80dee1.

• Also exit 0: clippy, acpiserver's and acpiserver-api's host tests, and kernel's clock tests at e80dee1. The head 31d1c68 changes only comments, and its checks and the kernel library compile.

• Metal r5 at e80dee1, both benches:

  • The orchestrator's runner recorded EXIT=0 for all six boots (t14-q-acpi5-t14-1..3, amd-q-acpi5-amd-1..3). The harness judge, run offline over each bench's readbacks, gives the table below.
  • On the AMD, counters' idle second passes with the step report admitted (7.635 s). It then fails at the T14's HWP lines.
  • Every changed job ran to its end on both benches.

  Evidence: https://github.com/ToyOSOrg/evidence/tree/653e90b25e9e54cd5fabf7409a443b84fdb77d46/pr-1163/metal-r5

Row	T14	AMD
counters	PASS (refinement admitted, 12.538 s)	FAIL: T14 row (HWP pm_enable lines; the AMD has no HWP)
acpi_server_events	PASS	FAIL: T14 row (acpi_counted)
acpi_tables_loaded	PASS	PASS
idle_entered	FAIL: AMD row	PASS
ps2server_arms	PASS	PASS
display_output_presents	PASS	FAIL: T14 row (no tgl-display)

Every red is a row on the other bench's boot, under the rows record above.

Round evidence:

• r1: https://github.com/ToyOSOrg/evidence/tree/6df178503cc4bd4fe8ac4fe8d81fca0528e3e502/pr-1163/r1
• r5: https://github.com/ToyOSOrg/evidence/tree/0daa14f8cb7b0385c5748dc33072ea692708ee5e/pr-1163/r5

🤖 Generated with Claude Code

[Japabu]

Review of #1163 at 441d30473 (reviewer, read-only)

Architecture. Timing idle_entered's and acpi_hold's waits from the job's own start is the correct fix: a wait on a line already in the log should not depend on how long earlier jobs took. Narrowing the counters row to admit first-time ACPI events is a reasonable claim change. The idle busy fraction and package power over the idle second are read and not held, so admitting the machine's own SCI loses nothing the row holds. The row still refuses every other line, and it reports admitted events instead of hiding them. Moving the server's " taken for the first time" into acpiserver-api is the right owner for that string. Net: +103/−92, including a 36-line host test.

On whether the row still catches regressions: a stray program line or acpiserver's periodic count inside the second is refused, and the host test pins both, with a negative control. A periodic timer that writes nothing to the log was never caught by this row, before or after the change, because the row judges lines only. That is unchanged and outside this PR.

Required

1. counters_metal's new anchor is not the end of the server's start, and the module doc says it is. tests/guest/rust-tests/src/bin/counters_metal.rs:14-19,186-195: acpi_said now returns at CONTROLLER_SERVED/CONTROLLER_NONE, and the header calls that "the end of what its start writes before any event". In system/acpiserver/src/main.rs:177-186, arm_controller comes before:

   • negotiate (the namespace line, after _OSC and any run-time loads);
   • declare_idle (IDLE_CENSUS);
   • arm_gpes;
   • find_power (POWER_SOURCES, after the batteries' first reading through the EC);
   • served_aml.

   Each of those lines is an ordinary start line, not a FIRST_TAKEN event, so the judge refuses it. Before this change, the first-query anchor on the T14 happened to fall after the start (query 0x40 at 12.52 s, start ended about 12.1 s). The PR's own figures put the T14's start end at 12.1 s, which is about when the boot's jobs begin. So a T14 boot where counters_metal starts near then can now place the census or power-sources line inside the idle second and red the row. That is a new flake of the kind this PR is closing. The staged metal request covers only the AMD, where the start ended at 7.0 s, long before the job. Anchor acpi_said on the server's last start line, and give it the same served_log::SERVER_START ceiling, which removes the separate 10 s ACPI_BOUND. If no line ends the start on every machine (POWER_SOURCES needs a battery), add one to acpiserver-api and have the server write it after served_aml. Correct the header to match.

2. FIRST_TAKEN is not yet the one source. tests/toyos.rs:7402 and :7405 (acpi_events_on_metal) still spell " taken for the first time" / "taken for the first time" literally beside the new constant. Match through acpiserver_api::FIRST_TAKEN there too.

3. Directly affected issue records were not updated.

   • issues/acpi-hold-gives-up-at-a-time-counted-from-boot-whatever-its-runner-was-given.md: its exit ("The hold is bounded from its own start … a job added before it cannot red acpi_server_events over a line the log holds") is what this PR implements for acpi_hold. Close it in this change and move its pointer in issues/a-t14-boot-that-outlogs-its-retention-loses-its-middle-and-the-rows-whose-lines-sat-there.md:86.
   • issues/acpiserver-lines-are-spelled-again-by-their-readers.md: the counters_metal.rs / acpi_said bullet is now stale, since that function matches only acpiserver-api constants. After item 2, taken for the first time also leaves the tests/toyos.rs bullet. Update the list.

Optional

• tests/toyos.rs:7183-7187 — the counters_on_metal doc sentence ("either edge's included because a line stamped in it may follow the read, but a first-time ACPI event's (…), which the machine raises when it chooses and the row reports: the second is the idle machine's, its own events aside") no longer parses on one read. Suggested wording: "No line is stamped in a millisecond from idle0's to idle1's, both edges included since a line stamped there may follow the read, except a first-time ACPI event ([idle_second_lines]), which the row reports."
• lib/acpiserver-api/src/lib.rs:11-14 — "What closes the server's word on an event" is inaccurate, because both lines continue after it (; its method ran, ; served by …). Suggested wording: "Follows the event's name in the server's line on the first take of a query or GPE."
• idle_second_lines admits any acpiserver line that contains FIRST_TAKEN. If a server regression repeated a "first time" line, the row would pass it. Requiring each admitted line's text to appear only once in the boot's log would keep "first time" true at no cost.
• tests/toyos.rs:7145-7146 — "named by its claim's row" adds nothing. (kernel/src/isa.rs) is enough.

Bounds. SERVER_START (30 s from the job's start) is justified at its site as a liveness ceiling well above the observed 12.1 s and 7.0 s starts. acpi_counted keeping its boot-relative rule is stated and stays under the existing T14-rows record.

Evidence. The host test, its negative control, clippy and the acpiserver tests are adequate for the judge. The guest-side wait change is still unproven on metal until the orchestrator's AMD boots. After required item 1, a T14 testcases boot is also needed, because item 1's risk is on the T14.

Verdict: SEND BACK.

[Japabu]

Follow-up review of #1163 at e80dee12b (reviewer, read-only)

Scope: the r1 required items, the two later commits that widen what the idle second admits, and the r5 metal evidence. Since 441d30473 the branch adds 141 lines and removes 150. That includes the closed record (−40) and three boot-counted waits replaced by START_LINE.

Architecture. acpiserver_api::STARTED, written once after served_aml and before serve, is the right anchor. The server owns the end of its start, and the API crate holds the one spelling of it. Every boot-counted job wait except acpi_counted's now waits from the job's own start. acpi_counted's exception (1d0d558) has a stated reason: an own-start bound overran the AMD's list bound in r3. Its exit lies with the rows record, so I am not reopening it. The kernel report heads moved into kernel::clock (pure), and the kernel writes through them. That is one spelling shared by producer and readers, not a new mechanism.

r1 required items

1. Anchor: fixed. counters_metal::acpi_said waits for STARTED under START_LINE, and ACPI_BOUND is gone. In system/acpiserver/src/main.rs:186 the line follows negotiate, declare_idle, arm_gpes, find_power and served_aml, so no start line can follow it.
2. FIRST_TAKEN: fixed at tests/toyos.rs:7414,7418.
3. Records: fixed. The acpi_hold record is deleted per issues/README.md, with no remaining reference to its slug outside issues/. The pointer is replaced, and the spelled-again list is updated. The optional once check was taken, and the host test covers a first time said twice.

Do the two "admits" commits stay narrow?

Yes. Each admitted clock line is kernel-sourced and matched by a prefix with exactly one producer: TSC_REFINED at clocksource.rs:839,845 and TSC_STEP_READ at :731. The clock's other lines, its slewing among them, are still refused, and the host test pins that. Neither line can hide a wakeup ToyOS causes. clocksource::check runs only from sched/driver.rs:526, on a scheduler pass that happens anyway, and is throttled to once per CHECK_EVERY_NS. So the report arms no timer and wakes no CPU of its own. The refinement is bounded by REPORT_FACTOR doubling. A second step demotes the TSC, and the demotion line is refused. A silent wakeup was never visible to this line-based row, as settled in r1.

Required

1. Comments now state a wider refusal and a stronger claim than the code holds.
   • tests/guest/rust-tests/src/bin/counters_metal.rs:14-19: "the counters row reports those lines there and refuses any other". Since 6fb03ce/e80dee12b, the row also admits the TSC refinement and step reports, so this is false. It also keeps a second copy of the admitted set, whose one home is idle_second_lines. Cut the paragraph to the wait plus a pointer, for example: "idle0 waits for the end of the ACPI server's start ([acpi_said]). What the counters row admits inside the idle second is idle_second_lines's (tests/toyos.rs)."
   • tests/toyos.rs:7186-7187: "what it holds is that ToyOS itself does not wake the idle machine". The row judges lines. A wakeup that writes nothing passes it, as r1 settled. Say what it holds, for example: "...each of which the row reports: nothing ToyOS writes on its own account lands in the second."

Optional

• tests/toyos.rs:7147-7158: the idle_second_lines doc is one sentence with nested parentheses ("Admitted are what the idle machine says of itself at times it does not choose by the job"). A list would read at a glance: "Admitted: acpiserver's first take of a query or GPE ([acpiserver_api::FIRST_TAKEN]) and [ACPI_FIRST_INTERRUPT], each only where the whole log holds its words once; the clock's TSC refinement ([kernel::clock::TSC_REFINED]), written on a scheduler pass and arming no timer; and its step report ([kernel::clock::TSC_STEP_READ]), which the AMD bench's deep idle provokes once a boot (issues/the-amd-benchs-tsc-steps-1-7-ms-forward-once-a-boot.md)."
• kernel/pure/clock/mod.rs:9-12: "written at spans the clock schedules after calibration, on no event" suggests the report has a schedule and wakeup of its own. "Written by the first scheduler pass after each doubling span" is accurate. It is also the fact that justifies admitting it.

Evidence (metal-r5, 653e90b25)

• The judge logs at e80dee1 support each claim. On the T14, counters passes with the refinement admitted and printed (12.538 s). The idle second starts after STARTED (11.574 s), which covers r1's T14 risk.
• On the AMD, idle_second_lines returned Ok with the step report printed (7.635 s, inside idle0 6.634 s → idle1). The row then fails at control_regs: cpu0 pm_enable=1, a T14 HWP check.
• idle_entered, acpi_tables_loaded and ps2server_arms pass on the AMD. acpi_server_events, ps2server_arms and display_output_presents pass on the T14, so every changed wait was exercised. No job was cut off by the list bound on either testcases boot.
• Every remaining red is a row on the other bench's boot: idle_entered on the T14; and on the AMD counters (HWP), acpi_server_events (acpi_counted) and display_output_presents (no tgl-display, tone_after_dc6 timing out at 30 s from its own start). All fall under issues/a-metal-row-names-no-machine-so-another-benchs-boot-carries-it.md.
• The six boots' exit 0 is the orchestrator's statement. The published excerpts do not show it, and amd-shared-debug-excerpt.txt is empty. No row judged here depends on it.
• The host test, its controls and the QEMU tsc_step/hpet_stall runs at e80dee1 are in r5. The two required edits touch comments only, so a compile is enough.

For the orchestrator: the PR body's Change and Tests sections still describe 441d30473: the controller-word anchor, SERVER_START, the old test name, and "refuses any other line". Later rounds are appended below them.

Verdict: SEND BACK (comment accuracy only).