2018-04-27 05:36:12

by Sebastian Gottschall

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

From: Sebastian Gottschall <[email protected]>

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 <[email protected]>

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(-)

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;
--
2.14.1


2018-04-27 19:10:02

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v3] ath10k: fix crash in recent 3.5.3 9984 firmware due wrong handling of peer_bw_rxnss_override parameter

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


Attachments:
(No filename) (6.97 kB)
.config.gz (61.41 kB)
Download all attachments