Skip to content

release-check: assert the NPU is held awake (power/control=on, runtime_status=active) - #1231

Merged
bong-water-water-bong merged 3 commits into
mainfrom
1bit/release-check-npu-awake
Nov 6, 2026
Merged

bong-water-water-bong merged 3 commits into
mainfrom
1bit/release-check-npu-awake

Conversation

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator

Owner decision (2026-11-05 23:4x ADT): "keep the npu awake on the stick" — c30's fix for the live-USB take's code 614. This PR is the release-check half; the image half (the udev rule that sets power/control=on for amdxdna, plus the early-boot fallback for a module loaded before udev) is the os/payload lane's.

Root cause the row exists for

c29 take, 2026-11-05 22:17:41: the NPU's driver runtime-PM suspends the array between requests, and the request that followed such a window died npu runtime: invalid call (code 614): state 8, run 0 of 28 — the picture page's prompt turn. Lemonade answered 500, the fallback model wrote the prompt, and the client saw a clean answer while the demo lost the beat. dmesg in the same second: aie2_legacy_tdr_detect + power-cycling NPU to recover + 1 context(s) skipped. Earlier NPU turns in that boot were fine (81.88 tok/s), so nothing about the model or the prompt was wrong.

What the row asserts

Two facts, read from the state this boot has — not from the intent of a rule file:

  1. power/control is on (the array cannot runtime-suspend), and
  2. runtime_status is active (it is actually running).

They are separate messages on purpose: control=on with runtime_status!=active is a different fault from control=auto. The device is found through /sys/bus/pci/drivers/amdxdna/[0-9]*, not a hardcoded PCI address, and the row waits up to 10 s for a just-resumed device to report active rather than reding a boot for the driver's workqueue. It joins HW_SKIPPED, so a VM's footer names it instead of implying it ran.

Measured

  • os/mini/release-check.sh --self-test PASS, 73 cases (7 new): on/active is the PASS; auto/suspended is the c29 take's state; auto/active, on/suspended, on/unreadable, and two no-device cases give FAIL/SKIP with distinct text.
  • The row block run against a real amdxdna device on hardware: PASS npu-power : the NPU is held awake (0000:c6:00.1): power/control=on, runtime_status=active — with zero clients holding /dev/accel/accel0, which is the property the fix depends on (control=on keeps the array up when nothing is using it).
  • tools/copyright.py --check rc 0; no new files.
  • Board expectation changes from 25 to 26 checks — the run sheets that quote the count are this lane's and get updated with the c30 gate.

Merging is the owner's (overnight rule: no merges by lanes). If this does not land before the c30 cut, the image carries c29's 25-check release check and the board's u4-release-check row will show it — the c30 hand-off says which it is.

…e_status=active)

Root cause this row exists for (c29 live-USB take, 2026-11-05 22:17:41): the NPU's
driver runtime-PM suspends the array between requests, and the request that
followed such a window died `npu runtime: invalid call (code 614): state 8,
run 0 of 28` - the picture page's prompt turn. Lemonade answered 500 and the
fallback model wrote the prompt, so the client saw a clean answer while the
demo lost the beat, and dmesg carried one `aie2_legacy_tdr_detect` plus a
power-cycle of the array.

Owner decision (2026-11-05 23:4x ADT, "keep the npu awake on the stick"): the
c30 fix is to never let the array runtime-suspend - a udev rule setting
power/control=on for the XDNA2 NPU plus an early-boot fallback for a module
that loaded before udev - and the release check gets a row for it.

This commit is the row (the image half is PR #1230: 90-1bit-npu-power.rules and
04onboard-npu-keepawake). It reads the state THIS boot has rather than trusting
a rule file, and asserts both facts, because power/control=on with
runtime_status!=active is a different fault with a different message. The
device is matched on PCI device id 0x17f0 - the same id the image's udev rule
matches (pci 1022:17f0) and the firmware row tests for - so a machine without
an NPU skips in the image's own words ("no XDNA2 NPU"). It waits up to 10 s for
a just-resumed device to report active rather than reding a boot for the
driver's workqueue, and it joins HW_SKIPPED so a VM footer names it instead of
implying it ran.

Measured:
  * `os/mini/release-check.sh --self-test` PASS, 73 cases (7 new: on/active is
    the PASS; auto/suspended is the c29 take's state; auto/active,
    on/suspended, on/unreadable and two no-device cases give FAIL/SKIP with
    distinct text).
  * the row block run against a real amdxdna device on hardware prints
    `PASS npu-power : the NPU is held awake (0000:c6:00.1): power/control=on,
    runtime_status=active`, with zero clients holding /dev/accel/accel0 - i.e.
    control=on keeps the array up when nothing is using it, which is the
    property the fix relies on.
  * board expectation changes from 25 to 26 checks (docs/run sheets that quote
    the count are the release lane's to update).
@bong-water-water-bong
bong-water-water-bong force-pushed the 1bit/release-check-npu-awake branch from 1b8d2b8 to f8ebc74 Compare November 6, 2026 03:27

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VERDICT: READY at f8ebc744.

Both measured claims reproduce here.

os/mini/release-check.sh --self-test -> release-check self-test PASS, and I counted the cases rather than taking the
number: 73 ok lines, exactly as the body says. The seven new ones fire with the verdicts you describe:

ok PASS power/control=on, runtime_status=active
ok FAIL power/control=auto (runtime_status=suspended): the array may runtime-suspend …
ok FAIL power/control=auto (runtime_status=active): …
ok FAIL power/control=on but runtime_status=suspended: held awake and not running
ok FAIL power/control=on but runtime_status is unreadable, so the array's state is not proven
ok SKIP no power/control to read        (x2: device absent, both fields absent)

The live row, lifted out of the shipped script and run against this box's real sysfs (function + row block taken
verbatim from the diff, helpers stubbed) gives your exact line:

PASS  npu-power : the NPU is held awake (0000:c6:00.1): power/control=on, runtime_status=active

with the device itself reporting on / active. bash -n rc 0; tools/copyright.py --check rc 0; no new files. The
shellcheck warnings that -S warning shows (SC1007, line 296) are pre-existing — origin/main's copy of the file
produces the same 5, and none is inside your hunks.

The design choices are the right ones and I checked them rather than nodding: npu-power joins HW_SKIPPED, so a VM's
footer names it instead of implying it ran; the device is found by id under /sys/bus/pci/devices/* rather than
0000:c6:00.1; and control=on + runtime_status!=active gets its own message, which is the distinction the c29 take
needed (it would have read auto/suspended).

Two nits, neither blocking:

  1. A kernel that reports runtime_status=unsupported would red this row with "held awake and not running", when it
    actually means the device cannot runtime-suspend at all — i.e. the property the row exists for already holds. Not
    reachable on amdxdna as far as I can see (PM off means no power/ dir and the row skips), but it is a false red on a
    release gate, so a case arm that PASSes with a note would cost nothing.
  2. The device is matched on the PCI device id alone, where the image's udev rule matches vendor and device. That is
    the same asymmetry I flagged on #1230's 04onboard-npu-keepawake — worth making the three agree (0x1022 check), so
    the row cannot report a wrong-vendor 0x17f0 as the NPU. Here the loop also keeps the last match (no break),
    which matters only in that impossible case.

Board count 25 -> 26 is consistent with what I read: the row is one pass/fail/skip on every boot that has an NPU,
and a skip on every VM boot.

I did not run it on the board — the on/active row is this box's, and the c29-shaped red (auto/suspended) is the
self-test's, which is the right instrument for it.

@github-actions

github-actions Bot commented Nov 6, 2026 •

Copy link
Copy Markdown

PR Reviewer Guide 🔍

(Review updated until commit 4b7c37c)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Power State Assertion Logic

The new npu_power_verdict function and its integration into the release check logic assert that the NPU's power/control is on and runtime_status is active. However, the function does not validate that the power/control and runtime_status values are valid, which could lead to false positives if invalid values are read from the sysfs entries. This could mask actual hardware issues or misconfigurations.

npu_power_verdict() {
    _c=$(printf '%s' "${1:-}" | tr -d ' \n\r\t') _s=$(printf '%s' "${2:-}" | tr -d ' \n\r\t')
    if [ -z "$_c" ]; then
        printf 'SKIP no power/control to read'; return 0
    fi
    if [ "$_c" != on ]; then
        printf 'FAIL power/control=%s (runtime_status=%s): the array may runtime-suspend between requests, which is the state the demo'"'"'s prompt turn died in' "$_c" "${_s:-unreadable}"
        return 0
    fi
    case "$_s" in
        active) printf 'PASS power/control=on, runtime_status=active' ;;
        '')     printf 'FAIL power/control=on but runtime_status is unreadable, so the array'"'"'s state is not proven' ;;
        *)      printf 'FAIL power/control=on but runtime_status=%s: held awake and not running' "$_s" ;;
    esac
}
Race Condition in Power State Check

The code waits up to 10 seconds for the NPU to transition to active state after setting power/control=on. While this attempts to handle a race condition, it does not guarantee that the NPU will transition to active within the timeout. If the NPU fails to transition to active within 10 seconds, the check will report a failure, potentially masking a hardware or driver issue that requires manual intervention.

    # A device the udev rule just resumed can take a moment to report `active`, and that race is not a fault: wait for
    # it rather than red a boot for the driver's own workqueue. Ten seconds, then the row says what it sees.
    _i=0
    while :; do
        _c=$(cat "$NPUDIR/power/control" 2>/dev/null)
        _s=$(cat "$NPUDIR/power/runtime_status" 2>/dev/null)
        [ "$_c" = on ] && [ "$_s" = active ] && break
        _i=$((_i + 1))
        [ "$_i" -lt 10 ] || break
        sleep 1
    done
    VP=$(npu_power_verdict "$_c" "$_s")
    case "$VP" in
        PASS*) pass npu-power "the NPU is held awake ($(basename "$NPUDIR")): ${VP#PASS }" ;;
        SKIP*) skip npu-power "${VP#SKIP }" ;;
        *)     fail npu-power "$(basename "$NPUDIR") ${VP#FAIL }" ;;
    esac
fi

@github-actions

github-actions Bot commented Nov 6, 2026

Copy link
Copy Markdown

Persistent review updated to latest commit 6316c7f

@github-actions

github-actions Bot commented Nov 6, 2026

Copy link
Copy Markdown

Persistent review updated to latest commit 4b7c37c

@bong-water-water-bong
bong-water-water-bong merged commit 809ec9c into main Nov 6, 2026
9 checks passed
@bong-water-water-bong
bong-water-water-bong deleted the 1bit/release-check-npu-awake branch November 6, 2026 10:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant