Windowed module protocol: every op admitted inside a 3.4s window after the firmware's observed temp poll (cadence 3.5-4.2s measured, stuck-at-last-fetch stale mechanism proven and avoided), acquire/window with panic on dead heartbeat; noise column shows green on/off cycle phase, cable-missing state unchanged

This commit is contained in:
flamingcow
2026-08-13 14:04:16 -07:00
parent d81594ffbf
commit a9f10d3055
8 changed files with 104 additions and 48 deletions
+7 -3
View File
@@ -24,9 +24,9 @@ Protocol and full verified catalog: [bcm84891l-mdio-commands.md](bcm84891l-mdio-
- **GETs must be invoked bare** — pre-writing *any* DATA register, not just the documented DATA1 display flag, leaves the handler executing as a no-op with results never written (sentinel pre-fills survived GET_SNR untouched, proven on hardware).
- **STATUS must never be written** — any user value (0x0000 and 0x0008 both tried) closes the mailbox: the next command is silently ignored until firmware restores it.
- **PASS cannot prove completion.** The previous command's PASS stays latched while a slow firmware still executes, and until then the DATA registers are firmware scratch — the die temperature turns up in them (0x004E0x0051 observed as "SNR" and "EEE mode"; the campaign's "anomalous 0x0047" was this same race).
- **Handler writes during firmware-busy windows wedge the µC permanently** — DATA/CMD writes landing while firmware does post-AN provisioning or training work (a window stretching ~10 s past relink) killed the SMI service four times; reads alone never once. Unmitigated — exposure accepted until the shared-mailbox contention is properly resolved.
- **Handler writes during firmware-busy windows wedge the µC permanently** — DATA/CMD writes landing while firmware does post-AN provisioning or training work (a window stretching ~10 s past relink) killed the SMI service four times; reads alone never once. The windowed protocol keeps steady-state writes clear of the temp poll's busy windows; bringup-era writes run unwindowed (they fire pre-AN, where writes have never wedged) and post-AN provisioning exposure remains as the diag path's accepted risk.
- **Bridge reads carry no fetch identity** — a fetch that outruns the delay leaves the previous transaction's data at 0xAD with no error. A killed host can also leave the pipeline latched a response behind *persistently* (PHY ID served a stuck stale value across processes until driver re-init); every appliance boot clears it by loading the driver, so only bench runs reusing a loaded driver see it, and the identity check at startup dies loudly on it.
- **Stale reads poisoned the corrected-error channel** — caught with per-register instrumentation: 3.33 reads served 1.147's resident 0x0011 (decoding as exactly 17 errored blocks — the recurring "+17"), SNR DATA values (+21/22), and 1.147 reads served 3.33's 0x8000 (count 16, charged twice by the rollover delta) and the die temperature. Every corrected burst observed to date decodes as a neighbor register; none is proven cable stress, and the noise-correlation was the µC being busy (stale window), not physics. The corrected channel is untrustworthy until the stale-read problem is solved; the internal temp client is a prime suspect for the periodic busy windows and is not silenceable (firmware-reliability notes).
- **Stale reads poisoned the corrected-error channel** — caught with per-register instrumentation: 3.33 reads served 1.147's resident 0x0011 (decoding as exactly 17 errored blocks — the recurring "+17"), SNR DATA values (+21/22), and 1.147 reads served 3.33's 0x8000 (count 16, charged twice by the rollover delta) and the die temperature. Every such burst decodes as a neighbor register. The source is the firmware's own temp poll and the fix is the windowed protocol (firmware-reliability notes): work confined to a fixed window after each observed poll ends the poisoning — reproduced at will by aiming reads at the poll, eliminated by avoiding it.
- Poll STATUS ~100 ms; frozen up to 2 s during 10GBASE-T training — only run after link-up.
## Proven diagnostics
@@ -55,7 +55,11 @@ Protocol and full verified catalog: [bcm84891l-mdio-commands.md](bcm84891l-mdio-
## Firmware-reliability notes
**The firmware is its own mailbox client — the handler is a shared, unarbitrated resource.** Watched read-only on an idle module: CMD sits at 0x0031 (`CMD_GET_CURRENT_TEMP`, bit 15 consumed) and DATA1 tracks the live die temperature (0x4F0x52 ≈ 7982 °C under load), refreshed every ~3.5 s — the firmware issues its own temperature command through the same CMD/STATUS/DATA registers the host uses, with no arbitration. Every mystery this explains: temperature appearing in DATA1 (it is that command's output — the campaign's "anomalous 0x0047" included), PASS satisfying a host poll when it belongs to the internal command, and the µC wedges (two writers colliding on CMD/DATA, likeliest when internal management activity spikes after AN events). The host side keeps exactly one conversation open at a time (one per-module lock around every whole operation, `phy.go`) and confines writes to quiet windows; **contention with the internal client is otherwise unresolved** — a host GET can still be preempted (~10% per command at the temp cadence) and its results replaced. Any GET whose answer lives in DATA1 is unusable; corroborate through IEEE registers or the wire. The hard rule that stands: write every DATA register explicitly before any SET — the handler executes stale DATA. **The internal poll is not silenceable through the handler**: disabling the temperature warning (SET 0x8032 `(0x007D, 5, 0, 0, 0)` and 0x8034 `(0xFFD8, 5, 0, 0, 0)` — DATA3 = 0 = warning off — both PASS) leaves CMD resident at 0x0031 with DATA1 tracking the die within seconds of the host going quiet; the ~3.5 s poll is independent of the warning machinery. GET 0x8033/0x8035 read DATA13 = 0 before and after those SETs (DATA4/5 leftover scratch) — DATA1-answer readbacks, unusable as verification either way.
**The firmware is its own mailbox client — the handler is a shared, unarbitrated resource.** Watched read-only on an idle module: CMD sits at 0x0031 (`CMD_GET_CURRENT_TEMP`, bit 15 consumed) and DATA1 tracks the live die temperature (0x4F0x52 ≈ 7982 °C under load) — the firmware issues its own temperature command through the same CMD/STATUS/DATA registers the host uses, with no arbitration. Every mystery this explains: temperature appearing in DATA1 (it is that command's output — the campaign's "anomalous 0x0047" included), PASS satisfying a host poll when it belongs to the internal command, and the µC wedges (two writers colliding on CMD/DATA, likeliest when internal management activity spikes after AN events). Any GET whose answer lives in DATA1 is unusable; corroborate through IEEE registers or the wire. The hard rule that stands: write every DATA register explicitly before any SET — the handler executes stale DATA. **The internal poll is not silenceable through the handler**: disabling the temperature warning (SET 0x8032 `(0x007D, 5, 0, 0, 0)` and 0x8034 `(0xFFD8, 5, 0, 0, 0)` — DATA3 = 0 = warning off — both PASS) leaves CMD resident at 0x0031 with DATA1 tracking the die within seconds of the host going quiet; the poll is independent of the warning machinery. GET 0x8033/0x8035 read DATA13 = 0 before and after those SETs (DATA4/5 leftover scratch) — DATA1-answer readbacks, unusable as verification either way.
**The internal poll's clock, measured on hardware.** One poll every 3.494.22 s, regime-dependent but rock-stable within a regime (±15 ms): ≈3.50 s warm and idle, ≈3.70 s, ≈4.01 s cool and idle, ≈4.21 s under continuous host mailbox load — host traffic *defers* the poll, never advances it, and nothing observed brings two polls closer than 3.49 s. The poll is link-independent: it runs straight through an AN restart and the whole retrain without missing a beat. Around each poll's service the bridge serves stale data for ~50 ms: every read returns the last value successfully fetched before the µC went busy (stuck-at-last-fetch, not one-behind), which is exactly how 3.33 inherits 1.147's 0x0011. The window closes *before* the poll is detectable — a CMD read returning the fresh resident 0x0031 is itself proof the µC serviced the fetch, so flip detection doubles as the all-clear.
**The windowed protocol (`phy.go` `admit`) — contention resolved by time-division.** Since the poll cannot be silenced, the host schedules around it: after bringup, every operation is admitted only inside a fixed 3.4 s window following each observed poll (CMD flipping to resident 0x0031, polled at 10 ms); at the cutoff the host goes quiet until the next flip. 3.4 s sits below every observed cadence, so a regime switch mid-run lands harmlessly in vacated time — no prediction, no period tracking, just the last observed flip plus two constants. The host's own commands re-arm detection (they leave CMD ≠ 0x0031); a missing heartbeat free-runs one window rather than stall (never observed — the heartbeat survives retrains); bringup runs before windowing is enabled and never waits. Validated on hardware in `~/work/phydiag-work/bcm_phaselock_bench.py`: aiming read bursts at the poll reproduces the poisoning on 100% of polls (the "+17" manufactured on demand); windowed operation ran 2,881 back-to-back batches — 16× the production rate, riding the cutoff — with zero stale values, and the production 1 Hz stream is unperturbed (blackout hides inside natural gaps, worst sample gap ~1.6 s). Corrected bursts since windowing no longer decode as neighbor registers (historically every one did) and correlate with amber SNR margin under full-rate traffic — real line corrections; the graded-noise correlation run is the confirmation path.
## ECD — recovered from the OpenBCM SDK, proven on hardware
+1 -10
View File
@@ -22,16 +22,7 @@ The register question is answered (post-FEC vs corrected-by-iteration histogram
No confirmed-safe path exists (every candidate lands in the µC danger window). The open decision is whether the capability is worth the NDA route or a sacrificial unit — the product doesn't need it for length ([modules/wiitek/](modules/wiitek/README.md), [modules/README.md](modules/README.md)).
## 5. Corrected-error channel under the internal temp client
Every corrected-error burst observed decodes as a stale neighbor register served under a busy
µC, and the firmware's internal ~3.5 s GET_CURRENT_TEMP poll — the prime suspect for the busy
windows — is not silenceable through the handler (temp-warning disable leaves it running,
proven on hardware — [modules/fs/](modules/fs/README.md)). The corrected channel stays
untrustworthy; the remaining path is the register-docs ask (Wiitek request sent; the FS
missing-chapter asks pending).
## 6. X520 bench divergences — features to restore on the product NIC
## 5. X520 bench divergences — features to restore on the product NIC
Running on the X520 (BCM development) required parking product-NIC capabilities the 82599 lacks. Each stays parked only until the ConnectX-5 is in; none is a settled design change:
+1 -1
View File
@@ -2,7 +2,7 @@
## Committed tree
AF_PACKET raw sockets everywhere (`sock.go`); flow-director steering; per-packet-MAC-rx-stamped rate buckets (`SO_TIMESTAMPING` cmsg, `rx_filter=ALL` as a hard host check — nics/README.md for what that demands of the NIC; **temporarily bypassed** in `ts.go` so BCM work can run on the X520, which cannot stamp — the check reports yellow and the panel rates read zero there; restore to fatal for the product NIC); read-time-stamped NIC-counter rates; test interfaces pinned to MTU 9000 with a 9018-byte jumbo in the size mix (the modules' jumbo path is exercised, not assumed); BCM module diagnostics (`phy.go`, over the patched-ixgbe `sff_i2c` debugfs, compound-op framing; one per-module lock around every whole operation; the firmware's own mailbox use still contends, unresolved — modules/fs/ for the full trap list): bringup identifies both modules and forces EEE off and jumbo on every boot — no trustworthy readback exists and no cable is guaranteed to probe through; the ECD — per-pair verdicts, lengths and pair maps are the length/wiring path — runs through one async path at startup and on every reset, never blocking the UI, with counters re-baselining only after the diag's own link blip so it is never charged to the run; a 1 Hz poller feeds per-pair SNR margin (vs the ≈26.5 dB operating point; green ≥ 3 dB, amber ≥ 1 dB — provisional until the graded-noise run) and the corrected-error set (PCS 3.33 errored blocks/BER, PMA 1.147 fast-retrain count) to the panel and console; framebuffer UI; harness.
AF_PACKET raw sockets everywhere (`sock.go`); flow-director steering; per-packet-MAC-rx-stamped rate buckets (`SO_TIMESTAMPING` cmsg, `rx_filter=ALL` as a hard host check — nics/README.md for what that demands of the NIC; **temporarily bypassed** in `ts.go` so BCM work can run on the X520, which cannot stamp — the check reports yellow and the panel rates read zero there; restore to fatal for the product NIC); read-time-stamped NIC-counter rates; test interfaces pinned to MTU 9000 with a 9018-byte jumbo in the size mix (the modules' jumbo path is exercised, not assumed); BCM module diagnostics (`phy.go`, over the patched-ixgbe `sff_i2c` debugfs, compound-op framing; one per-module lock around every whole operation, and steady-state work time-division-multiplexed with the firmware's own mailbox client — every operation admitted only inside a 3.4 s window after each observed internal temp poll, which ends the stale-read poisoning of the corrected channel — modules/fs/ for the full trap list and validation): bringup identifies both modules and forces EEE off and jumbo on every boot — no trustworthy readback exists and no cable is guaranteed to probe through; the ECD — per-pair verdicts, lengths and pair maps are the length/wiring path — runs through one async path at startup and on every reset, never blocking the UI, with counters re-baselining only after the diag's own link blip so it is never charged to the run; a 1 Hz poller feeds per-pair SNR margin (vs the ≈26.5 dB operating point; green ≥ 3 dB, amber ≥ 1 dB — provisional until the graded-noise run) and the corrected-error set (PCS 3.33 errored blocks/BER, PMA 1.147 fast-retrain count) to the panel and console; framebuffer UI; harness.
## Stashes
+4 -4
View File
@@ -354,7 +354,7 @@ func (d *direction) displayView() view {
// The same figures the panel draws, in the same order: the last second as
// rates and error flags with the noise cable riding at the end of them, then
// everything since the reset.
func totalRow(elapsed time.Duration, v view, target float64, phy phyDisplay, noiseMissing uint64) []string {
func totalRow(elapsed time.Duration, v view, target float64, phy phyDisplay, nv noiseView) []string {
return []string{
rateCell(v.rxGbps*1e9, target*1e9),
scaleSI(v.rxPPS),
@@ -364,7 +364,7 @@ func totalRow(elapsed time.Duration, v view, target float64, phy phyDisplay, noi
flagCell(v.window.link),
flagCell(v.window.internal),
correctedFlag(phy.recent),
flagCell(noiseMissing),
noiseCell(nv),
scaleTime(elapsed),
scaleCount(v.rxFrames),
scaleCount(v.rxBytes),
@@ -784,7 +784,7 @@ func run(aName, bName string) (err error) {
}
v, phy := measureView(diag, modules, totalView(views))
if err := disp.render(v, now.Sub(start), phy,
noise.missing()); err != nil {
noise.view()); err != nil {
return err
}
case now := <-tick.C:
@@ -794,7 +794,7 @@ func run(aName, bName string) (err error) {
}
v, phy := measureView(diag, modules, totalView(rows))
for _, line := range stats.emit(totalRow(elapsed, v, target, phy,
noise.missing())) {
noise.view())) {
fmt.Println(line)
}
}
+17 -7
View File
@@ -75,6 +75,8 @@ type noiser struct {
// phase. Latched across the down phase, where the missing carrier is our
// own doing and says nothing about the cable.
connected atomic.Bool
// The cycle's up phase: ports admin-up and the wire loud, training or frames.
radiating atomic.Bool
}
func newNoiser() (*noiser, error) {
@@ -113,14 +115,20 @@ func (n *noiser) names() []string {
return []string{n.eps[0].name, n.eps[1].name}
}
// Zero while the cable was there at the last verdict, one while it was not:
// the shape the error cells already colour by, so absence paints as the fault
// it is and presence as the usual green.
func (n *noiser) missing() uint64 {
if n.connected.Load() {
return 0
type noiseView struct {
missing uint64
on bool
}
// missing is zero while the cable was there at the last verdict, one while it
// was not: the shape the error cells already colour by, so absence paints as
// the fault it is. on is the cycle's phase, presence granted.
func (n *noiser) view() noiseView {
v := noiseView{on: n.radiating.Load()}
if !n.connected.Load() {
v.missing = 1
}
return 1
return v
}
// The ports were reachable when the noiser was built, so one that stops taking
@@ -173,6 +181,7 @@ func (n *noiser) run(done *atomic.Bool) {
for !done.Load() {
n.setLinks(fd, true)
n.radiating.Store(true)
linked := false
for end := time.Now().Add(noiseUpSpan); time.Now().Before(end) && !done.Load(); {
<-tick.C
@@ -190,6 +199,7 @@ func (n *noiser) run(done *atomic.Bool) {
n.connected.Store(linked)
n.setLinks(fd, false)
n.radiating.Store(false)
for end := time.Now().Add(noiseDownSpan); time.Now().Before(end) && !done.Load(); {
<-tick.C
}
+48 -17
View File
@@ -38,9 +38,16 @@ const (
bcmPHYIDHi = 0x3590
bcmPHYIDLo = 0x5081
bcmCmdResidentTemp uint16 = 0x0031
bcmReadDelayUs = 3000
bcmRetryDelayUs = 10000
bcmWindow = 3400 * time.Millisecond
bcmWindowFit = 100 * time.Millisecond
bcmFlipPoll = 10 * time.Millisecond
bcmFlipWait = 5 * time.Second
bcmStatusPoll = 100 * time.Millisecond
// Covers the handler's documented 2 s freeze during 10GBASE-T training.
bcmStatusTimeout = 3 * time.Second
@@ -68,7 +75,35 @@ type bcm struct {
// Every method holds it for its whole logical operation: exactly one
// host-side conversation with the module at a time, by construction.
mu sync.Mutex
mu sync.Mutex
windowEnd time.Time
windowed atomic.Bool
}
// The firmware's internal temp poll (every 3.54.2 s, never under 3.49) serves
// stale bridge reads for ~50 ms around it; work stays inside 3.4 s post-poll.
func (b *bcm) window() {
if !b.windowed.Load() || time.Now().Add(bcmWindowFit).Before(b.windowEnd) {
return
}
deadline := time.Now().Add(bcmFlipWait)
for {
v, err := b.mdioRead(bcmMMDVendor, bcmRegCmd)
if err != nil {
panic(fmt.Sprintf("%s: heartbeat poll: %v", b.ifname, err))
}
if v == bcmCmdResidentTemp || time.Now().After(deadline) {
b.windowEnd = time.Now().Add(bcmWindow)
return
}
time.Sleep(bcmFlipPoll)
}
}
func (b *bcm) acquire() func() {
b.mu.Lock()
b.window()
return b.mu.Unlock
}
func openBCM(ifname string) (*bcm, error) {
@@ -197,8 +232,7 @@ func (b *bcm) waitStatus(want func(uint16) bool) (uint16, error) {
// executing as a no-op); SETs must pass their full parameter set (the handler
// executes stale DATA).
func (b *bcm) command(code uint16, params ...uint16) ([5]uint16, error) {
b.mu.Lock()
defer b.mu.Unlock()
defer b.acquire()()
var data [5]uint16
if _, err := b.waitStatus(func(st uint16) bool {
@@ -235,8 +269,7 @@ func (b *bcm) command(code uint16, params ...uint16) ([5]uint16, error) {
}
func (b *bcm) identify() (string, error) {
b.mu.Lock()
defer b.mu.Unlock()
defer b.acquire()()
hi, err := b.mdioRead(1, 2)
if err != nil {
return "", err
@@ -259,8 +292,7 @@ func (b *bcm) identify() (string, error) {
// PMA 1.1 latches low, so the first read reports any drop since it was last
// read and the second reports the wire as it is now.
func (b *bcm) linkUp() (bool, error) {
b.mu.Lock()
defer b.mu.Unlock()
defer b.acquire()()
if _, err := b.mdioRead(1, 1); err != nil {
return false, err
}
@@ -282,8 +314,7 @@ func (b *bcm) forceJumbo() error {
}
func (b *bcm) restartAN() error {
b.mu.Lock()
defer b.mu.Unlock()
defer b.acquire()()
v, err := b.mdioRead(7, 0)
if err != nil {
return err
@@ -292,8 +323,7 @@ func (b *bcm) restartAN() error {
}
func (b *bcm) eeeAdvert() (uint16, error) {
b.mu.Lock()
defer b.mu.Unlock()
defer b.acquire()()
return b.mdioRead(7, 60)
}
@@ -318,8 +348,7 @@ func (b *bcm) snr() ([4]float64, error) {
}
func (b *bcm) pcsLatch() (blocks, ber uint64, err error) {
b.mu.Lock()
defer b.mu.Unlock()
defer b.acquire()()
v, err := b.mdioRead(3, 33)
if err != nil {
return 0, 0, err
@@ -328,8 +357,7 @@ func (b *bcm) pcsLatch() (blocks, ber uint64, err error) {
}
func (b *bcm) fastRetrainCount() (uint16, error) {
b.mu.Lock()
defer b.mu.Unlock()
defer b.acquire()()
v, err := b.mdioRead(1, 147)
if err != nil {
return 0, err
@@ -343,8 +371,7 @@ type ecdResult struct {
}
func (b *bcm) cableDiag() (ecdResult, error) {
b.mu.Lock()
defer b.mu.Unlock()
defer b.acquire()()
var res ecdResult
ctrl, err := b.mdioRead(bcmMMDVendor, bcmRegECDCtrl)
@@ -368,6 +395,7 @@ func (b *bcm) cableDiag() (ecdResult, error) {
}
time.Sleep(ecdPoll)
}
b.window()
v, err := b.mdioRead(1, bcmRegECDResult)
if err != nil {
return res, err
@@ -812,6 +840,9 @@ func moduleChecks(mods []*phyModule, names [2]string) []checkResult {
}
}
res.state = adv[0] + "/" + adv[1]
for _, m := range mods {
m.bcm.windowed.Store(true)
}
return append(out, res)
}
+12
View File
@@ -304,6 +304,18 @@ func correctedFlag(v uint64) string {
return paint("warn", cYellow)
}
// Green either way while the cable is present — the cycle's phase is state,
// not health. Red stays what it was: the cable missing.
func noiseCell(nv noiseView) string {
switch {
case nv.missing > 0:
return paint("ERR", cRed)
case nv.on:
return paint("on", cGreen)
}
return paint("off", cGreen)
}
// Per-interval rates jitter by a couple of percent at line rate, so green has
// to cover that. Yellow means a real shortfall, red means badly off.
const (
+14 -6
View File
@@ -350,8 +350,9 @@ func (d *display) chipAt(i, n, cols, x, w, y, h int, c rgb) (int, int, int) {
// Whether rather than how many: over a window this short a count changes faster
// than it can be read. The noise chip rides along at the end, presence rather
// than health: red is the cable missing, not the cable failing.
func (d *display) errChips(x, w, y int, e errs, recentCorrected, noiseMissing uint64) int {
// than health: red is the cable missing, not the cable failing, and a present
// cable names its cycle phase in green.
func (d *display) errChips(x, w, y int, e errs, recentCorrected uint64, nv noiseView) int {
n := len(errRows) + 2
for i, r := range errRows {
c := errColor(r.get(e))
@@ -364,9 +365,16 @@ func (d *display) errChips(x, w, y int, e errs, recentCorrected, noiseMissing ui
}
cx, cw, cy := d.chipAt(len(errRows), n, gridCols, x, w, y, d.chipH(), c)
d.centerIn(d.grid, cx, cw, cy+chipPadY, "corrected", c)
c = errColor(noiseMissing)
c = errColor(nv.missing)
label := "noise"
if nv.missing == 0 {
label = "noise off"
if nv.on {
label = "noise on"
}
}
cx, cw, cy = d.chipAt(len(errRows)+1, n, gridCols, x, w, y, d.chipH(), c)
d.centerIn(d.grid, cx, cw, cy+chipPadY, "noise", c)
d.centerIn(d.grid, cx, cw, cy+chipPadY, label, c)
return y + d.chipsH()
}
@@ -425,7 +433,7 @@ func correctedStat(v uint64) statCell {
return statCell{scaleCount(v), "corrected", col}
}
func (d *display) render(v view, elapsed time.Duration, phy phyDisplay, noiseMissing uint64) error {
func (d *display) render(v view, elapsed time.Duration, phy phyDisplay, nv noiseView) error {
fb := d.fb
fb.fill(uiBg)
@@ -435,7 +443,7 @@ func (d *display) render(v view, elapsed time.Duration, phy phyDisplay, noiseMis
{scaleSI(v.rxPPS), "packets/s", uiFg},
snrStat(phy),
})
d.errChips(x, w, d.nowYs[1], v.window, phy.recent, noiseMissing)
d.errChips(x, w, d.nowYs[1], v.window, phy.recent, nv)
x, w = d.panel(d.sincePanel, v.since.total() > 0 || phy.metresClass == clsBad)
d.stats(d.gridB, x, w, d.sinceYs[0], []statCell{