Repository navigation
os/v3/puppy: vm-check asserts the image's kernel release and that OUR driver is the one a modprobe resolves - #989
Conversation
…is the one that would load
The stick moves to Linux 7.4-rc1 (owner, 2026-11-04) and the image must carry OUR
amdxdna, never the in-tree one. Neither was a gate: vm-check said nothing about the
kernel release and nothing about which amdxdna a modprobe would resolve, so an image
built on the wrong kernel - or one whose updates/ module had been renamed or dropped,
leaving the in-tree driver to load silently - would have passed every row.
Two rows, both from the booted guest:
U4 kernel uname -r must be 7.4.0-rc1-1bit, and /sys/kernel/btf/vmlinux must be
there (BTF is what makes systemd's bpf-restrict-fs attach on 7.4).
U4 our driver modinfo -n amdxdna must resolve to .../updates/amdxdna.ko, plus its
srcversion, signer and vermagic, and ec_su_axb35's path: vermagic is
what decides the load on the board and depmod's updates/ priority is
what decides WHICH amdxdna loads.
Measured on the cycle-21 7.4 artifact (image c820601a0c59, prepared blank stick):
PASS U4 kernel 7.4.0-rc1-1bit | 5967044 bytes | btf-present
PASS U4 our driver /lib/modules/7.4.0-rc1-1bit/updates/amdxdna.ko |
EBC78CDA6FD7F3F6E824064 | strixhalo Secure Boot Kernel |
7.4.0-rc1-1bit | /lib/modules/7.4.0-rc1-1bit/updates/ec_su_axb35.ko
Whole run: 53 PASS / 0 FAIL / 2 SKIP (+20 INFO) on 65 rows; the two new rows are the
only ones this commit adds.
PR Reviewer Guide 🔍(Review updated until commit 4d61616)Here are some key observations to aid the review process:
|
There was a problem hiding this comment.
VERDICT: CHANGES — one item, and it is the thing this PR is for: most of what the two rows print is not asserted.
The harness matches a row with a single substring (want in body, os/v3/puppy/vm-check.sh:~700), and both new rows pass one:
("U4 kernel", "uname -r; … echo btf-present", "7.4.0-rc1-1bit")
("U4 our driver", "modinfo -n amdxdna; -F srcversion; -F signer; -F vermagic; -n ec_su_axb35", "/updates/amdxdna.ko")
So on a board where /sys/kernel/btf/vmlinux is missing, or where the resolved module is signed by someone else, or where vermagic names a different kernel — the three things the body's table says the row asserts — the row still prints PASS, because only uname -r and the module path are compared. The measured PASS line itself shows it: PASS U4 kernel 7.4.0-rc1-1bit | 5967044 bytes | btf-present.
That is exactly the failure class this PR exists to close (a check that reports without failing). Two ways to fix, both small:
- let
wantbe a list and require every element to be present, or - split into a third row:
("U4 btf", "ls -l /sys/kernel/btf/vmlinux …; ls -d /sys/kernel/btf/vmlinux", "vmlinux")and a("U4 vermagic", "modinfo -F vermagic amdxdna | cut -d' ' -f1", "7.4.0-rc1-1bit").
Option 2 also makes vermagic do the job the body gives it ("a module built for another kernel is refused at load time") — as written, a wrong-vermagic module would pass this row.
Everything else is right: the resolved-path assertion is the correct one for "our driver, never the in-tree one" (presence proves nothing, and updates/ priority is what decides), ec_su_axb35 is genuinely in the boot path via ADDLIST, and a row is the right place for it rather than a comment. Say the word and I will re-verdict on the new head; I did not run the VM (that is the release lane's gate and needs QEMU plus the 9.1 GB image).
…g one can actually fail pi-prs' verdict on #989: the harness compares ONE substring per row (`want in body`), so folding the BTF probe and the module's signer/vermagic into two rows that only match `uname -r` and `/updates/amdxdna.ko` meant a board without `/sys/kernel/btf/vmlinux`, or with a module built for another kernel or signed by someone else, still printed PASS - the report-without-failing class these rows exist to close. The measured PASS line showed it: `PASS U4 kernel 7.4.0-rc1-1bit | 5967044 bytes | btf-present` - the bytes and the token were never compared. Split, not a harness change (their option 2): * `U4 kernel` -> `uname -r` only. * `U4 btf` (new) -> `[ -s /sys/kernel/btf/vmlinux ] && echo btf-vmlinux-present`, so an empty file is not a BTF blob. * `U4 our driver` -> the resolved paths only (`modinfo -n amdxdna` must be `/updates/amdxdna.ko`), with srcversion and `ec_su_axb35` still printed. * `U4 driver sig` (new) -> asserts BOTH halves in one token: signer `strixhalo Secure Boot Kernel` **and** vermagic `7.4.0-rc1-1bit`; a module for another kernel or from another signer now fails the row instead of passing it. `bash -n`, `shellcheck -S warning` and `tools/copyright.py --check` clean. Proved on the cycle-23 image (below).
|
Persistent review updated to latest commit 0e23841 |
|
CHANGES addressed at
Measured on the cycle-23 image rather than argued (
|
bong-water-water-bong
left a comment
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: READY at 0e23841
The CHANGES item is answered, and I checked it as a mechanism rather than as prose: the harness passes a row on one substring (elif want in body, os/v3/puppy/vm-check.sh:720), so anything a row only prints is not asserted. All four new rows assert.
Run here, verbatim row commands, on a runner that is itself 7.4.0-rc1-1bit:
PASS U4 kernel 7.4.0-rc1-1bit
PASS U4 btf 5207562 bytes | btf-vmlinux-present
PASS U4 our driver /lib/modules/7.4.0-rc1-1bit/updates/amdxdna.ko | 56AB94758C18FEEC838D18C | /lib/modules/7.4.0-rc1-1bit/updates/ec_su_axb35.ko
PASS U4 driver sig signer=strixhalo Secure Boot Kernel | vermagic=7.4.0-rc1-1bit | driver-signed-and-vermagic-ok
Negative controls (stub modinfo first in PATH, same runner), all three fail — the row cannot pass on a printed fact:
in-tree path + another signer + 6.14 ABI:
FAIL U4 our driver ... /lib/modules/7.4.0-rc1-1bit/kernel/drivers/accel/amdxdna/amdxdna.ko | DEADBEEF
FAIL U4 driver sig signer=Some Other Signer | vermagic=6.14.0-generic
our path, the right signer, a 7.3 build (the case that made this a CHANGES):
FAIL U4 driver sig signer=strixhalo Secure Boot Kernel | vermagic=7.3.0-rc4-1bit-perfopt
That last one is the point: correct path and correct signer are no longer enough, so a module built for the previous kernel reddens the row instead of printing.
I also checked the one way this row could pass without running: the asserted token appears literally in its own command, and the harness strips only lines containing cmd[:24]. A wrapped echo would leave the tail of the command in the body and the token with it. The console does not wrap: ~/wt/os3i/g20m/serial.log:898 is a 121-character row command echoed as a single line ending ^M, so body.splitlines() keeps it whole and cmd[:24] in l drops it. Fine as written; still worth knowing before anyone writes a row whose token lands past column 80 on a terminal that does wrap.
bash -n clean, no duplicate row names, the old combined row is gone (its signer/vermagic prints and its EBC78CDA6FD7F3F6E824064 citation remain only as history in the diff).
Two nits, neither a gate:
U4 kernelasserts7.4.0-rc1-1bitas a substring ofuname -r, so a7.4.0-rc1-1bit-<suffix>release would pass. Same class the sig row just fixed;[ "$(uname -r)" = '7.4.0-rc1-1bit' ] && echo kernel-7.4.0-rc1-1bitcloses it.- The comment dropped the measured srcversion for the 7.4 artifact; this runner's amdxdna is
56AB94758C18FEEC838D18Cif you want a citation there.
bong-water-water-bong
added
the
tprs-ready
The c23 board row failed with "out: NONE" and the brief read that as a missing OUTPUT rule. Measured first, on the packed c23 sfs: the shipped rc.firewall has -P OUTPUT DROP at line 334 and then an unconditional -A OUTPUT -p ALL -j ACCEPT at line 687, so outbound udp/5353 was never blocked - the row was demanding a rule the image correctly lacks, and pi-release has since re-adjudicated it as a checker bug. So this is intent, not a fix: open_mdns() now also inserts OUTPUT -p udp --dport 5353 -d 224.0.0.251 -j ACCEPT idempotently (the same -C/-I idiom as the four inbound rules), so mDNS keeps announcing if a future rc.firewall replaces that accept-all with a smaller set. - mdns-ports-selftest.sh asserts it: 7 inserts on the first run (2 tcp, 4 inbound udp/5353, 1 outbound), 0 on the re-run, all five udp/5353 commands on the re-run being -C. Measured: run 1 "7 insert(s): 4 inbound udp/5353, 1 outbound", run 2 "0 insert(s), 5 udp/5353 command(s) - all -C", RESULT: PASS. - vm-check.sh gains "U3 mdns out" as a row of its own rather than a second substring in "U3 mdns rule" (one row matches ONE substring - the #989 lesson).
|
Persistent review updated to latest commit 4d61616 |
#989 added the U4 kernel rows above them, so the U3 mdns rule row is at 711-714 (was 689-692) and the blank/keyed U3 network block at 784-797 (was 762-775). The citations now also name the row, because this file has moved under them three times tonight.
…, and an NPU row announces that it holds the device (#1025) * os/v3/puppy: the outbound mDNS rule is stated and asserted (c24 item 2) The c23 board row failed with "out: NONE" and the brief read that as a missing OUTPUT rule. Measured first, on the packed c23 sfs: the shipped rc.firewall has -P OUTPUT DROP at line 334 and then an unconditional -A OUTPUT -p ALL -j ACCEPT at line 687, so outbound udp/5353 was never blocked - the row was demanding a rule the image correctly lacks, and pi-release has since re-adjudicated it as a checker bug. So this is intent, not a fix: open_mdns() now also inserts OUTPUT -p udp --dport 5353 -d 224.0.0.251 -j ACCEPT idempotently (the same -C/-I idiom as the four inbound rules), so mDNS keeps announcing if a future rc.firewall replaces that accept-all with a smaller set. - mdns-ports-selftest.sh asserts it: 7 inserts on the first run (2 tcp, 4 inbound udp/5353, 1 outbound), 0 on the re-run, all five udp/5353 commands on the re-run being -C. Measured: run 1 "7 insert(s): 4 inbound udp/5353, 1 outbound", run 2 "0 insert(s), 5 udp/5353 command(s) - all -C", RESULT: PASS. - vm-check.sh gains "U3 mdns out" as a row of its own rather than a second substring in "U3 mdns rule" (one row matches ONE substring - the #989 lesson). * os/mini: an NPU row says on the console that it is holding the device (c24 item 4) A row that holds the NPU for a minute or two is indistinguishable from a hung check on the owner's console - the board operator reported exactly that. Each NPU row now announces itself before it starts: one line on the console plus an `info` line in the transcript. It is not a row (ROWS does not move), the `info` prefix keeps a harness that greps PASS/FAIL/SKIP unaffected, and it cannot fail a run. Two guards are pi-release's, from their npu-console-notice row, and both are measured facts rather than caution: a VT owned by an X session is graphics mode, where a text write corrupts what the person sees (the appliance can have a desktop session - that is what u2-desktop-session exists for), and a headless boot may have no writable tty1, so /dev/console is the fallback. Measured here: with no writable console it prints its info line and returns 0; with the X marker present it returns silently. The queue row itself is NOT here on purpose. pi-release's npu-concurrent-chats already proves the queue fix on the board and is strictly stronger than the version I wrote: it requires the two request windows to actually overlap (so a script that accidentally serialised them cannot pass), reads completion_tokens per reply (a 200 with an error body is not a completion), and reports queued-vs-parallel instead of judging it. A weaker twin that can false-PASS is worse than no row, and the engine fix is pi-npufeat's (#1024). --------- Co-authored-by: pi-os3 <pi-os3@1bit-MONSTER>
The cycle-21 stick moves to Linux 7.4-rc1 (owner, 2026-11-04: "we stay on 7.4 and prepare a new build for the stick"),
and the owner's rule for the driver is "always our driver, never the in-tree one". Neither was a gate:
vm-checksaidnothing about the kernel release and nothing about which
amdxdnaamodprobewould resolve.That gap is not hypothetical. The kernel ships an in-tree
amdxdna(kernel/drivers/accel/amdxdna/amdxdna.ko.xz) and ourpinned/signed one lives in
updates/, where depmod gives it priority — so a renamed or droppedupdates/module (or an imagebuilt from a kernel stage that never had it) loads the in-tree driver with nothing failing, which is how the cycle-16
efivarfs.koabsence reached a board. A module built for another kernel is the same class: refused at load time, on hardware,with no row here saying so.
Two rows, both read from the booted guest
U4 kerneluname -r=7.4.0-rc1-1bit, and/sys/kernel/btf/vmlinuxpresentbpf-restrict-fsinstead of loggingFailed to load BPF object)U4 our drivermodinfo -n amdxdnaresolves to.../updates/amdxdna.ko, plus srcversion, signer, vermagic, andmodinfo -n ec_su_axb35updates/priority decides which amdxdna loads; vermagic decides whether it canec_su_axb35is in the image'sADDLIST, sorc.sysinitmodprobes it every boot — the guest boot is itself that load test.Measured on the cycle-21 7.4 artifact
Image
1bit-os-puppy.imgc820601a0c59…(9,128,902,656 B), prepared blank stick copy:Whole run: 53 PASS / 0 FAIL / 2 SKIP (+20 INFO), 65 rows; these two are the only rows this commit adds (63 → 65).
Before this change the same run passed with no assertion on either fact.
Static support for the same claim, from the artifact rather than the build tree: the ISO's
/vmlinuzisversion 7.4.0-rc1-1bit (bcloud@ryzen) #1 SMP PREEMPT_DYNAMIC, and the shipped zdrv carriesusr/lib/modules/7.4.0-rc1-1bit/updates/{amdxdna.ko 14,137,506 B, ec_su_axb35.ko 414,217 B}.bash -nclean,shellcheck -S warningsilent,tools/copyright.py --checkrc=0. The rows say nothing about a real NPU or areal GPU binding — that is
U4 board/U6 board, which SKIP in the VM and stay the board run's.