diff --git a/flake.nix b/flake.nix index de9f6db..7034e35 100644 --- a/flake.nix +++ b/flake.nix @@ -120,6 +120,7 @@ tools/ci/distro-package.sh \ tools/ci/vm-guest-smoke.sh \ tools/ci/vm-smoke.sh \ + tools/ci/kernel-compat-probe-test.sh \ userspace/bench/tbv_vllm_smoke.sh python -m py_compile \ userspace/bench/tbv_perftest_runner.py \ @@ -134,6 +135,43 @@ runHook postInstall ''; + meta = { + maintainers = with pkgs.lib.maintainers; [ georgewhewell ]; + }; + }; + # The kernel compat probe decides whether to compile code against a + # struct member only some kernels have. It once failed OPEN -- a missing + # header made it claim the member was present -- and broke the build on + # linux 6.18.37. This runs the probe against synthetic kernel trees so + # that regression cannot reach a builder again. + mkKernelCompatProbeCheck = + pkgs: + pkgs.stdenv.mkDerivation { + pname = "thunderbolt-ibverbs-kernel-compat-probe"; + version = "0.3.4"; + src = ./.; + + nativeBuildInputs = [ + pkgs.bash + pkgs.gnumake + pkgs.gnugrep + pkgs.gnused + ]; + + dontConfigure = true; + + buildPhase = '' + runHook preBuild + bash tools/ci/kernel-compat-probe-test.sh "$PWD/kernel/Makefile" + runHook postBuild + ''; + + installPhase = '' + runHook preInstall + mkdir -p "$out" + runHook postInstall + ''; + meta = { maintainers = with pkgs.lib.maintainers; [ georgewhewell ]; }; @@ -426,6 +464,7 @@ thunderbolt-ibverbs = pkgsAt.thunderbolt-ibverbs; portable-kernel-patches = mkPortableKernelPatchCheck pkgs; script-syntax = mkScriptSyntaxCheck pkgs; + kernel-compat-probe = mkKernelCompatProbeCheck pkgs; proto-smoke = mkProtoSmoke pkgs; rdma-core-usb4 = pkgsAt.rdma-core-usb4; verbs-smoke-build = mkVerbsSmokeBuild pkgs; diff --git a/kernel/Makefile b/kernel/Makefile index 9d6be07..8f54da5 100644 --- a/kernel/Makefile +++ b/kernel/Makefile @@ -11,7 +11,50 @@ thunderbolt_ibverbs-y := main.o profile.o backend.o configfs.o core.o debugfs.o ccflags-y := -Wall -Wextra -Wno-unused-parameter -I$(src) -I$(src)/.. -.PHONY: all clean modules modules_install help +# COMPAT PROBE. The Asahi USB4 tree carries struct tb_ring.interval_nsec, which +# stock kernels do not. Probe for it rather than requiring a patched kernel, so +# one source builds against either. +# +# Two properties this probe must have, both of them paid for by a CI failure: +# +# 1. Look under BOTH $(KDIR)/include and $(KDIR)/source/include. A kernel +# packaged as a SPLIT build/source tree keeps only generated headers under +# build/ and the real ones under source/ -- which is what nixpkgs ships and +# what the Hydra builders run. Those two are SIBLINGS +# (/lib/modules/$(KVER)/{build,source}), though some packagings nest +# source/ inside build/, so both spellings are tried. +# +# 2. FAIL CLOSED. The previous form was +# ifneq ($(shell grep -c interval_nsec $(TBV_TB_HDR) 2>/dev/null),0) +# and grep on a missing file prints NOTHING, so $(shell ...) expanded to the +# empty string, "" != "0" was true, and the flag was defined precisely when +# the probe could not check anything. Against linux 6.18.37 that produced +# "error: 'struct tb_ring' has no member named 'interval_nsec'" -- a probe +# whose failure mode was to assert the thing it exists to doubt. +# grep -l prints a filename on a match and nothing otherwise, so a missing +# or unreadable header can only ever yield "absent". +# +# The pattern matches a member DECLARATION (identifier followed by ; , or :) +# rather than the bare string, so prose in a comment cannot enable the code. +TBV_TB_HDR := $(firstword $(wildcard $(KDIR)/include/linux/thunderbolt.h \ + $(KDIR)/source/include/linux/thunderbolt.h \ + $(KDIR)/../source/include/linux/thunderbolt.h)) +tbv-kernel-has = $(if $(TBV_TB_HDR),$(shell grep -lE '$(1)[[:space:]]*[;,:]' $(TBV_TB_HDR) 2>/dev/null)) + +ifneq ($(call tbv-kernel-has,interval_nsec),) +ccflags-y += -DTBV_HAVE_RING_INTERVAL_NSEC +endif + +.PHONY: all clean modules modules_install help compat-report + +# Report what the COMPAT PROBE above concluded, without building anything. +# tools/ci/kernel-compat-probe-test.sh drives this against synthetic kernel +# trees; it is also the fastest way for a human to answer "why did this build +# think my kernel had that member?". +compat-report: + @echo "TBV_TB_HDR=$(TBV_TB_HDR)" + @echo "interval_nsec=$(if $(findstring TBV_HAVE_RING_INTERVAL_NSEC,$(ccflags-y)),yes,no)" + all: modules diff --git a/kernel/path.c b/kernel/path.c index 11a4d53..1b2091d 100644 --- a/kernel/path.c +++ b/kernel/path.c @@ -4,6 +4,7 @@ #include #include #include +#include #include #include #include @@ -1288,6 +1289,52 @@ int tbv_path_alloc_rings(struct tbv_path *path, struct tb_xdomain *xd, return ret; } +/* + * Apple NHI interrupt throttle (0xd004c, 256 ns units). + * + * The stock/Intel register (0x38c00) is programmed through + * tb_ring_throttling(), and MUST NOT be written on an Apple NHI -- the register + * lives elsewhere there. On Apple hardware the kernel's own ring activation + * writes it: apple_nhi_ring_interrupt_active() programs + * APPLE_CIO_NHI_IRQ_THROTTLE from ring->interval_nsec, and SKIPS the write + * when that field is zero. A zero module parameter therefore proves nothing + * was written, and the register keeps whatever it held -- 255 * 256 ns = + * 65.28 us on the machine this was measured on, visible as a latency floor + * that does not move with message size (2 B through 4 KB all ~65.3 us, + * ib_send_lat across a USB4 link). + * + * This module never set ring->interval_nsec, so on an Apple NHI the throttle + * was silently skipped forever. This applies to every ring on an Apple NHI -- + * including native-backend rings toward a Linux peer -- because the NHI is + * Apple's regardless of who is on the other end of the cable. + */ +static bool tbv_host_is_apple(void) +{ + return of_machine_is_compatible("apple,arm-platform"); +} + +static void tbv_path_apply_ring_interval(struct tbv_path *path) +{ +#ifdef TBV_HAVE_RING_INTERVAL_NSEC + unsigned int interval = READ_ONCE(nhi_interrupt_throttle_ns); + u32 before, after; + + if (!interval || !tbv_host_is_apple()) + return; + + if (!path->tx_ring || !path->rx_ring) + return; + + before = path->tx_ring->interval_nsec; + WRITE_ONCE(path->tx_ring->interval_nsec, interval); + WRITE_ONCE(path->rx_ring->interval_nsec, interval); + after = path->tx_ring->interval_nsec; + + pr_info("apple ring throttle: interval_nsec %u -> %u ns (kernel programs it at activation)\n", + before, after); +#endif +} + int tbv_path_start_rings(struct tbv_path *path) { u32 i; @@ -1296,6 +1343,10 @@ int tbv_path_start_rings(struct tbv_path *path) if (path->state != TBV_PATH_RING_ALLOCATED) return -EINVAL; + /* Must be set before activation: the kernel reads ring->interval_nsec + * when it programs the Apple NHI throttle register. */ + tbv_path_apply_ring_interval(path); + tb_ring_start(path->tx_ring); tb_ring_start(path->rx_ring); path->state = TBV_PATH_RING_STARTED; diff --git a/tools/ci/kernel-compat-probe-test.sh b/tools/ci/kernel-compat-probe-test.sh new file mode 100755 index 0000000..668028b --- /dev/null +++ b/tools/ci/kernel-compat-probe-test.sh @@ -0,0 +1,81 @@ +#!/usr/bin/env bash +# +# Prove the COMPAT PROBE in kernel/Makefile behaves against synthetic kernel +# trees, without needing a real kernel to build against. +# +# Why this test exists. The probe decides whether to compile the code that +# writes struct tb_ring.interval_nsec, a member the Asahi USB4 tree carries and +# stock kernels do not. It used to be: +# +# ifneq ($(shell grep -c interval_nsec $(KDIR)/include/linux/thunderbolt.h),0) +# +# which has two defects that combine into a build failure: +# +# * it looks only under $(KDIR)/include, and a kernel packaged as a SPLIT +# build/source tree (nixpkgs, and therefore the Hydra builders) keeps the +# real headers under $(KDIR)/source/include; and +# * grep on a missing file prints nothing, so $(shell ...) is "", "" != "0" +# is true, and the flag was defined precisely when the probe could not +# check -- a probe whose failure mode is to assert what it exists to doubt. +# +# Against linux 6.18.37 that produced: +# path.c: error: 'struct tb_ring' has no member named 'interval_nsec' +# +# The rule this encodes: a capability probe MUST fail closed. +set -euo pipefail + +repo_root=$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd) +makefile=${1:-$repo_root/kernel/Makefile} + +work=$(mktemp -d) +trap 'rm -rf "$work"' EXIT + +# $1 name $2 layout(flat|split|nested|none) $3 header body +fixture() { + local name=$1 layout=$2 body=$3 inc + case $layout in + flat) inc=$work/$name/include/linux ; kdir=$work/$name ;; + split) inc=$work/$name/source/include/linux ; kdir=$work/$name/build ;; + nested) inc=$work/$name/build/source/include/linux ; kdir=$work/$name/build ;; + none) mkdir -p "$work/$name/build" ; echo "$work/$name/build" ; return ;; + esac + mkdir -p "$inc" "$kdir" + printf '%s\n' "$body" > "$inc/thunderbolt.h" + echo "$kdir" +} + +probe() { + make -s -f "$makefile" KDIR="$1" compat-report \ + | sed -n 's/^interval_nsec=//p' +} + +fail=0 +expect() { # $1 description $2 KDIR $3 expected(yes|no) + local got; got=$(probe "$2") + if [ "$got" = "$3" ]; then + printf ' ok %-46s -> %s\n' "$1" "$got" + else + printf ' FAIL %-46s -> %s (want %s)\n' "$1" "$got" "$3" + fail=1 + fi +} + +HAS='struct tb_ring { int hop; unsigned int interval_nsec; };' +HASNT='struct tb_ring { int hop; };' +COMMENT='/* the kernel programs the throttle from ring->interval_nsec */ +struct tb_ring { int hop; };' + +echo "COMPAT PROBE matrix ($makefile)" +expect "split tree, member absent (the Hydra case)" "$(fixture split-absent split "$HASNT")" no +expect "split tree, member present" "$(fixture split-has split "$HAS")" yes +expect "nested source/ packaging, member present" "$(fixture nested-has nested "$HAS")" yes +expect "flat tree, member absent (stock kernel)" "$(fixture flat-absent flat "$HASNT")" no +expect "flat tree, member present (Asahi USB4)" "$(fixture flat-has flat "$HAS")" yes +expect "header mentions it only in a comment" "$(fixture comment-only flat "$COMMENT")" no +expect "no header at all (must fail CLOSED)" "$(fixture missing none '')" no + +if [ "$fail" -ne 0 ]; then + echo "kernel compat probe FAILED" >&2 + exit 1 +fi +echo "kernel compat probe: all cases correct"