Linux wireless drivers development
 help / color / mirror / Atom feed
* Re: [PATCH v3 1/2] mt76: mt7615: enable support for mesh
From: Ryder Lee @ 2019-06-06 15:18 UTC (permalink / raw)
  To: Sebastian Gottschall
  Cc: Felix Fietkau, Lorenzo Bianconi, Roy Luo, YF Luo, Yiwei Chung,
	Sean Wang, Chih-Min Chen, linux-wireless, linux-mediatek,
	linux-kernel
In-Reply-To: <a0a6f631-2eb1-87cc-5653-338c6126690c@newmedia-net.de>

On Thu, 2019-06-06 at 12:14 +0200, Sebastian Gottschall wrote:
> in addition you should take care about this problem which is raised up 
> if SAE is used. since AES-CMAC required tid to be non zero
> 
> WARNING: CPU: 2 PID: 15324 at 
> /home/seg/DEV/mt7621/src/router/private/compat-wireless-2017-09-03/net/mac80211/key.c:1096 
> mt76_wcid_key_setup+0x58/0x9c [mt76]
> Modules linked in: shortcut_fe gcm ghash_generic ctr gf128mul mt7615e 
> mt76 mac80211 compat
> CPU: 2 PID: 15324 Comm: wpa_supplicant Tainted: G        W 4.14.123 #106
> Stack : 00000000 87c2d000 00000000 8007d8b4 80480000 80482b9c 80610000 
> 805a4390
>          8057e2b4 854fb99c 87ed045c 805e4767 80578288 00000001 854fb940 
> 805e9f78
>          00000000 00000000 80640000 00000000 81147bb8 00000584 00000007 
> 00000000
>          00000000 80650000 80650000 20202020 80000000 00000000 80610000 
> 872b9fe0
>          872a2b14 00000448 00000000 87c2d000 00000010 8022d660 00000008 
> 80640008
>          ...
> Call Trace:
> [<800153e0>] show_stack+0x58/0x100
> [<8042e83c>] dump_stack+0x9c/0xe0
> [<800349f0>] __warn+0xe4/0x144
> [<8003468c>] warn_slowpath_null+0x1c/0x30
> [<872b9fe0>] mt76_wcid_key_setup+0x58/0x9c [mt76]
> [<87611690>] mt7615_eeprom_init+0x7b4/0xe9c [mt7615e]
> ---[ end trace e24aeb4b542e0dea ]---

This is fixed by Lorenzo's patch -
https://patchwork.kernel.org/patch/10976191/

Thanks.
Ryder


^ permalink raw reply

* Re: [PATCH v2 3/3] brcmfmac: sdio: Disable auto-tuning around commands expected to fail
From: Adrian Hunter @ 2019-06-06 13:59 UTC (permalink / raw)
  To: Douglas Anderson, Ulf Hansson, Kalle Valo, Arend van Spriel
  Cc: brcm80211-dev-list.pdl, linux-rockchip, Double Lo, briannorris,
	linux-wireless, Naveen Gupta, Madhan Mohan R, mka, Wright Feng,
	Chi-Hsien Lin, netdev, brcm80211-dev-list, David S. Miller,
	Franky Lin, linux-kernel, Hante Meuleman, YueHaibing,
	Michael Trimarchi
In-Reply-To: <20190603183740.239031-4-dianders@chromium.org>

On 3/06/19 9:37 PM, Douglas Anderson wrote:
> There are certain cases, notably when transitioning between sleep and
> active state, when Broadcom SDIO WiFi cards will produce errors on the
> SDIO bus.  This is evident from the source code where you can see that
> we try commands in a loop until we either get success or we've tried
> too many times.  The comment in the code reinforces this by saying
> "just one write attempt may fail"
> 
> Unfortunately these failures sometimes end up causing an "-EILSEQ"
> back to the core which triggers a retuning of the SDIO card and that
> blocks all traffic to the card until it's done.
> 
> Let's disable retuning around the commands we expect might fail.

It seems to me that re-tuning needs to be prevented before the
first access otherwise it might be attempted there, and it needs
to continue to be prevented during the transition when it might
reasonably be expected to fail.

What about something along these lines:

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
index 4e15ea57d4f5..d932780ef56e 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -664,9 +664,18 @@ brcmf_sdio_kso_control(struct brcmf_sdio *bus, bool on)
 	int err = 0;
 	int err_cnt = 0;
 	int try_cnt = 0;
+	int need_retune = 0;
+	bool retune_release = false;
 
 	brcmf_dbg(TRACE, "Enter: on=%d\n", on);
 
+	/* Cannot re-tune if device is asleep */
+	if (on) {
+		need_retune = sdio_retune_get_needed(bus->sdiodev->func1); // TODO: host->can_retune ? host->need_retune : 0
+		sdio_retune_hold_now(bus->sdiodev->func1); // TODO: add sdio_retune_hold_now()
+		retune_release = true;
+	}
+
 	wr_val = (on << SBSDIO_FUNC1_SLEEPCSR_KSO_SHIFT);
 	/* 1st KSO write goes to AOS wake up core if device is asleep  */
 	brcmf_sdiod_writeb(bus->sdiodev, SBSDIO_FUNC1_SLEEPCSR, wr_val, &err);
@@ -711,8 +720,16 @@ brcmf_sdio_kso_control(struct brcmf_sdio *bus, bool on)
 			err_cnt = 0;
 		}
 		/* bail out upon subsequent access errors */
-		if (err && (err_cnt++ > BRCMF_SDIO_MAX_ACCESS_ERRORS))
-			break;
+		if (err && (err_cnt++ > BRCMF_SDIO_MAX_ACCESS_ERRORS)) {
+			if (!retune_release)
+				break;
+			/*
+			 * Allow one more retry with re-tuning released in case
+			 * it helps.
+			 */
+			sdio_retune_release(bus->sdiodev->func1);
+			retune_release = false;
+		}
 
 		udelay(KSO_WAIT_US);
 		brcmf_sdiod_writeb(bus->sdiodev, SBSDIO_FUNC1_SLEEPCSR, wr_val,
@@ -727,6 +744,18 @@ brcmf_sdio_kso_control(struct brcmf_sdio *bus, bool on)
 	if (try_cnt > MAX_KSO_ATTEMPTS)
 		brcmf_err("max tries: rd_val=0x%x err=%d\n", rd_val, err);
 
+	if (retune_release) {
+		/*
+		 * CRC errors are not unexpected during the transition but they
+		 * also trigger re-tuning. Clear that here to avoid an
+		 * unnecessary re-tune if it wasn't already triggered to start
+		 * with.
+		 */
+		if (!need_retune)
+			sdio_retune_clear_needed(bus->sdiodev->func1); // TODO: host->need_retune = 0
+		sdio_retune_release(bus->sdiodev->func1); // TODO: add sdio_retune_release()
+	}
+
 	return err;
 }
 

^ permalink raw reply related

* Update to wireless-regdb about Italy
From: Giorgio Bernardi @ 2019-06-06 13:49 UTC (permalink / raw)
  To: linux-wireless

Hi

I'm Italian and I work in Italy, I realize that nowadays the regdb entry 
for Italy is as follows:

country IT: DFS-ETSI
     (2402 - 2482 @ 40), (20)
     (5170 - 5250 @ 80), (20), AUTO-BW, wmmrule=ETSI
     (5250 - 5330 @ 80), (20), DFS, AUTO-BW, wmmrule=ETSI
     (5490 - 5710 @ 160), (27), DFS, wmmrule=ETSI
     # 60 GHz band channels 1-4, ref: Etsi En 302 567
     (57000 - 66000 @ 2160), (40)

And it misses the lines:

     # Short Range Devices (SRD) (ETSI EN 300 440)
     (5725 - 5875 @ 80), (25 mW)

Common to may European Countries.

I dug a bit in the current Italian regulation that is online on the 
website of the
Ministry of Economic Development: https://www.mise.gov.it/index.php/en/

In the section about the "National Plan of Frequencies" (only in 
Italian) at the URL:

https://www.mise.gov.it/index.php/it/comunicazioni/radio/pnrf-piano-nazionale-di-ripartizione-delle-frequenze

Two PDF files are linked:

--Tabelle di attribuzione Tabella B (27,50 MHz – 10.000 MHz) (pdf)
https://www.mise.gov.it/images/stories/documenti/Tabella_B_2750_MHz-10000_Mhz.pdf
Which at page 28 allows the use for ISM according to the general 
European legislation: 2006/771/CE ERC/REC 70-03

--Note (esplicative, di carattere tecnico e con attribuzioni in deroga 
al piano) (pdf)
https://www.mise.gov.it/images/stories/documenti/NOTE-pnrf.pdf
Which at page 334, in the paragraph 3.2.3 states in a explicit way the 
permit to operate the in the band 5.725 ÷ 5.875 MHz,
with SRD and max power at 25 mW.

According to this regulation there's no reason not to have the:
  (5725 - 5875 @ 80), (25 mW)
Inserted for Italy in the regdb

Who can do it ?

-- 

Giorgio Bernardi


^ permalink raw reply

* [PATCH] mt7601u: do not schedule rx_tasklet when the device has been disconnected
From: Lorenzo Bianconi @ 2019-06-06 12:26 UTC (permalink / raw)
  To: kubakici; +Cc: linux-wireless, lorenzo.bianconi

Do not schedule rx_tasklet when the usb dongle is disconnected. This
patch fixes the common kernel warning reported when the device is
removed.

[   24.921354] usb 3-14: USB disconnect, device number 7
[   24.921593] ------------[ cut here ]------------
[   24.921594] RX urb mismatch
[   24.921675] WARNING: CPU: 4 PID: 163 at drivers/net/wireless/mediatek/mt7601u/dma.c:200 mt7601u_complete_rx+0xcb/0xd0 [mt7601u]
[   24.921769] CPU: 4 PID: 163 Comm: kworker/4:2 Tainted: G           OE     4.19.31-041931-generic #201903231635
[   24.921770] Hardware name: To Be Filled By O.E.M. To Be Filled By O.E.M./Z97 Extreme4, BIOS P1.30 05/23/2014
[   24.921782] Workqueue: usb_hub_wq hub_event
[   24.921797] RIP: 0010:mt7601u_complete_rx+0xcb/0xd0 [mt7601u]
[   24.921800] RSP: 0018:ffff9bd9cfd03d08 EFLAGS: 00010086
[   24.921802] RAX: 0000000000000000 RBX: ffff9bd9bf043540 RCX: 0000000000000006
[   24.921803] RDX: 0000000000000007 RSI: 0000000000000096 RDI: ffff9bd9cfd16420
[   24.921804] RBP: ffff9bd9cfd03d28 R08: 0000000000000002 R09: 00000000000003a8
[   24.921805] R10: 0000002f485fca34 R11: 0000000000000000 R12: ffff9bd9bf043c1c
[   24.921806] R13: ffff9bd9c62fa3c0 R14: 0000000000000082 R15: 0000000000000000
[   24.921807] FS:  0000000000000000(0000) GS:ffff9bd9cfd00000(0000) knlGS:0000000000000000
[   24.921808] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   24.921808] CR2: 00007fb2648b0000 CR3: 0000000142c0a004 CR4: 00000000001606e0
[   24.921809] Call Trace:
[   24.921812]  <IRQ>
[   24.921819]  __usb_hcd_giveback_urb+0x8b/0x140
[   24.921821]  usb_hcd_giveback_urb+0xca/0xe0
[   24.921828]  xhci_giveback_urb_in_irq.isra.42+0x82/0xf0
[   24.921834]  handle_cmd_completion+0xe02/0x10d0
[   24.921837]  xhci_irq+0x274/0x4a0
[   24.921838]  xhci_msi_irq+0x11/0x20
[   24.921851]  __handle_irq_event_percpu+0x44/0x190
[   24.921856]  handle_irq_event_percpu+0x32/0x80
[   24.921861]  handle_irq_event+0x3b/0x5a
[   24.921867]  handle_edge_irq+0x80/0x190
[   24.921874]  handle_irq+0x20/0x30
[   24.921889]  do_IRQ+0x4e/0xe0
[   24.921891]  common_interrupt+0xf/0xf
[   24.921892]  </IRQ>
[   24.921900] RIP: 0010:usb_hcd_flush_endpoint+0x78/0x180
[   24.921354] usb 3-14: USB disconnect, device number 7

Fixes: c869f77d6abb ("add mt7601u driver")
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
I will post a patch to fix tx side as well
---
 drivers/net/wireless/mediatek/mt7601u/dma.c | 33 ++++++++++-----------
 1 file changed, 16 insertions(+), 17 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt7601u/dma.c b/drivers/net/wireless/mediatek/mt7601u/dma.c
index f7edeffb2b19..e7703990b291 100644
--- a/drivers/net/wireless/mediatek/mt7601u/dma.c
+++ b/drivers/net/wireless/mediatek/mt7601u/dma.c
@@ -193,10 +193,20 @@ static void mt7601u_complete_rx(struct urb *urb)
 	struct mt7601u_rx_queue *q = &dev->rx_q;
 	unsigned long flags;
 
-	spin_lock_irqsave(&dev->rx_lock, flags);
+	switch (urb->status) {
+	case -ECONNRESET:
+	case -ESHUTDOWN:
+	case -ENOENT:
+		return;
+	default:
+		dev_err_ratelimited(dev->dev, "rx urb failed: %d\n",
+				    urb->status);
+		/* fall through */
+	case 0:
+		break;
+	}
 
-	if (mt7601u_urb_has_error(urb))
-		dev_err(dev->dev, "Error: RX urb failed:%d\n", urb->status);
+	spin_lock_irqsave(&dev->rx_lock, flags);
 	if (WARN_ONCE(q->e[q->end].urb != urb, "RX urb mismatch"))
 		goto out;
 
@@ -363,19 +373,10 @@ int mt7601u_dma_enqueue_tx(struct mt7601u_dev *dev, struct sk_buff *skb,
 static void mt7601u_kill_rx(struct mt7601u_dev *dev)
 {
 	int i;
-	unsigned long flags;
-
-	spin_lock_irqsave(&dev->rx_lock, flags);
 
-	for (i = 0; i < dev->rx_q.entries; i++) {
-		int next = dev->rx_q.end;
-
-		spin_unlock_irqrestore(&dev->rx_lock, flags);
-		usb_poison_urb(dev->rx_q.e[next].urb);
-		spin_lock_irqsave(&dev->rx_lock, flags);
-	}
-
-	spin_unlock_irqrestore(&dev->rx_lock, flags);
+	for (i = 0; i < dev->rx_q.entries; i++)
+		usb_poison_urb(dev->rx_q.e[i].urb);
+	tasklet_kill(&dev->rx_tasklet);
 }
 
 static int mt7601u_submit_rx_buf(struct mt7601u_dev *dev,
@@ -525,8 +526,6 @@ void mt7601u_dma_cleanup(struct mt7601u_dev *dev)
 {
 	mt7601u_kill_rx(dev);
 
-	tasklet_kill(&dev->rx_tasklet);
-
 	mt7601u_free_rx(dev);
 	mt7601u_free_tx(dev);
 
-- 
2.21.0


^ permalink raw reply related

* Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook
From: Christoph Hellwig @ 2019-06-06 11:43 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: Aaro Koskinen, Christoph Hellwig, Christian Zigotzky,
	Michael Ellerman, Larry Finger, linuxppc-dev, linux-wireless,
	linux-kernel
In-Reply-To: <f8df19ffe5b75537045119037459ae9ad4a1de39.camel@kernel.crashing.org>

On Thu, Jun 06, 2019 at 08:57:49PM +1000, Benjamin Herrenschmidt wrote:
> > Wow... that's an odd amount. One thing we could possibly do is add code
> > to limit the amount of RAM when we detect that device....
> 
> Sent too quickly... I mean that *or* force swiotlb at 30-bits on those systems based
> on detecting the presence of that device in the device-tree.

swiotlb doesn't really help you, as these days swiotlb on matters for
the dma_map* case.  What would help is a ZONE_DMA that covers these
devices.  No need to do the 24-bit x86 does, but 30-bit would do it.

WIP patch for testing below:

diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h
index b8286a2013b4..7a367ce87c41 100644
--- a/arch/powerpc/include/asm/page.h
+++ b/arch/powerpc/include/asm/page.h
@@ -319,6 +319,10 @@ struct vm_area_struct;
 #endif /* __ASSEMBLY__ */
 #include <asm/slice.h>
 
+#if 1 /* XXX: pmac?  dynamic discovery? */
+#define ARCH_ZONE_DMA_BITS 30
+#else
 #define ARCH_ZONE_DMA_BITS 31
+#endif
 
 #endif /* _ASM_POWERPC_PAGE_H */
diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index cba29131bccc..2540d3b2588c 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -248,7 +248,8 @@ void __init paging_init(void)
 	       (long int)((top_of_ram - total_ram) >> 20));
 
 #ifdef CONFIG_ZONE_DMA
-	max_zone_pfns[ZONE_DMA]	= min(max_low_pfn, 0x7fffffffUL >> PAGE_SHIFT);
+	max_zone_pfns[ZONE_DMA]	= min(max_low_pfn,
+			((1UL << ARCH_ZONE_DMA_BITS) - 1) >> PAGE_SHIFT);
 #endif
 	max_zone_pfns[ZONE_NORMAL] = max_low_pfn;
 #ifdef CONFIG_HIGHMEM

^ permalink raw reply related

* Re: KMSAN: uninit-value in rt2500usb_bbp_read
From: Alexander Potapenko @ 2019-06-06 10:59 UTC (permalink / raw)
  To: syzbot
  Cc: David Miller, helmut.schaa, kvalo, LKML, linux-wireless,
	Networking, sgruszka, syzkaller-bugs
In-Reply-To: <000000000000cf6a70058aa48695@google.com>

On Thu, Jun 6, 2019 at 11:42 AM syzbot
<syzbot+a106a5b084a6890d2607@syzkaller.appspotmail.com> wrote:
>
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit:    f75e4cfe kmsan: use kmsan_handle_urb() in urb.c
> git tree:       kmsan
> console output: https://syzkaller.appspot.com/x/log.txt?x=12f8b01ea00000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=602468164ccdc30a
> dashboard link: https://syzkaller.appspot.com/bug?extid=a106a5b084a6890d2607
> compiler:       clang version 9.0.0 (/home/glider/llvm/clang
> 06d00afa61eef8f7f501ebdb4e8612ea43ec2d78)
> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=14f746f2a00000
> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=153072d2a00000
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+a106a5b084a6890d2607@syzkaller.appspotmail.com
>
> usb 1-1: New USB device strings: Mfr=0, Product=0, SerialNumber=0
> usb 1-1: config 0 descriptor??
> usb 1-1: reset high-speed USB device number 2 using dummy_hcd
> usb 1-1: device descriptor read/64, error -71
> ieee80211 phy3: rt2x00usb_vendor_request: Error - Vendor Request 0x09
> failed for offset 0x0000 with error -71
> ieee80211 phy3: rt2x00usb_vendor_request: Error - Vendor Request 0x07
As this line suggests:
> failed for offset 0x04d0 with error -71
, rt2x00usb_vendor_req_buff_lock() fails with error -71, which means
it doesn't initialize the buffer.
But rt2500usb_register_read_lock() ignores that status code and just
assumes the data is always initialized.
> ==================================================================
> BUG: KMSAN: uninit-value in rt2500usb_regbusy_read
> drivers/net/wireless/ralink/rt2x00/rt2500usb.c:116 [inline]
> BUG: KMSAN: uninit-value in rt2500usb_bbp_read+0x174/0x640
> drivers/net/wireless/ralink/rt2x00/rt2500usb.c:172
> CPU: 1 PID: 4943 Comm: kworker/1:2 Not tainted 5.1.0+ #1
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> Google 01/01/2011
> Workqueue: usb_hub_wq hub_event
> Call Trace:
>   __dump_stack lib/dump_stack.c:77 [inline]
>   dump_stack+0x191/0x1f0 lib/dump_stack.c:113
>   kmsan_report+0x130/0x2a0 mm/kmsan/kmsan.c:622
>   __msan_warning+0x75/0xe0 mm/kmsan/kmsan_instr.c:310
>   rt2500usb_regbusy_read drivers/net/wireless/ralink/rt2x00/rt2500usb.c:116
> [inline]
>   rt2500usb_bbp_read+0x174/0x640
> drivers/net/wireless/ralink/rt2x00/rt2500usb.c:172
>   rt2500usb_validate_eeprom
> drivers/net/wireless/ralink/rt2x00/rt2500usb.c:1387 [inline]
>   rt2500usb_probe_hw+0x3b1/0x2230
> drivers/net/wireless/ralink/rt2x00/rt2500usb.c:1764
>   rt2x00lib_probe_dev+0xb81/0x3090
> drivers/net/wireless/ralink/rt2x00/rt2x00dev.c:1427
>   rt2x00usb_probe+0x7c7/0xf70
> drivers/net/wireless/ralink/rt2x00/rt2x00usb.c:837
>   rt2500usb_probe+0x50/0x60
> drivers/net/wireless/ralink/rt2x00/rt2500usb.c:1977
>   usb_probe_interface+0xd66/0x1320 drivers/usb/core/driver.c:361
>   really_probe+0xdae/0x1d80 drivers/base/dd.c:513
>   driver_probe_device+0x1b3/0x4f0 drivers/base/dd.c:671
>   __device_attach_driver+0x5b8/0x790 drivers/base/dd.c:778
>   bus_for_each_drv+0x28e/0x3b0 drivers/base/bus.c:454
>   __device_attach+0x454/0x730 drivers/base/dd.c:844
>   device_initial_probe+0x4a/0x60 drivers/base/dd.c:891
>   bus_probe_device+0x137/0x390 drivers/base/bus.c:514
>   device_add+0x288d/0x30e0 drivers/base/core.c:2106
>   usb_set_configuration+0x30dc/0x3750 drivers/usb/core/message.c:2027
>   generic_probe+0xe7/0x280 drivers/usb/core/generic.c:210
>   usb_probe_device+0x14c/0x200 drivers/usb/core/driver.c:266
>   really_probe+0xdae/0x1d80 drivers/base/dd.c:513
>   driver_probe_device+0x1b3/0x4f0 drivers/base/dd.c:671
>   __device_attach_driver+0x5b8/0x790 drivers/base/dd.c:778
>   bus_for_each_drv+0x28e/0x3b0 drivers/base/bus.c:454
>   __device_attach+0x454/0x730 drivers/base/dd.c:844
>   device_initial_probe+0x4a/0x60 drivers/base/dd.c:891
>   bus_probe_device+0x137/0x390 drivers/base/bus.c:514
>   device_add+0x288d/0x30e0 drivers/base/core.c:2106
>   usb_new_device+0x23e5/0x2ff0 drivers/usb/core/hub.c:2534
>   hub_port_connect drivers/usb/core/hub.c:5089 [inline]
>   hub_port_connect_change drivers/usb/core/hub.c:5204 [inline]
>   port_event drivers/usb/core/hub.c:5350 [inline]
>   hub_event+0x48d1/0x7290 drivers/usb/core/hub.c:5432
>   process_one_work+0x1572/0x1f00 kernel/workqueue.c:2269
>   worker_thread+0x111b/0x2460 kernel/workqueue.c:2415
>   kthread+0x4b5/0x4f0 kernel/kthread.c:254
>   ret_from_fork+0x35/0x40 arch/x86/entry/entry_64.S:355
>
> Local variable description: ----reg.i.i@rt2500usb_bbp_read
> Variable was created at:
>   rt2500usb_register_read_lock
> drivers/net/wireless/ralink/rt2x00/rt2500usb.c:72 [inline]
>   rt2500usb_regbusy_read drivers/net/wireless/ralink/rt2x00/rt2500usb.c:115
> [inline]
>   rt2500usb_bbp_read+0xa4/0x640
> drivers/net/wireless/ralink/rt2x00/rt2500usb.c:172
>   rt2500usb_validate_eeprom
> drivers/net/wireless/ralink/rt2x00/rt2500usb.c:1387 [inline]
>   rt2500usb_probe_hw+0x3b1/0x2230
> drivers/net/wireless/ralink/rt2x00/rt2500usb.c:1764
> ==================================================================
>
>
> ---
> This bug is generated by a bot. It may contain errors.
> See https://goo.gl/tpsmEJ for more information about syzbot.
> syzbot engineers can be reached at syzkaller@googlegroups.com.
>
> syzbot will keep track of this bug report. See:
> https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
> syzbot can test patches for this bug, for details see:
> https://goo.gl/tpsmEJ#testing-patches



-- 
Alexander Potapenko
Software Engineer

Google Germany GmbH
Erika-Mann-Straße, 33
80636 München

Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg

^ permalink raw reply

* Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook
From: Benjamin Herrenschmidt @ 2019-06-06 10:57 UTC (permalink / raw)
  To: Aaro Koskinen
  Cc: Christoph Hellwig, Christian Zigotzky, Michael Ellerman,
	Larry Finger, linuxppc-dev, linux-wireless, linux-kernel
In-Reply-To: <d87ac9a7faac0d5522cb496d74afc586410fed9c.camel@kernel.crashing.org>

On Thu, 2019-06-06 at 20:56 +1000, Benjamin Herrenschmidt wrote:
> On Thu, 2019-06-06 at 12:31 +0300, Aaro Koskinen wrote:
> > Hi,
> > 
> > On Thu, Jun 06, 2019 at 10:54:51AM +1000, Benjamin Herrenschmidt
> > wrote:
> > > On Thu, 2019-06-06 at 01:50 +0300, Aaro Koskinen wrote:
> > > > Hi,
> > > > 
> > > > When upgrading from v5.0 -> v5.1 on G4 PowerBook, I noticed WLAN
> > > > does
> > > > not work anymore:
> > > > 
> > > > [   42.004303] b43legacy-phy0: Loading firmware version 0x127,
> > > > patch level 14 (2005-04-18 02:36:27)
> > > > [   42.184837] b43legacy-phy0 debug: Chip initialized
> > > > [   42.184873] b43legacy-phy0 ERROR: The machine/kernel does not
> > > > support the required 30-bit DMA mask
> > > > 
> > > > The same happens with the current mainline.
> > > 
> > > How much RAM do you have ?
> > 
> > The system has 1129 MB RAM. Booting with mem=1G makes it work.
> 
> Wow... that's an odd amount. One thing we could possibly do is add code
> to limit the amount of RAM when we detect that device....

Sent too quickly... I mean that *or* force swiotlb at 30-bits on those systems based
on detecting the presence of that device in the device-tree.

Cheers,
Ben.



^ permalink raw reply

* Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook
From: Benjamin Herrenschmidt @ 2019-06-06 10:56 UTC (permalink / raw)
  To: Aaro Koskinen
  Cc: Christoph Hellwig, Christian Zigotzky, Michael Ellerman,
	Larry Finger, linuxppc-dev, linux-wireless, linux-kernel
In-Reply-To: <20190606093149.GA11598@darkstar.musicnaut.iki.fi>

On Thu, 2019-06-06 at 12:31 +0300, Aaro Koskinen wrote:
> Hi,
> 
> On Thu, Jun 06, 2019 at 10:54:51AM +1000, Benjamin Herrenschmidt
> wrote:
> > On Thu, 2019-06-06 at 01:50 +0300, Aaro Koskinen wrote:
> > > Hi,
> > > 
> > > When upgrading from v5.0 -> v5.1 on G4 PowerBook, I noticed WLAN
> > > does
> > > not work anymore:
> > > 
> > > [   42.004303] b43legacy-phy0: Loading firmware version 0x127,
> > > patch level 14 (2005-04-18 02:36:27)
> > > [   42.184837] b43legacy-phy0 debug: Chip initialized
> > > [   42.184873] b43legacy-phy0 ERROR: The machine/kernel does not
> > > support the required 30-bit DMA mask
> > > 
> > > The same happens with the current mainline.
> > 
> > How much RAM do you have ?
> 
> The system has 1129 MB RAM. Booting with mem=1G makes it work.

Wow... that's an odd amount. One thing we could possibly do is add code
to limit the amount of RAM when we detect that device....

Cheers,
Ben.



^ permalink raw reply

* Re: [PATCH v3 1/2] mt76: mt7615: enable support for mesh
From: Sebastian Gottschall @ 2019-06-06 10:14 UTC (permalink / raw)
  To: Ryder Lee, Felix Fietkau, Lorenzo Bianconi
  Cc: Roy Luo, YF Luo, Yiwei Chung, Sean Wang, Chih-Min Chen,
	linux-wireless, linux-mediatek, linux-kernel
In-Reply-To: <a1ff446dfc06e2443552e7ec2d754099aacce7df.1559541944.git.ryder.lee@mediatek.com>

in addition you should take care about this problem which is raised up 
if SAE is used. since AES-CMAC required tid to be non zero

WARNING: CPU: 2 PID: 15324 at 
/home/seg/DEV/mt7621/src/router/private/compat-wireless-2017-09-03/net/mac80211/key.c:1096 
mt76_wcid_key_setup+0x58/0x9c [mt76]
Modules linked in: shortcut_fe gcm ghash_generic ctr gf128mul mt7615e 
mt76 mac80211 compat
CPU: 2 PID: 15324 Comm: wpa_supplicant Tainted: G        W 4.14.123 #106
Stack : 00000000 87c2d000 00000000 8007d8b4 80480000 80482b9c 80610000 
805a4390
         8057e2b4 854fb99c 87ed045c 805e4767 80578288 00000001 854fb940 
805e9f78
         00000000 00000000 80640000 00000000 81147bb8 00000584 00000007 
00000000
         00000000 80650000 80650000 20202020 80000000 00000000 80610000 
872b9fe0
         872a2b14 00000448 00000000 87c2d000 00000010 8022d660 00000008 
80640008
         ...
Call Trace:
[<800153e0>] show_stack+0x58/0x100
[<8042e83c>] dump_stack+0x9c/0xe0
[<800349f0>] __warn+0xe4/0x144
[<8003468c>] warn_slowpath_null+0x1c/0x30
[<872b9fe0>] mt76_wcid_key_setup+0x58/0x9c [mt76]
[<87611690>] mt7615_eeprom_init+0x7b4/0xe9c [mt7615e]
---[ end trace e24aeb4b542e0dea ]---

Am 03.06.2019 um 08:08 schrieb Ryder Lee:
> Enable NL80211_IFTYPE_MESH_POINT and update its path.
>
> Signed-off-by: Ryder Lee <ryder.lee@mediatek.com>
> ---
> Changes since v3 - fix a wrong expression
> Changes since v2 - remove unused definitions
> ---
>   drivers/net/wireless/mediatek/mt76/mt7615/init.c | 6 ++++++
>   drivers/net/wireless/mediatek/mt76/mt7615/main.c | 1 +
>   drivers/net/wireless/mediatek/mt76/mt7615/mcu.c  | 4 +++-
>   drivers/net/wireless/mediatek/mt76/mt7615/mcu.h  | 6 ------
>   4 files changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/init.c b/drivers/net/wireless/mediatek/mt76/mt7615/init.c
> index 59f604f3161f..f860af6a42da 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7615/init.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7615/init.c
> @@ -133,6 +133,9 @@ static const struct ieee80211_iface_limit if_limits[] = {
>   	{
>   		.max = MT7615_MAX_INTERFACES,
>   		.types = BIT(NL80211_IFTYPE_AP) |
> +#ifdef CONFIG_MAC80211_MESH
> +			 BIT(NL80211_IFTYPE_MESH_POINT) |
> +#endif
>   			 BIT(NL80211_IFTYPE_STATION)
>   	}
>   };
> @@ -195,6 +198,9 @@ int mt7615_register_device(struct mt7615_dev *dev)
>   	dev->mt76.antenna_mask = 0xf;
>   
>   	wiphy->interface_modes = BIT(NL80211_IFTYPE_STATION) |
> +#ifdef CONFIG_MAC80211_MESH
> +				 BIT(NL80211_IFTYPE_MESH_POINT) |
> +#endif
>   				 BIT(NL80211_IFTYPE_AP);
>   
>   	ret = mt76_register_device(&dev->mt76, true, mt7615_rates,
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/main.c b/drivers/net/wireless/mediatek/mt76/mt7615/main.c
> index b0bb7cc12385..585e67fa2728 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7615/main.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7615/main.c
> @@ -37,6 +37,7 @@ static int get_omac_idx(enum nl80211_iftype type, u32 mask)
>   
>   	switch (type) {
>   	case NL80211_IFTYPE_AP:
> +	case NL80211_IFTYPE_MESH_POINT:
>   		/* ap use hw bssid 0 and ext bssid */
>   		if (~mask & BIT(HW_BSSID_0))
>   			return HW_BSSID_0;
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
> index 43f70195244c..e82297048449 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
> @@ -754,6 +754,7 @@ int mt7615_mcu_set_bss_info(struct mt7615_dev *dev,
>   
>   	switch (vif->type) {
>   	case NL80211_IFTYPE_AP:
> +	case NL80211_IFTYPE_MESH_POINT:
>   		tx_wlan_idx = mvif->sta.wcid.idx;
>   		conn_type = CONNECTION_INFRA_AP;
>   		break;
> @@ -968,7 +969,7 @@ int mt7615_mcu_add_wtbl(struct mt7615_dev *dev, struct ieee80211_vif *vif,
>   		.rx_wtbl = {
>   			.tag = cpu_to_le16(WTBL_RX),
>   			.len = cpu_to_le16(sizeof(struct wtbl_rx)),
> -			.rca1 = vif->type != NL80211_IFTYPE_AP,
> +			.rca1 = vif->type == NL80211_IFTYPE_STATION,
>   			.rca2 = 1,
>   			.rv = 1,
>   		},
> @@ -1042,6 +1043,7 @@ static void sta_rec_convert_vif_type(enum nl80211_iftype type, u32 *conn_type)
>   {
>   	switch (type) {
>   	case NL80211_IFTYPE_AP:
> +	case NL80211_IFTYPE_MESH_POINT:
>   		if (conn_type)
>   			*conn_type = CONNECTION_INFRA_STA;
>   		break;
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.h b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.h
> index e96efb13fa4d..0915cb735699 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.h
> +++ b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.h
> @@ -105,25 +105,19 @@ enum {
>   #define STA_TYPE_STA		BIT(0)
>   #define STA_TYPE_AP		BIT(1)
>   #define STA_TYPE_ADHOC		BIT(2)
> -#define STA_TYPE_TDLS		BIT(3)
>   #define STA_TYPE_WDS		BIT(4)
>   #define STA_TYPE_BC		BIT(5)
>   
>   #define NETWORK_INFRA		BIT(16)
>   #define NETWORK_P2P		BIT(17)
>   #define NETWORK_IBSS		BIT(18)
> -#define NETWORK_MESH		BIT(19)
> -#define NETWORK_BOW		BIT(20)
>   #define NETWORK_WDS		BIT(21)
>   
>   #define CONNECTION_INFRA_STA	(STA_TYPE_STA | NETWORK_INFRA)
>   #define CONNECTION_INFRA_AP	(STA_TYPE_AP | NETWORK_INFRA)
>   #define CONNECTION_P2P_GC	(STA_TYPE_STA | NETWORK_P2P)
>   #define CONNECTION_P2P_GO	(STA_TYPE_AP | NETWORK_P2P)
> -#define CONNECTION_MESH_STA	(STA_TYPE_STA | NETWORK_MESH)
> -#define CONNECTION_MESH_AP	(STA_TYPE_AP | NETWORK_MESH)
>   #define CONNECTION_IBSS_ADHOC	(STA_TYPE_ADHOC | NETWORK_IBSS)
> -#define CONNECTION_TDLS		(STA_TYPE_STA | NETWORK_INFRA | STA_TYPE_TDLS)
>   #define CONNECTION_WDS		(STA_TYPE_WDS | NETWORK_WDS)
>   #define CONNECTION_INFRA_BC	(STA_TYPE_BC | NETWORK_INFRA)
>   

^ permalink raw reply

* Re: [PATCH v3 1/2] mt76: mt7615: enable support for mesh
From: Sebastian Gottschall @ 2019-06-06 10:11 UTC (permalink / raw)
  To: Ryder Lee, Felix Fietkau, Lorenzo Bianconi
  Cc: Roy Luo, YF Luo, Yiwei Chung, Sean Wang, Chih-Min Chen,
	linux-wireless, linux-mediatek, linux-kernel
In-Reply-To: <a1ff446dfc06e2443552e7ec2d754099aacce7df.1559541944.git.ryder.lee@mediatek.com>

i tested your patch against a qca 9984 chipset using SAE and without 
encryption. both did not work. the devices are connecting, but no data 
connection is possible


Sebastian

Am 03.06.2019 um 08:08 schrieb Ryder Lee:
> Enable NL80211_IFTYPE_MESH_POINT and update its path.
>
> Signed-off-by: Ryder Lee <ryder.lee@mediatek.com>
> ---
> Changes since v3 - fix a wrong expression
> Changes since v2 - remove unused definitions
> ---
>   drivers/net/wireless/mediatek/mt76/mt7615/init.c | 6 ++++++
>   drivers/net/wireless/mediatek/mt76/mt7615/main.c | 1 +
>   drivers/net/wireless/mediatek/mt76/mt7615/mcu.c  | 4 +++-
>   drivers/net/wireless/mediatek/mt76/mt7615/mcu.h  | 6 ------
>   4 files changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/init.c b/drivers/net/wireless/mediatek/mt76/mt7615/init.c
> index 59f604f3161f..f860af6a42da 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7615/init.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7615/init.c
> @@ -133,6 +133,9 @@ static const struct ieee80211_iface_limit if_limits[] = {
>   	{
>   		.max = MT7615_MAX_INTERFACES,
>   		.types = BIT(NL80211_IFTYPE_AP) |
> +#ifdef CONFIG_MAC80211_MESH
> +			 BIT(NL80211_IFTYPE_MESH_POINT) |
> +#endif
>   			 BIT(NL80211_IFTYPE_STATION)
>   	}
>   };
> @@ -195,6 +198,9 @@ int mt7615_register_device(struct mt7615_dev *dev)
>   	dev->mt76.antenna_mask = 0xf;
>   
>   	wiphy->interface_modes = BIT(NL80211_IFTYPE_STATION) |
> +#ifdef CONFIG_MAC80211_MESH
> +				 BIT(NL80211_IFTYPE_MESH_POINT) |
> +#endif
>   				 BIT(NL80211_IFTYPE_AP);
>   
>   	ret = mt76_register_device(&dev->mt76, true, mt7615_rates,
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/main.c b/drivers/net/wireless/mediatek/mt76/mt7615/main.c
> index b0bb7cc12385..585e67fa2728 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7615/main.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7615/main.c
> @@ -37,6 +37,7 @@ static int get_omac_idx(enum nl80211_iftype type, u32 mask)
>   
>   	switch (type) {
>   	case NL80211_IFTYPE_AP:
> +	case NL80211_IFTYPE_MESH_POINT:
>   		/* ap use hw bssid 0 and ext bssid */
>   		if (~mask & BIT(HW_BSSID_0))
>   			return HW_BSSID_0;
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
> index 43f70195244c..e82297048449 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
> @@ -754,6 +754,7 @@ int mt7615_mcu_set_bss_info(struct mt7615_dev *dev,
>   
>   	switch (vif->type) {
>   	case NL80211_IFTYPE_AP:
> +	case NL80211_IFTYPE_MESH_POINT:
>   		tx_wlan_idx = mvif->sta.wcid.idx;
>   		conn_type = CONNECTION_INFRA_AP;
>   		break;
> @@ -968,7 +969,7 @@ int mt7615_mcu_add_wtbl(struct mt7615_dev *dev, struct ieee80211_vif *vif,
>   		.rx_wtbl = {
>   			.tag = cpu_to_le16(WTBL_RX),
>   			.len = cpu_to_le16(sizeof(struct wtbl_rx)),
> -			.rca1 = vif->type != NL80211_IFTYPE_AP,
> +			.rca1 = vif->type == NL80211_IFTYPE_STATION,
>   			.rca2 = 1,
>   			.rv = 1,
>   		},
> @@ -1042,6 +1043,7 @@ static void sta_rec_convert_vif_type(enum nl80211_iftype type, u32 *conn_type)
>   {
>   	switch (type) {
>   	case NL80211_IFTYPE_AP:
> +	case NL80211_IFTYPE_MESH_POINT:
>   		if (conn_type)
>   			*conn_type = CONNECTION_INFRA_STA;
>   		break;
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.h b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.h
> index e96efb13fa4d..0915cb735699 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.h
> +++ b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.h
> @@ -105,25 +105,19 @@ enum {
>   #define STA_TYPE_STA		BIT(0)
>   #define STA_TYPE_AP		BIT(1)
>   #define STA_TYPE_ADHOC		BIT(2)
> -#define STA_TYPE_TDLS		BIT(3)
>   #define STA_TYPE_WDS		BIT(4)
>   #define STA_TYPE_BC		BIT(5)
>   
>   #define NETWORK_INFRA		BIT(16)
>   #define NETWORK_P2P		BIT(17)
>   #define NETWORK_IBSS		BIT(18)
> -#define NETWORK_MESH		BIT(19)
> -#define NETWORK_BOW		BIT(20)
>   #define NETWORK_WDS		BIT(21)
>   
>   #define CONNECTION_INFRA_STA	(STA_TYPE_STA | NETWORK_INFRA)
>   #define CONNECTION_INFRA_AP	(STA_TYPE_AP | NETWORK_INFRA)
>   #define CONNECTION_P2P_GC	(STA_TYPE_STA | NETWORK_P2P)
>   #define CONNECTION_P2P_GO	(STA_TYPE_AP | NETWORK_P2P)
> -#define CONNECTION_MESH_STA	(STA_TYPE_STA | NETWORK_MESH)
> -#define CONNECTION_MESH_AP	(STA_TYPE_AP | NETWORK_MESH)
>   #define CONNECTION_IBSS_ADHOC	(STA_TYPE_ADHOC | NETWORK_IBSS)
> -#define CONNECTION_TDLS		(STA_TYPE_STA | NETWORK_INFRA | STA_TYPE_TDLS)
>   #define CONNECTION_WDS		(STA_TYPE_WDS | NETWORK_WDS)
>   #define CONNECTION_INFRA_BC	(STA_TYPE_BC | NETWORK_INFRA)
>   

^ permalink raw reply

* KMSAN: uninit-value in rt2500usb_bbp_read
From: syzbot @ 2019-06-06  9:42 UTC (permalink / raw)
  To: davem, glider, helmut.schaa, kvalo, linux-kernel, linux-wireless,
	netdev, sgruszka, syzkaller-bugs

Hello,

syzbot found the following crash on:

HEAD commit:    f75e4cfe kmsan: use kmsan_handle_urb() in urb.c
git tree:       kmsan
console output: https://syzkaller.appspot.com/x/log.txt?x=12f8b01ea00000
kernel config:  https://syzkaller.appspot.com/x/.config?x=602468164ccdc30a
dashboard link: https://syzkaller.appspot.com/bug?extid=a106a5b084a6890d2607
compiler:       clang version 9.0.0 (/home/glider/llvm/clang  
06d00afa61eef8f7f501ebdb4e8612ea43ec2d78)
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=14f746f2a00000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=153072d2a00000

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+a106a5b084a6890d2607@syzkaller.appspotmail.com

usb 1-1: New USB device strings: Mfr=0, Product=0, SerialNumber=0
usb 1-1: config 0 descriptor??
usb 1-1: reset high-speed USB device number 2 using dummy_hcd
usb 1-1: device descriptor read/64, error -71
ieee80211 phy3: rt2x00usb_vendor_request: Error - Vendor Request 0x09  
failed for offset 0x0000 with error -71
ieee80211 phy3: rt2x00usb_vendor_request: Error - Vendor Request 0x07  
failed for offset 0x04d0 with error -71
==================================================================
BUG: KMSAN: uninit-value in rt2500usb_regbusy_read  
drivers/net/wireless/ralink/rt2x00/rt2500usb.c:116 [inline]
BUG: KMSAN: uninit-value in rt2500usb_bbp_read+0x174/0x640  
drivers/net/wireless/ralink/rt2x00/rt2500usb.c:172
CPU: 1 PID: 4943 Comm: kworker/1:2 Not tainted 5.1.0+ #1
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011
Workqueue: usb_hub_wq hub_event
Call Trace:
  __dump_stack lib/dump_stack.c:77 [inline]
  dump_stack+0x191/0x1f0 lib/dump_stack.c:113
  kmsan_report+0x130/0x2a0 mm/kmsan/kmsan.c:622
  __msan_warning+0x75/0xe0 mm/kmsan/kmsan_instr.c:310
  rt2500usb_regbusy_read drivers/net/wireless/ralink/rt2x00/rt2500usb.c:116  
[inline]
  rt2500usb_bbp_read+0x174/0x640  
drivers/net/wireless/ralink/rt2x00/rt2500usb.c:172
  rt2500usb_validate_eeprom  
drivers/net/wireless/ralink/rt2x00/rt2500usb.c:1387 [inline]
  rt2500usb_probe_hw+0x3b1/0x2230  
drivers/net/wireless/ralink/rt2x00/rt2500usb.c:1764
  rt2x00lib_probe_dev+0xb81/0x3090  
drivers/net/wireless/ralink/rt2x00/rt2x00dev.c:1427
  rt2x00usb_probe+0x7c7/0xf70  
drivers/net/wireless/ralink/rt2x00/rt2x00usb.c:837
  rt2500usb_probe+0x50/0x60  
drivers/net/wireless/ralink/rt2x00/rt2500usb.c:1977
  usb_probe_interface+0xd66/0x1320 drivers/usb/core/driver.c:361
  really_probe+0xdae/0x1d80 drivers/base/dd.c:513
  driver_probe_device+0x1b3/0x4f0 drivers/base/dd.c:671
  __device_attach_driver+0x5b8/0x790 drivers/base/dd.c:778
  bus_for_each_drv+0x28e/0x3b0 drivers/base/bus.c:454
  __device_attach+0x454/0x730 drivers/base/dd.c:844
  device_initial_probe+0x4a/0x60 drivers/base/dd.c:891
  bus_probe_device+0x137/0x390 drivers/base/bus.c:514
  device_add+0x288d/0x30e0 drivers/base/core.c:2106
  usb_set_configuration+0x30dc/0x3750 drivers/usb/core/message.c:2027
  generic_probe+0xe7/0x280 drivers/usb/core/generic.c:210
  usb_probe_device+0x14c/0x200 drivers/usb/core/driver.c:266
  really_probe+0xdae/0x1d80 drivers/base/dd.c:513
  driver_probe_device+0x1b3/0x4f0 drivers/base/dd.c:671
  __device_attach_driver+0x5b8/0x790 drivers/base/dd.c:778
  bus_for_each_drv+0x28e/0x3b0 drivers/base/bus.c:454
  __device_attach+0x454/0x730 drivers/base/dd.c:844
  device_initial_probe+0x4a/0x60 drivers/base/dd.c:891
  bus_probe_device+0x137/0x390 drivers/base/bus.c:514
  device_add+0x288d/0x30e0 drivers/base/core.c:2106
  usb_new_device+0x23e5/0x2ff0 drivers/usb/core/hub.c:2534
  hub_port_connect drivers/usb/core/hub.c:5089 [inline]
  hub_port_connect_change drivers/usb/core/hub.c:5204 [inline]
  port_event drivers/usb/core/hub.c:5350 [inline]
  hub_event+0x48d1/0x7290 drivers/usb/core/hub.c:5432
  process_one_work+0x1572/0x1f00 kernel/workqueue.c:2269
  worker_thread+0x111b/0x2460 kernel/workqueue.c:2415
  kthread+0x4b5/0x4f0 kernel/kthread.c:254
  ret_from_fork+0x35/0x40 arch/x86/entry/entry_64.S:355

Local variable description: ----reg.i.i@rt2500usb_bbp_read
Variable was created at:
  rt2500usb_register_read_lock  
drivers/net/wireless/ralink/rt2x00/rt2500usb.c:72 [inline]
  rt2500usb_regbusy_read drivers/net/wireless/ralink/rt2x00/rt2500usb.c:115  
[inline]
  rt2500usb_bbp_read+0xa4/0x640  
drivers/net/wireless/ralink/rt2x00/rt2500usb.c:172
  rt2500usb_validate_eeprom  
drivers/net/wireless/ralink/rt2x00/rt2500usb.c:1387 [inline]
  rt2500usb_probe_hw+0x3b1/0x2230  
drivers/net/wireless/ralink/rt2x00/rt2500usb.c:1764
==================================================================


---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches

^ permalink raw reply

* Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook
From: Aaro Koskinen @ 2019-06-06  9:31 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: Christoph Hellwig, Christian Zigotzky, Michael Ellerman,
	Larry Finger, linuxppc-dev, linux-wireless, linux-kernel
In-Reply-To: <dfe6451c93574b61d4bdde4a05c5f8ccf86b31a0.camel@kernel.crashing.org>

Hi,

On Thu, Jun 06, 2019 at 10:54:51AM +1000, Benjamin Herrenschmidt wrote:
> On Thu, 2019-06-06 at 01:50 +0300, Aaro Koskinen wrote:
> > Hi,
> > 
> > When upgrading from v5.0 -> v5.1 on G4 PowerBook, I noticed WLAN does
> > not work anymore:
> > 
> > [   42.004303] b43legacy-phy0: Loading firmware version 0x127, patch level 14 (2005-04-18 02:36:27)
> > [   42.184837] b43legacy-phy0 debug: Chip initialized
> > [   42.184873] b43legacy-phy0 ERROR: The machine/kernel does not support the required 30-bit DMA mask
> > 
> > The same happens with the current mainline.
> 
> How much RAM do you have ?

The system has 1129 MB RAM. Booting with mem=1G makes it work.

A.

^ permalink raw reply

* Re: [PATCH v2 0/2] mt76: usb: fix A-MSDU support
From: Felix Fietkau @ 2019-06-06  8:47 UTC (permalink / raw)
  To: Lorenzo Bianconi; +Cc: kvalo, linux-wireless, lorenzo.bianconi, sgruszka
In-Reply-To: <cover.1559293385.git.lorenzo@kernel.org>

On 2019-05-31 11:38, Lorenzo Bianconi wrote:
> Reallocate the skb if there is no enough space to manage the AMSDU rx packets.
> Do not always copy the first part of received frames if A-MSDU is enabled
> for SG capable devices
> 
> Changes since v1:
> - do not allocate multiple page buffers but rely on fragmented skbs
>   if there is no enough space to manage the AMSDU rx packets
> 
> @Felix: do you prefer to take this series in your tree or is it better
> to merge it in wireless-drivers?
Going through wireless-drivers is fine with me.

Acked-by: Felix Fietkau <nbd@nbd.name>

- Felix

^ permalink raw reply

* Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook
From: Christoph Hellwig @ 2019-06-06  6:40 UTC (permalink / raw)
  To: Larry Finger
  Cc: Aaro Koskinen, Christoph Hellwig, Christian Zigotzky,
	Michael Ellerman, linux-kernel, linux-wireless, linuxppc-dev
In-Reply-To: <35b3f09b-b371-e2cc-4436-120c67e2f1fb@lwfinger.net>

On Wed, Jun 05, 2019 at 10:06:18PM -0500, Larry Finger wrote:
> First of all, you have my sympathy for the laborious bisection on a 
> PowerBook G4. I have done several myself. Thank you.
>
> I confirm your results.
>
> The ppc code has a maximum DMA size of 31 bits, thus a 32-bit request will 
> fail. Why the 30-bit fallback fails in b43legacy fails while it works in 
> b43 is a mystery.
>
> Although dma_nommu_dma_supported() may be "largely identical" to 
> dma_direct_supported(), they obviously differ. Routine 
> dma_nommu_dma_supported() returns 1 for 32-bit systems, but I do not know 
> what dma_direct_supported() returns.
>
> I am trying to find a patch.

	if (IS_ENABLED(CONFIG_ZONE_DMA))
		min_mask = DMA_BIT_MASK(ARCH_ZONE_DMA_BITS);
	else
		min_mask = DMA_BIT_MASK(32);

	min_mask = min_t(u64, min_mask, (max_pfn - 1) << PAGE_SHIFT);
	return mask >= __phys_to_dma(dev, min_mask);

So the smaller or:

 (1) 32-bit
 (2) ARCH_ZONE_DMA_BITS
 (3) the actual amount of memory in the system

modolo any DMA offsets that come into play.

No offsets should exists on pmac, and ARCH_ZONE_DMA_BITS is 31 on
powerpc.  So unless the system has 1GB or less memory it will probably
return false for b43, because it can't actually guarantee reliable
allocation.  It will work fine on x86 with the smaller ZONE_DMA.

^ permalink raw reply

* Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook
From: Larry Finger @ 2019-06-06  3:06 UTC (permalink / raw)
  To: Aaro Koskinen, Christoph Hellwig, Christian Zigotzky,
	Michael Ellerman
  Cc: linux-kernel, linux-wireless, linuxppc-dev
In-Reply-To: <20190605225059.GA9953@darkstar.musicnaut.iki.fi>

On 6/5/19 5:50 PM, Aaro Koskinen wrote:
> Hi,
> 
> When upgrading from v5.0 -> v5.1 on G4 PowerBook, I noticed WLAN does
> not work anymore:
> 
> [   42.004303] b43legacy-phy0: Loading firmware version 0x127, patch level 14 (2005-04-18 02:36:27)
> [   42.184837] b43legacy-phy0 debug: Chip initialized
> [   42.184873] b43legacy-phy0 ERROR: The machine/kernel does not support the required 30-bit DMA mask
> 
> The same happens with the current mainline.
> 
> Bisected to:
> 
> 	commit 65a21b71f948406201e4f62e41f06513350ca390
> 	Author: Christoph Hellwig <hch@lst.de>
> 	Date:   Wed Feb 13 08:01:26 2019 +0100
> 
> 	    powerpc/dma: remove dma_nommu_dma_supported
> 
> 	    This function is largely identical to the generic version used
> 	    everywhere else.  Replace it with the generic version.
> 
> 	    Signed-off-by: Christoph Hellwig <hch@lst.de>
> 	    Tested-by: Christian Zigotzky <chzigotzky@xenosoft.de>
> 	    Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>

Aaro,

First of all, you have my sympathy for the laborious bisection on a PowerBook 
G4. I have done several myself. Thank you.

I confirm your results.

The ppc code has a maximum DMA size of 31 bits, thus a 32-bit request will fail. 
Why the 30-bit fallback fails in b43legacy fails while it works in b43 is a mystery.

Although dma_nommu_dma_supported() may be "largely identical" to 
dma_direct_supported(), they obviously differ. Routine dma_nommu_dma_supported() 
returns 1 for 32-bit systems, but I do not know what dma_direct_supported() returns.

I am trying to find a patch.

Larry

^ permalink raw reply

* Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook
From: Benjamin Herrenschmidt @ 2019-06-06  0:54 UTC (permalink / raw)
  To: Aaro Koskinen, Christoph Hellwig, Christian Zigotzky,
	Michael Ellerman, Larry Finger
  Cc: linuxppc-dev, linux-wireless, linux-kernel
In-Reply-To: <20190605225059.GA9953@darkstar.musicnaut.iki.fi>

On Thu, 2019-06-06 at 01:50 +0300, Aaro Koskinen wrote:
> Hi,
> 
> When upgrading from v5.0 -> v5.1 on G4 PowerBook, I noticed WLAN does
> not work anymore:
> 
> [   42.004303] b43legacy-phy0: Loading firmware version 0x127, patch level 14 (2005-04-18 02:36:27)
> [   42.184837] b43legacy-phy0 debug: Chip initialized
> [   42.184873] b43legacy-phy0 ERROR: The machine/kernel does not support the required 30-bit DMA mask
> 
> The same happens with the current mainline.

How much RAM do you have ?

Ben.

> 
> Bisected to:
> 
> 	commit 65a21b71f948406201e4f62e41f06513350ca390
> 	Author: Christoph Hellwig <hch@lst.de>
> 	Date:   Wed Feb 13 08:01:26 2019 +0100
> 
> 	    powerpc/dma: remove dma_nommu_dma_supported
> 
> 	    This function is largely identical to the generic version used
> 	    everywhere else.  Replace it with the generic version.
> 
> 	    Signed-off-by: Christoph Hellwig <hch@lst.de>
> 	    Tested-by: Christian Zigotzky <chzigotzky@xenosoft.de>
> 	    Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> 
> A.


^ permalink raw reply

* [BISECTED REGRESSION] b43legacy broken on G4 PowerBook
From: Aaro Koskinen @ 2019-06-05 22:50 UTC (permalink / raw)
  To: Christoph Hellwig, Christian Zigotzky, Michael Ellerman,
	Larry Finger
  Cc: linux-kernel, linux-wireless, linuxppc-dev

Hi,

When upgrading from v5.0 -> v5.1 on G4 PowerBook, I noticed WLAN does
not work anymore:

[   42.004303] b43legacy-phy0: Loading firmware version 0x127, patch level 14 (2005-04-18 02:36:27)
[   42.184837] b43legacy-phy0 debug: Chip initialized
[   42.184873] b43legacy-phy0 ERROR: The machine/kernel does not support the required 30-bit DMA mask

The same happens with the current mainline.

Bisected to:

	commit 65a21b71f948406201e4f62e41f06513350ca390
	Author: Christoph Hellwig <hch@lst.de>
	Date:   Wed Feb 13 08:01:26 2019 +0100

	    powerpc/dma: remove dma_nommu_dma_supported

	    This function is largely identical to the generic version used
	    everywhere else.  Replace it with the generic version.

	    Signed-off-by: Christoph Hellwig <hch@lst.de>
	    Tested-by: Christian Zigotzky <chzigotzky@xenosoft.de>
	    Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>

A.

^ permalink raw reply

* Re: [PATCH v2 2/3] mmc: core: API for temporarily disabling auto-retuning due to errors
From: Doug Anderson @ 2019-06-05 22:51 UTC (permalink / raw)
  To: Arend Van Spriel
  Cc: Ulf Hansson, Kalle Valo, Adrian Hunter, brcm80211-dev-list.pdl,
	open list:ARM/Rockchip SoC..., Double Lo, Brian Norris,
	linux-wireless, Naveen Gupta, Madhan Mohan R, Matthias Kaehlcke,
	Wright Feng, Chi-Hsien Lin, netdev, brcm80211-dev-list,
	Linux MMC List, LKML, Shawn Lin, Wolfram Sang, Avri Altman
In-Reply-To: <25fe1725-76fa-2739-1427-b0e8823ea4ae@broadcom.com>

Hi,

On Wed, Jun 5, 2019 at 12:54 AM Arend Van Spriel
<arend.vanspriel@broadcom.com> wrote:
>
> On 6/3/2019 8:37 PM, Douglas Anderson wrote:
> > Normally when the MMC core sees an "-EILSEQ" error returned by a host
> > controller then it will trigger a retuning of the card.  This is
> > generally a good idea.
> >
> > However, if a command is expected to sometimes cause transfer errors
> > then these transfer errors shouldn't cause a re-tuning.  This
> > re-tuning will be a needless waste of time.  One example case where a
> > transfer is expected to cause errors is when transitioning between
> > idle (sometimes referred to as "sleep" in Broadcom code) and active
> > state on certain Broadcom WiFi cards.  Specifically if the card was
> > already transitioning between states when the command was sent it
> > could cause an error on the SDIO bus.
> >
> > Let's add an API that the SDIO card drivers can call that will
> > temporarily disable the auto-tuning functionality.  Then we can add a
> > call to this in the Broadcom WiFi driver and any other driver that
> > might have similar needs.
> >
> > NOTE: this makes the assumption that the card is already tuned well
> > enough that it's OK to disable the auto-retuning during one of these
> > error-prone situations.  Presumably the driver code performing the
> > error-prone transfer knows how to recover / retry from errors.  ...and
> > after we can get back to a state where transfers are no longer
> > error-prone then we can enable the auto-retuning again.  If we truly
> > find ourselves in a case where the card needs to be retuned sometimes
> > to handle one of these error-prone transfers then we can always try a
> > few transfers first without auto-retuning and then re-try with
> > auto-retuning if the first few fail.
> >
> > Without this change on rk3288-veyron-minnie I periodically see this in
> > the logs of a machine just sitting there idle:
> >    dwmmc_rockchip ff0d0000.dwmmc: Successfully tuned phase to XYZ
> >
> > Fixes: bd11e8bd03ca ("mmc: core: Flag re-tuning is needed on CRC errors")
> > Signed-off-by: Douglas Anderson <dianders@chromium.org>
> > ---
> > Note that are are a whole boatload of different ways that we could
> > provide an API for the Broadcom WiFi SDIO driver.  This patch
> > illustrates one way but if maintainers feel strongly that this is too
> > ugly and have a better idea then I can give it a shot too.  From a
> > purist point of view I kinda felt that the "expect errors" really
> > belonged as part of the mmc_request structure, but getting it into
> > there meant changing a whole pile of core SD/MMC APIs.  Simply adding
> > it to the host seemed to match the current style better and was a less
> > intrusive change.
>
> Hi Doug,
>
> Sorry for bringing this up, but there used to be an issue with retuning
> in general, ie. the device handled tuning command 19 only once after
> startup. I guess that is no longer an issue given your results.

Right.  It definitely used to just happen once at bootup and you were
out of luck if that value was bad for some reason or if conditions
changed.  In cases in my own personal experience it was actually fine
to just tune once at bootup once all the tuning bugs in the controller
were fixed.  ...but I can imagine that some controllers could use
delay elements that drift more.  ...and in any case if you're getting
CRC errors trying a re-tuning seems a sensible thing to do anyway
(unless the CRC error was expected).

Looking at commit bd11e8bd03ca ("mmc: core: Flag re-tuning is needed
on CRC errors") you can definitely see evidence that tuning can happen
again after bootup.


> I guess
> the problem goes away when you disable device sleep functionality. No
> what you want in terms of power consumption, but would be good to know.
> You can disable it with below patch.

I can try testing this if it's useful, but I'm not sure what it will
prove.  I definitely don't want to disable device sleep, so it's not a
long term solution.  Are you just looking for extra evidence that this
is indeed my problem?  I don't think I need any extra evidence, do I?
The fact that patch #3 in this series fixes my problems (plus
debugging I did to arrive at that patch) means we absolutely know that
brcmf_sdio_kso_control() is responsible for the CRC errors that caused
the unneeded tuning.  Setting BRCMF_IDLE_INTERVAL to 0 will
effectively prevent brcmf_sdio_kso_control() from doing anything
useful (except in full system suspend, but that's not the case I was
testing anyway).

-Doug

^ permalink raw reply

* RE: [EXT] [PATCH 1/2] mwifiex: dispatch/rotate from reorder table atomically
From: Ganapathi Bhat @ 2019-06-05  9:15 UTC (permalink / raw)
  To: Brian Norris, Nishant Sarmukadam, Amitkumar Karwar, Xinming Hu
  Cc: linux-kernel@vger.kernel.org, linux-wireless@vger.kernel.org,
	Doug Anderson
In-Reply-To: <20190604205323.200361-2-briannorris@chromium.org>

Hi Brian,

> (1) iterating / clearing the mwifiex reordering table
> (2) dispatching received packets to upper layers
> 
> This makes it much harder to make lock recursion mistakes, as these two
> steps no longer need to hold the same locks.

Yes, this is clean;

> 
> Testing: I've played with a variety of stress tests, including download stress
> tests on the same APs which caught regressions with commit
> 5188d5453bc9 ("mwifiex: restructure rx_reorder_tbl_lock usage"). I've
> primarily tested on Marvell 8997 / PCIe, although I've given 8897 / SDIO a
> quick spin as well.
> 

Thanks a lot for this; We will also run the tests locally; But, I find the change is good;

Acked-by: Ganapathi Bhat <gbhat@marvell.com>

Regards,
Ganapathi

^ permalink raw reply

* Re: [PATCH v2 2/3] mmc: core: API for temporarily disabling auto-retuning due to errors
From: Arend Van Spriel @ 2019-06-05  7:54 UTC (permalink / raw)
  To: Douglas Anderson, Ulf Hansson, Kalle Valo, Adrian Hunter
  Cc: brcm80211-dev-list.pdl, linux-rockchip, Double Lo, briannorris,
	linux-wireless, Naveen Gupta, Madhan Mohan R, mka, Wright Feng,
	Chi-Hsien Lin, netdev, brcm80211-dev-list, linux-mmc,
	linux-kernel, Shawn Lin, Wolfram Sang, Avri Altman
In-Reply-To: <20190603183740.239031-3-dianders@chromium.org>

On 6/3/2019 8:37 PM, Douglas Anderson wrote:
> Normally when the MMC core sees an "-EILSEQ" error returned by a host
> controller then it will trigger a retuning of the card.  This is
> generally a good idea.
> 
> However, if a command is expected to sometimes cause transfer errors
> then these transfer errors shouldn't cause a re-tuning.  This
> re-tuning will be a needless waste of time.  One example case where a
> transfer is expected to cause errors is when transitioning between
> idle (sometimes referred to as "sleep" in Broadcom code) and active
> state on certain Broadcom WiFi cards.  Specifically if the card was
> already transitioning between states when the command was sent it
> could cause an error on the SDIO bus.
> 
> Let's add an API that the SDIO card drivers can call that will
> temporarily disable the auto-tuning functionality.  Then we can add a
> call to this in the Broadcom WiFi driver and any other driver that
> might have similar needs.
> 
> NOTE: this makes the assumption that the card is already tuned well
> enough that it's OK to disable the auto-retuning during one of these
> error-prone situations.  Presumably the driver code performing the
> error-prone transfer knows how to recover / retry from errors.  ...and
> after we can get back to a state where transfers are no longer
> error-prone then we can enable the auto-retuning again.  If we truly
> find ourselves in a case where the card needs to be retuned sometimes
> to handle one of these error-prone transfers then we can always try a
> few transfers first without auto-retuning and then re-try with
> auto-retuning if the first few fail.
> 
> Without this change on rk3288-veyron-minnie I periodically see this in
> the logs of a machine just sitting there idle:
>    dwmmc_rockchip ff0d0000.dwmmc: Successfully tuned phase to XYZ
> 
> Fixes: bd11e8bd03ca ("mmc: core: Flag re-tuning is needed on CRC errors")
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
> Note that are are a whole boatload of different ways that we could
> provide an API for the Broadcom WiFi SDIO driver.  This patch
> illustrates one way but if maintainers feel strongly that this is too
> ugly and have a better idea then I can give it a shot too.  From a
> purist point of view I kinda felt that the "expect errors" really
> belonged as part of the mmc_request structure, but getting it into
> there meant changing a whole pile of core SD/MMC APIs.  Simply adding
> it to the host seemed to match the current style better and was a less
> intrusive change.

Hi Doug,

Sorry for bringing this up, but there used to be an issue with retuning 
in general, ie. the device handled tuning command 19 only once after 
startup. I guess that is no longer an issue given your results. I guess 
the problem goes away when you disable device sleep functionality. No 
what you want in terms of power consumption, but would be good to know. 
You can disable it with below patch.

Regards,
Arend
---
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c 
b/drivers
index 15a40fd..18e90bd 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -307,7 +307,7 @@ struct rte_console {
  #define BRCMF_IDLE_ACTIVE      0       /* Do not request any SD clock 
change
                                          * when idle
                                          */
-#define BRCMF_IDLE_INTERVAL    1
+#define BRCMF_IDLE_INTERVAL    0

  #define KSO_WAIT_US 50
  #define MAX_KSO_ATTEMPTS (PMU_MAX_TRANSITION_DLY/KSO_WAIT_US)


^ permalink raw reply related

* Re: [RFC PATCH v4] rtl8xxxu: Improve TX performance of RTL8723BU on rtl8xxxu driver
From: Chris Chiu @ 2019-06-05  2:17 UTC (permalink / raw)
  To: Jes Sorensen
  Cc: Kalle Valo, David Miller, linux-wireless, netdev, Linux Kernel,
	Linux Upstreaming Team
In-Reply-To: <f1c54f97-16a5-2618-569b-9101f9657fcb@gmail.com>

On Tue, Jun 4, 2019 at 3:21 AM Jes Sorensen <jes.sorensen@gmail.com> wrote:
>
> On 5/31/19 5:12 AM, Chris Chiu wrote:
> > We have 3 laptops which connect the wifi by the same RTL8723BU.
> > The PCI VID/PID of the wifi chip is 10EC:B720 which is supported.
> > They have the same problem with the in-kernel rtl8xxxu driver, the
> > iperf (as a client to an ethernet-connected server) gets ~1Mbps.
> > Nevertheless, the signal strength is reported as around -40dBm,
> > which is quite good. From the wireshark capture, the tx rate for each
> > data and qos data packet is only 1Mbps. Compare to the Realtek driver
> > at https://github.com/lwfinger/rtl8723bu, the same iperf test gets
> > ~12Mbps or better. The signal strength is reported similarly around
> > -40dBm. That's why we want to improve.
> >
> > After reading the source code of the rtl8xxxu driver and Realtek's, the
> > major difference is that Realtek's driver has a watchdog which will keep
> > monitoring the signal quality and updating the rate mask just like the
> > rtl8xxxu_gen2_update_rate_mask() does if signal quality changes.
> > And this kind of watchdog also exists in rtlwifi driver of some specific
> > chips, ex rtl8192ee, rtl8188ee, rtl8723ae, rtl8821ae...etc. They have
> > the same member function named dm_watchdog and will invoke the
> > corresponding dm_refresh_rate_adaptive_mask to adjust the tx rate
> > mask.
> >
> > With this commit, the tx rate of each data and qos data packet will
> > be 39Mbps (MCS4) with the 0xF00000 as the tx rate mask. The 20th bit
> > to 23th bit means MCS4 to MCS7. It means that the firmware still picks
> > the lowest rate from the rate mask and explains why the tx rate of
> > data and qos data is always lowest 1Mbps because the default rate mask
> > passed is always 0xFFFFFFF ranges from the basic CCK rate, OFDM rate,
> > and MCS rate. However, with Realtek's driver, the tx rate observed from
> > wireshark under the same condition is almost 65Mbps or 72Mbps.
> >
> > I believe the firmware of RTL8723BU may need fix. And I think we
> > can still bring in the dm_watchdog as rtlwifi to improve from the
> > driver side. Please leave precious comments for my commits and
> > suggest what I can do better. Or suggest if there's any better idea
> > to fix this. Thanks.
> >
> > Signed-off-by: Chris Chiu <chiu@endlessm.com>
>
> I am really pleased to see you're investigating some of these issues,
> since I've been pretty swamped and not had time to work on this driver
> for a long time.
>
> The firmware should allow for two rate modes, either firmware handled or
> controlled by the driver. Ideally we would want the driver to handle it,
> but I never was able to make that work reliable.
>
> This fix should at least improve the situation, and it may explain some
> of the performance issues with the 8192eu as well?
>
> > diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> > index 8828baf26e7b..216f603827a8 100644
> > --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> > +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> > @@ -1195,6 +1195,44 @@ struct rtl8723bu_c2h {
> >
> >  struct rtl8xxxu_fileops;
> >
> > +/*mlme related.*/
> > +enum wireless_mode {
> > +     WIRELESS_MODE_UNKNOWN = 0,
> > +     /* Sub-Element */
> > +     WIRELESS_MODE_B = BIT(0),
> > +     WIRELESS_MODE_G = BIT(1),
> > +     WIRELESS_MODE_A = BIT(2),
> > +     WIRELESS_MODE_N_24G = BIT(3),
> > +     WIRELESS_MODE_N_5G = BIT(4),
> > +     WIRELESS_AUTO = BIT(5),
> > +     WIRELESS_MODE_AC = BIT(6),
> > +     WIRELESS_MODE_MAX = 0x7F,
> > +};
> > +
> > +/* from rtlwifi/wifi.h */
> > +enum ratr_table_mode_new {
> > +     RATEID_IDX_BGN_40M_2SS = 0,
> > +     RATEID_IDX_BGN_40M_1SS = 1,
> > +     RATEID_IDX_BGN_20M_2SS_BN = 2,
> > +     RATEID_IDX_BGN_20M_1SS_BN = 3,
> > +     RATEID_IDX_GN_N2SS = 4,
> > +     RATEID_IDX_GN_N1SS = 5,
> > +     RATEID_IDX_BG = 6,
> > +     RATEID_IDX_G = 7,
> > +     RATEID_IDX_B = 8,
> > +     RATEID_IDX_VHT_2SS = 9,
> > +     RATEID_IDX_VHT_1SS = 10,
> > +     RATEID_IDX_MIX1 = 11,
> > +     RATEID_IDX_MIX2 = 12,
> > +     RATEID_IDX_VHT_3SS = 13,
> > +     RATEID_IDX_BGN_3SS = 14,
> > +};
> > +
> > +#define RTL8XXXU_RATR_STA_INIT 0
> > +#define RTL8XXXU_RATR_STA_HIGH 1
> > +#define RTL8XXXU_RATR_STA_MID  2
> > +#define RTL8XXXU_RATR_STA_LOW  3
> > +
>
> >  extern struct rtl8xxxu_fileops rtl8192cu_fops;
> >  extern struct rtl8xxxu_fileops rtl8192eu_fops;
> > diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c
> > index 26b674aca125..2071ab9fd001 100644
> > --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c
> > +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c
> > @@ -1645,6 +1645,148 @@ static void rtl8723bu_init_statistics(struct rtl8xxxu_priv *priv)
> >       rtl8xxxu_write32(priv, REG_OFDM0_FA_RSTC, val32);
> >  }
> >
> > +static u8 rtl8723b_signal_to_rssi(int signal)
> > +{
> > +     if (signal < -95)
> > +             signal = -95;
> > +     return (u8)(signal + 95);
> > +}
>
> Could you make this more generic so it can be used by the other sub-drivers?
>
Sure. I'll do that.

> > +static void rtl8723b_refresh_rate_mask(struct rtl8xxxu_priv *priv,
> > +                                    int signal, struct ieee80211_sta *sta)
> > +{
> > +     struct ieee80211_hw *hw = priv->hw;
> > +     u16 wireless_mode;
> > +     u8 rssi_level, ratr_idx;
> > +     u8 txbw_40mhz;
> > +     u8 rssi, rssi_thresh_high, rssi_thresh_low;
> > +
> > +     rssi_level = priv->rssi_level;
> > +     rssi = rtl8723b_signal_to_rssi(signal);
> > +     txbw_40mhz = (hw->conf.chandef.width == NL80211_CHAN_WIDTH_40) ? 1 : 0;
> > +
> > +     switch (rssi_level) {
> > +     case RTL8XXXU_RATR_STA_HIGH:
> > +             rssi_thresh_high = 50;
> > +             rssi_thresh_low = 20;
> > +             break;
> > +     case RTL8XXXU_RATR_STA_MID:
> > +             rssi_thresh_high = 55;
> > +             rssi_thresh_low = 20;
> > +             break;
> > +     case RTL8XXXU_RATR_STA_LOW:
> > +             rssi_thresh_high = 60;
> > +             rssi_thresh_low = 25;
> > +             break;
> > +     default:
> > +             rssi_thresh_high = 50;
> > +             rssi_thresh_low = 20;
> > +             break;
> > +     }
>
> Can we make this use defined values with some explanation rather than
> hard coded values?
>

I also thought about this. So I refer to the same refresh_rateadaotive_mask
in rtlwifi/rtl8192se/dm.c, rtlwifi/rtl8723ae/dm.c, and rtl8188ee...etc. They
don't give a better explanation. And I also don't know if these values can be
generally applied to other subdrivers or specifically for 8723b series, for
example, the rtl8192se use different values for the threshold. It maybe due
to different noise floor for different chip?  I'm not sure. I took these values
from vendor driver and rtl8188ee. I can simply use defined values to replace
but I have to admit it's hard to find a good explanation.

> > +     if (rssi > rssi_thresh_high)
> > +             rssi_level = RTL8XXXU_RATR_STA_HIGH;
> > +     else if (rssi > rssi_thresh_low)
> > +             rssi_level = RTL8XXXU_RATR_STA_MID;
> > +     else
> > +             rssi_level = RTL8XXXU_RATR_STA_LOW;
> > +
> > +     if (rssi_level != priv->rssi_level) {
> > +             int sgi = 0;
> > +             u32 rate_bitmap = 0;
> > +
> > +             rcu_read_lock();
> > +             rate_bitmap = (sta->supp_rates[0] & 0xfff) |
> > +                             (sta->ht_cap.mcs.rx_mask[0] << 12) |
> > +                             (sta->ht_cap.mcs.rx_mask[1] << 20);
> > +             if (sta->ht_cap.cap &
> > +                 (IEEE80211_HT_CAP_SGI_40 | IEEE80211_HT_CAP_SGI_20))
> > +                     sgi = 1;
> > +             rcu_read_unlock();
> > +
> > +             wireless_mode = rtl8xxxu_wireless_mode(hw, sta);
> > +             switch (wireless_mode) {
> > +             case WIRELESS_MODE_B:
> > +                     ratr_idx = RATEID_IDX_B;
> > +                     if (rate_bitmap & 0x0000000c)
> > +                             rate_bitmap &= 0x0000000d;
> > +                     else
> > +                             rate_bitmap &= 0x0000000f;
> > +                     break;
> > +             case WIRELESS_MODE_A:
> > +             case WIRELESS_MODE_G:
> > +                     ratr_idx = RATEID_IDX_G;
> > +                     if (rssi_level == RTL8XXXU_RATR_STA_HIGH)
> > +                             rate_bitmap &= 0x00000f00;
> > +                     else
> > +                             rate_bitmap &= 0x00000ff0;
> > +                     break;
> > +             case (WIRELESS_MODE_B | WIRELESS_MODE_G):
> > +                     ratr_idx = RATEID_IDX_BG;
> > +                     if (rssi_level == RTL8XXXU_RATR_STA_HIGH)
> > +                             rate_bitmap &= 0x00000f00;
> > +                     else if (rssi_level == RTL8XXXU_RATR_STA_MID)
> > +                             rate_bitmap &= 0x00000ff0;
> > +                     else
> > +                             rate_bitmap &= 0x00000ff5;
> > +                     break;
>
> It would be nice as well to get all these masks into generic names.
>

I also take these mask values from the update_hal_rate_mask of the
vendor driver and other realtek drivers under rtlwifi. I thought about to
define the lower 12 bits like RTL8XXXU_BG_RATE_MASK, 13~20 bits
as RTL8XXXU_MCS0_7_RATE_MASK. But it's still hard to express
all the combinations here. So I just leave it as it is. I can try to add
explanations for the rate mapping of each bit. It would be a lot easier.

> > +             case WIRELESS_MODE_N_24G:
> > +             case WIRELESS_MODE_N_5G:
> > +             case (WIRELESS_MODE_G | WIRELESS_MODE_N_24G):
> > +             case (WIRELESS_MODE_A | WIRELESS_MODE_N_5G):
> > +                     if (priv->tx_paths == 2 && priv->rx_paths == 2)
> > +                             ratr_idx = RATEID_IDX_GN_N2SS;
> > +                     else
> > +                             ratr_idx = RATEID_IDX_GN_N1SS;
> > +             case (WIRELESS_MODE_B | WIRELESS_MODE_G | WIRELESS_MODE_N_24G):
> > +             case (WIRELESS_MODE_B | WIRELESS_MODE_N_24G):
> > +                     if (txbw_40mhz) {
> > +                             if (priv->tx_paths == 2 && priv->rx_paths == 2)
> > +                                     ratr_idx = RATEID_IDX_BGN_40M_2SS;
> > +                             else
> > +                                     ratr_idx = RATEID_IDX_BGN_40M_1SS;
> > +                     } else {
> > +                             if (priv->tx_paths == 2 && priv->rx_paths == 2)
> > +                                     ratr_idx = RATEID_IDX_BGN_20M_2SS_BN;
> > +                             else
> > +                                     ratr_idx = RATEID_IDX_BGN_20M_1SS_BN;
> > +                     }
> > +
> > +                     if (priv->tx_paths == 2 && priv->rx_paths == 2) {
> > +                             if (rssi_level == RTL8XXXU_RATR_STA_HIGH) {
> > +                                     rate_bitmap &= 0x0f8f0000;
> > +                             } else if (rssi_level == RTL8XXXU_RATR_STA_MID) {
> > +                                     rate_bitmap &= 0x0f8ff000;
> > +                             } else {
> > +                                     if (txbw_40mhz)
> > +                                             rate_bitmap &= 0x0f8ff015;
> > +                                     else
> > +                                             rate_bitmap &= 0x0f8ff005;
> > +                             }
> > +                     } else {
> > +                             if (rssi_level == RTL8XXXU_RATR_STA_HIGH) {
> > +                                     rate_bitmap &= 0x000f0000;
> > +                             } else if (rssi_level == RTL8XXXU_RATR_STA_MID) {
> > +                                     rate_bitmap &= 0x000ff000;
> > +                             } else {
> > +                                     if (txbw_40mhz)
> > +                                             rate_bitmap &= 0x000ff015;
> > +                                     else
> > +                                             rate_bitmap &= 0x000ff005;
> > +                             }
> > +                     }
> > +                     break;
> > +             default:
> > +                     ratr_idx = RATEID_IDX_BGN_40M_2SS;
> > +                     rate_bitmap &= 0x0fffffff;
> > +                     break;
> > +             }
> > +
> > +             priv->rssi_level = rssi_level;
> > +             priv->fops->update_rate_mask(priv, rate_bitmap, ratr_idx, sgi);
> > +     }
> > +}
> > +
>
> In general I think all of this should be fairly generic and the other
> subdrivers should be able to benefit from it?
>
>
I agree. Mabe separates the rssi level judgement function to be chip specific,
and move the whole refresh_rate_mask thing generic?

> >  struct rtl8xxxu_fileops rtl8723bu_fops = {
> >       .parse_efuse = rtl8723bu_parse_efuse,
> >       .load_firmware = rtl8723bu_load_firmware,
> > @@ -1665,6 +1807,7 @@ struct rtl8xxxu_fileops rtl8723bu_fops = {
> >       .usb_quirks = rtl8xxxu_gen2_usb_quirks,
> >       .set_tx_power = rtl8723b_set_tx_power,
> >       .update_rate_mask = rtl8xxxu_gen2_update_rate_mask,
> > +     .refresh_rate_mask = rtl8723b_refresh_rate_mask,
> >       .report_connect = rtl8xxxu_gen2_report_connect,
> >       .fill_txdesc = rtl8xxxu_fill_txdesc_v2,
> >       .writeN_block_size = 1024,
> > diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> > index 039e5ca9d2e4..be322402ca01 100644
> > --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> > +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> > @@ -4311,7 +4311,8 @@ static void rtl8xxxu_sw_scan_complete(struct ieee80211_hw *hw,
> >       rtl8xxxu_write8(priv, REG_BEACON_CTRL, val8);
> >  }
> >
> > -void rtl8xxxu_update_rate_mask(struct rtl8xxxu_priv *priv, u32 ramask, int sgi)
> > +void rtl8xxxu_update_rate_mask(struct rtl8xxxu_priv *priv,
> > +                            u32 ramask, u8 rateid, int sgi)
> >  {
> >       struct h2c_cmd h2c;
> >
> > @@ -4331,7 +4332,7 @@ void rtl8xxxu_update_rate_mask(struct rtl8xxxu_priv *priv, u32 ramask, int sgi)
> >  }
> >
> >  void rtl8xxxu_gen2_update_rate_mask(struct rtl8xxxu_priv *priv,
> > -                                 u32 ramask, int sgi)
> > +                                 u32 ramask, u8 rateid, int sgi)
> >  {
> >       struct h2c_cmd h2c;
> >       u8 bw = 0;
> > @@ -4345,7 +4346,7 @@ void rtl8xxxu_gen2_update_rate_mask(struct rtl8xxxu_priv *priv,
> >       h2c.b_macid_cfg.ramask3 = (ramask >> 24) & 0xff;
> >
> >       h2c.ramask.arg = 0x80;
> > -     h2c.b_macid_cfg.data1 = 0;
> > +     h2c.b_macid_cfg.data1 = rateid;
> >       if (sgi)
> >               h2c.b_macid_cfg.data1 |= BIT(7);
> >
> > @@ -4485,6 +4486,40 @@ static void rtl8xxxu_set_basic_rates(struct rtl8xxxu_priv *priv, u32 rate_cfg)
> >       rtl8xxxu_write8(priv, REG_INIRTS_RATE_SEL, rate_idx);
> >  }
> >
> > +u16
> > +rtl8xxxu_wireless_mode(struct ieee80211_hw *hw, struct ieee80211_sta *sta)
> > +{
> > +     u16 network_type = WIRELESS_MODE_UNKNOWN;
> > +     u32 rate_mask;
> > +
> > +     rate_mask = (sta->supp_rates[0] & 0xfff) |
> > +                 (sta->ht_cap.mcs.rx_mask[0] << 12) |
> > +                 (sta->ht_cap.mcs.rx_mask[0] << 20);
> > +
> > +     if (hw->conf.chandef.chan->band == NL80211_BAND_5GHZ) {
> > +             if (sta->vht_cap.vht_supported)
> > +                     network_type = WIRELESS_MODE_AC;
> > +             else if (sta->ht_cap.ht_supported)
> > +                     network_type = WIRELESS_MODE_N_5G;
> > +
> > +             network_type |= WIRELESS_MODE_A;
> > +     } else {
> > +             if (sta->vht_cap.vht_supported)
> > +                     network_type = WIRELESS_MODE_AC;
> > +             else if (sta->ht_cap.ht_supported)
> > +                     network_type = WIRELESS_MODE_N_24G;
> > +
> > +             if (sta->supp_rates[0] <= 0xf)
> > +                     network_type |= WIRELESS_MODE_B;
> > +             else if (sta->supp_rates[0] & 0xf)
> > +                     network_type |= (WIRELESS_MODE_B | WIRELESS_MODE_G);
> > +             else
> > +                     network_type |= WIRELESS_MODE_G;
> > +     }
> > +
> > +     return network_type;
> > +}
>
> I always hated the wireless_mode nonsense in the realtek driver, but
> maybe we cannot avoid it :(
>
> Cheers,
> Jes

^ permalink raw reply

* [PATCH] mt76: mt7615: remove key check in mt7615_mcu_set_wtbl_key
From: Lorenzo Bianconi @ 2019-06-04 22:12 UTC (permalink / raw)
  To: nbd; +Cc: lorenzo.bianconi, linux-wireless, ryder.lee, royluo

Do not check key pointer in mt7615_mcu_set_wtbl_key since if set_key_cmd
is SET_KEY, key will be always not NULL. This patch will address a false
positive reported by Coverity-Scan

Addresses-Coverity-ID: 1445463 ("Dereference after null check")
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
This patch is based on 'mt76: mt7615: fix slow performance when enable
encryption'
https://patchwork.kernel.org/patch/10972385/
---
 drivers/net/wireless/mediatek/mt76/mt7615/mcu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
index d104435cd901..7a41c37e7460 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
@@ -879,7 +879,7 @@ int mt7615_mcu_set_wtbl_key(struct mt7615_dev *dev, int wcid,
 		u8 cipher;
 
 		cipher = mt7615_get_key_info(key, req.key.key_material);
-		if (cipher == MT_CIPHER_NONE && key)
+		if (cipher == MT_CIPHER_NONE)
 			return -EOPNOTSUPP;
 
 		req.key.rkv = 1;
-- 
2.21.0


^ permalink raw reply related

* Re: [EXT] Re: [4.20 PATCH] Revert "mwifiex: restructure rx_reorder_tbl_lock usage"
From: Brian Norris @ 2019-06-04 20:57 UTC (permalink / raw)
  To: Ganapathi Bhat
  Cc: linux-kernel@vger.kernel.org, Amitkumar Karwar,
	Nishant Sarmukadam, Xinming Hu, linux-wireless@vger.kernel.org,
	stable@vger.kernel.org
In-Reply-To: <MN2PR18MB26376D3A660956396D0E60AFA0150@MN2PR18MB2637.namprd18.prod.outlook.com>

Hi Ganapathi,

On Mon, Jun 3, 2019 at 8:04 PM Ganapathi Bhat <gbhat@marvell.com> wrote:
> > There are a few possible ways to handle this:
> > (a) prevent processing softirqs in that context; e.g., with
> >     local_bh_disable(). This seems somewhat of a hack.
> >     (Side note: I think most of the locks in this driver really could be
> >     spin_lock_bh(), not spin_lock_irqsave() -- we don't really care
> >     about hardirq context for 99% of these locks.)
> > (b) restructure so that packet processing (e.g., netif_rx_ni()) is done
> >     outside of the spinlock.
> >
> > It's actually not that hard to do (b). You can just queue your skb's up in a
> > temporary sk_buff_head list and process them all at once after you've
> > finished processing the reorder table. I have a local patch to do this, and I
> > might send it your way if I can give it a bit more testing.
>
> OK; That will be good; We will run a complete test after the patch; (OR we can work on this, share for review);

I gave my work another round of testing and submitted it here:

https://patchwork.kernel.org/cover/10976089/
[PATCH 0/2] mwifiex: spinlock usage improvements

Feel free to give it a spin. It doesn't completely resolve everything
you were trying to fix, but I believe it helps nudge things in the
right direction.

Brian

^ permalink raw reply

* [PATCH 0/2] mwifiex: spinlock usage improvements
From: Brian Norris @ 2019-06-04 20:53 UTC (permalink / raw)
  To: Ganapathi Bhat, Nishant Sarmukadam, Amitkumar Karwar, Xinming Hu
  Cc: linux-kernel, linux-wireless, Doug Anderson, Brian Norris

This series follows up on some notes from this thread:

  http://lkml.kernel.org/linux-wireless/20181130175957.167031-1-briannorris@chromium.org
  Subject: [4.20 PATCH] Revert "mwifiex: restructure rx_reorder_tbl_lock usage"

where Ganapathi suggested I send out my work. So here goes.

In particular, patch 1 is a step toward helping apply Ganapathi's
original "mwifiex: restructure rx_reorder_tbl_lock usage" solution
without regression, by logically separating the two operations (and
therefore, the locking patterns) involved in that deadlock. It doesn't
re-apply that change, nor does it 100% unblock such a solution, but at
least it's a step in the right direction, as I understand it.

Patch 2 is a change I noticed should be possible along the way. There
are a number of reasons we probably shouldn't be disabling hardirqs when
it's not necessary, but one funny side effect: bugs noticed in the above
"revert" patch would no longer happen. This is because
mwifiex_recv_packet() bases softirq decisions on in_interrupt() (see
description in include/linux/preempt.h), so it will automatically skip
softirq processing if we have BH disabled, but not if we only have hard
IRQs disabled. In other words, if we have such an incorrect nesting bug
in the future (this time with BH disabled), we will now skip softirq
processing and therefore sidestep this sort of bug. [1]

[Related note: softirq masking is weird:
https://lwn.net/Articles/779738/]

It's also possible we can improve system responsiveness and
debuggability by keeping (hard) IRQs enabled more often, although I
didn't measure any particular effect here, and most of these contexts
should be rather quick.

I've done a variety of performance and stress tests for this series, on
both 8897/SDIO and 8997/PCIe, and I haven't seen any decrease in
performance or stability. Or, any change in performance appears to be
within the range of "noise".

I'd appreciate any testing others can do on this series though, as
Ganapathi did offer to try this out.

Regards,
Brian

[1] Side note: the usage of 'in_interrupt()' in mwifiex_recv_packet() is
probably not really a good idea. But it does have a helpful side effect
for this particular sort of bug.


Brian Norris (2):
  mwifiex: dispatch/rotate from reorder table atomically
  mwifiex: don't disable hardirqs; just softirqs

 drivers/net/wireless/marvell/mwifiex/11n.c    |  53 +++-----
 drivers/net/wireless/marvell/mwifiex/11n.h    |   5 +-
 .../net/wireless/marvell/mwifiex/11n_aggr.c   |  24 ++--
 .../wireless/marvell/mwifiex/11n_rxreorder.c  | 125 ++++++++----------
 .../net/wireless/marvell/mwifiex/cfg80211.c   |  35 +++--
 drivers/net/wireless/marvell/mwifiex/cmdevt.c |  76 +++++------
 drivers/net/wireless/marvell/mwifiex/init.c   |  32 ++---
 drivers/net/wireless/marvell/mwifiex/main.c   |  29 ++--
 drivers/net/wireless/marvell/mwifiex/scan.c   |  58 ++++----
 .../wireless/marvell/mwifiex/sta_cmdresp.c    |   5 +-
 .../net/wireless/marvell/mwifiex/sta_event.c  |  10 +-
 drivers/net/wireless/marvell/mwifiex/tdls.c   |  68 ++++------
 drivers/net/wireless/marvell/mwifiex/txrx.c   |   5 +-
 .../net/wireless/marvell/mwifiex/uap_txrx.c   |  10 +-
 drivers/net/wireless/marvell/mwifiex/usb.c    |  10 +-
 drivers/net/wireless/marvell/mwifiex/util.c   |  15 +--
 drivers/net/wireless/marvell/mwifiex/wmm.c    |  92 +++++--------
 17 files changed, 269 insertions(+), 383 deletions(-)

-- 
2.22.0.rc1.311.g5d7573a151-goog


^ permalink raw reply

* [PATCH 1/2] mwifiex: dispatch/rotate from reorder table atomically
From: Brian Norris @ 2019-06-04 20:53 UTC (permalink / raw)
  To: Ganapathi Bhat, Nishant Sarmukadam, Amitkumar Karwar, Xinming Hu
  Cc: linux-kernel, linux-wireless, Doug Anderson, Brian Norris
In-Reply-To: <20190604205323.200361-1-briannorris@chromium.org>

mwifiex_11n_scan_and_dispatch() and
mwifiex_11n_dispatch_pkt_until_start_win() share similar patterns, where
they perform a few different actions on the same table, using the same
lock, but non-atomically. There have been other attempts to clean up
this sort of behavior, but they have had problems (incomplete;
introducing new deadlocks).

We can improve these functions' atomicity by queueing up our RX packets
in a list, to dispatch at the end of the function. This avoids problems
of another operation modifying the table in between our dispatch and
rotation operations.

This was inspired by investigations around this:

  http://lkml.kernel.org/linux-wireless/20181130175957.167031-1-briannorris@chromium.org
  Subject: [4.20 PATCH] Revert "mwifiex: restructure rx_reorder_tbl_lock usage"

While the original (now-reverted) patch had good intentions in
restructuring some of the locking patterns in this driver, it missed an
important detail: we cannot defer to softirq contexts while already in
an atomic context. We can help avoid this sort of problem by separating
the two steps of:
(1) iterating / clearing the mwifiex reordering table
(2) dispatching received packets to upper layers

This makes it much harder to make lock recursion mistakes, as these
two steps no longer need to hold the same locks.

Testing: I've played with a variety of stress tests, including download
stress tests on the same APs which caught regressions with commit
5188d5453bc9 ("mwifiex: restructure rx_reorder_tbl_lock usage"). I've
primarily tested on Marvell 8997 / PCIe, although I've given 8897 / SDIO
a quick spin as well.

Signed-off-by: Brian Norris <briannorris@chromium.org>
---
 .../wireless/marvell/mwifiex/11n_rxreorder.c  | 43 +++++++++++--------
 1 file changed, 24 insertions(+), 19 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/11n_rxreorder.c b/drivers/net/wireless/marvell/mwifiex/11n_rxreorder.c
index 5380fba652cc..77bdf16d6573 100644
--- a/drivers/net/wireless/marvell/mwifiex/11n_rxreorder.c
+++ b/drivers/net/wireless/marvell/mwifiex/11n_rxreorder.c
@@ -76,7 +76,8 @@ static int mwifiex_11n_dispatch_amsdu_pkt(struct mwifiex_private *priv,
 /* This function will process the rx packet and forward it to kernel/upper
  * layer.
  */
-static int mwifiex_11n_dispatch_pkt(struct mwifiex_private *priv, void *payload)
+static int mwifiex_11n_dispatch_pkt(struct mwifiex_private *priv,
+				    struct sk_buff *payload)
 {
 
 	int ret;
@@ -109,27 +110,26 @@ mwifiex_11n_dispatch_pkt_until_start_win(struct mwifiex_private *priv,
 					 struct mwifiex_rx_reorder_tbl *tbl,
 					 int start_win)
 {
+	struct sk_buff_head list;
+	struct sk_buff *skb;
 	int pkt_to_send, i;
-	void *rx_tmp_ptr;
 	unsigned long flags;
 
+	__skb_queue_head_init(&list);
+	spin_lock_irqsave(&priv->rx_reorder_tbl_lock, flags);
+
 	pkt_to_send = (start_win > tbl->start_win) ?
 		      min((start_win - tbl->start_win), tbl->win_size) :
 		      tbl->win_size;
 
 	for (i = 0; i < pkt_to_send; ++i) {
-		spin_lock_irqsave(&priv->rx_reorder_tbl_lock, flags);
-		rx_tmp_ptr = NULL;
 		if (tbl->rx_reorder_ptr[i]) {
-			rx_tmp_ptr = tbl->rx_reorder_ptr[i];
+			skb = tbl->rx_reorder_ptr[i];
+			__skb_queue_tail(&list, skb);
 			tbl->rx_reorder_ptr[i] = NULL;
 		}
-		spin_unlock_irqrestore(&priv->rx_reorder_tbl_lock, flags);
-		if (rx_tmp_ptr)
-			mwifiex_11n_dispatch_pkt(priv, rx_tmp_ptr);
 	}
 
-	spin_lock_irqsave(&priv->rx_reorder_tbl_lock, flags);
 	/*
 	 * We don't have a circular buffer, hence use rotation to simulate
 	 * circular buffer
@@ -141,6 +141,9 @@ mwifiex_11n_dispatch_pkt_until_start_win(struct mwifiex_private *priv,
 
 	tbl->start_win = start_win;
 	spin_unlock_irqrestore(&priv->rx_reorder_tbl_lock, flags);
+
+	while ((skb = __skb_dequeue(&list)))
+		mwifiex_11n_dispatch_pkt(priv, skb);
 }
 
 /*
@@ -155,24 +158,22 @@ static void
 mwifiex_11n_scan_and_dispatch(struct mwifiex_private *priv,
 			      struct mwifiex_rx_reorder_tbl *tbl)
 {
+	struct sk_buff_head list;
+	struct sk_buff *skb;
 	int i, j, xchg;
-	void *rx_tmp_ptr;
 	unsigned long flags;
 
+	__skb_queue_head_init(&list);
+	spin_lock_irqsave(&priv->rx_reorder_tbl_lock, flags);
+
 	for (i = 0; i < tbl->win_size; ++i) {
-		spin_lock_irqsave(&priv->rx_reorder_tbl_lock, flags);
-		if (!tbl->rx_reorder_ptr[i]) {
-			spin_unlock_irqrestore(&priv->rx_reorder_tbl_lock,
-					       flags);
+		if (!tbl->rx_reorder_ptr[i])
 			break;
-		}
-		rx_tmp_ptr = tbl->rx_reorder_ptr[i];
+		skb = tbl->rx_reorder_ptr[i];
+		__skb_queue_tail(&list, skb);
 		tbl->rx_reorder_ptr[i] = NULL;
-		spin_unlock_irqrestore(&priv->rx_reorder_tbl_lock, flags);
-		mwifiex_11n_dispatch_pkt(priv, rx_tmp_ptr);
 	}
 
-	spin_lock_irqsave(&priv->rx_reorder_tbl_lock, flags);
 	/*
 	 * We don't have a circular buffer, hence use rotation to simulate
 	 * circular buffer
@@ -185,7 +186,11 @@ mwifiex_11n_scan_and_dispatch(struct mwifiex_private *priv,
 		}
 	}
 	tbl->start_win = (tbl->start_win + i) & (MAX_TID_VALUE - 1);
+
 	spin_unlock_irqrestore(&priv->rx_reorder_tbl_lock, flags);
+
+	while ((skb = __skb_dequeue(&list)))
+		mwifiex_11n_dispatch_pkt(priv, skb);
 }
 
 /*
-- 
2.22.0.rc1.311.g5d7573a151-goog


^ 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