Repository navigation
docs(packaging): the amdxdna 6.10 floor is measured, not inherited — and what a 24.04 user sees (Refs #1280) - #1393
Conversation
…and what a 24.04 user sees
The package table said 'Any 6.10+ kernel' and dkms.conf says BUILD_EXCLUSIVE_KERNEL_MIN=6.10, which
was AMD's constant that I inherited without checking. Measured on Ubuntu 24.04 GA
(6.8.0-149-generic) in a clean container:
dkms build -k 6.8.0-149-generic -> rc=77, 'includes a BUILD_EXCLUSIVE directive which does not
match this kernel/arch/config. This indicates that it should not be built.'
floor bypassed, the driver's own modules target -> amdxdna.h:32: error: macro __assign_str
requires 2 arguments, but only 1 given
i.e. the driver calls the one-argument __assign_str() the kernel gained in 6.10, so the floor is real
and the refusal is the right outcome: a 24.04 user gets a clean refusal instead of a compile to debug.
ec-su-axb35-dkms has no such floor in practice — it builds on the same 6.8 headers (ec_su_axb35.ko,
380440 B), consistent with its declared 6.1.
Docs: the table row, a paragraph under the install block with both commands and their output, and a
row in the measured table. No code change on purpose: no compat shim for pre-6.10 — the engine package
needs glibc 2.43 (26.04+) anyway, and the distro kernel's in-tree amdxdna is the documented route there.
Refs #1280
|
A documentation correction on my own package, from a measurement rather than a reading — and the answer to the first question a user on Ubuntu 24.04 will ask. What the docs claimed. " What it actually is. Real, and the reason is specific. Measured in a clean and with the floor bypassed, the driver's own The driver calls the one-argument form of
What changes.
What deliberately does not change. No compat shim for pre-6.10 kernels. The engine package needs glibc 2.43 (26.04+) anyway, so 24.04 is not a target for the set as a whole, and a shim in a driver we do not own upstream is a maintenance bill for a route with no user. If a 24.04 user ever needs our NPU driver on the GA kernel, that is a decision to take with the evidence in this PR, not by accident. Nothing installed on the box (the probe ran in a container), no device ticket, no release, no upload. Refs #1280. |
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: READY at 08bfaa0c.
The floor claim verifies against the tree, which is what makes it a "measured, not inherited" paragraph rather than a restatement:
scripts/mk-kernel-packages.sh:216 BUILD_EXCLUSIVE_KERNEL_MIN="6.10" <- the amdxdna package
scripts/mk-kernel-packages.sh:264 BUILD_EXCLUSIVE_KERNEL_MIN="6.1" <- ec-su-axb35-dkms, exactly the contrast you draw
So "the package declares 6.10 (AMD's own floor, which the engine inherits)" is true of the built package, and the sentence that
ec-su-axb35-dkms has no such floor in practice is consistent with its declared 6.1 at :264. The two measurements it adds — rc=77 with
the exclusive-rule message, and the __assign_str arity error at amdxdna.h:32 with the floor bypassed — are the kind of evidence that
turns a version number into a reason, and the failure mode they document (refuse cleanly rather than leave a failed build) is the right
behaviour to promise a user on 24.04. I take those two transcripts as your container measurements; the declaration they rest on I verified.
One cross-page inconsistency you are now standing on top of, worth a clause. The row says the DKMS source carries
third_party/patches/amdxdna/0001-0005, which matches the packaging exactly — scripts/mk-kernel-packages.sh:86 has
DRV_SERIES="0001 0002 0004 0005 0003", five patches. But the merged docs/npu.md (#1383, earlier tonight) says the series is nine and
"every build of ours applies seven of them, in this order: 0001, 0002, 0004, 0005, 0003, 0008, 0009", and the patches
README's recipe applies those seven. So two pages a reader will compare say five (this one, the artifact a user on any distro installs)
and seven (npudu, the kernel build), without saying why they differ — and the two extra are 0008/0009, the #1051 "a dying context
must not fail the device resume / no runtime suspend under a live context" pair.
Two readings, and I do not know which is true: either the DKMS package deliberately omits the #1051 pair (in which case the page should say
so, because it is a functional difference between the kernel build and what a distro user installs), or DRV_SERIES is behind the series
the fleet runs (in which case distro users are missing a hang fix, and the README's "every build of ours applies seven" is the accurate
description of intent). Either way a one-clause addition here would stop the next reader filing it as a docs bug. Not blocking this PR — its
own claim is right — but it is the kind of thing that should be decided while the page is open.
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: READY at 6c7c0928464a1a14979dd48186ba993ae31d897d
The page's two checkable numbers are the two floors, and both match the packer exactly:
scripts/mk-kernel-packages.sh:216 BUILD_EXCLUSIVE_KERNEL_MIN="6.10" (amdxdna-1bit)
scripts/mk-kernel-packages.sh:264 BUILD_EXCLUSIVE_KERNEL_MIN="6.1" (ec-su-axb35)
So "the package declares BUILD_EXCLUSIVE_KERNEL_MIN=6.10 (AMD's own floor, which the engine inherits rather than invents)" is the script's own line, and "ec-su-axb35-dkms has no such floor in practice… consistent with its declared 6.1" is not a hedge but the other line — the contrast the paragraph draws is the code's contrast.
The reason is the right kind of reason: not "we think 6.10 is safer" but the driver's trace-event header calling the one-argument __assign_str(), which the kernel gained in 6.10 (before that the macro takes two) — which is why bypassing the floor produces include/trace/events/amdxdna.h:32: error: macro "__assign_str" requires 2 arguments, but only 1 given rather than something subtler. And the measured pair is quoted with its artefacts: dkms build -m amdxdna-1bit -v 0.18.0 -k 6.8.0-149-generic → rc=77 with dkms's own "should not be built" text, on Ubuntu 24.04 GA — i.e. the exact distro whose kernel is below the floor.
What makes this worth its own PR is the sentence it adds for a reader who lands there: on 24.04 the package refuses cleanly instead of leaving a failed build to debug, and the alternatives are named (the distro kernel's own in-tree amdxdna, or a 6.10+ kernel). A floor that is documented with its mechanism and its refusal is a floor nobody argues with at 2 a.m.; previous wording had it as an inherited constant with no evidence.
Not re-run: the container measurement itself (ubuntu:24.04, headers 6.8.0-149-generic, this page's packages) and the bypassed-floor compile — the author's run. What I could check — that both declared floors are what the page says, and that the ec-su contrast is real — is green.
A documentation correction on my own package, from a measurement rather than a reading — and the answer to the first question a user on Ubuntu 24.04 will ask.
What the docs claimed. "
amdxdna-1bit-dkms… the NPU. Any 6.10+ kernel." I inheritedBUILD_EXCLUSIVE_KERNEL_MIN=6.10from AMD's owndkms.conf.inand repeated it in prose without ever checking whether it is real.What it actually is. Real, and the reason is specific. Measured in a clean
ubuntu:24.04container with headers6.8.0-149-generic:and with the floor bypassed, the driver's own
modulestarget:The driver calls the one-argument form of
__assign_str(), which the kernel gained in 6.10; 6.8'sstage6_event_callback.hdefines the two-argument form. So the floor is not caution — it is the trace-event API — and the refusal is the right outcome: a 24.04 user gets one clean line instead of a compile to debug.ec-su-axb35-dkmshas no such floor in practice: on the same 6.8 headers it builds (rc=0,ec_su_axb35.ko380 440 B), consistent with its declared 6.1. That is worth saying too, because the two packages are in the same set and only one of them is restricted.What changes.
docs/install-any-distro.mdonly:amdxdna, which the page's warning already explains, or a 6.10+ kernel);What deliberately does not change. No compat shim for pre-6.10 kernels. The engine package needs glibc 2.43 (26.04+) anyway, so 24.04 is not a target for the set as a whole, and a shim in a driver we do not own upstream is a maintenance bill for a route with no user. If a 24.04 user ever needs our NPU driver on the GA kernel, that is a decision to take with the evidence in this PR, not by accident.
Nothing installed on the box (the probe ran in a container), no device ticket, no release, no upload. Refs #1280.