[PATCH] Revert "wlcore: Adding suppoprt for IGTK key in wlcore driver"

[PATCH] Revert "wlcore: Adding suppoprt for IGTK key in wlcore driver"

[Mauro Carvalho Chehab]

This patch causes a regression betwen Kernel 5.7 and 5.8 at wlcore:
with it applied, WiFi stops working, and the Kernel starts printing
this message every second:

   wlcore: PHY firmware version: Rev 8.2.0.0.242
   wlcore: firmware booted (Rev 8.9.0.0.79)
   wlcore: ERROR command execute failure 14
   ------------[ cut here ]------------
   WARNING: CPU: 0 PID: 133 at drivers/net/wireless/ti/wlcore/main.c:795 wl12xx_queue_recovery_work.part.0+0x6c/0x74 [wlcore]
   Modules linked in: wl18xx wlcore mac80211 libarc4 cfg80211 rfkill snd_soc_hdmi_codec crct10dif_ce wlcore_sdio adv7511 cec kirin9xx_drm(C) kirin9xx_dw_drm_dsi(C) drm_kms_helper drm ip_tables x_tables ipv6 nf_defrag_ipv6
   CPU: 0 PID: 133 Comm: kworker/0:1 Tainted: G        WC        5.8.0+ #186
   Hardware name: HiKey970 (DT)
   Workqueue: events_freezable ieee80211_restart_work [mac80211]
   pstate: 60000005 (nZCv daif -PAN -UAO BTYPE=--)
   pc : wl12xx_queue_recovery_work.part.0+0x6c/0x74 [wlcore]
   lr : wl12xx_queue_recovery_work+0x24/0x30 [wlcore]
   sp : ffff8000126c3a60
   x29: ffff8000126c3a60 x28: 00000000000025de
   x27: 0000000000000010 x26: 0000000000000005
   x25: ffff0001a5d49e80 x24: ffff8000092cf580
   x23: ffff0001b7c12623 x22: ffff0001b6fcf2e8
   x21: ffff0001b7e46200 x20: 00000000fffffffb
   x19: ffff0001a78e6400 x18: 0000000000000030
   x17: 0000000000000001 x16: 0000000000000001
   x15: ffff0001b7e46670 x14: ffffffffffffffff
   x13: ffff8000926c37d7 x12: ffff8000126c37e0
   x11: ffff800011e01000 x10: ffff8000120526d0
   x9 : 0000000000000000 x8 : 3431206572756c69
   x7 : 6166206574756365 x6 : 0000000000000c2c
   x5 : 0000000000000000 x4 : ffff0001bf1361e8
   x3 : ffff0001bf1790b0 x2 : 0000000000000000
   x1 : ffff0001a5d49e80 x0 : 0000000000000001
   Call trace:
    wl12xx_queue_recovery_work.part.0+0x6c/0x74 [wlcore]
    wl12xx_queue_recovery_work+0x24/0x30 [wlcore]
    wl1271_cmd_set_sta_key+0x258/0x25c [wlcore]
    wl1271_set_key+0x7c/0x2dc [wlcore]
    wlcore_set_key+0xe4/0x360 [wlcore]
    wl18xx_set_key+0x48/0x1d0 [wl18xx]
    wlcore_op_set_key+0xa4/0x180 [wlcore]
    ieee80211_key_enable_hw_accel+0xb0/0x2d0 [mac80211]
    ieee80211_reenable_keys+0x70/0x110 [mac80211]
    ieee80211_reconfig+0xa00/0xca0 [mac80211]
    ieee80211_restart_work+0xc4/0xfc [mac80211]
    process_one_work+0x1cc/0x350
    worker_thread+0x13c/0x470
    kthread+0x154/0x160
    ret_from_fork+0x10/0x30
   ---[ end trace b1f722abf9af5919 ]---
   wlcore: WARNING could not set keys
   wlcore: ERROR Could not add or replace key
   wlan0: failed to set key (4, ff:ff:ff:ff:ff:ff) to hardware (-5)
   wlcore: Hardware recovery in progress. FW ver: Rev 8.9.0.0.79
   wlcore: pc: 0x0, hint_sts: 0x00000040 count: 39
   wlcore: down
   wlcore: down
   ieee80211 phy0: Hardware restart was requested
   mmc_host mmc0: Bus speed (slot 0) = 400000Hz (slot req 400000Hz, actual 400000HZ div = 0)
   mmc_host mmc0: Bus speed (slot 0) = 25000000Hz (slot req 25000000Hz, actual 25000000HZ div = 0)
   wlcore: PHY firmware version: Rev 8.2.0.0.242
   wlcore: firmware booted (Rev 8.9.0.0.79)
   wlcore: ERROR command execute failure 14
   ------------[ cut here ]------------

Tested on Hikey 970.

This reverts commit 2b7aadd3b9e17e8b81eeb8d9cc46756ae4658265.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 drivers/net/wireless/ti/wlcore/cmd.h  | 1 -
 drivers/net/wireless/ti/wlcore/main.c | 4 ----
 2 files changed, 5 deletions(-)

diff --git a/drivers/net/wireless/ti/wlcore/cmd.h b/drivers/net/wireless/ti/wlcore/cmd.h
index 9acd8a41ea61..f2609d5b6bf7 100644
--- a/drivers/net/wireless/ti/wlcore/cmd.h
+++ b/drivers/net/wireless/ti/wlcore/cmd.h
@@ -458,7 +458,6 @@ enum wl1271_cmd_key_type {
 	KEY_TKIP = 2,
 	KEY_AES  = 3,
 	KEY_GEM  = 4,
-	KEY_IGTK  = 5,
 };
 
 struct wl1271_cmd_set_keys {
diff --git a/drivers/net/wireless/ti/wlcore/main.c b/drivers/net/wireless/ti/wlcore/main.c
index de6c8a7589ca..ef169de99224 100644
--- a/drivers/net/wireless/ti/wlcore/main.c
+++ b/drivers/net/wireless/ti/wlcore/main.c
@@ -3550,9 +3550,6 @@ int wlcore_set_key(struct wl1271 *wl, enum set_key_cmd cmd,
 	case WL1271_CIPHER_SUITE_GEM:
 		key_type = KEY_GEM;
 		break;
-	case WLAN_CIPHER_SUITE_AES_CMAC:
-		key_type = KEY_IGTK;
-		break;
 	default:
 		wl1271_error("Unknown key algo 0x%x", key_conf->cipher);
 
@@ -6222,7 +6219,6 @@ static int wl1271_init_ieee80211(struct wl1271 *wl)
 		WLAN_CIPHER_SUITE_TKIP,
 		WLAN_CIPHER_SUITE_CCMP,
 		WL1271_CIPHER_SUITE_GEM,
-		WLAN_CIPHER_SUITE_AES_CMAC,
 	};
 
 	/* The tx descriptor buffer */
-- 
2.26.2

Re: [PATCH] Revert "wlcore: Adding suppoprt for IGTK key in wlcore driver"

[Kalle Valo]

Mauro Carvalho Chehab <mchehab+huawei@kernel.org> writes:

> This patch causes a regression betwen Kernel 5.7 and 5.8 at wlcore:
> with it applied, WiFi stops working, and the Kernel starts printing
> this message every second:
>
>    wlcore: PHY firmware version: Rev 8.2.0.0.242
>    wlcore: firmware booted (Rev 8.9.0.0.79)
>    wlcore: ERROR command execute failure 14
>    ------------[ cut here ]------------
>    WARNING: CPU: 0 PID: 133 at drivers/net/wireless/ti/wlcore/main.c:795 wl12xx_queue_recovery_work.part.0+0x6c/0x74 [wlcore]
>    Modules linked in: wl18xx wlcore mac80211 libarc4 cfg80211 rfkill snd_soc_hdmi_codec crct10dif_ce wlcore_sdio adv7511 cec kirin9xx_drm(C) kirin9xx_dw_drm_dsi(C) drm_kms_helper drm ip_tables x_tables ipv6 nf_defrag_ipv6
>    CPU: 0 PID: 133 Comm: kworker/0:1 Tainted: G        WC        5.8.0+ #186
>    Hardware name: HiKey970 (DT)
>    Workqueue: events_freezable ieee80211_restart_work [mac80211]
>    pstate: 60000005 (nZCv daif -PAN -UAO BTYPE=--)
>    pc : wl12xx_queue_recovery_work.part.0+0x6c/0x74 [wlcore]
>    lr : wl12xx_queue_recovery_work+0x24/0x30 [wlcore]
>    sp : ffff8000126c3a60
>    x29: ffff8000126c3a60 x28: 00000000000025de
>    x27: 0000000000000010 x26: 0000000000000005
>    x25: ffff0001a5d49e80 x24: ffff8000092cf580
>    x23: ffff0001b7c12623 x22: ffff0001b6fcf2e8
>    x21: ffff0001b7e46200 x20: 00000000fffffffb
>    x19: ffff0001a78e6400 x18: 0000000000000030
>    x17: 0000000000000001 x16: 0000000000000001
>    x15: ffff0001b7e46670 x14: ffffffffffffffff
>    x13: ffff8000926c37d7 x12: ffff8000126c37e0
>    x11: ffff800011e01000 x10: ffff8000120526d0
>    x9 : 0000000000000000 x8 : 3431206572756c69
>    x7 : 6166206574756365 x6 : 0000000000000c2c
>    x5 : 0000000000000000 x4 : ffff0001bf1361e8
>    x3 : ffff0001bf1790b0 x2 : 0000000000000000
>    x1 : ffff0001a5d49e80 x0 : 0000000000000001
>    Call trace:
>     wl12xx_queue_recovery_work.part.0+0x6c/0x74 [wlcore]
>     wl12xx_queue_recovery_work+0x24/0x30 [wlcore]
>     wl1271_cmd_set_sta_key+0x258/0x25c [wlcore]
>     wl1271_set_key+0x7c/0x2dc [wlcore]
>     wlcore_set_key+0xe4/0x360 [wlcore]
>     wl18xx_set_key+0x48/0x1d0 [wl18xx]
>     wlcore_op_set_key+0xa4/0x180 [wlcore]
>     ieee80211_key_enable_hw_accel+0xb0/0x2d0 [mac80211]
>     ieee80211_reenable_keys+0x70/0x110 [mac80211]
>     ieee80211_reconfig+0xa00/0xca0 [mac80211]
>     ieee80211_restart_work+0xc4/0xfc [mac80211]
>     process_one_work+0x1cc/0x350
>     worker_thread+0x13c/0x470
>     kthread+0x154/0x160
>     ret_from_fork+0x10/0x30
>    ---[ end trace b1f722abf9af5919 ]---
>    wlcore: WARNING could not set keys
>    wlcore: ERROR Could not add or replace key
>    wlan0: failed to set key (4, ff:ff:ff:ff:ff:ff) to hardware (-5)
>    wlcore: Hardware recovery in progress. FW ver: Rev 8.9.0.0.79
>    wlcore: pc: 0x0, hint_sts: 0x00000040 count: 39
>    wlcore: down
>    wlcore: down
>    ieee80211 phy0: Hardware restart was requested
>    mmc_host mmc0: Bus speed (slot 0) = 400000Hz (slot req 400000Hz, actual 400000HZ div = 0)
>    mmc_host mmc0: Bus speed (slot 0) = 25000000Hz (slot req 25000000Hz, actual 25000000HZ div = 0)
>    wlcore: PHY firmware version: Rev 8.2.0.0.242
>    wlcore: firmware booted (Rev 8.9.0.0.79)
>    wlcore: ERROR command execute failure 14
>    ------------[ cut here ]------------
>
> Tested on Hikey 970.
>
> This reverts commit 2b7aadd3b9e17e8b81eeb8d9cc46756ae4658265.
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

Raz, do you have a fix for this? Or should I just queue this revert to
v5.9?

-- 
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH] Revert "wlcore: Adding suppoprt for IGTK key in wlcore driver"

[Steve deRosier]

On Tue, Aug 25, 2020 at 10:49 PM Mauro Carvalho Chehab
<mchehab+huawei@kernel.org> wrote:
>
> This patch causes a regression betwen Kernel 5.7 and 5.8 at wlcore:
> with it applied, WiFi stops working, and the Kernel starts printing
> this message every second:
>
>    wlcore: PHY firmware version: Rev 8.2.0.0.242
>    wlcore: firmware booted (Rev 8.9.0.0.79)
>    wlcore: ERROR command execute failure 14

Only if NO firmware for the device in question supports the `KEY_IGTK`
value, then this revert is appropriate. Otherwise, it likely isn't.
 My suspicion is that the feature that `KEY_IGTK` is enabling is
specific to a newer firmware that Mauro hasn't upgraded to. What the
OP should do is find the updated firmware and give it a try.

AND - since there's some firmware the feature doesn't work with, the
driver should be fixed to detect the running firmware version and not
do things that the firmware doesn't support.  AND the firmware writer
should also make it so the firmware doesn't barf on bad input and
instead rejects it politely.

But I will say I'm making an educated guess; while I have played with
the TI devices in the past, it was years ago and I won't claim to be
an expert. I also am unable to fix it myself at this time.

I'd just rather see it fixed properly instead of a knee-jerk reaction
of reverting it simply because the OP doesn't have current firmware.

And let's revisit the discussion of having a kernel splat because an
unrelated piece of code fails yet the driver does exactly what it is
supposed to do. We shouldn't be dumping registers and stack-trace when
the code that crashed has nothing to do with the registers and
stack-trace outputted. It is a false positive.  A simple printk WARN
or ERROR should output notifying us that the chip firmware has crashed
and why.  IMHO.

- Steve

Re: [PATCH] Revert "wlcore: Adding suppoprt for IGTK key in wlcore driver"

[Mauro Carvalho Chehab]

Em Thu, 27 Aug 2020 08:48:30 -0700
Steve deRosier <derosier@gmail.com> escreveu:

> On Tue, Aug 25, 2020 at 10:49 PM Mauro Carvalho Chehab
> <mchehab+huawei@kernel.org> wrote:
> >
> > This patch causes a regression betwen Kernel 5.7 and 5.8 at wlcore:
> > with it applied, WiFi stops working, and the Kernel starts printing
> > this message every second:
> >
> >    wlcore: PHY firmware version: Rev 8.2.0.0.242
> >    wlcore: firmware booted (Rev 8.9.0.0.79)
> >    wlcore: ERROR command execute failure 14  
> 
> Only if NO firmware for the device in question supports the `KEY_IGTK`
> value, then this revert is appropriate. Otherwise, it likely isn't.

Yeah, that's what I suspect too: some specific firmware is required
for KEY_IGTK to work.

>  My suspicion is that the feature that `KEY_IGTK` is enabling is
> specific to a newer firmware that Mauro hasn't upgraded to. What the
> OP should do is find the updated firmware and give it a try.

I didn't try checking if linux-firmware tree has a newer version on
it. I'm using Debian Bullseye on this device. So, I suspect that
it may have a relatively new firmware.

Btw, that's also the version that came together with Fedora 32:

	$ strings /lib/firmware/ti-connectivity/wl18xx-fw-4.bin |grep FRev
	FRev 8.9.0.0.79
	FRev 8.2.0.0.242

Looking at:
	https://git.ti.com/cgit/wilink8-wlan/wl18xx_fw/

It sounds that there's a newer version released this year:

	2020-05-28	Updated to FW 8.9.0.0.81
	2018-07-29	Updated to FW 8.9.0.0.79

However, it doesn't reached linux-firmware upstream yet:

	$ git log --pretty=oneline ti-connectivity/wl18xx-fw-4.bin
	3a5103fc3c29 wl18xx: update firmware file 8.9.0.0.79
	65b1c68c63f9 wl18xx: update firmware file 8.9.0.0.76
	dbb85a5154a5 wl18xx: update firmware file
	69a250dd556b wl18xx: update firmware file
	dbe3f134bb69 wl18xx: update firmware file, remove conf file
	dab4b79b3fbc wl18xx: add version 4 of the wl18xx firmware

> AND - since there's some firmware the feature doesn't work with, the
> driver should be fixed to detect the running firmware version and not
> do things that the firmware doesn't support.  AND the firmware writer
> should also make it so the firmware doesn't barf on bad input and
> instead rejects it politely.

Agreed. The main issue here seems to be that the current patch
assumes that this feature is available. A proper approach would 
be to check if this feature is available before trying to use it.

Now, I dunno if version 8.9.0.0.81 has what's required for it to
work - or if KEY_IGTK require some custom firmware version.

If it works with such version, one way would be to add a check
for this specific version, disabling KEY_IGTK otherwise.

Also, someone from TI should be sending the newer version to
be added at linux-firmware.

I'll try to do a test maybe tomorrow.

> But I will say I'm making an educated guess; while I have played with
> the TI devices in the past, it was years ago and I won't claim to be
> an expert. I also am unable to fix it myself at this time.
> 
> I'd just rather see it fixed properly instead of a knee-jerk reaction
> of reverting it simply because the OP doesn't have current firmware.

> And let's revisit the discussion of having a kernel splat because an
> unrelated piece of code fails yet the driver does exactly what it is
> supposed to do. We shouldn't be dumping registers and stack-trace when
> the code that crashed has nothing to do with the registers and
> stack-trace outputted. It is a false positive.  A simple printk WARN
> or ERROR should output notifying us that the chip firmware has crashed
> and why.  IMHO.

Thanks,
Mauro

Re: [PATCH] Revert "wlcore: Adding suppoprt for IGTK key in wlcore driver"

[Steve deRosier]

Hi Mauro,

On Thu, Aug 27, 2020 at 10:42 AM Mauro Carvalho Chehab
<mchehab+huawei@kernel.org> wrote:
>
> Em Thu, 27 Aug 2020 08:48:30 -0700
> Steve deRosier <derosier@gmail.com> escreveu:
>
> > On Tue, Aug 25, 2020 at 10:49 PM Mauro Carvalho Chehab
> > <mchehab+huawei@kernel.org> wrote:
> > >
> > > This patch causes a regression betwen Kernel 5.7 and 5.8 at wlcore:
> > > with it applied, WiFi stops working, and the Kernel starts printing
> > > this message every second:
> > >
> > >    wlcore: PHY firmware version: Rev 8.2.0.0.242
> > >    wlcore: firmware booted (Rev 8.9.0.0.79)
> > >    wlcore: ERROR command execute failure 14
> >
> > Only if NO firmware for the device in question supports the `KEY_IGTK`
> > value, then this revert is appropriate. Otherwise, it likely isn't.
>
> Yeah, that's what I suspect too: some specific firmware is required
> for KEY_IGTK to work.
>
> >  My suspicion is that the feature that `KEY_IGTK` is enabling is
> > specific to a newer firmware that Mauro hasn't upgraded to. What the
> > OP should do is find the updated firmware and give it a try.
>
> I didn't try checking if linux-firmware tree has a newer version on
> it. I'm using Debian Bullseye on this device. So, I suspect that
> it may have a relatively new firmware.
>
> Btw, that's also the version that came together with Fedora 32:
>
>         $ strings /lib/firmware/ti-connectivity/wl18xx-fw-4.bin |grep FRev
>         FRev 8.9.0.0.79
>         FRev 8.2.0.0.242
>
> Looking at:
>         https://git.ti.com/cgit/wilink8-wlan/wl18xx_fw/
>
> It sounds that there's a newer version released this year:
>
>         2020-05-28      Updated to FW 8.9.0.0.81
>         2018-07-29      Updated to FW 8.9.0.0.79
>
> However, it doesn't reached linux-firmware upstream yet:
>
>         $ git log --pretty=oneline ti-connectivity/wl18xx-fw-4.bin
>         3a5103fc3c29 wl18xx: update firmware file 8.9.0.0.79
>         65b1c68c63f9 wl18xx: update firmware file 8.9.0.0.76
>         dbb85a5154a5 wl18xx: update firmware file
>         69a250dd556b wl18xx: update firmware file
>         dbe3f134bb69 wl18xx: update firmware file, remove conf file
>         dab4b79b3fbc wl18xx: add version 4 of the wl18xx firmware
>
> > AND - since there's some firmware the feature doesn't work with, the
> > driver should be fixed to detect the running firmware version and not
> > do things that the firmware doesn't support.  AND the firmware writer
> > should also make it so the firmware doesn't barf on bad input and
> > instead rejects it politely.
>
> Agreed. The main issue here seems to be that the current patch
> assumes that this feature is available. A proper approach would
> be to check if this feature is available before trying to use it.
>
> Now, I dunno if version 8.9.0.0.81 has what's required for it to
> work - or if KEY_IGTK require some custom firmware version.
>
> If it works with such version, one way would be to add a check
> for this specific version, disabling KEY_IGTK otherwise.
>
> Also, someone from TI should be sending the newer version to
> be added at linux-firmware.
>
> I'll try to do a test maybe tomorrow.
>

I think we're totally agreed on all of the above points.
Fundamentally: the orig patch should've been coded defensively and
tested properly since clearly it causes certain firmwares to break.
Be nice if TI would both update the firmware and also update the
driver to detect the relevant version for features.  I don't know
about this one, but I do know the QCA firmwares (and others) have a
set of feature flags that are detected by the drivers to determine
what is supported.

I look forward to hearing the results of your test.  This whole thing
has gotten me interested. I'd be tempted to pull out the relevant dev
boards and play with them myself, but IIRC they got sent back to a
previous employer and I don't have access to them anymore.


> > But I will say I'm making an educated guess; while I have played with
> > the TI devices in the past, it was years ago and I won't claim to be
> > an expert. I also am unable to fix it myself at this time.
> >
> > I'd just rather see it fixed properly instead of a knee-jerk reaction
> > of reverting it simply because the OP doesn't have current firmware.
>
> > And let's revisit the discussion of having a kernel splat because an
> > unrelated piece of code fails yet the driver does exactly what it is
> > supposed to do. We shouldn't be dumping registers and stack-trace when
> > the code that crashed has nothing to do with the registers and
> > stack-trace outputted. It is a false positive.  A simple printk WARN
> > or ERROR should output notifying us that the chip firmware has crashed
> > and why.  IMHO.
>
> Thanks,
> Mauro

Thanks,
- Steve

Re: [PATCH] Revert "wlcore: Adding suppoprt for IGTK key in wlcore driver"

[Mauro Carvalho Chehab]

Em Thu, 27 Aug 2020 13:36:28 -0700
Steve deRosier <derosier@gmail.com> escreveu:

> > > And let's revisit the discussion of having a kernel splat because an
> > > unrelated piece of code fails yet the driver does exactly what it is
> > > supposed to do. We shouldn't be dumping registers and stack-trace when
> > > the code that crashed has nothing to do with the registers and
> > > stack-trace outputted. It is a false positive.  A simple printk WARN
> > > or ERROR should output notifying us that the chip firmware has crashed
> > > and why.  IMHO. 

Yeah, that WARN_ON() is disturbing.

Sometimes, it prints it here out of the blue at the first time it
tries to use WiFi:

[    4.502250] mmc_host mmc0: Bus speed (slot 0) = 400000Hz (slot req 400000Hz, actual 400000HZ div = 0)
[    4.542376] mmc_host mmc0: Bus speed (slot 0) = 25000000Hz (slot req 25000000Hz, actual 25000000HZ div = 0)
[    4.678228] mmc_host mmc0: Bus speed (slot 0) = 400000Hz (slot req 400000Hz, actual 400000HZ div = 0)
[    4.719082] mmc_host mmc0: Bus speed (slot 0) = 25000000Hz (slot req 25000000Hz, actual 25000000HZ div = 0)
[    4.830243] mmc_host mmc0: Bus speed (slot 0) = 400000Hz (slot req 400000Hz, actual 400000HZ div = 0)
[    4.870524] mmc_host mmc0: Bus speed (slot 0) = 25000000Hz (slot req 25000000Hz, actual 25000000HZ div = 0)
[    5.088650] wlcore: wl18xx HW: 183x or 180x, PG 2.2 (ROM 0x11)
[    5.095260] wlcore: WARNING Detected unconfigured mac address in nvs, derive from fuse instead.
[    5.104030] wlcore: WARNING This default nvs file can be removed from the file system
[    5.114699] wlcore: loaded
[    5.270777] mmc_host mmc0: Bus speed (slot 0) = 400000Hz (slot req 400000Hz, actual 400000HZ div = 0)
[    5.310835] mmc_host mmc0: Bus speed (slot 0) = 25000000Hz (slot req 25000000Hz, actual 25000000HZ div = 0)
[    5.414725] mmc_host mmc0: Bus speed (slot 0) = 400000Hz (slot req 400000Hz, actual 400000HZ div = 0)
[    5.454684] mmc_host mmc0: Bus speed (slot 0) = 25000000Hz (slot req 25000000Hz, actual 25000000HZ div = 0)
[    5.751078] wlcore: PHY firmware version: Rev 8.2.0.0.243
[    5.799065] wlcore: firmware booted (Rev 8.9.0.0.81)
[    5.804035] wlcore: ERROR Couldn't parse firmware version string

[    5.821800] wlcore: down
[    5.946770] mmc_host mmc0: Bus speed (slot 0) = 400000Hz (slot req 400000Hz, actual 400000HZ div = 0)
[    5.986777] mmc_host mmc0: Bus speed (slot 0) = 25000000Hz (slot req 25000000Hz, actual 25000000HZ div = 0)
[    6.878108] ------------[ cut here ]------------
[    6.882741] WARNING: CPU: 3 PID: 297 at drivers/net/wireless/ti/wlcore/sdio.c:78 wl12xx_sdio_raw_read+0x11c/0x1c0 [wlcore_sdio]
[    6.894220] Modules linked in: wl18xx wlcore mac80211 libarc4 cfg80211 rfkill snd_soc_hdmi_codec wlcore_sdio adv7511 cec kirin9xx_drm(C) crct10dif_ce kirin9xx_dw_drm_dsi(C) drm_kms_helper drm ip_tables x_tables ipv6 nf_defrag_ipv6
[    6.914682] CPU: 3 PID: 297 Comm: NetworkManager Tainted: G         C        5.8.0+ #197
[    6.922771] Hardware name: HiKey970 (DT)
[    6.926693] pstate: 60000005 (nZCv daif -PAN -UAO BTYPE=--)
[    6.932263] pc : wl12xx_sdio_raw_read+0x11c/0x1c0 [wlcore_sdio]
[    6.938181] lr : wl12xx_sdio_raw_read+0x8c/0x1c0 [wlcore_sdio]
[    6.944009] sp : ffff800012793140
[    6.947318] x29: ffff800012793140 x28: 0000000000000000 
[    6.952626] x27: ffff0001a79ba258 x26: ffff0001a79b9e80 
[    6.957935] x25: 0000000000000000 x24: ffff0001ac5db100 
[    6.963243] x23: 0000000000000004 x22: ffff0001b19b1810 
[    6.968552] x21: ffff0001b17a0000 x20: 0000000000013738 
[    6.973859] x19: ffff0001b6988800 x18: 00000002e3eebd18 
[    6.979168] x17: 0000000000000001 x16: 0000000000000001 
[    6.984476] x15: 0008291e1896a232 x14: 0008290c7b540ede 
[    6.989784] x13: 00000000000003c4 x12: 00000000fa83b2da 
[    6.995093] x11: 00000000000003c4 x10: 00000000000009c0 
[    7.000401] x9 : ffff800012792d20 x8 : ffff0001b17a0a20 
[    7.005708] x7 : 0000000000000001 x6 : 0000000001fb396f 
[    7.011017] x5 : 00ffffffffffffff x4 : 0000000000000000 
[    7.016325] x3 : ffff0001b760e104 x2 : 0000000000000000 
[    7.021633] x1 : ffff0001b17a0000 x0 : 00000000ffffff92 
[    7.026943] Call trace:
[    7.029386]  wl12xx_sdio_raw_read+0x11c/0x1c0 [wlcore_sdio]
[    7.034978]  wl18xx_boot+0x414/0x890 [wl18xx]
[    7.039373]  wl1271_op_add_interface+0x784/0xa60 [wlcore]
[    7.044843]  drv_add_interface+0x38/0x84 [mac80211]
[    7.049750]  ieee80211_do_open+0x59c/0x8cc [mac80211]
[    7.054829]  ieee80211_open+0x48/0x70 [mac80211]
[    7.059453]  __dev_open+0xe4/0x190
[    7.062852]  __dev_change_flags+0x180/0x1f0
[    7.067031]  dev_change_flags+0x24/0x64
[    7.070866]  do_setlink+0x20c/0xc40
[    7.074349]  __rtnl_newlink+0x500/0x820
[    7.078180]  rtnl_newlink+0x4c/0x80
[    7.081664]  rtnetlink_rcv_msg+0x11c/0x340
[    7.085759]  netlink_rcv_skb+0x58/0x11c
[    7.089591]  rtnetlink_rcv+0x18/0x2c
[    7.093163]  netlink_unicast+0x25c/0x320
[    7.097080]  netlink_sendmsg+0x190/0x3a0
[    7.101003]  ____sys_sendmsg+0x1d8/0x230
[    7.104921]  ___sys_sendmsg+0x80/0xd0
[    7.108579]  __sys_sendmsg+0x68/0xc4
[    7.112150]  __arm64_sys_sendmsg+0x28/0x3c
[    7.116247]  el0_svc_common.constprop.0+0x6c/0x170
[    7.121035]  do_el0_svc+0x24/0x90
[    7.124347]  el0_sync_handler+0x90/0x19c
[    7.128266]  el0_sync+0x158/0x180
[    7.131576] ---[ end trace 4dc4a75fcea462c4 ]---
[    7.136220] wl1271_sdio mmc0:0001:2: sdio read failed (-110)

It then continues:

[    7.262730] mmc_host mmc0: Bus speed (slot 0) = 400000Hz (slot req 400000Hz, actual 400000HZ div = 0)
[    7.302704] mmc_host mmc0: Bus speed (slot 0) = 25000000Hz (slot req 25000000Hz, actual 25000000HZ div = 0)
[    7.406723] mmc_host mmc0: Bus speed (slot 0) = 400000Hz (slot req 400000Hz, actual 400000HZ div = 0)
[    7.446674] mmc_host mmc0: Bus speed (slot 0) = 25000000Hz (slot req 25000000Hz, actual 25000000HZ div = 0)
[    7.738299] wlcore: PHY firmware version: Rev 8.2.0.0.243
[    7.787048] wlcore: firmware booted (Rev 8.9.0.0.81)
[    7.792020] wlcore: ERROR Couldn't parse firmware version string

[   16.536187] random: crng init done
[   16.539597] random: 7 urandom warning(s) missed due to ratelimiting
[   23.820340] wlcore: down
[   24.534944] wlan0: authenticate with de:ad:de:ad:de:ad
[   24.542583] wlan0: send auth to de:ad:de:ad:de:ad (try 1/3)
[   24.561316] wlan0: authenticated
[   24.568198] wlan0: associate with de:ad:de:ad:de:ad (try 1/3)
[   24.575998] wlan0: RX AssocResp from de:ad:de:ad:de:ad (capab=0x431 status=0 aid=3)
[   24.589510] wlan0: associated

And WiFi works properly.

Maybe it is not properly handling -EPROBE_DEFER somewhere.


Thanks,
Mauro

Re: [PATCH] Revert "wlcore: Adding suppoprt for IGTK key in wlcore driver"

[Mauro Carvalho Chehab]

Em Thu, 27 Aug 2020 13:36:28 -0700
Steve deRosier <derosier@gmail.com> escreveu:

> Hi Mauro,
> 
> On Thu, Aug 27, 2020 at 10:42 AM Mauro Carvalho Chehab
> <mchehab+huawei@kernel.org> wrote:
> >
> > Em Thu, 27 Aug 2020 08:48:30 -0700
> > Steve deRosier <derosier@gmail.com> escreveu:
> >  
> > > On Tue, Aug 25, 2020 at 10:49 PM Mauro Carvalho Chehab
> > > <mchehab+huawei@kernel.org> wrote:  
> > > >
> > > > This patch causes a regression betwen Kernel 5.7 and 5.8 at wlcore:
> > > > with it applied, WiFi stops working, and the Kernel starts printing
> > > > this message every second:
> > > >
> > > >    wlcore: PHY firmware version: Rev 8.2.0.0.242
> > > >    wlcore: firmware booted (Rev 8.9.0.0.79)
> > > >    wlcore: ERROR command execute failure 14  
> > >
> > > Only if NO firmware for the device in question supports the `KEY_IGTK`
> > > value, then this revert is appropriate. Otherwise, it likely isn't.  
> >
> > Yeah, that's what I suspect too: some specific firmware is required
> > for KEY_IGTK to work.
> >  
> > >  My suspicion is that the feature that `KEY_IGTK` is enabling is
> > > specific to a newer firmware that Mauro hasn't upgraded to. What the
> > > OP should do is find the updated firmware and give it a try.  
> >
> > I didn't try checking if linux-firmware tree has a newer version on
> > it. I'm using Debian Bullseye on this device. So, I suspect that
> > it may have a relatively new firmware.
> >
> > Btw, that's also the version that came together with Fedora 32:
> >
> >         $ strings /lib/firmware/ti-connectivity/wl18xx-fw-4.bin |grep FRev
> >         FRev 8.9.0.0.79
> >         FRev 8.2.0.0.242
> >
> > Looking at:
> >         https://git.ti.com/cgit/wilink8-wlan/wl18xx_fw/
> >
> > It sounds that there's a newer version released this year:
> >
> >         2020-05-28      Updated to FW 8.9.0.0.81
> >         2018-07-29      Updated to FW 8.9.0.0.79
> >
> > However, it doesn't reached linux-firmware upstream yet:
> >
> >         $ git log --pretty=oneline ti-connectivity/wl18xx-fw-4.bin
> >         3a5103fc3c29 wl18xx: update firmware file 8.9.0.0.79
> >         65b1c68c63f9 wl18xx: update firmware file 8.9.0.0.76
> >         dbb85a5154a5 wl18xx: update firmware file
> >         69a250dd556b wl18xx: update firmware file
> >         dbe3f134bb69 wl18xx: update firmware file, remove conf file
> >         dab4b79b3fbc wl18xx: add version 4 of the wl18xx firmware
> >  
> > > AND - since there's some firmware the feature doesn't work with, the
> > > driver should be fixed to detect the running firmware version and not
> > > do things that the firmware doesn't support.  AND the firmware writer
> > > should also make it so the firmware doesn't barf on bad input and
> > > instead rejects it politely.  
> >
> > Agreed. The main issue here seems to be that the current patch
> > assumes that this feature is available. A proper approach would
> > be to check if this feature is available before trying to use it.
> >
> > Now, I dunno if version 8.9.0.0.81 has what's required for it to
> > work - or if KEY_IGTK require some custom firmware version.
> >
> > If it works with such version, one way would be to add a check
> > for this specific version, disabling KEY_IGTK otherwise.
> >
> > Also, someone from TI should be sending the newer version to
> > be added at linux-firmware.
> >
> > I'll try to do a test maybe tomorrow.
> >  
> 
> I think we're totally agreed on all of the above points.
> Fundamentally: the orig patch should've been coded defensively and
> tested properly since clearly it causes certain firmwares to break.
> Be nice if TI would both update the firmware and also update the
> driver to detect the relevant version for features.  I don't know
> about this one, but I do know the QCA firmwares (and others) have a
> set of feature flags that are detected by the drivers to determine
> what is supported.
> 
> I look forward to hearing the results of your test.  This whole thing
> has gotten me interested. I'd be tempted to pull out the relevant dev
> boards and play with them myself, but IIRC they got sent back to a
> previous employer and I don't have access to them anymore.

I upgraded to the newest firmware available at TI firmware site:

	https://git.ti.com/cgit/wilink8-wlan/wl18xx_fw/log/

No joy. It worked once, but I guess it just selected some different
cipher, as the access point it is logging in is set to WPA2-PSK
with auto cipher.

I even wrote a patch that checks the version, enabling AES-CMAC
algorithm only with newer firmware (see below).

Maybe this feature will only be available on some future firmware
or it requires some custom-made one. It may also require some
newer version of the chipset.

So, for now, I suggest to revert this patch, c/c stable.

A later patch can be re-enable it, once some additional logic
gets added in order to validate if this algorithm is
properly supported by the hardware/firmware.


Thanks,
Mauro

[PATCH] net: wireless: wlcore: fix support for IGTK key

Changeset 2b7aadd3b9e1 ("wlcore: Adding suppoprt for IGTK key in wlcore driver")
added support for AEC-CMAC cipher suite.

However, this only works with the very newest firmware version
(8.9.0.0.81). Such firmware weren't even pushed to linux-firmware
git tree yet:

	https://git.ti.com/cgit/wilink8-wlan/wl18xx_fw/log/

Due to that, it causes a regression betwen Kernel 5.7 and 5.8:
with such patch applied, WiFi stops working, and the Kernel starts
printing this message every second:

   wlcore: PHY firmware version: Rev 8.2.0.0.242
   wlcore: firmware booted (Rev 8.9.0.0.79)
   wlcore: ERROR command execute failure 14
   ------------[ cut here ]------------
   WARNING: CPU: 0 PID: 133 at drivers/net/wireless/ti/wlcore/main.c:795 wl12xx_queue_recovery_work.part.0+0x6c/0x74 [wlcore]
   Modules linked in: wl18xx wlcore mac80211 libarc4 cfg80211 rfkill snd_soc_hdmi_codec crct10dif_ce wlcore_sdio adv7511 cec kirin9xx_drm(C) kirin9xx_dw_drm_dsi(C) drm_kms_helper drm ip_tables x_tables ipv6 nf_defrag_ipv6
   CPU: 0 PID: 133 Comm: kworker/0:1 Tainted: G        WC        5.8.0+ #186
   Hardware name: HiKey970 (DT)
   Workqueue: events_freezable ieee80211_restart_work [mac80211]
   pstate: 60000005 (nZCv daif -PAN -UAO BTYPE=--)
   pc : wl12xx_queue_recovery_work.part.0+0x6c/0x74 [wlcore]
   lr : wl12xx_queue_recovery_work+0x24/0x30 [wlcore]
   sp : ffff8000126c3a60
   x29: ffff8000126c3a60 x28: 00000000000025de
   x27: 0000000000000010 x26: 0000000000000005
   x25: ffff0001a5d49e80 x24: ffff8000092cf580
   x23: ffff0001b7c12623 x22: ffff0001b6fcf2e8
   x21: ffff0001b7e46200 x20: 00000000fffffffb
   x19: ffff0001a78e6400 x18: 0000000000000030
   x17: 0000000000000001 x16: 0000000000000001
   x15: ffff0001b7e46670 x14: ffffffffffffffff
   x13: ffff8000926c37d7 x12: ffff8000126c37e0
   x11: ffff800011e01000 x10: ffff8000120526d0
   x9 : 0000000000000000 x8 : 3431206572756c69
   x7 : 6166206574756365 x6 : 0000000000000c2c
   x5 : 0000000000000000 x4 : ffff0001bf1361e8
   x3 : ffff0001bf1790b0 x2 : 0000000000000000
   x1 : ffff0001a5d49e80 x0 : 0000000000000001
   Call trace:
    wl12xx_queue_recovery_work.part.0+0x6c/0x74 [wlcore]
    wl12xx_queue_recovery_work+0x24/0x30 [wlcore]
    wl1271_cmd_set_sta_key+0x258/0x25c [wlcore]
    wl1271_set_key+0x7c/0x2dc [wlcore]
    wlcore_set_key+0xe4/0x360 [wlcore]
    wl18xx_set_key+0x48/0x1d0 [wl18xx]
    wlcore_op_set_key+0xa4/0x180 [wlcore]
    ieee80211_key_enable_hw_accel+0xb0/0x2d0 [mac80211]
    ieee80211_reenable_keys+0x70/0x110 [mac80211]
    ieee80211_reconfig+0xa00/0xca0 [mac80211]
    ieee80211_restart_work+0xc4/0xfc [mac80211]
    process_one_work+0x1cc/0x350
    worker_thread+0x13c/0x470
    kthread+0x154/0x160
    ret_from_fork+0x10/0x30
   ---[ end trace b1f722abf9af5919 ]---
   wlcore: WARNING could not set keys
   wlcore: ERROR Could not add or replace key
   wlan0: failed to set key (4, ff:ff:ff:ff:ff:ff) to hardware (-5)
   wlcore: Hardware recovery in progress. FW ver: Rev 8.9.0.0.79
   wlcore: pc: 0x0, hint_sts: 0x00000040 count: 39
   wlcore: down
   wlcore: down
   ieee80211 phy0: Hardware restart was requested
   mmc_host mmc0: Bus speed (slot 0) = 400000Hz (slot req 400000Hz, actual 400000HZ div = 0)
   mmc_host mmc0: Bus speed (slot 0) = 25000000Hz (slot req 25000000Hz, actual 25000000HZ div = 0)
   wlcore: PHY firmware version: Rev 8.2.0.0.242
   wlcore: firmware booted (Rev 8.9.0.0.79)
   wlcore: ERROR command execute failure 14
   ------------[ cut here ]------------

Fix it by adding some code that will check if the firmware version
is at least version 8.9.0.0.81.

Fixes: 2b7aadd3b9e1 ("wlcore: Adding suppoprt for IGTK key in wlcore driver")
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

diff --git a/drivers/net/wireless/ti/wlcore/main.c b/drivers/net/wireless/ti/wlcore/main.c
index de6c8a7589ca..db9e410c5d0b 100644
--- a/drivers/net/wireless/ti/wlcore/main.c
+++ b/drivers/net/wireless/ti/wlcore/main.c
@@ -14,6 +14,7 @@
 #include <linux/irq.h>
 #include <linux/pm_runtime.h>
 #include <linux/pm_wakeirq.h>
+#include <linux/ctype.h>
 
 #include "wlcore.h"
 #include "debug.h"
@@ -1082,6 +1083,45 @@ static int wl12xx_chip_wakeup(struct wl1271 *wl, bool plt)
 	return ret;
 }
 
+bool wl1271_check_aes_cmac_cypher(struct wl1271 *wl)
+{
+	int ver[5] = { };
+	int ret;
+	const char *p = wl->chip.fw_ver_str;
+
+
+	/* The string starts with "Rev ". Ignore it */
+	while (*p && !isdigit(*p))
+		p++;
+
+	ret = sscanf(p, "%d.%d.%d.%d.%d",
+		     &ver[0], &ver[1], &ver[2], &ver[3], &ver[4]);
+
+	if (ret != ARRAY_SIZE(ver)) {
+		wl1271_info("Parsed version: %d.%d.%d.%d.%d\n",
+			    ver[0], ver[1], ver[2], ver[3], ver[4]);
+		wl1271_error("Couldn't parse firmware version string: %d\n", ret);
+		return false;
+	}
+
+	/*
+	 * Only versions equal (and probably above) 8.9.0.0.81
+	 * supports such feature.
+	 */
+	if (ver[0] < 8)
+		return false;
+	if (ver[1] < 9)
+		return false;
+	if (ver[2] > 0)
+		return true;
+	if (ver[3] > 0)
+		return true;
+	if (ver[4] >= 81)
+		return true;
+
+	return false;
+}
+
 int wl1271_plt_start(struct wl1271 *wl, const enum plt_mode plt_mode)
 {
 	int retries = WL1271_BOOT_RETRIES;
@@ -1133,6 +1173,8 @@ int wl1271_plt_start(struct wl1271 *wl, const enum plt_mode plt_mode)
 		strncpy(wiphy->fw_version, wl->chip.fw_ver_str,
 			sizeof(wiphy->fw_version));
 
+		wl->has_aes_cmac_cipher = wl1271_check_aes_cmac_cypher(wl);
+
 		goto out;
 
 power_off:
@@ -2358,6 +2400,8 @@ static int wl12xx_init_fw(struct wl1271 *wl)
 	strncpy(wiphy->fw_version, wl->chip.fw_ver_str,
 		sizeof(wiphy->fw_version));
 
+	wl->has_aes_cmac_cipher = wl1271_check_aes_cmac_cypher(wl);
+
 	/*
 	 * Now we know if 11a is supported (info from the NVS), so disable
 	 * 11a channels if not supported
@@ -3551,6 +3595,10 @@ int wlcore_set_key(struct wl1271 *wl, enum set_key_cmd cmd,
 		key_type = KEY_GEM;
 		break;
 	case WLAN_CIPHER_SUITE_AES_CMAC:
+		if (!wl->has_aes_cmac_cipher) {
+			wl1271_error("AEC CMAC cipher not available on this firmware version\n");
+			return -EOPNOTSUPP;
+		}
 		key_type = KEY_IGTK;
 		break;
 	default:
diff --git a/drivers/net/wireless/ti/wlcore/wlcore.h b/drivers/net/wireless/ti/wlcore/wlcore.h
index b7821311ac75..26a2bd9b2df1 100644
--- a/drivers/net/wireless/ti/wlcore/wlcore.h
+++ b/drivers/net/wireless/ti/wlcore/wlcore.h
@@ -213,6 +213,8 @@ struct wl1271 {
 	void *nvs;
 	size_t nvs_len;
 
+	u32 has_aes_cmac_cipher:1;
+
 	s8 hw_pg_ver;
 
 	/* address read from the fuse ROM */

Re: [PATCH] Revert "wlcore: Adding suppoprt for IGTK key in wlcore driver"

[Kalle Valo]

Steve deRosier <derosier@gmail.com> writes:

> On Tue, Aug 25, 2020 at 10:49 PM Mauro Carvalho Chehab
> <mchehab+huawei@kernel.org> wrote:
>>
>> This patch causes a regression betwen Kernel 5.7 and 5.8 at wlcore:
>> with it applied, WiFi stops working, and the Kernel starts printing
>> this message every second:
>>
>>    wlcore: PHY firmware version: Rev 8.2.0.0.242
>>    wlcore: firmware booted (Rev 8.9.0.0.79)
>>    wlcore: ERROR command execute failure 14
>
> Only if NO firmware for the device in question supports the `KEY_IGTK`
> value, then this revert is appropriate. Otherwise, it likely isn't.
>  My suspicion is that the feature that `KEY_IGTK` is enabling is
> specific to a newer firmware that Mauro hasn't upgraded to. What the
> OP should do is find the updated firmware and give it a try.
>
> AND - since there's some firmware the feature doesn't work with, the
> driver should be fixed to detect the running firmware version and not
> do things that the firmware doesn't support.  AND the firmware writer
> should also make it so the firmware doesn't barf on bad input and
> instead rejects it politely.
>
> But I will say I'm making an educated guess; while I have played with
> the TI devices in the past, it was years ago and I won't claim to be
> an expert. I also am unable to fix it myself at this time.
>
> I'd just rather see it fixed properly instead of a knee-jerk reaction
> of reverting it simply because the OP doesn't have current firmware.

Yeah, a proper fix for this is of course better but if there's no fix,
say within the next week or so, let's revert this. A new version of the
patch implementing IGTK, with proper feature detection, can be always
added later.

-- 
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH] Revert "wlcore: Adding suppoprt for IGTK key in wlcore driver"

[Kalle Valo]

Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote:

> This patch causes a regression betwen Kernel 5.7 and 5.8 at wlcore:
> with it applied, WiFi stops working, and the Kernel starts printing
> this message every second:
> 
>    wlcore: PHY firmware version: Rev 8.2.0.0.242
>    wlcore: firmware booted (Rev 8.9.0.0.79)
>    wlcore: ERROR command execute failure 14
>    ------------[ cut here ]------------
>    WARNING: CPU: 0 PID: 133 at drivers/net/wireless/ti/wlcore/main.c:795 wl12xx_queue_recovery_work.part.0+0x6c/0x74 [wlcore]
>    Modules linked in: wl18xx wlcore mac80211 libarc4 cfg80211 rfkill snd_soc_hdmi_codec crct10dif_ce wlcore_sdio adv7511 cec kirin9xx_drm(C) kirin9xx_dw_drm_dsi(C) drm_kms_helper drm ip_tables x_tables ipv6 nf_defrag_ipv6
>    CPU: 0 PID: 133 Comm: kworker/0:1 Tainted: G        WC        5.8.0+ #186
>    Hardware name: HiKey970 (DT)
>    Workqueue: events_freezable ieee80211_restart_work [mac80211]
>    pstate: 60000005 (nZCv daif -PAN -UAO BTYPE=--)
>    pc : wl12xx_queue_recovery_work.part.0+0x6c/0x74 [wlcore]
>    lr : wl12xx_queue_recovery_work+0x24/0x30 [wlcore]
>    sp : ffff8000126c3a60
>    x29: ffff8000126c3a60 x28: 00000000000025de
>    x27: 0000000000000010 x26: 0000000000000005
>    x25: ffff0001a5d49e80 x24: ffff8000092cf580
>    x23: ffff0001b7c12623 x22: ffff0001b6fcf2e8
>    x21: ffff0001b7e46200 x20: 00000000fffffffb
>    x19: ffff0001a78e6400 x18: 0000000000000030
>    x17: 0000000000000001 x16: 0000000000000001
>    x15: ffff0001b7e46670 x14: ffffffffffffffff
>    x13: ffff8000926c37d7 x12: ffff8000126c37e0
>    x11: ffff800011e01000 x10: ffff8000120526d0
>    x9 : 0000000000000000 x8 : 3431206572756c69
>    x7 : 6166206574756365 x6 : 0000000000000c2c
>    x5 : 0000000000000000 x4 : ffff0001bf1361e8
>    x3 : ffff0001bf1790b0 x2 : 0000000000000000
>    x1 : ffff0001a5d49e80 x0 : 0000000000000001
>    Call trace:
>     wl12xx_queue_recovery_work.part.0+0x6c/0x74 [wlcore]
>     wl12xx_queue_recovery_work+0x24/0x30 [wlcore]
>     wl1271_cmd_set_sta_key+0x258/0x25c [wlcore]
>     wl1271_set_key+0x7c/0x2dc [wlcore]
>     wlcore_set_key+0xe4/0x360 [wlcore]
>     wl18xx_set_key+0x48/0x1d0 [wl18xx]
>     wlcore_op_set_key+0xa4/0x180 [wlcore]
>     ieee80211_key_enable_hw_accel+0xb0/0x2d0 [mac80211]
>     ieee80211_reenable_keys+0x70/0x110 [mac80211]
>     ieee80211_reconfig+0xa00/0xca0 [mac80211]
>     ieee80211_restart_work+0xc4/0xfc [mac80211]
>     process_one_work+0x1cc/0x350
>     worker_thread+0x13c/0x470
>     kthread+0x154/0x160
>     ret_from_fork+0x10/0x30
>    ---[ end trace b1f722abf9af5919 ]---
>    wlcore: WARNING could not set keys
>    wlcore: ERROR Could not add or replace key
>    wlan0: failed to set key (4, ff:ff:ff:ff:ff:ff) to hardware (-5)
>    wlcore: Hardware recovery in progress. FW ver: Rev 8.9.0.0.79
>    wlcore: pc: 0x0, hint_sts: 0x00000040 count: 39
>    wlcore: down
>    wlcore: down
>    ieee80211 phy0: Hardware restart was requested
>    mmc_host mmc0: Bus speed (slot 0) = 400000Hz (slot req 400000Hz, actual 400000HZ div = 0)
>    mmc_host mmc0: Bus speed (slot 0) = 25000000Hz (slot req 25000000Hz, actual 25000000HZ div = 0)
>    wlcore: PHY firmware version: Rev 8.2.0.0.242
>    wlcore: firmware booted (Rev 8.9.0.0.79)
>    wlcore: ERROR command execute failure 14
>    ------------[ cut here ]------------
> 
> Tested on Hikey 970.
> 
> This reverts commit 2b7aadd3b9e17e8b81eeb8d9cc46756ae4658265.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

Any updates? If I don't hear anything I will just queue this to v5.9.

Reminder to myself: remove Mauro's duplicate s-o-b tag, that's a patchwork bug

-- 
https://patchwork.kernel.org/patch/11737193/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

RE: [EXTERNAL] Re: [PATCH] Revert "wlcore: Adding suppoprt for IGTK key in wlcore driver"

[Bouganim, Raz]

We are going to release a new FW version 8.9.0.0.83 that contains support with the new IGTK key.

In addition, we also going to release a new patch that mandates the driver to work with an 8.9.0.0.83 FW version or above.

We going to push it today/tomorrow.

[PATCH] wl18xx: Update the latest firmware supported
This patch mandates the driver to work with an 8.9.0.0.83 FW version or above.
This is to fix a kernel panic caused by establishing a PMF/WPA3 connection with older FW versions.

You can get the latest firmware at:
git://git.ti.com/wilink8-wlan/wl18xx_fw.git

Signed-off-by: Raz Bouganim <r-bouganim@ti.com>
---
 drivers/net/wireless/ti/wl18xx/wl18xx.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ti/wl18xx/wl18xx.h b/drivers/net/wireless/ti/wl18xx/wl18xx.h
index b642e0c..03ad7f6 100644
--- a/drivers/net/wireless/ti/wl18xx/wl18xx.h
+++ b/drivers/net/wireless/ti/wl18xx/wl18xx.h
@@ -15,7 +15,7 @@
 #define WL18XX_IFTYPE_VER	9
 #define WL18XX_MAJOR_VER	WLCORE_FW_VER_IGNORE
 #define WL18XX_SUBTYPE_VER	WLCORE_FW_VER_IGNORE
-#define WL18XX_MINOR_VER	58
+#define WL18XX_MINOR_VER	83
 
 #define WL18XX_CMD_MAX_SIZE          740
 
--
1.9.1
-----Original Message-----
From: kvalo=codeaurora.org@mg.codeaurora.org [mailto:kvalo=codeaurora.org@mg.codeaurora.org] On Behalf Of Kalle Valo
Sent: Tuesday, September 1, 2020 12:31 PM
To: Mauro Carvalho Chehab
Cc: linuxarm@huawei.com; mauro.chehab@huawei.com; Mauro Carvalho Chehab; John Stultz; Manivannan Sadhasivam; David S. Miller; Jakub Kicinski; Hahn, Maital; Gustavo A. R. Silva; Bouganim, Raz; Tony Lindgren; Dinghao Liu; Johannes Berg; Fuqian Huang; linux-wireless@vger.kernel.org; netdev@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: [EXTERNAL] Re: [PATCH] Revert "wlcore: Adding suppoprt for IGTK key in wlcore driver"

Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote:

> This patch causes a regression betwen Kernel 5.7 and 5.8 at wlcore:
> with it applied, WiFi stops working, and the Kernel starts printing
> this message every second:
> 
>    wlcore: PHY firmware version: Rev 8.2.0.0.242
>    wlcore: firmware booted (Rev 8.9.0.0.79)
>    wlcore: ERROR command execute failure 14
>    ------------[ cut here ]------------
>    WARNING: CPU: 0 PID: 133 at drivers/net/wireless/ti/wlcore/main.c:795 wl12xx_queue_recovery_work.part.0+0x6c/0x74 [wlcore]
>    Modules linked in: wl18xx wlcore mac80211 libarc4 cfg80211 rfkill snd_soc_hdmi_codec crct10dif_ce wlcore_sdio adv7511 cec kirin9xx_drm(C) kirin9xx_dw_drm_dsi(C) drm_kms_helper drm ip_tables x_tables ipv6 nf_defrag_ipv6
>    CPU: 0 PID: 133 Comm: kworker/0:1 Tainted: G        WC        5.8.0+ #186
>    Hardware name: HiKey970 (DT)
>    Workqueue: events_freezable ieee80211_restart_work [mac80211]
>    pstate: 60000005 (nZCv daif -PAN -UAO BTYPE=--)
>    pc : wl12xx_queue_recovery_work.part.0+0x6c/0x74 [wlcore]
>    lr : wl12xx_queue_recovery_work+0x24/0x30 [wlcore]
>    sp : ffff8000126c3a60
>    x29: ffff8000126c3a60 x28: 00000000000025de
>    x27: 0000000000000010 x26: 0000000000000005
>    x25: ffff0001a5d49e80 x24: ffff8000092cf580
>    x23: ffff0001b7c12623 x22: ffff0001b6fcf2e8
>    x21: ffff0001b7e46200 x20: 00000000fffffffb
>    x19: ffff0001a78e6400 x18: 0000000000000030
>    x17: 0000000000000001 x16: 0000000000000001
>    x15: ffff0001b7e46670 x14: ffffffffffffffff
>    x13: ffff8000926c37d7 x12: ffff8000126c37e0
>    x11: ffff800011e01000 x10: ffff8000120526d0
>    x9 : 0000000000000000 x8 : 3431206572756c69
>    x7 : 6166206574756365 x6 : 0000000000000c2c
>    x5 : 0000000000000000 x4 : ffff0001bf1361e8
>    x3 : ffff0001bf1790b0 x2 : 0000000000000000
>    x1 : ffff0001a5d49e80 x0 : 0000000000000001
>    Call trace:
>     wl12xx_queue_recovery_work.part.0+0x6c/0x74 [wlcore]
>     wl12xx_queue_recovery_work+0x24/0x30 [wlcore]
>     wl1271_cmd_set_sta_key+0x258/0x25c [wlcore]
>     wl1271_set_key+0x7c/0x2dc [wlcore]
>     wlcore_set_key+0xe4/0x360 [wlcore]
>     wl18xx_set_key+0x48/0x1d0 [wl18xx]
>     wlcore_op_set_key+0xa4/0x180 [wlcore]
>     ieee80211_key_enable_hw_accel+0xb0/0x2d0 [mac80211]
>     ieee80211_reenable_keys+0x70/0x110 [mac80211]
>     ieee80211_reconfig+0xa00/0xca0 [mac80211]
>     ieee80211_restart_work+0xc4/0xfc [mac80211]
>     process_one_work+0x1cc/0x350
>     worker_thread+0x13c/0x470
>     kthread+0x154/0x160
>     ret_from_fork+0x10/0x30
>    ---[ end trace b1f722abf9af5919 ]---
>    wlcore: WARNING could not set keys
>    wlcore: ERROR Could not add or replace key
>    wlan0: failed to set key (4, ff:ff:ff:ff:ff:ff) to hardware (-5)
>    wlcore: Hardware recovery in progress. FW ver: Rev 8.9.0.0.79
>    wlcore: pc: 0x0, hint_sts: 0x00000040 count: 39
>    wlcore: down
>    wlcore: down
>    ieee80211 phy0: Hardware restart was requested
>    mmc_host mmc0: Bus speed (slot 0) = 400000Hz (slot req 400000Hz, actual 400000HZ div = 0)
>    mmc_host mmc0: Bus speed (slot 0) = 25000000Hz (slot req 25000000Hz, actual 25000000HZ div = 0)
>    wlcore: PHY firmware version: Rev 8.2.0.0.242
>    wlcore: firmware booted (Rev 8.9.0.0.79)
>    wlcore: ERROR command execute failure 14
>    ------------[ cut here ]------------
> 
> Tested on Hikey 970.
> 
> This reverts commit 2b7aadd3b9e17e8b81eeb8d9cc46756ae4658265.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

Any updates? If I don't hear anything I will just queue this to v5.9.

Reminder to myself: remove Mauro's duplicate s-o-b tag, that's a patchwork bug

-- 
https://patchwork.kernel.org/patch/11737193/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [EXTERNAL] Re: [PATCH] Revert "wlcore: Adding suppoprt for IGTK key in wlcore driver"

[Kalle Valo]

"Bouganim, Raz" <r-bouganim@ti.com> writes:

> We are going to release a new FW version 8.9.0.0.83 that contains
> support with the new IGTK key.
>
> In addition, we also going to release a new patch that mandates the
> driver to work with an 8.9.0.0.83 FW version or above.
>
> We going to push it today/tomorrow.

You shouldn't break the support for old firmware, instead please
implement it so that both old and new firmware are supported at the same
time.

-- 
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

[PATCH v2] net: wireless: wlcore: fix support for IGTK key

[Mauro Carvalho Chehab]

Changeset 2b7aadd3b9e1 ("wlcore: Adding suppoprt for IGTK key in wlcore driver")
added support for AEC CMAC cipher suite.

However, this only works with the very newest firmware version
(8.9.0.0.83). Such firmware weren't even pushed to linux-firmware
git tree yet:

	https://git.ti.com/cgit/wilink8-wlan/wl18xx_fw/log/

Due to that, it causes a regression betwen Kernel 5.7 and 5.8:
with such patch applied, WiFi stops working, and the Kernel starts
printing this message every second:

   wlcore: PHY firmware version: Rev 8.2.0.0.242
   wlcore: firmware booted (Rev 8.9.0.0.79)
   wlcore: ERROR command execute failure 14
   ------------[ cut here ]------------
   WARNING: CPU: 0 PID: 133 at drivers/net/wireless/ti/wlcore/main.c:795 wl12xx_queue_recovery_work.part.0+0x6c/0x74 [wlcore]
   Modules linked in: wl18xx wlcore mac80211 libarc4 cfg80211 rfkill snd_soc_hdmi_codec crct10dif_ce wlcore_sdio adv7511 cec kirin9xx_drm(C) kirin9xx_dw_drm_dsi(C) drm_kms_helper drm ip_tables x_tables ipv6 nf_defrag_ipv6
   CPU: 0 PID: 133 Comm: kworker/0:1 Tainted: G        WC        5.8.0+ #186
   Hardware name: HiKey970 (DT)
   Workqueue: events_freezable ieee80211_restart_work [mac80211]
   pstate: 60000005 (nZCv daif -PAN -UAO BTYPE=--)
   pc : wl12xx_queue_recovery_work.part.0+0x6c/0x74 [wlcore]
   lr : wl12xx_queue_recovery_work+0x24/0x30 [wlcore]
   sp : ffff8000126c3a60
   x29: ffff8000126c3a60 x28: 00000000000025de
   x27: 0000000000000010 x26: 0000000000000005
   x25: ffff0001a5d49e80 x24: ffff8000092cf580
   x23: ffff0001b7c12623 x22: ffff0001b6fcf2e8
   x21: ffff0001b7e46200 x20: 00000000fffffffb
   x19: ffff0001a78e6400 x18: 0000000000000030
   x17: 0000000000000001 x16: 0000000000000001
   x15: ffff0001b7e46670 x14: ffffffffffffffff
   x13: ffff8000926c37d7 x12: ffff8000126c37e0
   x11: ffff800011e01000 x10: ffff8000120526d0
   x9 : 0000000000000000 x8 : 3431206572756c69
   x7 : 6166206574756365 x6 : 0000000000000c2c
   x5 : 0000000000000000 x4 : ffff0001bf1361e8
   x3 : ffff0001bf1790b0 x2 : 0000000000000000
   x1 : ffff0001a5d49e80 x0 : 0000000000000001
   Call trace:
    wl12xx_queue_recovery_work.part.0+0x6c/0x74 [wlcore]
    wl12xx_queue_recovery_work+0x24/0x30 [wlcore]
    wl1271_cmd_set_sta_key+0x258/0x25c [wlcore]
    wl1271_set_key+0x7c/0x2dc [wlcore]
    wlcore_set_key+0xe4/0x360 [wlcore]
    wl18xx_set_key+0x48/0x1d0 [wl18xx]
    wlcore_op_set_key+0xa4/0x180 [wlcore]
    ieee80211_key_enable_hw_accel+0xb0/0x2d0 [mac80211]
    ieee80211_reenable_keys+0x70/0x110 [mac80211]
    ieee80211_reconfig+0xa00/0xca0 [mac80211]
    ieee80211_restart_work+0xc4/0xfc [mac80211]
    process_one_work+0x1cc/0x350
    worker_thread+0x13c/0x470
    kthread+0x154/0x160
    ret_from_fork+0x10/0x30
   ---[ end trace b1f722abf9af5919 ]---
   wlcore: WARNING could not set keys
   wlcore: ERROR Could not add or replace key
   wlan0: failed to set key (4, ff:ff:ff:ff:ff:ff) to hardware (-5)
   wlcore: Hardware recovery in progress. FW ver: Rev 8.9.0.0.79
   wlcore: pc: 0x0, hint_sts: 0x00000040 count: 39
   wlcore: down
   wlcore: down
   ieee80211 phy0: Hardware restart was requested
   mmc_host mmc0: Bus speed (slot 0) = 400000Hz (slot req 400000Hz, actual 400000HZ div = 0)
   mmc_host mmc0: Bus speed (slot 0) = 25000000Hz (slot req 25000000Hz, actual 25000000HZ div = 0)
   wlcore: PHY firmware version: Rev 8.2.0.0.242
   wlcore: firmware booted (Rev 8.9.0.0.79)
   wlcore: ERROR command execute failure 14
   ------------[ cut here ]------------

Fix it by adding some code that will check if the firmware version
is at least version 8.9.0.0.83.

Tested on Hikey 970.

Fixes: 2b7aadd3b9e1 ("wlcore: Adding suppoprt for IGTK key in wlcore driver")
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---

Hi Raz,

Maybe this patch could be useful once firmware 8.9.0.0.83 is released.

v2: change it to require minimal verson 8.9.0.0.83 for IGTK key

 drivers/net/wireless/ti/wlcore/main.c   | 48 +++++++++++++++++++++++++
 drivers/net/wireless/ti/wlcore/wlcore.h |  2 ++
 2 files changed, 50 insertions(+)

diff --git a/drivers/net/wireless/ti/wlcore/main.c b/drivers/net/wireless/ti/wlcore/main.c
index de6c8a7589ca..4332eaa1a733 100644
--- a/drivers/net/wireless/ti/wlcore/main.c
+++ b/drivers/net/wireless/ti/wlcore/main.c
@@ -14,6 +14,7 @@
 #include <linux/irq.h>
 #include <linux/pm_runtime.h>
 #include <linux/pm_wakeirq.h>
+#include <linux/ctype.h>
 
 #include "wlcore.h"
 #include "debug.h"
@@ -1082,6 +1083,45 @@ static int wl12xx_chip_wakeup(struct wl1271 *wl, bool plt)
 	return ret;
 }
 
+static bool wl1271_check_aes_cmac_cypher(struct wl1271 *wl)
+{
+	int ver[5] = { };
+	int ret;
+	const char *p = wl->chip.fw_ver_str;
+
+
+	/* The string starts with "Rev ". Ignore it */
+	while (*p && !isdigit(*p))
+		p++;
+
+	ret = sscanf(p, "%d.%d.%d.%d.%d",
+		     &ver[0], &ver[1], &ver[2], &ver[3], &ver[4]);
+
+	if (ret != ARRAY_SIZE(ver)) {
+		wl1271_info("Parsed version: %d.%d.%d.%d.%d\n",
+			    ver[0], ver[1], ver[2], ver[3], ver[4]);
+		wl1271_error("Couldn't parse firmware version string: %d\n", ret);
+		return false;
+	}
+
+	/*
+	 * Only versions equal (and probably above) 8.9.0.0.83
+	 * supports such feature.
+	 */
+	if (ver[0] < 8)
+		return false;
+	if (ver[1] < 9)
+		return false;
+	if (ver[2] > 0)
+		return true;
+	if (ver[3] > 0)
+		return true;
+	if (ver[4] >= 83)
+		return true;
+
+	return false;
+}
+
 int wl1271_plt_start(struct wl1271 *wl, const enum plt_mode plt_mode)
 {
 	int retries = WL1271_BOOT_RETRIES;
@@ -1133,6 +1173,8 @@ int wl1271_plt_start(struct wl1271 *wl, const enum plt_mode plt_mode)
 		strncpy(wiphy->fw_version, wl->chip.fw_ver_str,
 			sizeof(wiphy->fw_version));
 
+		wl->has_aes_cmac_cipher = wl1271_check_aes_cmac_cypher(wl);
+
 		goto out;
 
 power_off:
@@ -2358,6 +2400,8 @@ static int wl12xx_init_fw(struct wl1271 *wl)
 	strncpy(wiphy->fw_version, wl->chip.fw_ver_str,
 		sizeof(wiphy->fw_version));
 
+	wl->has_aes_cmac_cipher = wl1271_check_aes_cmac_cypher(wl);
+
 	/*
 	 * Now we know if 11a is supported (info from the NVS), so disable
 	 * 11a channels if not supported
@@ -3551,6 +3595,10 @@ int wlcore_set_key(struct wl1271 *wl, enum set_key_cmd cmd,
 		key_type = KEY_GEM;
 		break;
 	case WLAN_CIPHER_SUITE_AES_CMAC:
+		if (!wl->has_aes_cmac_cipher) {
+			wl1271_error("AEC CMAC cipher not available on this firmware version\n");
+			return -EOPNOTSUPP;
+		}
 		key_type = KEY_IGTK;
 		break;
 	default:
diff --git a/drivers/net/wireless/ti/wlcore/wlcore.h b/drivers/net/wireless/ti/wlcore/wlcore.h
index b7821311ac75..26a2bd9b2df1 100644
--- a/drivers/net/wireless/ti/wlcore/wlcore.h
+++ b/drivers/net/wireless/ti/wlcore/wlcore.h
@@ -213,6 +213,8 @@ struct wl1271 {
 	void *nvs;
 	size_t nvs_len;
 
+	u32 has_aes_cmac_cipher:1;
+
 	s8 hw_pg_ver;
 
 	/* address read from the fuse ROM */
-- 
2.26.2

RE: [EXTERNAL] [PATCH v2] net: wireless: wlcore: fix support for IGTK key

[Bouganim, Raz]

Hi,

This is not a really good solution, this patch will cause a wrong status to the user:
The supplicant will give status to the user that we are connected although we are not able to encrypt a management packet in addition, the supplicant will try over and over to send the AES_CMAC key.

The right solution is the override the supported cipher that is configured when the wlcore is up:
If the version is below 8.9.0.0.83 the cipher suite configured to wl->hw->wiphy->cipher_suites
cipher_suites[] = {
        WLAN_CIPHER_SUITE_WEP40,
        WLAN_CIPHER_SUITE_WEP104,
        WLAN_CIPHER_SUITE_TKIP,
        WLAN_CIPHER_SUITE_CCMP,
        WL1271_CIPHER_SUITE_GEM,
    };


 however, I don't know if it is a good solution to decide the cipher depends on the FW version because we are not able to know what is the FW version when we are loaded the wlcore driver.

What do you think? 


-----Original Message-----
From: Mauro Carvalho Chehab [mailto:mchehab@kernel.org] On Behalf Of Mauro Carvalho Chehab
Sent: Monday, September 7, 2020 9:38 AM
To: Bouganim, Raz
Cc: linuxarm@huawei.com; mauro.chehab@huawei.com; Mauro Carvalho Chehab; John Stultz; Manivannan Sadhasivam; Kalle Valo; David S. Miller; Jakub Kicinski; Tony Lindgren; Dinghao Liu; Mishol, Guy; Hahn, Maital; Johannes Berg; Fuqian Huang; linux-wireless@vger.kernel.org; netdev@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: [EXTERNAL] [PATCH v2] net: wireless: wlcore: fix support for IGTK key

Changeset 2b7aadd3b9e1 ("wlcore: Adding suppoprt for IGTK key in wlcore driver")
added support for AEC CMAC cipher suite.

However, this only works with the very newest firmware version
(8.9.0.0.83). Such firmware weren't even pushed to linux-firmware
git tree yet:

	https://git.ti.com/cgit/wilink8-wlan/wl18xx_fw/log/

Due to that, it causes a regression betwen Kernel 5.7 and 5.8:
with such patch applied, WiFi stops working, and the Kernel starts
printing this message every second:

   wlcore: PHY firmware version: Rev 8.2.0.0.242
   wlcore: firmware booted (Rev 8.9.0.0.79)
   wlcore: ERROR command execute failure 14
   ------------[ cut here ]------------
   WARNING: CPU: 0 PID: 133 at drivers/net/wireless/ti/wlcore/main.c:795 wl12xx_queue_recovery_work.part.0+0x6c/0x74 [wlcore]
   Modules linked in: wl18xx wlcore mac80211 libarc4 cfg80211 rfkill snd_soc_hdmi_codec crct10dif_ce wlcore_sdio adv7511 cec kirin9xx_drm(C) kirin9xx_dw_drm_dsi(C) drm_kms_helper drm ip_tables x_tables ipv6 nf_defrag_ipv6
   CPU: 0 PID: 133 Comm: kworker/0:1 Tainted: G        WC        5.8.0+ #186
   Hardware name: HiKey970 (DT)
   Workqueue: events_freezable ieee80211_restart_work [mac80211]
   pstate: 60000005 (nZCv daif -PAN -UAO BTYPE=--)
   pc : wl12xx_queue_recovery_work.part.0+0x6c/0x74 [wlcore]
   lr : wl12xx_queue_recovery_work+0x24/0x30 [wlcore]
   sp : ffff8000126c3a60
   x29: ffff8000126c3a60 x28: 00000000000025de
   x27: 0000000000000010 x26: 0000000000000005
   x25: ffff0001a5d49e80 x24: ffff8000092cf580
   x23: ffff0001b7c12623 x22: ffff0001b6fcf2e8
   x21: ffff0001b7e46200 x20: 00000000fffffffb
   x19: ffff0001a78e6400 x18: 0000000000000030
   x17: 0000000000000001 x16: 0000000000000001
   x15: ffff0001b7e46670 x14: ffffffffffffffff
   x13: ffff8000926c37d7 x12: ffff8000126c37e0
   x11: ffff800011e01000 x10: ffff8000120526d0
   x9 : 0000000000000000 x8 : 3431206572756c69
   x7 : 6166206574756365 x6 : 0000000000000c2c
   x5 : 0000000000000000 x4 : ffff0001bf1361e8
   x3 : ffff0001bf1790b0 x2 : 0000000000000000
   x1 : ffff0001a5d49e80 x0 : 0000000000000001
   Call trace:
    wl12xx_queue_recovery_work.part.0+0x6c/0x74 [wlcore]
    wl12xx_queue_recovery_work+0x24/0x30 [wlcore]
    wl1271_cmd_set_sta_key+0x258/0x25c [wlcore]
    wl1271_set_key+0x7c/0x2dc [wlcore]
    wlcore_set_key+0xe4/0x360 [wlcore]
    wl18xx_set_key+0x48/0x1d0 [wl18xx]
    wlcore_op_set_key+0xa4/0x180 [wlcore]
    ieee80211_key_enable_hw_accel+0xb0/0x2d0 [mac80211]
    ieee80211_reenable_keys+0x70/0x110 [mac80211]
    ieee80211_reconfig+0xa00/0xca0 [mac80211]
    ieee80211_restart_work+0xc4/0xfc [mac80211]
    process_one_work+0x1cc/0x350
    worker_thread+0x13c/0x470
    kthread+0x154/0x160
    ret_from_fork+0x10/0x30
   ---[ end trace b1f722abf9af5919 ]---
   wlcore: WARNING could not set keys
   wlcore: ERROR Could not add or replace key
   wlan0: failed to set key (4, ff:ff:ff:ff:ff:ff) to hardware (-5)
   wlcore: Hardware recovery in progress. FW ver: Rev 8.9.0.0.79
   wlcore: pc: 0x0, hint_sts: 0x00000040 count: 39
   wlcore: down
   wlcore: down
   ieee80211 phy0: Hardware restart was requested
   mmc_host mmc0: Bus speed (slot 0) = 400000Hz (slot req 400000Hz, actual 400000HZ div = 0)
   mmc_host mmc0: Bus speed (slot 0) = 25000000Hz (slot req 25000000Hz, actual 25000000HZ div = 0)
   wlcore: PHY firmware version: Rev 8.2.0.0.242
   wlcore: firmware booted (Rev 8.9.0.0.79)
   wlcore: ERROR command execute failure 14
   ------------[ cut here ]------------

Fix it by adding some code that will check if the firmware version
is at least version 8.9.0.0.83.

Tested on Hikey 970.

Fixes: 2b7aadd3b9e1 ("wlcore: Adding suppoprt for IGTK key in wlcore driver")
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---

Hi Raz,

Maybe this patch could be useful once firmware 8.9.0.0.83 is released.

v2: change it to require minimal verson 8.9.0.0.83 for IGTK key

 drivers/net/wireless/ti/wlcore/main.c   | 48 +++++++++++++++++++++++++
 drivers/net/wireless/ti/wlcore/wlcore.h |  2 ++
 2 files changed, 50 insertions(+)

diff --git a/drivers/net/wireless/ti/wlcore/main.c b/drivers/net/wireless/ti/wlcore/main.c
index de6c8a7589ca..4332eaa1a733 100644
--- a/drivers/net/wireless/ti/wlcore/main.c
+++ b/drivers/net/wireless/ti/wlcore/main.c
@@ -14,6 +14,7 @@
 #include <linux/irq.h>
 #include <linux/pm_runtime.h>
 #include <linux/pm_wakeirq.h>
+#include <linux/ctype.h>
 
 #include "wlcore.h"
 #include "debug.h"
@@ -1082,6 +1083,45 @@ static int wl12xx_chip_wakeup(struct wl1271 *wl, bool plt)
 	return ret;
 }
 
+static bool wl1271_check_aes_cmac_cypher(struct wl1271 *wl)
+{
+	int ver[5] = { };
+	int ret;
+	const char *p = wl->chip.fw_ver_str;
+
+
+	/* The string starts with "Rev ". Ignore it */
+	while (*p && !isdigit(*p))
+		p++;
+
+	ret = sscanf(p, "%d.%d.%d.%d.%d",
+		     &ver[0], &ver[1], &ver[2], &ver[3], &ver[4]);
+
+	if (ret != ARRAY_SIZE(ver)) {
+		wl1271_info("Parsed version: %d.%d.%d.%d.%d\n",
+			    ver[0], ver[1], ver[2], ver[3], ver[4]);
+		wl1271_error("Couldn't parse firmware version string: %d\n", ret);
+		return false;
+	}
+
+	/*
+	 * Only versions equal (and probably above) 8.9.0.0.83
+	 * supports such feature.
+	 */
+	if (ver[0] < 8)
+		return false;
+	if (ver[1] < 9)
+		return false;
+	if (ver[2] > 0)
+		return true;
+	if (ver[3] > 0)
+		return true;
+	if (ver[4] >= 83)
+		return true;
+
+	return false;
+}
+
 int wl1271_plt_start(struct wl1271 *wl, const enum plt_mode plt_mode)
 {
 	int retries = WL1271_BOOT_RETRIES;
@@ -1133,6 +1173,8 @@ int wl1271_plt_start(struct wl1271 *wl, const enum plt_mode plt_mode)
 		strncpy(wiphy->fw_version, wl->chip.fw_ver_str,
 			sizeof(wiphy->fw_version));
 
+		wl->has_aes_cmac_cipher = wl1271_check_aes_cmac_cypher(wl);
+
 		goto out;
 
 power_off:
@@ -2358,6 +2400,8 @@ static int wl12xx_init_fw(struct wl1271 *wl)
 	strncpy(wiphy->fw_version, wl->chip.fw_ver_str,
 		sizeof(wiphy->fw_version));
 
+	wl->has_aes_cmac_cipher = wl1271_check_aes_cmac_cypher(wl);
+
 	/*
 	 * Now we know if 11a is supported (info from the NVS), so disable
 	 * 11a channels if not supported
@@ -3551,6 +3595,10 @@ int wlcore_set_key(struct wl1271 *wl, enum set_key_cmd cmd,
 		key_type = KEY_GEM;
 		break;
 	case WLAN_CIPHER_SUITE_AES_CMAC:
+		if (!wl->has_aes_cmac_cipher) {
+			wl1271_error("AEC CMAC cipher not available on this firmware version\n");
+			return -EOPNOTSUPP;
+		}
 		key_type = KEY_IGTK;
 		break;
 	default:
diff --git a/drivers/net/wireless/ti/wlcore/wlcore.h b/drivers/net/wireless/ti/wlcore/wlcore.h
index b7821311ac75..26a2bd9b2df1 100644
--- a/drivers/net/wireless/ti/wlcore/wlcore.h
+++ b/drivers/net/wireless/ti/wlcore/wlcore.h
@@ -213,6 +213,8 @@ struct wl1271 {
 	void *nvs;
 	size_t nvs_len;
 
+	u32 has_aes_cmac_cipher:1;
+
 	s8 hw_pg_ver;
 
 	/* address read from the fuse ROM */
-- 
2.26.2

Re: [PATCH] Revert "wlcore: Adding suppoprt for IGTK key in wlcore driver"

[Kalle Valo]

Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote:

> This patch causes a regression betwen Kernel 5.7 and 5.8 at wlcore:
> with it applied, WiFi stops working, and the Kernel starts printing
> this message every second:
> 
>    wlcore: PHY firmware version: Rev 8.2.0.0.242
>    wlcore: firmware booted (Rev 8.9.0.0.79)
>    wlcore: ERROR command execute failure 14
>    ------------[ cut here ]------------
>    WARNING: CPU: 0 PID: 133 at drivers/net/wireless/ti/wlcore/main.c:795 wl12xx_queue_recovery_work.part.0+0x6c/0x74 [wlcore]
>    Modules linked in: wl18xx wlcore mac80211 libarc4 cfg80211 rfkill snd_soc_hdmi_codec crct10dif_ce wlcore_sdio adv7511 cec kirin9xx_drm(C) kirin9xx_dw_drm_dsi(C) drm_kms_helper drm ip_tables x_tables ipv6 nf_defrag_ipv6
>    CPU: 0 PID: 133 Comm: kworker/0:1 Tainted: G        WC        5.8.0+ #186
>    Hardware name: HiKey970 (DT)
>    Workqueue: events_freezable ieee80211_restart_work [mac80211]
>    pstate: 60000005 (nZCv daif -PAN -UAO BTYPE=--)
>    pc : wl12xx_queue_recovery_work.part.0+0x6c/0x74 [wlcore]
>    lr : wl12xx_queue_recovery_work+0x24/0x30 [wlcore]
>    sp : ffff8000126c3a60
>    x29: ffff8000126c3a60 x28: 00000000000025de
>    x27: 0000000000000010 x26: 0000000000000005
>    x25: ffff0001a5d49e80 x24: ffff8000092cf580
>    x23: ffff0001b7c12623 x22: ffff0001b6fcf2e8
>    x21: ffff0001b7e46200 x20: 00000000fffffffb
>    x19: ffff0001a78e6400 x18: 0000000000000030
>    x17: 0000000000000001 x16: 0000000000000001
>    x15: ffff0001b7e46670 x14: ffffffffffffffff
>    x13: ffff8000926c37d7 x12: ffff8000126c37e0
>    x11: ffff800011e01000 x10: ffff8000120526d0
>    x9 : 0000000000000000 x8 : 3431206572756c69
>    x7 : 6166206574756365 x6 : 0000000000000c2c
>    x5 : 0000000000000000 x4 : ffff0001bf1361e8
>    x3 : ffff0001bf1790b0 x2 : 0000000000000000
>    x1 : ffff0001a5d49e80 x0 : 0000000000000001
>    Call trace:
>     wl12xx_queue_recovery_work.part.0+0x6c/0x74 [wlcore]
>     wl12xx_queue_recovery_work+0x24/0x30 [wlcore]
>     wl1271_cmd_set_sta_key+0x258/0x25c [wlcore]
>     wl1271_set_key+0x7c/0x2dc [wlcore]
>     wlcore_set_key+0xe4/0x360 [wlcore]
>     wl18xx_set_key+0x48/0x1d0 [wl18xx]
>     wlcore_op_set_key+0xa4/0x180 [wlcore]
>     ieee80211_key_enable_hw_accel+0xb0/0x2d0 [mac80211]
>     ieee80211_reenable_keys+0x70/0x110 [mac80211]
>     ieee80211_reconfig+0xa00/0xca0 [mac80211]
>     ieee80211_restart_work+0xc4/0xfc [mac80211]
>     process_one_work+0x1cc/0x350
>     worker_thread+0x13c/0x470
>     kthread+0x154/0x160
>     ret_from_fork+0x10/0x30
>    ---[ end trace b1f722abf9af5919 ]---
>    wlcore: WARNING could not set keys
>    wlcore: ERROR Could not add or replace key
>    wlan0: failed to set key (4, ff:ff:ff:ff:ff:ff) to hardware (-5)
>    wlcore: Hardware recovery in progress. FW ver: Rev 8.9.0.0.79
>    wlcore: pc: 0x0, hint_sts: 0x00000040 count: 39
>    wlcore: down
>    wlcore: down
>    ieee80211 phy0: Hardware restart was requested
>    mmc_host mmc0: Bus speed (slot 0) = 400000Hz (slot req 400000Hz, actual 400000HZ div = 0)
>    mmc_host mmc0: Bus speed (slot 0) = 25000000Hz (slot req 25000000Hz, actual 25000000HZ div = 0)
>    wlcore: PHY firmware version: Rev 8.2.0.0.242
>    wlcore: firmware booted (Rev 8.9.0.0.79)
>    wlcore: ERROR command execute failure 14
>    ------------[ cut here ]------------
> 
> Tested on Hikey 970.
> 
> This reverts commit 2b7aadd3b9e17e8b81eeb8d9cc46756ae4658265.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> Signed-off-by: Raz Bouganim <r-bouganim@ti.com>

I'm going to apply this revert now and queue for v5.9. The feature can be added
again for v5.10 with proper support for older firmware versions.

-- 
https://patchwork.kernel.org/patch/11737193/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: Revert "wlcore: Adding suppoprt for IGTK key in wlcore driver"

[Kalle Valo]

Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote:

> This patch causes a regression betwen Kernel 5.7 and 5.8 at wlcore:
> with it applied, WiFi stops working, and the Kernel starts printing
> this message every second:
> 
>    wlcore: PHY firmware version: Rev 8.2.0.0.242
>    wlcore: firmware booted (Rev 8.9.0.0.79)
>    wlcore: ERROR command execute failure 14
>    ------------[ cut here ]------------
>    WARNING: CPU: 0 PID: 133 at drivers/net/wireless/ti/wlcore/main.c:795 wl12xx_queue_recovery_work.part.0+0x6c/0x74 [wlcore]
>    Modules linked in: wl18xx wlcore mac80211 libarc4 cfg80211 rfkill snd_soc_hdmi_codec crct10dif_ce wlcore_sdio adv7511 cec kirin9xx_drm(C) kirin9xx_dw_drm_dsi(C) drm_kms_helper drm ip_tables x_tables ipv6 nf_defrag_ipv6
>    CPU: 0 PID: 133 Comm: kworker/0:1 Tainted: G        WC        5.8.0+ #186
>    Hardware name: HiKey970 (DT)
>    Workqueue: events_freezable ieee80211_restart_work [mac80211]
>    pstate: 60000005 (nZCv daif -PAN -UAO BTYPE=--)
>    pc : wl12xx_queue_recovery_work.part.0+0x6c/0x74 [wlcore]
>    lr : wl12xx_queue_recovery_work+0x24/0x30 [wlcore]
>    sp : ffff8000126c3a60
>    x29: ffff8000126c3a60 x28: 00000000000025de
>    x27: 0000000000000010 x26: 0000000000000005
>    x25: ffff0001a5d49e80 x24: ffff8000092cf580
>    x23: ffff0001b7c12623 x22: ffff0001b6fcf2e8
>    x21: ffff0001b7e46200 x20: 00000000fffffffb
>    x19: ffff0001a78e6400 x18: 0000000000000030
>    x17: 0000000000000001 x16: 0000000000000001
>    x15: ffff0001b7e46670 x14: ffffffffffffffff
>    x13: ffff8000926c37d7 x12: ffff8000126c37e0
>    x11: ffff800011e01000 x10: ffff8000120526d0
>    x9 : 0000000000000000 x8 : 3431206572756c69
>    x7 : 6166206574756365 x6 : 0000000000000c2c
>    x5 : 0000000000000000 x4 : ffff0001bf1361e8
>    x3 : ffff0001bf1790b0 x2 : 0000000000000000
>    x1 : ffff0001a5d49e80 x0 : 0000000000000001
>    Call trace:
>     wl12xx_queue_recovery_work.part.0+0x6c/0x74 [wlcore]
>     wl12xx_queue_recovery_work+0x24/0x30 [wlcore]
>     wl1271_cmd_set_sta_key+0x258/0x25c [wlcore]
>     wl1271_set_key+0x7c/0x2dc [wlcore]
>     wlcore_set_key+0xe4/0x360 [wlcore]
>     wl18xx_set_key+0x48/0x1d0 [wl18xx]
>     wlcore_op_set_key+0xa4/0x180 [wlcore]
>     ieee80211_key_enable_hw_accel+0xb0/0x2d0 [mac80211]
>     ieee80211_reenable_keys+0x70/0x110 [mac80211]
>     ieee80211_reconfig+0xa00/0xca0 [mac80211]
>     ieee80211_restart_work+0xc4/0xfc [mac80211]
>     process_one_work+0x1cc/0x350
>     worker_thread+0x13c/0x470
>     kthread+0x154/0x160
>     ret_from_fork+0x10/0x30
>    ---[ end trace b1f722abf9af5919 ]---
>    wlcore: WARNING could not set keys
>    wlcore: ERROR Could not add or replace key
>    wlan0: failed to set key (4, ff:ff:ff:ff:ff:ff) to hardware (-5)
>    wlcore: Hardware recovery in progress. FW ver: Rev 8.9.0.0.79
>    wlcore: pc: 0x0, hint_sts: 0x00000040 count: 39
>    wlcore: down
>    wlcore: down
>    ieee80211 phy0: Hardware restart was requested
>    mmc_host mmc0: Bus speed (slot 0) = 400000Hz (slot req 400000Hz, actual 400000HZ div = 0)
>    mmc_host mmc0: Bus speed (slot 0) = 25000000Hz (slot req 25000000Hz, actual 25000000HZ div = 0)
>    wlcore: PHY firmware version: Rev 8.2.0.0.242
>    wlcore: firmware booted (Rev 8.9.0.0.79)
>    wlcore: ERROR command execute failure 14
>    ------------[ cut here ]------------
> 
> Tested on Hikey 970.
> 
> This reverts commit 2b7aadd3b9e17e8b81eeb8d9cc46756ae4658265.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

Patch applied to wireless-drivers.git, thanks.

1264c1e0cfe5 Revert "wlcore: Adding suppoprt for IGTK key in wlcore driver"

-- 
https://patchwork.kernel.org/patch/11737193/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches