Linux wireless drivers development
 help / color / mirror / Atom feed
* [PATCH 13/16] iwlwifi: mvm: fix frame drop from the reordering buffer
From: Luca Coelho @ 2019-07-20 10:25 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless, Emmanuel Grumbach, Luca Coelho
In-Reply-To: <20190720102545.5952-1-luca@coelho.fi>

From: Emmanuel Grumbach <emmanuel.grumbach@intel.com>

An earlier patch made sure that the queues are not lagging
too far behind. This means that iwl_mvm_release_frames
should not be called with a head_sn too far behind NSSN.

Don't take the risk to change completely the entry
condition to iwl_mvm_release_frames, but don't update
the head_sn is the NSSN is more than 2048 packets ahead
of us. Since this just cannot be right. This means that
the scenario described here happened. We are queue 0.

	Q:0				Q:1
	head_sn: 0    -> 2047
					head_sn: 2048

	Lots of packets arrive:
	head_sn: 2047 -> 2150

					send NSSN_SYNC notification

	Handle notification
	from the firmware and
	do NOT move the head_sn
	back to 2048

Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c | 44 ++++++++++++++-----
 1 file changed, 33 insertions(+), 11 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c b/drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c
index 4f4fdaf49eef..854edd7d7103 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c
@@ -518,12 +518,17 @@ static void iwl_mvm_sync_nssn(struct iwl_mvm *mvm, u8 baid, u16 nssn)
 
 #define RX_REORDER_BUF_TIMEOUT_MQ (HZ / 10)
 
+enum iwl_mvm_release_flags {
+	IWL_MVM_RELEASE_SEND_RSS_SYNC = BIT(0),
+	IWL_MVM_RELEASE_FROM_RSS_SYNC = BIT(1),
+};
+
 static void iwl_mvm_release_frames(struct iwl_mvm *mvm,
 				   struct ieee80211_sta *sta,
 				   struct napi_struct *napi,
 				   struct iwl_mvm_baid_data *baid_data,
 				   struct iwl_mvm_reorder_buffer *reorder_buf,
-				   u16 nssn, bool sync_rss)
+				   u16 nssn, u32 flags)
 {
 	struct iwl_mvm_reorder_buf_entry *entries =
 		&baid_data->entries[reorder_buf->queue *
@@ -532,6 +537,18 @@ static void iwl_mvm_release_frames(struct iwl_mvm *mvm,
 
 	lockdep_assert_held(&reorder_buf->lock);
 
+	/*
+	 * We keep the NSSN not too far behind, if we are sync'ing it and it
+	 * is more than 2048 ahead of us, it must be behind us. Discard it.
+	 * This can happen if the queue that hit the 0 / 2048 seqno was lagging
+	 * behind and this queue already processed packets. The next if
+	 * would have caught cases where this queue would have processed less
+	 * than 64 packets, but it may have processed more than 64 packets.
+	 */
+	if ((flags & IWL_MVM_RELEASE_FROM_RSS_SYNC) &&
+	    ieee80211_sn_less(nssn, ssn))
+		goto set_timer;
+
 	/* ignore nssn smaller than head sn - this can happen due to timeout */
 	if (iwl_mvm_is_sn_less(nssn, ssn, reorder_buf->buf_size))
 		goto set_timer;
@@ -542,7 +559,8 @@ static void iwl_mvm_release_frames(struct iwl_mvm *mvm,
 		struct sk_buff *skb;
 
 		ssn = ieee80211_sn_inc(ssn);
-		if (sync_rss && (ssn == 2048 || ssn == 0))
+		if ((flags & IWL_MVM_RELEASE_SEND_RSS_SYNC) &&
+		    (ssn == 2048 || ssn == 0))
 			iwl_mvm_sync_nssn(mvm, baid_data->baid, ssn);
 
 		/*
@@ -631,7 +649,7 @@ void iwl_mvm_reorder_timer_expired(struct timer_list *t)
 		iwl_mvm_event_frame_timeout_callback(buf->mvm, mvmsta->vif,
 						     sta, baid_data->tid);
 		iwl_mvm_release_frames(buf->mvm, sta, NULL, baid_data,
-				       buf, sn, true);
+				       buf, sn, IWL_MVM_RELEASE_SEND_RSS_SYNC);
 		rcu_read_unlock();
 	} else {
 		/*
@@ -674,7 +692,7 @@ static void iwl_mvm_del_ba(struct iwl_mvm *mvm, int queue,
 	iwl_mvm_release_frames(mvm, sta, NULL, ba_data, reorder_buf,
 			       ieee80211_sn_add(reorder_buf->head_sn,
 						reorder_buf->buf_size),
-			       false);
+			       0);
 	spin_unlock_bh(&reorder_buf->lock);
 	del_timer_sync(&reorder_buf->reorder_timer);
 
@@ -684,7 +702,8 @@ static void iwl_mvm_del_ba(struct iwl_mvm *mvm, int queue,
 
 static void iwl_mvm_release_frames_from_notif(struct iwl_mvm *mvm,
 					      struct napi_struct *napi,
-					      u8 baid, u16 nssn, int queue)
+					      u8 baid, u16 nssn, int queue,
+					      u32 flags)
 {
 	struct ieee80211_sta *sta;
 	struct iwl_mvm_reorder_buffer *reorder_buf;
@@ -711,7 +730,7 @@ static void iwl_mvm_release_frames_from_notif(struct iwl_mvm *mvm,
 
 	spin_lock_bh(&reorder_buf->lock);
 	iwl_mvm_release_frames(mvm, sta, napi, ba_data,
-			       reorder_buf, nssn, false);
+			       reorder_buf, nssn, flags);
 	spin_unlock_bh(&reorder_buf->lock);
 
 out:
@@ -723,7 +742,8 @@ static void iwl_mvm_nssn_sync(struct iwl_mvm *mvm,
 			      const struct iwl_mvm_nssn_sync_data *data)
 {
 	iwl_mvm_release_frames_from_notif(mvm, napi, data->baid,
-					  data->nssn, queue);
+					  data->nssn, queue,
+					  IWL_MVM_RELEASE_FROM_RSS_SYNC);
 }
 
 void iwl_mvm_rx_queue_notif(struct iwl_mvm *mvm, struct napi_struct *napi,
@@ -851,7 +871,7 @@ static bool iwl_mvm_reorder(struct iwl_mvm *mvm,
 
 	if (ieee80211_is_back_req(hdr->frame_control)) {
 		iwl_mvm_release_frames(mvm, sta, napi, baid_data,
-				       buffer, nssn, false);
+				       buffer, nssn, 0);
 		goto drop;
 	}
 
@@ -871,7 +891,7 @@ static bool iwl_mvm_reorder(struct iwl_mvm *mvm,
 		u16 min_sn = ieee80211_sn_less(sn, nssn) ? sn : nssn;
 
 		iwl_mvm_release_frames(mvm, sta, napi, baid_data, buffer,
-				       min_sn, true);
+				       min_sn, IWL_MVM_RELEASE_SEND_RSS_SYNC);
 	}
 
 	/* drop any oudated packets */
@@ -963,7 +983,8 @@ static bool iwl_mvm_reorder(struct iwl_mvm *mvm,
 	 */
 	if (!amsdu || last_subframe)
 		iwl_mvm_release_frames(mvm, sta, napi, baid_data,
-				       buffer, nssn, true);
+				       buffer, nssn,
+				       IWL_MVM_RELEASE_SEND_RSS_SYNC);
 
 	spin_unlock_bh(&buffer->lock);
 	return true;
@@ -1936,5 +1957,6 @@ void iwl_mvm_rx_frame_release(struct iwl_mvm *mvm, struct napi_struct *napi,
 	struct iwl_frame_release *release = (void *)pkt->data;
 
 	iwl_mvm_release_frames_from_notif(mvm, napi, release->baid,
-					  le16_to_cpu(release->nssn), queue);
+					  le16_to_cpu(release->nssn),
+					  queue, 0);
 }
-- 
2.20.1


^ permalink raw reply related

* [PATCH 12/16] iwlwifi: mvm: replace RS mutex with a spin_lock
From: Luca Coelho @ 2019-07-20 10:25 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless, Gregory Greenman, Luca Coelho
In-Reply-To: <20190720102545.5952-1-luca@coelho.fi>

From: Gregory Greenman <gregory.greenman@intel.com>

The solution with the worker still had a bug, as in order
to get sta, rcu_read_lock should be used and thus no mutex
can be used inside iwl_mvm_rs_rate_init.

Also, spin_lock is a simpler solution, no need to spawn a
dedicated worker.

Signed-off-by: Gregory Greenman <gregory.greenman@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 drivers/net/wireless/intel/iwlwifi/mvm/rs.c  | 530 +++++++++----------
 drivers/net/wireless/intel/iwlwifi/mvm/rs.h  |   6 +-
 drivers/net/wireless/intel/iwlwifi/mvm/sta.c |   6 +-
 drivers/net/wireless/intel/iwlwifi/mvm/sta.h |   1 -
 4 files changed, 258 insertions(+), 285 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/rs.c b/drivers/net/wireless/intel/iwlwifi/mvm/rs.c
index bd977ec8629b..3fa50b1955bb 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/rs.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/rs.c
@@ -1197,278 +1197,6 @@ static u8 rs_get_tid(struct ieee80211_hdr *hdr)
 	return tid;
 }
 
-void iwl_mvm_rs_init_wk(struct work_struct *wk)
-{
-	struct iwl_mvm_sta *mvmsta = container_of(wk, struct iwl_mvm_sta,
-						  rs_init_wk);
-	struct iwl_mvm_vif *mvmvif = iwl_mvm_vif_from_mac80211(mvmsta->vif);
-	struct ieee80211_sta *sta;
-
-	rcu_read_lock();
-
-	sta = rcu_dereference(mvmvif->mvm->fw_id_to_mac_id[mvmsta->sta_id]);
-	if (WARN_ON_ONCE(IS_ERR_OR_NULL(sta))) {
-		rcu_read_unlock();
-		return;
-	}
-
-	iwl_mvm_rs_rate_init(mvmvif->mvm, sta, mvmvif->phy_ctxt->channel->band,
-			     true);
-
-	rcu_read_unlock();
-}
-
-static void __iwl_mvm_rs_tx_status(struct iwl_mvm *mvm,
-				   struct ieee80211_sta *sta,
-				   int tid, struct ieee80211_tx_info *info,
-				   bool ndp)
-{
-	int legacy_success;
-	int retries;
-	int i;
-	struct iwl_lq_cmd *table;
-	u32 lq_hwrate;
-	struct rs_rate lq_rate, tx_resp_rate;
-	struct iwl_scale_tbl_info *curr_tbl, *other_tbl, *tmp_tbl;
-	u32 tlc_info = (uintptr_t)info->status.status_driver_data[0];
-	u8 reduced_txp = tlc_info & RS_DRV_DATA_TXP_MSK;
-	u8 lq_color = RS_DRV_DATA_LQ_COLOR_GET(tlc_info);
-	u32 tx_resp_hwrate = (uintptr_t)info->status.status_driver_data[1];
-	struct iwl_mvm_sta *mvmsta = iwl_mvm_sta_from_mac80211(sta);
-	struct iwl_lq_sta *lq_sta = &mvmsta->lq_sta.rs_drv;
-
-	/* Treat uninitialized rate scaling data same as non-existing. */
-	if (!lq_sta) {
-		IWL_DEBUG_RATE(mvm, "Station rate scaling not created yet.\n");
-		return;
-	} else if (!lq_sta->pers.drv) {
-		IWL_DEBUG_RATE(mvm, "Rate scaling not initialized yet.\n");
-		return;
-	}
-
-	/* This packet was aggregated but doesn't carry status info */
-	if ((info->flags & IEEE80211_TX_CTL_AMPDU) &&
-	    !(info->flags & IEEE80211_TX_STAT_AMPDU))
-		return;
-
-	if (rs_rate_from_ucode_rate(tx_resp_hwrate, info->band,
-				    &tx_resp_rate)) {
-		WARN_ON_ONCE(1);
-		return;
-	}
-
-#ifdef CONFIG_MAC80211_DEBUGFS
-	/* Disable last tx check if we are debugging with fixed rate but
-	 * update tx stats */
-	if (lq_sta->pers.dbg_fixed_rate) {
-		int index = tx_resp_rate.index;
-		enum rs_column column;
-		int attempts, success;
-
-		column = rs_get_column_from_rate(&tx_resp_rate);
-		if (WARN_ONCE(column == RS_COLUMN_INVALID,
-			      "Can't map rate 0x%x to column",
-			      tx_resp_hwrate))
-			return;
-
-		if (info->flags & IEEE80211_TX_STAT_AMPDU) {
-			attempts = info->status.ampdu_len;
-			success = info->status.ampdu_ack_len;
-		} else {
-			attempts = info->status.rates[0].count;
-			success = !!(info->flags & IEEE80211_TX_STAT_ACK);
-		}
-
-		lq_sta->pers.tx_stats[column][index].total += attempts;
-		lq_sta->pers.tx_stats[column][index].success += success;
-
-		IWL_DEBUG_RATE(mvm, "Fixed rate 0x%x success %d attempts %d\n",
-			       tx_resp_hwrate, success, attempts);
-		return;
-	}
-#endif
-
-	if (time_after(jiffies,
-		       (unsigned long)(lq_sta->last_tx +
-				       (IWL_MVM_RS_IDLE_TIMEOUT * HZ)))) {
-		IWL_DEBUG_RATE(mvm, "Tx idle for too long. reinit rs\n");
-		schedule_work(&mvmsta->rs_init_wk);
-		return;
-	}
-	lq_sta->last_tx = jiffies;
-
-	/* Ignore this Tx frame response if its initial rate doesn't match
-	 * that of latest Link Quality command.  There may be stragglers
-	 * from a previous Link Quality command, but we're no longer interested
-	 * in those; they're either from the "active" mode while we're trying
-	 * to check "search" mode, or a prior "search" mode after we've moved
-	 * to a new "search" mode (which might become the new "active" mode).
-	 */
-	table = &lq_sta->lq;
-	lq_hwrate = le32_to_cpu(table->rs_table[0]);
-	if (rs_rate_from_ucode_rate(lq_hwrate, info->band, &lq_rate)) {
-		WARN_ON_ONCE(1);
-		return;
-	}
-
-	/* Here we actually compare this rate to the latest LQ command */
-	if (lq_color != LQ_FLAG_COLOR_GET(table->flags)) {
-		IWL_DEBUG_RATE(mvm,
-			       "tx resp color 0x%x does not match 0x%x\n",
-			       lq_color, LQ_FLAG_COLOR_GET(table->flags));
-
-		/*
-		 * Since rates mis-match, the last LQ command may have failed.
-		 * After IWL_MISSED_RATE_MAX mis-matches, resync the uCode with
-		 * ... driver.
-		 */
-		lq_sta->missed_rate_counter++;
-		if (lq_sta->missed_rate_counter > IWL_MVM_RS_MISSED_RATE_MAX) {
-			lq_sta->missed_rate_counter = 0;
-			IWL_DEBUG_RATE(mvm,
-				       "Too many rates mismatch. Send sync LQ. rs_state %d\n",
-				       lq_sta->rs_state);
-			iwl_mvm_send_lq_cmd(mvm, &lq_sta->lq);
-		}
-		/* Regardless, ignore this status info for outdated rate */
-		return;
-	} else
-		/* Rate did match, so reset the missed_rate_counter */
-		lq_sta->missed_rate_counter = 0;
-
-	if (!lq_sta->search_better_tbl) {
-		curr_tbl = &(lq_sta->lq_info[lq_sta->active_tbl]);
-		other_tbl = &(lq_sta->lq_info[1 - lq_sta->active_tbl]);
-	} else {
-		curr_tbl = &(lq_sta->lq_info[1 - lq_sta->active_tbl]);
-		other_tbl = &(lq_sta->lq_info[lq_sta->active_tbl]);
-	}
-
-	if (WARN_ON_ONCE(!rs_rate_column_match(&lq_rate, &curr_tbl->rate))) {
-		IWL_DEBUG_RATE(mvm,
-			       "Neither active nor search matches tx rate\n");
-		tmp_tbl = &(lq_sta->lq_info[lq_sta->active_tbl]);
-		rs_dump_rate(mvm, &tmp_tbl->rate, "ACTIVE");
-		tmp_tbl = &(lq_sta->lq_info[1 - lq_sta->active_tbl]);
-		rs_dump_rate(mvm, &tmp_tbl->rate, "SEARCH");
-		rs_dump_rate(mvm, &lq_rate, "ACTUAL");
-
-		/*
-		 * no matching table found, let's by-pass the data collection
-		 * and continue to perform rate scale to find the rate table
-		 */
-		rs_stay_in_table(lq_sta, true);
-		goto done;
-	}
-
-	/*
-	 * Updating the frame history depends on whether packets were
-	 * aggregated.
-	 *
-	 * For aggregation, all packets were transmitted at the same rate, the
-	 * first index into rate scale table.
-	 */
-	if (info->flags & IEEE80211_TX_STAT_AMPDU) {
-		rs_collect_tpc_data(mvm, lq_sta, curr_tbl, tx_resp_rate.index,
-				    info->status.ampdu_len,
-				    info->status.ampdu_ack_len,
-				    reduced_txp);
-
-		/* ampdu_ack_len = 0 marks no BA was received. For TLC, treat
-		 * it as a single frame loss as we don't want the success ratio
-		 * to dip too quickly because a BA wasn't received.
-		 * For TPC, there's no need for this optimisation since we want
-		 * to recover very quickly from a bad power reduction and,
-		 * therefore we'd like the success ratio to get an immediate hit
-		 * when failing to get a BA, so we'd switch back to a lower or
-		 * zero power reduction. When FW transmits agg with a rate
-		 * different from the initial rate, it will not use reduced txp
-		 * and will send BA notification twice (one empty with reduced
-		 * txp equal to the value from LQ and one with reduced txp 0).
-		 * We need to update counters for each txp level accordingly.
-		 */
-		if (info->status.ampdu_ack_len == 0)
-			info->status.ampdu_len = 1;
-
-		rs_collect_tlc_data(mvm, mvmsta, tid, curr_tbl,
-				    tx_resp_rate.index,
-				    info->status.ampdu_len,
-				    info->status.ampdu_ack_len);
-
-		/* Update success/fail counts if not searching for new mode */
-		if (lq_sta->rs_state == RS_STATE_STAY_IN_COLUMN) {
-			lq_sta->total_success += info->status.ampdu_ack_len;
-			lq_sta->total_failed += (info->status.ampdu_len -
-					info->status.ampdu_ack_len);
-		}
-	} else {
-		/* For legacy, update frame history with for each Tx retry. */
-		retries = info->status.rates[0].count - 1;
-		/* HW doesn't send more than 15 retries */
-		retries = min(retries, 15);
-
-		/* The last transmission may have been successful */
-		legacy_success = !!(info->flags & IEEE80211_TX_STAT_ACK);
-		/* Collect data for each rate used during failed TX attempts */
-		for (i = 0; i <= retries; ++i) {
-			lq_hwrate = le32_to_cpu(table->rs_table[i]);
-			if (rs_rate_from_ucode_rate(lq_hwrate, info->band,
-						    &lq_rate)) {
-				WARN_ON_ONCE(1);
-				return;
-			}
-
-			/*
-			 * Only collect stats if retried rate is in the same RS
-			 * table as active/search.
-			 */
-			if (rs_rate_column_match(&lq_rate, &curr_tbl->rate))
-				tmp_tbl = curr_tbl;
-			else if (rs_rate_column_match(&lq_rate,
-						      &other_tbl->rate))
-				tmp_tbl = other_tbl;
-			else
-				continue;
-
-			rs_collect_tpc_data(mvm, lq_sta, tmp_tbl,
-					    tx_resp_rate.index, 1,
-					    i < retries ? 0 : legacy_success,
-					    reduced_txp);
-			rs_collect_tlc_data(mvm, mvmsta, tid, tmp_tbl,
-					    tx_resp_rate.index, 1,
-					    i < retries ? 0 : legacy_success);
-		}
-
-		/* Update success/fail counts if not searching for new mode */
-		if (lq_sta->rs_state == RS_STATE_STAY_IN_COLUMN) {
-			lq_sta->total_success += legacy_success;
-			lq_sta->total_failed += retries + (1 - legacy_success);
-		}
-	}
-	/* The last TX rate is cached in lq_sta; it's set in if/else above */
-	lq_sta->last_rate_n_flags = lq_hwrate;
-	IWL_DEBUG_RATE(mvm, "reduced txpower: %d\n", reduced_txp);
-done:
-	/* See if there's a better rate or modulation mode to try. */
-	if (sta->supp_rates[info->band])
-		rs_rate_scale_perform(mvm, sta, lq_sta, tid, ndp);
-}
-
-void iwl_mvm_rs_tx_status(struct iwl_mvm *mvm, struct ieee80211_sta *sta,
-			  int tid, struct ieee80211_tx_info *info, bool ndp)
-{
-	struct iwl_mvm_sta *mvmsta = iwl_mvm_sta_from_mac80211(sta);
-
-	/* If it's locked we are in middle of init flow
-	 * just wait for next tx status to update the lq_sta data
-	 */
-	if (!mutex_trylock(&mvmsta->lq_sta.rs_drv.mutex))
-		return;
-
-	__iwl_mvm_rs_tx_status(mvm, sta, tid, info, ndp);
-	mutex_unlock(&mvmsta->lq_sta.rs_drv.mutex);
-}
-
 /*
  * mac80211 sends us Tx status
  */
@@ -3226,6 +2954,8 @@ static void rs_drv_rate_init(struct iwl_mvm *mvm, struct ieee80211_sta *sta,
 	struct ieee80211_supported_band *sband;
 	unsigned long supp; /* must be unsigned long for for_each_set_bit */
 
+	lockdep_assert_held(&mvmsta->lq_sta.rs_drv.pers.lock);
+
 	/* clear all non-persistent lq data */
 	memset(lq_sta, 0, offsetof(typeof(*lq_sta), pers));
 
@@ -3318,6 +3048,258 @@ static void rs_drv_rate_update(void *mvm_r,
 	iwl_mvm_rs_rate_init(mvm, sta, sband->band, true);
 }
 
+static void __iwl_mvm_rs_tx_status(struct iwl_mvm *mvm,
+				   struct ieee80211_sta *sta,
+				   int tid, struct ieee80211_tx_info *info,
+				   bool ndp)
+{
+	int legacy_success;
+	int retries;
+	int i;
+	struct iwl_lq_cmd *table;
+	u32 lq_hwrate;
+	struct rs_rate lq_rate, tx_resp_rate;
+	struct iwl_scale_tbl_info *curr_tbl, *other_tbl, *tmp_tbl;
+	u32 tlc_info = (uintptr_t)info->status.status_driver_data[0];
+	u8 reduced_txp = tlc_info & RS_DRV_DATA_TXP_MSK;
+	u8 lq_color = RS_DRV_DATA_LQ_COLOR_GET(tlc_info);
+	u32 tx_resp_hwrate = (uintptr_t)info->status.status_driver_data[1];
+	struct iwl_mvm_sta *mvmsta = iwl_mvm_sta_from_mac80211(sta);
+	struct iwl_lq_sta *lq_sta = &mvmsta->lq_sta.rs_drv;
+
+	/* Treat uninitialized rate scaling data same as non-existing. */
+	if (!lq_sta) {
+		IWL_DEBUG_RATE(mvm, "Station rate scaling not created yet.\n");
+		return;
+	} else if (!lq_sta->pers.drv) {
+		IWL_DEBUG_RATE(mvm, "Rate scaling not initialized yet.\n");
+		return;
+	}
+
+	/* This packet was aggregated but doesn't carry status info */
+	if ((info->flags & IEEE80211_TX_CTL_AMPDU) &&
+	    !(info->flags & IEEE80211_TX_STAT_AMPDU))
+		return;
+
+	if (rs_rate_from_ucode_rate(tx_resp_hwrate, info->band,
+				    &tx_resp_rate)) {
+		WARN_ON_ONCE(1);
+		return;
+	}
+
+#ifdef CONFIG_MAC80211_DEBUGFS
+	/* Disable last tx check if we are debugging with fixed rate but
+	 * update tx stats
+	 */
+	if (lq_sta->pers.dbg_fixed_rate) {
+		int index = tx_resp_rate.index;
+		enum rs_column column;
+		int attempts, success;
+
+		column = rs_get_column_from_rate(&tx_resp_rate);
+		if (WARN_ONCE(column == RS_COLUMN_INVALID,
+			      "Can't map rate 0x%x to column",
+			      tx_resp_hwrate))
+			return;
+
+		if (info->flags & IEEE80211_TX_STAT_AMPDU) {
+			attempts = info->status.ampdu_len;
+			success = info->status.ampdu_ack_len;
+		} else {
+			attempts = info->status.rates[0].count;
+			success = !!(info->flags & IEEE80211_TX_STAT_ACK);
+		}
+
+		lq_sta->pers.tx_stats[column][index].total += attempts;
+		lq_sta->pers.tx_stats[column][index].success += success;
+
+		IWL_DEBUG_RATE(mvm, "Fixed rate 0x%x success %d attempts %d\n",
+			       tx_resp_hwrate, success, attempts);
+		return;
+	}
+#endif
+
+	if (time_after(jiffies,
+		       (unsigned long)(lq_sta->last_tx +
+				       (IWL_MVM_RS_IDLE_TIMEOUT * HZ)))) {
+		IWL_DEBUG_RATE(mvm, "Tx idle for too long. reinit rs\n");
+		/* reach here only in case of driver RS, call directly
+		 * the unlocked version
+		 */
+		rs_drv_rate_init(mvm, sta, info->band);
+		return;
+	}
+	lq_sta->last_tx = jiffies;
+
+	/* Ignore this Tx frame response if its initial rate doesn't match
+	 * that of latest Link Quality command.  There may be stragglers
+	 * from a previous Link Quality command, but we're no longer interested
+	 * in those; they're either from the "active" mode while we're trying
+	 * to check "search" mode, or a prior "search" mode after we've moved
+	 * to a new "search" mode (which might become the new "active" mode).
+	 */
+	table = &lq_sta->lq;
+	lq_hwrate = le32_to_cpu(table->rs_table[0]);
+	if (rs_rate_from_ucode_rate(lq_hwrate, info->band, &lq_rate)) {
+		WARN_ON_ONCE(1);
+		return;
+	}
+
+	/* Here we actually compare this rate to the latest LQ command */
+	if (lq_color != LQ_FLAG_COLOR_GET(table->flags)) {
+		IWL_DEBUG_RATE(mvm,
+			       "tx resp color 0x%x does not match 0x%x\n",
+			       lq_color, LQ_FLAG_COLOR_GET(table->flags));
+
+		/* Since rates mis-match, the last LQ command may have failed.
+		 * After IWL_MISSED_RATE_MAX mis-matches, resync the uCode with
+		 * ... driver.
+		 */
+		lq_sta->missed_rate_counter++;
+		if (lq_sta->missed_rate_counter > IWL_MVM_RS_MISSED_RATE_MAX) {
+			lq_sta->missed_rate_counter = 0;
+			IWL_DEBUG_RATE(mvm,
+				       "Too many rates mismatch. Send sync LQ. rs_state %d\n",
+				       lq_sta->rs_state);
+			iwl_mvm_send_lq_cmd(mvm, &lq_sta->lq);
+		}
+		/* Regardless, ignore this status info for outdated rate */
+		return;
+	}
+
+	/* Rate did match, so reset the missed_rate_counter */
+	lq_sta->missed_rate_counter = 0;
+
+	if (!lq_sta->search_better_tbl) {
+		curr_tbl = &lq_sta->lq_info[lq_sta->active_tbl];
+		other_tbl = &lq_sta->lq_info[1 - lq_sta->active_tbl];
+	} else {
+		curr_tbl = &lq_sta->lq_info[1 - lq_sta->active_tbl];
+		other_tbl = &lq_sta->lq_info[lq_sta->active_tbl];
+	}
+
+	if (WARN_ON_ONCE(!rs_rate_column_match(&lq_rate, &curr_tbl->rate))) {
+		IWL_DEBUG_RATE(mvm,
+			       "Neither active nor search matches tx rate\n");
+		tmp_tbl = &lq_sta->lq_info[lq_sta->active_tbl];
+		rs_dump_rate(mvm, &tmp_tbl->rate, "ACTIVE");
+		tmp_tbl = &lq_sta->lq_info[1 - lq_sta->active_tbl];
+		rs_dump_rate(mvm, &tmp_tbl->rate, "SEARCH");
+		rs_dump_rate(mvm, &lq_rate, "ACTUAL");
+
+		/* no matching table found, let's by-pass the data collection
+		 * and continue to perform rate scale to find the rate table
+		 */
+		rs_stay_in_table(lq_sta, true);
+		goto done;
+	}
+
+	/* Updating the frame history depends on whether packets were
+	 * aggregated.
+	 *
+	 * For aggregation, all packets were transmitted at the same rate, the
+	 * first index into rate scale table.
+	 */
+	if (info->flags & IEEE80211_TX_STAT_AMPDU) {
+		rs_collect_tpc_data(mvm, lq_sta, curr_tbl, tx_resp_rate.index,
+				    info->status.ampdu_len,
+				    info->status.ampdu_ack_len,
+				    reduced_txp);
+
+		/* ampdu_ack_len = 0 marks no BA was received. For TLC, treat
+		 * it as a single frame loss as we don't want the success ratio
+		 * to dip too quickly because a BA wasn't received.
+		 * For TPC, there's no need for this optimisation since we want
+		 * to recover very quickly from a bad power reduction and,
+		 * therefore we'd like the success ratio to get an immediate hit
+		 * when failing to get a BA, so we'd switch back to a lower or
+		 * zero power reduction. When FW transmits agg with a rate
+		 * different from the initial rate, it will not use reduced txp
+		 * and will send BA notification twice (one empty with reduced
+		 * txp equal to the value from LQ and one with reduced txp 0).
+		 * We need to update counters for each txp level accordingly.
+		 */
+		if (info->status.ampdu_ack_len == 0)
+			info->status.ampdu_len = 1;
+
+		rs_collect_tlc_data(mvm, mvmsta, tid, curr_tbl,
+				    tx_resp_rate.index,
+				    info->status.ampdu_len,
+				    info->status.ampdu_ack_len);
+
+		/* Update success/fail counts if not searching for new mode */
+		if (lq_sta->rs_state == RS_STATE_STAY_IN_COLUMN) {
+			lq_sta->total_success += info->status.ampdu_ack_len;
+			lq_sta->total_failed += (info->status.ampdu_len -
+					info->status.ampdu_ack_len);
+		}
+	} else {
+		/* For legacy, update frame history with for each Tx retry. */
+		retries = info->status.rates[0].count - 1;
+		/* HW doesn't send more than 15 retries */
+		retries = min(retries, 15);
+
+		/* The last transmission may have been successful */
+		legacy_success = !!(info->flags & IEEE80211_TX_STAT_ACK);
+		/* Collect data for each rate used during failed TX attempts */
+		for (i = 0; i <= retries; ++i) {
+			lq_hwrate = le32_to_cpu(table->rs_table[i]);
+			if (rs_rate_from_ucode_rate(lq_hwrate, info->band,
+						    &lq_rate)) {
+				WARN_ON_ONCE(1);
+				return;
+			}
+
+			/* Only collect stats if retried rate is in the same RS
+			 * table as active/search.
+			 */
+			if (rs_rate_column_match(&lq_rate, &curr_tbl->rate))
+				tmp_tbl = curr_tbl;
+			else if (rs_rate_column_match(&lq_rate,
+						      &other_tbl->rate))
+				tmp_tbl = other_tbl;
+			else
+				continue;
+
+			rs_collect_tpc_data(mvm, lq_sta, tmp_tbl,
+					    tx_resp_rate.index, 1,
+					    i < retries ? 0 : legacy_success,
+					    reduced_txp);
+			rs_collect_tlc_data(mvm, mvmsta, tid, tmp_tbl,
+					    tx_resp_rate.index, 1,
+					    i < retries ? 0 : legacy_success);
+		}
+
+		/* Update success/fail counts if not searching for new mode */
+		if (lq_sta->rs_state == RS_STATE_STAY_IN_COLUMN) {
+			lq_sta->total_success += legacy_success;
+			lq_sta->total_failed += retries + (1 - legacy_success);
+		}
+	}
+	/* The last TX rate is cached in lq_sta; it's set in if/else above */
+	lq_sta->last_rate_n_flags = lq_hwrate;
+	IWL_DEBUG_RATE(mvm, "reduced txpower: %d\n", reduced_txp);
+done:
+	/* See if there's a better rate or modulation mode to try. */
+	if (sta->supp_rates[info->band])
+		rs_rate_scale_perform(mvm, sta, lq_sta, tid, ndp);
+}
+
+void iwl_mvm_rs_tx_status(struct iwl_mvm *mvm, struct ieee80211_sta *sta,
+			  int tid, struct ieee80211_tx_info *info, bool ndp)
+{
+	struct iwl_mvm_sta *mvmsta = iwl_mvm_sta_from_mac80211(sta);
+
+	/* If it's locked we are in middle of init flow
+	 * just wait for next tx status to update the lq_sta data
+	 */
+	if (!spin_trylock(&mvmsta->lq_sta.rs_drv.pers.lock))
+		return;
+
+	__iwl_mvm_rs_tx_status(mvm, sta, tid, info, ndp);
+	spin_unlock(&mvmsta->lq_sta.rs_drv.pers.lock);
+}
+
 #ifdef CONFIG_MAC80211_DEBUGFS
 static void rs_build_rates_table_from_fixed(struct iwl_mvm *mvm,
 					    struct iwl_lq_cmd *lq_cmd,
@@ -4177,9 +4159,9 @@ void iwl_mvm_rs_rate_init(struct iwl_mvm *mvm, struct ieee80211_sta *sta,
 	} else {
 		struct iwl_mvm_sta *mvmsta = iwl_mvm_sta_from_mac80211(sta);
 
-		mutex_lock(&mvmsta->lq_sta.rs_drv.mutex);
+		spin_lock(&mvmsta->lq_sta.rs_drv.pers.lock);
 		rs_drv_rate_init(mvm, sta, band);
-		mutex_unlock(&mvmsta->lq_sta.rs_drv.mutex);
+		spin_unlock(&mvmsta->lq_sta.rs_drv.pers.lock);
 	}
 }
 
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/rs.h b/drivers/net/wireless/intel/iwlwifi/mvm/rs.h
index 086f47e2a4f0..428642e66658 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/rs.h
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/rs.h
@@ -376,9 +376,6 @@ struct iwl_lq_sta {
 	/* tx power reduce for this sta */
 	int tpc_reduce;
 
-	/* avoid races of reinit and update table from rx_tx */
-	struct mutex mutex;
-
 	/* persistent fields - initialized only once - keep last! */
 	struct lq_sta_pers {
 #ifdef CONFIG_MAC80211_DEBUGFS
@@ -393,6 +390,7 @@ struct iwl_lq_sta {
 		s8 last_rssi;
 		struct rs_rate_stats tx_stats[RS_COLUMN_COUNT][IWL_RATE_COUNT];
 		struct iwl_mvm *drv;
+		spinlock_t lock; /* for races in reinit/update table */
 	} pers;
 };
 
@@ -443,8 +441,6 @@ struct iwl_mvm_sta;
 int iwl_mvm_tx_protection(struct iwl_mvm *mvm, struct iwl_mvm_sta *mvmsta,
 			  bool enable);
 
-void iwl_mvm_rs_init_wk(struct work_struct *wk);
-
 #ifdef CONFIG_IWLWIFI_DEBUGFS
 void iwl_mvm_reset_frame_stats(struct iwl_mvm *mvm);
 #endif
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/sta.c b/drivers/net/wireless/intel/iwlwifi/mvm/sta.c
index 88d16b5442e7..10f18536dd0d 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/sta.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/sta.c
@@ -1685,9 +1685,7 @@ int iwl_mvm_add_sta(struct iwl_mvm *mvm,
 	if (iwl_mvm_has_tlc_offload(mvm))
 		iwl_mvm_rs_add_sta(mvm, mvm_sta);
 	else
-		mutex_init(&mvm_sta->lq_sta.rs_drv.mutex);
-
-	INIT_WORK(&mvm_sta->rs_init_wk, iwl_mvm_rs_init_wk);
+		spin_lock_init(&mvm_sta->lq_sta.rs_drv.pers.lock);
 
 	iwl_mvm_toggle_tx_ant(mvm, &mvm_sta->tx_ant);
 
@@ -1850,8 +1848,6 @@ int iwl_mvm_rm_sta(struct iwl_mvm *mvm,
 	if (ret)
 		return ret;
 
-	cancel_work_sync(&mvm_sta->rs_init_wk);
-
 	/* flush its queues here since we are freeing mvm_sta */
 	ret = iwl_mvm_flush_sta(mvm, mvm_sta, false, 0);
 	if (ret)
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/sta.h b/drivers/net/wireless/intel/iwlwifi/mvm/sta.h
index 4823c06e6909..8d70093847cb 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/sta.h
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/sta.h
@@ -432,7 +432,6 @@ struct iwl_mvm_sta {
 		struct iwl_lq_sta_rs_fw rs_fw;
 		struct iwl_lq_sta rs_drv;
 	} lq_sta;
-	struct work_struct rs_init_wk;
 	struct ieee80211_vif *vif;
 	struct iwl_mvm_key_pn __rcu *ptk_pn[4];
 	struct iwl_mvm_rxq_dup_data *dup_data;
-- 
2.20.1


^ permalink raw reply related

* [PATCH 16/16] iwlwifi: mvm: fix version check for GEO_TX_POWER_LIMIT support
From: Luca Coelho @ 2019-07-20 10:25 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless, Luca Coelho
In-Reply-To: <20190720102545.5952-1-luca@coelho.fi>

From: Luca Coelho <luciano.coelho@intel.com>

We erroneously added a check for FW API version 41 before sending
GEO_TX_POWER_LIMIT, but this was already implemented in version 38.
Additionally, it was cherry-picked to older versions, namely 17, 26
and 29, so check for those as well.

Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 drivers/net/wireless/intel/iwlwifi/mvm/fw.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/fw.c b/drivers/net/wireless/intel/iwlwifi/mvm/fw.c
index a837cf40afde..00c89bcfdf6a 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/fw.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/fw.c
@@ -886,9 +886,14 @@ static bool iwl_mvm_sar_geo_support(struct iwl_mvm *mvm)
 	 * The GEO_TX_POWER_LIMIT command is not supported on earlier
 	 * firmware versions.  Unfortunately, we don't have a TLV API
 	 * flag to rely on, so rely on the major version which is in
-	 * the first byte of ucode_ver.
+	 * the first byte of ucode_ver.  This was implemented
+	 * initially on version 38 and then backported to 36, 29 and
+	 * 17.
 	 */
-	return IWL_UCODE_SERIAL(mvm->fw->ucode_ver) >= 41;
+	return IWL_UCODE_SERIAL(mvm->fw->ucode_ver) >= 38 ||
+	       IWL_UCODE_SERIAL(mvm->fw->ucode_ver) == 36 ||
+	       IWL_UCODE_SERIAL(mvm->fw->ucode_ver) == 29 ||
+	       IWL_UCODE_SERIAL(mvm->fw->ucode_ver) == 17;
 }
 
 int iwl_mvm_get_sar_geo_profile(struct iwl_mvm *mvm)
-- 
2.20.1


^ permalink raw reply related

* [PATCH 15/16] iwlwifi: add 3 new IDs for the 9000 series (iwl9260_2ac_160_cfg)
From: Luca Coelho @ 2019-07-20 10:25 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless, Ihab Zhaika, Luca Coelho
In-Reply-To: <20190720102545.5952-1-luca@coelho.fi>

From: Ihab Zhaika <ihab.zhaika@intel.com>

Add a few PCI ID'S for 9000 series.

Signed-off-by: Ihab Zhaika <ihab.zhaika@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 drivers/net/wireless/intel/iwlwifi/pcie/drv.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/drv.c b/drivers/net/wireless/intel/iwlwifi/pcie/drv.c
index ea2a03d4bf55..de711c1160d3 100644
--- a/drivers/net/wireless/intel/iwlwifi/pcie/drv.c
+++ b/drivers/net/wireless/intel/iwlwifi/pcie/drv.c
@@ -604,10 +604,13 @@ static const struct pci_device_id iwl_hw_card_ids[] = {
 	{IWL_PCI_DEVICE(0x2526, 0x40A4, iwl9460_2ac_cfg)},
 	{IWL_PCI_DEVICE(0x2526, 0x4234, iwl9560_2ac_cfg_soc)},
 	{IWL_PCI_DEVICE(0x2526, 0x42A4, iwl9462_2ac_cfg_soc)},
+	{IWL_PCI_DEVICE(0x2526, 0x6010, iwl9260_2ac_160_cfg)},
 	{IWL_PCI_DEVICE(0x2526, 0x6014, iwl9260_2ac_160_cfg)},
 	{IWL_PCI_DEVICE(0x2526, 0x8014, iwl9260_2ac_160_cfg)},
 	{IWL_PCI_DEVICE(0x2526, 0x8010, iwl9260_2ac_160_cfg)},
 	{IWL_PCI_DEVICE(0x2526, 0xA014, iwl9260_2ac_160_cfg)},
+	{IWL_PCI_DEVICE(0x2526, 0xE010, iwl9260_2ac_160_cfg)},
+	{IWL_PCI_DEVICE(0x2526, 0xE014, iwl9260_2ac_160_cfg)},
 	{IWL_PCI_DEVICE(0x271B, 0x0010, iwl9160_2ac_cfg)},
 	{IWL_PCI_DEVICE(0x271B, 0x0014, iwl9160_2ac_cfg)},
 	{IWL_PCI_DEVICE(0x271B, 0x0210, iwl9160_2ac_cfg)},
-- 
2.20.1


^ permalink raw reply related

* [PATCH 11/16] iwlwifi: mvm: send LQ command always ASYNC
From: Luca Coelho @ 2019-07-20 10:25 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless, Gregory Greenman, Luca Coelho
In-Reply-To: <20190720102545.5952-1-luca@coelho.fi>

From: Gregory Greenman <gregory.greenman@intel.com>

The only place where the command was sent as SYNC is during
init and this is not really critical. This change is required
for replacing RS mutex with a spinlock (in the subsequent patch),
since SYNC comamnd requres sleeping and thus the flow cannot
be done when holding a spinlock.

Signed-off-by: Gregory Greenman <gregory.greenman@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 drivers/net/wireless/intel/iwlwifi/mvm/mvm.h  |  2 +-
 drivers/net/wireless/intel/iwlwifi/mvm/rs.c   | 23 ++++++++++---------
 drivers/net/wireless/intel/iwlwifi/mvm/sta.c  |  2 +-
 .../net/wireless/intel/iwlwifi/mvm/utils.c    |  4 ++--
 4 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h b/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h
index fd1764df592f..a263cc629d75 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h
@@ -1813,7 +1813,7 @@ iwl_mvm_vif_dbgfs_clean(struct iwl_mvm *mvm, struct ieee80211_vif *vif)
 #endif /* CONFIG_IWLWIFI_DEBUGFS */
 
 /* rate scaling */
-int iwl_mvm_send_lq_cmd(struct iwl_mvm *mvm, struct iwl_lq_cmd *lq, bool sync);
+int iwl_mvm_send_lq_cmd(struct iwl_mvm *mvm, struct iwl_lq_cmd *lq);
 void iwl_mvm_update_frame_stats(struct iwl_mvm *mvm, u32 rate, bool agg);
 int rs_pretty_print_rate(char *buf, int bufsz, const u32 rate);
 void rs_update_last_rssi(struct iwl_mvm *mvm,
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/rs.c b/drivers/net/wireless/intel/iwlwifi/mvm/rs.c
index b50a47e86ef0..bd977ec8629b 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/rs.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/rs.c
@@ -1328,7 +1328,7 @@ static void __iwl_mvm_rs_tx_status(struct iwl_mvm *mvm,
 			IWL_DEBUG_RATE(mvm,
 				       "Too many rates mismatch. Send sync LQ. rs_state %d\n",
 				       lq_sta->rs_state);
-			iwl_mvm_send_lq_cmd(mvm, &lq_sta->lq, false);
+			iwl_mvm_send_lq_cmd(mvm, &lq_sta->lq);
 		}
 		/* Regardless, ignore this status info for outdated rate */
 		return;
@@ -1390,7 +1390,8 @@ static void __iwl_mvm_rs_tx_status(struct iwl_mvm *mvm,
 		if (info->status.ampdu_ack_len == 0)
 			info->status.ampdu_len = 1;
 
-		rs_collect_tlc_data(mvm, mvmsta, tid, curr_tbl, tx_resp_rate.index,
+		rs_collect_tlc_data(mvm, mvmsta, tid, curr_tbl,
+				    tx_resp_rate.index,
 				    info->status.ampdu_len,
 				    info->status.ampdu_ack_len);
 
@@ -1833,7 +1834,7 @@ static void rs_update_rate_tbl(struct iwl_mvm *mvm,
 			       struct iwl_scale_tbl_info *tbl)
 {
 	rs_fill_lq_cmd(mvm, sta, lq_sta, &tbl->rate);
-	iwl_mvm_send_lq_cmd(mvm, &lq_sta->lq, false);
+	iwl_mvm_send_lq_cmd(mvm, &lq_sta->lq);
 }
 
 static bool rs_tweak_rate_tbl(struct iwl_mvm *mvm,
@@ -2935,7 +2936,7 @@ void rs_update_last_rssi(struct iwl_mvm *mvm,
 static void rs_initialize_lq(struct iwl_mvm *mvm,
 			     struct ieee80211_sta *sta,
 			     struct iwl_lq_sta *lq_sta,
-			     enum nl80211_band band, bool update)
+			     enum nl80211_band band)
 {
 	struct iwl_scale_tbl_info *tbl;
 	struct rs_rate *rate;
@@ -2965,7 +2966,7 @@ static void rs_initialize_lq(struct iwl_mvm *mvm,
 	rs_set_expected_tpt_table(lq_sta, tbl);
 	rs_fill_lq_cmd(mvm, sta, lq_sta, rate);
 	/* TODO restore station should remember the lq cmd */
-	iwl_mvm_send_lq_cmd(mvm, &lq_sta->lq, !update);
+	iwl_mvm_send_lq_cmd(mvm, &lq_sta->lq);
 }
 
 static void rs_drv_get_rate(void *mvm_r, struct ieee80211_sta *sta,
@@ -3214,7 +3215,7 @@ void iwl_mvm_update_frame_stats(struct iwl_mvm *mvm, u32 rate, bool agg)
  * Called after adding a new station to initialize rate scaling
  */
 static void rs_drv_rate_init(struct iwl_mvm *mvm, struct ieee80211_sta *sta,
-			     enum nl80211_band band, bool update)
+			     enum nl80211_band band)
 {
 	int i, j;
 	struct ieee80211_hw *hw = mvm->hw;
@@ -3294,7 +3295,7 @@ static void rs_drv_rate_init(struct iwl_mvm *mvm, struct ieee80211_sta *sta,
 #ifdef CONFIG_IWLWIFI_DEBUGFS
 	iwl_mvm_reset_frame_stats(mvm);
 #endif
-	rs_initialize_lq(mvm, sta, lq_sta, band, update);
+	rs_initialize_lq(mvm, sta, lq_sta, band);
 }
 
 static void rs_drv_rate_update(void *mvm_r,
@@ -3608,7 +3609,7 @@ static void rs_set_lq_ss_params(struct iwl_mvm *mvm,
 
 		bfersta_ss_params &= ~LQ_SS_BFER_ALLOWED;
 		bfersta_lq_cmd->ss_params = cpu_to_le32(bfersta_ss_params);
-		iwl_mvm_send_lq_cmd(mvm, bfersta_lq_cmd, false);
+		iwl_mvm_send_lq_cmd(mvm, bfersta_lq_cmd);
 
 		ss_params |= LQ_SS_BFER_ALLOWED;
 		IWL_DEBUG_RATE(mvm,
@@ -3774,7 +3775,7 @@ static void rs_program_fix_rate(struct iwl_mvm *mvm,
 
 	if (lq_sta->pers.dbg_fixed_rate) {
 		rs_fill_lq_cmd(mvm, NULL, lq_sta, NULL);
-		iwl_mvm_send_lq_cmd(lq_sta->pers.drv, &lq_sta->lq, false);
+		iwl_mvm_send_lq_cmd(lq_sta->pers.drv, &lq_sta->lq);
 	}
 }
 
@@ -4177,7 +4178,7 @@ void iwl_mvm_rs_rate_init(struct iwl_mvm *mvm, struct ieee80211_sta *sta,
 		struct iwl_mvm_sta *mvmsta = iwl_mvm_sta_from_mac80211(sta);
 
 		mutex_lock(&mvmsta->lq_sta.rs_drv.mutex);
-		rs_drv_rate_init(mvm, sta, band, update);
+		rs_drv_rate_init(mvm, sta, band);
 		mutex_unlock(&mvmsta->lq_sta.rs_drv.mutex);
 	}
 }
@@ -4209,7 +4210,7 @@ static int rs_drv_tx_protection(struct iwl_mvm *mvm, struct iwl_mvm_sta *mvmsta,
 			lq->flags &= ~LQ_FLAG_USE_RTS_MSK;
 	}
 
-	return iwl_mvm_send_lq_cmd(mvm, lq, false);
+	return iwl_mvm_send_lq_cmd(mvm, lq);
 }
 
 /**
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/sta.c b/drivers/net/wireless/intel/iwlwifi/mvm/sta.c
index 23fd3108adb9..88d16b5442e7 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/sta.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/sta.c
@@ -2978,7 +2978,7 @@ int iwl_mvm_sta_tx_agg_oper(struct iwl_mvm *mvm, struct ieee80211_vif *vif,
 	IWL_DEBUG_HT(mvm, "Tx aggregation enabled on ra = %pM tid = %d\n",
 		     sta->addr, tid);
 
-	return iwl_mvm_send_lq_cmd(mvm, &mvmsta->lq_sta.rs_drv.lq, false);
+	return iwl_mvm_send_lq_cmd(mvm, &mvmsta->lq_sta.rs_drv.lq);
 }
 
 static void iwl_mvm_unreserve_agg_queue(struct iwl_mvm *mvm,
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/utils.c b/drivers/net/wireless/intel/iwlwifi/mvm/utils.c
index 9ecd5f09615a..b8e20a01c192 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/utils.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/utils.c
@@ -653,12 +653,12 @@ int iwl_mvm_reconfig_scd(struct iwl_mvm *mvm, int queue, int fifo, int sta_id,
  * this case to clear the state indicating that station creation is in
  * progress.
  */
-int iwl_mvm_send_lq_cmd(struct iwl_mvm *mvm, struct iwl_lq_cmd *lq, bool sync)
+int iwl_mvm_send_lq_cmd(struct iwl_mvm *mvm, struct iwl_lq_cmd *lq)
 {
 	struct iwl_host_cmd cmd = {
 		.id = LQ_CMD,
 		.len = { sizeof(struct iwl_lq_cmd), },
-		.flags = sync ? 0 : CMD_ASYNC,
+		.flags = CMD_ASYNC,
 		.data = { lq, },
 	};
 
-- 
2.20.1


^ permalink raw reply related

* [PATCH 10/16] iwlwifi: mvm: fix comparison of u32 variable with less than zero
From: Luca Coelho @ 2019-07-20 10:25 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless, Colin Ian King, Luca Coelho
In-Reply-To: <20190720102545.5952-1-luca@coelho.fi>

From: Colin Ian King <colin.king@canonical.com>

The comparison of the u32 variable wgds_tbl_idx with less than zero is
always going to be false because it is unsigned.  Fix this by making
wgds_tbl_idx a plain signed int.

Addresses-Coverity: ("Unsigned compared against 0")
Fixes: 4fd445a2c855 ("iwlwifi: mvm: Add log information about SAR status")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 drivers/net/wireless/intel/iwlwifi/mvm/nvm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/nvm.c b/drivers/net/wireless/intel/iwlwifi/mvm/nvm.c
index 719f793b3487..a9bb43a2f27b 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/nvm.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/nvm.c
@@ -620,7 +620,7 @@ void iwl_mvm_rx_chub_update_mcc(struct iwl_mvm *mvm,
 	enum iwl_mcc_source src;
 	char mcc[3];
 	struct ieee80211_regdomain *regd;
-	u32 wgds_tbl_idx;
+	int wgds_tbl_idx;
 
 	lockdep_assert_held(&mvm->mutex);
 
-- 
2.20.1


^ permalink raw reply related

* [PATCH 14/16] iwlwifi: mvm: fix possible out-of-bounds read when accessing lq_info
From: Luca Coelho @ 2019-07-20 10:25 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless, Gregory Greenman, Luca Coelho
In-Reply-To: <20190720102545.5952-1-luca@coelho.fi>

From: Gregory Greenman <gregory.greenman@intel.com>

lq_info is an arary of size 2, active_tbl index is u8.
When accessing lq_info[1 - active_tbl], theoretically it's possible
that the access will be made to a negative index value.

Signed-off-by: Gregory Greenman <gregory.greenman@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 drivers/net/wireless/intel/iwlwifi/mvm/rs.c | 28 +++++++++++++++------
 1 file changed, 20 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/rs.c b/drivers/net/wireless/intel/iwlwifi/mvm/rs.c
index 3fa50b1955bb..d3f04acfbacb 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/rs.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/rs.c
@@ -1352,6 +1352,18 @@ static void rs_set_expected_tpt_table(struct iwl_lq_sta *lq_sta,
 	tbl->expected_tpt = rs_get_expected_tpt_table(lq_sta, column, rate->bw);
 }
 
+/* rs uses two tables, one is active and the second is for searching better
+ * configuration. This function, according to the index of the currently
+ * active table returns the search table, which is located at the
+ * index complementary to 1 according to the active table (active = 1,
+ * search = 0 or active = 0, search = 1).
+ * Since lq_info is an arary of size 2, make sure index cannot be out of bounds.
+ */
+static inline u8 rs_search_tbl(u8 active_tbl)
+{
+	return (active_tbl ^ 1) & 1;
+}
+
 static s32 rs_get_best_rate(struct iwl_mvm *mvm,
 			    struct iwl_lq_sta *lq_sta,
 			    struct iwl_scale_tbl_info *tbl,	/* "search" */
@@ -1699,9 +1711,9 @@ static int rs_switch_to_column(struct iwl_mvm *mvm,
 			       struct ieee80211_sta *sta,
 			       enum rs_column col_id)
 {
-	struct iwl_scale_tbl_info *tbl = &(lq_sta->lq_info[lq_sta->active_tbl]);
+	struct iwl_scale_tbl_info *tbl = &lq_sta->lq_info[lq_sta->active_tbl];
 	struct iwl_scale_tbl_info *search_tbl =
-				&(lq_sta->lq_info[(1 - lq_sta->active_tbl)]);
+		&lq_sta->lq_info[rs_search_tbl(lq_sta->active_tbl)];
 	struct rs_rate *rate = &search_tbl->rate;
 	const struct rs_tx_column *column = &rs_tx_columns[col_id];
 	const struct rs_tx_column *curr_column = &rs_tx_columns[tbl->column];
@@ -2109,7 +2121,7 @@ static void rs_rate_scale_perform(struct iwl_mvm *mvm,
 	if (!lq_sta->search_better_tbl)
 		active_tbl = lq_sta->active_tbl;
 	else
-		active_tbl = 1 - lq_sta->active_tbl;
+		active_tbl = rs_search_tbl(lq_sta->active_tbl);
 
 	tbl = &(lq_sta->lq_info[active_tbl]);
 	rate = &tbl->rate;
@@ -2333,7 +2345,7 @@ static void rs_rate_scale_perform(struct iwl_mvm *mvm,
 		/* If new "search" mode was selected, set up in uCode table */
 		if (lq_sta->search_better_tbl) {
 			/* Access the "search" table, clear its history. */
-			tbl = &(lq_sta->lq_info[(1 - lq_sta->active_tbl)]);
+			tbl = &lq_sta->lq_info[rs_search_tbl(lq_sta->active_tbl)];
 			rs_rate_scale_clear_tbl_windows(mvm, tbl);
 
 			/* Use new "search" start rate */
@@ -2676,7 +2688,7 @@ static void rs_initialize_lq(struct iwl_mvm *mvm,
 	if (!lq_sta->search_better_tbl)
 		active_tbl = lq_sta->active_tbl;
 	else
-		active_tbl = 1 - lq_sta->active_tbl;
+		active_tbl = rs_search_tbl(lq_sta->active_tbl);
 
 	tbl = &(lq_sta->lq_info[active_tbl]);
 	rate = &tbl->rate;
@@ -3172,9 +3184,9 @@ static void __iwl_mvm_rs_tx_status(struct iwl_mvm *mvm,
 
 	if (!lq_sta->search_better_tbl) {
 		curr_tbl = &lq_sta->lq_info[lq_sta->active_tbl];
-		other_tbl = &lq_sta->lq_info[1 - lq_sta->active_tbl];
+		other_tbl = &lq_sta->lq_info[rs_search_tbl(lq_sta->active_tbl)];
 	} else {
-		curr_tbl = &lq_sta->lq_info[1 - lq_sta->active_tbl];
+		curr_tbl = &lq_sta->lq_info[rs_search_tbl(lq_sta->active_tbl)];
 		other_tbl = &lq_sta->lq_info[lq_sta->active_tbl];
 	}
 
@@ -3183,7 +3195,7 @@ static void __iwl_mvm_rs_tx_status(struct iwl_mvm *mvm,
 			       "Neither active nor search matches tx rate\n");
 		tmp_tbl = &lq_sta->lq_info[lq_sta->active_tbl];
 		rs_dump_rate(mvm, &tmp_tbl->rate, "ACTIVE");
-		tmp_tbl = &lq_sta->lq_info[1 - lq_sta->active_tbl];
+		tmp_tbl = &lq_sta->lq_info[rs_search_tbl(lq_sta->active_tbl)];
 		rs_dump_rate(mvm, &tmp_tbl->rate, "SEARCH");
 		rs_dump_rate(mvm, &lq_rate, "ACTUAL");
 
-- 
2.20.1


^ permalink raw reply related

* [PATCH 09/16] iwlwifi: fix locking in delayed GTK setting
From: Luca Coelho @ 2019-07-20 10:25 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless, Johannes Berg, Luca Coelho
In-Reply-To: <20190720102545.5952-1-luca@coelho.fi>

From: Johannes Berg <johannes.berg@intel.com>

This code clearly never could have worked, since it locks
while already locked. Add an unlocked __iwl_mvm_mac_set_key()
variant that doesn't do locking to fix that.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 .../net/wireless/intel/iwlwifi/mvm/mac80211.c | 39 ++++++++++++-------
 1 file changed, 26 insertions(+), 13 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c b/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c
index 982682ec74fd..1c904b5226aa 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c
@@ -207,11 +207,11 @@ static const struct cfg80211_pmsr_capabilities iwl_mvm_pmsr_capa = {
 	},
 };
 
-static int iwl_mvm_mac_set_key(struct ieee80211_hw *hw,
-			       enum set_key_cmd cmd,
-			       struct ieee80211_vif *vif,
-			       struct ieee80211_sta *sta,
-			       struct ieee80211_key_conf *key);
+static int __iwl_mvm_mac_set_key(struct ieee80211_hw *hw,
+				 enum set_key_cmd cmd,
+				 struct ieee80211_vif *vif,
+				 struct ieee80211_sta *sta,
+				 struct ieee80211_key_conf *key);
 
 void iwl_mvm_ref(struct iwl_mvm *mvm, enum iwl_mvm_ref_type ref_type)
 {
@@ -2738,7 +2738,7 @@ static int iwl_mvm_start_ap_ibss(struct ieee80211_hw *hw,
 
 		mvmvif->ap_early_keys[i] = NULL;
 
-		ret = iwl_mvm_mac_set_key(hw, SET_KEY, vif, NULL, key);
+		ret = __iwl_mvm_mac_set_key(hw, SET_KEY, vif, NULL, key);
 		if (ret)
 			goto out_quota_failed;
 	}
@@ -3506,11 +3506,11 @@ static int iwl_mvm_mac_sched_scan_stop(struct ieee80211_hw *hw,
 	return ret;
 }
 
-static int iwl_mvm_mac_set_key(struct ieee80211_hw *hw,
-			       enum set_key_cmd cmd,
-			       struct ieee80211_vif *vif,
-			       struct ieee80211_sta *sta,
-			       struct ieee80211_key_conf *key)
+static int __iwl_mvm_mac_set_key(struct ieee80211_hw *hw,
+				 enum set_key_cmd cmd,
+				 struct ieee80211_vif *vif,
+				 struct ieee80211_sta *sta,
+				 struct ieee80211_key_conf *key)
 {
 	struct iwl_mvm_vif *mvmvif = iwl_mvm_vif_from_mac80211(vif);
 	struct iwl_mvm *mvm = IWL_MAC80211_GET_MVM(hw);
@@ -3565,8 +3565,6 @@ static int iwl_mvm_mac_set_key(struct ieee80211_hw *hw,
 			return -EOPNOTSUPP;
 	}
 
-	mutex_lock(&mvm->mutex);
-
 	switch (cmd) {
 	case SET_KEY:
 		if ((vif->type == NL80211_IFTYPE_ADHOC ||
@@ -3712,7 +3710,22 @@ static int iwl_mvm_mac_set_key(struct ieee80211_hw *hw,
 		ret = -EINVAL;
 	}
 
+	return ret;
+}
+
+static int iwl_mvm_mac_set_key(struct ieee80211_hw *hw,
+			       enum set_key_cmd cmd,
+			       struct ieee80211_vif *vif,
+			       struct ieee80211_sta *sta,
+			       struct ieee80211_key_conf *key)
+{
+	struct iwl_mvm *mvm = IWL_MAC80211_GET_MVM(hw);
+	int ret;
+
+	mutex_lock(&mvm->mutex);
+	ret = __iwl_mvm_mac_set_key(hw, cmd, vif, sta, key);
 	mutex_unlock(&mvm->mutex);
+
 	return ret;
 }
 
-- 
2.20.1


^ permalink raw reply related

* [PATCH 08/16] iwlwifi: dbg_ini: move iwl_dbg_tlv_free outside of debugfs ifdef
From: Luca Coelho @ 2019-07-20 10:25 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless, Shahar S Matityahu, Luca Coelho
In-Reply-To: <20190720102545.5952-1-luca@coelho.fi>

From: Shahar S Matityahu <shahar.s.matityahu@intel.com>

The driver should call iwl_dbg_tlv_free even if debugfs is not defined
since ini mode does not depend on debugfs ifdef.

Signed-off-by: Shahar S Matityahu <shahar.s.matityahu@intel.com>
Fixes: 68f6f492c4fa ("iwlwifi: trans: support loading ini TLVs from external file")
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 drivers/net/wireless/intel/iwlwifi/iwl-drv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
index d91686b9a540..38672dd5aae9 100644
--- a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
+++ b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
@@ -1662,8 +1662,8 @@ struct iwl_drv *iwl_drv_start(struct iwl_trans *trans)
 err_fw:
 #ifdef CONFIG_IWLWIFI_DEBUGFS
 	debugfs_remove_recursive(drv->dbgfs_drv);
-	iwl_fw_dbg_free(drv->trans);
 #endif
+	iwl_fw_dbg_free(drv->trans);
 	kfree(drv);
 err:
 	return ERR_PTR(ret);
-- 
2.20.1


^ permalink raw reply related

* [PATCH 07/16] iwlwifi: dbg_ini: move iwl_dbg_tlv_load_bin out of debug override ifdef
From: Luca Coelho @ 2019-07-20 10:25 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless, Shahar S Matityahu, Luca Coelho
In-Reply-To: <20190720102545.5952-1-luca@coelho.fi>

From: Shahar S Matityahu <shahar.s.matityahu@intel.com>

ini debug mode should work even if debug override is not defined.

Signed-off-by: Shahar S Matityahu <shahar.s.matityahu@intel.com>
Fixes: 68f6f492c4fa ("iwlwifi: trans: support loading ini TLVs from external file")
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 drivers/net/wireless/intel/iwlwifi/iwl-drv.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
index 57d09049e615..d91686b9a540 100644
--- a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
+++ b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
@@ -1640,6 +1640,8 @@ struct iwl_drv *iwl_drv_start(struct iwl_trans *trans)
 	init_completion(&drv->request_firmware_complete);
 	INIT_LIST_HEAD(&drv->list);
 
+	iwl_load_fw_dbg_tlv(drv->trans->dev, drv->trans);
+
 #ifdef CONFIG_IWLWIFI_DEBUGFS
 	/* Create the device debugfs entries. */
 	drv->dbgfs_drv = debugfs_create_dir(dev_name(trans->dev),
-- 
2.20.1


^ permalink raw reply related

* [PATCH 06/16] iwlwifi: mvm: add a wrapper around rs_tx_status to handle locks
From: Luca Coelho @ 2019-07-20 10:25 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless, Gregory Greenman, Luca Coelho
In-Reply-To: <20190720102545.5952-1-luca@coelho.fi>

From: Gregory Greenman <gregory.greenman@intel.com>

iwl_mvm_rs_tx_status can be called from two places in the code, but the
mutex is taken only on one of the calls. Split it into a wrapper taking
locks and an internal __iwl_mvm_rs_tx_status function.

Signed-off-by: Gregory Greenman <gregory.greenman@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 drivers/net/wireless/intel/iwlwifi/mvm/rs.c | 28 ++++++++++++++-------
 1 file changed, 19 insertions(+), 9 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/rs.c b/drivers/net/wireless/intel/iwlwifi/mvm/rs.c
index 02b4ef92543f..b50a47e86ef0 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/rs.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/rs.c
@@ -1218,8 +1218,10 @@ void iwl_mvm_rs_init_wk(struct work_struct *wk)
 	rcu_read_unlock();
 }
 
-void iwl_mvm_rs_tx_status(struct iwl_mvm *mvm, struct ieee80211_sta *sta,
-			  int tid, struct ieee80211_tx_info *info, bool ndp)
+static void __iwl_mvm_rs_tx_status(struct iwl_mvm *mvm,
+				   struct ieee80211_sta *sta,
+				   int tid, struct ieee80211_tx_info *info,
+				   bool ndp)
 {
 	int legacy_success;
 	int retries;
@@ -1451,6 +1453,21 @@ void iwl_mvm_rs_tx_status(struct iwl_mvm *mvm, struct ieee80211_sta *sta,
 		rs_rate_scale_perform(mvm, sta, lq_sta, tid, ndp);
 }
 
+void iwl_mvm_rs_tx_status(struct iwl_mvm *mvm, struct ieee80211_sta *sta,
+			  int tid, struct ieee80211_tx_info *info, bool ndp)
+{
+	struct iwl_mvm_sta *mvmsta = iwl_mvm_sta_from_mac80211(sta);
+
+	/* If it's locked we are in middle of init flow
+	 * just wait for next tx status to update the lq_sta data
+	 */
+	if (!mutex_trylock(&mvmsta->lq_sta.rs_drv.mutex))
+		return;
+
+	__iwl_mvm_rs_tx_status(mvm, sta, tid, info, ndp);
+	mutex_unlock(&mvmsta->lq_sta.rs_drv.mutex);
+}
+
 /*
  * mac80211 sends us Tx status
  */
@@ -1472,15 +1489,8 @@ static void rs_drv_mac80211_tx_status(void *mvm_r,
 	    info->flags & IEEE80211_TX_CTL_NO_ACK)
 		return;
 
-	/* If it's locked we are in middle of init flow
-	 * just wait for next tx status to update the lq_sta data
-	 */
-	if (!mutex_trylock(&mvmsta->lq_sta.rs_drv.mutex))
-		return;
-
 	iwl_mvm_rs_tx_status(mvm, sta, rs_get_tid(hdr), info,
 			     ieee80211_is_qos_nullfunc(hdr->frame_control));
-	mutex_unlock(&mvmsta->lq_sta.rs_drv.mutex);
 }
 
 /*
-- 
2.20.1


^ permalink raw reply related

* [PATCH 05/16] iwlwifi: mvm: add a loose synchronization of the NSSN across Rx queues
From: Luca Coelho @ 2019-07-20 10:25 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless, Emmanuel Grumbach, Luca Coelho
In-Reply-To: <20190720102545.5952-1-luca@coelho.fi>

From: Emmanuel Grumbach <emmanuel.grumbach@intel.com>

In order to support MSI-X efficiently, we want to avoid
communication across Rx queues. Each Rx queue should have
all the data it needs to process a packet.

The reordering buffer is a challenge in the MSI-X world
since we can have a single BA session whose packets are
directed to different queues. This is why each queue has
its own reordering buffer. The hardware is able to hint
the driver whether we have a hole or not, which allows
the driver to know whether it can release a packet or not.
This indication is called NSSN. Roughly, if the packet's
SN is lower than the NSSN, we can release the packet to
the stack. The NSSN is the SN of the newest packet received
without any holes + 1.

This is working as long as we don't have packets that we
release because of a timeout. When that happens, we could
have taken the decision to release a packet after we have
been waiting for its predecessor for too long. If this
predecessor comes later, we have to drop it because we
can't release packets out of order. In that case, the
hardware will give us an indication that we can we release
the packet (SN < NSSN), but the packet still needs to be
dropped.
This is why we sometimes need to ignore the NSSN and we
track the head_sn in software.
Here is a specific example of this:

1) Rx queue 1 got packets: 480, 482, 483
2) We release 480 to to the stack and wait for 481
3) NSSN is now 481
4) The timeout expires
5) We release 482 and 483, NSSN is still 480
6) 481 arrives its NSSN is 484.

We need to drop 481 even if 481 < 484. This is why we'll
update the head_sn to 484 at step 2. The flow now is:

1) Rx queue 1 got packets: 480, 482, 483
2) We release 480 to to the stack and wait for 481
3) NSSN is now 481 / head_sn is 481
4) The timeout expires
5) We release 482 and 483, NSSN is still 480 but head_sn is 484.
6) 481 arrives its NSSN is 484, but head_sn is 484 and we drop it.

This code introduces another problem in case all the traffic
goes well (no hole, no timeout):

Rx queue 1: 0   -> 483   (head_sn = 484)
Rx queue 2: 501 -> 4095  (head_sn = 0)
Rx queue 2: 0   -> 480   (head_sn = 481)
Rx queue 1: 481 but head_sn = 484 and we drop it.

At this point, the SN of queue 1 is far behind: more than
4040 packets behind. Queue 1 will consider 481 "old"
because 481 is in [501-64:501] whereas it is a very new
packet.

In order to fix that, send an Rx notification from time to
time (twice across the full set of 4096 packets) to make
sure no Rx queue is lagging too far behind.

What will happen then is:

Rx queue 1: 0    -> 483       (head_sn = 484)
Rx queue 2: 501  -> 2047      (head_sn = 2048)
Rx queue 1: Sync nofication   (head_sn = 2048)
Rx queue 2: 2048 -> 4095      (head_sn = 0)
Rx queue 1: Sync notification (head_sn = 0)
Rx queue 2: 1    -> 481       (head_sn = 482)
Rx queue 1: 481 and head_sn = 0.

In queue 1's data, head_sn is now 0, the packet coming in
is 481, it'll understand that the new packet is new and it
won't be dropped.

Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 .../net/wireless/intel/iwlwifi/mvm/mac80211.c |  5 +-
 drivers/net/wireless/intel/iwlwifi/mvm/mvm.h  |  2 +-
 drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c | 64 +++++++++++++++----
 3 files changed, 56 insertions(+), 15 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c b/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c
index 99fa440bc639..982682ec74fd 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c
@@ -5053,7 +5053,6 @@ void iwl_mvm_sync_rx_queues_internal(struct iwl_mvm *mvm,
 	u32 qmask = BIT(mvm->trans->num_rx_queues) - 1;
 	int ret;
 
-	lockdep_assert_held(&mvm->mutex);
 
 	if (!iwl_mvm_has_new_rx_api(mvm))
 		return;
@@ -5064,13 +5063,15 @@ void iwl_mvm_sync_rx_queues_internal(struct iwl_mvm *mvm,
 		atomic_set(&mvm->queue_sync_counter,
 			   mvm->trans->num_rx_queues);
 
-	ret = iwl_mvm_notify_rx_queue(mvm, qmask, (u8 *)notif, size);
+	ret = iwl_mvm_notify_rx_queue(mvm, qmask, (u8 *)notif,
+				      size, !notif->sync);
 	if (ret) {
 		IWL_ERR(mvm, "Failed to trigger RX queues sync (%d)\n", ret);
 		goto out;
 	}
 
 	if (notif->sync) {
+		lockdep_assert_held(&mvm->mutex);
 		ret = wait_event_timeout(mvm->rx_sync_waitq,
 					 atomic_read(&mvm->queue_sync_counter) == 0 ||
 					 iwl_mvm_is_radio_killed(mvm),
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h b/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h
index c1e8b4766b0c..fd1764df592f 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h
@@ -1664,7 +1664,7 @@ void iwl_mvm_rx_monitor_no_data(struct iwl_mvm *mvm, struct napi_struct *napi,
 void iwl_mvm_rx_frame_release(struct iwl_mvm *mvm, struct napi_struct *napi,
 			      struct iwl_rx_cmd_buffer *rxb, int queue);
 int iwl_mvm_notify_rx_queue(struct iwl_mvm *mvm, u32 rxq_mask,
-			    const u8 *data, u32 count);
+			    const u8 *data, u32 count, bool async);
 void iwl_mvm_rx_queue_notif(struct iwl_mvm *mvm, struct napi_struct *napi,
 			    struct iwl_rx_cmd_buffer *rxb, int queue);
 void iwl_mvm_rx_tx_cmd(struct iwl_mvm *mvm, struct iwl_rx_cmd_buffer *rxb);
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c b/drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c
index 16078aa7c95f..4f4fdaf49eef 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c
@@ -463,7 +463,7 @@ static bool iwl_mvm_is_dup(struct ieee80211_sta *sta, int queue,
 }
 
 int iwl_mvm_notify_rx_queue(struct iwl_mvm *mvm, u32 rxq_mask,
-			    const u8 *data, u32 count)
+			    const u8 *data, u32 count, bool async)
 {
 	u8 buf[sizeof(struct iwl_rxq_sync_cmd) +
 	       sizeof(struct iwl_mvm_rss_sync_notif)];
@@ -487,7 +487,7 @@ int iwl_mvm_notify_rx_queue(struct iwl_mvm *mvm, u32 rxq_mask,
 	ret = iwl_mvm_send_cmd_pdu(mvm,
 				   WIDE_ID(DATA_PATH_GROUP,
 					   TRIGGER_RX_QUEUES_NOTIF_CMD),
-				   0, data_size, cmd);
+				   async ? CMD_ASYNC : 0, data_size, cmd);
 
 	return ret;
 }
@@ -504,6 +504,18 @@ static bool iwl_mvm_is_sn_less(u16 sn1, u16 sn2, u16 buffer_size)
 	       !ieee80211_sn_less(sn1, sn2 - buffer_size);
 }
 
+static void iwl_mvm_sync_nssn(struct iwl_mvm *mvm, u8 baid, u16 nssn)
+{
+	struct iwl_mvm_rss_sync_notif notif = {
+		.metadata.type = IWL_MVM_RXQ_NSSN_SYNC,
+		.metadata.sync = 0,
+		.nssn_sync.baid = baid,
+		.nssn_sync.nssn = nssn,
+	};
+
+	iwl_mvm_sync_rx_queues_internal(mvm, (void *)&notif, sizeof(notif));
+}
+
 #define RX_REORDER_BUF_TIMEOUT_MQ (HZ / 10)
 
 static void iwl_mvm_release_frames(struct iwl_mvm *mvm,
@@ -511,7 +523,7 @@ static void iwl_mvm_release_frames(struct iwl_mvm *mvm,
 				   struct napi_struct *napi,
 				   struct iwl_mvm_baid_data *baid_data,
 				   struct iwl_mvm_reorder_buffer *reorder_buf,
-				   u16 nssn)
+				   u16 nssn, bool sync_rss)
 {
 	struct iwl_mvm_reorder_buf_entry *entries =
 		&baid_data->entries[reorder_buf->queue *
@@ -530,6 +542,8 @@ static void iwl_mvm_release_frames(struct iwl_mvm *mvm,
 		struct sk_buff *skb;
 
 		ssn = ieee80211_sn_inc(ssn);
+		if (sync_rss && (ssn == 2048 || ssn == 0))
+			iwl_mvm_sync_nssn(mvm, baid_data->baid, ssn);
 
 		/*
 		 * Empty the list. Will have more than one frame for A-MSDU.
@@ -616,7 +630,8 @@ void iwl_mvm_reorder_timer_expired(struct timer_list *t)
 			     sta_id, sn);
 		iwl_mvm_event_frame_timeout_callback(buf->mvm, mvmsta->vif,
 						     sta, baid_data->tid);
-		iwl_mvm_release_frames(buf->mvm, sta, NULL, baid_data, buf, sn);
+		iwl_mvm_release_frames(buf->mvm, sta, NULL, baid_data,
+				       buf, sn, true);
 		rcu_read_unlock();
 	} else {
 		/*
@@ -658,7 +673,8 @@ static void iwl_mvm_del_ba(struct iwl_mvm *mvm, int queue,
 	spin_lock_bh(&reorder_buf->lock);
 	iwl_mvm_release_frames(mvm, sta, NULL, ba_data, reorder_buf,
 			       ieee80211_sn_add(reorder_buf->head_sn,
-						reorder_buf->buf_size));
+						reorder_buf->buf_size),
+			       false);
 	spin_unlock_bh(&reorder_buf->lock);
 	del_timer_sync(&reorder_buf->reorder_timer);
 
@@ -694,7 +710,8 @@ static void iwl_mvm_release_frames_from_notif(struct iwl_mvm *mvm,
 	reorder_buf = &ba_data->reorder_buf[queue];
 
 	spin_lock_bh(&reorder_buf->lock);
-	iwl_mvm_release_frames(mvm, sta, napi, ba_data, reorder_buf, nssn);
+	iwl_mvm_release_frames(mvm, sta, napi, ba_data,
+			       reorder_buf, nssn, false);
 	spin_unlock_bh(&reorder_buf->lock);
 
 out:
@@ -833,7 +850,8 @@ static bool iwl_mvm_reorder(struct iwl_mvm *mvm,
 	}
 
 	if (ieee80211_is_back_req(hdr->frame_control)) {
-		iwl_mvm_release_frames(mvm, sta, napi, baid_data, buffer, nssn);
+		iwl_mvm_release_frames(mvm, sta, napi, baid_data,
+				       buffer, nssn, false);
 		goto drop;
 	}
 
@@ -842,7 +860,10 @@ static bool iwl_mvm_reorder(struct iwl_mvm *mvm,
 	 * If the SN is smaller than the NSSN it might need to first go into
 	 * the reorder buffer, in which case we just release up to it and the
 	 * rest of the function will take care of storing it and releasing up to
-	 * the nssn
+	 * the nssn.
+	 * This should not happen. This queue has been lagging and it should
+	 * have been updated by a IWL_MVM_RXQ_NSSN_SYNC notification. Be nice
+	 * and update the other queues.
 	 */
 	if (!iwl_mvm_is_sn_less(nssn, buffer->head_sn + buffer->buf_size,
 				buffer->buf_size) ||
@@ -850,7 +871,7 @@ static bool iwl_mvm_reorder(struct iwl_mvm *mvm,
 		u16 min_sn = ieee80211_sn_less(sn, nssn) ? sn : nssn;
 
 		iwl_mvm_release_frames(mvm, sta, napi, baid_data, buffer,
-				       min_sn);
+				       min_sn, true);
 	}
 
 	/* drop any oudated packets */
@@ -861,8 +882,23 @@ static bool iwl_mvm_reorder(struct iwl_mvm *mvm,
 	if (!buffer->num_stored && ieee80211_sn_less(sn, nssn)) {
 		if (iwl_mvm_is_sn_less(buffer->head_sn, nssn,
 				       buffer->buf_size) &&
-		   (!amsdu || last_subframe))
+		   (!amsdu || last_subframe)) {
+			/*
+			 * If we crossed the 2048 or 0 SN, notify all the
+			 * queues. This is done in order to avoid having a
+			 * head_sn that lags behind for too long. When that
+			 * happens, we can get to a situation where the head_sn
+			 * is within the interval [nssn - buf_size : nssn]
+			 * which will make us think that the nssn is a packet
+			 * that we already freed because of the reordering
+			 * buffer and we will ignore it. So maintain the
+			 * head_sn somewhat updated across all the queues:
+			 * when it crosses 0 and 2048.
+			 */
+			if (sn == 2048 || sn == 0)
+				iwl_mvm_sync_nssn(mvm, baid, sn);
 			buffer->head_sn = nssn;
+		}
 		/* No need to update AMSDU last SN - we are moving the head */
 		spin_unlock_bh(&buffer->lock);
 		return false;
@@ -877,8 +913,11 @@ static bool iwl_mvm_reorder(struct iwl_mvm *mvm,
 	 * while technically there is no hole and we can move forward.
 	 */
 	if (!buffer->num_stored && sn == buffer->head_sn) {
-		if (!amsdu || last_subframe)
+		if (!amsdu || last_subframe) {
+			if (sn == 2048 || sn == 0)
+				iwl_mvm_sync_nssn(mvm, baid, sn);
 			buffer->head_sn = ieee80211_sn_inc(buffer->head_sn);
+		}
 		/* No need to update AMSDU last SN - we are moving the head */
 		spin_unlock_bh(&buffer->lock);
 		return false;
@@ -923,7 +962,8 @@ static bool iwl_mvm_reorder(struct iwl_mvm *mvm,
 	 * release notification with up to date NSSN.
 	 */
 	if (!amsdu || last_subframe)
-		iwl_mvm_release_frames(mvm, sta, napi, baid_data, buffer, nssn);
+		iwl_mvm_release_frames(mvm, sta, napi, baid_data,
+				       buffer, nssn, true);
 
 	spin_unlock_bh(&buffer->lock);
 	return true;
-- 
2.20.1


^ permalink raw reply related

* [PATCH 04/16] iwlwiif: mvm: refactor iwl_mvm_notify_rx_queue
From: Luca Coelho @ 2019-07-20 10:25 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless, Emmanuel Grumbach, Luca Coelho
In-Reply-To: <20190720102545.5952-1-luca@coelho.fi>

From: Emmanuel Grumbach <emmanuel.grumbach@intel.com>

Instead of allocating memory for which we have an upper
limit, use a small buffer on stack.

Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 drivers/net/wireless/intel/iwlwifi/fw/api/rx.h |  1 -
 drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c  | 17 +++++++++--------
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/fw/api/rx.h b/drivers/net/wireless/intel/iwlwifi/fw/api/rx.h
index ed69eec4fcd9..9b0bb89599fc 100644
--- a/drivers/net/wireless/intel/iwlwifi/fw/api/rx.h
+++ b/drivers/net/wireless/intel/iwlwifi/fw/api/rx.h
@@ -776,7 +776,6 @@ struct iwl_rss_config_cmd {
 	u8 indirection_table[IWL_RSS_INDIRECTION_TABLE_SIZE];
 } __packed; /* RSS_CONFIG_CMD_API_S_VER_1 */
 
-#define IWL_MULTI_QUEUE_SYNC_MSG_MAX_SIZE 128
 #define IWL_MULTI_QUEUE_SYNC_SENDER_POS 0
 #define IWL_MULTI_QUEUE_SYNC_SENDER_MSK 0xf
 
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c b/drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c
index bf097329efa2..16078aa7c95f 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c
@@ -465,18 +465,20 @@ static bool iwl_mvm_is_dup(struct ieee80211_sta *sta, int queue,
 int iwl_mvm_notify_rx_queue(struct iwl_mvm *mvm, u32 rxq_mask,
 			    const u8 *data, u32 count)
 {
-	struct iwl_rxq_sync_cmd *cmd;
+	u8 buf[sizeof(struct iwl_rxq_sync_cmd) +
+	       sizeof(struct iwl_mvm_rss_sync_notif)];
+	struct iwl_rxq_sync_cmd *cmd = (void *)buf;
 	u32 data_size = sizeof(*cmd) + count;
 	int ret;
 
-	/* should be DWORD aligned */
-	if (WARN_ON(count & 3 || count > IWL_MULTI_QUEUE_SYNC_MSG_MAX_SIZE))
+	/*
+	 * size must be a multiple of DWORD
+	 * Ensure we don't overflow buf
+	 */
+	if (WARN_ON(count & 3 ||
+		    count > sizeof(struct iwl_mvm_rss_sync_notif)))
 		return -EINVAL;
 
-	cmd = kzalloc(data_size, GFP_KERNEL);
-	if (!cmd)
-		return -ENOMEM;
-
 	cmd->rxq_mask = cpu_to_le32(rxq_mask);
 	cmd->count =  cpu_to_le32(count);
 	cmd->flags = 0;
@@ -487,7 +489,6 @@ int iwl_mvm_notify_rx_queue(struct iwl_mvm *mvm, u32 rxq_mask,
 					   TRIGGER_RX_QUEUES_NOTIF_CMD),
 				   0, data_size, cmd);
 
-	kfree(cmd);
 	return ret;
 }
 
-- 
2.20.1


^ permalink raw reply related

* [PATCH 03/16] iwlwifi: mvm: add a new RSS sync notification for NSSN sync
From: Luca Coelho @ 2019-07-20 10:25 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless, Emmanuel Grumbach, Luca Coelho
In-Reply-To: <20190720102545.5952-1-luca@coelho.fi>

From: Emmanuel Grumbach <emmanuel.grumbach@intel.com>

We will soon be using a new notification that will be
initiated by the driver, sent to the firmware and sent
back to all the RSS queues by the firmware. This new
notification will be useful to synchronize the NSSN across
all the queues.

For now, don't send the notification, just add the code to
handle it. Later patch will add the code to actually send
it.

While at it, validate the baid coming from the firmware to
avoid accessing an array with a bad index in the driver.

Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 .../net/wireless/intel/iwlwifi/fw/api/rx.h    |  2 +
 drivers/net/wireless/intel/iwlwifi/mvm/mvm.h  |  4 +-
 drivers/net/wireless/intel/iwlwifi/mvm/ops.c  |  4 +-
 drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c | 84 ++++++++++++-------
 drivers/net/wireless/intel/iwlwifi/mvm/sta.h  |  6 ++
 5 files changed, 64 insertions(+), 36 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/fw/api/rx.h b/drivers/net/wireless/intel/iwlwifi/fw/api/rx.h
index d55312ef58c9..ed69eec4fcd9 100644
--- a/drivers/net/wireless/intel/iwlwifi/fw/api/rx.h
+++ b/drivers/net/wireless/intel/iwlwifi/fw/api/rx.h
@@ -812,10 +812,12 @@ struct iwl_rxq_sync_notification {
  *
  * @IWL_MVM_RXQ_EMPTY: empty sync notification
  * @IWL_MVM_RXQ_NOTIF_DEL_BA: notify RSS queues of delBA
+ * @IWL_MVM_RXQ_NSSN_SYNC: notify all the RSS queues with the new NSSN
  */
 enum iwl_mvm_rxq_notif_type {
 	IWL_MVM_RXQ_EMPTY,
 	IWL_MVM_RXQ_NOTIF_DEL_BA,
+	IWL_MVM_RXQ_NSSN_SYNC,
 };
 
 /**
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h b/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h
index 48c77af54e99..c1e8b4766b0c 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h
@@ -1665,8 +1665,8 @@ void iwl_mvm_rx_frame_release(struct iwl_mvm *mvm, struct napi_struct *napi,
 			      struct iwl_rx_cmd_buffer *rxb, int queue);
 int iwl_mvm_notify_rx_queue(struct iwl_mvm *mvm, u32 rxq_mask,
 			    const u8 *data, u32 count);
-void iwl_mvm_rx_queue_notif(struct iwl_mvm *mvm, struct iwl_rx_cmd_buffer *rxb,
-			    int queue);
+void iwl_mvm_rx_queue_notif(struct iwl_mvm *mvm, struct napi_struct *napi,
+			    struct iwl_rx_cmd_buffer *rxb, int queue);
 void iwl_mvm_rx_tx_cmd(struct iwl_mvm *mvm, struct iwl_rx_cmd_buffer *rxb);
 void iwl_mvm_mfu_assert_dump_notif(struct iwl_mvm *mvm,
 				   struct iwl_rx_cmd_buffer *rxb);
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/ops.c b/drivers/net/wireless/intel/iwlwifi/mvm/ops.c
index d7d6f3398f86..4888054dc3d8 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/ops.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/ops.c
@@ -1088,7 +1088,7 @@ static void iwl_mvm_rx_mq(struct iwl_op_mode *op_mode,
 		iwl_mvm_rx_mpdu_mq(mvm, napi, rxb, 0);
 	else if (unlikely(cmd == WIDE_ID(DATA_PATH_GROUP,
 					 RX_QUEUES_NOTIFICATION)))
-		iwl_mvm_rx_queue_notif(mvm, rxb, 0);
+		iwl_mvm_rx_queue_notif(mvm, napi, rxb, 0);
 	else if (cmd == WIDE_ID(LEGACY_GROUP, FRAME_RELEASE))
 		iwl_mvm_rx_frame_release(mvm, napi, rxb, 0);
 	else if (cmd == WIDE_ID(DATA_PATH_GROUP, RX_NO_DATA_NOTIF))
@@ -1812,7 +1812,7 @@ static void iwl_mvm_rx_mq_rss(struct iwl_op_mode *op_mode,
 		iwl_mvm_rx_frame_release(mvm, napi, rxb, queue);
 	else if (unlikely(cmd == WIDE_ID(DATA_PATH_GROUP,
 					 RX_QUEUES_NOTIFICATION)))
-		iwl_mvm_rx_queue_notif(mvm, rxb, queue);
+		iwl_mvm_rx_queue_notif(mvm, napi, rxb, queue);
 	else if (likely(cmd == WIDE_ID(LEGACY_GROUP, REPLY_RX_MPDU_CMD)))
 		iwl_mvm_rx_mpdu_mq(mvm, napi, rxb, queue);
 }
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c b/drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c
index 64f950501287..bf097329efa2 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c
@@ -665,8 +665,51 @@ static void iwl_mvm_del_ba(struct iwl_mvm *mvm, int queue,
 	rcu_read_unlock();
 }
 
-void iwl_mvm_rx_queue_notif(struct iwl_mvm *mvm, struct iwl_rx_cmd_buffer *rxb,
-			    int queue)
+static void iwl_mvm_release_frames_from_notif(struct iwl_mvm *mvm,
+					      struct napi_struct *napi,
+					      u8 baid, u16 nssn, int queue)
+{
+	struct ieee80211_sta *sta;
+	struct iwl_mvm_reorder_buffer *reorder_buf;
+	struct iwl_mvm_baid_data *ba_data;
+
+	IWL_DEBUG_HT(mvm, "Frame release notification for BAID %u, NSSN %d\n",
+		     baid, nssn);
+
+	if (WARN_ON_ONCE(baid == IWL_RX_REORDER_DATA_INVALID_BAID ||
+			 baid >= ARRAY_SIZE(mvm->baid_map)))
+		return;
+
+	rcu_read_lock();
+
+	ba_data = rcu_dereference(mvm->baid_map[baid]);
+	if (WARN_ON_ONCE(!ba_data))
+		goto out;
+
+	sta = rcu_dereference(mvm->fw_id_to_mac_id[ba_data->sta_id]);
+	if (WARN_ON_ONCE(IS_ERR_OR_NULL(sta)))
+		goto out;
+
+	reorder_buf = &ba_data->reorder_buf[queue];
+
+	spin_lock_bh(&reorder_buf->lock);
+	iwl_mvm_release_frames(mvm, sta, napi, ba_data, reorder_buf, nssn);
+	spin_unlock_bh(&reorder_buf->lock);
+
+out:
+	rcu_read_unlock();
+}
+
+static void iwl_mvm_nssn_sync(struct iwl_mvm *mvm,
+			      struct napi_struct *napi, int queue,
+			      const struct iwl_mvm_nssn_sync_data *data)
+{
+	iwl_mvm_release_frames_from_notif(mvm, napi, data->baid,
+					  data->nssn, queue);
+}
+
+void iwl_mvm_rx_queue_notif(struct iwl_mvm *mvm, struct napi_struct *napi,
+			    struct iwl_rx_cmd_buffer *rxb, int queue)
 {
 	struct iwl_rx_packet *pkt = rxb_addr(rxb);
 	struct iwl_rxq_sync_notification *notif;
@@ -687,6 +730,10 @@ void iwl_mvm_rx_queue_notif(struct iwl_mvm *mvm, struct iwl_rx_cmd_buffer *rxb,
 	case IWL_MVM_RXQ_NOTIF_DEL_BA:
 		iwl_mvm_del_ba(mvm, queue, (void *)internal_notif->data);
 		break;
+	case IWL_MVM_RXQ_NSSN_SYNC:
+		iwl_mvm_nssn_sync(mvm, napi, queue,
+				  (void *)internal_notif->data);
+		break;
 	default:
 		WARN_ONCE(1, "Invalid identifier %d", internal_notif->type);
 	}
@@ -1840,40 +1887,13 @@ void iwl_mvm_rx_monitor_no_data(struct iwl_mvm *mvm, struct napi_struct *napi,
 out:
 	rcu_read_unlock();
 }
+
 void iwl_mvm_rx_frame_release(struct iwl_mvm *mvm, struct napi_struct *napi,
 			      struct iwl_rx_cmd_buffer *rxb, int queue)
 {
 	struct iwl_rx_packet *pkt = rxb_addr(rxb);
 	struct iwl_frame_release *release = (void *)pkt->data;
-	struct ieee80211_sta *sta;
-	struct iwl_mvm_reorder_buffer *reorder_buf;
-	struct iwl_mvm_baid_data *ba_data;
-
-	int baid = release->baid;
 
-	IWL_DEBUG_HT(mvm, "Frame release notification for BAID %u, NSSN %d\n",
-		     release->baid, le16_to_cpu(release->nssn));
-
-	if (WARN_ON_ONCE(baid == IWL_RX_REORDER_DATA_INVALID_BAID))
-		return;
-
-	rcu_read_lock();
-
-	ba_data = rcu_dereference(mvm->baid_map[baid]);
-	if (WARN_ON_ONCE(!ba_data))
-		goto out;
-
-	sta = rcu_dereference(mvm->fw_id_to_mac_id[ba_data->sta_id]);
-	if (WARN_ON_ONCE(IS_ERR_OR_NULL(sta)))
-		goto out;
-
-	reorder_buf = &ba_data->reorder_buf[queue];
-
-	spin_lock_bh(&reorder_buf->lock);
-	iwl_mvm_release_frames(mvm, sta, napi, ba_data, reorder_buf,
-			       le16_to_cpu(release->nssn));
-	spin_unlock_bh(&reorder_buf->lock);
-
-out:
-	rcu_read_unlock();
+	iwl_mvm_release_frames_from_notif(mvm, napi, release->baid,
+					  le16_to_cpu(release->nssn), queue);
 }
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/sta.h b/drivers/net/wireless/intel/iwlwifi/mvm/sta.h
index 79d655b3fce0..4823c06e6909 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/sta.h
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/sta.h
@@ -343,10 +343,16 @@ struct iwl_mvm_delba_data {
 	u32 baid;
 } __packed;
 
+struct iwl_mvm_nssn_sync_data {
+	u32 baid;
+	u32 nssn;
+} __packed;
+
 struct iwl_mvm_rss_sync_notif {
 	struct iwl_mvm_internal_rxq_notif metadata;
 	union {
 		struct iwl_mvm_delba_data delba;
+		struct iwl_mvm_nssn_sync_data nssn_sync;
 	};
 } __packed;
 
-- 
2.20.1


^ permalink raw reply related

* [PATCH 01/16] iwlwifi: mvm: don't send GEO_TX_POWER_LIMIT on version < 41
From: Luca Coelho @ 2019-07-20 10:25 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless, Luca Coelho, stable
In-Reply-To: <20190720102545.5952-1-luca@coelho.fi>

From: Luca Coelho <luciano.coelho@intel.com>

Firmware versions before 41 don't support the GEO_TX_POWER_LIMIT
command, and sending it to the firmware will cause a firmware crash.
We allow this via debugfs, so we need to return an error value in case
it's not supported.

This had already been fixed during init, when we send the command if
the ACPI WGDS table is present.  Fix it also for the other,
userspace-triggered case.

Cc: stable@vger.kernel.org
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 drivers/net/wireless/intel/iwlwifi/mvm/fw.c | 22 ++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/fw.c b/drivers/net/wireless/intel/iwlwifi/mvm/fw.c
index 1d608e9e9101..a837cf40afde 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/fw.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/fw.c
@@ -880,6 +880,17 @@ int iwl_mvm_sar_select_profile(struct iwl_mvm *mvm, int prof_a, int prof_b)
 	return iwl_mvm_send_cmd_pdu(mvm, REDUCE_TX_POWER_CMD, 0, len, &cmd);
 }
 
+static bool iwl_mvm_sar_geo_support(struct iwl_mvm *mvm)
+{
+	/*
+	 * The GEO_TX_POWER_LIMIT command is not supported on earlier
+	 * firmware versions.  Unfortunately, we don't have a TLV API
+	 * flag to rely on, so rely on the major version which is in
+	 * the first byte of ucode_ver.
+	 */
+	return IWL_UCODE_SERIAL(mvm->fw->ucode_ver) >= 41;
+}
+
 int iwl_mvm_get_sar_geo_profile(struct iwl_mvm *mvm)
 {
 	struct iwl_geo_tx_power_profiles_resp *resp;
@@ -909,6 +920,9 @@ int iwl_mvm_get_sar_geo_profile(struct iwl_mvm *mvm)
 		.data = { data },
 	};
 
+	if (!iwl_mvm_sar_geo_support(mvm))
+		return -EOPNOTSUPP;
+
 	ret = iwl_mvm_send_cmd(mvm, &cmd);
 	if (ret) {
 		IWL_ERR(mvm, "Failed to get geographic profile info %d\n", ret);
@@ -934,13 +948,7 @@ static int iwl_mvm_sar_geo_init(struct iwl_mvm *mvm)
 	int ret, i, j;
 	u16 cmd_wide_id =  WIDE_ID(PHY_OPS_GROUP, GEO_TX_POWER_LIMIT);
 
-	/*
-	 * This command is not supported on earlier firmware versions.
-	 * Unfortunately, we don't have a TLV API flag to rely on, so
-	 * rely on the major version which is in the first byte of
-	 * ucode_ver.
-	 */
-	if (IWL_UCODE_SERIAL(mvm->fw->ucode_ver) < 41)
+	if (!iwl_mvm_sar_geo_support(mvm))
 		return 0;
 
 	ret = iwl_mvm_sar_get_wgds_table(mvm);
-- 
2.20.1


^ permalink raw reply related

* [PATCH 02/16] iwlwifi: mvm: prepare the ground for more RSS notifications
From: Luca Coelho @ 2019-07-20 10:25 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless, Emmanuel Grumbach, Luca Coelho
In-Reply-To: <20190720102545.5952-1-luca@coelho.fi>

From: Emmanuel Grumbach <emmanuel.grumbach@intel.com>

We will need a new type of synchronization message going
through all the RSS queues. Prepare the ground for this.

Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 drivers/net/wireless/intel/iwlwifi/mvm/sta.c | 2 +-
 drivers/net/wireless/intel/iwlwifi/mvm/sta.h | 6 ++++--
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/sta.c b/drivers/net/wireless/intel/iwlwifi/mvm/sta.c
index ac9bc65c4d15..23fd3108adb9 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/sta.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/sta.c
@@ -2427,7 +2427,7 @@ int iwl_mvm_rm_mcast_sta(struct iwl_mvm *mvm, struct ieee80211_vif *vif)
 
 static void iwl_mvm_sync_rxq_del_ba(struct iwl_mvm *mvm, u8 baid)
 {
-	struct iwl_mvm_delba_notif notif = {
+	struct iwl_mvm_rss_sync_notif notif = {
 		.metadata.type = IWL_MVM_RXQ_NOTIF_DEL_BA,
 		.metadata.sync = 1,
 		.delba.baid = baid,
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/sta.h b/drivers/net/wireless/intel/iwlwifi/mvm/sta.h
index 84139fc38c34..79d655b3fce0 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/sta.h
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/sta.h
@@ -343,9 +343,11 @@ struct iwl_mvm_delba_data {
 	u32 baid;
 } __packed;
 
-struct iwl_mvm_delba_notif {
+struct iwl_mvm_rss_sync_notif {
 	struct iwl_mvm_internal_rxq_notif metadata;
-	struct iwl_mvm_delba_data delba;
+	union {
+		struct iwl_mvm_delba_data delba;
+	};
 } __packed;
 
 /**
-- 
2.20.1


^ permalink raw reply related

* [PATCH 00/16] iwlwifi: fixes intended for 5.3 2019-07-20
From: Luca Coelho @ 2019-07-20 10:25 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless, Luca Coelho

From: Luca Coelho <luciano.coelho@intel.com>

This is the second patchset with fixes for v5.3.

The changes are:

* Fix for an NSSN syncronization issue;
* Some fixes in rate-scaling;
* Fix for an "unknown command" bug with some FW versions;
* A couple of debug infrastructure fixes;
* One locking fix with GTK;
* Small fix for an error path (the error was ignored due to a type
  issue);

As usual, I'm pushing this to a pending branch, for kbuild bot.  I can
send you a pull-request for this or I can assign them to you so you
can apply them directly to wireless-drivers.  Let me know what you
prefer.


Cheers,
Luca.


Colin Ian King (1):
  iwlwifi: mvm: fix comparison of u32 variable with less than zero

Emmanuel Grumbach (5):
  iwlwifi: mvm: prepare the ground for more RSS notifications
  iwlwifi: mvm: add a new RSS sync notification for NSSN sync
  iwlwiif: mvm: refactor iwl_mvm_notify_rx_queue
  iwlwifi: mvm: add a loose synchronization of the NSSN across Rx queues
  iwlwifi: mvm: fix frame drop from the reordering buffer

Gregory Greenman (4):
  iwlwifi: mvm: add a wrapper around rs_tx_status to handle locks
  iwlwifi: mvm: send LQ command always ASYNC
  iwlwifi: mvm: replace RS mutex with a spin_lock
  iwlwifi: mvm: fix possible out-of-bounds read when accessing lq_info

Ihab Zhaika (1):
  iwlwifi: add 3 new IDs for the 9000 series (iwl9260_2ac_160_cfg)

Johannes Berg (1):
  iwlwifi: fix locking in delayed GTK setting

Luca Coelho (2):
  iwlwifi: mvm: don't send GEO_TX_POWER_LIMIT on version < 41
  iwlwifi: mvm: fix version check for GEO_TX_POWER_LIMIT support

Shahar S Matityahu (2):
  iwlwifi: dbg_ini: move iwl_dbg_tlv_load_bin out of debug override
    ifdef
  iwlwifi: dbg_ini: move iwl_dbg_tlv_free outside of debugfs ifdef

 .../net/wireless/intel/iwlwifi/fw/api/rx.h    |   3 +-
 drivers/net/wireless/intel/iwlwifi/iwl-drv.c  |   4 +-
 drivers/net/wireless/intel/iwlwifi/mvm/fw.c   |  27 +-
 .../net/wireless/intel/iwlwifi/mvm/mac80211.c |  44 +-
 drivers/net/wireless/intel/iwlwifi/mvm/mvm.h  |   8 +-
 drivers/net/wireless/intel/iwlwifi/mvm/nvm.c  |   2 +-
 drivers/net/wireless/intel/iwlwifi/mvm/ops.c  |   4 +-
 drivers/net/wireless/intel/iwlwifi/mvm/rs.c   | 559 +++++++++---------
 drivers/net/wireless/intel/iwlwifi/mvm/rs.h   |   6 +-
 drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c | 185 ++++--
 drivers/net/wireless/intel/iwlwifi/mvm/sta.c  |  10 +-
 drivers/net/wireless/intel/iwlwifi/mvm/sta.h  |  13 +-
 .../net/wireless/intel/iwlwifi/mvm/utils.c    |   4 +-
 drivers/net/wireless/intel/iwlwifi/pcie/drv.c |   3 +
 14 files changed, 496 insertions(+), 376 deletions(-)

-- 
2.20.1


^ permalink raw reply

* pull request: iwlwifi firmware updates 2019-07-20
From: Luca Coelho @ 2019-07-20  8:03 UTC (permalink / raw)
  To: linux-firmware@kernel.org
  Cc: linux-wireless@vger.kernel.org, linuxwifi, kyle@infradead.org,
	jwboyer@kernel.org, ben@decadent.org.uk, dor.shaish

[-- Attachment #1: Type: text/plain, Size: 1957 bytes --]

Hi,

This contains some updated firmwares for all our currently maintained
FW binaries.

Please pull or let me know if there are any issues.

--
Cheers,
Luca.


The following changes since commit bf13a71b18af229b4c900b321ef1f8443028ded8:

  Merge branch 'guc_v33' of git://anongit.freedesktop.org/drm/drm-firmware (2019-07-17 09:05:52 -0400)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/iwlwifi/linux-firmware.git tags/iwlwifi-fw-2019-07-20

for you to fetch changes up to cd6cb7bc50aa77d531c4417ffe1237510b71c73e:

  iwlwifi: update -48 FWs for Qu and cc (2019-07-20 10:58:24 +0300)

----------------------------------------------------------------
iwlwifi: update a bunch of FW binaries

----------------------------------------------------------------
Luca Coelho (2):
      iwlwifi: update FWs for 3168, 7265D, 9000, 9260, 8000, 8265 and cc
      iwlwifi: update -48 FWs for Qu and cc

 iwlwifi-3168-29.ucode             | Bin 1036276 -> 1036300 bytes
 iwlwifi-7265D-29.ucode            | Bin 1036432 -> 1036668 bytes
 iwlwifi-8000C-36.ucode            | Bin 2400700 -> 2401356 bytes
 iwlwifi-8265-36.ucode             | Bin 2414296 -> 2414592 bytes
 iwlwifi-9000-pu-b0-jf-b0-46.ucode | Bin 1460788 -> 1467952 bytes
 iwlwifi-9260-th-b0-jf-b0-46.ucode | Bin 1462324 -> 1469012 bytes
 iwlwifi-Qu-b0-hr-b0-48.ucode      | Bin 1106208 -> 1106204 bytes
 iwlwifi-Qu-b0-jf-b0-48.ucode      | Bin 1053156 -> 1052772 bytes
 iwlwifi-Qu-c0-hr-b0-48.ucode      | Bin 1106228 -> 1106224 bytes
 iwlwifi-Qu-c0-jf-b0-48.ucode      | Bin 1053176 -> 1052792 bytes
 iwlwifi-QuZ-a0-hr-b0-48.ucode     | Bin 1105648 -> 1105644 bytes
 iwlwifi-QuZ-a0-jf-b0-48.ucode     | Bin 1052968 -> 1052584 bytes
 iwlwifi-cc-a0-46.ucode            | Bin 1044072 -> 1044452 bytes
 iwlwifi-cc-a0-48.ucode            | Bin 1096684 -> 1096680 bytes
 14 files changed, 0 insertions(+), 0 deletions(-)

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH 2/2] mwifiex: Make use of the new sdio_trigger_replug() API to reset
From: Brian Norris @ 2019-07-19 23:36 UTC (permalink / raw)
  To: Douglas Anderson
  Cc: Ulf Hansson, Kalle Valo, Adrian Hunter, Ganapathi Bhat,
	linux-wireless, Amitkumar Karwar, linux-rockchip, Wolfram Sang,
	Nishant Sarmukadam, netdev, Avri Altman, linux-mmc, davem,
	Xinming Hu, linux-kernel, Andreas Fenkart
In-Reply-To: <20190716164209.62320-3-dianders@chromium.org>

Hi Doug,

On Tue, Jul 16, 2019 at 09:42:09AM -0700, Doug Anderson wrote:
> As described in the patch ("mmc: core: Add sdio_trigger_replug()
> API"), the current mwifiex_sdio_card_reset() is broken in the cases
> where we're running Bluetooth on a second SDIO func on the same card
> as WiFi.  The problem goes away if we just use the
> sdio_trigger_replug() API call.

I'm unfortunately not a good evaluator of SDIO/MMC stuff, so I'll mostly
leave that to others and assume that the "replug" description is pretty
much all I need to know.

> NOTE: Even though with this new solution there is less of a reason to
> do our work from a workqueue (the unplug / plug mechanism we're using
> is possible for a human to perform at any time so the stack is
> supposed to handle it without it needing to be called from a special
> context), we still need a workqueue because the Marvell reset function
> could called from a context where sleeping is invalid and thus we
> can't claim the host.  One example is Marvell's wakeup_timer_fn().
> 
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
> 
>  drivers/net/wireless/marvell/mwifiex/sdio.c | 14 +++-----------
>  1 file changed, 3 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c b/drivers/net/wireless/marvell/mwifiex/sdio.c
> index 24c041dad9f6..f77ad2615f08 100644
> --- a/drivers/net/wireless/marvell/mwifiex/sdio.c
> +++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
> @@ -2218,14 +2218,6 @@ static void mwifiex_sdio_card_reset_work(struct mwifiex_adapter *adapter)
>  {
>  	struct sdio_mmc_card *card = adapter->card;
>  	struct sdio_func *func = card->func;
> -	int ret;
> -
> -	mwifiex_shutdown_sw(adapter);

I'm very mildly unhappy to see this driver diverge from the PCIe one
again, but the only way it makes sense to do things the same is if there
is such thing as a "function level reset" for SDIO (i.e., doesn't also
kill the Bluetooth function). But it appears we don't really have such a
thing.

> -
> -	/* power cycle the adapter */
> -	sdio_claim_host(func);
> -	mmc_hw_reset(func->card->host);
> -	sdio_release_host(func);
>  
>  	/* Previous save_adapter won't be valid after this. We will cancel

^^^ FTR, the "save_adapter" note was already obsolete as of

  cc75c577806a mwifiex: get rid of global save_adapter and sdio_work

but the clear_bit() calls were (before this patch) still useful for
other reasons.

>  	 * pending work requests.
> @@ -2233,9 +2225,9 @@ static void mwifiex_sdio_card_reset_work(struct mwifiex_adapter *adapter)
>  	clear_bit(MWIFIEX_IFACE_WORK_DEVICE_DUMP, &card->work_flags);
>  	clear_bit(MWIFIEX_IFACE_WORK_CARD_RESET, &card->work_flags);

But now, I don't think you need these clear_bit() calls any more --
you're totally destroying the card and its workqueue on remove(). (And
anyway, MWIFIEX_IFACE_WORK_CARD_RESET was just cleared by your caller.)

>  
> -	ret = mwifiex_reinit_sw(adapter);
> -	if (ret)
> -		dev_err(&func->dev, "reinit failed: %d\n", ret);
> +	sdio_claim_host(func);
> +	sdio_trigger_replug(func);
> +	sdio_release_host(func);

And...we're approximately back to where we were 4 years ago :)

commit b4336a282db86b298b70563f8ed51782b36b772c
Author: Andreas Fenkart <afenkart@gmail.com>
Date:   Thu Jul 16 18:50:01 2015 +0200

    mwifiex: sdio: reset adapter using mmc_hw_reset

Anyway, assuming the "function reset" thing isn't workable, and you drop
the clear_bit() stuff, I think this is fine:

Reviewed-by: Brian Norris <briannorris@chromium.org>

>  }
>  
>  /* This function read/write firmware */
> -- 
> 2.22.0.510.g264f2c817a-goog
> 

^ permalink raw reply

* Re: Regression with the latest iwlwifi-9260-*-46.ucode
From: Takashi Iwai @ 2019-07-19 18:07 UTC (permalink / raw)
  To: Luciano Coelho
  Cc: dor.shaish, Josh Boyer, Johannes Berg, Emmanuel Grumbach,
	linux-wireless, linux-kernel
In-Reply-To: <280dad08ba9864755c3c45ed3ce26d602fe18a49.camel@intel.com>

On Fri, 19 Jul 2019 18:36:53 +0200,
Luciano Coelho wrote:
> 
> Adding Dor.
> 
> Hi Takashi,
> 
> Do you have full logs of the crash? We can't see much from the log
> snippet pasted in the bug report.

OK, I'll ask reporters.  If you have a SUSE/openSUSE bugzilla account,
feel free to join there.

> In any case, today or tomorrow, I'll release a new FW version for 9260,
> hopefully it will solve the problem.  I'll send you a link so you can
> test it when I push it to our tree (which feeds linux-firmware.git).

Good to hear.  I'll create a test package once when I have the new
firmware.


Thanks!

Takashi


> 
> Thanks for reporting!
> 
> --
> Cheers,
> Luca.
> 
> 
> On Fri, 2019-07-19 at 17:35 +0200, Takashi Iwai wrote:
> > Hi,
> > 
> > we've got a report about the regression with the latest iwlwifi 9260
> > *-46.ucode that landed recently in linux-firmware tree.  WiFi doesn't
> > work any longer on some Dell machines with that firmware.  See details
> > in:
> >   https://bugzilla.opensuse.org/show_bug.cgi?id=1142128
> > 
> > Currently we're reverting to the previously released firmware files.
> > 
> > Is it a known problem that has been already addressed?
> > If not, shall we revert the changes as a tentative workaround?
> > 
> > 
> > Thanks!
> > 
> > Takashi
> 

^ permalink raw reply

* Re: Regression with the latest iwlwifi-9260-*-46.ucode
From: Luciano Coelho @ 2019-07-19 16:36 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: dor.shaish, Josh Boyer, Johannes Berg, Emmanuel Grumbach,
	linux-wireless, linux-kernel
In-Reply-To: <s5hr26m9gvc.wl-tiwai@suse.de>

Adding Dor.

Hi Takashi,

Do you have full logs of the crash? We can't see much from the log
snippet pasted in the bug report.

In any case, today or tomorrow, I'll release a new FW version for 9260,
hopefully it will solve the problem.  I'll send you a link so you can
test it when I push it to our tree (which feeds linux-firmware.git).

Thanks for reporting!

--
Cheers,
Luca.


On Fri, 2019-07-19 at 17:35 +0200, Takashi Iwai wrote:
> Hi,
> 
> we've got a report about the regression with the latest iwlwifi 9260
> *-46.ucode that landed recently in linux-firmware tree.  WiFi doesn't
> work any longer on some Dell machines with that firmware.  See details
> in:
>   https://bugzilla.opensuse.org/show_bug.cgi?id=1142128
> 
> Currently we're reverting to the previously released firmware files.
> 
> Is it a known problem that has been already addressed?
> If not, shall we revert the changes as a tentative workaround?
> 
> 
> Thanks!
> 
> Takashi


^ permalink raw reply

* Regression with the latest iwlwifi-9260-*-46.ucode
From: Takashi Iwai @ 2019-07-19 15:35 UTC (permalink / raw)
  To: Luca Coelho
  Cc: Josh Boyer, Johannes Berg, Emmanuel Grumbach, linux-wireless,
	linux-kernel

Hi,

we've got a report about the regression with the latest iwlwifi 9260
*-46.ucode that landed recently in linux-firmware tree.  WiFi doesn't
work any longer on some Dell machines with that firmware.  See details
in:
  https://bugzilla.opensuse.org/show_bug.cgi?id=1142128

Currently we're reverting to the previously released firmware files.

Is it a known problem that has been already addressed?
If not, shall we revert the changes as a tentative workaround?


Thanks!

Takashi

^ permalink raw reply

* Re: [PATCH] staging: wilc1000: Merge memcpy + le32_to_cpus to get_unaligned_le32
From: Dan Carpenter @ 2019-07-19 14:04 UTC (permalink / raw)
  To: Ajay.Kathat
  Cc: devel, linux-wireless, gregkh, hslester96, linux-kernel,
	Adham.Abozaeid
In-Reply-To: <20190719140133.GH3111@kadam>

On Fri, Jul 19, 2019 at 05:01:33PM +0300, Dan Carpenter wrote:
> On Fri, Jul 19, 2019 at 12:05:07PM +0000, Ajay.Kathat@microchip.com wrote:
> > 
> > On 7/19/2019 5:16 PM, Chuhong Yuan wrote:
> > > 
> > > <Ajay.Kathat@microchip.com> 于2019年7月19日周五 下午7:34写道:
> > >>
> > >> On 7/19/2019 1:40 PM, Chuhong Yuan wrote:
> > >>>
> > >>> Merge the combo use of memcpy and le32_to_cpus.
> > >>> Use get_unaligned_le32 instead.
> > >>> This simplifies the code.
> > >>>
> > >>> Signed-off-by: Chuhong Yuan <hslester96@gmail.com>
> > >>> ---
> > >>>  drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 3 +--
> > >>>  1 file changed, 1 insertion(+), 2 deletions(-)
> > >>>
> > >>> diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
> > >>> index d72fdd333050..12fb4add05ec 100644
> > >>> --- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
> > >>> +++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
> > >>> @@ -1038,8 +1038,7 @@ void wilc_wfi_p2p_rx(struct wilc_vif *vif, u8 *buff, u32 size)
> > >>>       s32 freq;
> > >>>       __le16 fc;
> > >>>
> > >>> -     memcpy(&header, (buff - HOST_HDR_OFFSET), HOST_HDR_OFFSET);
> > >>> -     le32_to_cpus(&header);
> > >>> +     header = get_unaligned_le32(buff - HOST_HDR_OFFSET);
> > >>>       pkt_offset = GET_PKT_OFFSET(header);
> > >>>
> > >>>       if (pkt_offset & IS_MANAGMEMENT_CALLBACK) {
> > >>>
> > >>
> > >> Thanks for sending the patches.
> > >>
> > >> The code change looks okay to me. Just a minor comment, avoid the use of
> > >> same subject line for different patches.
> > > 
> > > These two patches are in the same subsystem and solve the same problem.
> > > I splitted them into two patches by mistake since I did not notice the problems
> > > in the second patch when I sent the first one.
> > > Should I merge the two patches and resend?
> > > 
> > 
> > Yes, please go ahead, merge the patches and send the updated version.
> > 
> 
> This is wrong advice.  Don't merge the patches because they are for
> different drivers.  The original subjects are fine because the
> subsystems are different so that's okay.
> 

Oh...  My bad...  I was looking at the wrong patches.  :P  You are
100% correct Ajay.  Merge the two patches and always make sure to not
send multiple patches with the same subject.

regards,
dan carpenter


^ permalink raw reply

* Re: [PATCH] staging: wilc1000: Merge memcpy + le32_to_cpus to get_unaligned_le32
From: Dan Carpenter @ 2019-07-19 14:01 UTC (permalink / raw)
  To: Ajay.Kathat
  Cc: hslester96, devel, gregkh, linux-wireless, linux-kernel,
	Adham.Abozaeid
In-Reply-To: <82dc6d2e-5f62-b2c7-296a-38f781628ec5@microchip.com>

On Fri, Jul 19, 2019 at 12:05:07PM +0000, Ajay.Kathat@microchip.com wrote:
> 
> On 7/19/2019 5:16 PM, Chuhong Yuan wrote:
> > 
> > <Ajay.Kathat@microchip.com> 于2019年7月19日周五 下午7:34写道:
> >>
> >> On 7/19/2019 1:40 PM, Chuhong Yuan wrote:
> >>>
> >>> Merge the combo use of memcpy and le32_to_cpus.
> >>> Use get_unaligned_le32 instead.
> >>> This simplifies the code.
> >>>
> >>> Signed-off-by: Chuhong Yuan <hslester96@gmail.com>
> >>> ---
> >>>  drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 3 +--
> >>>  1 file changed, 1 insertion(+), 2 deletions(-)
> >>>
> >>> diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
> >>> index d72fdd333050..12fb4add05ec 100644
> >>> --- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
> >>> +++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
> >>> @@ -1038,8 +1038,7 @@ void wilc_wfi_p2p_rx(struct wilc_vif *vif, u8 *buff, u32 size)
> >>>       s32 freq;
> >>>       __le16 fc;
> >>>
> >>> -     memcpy(&header, (buff - HOST_HDR_OFFSET), HOST_HDR_OFFSET);
> >>> -     le32_to_cpus(&header);
> >>> +     header = get_unaligned_le32(buff - HOST_HDR_OFFSET);
> >>>       pkt_offset = GET_PKT_OFFSET(header);
> >>>
> >>>       if (pkt_offset & IS_MANAGMEMENT_CALLBACK) {
> >>>
> >>
> >> Thanks for sending the patches.
> >>
> >> The code change looks okay to me. Just a minor comment, avoid the use of
> >> same subject line for different patches.
> > 
> > These two patches are in the same subsystem and solve the same problem.
> > I splitted them into two patches by mistake since I did not notice the problems
> > in the second patch when I sent the first one.
> > Should I merge the two patches and resend?
> > 
> 
> Yes, please go ahead, merge the patches and send the updated version.
> 

This is wrong advice.  Don't merge the patches because they are for
different drivers.  The original subjects are fine because the
subsystems are different so that's okay.

regards,
dan carpenter


^ permalink raw reply

* [PATCH v2] staging: wilc1000: Merge memcpy + le32_to_cpus to get_unaligned_le32
From: Chuhong Yuan @ 2019-07-19 12:16 UTC (permalink / raw)
  Cc: Adham Abozaeid, Ajay Singh, Greg Kroah-Hartman, linux-wireless,
	devel, linux-kernel, Chuhong Yuan

Merge the combo use of memcpy and le32_to_cpus.
Use get_unaligned_le32 instead.
This simplifies the code.

Signed-off-by: Chuhong Yuan <hslester96@gmail.com>
---
Changes in v2:
 - Merge the two patches with the same
   subject line.

 drivers/staging/wilc1000/wilc_mon.c               | 3 +--
 drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 3 +--
 drivers/staging/wilc1000/wilc_wlan.c              | 9 +++------
 3 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/wilc1000/wilc_mon.c b/drivers/staging/wilc1000/wilc_mon.c
index 7d7933d40924..d6f14f69ad64 100644
--- a/drivers/staging/wilc1000/wilc_mon.c
+++ b/drivers/staging/wilc1000/wilc_mon.c
@@ -35,8 +35,7 @@ void wilc_wfi_monitor_rx(struct net_device *mon_dev, u8 *buff, u32 size)
 		return;
 
 	/* Get WILC header */
-	memcpy(&header, (buff - HOST_HDR_OFFSET), HOST_HDR_OFFSET);
-	le32_to_cpus(&header);
+	header = get_unaligned_le32(buff - HOST_HDR_OFFSET);
 	/*
 	 * The packet offset field contain info about what type of management
 	 * the frame we are dealing with and ack status
diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
index d72fdd333050..12fb4add05ec 100644
--- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
+++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
@@ -1038,8 +1038,7 @@ void wilc_wfi_p2p_rx(struct wilc_vif *vif, u8 *buff, u32 size)
 	s32 freq;
 	__le16 fc;
 
-	memcpy(&header, (buff - HOST_HDR_OFFSET), HOST_HDR_OFFSET);
-	le32_to_cpus(&header);
+	header = get_unaligned_le32(buff - HOST_HDR_OFFSET);
 	pkt_offset = GET_PKT_OFFSET(header);
 
 	if (pkt_offset & IS_MANAGMEMENT_CALLBACK) {
diff --git a/drivers/staging/wilc1000/wilc_wlan.c b/drivers/staging/wilc1000/wilc_wlan.c
index d46876edcfeb..7d438ae90c3e 100644
--- a/drivers/staging/wilc1000/wilc_wlan.c
+++ b/drivers/staging/wilc1000/wilc_wlan.c
@@ -703,8 +703,7 @@ static void wilc_wlan_handle_rx_buff(struct wilc *wilc, u8 *buffer, int size)
 
 	do {
 		buff_ptr = buffer + offset;
-		memcpy(&header, buff_ptr, 4);
-		le32_to_cpus(&header);
+		header = get_unaligned_le32(buff_ptr);
 
 		is_cfg_packet = (header >> 31) & 0x1;
 		pkt_offset = (header >> 22) & 0x1ff;
@@ -874,10 +873,8 @@ int wilc_wlan_firmware_download(struct wilc *wilc, const u8 *buffer,
 
 	offset = 0;
 	do {
-		memcpy(&addr, &buffer[offset], 4);
-		memcpy(&size, &buffer[offset + 4], 4);
-		le32_to_cpus(&addr);
-		le32_to_cpus(&size);
+		addr = get_unaligned_le32(&buffer[offset]);
+		size = get_unaligned_le32(&buffer[offset + 4]);
 		acquire_bus(wilc, WILC_BUS_ACQUIRE_ONLY);
 		offset += 8;
 		while (((int)size) && (offset < buffer_size)) {
-- 
2.20.1


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox