[PATCH] iwlwifi: pcie: fix DMA memory mapping / unmapping

[PATCH] iwlwifi: pcie: fix DMA memory mapping / unmapping

[Emmanuel Grumbach]

22000 devices (previously referenced as A000) can support
short transmit queues. This means that we have less DMA
descriptors (TFD) for those shorter queues.
Previous devices must still have 256 TFDs for each queue
even if those 256 TFDs point to fewer buffers.

When I introduced support for the short queues for 22000
I broke older devices by assuming that they can also have
less TFDs in their queues. This led to several problems:

1) the payload of the commands weren't unmapped properly
   which caused the SWIOTLB to complain at some point.
2) the hardware could get confused and we get hardware
   crashes.

The corresponding bugzilla entries are:

https://bugzilla.kernel.org/show_bug.cgi?id=198201
https://bugzilla.kernel.org/show_bug.cgi?id=198265

Cc: stable@vger.kernel.org # 4.14+
Fixes: 4ecab5616023 ("iwlwifi: pcie: support short Tx queues for A000 device family")
Reviewed-by: Sharon, Sara <sara.sharon@intel.com>
Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
---
Hi Kalle,

Luca is on vacation is 4.15 will be closed soon.
I am fixing here a bug that caused much troube on our side.
There are two bugzillas on it. Users on both bugs validated
this fix.
Please apply this on wireless-drivers.git directly and I'll sync
with Luca when he'll be back.

Thank you!
---
 drivers/net/wireless/intel/iwlwifi/pcie/internal.h | 10 +++++++---
 drivers/net/wireless/intel/iwlwifi/pcie/tx-gen2.c  | 11 +++--------
 drivers/net/wireless/intel/iwlwifi/pcie/tx.c       |  8 ++++----
 3 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/internal.h b/drivers/net/wireless/intel/iwlwifi/pcie/internal.h
index d749abeca3ae..403e65c309d0 100644
--- a/drivers/net/wireless/intel/iwlwifi/pcie/internal.h
+++ b/drivers/net/wireless/intel/iwlwifi/pcie/internal.h
@@ -670,11 +670,15 @@ static inline u8 iwl_pcie_get_cmd_index(struct iwl_txq *q, u32 index)
 	return index & (q->n_window - 1);
 }
 
-static inline void *iwl_pcie_get_tfd(struct iwl_trans_pcie *trans_pcie,
+static inline void *iwl_pcie_get_tfd(struct iwl_trans *trans,
 				     struct iwl_txq *txq, int idx)
 {
-	return txq->tfds + trans_pcie->tfd_size * iwl_pcie_get_cmd_index(txq,
-									 idx);
+	struct iwl_trans_pcie *trans_pcie = IWL_TRANS_GET_PCIE_TRANS(trans);
+
+	if (trans->cfg->use_tfh)
+		idx = iwl_pcie_get_cmd_index(txq, idx);
+
+	return txq->tfds + trans_pcie->tfd_size * idx;
 }
 
 static inline void iwl_enable_rfkill_int(struct iwl_trans *trans)
diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/tx-gen2.c b/drivers/net/wireless/intel/iwlwifi/pcie/tx-gen2.c
index 16b345f54ff0..6d0a907d5ba5 100644
--- a/drivers/net/wireless/intel/iwlwifi/pcie/tx-gen2.c
+++ b/drivers/net/wireless/intel/iwlwifi/pcie/tx-gen2.c
@@ -171,8 +171,6 @@ static void iwl_pcie_gen2_tfd_unmap(struct iwl_trans *trans,
 
 static void iwl_pcie_gen2_free_tfd(struct iwl_trans *trans, struct iwl_txq *txq)
 {
-	struct iwl_trans_pcie *trans_pcie = IWL_TRANS_GET_PCIE_TRANS(trans);
-
 	/* rd_ptr is bounded by TFD_QUEUE_SIZE_MAX and
 	 * idx is bounded by n_window
 	 */
@@ -181,7 +179,7 @@ static void iwl_pcie_gen2_free_tfd(struct iwl_trans *trans, struct iwl_txq *txq)
 	lockdep_assert_held(&txq->lock);
 
 	iwl_pcie_gen2_tfd_unmap(trans, &txq->entries[idx].meta,
-				iwl_pcie_get_tfd(trans_pcie, txq, idx));
+				iwl_pcie_get_tfd(trans, txq, idx));
 
 	/* free SKB */
 	if (txq->entries) {
@@ -364,11 +362,9 @@ struct iwl_tfh_tfd *iwl_pcie_gen2_build_tfd(struct iwl_trans *trans,
 					    struct sk_buff *skb,
 					    struct iwl_cmd_meta *out_meta)
 {
-	struct iwl_trans_pcie *trans_pcie = IWL_TRANS_GET_PCIE_TRANS(trans);
 	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
 	int idx = iwl_pcie_get_cmd_index(txq, txq->write_ptr);
-	struct iwl_tfh_tfd *tfd =
-		iwl_pcie_get_tfd(trans_pcie, txq, idx);
+	struct iwl_tfh_tfd *tfd = iwl_pcie_get_tfd(trans, txq, idx);
 	dma_addr_t tb_phys;
 	bool amsdu;
 	int i, len, tb1_len, tb2_len, hdr_len;
@@ -565,8 +561,7 @@ static int iwl_pcie_gen2_enqueue_hcmd(struct iwl_trans *trans,
 	u8 group_id = iwl_cmd_groupid(cmd->id);
 	const u8 *cmddata[IWL_MAX_CMD_TBS_PER_TFD];
 	u16 cmdlen[IWL_MAX_CMD_TBS_PER_TFD];
-	struct iwl_tfh_tfd *tfd =
-		iwl_pcie_get_tfd(trans_pcie, txq, txq->write_ptr);
+	struct iwl_tfh_tfd *tfd = iwl_pcie_get_tfd(trans, txq, txq->write_ptr);
 
 	memset(tfd, 0, sizeof(*tfd));
 
diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/tx.c b/drivers/net/wireless/intel/iwlwifi/pcie/tx.c
index fed6d842a5e1..3f85713c41dc 100644
--- a/drivers/net/wireless/intel/iwlwifi/pcie/tx.c
+++ b/drivers/net/wireless/intel/iwlwifi/pcie/tx.c
@@ -373,7 +373,7 @@ static void iwl_pcie_tfd_unmap(struct iwl_trans *trans,
 {
 	struct iwl_trans_pcie *trans_pcie = IWL_TRANS_GET_PCIE_TRANS(trans);
 	int i, num_tbs;
-	void *tfd = iwl_pcie_get_tfd(trans_pcie, txq, index);
+	void *tfd = iwl_pcie_get_tfd(trans, txq, index);
 
 	/* Sanity check on number of chunks */
 	num_tbs = iwl_pcie_tfd_get_num_tbs(trans, tfd);
@@ -2018,7 +2018,7 @@ static int iwl_fill_data_tbs(struct iwl_trans *trans, struct sk_buff *skb,
 	}
 
 	trace_iwlwifi_dev_tx(trans->dev, skb,
-			     iwl_pcie_get_tfd(trans_pcie, txq, txq->write_ptr),
+			     iwl_pcie_get_tfd(trans, txq, txq->write_ptr),
 			     trans_pcie->tfd_size,
 			     &dev_cmd->hdr, IWL_FIRST_TB_SIZE + tb1_len,
 			     hdr_len);
@@ -2092,7 +2092,7 @@ static int iwl_fill_data_tbs_amsdu(struct iwl_trans *trans, struct sk_buff *skb,
 		IEEE80211_CCMP_HDR_LEN : 0;
 
 	trace_iwlwifi_dev_tx(trans->dev, skb,
-			     iwl_pcie_get_tfd(trans_pcie, txq, txq->write_ptr),
+			     iwl_pcie_get_tfd(trans, txq, txq->write_ptr),
 			     trans_pcie->tfd_size,
 			     &dev_cmd->hdr, IWL_FIRST_TB_SIZE + tb1_len, 0);
 
@@ -2425,7 +2425,7 @@ int iwl_trans_pcie_tx(struct iwl_trans *trans, struct sk_buff *skb,
 	memcpy(&txq->first_tb_bufs[txq->write_ptr], &dev_cmd->hdr,
 	       IWL_FIRST_TB_SIZE);
 
-	tfd = iwl_pcie_get_tfd(trans_pcie, txq, txq->write_ptr);
+	tfd = iwl_pcie_get_tfd(trans, txq, txq->write_ptr);
 	/* Set up entry for this TFD in Tx byte-count array */
 	iwl_pcie_txq_update_byte_cnt_tbl(trans, txq, le16_to_cpu(tx_cmd->len),
 					 iwl_pcie_tfd_get_num_tbs(trans, tfd));
-- 
2.9.3

Re: [PATCH] iwlwifi: pcie: fix DMA memory mapping / unmapping

[Kalle Valo]

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

> 22000 devices (previously referenced as A000) can support
> short transmit queues. This means that we have less DMA
> descriptors (TFD) for those shorter queues.
> Previous devices must still have 256 TFDs for each queue
> even if those 256 TFDs point to fewer buffers.
>
> When I introduced support for the short queues for 22000
> I broke older devices by assuming that they can also have
> less TFDs in their queues. This led to several problems:
>
> 1) the payload of the commands weren't unmapped properly
>    which caused the SWIOTLB to complain at some point.
> 2) the hardware could get confused and we get hardware
>    crashes.
>
> The corresponding bugzilla entries are:
>
> https://bugzilla.kernel.org/show_bug.cgi?id=198201
> https://bugzilla.kernel.org/show_bug.cgi?id=198265
>
> Cc: stable@vger.kernel.org # 4.14+
> Fixes: 4ecab5616023 ("iwlwifi: pcie: support short Tx queues for A000 device family")
> Reviewed-by: Sharon, Sara <sara.sharon@intel.com>
> Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
> ---
> Hi Kalle,
>
> Luca is on vacation is 4.15 will be closed soon.
> I am fixing here a bug that caused much troube on our side.
> There are two bugzillas on it. Users on both bugs validated
> this fix.
> Please apply this on wireless-drivers.git directly and I'll sync
> with Luca when he'll be back.

Ok, I'll queue this for 4.15.

-- 
Kalle Valo

Re: iwlwifi: pcie: fix DMA memory mapping / unmapping

[Kalle Valo]

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

> 22000 devices (previously referenced as A000) can support
> short transmit queues. This means that we have less DMA
> descriptors (TFD) for those shorter queues.
> Previous devices must still have 256 TFDs for each queue
> even if those 256 TFDs point to fewer buffers.
> 
> When I introduced support for the short queues for 22000
> I broke older devices by assuming that they can also have
> less TFDs in their queues. This led to several problems:
> 
> 1) the payload of the commands weren't unmapped properly
>    which caused the SWIOTLB to complain at some point.
> 2) the hardware could get confused and we get hardware
>    crashes.
> 
> The corresponding bugzilla entries are:
> 
> https://bugzilla.kernel.org/show_bug.cgi?id=198201
> https://bugzilla.kernel.org/show_bug.cgi?id=198265
> 
> Cc: stable@vger.kernel.org # 4.14+
> Fixes: 4ecab5616023 ("iwlwifi: pcie: support short Tx queues for A000 device family")
> Reviewed-by: Sharon, Sara <sara.sharon@intel.com>
> Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>

Patch applied to wireless-drivers.git, thanks.

943309d4aad6 iwlwifi: pcie: fix DMA memory mapping / unmapping

-- 
https://patchwork.kernel.org/patch/10143989/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches