linux-kernel - RE: [PATCH] mwifiex: pcie: implement timeout loop for FW programming doorbell

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 24 Nov 2016 13:16:37 +0000
From:   Amitkumar Karwar <akarwar@...vell.com>
To:     Brian Norris <briannorris@...omium.org>,
        Nishant Sarmukadam <nishants@...vell.com>
CC:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Kalle Valo <kvalo@...eaurora.org>,
        "linux-wireless@...r.kernel.org" <linux-wireless@...r.kernel.org>,
        Cathy Luo <cluo@...vell.com>,
        "Dmitry Torokhov" <dmitry.torokhov@...il.com>
Subject: RE: [PATCH] mwifiex: pcie: implement timeout loop for FW programming
 doorbell

> From: Brian Norris [mailto:briannorris@...omium.org]
> Sent: Wednesday, November 23, 2016 8:10 AM
> To: Amitkumar Karwar; Nishant Sarmukadam
> Cc: linux-kernel@...r.kernel.org; Kalle Valo; linux-
> wireless@...r.kernel.org; Cathy Luo; Dmitry Torokhov; Brian Norris
> Subject: [PATCH] mwifiex: pcie: implement timeout loop for FW
> programming doorbell
> 
> Marvell Wifi PCIe modules don't always behave nicely for PCIe power
> management when their firmware hasn't been loaded, particularly after
> suspending the PCIe link one or more times. When this happens, we might
> end up spinning forever in this status-polling tight loop. Let's make
> this less tight by adding a timeout and by sleeping a bit in between
> reads, as we do with the other similar loops.
> 
> This prevents us from hogging a CPU even in such pathological cases,
> and allows the FW initialization to just fail gracefully instead.
> 
> I chose the same polling parameters as the earlier loop in this
> function, and empirically, I found that this loop never makes it more
> than about 12 cycles in a sane FW init sequence. I had no official
> information on the actual intended latency for this portion of the
> download.
> 
> Signed-off-by: Brian Norris <briannorris@...omium.org>
> ---
>  drivers/net/wireless/marvell/mwifiex/pcie.c | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c
> b/drivers/net/wireless/marvell/mwifiex/pcie.c
> index 4b89f557d0b6..9f9ea1350591 100644
> --- a/drivers/net/wireless/marvell/mwifiex/pcie.c
> +++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
> @@ -2050,7 +2050,7 @@ static int mwifiex_prog_fw_w_helper(struct
> mwifiex_adapter *adapter,
>  		}
> 
>  		/* Wait for the command done interrupt */
> -		do {
> +		for (tries = 0; tries < MAX_POLL_TRIES; tries++) {
>  			if (mwifiex_read_reg(adapter, PCIE_CPU_INT_STATUS,
>  					     &ireg_intr)) {
>  				mwifiex_dbg(adapter, ERROR,
> @@ -2062,8 +2062,18 @@ static int mwifiex_prog_fw_w_helper(struct
> mwifiex_adapter *adapter,
>  				ret = -1;
>  				goto done;
>  			}
> -		} while ((ireg_intr & CPU_INTR_DOOR_BELL) ==
> -			 CPU_INTR_DOOR_BELL);
> +			if (!(ireg_intr & CPU_INTR_DOOR_BELL))
> +				break;
> +			usleep_range(10, 20);
> +		}
> +		if (ireg_intr & CPU_INTR_DOOR_BELL) {
> +			mwifiex_dbg(adapter, ERROR, "%s: Card failed to ACK
> download\n",
> +				    __func__);
> +			mwifiex_unmap_pci_memory(adapter, skb,
> +						 PCI_DMA_TODEVICE);
> +			ret = -1;
> +			goto done;
> +		}
> 
>  		mwifiex_unmap_pci_memory(adapter, skb, PCI_DMA_TODEVICE);
> 

Patch looks fine to me.

Acked-by: Amitkumar Karwar <akarwar@...vell.com>

Regards,
Amitkumar