diff mbox

[v3] ath10k: fix crash in recent 3.5.3 9984 firmware due wrong handling of peer_bw_rxnss_override parameter

Message ID 20180427053603.12022-1-s.gottschall@dd-wrt.com (mailing list archive)
State Rejected
Delegated to: Kalle Valo
Headers show

Commit Message

Sebastian Gottschall April 27, 2018, 5:36 a.m. UTC 
From: Sebastian Gottschall <s.gottschall@dd-wrt.com>

current handling of peer_bw_rxnss_override parameter is based on guessing the VHT160/8080 capability by rx rate. this is wrong and may lead
to a non initialized peer_bw_rxnss_override parameter which is required since VHT160 operation mode only supports 2x2 chainmasks in addition the original code
initialized the parameter with wrong masked values.
This patch uses the peer phymode and peer nss information for correct initialisation of the peer_bw_rxnss_override parameter.
if this peer information is not available, we initialize the parameter by minimum nss which is suggested by QCA as temporary workaround according
to the QCA sourcecodes.

Signed-off-by: Sebastian Gottschall <s.gottschall@dd-wrt.com>

v2: remove debug messages
v3: apply some cosmetics, update documentation
---
 drivers/net/wireless/ath/ath10k/mac.c | 38 +++++++++++++++++++++--------------
 drivers/net/wireless/ath/ath10k/wmi.c |  7 +------
 drivers/net/wireless/ath/ath10k/wmi.h | 14 ++++++++++++-
 3 files changed, 37 insertions(+), 22 deletions(-)

Comments

kernel test robot April 27, 2018, 7:09 p.m. UTC | #1 
Hi Sebastian,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on ath6kl/ath-next]
[also build test WARNING on v4.17-rc2 next-20180426]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/s-gottschall-dd-wrt-com/ath10k-fix-crash-in-recent-3-5-3-9984-firmware-due-wrong-handling-of-peer_bw_rxnss_override-parameter/20180427-234051
base:   https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git ath-next
config: i386-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All warnings (new ones prefixed by >>):

   drivers/net/wireless/ath/ath10k/mac.c: In function 'ath10k_peer_assoc_h_vht':
>> drivers/net/wireless/ath/ath10k/mac.c:2534:3: warning: enumeration value 'MODE_11A' not handled in switch [-Wswitch]
      switch(arg->peer_phymode) {
      ^~~~~~
>> drivers/net/wireless/ath/ath10k/mac.c:2534:3: warning: enumeration value 'MODE_11G' not handled in switch [-Wswitch]
>> drivers/net/wireless/ath/ath10k/mac.c:2534:3: warning: enumeration value 'MODE_11B' not handled in switch [-Wswitch]
>> drivers/net/wireless/ath/ath10k/mac.c:2534:3: warning: enumeration value 'MODE_11GONLY' not handled in switch [-Wswitch]
>> drivers/net/wireless/ath/ath10k/mac.c:2534:3: warning: enumeration value 'MODE_11NA_HT20' not handled in switch [-Wswitch]
>> drivers/net/wireless/ath/ath10k/mac.c:2534:3: warning: enumeration value 'MODE_11NG_HT20' not handled in switch [-Wswitch]
>> drivers/net/wireless/ath/ath10k/mac.c:2534:3: warning: enumeration value 'MODE_11NA_HT40' not handled in switch [-Wswitch]
>> drivers/net/wireless/ath/ath10k/mac.c:2534:3: warning: enumeration value 'MODE_11NG_HT40' not handled in switch [-Wswitch]
>> drivers/net/wireless/ath/ath10k/mac.c:2534:3: warning: enumeration value 'MODE_11AC_VHT20' not handled in switch [-Wswitch]
>> drivers/net/wireless/ath/ath10k/mac.c:2534:3: warning: enumeration value 'MODE_11AC_VHT40' not handled in switch [-Wswitch]
>> drivers/net/wireless/ath/ath10k/mac.c:2534:3: warning: enumeration value 'MODE_11AC_VHT80' not handled in switch [-Wswitch]
>> drivers/net/wireless/ath/ath10k/mac.c:2534:3: warning: enumeration value 'MODE_11AC_VHT20_2G' not handled in switch [-Wswitch]
>> drivers/net/wireless/ath/ath10k/mac.c:2534:3: warning: enumeration value 'MODE_11AC_VHT40_2G' not handled in switch [-Wswitch]
>> drivers/net/wireless/ath/ath10k/mac.c:2534:3: warning: enumeration value 'MODE_11AC_VHT80_2G' not handled in switch [-Wswitch]
>> drivers/net/wireless/ath/ath10k/mac.c:2534:3: warning: enumeration value 'MODE_UNKNOWN' not handled in switch [-Wswitch]
>> drivers/net/wireless/ath/ath10k/mac.c:2534:3: warning: enumeration value 'MODE_MAX' not handled in switch [-Wswitch]

vim +/MODE_11A +2534 drivers/net/wireless/ath/ath10k/mac.c

  2457	
  2458	static void ath10k_peer_assoc_h_vht(struct ath10k *ar,
  2459					    struct ieee80211_vif *vif,
  2460					    struct ieee80211_sta *sta,
  2461					    struct wmi_peer_assoc_complete_arg *arg)
  2462	{
  2463		const struct ieee80211_sta_vht_cap *vht_cap = &sta->vht_cap;
  2464		struct ath10k_vif *arvif = (void *)vif->drv_priv;
  2465		struct cfg80211_chan_def def;
  2466		enum nl80211_band band;
  2467		const u16 *vht_mcs_mask;
  2468		u8 ampdu_factor;
  2469		u8 max_nss, vht_mcs;
  2470		int i;
  2471	
  2472		if (WARN_ON(ath10k_mac_vif_chan(vif, &def)))
  2473			return;
  2474	
  2475		if (!vht_cap->vht_supported)
  2476			return;
  2477	
  2478		band = def.chan->band;
  2479		vht_mcs_mask = arvif->bitrate_mask.control[band].vht_mcs;
  2480	
  2481		if (ath10k_peer_assoc_h_vht_masked(vht_mcs_mask))
  2482			return;
  2483	
  2484		arg->peer_flags |= ar->wmi.peer_flags->vht;
  2485	
  2486		if (def.chan->band == NL80211_BAND_2GHZ)
  2487			arg->peer_flags |= ar->wmi.peer_flags->vht_2g;
  2488	
  2489		arg->peer_vht_caps = vht_cap->cap;
  2490	
  2491		ampdu_factor = (vht_cap->cap &
  2492				IEEE80211_VHT_CAP_MAX_A_MPDU_LENGTH_EXPONENT_MASK) >>
  2493			       IEEE80211_VHT_CAP_MAX_A_MPDU_LENGTH_EXPONENT_SHIFT;
  2494	
  2495		/* Workaround: Some Netgear/Linksys 11ac APs set Rx A-MPDU factor to
  2496		 * zero in VHT IE. Using it would result in degraded throughput.
  2497		 * arg->peer_max_mpdu at this point contains HT max_mpdu so keep
  2498		 * it if VHT max_mpdu is smaller.
  2499		 */
  2500		arg->peer_max_mpdu = max(arg->peer_max_mpdu,
  2501					 (1U << (IEEE80211_HT_MAX_AMPDU_FACTOR +
  2502						ampdu_factor)) - 1);
  2503	
  2504		if (sta->bandwidth == IEEE80211_STA_RX_BW_80)
  2505			arg->peer_flags |= ar->wmi.peer_flags->bw80;
  2506	
  2507		if (sta->bandwidth == IEEE80211_STA_RX_BW_160)
  2508			arg->peer_flags |= ar->wmi.peer_flags->bw160;
  2509	
  2510		/* Calculate peer NSS capability from VHT capabilities if STA
  2511		 * supports VHT.
  2512		 */
  2513		for (i = 0, max_nss = 0, vht_mcs = 0; i < NL80211_VHT_NSS_MAX; i++) {
  2514			vht_mcs = __le16_to_cpu(vht_cap->vht_mcs.rx_mcs_map) >>
  2515				  (2 * i) & 3;
  2516	
  2517			if ((vht_mcs != IEEE80211_VHT_MCS_NOT_SUPPORTED) &&
  2518			    vht_mcs_mask[i])
  2519				max_nss = i + 1;
  2520		}
  2521		arg->peer_num_spatial_streams = min(sta->rx_nss, max_nss);
  2522		arg->peer_vht_rates.rx_max_rate =
  2523			__le16_to_cpu(vht_cap->vht_mcs.rx_highest);
  2524		arg->peer_vht_rates.rx_mcs_set =
  2525			__le16_to_cpu(vht_cap->vht_mcs.rx_mcs_map);
  2526		arg->peer_vht_rates.tx_max_rate =
  2527			__le16_to_cpu(vht_cap->vht_mcs.tx_highest);
  2528		arg->peer_vht_rates.tx_mcs_set = ath10k_peer_assoc_h_vht_limit(
  2529			__le16_to_cpu(vht_cap->vht_mcs.tx_mcs_map), vht_mcs_mask);
  2530		arg->peer_bw_rxnss_override = 0;
  2531	
  2532		if (arg->peer_num_spatial_streams) {
  2533			/* in case if peer is connected with vht160 or vht80+80, we need to properly adjust rxnss parameters */
> 2534			switch(arg->peer_phymode) {
  2535			case MODE_11AC_VHT80_80:
  2536				arg->peer_bw_rxnss_override = BW_NSS_FWCONF_80_80(arg->peer_num_spatial_streams);
  2537			/* fall through */
  2538			case MODE_11AC_VHT160:
  2539				arg->peer_bw_rxnss_override |= BW_NSS_FWCONF_160(arg->peer_num_spatial_streams);
  2540			break;
  2541			}
  2542		}
  2543	
  2544		/* in case peer has no nss properties for some reasons, we set local nss to minimum (1x1) to avoid a 
  2545		 * firmware assert / crash. this applies only to VHT160 or VHT80+80 and is a WAR for clients breaking
  2546		 * the spec.
  2547		 */
  2548	
  2549		if (!arg->peer_num_spatial_streams && (arg->peer_phymode == MODE_11AC_VHT80_80 || arg->peer_phymode == MODE_11AC_VHT160)) {
  2550			arg->peer_bw_rxnss_override = BW_NSS_FWCONF_MAP_ENABLE;
  2551		}
  2552	
  2553		ath10k_dbg(ar, ATH10K_DBG_MAC, "mac vht peer %pM max_mpdu %d flags 0x%x\n",
  2554			   sta->addr, arg->peer_max_mpdu, arg->peer_flags);
  2555	}
  2556	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
diff mbox

Patch

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 5be6386ede8f..594db0713186 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -2528,23 +2528,31 @@  static void ath10k_peer_assoc_h_vht(struct ath10k *ar,
 		__le16_to_cpu(vht_cap->vht_mcs.tx_highest);
 	arg->peer_vht_rates.tx_mcs_set = ath10k_peer_assoc_h_vht_limit(
 		__le16_to_cpu(vht_cap->vht_mcs.tx_mcs_map), vht_mcs_mask);
+	arg->peer_bw_rxnss_override = 0;
 
-	ath10k_dbg(ar, ATH10K_DBG_MAC, "mac vht peer %pM max_mpdu %d flags 0x%x\n",
-		   sta->addr, arg->peer_max_mpdu, arg->peer_flags);
-
-	if (arg->peer_vht_rates.rx_max_rate &&
-	    (sta->vht_cap.cap & IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_MASK)) {
-		switch (arg->peer_vht_rates.rx_max_rate) {
-		case 1560:
-			/* Must be 2x2 at 160Mhz is all it can do. */
-			arg->peer_bw_rxnss_override = 2;
-			break;
-		case 780:
-			/* Can only do 1x1 at 160Mhz (Long Guard Interval) */
-			arg->peer_bw_rxnss_override = 1;
-			break;
+	if (arg->peer_num_spatial_streams) {
+		/* in case if peer is connected with vht160 or vht80+80, we need to properly adjust rxnss parameters */
+		switch(arg->peer_phymode) {
+		case MODE_11AC_VHT80_80:
+			arg->peer_bw_rxnss_override = BW_NSS_FWCONF_80_80(arg->peer_num_spatial_streams);
+		/* fall through */
+		case MODE_11AC_VHT160:
+			arg->peer_bw_rxnss_override |= BW_NSS_FWCONF_160(arg->peer_num_spatial_streams);
+		break;
 		}
 	}
+
+	/* in case peer has no nss properties for some reasons, we set local nss to minimum (1x1) to avoid a 
+	 * firmware assert / crash. this applies only to VHT160 or VHT80+80 and is a WAR for clients breaking
+	 * the spec.
+	 */
+
+	if (!arg->peer_num_spatial_streams && (arg->peer_phymode == MODE_11AC_VHT80_80 || arg->peer_phymode == MODE_11AC_VHT160)) {
+		arg->peer_bw_rxnss_override = BW_NSS_FWCONF_MAP_ENABLE;
+	}
+
+	ath10k_dbg(ar, ATH10K_DBG_MAC, "mac vht peer %pM max_mpdu %d flags 0x%x\n",
+		   sta->addr, arg->peer_max_mpdu, arg->peer_flags);
 }
 
 static void ath10k_peer_assoc_h_qos(struct ath10k *ar,
@@ -2696,9 +2704,9 @@  static int ath10k_peer_assoc_prepare(struct ath10k *ar,
 	ath10k_peer_assoc_h_crypto(ar, vif, sta, arg);
 	ath10k_peer_assoc_h_rates(ar, vif, sta, arg);
 	ath10k_peer_assoc_h_ht(ar, vif, sta, arg);
+	ath10k_peer_assoc_h_phymode(ar, vif, sta, arg);
 	ath10k_peer_assoc_h_vht(ar, vif, sta, arg);
 	ath10k_peer_assoc_h_qos(ar, vif, sta, arg);
-	ath10k_peer_assoc_h_phymode(ar, vif, sta, arg);
 
 	return 0;
 }
diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index 2c36256a441d..3797dca317ff 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -7211,12 +7211,7 @@  ath10k_wmi_peer_assoc_fill_10_4(struct ath10k *ar, void *buf,
 	struct wmi_10_4_peer_assoc_complete_cmd *cmd = buf;
 
 	ath10k_wmi_peer_assoc_fill_10_2(ar, buf, arg);
-	if (arg->peer_bw_rxnss_override)
-		cmd->peer_bw_rxnss_override =
-			__cpu_to_le32((arg->peer_bw_rxnss_override - 1) |
-				      BIT(PEER_BW_RXNSS_OVERRIDE_OFFSET));
-	else
-		cmd->peer_bw_rxnss_override = 0;
+	cmd->peer_bw_rxnss_override = __cpu_to_le32(arg->peer_bw_rxnss_override);
 }
 
 static int
diff --git a/drivers/net/wireless/ath/ath10k/wmi.h b/drivers/net/wireless/ath/ath10k/wmi.h
index 46ae19bb2c92..1fe0aa5523a6 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.h
+++ b/drivers/net/wireless/ath/ath10k/wmi.h
@@ -6380,7 +6380,19 @@  struct wmi_10_2_peer_assoc_complete_cmd {
 	__le32 info0; /* WMI_PEER_ASSOC_INFO0_ */
 } __packed;
 
-#define PEER_BW_RXNSS_OVERRIDE_OFFSET  31
+#define BW_NSS_FWCONF_MAP_ENABLE             (1 << 31)
+#define BW_NSS_FWCONF_MAP_160MHZ_S           (0)
+#define BW_NSS_FWCONF_MAP_160MHZ_M           (0x00000007)
+#define BW_NSS_FWCONF_MAP_80_80MHZ_S         (3)
+#define BW_NSS_FWCONF_MAP_80_80MHZ_M         (0x00000038)
+#define BW_NSS_FWCONF_MAP_M                  (0x0000003F)
+
+#define GET_BW_NSS_FWCONF_160(x)             ((((x) & BW_NSS_FWCONF_MAP_160MHZ_M) >> BW_NSS_FWCONF_MAP_160MHZ_S) + 1)
+#define GET_BW_NSS_FWCONF_80_80(x)           ((((x) & BW_NSS_FWCONF_MAP_80_80MHZ_M) >> BW_NSS_FWCONF_MAP_80_80MHZ_S) + 1)
+
+/* Values defined to set 160 MHz Bandwidth NSS Mapping into FW*/
+#define BW_NSS_FWCONF_160(x)          (BW_NSS_FWCONF_MAP_ENABLE | (((x - 1) << BW_NSS_FWCONF_MAP_160MHZ_S) & BW_NSS_FWCONF_MAP_160MHZ_M))
+#define BW_NSS_FWCONF_80_80(x)        (BW_NSS_FWCONF_MAP_ENABLE | (((x - 1) << BW_NSS_FWCONF_MAP_80_80MHZ_S) & BW_NSS_FWCONF_MAP_80_80MHZ_M))
 
 struct wmi_10_4_peer_assoc_complete_cmd {
 	struct wmi_10_2_peer_assoc_complete_cmd cmd;