Skip to contentSkip to conversation

wifi: ath12k: fix chain-mask shift on 64-bit big-endian

PATCH In review Michael Pfeifroth 2 participants 2 messages Sep 16 – Sep 17 Qualcomm ATH12K wireless driver

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 wifi: ath12k: fix chain-mask shift on 64-bit big-endian

ath12k_pull_mac_phy_cap_svc_ready_ext() derives the tx/rx chain-mask shift with find_first_bit((unsigned long *)&pdev_cap->rx_chain_mask, 32).

pdev_cap->{tx,rx}_chain_mask are u32, but find_first_bit() treats the pointer as an array of unsigned long. On 64-bit big-endian, unsigned long is 8 bytes, so the u32 value ends up in the high 32 bits of the word while find_first_bit(..., 32) only scans the low 32 bits, i.e. the adjacent struct member. rx_chain_mask_shift is therefore computed as 32, ath12k_mac_setup_ht_vht_cap() evaluates cfg_rx_chainmask >> 32 == 0, and the driver advertises a VHT RX-MCS map with all streams "not supported". mac80211 then rejects the VHT capability ("Ignoring VHT IE ... due to invalid rx_mcs_map"), so the link only comes up as HT40 and never negotiates 80/160 MHz. tx_chain_mask_shift happens to scan a zero word and stays 0, so only the RX side is affected.

Use __ffs() on the mask value instead of pointing find_first_bit() at the u32's storage. __ffs() takes its argument by value, so the shift is computed correctly regardless of word size or endianness. The chain masks are always non-zero here, so __ffs() is well defined.

Fixes d889913205cf ("wifi: ath12k: driver for Qualcomm Wi-Fi 7 devices")
---
 drivers/net/wireless/ath/ath12k/wmi.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)
drivers/net/wireless/ath/ath12k/wmi.c +2 -4
@@ -563,10 +563,8 @@ ath12k_pull_mac_phy_cap_svc_ready_ext(struct ath12k_wmi_pdev *wmi_handle,
 	 * for tx/rx chainmask which will be used to advertise supported ht/vht rates to
 	 * mac80211.
 	 */
-	pdev_cap->tx_chain_mask_shift =
-			find_first_bit((unsigned long *)&pdev_cap->tx_chain_mask, 32);
-	pdev_cap->rx_chain_mask_shift =
-			find_first_bit((unsigned long *)&pdev_cap->rx_chain_mask, 32);
+	pdev_cap->tx_chain_mask_shift = __ffs(pdev_cap->tx_chain_mask);
+	pdev_cap->rx_chain_mask_shift = __ffs(pdev_cap->rx_chain_mask);
 
 	if (supported_bands & WMI_HOST_WLAN_2GHZ_CAP) {
 		cap_band = &pdev_cap->band[NL80211_BAND_2GHZ];
Reviewed-by on v1 1/1
#1 lore raw
Quoted 15 lines · Michael Pfeifroth

ath12k_pull_mac_phy_cap_svc_ready_ext() derives the tx/rx chain-mask shift with find_first_bit((unsigned long *)&pdev_cap->rx_chain_mask, 32).

pdev_cap->{tx,rx}_chain_mask are u32, but find_first_bit() treats the pointer as an array of unsigned long. On 64-bit big-endian, unsigned long is 8 bytes, so the u32 value ends up in the high 32 bits of the word while find_first_bit(..., 32) only scans the low 32 bits, i.e. the adjacent struct member. rx_chain_mask_shift is therefore computed as 32, ath12k_mac_setup_ht_vht_cap() evaluates cfg_rx_chainmask >> 32 == 0, and the driver advertises a VHT RX-MCS map with all streams "not supported". mac80211 then rejects the VHT capability ("Ignoring VHT IE ... due to invalid rx_mcs_map"), so the link only comes up as HT40 and never negotiates 80/160 MHz. tx_chain_mask_shift happens to scan a zero word and stays 0, so only the RX side is affected.

Quoted 8 lines

Use __ffs() on the mask value instead of pointing find_first_bit() at the u32's storage. __ffs() takes its argument by value, so the shift is computed correctly regardless of word size or endianness. The chain masks are always non-zero here, so __ffs() is well defined.

Fixes d889913205cf ("wifi: ath12k: driver for Qualcomm Wi-Fi 7 devices")

ath11k has the same issue, interested to fix it as well?

anyway: