Repository navigation
kestrel: hardware ACK responder on the 8852C - #489
Conversation
On the 8852C, SetAckResponder refused: the firmware ignores the addr_cam H2C altogether. After it neither the address CAM entry nor the BSSID CAM entry it carries is written, and nothing changed that: a byte-for-byte replay of the vendor Windows driver's association H2Cs, a join_info alone, a 64-byte addr_cam. The same replay commits on the 8852B. The RMAC still answers from the CAM, so on this die the arm writes the station entry from the host (HalKestrel::write_addr_cam_sta), the way the vendor's mac_upd_addr_cam does when the firmware is not running: the same short entry the 8852B firmware commits, VALID cleared first (a retarget rewrites a live entry), then the body, then VALID. No H2C is sent, and the clear is the host clear alone (no join_info disconnect for a station the firmware never registered). - The responder's slot is per die (ack_slot()): 127 on the 8852B (unchanged) and 63 on the 8852C, its last entry: the CAM reads 0xdeadbeef from entry 64 up. RegisterPeerSta/RegisterBeamformee keep refusing it. - The response-abort gate clear is mandatory on this die: its EDCCA reads its own floor as busy, and with the gates armed every ACK was dropped. - The station entry encoding moves to KestrelAddrCam.h and is shared by the 8852B's addr_cam H2C (byte-identical payload) and the 8852C's host write; tests/kestrel_addr_cam_selftest.cpp pins it against hardware read-backs (an entry the 8852B firmware committed, one the host wrote on an 8852C). - ack_responder_ok is true on both dies; the SetAckResponder contract and the flag's comment say that on the 8852C a true return does not guarantee every bring-up answers (see below). Measured, TX50UH (RTL8832CU) responder, TX20U Nano (RTL8852BU) soliciting and counting the ACKs it receives, ch161, MCS1, one attempt per frame, 10 s per cell, 3 runs per row: 13831/14041 armed at Init, 14239/14408 retargeted, armed at InitWrite 98.9% and 97.0% (third run a dead session), 0/5993 at the old address, 0/6062 never armed, 0/5965 after a live disarm; arm/retarget/clear/re-arm correct in the live cycle; injection delivery unchanged armed vs unarmed. The RX descriptor shows every frame to the responder matching the host-written entry. 8852B regression on this code: 99.5-99.8% armed and retargeted, 0 after a disarm, cycle correct, slot 127. Not solved: some 8852C bring-ups answer no ACK for the whole session while the entry reads back and the adapter airs its own frames. It tracks run time (0 of 12 sessions straight after a long power-off, 3 of the next 12; up to three in four after a day of back-to-back runs). The MAC configuration registers are identical to a good session's; the difference is that the MAC classifies the soliciting frames as its own BSS's (WMAC_NAV_UP_INFO, PS_RXINFO). Response NAV checks, the BSSID CAM, the injector, host RX cost and arm timing are ruled out; the doc lists what was measured. Full MSVC build and ctest pass (79 tests with the new selftest, 4 skipped as on master).
|
/review |
Code Review by Qodo
1.
|
- ack_responder_ok is false on the 8852C: a session that answers no ACK reads its entry back like a good one, so the flag could not tell a caller the loop closes. SetAckResponder still arms it; true on the 8852B as before. - The host write of the ADDR_CAM entry reports a failed register transfer and stops before setting VALID again, and the 8852C arm is confirmed by reading the whole entry back (addr_cam_sta_holds), not only VALID + SMA. Dword 5 reads 0xdeadbeef on the 8852C whatever is written (measured), so it is skipped. Re-measured: 3 sessions armed, 99.1 / 98.6 / 98.4% ACKed. - RegisterPeerSta / RegisterBeamformee refuse an ADDR_CAM index past the die's CAM (64 entries on the 8852C, 128 on the 8852B) instead of sending an entry that never lands.
josephnef
left a comment
There was a problem hiding this comment.
Reviewed at e40d7b1 on the bench: two TX50UH (RTL8832CU, both on USB 2.0 hub ports) as responder, an 8812CU (Jaguar3, CCX tx.report) soliciting unicast QoS-Data at MCS3, retry limit 0, ch161, 8 s per cell. Build + ctest 83/83 on Linux.
The address CAM on the 8852C is 128 entries at a 0x20 stride, not 64 at 0x40
The vendor's get_addr_cam_size (mac_ax/addr_cam.c) returns ADDR_CAM_ENT_LONG_SIZE (0x40) for the 8852A/8852B/8851B/8852BT and ADDR_CAM_ENT_SHORT_SIZE (0x20) for everything else, the 8852C included; hw_info_ax.h has ADDRCAM_NUM_8852C 128; and the no-firmware mac_upd_addr_cam path writes entry N at N * a_info.len. HalKestrel uses the LONG stride on both dies, so on the C die:
- "0xdeadbeef from entry 64 up" is 128 × 0x20 = 64 × 0x40: the end of a 128-entry CAM, not a 64-entry one.
- The host write at "entry 63" lands at byte 63 × 0x40 = hardware entry 126, which is exactly what the long RX descriptor told you (
index reads 126). It works because the RMAC searches up to 0x7f, not because 63 is the last entry. addr_cam_entries() = 64and the newpeer_slot_reservedbound refuse legitimate indices 64..127 (the firmware-side peer path uses hardware indices), andaddr_cam_holds/clear_addr_cam_entryread and clear the wrong entry for any index the firmware wrote on this die.
Measured with the stride set per die (scratch patch, host write at 63 × 0x20): the entry reads back at the 0x20 offset (03900005 00000001 56341202 00029a78 01000000 ... 0000403f), nothing at the 0x40 offset, and the responder answers 96.8–100% in 14 of 15 armed sessions (one dead bring-up, below). Suggest: a per-die stride in HalKestrel (addr_cam_holds, clear_addr_cam_entry, write_addr_cam_sta, addr_cam_sta_holds), addr_cam_entries() 128 on both dies, and the ack_slot() / doc / AdapterCaps.h wording to match (whether the slot stays 63 or goes back to 127 is your call; the geometry text is what has to change).
The "firmware ignores the addr_cam H2C" conclusion survives the stride question: with the H2C path forced on the 8852C at slot 63 (register_infra_sta, join_info + addr_cam), the entry is VALID-clear at both the 0x20 and the 0x40 offset, and the arm refuses as before.
The dead bring-ups reproduce here, and they are not thermal or uptime
Each row is one series of back-to-back sessions, "dead" = armed, entry read back, 0 ACKs for the whole cell; the others answered 91–100% (one 91.2%, the rest ≥ 95%).
| series | dead / armed sessions |
|---|---|
| unit 1, soft re-run, PR layout (hw entry 126) | 5 / 14 |
| unit 2, soft re-run, PR layout | 3 / 7 (incl. its disarm cell, dead before the disarm) |
| unit 1, VBUS cold cycle before every session, PR layout | 1 / 13 |
| unit 1, soft re-run, host write at hw entry 63 | 1 / 16 |
The RF thermal meter sat at raw 29–30 (delta 2–3) through all of it, dead sessions came interleaved with good ones (dead/good/dead/good/dead in one series), and one came on the second session after a 3 s VBUS cut. So "tracks how long the adapter has been running" and "letting the adapter cool down is what brought it back" don't hold on this bench; I'd drop the thermal/uptime reading from the doc or state it beside this counter-observation. A real cold cycle and the entry placement both lowered the rate here but neither removed it (the 63-vs-126 difference is 1/16 vs 8/21, suggestive, not a finding). The responder's bring-up log of a dead session differs from a good one only in the EDCCA pwdB noise line, which agrees with your "nothing on the host tells them apart".
The live-disarm row needs the same session to have answered first
On this die a cell that reads 0 after a disarm is indistinguishable from a dead bring-up unless it answered before the disarm. I solicited across the disarm (DEVOURER_ACK_DISARM_AFTER_MS=6000, TX for 16 s): 442/512 ACKed before, then a clean step to 0/2389 (the last ACK and the first miss 5 ms apart), in all three live disarm cells; one more disarm cell (unit 2) was a dead session and says nothing. The AdapterCaps.h "0/5965 after a live disarm" row should say how it was measured, or be re-measured that way.
Smaller
- The PR description says
ack_responder_okis true on both dies; the code after the review fixes reports false on the 8852C. Please reconcile. c.ack_responder_ok = _variant != kestrel::ChipVariant::C8852Creads as "every die but the C";== C8852Bsays what was measured and keeps a future variant from inheriting true.- With a 128-entry CAM on both dies the
addr_cam_idx >= addr_cam_entries()refusal has nothing left to refuse (uint8_t), so it can go. - Doc: "Measured in failed sessions: ... the slot" is listed as ruled out, but with the stride above the slot that was tested is hardware entry 126 only.
8852B: not on this bench, not re-measured.
The 8852C lays its address CAM out at the SHORT entry (0x20), not the 8852B's LONG one (0x40): the vendor's get_addr_cam_size returns LONG only for the 8852A/8852B/8851B/8852BT, and mac_upd_addr_cam writes entry N at N * len. HalKestrel used 0x40 on both, so the 8852C "entry 63" was hardware entry 126 and "0xdeadbeef from entry 64 up" was the end of a 128-entry CAM. addr_cam_entry_addr() now picks the stride per die for the host write, the read-backs and the clear; the 8852B is unchanged. - Measured on the 8852C: the entry reads back at the 0x20 offset (dword 5 still reads 0xdeadbeef and stays skipped), the 0x40 offset holds power-up garbage, and the responder answers. - 128 entries on both dies; the peer-registration bound refuses 128..255. - ack_responder_ok == C8852B, so a future variant does not inherit true. - Live disarm re-measured with the solicitor running across the disarm in one session: 8852C 3812/3837 then 0/1616, 3846/3848 then 0/1623; 8852B 3842/3848 then 0/1773. - Dead 8852C bring-ups at hardware entry 63: 6 of 15, interleaved with good sessions. The doc no longer reads them as thermal/uptime and states both benches' observations; the slot is not ruled out.
|
@josephnef Thanks for the bench time, all of this was right. Fixed in 45720c9. CAM stride. Confirmed in the vendor tree ( Dead bring-ups. I reproduced your reading: at hardware entry 63, 6 of 15 sessions were dead, in runs (3 dead, 3 good, 2 dead, 4 good). So entry 63 doesn't explain your 1/16 either. The thermal/uptime reading is gone from the doc; it now gives both benches' observations side by side, and the slot is no longer listed as ruled out. Live disarm. Re-measured your way, the solicitor running across the disarm in one session: 3812/3837 then 0/1616, and 3846/3848 then 0/1623 (a third cell was a dead session, left out). 8852B the same way: 3842/3848 then 0/1773. Smaller.
8852B with this build: 3 sessions 99.8-99.9%, stride unchanged there. MSVC build and |
|
Follow-up to the review: a dead-vs-good bisect with the CAM and tables read at the C die's 0x20 stride. Same bench (TX50UH bus 3 as responder, 8812CU soliciting with CCX Nothing host-readable separates a dead bring-up from a good one. Dumped right after the arm and again at session end, 16 sessions with known outcomes: ADDR_CAM entry (hardware index 126), BSSID CAM, every MAC register in 0x8000-0x8FFF / 0xC000-0xCFFF / 0xD800-0xDBFF except counters, the whole BB space 0x10000-0x17FFF, both RF paths' registers at arm, GNT_SW_CTRL / GNT_VAL. The per-macid DMAC and CMAC control tables are uninitialised SRAM in an RX-only session (1021 of 1024 words change per bring-up, macid 0 included) and no bit of them separates the outcomes (12 separators in 8192 bits is the noise floor for 3 good sessions). What does differ. Excluded by measurement. Temperature: the rate climbed to ~85% over the day on ch161 and ch6 and on both units with the meter flat (raw 26-30), and 4 sessions after 120 s VBUS-offs were dead at the coldest reading of the day. Retained state: the same 120 s cuts. The TX chain: in dead sessions on ch6 the responder injected normally (619-642 of 622-645 sends witnessed) while answering 0 ACKs. The solicitor: the witness logs ~1100-1250 fewer frames in a dead session, i.e. the ACKs are not on air. Levers that changed nothing (8-11 sessions each): a host-written minimal or vendor-default ( Two side notes. On the C die None of this blocks the PR once the stride geometry and the doc's thermal/uptime reading are fixed; the root cause is tracked in #490. |
josephnef
left a comment
There was a problem hiding this comment.
45720c9 re-checked on the bench: build + ctest 83/83; TX50UH (bus 3) arms at hardware entry 63 at the 0x20 stride and reads back there; 6 sessions on ch6 with the 8812CU soliciting: 95.9 / 97.0 / 94.6 / dead / dead / dead (the dead rate is this bench's afternoon state, #490, unchanged by the PR); never-armed 0/1341; live disarm with the solicitor running across it: 41% of the first 40% of reports then 0/1157 after, one clean step. Keeping the >=128 refusal is fine. Approving.
Addressed in 45720c9; approved on that head.
The problem
On the 8852C,
SetAckResponderrefused: the firmware ignores the addr_cam H2C altogether. After it, neither the address CAM entry nor the BSSID CAM entry it carries is written, and nothing changed that: a byte-for-byte replay of the vendor Windows driver's association H2Cs (CMAC table, DMAC table, join_info, addr_cam), a join_info alone, and a 64-byte addr_cam. The same replay commits on the 8852B.The change
HalKestrel::write_addr_cam_sta), the way the vendor'smac_upd_addr_camdoes when the firmware is not running: VALID cleared first (a retarget rewrites a live entry), then the body, then VALID. No H2C is sent, and the clear is the host clear alone. The 8852B path is unchanged.addr_cam_entry_addr), as the vendor'sget_addr_cam_sizelays it out: 0x40 on the 8852B, 0x20 on the 8852C, 128 entries on both.HalKestrelused 0x40 on both before; it only mattered on the 8852C.ack_slot()): 127 on the 8852B, as before, and 63 on the 8852C.RegisterPeerSta/RegisterBeamformeekeep refusing it, and now also refuse an ADDR_CAM index past the 128-entry CAM.KestrelAddrCam.h, shared by the 8852B's addr_cam H2C (the payload is byte-identical to before) and the 8852C's host write. A new headless selftest,kestrel_addr_cam, pins it against hardware read-backs: an entry the 8852B firmware committed, and one the host wrote on an 8852C.0xdeadbeefon this die whatever is written), not only VALID and the address.ack_responder_okis true on the 8852B only (== C8852B) and false on the 8852C: a dead session reads its entry back like a good one, so the flag could not tell a caller the loop closes.SetAckResponderstill arms the 8852C (below).AdapterCaps.handdocs/kestrel-ack-responder.mdare updated.Measured
TX50UH (RTL8832CU) as responder, TX20U Nano (RTL8852BU) soliciting and counting the ACKs it receives, ch161, MCS1, one attempt per frame, 10 s per cell, 3 runs per row. This matrix was taken with the entry at hardware entry 126 (the 0x40-stride layout); the re-measurement at entry 63 follows it.
Arm A, retarget to B, clear, re-arm A in one session: correct in the live run (A 98.3%, then A 0% / B 99.0%, both 0% after the clear, A 99.0% after the re-arm); the other two runs were dead sessions. Injection delivery to a witness is unchanged armed vs unarmed. The long RX descriptor shows every frame addressed to the responder matching the host-written entry (macid 63).
At hardware entry 63 (8 s per cell): live disarm with the solicitor running across it in one session, 3812/3837 then 0/1616 and 3846/3848 then 0/1623 (a third cell was a dead session); 15 armed sessions back to back, 9 answering 97.5-99.9% and 6 dead.
8852B regression on this code (0x40 stride, unchanged): 3 sessions 99.8-99.9%, live disarm across the solicitor 3842/3848 then 0/1773. Earlier, at the first revision: 99.5-99.8% armed and retargeted, 0 after a live disarm, the arm/retarget/clear/re-arm cycle correct, still on slot 127.
Full MSVC build and
ctestpass (79 tests on Windows).What this does not solve
Some 8852C bring-ups answer no ACK at all: the responder arms, the entry reads back, the adapter airs its own frames, and it answers nothing for the whole session. How often varies, and no cause is established: 3 of 12 and 6 of 15 sessions here, and on the maintainer's bench 8 of 21 at entry 126 and 1 of 16 at entry 63. Dead sessions come interleaved with good ones, a VBUS cold cycle before every session lowered the rate but did not remove it, and the thermal meter stayed flat, so it is not read as thermal or uptime-related.
In failed sessions the MAC configuration registers are identical to a good session's, and the response abort counters stay at 0 (the response is never attempted). What does differ is that the MAC classifies the soliciting frames as its own BSS's traffic (
R_AX_WMAC_NAV_UP_INFOrecords intra-BSS NAV updates,R_AX_PS_RXINFOflags port-0 data). Ruled out by measurement: the response NAV checks, the BSSID CAM, the injector, the host's per-frame RX cost, and arm timing. The doc lists what was measured. I don't have the cause yet.One bench, one pair of dongles, near field.