From 3963a1b70a572132aced1c1a0033e1c8249a0c7e Mon Sep 17 00:00:00 2001 From: hathach Date: Fri, 14 Aug 2026 01:08:40 +0700 Subject: test/hil, ci: contain a wedged USB stack instead of stranding the runner A wedged USB device used to take the whole HIL run with it. Every worker that touched the poisoned node blocked uninterruptibly, the pool could not be joined, map_async discarded every board's result, and the job ran to the GitHub ceiling with no report at all -- while the self-hosted runner's single job slot stayed occupied and every queued job waited behind it. Bound the calls a worker makes itself. read_sysfs, bounded_open and run_cmd all answer within a wall clock; read_sysfs distinguishes "absent" from "unknown", because a blocked read is not evidence of absence, and caps stranded readers at four (each costs a thread and an fd for the life of the process) after which the worker declares itself blind. mtype, the gio unmount, the libmtp session and the arecord/iperf reaps go through those bounds; the MTP session runs in a disposable subprocess, since libmtp's ctypes calls block unkillably in D state. Bound the run. A pool guard (HIL_POOL_TIMEOUT, 60 min) fires before any job ceiling and still writes a report. When the pool will not shut down, the sweep kills what the workers spawned -- descendants, not just direct children, since flashers run in their own session -- confirms each kill actually landed, and exits early so the runner is freed. Whatever survived is named in the report. Deliberately shallow past that point. We do not re-scan process groups, prove pid ownership, or escalate through sudo: a root-owned survivor is reported, not force-killed, because signalling a pid we cannot prove is ours is the worse failure, and the job ceiling backstops whatever this misses. A D-state holder was never killable anyway. Recover instead of reporting a wedge. A HUNG usbtest case reflashes its own DUT through its roster flasher, but only where the flasher can reach its probe past a poisoned node -- openocd pinned to a validated vid_pid, or esptool. Where it cannot, the run says so rather than reserving budget for a path that cannot fire. Raise the CI ceilings above the pool guard so the guard fires first and still writes its report, and pin --retry 1 on every HIL leg: the guard is a flat constant and does not scale with max_retry, so argparse's default of 3 would triple the serialized usbtest tail against an unchanged guard. Split the module: execution in hil_test/hil_flash/usbtest, infrastructure in helper/ (locking, health, selection, shared bounded IO), and the two matrix generators into .github/scripts/ -- ci_set_matrix.py sat in workflows/, where GitHub treats every file as a workflow definition. 193 tests cover the bounded paths, the kill ladder, the guard and the selector against synthetic /proc trees and PATH-injected fakes; a real wedge cannot be manufactured on demand. --- .../2026-07-29-hil-pr-scoped-selection-design.md | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) (limited to 'docs') diff --git a/docs/superpowers/specs/2026-07-29-hil-pr-scoped-selection-design.md b/docs/superpowers/specs/2026-07-29-hil-pr-scoped-selection-design.md index 8158758bc..898b3c8ab 100644 --- a/docs/superpowers/specs/2026-07-29-hil-pr-scoped-selection-design.md +++ b/docs/superpowers/specs/2026-07-29-hil-pr-scoped-selection-design.md @@ -1,4 +1,4 @@ -# PR-scoped HIL selection: hil_select.py +# PR-scoped HIL selection: helper/hil_select.py **Date:** 2026-07-29 **Branch:** `claude/hil-select` (based on `claude/hil-pool-check`, which carries the @@ -26,17 +26,17 @@ confident; every uncertainty widens to the full matrix. - Scoping push/master/scheduled runs (always full). - Changing hil_test.py behavior (the selector only *composes* existing `-b`/`-bt` args). -## Component: `test/hil/hil_select.py` +## Component: `test/hil/helper/hil_select.py` Stdlib-only, importable and CLI. Lives beside the harness so `hil_ci.sh` copies are unaffected (it runs on the GitHub runner / dev PC, not on the rig). It must NOT import `hil_test.py` (which drags pyserial/pymtp onto the bare GitHub runner): the three test lists -(`device_tests`, `dual_tests`, `host_test`) move verbatim into a tiny stdlib-only -`test/hil/hil_examples.py` that both `hil_test.py` and `hil_select.py` import (behavior -preserving; `hil_ci.sh` scp list gains the new file). +(`device_tests`, `dual_tests`, `host_test`) move verbatim into the stdlib-only +`test/hil/helper/hil_util.py` that both `hil_test.py` and `hil_select.py` import (behavior +preserving; `hil_ci.sh` copies the whole `helper/` directory). ``` -python3 test/hil/hil_select.py --base [--diff-file ] CONFIG.json [CONFIG.json...] +python3 test/hil/helper/hil_select.py --base [--diff-file ] CONFIG.json [CONFIG.json...] ``` - `--base REF`: changed files = `git diff --name-only $(git merge-base HEAD REF)..HEAD` @@ -118,7 +118,7 @@ is skipped, not widened (running unrelated boards would test nothing relevant). ## CI wiring (`.github/workflows/build.yml`) - `set-matrix` (PR events only): after generating today's matrices, run - `hil_select.py --base origin/${{ github.base_ref }} test/hil/tinyusb.json test/hil/hfp.json` + `helper/hil_select.py --base origin/${{ github.base_ref }} test/hil/tinyusb.json test/hil/hfp.json` (checkout with enough history to reach the merge base: `fetch-depth: 0` on this one job, or an explicit `git fetch origin $BASE_REF`). New job outputs: `hil_select_full`, `hil_args_tinyusb`, `hil_args_hfp`, plus the selected-board list consumed by the matrix @@ -137,15 +137,15 @@ is skipped, not widened (running unrelated boards would test nothing relevant). ## Local use - pre-pr's "Map changes to boards" step delegates to - `python3 test/hil/hil_select.py --base $BASE test/hil/tinyusb.json` and derives its + `python3 test/hil/helper/hil_select.py --base $BASE test/hil/tinyusb.json` and derives its one-board-per-family sample from the selector's board set (its capping/sampling policy is unchanged — the selector provides the affected set, pre-pr samples it). -- Manual: `python3 test/hil/hil_test.py -B examples $(python3 test/hil/hil_select.py --base master test/hil/tinyusb.json | jq -r '.args["tinyusb.json"]') test/hil/tinyusb.json` +- Manual: `python3 test/hil/hil_test.py -B examples $(python3 test/hil/helper/hil_select.py --base master test/hil/tinyusb.json | jq -r '.args["tinyusb.json"]') test/hil/tinyusb.json` — documented in the hil skill. ## Testing -`test/hil/test_hil_select.py` — stdlib `unittest`, no hardware, injected diffs via +`test/hil/test/test_hil_select.py` — stdlib `unittest`, no hardware, injected diffs via `--diff-file`/API. Cases (the acceptance examples): 1. `src/portable/raspberrypi/rp2040/dcd_rp2040.c` → only rp2040-family roster boards, device tests only, host-only boards absent, `full` false. @@ -161,7 +161,7 @@ is skipped, not widened (running unrelated boards would test nothing relevant). 7. `hw/bsp/rp2040/family.cmake` → rp2040-family boards, all their tests. 8. Mixed device+host diff → no pruning (both roles present). The suite runs in `set-matrix` before the selector is used, and locally via -`python3 test/hil/test_hil_select.py`. +`python3 test/hil/test/test_hil_select.py`. ## Safety properties -- cgit v1.3.1 From f822f69a9871b2115c5213889d70411da66ca1b1 Mon Sep 17 00:00:00 2001 From: hathach Date: Fri, 14 Aug 2026 01:08:50 +0700 Subject: skills, docs: rewrite USB recovery from the live incidents Two things the rig taught us that the old guidance got wrong. A usbfs ioctl wedged in D state cannot be freed on a running kernel. It holds the device lock, so usb_disconnect() blocks behind it; reboot(2) walks device_shutdown() and takes the same lock, so every userspace reboot stalls too. Only sysrq b (emergency_restart, which skips device_shutdown) or hypervisor action clears it -- all cited to the kernel source. The recovery ladder is generic across rigs now (ci.lan, hifiphile, a bench PC) and ends at hypervisor escalation only where host access exists. Two claims are corrected outright: JLinkExe is NOT convoy-safe, and a park-flash cannot free a device-lock owner. The hil skill's banner list is what an operator agent matches a report against, so it enumerates the banners that actually exist, including the D-state note -- which is explicitly NOT a wedge, since a healthy in-flight testusb is uninterruptible for most of every case and a concurrent CI battery would otherwise turn a clean run red. --- .claude/agents/hil-operator.md | 14 +- .claude/agents/pr-monitor.md | 2 +- .claude/agents/target-debugger.md | 4 +- .claude/skills/etm-trace/SKILL.md | 2 +- .claude/skills/hil-pool-check/SKILL.md | 17 +- .claude/skills/hil/SKILL.md | 68 ++++- .claude/skills/pre-pr/SKILL.md | 2 +- .claude/skills/target-debug/SKILL.md | 4 +- .claude/skills/usb-kernel-recover/SKILL.md | 290 +++++++++++++-------- .../usb-kernel-recover/scripts/usb_recover.sh | 122 +++++---- .claude/skills/usbtest/SKILL.md | 30 +++ .claude/workflows/hil-validate.js | 4 +- .claude/workflows/pr-babysit.js | 2 +- CLAUDE.md | 8 + .../2026-07-30-hil-usbtest-fleet-wedge-design.md | 236 +++++++++++++++++ 15 files changed, 599 insertions(+), 206 deletions(-) create mode 100644 docs/superpowers/specs/2026-07-30-hil-usbtest-fleet-wedge-design.md (limited to 'docs') diff --git a/.claude/agents/hil-operator.md b/.claude/agents/hil-operator.md index 6f04f6dcc..ebc9251cc 100644 --- a/.claude/agents/hil-operator.md +++ b/.claude/agents/hil-operator.md @@ -18,19 +18,23 @@ The GitHub Actions runner keeps running during your work. Per-board flock locks - `python3 test/hil/hil_test.py ...` runs: do NOT pre-hold those boards — `hil_test.py` self-locks each board for its flash+test and would fail fast with `board locked` against your own hold. - ANY other hardware action (JLinkExe/openocd/GDB, manual flash, usbtest.py, serial poking): hold first, release when done — release is mandatory cleanup (a crashed holder auto-releases via kernel flock, but do not rely on it): ```bash - python3 test/hil/hil_lock.py hold --reason "" + python3 test/hil/helper/hil_lock.py hold --reason "" # ... hardware work ... - python3 test/hil/hil_lock.py release + python3 test/hil/helper/hil_lock.py release ``` -- Rig-wide operations (uhubctl power cycling, pci-rebind — they renumber buses): `python3 test/hil/hil_lock.py hold --all --reason ""` first. +- Rig-wide operations (uhubctl power cycling, controller resets — they renumber buses): `python3 test/hil/helper/hil_lock.py hold --all --reason ""` first. - If a lock is already held by someone else: report holder/reason (`hil_lock.py status`) — never force, never kill the holder. If the holder's reason is `hil_test.py`, that is a concurrent CI job mid-test on the board: waiting a few minutes and retrying once is appropriate when your task allows; otherwise return the holder info so the orchestrator can ask the user. - You cannot ask the user anything. Bypassing a lock (`HIL_NO_BOARD_LOCK=1`, or proceeding with manual hardware work despite a held lock) is allowed ONLY when your prompt explicitly states the user authorized forcing. ## Hard rules -- HIL runs take 2–5 min per board: use Bash timeouts >= 20 min (1200000 ms) and NEVER cancel early. +- HIL runs take 2-5 min per board, but a stuck fleet runs to `HIL_POOL_TIMEOUT` — 60 min + unless the env pins it; the run logs its guard in the startup line. That far exceeds the + Bash tool's 10 min foreground cap: run it in the background and wait + for the completion notification. A foreground timeout kills the run before hil_test.py + can write its report. NEVER cancel early. - One hardware action at a time. You are never run concurrently with another hil-operator. -- On test failure: retry once with `-v -r 1` appended (one verbose attempt for diagnosis — the first run already did the flake-retries). If a board/fixture stops enumerating or tools hang in D state, consult usb-kernel-recover and capture `dmesg | tail -50` into `detail`; set `wedged` true. +- On test failure: retry once with `-v -r 1` appended (one verbose attempt for diagnosis; a usbtest battery that produced per-case verdicts is NOT auto-retried, so its result already stands). If a board/fixture stops enumerating, or a tool of YOURS hangs in D state, consult usb-kernel-recover and capture `dmesg | tail -50` into `detail`; set `wedged` true. A `> **Rig note.**` banner reporting someone else's D-state process is not that — see the hil skill's banner list. ## Output contract diff --git a/.claude/agents/pr-monitor.md b/.claude/agents/pr-monitor.md index 7dba91fea..77777b0fb 100644 --- a/.claude/agents/pr-monitor.md +++ b/.claude/agents/pr-monitor.md @@ -9,7 +9,7 @@ You triage exactly one PR (number given in your prompt) using `gh`. You never mo ## CI triage -1. `gh pr checks `. If checks are running and your prompt says to wait, use `gh pr checks --watch` with a Bash timeout >= 30 min. +1. `gh pr checks `. If checks are running and your prompt says to wait, run `gh pr checks --watch` as a BACKGROUND Bash task (the foreground timeout is capped at 10 min). 2. For each failing check, find its run and read the failure: `gh run view --log-failed | head -150`. 3. Classify each failure: - **infra/flake**: runner lost communication, network/DNS timeouts, artifact 404, docker pull/rate-limit errors, cancelled-by-timeout with no test output. diff --git a/.claude/agents/target-debugger.md b/.claude/agents/target-debugger.md index 655b5f512..1b6931307 100644 --- a/.claude/agents/target-debugger.md +++ b/.claude/agents/target-debugger.md @@ -46,8 +46,8 @@ the next technique you would try. ## Lock discipline -- Hold the board lock for the WHOLE session (`hil_lock.py hold - --reason "target debug: "`). Multi-hour holds are fine; never stop the +- Hold the board lock for the WHOLE session (`python3 test/hil/helper/hil_lock.py + hold --reason "target debug: "`). Multi-hour holds are fine; never stop the actions-runner. Locks held by others: report holder/reason, never force unless your prompt states the user authorized it. - `hil_test.py` self-locks: release your hold before any `hil_test.py` run, diff --git a/.claude/skills/etm-trace/SKILL.md b/.claude/skills/etm-trace/SKILL.md index 9e2505736..99e89729c 100644 --- a/.claude/skills/etm-trace/SKILL.md +++ b/.claude/skills/etm-trace/SKILL.md @@ -43,7 +43,7 @@ this skill for exact counts, coverage, or instruction-by-instruction history. capture script uses automation port **19201**, never an interactive Ozone's 19200. - Hold the board lock (see the `hil` skill): - `python3 test/hil/hil_lock.py hold --reason "etm capture"`. + `python3 test/hil/helper/hil_lock.py hold --reason "etm capture"`. - Committed `hw/bsp/**/ozone/*.jdebug` are the maintainer's interactive projects — automation never opens them (Ozone rewrites project files); the script generates a throwaway project. diff --git a/.claude/skills/hil-pool-check/SKILL.md b/.claude/skills/hil-pool-check/SKILL.md index 49d252f62..6a8f66087 100644 --- a/.claude/skills/hil-pool-check/SKILL.md +++ b/.claude/skills/hil-pool-check/SKILL.md @@ -5,7 +5,7 @@ description: Use when asked for a pool check or board/probe health scan on a Tin # HIL Pool Check (board/probe health) -Health-scan the HIL board pool with `test/hil/hil_pool_check.py`: per board it checks the flash +Health-scan the HIL board pool with `test/hil/helper/hil_pool_check.py`: per board it checks the flash probe is on the USB bus, flashes a light example (`device/dfu_runtime`; host-only boards get `host/device_info`, verified by serial output), waits for the board's uid to re-enumerate, applies safe per-device recovery (probe authorized-toggle, board reset), re-parks with @@ -21,19 +21,19 @@ pool check holds fails it as "board locked" — prefer running between CI runs. A request for a "pool check" means the DEFAULT full check below. Use `--scan-only` only when the user explicitly asks for a quick look, or when you have VERIFIED a CI sweep is mid-run right now -(`python3 test/hil/hil_lock.py status` shows `hil_test.py` holders) — "CI might be running" is not +(`python3 test/hil/helper/hil_lock.py status` shows `hil_test.py` holders) — "CI might be running" is not that predicate: the full check is already lock-safe (CI-held boards report 🔒 locked and are never touched), so an unconfirmed suspicion is no reason to downgrade. In either scan case say which mode ran and why; never silently substitute the scan for the full check. ```bash -python3 test/hil/hil_pool_check.py # full check: ~10 s + ~1-2 s/board with firmware built; +python3 test/hil/helper/hil_pool_check.py # full check: ~10 s + ~1-2 s/board with firmware built; # first run on an unbuilt tree takes minutes (it builds) -python3 test/hil/hil_pool_check.py --scan-only # USB presence only, <1 s, no locks/flashing/building -python3 test/hil/hil_pool_check.py -b BOARD [-b …] # subset; may name boards-skip (parked) entries +python3 test/hil/helper/hil_pool_check.py --scan-only # USB presence only, <1 s, no locks/flashing/building +python3 test/hil/helper/hil_pool_check.py -b BOARD [-b …] # subset; may name boards-skip (parked) entries # from a dev PC, against the ci rig (bash -lc: flashers like STM32_Programmer_CLI live in ~/bin): -ssh ci.lan 'bash -lc "cd ~/code/tinyusb && python3 test/hil/hil_pool_check.py"' +ssh ci.lan 'bash -lc "cd ~/code/tinyusb && python3 test/hil/helper/hil_pool_check.py"' ``` ## Notes @@ -45,8 +45,9 @@ ESP-IDF env (`get-idf`) for espressif — which also needs `esptool` on PATH (pi `~/.local/bin/esptool`; a non-login shell may lack it — run via `bash -lc`). An explicit `-B` is searched exclusively for *existing* firmware; builds still land in `cmake-build/` and are noted `built `. Espressif boards park too when the IDF env is present. A first run on an -unbuilt tree builds for many minutes: use a command timeout ≥ 30 min and NEVER cancel early — a -killed run leaves detached cmake/ninja children still writing to `cmake-build/`. +unbuilt tree builds for many minutes: the Bash tool caps a foreground timeout at 10 min, so run +it in the BACKGROUND and NEVER cancel early — a killed run leaves detached cmake/ninja children +still writing to `cmake-build/` with the board locks held under a protected reason. Statuses: `ok` (flashed and verified; in `--scan-only` it only means the probe is present), `flash-failed` (firmware delivery failed: probe missing, build failed, flasher error, silent diff --git a/.claude/skills/hil/SKILL.md b/.claude/skills/hil/SKILL.md index f273be120..093f345b2 100644 --- a/.claude/skills/hil/SKILL.md +++ b/.claude/skills/hil/SKILL.md @@ -1,6 +1,6 @@ --- name: hil -description: Use when running TinyUSB Hardware-in-the-Loop (HIL) tests on physical boards, debugging HIL failures, or copying firmware to the ci.lan test rig. Covers per-host config selection (infra rigs ci/tusb use tinyusb.json/hfp.json, any dev PC uses local.json), local and remote execution, the board-lock protocol, and debugging tips. For board/probe health scans ("pool check") use the hil-pool-check skill. +description: Use when running TinyUSB Hardware-in-the-Loop (HIL) tests on physical boards, when a HIL run fails, hangs, reports a board locked, or produces a report you need to interpret, or when copying firmware to a test rig (ci.lan, hifiphile/tusb, or a dev PC). For board/probe health scans ("pool check") use the hil-pool-check skill instead. --- # Hardware-in-the-Loop (HIL) Testing @@ -26,28 +26,28 @@ The `ci` rig also hosts a GitHub Actions runner that flashes boards and runs HIL - For hardware work outside `hil_test.py` (JLink/GDB, manual flashing, `usbtest.py`, serial poking), hold the lock first: ```bash -python3 test/hil/hil_lock.py hold BOARD [BOARD...] --reason "why" +python3 test/hil/helper/hil_lock.py hold BOARD [BOARD...] --reason "why" # ... hardware work ... -python3 test/hil/hil_lock.py release BOARD [BOARD...] +python3 test/hil/helper/hil_lock.py release BOARD [BOARD...] ``` - Never pre-hold boards you are about to run `hil_test.py` on — it self-locks and would treat your own hold as a conflict. -- Rig-wide operations (uhubctl power cycling, pci-rebind — bus renumbering) affect every board: `hil_lock.py hold --all --reason "..."` first. +- Rig-wide operations (uhubctl power cycling, controller resets — bus renumbering) affect every board: `hil_lock.py hold --all --reason "..."` first. - `hil_lock.py status` lists holders. Locks auto-release when the holder process dies (kernel flock); `/tmp` clears on reboot. - Forcing past a lock: `HIL_NO_BOARD_LOCK=1 python3 test/hil/hil_test.py ...` bypasses the guard without killing the holder. Only with the user's explicit go-ahead — they accept the risk of colliding with whatever holds the board. ## Pool check (board/probe health) -Board/probe health scanning (`test/hil/hil_pool_check.py`) has its own skill: **hil-pool-check**. +Board/probe health scanning (`test/hil/helper/hil_pool_check.py`) has its own skill: **hil-pool-check**. Use it before a HIL campaign, after rig maintenance/reboot, or when boards fail to flash. ## PR-scoped selection -`test/hil/hil_select.py` maps a diff to affected boards/tests (used by CI on PRs; fail-open +`test/hil/helper/hil_select.py` maps a diff to affected boards/tests (used by CI on PRs; fail-open to the full matrix). Manual use: ```bash -SEL=$(python3 test/hil/hil_select.py --base master test/hil/tinyusb.json) +SEL=$(python3 test/hil/helper/hil_select.py --base master test/hil/tinyusb.json) FULL=$(printf '%s' "$SEL" | python3 -c "import json,sys; print(json.load(sys.stdin)['full'])") ARGS=$(printf '%s' "$SEL" | python3 -c "import json,sys; print(json.load(sys.stdin)['args']['tinyusb.json'])") if [ "$FULL" = "True" ] || [ -n "$ARGS" ]; then @@ -60,7 +60,20 @@ fi Read `full`, never `args` alone: `args` is empty for BOTH `full: true` (run the whole matrix — a broad or unclassified change) and "nothing selected" (skip). Skip only when `full` is false AND `args` is empty. -Unit suite: `python3 test/hil/test_hil_select.py` (no hardware). +Unit suites (no hardware), all four run by the `hil-test`/`hil-select-test` pre-commit +hooks: `test_hil_select.py` covers only board selection. The containment work --- bounded +reads, the kill ladders, the build and pool guards --- lives in `test_hil_bounded.py`, +`test_hil_health.py` and `test_hil_util.py`, so run all four when changing `test/hil`: +`for f in test/hil/test/test_*.py; do python3 "$f"; done` (~55s). + +## Pre-flight rig health check + +`hil_test.py` notes any process already in D state when the run starts, as one line above +the table. It never aborts, and it is a hint rather than a diagnosis. What bounds a stuck +run is `HIL_POOL_TIMEOUT` plus the job's `timeout-minutes`; what diagnoses a wedged rig is +the `hil-pool-check` skill. + +See the `usb-kernel-recover` skill for what a real wedge looks like and how to clear it. ## Prerequisites @@ -103,11 +116,44 @@ Env overrides: `REMOTE`, `REMOTE_DIR`, `CONFIG`. Fails fast if the build dir/rep ## Timing -Runs take 2-5 min. Use a timeout ≥ 20 min (1200000 ms). NEVER cancel early. +Runs take 2-5 min per board, but a stuck fleet runs to `HIL_POOL_TIMEOUT` — 60 min +unless the env pins it. The run logs its guard in the startup line; never declare a run +stuck before THAT value has elapsed. +The Bash tool caps a foreground timeout at 10 min, so **run it in the background** and +wait for the completion notification -- never a foreground timeout, which would kill +the run before its own guard can write a report. NEVER cancel early. ## Reporting The user-facing answer to a HIL run IS the tool's summary table: paste the complete per-board table (and footer counts) verbatim — never truncate rows or reduce it to a prose digest; at most -one line of commentary below it. On failure, retry with `-v`; if that's not enough, add temporary -debug prints to `hil_test.py`. +one line of commentary below it. + +**First check what sits above the table.** Seven banners can appear there; match on a +PREFIX, since each carries trailing detail and one is a blockquote: + +- `**HIL run abandoned: worker pool timed out after …s.**` — no results were collected this + attempt, so any table below is a PREVIOUS attempt's. Report the abandonment, never those + rows, and never `"pass": true`. +- `**HIL run aborted: a worker raised …**` — same rule: a worker crashed before results + were collected; any table below is stale. Report the abort, never the rows. +- `**HIL run abandoned: the worker pool would not shut down.**` — DIFFERENT: the table + below IS this run's, but the pool could not be shut down afterwards (the job exits + non-zero even if every board passed). Report the results AND the abandonment; never + `"pass": true`. +- `**HIL run selected no boards.**` — the filters intersected to nothing, so there is no + table at all. Report that (and the filter shown), never `"pass": true`. +- `> **Rig note.**` — a process was in D state when the run started. This is NOT a wedge: + a healthy in-flight testusb is uninterruptible for most of every case, and the rig + supports a dev run alongside CI. On its own it is never `wedged: true` and never turns a + green table into `"pass": false`. Mention it only when a board below failed, as the first + thing to check. +- `> **Rig dirty.**` — a process survived SIGKILL and still holds a probe or usbfs node + into the NEXT job. The table below is this run's and can be reported, but say the rig is + dirty: the next job starts degraded and nothing in the harness can clear it. +- `> **Not all verdicts are evidence.**` — one or more workers went blind on sysfs, so + "device not found" from the named boards means "could not tell". Do NOT report their red + cells as broken boards. + +On failure, retry with `-v`; if that's not enough, add temporary debug prints to +`hil_test.py`. diff --git a/.claude/skills/pre-pr/SKILL.md b/.claude/skills/pre-pr/SKILL.md index 3829b4b9e..b96750e4f 100644 --- a/.claude/skills/pre-pr/SKILL.md +++ b/.claude/skills/pre-pr/SKILL.md @@ -15,7 +15,7 @@ Run the software + hardware gate for the current branch. The user invoking this ## 2. Map changes to boards -- `python3 test/hil/hil_select.py --base $BASE test/hil/tinyusb.json` → JSON with the affected +- `python3 test/hil/helper/hil_select.py --base $BASE test/hil/tinyusb.json` → JSON with the affected bsp `families`, the affected rig `boards`, and per-file `reasons`. `full: true` means a broad/infra change. - Affected families = `families` ∪ the family of every name in `boards`. Neither half is diff --git a/.claude/skills/target-debug/SKILL.md b/.claude/skills/target-debug/SKILL.md index 28678c309..9a61a86c7 100644 --- a/.claude/skills/target-debug/SKILL.md +++ b/.claude/skills/target-debug/SKILL.md @@ -30,9 +30,9 @@ Hold the board lock for the WHOLE manual session; never stop the actions-runner (see the `hil` skill for the full lock protocol): ```bash -python3 test/hil/hil_lock.py hold --reason "target debug: " +python3 test/hil/helper/hil_lock.py hold --reason "target debug: " # ... instrument / build / flash / capture / GDB ... -python3 test/hil/hil_lock.py release +python3 test/hil/helper/hil_lock.py release ``` Board → probe mapping: `test/hil/tinyusb.json` — `flasher.name` is the probe diff --git a/.claude/skills/usb-kernel-recover/SKILL.md b/.claude/skills/usb-kernel-recover/SKILL.md index ea5931cc4..009090769 100644 --- a/.claude/skills/usb-kernel-recover/SKILL.md +++ b/.claude/skills/usb-kernel-recover/SKILL.md @@ -1,136 +1,206 @@ --- name: usb-kernel-recover -description: Use when a USB device or fixture attached to the ci HIL rig's Linux host is stuck, hung, not enumerating, or wedged after a failed flash or test, or when processes touching USB (testusb, JLinkExe, uhubctl, libusb tools) start hanging in D state. Linux-kernel-side only — a bus owned by a TinyUSB host is out of reach (reset the target / cycle its VBUS instead); the rig's probes and serial fixtures always remain in scope. +description: Use when a USB device or fixture on a HIL rig's Linux host (ci.lan, hifiphile/tusb, a bench PC) is wedged, not enumerating, or when processes touching USB (testusb, JLinkExe, uhubctl, openocd, libusb tools) hang in D state. Linux-host side only — a bus owned by a TinyUSB host is out of reach. --- # USB Recovery on the HIL Rig (Linux kernel side) -Run this skill's `scripts/usb_recover.sh` with `sudo`. It wraps the sysfs reset -actions, a uhubctl power-cycle escalator, and a resolver: +**The rule:** a wedged usbfs ioctl holds that device's `device_lock` +(`usbdev_do_ioctl` takes `usb_lock_device`, the uninterruptible variant — +v6.12.96 devio.c:2609) and the driver under it waits in a plain +`wait_for_completion()` with no timeout (usbtest.c:1404; `usb_sg_wait`, +message.c:765). Nothing that also takes that lock can help. Only two levers +don't: **failing the URB at the device** (rung 1) and **the port-side data-line +drop** (rung 2). + +## 1. Triage: find the holder + +```bash +ps -eo pid,stat,etimes,wchan:22,args | awk '$2 ~ /D/' +sudo cat /proc//stack # never opens the node, so it cannot block +``` + +- **`S` = victim.** Lock-taking sysfs *reads* use `usb_lock_device_interruptible` + (sysfs.c:124-139, 11 sites), so readers are killable and `timeout` bounds them. + Ignore them; they unwind by themselves. +- **`D` = the holder, or a writer that took the uninterruptible path.** + +| Stack shows | Meaning | Go to | +|---|---|---| +| `usbdev_ioctl` + a driver module (`[usbtest]`) | **owner, holds the lock** | rung 3 — terminal | +| `usbdev_ioctl`, no driver frames | owner waiting on a URB | rung 1 (DUT) / rung 2 (probe) | +| `usbdev_open`, sysfs reads | victim | ignore | +| `tee .../usbtest/new_id`, `bind`, `unbind` | **victim that SPREADS it** | stop issuing them | +| `hub_event` in a kworker | teardown stuck behind an owner | rung 3 | + +Driver-bind writes are not passive: `__device_driver_lock` (drivers/base/dd.c) +takes `device_lock()` uninterruptibly **and `device_lock(parent)`**, because +`usb_bus_type` sets `.need_parent_lock = true` (driver.c:2048) — each one holds +the HUB's lock, which is how one wedged port takes a whole bus down. + +Map the holder to a busport with **lock-free attrs only** (`devnum`, `idVendor`, +`idProduct` are `usb_descriptor_attr*`, plain `sysfs_emit`, sysfs.c:688-705): ```bash -# all examples below abbreviate: sudo .claude/skills/usb-kernel-recover/scripts/usb_recover.sh -sudo usb_recover.sh resolve /dev/ttyACM3 # /dev node -> busport (e.g. 3-4.7); also ttyUSB*, sg* -sudo usb_recover.sh authorized # deauthorize+reauthorize: re-enumerate, no VBUS cut -sudo usb_recover.sh rebind # usb driver unbind+bind: re-probe -sudo usb_recover.sh hub-cycle # uhubctl VBUS cycle of the feeding port, walking parent hub - # -> root port until the device re-enumerates -sudo usb_recover.sh root-cycle [serial] # uhubctl VBUS cut straight at the ROOT port (real ppps), no - # leaf walk, no device-lock touch: the D-state cure. - # [serial] is checked and a mismatch refused. -sudo usb_recover.sh pci-rebind # whole HCD controller unbind+bind, e.g. 0000:02:00.0 -sudo usb_recover.sh pci-bind [drv] # re-bind a DRIVERLESS controller (auto-tries xHCI drivers) +for d in /sys/bus/usb/devices/-*/; do + [ "$(cat $d/devnum)" = "" ] && echo "$d $(cat $d/idVendor):$(cat $d/idProduct)" +done +grep -l /sys/bus/usb/devices/*/serial # only on a HEALTHY device ``` -`hub-cycle` caveats: leaf hubs that gang (or fake) port power switching bounce -**all siblings** on that hub when cycled; a **self-powered** leaf hub keeps -downstream VBUS up, so cycling it only resets its uplink — that's why the walk -escalates to the root port, where the Renesas cards' per-port power (ppps) is -real. A device that is wedged but bus-powered from a switching hub gets a true -power cycle; one on a self-powered hub may only get a re-enumeration. +## 2. Shield first (prerequisite for anything using libusb) -## Decide first: is anything stuck in D state? +A wedged device blocks every enumerator that reads its locking attributes — +JLinkExe, uhubctl, openocd's HID fallback. `chmod 000` makes the VFS reject the +read before `->show()` runs, so they skip it and keep enumerating: ```bash -ps -eo pid,stat,wchan:30,cmd | awk '$2 ~ /D/' +for f in bNumInterfaces bmAttributes bMaxPower configuration bConfigurationValue \ + product manufacturer serial avoid_reset_quirk; do + sudo chmod 000 /sys/bus/usb/devices//$f +done ``` -**If yes** (uninterruptible sleep, typically a usbfs ioctl — e.g. testusb inside -`usb_sg_wait`): cut VBUS at the root port, and nothing else. +- Shield the **leaf, its parent hub, and the root hub** (`usb`) — a stuck + uhubctl locks the root hub too. +- **Run the recovery tool as NON-root**: root has `CAP_DAC_OVERRIDE`, ignores the + `000`, and blocks anyway. +- **Only those nine.** `descriptors`, `busnum`, `devnum`, `speed`, `idVendor`, + `idProduct` are lock-free and libusb needs them; a blanket `chmod` breaks + enumeration instead of fixing it. +- `chmod` never blocks (inode setattr, no `show()`), so it works on a fully + wedged device. +- **Not needed for openocd pinned with `vid_pid`** — it matches the cached + descriptor and skips a foreign device before `libusb_open` + (cmsis_dap_usb_bulk.c:107, bulk backend; the HID fallback ignores the pin). +- Leaf shields vanish on re-enumeration; **the root hub's must be restored**: + `sudo chmod "$(stat -c %a /sys/bus/usb/devices/usb/$f)" …/usb/$f` + +## 3. The rungs — go straight to the one triage names + +**Rung 1 — wedged DUT: reset it through its own probe.** ```bash -sudo usb_recover.sh root-cycle # e.g. 11-3.7 -> cycles bus 11 root port 3 +printf "r\ng\nq\n" > /tmp/rec.jlink +JLinkExe -device -if SWD -speed 4000 -SelectEmuBySN \ + -autoconnect 1 -nogui 1 -CommandFile /tmp/rec.jlink ``` -This drops power to the wedged device, so its in-flight URB fails and the ioctl -returns. It targets the *root hub* — a different USB device from the wedged one — -and never *writes* the wedged device's sysfs. It reads a few attributes from it — -`idVendor`/`idProduct`/`serial`/`product` to report and check the target, and the -directory inode plus `devnum` afterwards — none of which take the device lock, so -it does not join the convoy the way `authorized`/`rebind`/`pci-rebind` do. -Recovery is proven by that inode changing — a real disconnect destroys the -kobject and reconnecting creates a new one, whereas a disconnect blocked on the -device lock leaves it untouched. It exits non-zero if the device does not come -back; a **zero exit only means it re-enumerated**, so still confirm the D-state -process actually let go. Pass the expected serial as a third argument and it -refuses a busport that now names a different device. - -It bounces **every fixture under that root port** — on ci that is up to 25 -devices. Hold the affected boards' locks first if you can, but note -`hil_lock.py` uses `LOCK_EX | LOCK_NB` and so fails immediately when CI already -holds them; there is no wait-for-lock. When CI is mid-run you are choosing -between bouncing its fixtures and leaving the bus wedged for everything. The -automated path in `usbtest.py` takes no locks at all and accepts that collateral -deliberately: by the time a D-state wedge exists the convoy will take the bus -down anyway. - -(The VBUS mechanism is verified on the ci rig — the leaf hubs report -`bmAttributes=e0`, "self-powered", but are physically bus-powered with no adapter, -so a root-port cut really does kill downstream power. Do not re-derive this from -the descriptor; it lies. Not yet confirmed against a live D-state wedge. If -`uhubctl` itself hangs, the convoy has already spread — escalate.) - -If `root-cycle` does not free the D-state process, there is no software cure -left: ask the operator for a full PVE **host** power cycle. A VM reboot is NOT -reliable (downstream hubs can latch up across the PCIe reset and need a physical -replug), and a graceful reboot stalls on the D-state process anyway. Do NOT fall -through to `pci-rebind` (see next). - -**`pci-rebind` can strand the controller driverless.** Its unbind succeeds but, -with a D-state process still holding a URB, the *re-bind* hangs — leaving the -PCI device with **no driver** (`/sys/bus/pci/devices//driver` gone) and the -whole controller's fixtures offline. A second `pci-rebind` then dies with "no -driver bound". Recover with `pci-bind ` (re-attaches the xHCI driver); -if that also hangs because the D-state URB is unkillable, only a full PVE host -power cycle (operator action) recovers. The Renesas binds via `xhci-pci-renesas` (firmware loader), others via -`xhci_hcd` — `pci-bind` auto-tries both, or pass the driver explicitly. - -**Ordering is critical.** `authorized`/`rebind`/`pci-rebind` all take the -per-device lock the stuck ioctl holds — they block and join the convoy, and -soon every libusb tool (uhubctl, JLinkExe) hangs too. Worse, a blocked -`pci-rebind` grabs the PCI device lock on its way in and can wedge the whole -function, after which **only a full PVE host power cycle recovers**. `root-cycle` -first, and never `pci-rebind` a D-state wedge. - -**If no** (device merely dead or silent), escalate gently: - -1. `authorized ` — re-enumerates just that device -2. `rebind ` — re-probe; also worth trying on the parent hub's busport -3. `hub-cycle ` — VBUS cycle of the feeding port, walking up to the - root port; may bounce sibling fixtures on ganged hubs -4. `pci-rebind ` — last resort: bounces every fixture on that controller - -## Finding targets +Reset **before** park-flash: non-destructive (the firmware under test survives +for autopsy), no flash wear, and no bad park image — a `wfe`/`wfi` park has +bricked SWD on mimxrt1064_evk and max32666fthr through a power cycle. +`ResetTarget` measures 128-129 ms; cleared 57 → 0, 26 → 0 and 5 → 0 D-state +processes, single shot each. Mechanism: chip reset drops the pull-up → +`usb_hcd_flush_endpoint` unlinks the URB `-ESHUTDOWN` (hcd.c:1783) → the +completion fires → the ioctl returns → the lock releases. + +Works on i.MX RT (`USBCMD.RS` = 0 detaches, RT1050 RM Rev 3 p.2453) **and on +DWC2** — measured 2026-08-16 on stm32f407disco: `r; g` gave +`usb 13-2.2: USB disconnect, device number 107`, re-enumerating 325 ms later. +(A bare **halt** does not: the core keeps running with the pull-up asserted.) + +**Park-flash** (`--recover-board`/`--recover-fw`, what `usbtest.py` automates) is +the fallback where the reset cannot reach the peripheral. Delivery must be +convoy-safe: **openocd pinned with `vid_pid`**, or esptool (`-p `). +JLinkExe selects by serial, which needs `libusb_open`, so it needs the shield. + +**Rung 2 — wedged PROBE: `root-cycle`.** A probe has no probe to reset it, so the +port-side drop is the only lock-free lever left. It commands the ROOT hub and +never touches the wedged device's lock. + +```bash +sudo usb_recover.sh root-cycle [expected-serial] +``` + +Bounces **every fixture under that root port** (up to 25 here). Renesas `ppps` +disables D+/D− only — VBUS stays up, so it is a forced re-enumeration, not a +power cycle. Success is the sysfs inode changing, not uhubctl's exit code. + +**Rung 3 — terminal case: a driver ioctl that OWNS the lock.** No software cure: +the task is uninterruptible and SIGKILL is queued, not delivered. Reboot with +**sysrq**, never `reboot(2)` — a graceful reboot runs `device_shutdown()`, which +takes every device lock and stalls on the wedged one. + +```bash +echo b | sudo tee /proc/sysrq-trigger # after: sync; sudo umount -a +``` + +**Rung 4 — hypervisor.** ci.lan only, and never needed in eight recorded wedges: +`qm stop && qm start ` from the PVE host. A VM *reboot* is not +reliable — hubs can latch across the PCIe reset. + +## 3b. If the CONTROLLER is dead, not a device + +Signature: `xhci-pci-renesas : Timeout while waiting for setup device +command`, devices on that controller failing to enumerate, or its buses gone — +as opposed to ONE device wedged. The rungs above cannot help; the controller +itself needs re-initialising. + +```bash +sudo usb_recover.sh pci-rebind # unbind + bind the whole xHCI +sudo usb_recover.sh pci-bind # only if it ends up driverless +``` + +Measured on ci.lan 2026-08-17 02:34:41 after a `hub-cycle` failed to take: unbind +deregistered buses 17 and 18, the re-bind registered new buses **1 and 2** one +second later, and every fixture re-enumerated. **It renumbers every bus that +controller owns**, so hold all affected boards' locks first (`hil_lock.py hold +--all`) and re-derive busports afterwards. + +Do NOT reach for it while a device-lock convoy is live — see Common mistakes. + +## 4. If nothing is in D state + +The device is dead or silent, not wedged. `sudo usb_recover.sh authorized +` unconfigures and reconfigures it (`usb_set_configuration(dev, -1)` +then re-choose, hub.c) — it fixes stale driver/interface state, does **not** +replug: the `usb_device` survives, so most probes keep their sysfs node. If that +does not take, the device is wedged rather than silent — go to rung 1 or 2. +`resolve ` maps `/dev/ttyACM3` → busport. + +**It takes `usb_lock_device` uninterruptibly** (hub.c `usb_deauthorize_device`), +so it is safe only while nothing is in D state. + +## 5. Before declaring the rig healthy ```bash -grep -l /sys/bus/usb/devices/*/serial # serial -> busport (dir name) -readlink -f /sys/bus/usb/devices/usb # bus N -> its PCI addr in the path +ps -eo stat,args | awk '$1 ~ /D/' | wc -l # must be 0 +timeout 15 lsusb # rc 0 and a sane device count +sudo uhubctl -l -p # "0000 off" = never came back +sudo uhubctl -l -p -a on ``` -Rig layout (2026-07-15, two Renesas uPD720201 cards; bus numbers renumber every -boot — re-derive with `readlink`): AMD `0000:02:00.0` = the debug-probe tree -(J-Links, ST-Links, WCH-Links), no port power switching; Renesas `0000:01:00.0` -and `0000:03:00.0` = DUT device hubs + serial fixtures, and ALL their root-hub -ports have real per-port power (`ppps`, 4+4 each) — `sudo uhubctl -l -p - -a cycle` cuts VBUS to the leaf hub on that port. The 1a40:0201 leaf -hubs themselves claim "ganged" switching but do not actually cut power. +Observed: 5 boards missing with a completely clean D-state list, because +`usb17-port2` sat at `disable=1`. ## Common mistakes -- `resolve` takes a **/dev node**, not a busport or serial ("no such device node"). -- `authorized`/`rebind`/`hub-cycle`/`root-cycle` take a **busport** (`3-4.7`); - `pci-rebind`/`pci-bind` take a **PCI addr**. -- Command produces no output and doesn't return → it is blocked on the device - lock: a D-state holder exists; see above. -- Trying `pci-rebind` on a D-state hang — its re-bind hangs and strands the - controller **driverless**; recover with `pci-bind `, or a PVE host power - cycle if the D-state URB is unkillable. Use `root-cycle` for D-state, never - `pci-rebind`. -- Writing `/sys/bus/pci/devices//reset` because the attribute is there. No - rig controller has FLR, so it becomes a PCIe bus reset that resets the xHCI - behind its live driver — the write succeeds, the card is halted for good, and - only a PVE host power cycle brings it back. Use `root-cycle`. -- `root-cycle` bounces **every** fixture under that root port, not just the target - — hold the sibling boards' locks first. -- A J-Link reset (`r; go`) does not disconnect a wedged DUT from the host: the - DWC2 soft-connect pullup stays up through a core halt, so stuck URBs stay stuck. +- **`uhubctl -a cycle` on a root port without `-S`.** It writes sysfs + `disable`, and `disable_store` takes `usb_lock_device(hdev)` uninterruptibly + then calls `usb_disconnect(child)` inside it (port.c) — against a wedged child + that blocks while holding the root hub's lock, poisoning the bus. + `usb_recover.sh` passes `-S`. +- **`echo 1 > .../remove`** to make a wedged device "go away": `remove_store` is + the one attribute in sysfs.c taking the uninterruptible `usb_lock_device` + (sysfs.c:765). It joins the convoy instead of clearing it. +- **`authorized` on anything wedged** — same uninterruptible lock. Driver + unbind/bind (`/sys/bus/usb/drivers/usb/{unbind,bind}`) does the same + unconfigure/reconfigure via `usb_generic_driver_disconnect` (generic.c) but ALSO + takes the parent hub's lock (`need_parent_lock`), so it is strictly worse; it was + removed from `usb_recover.sh` for that reason. +- **`pci-rebind` for a wedged DEVICE.** It is the cure for a dead CONTROLLER (see + below), not for a device-lock convoy: with a live D-state URB the re-bind can + hang and leave the controller with **no driver** and every fixture offline + (observed once). Recover that with `pci-bind `. +- **Writing `/sys/bus/pci/devices//reset`** — no rig controller has FLR, so + it becomes a bus reset behind a live driver: card halted, host power cycle. +- **Resetting a victim's board.** Two boards were reset innocently before anyone + found the holder. Map by `devnum`, not by which board "should" be running. +- **Assuming one controller.** Observed: 26 D-state processes across three xHCI + controllers, all cleared by one probe reset on one device. + +## Rig layout (ci.lan, bus numbers renumber every boot) + +`readlink -f /sys/bus/usb/devices/usb` → its PCI address. AMD `0000:02:00.0` +has no port-power switching; Renesas `0000:01:00.0` (probe tree) and +`0000:03:00.0`/`0000:05:00.0` (DUT hubs) have real per-port `ppps`. diff --git a/.claude/skills/usb-kernel-recover/scripts/usb_recover.sh b/.claude/skills/usb-kernel-recover/scripts/usb_recover.sh index 2230602b9..876e0938b 100755 --- a/.claude/skills/usb-kernel-recover/scripts/usb_recover.sh +++ b/.claude/skills/usb-kernel-recover/scripts/usb_recover.sh @@ -4,21 +4,15 @@ # # Usage: # sudo usb_recover.sh authorized # e.g. 3-2 -> deauthorize+reauthorize (re-enumerate, NO VBUS cut) -# sudo usb_recover.sh rebind # e.g. 3-2 -> usb driver unbind+bind (re-probe) -# sudo usb_recover.sh pci-rebind # e.g. 0000:01:00.0 -> HCD unbind+bind (WHOLE controller) -# sudo usb_recover.sh pci-bind [driver] # bind a DRIVERLESS controller (e.g. after a pci-rebind -# # whose re-bind hung and left it unbound). Auto-tries the xHCI -# # drivers (xhci-pci-renesas, xhci_hcd) unless one is named. -# sudo usb_recover.sh hub-cycle # e.g. 13-1.6 -> uhubctl power-cycle of the port feeding it, -# # walking upstream (parent hub -> root port) until the device -# # re-enumerates. Ganged/fake-switching hubs may bounce ALL -# # siblings; self-powered hubs only reset their uplink, which -# # is why the walk ends at the root port (real xHCI ppps). -# sudo usb_recover.sh root-cycle [serial] # e.g. 13-1.6 -> uhubctl VBUS cut at the ROOT port feeding +# sudo usb_recover.sh root-cycle [serial] # e.g. 13-1.6 -> uhubctl port-off/on at the ROOT port feeding # # it; [serial] is verified against the device and refused on mismatch, # # skipping the leaf hubs (which fake ganged switching and do not # # actually cut power). Bounces every sibling under that root port. # # The D-state escape: no device lock, so it cannot convoy. +# sudo usb_recover.sh pci-rebind # e.g. 0000:05:00.0 -> unbind+bind the whole xHCI +# # controller. For a DEAD CONTROLLER, not a wedged +# # device: it renumbers every bus it owns. +# sudo usb_recover.sh pci-bind [drv] # re-attach a driver to a DRIVERLESS controller # sudo usb_recover.sh resolve # e.g. /dev/ttyACM3 -> print its (no privilege needed) set -euo pipefail @@ -28,6 +22,34 @@ DRIVER_RE='^[A-Za-z0-9_-]+$' die() { echo "usb_recover: $*" >&2; exit 1; } +lock_read() { + # Read an attribute served under the device lock (serial, product) with a 2s bound. + # Prints the value, '' when the attribute is absent, or '?' when it did not answer. + # + # Bounding these is load-bearing, not defensive: they are the FIRST thing root-cycle + # does, so on a real wedge an unbounded read blocks before reaching uhubctl at all + # (observed live: one attempt sat 3h; three concurrent invocations all frozen there). + # The operator then reads that as "recovery didn't work" and escalates to a bare + # `uhubctl -a cycle`, which tears the subtree down and blocks holding the ROOT HUB + # lock -- taking the whole bus with it. That is how one wedge becomes an incident. + # + # `timeout` is enough, though this said for a while that it was not (claiming the read + # sits in D state, where SIGKILL is not delivered, so timeout waitpid()s forever). It + # does not: v6.12.101 drivers/usb/core/sysfs.c takes the lock for every READ through + # usb_lock_device_interruptible -> device_lock_interruptible -> mutex_lock_interruptible, + # so the waiter sleeps INTERRUPTIBLY and SIGTERM ends it. Uninterruptible is the usbfs + # ioctl HOLDER, not us. The abandon-a-background-reader dance that claim justified is + # gone, and with it a fail-open where an absent attribute answered '?' -- the wedge + # signature, which root-cycle reads as "cannot confirm serial, proceed". + local v rc=0 + # `|| rc=$?`, never a bare assignment: under this script's `set -e` a command + # substitution that FAILS (an absent attribute -- most hubs and probes have no + # iSerialNumber, and `product` is often missing) exits the whole recovery script. + v=$(timeout 2 cat "$1" 2>/dev/null) || rc=$? + [ "$rc" -eq 124 ] && { echo '?'; return; } # timed out: nobody answered + printf '%s\n' "$v" +} + # Generation marker for "did this device actually re-enumerate". A real disconnect destroys the # usb_device and its sysfs kobject; reconnecting creates a new one, and kernfs hands out inode # numbers monotonically, so the directory inode changes. Verified on the rig: ports re-enumerated @@ -49,9 +71,6 @@ die() { echo "usb_recover: $*" >&2; exit 1; } # The trailing slash is load-bearing: /sys/bus/usb/devices/ is a SYMLINK with its own # separate inode, so without it stat reports the link rather than the device it points at, and the # value would never change. Do not "tidy" it away. -sysfs_gen() { stat -c %i "/sys/bus/usb/devices/$1/" 2>/dev/null || echo none; } -usage() { grep -E '^# sudo usb_recover' "$0" >&2; exit 2; } - # Refuse to touch a PCI function that is not a USB controller (class 0x0c03xx), so a stray or # mistyped BDF can't unbind/reset an unrelated device (storage, NIC) on a shared HIL host. require_usb_controller() { @@ -60,6 +79,9 @@ require_usb_controller() { [[ "$cls" =~ ^0x0c03 ]] || die "$addr is not a USB controller (class $cls); refusing" } +sysfs_gen() { stat -c %i "/sys/bus/usb/devices/$1/" 2>/dev/null || echo none; } +usage() { grep -E '^# sudo usb_recover' "$0" >&2; exit 2; } + # Resolve a /dev node (ttyACMx, ttyUSBx, sgN, ...) up to its USB device busport. resolve() { local node=$1 syspath dev @@ -89,13 +111,6 @@ case "$action" in echo 0 > "$d/authorized"; sleep 1; echo 1 > "$d/authorized" echo "re-authorized $target" ;; - rebind) - [[ "$target" =~ $USBPATH_RE ]] || die "bad usb path: $target" - [ -e "/sys/bus/usb/devices/$target" ] || die "no such usb device: $target" - echo "$target" > /sys/bus/usb/drivers/usb/unbind; sleep 1 - echo "$target" > /sys/bus/usb/drivers/usb/bind - echo "rebound $target" - ;; pci-rebind) [[ "$target" =~ $PCI_RE ]] || die "bad pci addr: $target" require_usb_controller "$target" @@ -128,39 +143,10 @@ case "$action" in die "could not bind $target with a known xHCI driver; pass the driver explicitly" fi ;; - hub-cycle) - [[ "$target" =~ $USBPATH_RE ]] || die "bad usb path: $target" - UHUBCTL=$(command -v uhubctl || echo /sbin/uhubctl) - [ -x "$UHUBCTL" ] || die "uhubctl not installed" - # sysfs generation, not node existence: a disconnect blocked on the device lock leaves the - # old node (and its idVendor) in place, so an existence check reports success without anything - # having happened -- and the walk to the root port, which is the part that actually cuts power - # on these fake-ganged leaf hubs, would never run. - gen=$(sysfs_gen "$target") - dev="$target" - while :; do - if [[ "$dev" =~ ^([0-9]+)-([0-9]+)$ ]]; then # parent is the root hub - loc="${BASH_REMATCH[1]}"; port="${BASH_REMATCH[2]}"; up="" - else # parent is a downstream hub - loc="${dev%.*}"; port="${dev##*.}"; up="$loc" - fi - echo "hub-cycle: power-cycling hub $loc port $port (feeds $dev)" - "$UHUBCTL" -l "$loc" -p "$port" -a cycle -d 5 -f || echo " (uhubctl failed at $loc; walking up)" - for _ in $(seq 1 10); do - sleep 1 - now=$(sysfs_gen "$target") - if [ "$now" != none ] && [ "$now" != "$gen" ]; then - echo "recovered: $target re-enumerated (gen $gen -> $now)"; exit 0 - fi - done - [ -n "$up" ] || break - dev="$up" - done - die "hub-cycle: $target still not enumerated after cycling up to the root port" - ;; root-cycle) - # VBUS cut at the ROOT port, where xHCI ppps is real. Unlike hub-cycle this does not walk up - # from the leaf (the 1a40:0201 hubs claim ganged switching but never cut power) and never + # Port-off/on at the ROOT port. NOTE: the Renesas ppps only disables D+/D- (VBUS stays up), + # so this is a forced re-enumeration, not a power cycle. It goes straight at the root port -- + # no leaf walk (the 1a40:0201 hubs claim ganged switching but never cut power) -- and never # writes the wedged device's sysfs or takes its lock, so it cannot join a D-state convoy. # uhubctl exits 0 even when it does nothing ("No compatible devices detected" still returns # 0), so its status proves nothing -- the sysfs_gen check below is the only real verdict. @@ -174,21 +160,33 @@ case "$action" in # wrong target is at least visible. [ -e "/sys/bus/usb/devices/$target" ] || die "no such usb device: $target" idf="/sys/bus/usb/devices/$target" - serial=$(cat "$idf/serial" 2>/dev/null || echo -) + serial=$(lock_read "$idf/serial") expect=${3:-} - [ -z "$expect" ] || [ "$expect" = "$serial" ] || \ + if [ -n "$expect" ] && [ "$serial" = '?' ]; then + # Warn and PROCEED: an unreadable serial is the wedge signature itself, so refusing + # here would block the cure on exactly the condition it exists for. The identity + # guard is lost for this call -- say so, because the cost of a wrong target is the + # whole subtree. + echo "root-cycle: WARNING $target's serial did not answer (it is wedged), so '$expect'" \ + "could NOT be confirmed; proceeding, but verify the busport if siblings drop" >&2 + elif [ -n "$expect" ] && [ "$expect" != "$serial" ]; then die "root-cycle: $target has serial '$serial', expected '$expect' — stale busport, refusing" + fi + # idVendor/idProduct are usb_descriptor_attr_le16: served WITHOUT the device lock, so + # a plain cat is safe on a wedged device. serial/product are usb_string_attr and are not. echo "root-cycle: target $target is $(cat "$idf/idVendor" 2>/dev/null || echo -):$(cat "$idf/idProduct" 2>/dev/null || echo -)" \ - "serial=$serial product=$(cat "$idf/product" 2>/dev/null || echo -)" + "serial=$serial product=$(lock_read "$idf/product")" bus=${target%%-*}; rest=${target#*-}; rootport=${rest%%.*} gen=$(sysfs_gen "$target") - echo "root-cycle: cutting VBUS on bus $bus root port $rootport (feeds $target, bounces its siblings)" - # -S is load-bearing. By default uhubctl writes /sys/.../usb-port/disable (verified: + echo "root-cycle: disabling D+/D- on bus $bus root port $rootport (no VBUS cut; feeds $target, bounces its siblings)" + # -S is load-bearing. By default uhubctl writes /sys/.../usb-port/disable (observed: # two O_WRONLY opens per cycle), and the kernel's disable_store() takes the ROOT HUB's lock and - # synchronously usb_disconnect()s the child BEFORE cutting power -- against a wedged device that - # blocks on the lock we are trying to free, so power would never drop and uhubctl would D-state - # holding the root hub's lock, poisoning the whole bus. -S forces the libusb path, which sends - # the power-off control transfer straight to the root hub with no child-disconnect in front. + # synchronously usb_disconnect()s the child BEFORE cutting power -- confirmed in v6.12.96 + # drivers/usb/core/port.c: usb_lock_device(hdev), the UNINTERRUPTIBLE variant, then + # usb_disconnect(&port_dev->child) inside it. Against a wedged device that disconnect blocks on + # the very lock we are trying to free, so power never drops and uhubctl D-states holding the + # root hub's lock, poisoning the whole bus. -S forces the libusb path, which sends the + # power-off control transfer straight to the root hub with no child-disconnect in front. "$UHUBCTL" -S -l "$bus" -p "$rootport" -a cycle -d 5 \ || die "uhubctl failed to cycle bus $bus port $rootport" for _ in $(seq 1 10); do diff --git a/.claude/skills/usbtest/SKILL.md b/.claude/skills/usbtest/SKILL.md index 76a01839c..6197ccd51 100644 --- a/.claude/skills/usbtest/SKILL.md +++ b/.claude/skills/usbtest/SKILL.md @@ -29,6 +29,11 @@ python3 test/hil/usbtest.py --serial --keep-binding --tests 29 # one case ``` - **Always `--keep-binding`**: the cleanup unbind path has wedged host xHCIs (`usb_hcd_alloc_bandwidth`). +- CI (`hil_test.py`) additionally passes `--budget`, `--outer-timeout` and + `--recover-board`/`--recover-fw`: on a HUNG case the battery aborts, RESETS the DUT + through its roster probe (non-destructive, ~130 ms) and reflashes only if that does not + clear the wedge (see usb-kernel-recover). Manual runs without those flags leave a HUNG + device wedged and skip cleanup — expected; reset or reflash it yourself. - Always settle a few seconds after flashing — enumeration can bounce once; testusb into the gap sees the device drop mid-case. - On a CI rig: stop the actions runner before touching hardware; restart after. Never run two @@ -87,6 +92,28 @@ python3 test/hil/usbtest.py --serial --keep-binding --tests 29 # one case | 5 | EIO — iso packet errors (check `dmesg`: "N errors out of M") | | 71 | EPROTO — device answered wrong / too slow (after HC retries) | +**Step 0 — read what the case actually does.** The kernel module is ground truth; +the table above is a summary. Do this before theorising, and always before deciding +whether a hung case is recoverable. Fetch the rig's exact version (`uname -r`): + +```bash +curl -sO "https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/plain/drivers/usb/misc/usbtest.c?h=v6.12.96" +# case N lives under `case N:` in usbtest_do_ioctl(); tools/usb/testusb.c maps the flags: +# -c = param.iterations, -s = param.length, -g = param.sglen (NOT what they read like) +``` + +- **Real traffic and pass criteria.** Case 24 at `-c 256 -s 1024 -g 8` is 256 rounds + of 8 bulk-OUT URBs, unlinking `urbs[num-4]`/`urbs[num-2]` and requiring + `-ECONNRESET` on those two plus normal completion on the other 6 — not the + "256 URBs" the flags suggest. +- **Whether the wait is bounded** — decisive for recovery. `simple_io` uses + `wait_for_completion_timeout` (:481); the unlink paths use a bare + `wait_for_completion` (:1502, :1615). A device stalling there wedges the ioctl in + **D state permanently** — it holds the device lock, so nothing recovers it + (usb-kernel-recover, "The terminal case"). Knowing this first stops you burning + the rig on attempts that cannot work. +- **Which DCD path is implicated**, precisely rather than by category. + 1. `usbtest.py` per-case output + its captured `dmesg` (`TEST n` markers bracket each case). 2. **usbmon** (`usbmon` skill): URB-level ground truth. **It cannot show data toggles or NAKs** — a toggle desync and a dead endpoint look identical (Submits without Completes); distinguish @@ -121,3 +148,6 @@ python3 test/hil/usbtest.py --serial --keep-binding --tests 29 # one case - "It works on gcc" → clang/IAR/LTO/make still pending. - "Fixed iso IN" → apply the same exemption to iso OUT (toggle logic is symmetric). - A clean single-board run does not validate concurrent/fleet behavior — batteries serialize. +- Reasoning about a case from its name or table row → open `usbtest.c` (step 0). The + flags don't mean what they look like, and recoverability is a property of that + case's wait, not of the rig. diff --git a/.claude/workflows/hil-validate.js b/.claude/workflows/hil-validate.js index 50559135f..136f9075e 100644 --- a/.claude/workflows/hil-validate.js +++ b/.claude/workflows/hil-validate.js @@ -26,8 +26,8 @@ const runBoard = (b) => agent( ? 'THE USER HAS EXPLICITLY AUTHORIZED FORCING: run hil_test.py with HIL_NO_BOARD_LOCK=1 in the environment (bypasses the board lock check; do NOT release or kill the existing holder). ' : 'If the run fails because the board lock is held (a dev session or concurrent CI job), report pass=false and set detail to start EXACTLY with "board locked:" followed by the holder JSON verbatim — never force the lock. ') + 'Reserve the phrase "board locked" strictly for lock contention; describe a frozen or non-enumerating board as "unresponsive" instead. ' + - `Firmware is in examples/cmake-build-${b}. Use the config for this host (hostname first), single-board flag -b ${b}, Bash timeout >= 20 min, never cancel early. ` + - 'On non-lock failures retry once with -v -r 1 (one verbose attempt for diagnosis — the first run already did the flake-retries). wedged=true if the board/fixture is unresponsive after the run (capture dmesg | tail -50 into detail).', + `Firmware is in examples/cmake-build-${b}. Use the config for this host (hostname first), single-board flag -b ${b}. Run hil_test.py as a BACKGROUND Bash task and wait for it (a stuck fleet runs to its pool guard, 60 min by default — beyond any foreground timeout); never cancel it early. ` + + 'On non-lock failures retry once with -v -r 1 (one verbose attempt for diagnosis; note a usbtest battery that produced per-case verdicts is NOT auto-retried, so its result already stands). wedged=true if the board/fixture is unresponsive after the run (capture dmesg | tail -50 into detail).', { label: `hil:${b}`, phase: 'HIL', agentType: 'hil-operator', schema: HIL }, ) diff --git a/.claude/workflows/pr-babysit.js b/.claude/workflows/pr-babysit.js index 8bb414114..406213a5f 100644 --- a/.claude/workflows/pr-babysit.js +++ b/.claude/workflows/pr-babysit.js @@ -109,7 +109,7 @@ const history = [] const repliedIds = new Set() // issue comments can't be thread-resolved, so they re-harvest every cycle — never reply twice for (let cycle = 1; cycle <= maxCycles; cycle++) { const t = await agent( - `Triage PR #${args.pr}. If checks are still running, wait for them first (gh pr checks ${args.pr} --watch, Bash timeout >= 30 min). ` + + `Triage PR #${args.pr}. If checks are still running, wait for them first (gh pr checks ${args.pr} --watch as a BACKGROUND Bash task; the foreground timeout is capped at 10 min). ` + 'Then follow your triage procedure: classify CI failures, re-run infra ones, harvest and adversarially validate bot review findings, draft replies for invalid/stale ones.', { label: `triage#${cycle}`, phase: 'Triage', agentType: 'pr-monitor', schema: TRIAGE }, ) diff --git a/CLAUDE.md b/CLAUDE.md index fd4b9b8e0..7a493e5db 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -118,6 +118,14 @@ Cutting a release — version bump, regenerated files, the per-release changelog ## References - MCU reference manuals, datasheets, schematics: before answering register/bitfield/pinout/errata/timing questions from memory or the web — or changing a specific dcd/hcd driver — use the `read-doc` skill (`.claude/skills/read-doc/SKILL.md`) to cross-check against docs in `$HOME/Documents/calibre-library`; tell the user if the needed document is missing (skill no-ops if the library is absent). +- Linux kernel behaviour (usbfs, usbtest, sysfs attributes, device locks, D state): never + infer it from symptoms — read the source for the *running* version. It refutes as often + as it confirms: it has killed two plausible dcd theories and corrected a recovery skill's + own attribute list. + ```bash + V=$(uname -r | grep -oE '^[0-9]+\.[0-9]+\.[0-9]+') # on the rig: ssh ci.lan uname -r + curl -fsSL "https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/plain/drivers/usb/core/sysfs.c?h=v$V" + ``` - Supported MCUs/boards: `hw/bsp/` and `docs/reference/boards.rst`. - USB classes: `src/class/{cdc,hid,msc,audio,…}/` — each has `*_device.c` and `*_host.c`. - Key files: `src/tusb.h`, `src/tusb_config.h`, `tools/get_deps.py`, `tools/build.py`, `test/unit-test/project.yml`. diff --git a/docs/superpowers/specs/2026-07-30-hil-usbtest-fleet-wedge-design.md b/docs/superpowers/specs/2026-07-30-hil-usbtest-fleet-wedge-design.md new file mode 100644 index 000000000..3ed0c1519 --- /dev/null +++ b/docs/superpowers/specs/2026-07-30-hil-usbtest-fleet-wedge-design.md @@ -0,0 +1,236 @@ +# HIL fleet-wedge containment + +Date: 2026-07-30 +Status: implemented, then superseded in part — addendum last checked 2026-08-12 +against the shipped code; where they disagree the CODE and the usb-kernel-recover +skill win, never this document. + +- **Pool guard.** A single constant, not the flat 4200s below and not a derivation: + `POOL_TIMEOUT = pos_int_env('HIL_POOL_TIMEOUT', 3600)`. A per-controller model briefly + lived here and was removed -- it under-modelled the flash phase and could INVERT + (adding a usbtest board lowered the guard, because the derived value fell below the + baseline it was meant to raise). The guard's only job is to stop a wedged pool short + of the job ceiling so the report still gets written; predicting a healthy run's + duration is a different problem. `pos_int_env` warns only on a non-integer or a value + <= 0: there is NO upper clamp and no warning above any threshold, so a pin larger than + a job ceiling silently restores the inversion this work removed. +- **Job ceilings.** 90/90/120 min (build.yml), not 60/60/90 and not the 85/115 below. + They must clear the 3600s guard plus the pre-pool checkout/artifact merge and the + post-guard sweep and report upload. No job pins `HIL_POOL_TIMEOUT`. +- **Battery budgets.** `USBTEST_BATTERY_BUDGET` 260s, `USBTEST_RECOVERY_BUDGET` 250s. + The 200s-with-a-197s-floor derivation recorded here was never shipped; the floor + assertion was removed with it. +- **HUNG recovery.** Reflash of the DUT through its roster flasher + (`usbtest.py --recover-board/--recover-fw`), not the root-cycle-first recovery in + section 1d — replaced after the 2026-08-11 ppps measurement (uhubctl never cuts + VBUS; root-cycle is probe-only). Since 2026-08-12 the reflash is SKIPPED + when `hil_flash.convoy_safe(board['flasher'])` is false (usbtest.py:675): the flasher + would enumerate by opening usbfs nodes, block on the same convoy, and become a second + stray rather than clear the first. A holder that owns the device lock inside a driver + ioctl is terminal either way -- a reflash only produces a disconnect, and + `usb_disconnect()` needs that same lock -- and that state needs a reboot. + +Step 0 done — the host was rebooted 2026-07-30 14:11 and the rig +came back clean. The device that triggered this incident was removed from the rig, so +only the containment work remains relevant. +Rig: `ci.lan` (Proxmox guest on `pve.lan`) + +## Problem + +On 2026-07-29/30 every board in the `ci.lan` usbtest fleet failed, `openocd` processes +landed in uninterruptible sleep, and no subsequent HIL run could start. Two GitHub +Actions runs were stranded: `30484641269` sat `in_progress` for over eight hours +(past GitHub's own 360-minute default), and `30485082274` sat `queued` behind it from +2026-07-29 19:35 UTC onward. Both report directories were written empty. + +A reboot of the `ci` guest at 10:48 did not clear the condition: the same kernel state +re-formed at 10:52:23. + +## Root cause + +Five layers, each independently observable. + +### 1. A permanently wedged hub worker holds a root-hub device lock + +A device that repeatedly re-asserts connect while failing to enumerate keeps +`hub_event()` busy, and `hub_event()` holds `usb_lock_device(hdev)` on its hub for its +whole run (hub.c:5896/5989). The `usb_hub_wq` worker sits in `hub_port_reset`, so that +hub's `device_lock` is effectively never released: + +``` +kworker/14:6+usb_hub_wq (state D, 400+ s) + msleep+0x2b + hub_port_reset+0x1a4 [usbcore] + hub_event+0x727 [usbcore] +``` + +`usb usbN-portM: Cannot enable. Maybe the USB cable is bad?` is logged every four seconds +for as long as it lasts. + +Verified against hub.c v6.12.96 rather than inferred: the kernel does **not** retry +without bound, and root and downstream ports are bounded identically — +`hub_port_reset()` tries `PORT_RESET_TRIES` then logs that message (hub.c:3149), +`hub_port_connect()` wraps it in `PORT_INIT_TRIES` = 4 and disables the port on give-up +(hub.c:5455/5619). A count in the thousands is therefore that many separate connect +events, not one runaway loop, and it indicts the device rather than the port. + +### 2. A parked board storms the second controller + +`ra6m5_ek` (`test/hil/tinyusb.json`, uid `8419032D32363657364EF4622D294B4E`, at +`13-3.3`) runs dfu firmware (`cafe:400b`) and re-enumerates every 1-2 seconds +continuously, wrapping the entire bus-13 devnum space (`...120 -> 127 -> 4 -> 6 -> 10`). +This is standing `hub_event` and Address-Device pressure on controller `03:00.0`, +concurrent with parallel usbtest batteries on the same silicon. + +The board is already listed in `boards-skip`, which is precisely why it storms: +`boards-skip` stops testing a board but never parks it, so it keeps running whatever +firmware it last received. Park-flash only runs as teardown of a board that actually +executed tests. + +### 3. The kernel `usbtest` control-queue case waits without a timeout + +`test_ctrl_queue` blocks on an untimed `wait_for_completion()` while `usbdev_ioctl` +holds the DUT's `device_lock`: + +``` +wait_for_completion+0x8a <- no _timeout variant +test_ctrl_queue+0x4ab [usbtest] +usbtest_do_ioctl+0x501 [usbtest] +usbdev_ioctl+0x6b8 [usbcore] +``` + +`--timeout 60` in `test/hil/usbtest.py` is a subprocess timeout only. `SIGKILL` is not +delivered to a task in uninterruptible sleep. `usbtest.py` already recognises this and +reports `HUNG`, then calls `usb_recover.sh root-cycle`. + +### 4. openocd inherits the convoy and the whole fleet dies + +Once a device lock is stuck, `port_event()` takes a child device's lock to warm-reset +it and blocks while still holding its hub's lock. Any later +`open("/dev/bus/usb/BBB/DDD")` against such a device blocks uninterruptibly: + +``` +usbdev_open+0xdc [usbcore] -> __mutex_lock +chrdev_open -> do_sys_openat2 -> __x64_sys_openat +``` + +That is the state of the three `openocd` processes at 04:16:51 (pids 207921, 207987, +208034) — the flasher, unkillable. Because one controller carries two buses, a single +convoy takes out every board on both, which is why the failure presents as the entire +fleet. + +The existing `HUNG` recovery cannot help here. A root-port VBUS cycle frees a +*device-lock* holder; it cannot free a lock held by a stuck *hub worker*, and on this +rig the cycle lands on the controller that is already wedged. + +### 5. Nothing bounds the damage, so one bad run becomes a CI outage + +- `hil-tinyusb` and `hil-tinyusb-esp` in `.github/workflows/build.yml` carry no + `timeout-minutes`. Only `hil-hfp-iar` does. +- `ci.lan` runs a single runner service, so there is one job slot. +- `test/hil/hil_test.py` bounds the pool with `POOL_TIMEOUT` (4200 s), and that guard + fires correctly — but the recovery path does not survive a D-state worker: + +```python +with Pool(processes=os.cpu_count() or 1, initializer=init_worker, initargs=initargs) as pool: + async_ret = pool.map_async(test_board, config_boards) + try: + mret = async_ret.get(timeout=POOL_TIMEOUT) + except MpTimeoutError: + pool.terminate() + pool.join() # blocks forever: a D-state worker never reaps + raise RuntimeError(f'HIL worker pool timed out after {POOL_TIMEOUT}s') +``` + +`multiprocessing` joins workers unbounded, so both `pool.terminate()` and +`pool.join()` hang, as does the `with Pool(...)` exit on the success path. Normal +`hil-tinyusb (tinyusb.json)` runs take 10-20 minutes; one recent run took 71.3 +minutes, which is the 70-minute guard firing and succeeding. The eight-hour run is the +pathological case. + +## Design + +### Step 0 — recovery (manual prerequisite) + +Power-cycle the PVE **host**, not the `ci` guest. A guest reboot is not sufficient; +hubs latch up across the PCIe reset, which the 10:48 reboot demonstrated. Nothing +below can be verified until the rig is clean. + +### Section 1 — CI containment + +**1a. Two layered timers.** An inner guard inside `hil_test.py` (`POOL_TIMEOUT`, 70 min) +that fails gracefully -- it writes a report naming the timeout and the dispatched boards, +shuts the pool down and exits -- and an outer `timeout-minutes` per rig job (85 for the +hil-tinyusb jobs; 115 for hil-hfp-iar, which also builds four boards with IAR in the same +job) as the backstop for when even exiting cannot free the runner. The ceiling must stay +ABOVE the inner guard, or GitHub kills the job before the report is written. + +> **Corrected after measurement.** An earlier revision cut the guard to 30 min on the +> reading that real runs take 9-17 min and everything longer was the old guard firing. +> That was wrong. `hil_lock.py` records 22.2/14.3/12.5/10.8 min at usbtest width 1/2/3/4, +> and raising the per-battery budget to 380s made hung boards cost more again. The 30 min +> guard then fired on 5 of the last 8 HIL job executions across both rigs, and because +> `map_async` is all-or-nothing each of those runs published a banner instead of any +> per-board result. Restored to 4200s, the value whose original rationale -- usbtest +> batteries are serialized fleet-wide, lengthening the tail -- was correct. + +**1b. Bound the pool shutdown.** Add a helper to `test/hil/hil_test.py`: + +```python +def _shutdown_pool(pool, grace=30): + """terminate() a Pool without ever blocking forever: multiprocessing joins its + workers unbounded, and a worker in uninterruptible sleep (wedged usbfs) never + reaps -- which would hold the runner's only job slot indefinitely.""" + t = threading.Thread(target=pool.terminate, daemon=True) + t.start() + t.join(grace) + return not t.is_alive() +``` + +On the `MpTimeoutError` path: write the report first, recording the boards that never +reported so the run stops producing an empty report directory; then `_shutdown_pool`; +then `os._exit(1)` if it did not return. The hard exit is the point — it is the only +way past a kernel-side unkillable child. Use the same helper for the `with Pool(...)` +exit path. + +**1c. Pre-flight rig health check.** `check_rig_health()` runs before the build and +**never aborts**. It probes `/proc` unprivileged (dmesg is restricted on the rig) for a +wedged `usb_hub_wq` worker, and reports a `/proc` too restricted to trust as its own +distinct cause rather than as a diagnosed fault. + +It is deliberately non-fatal: the rig is unattended and every remedy for a real wedge is +manual, so aborting would not fix anything -- it would discard the per-board results the +run can still collect and leave CI red until a human noticed. It emits a GitHub +`::error::` annotation and continues. The automatic containment is 1a and 1b, which bound +a stuck run and explain it without anyone touching the rig. + +**1d. Order the recovery correctly.** In `test/hil/usbtest.py`, attempt +`usb_recover.sh root-cycle` FIRST on a `HUNG` case, and only check for a wedged hub worker +*afterwards*. + +> **Corrected during implementation.** This section originally said to check for a wedged +> worker *before* the cycle and skip it on a hit. That is backwards. Our own stuck +> `testusb` holds the DUT's device lock, so any port event drives a hub worker into +> `usb_lock_device()` on it -- uninterruptible, so it reads `D` in ~100% of samples and the +> confirmation window makes the wrong verdict *more* confident, not less. Cutting VBUS is +> precisely what completes the in-flight URB, returns the ioctl and frees that worker, so +> gating on that signature would suppress the recovery in the exact ordering it exists for. +> A worker still wedged after the cycle is the genuinely unrecoverable case, and that is +> what the code now reports. + +## Verification + +- Unit-test `shutdown_pool` and the `hil_health` detectors against a synthetic `/proc`. + A real wedge cannot be manufactured on demand, so they are tested against fabricated + inputs rather than live hardware. +- Confirm the detectors flag a genuinely wedged rig, and return clean on a healthy one. +- One clean full-fleet `hil_test.py` run to prove `check_rig_health` does not + false-abort. + +## Out of scope + +- **`ra6m5_ek` park and its dfu reset loop.** Dropped by decision. Consequence: the + layer-2 devnum storm remains as standing pressure on controller `03:00.0`. Unplugging + the board or flashing `board_test` by hand resolves it without any code change. +- **An unattended PVE watchdog** that detects the wedge and power-cycles the host. + Declined: more moving parts, and it can cut a running CI job. -- cgit v1.3.1 From c7290c4d3167766055f492de43f1ede83940e23c Mon Sep 17 00:00:00 2001 From: hathach Date: Mon, 17 Aug 2026 19:20:32 +0700 Subject: docs: hand off follow-up work as per-PR plans Records the convention in CLAUDE.md -- deferred work is a SEPARATE scope that deserves its own PR, written by another session, so it is handed off as a writing-plans doc in docs/superpowers/followup/pr-.md rather than accumulated in the PR that found it. Five handoffs from #3803: flasher_recover (convoy-safe recovery for J-Link boards, seven validated on the rig), the blindness reporting gaps, the usbtest recovery reserve, the IAR re-run spec, and the pci-rebind stranding question. Each carries what is already established with its citations and measurements, what remains, and why it was split out. One doc per follow-up, not one per PR: a per-PR file invites unrelated work into the same document and rots as a unit. --- CLAUDE.md | 1 + .../superpowers/followup/pr3803-flasher-recover.md | 280 +++++++++++++++++++++ .../followup/pr3803-hil-blindness-reporting.md | 185 ++++++++++++++ .../followup/pr3803-hil-iar-rerun-spec.md | 118 +++++++++ .../followup/pr3803-pci-rebind-stranding.md | 157 ++++++++++++ .../followup/pr3803-usbtest-recovery-reserve.md | 175 +++++++++++++ 6 files changed, 916 insertions(+) create mode 100644 docs/superpowers/followup/pr3803-flasher-recover.md create mode 100644 docs/superpowers/followup/pr3803-hil-blindness-reporting.md create mode 100644 docs/superpowers/followup/pr3803-hil-iar-rerun-spec.md create mode 100644 docs/superpowers/followup/pr3803-pci-rebind-stranding.md create mode 100644 docs/superpowers/followup/pr3803-usbtest-recovery-reserve.md (limited to 'docs') diff --git a/CLAUDE.md b/CLAUDE.md index 7a493e5db..4198081fb 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -20,6 +20,7 @@ Bias toward caution over speed. For trivial tasks, use judgment. - **Safety:** no dynamic allocation; defer ISR work to task context; use `TU_ASSERT()` for error checks; always check return values; include order: C stdlib → tusb common → drivers → classes. - **Layout:** `src/` core, `hw/{mcu,bsp}/` MCU+BSP, `examples/{device,host,dual}/`, `test/{unit-test,fuzz,hil}/`, `docs/`, `tools/`. - **Commits/PRs:** imperative mood, scoped changes, link issues, include test/build evidence. After opening a PR, drive it to green: address automated review comments (Copilot/Codex/Claude) and fix failing CI, pushing follow-ups until checks pass and threads resolve. Useful: `gh pr checks --watch`, `gh pr view --comments`. +- **Deferred work:** work that is worth doing but is a *separate scope* from the current PR — it deserves its own PR, written by a different session. Write it as a **handoff** with the `superpowers:writing-plans` skill, one doc per follow-up, in `docs/superpowers/followup/pr-.md` (the PR it was split out of, so the origin stays traceable). Say what is already established (with citations/measurements), what remains, and why it was split out. Delete the doc when its PR lands. Never bundle unrelated follow-ups into one file. - **Formatting/lint:** `clang-format` (`.clang-format`), `codespell` (`.codespellrc`); run `pre-commit run --all-files` before submitting. ## Bootstrap diff --git a/docs/superpowers/followup/pr3803-flasher-recover.md b/docs/superpowers/followup/pr3803-flasher-recover.md new file mode 100644 index 000000000..e9fff7480 --- /dev/null +++ b/docs/superpowers/followup/pr3803-flasher-recover.md @@ -0,0 +1,280 @@ +# `flasher_recover` Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Give the 15 HIL boards whose flasher cannot reach its probe past a poisoned usbfs +node a second, convoy-safe flasher used only for recovery. + +**Architecture:** An optional roster key `flasher_recover` beside `flasher`. +`hil_flash.recover_flasher(board)` picks it when present; `hil_test` substitutes it into the +`--recover-board` JSON so `usbtest.py` never learns a second entry exists. Delivery over +openocd's jlink driver is convoy-safe by construction, but the flash command form must +differ from the one `flash_openocd` uses, so the recovery gets its own flasher name. + +**Tech Stack:** Python 3.13 stdlib, openocd 0.12.0+dev (build 0ce743125 on ci.lan), +libjaylink, J-Link probes. + +## Global Constraints + +- Roster JSON: `test/hil/tinyusb.json`. `flasher_recover` is OPTIONAL; absent means today's + behaviour (`recover_flasher` returns the primary). +- Never change the shape of `board['flasher']` — it is read as a dict in `hil_flash`, + `hil_test`, `usbtest`, `hil_pool_check`, `hil_select` and the roster lint, and is shipped + as JSON to a subprocess. +- Flasher dispatch is by name: `getattr(hil_flash, f'flash_{name}')` / `reset_{name}`. +- `RECOVER_FLASH_TIMEOUT = 90`, `RECOVER_RESET_TIMEOUT = 30` (`usbtest.py`). Any board whose + flash cannot finish inside 90 s is not a candidate. +- Tests run offline: `cd test/hil && python3 test/test_hil_select.py`. + +## What is already established + +**Landed on PR #3803 and inert without roster entries:** `hil_flash.recover_flasher()`, +`convoy_safe()` accepting openocd-over-jlink, `hil_test` substituting the recovery flasher +into `--recover-board`, and `test_hil_select.FlasherRecoverEntry` (4 tests). + +**Verified in source:** +- openocd's jlink driver ignores `adapter usb vid_pid` — `jlink.c` never reads + `adapter_usb_get_vids/pids`; selection is `adapter serial` / USB address / usb location. + Do NOT lint a jlink recovery entry for `vid_pid`. +- It is convoy-safe anyway: libjaylink `discovery_usb.c` returns early unless + `idVendor == 0x1366` and the PID is in its table, and only THEN calls `libusb_open`. A + wedged `cafe:4010` DUT is never opened. +- CMSIS-DAP stays pin-gated: `cmsis_dap_usb_bulk.c:107` skips before `libusb_open`, and + `id_filter` is only `vids[0] || pids[0]`. + +**Measured on ci.lan 2026-08-17**, base args +`-f interface/jlink.cfg -c "transport select swd" -c "adapter speed 4000" -f target/`: + +| Board | target cfg | flash | reset | +|--------------------------|--------------|-------|-------| +| stm32f407disco | stm32f4x | OK | OK | +| stm32f072disco | stm32f0x | OK | OK | +| stm32f723disco | stm32f7x | OK | OK | +| stm32l476disco | stm32l4x | OK | OK | +| feather_nrf52840_express | nrf52 | OK | OK | +| metro_m4_express | atsame5x | OK | OK | +| frdm_k64f | k60 | OK | OK | + +`frdm_k64f` is host-only (`tests.device == false`) — verify its reset over UART +(`/dev/serial/by-id/usb-SEGGER_J-Link_000621000000-if00`), never by USB disconnect. + +**Excluded, with reasons:** `lpcxpresso11u37` — 118 s for 24 KB at 1 MHz with a verify +mismatch, versus 0.277 s via JLinkExe; cannot fit `RECOVER_FLASH_TIMEOUT`. +`mimxrt1064_evk`, `ra4m1_ek`, `nrf54lm20dk` — no target config exists in this openocd +build, so they cannot be covered at all. **The board that wedges most (mimxrt1064_evk) is +therefore still uncovered by this work.** + +**The blocker this plan solves:** `flash_openocd` issues `program verify reset exit`, +which fails over the jlink transport on BOTH families tried (`stm32f4x`, `stm32f0x`) with +`Examination failed` → `auto_probe failed`, with or without a preceding `init; reset halt`. +Every successful flash above used the explicit sequence in Task 1. + +**Why this is a separate PR:** it adds a roster capability and a new flasher backend, which +is a different scope from containing a wedge; and it needs bench time on seven boards. + +## File Structure + +- `test/hil/hil_flash.py` — add `flash_openocd_seq` / `reset_openocd_seq`; extend + `convoy_safe` to accept the new name. This is the only file that learns the command form. +- `test/hil/tinyusb.json` — seven `flasher_recover` entries. +- `test/hil/test/test_hil_select.py` — extend `FlasherRecoverEntry`; add a roster lint. + +--- + +### Task 1: `openocd_seq` flasher backend + +**Files:** +- Modify: `test/hil/hil_flash.py` (beside `flash_openocd`, ~line 100) +- Test: `test/hil/test/test_hil_select.py` + +**Interfaces:** +- Consumes: `_openocd_cmd_base(flasher)`, `hil_util.run_cmd`. +- Produces: `flash_openocd_seq(board, firmware, timeout=None)`, + `reset_openocd_seq(board, timeout=None)`, both returning + `subprocess.CompletedProcess`; `convoy_safe()` returns True for + `{'name': 'openocd_seq', 'args': '...interface/jlink.cfg...'}`. + +- [ ] **Step 1: Write the failing test** + +```python + def test_openocd_seq_is_convoy_safe_over_jlink(self): + self.assertTrue(hil_flash.convoy_safe( + {'name': 'openocd_seq', 'args': '-f interface/jlink.cfg -f target/stm32f4x.cfg'})) + + def test_openocd_seq_uses_explicit_flash_commands_not_program(self): + """`program` fails over the jlink transport: Examination failed -> auto_probe + failed, measured on stm32f4x and stm32f0x.""" + seen = {} + real = hil_util.run_cmd + hil_util.run_cmd = lambda cmd, **k: seen.setdefault('cmd', cmd) or real('true') + try: + hil_flash.flash_openocd_seq( + {'flasher': {'name': 'openocd_seq', 'uid': 'X', 'args': '-f interface/jlink.cfg'}}, + '/tmp/fw.elf', timeout=5) + finally: + hil_util.run_cmd = real + self.assertIn('flash write_image erase /tmp/fw.elf', seen['cmd']) + self.assertIn('verify_image /tmp/fw.elf', seen['cmd']) + self.assertNotIn('program ', seen['cmd']) +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `cd test/hil && python3 test/test_hil_select.py FlasherRecoverEntry -v` +Expected: FAIL — `module 'hil_flash' has no attribute 'flash_openocd_seq'` + +- [ ] **Step 3: Write minimal implementation** + +```python +def flash_openocd_seq(board, firmware, timeout=None): + # Explicit commands, NOT `program`: over the jlink transport `program` fails at the + # flash bank probe ("Examination failed" -> "auto_probe failed"), measured on + # stm32f4x and stm32f0x, with or without a preceding reset halt. This sequence + # succeeded on all seven candidate boards. + flasher = board['flasher'] + verify = f' -c "verify_image {firmware}"' if flasher.get('verify', True) else '' + return hil_util.run_cmd( + f'{_openocd_cmd_base(flasher)} -c "init" -c "reset halt" ' + f'-c "flash write_image erase {firmware}"{verify} -c "reset run" -c "shutdown"', + timeout=timeout) + + +def reset_openocd_seq(board, timeout=None): + flasher = board['flasher'] + return hil_util.run_cmd( + f'{_openocd_cmd_base(flasher)} -c "init" -c "reset run" -c "shutdown"', + timeout=timeout) +``` + +In `convoy_safe`, replace `if name != 'openocd':` with: + +```python + if name not in ('openocd', 'openocd_seq'): + return False +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `cd test/hil && python3 test/test_hil_select.py FlasherRecoverEntry -v` +Expected: PASS + +- [ ] **Step 5: Commit** + +```bash +git add test/hil/hil_flash.py test/hil/test/test_hil_select.py +git commit -m "hil: add openocd_seq flasher for convoy-safe recovery delivery" +``` + +--- + +### Task 2: Roster entries for the seven validated boards + +**Files:** +- Modify: `test/hil/tinyusb.json` +- Test: `test/hil/test/test_hil_select.py` + +**Interfaces:** +- Consumes: `flash_openocd_seq` / `reset_openocd_seq` from Task 1. +- Produces: seven boards for which `hil_flash.convoy_safe(hil_flash.recover_flasher(b))` + is True. + +- [ ] **Step 1: Write the failing test** + +```python + def test_roster_recover_entries_are_convoy_safe_and_named_openocd_seq(self): + import json, pathlib + roster = json.loads((pathlib.Path(__file__).parent.parent / 'tinyusb.json').read_text()) + recover = [b for b in roster['boards'] if 'flasher_recover' in b] + self.assertGreaterEqual(len(recover), 7) + for b in recover: + f = b['flasher_recover'] + self.assertEqual(f['name'], 'openocd_seq', b['name']) + self.assertIn('interface/jlink.cfg', f['args'], b['name']) + self.assertIn('adapter speed', f['args'], b['name']) # required; see below + self.assertTrue(hil_flash.convoy_safe(f), b['name']) +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `cd test/hil && python3 test/test_hil_select.py FlasherRecoverEntry -v` +Expected: FAIL — `0 >= 7` + +- [ ] **Step 3: Add the entries** + +`adapter speed` is REQUIRED: without it examination fails outright on the jlink driver. +Add to each board below, using the SAME `uid` as its primary jlink entry: + +```json +"flasher_recover": { + "name": "openocd_seq", + "uid": "", + "args": "-f interface/jlink.cfg -c \"transport select swd\" -c \"adapter speed 4000\" -f target/.cfg" +} +``` + +| Board | `uid` | `` | +|--------------------------|----------------|-----------| +| stm32f407disco | 000773661813 | stm32f4x | +| stm32f072disco | 779541626 | stm32f0x | +| stm32f723disco | 000776606156 | stm32f7x | +| stm32l476disco | 777632258 | stm32l4x | +| feather_nrf52840_express | 681295394 | nrf52 | +| metro_m4_express | 123456 | atsame5x | +| frdm_k64f | 000621000000 | k60 | + +- [ ] **Step 4: Run test to verify it passes** + +Run: `cd test/hil && python3 test/test_hil_select.py -v` +Expected: PASS, and no other selector test regresses. + +- [ ] **Step 5: Commit** + +```bash +git add test/hil/tinyusb.json test/hil/test/test_hil_select.py +git commit -m "hil: give seven J-Link boards a convoy-safe recovery flasher" +``` + +--- + +### Task 3: Bench validation on the rig + +**Files:** none — this task produces evidence, not code. + +- [ ] **Step 1: Confirm the rig is idle and take the locks** + +```bash +ssh hathach@ci.lan 'if pgrep -f "[h]il_test.py" >/dev/null; then echo BUSY; exit 1; fi' +ssh hathach@ci.lan 'cd ~/actions-runner/_work/tinyusb/tinyusb && \ + nohup timeout 900 python3 test/hil/helper/hil_lock.py hold --reason "flasher_recover validation" &' +``` + +Guard with `if`, never `cmd && echo || echo` — that form only gates the echo and will take +locks during a live CI run. + +- [ ] **Step 2: For each board, flash then reset through the recovery entry** + +```bash +python3 test/hil/hil_test.py -b test/hil/tinyusb.json # normal path still works +``` + +Then force the recovery path by running usbtest with the recovery flags and a firmware that +hangs a case, or drive `hil_flash.flash_openocd_seq` / `reset_openocd_seq` directly. + +- [ ] **Step 3: Verify** + +Device boards: `sudo dmesg` shows `USB disconnect` then a fresh enumeration. +`frdm_k64f`: UART shows the boot banner (see above). +Every flash must finish well inside `RECOVER_FLASH_TIMEOUT` (90 s). + +- [ ] **Step 4: Release locks and record the results in the PR body** + +--- + +## Out of scope, and why + +- **`mimxrt1064_evk`** needs an i.MX RT target config that this openocd build does not + have. Sourcing or writing one is its own investigation; until then the board with the + most wedges has no automated recovery. +- **Changing `flash_openocd`** to the explicit form would cover these boards without a new + name, but `program` is what nine pinned CMSIS-DAP boards use in CI daily and no CMSIS-DAP + image could be built in the originating worktree (no pico-sdk) to re-validate it. diff --git a/docs/superpowers/followup/pr3803-hil-blindness-reporting.md b/docs/superpowers/followup/pr3803-hil-blindness-reporting.md new file mode 100644 index 000000000..69ff939b0 --- /dev/null +++ b/docs/superpowers/followup/pr3803-hil-blindness-reporting.md @@ -0,0 +1,185 @@ +# Blindness Reporting Gaps Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Make a HIL worker's sysfs blindness reach the report in the two cases where it +currently does not — an untested producer, and a board that raises. + +**Architecture:** A worker returns `hil_util.sysfs_blind()` as the last field of its result +tuple; `_blind_note()` turns that into a report banner. Two holes: nothing tests the +producer, and a board that raises returns no tuple at all, so its blindness is lost. + +**Tech Stack:** Python 3.13 stdlib, multiprocessing Pool with `maxtasksperchild=1`. + +## Global Constraints + +- A blind worker answers `SYSFS_UNKNOWN` for every attribute, so its "device not found" + means "could not tell". The report must say so or a red cell reads as a broken board. +- `maxtasksperchild=1`: one worker per board, so the flag is per-board and must not be + smeared across boards. +- Tests: `cd test/hil && python3 test/test_hil_bounded.py`. + +## What is already established + +- `hil_test.test_board` returns `(..., hil_util.sysfs_blind(), stray)`; `_blind_note(mret)` + renders the banner; wired into all three report paths. +- **The producer is provably untested**: replacing `hil_util.sysfs_blind()` with `False` in + the return leaves all tests green. Nothing drives `test_board` — it needs a board dict, a + real flock, a flasher and `test_example` per test. +- Blindness fired for real on ci.lan: four workers went blind in one run, and cells failed + *because* of it (`Printer device not found ... (this worker is blind)`). + +**Why this is a separate PR:** closing it means making `test_board` testable, which is a +refactor of the harness's orchestration layer — a different scope from the containment +work, and the reason the gap was accepted rather than papered over. + +## File Structure + +- `test/hil/hil_test.py` — extract the result-tuple assembly from `test_board` so it can be + built and asserted without running a board; carry blindness out of the raise path. +- `test/hil/test/test_hil_bounded.py` — tests for both. + +--- + +### Task 1: Make the result tuple assembly testable + +**Files:** +- Modify: `test/hil/hil_test.py` (`test_board`, the `return (name, err_count, ...)` at the + end of the try block) +- Test: `test/hil/test/test_hil_bounded.py` + +**Interfaces:** +- Produces: `_board_result(name, err_count, failed_tests, rows, t_total, board_wide_fail)` + returning the 7-tuple `(name, err_count, failed, rows, t_total, blind, stray)`, reading + `hil_util.sysfs_blind()` and `hil_health.kill_own_children()` itself. + +- [ ] **Step 1: Write the failing test** + +```python +class BoardResultCarriesBlindness(unittest.TestCase): + def test_a_blind_worker_reports_it(self): + from helper import hil_util, hil_health + self.addCleanup(setattr, hil_util, 'sysfs_blind', hil_util.sysfs_blind) + self.addCleanup(setattr, hil_health, 'kill_own_children', hil_health.kill_own_children) + hil_util.sysfs_blind = lambda: True + hil_health.kill_own_children = lambda: 0 + row = hil_test._board_result('b', 0, [], [], 1.0, False) + self.assertTrue(row[5], 'blindness did not reach the result tuple') + self.assertIn('b', hil_test._blind_note([row])) + + def test_a_sighted_worker_does_not(self): + from helper import hil_util, hil_health + self.addCleanup(setattr, hil_util, 'sysfs_blind', hil_util.sysfs_blind) + self.addCleanup(setattr, hil_health, 'kill_own_children', hil_health.kill_own_children) + hil_util.sysfs_blind = lambda: False + hil_health.kill_own_children = lambda: 0 + row = hil_test._board_result('b', 0, [], [], 1.0, False) + self.assertFalse(row[5]) + self.assertEqual(hil_test._blind_note([row]), '') +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `cd test/hil && python3 test/test_hil_bounded.py BoardResultCarriesBlindness -v` +Expected: FAIL — `module 'hil_test' has no attribute '_board_result'` + +- [ ] **Step 3: Write minimal implementation** + +```python +def _board_result(name, err_count, failed_tests, rows, t_total, board_wide_fail): + """Assemble a worker's result tuple. Separate from test_board so the two fields only + the WORKER can answer -- its process-global blindness latch and what it could not kill + -- are testable without running a board.""" + stray = hil_health.kill_own_children() + return (name, err_count, [] if board_wide_fail else sorted(set(failed_tests)), + rows, t_total, hil_util.sysfs_blind(), stray) +``` + +Replace the tail of `test_board` with: + +```python + return _board_result(name, err_count, failed_tests, rows, t_total, board_wide_fail) +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `cd test/hil && python3 test/test_hil_bounded.py -v` +Expected: PASS, and the existing `BlindWorkerReachesTheReport` tests still pass. + +- [ ] **Step 5: Verify the mutation is now caught** + +Replace `hil_util.sysfs_blind()` with `False` inside `_board_result` and re-run; the suite +MUST fail. Restore it. + +- [ ] **Step 6: Commit** + +```bash +git add test/hil/hil_test.py test/hil/test/test_hil_bounded.py +git commit -m "test/hil: make the worker result tuple testable, covering blindness" +``` + +--- + +### Task 2: Carry blindness out of the worker-raise path + +**Files:** +- Modify: `test/hil/hil_test.py` (`test_board`'s except/finally, and `main`'s worker-raise + handler that builds synthetic rows) +- Test: `test/hil/test/test_hil_bounded.py` + +**Interfaces:** +- Consumes: `_board_result` from Task 1. +- Produces: a board that raises still contributes a row whose blindness field is accurate. + +- [ ] **Step 1: Write the failing test** + +```python + def test_a_board_that_raises_still_reports_blindness(self): + """The result tuple is returned inside a try whose finally only releases the lock, + so a board that dies by exception contributed nothing -- and its blindness, the + thing that most explains its failure, was lost with it.""" + from helper import hil_util + self.addCleanup(setattr, hil_util, 'sysfs_blind', hil_util.sysfs_blind) + hil_util.sysfs_blind = lambda: True + row = hil_test._board_result_on_error('b', RuntimeError('boom')) + self.assertTrue(row[5]) + self.assertIn('b', hil_test._blind_note([row])) +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `cd test/hil && python3 test/test_hil_bounded.py BoardResultCarriesBlindness -v` +Expected: FAIL — no `_board_result_on_error` + +- [ ] **Step 3: Write minimal implementation** + +```python +def _board_result_on_error(name, exc): + """A row for a board that died by exception. err_count 1, no per-test detail, but the + blindness and stray fields are still accurate -- they explain the failure more often + than the exception text does.""" + rows = [(name, {BOUNDARY_CELL: f'{REPORT_CELL["fail"]} {type(exc).__name__}'}, None)] + return _board_result(name, 1, [], rows, 0.0, True) +``` + +Wrap the body of `test_board` so the exception path returns it instead of propagating. + +- [ ] **Step 4: Run test to verify it passes** + +Run: `cd test/hil && python3 test/test_hil_bounded.py -v` +Expected: PASS + +- [ ] **Step 5: Commit** + +```bash +git add test/hil/hil_test.py test/hil/test/test_hil_bounded.py +git commit -m "test/hil: keep a raising board's blindness in the report" +``` + +--- + +## Caution + +`test_board`'s `finally` releases the board flock. Any restructuring MUST keep that +release on every path, including the new error path — a leaked flock locks the board until +the host reboots. diff --git a/docs/superpowers/followup/pr3803-hil-iar-rerun-spec.md b/docs/superpowers/followup/pr3803-hil-iar-rerun-spec.md new file mode 100644 index 000000000..fe377f741 --- /dev/null +++ b/docs/superpowers/followup/pr3803-hil-iar-rerun-spec.md @@ -0,0 +1,118 @@ +# IAR HIL Leg Re-run Spec Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Let the `hil-hfp-iar` CI leg re-run only its failed boards, as the other two HIL +legs already do. + +**Architecture:** `hil_test.py` writes a `.failed` spec into `HIL_REPORT_DIR`; a +workflow step reads it on the next attempt and passes the boards back as arguments. The IAR +leg passes `--retry 1` like the others but sets no `HIL_REPORT_DIR` and has no read-back +step, so its spec is written into the workspace and never read. + +**Tech Stack:** GitHub Actions YAML, self-hosted runner. + +## Global Constraints + +- `.github/workflows/build.yml`. The two working legs are `hil-tinyusb` (matrix) — see its + `Set HIL report dir (per run+job; persists across run attempts)` and `Get re-run spec from + previous attempt` steps — and they are the pattern to copy. +- The report dir must be keyed by run id AND job so a matrix leg does not collide with + another, and must survive across run attempts (that is the whole point). +- The IAR leg is the only HIL job that BUILDS inline; its `Build` step is bounded at + `timeout-minutes: 30` under a 120-minute job ceiling. Do not disturb that. + +## What is already established + +- Verified by reading the workflow: `hil-hfp-iar` has neither `HIL_REPORT_DIR` nor a + `Get re-run spec` step, while passing `--retry 1`. +- Consequence: a GitHub re-run of that job re-tests its whole matrix. **This is not a + regression** — that leg never had the mechanism — and the unread spec costs only a file. +- The report artifact upload for that leg is named `hil-report-hfp-iar`. + +**Why this is a separate PR:** it is CI plumbing with no code change, it needs a real +re-run on the self-hosted runner to prove, and it duplicates ~15 lines of workflow that +would be better factored — a decision worth making on its own. + +## File Structure + +- `.github/workflows/build.yml` — the `hil-hfp-iar` job only. + +--- + +### Task 1: Give the IAR leg a persistent report dir and a re-run spec + +**Files:** +- Modify: `.github/workflows/build.yml` (job `hil-hfp-iar`) + +**Interfaces:** +- Consumes: `hil_test.py`'s existing `--report-dir` / `.failed` behaviour — no code change. +- Produces: `env.HIL_REPORT_DIR` for the job, and `$RERUN_ARGS` for the test step. + +- [ ] **Step 1: Copy the two steps from `hil-tinyusb`, before the Build step** + +```yaml + - name: Set HIL report dir (per run+job; persists across run attempts) + run: | + BASE=$HOME/hil-reports + echo "HIL_REPORT_DIR=$BASE/${GITHUB_RUN_ID}-hfp-iar" >> "$GITHUB_ENV" + + - name: Get re-run spec from previous attempt + run: | + SPEC="$HIL_REPORT_DIR/hfp.json.failed" + if [ -f "$SPEC" ]; then + echo "RERUN_ARGS=$(cat "$SPEC")" >> "$GITHUB_ENV" + echo "re-running only: $(cat "$SPEC")" + fi +``` + +Match the exact spec filename `hil_test.py` writes for this leg's config — read +`_write_failed_spec` and the `failed_fname` construction rather than assuming. + +- [ ] **Step 2: Pass the spec to the test step** + +```yaml + python3 test/hil/hil_test.py --retry 1 $SEL_ARGS hfp.json $RERUN_ARGS +``` + +`--retry 1` stays FIRST so argparse's last-wins keeps any explicit override working. + +- [ ] **Step 3: Point the artifact upload at the report dir** + +```yaml + path: ${{ env.HIL_REPORT_DIR }}/hil_report.md +``` + +- [ ] **Step 4: Validate the YAML** + +Run: `python3 -c "import yaml,sys; d=yaml.safe_load(open('.github/workflows/build.yml')); j=d['jobs']['hil-hfp-iar']; print(j['timeout-minutes'], [s.get('name') for s in j['steps']])"` +Expected: the ceiling is still 120, the Build step still carries `timeout-minutes: 30`, and +the two new steps appear before Build. + +- [ ] **Step 5: Commit** + +```bash +git add .github/workflows/build.yml +git commit -m "ci: let the IAR HIL leg re-run only its failed boards" +``` + +--- + +### Task 2: Prove it on a real re-run + +**Files:** none — evidence only. + +- [ ] **Step 1:** Push and let `hil-hfp-iar` run to a failure (or force one). +- [ ] **Step 2:** Confirm `$HIL_REPORT_DIR/hfp.json.failed` exists on the runner after the + job. +- [ ] **Step 3:** Use GitHub's "Re-run failed jobs" and confirm the log line + `re-running only: ...` and that only those boards are tested. +- [ ] **Step 4:** Record the run URL in the PR body. + +--- + +## Consider first + +Three jobs would then carry the same ~15 lines. Factoring them into a composite action, or +computing the report dir inside `hil_test.py` from `GITHUB_RUN_ID`, may be the better +change — decide that before copying the block a third time. diff --git a/docs/superpowers/followup/pr3803-pci-rebind-stranding.md b/docs/superpowers/followup/pr3803-pci-rebind-stranding.md new file mode 100644 index 000000000..de1f7163b --- /dev/null +++ b/docs/superpowers/followup/pr3803-pci-rebind-stranding.md @@ -0,0 +1,157 @@ +# `pci-rebind` Stranding Investigation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Settle when a PCI unbind/rebind of an xHCI controller strands it driverless, so +the `usb-kernel-recover` skill can state a rule instead of a hypothesis. + +**Architecture:** No product code. This is a controlled reproduction against the rig's +kernel, ending in a documentation change and — if the boundary turns out to be +detectable — a guard in `usb_recover.sh`. + +**Tech Stack:** Linux 6.12.96 (ci.lan), Renesas uPD720201 xHCI, `usb_recover.sh`. + +## Global Constraints + +- ci.lan is a live CI rig. Take every affected board's lock first + (`hil_lock.py hold --all --reason ...`) and confirm no `hil_test.py` is running, with an + `if`, not an `&&` chain. +- A stranded controller takes every fixture on it offline; recovery is + `usb_recover.sh pci-bind ` or, failing that, a PVE **host** power cycle — an + operator action. Do not start this without being able to reach the host. +- The rig has two Renesas controllers plus an AMD one; pick the controller with the fewest + fixtures for the experiment. + +## What is already established + +**The skill claimed, unconditionally, that `pci-rebind`'s re-bind hangs on the D-state URB +and leaves the controller with no driver.** That claim was generalised from ONE observation +and was used to delete `pci-rebind` and `pci-bind` from `usb_recover.sh` entirely. + +**It was refuted in the field on 2026-08-17.** After `hub-cycle 17-2.7` failed to clear a +wedge, `pci-rebind 0000:05:00.0` recovered the controller in about one second: + +``` +02:34:41 remove, state 4 / USB bus 18 deregistered +02:34:41 remove, state 1 / USB bus 17 deregistered +02:34:42 xHCI Host Controller / new USB bus registered, assigned bus number 1 +02:34:42 new USB bus registered, assigned bus number 2 +``` + +Both actions were restored, with the guidance scoped to failure mode: **dead controller → +use it; device-lock convoy → do not**. Buses renumbered 17/18 → 1/2, which is why rig-wide +operations need every board's lock. + +**What is NOT known:** why the earlier attempt stranded and this one did not. The leading +hypothesis is that it turns on whether a live D-state URB exists **on that controller** at +the moment of the re-bind — but in the 02:34 incident the wedged board (17-2.7) was on that +very controller, which weakens it. An alternative is that `hub-cycle` had already cleared +the holder, leaving only a dead controller. + +**Why this is a separate PR:** it is an experiment that risks taking the rig offline, and +its output is a documentation change plus possibly a guard — a different scope from any +code change. + +## File Structure + +- `.claude/skills/usb-kernel-recover/SKILL.md` — replace the hypothesis in section 3b and + the Common-mistakes entry with whatever the experiment establishes. +- `.claude/skills/usb-kernel-recover/scripts/usb_recover.sh` — only if the boundary is + detectable from userspace. + +--- + +### Task 1: Reproduce a controller-scoped D-state wedge + +**Files:** none. + +- [ ] **Step 1: Establish the safety net** + +```bash +ssh hathach@ci.lan 'if pgrep -f "[h]il_test.py" >/dev/null; then echo BUSY; exit 1; fi' +# hold ALL boards on the target controller +``` + +Confirm host access to pve.lan before continuing. + +- [ ] **Step 2: Create a wedge deliberately** + +Run `usbtest.py` against a board known to hang (`mimxrt1064_evk` has wedged eight times, +TEST 9/10/24/27), or drive `testusb` directly until a case does not return. + +- [ ] **Step 3: Confirm the holder and its controller** + +```bash +ps -eo pid,stat,etimes,wchan:22,args | awk '$2 ~ /D/' +sudo cat /proc//stack # usbdev_ioctl + [usbtest] = the owner +readlink -f /sys/bus/usb/devices/usb # bus -> PCI addr +``` + +Record whether the holder is on the SAME controller you will rebind. + +--- + +### Task 2: Rebind and record the outcome + +**Files:** none. + +- [ ] **Step 1: Rebind, with a bounded observer** + +```bash +timeout 120 sudo usb_recover.sh pci-rebind ; echo "rc=$?" +``` + +- [ ] **Step 2: Record which of the three outcomes occurred** + +1. Re-bind completes, controller recovers (as on 2026-08-17). +2. Re-bind hangs; `/sys/bus/pci/devices//driver` is gone → **stranded**. +3. Re-bind completes but the wedge persists. + +Capture `sudo journalctl -k --since ...` around the attempt either way. + +- [ ] **Step 3: If stranded, recover** + +```bash +sudo usb_recover.sh pci-bind +``` + +If that hangs too, the only remaining step is a PVE host power cycle — an operator action. + +- [ ] **Step 4: Repeat at least three times** + +One observation is what produced the wrong rule in the first place. Vary whether a D-state +holder is live on that controller at rebind time; that is the hypothesis under test. + +--- + +### Task 3: Write down what was learned + +**Files:** +- Modify: `.claude/skills/usb-kernel-recover/SKILL.md` + +- [ ] **Step 1: Replace section 3b's scoping with the measured rule** + +State the condition under which stranding occurs, with the journal lines. If the experiment +does NOT reproduce stranding, say that too, with the attempt count — "not reproduced in N +attempts" is a better record than an unexplained warning. + +- [ ] **Step 2: If the boundary is detectable, guard the script** + +For example, refuse `pci-rebind` when a D-state holder exists on that controller, since the +holder is enumerable from `/proc` and the controller from `readlink`. Only add this if the +experiment shows it predicts the outcome. + +- [ ] **Step 3: Commit** + +```bash +git add .claude/skills/usb-kernel-recover/ +git commit -m "skills: replace the pci-rebind stranding hypothesis with measurement" +``` + +--- + +## Abort criteria + +Stop and hand back to the operator if: a rebind strands the controller and `pci-bind` does +not recover it; `uhubctl` starts hanging (the convoy has spread to the hub locks); or a CI +run starts while the rig is in a broken state. diff --git a/docs/superpowers/followup/pr3803-usbtest-recovery-reserve.md b/docs/superpowers/followup/pr3803-usbtest-recovery-reserve.md new file mode 100644 index 000000000..eb8959520 --- /dev/null +++ b/docs/superpowers/followup/pr3803-usbtest-recovery-reserve.md @@ -0,0 +1,175 @@ +# usbtest Recovery Reserve Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Make the post-hang recovery reserve a derived, asserted property instead of an +accident of four independently-set constants. + +**Architecture:** `hil_test` passes `--budget` and `--outer-timeout` to `usbtest.py`, which +decides at runtime whether a recovery still fits. Today the reserve survives only because +the four numbers happen to line up; nothing ties them together or fails when they stop. + +**Tech Stack:** Python 3.13 stdlib. + +## Global Constraints + +- `usbtest.py`: `RECOVER_FLASH_TIMEOUT = 90`, `RECOVER_RESET_TIMEOUT = 30`. +- `hil_test.py`: `USBTEST_BATTERY_BUDGET = 260`, `USBTEST_RECOVERY_BUDGET = 250`, + `USBTEST_OVERSHOOT = 120`; `outer = BATTERY_BUDGET + (RECOVERY_BUDGET if recovery else + OVERSHOOT)`, used for both the child's `--outer-timeout` and the parent's `run_cmd` bound. +- All five are env-overridable via `hil_util.pos_int_env`, so a rig can change them. +- Tests: `cd test/hil && python3 test/test_hil_health.py` and `test_hil_bounded.py`. + +## What is already established + +The reserve holds at the shipped values, checked by hand: + +- The battery checks its budget BEFORE dispatching a case, so it can overshoot by one + case — worst case `260 + 60 + 5 = 325 s`. +- Recovery is gated on `_time_left() >= RECOVER_RESET_TIMEOUT`, where + `_time_left() = outer_timeout - elapsed - 35`; with `outer = 510` that allows recovery + until `elapsed = 445 s`, and the reflash until `385 s`. +- So ~60 s of margin survives, and recovery does fire. + +**The defect is structural, not arithmetic:** lower `--outer-timeout`, raise `--timeout`, or +raise `USBTEST_BATTERY_BUDGET` via the env and the reserve silently disappears. The failure +mode is a skipped reflash that leaves the D-state holder for the next job — the exact thing +the containment exists to prevent — with no error anywhere. + +**Why this is a separate PR:** it changes the timing contract between `hil_test` and +`usbtest.py`, which affects every board's run duration, so it wants its own review and a +full rig run. + +## File Structure + +- `test/hil/usbtest.py` — a `reserve_ok()` predicate plus a startup assertion. +- `test/hil/hil_test.py` — derive the battery budget from the outer bound rather than + setting both independently. +- `test/hil/test/test_hil_health.py` — tests. + +--- + +### Task 1: Assert the reserve at startup + +**Files:** +- Modify: `test/hil/usbtest.py` (constants block, and `main()` after argparse) +- Test: `test/hil/test/test_hil_health.py` + +**Interfaces:** +- Produces: `usbtest.reserve_ok(budget, outer, case_timeout)` returning bool. + +- [ ] **Step 1: Write the failing test** + +```python +class RecoveryReserveIsChecked(unittest.TestCase): + """The battery may overshoot its budget by ONE already-started case, so the outer bound + must leave room for that overshoot AND a bounded recovery afterwards.""" + + def setUp(self): + import usbtest + self.u = usbtest + + def test_the_shipped_numbers_leave_room(self): + self.assertTrue(self.u.reserve_ok(budget=260, outer=510, case_timeout=60)) + + def test_a_tighter_outer_bound_is_rejected(self): + self.assertFalse(self.u.reserve_ok(budget=260, outer=380, case_timeout=60)) + + def test_a_longer_case_timeout_is_rejected(self): + self.assertFalse(self.u.reserve_ok(budget=260, outer=510, case_timeout=200)) +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `cd test/hil && python3 test/test_hil_health.py RecoveryReserveIsChecked -v` +Expected: FAIL — `module 'usbtest' has no attribute 'reserve_ok'` + +- [ ] **Step 3: Write minimal implementation** + +```python +def reserve_ok(budget: int, outer: int, case_timeout: int) -> bool: + """Does `outer` leave room for the battery's worst case AND a bounded recovery? + + The budget is checked BEFORE dispatch, so the battery can run to + `budget + case_timeout + 5` (the +5 is run_case's reap). _time_left() subtracts a + further 35 s of fixed tail. A reflash needs RECOVER_FLASH_TIMEOUT beyond that. + """ + worst_case_end = budget + case_timeout + 5 + return outer - worst_case_end - 35 >= RECOVER_FLASH_TIMEOUT +``` + +In `main()`, after parsing args: + +```python + if args.budget and args.outer_timeout and not reserve_ok( + args.budget, args.outer_timeout, args.timeout): + print(f'warning: --outer-timeout {args.outer_timeout} leaves no room for a bounded ' + f'recovery after a --budget {args.budget} battery with --timeout ' + f'{args.timeout} cases; a HUNG board will be left wedged', file=sys.stderr) +``` + +Warn, do not exit: a caller that deliberately runs without recovery is legitimate. + +- [ ] **Step 4: Run test to verify it passes** + +Run: `cd test/hil && python3 test/test_hil_health.py RecoveryReserveIsChecked -v` +Expected: PASS + +- [ ] **Step 5: Commit** + +```bash +git add test/hil/usbtest.py test/hil/test/test_hil_health.py +git commit -m "usbtest: check the recovery reserve instead of assuming it" +``` + +--- + +### Task 2: Derive the outer bound from one place + +**Files:** +- Modify: `test/hil/hil_test.py` (constants block ~line 227, and `test_device_usbtest`) +- Test: `test/hil/test/test_hil_bounded.py` + +**Interfaces:** +- Consumes: `usbtest.reserve_ok` semantics (duplicate the arithmetic, do not import + usbtest — `hil_test` must not import it). +- Produces: an assertion at module import that the shipped constants satisfy the reserve. + +- [ ] **Step 1: Write the failing test** + +```python + def test_the_shipped_constants_satisfy_the_reserve(self): + """Whatever the env overrides, the pair hil_test computes must leave recovery room: + outer - (budget + case_timeout + 5) - 35 >= 90.""" + outer = hil_test.USBTEST_BATTERY_BUDGET + hil_test.USBTEST_RECOVERY_BUDGET + self.assertGreaterEqual(outer - (hil_test.USBTEST_BATTERY_BUDGET + 60 + 5) - 35, 90) +``` + +- [ ] **Step 2: Run test to verify it fails** + +Temporarily set `HIL_USBTEST_RECOVERY_BUDGET=100` and run; expect FAIL. Unset. + +- [ ] **Step 3: Add the guard** + +```python +# The recovery reserve is a PROPERTY of these two, not a coincidence: the battery may +# overshoot its budget by one already-started case (checked before dispatch), and a bounded +# reflash needs 90 s after a 35 s fixed tail. Env overrides make this checkable at import +# rather than discoverable when a wedge is left unrecovered. +if USBTEST_RECOVERY_BUDGET - 60 - 5 - 35 < 90: + print(f'warning: HIL_USBTEST_RECOVERY_BUDGET={USBTEST_RECOVERY_BUDGET} leaves no room ' + f'for a bounded reflash after a one-case overshoot; HUNG boards will stay wedged', + file=sys.stderr) +``` + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `cd test/hil && python3 test/test_hil_bounded.py -v` +Expected: PASS + +- [ ] **Step 5: Commit** + +```bash +git add test/hil/hil_test.py test/hil/test/test_hil_bounded.py +git commit -m "hil: warn when the timeout constants leave no recovery reserve" +``` -- cgit v1.3.1