diff mbox series

[2/3] brcmfmac: return error when getting invalid max_flowrings from dongle

Message ID 20220929031001.9962-3-ian.lin@infineon.com
State New
Headers show
Series brcmfmac: PCIE debug mechanism series | expand

Commit Message

Ian Lin Sept. 29, 2022, 3:10 a.m. UTC
From: Wright Feng <wright.feng@cypress.com>

When firmware hit trap at initialization, host will read abnormal
max_flowrings number from dongle, and it will cause kernel panic when
doing iowrite to initialize dongle ring.
To detect this error at early stage, we directly return error when getting
invalid max_flowrings(>256).

Signed-off-by: Wright Feng <wright.feng@cypress.com>
Signed-off-by: Chi-hsien Lin <chi-hsien.lin@cypress.com>
Signed-off-by: Ian Lin <ian.lin@infineon.com>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Arend Van Spriel Oct. 10, 2022, 9:58 a.m. UTC | #1
On 9/29/2022 5:10 AM, Ian Lin wrote:
> From: Wright Feng <wright.feng@cypress.com>
> 
> When firmware hit trap at initialization, host will read abnormal
> max_flowrings number from dongle, and it will cause kernel panic when
> doing iowrite to initialize dongle ring.
> To detect this error at early stage, we directly return error when getting
> invalid max_flowrings(>256).
> 
> Signed-off-by: Wright Feng <wright.feng@cypress.com>
> Signed-off-by: Chi-hsien Lin <chi-hsien.lin@cypress.com>
> Signed-off-by: Ian Lin <ian.lin@infineon.com>
> ---
>   drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
> index 2b7ebbd7b5b4..1becd50038ab 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
> @@ -1228,6 +1228,10 @@ static int brcmf_pcie_init_ringbuffers(struct brcmf_pciedev_info *devinfo)
>   				BRCMF_NROF_H2D_COMMON_MSGRINGS;
>   		max_completionrings = BRCMF_NROF_D2H_COMMON_MSGRINGS;
>   	}
> +	if (max_flowrings > 256) {
This limit is a bit of a magic value. I do know there are chipsets that 
support more that 256 flowrings so this is a hard limitation. I should 
really get the multi-vendor support in place so we can have the 
limitation only for Infineon/Cypress chips. I will try to revive that 
thread.

Regards,
Arend
chainofflowers Jan. 8, 2023, 8:53 p.m. UTC | #2
Hi! This patch:
https://lore.kernel.org/all/20220929031001.9962-3-ian.lin@infineon.com/

causes my Asus PCE-AC88 (1043:86fb) to stop working, because when modprobe 
loads brcmfmac it throws this error:
---
[ 2740.647600] brcmfmac 0000:0d:00.0: brcmf_pcie_init_ringbuffers: invalid 
max_flowrings(264).
---

This is also being reported on reddit:
https://www.reddit.com/r/archlinux/comments/104pqv9/
no_wifi_since_kernel_update_yesterday/

(also, see https://forum.manjaro.org/t/brcmfmac-fails-to-load/131128)

I think that the check on value > 256 is used as a kind of red herring to 
indirectly spot an inconsistent properties report from the PCI bus (?) about 
the network card.
Before that patch, the driver was correctly working for me and I could not 
observe any kernel panic. Maybe, at least in my case, the wrong value was 
still reported but could be safely ignored, as at a later point in time, when 
the card is initialised, everything worked fine.

I am now stuck at kernel 6.1.1, if I upgrade I cannot use the card.
Is there anything I can do to circumvent this issue? May I help somehow with 
debugging & testing? Please let me know...

Thanks,


(c)
Arend Van Spriel Jan. 9, 2023, 8:52 a.m. UTC | #3
On 1/8/2023 9:53 PM, chainofflowers wrote:
> Hi! This patch:
> https://lore.kernel.org/all/20220929031001.9962-3-ian.lin@infineon.com/
> 
> causes my Asus PCE-AC88 (1043:86fb) to stop working, because when modprobe
> loads brcmfmac it throws this error:
> ---
> [ 2740.647600] brcmfmac 0000:0d:00.0: brcmf_pcie_init_ringbuffers: invalid
> max_flowrings(264).
> ---
> 
> This is also being reported on reddit:
> https://www.reddit.com/r/archlinux/comments/104pqv9/
> no_wifi_since_kernel_update_yesterday/
> 
> (also, see https://forum.manjaro.org/t/brcmfmac-fails-to-load/131128)
> 
> I think that the check on value > 256 is used as a kind of red herring to
> indirectly spot an inconsistent properties report from the PCI bus (?) about
> the network card.

The firmware loaded into the wifi device requests the host for 264 
flowrings. This was apparently not expected to be a valid value, but 
your device shows it is.

> Before that patch, the driver was correctly working for me and I could not
> observe any kernel panic. Maybe, at least in my case, the wrong value was
> still reported but could be safely ignored, as at a later point in time, when
> the card is initialised, everything worked fine.
> 
> I am now stuck at kernel 6.1.1, if I upgrade I cannot use the card.
> Is there anything I can do to circumvent this issue? May I help somehow with
> debugging & testing? Please let me know...

6.2 is not formally released so I suppose you are an early adopter and 
aware of the risk that implies ;-) There is no real debugging needed. I 
will figure out what a sensible value would be and come up with a patch. 
If you can test that that would be great. Will let you know.

Regards,
Arend
chainofflowers Jan. 9, 2023, 5:23 p.m. UTC | #4
On 09.01.23 09:52, Arend van Spriel wrote:
> 6.2 is not formally released so I suppose you are an early adopter and 
> aware of the risk that implies ;-) There is no real debugging needed. I 
> will figure out what a sensible value would be and come up with a patch. 

I am on 6.1. Version 6.1.1 works, 6.1.3 does not.
Will this patch be provided with 6.1.4? Or shall I wait until 6.2.0?

> If you can test that that would be great. Will let you know.
Sure! :)
diff mbox series

Patch

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
index 2b7ebbd7b5b4..1becd50038ab 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
@@ -1228,6 +1228,10 @@  static int brcmf_pcie_init_ringbuffers(struct brcmf_pciedev_info *devinfo)
 				BRCMF_NROF_H2D_COMMON_MSGRINGS;
 		max_completionrings = BRCMF_NROF_D2H_COMMON_MSGRINGS;
 	}
+	if (max_flowrings > 256) {
+		brcmf_err(bus, "invalid max_flowrings(%d)\n", max_flowrings);
+		return -EIO;
+	}
 
 	if (devinfo->dma_idx_sz != 0) {
 		bufsz = (max_submissionrings + max_completionrings) *