diff --git a/docs/modules/fs/README.md b/docs/modules/fs/README.md index dd1d269..8f6c6c3 100644 --- a/docs/modules/fs/README.md +++ b/docs/modules/fs/README.md @@ -23,9 +23,9 @@ Protocol and full verified catalog: [bcm84891l-mdio-commands.md](bcm84891l-mdio- - **SET commands execute stale DATA** — the handler never clears DATA registers; write every parameter register explicitly before any SET. - **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 (0x004E–0x0051 observed as "SNR" and "EEE mode"; the campaign's "anomalous 0x0047" was this same race). cabletest reads results twice and requires agreement (DATA1 excluded — temperature lands there autonomously), and discards SNR samples below 15 dB absolute, far under anything a trained link produces and exactly where every garbage signature sits. +- **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 (0x004E–0x0051 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. Every handler write — SETs, GET commands, the ECD trigger — runs only in a quiet window: carrier up and stable ≥ 10 s (`phy.go`). -- **Bridge reads carry no fetch identity** — a fetch that outruns the delay leaves the previous transaction's data at 0xAD with no error. cabletest brackets read batches with a known-answer canary (PHY ID 1.2 = 0x3590) and discards the batch on mismatch (`phy.go`). +- **Bridge reads carry no fetch identity** — a fetch that outruns the delay leaves the previous transaction's data at 0xAD with no error. - Poll STATUS ~100 ms; frozen up to 2 s during 10GBASE-T training — only run after link-up. ## Proven diagnostics @@ -54,7 +54,7 @@ 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 (0x4F–0x52 ≈ 79–82 °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). Host defenses in `phy.go`: quiet-window writes, double-read results (DATA1 excluded), a CMD readback proving the command was not interleaved, the PHY-ID canary, and the SNR floor. 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 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 (0x4F–0x52 ≈ 79–82 °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. ## ECD — recovered from the OpenBCM SDK, proven on hardware diff --git a/docs/state.md b/docs/state.md index 87ab7e4..f44c746 100644 --- a/docs/state.md +++ b/docs/state.md @@ -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; handler writes confined to quiet windows — the µC wedges otherwise — with results double-read, canary-validated, and floor-checked — 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, handler writes confined to quiet windows — the µC wedges otherwise; 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. ## Stashes diff --git a/phy.go b/phy.go index c12eb8c..36dc4c4 100644 --- a/phy.go +++ b/phy.go @@ -67,8 +67,8 @@ type bcm struct { ifname string path string - // Guards multi-op sequences only; single reads are already atomic on the - // wire through the compound op. + // 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 } @@ -191,27 +191,9 @@ func (b *bcm) waitStatus(want func(uint16) bool) (uint16, error) { return 0, fmt.Errorf("%s: command handler stuck, status %#04x", b.ifname, st) } -// The bridge gives no signal tying a response to its fetch, so a known-answer -// read validates each batch: 0x3590 back from PHY ID 1.2 proves the bridge -// was serving timely through the window; anything else discards the batch. -func (b *bcm) canary() error { - v, err := b.mdioRead(1, 2) - if err != nil { - return err - } - if v != bcmPHYIDHi { - return fmt.Errorf("%s: bridge served %#04x for the PHY ID canary", b.ifname, v) - } - return nil -} - -// GETs must be invoked bare: pre-writing any DATA register — not just the -// documented DATA1 flag — leaves the handler executing as a no-op. SETs must -// pass their full parameter set (the handler executes stale DATA) and get -// settle time in place of unprovable completion. Results are read twice and -// must agree — a stale PASS otherwise serves the handler's in-flight scratch -// — with DATA1 excluded from the comparison (firmware writes temperature -// there autonomously). +// GETs must be invoked bare (pre-writing any DATA register leaves the handler +// executing as a no-op); SETs must pass their full parameter set (the handler +// executes stale DATA) and get settle time in place of unprovable completion. func (b *bcm) command(code uint16, params ...uint16) ([5]uint16, error) { b.mu.Lock() defer b.mu.Unlock() @@ -242,69 +224,44 @@ func (b *bcm) command(code uint16, params ...uint16) ([5]uint16, error) { if st == bcmStError { return data, fmt.Errorf("%s: command %#04x returned ERROR", b.ifname, code) } - // A SET's DATA registers are its parameters, scribbled over by firmware - // afterwards; there is nothing to read back. if len(params) > 0 { - return data, b.canary() + return data, nil } for i := range data { if data[i], err = b.mdioRead(bcmMMDVendor, bcmRegData1+uint16(i)); err != nil { return data, err } } - for i := 1; i < len(data); i++ { - again, err := b.mdioRead(bcmMMDVendor, bcmRegData1+uint16(i)) - if err != nil { - return data, err - } - if again != data[i] { - return data, fmt.Errorf("%s: command %#04x results unstable", b.ifname, code) - } - } - // The firmware is its own mailbox client (an internal GET_CURRENT_TEMP - // every few seconds); CMD still holding our code proves a GET's results - // are ours. Bit 15 is consumed on acceptance. - cmdv, err := b.mdioRead(bcmMMDVendor, bcmRegCmd) - if err != nil { - return data, err - } - if cmdv != code&^0x8000 { - return data, fmt.Errorf("%s: command %#04x preempted, CMD reads %#04x", b.ifname, code, cmdv) - } - if err := b.canary(); err != nil { - return data, err - } return data, nil } -// Retried against the known constant, long enough to outlast a µC left busy -// by a dead process's in-flight diag or a link mid-training. func (b *bcm) identify() (string, error) { - var hi, lo uint16 - var err error - for i := 0; i < 30; i++ { - if hi, err = b.mdioRead(1, 2); err != nil { - return "", err - } - if lo, err = b.mdioRead(1, 3); err != nil { - return "", err - } - if hi == bcmPHYIDHi && lo == bcmPHYIDLo { - sn, err := b.eeprom(68, 16) - if err != nil { - return "", err - } - return "BCM84891L sn " + strings.TrimSpace(string(sn)), nil - } - time.Sleep(500 * time.Millisecond) + b.mu.Lock() + defer b.mu.Unlock() + hi, err := b.mdioRead(1, 2) + if err != nil { + return "", err } - return "", fmt.Errorf("%s: PHY ID %#04x:%#04x, want %#04x:%#04x", - b.ifname, hi, lo, bcmPHYIDHi, bcmPHYIDLo) + lo, err := b.mdioRead(1, 3) + if err != nil { + return "", err + } + if hi != bcmPHYIDHi || lo != bcmPHYIDLo { + return "", fmt.Errorf("%s: PHY ID %#04x:%#04x, want %#04x:%#04x", + b.ifname, hi, lo, bcmPHYIDHi, bcmPHYIDLo) + } + sn, err := b.eeprom(68, 16) + if err != nil { + return "", err + } + return "BCM84891L sn " + strings.TrimSpace(string(sn)), nil } // 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() if _, err := b.mdioRead(1, 1); err != nil { return false, err } @@ -326,6 +283,8 @@ func (b *bcm) forceJumbo() error { } func (b *bcm) restartAN() error { + b.mu.Lock() + defer b.mu.Unlock() v, err := b.mdioRead(7, 0) if err != nil { return err @@ -334,6 +293,8 @@ func (b *bcm) restartAN() error { } func (b *bcm) eeeAdvert() (uint16, error) { + b.mu.Lock() + defer b.mu.Unlock() return b.mdioRead(7, 60) } @@ -358,6 +319,8 @@ func (b *bcm) snr() ([4]float64, error) { } func (b *bcm) pcsLatch() (blocks, ber uint64, err error) { + b.mu.Lock() + defer b.mu.Unlock() v, err := b.mdioRead(3, 33) if err != nil { return 0, 0, err @@ -366,6 +329,8 @@ func (b *bcm) pcsLatch() (blocks, ber uint64, err error) { } func (b *bcm) fastRetrainCount() (uint16, error) { + b.mu.Lock() + defer b.mu.Unlock() v, err := b.mdioRead(1, 147) if err != nil { return 0, err @@ -404,30 +369,19 @@ func (b *bcm) cableDiag() (ecdResult, error) { } time.Sleep(ecdPoll) } - // The run's own blip leaves the µC busy training, so a failed canary here - // means try the latched results again shortly, not give up. - var canaryErr error - for try := 0; try < 20; try++ { - if try > 0 { - time.Sleep(500 * time.Millisecond) - } - v, err := b.mdioRead(1, bcmRegECDResult) + v, err := b.mdioRead(1, bcmRegECDResult) + if err != nil { + return res, err + } + for i := range res.verdicts { + res.verdicts[i] = int(v>>(4*i)) & 0xF + m, err := b.mdioRead(1, bcmRegECDLen+uint16(i)) if err != nil { return res, err } - for i := range res.verdicts { - res.verdicts[i] = int(v>>(4*i)) & 0xF - m, err := b.mdioRead(1, bcmRegECDLen+uint16(i)) - if err != nil { - return res, err - } - res.metres[i] = int(m) - } - if canaryErr = b.canary(); canaryErr == nil { - return res, nil - } + res.metres[i] = int(m) } - return res, canaryErr + return res, nil } const ( @@ -477,9 +431,6 @@ func (m *phyModule) poll() error { } quiet := !m.upSince.IsZero() && time.Since(m.upSince) >= bcmQuiet - if err := m.bcm.canary(); err != nil { - return err - } link, err := m.bcm.linkUp() if err != nil { return err @@ -490,13 +441,6 @@ func (m *phyModule) poll() error { if snr, err = m.bcm.snr(); err != nil { return err } - // The proven garbage signatures — die temperature, handler status — - // all sit far below any SNR a trained link can have. - for _, s := range snr { - if s < 15 { - return fmt.Errorf("%s: implausible SNR %.1f discarded", m.bcm.ifname, s) - } - } } blocks, ber, err := m.bcm.pcsLatch() if err != nil { @@ -506,9 +450,6 @@ func (m *phyModule) poll() error { if err != nil { return err } - if err := m.bcm.canary(); err != nil { - return err - } m.mu.Lock() m.sampled = true