From: Stefan Sperling Subject: qwx fixes (was: Re: qwz: enable WPA2 association on WCN7850) To: Marcus Glocker , tech@openbsd.org, Mark Kettenis , "Kirill A. Korinsky" , Patrick Wildt Date: Sun, 26 Apr 2026 13:15:54 +0200 On Sun, Apr 26, 2026 at 07:56:10AM +0200, Stefan Sperling wrote: > I will try out your change to see how qwx behaves with it. This patch ports the most obvious (to me) fixes from your qwz diff to qwx: - Set WMI_PEER_AUTHORIZE only after the WPA handshake completes. However, we should be testing this on unencrypted networks as well. I suspect your patch broke that case. My diff is still sending WMI_PEER_AUTHORIZE to the firmware as before if WPA/RSN is disabled, cause otherwise we would never send it on non-WPA networks. - Init the REO queue for the non-QoS TID. I have also adjusted the constant used for sizing the peer's rx_tid array in the header file. - Interrupt vector fix in qwx_pcic_ext_irq_config. - Assoc ID fix. Two additional bugs I'm investigating here are: I've seen my laptop crash in the qwx newstate task upon resuming from hibernation, where the task was trying to move to AUTH state. qwx_stop() is called when suspending and it removes the newstate task from its task queue if already scheduled. However, there is a window where FLAG_CRASH_FLUSH is unset while IFF_RUNNING is still set. If we enter qwx_newstate() in this window for some reason (e.g. due to frames received in interrupt context), the newstate task might get scheduled again. I'm not sure yet if this change fixes the crash I saw, but closing the window where the flags are set badly cannot hurt. The second issue is that we never reset the MHI ring's "queued" counter even across ifconfig down/up. I suspect this can eventually cause kernel or firmware panics when the interface goes down and back up over time. Reset the counter in qwx_mhi_init_cmd_ring since the command ring is cleared at this point and should be considered empty from there on. Ok for all or some of this? M sys/dev/ic/qwx.c | 29+ 15- M sys/dev/ic/qwxvar.h | 1+ 1- M sys/dev/pci/if_qwx_pci.c | 3+ 1- 3 files changed, 33 insertions(+), 17 deletions(-) commit - 9ce5d767a9ce105eb003f5e22b4e7bcb1da0c6ea commit + bf705e227e1fb1fca5555eb22c68dcd9268e4193 blob - 333da499f818b16c992fbff9251463a35b7b7097 blob + b5bd277bd1bae458de12f4cb5feac0e2e64c1204 --- sys/dev/ic/qwx.c +++ sys/dev/ic/qwx.c @@ -417,8 +417,6 @@ qwx_stop(struct ifnet *ifp) qwx_del_task(sc, systq, &sc->bgscan_task); refcnt_finalize(&sc->task_refs, "qwxstop"); - clear_bit(ATH11K_FLAG_CRASH_FLUSH, sc->sc_flags); - if (ic->ic_opmode == IEEE80211_M_STA && ic->ic_state == IEEE80211_S_RUN && (ic->ic_bss->ni_flags & IEEE80211_NODE_MFP)) @@ -431,6 +429,8 @@ qwx_stop(struct ifnet *ifp) ifp->if_flags &= ~IFF_RUNNING; ifq_clr_oactive(&ifp->if_snd); + clear_bit(ATH11K_FLAG_CRASH_FLUSH, sc->sc_flags); + /* * Manually run the newstate task's code for switching to INIT state. * This reconfigures firmware state to stop scanning, or disassociate @@ -910,8 +910,20 @@ qwx_add_sta_key(struct qwx_softc *sc, struct ieee80211 nq->flags |= QWX_NODE_FLAG_HAVE_PAIRWISE_KEY; if ((nq->flags & want_keymask) == want_keymask) { + uint8_t pdev_id = 0; /* TODO: derive pdev ID somehow? */ + DPRINTF("marking port %s valid\n", ether_sprintf(ni->ni_macaddr)); + + /* 4-way handshake done; authorize the BSS peer now. */ + ret = qwx_wmi_set_peer_param(sc, ni->ni_macaddr, arvif->vdev_id, + pdev_id, WMI_PEER_AUTHORIZE, 1); + if (ret) { + printf("%s: unable to authorize BSS peer: %d\n", + sc->sc_dev.dv_xname, ret); + return ret; + } + ni->ni_port_valid = 1; ieee80211_set_link_state(ic, LINK_STATE_UP); } @@ -1033,7 +1045,7 @@ qwx_clear_pn_replay_config(struct qwx_softc *sc, struc HAL_REO_CMD_UPD0_PN_CHECK | HAL_REO_CMD_UPD0_SVLD; - for (tid = 0; tid < IEEE80211_NUM_TID; tid++) { + for (tid = 0; tid <= HAL_DESC_REO_NON_QOS_TID; tid++) { rx_tid = &peer->rx_tid[tid]; if (!rx_tid->active) continue; @@ -24702,7 +24714,7 @@ qwx_peer_frags_flush(struct qwx_softc *sc, struct ath1 #ifdef notyet lockdep_assert_held(&ar->ab->base_lock); #endif - for (i = 0; i < IEEE80211_NUM_TID; i++) { + for (i = 0; i <= HAL_DESC_REO_NON_QOS_TID; i++) { rx_tid = &peer->rx_tid[i]; qwx_dp_rx_frags_cleanup(sc, rx_tid, 1); @@ -24722,7 +24734,7 @@ qwx_peer_rx_tid_cleanup(struct qwx_softc *sc, struct a #ifdef notyet lockdep_assert_held(&ar->ab->base_lock); #endif - for (i = 0; i < IEEE80211_NUM_TID; i++) { + for (i = 0; i <= HAL_DESC_REO_NON_QOS_TID; i++) { rx_tid = &peer->rx_tid[i]; qwx_peer_rx_tid_delete(sc, peer, i); @@ -24949,7 +24961,7 @@ qwx_dp_peer_setup(struct qwx_softc *sc, int vdev_id, i return ret; } - for (tid = 0; tid < IEEE80211_NUM_TID; tid++) { + for (tid = 0; tid <= HAL_DESC_REO_NON_QOS_TID; tid++) { ret = qwx_peer_rx_tid_setup(sc, ni, vdev_id, pdev_id, tid, 1, 0, HAL_PN_TYPE_NONE); if (ret) { @@ -25032,7 +25044,7 @@ qwx_dp_peer_rx_pn_replay_config(struct qwx_softc *sc, if (peer == NULL) return EINVAL; - for (tid = 0; tid < IEEE80211_NUM_TID; tid++) { + for (tid = 0; tid <= HAL_DESC_REO_NON_QOS_TID; tid++) { rx_tid = &peer->rx_tid[tid]; if (!rx_tid->active) continue; @@ -26189,7 +26201,7 @@ qwx_peer_assoc_h_basic(struct qwx_softc *sc, struct qw IEEE80211_ADDR_COPY(arg->peer_mac, ni->ni_macaddr); arg->vdev_id = arvif->vdev_id; - arg->peer_associd = ni->ni_associd; + arg->peer_associd = IEEE80211_AID(ni->ni_associd); arg->auth_flag = 1; arg->peer_listen_intval = ni->ni_intval; arg->peer_nss = 1; @@ -26627,7 +26639,7 @@ qwx_assoc(struct qwx_softc *sc) WARN_ON(arvif->is_up); #endif - arvif->aid = ni->ni_associd; + arvif->aid = IEEE80211_AID(ni->ni_associd); IEEE80211_ADDR_COPY(arvif->bssid, ni->ni_bssid); sc->bss_peer_id = nq->peer_id; @@ -26666,12 +26678,14 @@ qwx_run(struct qwx_softc *sc) DNPRINTF(QWX_D_MAC, "%s: vdev %d up (associated) bssid %s aid %d\n", __func__, arvif->vdev_id, ether_sprintf(ni->ni_bssid), arvif->aid); - ret = qwx_wmi_set_peer_param(sc, ni->ni_macaddr, arvif->vdev_id, - pdev_id, WMI_PEER_AUTHORIZE, 1); - if (ret) { - printf("%s: unable to authorize BSS peer: %d\n", - sc->sc_dev.dv_xname, ret); - return ret; + if ((ic->ic_flags & IEEE80211_F_RSNON) == 0 || ni->ni_port_valid) { + ret = qwx_wmi_set_peer_param(sc, ni->ni_macaddr, arvif->vdev_id, + pdev_id, WMI_PEER_AUTHORIZE, 1); + if (ret) { + printf("%s: unable to authorize BSS peer: %d\n", + sc->sc_dev.dv_xname, ret); + return ret; + } } return 0; blob - 1d0699c9aec0fe9f6ed45a564cadda2a93de6937 blob + d0fff9701bfccdfbb380757b05f32ce24988acff --- sys/dev/ic/qwxvar.h +++ sys/dev/ic/qwxvar.h @@ -1790,7 +1790,7 @@ struct ath11k_peer { /* protected by ab->data_lock */ struct ieee80211_key_conf *keys[WMI_MAX_KEY_INDEX + 1]; #endif - struct dp_rx_tid rx_tid[IEEE80211_NUM_TID + 1]; + struct dp_rx_tid rx_tid[HAL_DESC_REO_NON_QOS_TID + 1]; #if 0 /* peer id based rhashtable list pointer */ struct rhash_head rhash_id; blob - e8c86c7809bebb4b78b1ecb1ed39e0d4bd08b8bc blob + 7037646d93e1cf829a33510700f0e8f3654adf2f --- sys/dev/pci/if_qwx_pci.c +++ sys/dev/pci/if_qwx_pci.c @@ -1650,9 +1650,10 @@ qwx_pcic_ext_irq_config(struct qwx_softc *sc, struct p if (num_irq) { int irq_idx = irq_grp->irqs[0]; + int vector = (i % num_vectors) + base_vector; pci_intr_handle_t ih; - if (pci_intr_map_msivec(pa, irq_idx, &ih) != 0 && + if (pci_intr_map_msivec(pa, vector, &ih) != 0 && pci_intr_map(pa, &ih) != 0) { printf("%s: can't map interrupt\n", sc->sc_dev.dv_xname); @@ -2539,6 +2540,7 @@ qwx_mhi_init_cmd_ring(struct qwx_pci_softc *psc) len = ring->size; ring->rp = ring->wp = paddr; + ring->queued = 0; c = (struct qwx_mhi_cmd_ctxt *)QWX_DMA_KVA(psc->cmd_ctxt); c->rbase = htole64(paddr);