Re: mt76x2e hardware restart

[Lorenzo Bianconi <lorenzo.bianconi@redhat.com>]

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

> > Hello.
> > 
> > On 15.10.2019 18:52, Oleksandr Natalenko wrote:
> > > Thanks for the answer and the IRC discussion. As agreed I've applied
> > > [1] and [2], and have just swapped the card to try it again. So far,
> > > it works fine in 5 GHz band in 802.11ac mode as an AP.
> > > 
> > > I'll give it more load with my phone over evening, and we can discuss
> > > what to do next (if needed) tomorrow again. Or feel free to drop me an
> > > email today.
> > > 
> > > Thanks for your efforts.
> > > 
> > > [1]
> > > https://github.com/LorenzoBianconi/wireless-drivers-next/commit/cf3436c42a297967235a9c9778620c585100529e.patch
> > > [2]
> > > https://github.com/LorenzoBianconi/wireless-drivers-next/commit/aad256eb62620f9646d39c1aa69234f50c89eed8.patch
> > 
> > As agreed, here are iperf3 results, AP to STA distance is 2 meters.
> > 
> > Client sends, TCP:
> > 
> > [ ID] Interval           Transfer     Bitrate         Retr
> > [  5]   0.00-10.00  sec  70.4 MBytes  59.0 Mbits/sec  3800
> > sender
> > [  5]   0.00-10.03  sec  70.0 MBytes  58.6 Mbits/sec
> > receiver
> > 
> > Client receives, TCP:
> > 
> > [ ID] Interval           Transfer     Bitrate         Retr
> > [  5]   0.00-10.06  sec   196 MBytes   163 Mbits/sec  3081
> > sender
> > [  5]   0.00-10.01  sec   191 MBytes   160 Mbits/sec
> > receiver
> > 
> > Client sends, UDP, 128 streams:
> > 
> > [ ID] Interval           Transfer     Bitrate         Jitter    Lost/Total
> > Datagrams
> > [SUM]   0.00-10.00  sec   160 MBytes   134 Mbits/sec  0.000 ms  0/115894
> > (0%)  sender
> > [SUM]   0.00-10.01  sec   160 MBytes   134 Mbits/sec  0.347 ms  0/115892
> > (0%)  receiver
> > 
> > Client receives, UDP, 128 streams:
> > 
> > [ ID] Interval           Transfer     Bitrate         Jitter    Lost/Total
> > Datagrams
> > [SUM]   0.00-10.01  sec   119 MBytes  99.4 Mbits/sec  0.000 ms  0/85888 (0%)
> > sender
> > [SUM]   0.00-10.00  sec   119 MBytes  99.5 Mbits/sec  0.877 ms  0/85888 (0%)
> > receiver
> > 
> > Given the HW is not the most powerful, the key point here is that nothing
> > crashed after doing these tests.
> 
> Hi Oleksandr,
> 
> thx a lot for testing these 2 patches. Now we need to understand why the chip
> hangs if we enable scatter gather dma transfer on x86 while it is working fine
> on multiple mips/arm devices (patch 2/2 just disable it for debugging).

Hi Oleksandr,

I think I spotted the SG issue on mt76x2e. Could you please:
- keep pcie_aspm patch I sent
- remove the debug patch where I disabled TX Scatter-Gather on mt76x2e
- apply the following patch

Regards,
Lorenzo

mt76: dma: fix buffer unmap with non-linear skbs

mt76 dma layer is supposed to unmap skb data buffers while keep txwi mapped
on hw dma ring. At the moment mt76 wrongly unmap txwi or does not unmap data
fragments in even positions for non-linear skbs. This issue may result in hw
hangs with A-MSUD if the system relies on IOMMU or SWIOTLB.
Fix this behaviour marking first and last queue entries introducing
MT_QUEUE_ENTRY_FIRST and MT_QUEUE_ENTRY_LAST flags and properly unmap
data fragments

Fixes: 17f1de56df05 ("mt76: add common code shared between multiple chipsets")
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 drivers/net/wireless/mediatek/mt76/dma.c  | 33 +++++++++++++----------
 drivers/net/wireless/mediatek/mt76/mt76.h |  3 +++
 2 files changed, 22 insertions(+), 14 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/dma.c b/drivers/net/wireless/mediatek/mt76/dma.c
index 4da7cffbab29..a3026a0ca8c5 100644
--- a/drivers/net/wireless/mediatek/mt76/dma.c
+++ b/drivers/net/wireless/mediatek/mt76/dma.c
@@ -54,7 +54,7 @@ mt76_dma_add_buf(struct mt76_dev *dev, struct mt76_queue *q,
 	int i, idx = -1;
 
 	if (txwi)
-		q->entry[q->head].txwi = DMA_DUMMY_DATA;
+		q->entry[q->head].flags = MT_QUEUE_ENTRY_FIRST;
 
 	for (i = 0; i < nbufs; i += 2, buf += 2) {
 		u32 buf0 = buf[0].addr, buf1 = 0;
@@ -83,6 +83,7 @@ mt76_dma_add_buf(struct mt76_dev *dev, struct mt76_queue *q,
 		q->queued++;
 	}
 
+	q->entry[idx].flags |= MT_QUEUE_ENTRY_LAST;
 	q->entry[idx].txwi = txwi;
 	q->entry[idx].skb = skb;
 
@@ -93,27 +94,31 @@ static void
 mt76_dma_tx_cleanup_idx(struct mt76_dev *dev, struct mt76_queue *q, int idx,
 			struct mt76_queue_entry *prev_e)
 {
+	__le32 addr, __ctrl = READ_ONCE(q->desc[idx].ctrl);
 	struct mt76_queue_entry *e = &q->entry[idx];
-	__le32 __ctrl = READ_ONCE(q->desc[idx].ctrl);
-	u32 ctrl = le32_to_cpu(__ctrl);
-
-	if (!e->txwi || !e->skb) {
-		__le32 addr = READ_ONCE(q->desc[idx].buf0);
-		u32 len = FIELD_GET(MT_DMA_CTL_SD_LEN0, ctrl);
+	u32 len, ctrl = le32_to_cpu(__ctrl);
 
+	if (e->flags & MT_QUEUE_ENTRY_FIRST) {
+		addr = READ_ONCE(q->desc[idx].buf1);
+		len = FIELD_GET(MT_DMA_CTL_SD_LEN1, ctrl);
 		dma_unmap_single(dev->dev, le32_to_cpu(addr), len,
 				 DMA_TO_DEVICE);
-	}
-
-	if (!(ctrl & MT_DMA_CTL_LAST_SEC0)) {
-		__le32 addr = READ_ONCE(q->desc[idx].buf1);
-		u32 len = FIELD_GET(MT_DMA_CTL_SD_LEN1, ctrl);
-
+	} else {
+		addr = READ_ONCE(q->desc[idx].buf0);
+		len = FIELD_GET(MT_DMA_CTL_SD_LEN0, ctrl);
 		dma_unmap_single(dev->dev, le32_to_cpu(addr), len,
 				 DMA_TO_DEVICE);
+		if (e->txwi &&
+		    ((ctrl & MT_DMA_CTL_LAST_SEC1) ||
+		     !(e->flags & MT_QUEUE_ENTRY_LAST))) {
+			addr = READ_ONCE(q->desc[idx].buf1);
+			len = FIELD_GET(MT_DMA_CTL_SD_LEN1, ctrl);
+			dma_unmap_single(dev->dev, le32_to_cpu(addr), len,
+					 DMA_TO_DEVICE);
+		}
 	}
 
-	if (e->txwi == DMA_DUMMY_DATA)
+	if (!(e->flags & MT_QUEUE_ENTRY_LAST))
 		e->txwi = NULL;
 
 	if (e->skb == DMA_DUMMY_DATA)
diff --git a/drivers/net/wireless/mediatek/mt76/mt76.h b/drivers/net/wireless/mediatek/mt76/mt76.h
index e95a5893f93b..b0ac82b31789 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76.h
+++ b/drivers/net/wireless/mediatek/mt76/mt76.h
@@ -83,6 +83,8 @@ struct mt76_tx_info {
 	u32 info;
 };
 
+#define MT_QUEUE_ENTRY_FIRST	BIT(0)
+#define MT_QUEUE_ENTRY_LAST	BIT(1)
 struct mt76_queue_entry {
 	union {
 		void *buf;
@@ -95,6 +97,7 @@ struct mt76_queue_entry {
 	enum mt76_txq_id qid;
 	bool schedule;
 	bool done;
+	u32 flags;
 };
 
 struct mt76_queue_regs {
-- 
2.21.0

> 
> Regards,
> Lorenzo
> 
> > 
> > -- 
> >   Oleksandr Natalenko (post-factum)



[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]