Linux wireless drivers development
 help / color / mirror / Atom feed
* Re: [RFC PATCH v3] rtl8xxxu: Improve TX performance of RTL8723BU on rtl8xxxu driver
From: Larry Finger @ 2019-05-29 18:22 UTC (permalink / raw)
  To: Chris Chiu, jes.sorensen, kvalo, davem
  Cc: linux-wireless, netdev, linux-kernel, linux
In-Reply-To: <20190529050335.72061-1-chiu@endlessm.com>

On 5/29/19 12:03 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 driver from
> https://github.com/lwfinger/rtl8723bu, the same iperf test gets ~12
> Mbps or more. The signal strength is reported similarly around
> -40dBm. That's why we want to improve.

The driver at GitHub was written by Realtek. I only published it in a prominent 
location, and fix it for kernel API changes. I would say "the Realtek driver at 
https://...", and every mention of "Larry's driver" should say "Realtek's 
driver". That attribution is more correct.
> 
> After reading the source code of the rtl8xxxu driver and Larry's, the
> major difference is that Larry'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 Larry'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 have not tested this patch, but I plan to soon.

Larry



^ permalink raw reply

* Re: [RFC PATCH v3] rtl8xxxu: Improve TX performance of RTL8723BU on rtl8xxxu driver
From: Daniel Drake @ 2019-05-29 18:11 UTC (permalink / raw)
  To: Chris Chiu
  Cc: Jes Sorensen, Kalle Valo, David Miller, linux-wireless, netdev,
	Linux Kernel, Linux Upstreaming Team
In-Reply-To: <20190529050335.72061-1-chiu@endlessm.com>

Hi Chris,

On Tue, May 28, 2019 at 11:03 PM Chris Chiu <chiu@endlessm.com> wrote:
> +       /*
> +        * Single virtual interface permitted since the driver supports STATION
> +        * mode only.

I think you can be a bit more explicit by saying e.g.:

Only one virtual interface permitted because only STA mode is
supported and no iface_combinations are provided.

> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> index 039e5ca9d2e4..2d612c2df5b2 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> @@ -4345,7 +4345,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 = priv->ratr_index;

I think ratr_index can be moved to be a function parameter of the
update_rate_mask function. It looks like all callsites already know
which value they want to set. Then you don't have to store it in the
priv structure.

> @@ -5471,6 +5509,10 @@ static int rtl8xxxu_add_interface(struct ieee80211_hw *hw,
>
>         switch (vif->type) {
>         case NL80211_IFTYPE_STATION:
> +               if (!priv->vif)
> +                       priv->vif = vif;
> +               else
> +                       return -EOPNOTSUPP;
>                 rtl8xxxu_stop_tx_beacon(priv);

rtl8xxxu_remove_interface should also set priv->vif back to NULL.

> @@ -6183,6 +6259,8 @@ static void rtl8xxxu_disconnect(struct usb_interface *interface)
>         mutex_destroy(&priv->usb_buf_mutex);
>         mutex_destroy(&priv->h2c_mutex);
>
> +       cancel_delayed_work_sync(&priv->ra_watchdog);

Given that the work was started in rtl8xxxu_start, I think it should
be cancelled in rtl8xxxu_stop() instead.

Daniel

^ permalink raw reply

* Re: [PATCH 02/10] cfg80211: use BIT_ULL in cfg80211_parse_mbssid_data()
From: Kalle Valo @ 2019-05-29 16:28 UTC (permalink / raw)
  To: Luca Coelho; +Cc: johannes, linux-wireless, Luca Coelho, Dan Carpented
In-Reply-To: <20190529122537.8564-3-luca@coelho.fi>

Luca Coelho <luca@coelho.fi> writes:

> From: Luca Coelho <luciano.coelho@intel.com>
>
> The seen_indices variable is u64 and in other parts of the code we
> assume mbssid_index_ie[2] can be up to 45, so we should use the 64-bit
> versions of BIT, namely, BIT_ULL().
>
> Reported-by: Dan Carpented <dan.carpenter@oracle.com>

s/ted/ter/ :)

-- 
Kalle Valo

^ permalink raw reply

* Re: FYI: vendor specific nl80211 API upstream
From: Denis Kenzior @ 2019-05-29 15:49 UTC (permalink / raw)
  To: Johannes Berg, linux-wireless
In-Reply-To: <f3847d4efe6822bba3948993ddc4cde31615436f.camel@sipsolutions.net>

Hi Johannes,

On 05/29/2019 04:09 AM, Johannes Berg wrote:
> On Tue, 2019-05-28 at 12:36 -0500, Denis Kenzior wrote:
>>
>> I'm guessing that you guys considered and rejected the idea of pushing
>> these out to a separate, vendor specific genl family instead?
> 
> We do actually use that internally (though mostly for cases where we
> don't have a cfg80211 connection like manufacturing support), but vendor
> commands are there and people do like to use them :-)

And herein lies the danger.  If you make it too easy to add vendor APIs, 
there's no incentive for the vendors to do anything else.  In the end 
this all becomes a mess for userspace to deal with.

One idea off the top of my head is to introduce a concept of 
'experimental' APIs in NL80211, ones that are not guaranteed to be ABI 
stable going forward.  Specifically for dealing with such 'vendor' APIs. 
  The semantic difference might be subtle, but I think the effect will 
be drastically different.  E.g. people will approach this more seriously 
and you will get more people reviewing the API.

> 
> The idea with formalizing this is that they actually get more
> visibility, and I hope that this will lead to more forming of real
> nl80211 API too.

What about ABI guarantees (to tie it in with the discussion above) ?
If the vendor wants to change their API, can they?  Are NL80211 APIs 
stable unless they are vendor APIs?

Anyhow, speaking from experience with oFono, which has to deal with a 
bazillion of wwan modem vendors, I suspect that the opposite will 
actually happen.  Any time we let through a vendor API, the vendor lost 
any interest in generalizing it further.  And it becomes a huge pain to 
implement a proper generic one later.  I get that there are cases where 
something just cannot be generalized.  In that case it belongs on a 
separate genl family (or whatever) altogether.

So I would highly encourage you to reconsider this decision and 
deprecate vendor APIs altogether.  If someone really cares, they can 
implement their own genl family.  It is really not that hard.  And then 
they control the API, API stability policy, etc.

Regards,
-Denis

^ permalink raw reply

* Re: [PATCH] rtlwifi: Fix null-pointer dereferences in error handling code of rtl_pci_probe()
From: Larry Finger @ 2019-05-29 15:48 UTC (permalink / raw)
  To: Jia-Ju Bai, Kalle Valo
  Cc: pkshih, davem, linux-wireless, netdev, linux-kernel
In-Reply-To: <357682ba-d1ae-23b1-2372-b1a33d2ba1ac@gmail.com>

On 5/29/19 5:30 AM, Jia-Ju Bai wrote:
> 
> 
> On 2019/5/28 21:00, Larry Finger wrote:
>> On 5/28/19 6:55 AM, Kalle Valo wrote:
>>> Jia-Ju Bai <baijiaju1990@gmail.com> wrote:
>>>
>>>> *BUG 1:
>>>> In rtl_pci_probe(), when rtlpriv->cfg->ops->init_sw_vars() fails,
>>>> rtl_deinit_core() in the error handling code is executed.
>>>> rtl_deinit_core() calls rtl_free_entries_from_scan_list(), which uses
>>>> rtlpriv->scan_list.list in list_for_each_entry_safe(), but it has been
>>>> initialized. Thus a null-pointer dereference occurs.
>>>> The reason is that rtlpriv->scan_list.list is initialized by
>>>> INIT_LIST_HEAD() in rtl_init_core(), which has not been called.
>>>>
>>>> To fix this bug, rtl_deinit_core() should not be called when
>>>> rtlpriv->cfg->ops->init_sw_vars() fails.
>>>>
>>>> *BUG 2:
>>>> In rtl_pci_probe(), rtl_init_core() can fail when rtl_regd_init() in
>>>> this function fails, and rtlpriv->scan_list.list has not been
>>>> initialized by INIT_LIST_HEAD(). Then, rtl_deinit_core() in the error
>>>> handling code of rtl_pci_probe() is executed. Finally, a null-pointer
>>>> dereference occurs due to the same reason of the above bug.
>>>>
>>>> To fix this bug, the initialization of lists in rtl_init_core() are
>>>> performed before the call to rtl_regd_init().
>>>>
>>>> These bugs are found by a runtime fuzzing tool named FIZZER written by
>>>> us.
>>>>
>>>> Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
>>>
>>> Ping & Larry, is this ok to take?
>>>
>>
>> Kalle,
>>
>> Not at the moment. In reviewing the code, I was unable to see how this 
>> situation could develop, and his backtrace did not mention any rtlwifi code. 
>> For that reason, I asked him to add printk stat4ements to show the last part 
>> of rtl_pci that executed correctly. In 
>> https://marc.info/?l=linux-wireless&m=155788322631134&w=2, he promised to do 
>> that, but I have not seen the result.
>>
> 
> Hi Larry,
> 
> This patch is not related to the message you mentioned.
> That message is about an occasional crash that I reported.
> That crash occurred when request_irq() in rtl_pci_intr_mode_legacy() in 
> rtl_pci_intr_mode_decide() fails.
> I have added printk statements and try to reproduce and debug that crash, but 
> that crash does not always occur, and I still do not know the root cause of that 
> crash.
> 
> The null-pointer dereferences fixed by this patch are different from that crash, 
> and they always occur when the related functions fail.
> So please review these null-pointer dereferences, thanks :)
> 
> 
> Best wishes,
> Jia-Ju Bai

Sorry if I got confused. Kalle has dropped this patch, thus you will need to 
submit a new version.

Larry


^ permalink raw reply

* Re: [PATCH 01/11] rtw88: resolve order of tx power setting routines
From: Larry Finger @ 2019-05-29 15:16 UTC (permalink / raw)
  To: yhchuang, kvalo; +Cc: linux-wireless
In-Reply-To: <1559116487-5244-2-git-send-email-yhchuang@realtek.com>

On 5/29/19 2:54 AM, yhchuang@realtek.com wrote:
> From: Yan-Hsuan Chuang <yhchuang@realtek.com>
> 
> Some functions that should be static are unnecessarily exposed, remove
> their declaration in header file phy.h.
> 
> After resolving their declaration order, they can be declared as static.
> So this commit changes nothing except the order and marking them static.
> 
> Signed-off-by: Yan-Hsuan Chuang <yhchuang@realtek.com>

This patch does not apply. Using quilt to see what is wrong, there are 6 changes 
that have already been applied.

Larry

^ permalink raw reply

* [PATCH] rtw88: Remove set but not used variable 'ip_sel' and 'orig'
From: YueHaibing @ 2019-05-29 14:57 UTC (permalink / raw)
  To: yhchuang, kvalo, davem; +Cc: linux-kernel, netdev, linux-wireless, YueHaibing

Fixes gcc '-Wunused-but-set-variable' warnings:

drivers/net/wireless/realtek/rtw88/pci.c: In function rtw_pci_phy_cfg:
drivers/net/wireless/realtek/rtw88/pci.c:978:6: warning: variable ip_sel set but not used [-Wunused-but-set-variable]
drivers/net/wireless/realtek/rtw88/phy.c: In function phy_tx_power_limit_config:
drivers/net/wireless/realtek/rtw88/phy.c:1607:11: warning: variable orig set but not used [-Wunused-but-set-variable]

They are never used, so can be removed.

Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
 drivers/net/wireless/realtek/rtw88/pci.c | 3 ---
 drivers/net/wireless/realtek/rtw88/phy.c | 3 +--
 2 files changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw88/pci.c b/drivers/net/wireless/realtek/rtw88/pci.c
index 353871c27779..8329f4e447b7 100644
--- a/drivers/net/wireless/realtek/rtw88/pci.c
+++ b/drivers/net/wireless/realtek/rtw88/pci.c
@@ -977,7 +977,6 @@ static void rtw_pci_phy_cfg(struct rtw_dev *rtwdev)
 	u16 cut;
 	u16 value;
 	u16 offset;
-	u16 ip_sel;
 	int i;
 
 	cut = BIT(0) << rtwdev->hal.cut_version;
@@ -990,7 +989,6 @@ static void rtw_pci_phy_cfg(struct rtw_dev *rtwdev)
 			break;
 		offset = para->offset;
 		value = para->value;
-		ip_sel = para->ip_sel;
 		if (para->ip_sel == RTW_IP_SEL_PHY)
 			rtw_mdio_write(rtwdev, offset, value, true);
 		else
@@ -1005,7 +1003,6 @@ static void rtw_pci_phy_cfg(struct rtw_dev *rtwdev)
 			break;
 		offset = para->offset;
 		value = para->value;
-		ip_sel = para->ip_sel;
 		if (para->ip_sel == RTW_IP_SEL_PHY)
 			rtw_mdio_write(rtwdev, offset, value, false);
 		else
diff --git a/drivers/net/wireless/realtek/rtw88/phy.c b/drivers/net/wireless/realtek/rtw88/phy.c
index 404d89432c96..c3e75ffe27b5 100644
--- a/drivers/net/wireless/realtek/rtw88/phy.c
+++ b/drivers/net/wireless/realtek/rtw88/phy.c
@@ -1604,12 +1604,11 @@ void rtw_phy_tx_power_by_rate_config(struct rtw_hal *hal)
 static void
 phy_tx_power_limit_config(struct rtw_hal *hal, u8 regd, u8 bw, u8 rs)
 {
-	s8 base, orig;
+	s8 base;
 	u8 ch;
 
 	for (ch = 0; ch < RTW_MAX_CHANNEL_NUM_2G; ch++) {
 		base = hal->tx_pwr_by_rate_base_2g[0][rs];
-		orig = hal->tx_pwr_limit_2g[regd][bw][rs][ch];
 		hal->tx_pwr_limit_2g[regd][bw][rs][ch] -= base;
 	}
 
-- 
2.17.1



^ permalink raw reply related

* [PATCH] mt76: Remove set but not used variables 'pid' and 'final_mpdu'
From: YueHaibing @ 2019-05-29 14:53 UTC (permalink / raw)
  To: nbd, lorenzo.bianconi83, ryder.lee, royluo, kvalo, matthias.bgg,
	sgruszka
  Cc: linux-kernel, netdev, linux-wireless, linux-arm-kernel,
	linux-mediatek, davem, YueHaibing

Fixes gcc '-Wunused-but-set-variable' warnings:

drivers/net/wireless/mediatek/mt76/mt7603/mac.c: In function mt7603_fill_txs:
drivers/net/wireless/mediatek/mt76/mt7603/mac.c:969:5: warning: variable pid set but not used [-Wunused-but-set-variable]
drivers/net/wireless/mediatek/mt76/mt7603/mac.c:961:7: warning: variable final_mpdu set but not used [-Wunused-but-set-variable]
drivers/net/wireless/mediatek/mt76/mt7615/mac.c: In function mt7615_fill_txs:
drivers/net/wireless/mediatek/mt76/mt7615/mac.c:555:5: warning: variable pid set but not used [-Wunused-but-set-variable]
drivers/net/wireless/mediatek/mt76/mt7615/mac.c:552:19: warning: variable final_mpdu set but not used [-Wunused-but-set-variable]

They are never used, so can be removed.

Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
 drivers/net/wireless/mediatek/mt76/mt7603/mac.c | 4 ----
 drivers/net/wireless/mediatek/mt76/mt7615/mac.c | 5 +----
 2 files changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt7603/mac.c b/drivers/net/wireless/mediatek/mt76/mt7603/mac.c
index 6d506e34c3ee..5182a36276fc 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7603/mac.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7603/mac.c
@@ -958,7 +958,6 @@ mt7603_fill_txs(struct mt7603_dev *dev, struct mt7603_sta *sta,
 	int final_idx = 0;
 	u32 final_rate;
 	u32 final_rate_flags;
-	bool final_mpdu;
 	bool ack_timeout;
 	bool fixed_rate;
 	bool probe;
@@ -966,7 +965,6 @@ mt7603_fill_txs(struct mt7603_dev *dev, struct mt7603_sta *sta,
 	bool cck = false;
 	int count;
 	u32 txs;
-	u8 pid;
 	int idx;
 	int i;
 
@@ -974,9 +972,7 @@ mt7603_fill_txs(struct mt7603_dev *dev, struct mt7603_sta *sta,
 	probe = !!(info->flags & IEEE80211_TX_CTL_RATE_CTRL_PROBE);
 
 	txs = le32_to_cpu(txs_data[4]);
-	final_mpdu = txs & MT_TXS4_ACKED_MPDU;
 	ampdu = !fixed_rate && (txs & MT_TXS4_AMPDU);
-	pid = FIELD_GET(MT_TXS4_PID, txs);
 	count = FIELD_GET(MT_TXS4_TX_COUNT, txs);
 
 	txs = le32_to_cpu(txs_data[0]);
diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/mac.c b/drivers/net/wireless/mediatek/mt76/mt7615/mac.c
index b8f48d10f27a..a51bfb6990b3 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7615/mac.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7615/mac.c
@@ -549,23 +549,20 @@ static bool mt7615_fill_txs(struct mt7615_dev *dev, struct mt7615_sta *sta,
 {
 	struct ieee80211_supported_band *sband;
 	int i, idx, count, final_idx = 0;
-	bool fixed_rate, final_mpdu, ack_timeout;
+	bool fixed_rate, ack_timeout;
 	bool probe, ampdu, cck = false;
 	u32 final_rate, final_rate_flags, final_nss, txs;
-	u8 pid;
 
 	fixed_rate = info->status.rates[0].count;
 	probe = !!(info->flags & IEEE80211_TX_CTL_RATE_CTRL_PROBE);
 
 	txs = le32_to_cpu(txs_data[1]);
-	final_mpdu = txs & MT_TXS1_ACKED_MPDU;
 	ampdu = !fixed_rate && (txs & MT_TXS1_AMPDU);
 
 	txs = le32_to_cpu(txs_data[3]);
 	count = FIELD_GET(MT_TXS3_TX_COUNT, txs);
 
 	txs = le32_to_cpu(txs_data[0]);
-	pid = FIELD_GET(MT_TXS0_PID, txs);
 	final_rate = FIELD_GET(MT_TXS0_TX_RATE, txs);
 	ack_timeout = txs & MT_TXS0_ACK_TIMEOUT;
 
-- 
2.17.1



^ permalink raw reply related

* Re: [PATCH 1/3] brcmfmac: re-enable command decode in sdio_aos for BRCM 4354
From: Kalle Valo @ 2019-05-29 14:51 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Arend van Spriel, Madhan Mohan R, Ulf Hansson, LKML,
	Hante Meuleman, David S. Miller, netdev, Chi-Hsien Lin,
	Brian Norris, linux-wireless, YueHaibing, Adrian Hunter,
	open list:ARM/Rockchip SoC..., brcm80211-dev-list.pdl,
	Matthias Kaehlcke, Naveen Gupta, Wright Feng, brcm80211-dev-list,
	Double Lo, Franky Lin
In-Reply-To: <CAD=FV=VtxdEeFQsdF=U7-_7R+TXfVmA2_JMB_-WYidGHTLDgLw@mail.gmail.com>

Doug Anderson <dianders@chromium.org> writes:

> Hi,
>
> On Tue, May 28, 2019 at 5:18 AM Kalle Valo <kvalo@codeaurora.org> wrote:
>>
>> Douglas Anderson <dianders@chromium.org> wrote:
>>
>> > In commit 29f6589140a1 ("brcmfmac: disable command decode in
>> > sdio_aos") we disabled something called "command decode in sdio_aos"
>> > for a whole bunch of Broadcom SDIO WiFi parts.
>> >
>> > After that patch landed I find that my kernel log on
>> > rk3288-veyron-minnie and rk3288-veyron-speedy is filled with:
>> >   brcmfmac: brcmf_sdio_bus_sleep: error while changing bus sleep state -110
>> >
>> > This seems to happen every time the Broadcom WiFi transitions out of
>> > sleep mode.  Reverting the part of the commit that affects the WiFi on
>> > my boards fixes the problem for me, so that's what this patch does.
>> >
>> > Note that, in general, the justification in the original commit seemed
>> > a little weak.  It looked like someone was testing on a SD card
>> > controller that would sometimes die if there were CRC errors on the
>> > bus.  This used to happen back in early days of dw_mmc (the controller
>> > on my boards), but we fixed it.  Disabling a feature on all boards
>> > just because one SD card controller is broken seems bad.  ...so
>> > instead of just this patch possibly the right thing to do is to fully
>> > revert the original commit.
>> >
>> > Fixes: 29f6589140a1 ("brcmfmac: disable command decode in sdio_aos")
>> > Signed-off-by: Douglas Anderson <dianders@chromium.org>
>>
>> I don't see patch 2 in patchwork and I assume discussion continues.
>
> Apologies.  I made sure to CC you individually on all the patches but
> didn't think about the fact that you use patchwork to manage and so
> didn't ensure all patches made it to all lists (by default each patch
> gets recipients individually from get_maintainer).  I'll make sure to
> fix for patch set #2.  If you want to see all the patches, you can at
> least find them on lore.kernel.org linked from the cover:
>
> https://lore.kernel.org/patchwork/cover/1075373/

No worries, I had the thread on my email but was just too busy to check.
So I instead wrote down my thought process so that somebode can correct
me in case I have misunderstood. I usually do that when it's not clear
what the next action should be.

-- 
Kalle Valo

^ permalink raw reply

* Re: [PATCH] rtlwifi: rtl8192cu: fix error handle when usb probe failed
From: Larry Finger @ 2019-05-29 14:51 UTC (permalink / raw)
  To: pkshih, kvalo; +Cc: linux-wireless, andreyknvl
In-Reply-To: <20190529065730.25951-1-pkshih@realtek.com>

On 5/29/19 1:57 AM, pkshih@realtek.com wrote:
> From: Ping-Ke Shih <pkshih@realtek.com>
> 
> rtl_usb_probe() must do error handle rtl_deinit_core() only if
> rtl_init_core() is done, otherwise goto error_out2.
> 
> | usb 1-1: New USB device strings: Mfr=0, Product=0, SerialNumber=0
> | rtl_usb: reg 0xf0, usbctrl_vendorreq TimeOut! status:0xffffffb9 value=0x0
> | rtl8192cu: Chip version 0x10
> | rtl_usb: reg 0xa, usbctrl_vendorreq TimeOut! status:0xffffffb9 value=0x0
> | rtl_usb: Too few input end points found
> | INFO: trying to register non-static key.
> | the code is fine but needs lockdep annotation.
> | turning off the locking correctness validator.
> | CPU: 0 PID: 12 Comm: kworker/0:1 Not tainted 5.1.0-rc4-319354-g9a33b36 #3
> | 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+0xe8/0x16e lib/dump_stack.c:113
> |   assign_lock_key kernel/locking/lockdep.c:786 [inline]
> |   register_lock_class+0x11b8/0x1250 kernel/locking/lockdep.c:1095
> |   __lock_acquire+0xfb/0x37c0 kernel/locking/lockdep.c:3582
> |   lock_acquire+0x10d/0x2f0 kernel/locking/lockdep.c:4211
> |   __raw_spin_lock_irqsave include/linux/spinlock_api_smp.h:110 [inline]
> |   _raw_spin_lock_irqsave+0x44/0x60 kernel/locking/spinlock.c:152
> |   rtl_c2hcmd_launcher+0xd1/0x390
> | drivers/net/wireless/realtek/rtlwifi/base.c:2344
> |   rtl_deinit_core+0x25/0x2d0 drivers/net/wireless/realtek/rtlwifi/base.c:574
> |   rtl_usb_probe.cold+0x861/0xa70
> | drivers/net/wireless/realtek/rtlwifi/usb.c:1093
> |   usb_probe_interface+0x31d/0x820 drivers/usb/core/driver.c:361
> |   really_probe+0x2da/0xb10 drivers/base/dd.c:509
> |   driver_probe_device+0x21d/0x350 drivers/base/dd.c:671
> |   __device_attach_driver+0x1d8/0x290 drivers/base/dd.c:778
> |   bus_for_each_drv+0x163/0x1e0 drivers/base/bus.c:454
> |   __device_attach+0x223/0x3a0 drivers/base/dd.c:844
> |   bus_probe_device+0x1f1/0x2a0 drivers/base/bus.c:514
> |   device_add+0xad2/0x16e0 drivers/base/core.c:2106
> |   usb_set_configuration+0xdf7/0x1740 drivers/usb/core/message.c:2021
> |   generic_probe+0xa2/0xda drivers/usb/core/generic.c:210
> |   usb_probe_device+0xc0/0x150 drivers/usb/core/driver.c:266
> |   really_probe+0x2da/0xb10 drivers/base/dd.c:509
> |   driver_probe_device+0x21d/0x350 drivers/base/dd.c:671
> |   __device_attach_driver+0x1d8/0x290 drivers/base/dd.c:778
> |   bus_for_each_drv+0x163/0x1e0 drivers/base/bus.c:454
> |   __device_attach+0x223/0x3a0 drivers/base/dd.c:844
> |   bus_probe_device+0x1f1/0x2a0 drivers/base/bus.c:514
> |   device_add+0xad2/0x16e0 drivers/base/core.c:2106
> |   usb_new_device.cold+0x537/0xccf 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+0x138e/0x3b00 drivers/usb/core/hub.c:5432
> |   process_one_work+0x90f/0x1580 kernel/workqueue.c:2269
> |   worker_thread+0x9b/0xe20 kernel/workqueue.c:2415
> |   kthread+0x313/0x420 kernel/kthread.c:253
> |   ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:352
> 
> Reported-by: syzbot+1fcc5ef45175fc774231@syzkaller.appspotmail.com
> Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>

I agree that this is a good fix.

Acked-by: Larry Finger <Larry.Finger@lwfinger.net>

Larry

> ---
>   drivers/net/wireless/realtek/rtlwifi/usb.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/wireless/realtek/rtlwifi/usb.c b/drivers/net/wireless/realtek/rtlwifi/usb.c
> index e24fda5e9087..34d68dbf4b4c 100644
> --- a/drivers/net/wireless/realtek/rtlwifi/usb.c
> +++ b/drivers/net/wireless/realtek/rtlwifi/usb.c
> @@ -1064,13 +1064,13 @@ int rtl_usb_probe(struct usb_interface *intf,
>   	rtlpriv->cfg->ops->read_eeprom_info(hw);
>   	err = _rtl_usb_init(hw);
>   	if (err)
> -		goto error_out;
> +		goto error_out2;
>   	rtl_usb_init_sw(hw);
>   	/* Init mac80211 sw */
>   	err = rtl_init_core(hw);
>   	if (err) {
>   		pr_err("Can't allocate sw for mac80211\n");
> -		goto error_out;
> +		goto error_out2;
>   	}
>   	if (rtlpriv->cfg->ops->init_sw_vars(hw)) {
>   		pr_err("Can't init_sw_vars\n");
> @@ -1091,6 +1091,7 @@ int rtl_usb_probe(struct usb_interface *intf,
>   
>   error_out:
>   	rtl_deinit_core(hw);
> +error_out2:
>   	_rtl_usb_io_handler_release(hw);
>   	usb_put_dev(udev);
>   	complete(&rtlpriv->firmware_loading_complete);
> 


^ permalink raw reply

* Re: [PATCH 4/7] iwlwifi: print fseq info upon fw assert
From: Kalle Valo @ 2019-05-29 14:39 UTC (permalink / raw)
  To: Luca Coelho; +Cc: linux-wireless, Shahar S Matityahu, Luca Coelho
In-Reply-To: <20190529133955.31082-5-luca@coelho.fi>

Luca Coelho <luca@coelho.fi> writes:

> From: Shahar S Matityahu <shahar.s.matityahu@intel.com>
>
> Read fseq info from FW registers and print it upon fw assert.
> The print is needed since the fseq version coming from the TLV might
> not be the actual version that is used.
>
> Signed-off-by: Shahar S Matityahu <shahar.s.matityahu@intel.com>
> Signed-off-by: Luca Coelho <luciano.coelho@intel.com>

[...]

> +void iwl_fw_error_print_fseq_regs(struct iwl_fw_runtime *fwrt)
> +{
> +	struct iwl_trans *trans = fwrt->trans;
> +	unsigned long flags;
> +	int i;
> +	struct {
> +		u32 addr;
> +		const char *str;
> +	} fseq_regs[] = {
> +		FSEQ_REG(FSEQ_ERROR_CODE),
> +		FSEQ_REG(FSEQ_TOP_INIT_VERSION),
> +		FSEQ_REG(FSEQ_CNVIO_INIT_VERSION),
> +		FSEQ_REG(FSEQ_OTP_VERSION),
> +		FSEQ_REG(FSEQ_TOP_CONTENT_VERSION),
> +		FSEQ_REG(FSEQ_ALIVE_TOKEN),
> +		FSEQ_REG(FSEQ_CNVI_ID),
> +		FSEQ_REG(FSEQ_CNVR_ID),
> +		FSEQ_REG(CNVI_AUX_MISC_CHIP),
> +		FSEQ_REG(CNVR_AUX_MISC_CHIP),
> +		FSEQ_REG(CNVR_SCU_SD_REGS_SD_REG_DIG_DCDC_VTRIM),
> +		FSEQ_REG(CNVR_SCU_SD_REGS_SD_REG_ACTIVE_VDIG_MIRROR),
> +	};

Can fseq_regs be static const?

-- 
Kalle Valo

^ permalink raw reply

* [PATCH 0/7] wlwifi: fixes intended for 5.2 2019-05-29
From: Luca Coelho @ 2019-05-29 13:39 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless, Luca Coelho

From: Luca Coelho <luciano.coelho@intel.com>

Hi,

This is the first patchset with fixes for 5.2.

The changes are:

* Fix rfkill bug with newer devices;
* Fix a potential double-free;
* Remove an obsolete debugfs file that, when used with new devices ,
  caused an oops;
* Fix a sleep with a spinlock held;
* Use the correct firmware when using a Killer NIC;
* Fix PCI persistence register for newer families;
* Print a firmware-related ID to help debug mismatch issues;

As usual, I'm pushing this to a pending branch, for kbuild bot, and
will send a pull-request later.

Please review.

Cheers,
Luca.


Emmanuel Grumbach (1):
  iwlwifi: fix load in rfkill flow for unified firmware

Jia-Ju Bai (1):
  iwlwifi: Fix double-free problems in iwl_req_fw_callback()

Johannes Berg (1):
  iwlwifi: mvm: remove d3_sram debugfs file

Lior Cohen (1):
  iwlwifi: mvm: change TLC config cmd sent by rs to be async

Matt Chen (1):
  iwlwifi: fix AX201 killer sku loading firmware issue

Shahar S Matityahu (2):
  iwlwifi: clear persistence bit according to device family
  iwlwifi: print fseq info upon fw assert

 drivers/net/wireless/intel/iwlwifi/fw/dbg.c   | 39 +++++++++++++
 drivers/net/wireless/intel/iwlwifi/fw/dbg.h   |  2 +
 drivers/net/wireless/intel/iwlwifi/iwl-drv.c  |  1 -
 drivers/net/wireless/intel/iwlwifi/iwl-prph.h | 22 ++++++-
 drivers/net/wireless/intel/iwlwifi/mvm/d3.c   | 22 -------
 .../net/wireless/intel/iwlwifi/mvm/debugfs.c  | 57 -------------------
 drivers/net/wireless/intel/iwlwifi/mvm/fw.c   | 23 ++++++--
 .../net/wireless/intel/iwlwifi/mvm/mac80211.c |  2 +-
 drivers/net/wireless/intel/iwlwifi/mvm/mvm.h  |  4 +-
 drivers/net/wireless/intel/iwlwifi/mvm/ops.c  | 20 ++++---
 .../net/wireless/intel/iwlwifi/mvm/rs-fw.c    |  3 +-
 .../net/wireless/intel/iwlwifi/mvm/utils.c    |  2 +
 .../wireless/intel/iwlwifi/pcie/internal.h    |  2 +-
 .../net/wireless/intel/iwlwifi/pcie/trans.c   | 53 ++++++++++++-----
 14 files changed, 136 insertions(+), 116 deletions(-)

-- 
2.20.1


^ permalink raw reply

* [PATCH 1/7] iwlwifi: mvm: remove d3_sram debugfs file
From: Luca Coelho @ 2019-05-29 13:39 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless, Johannes Berg, Luca Coelho
In-Reply-To: <20190529133955.31082-1-luca@coelho.fi>

From: Johannes Berg <johannes.berg@intel.com>

This debugfs file is really old, and cannot work properly since
the unified image support. Rather than trying to make it work,
which is difficult now due to multiple images (LMAC/UMAC etc.)
just remove it - we no longer need it since we properly do a FW
coredump even in D3 cases.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 drivers/net/wireless/intel/iwlwifi/mvm/d3.c   | 22 -------
 .../net/wireless/intel/iwlwifi/mvm/debugfs.c  | 57 -------------------
 drivers/net/wireless/intel/iwlwifi/mvm/mvm.h  |  2 -
 drivers/net/wireless/intel/iwlwifi/mvm/ops.c  |  3 -
 4 files changed, 84 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/d3.c b/drivers/net/wireless/intel/iwlwifi/mvm/d3.c
index 60f5d337f16d..e7e68fb2bd29 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/d3.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/d3.c
@@ -1972,26 +1972,6 @@ static void iwl_mvm_query_netdetect_reasons(struct iwl_mvm *mvm,
 	}
 }
 
-static void iwl_mvm_read_d3_sram(struct iwl_mvm *mvm)
-{
-#ifdef CONFIG_IWLWIFI_DEBUGFS
-	const struct fw_img *img = &mvm->fw->img[IWL_UCODE_WOWLAN];
-	u32 len = img->sec[IWL_UCODE_SECTION_DATA].len;
-	u32 offs = img->sec[IWL_UCODE_SECTION_DATA].offset;
-
-	if (!mvm->store_d3_resume_sram)
-		return;
-
-	if (!mvm->d3_resume_sram) {
-		mvm->d3_resume_sram = kzalloc(len, GFP_KERNEL);
-		if (!mvm->d3_resume_sram)
-			return;
-	}
-
-	iwl_trans_read_mem_bytes(mvm->trans, offs, mvm->d3_resume_sram, len);
-#endif
-}
-
 static void iwl_mvm_d3_disconnect_iter(void *data, u8 *mac,
 				       struct ieee80211_vif *vif)
 {
@@ -2054,8 +2034,6 @@ static int __iwl_mvm_resume(struct iwl_mvm *mvm, bool test)
 	}
 
 	iwl_fw_dbg_read_d3_debug_data(&mvm->fwrt);
-	/* query SRAM first in case we want event logging */
-	iwl_mvm_read_d3_sram(mvm);
 
 	if (iwl_mvm_check_rt_status(mvm, vif)) {
 		set_bit(STATUS_FW_ERROR, &mvm->trans->status);
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c b/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c
index d4ff6b44de2c..5b1bb76c5d28 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c
@@ -1557,59 +1557,6 @@ static ssize_t iwl_dbgfs_bcast_filters_macs_write(struct iwl_mvm *mvm,
 }
 #endif
 
-#ifdef CONFIG_PM_SLEEP
-static ssize_t iwl_dbgfs_d3_sram_write(struct iwl_mvm *mvm, char *buf,
-				       size_t count, loff_t *ppos)
-{
-	int store;
-
-	if (sscanf(buf, "%d", &store) != 1)
-		return -EINVAL;
-
-	mvm->store_d3_resume_sram = store;
-
-	return count;
-}
-
-static ssize_t iwl_dbgfs_d3_sram_read(struct file *file, char __user *user_buf,
-				      size_t count, loff_t *ppos)
-{
-	struct iwl_mvm *mvm = file->private_data;
-	const struct fw_img *img;
-	int ofs, len, pos = 0;
-	size_t bufsz, ret;
-	char *buf;
-	u8 *ptr = mvm->d3_resume_sram;
-
-	img = &mvm->fw->img[IWL_UCODE_WOWLAN];
-	len = img->sec[IWL_UCODE_SECTION_DATA].len;
-
-	bufsz = len * 4 + 256;
-	buf = kzalloc(bufsz, GFP_KERNEL);
-	if (!buf)
-		return -ENOMEM;
-
-	pos += scnprintf(buf, bufsz, "D3 SRAM capture: %sabled\n",
-			 mvm->store_d3_resume_sram ? "en" : "dis");
-
-	if (ptr) {
-		for (ofs = 0; ofs < len; ofs += 16) {
-			pos += scnprintf(buf + pos, bufsz - pos,
-					 "0x%.4x %16ph\n", ofs, ptr + ofs);
-		}
-	} else {
-		pos += scnprintf(buf + pos, bufsz - pos,
-				 "(no data captured)\n");
-	}
-
-	ret = simple_read_from_buffer(user_buf, count, ppos, buf, pos);
-
-	kfree(buf);
-
-	return ret;
-}
-#endif
-
 #define PRINT_MVM_REF(ref) do {						\
 	if (mvm->refs[ref])						\
 		pos += scnprintf(buf + pos, bufsz - pos,		\
@@ -1940,9 +1887,6 @@ MVM_DEBUGFS_READ_WRITE_FILE_OPS(bcast_filters, 256);
 MVM_DEBUGFS_READ_WRITE_FILE_OPS(bcast_filters_macs, 256);
 #endif
 
-#ifdef CONFIG_PM_SLEEP
-MVM_DEBUGFS_READ_WRITE_FILE_OPS(d3_sram, 8);
-#endif
 #ifdef CONFIG_ACPI
 MVM_DEBUGFS_READ_FILE_OPS(sar_geo_profile);
 #endif
@@ -2159,7 +2103,6 @@ void iwl_mvm_dbgfs_register(struct iwl_mvm *mvm, struct dentry *dbgfs_dir)
 #endif
 
 #ifdef CONFIG_PM_SLEEP
-	MVM_DEBUGFS_ADD_FILE(d3_sram, mvm->debugfs_dir, 0600);
 	MVM_DEBUGFS_ADD_FILE(d3_test, mvm->debugfs_dir, 0400);
 	debugfs_create_bool("d3_wake_sysassert", 0600, mvm->debugfs_dir,
 			    &mvm->d3_wake_sysassert);
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h b/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h
index 8dc2a9850bc5..7b829a5be773 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h
@@ -1039,8 +1039,6 @@ struct iwl_mvm {
 #ifdef CONFIG_IWLWIFI_DEBUGFS
 	bool d3_wake_sysassert;
 	bool d3_test_active;
-	bool store_d3_resume_sram;
-	void *d3_resume_sram;
 	u32 d3_test_pme_ptr;
 	struct ieee80211_vif *keep_vif;
 	u32 last_netdetect_scans; /* no. of scans in the last net-detect wake */
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/ops.c b/drivers/net/wireless/intel/iwlwifi/mvm/ops.c
index acd2fda12466..004de67f9157 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/ops.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/ops.c
@@ -918,9 +918,6 @@ static void iwl_op_mode_mvm_stop(struct iwl_op_mode *op_mode)
 	kfree(mvm->error_recovery_buf);
 	mvm->error_recovery_buf = NULL;
 
-#if defined(CONFIG_PM_SLEEP) && defined(CONFIG_IWLWIFI_DEBUGFS)
-	kfree(mvm->d3_resume_sram);
-#endif
 	iwl_trans_op_mode_leave(mvm->trans);
 
 	iwl_phy_db_free(mvm->phy_db);
-- 
2.20.1


^ permalink raw reply related

* [PATCH 6/7] iwlwifi: Fix double-free problems in iwl_req_fw_callback()
From: Luca Coelho @ 2019-05-29 13:39 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless, Jia-Ju Bai, Luca Coelho
In-Reply-To: <20190529133955.31082-1-luca@coelho.fi>

From: Jia-Ju Bai <baijiaju1990@gmail.com>

In the error handling code of iwl_req_fw_callback(), iwl_dealloc_ucode()
is called to free data. In iwl_drv_stop(), iwl_dealloc_ucode() is called
again, which can cause double-free problems.

To fix this bug, the call to iwl_dealloc_ucode() in
iwl_req_fw_callback() is deleted.

This bug is found by a runtime fuzzing tool named FIZZER written by us.

Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 drivers/net/wireless/intel/iwlwifi/iwl-drv.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
index 852d3cbfc719..fba242284507 100644
--- a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
+++ b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
@@ -1597,7 +1597,6 @@ static void iwl_req_fw_callback(const struct firmware *ucode_raw, void *context)
 	goto free;
 
  out_free_fw:
-	iwl_dealloc_ucode(drv);
 	release_firmware(ucode_raw);
  out_unbind:
 	complete(&drv->request_firmware_complete);
-- 
2.20.1


^ permalink raw reply related

* [PATCH 7/7] iwlwifi: mvm: change TLC config cmd sent by rs to be async
From: Luca Coelho @ 2019-05-29 13:39 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless, Lior Cohen, Luca Coelho
In-Reply-To: <20190529133955.31082-1-luca@coelho.fi>

From: Lior Cohen <lior2.cohen@intel.com>

The TLC_MNG_CONFIG sync cmd sent by the rs leads to a kernel warning
of sleeping while in rcu read-side critical section. The fix is to
change the command to be ASYNC (not blocking for the response anymore).

Signed-off-by: Lior Cohen <lior2.cohen@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 drivers/net/wireless/intel/iwlwifi/mvm/rs-fw.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/rs-fw.c b/drivers/net/wireless/intel/iwlwifi/mvm/rs-fw.c
index 659e21b2d4e7..be62f499c595 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/rs-fw.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/rs-fw.c
@@ -441,7 +441,8 @@ void rs_fw_rate_init(struct iwl_mvm *mvm, struct ieee80211_sta *sta,
 	 */
 	sta->max_amsdu_len = max_amsdu_len;
 
-	ret = iwl_mvm_send_cmd_pdu(mvm, cmd_id, 0, sizeof(cfg_cmd), &cfg_cmd);
+	ret = iwl_mvm_send_cmd_pdu(mvm, cmd_id, CMD_ASYNC, sizeof(cfg_cmd),
+				   &cfg_cmd);
 	if (ret)
 		IWL_ERR(mvm, "Failed to send rate scale config (%d)\n", ret);
 }
-- 
2.20.1


^ permalink raw reply related

* [PATCH 5/7] iwlwifi: fix AX201 killer sku loading firmware issue
From: Luca Coelho @ 2019-05-29 13:39 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless, Matt Chen, Luca Coelho
In-Reply-To: <20190529133955.31082-1-luca@coelho.fi>

From: Matt Chen <matt.chen@intel.com>

When try to bring up the AX201 2 killer sku, we
run into:
[81261.392463] iwlwifi 0000:01:00.0: loaded firmware version 46.8c20f243.0 op_mode iwlmvm
[81261.407407] iwlwifi 0000:01:00.0: Detected Intel(R) Dual Band Wireless AX 22000, REV=0x340
[81262.424778] iwlwifi 0000:01:00.0: Collecting data: trigger 16 fired.
[81262.673359] iwlwifi 0000:01:00.0: Start IWL Error Log Dump:
[81262.673365] iwlwifi 0000:01:00.0: Status: 0x00000000, count: -906373681
[81262.673368] iwlwifi 0000:01:00.0: Loaded firmware version: 46.8c20f243.0
[81262.673371] iwlwifi 0000:01:00.0: 0x507C015D | ADVANCED_SYSASSERT

Fix this issue by adding 2 more cfg to avoid modifying the
original cfg configuration.

Signed-off-by: Matt Chen <matt.chen@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 drivers/net/wireless/intel/iwlwifi/pcie/trans.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
index 21da18af0155..dfa1bed124aa 100644
--- a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
+++ b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
@@ -3598,7 +3598,9 @@ struct iwl_trans *iwl_trans_pcie_alloc(struct pci_dev *pdev,
 		}
 	} else if (CSR_HW_RF_ID_TYPE_CHIP_ID(trans->hw_rf_id) ==
 		   CSR_HW_RF_ID_TYPE_CHIP_ID(CSR_HW_RF_ID_TYPE_HR) &&
-		   (trans->cfg != &iwl_ax200_cfg_cc ||
+		   ((trans->cfg != &iwl_ax200_cfg_cc &&
+		    trans->cfg != &killer1650x_2ax_cfg &&
+		    trans->cfg != &killer1650w_2ax_cfg) ||
 		    trans->hw_rev == CSR_HW_REV_TYPE_QNJ_B0)) {
 		u32 hw_status;
 
-- 
2.20.1


^ permalink raw reply related

* [PATCH 3/7] iwlwifi: clear persistence bit according to device family
From: Luca Coelho @ 2019-05-29 13:39 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless, Shahar S Matityahu, Luca Coelho
In-Reply-To: <20190529133955.31082-1-luca@coelho.fi>

From: Shahar S Matityahu <shahar.s.matityahu@intel.com>

The driver attempts to clear persistence bit on any device familiy even
though only 9000 and 22000 families require it. Clear the bit only on
the relevant device families.

Each HW has different address to the write protection register. Use the
right register for each HW

Signed-off-by: Shahar S Matityahu <shahar.s.matityahu@intel.com>
Fixes: 8954e1eb2270 ("iwlwifi: trans: Clear persistence bit when starting the FW")
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 drivers/net/wireless/intel/iwlwifi/iwl-prph.h |  7 ++-
 .../net/wireless/intel/iwlwifi/pcie/trans.c   | 46 +++++++++++++------
 2 files changed, 39 insertions(+), 14 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-prph.h b/drivers/net/wireless/intel/iwlwifi/iwl-prph.h
index 8e6a0c363c0d..925f308764bf 100644
--- a/drivers/net/wireless/intel/iwlwifi/iwl-prph.h
+++ b/drivers/net/wireless/intel/iwlwifi/iwl-prph.h
@@ -408,7 +408,12 @@ enum aux_misc_master1_en {
 #define AUX_MISC_MASTER1_SMPHR_STATUS	0xA20800
 #define RSA_ENABLE			0xA24B08
 #define PREG_AUX_BUS_WPROT_0		0xA04CC0
-#define PREG_PRPH_WPROT_0		0xA04CE0
+
+/* device family 9000 WPROT register */
+#define PREG_PRPH_WPROT_9000		0xA04CE0
+/* device family 22000 WPROT register */
+#define PREG_PRPH_WPROT_22000		0xA04D00
+
 #define SB_CPU_1_STATUS			0xA01E30
 #define SB_CPU_2_STATUS			0xA01E34
 #define UMAG_SB_CPU_1_STATUS		0xA038C0
diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
index 803fcbac4152..e9d1075d91db 100644
--- a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
+++ b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
@@ -1698,26 +1698,26 @@ static int iwl_pcie_init_msix_handler(struct pci_dev *pdev,
 	return 0;
 }
 
-static int _iwl_trans_pcie_start_hw(struct iwl_trans *trans, bool low_power)
+static int iwl_trans_pcie_clear_persistence_bit(struct iwl_trans *trans)
 {
-	struct iwl_trans_pcie *trans_pcie = IWL_TRANS_GET_PCIE_TRANS(trans);
-	u32 hpm;
-	int err;
-
-	lockdep_assert_held(&trans_pcie->mutex);
+	u32 hpm, wprot;
 
-	err = iwl_pcie_prepare_card_hw(trans);
-	if (err) {
-		IWL_ERR(trans, "Error while preparing HW: %d\n", err);
-		return err;
+	switch (trans->cfg->device_family) {
+	case IWL_DEVICE_FAMILY_9000:
+		wprot = PREG_PRPH_WPROT_9000;
+		break;
+	case IWL_DEVICE_FAMILY_22000:
+		wprot = PREG_PRPH_WPROT_22000;
+		break;
+	default:
+		return 0;
 	}
 
 	hpm = iwl_read_umac_prph_no_grab(trans, HPM_DEBUG);
 	if (hpm != 0xa5a5a5a0 && (hpm & PERSISTENCE_BIT)) {
-		int wfpm_val = iwl_read_umac_prph_no_grab(trans,
-							  PREG_PRPH_WPROT_0);
+		u32 wprot_val = iwl_read_umac_prph_no_grab(trans, wprot);
 
-		if (wfpm_val & PREG_WFPM_ACCESS) {
+		if (wprot_val & PREG_WFPM_ACCESS) {
 			IWL_ERR(trans,
 				"Error, can not clear persistence bit\n");
 			return -EPERM;
@@ -1726,6 +1726,26 @@ static int _iwl_trans_pcie_start_hw(struct iwl_trans *trans, bool low_power)
 					    hpm & ~PERSISTENCE_BIT);
 	}
 
+	return 0;
+}
+
+static int _iwl_trans_pcie_start_hw(struct iwl_trans *trans, bool low_power)
+{
+	struct iwl_trans_pcie *trans_pcie = IWL_TRANS_GET_PCIE_TRANS(trans);
+	int err;
+
+	lockdep_assert_held(&trans_pcie->mutex);
+
+	err = iwl_pcie_prepare_card_hw(trans);
+	if (err) {
+		IWL_ERR(trans, "Error while preparing HW: %d\n", err);
+		return err;
+	}
+
+	err = iwl_trans_pcie_clear_persistence_bit(trans);
+	if (err)
+		return err;
+
 	iwl_trans_pcie_sw_reset(trans);
 
 	err = iwl_pcie_apm_init(trans);
-- 
2.20.1


^ permalink raw reply related

* [PATCH 4/7] iwlwifi: print fseq info upon fw assert
From: Luca Coelho @ 2019-05-29 13:39 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless, Shahar S Matityahu, Luca Coelho
In-Reply-To: <20190529133955.31082-1-luca@coelho.fi>

From: Shahar S Matityahu <shahar.s.matityahu@intel.com>

Read fseq info from FW registers and print it upon fw assert.
The print is needed since the fseq version coming from the TLV might
not be the actual version that is used.

Signed-off-by: Shahar S Matityahu <shahar.s.matityahu@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 drivers/net/wireless/intel/iwlwifi/fw/dbg.c   | 39 +++++++++++++++++++
 drivers/net/wireless/intel/iwlwifi/fw/dbg.h   |  2 +
 drivers/net/wireless/intel/iwlwifi/iwl-prph.h | 15 ++++++-
 .../net/wireless/intel/iwlwifi/mvm/utils.c    |  2 +
 .../net/wireless/intel/iwlwifi/pcie/trans.c   |  3 +-
 5 files changed, 59 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/fw/dbg.c b/drivers/net/wireless/intel/iwlwifi/fw/dbg.c
index 5f52e40a2903..33d7bc5500db 100644
--- a/drivers/net/wireless/intel/iwlwifi/fw/dbg.c
+++ b/drivers/net/wireless/intel/iwlwifi/fw/dbg.c
@@ -2747,3 +2747,42 @@ void iwl_fw_dbg_periodic_trig_handler(struct timer_list *t)
 			  jiffies + msecs_to_jiffies(collect_interval));
 	}
 }
+
+#define FSEQ_REG(x) { .addr = (x), .str = #x, }
+
+void iwl_fw_error_print_fseq_regs(struct iwl_fw_runtime *fwrt)
+{
+	struct iwl_trans *trans = fwrt->trans;
+	unsigned long flags;
+	int i;
+	struct {
+		u32 addr;
+		const char *str;
+	} fseq_regs[] = {
+		FSEQ_REG(FSEQ_ERROR_CODE),
+		FSEQ_REG(FSEQ_TOP_INIT_VERSION),
+		FSEQ_REG(FSEQ_CNVIO_INIT_VERSION),
+		FSEQ_REG(FSEQ_OTP_VERSION),
+		FSEQ_REG(FSEQ_TOP_CONTENT_VERSION),
+		FSEQ_REG(FSEQ_ALIVE_TOKEN),
+		FSEQ_REG(FSEQ_CNVI_ID),
+		FSEQ_REG(FSEQ_CNVR_ID),
+		FSEQ_REG(CNVI_AUX_MISC_CHIP),
+		FSEQ_REG(CNVR_AUX_MISC_CHIP),
+		FSEQ_REG(CNVR_SCU_SD_REGS_SD_REG_DIG_DCDC_VTRIM),
+		FSEQ_REG(CNVR_SCU_SD_REGS_SD_REG_ACTIVE_VDIG_MIRROR),
+	};
+
+	if (!iwl_trans_grab_nic_access(trans, &flags))
+		return;
+
+	IWL_ERR(fwrt, "Fseq Registers:\n");
+
+	for (i = 0; i < ARRAY_SIZE(fseq_regs); i++)
+		IWL_ERR(fwrt, "0x%08X | %s\n",
+			iwl_read_prph_no_grab(trans, fseq_regs[i].addr),
+			fseq_regs[i].str);
+
+	iwl_trans_release_nic_access(trans, &flags);
+}
+IWL_EXPORT_SYMBOL(iwl_fw_error_print_fseq_regs);
diff --git a/drivers/net/wireless/intel/iwlwifi/fw/dbg.h b/drivers/net/wireless/intel/iwlwifi/fw/dbg.h
index 2a9e560a906b..fd0ad220e961 100644
--- a/drivers/net/wireless/intel/iwlwifi/fw/dbg.h
+++ b/drivers/net/wireless/intel/iwlwifi/fw/dbg.h
@@ -471,4 +471,6 @@ static inline void iwl_fw_error_collect(struct iwl_fw_runtime *fwrt)
 }
 
 void iwl_fw_dbg_periodic_trig_handler(struct timer_list *t);
+
+void iwl_fw_error_print_fseq_regs(struct iwl_fw_runtime *fwrt);
 #endif  /* __iwl_fw_dbg_h__ */
diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-prph.h b/drivers/net/wireless/intel/iwlwifi/iwl-prph.h
index 925f308764bf..8d930bfe0727 100644
--- a/drivers/net/wireless/intel/iwlwifi/iwl-prph.h
+++ b/drivers/net/wireless/intel/iwlwifi/iwl-prph.h
@@ -395,7 +395,11 @@ enum {
 	WFPM_AUX_CTL_AUX_IF_MAC_OWNER_MSK	= 0x80000000,
 };
 
-#define AUX_MISC_REG			0xA200B0
+#define CNVI_AUX_MISC_CHIP				0xA200B0
+#define CNVR_AUX_MISC_CHIP				0xA2B800
+#define CNVR_SCU_SD_REGS_SD_REG_DIG_DCDC_VTRIM		0xA29890
+#define CNVR_SCU_SD_REGS_SD_REG_ACTIVE_VDIG_MIRROR	0xA29938
+
 enum {
 	HW_STEP_LOCATION_BITS = 24,
 };
@@ -447,4 +451,13 @@ enum {
 
 #define UREG_DOORBELL_TO_ISR6		0xA05C04
 #define UREG_DOORBELL_TO_ISR6_NMI_BIT	BIT(0)
+
+#define FSEQ_ERROR_CODE			0xA340C8
+#define FSEQ_TOP_INIT_VERSION		0xA34038
+#define FSEQ_CNVIO_INIT_VERSION		0xA3403C
+#define FSEQ_OTP_VERSION		0xA340FC
+#define FSEQ_TOP_CONTENT_VERSION	0xA340F4
+#define FSEQ_ALIVE_TOKEN		0xA340F0
+#define FSEQ_CNVI_ID			0xA3408C
+#define FSEQ_CNVR_ID			0xA34090
 #endif				/* __iwl_prph_h__ */
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/utils.c b/drivers/net/wireless/intel/iwlwifi/mvm/utils.c
index b9914efc55c4..cc56ab88fb43 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/utils.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/utils.c
@@ -596,6 +596,8 @@ void iwl_mvm_dump_nic_error_log(struct iwl_mvm *mvm)
 		iwl_mvm_dump_lmac_error_log(mvm, 1);
 
 	iwl_mvm_dump_umac_error_log(mvm);
+
+	iwl_fw_error_print_fseq_regs(&mvm->fwrt);
 }
 
 int iwl_mvm_reconfig_scd(struct iwl_mvm *mvm, int queue, int fifo, int sta_id,
diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
index e9d1075d91db..21da18af0155 100644
--- a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
+++ b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
@@ -3546,7 +3546,8 @@ struct iwl_trans *iwl_trans_pcie_alloc(struct pci_dev *pdev,
 			hw_step |= ENABLE_WFPM;
 			iwl_write_umac_prph_no_grab(trans, WFPM_CTRL_REG,
 						    hw_step);
-			hw_step = iwl_read_prph_no_grab(trans, AUX_MISC_REG);
+			hw_step = iwl_read_prph_no_grab(trans,
+							CNVI_AUX_MISC_CHIP);
 			hw_step = (hw_step >> HW_STEP_LOCATION_BITS) & 0xF;
 			if (hw_step == 0x3)
 				trans->hw_rev = (trans->hw_rev & 0xFFFFFFF3) |
-- 
2.20.1


^ permalink raw reply related

* [PATCH 2/7] iwlwifi: fix load in rfkill flow for unified firmware
From: Luca Coelho @ 2019-05-29 13:39 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless, Emmanuel Grumbach, Luca Coelho
In-Reply-To: <20190529133955.31082-1-luca@coelho.fi>

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

When we have a single image (same firmware image for INIT and
OPERATIONAL), we couldn't load the driver and register to the
stack if we had hardware RF-Kill asserted.

Fix this. This required a few changes:

1) Run the firmware as part of the INIT phase even if its
   ucode_type is not IWL_UCODE_INIT.
2) Send the commands that are sent to the unified image in
   INIT flow even in RF-Kill.
3) Don't ask the transport to stop the hardware upon RF-Kill
   interrupt if the RF-Kill is asserted.
4) Allow the RF-Kill interrupt to take us out of L1A so that
   the RF-Kill interrupt will be received by the host (to
   enable the radio).

Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 drivers/net/wireless/intel/iwlwifi/mvm/fw.c   | 23 ++++++++++++++-----
 .../net/wireless/intel/iwlwifi/mvm/mac80211.c |  2 +-
 drivers/net/wireless/intel/iwlwifi/mvm/mvm.h  |  2 +-
 drivers/net/wireless/intel/iwlwifi/mvm/ops.c  | 17 ++++++++++----
 .../wireless/intel/iwlwifi/pcie/internal.h    |  2 +-
 5 files changed, 33 insertions(+), 13 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/fw.c b/drivers/net/wireless/intel/iwlwifi/mvm/fw.c
index ab68b5d53ec9..153717587aeb 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/fw.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/fw.c
@@ -311,6 +311,8 @@ static int iwl_mvm_load_ucode_wait_alive(struct iwl_mvm *mvm,
 	int ret;
 	enum iwl_ucode_type old_type = mvm->fwrt.cur_fw_img;
 	static const u16 alive_cmd[] = { MVM_ALIVE };
+	bool run_in_rfkill =
+		ucode_type == IWL_UCODE_INIT || iwl_mvm_has_unified_ucode(mvm);
 
 	if (ucode_type == IWL_UCODE_REGULAR &&
 	    iwl_fw_dbg_conf_usniffer(mvm->fw, FW_DBG_START_FROM_ALIVE) &&
@@ -328,7 +330,12 @@ static int iwl_mvm_load_ucode_wait_alive(struct iwl_mvm *mvm,
 				   alive_cmd, ARRAY_SIZE(alive_cmd),
 				   iwl_alive_fn, &alive_data);
 
-	ret = iwl_trans_start_fw(mvm->trans, fw, ucode_type == IWL_UCODE_INIT);
+	/*
+	 * We want to load the INIT firmware even in RFKILL
+	 * For the unified firmware case, the ucode_type is not
+	 * INIT, but we still need to run it.
+	 */
+	ret = iwl_trans_start_fw(mvm->trans, fw, run_in_rfkill);
 	if (ret) {
 		iwl_fw_set_current_image(&mvm->fwrt, old_type);
 		iwl_remove_notification(&mvm->notif_wait, &alive_wait);
@@ -433,7 +440,8 @@ static int iwl_run_unified_mvm_ucode(struct iwl_mvm *mvm, bool read_nvm)
 	 * commands
 	 */
 	ret = iwl_mvm_send_cmd_pdu(mvm, WIDE_ID(SYSTEM_GROUP,
-						INIT_EXTENDED_CFG_CMD), 0,
+						INIT_EXTENDED_CFG_CMD),
+				   CMD_SEND_IN_RFKILL,
 				   sizeof(init_cfg), &init_cfg);
 	if (ret) {
 		IWL_ERR(mvm, "Failed to run init config command: %d\n",
@@ -457,7 +465,8 @@ static int iwl_run_unified_mvm_ucode(struct iwl_mvm *mvm, bool read_nvm)
 	}
 
 	ret = iwl_mvm_send_cmd_pdu(mvm, WIDE_ID(REGULATORY_AND_NVM_GROUP,
-						NVM_ACCESS_COMPLETE), 0,
+						NVM_ACCESS_COMPLETE),
+				   CMD_SEND_IN_RFKILL,
 				   sizeof(nvm_complete), &nvm_complete);
 	if (ret) {
 		IWL_ERR(mvm, "Failed to run complete NVM access: %d\n",
@@ -482,6 +491,8 @@ static int iwl_run_unified_mvm_ucode(struct iwl_mvm *mvm, bool read_nvm)
 		}
 	}
 
+	mvm->rfkill_safe_init_done = true;
+
 	return 0;
 
 error:
@@ -526,7 +537,7 @@ int iwl_run_init_mvm_ucode(struct iwl_mvm *mvm, bool read_nvm)
 
 	lockdep_assert_held(&mvm->mutex);
 
-	if (WARN_ON_ONCE(mvm->calibrating))
+	if (WARN_ON_ONCE(mvm->rfkill_safe_init_done))
 		return 0;
 
 	iwl_init_notification_wait(&mvm->notif_wait,
@@ -576,7 +587,7 @@ int iwl_run_init_mvm_ucode(struct iwl_mvm *mvm, bool read_nvm)
 		goto remove_notif;
 	}
 
-	mvm->calibrating = true;
+	mvm->rfkill_safe_init_done = true;
 
 	/* Send TX valid antennas before triggering calibrations */
 	ret = iwl_send_tx_ant_cfg(mvm, iwl_mvm_get_valid_tx_ant(mvm));
@@ -612,7 +623,7 @@ int iwl_run_init_mvm_ucode(struct iwl_mvm *mvm, bool read_nvm)
 remove_notif:
 	iwl_remove_notification(&mvm->notif_wait, &calib_wait);
 out:
-	mvm->calibrating = false;
+	mvm->rfkill_safe_init_done = false;
 	if (iwlmvm_mod_params.init_dbg && !mvm->nvm_data) {
 		/* we want to debug INIT and we have no NVM - fake */
 		mvm->nvm_data = kzalloc(sizeof(struct iwl_nvm_data) +
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c b/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c
index 5c52469288be..fdbabca0280e 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c
@@ -1209,7 +1209,7 @@ static void iwl_mvm_restart_cleanup(struct iwl_mvm *mvm)
 
 	mvm->scan_status = 0;
 	mvm->ps_disabled = false;
-	mvm->calibrating = false;
+	mvm->rfkill_safe_init_done = false;
 
 	/* just in case one was running */
 	iwl_mvm_cleanup_roc_te(mvm);
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h b/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h
index 7b829a5be773..02efcf2189c4 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h
@@ -880,7 +880,7 @@ struct iwl_mvm {
 	struct iwl_mvm_vif *bf_allowed_vif;
 
 	bool hw_registered;
-	bool calibrating;
+	bool rfkill_safe_init_done;
 	bool support_umac_log;
 
 	u32 ampdu_ref;
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/ops.c b/drivers/net/wireless/intel/iwlwifi/mvm/ops.c
index 004de67f9157..fad3bf563712 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/ops.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/ops.c
@@ -1209,7 +1209,8 @@ void iwl_mvm_set_hw_ctkill_state(struct iwl_mvm *mvm, bool state)
 static bool iwl_mvm_set_hw_rfkill_state(struct iwl_op_mode *op_mode, bool state)
 {
 	struct iwl_mvm *mvm = IWL_OP_MODE_GET_MVM(op_mode);
-	bool calibrating = READ_ONCE(mvm->calibrating);
+	bool rfkill_safe_init_done = READ_ONCE(mvm->rfkill_safe_init_done);
+	bool unified = iwl_mvm_has_unified_ucode(mvm);
 
 	if (state)
 		set_bit(IWL_MVM_STATUS_HW_RFKILL, &mvm->status);
@@ -1218,15 +1219,23 @@ static bool iwl_mvm_set_hw_rfkill_state(struct iwl_op_mode *op_mode, bool state)
 
 	iwl_mvm_set_rfkill_state(mvm);
 
-	/* iwl_run_init_mvm_ucode is waiting for results, abort it */
-	if (calibrating)
+	 /* iwl_run_init_mvm_ucode is waiting for results, abort it. */
+	if (rfkill_safe_init_done)
 		iwl_abort_notification_waits(&mvm->notif_wait);
 
+	/*
+	 * Don't ask the transport to stop the firmware. We'll do it
+	 * after cfg80211 takes us down.
+	 */
+	if (unified)
+		return false;
+
 	/*
 	 * Stop the device if we run OPERATIONAL firmware or if we are in the
 	 * middle of the calibrations.
 	 */
-	return state && (mvm->fwrt.cur_fw_img != IWL_UCODE_INIT || calibrating);
+	return state && (mvm->fwrt.cur_fw_img != IWL_UCODE_INIT ||
+			 rfkill_safe_init_done);
 }
 
 static void iwl_mvm_free_skb(struct iwl_op_mode *op_mode, struct sk_buff *skb)
diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/internal.h b/drivers/net/wireless/intel/iwlwifi/pcie/internal.h
index b513037dc066..85973dd57234 100644
--- a/drivers/net/wireless/intel/iwlwifi/pcie/internal.h
+++ b/drivers/net/wireless/intel/iwlwifi/pcie/internal.h
@@ -928,7 +928,7 @@ static inline void iwl_enable_rfkill_int(struct iwl_trans *trans)
 					   MSIX_HW_INT_CAUSES_REG_RF_KILL);
 	}
 
-	if (trans->cfg->device_family == IWL_DEVICE_FAMILY_9000) {
+	if (trans->cfg->device_family >= IWL_DEVICE_FAMILY_9000) {
 		/*
 		 * On 9000-series devices this bit isn't enabled by default, so
 		 * when we power down the device we need set the bit to allow it
-- 
2.20.1


^ permalink raw reply related

* [PATCH 0/2] Buffer overflow / read checks in mwifiex
From: Takashi Iwai @ 2019-05-29 12:52 UTC (permalink / raw)
  To: linux-wireless
  Cc: Amitkumar Karwar, Nishant Sarmukadam, Ganapathi Bhat, Xinming Hu,
	Kalle Valo, huangwen, Solar Designer, Marcus Meissner

Hi,

this is fixes for a few spots that perform memcpy() without checking
the source and the destination size in mwifiex driver, which may lead
to buffer overflows or read over boundary.


Takashi

===

Takashi Iwai (2):
  mwifiex: Fix possible buffer overflows at parsing bss descriptor
  mwifiex: Abort at too short BSS descriptor element

 drivers/net/wireless/marvell/mwifiex/scan.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

-- 
2.16.4


^ permalink raw reply

* [PATCH 1/2] mwifiex: Fix possible buffer overflows at parsing bss descriptor
From: Takashi Iwai @ 2019-05-29 12:52 UTC (permalink / raw)
  To: linux-wireless
  Cc: Amitkumar Karwar, Nishant Sarmukadam, Ganapathi Bhat, Xinming Hu,
	Kalle Valo, huangwen, Solar Designer, Marcus Meissner
In-Reply-To: <20190529125220.17066-1-tiwai@suse.de>

mwifiex_update_bss_desc_with_ie() calls memcpy() unconditionally in
a couple places without checking the destination size.  Since the
source is given from user-space, this may trigger a heap buffer
overflow.

Fix it by putting the length check before performing memcpy().

This fix addresses CVE-2019-3846.

Reported-by: huangwen <huangwen@venustech.com.cn>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/net/wireless/marvell/mwifiex/scan.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/wireless/marvell/mwifiex/scan.c b/drivers/net/wireless/marvell/mwifiex/scan.c
index 935778ec9a1b..64ab6fe78c0d 100644
--- a/drivers/net/wireless/marvell/mwifiex/scan.c
+++ b/drivers/net/wireless/marvell/mwifiex/scan.c
@@ -1247,6 +1247,8 @@ int mwifiex_update_bss_desc_with_ie(struct mwifiex_adapter *adapter,
 		}
 		switch (element_id) {
 		case WLAN_EID_SSID:
+			if (element_len > IEEE80211_MAX_SSID_LEN)
+				return -EINVAL;
 			bss_entry->ssid.ssid_len = element_len;
 			memcpy(bss_entry->ssid.ssid, (current_ptr + 2),
 			       element_len);
@@ -1256,6 +1258,8 @@ int mwifiex_update_bss_desc_with_ie(struct mwifiex_adapter *adapter,
 			break;
 
 		case WLAN_EID_SUPP_RATES:
+			if (element_len > MWIFIEX_SUPPORTED_RATES)
+				return -EINVAL;
 			memcpy(bss_entry->data_rates, current_ptr + 2,
 			       element_len);
 			memcpy(bss_entry->supported_rates, current_ptr + 2,
-- 
2.16.4


^ permalink raw reply related

* [PATCH 2/2] mwifiex: Abort at too short BSS descriptor element
From: Takashi Iwai @ 2019-05-29 12:52 UTC (permalink / raw)
  To: linux-wireless
  Cc: Amitkumar Karwar, Nishant Sarmukadam, Ganapathi Bhat, Xinming Hu,
	Kalle Valo, huangwen, Solar Designer, Marcus Meissner
In-Reply-To: <20190529125220.17066-1-tiwai@suse.de>

Currently mwifiex_update_bss_desc_with_ie() implicitly assumes that
the source descriptor entries contain the enough size for each type
and performs copying without checking the source size.  This may lead
to read over boundary.

Fix this by putting the source size check in appropriate places.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/net/wireless/marvell/mwifiex/scan.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/net/wireless/marvell/mwifiex/scan.c b/drivers/net/wireless/marvell/mwifiex/scan.c
index 64ab6fe78c0d..c269a0de9413 100644
--- a/drivers/net/wireless/marvell/mwifiex/scan.c
+++ b/drivers/net/wireless/marvell/mwifiex/scan.c
@@ -1269,6 +1269,8 @@ int mwifiex_update_bss_desc_with_ie(struct mwifiex_adapter *adapter,
 			break;
 
 		case WLAN_EID_FH_PARAMS:
+			if (element_len + 2 < sizeof(*fh_param_set))
+				return -EINVAL;
 			fh_param_set =
 				(struct ieee_types_fh_param_set *) current_ptr;
 			memcpy(&bss_entry->phy_param_set.fh_param_set,
@@ -1277,6 +1279,8 @@ int mwifiex_update_bss_desc_with_ie(struct mwifiex_adapter *adapter,
 			break;
 
 		case WLAN_EID_DS_PARAMS:
+			if (element_len + 2 < sizeof(*ds_param_set))
+				return -EINVAL;
 			ds_param_set =
 				(struct ieee_types_ds_param_set *) current_ptr;
 
@@ -1288,6 +1292,8 @@ int mwifiex_update_bss_desc_with_ie(struct mwifiex_adapter *adapter,
 			break;
 
 		case WLAN_EID_CF_PARAMS:
+			if (element_len + 2 < sizeof(*cf_param_set))
+				return -EINVAL;
 			cf_param_set =
 				(struct ieee_types_cf_param_set *) current_ptr;
 			memcpy(&bss_entry->ss_param_set.cf_param_set,
@@ -1296,6 +1302,8 @@ int mwifiex_update_bss_desc_with_ie(struct mwifiex_adapter *adapter,
 			break;
 
 		case WLAN_EID_IBSS_PARAMS:
+			if (element_len + 2 < sizeof(*ibss_param_set))
+				return -EINVAL;
 			ibss_param_set =
 				(struct ieee_types_ibss_param_set *)
 				current_ptr;
@@ -1305,10 +1313,14 @@ int mwifiex_update_bss_desc_with_ie(struct mwifiex_adapter *adapter,
 			break;
 
 		case WLAN_EID_ERP_INFO:
+			if (!element_len)
+				return -EINVAL;
 			bss_entry->erp_flags = *(current_ptr + 2);
 			break;
 
 		case WLAN_EID_PWR_CONSTRAINT:
+			if (!element_len)
+				return -EINVAL;
 			bss_entry->local_constraint = *(current_ptr + 2);
 			bss_entry->sensed_11h = true;
 			break;
@@ -1349,6 +1361,9 @@ int mwifiex_update_bss_desc_with_ie(struct mwifiex_adapter *adapter,
 			break;
 
 		case WLAN_EID_VENDOR_SPECIFIC:
+			if (element_len + 2 < sizeof(vendor_ie->vend_hdr))
+				return -EINVAL;
+
 			vendor_ie = (struct ieee_types_vendor_specific *)
 					current_ptr;
 
-- 
2.16.4


^ permalink raw reply related

* [PATCH 04/10] cfg80211: util: fix bit count off by one
From: Luca Coelho @ 2019-05-29 12:25 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Mordechay Goodstein, Luca Coelho
In-Reply-To: <20190529122537.8564-1-luca@coelho.fi>

From: Mordechay Goodstein <mordechay.goodstein@intel.com>

The bits of Rx MCS Map in VHT capability were enumerated
with index transform - index i -> (i + 1) bit => nss i. BUG!
while it should be -   index i -> (i + 1) bit => (i + 1) nss.

The bug was exposed in commit a53b2a0b1245 ("iwlwifi: mvm: implement VHT
extended NSS support in rs.c"), where iwlwifi started using the
function.

Signed-off-by: Mordechay Goodstein <mordechay.goodstein@intel.com>
Fixes: b0aa75f0b1b2 ("ieee80211: add new VHT capability fields/parsing")
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 net/wireless/util.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/wireless/util.c b/net/wireless/util.c
index cf63b635afc0..33dbf7ee9b97 100644
--- a/net/wireless/util.c
+++ b/net/wireless/util.c
@@ -1998,7 +1998,7 @@ int ieee80211_get_vht_max_nss(struct ieee80211_vht_cap *cap,
 			continue;
 
 		if (supp >= mcs_encoding) {
-			max_vht_nss = i;
+			max_vht_nss = i + 1;
 			break;
 		}
 	}
-- 
2.20.1


^ permalink raw reply related

* [PATCH 02/10] cfg80211: use BIT_ULL in cfg80211_parse_mbssid_data()
From: Luca Coelho @ 2019-05-29 12:25 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Luca Coelho, Dan Carpented
In-Reply-To: <20190529122537.8564-1-luca@coelho.fi>

From: Luca Coelho <luciano.coelho@intel.com>

The seen_indices variable is u64 and in other parts of the code we
assume mbssid_index_ie[2] can be up to 45, so we should use the 64-bit
versions of BIT, namely, BIT_ULL().

Reported-by: Dan Carpented <dan.carpenter@oracle.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 net/wireless/scan.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/wireless/scan.c b/net/wireless/scan.c
index f347387f195a..2ea268ef1c43 100644
--- a/net/wireless/scan.c
+++ b/net/wireless/scan.c
@@ -1602,12 +1602,12 @@ static void cfg80211_parse_mbssid_data(struct wiphy *wiphy,
 				continue;
 			}
 
-			if (seen_indices & BIT(mbssid_index_ie[2]))
+			if (seen_indices & BIT_ULL(mbssid_index_ie[2]))
 				/* We don't support legacy split of a profile */
 				net_dbg_ratelimited("Partial info for BSSID index %d\n",
 						    mbssid_index_ie[2]);
 
-			seen_indices |= BIT(mbssid_index_ie[2]);
+			seen_indices |= BIT_ULL(mbssid_index_ie[2]);
 
 			non_tx_data->bssid_index = mbssid_index_ie[2];
 			non_tx_data->max_bssid_indicator = elem->data[0];
-- 
2.20.1


^ permalink raw reply related

* [PATCH 05/10] cfg80211: Add a function to iterate all BSS entries
From: Luca Coelho @ 2019-05-29 12:25 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Ilan Peer, Luca Coelho
In-Reply-To: <20190529122537.8564-1-luca@coelho.fi>

From: Ilan Peer <ilan.peer@intel.com>

Add a function that iterates over the BSS entries associated with a
given wiphy and calls a callback for each iterated BSS. This can be
used by drivers in various ways, e.g., to evaluate some property for
all the BSSs in the medium.

Signed-off-by: Ilan Peer <ilan.peer@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 include/net/cfg80211.h | 20 ++++++++++++++++++++
 net/wireless/scan.c    | 21 +++++++++++++++++++++
 2 files changed, 41 insertions(+)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index c19687833493..d5c1cfef443a 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -5721,6 +5721,26 @@ void cfg80211_put_bss(struct wiphy *wiphy, struct cfg80211_bss *bss);
  */
 void cfg80211_unlink_bss(struct wiphy *wiphy, struct cfg80211_bss *bss);
 
+/**
+ * cfg80211_bss_iter - iterate all BSS entries
+ *
+ * This function iterates over the BSS entries associated with the given wiphy
+ * and calls the callback for the iterated BSS. The iterator function is not
+ * allowed to call functions that might modify the internal state of the BSS DB.
+ *
+ * @wiphy: the wiphy
+ * @chandef: if given, the iterator function will be called only if the channel
+ *     of the currently iterated BSS is a subset of the given channel.
+ * @iter: the iterator function to call
+ * @iter_data: an argument to the iterator function
+ */
+void cfg80211_bss_iter(struct wiphy *wiphy,
+		       struct cfg80211_chan_def *chandef,
+		       void (*iter)(struct wiphy *wiphy,
+				    struct cfg80211_bss *bss,
+				    void *data),
+		       void *iter_data);
+
 static inline enum nl80211_bss_scan_width
 cfg80211_chandef_to_scan_width(const struct cfg80211_chan_def *chandef)
 {
diff --git a/net/wireless/scan.c b/net/wireless/scan.c
index 2ea268ef1c43..d66e6d4b7555 100644
--- a/net/wireless/scan.c
+++ b/net/wireless/scan.c
@@ -1974,6 +1974,27 @@ void cfg80211_unlink_bss(struct wiphy *wiphy, struct cfg80211_bss *pub)
 }
 EXPORT_SYMBOL(cfg80211_unlink_bss);
 
+void cfg80211_bss_iter(struct wiphy *wiphy,
+		       struct cfg80211_chan_def *chandef,
+		       void (*iter)(struct wiphy *wiphy,
+				    struct cfg80211_bss *bss,
+				    void *data),
+		       void *iter_data)
+{
+	struct cfg80211_registered_device *rdev = wiphy_to_rdev(wiphy);
+	struct cfg80211_internal_bss *bss;
+
+	spin_lock_bh(&rdev->bss_lock);
+
+	list_for_each_entry(bss, &rdev->bss_list, list) {
+		if (!chandef || cfg80211_is_sub_chan(chandef, bss->pub.channel))
+			iter(wiphy, &bss->pub, iter_data);
+	}
+
+	spin_unlock_bh(&rdev->bss_lock);
+}
+EXPORT_SYMBOL(cfg80211_bss_iter);
+
 #ifdef CONFIG_CFG80211_WEXT
 static struct cfg80211_registered_device *
 cfg80211_get_dev_from_ifindex(struct net *net, int ifindex)
-- 
2.20.1


^ 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