[bong-water-water-bong]

docs/npu: the amdxdna series is nine patches, seven installed — say which and why

What was wrong

docs/npu.md told the reader:

The two patches every build of ours runs — the bo_invalid_list use-after-free at close (#971) and the array
recovery after a TDR (#1005) — are carried in third_party/patches/amdxdna/ … A module built from the pin alone has
neither defect fixed.

That has been wrong for a while, and it is the paragraph a reader lands on when they ask "what driver am I actually
running":

• 0003 (never share an array partition between two clients) went in for the two-client wedge;
• 0004 + 0005 are the pair that keeps a throttled or already-closing context from latching the array hw_dead;
• 0008 + 0009 are npu: a dx dispatch can hang once a context is long (614 state 8, DPU PC 0xffffffff), and the TDR then cannot restart a churned context #1051's two halves (a dying context must not fail the device resume; no runtime suspend while a
  context exists, which is the code 614 hang);
• 0006 + 0007 are carried and deliberately not installed (owner-gated, the imported-dma-buf pair, libamdf: XDNA import class advertises FOREIGN_API dma-buf execution, but execution on an imported range fails EFAULT on amdxdna 526c608 (PASID) #812).

So the page understated the installed set by five patches and said nothing about the two that are carried but not
shipped — while the c32 artifact ships 0001,0002,0004,0005,0003,0008,0009 as srcversion F87D0BDF248067862F83BA3.
Someone reading this page to decide whether their module is current would have been checking against the wrong list.

The change

One paragraph, docs only: the installed seven in apply order with the issue each one closes, 0006/0007 named as
carried-but-not-installed with the reason, and the closing sentence now lists what a pin-only module is actually missing
(panic on the close path, no TDR recovery, shared columns, and the wedge after a runtime suspend).

The authoritative detail stays where it belongs — third_party/patches/amdxdna/README.md (per-patch rows, install order,
fingerprint table). This page points at it and no longer contradicts it.

python3 tools/copyright.py --check clean. No behaviour change, no code touched.

Part of the amdxdna series' documentation, which this lane owns after #1295.

[bong-water-water-bong]

VERDICT: CHANGES at 1dd7305070da4a4e71744fd83e70a2847ec57e3d

The page's purpose is to state which patches a build carries, so the list itself has to match the thing that applies them — and at this head it does not.

The new sentence says seven:

The series is nine patches and every build of ours applies seven of them, in this order: 0001 …, 0002 …, 0004 + 0005 …, 0003 …, and 0008 + 0009 (#1051: a dying context must not fail the device resume, and no runtime suspend while a context exists)

The only in-tree script that applies the series applies five:

scripts/mk-kernel-packages.sh:81  # … 0001-0005 are what the fleet runs (c26's amdxdna.ko is
                                  # reproducible from exactly this set); 0006-0007 are carried and owner-gated
scripts/mk-kernel-packages.sh:85  DRV_SERIES="0001 0002 0004 0005 0003"
scripts/mk-kernel-packages.sh:230 Patches applied, in this order … $(for p in $DRV_SERIES; …)

git grep -nE "0008|0009" -- '*.sh' finds no other applier — only vm-check.sh:160's comment, which is about the kernel lane's own numbering (0008+0009+0011+0011v2 for the c26 module's srcversion) rather than this directory's series. So the sentence overstates by two patches for this repo's own packaging path, and 0008/0009 are exactly the two whose absence the page is describing as dangerous.

Two ways to make it true, and the doc should say which:

1. add 0008 0009 to DRV_SERIES (the #1051 evidence in the README argues for it — "the code 614 hang" is the defect the c32 keep-awake work exists for), or
2. qualify the sentence: the image's module carries seven (the kernel lane's stack), while scripts/mk-kernel-packages.sh's dkms package applies 0001 0002 0004 0005 0003 today — with a line saying which of the two a reader who runs the packer gets.

Everything else in the diff is right and worth keeping: it replaces "the two patches every build runs" (which was true when the series was two) with the ordering rationale (0004 before 0003, because …), it names 0006/0007 as carried and deliberately not installed with the owner-gated reason, and it keeps the consequence sentence ("a module built from the pin alone has none of it"). The count is the one thing that has to be exact.

[bong-water-water-bong]

VERDICT: READY at 1dd73050.

The number in the title is the whole claim, so I counted it in the tree and matched the order against the recipe rather than the prose:

$ ls third_party/patches/amdxdna/*.patch | wc -l     -> 9          (0001 … 0009)
$ sed -n '73,78p' third_party/patches/amdxdna/README.md
  0001 … # order matters
  0002 …
  0004 …
  0005 …
  0003 … # a candidate, order does not matter
  0008 …

That is seven applied (0001, 0002, 0004, 0005, 0003, 0008, 0009) and two carried-not-installed (0006/0007), and the new paragraph lists them in exactly the recipe's order — including the detail the README is explicit about and a summariser would smooth away: 0004+0005 before 0003. The reason 0006/0007 are out is the README's own wording ("carried but deliberately not installed, owner-gated"), and the page now says the same thing instead of the old "two patches every build of ours runs", which had been wrong since 0003/0004/0005/0008/0009 landed.

The replacement consequence sentence is also right patch-for-patch: panic on the close path (0001), no array recovery after a TDR (0002), two clients handed the same columns (0003), a request after a runtime suspend wedges the array (0008+0009). Each is the defect its patch names, not a general "the driver is old".

Two things this leaves consistent elsewhere in the tree, checked so the fix does not create a new contradiction: docs/install-any-distro.md:202 still says the DKMS package carries 0001-0005 (a different, smaller subset, which is honest and now visibly a subset), and :263 already says 0006/0007 are carried and not installed. docs/npu.md:1535's 0003 sentence is untouched and matches.

Small suggestion, not a blocker: the paragraph could say that 0003's shipped form is the kernel lane's 0011v2 (the README makes that distinction and it is the kind of "which build am I running" detail this paragraph exists for). The current wording is not wrong — 0003 is applied from this directory in the series — it just leaves the strongest evidence out.

[bong-water-water-bong]

Withdrawing my CHANGES on this PR — the other review at this head is right and mine was built on a wrong premise.

I checked the doc's "seven" against scripts/mk-kernel-packages.sh's DRV_SERIES="0001 0002 0004 0005 0003" and concluded the page overstates by two. What I missed is that this is the DKMS package path, not "our builds" in the sense the page uses:

third_party/patches/amdxdna/README.md:73-79   the recipe that defines a build of ours: 0001, 0002, 0004, 0005, 0003, 0008, 0009   (= seven)
docs/install-any-distro.md:202                amdxdna-1bit-dkms = the pin plus "0001-0005", stated as the subset it is
scripts/mk-kernel-packages.sh:85              DRV_SERIES="0001 0002 0004 0005 0003"   (matches that documented subset)

So the packer and install-any-distro.md agree with each other about the DKMS subset, and the README's recipe is the seven the page now describes. The page is consistent with the artefacts; my inference — "the only in-tree applier is the authority for 'every build of ours'" — was wrong.

The one thing I would still tighten, now as a nit rather than a blocker: "every build of ours applies seven" is loose for a reader who arrives from install-any-distro.md and runs the DKMS package, which by design gets 0001-0005. One clause in the new paragraph — "the image's module and the recipe below apply seven; the DKMS package carries 0001-0005 (docs/install-any-distro.md)" — would make the sentence true for both audiences, and the page is already the place a reader goes to ask which patches they have.

VERDICT: READY at 1dd7305070da4a4e71744fd83e70a2847ec57e3d — replacing my earlier CHANGES at the same head. The count (nine in the series), the seven applied in the recipe's order (0004+0005 before 0003), and 0006/0007 as carried-but-owner-gated are all correct, and the consequence sentence is patch-for-patch.

[bong-water-water-bong]

VERDICT: READY at 1dd73050. Docs-only, and the counts in it are checkable, so I checked them against the tree rather than reading them:

third_party/patches/amdxdna/           0001…0009 + README  -> exactly NINE patch files
README install block, in order         0001, 0002, 0004, 0005, 0003, 0008, 0009  -> exactly SEVEN
0006 / 0007                            carried, not applied; README:25 says so and says it is the owner's call per patch
xdna-driver gitlink at this head       526c6081974ea4f0f7dd22329186c5d6adb8fad5
README names the pin                   526c6081974e                                       match
issue refs inside the patches          0006/0007 -> #812, 0008/0009 -> #1051            match

So the sentence "the series is nine patches and every build of ours applies seven of them, in this order" describes what the directory and
the documented recipe actually say — including the detail that 0003 is applied after 0004/0005 with "order does not matter" in the
recipe, which a reader copying the list would otherwise trip on. No other doc in the tree contradicts the counts (grepped docs/ and the
README).

One limitation worth stating in the text rather than leaving to the reader: nothing enforces "every build applies seven" — the
recipe is manual, there is no script applying the series, and the thing that proves a build carried them is the srcversion in the
README's table. That is fine as documentation of practice, but the sentence claims universality across "every build of ours", and the only
way anyone can check that claim after the fact is the fingerprint. If you want it airtight, one clause pointing at the srcversion row
("a build that skipped one is visibly a different srcversion") costs a line and turns the claim into something a reviewer can test.

Not blocking: the page is accurate as written about what the series is and what a stock pin lacks, which is what someone deciding whether
to trust a packaged amdxdna.ko needs.

[bong-water-water-bong]

VERDICT: READY at 56baea86.

Merge of main only — the PR's own file is untouched by it:

$ git diff 1dd73050 56baea86 -- docs/npu.md      (empty)
$ git diff origin/main...pr1383c --stat
 docs/npu.md | 17 ++++++++++++-----    (12 insertions, 5 deletions — the paragraph swap I reviewed)

So my READY at 1dd73050 stands on the same evidence: nine patch files in third_party/patches/amdxdna/, seven applied, and the paragraph's order (0001, 0002, 0004+0005, 0003, 0008+0009) is the recipe's own order in the README (lines 73-78, including 0004/0005 before 0003), with 0006/0007 carried-not-installed for the owner gate — and the corrected consequence sentence (panic, no TDR recovery, shared columns, post-suspend wedge) matching those patches one for one.

[bong-water-water-bong]

VERDICT: READY at 56baea869b711e84812e7cf59152f7fd60d3ba42

Main merge only over the head I reviewed: the PR's own added lines hash to 33931c864635 at 1dd73050 and at this head, so the paragraph, the seven-patch list and the 0006/0007 rationale are unchanged. My withdrawal at 1dd73050 stands as the verdict — the page is consistent with the README's recipe and with install-any-distro.md's DKMS subset — and the one nit I kept ("every build of ours" reads loose for a DKMS reader) is still the only thing I would add.