[PATCH 1/5] wifi: ath12k: fix MLO station firmware crash recovery
Baochen Qiang
baochen.qiang at oss.qualcomm.com
Fri Jul 31 01:48:19 PDT 2026
On 7/28/2026 12:27 AM, Jose Ignacio Tornos Martinez wrote:
> ATH12K_FLAG_RECOVERY is cleared too early in
> ath12k_core_reconfigure_on_crash(), before mac80211 runs
> ieee80211_reconfig(). By the time mac80211 calls back into the driver
> (sta_state, change_vif_links, set_key), the RECOVERY flag is already false,
> so the driver treats recovery callbacks as normal operations.
>
> This causes several problems during MLO recovery:
>
> - ath12k_mac_op_sta_state() tries to activate MLO links during the
> AUTH->ASSOC transition, calling ieee80211_set_active_links()
> recursively, which triggers a WARNING at net/mac80211/link.c.
>
> - ath12k_mac_op_change_vif_links() processes link removal during
> reconfig, causing inconsistent state.
>
> - ath12k_mac_set_key() fails with "cannot install key for non-existent
> peer" because peers do not exist yet during reconfig. Keys will be
> re-established during normal reconnection after
> ieee80211_hw_restart_disconnect() triggers a fresh association.
>
> - ath12k_mac_flush() waits for pending TX to complete, but after a
> firmware crash the TX will never complete, causing a 20 second timeout.
>
> - ath12k_mac_station_remove() calls ath12k_bss_disassoc() and
> ath12k_mac_vdev_stop() which send WMI commands to dead firmware,
> causing timeouts that delay recovery.
>
> - ath12k_clear_peer_keys() tries to look up and clear peer keys, but
> peers are already gone after firmware crash.
>
> - ath12k_dp_rx_ampdu_stop() dereferences per-link station state that
> may not be valid during crash teardown.
>
> - ath12k_peer_mlo_link_peers_delete() sends WMI peer delete commands
> for each MLO link peer. With dead firmware these time out and can
> trigger cascading resets.
>
> - HAL srng source ring operations (ath12k_hal_srng_src_num_free,
> ath12k_hal_srng_src_get_next_entry, ath12k_hal_srng_access_end)
> may attempt MMIO access to hardware that is no longer responsive
> if TX paths race with the crash. Guard these to prevent potential
> hard lockups on unresponsive hardware.
>
> These issues were observed during sporadic firmware crashes in MLO
> operation. To allow systematic testing and reproduction, the debugfs
> simulate_fw_crash interface was used to trigger controlled firmware
> crashes during active MLO connections with traffic.
>
> Fix by moving clear_bit(ATH12K_FLAG_RECOVERY) from
> ath12k_core_reconfigure_on_crash() to ath12k_mac_op_reconfig_complete(),
> so the flag stays set through the entire mac80211 reconfig phase. Add
> ATH12K_FLAG_RECOVERY checks in change_vif_links, set_key, and sta_state
> to skip operations that are invalid during recovery. Add
> ATH12K_FLAG_CRASH_FLUSH checks in mac_flush, station_remove,
> clear_peer_keys, dp_rx_ampdu_stop, peer_mlo_link_peers_delete, and the
> HAL srng source ring helpers to return immediately when the firmware is
> dead.
>
> Tested on WCN7850 with MLO (Wi-Fi 7).
>
> Signed-off-by: Jose Ignacio Tornos Martinez <jtornosm at redhat.com>
> ---
> drivers/net/wireless/ath/ath12k/core.c | 2 --
> drivers/net/wireless/ath/ath12k/dp_rx.c | 3 +++
> drivers/net/wireless/ath/ath12k/hal.c | 10 ++++++++++
> drivers/net/wireless/ath/ath12k/mac.c | 19 +++++++++++++++++--
> drivers/net/wireless/ath/ath12k/peer.c | 6 ++++++
> 5 files changed, 36 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath12k/core.c b/drivers/net/wireless/ath/ath12k/core.c
> index 742d4fd1b598..5c3883b19da1 100644
> --- a/drivers/net/wireless/ath/ath12k/core.c
> +++ b/drivers/net/wireless/ath/ath12k/core.c
> @@ -1410,8 +1410,6 @@ static int ath12k_core_reconfigure_on_crash(struct ath12k_base *ab)
> if (ret)
> goto err_hal_srng_deinit;
>
> - clear_bit(ATH12K_FLAG_RECOVERY, &ab->dev_flags);
> -
> return 0;
>
> err_hal_srng_deinit:
> diff --git a/drivers/net/wireless/ath/ath12k/dp_rx.c b/drivers/net/wireless/ath/ath12k/dp_rx.c
> index 8fa0e90b4531..473855ded8a7 100644
> --- a/drivers/net/wireless/ath/ath12k/dp_rx.c
> +++ b/drivers/net/wireless/ath/ath12k/dp_rx.c
> @@ -751,6 +751,9 @@ int ath12k_dp_rx_ampdu_stop(struct ath12k *ar,
>
> lockdep_assert_wiphy(ath12k_ar_to_hw(ar)->wiphy);
>
> + if (test_bit(ATH12K_FLAG_CRASH_FLUSH, &ab->dev_flags))
> + return 0;
> +
> arsta = wiphy_dereference(ath12k_ar_to_hw(ar)->wiphy,
> ahsta->link[link_id]);
> if (!arsta)
> diff --git a/drivers/net/wireless/ath/ath12k/hal.c b/drivers/net/wireless/ath/ath12k/hal.c
> index 071cb5d30931..6d3f4bca46a3 100644
> --- a/drivers/net/wireless/ath/ath12k/hal.c
> +++ b/drivers/net/wireless/ath/ath12k/hal.c
> @@ -376,6 +376,9 @@ int ath12k_hal_srng_src_num_free(struct ath12k_base *ab, struct hal_srng *srng,
>
> lockdep_assert_held(&srng->lock);
>
> + if (unlikely(test_bit(ATH12K_FLAG_CRASH_FLUSH, &ab->dev_flags)))
ath12k_hal_srng_src_num_free(), ath12k_hal_srng_src_get_next_entry(), and
ath12k_hal_srng_access_end() sit on the per-packet TX/RX hot path. Adding an
ATH12K_FLAG_CRASH_FLUSH test in the lowest HAL layer overloads a global flag onto all srng
operations and is a layering violation — the HAL should not know about device-crash semantics.
> + return 0;
> +
> hp = srng->u.src_ring.hp;
>
> if (sync_hw_ptr) {
> @@ -419,6 +422,9 @@ void *ath12k_hal_srng_src_get_next_entry(struct ath12k_base *ab,
>
> lockdep_assert_held(&srng->lock);
>
> + if (unlikely(test_bit(ATH12K_FLAG_CRASH_FLUSH, &ab->dev_flags)))
> + return NULL;
> +
> /* TODO: Using % is expensive, but we have to do this since size of some
> * SRNG rings is not power of 2 (due to descriptor sizes). Need to see
> * if separate function is defined for rings having power of 2 ring size
> @@ -524,6 +530,10 @@ void ath12k_hal_srng_access_end(struct ath12k_base *ab, struct hal_srng *srng)
> {
> lockdep_assert_held(&srng->lock);
>
> + if (srng->ring_dir == HAL_SRNG_DIR_SRC &&
> + unlikely(test_bit(ATH12K_FLAG_CRASH_FLUSH, &ab->dev_flags)))
> + return;
> +
> if (srng->flags & HAL_SRNG_FLAGS_LMAC_RING) {
> /* For LMAC rings, ring pointer updates are done through FW and
> * hence written to a shared memory location that is read by FW
> diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
> index 51c4df32e716..c559ced9e524 100644
> --- a/drivers/net/wireless/ath/ath12k/mac.c
> +++ b/drivers/net/wireless/ath/ath12k/mac.c
> @@ -4278,6 +4278,9 @@ ath12k_mac_op_change_vif_links(struct ieee80211_hw *hw,
>
> lockdep_assert_wiphy(hw->wiphy);
>
> + if (old_links && test_bit(ATH12K_FLAG_RECOVERY, &ah->radio[0].ab->dev_flags))
why it is limited to the first radio?
> + return -EINVAL;
> +
> ath12k_generic_dbg(ATH12K_DBG_MAC,
> "mac vif link changed for MLD %pM old_links 0x%x new_links 0x%x\n",
> vif->addr, old_links, new_links);
> @@ -5956,6 +5959,9 @@ static int ath12k_clear_peer_keys(struct ath12k_link_vif *arvif,
>
> lockdep_assert_wiphy(ath12k_ar_to_hw(ar)->wiphy);
>
> + if (test_bit(ATH12K_FLAG_CRASH_FLUSH, &ab->dev_flags))
> + return 0;
> +
> spin_lock_bh(&dp->dp_lock);
> peer = ath12k_dp_link_peer_find_by_vdev_and_addr(dp, arvif->vdev_id, addr);
> if (!peer || !peer->dp_peer) {
> @@ -6031,6 +6037,8 @@ static int ath12k_mac_set_key(struct ath12k *ar, enum set_key_cmd cmd,
> spin_unlock_bh(&dp->dp_lock);
>
> if (cmd == SET_KEY) {
> + if (test_bit(ATH12K_FLAG_RECOVERY, &ab->dev_flags))
> + return 0;
> ath12k_warn(ab, "cannot install key for non-existent peer %pM\n",
> peer_addr);
> return -EOPNOTSUPP;
> @@ -7045,7 +7053,8 @@ static int ath12k_mac_station_remove(struct ath12k *ar,
>
> wiphy_work_cancel(ar->ah->hw->wiphy, &arsta->update_wk);
>
> - if (ahvif->vdev_type == WMI_VDEV_TYPE_STA) {
> + if (ahvif->vdev_type == WMI_VDEV_TYPE_STA &&
> + !test_bit(ATH12K_FLAG_CRASH_FLUSH, &ar->ab->dev_flags)) {
> ath12k_bss_disassoc(ar, arvif);
> ret = ath12k_mac_vdev_stop(arvif);
> if (ret)
> @@ -7795,7 +7804,8 @@ int ath12k_mac_op_sta_state(struct ieee80211_hw *hw,
> * about to move to the associated state.
> */
> if (ieee80211_vif_is_mld(vif) && vif->type == NL80211_IFTYPE_STATION &&
> - old_state == IEEE80211_STA_AUTH && new_state == IEEE80211_STA_ASSOC) {
> + old_state == IEEE80211_STA_AUTH && new_state == IEEE80211_STA_ASSOC &&
> + !test_bit(ATH12K_FLAG_RECOVERY, &ah->radio[0].ab->dev_flags)) {
> /* TODO: for now only do link selection for single device
> * MLO case. Other cases would be handled in the future.
> */
> @@ -12596,6 +12606,9 @@ static int ath12k_mac_flush(struct ath12k *ar)
> long time_left;
> int ret = 0;
>
> + if (test_bit(ATH12K_FLAG_CRASH_FLUSH, &ar->ab->dev_flags))
> + return -ESHUTDOWN;
> +
> time_left = wait_event_timeout(ar->dp.tx_empty_waitq,
> (atomic_read(&ar->dp.num_tx_pending) == 0),
> ATH12K_FLUSH_TIMEOUT);
> @@ -13494,6 +13507,8 @@ ath12k_mac_op_reconfig_complete(struct ieee80211_hw *hw,
> for_each_ar(ah, ar, i) {
> ab = ar->ab;
>
> + clear_bit(ATH12K_FLAG_RECOVERY, &ab->dev_flags);
> +
> ath12k_warn(ar->ab, "pdev %d successfully recovered\n",
> ar->pdev->pdev_id);
>
> diff --git a/drivers/net/wireless/ath/ath12k/peer.c b/drivers/net/wireless/ath/ath12k/peer.c
> index 2681a047d4d5..e96eb78bbc0c 100644
> --- a/drivers/net/wireless/ath/ath12k/peer.c
> +++ b/drivers/net/wireless/ath/ath12k/peer.c
> @@ -297,6 +297,12 @@ int ath12k_peer_mlo_link_peers_delete(struct ath12k_vif *ahvif, struct ath12k_st
> if (!sta->mlo)
> return -EINVAL;
>
> + /* During firmware crash, peers are already gone. Skip WMI peer delete
> + * to avoid timeouts that delay recovery and can trigger cascading resets.
> + */
> + if (test_bit(ATH12K_FLAG_CRASH_FLUSH, &ah->radio[0].ab->dev_flags))
> + return 0;
> +
> /* FW expects delete of all link peers at once before waiting for reception
> * of peer unmap or delete responses
> */
More information about the ath12k
mailing list