2016-11-01 12:08:36

by Xinming Hu

[permalink] [raw]
Subject: [PATCH v2 01/12] mwifiex: check tx_hw_pending before downloading sleep confirm

From: Shengzhen Li <[email protected]>

We may get SLEEP event from firmware even if TXDone interrupt
for last Tx packet is still pending. In this case, we may
end up accessing PCIe memory for handling TXDone after power
save handshake is completed. This causes kernel crash with
external abort.

This patch will only allow downloading sleep confirm
when no tx done interrupt is pending in the hardware.

---
v2: address format issues(Brain)
---
Signed-off-by: Cathy Luo <[email protected]>
Signed-off-by: Shengzhen Li <[email protected]>
Signed-off-by: Xinming Hu <[email protected]>
Signed-off-by: Amitkumar Karwar <[email protected]>
---
drivers/net/wireless/marvell/mwifiex/cmdevt.c | 5 +++--
drivers/net/wireless/marvell/mwifiex/init.c | 1 +
drivers/net/wireless/marvell/mwifiex/main.h | 1 +
drivers/net/wireless/marvell/mwifiex/pcie.c | 5 +++++
4 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/cmdevt.c b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
index 5347728..25a7475 100644
--- a/drivers/net/wireless/marvell/mwifiex/cmdevt.c
+++ b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
@@ -1118,13 +1118,14 @@ mwifiex_cancel_pending_ioctl(struct mwifiex_adapter *adapter)
void
mwifiex_check_ps_cond(struct mwifiex_adapter *adapter)
{
- if (!adapter->cmd_sent &&
+ if (!adapter->cmd_sent && !atomic_read(&adapter->tx_hw_pending) &&
!adapter->curr_cmd && !IS_CARD_RX_RCVD(adapter))
mwifiex_dnld_sleep_confirm_cmd(adapter);
else
mwifiex_dbg(adapter, CMD,
- "cmd: Delay Sleep Confirm (%s%s%s)\n",
+ "cmd: Delay Sleep Confirm (%s%s%s%s)\n",
(adapter->cmd_sent) ? "D" : "",
+ atomic_read(&adapter->tx_hw_pending) ? "T" : "",
(adapter->curr_cmd) ? "C" : "",
(IS_CARD_RX_RCVD(adapter)) ? "R" : "");
}
diff --git a/drivers/net/wireless/marvell/mwifiex/init.c b/drivers/net/wireless/marvell/mwifiex/init.c
index 82839d9..b36cb3f 100644
--- a/drivers/net/wireless/marvell/mwifiex/init.c
+++ b/drivers/net/wireless/marvell/mwifiex/init.c
@@ -270,6 +270,7 @@ static void mwifiex_init_adapter(struct mwifiex_adapter *adapter)
adapter->adhoc_11n_enabled = false;

mwifiex_wmm_init(adapter);
+ atomic_set(&adapter->tx_hw_pending, 0);

sleep_cfm_buf = (struct mwifiex_opt_sleep_confirm *)
adapter->sleep_cfm->data;
diff --git a/drivers/net/wireless/marvell/mwifiex/main.h b/drivers/net/wireless/marvell/mwifiex/main.h
index d61fe3a..7f67f23 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.h
+++ b/drivers/net/wireless/marvell/mwifiex/main.h
@@ -857,6 +857,7 @@ struct mwifiex_adapter {
atomic_t rx_pending;
atomic_t tx_pending;
atomic_t cmd_pending;
+ atomic_t tx_hw_pending;
struct workqueue_struct *workqueue;
struct work_struct main_work;
struct workqueue_struct *rx_workqueue;
diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c b/drivers/net/wireless/marvell/mwifiex/pcie.c
index 063c707..4aa5d91 100644
--- a/drivers/net/wireless/marvell/mwifiex/pcie.c
+++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
@@ -516,6 +516,7 @@ static int mwifiex_pcie_enable_host_int(struct mwifiex_adapter *adapter)
}
}

+ atomic_set(&adapter->tx_hw_pending, 0);
return 0;
}

@@ -689,6 +690,7 @@ static void mwifiex_cleanup_txq_ring(struct mwifiex_adapter *adapter)
card->tx_buf_list[i] = NULL;
}

+ atomic_set(&adapter->tx_hw_pending, 0);
return;
}

@@ -1126,6 +1128,7 @@ static int mwifiex_pcie_send_data_complete(struct mwifiex_adapter *adapter)
-1);
else
mwifiex_write_data_complete(adapter, skb, 0, 0);
+ atomic_dec(&adapter->tx_hw_pending);
}

card->tx_buf_list[wrdoneidx] = NULL;
@@ -1218,6 +1221,7 @@ mwifiex_pcie_send_data(struct mwifiex_adapter *adapter, struct sk_buff *skb,
wrindx = (card->txbd_wrptr & reg->tx_mask) >> reg->tx_start_ptr;
buf_pa = MWIFIEX_SKB_DMA_ADDR(skb);
card->tx_buf_list[wrindx] = skb;
+ atomic_inc(&adapter->tx_hw_pending);

if (reg->pfu_enabled) {
desc2 = card->txbd_ring[wrindx];
@@ -1295,6 +1299,7 @@ mwifiex_pcie_send_data(struct mwifiex_adapter *adapter, struct sk_buff *skb,
done_unmap:
mwifiex_unmap_pci_memory(adapter, skb, PCI_DMA_TODEVICE);
card->tx_buf_list[wrindx] = NULL;
+ atomic_dec(&adapter->tx_hw_pending);
if (reg->pfu_enabled)
memset(desc2, 0, sizeof(*desc2));
else
--
1.8.1.4


2016-11-01 12:08:39

by Xinming Hu

[permalink] [raw]
Subject: [PATCH v2 02/12] mwifiex: complete blocked power save handshake in main process

From: Shengzhen Li <[email protected]>

Power save handshake with firmware might be blocked by on-going
data transfer.
this patch check the PS status in main process and complete
previous blocked PS handshake.
this patch also remove redudant check before call
mwifiex_check_ps_cond fuction.

---
v2: 1. remove redudant check(Brain)
3. reorgnized to follow tx_hw_pending patch
---
Signed-off-by: Cathy Luo <[email protected]>
Signed-off-by: Shengzhen Li <[email protected]>
Signed-off-by: Xinming Hu <[email protected]>
Signed-off-by: Amitkumar Karwar <[email protected]>
---
drivers/net/wireless/marvell/mwifiex/main.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/main.c b/drivers/net/wireless/marvell/mwifiex/main.c
index 2478ccd..d700c44 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.c
+++ b/drivers/net/wireless/marvell/mwifiex/main.c
@@ -308,6 +308,9 @@ process_start:
/* We have tried to wakeup the card already */
if (adapter->pm_wakeup_fw_try)
break;
+ if (adapter->ps_state == PS_STATE_PRE_SLEEP)
+ mwifiex_check_ps_cond(adapter);
+
if (adapter->ps_state != PS_STATE_AWAKE)
break;
if (adapter->tx_lock_flag) {
@@ -355,10 +358,8 @@ process_start:

/* Check if we need to confirm Sleep Request
received previously */
- if (adapter->ps_state == PS_STATE_PRE_SLEEP) {
- if (!adapter->cmd_sent && !adapter->curr_cmd)
- mwifiex_check_ps_cond(adapter);
- }
+ if (adapter->ps_state == PS_STATE_PRE_SLEEP)
+ mwifiex_check_ps_cond(adapter);

/* * The ps_state may have been changed during processing of
* Sleep Request event.
--
1.8.1.4

2016-11-08 11:06:55

by Xinming Hu

[permalink] [raw]
Subject: RE: [PATCH v2 01/12] mwifiex: check tx_hw_pending before downloading sleep confirm

SGkNCg0KPiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiBGcm9tOiBCcmlhbiBOb3JyaXMg
W21haWx0bzpicmlhbm5vcnJpc0BjaHJvbWl1bS5vcmddDQo+IFNlbnQ6IDIwMTbE6jEx1MI0yNUg
MDoxMg0KPiBUbzogWGlubWluZyBIdQ0KPiBDYzogTGludXggV2lyZWxlc3M7IEthbGxlIFZhbG87
IERtaXRyeSBUb3Jva2hvdjsgQW1pdGt1bWFyIEthcndhcjsgQ2F0aHkgTHVvOw0KPiBTaGVuZ3po
ZW4gTGk7IFhpbm1pbmcgSHU7IFdlaS1OaW5nIEh1YW5nDQo+IFN1YmplY3Q6IFJlOiBbUEFUQ0gg
djIgMDEvMTJdIG13aWZpZXg6IGNoZWNrIHR4X2h3X3BlbmRpbmcgYmVmb3JlDQo+IGRvd25sb2Fk
aW5nIHNsZWVwIGNvbmZpcm0NCj4gDQo+ICsgV2VpLU5pbmcNCj4gDQo+IE9uIFR1ZSwgTm92IDAx
LCAyMDE2IGF0IDA4OjA4OjE3UE0gKzA4MDAsIFhpbm1pbmcgSHUgd3JvdGU6DQo+ID4gRnJvbTog
U2hlbmd6aGVuIExpIDxzemxpQG1hcnZlbGwuY29tPg0KPiA+DQo+ID4gV2UgbWF5IGdldCBTTEVF
UCBldmVudCBmcm9tIGZpcm13YXJlIGV2ZW4gaWYgVFhEb25lIGludGVycnVwdCBmb3IgbGFzdA0K
PiA+IFR4IHBhY2tldCBpcyBzdGlsbCBwZW5kaW5nLiBJbiB0aGlzIGNhc2UsIHdlIG1heSBlbmQg
dXAgYWNjZXNzaW5nIFBDSWUNCj4gPiBtZW1vcnkgZm9yIGhhbmRsaW5nIFRYRG9uZSBhZnRlciBw
b3dlciBzYXZlIGhhbmRzaGFrZSBpcyBjb21wbGV0ZWQuDQo+ID4gVGhpcyBjYXVzZXMga2VybmVs
IGNyYXNoIHdpdGggZXh0ZXJuYWwgYWJvcnQuDQo+ID4NCj4gPiBUaGlzIHBhdGNoIHdpbGwgb25s
eSBhbGxvdyBkb3dubG9hZGluZyBzbGVlcCBjb25maXJtIHdoZW4gbm8gdHggZG9uZQ0KPiA+IGlu
dGVycnVwdCBpcyBwZW5kaW5nIGluIHRoZSBoYXJkd2FyZS4NCj4gPg0KPiA+IC0tLQ0KPiA+IHYy
OiBhZGRyZXNzIGZvcm1hdCBpc3N1ZXMoQnJhaW4pDQo+ID4gLS0tDQo+IA0KPiBOaXQ6IHR5cGlj
YWxseSwgaXQncyBiZXN0IGlmIHRoZSBjaGFuZ2Vsb2cgaXMgcGxhY2VkIGFmdGVyIHRoZSBTaWdu
LW9mZnMsIHNvIHRoYXQgdGhlDQo+IGZpcnN0ICItLS0iIGxpbmUgZGVub3RlcyB0aGUgYmVnaW5u
aW5nIG9mIHRleHQgdGhhdCBjYW4gYmUgZGlzY2FyZGVkLiAoVHlwaWNhbGx5DQo+IHRoZSBjaGFu
Z2Vsb2cgZG9lc24ndCBnbyBpbnRvIHRoZSBmaW5hbCBnaXQgY29tbWl0LCBhbmQgaXQgYWxzbyBo
YXBwZW5zIHRoYXQNCj4gZ2l0LWFtIGRpc2NhcmRzIGV2ZXJ5dGhpbmcgYmV0d2VlbiB0aGUgZmly
c3QgIi0tLSIgbGluZSBhbmQgdGhlIGFjdHVhbCBwYXRjaA0KPiBjb250ZW50LikNCj4gDQoNCk9r
LCB3ZSB3aWxsIGFkZHJlc3MgdGhpcyBpbiB1cGRhdGVkIHZlcnNpb24uDQoNCj4gPiBTaWduZWQt
b2ZmLWJ5OiBDYXRoeSBMdW8gPGNsdW9AbWFydmVsbC5jb20+DQo+ID4gU2lnbmVkLW9mZi1ieTog
U2hlbmd6aGVuIExpIDxzemxpQG1hcnZlbGwuY29tPg0KPiA+IFNpZ25lZC1vZmYtYnk6IFhpbm1p
bmcgSHUgPGh1eG1AbWFydmVsbC5jb20+DQo+ID4gU2lnbmVkLW9mZi1ieTogQW1pdGt1bWFyIEth
cndhciA8YWthcndhckBtYXJ2ZWxsLmNvbT4NCj4gPiAtLS0NCj4gPiAgZHJpdmVycy9uZXQvd2ly
ZWxlc3MvbWFydmVsbC9td2lmaWV4L2NtZGV2dC5jIHwgNSArKystLQ0KPiA+ICBkcml2ZXJzL25l
dC93aXJlbGVzcy9tYXJ2ZWxsL213aWZpZXgvaW5pdC5jICAgfCAxICsNCj4gPiAgZHJpdmVycy9u
ZXQvd2lyZWxlc3MvbWFydmVsbC9td2lmaWV4L21haW4uaCAgIHwgMSArDQo+ID4gIGRyaXZlcnMv
bmV0L3dpcmVsZXNzL21hcnZlbGwvbXdpZmlleC9wY2llLmMgICB8IDUgKysrKysNCj4gPiAgNCBm
aWxlcyBjaGFuZ2VkLCAxMCBpbnNlcnRpb25zKCspLCAyIGRlbGV0aW9ucygtKQ0KPiANCj4gT3Zl
cmFsbCwgSSB0aGluayB0aGlzIGNoYW5nZSBpcyBnb29kLCBhbmQgaXQgdGVzdHMgb3V0IGZpbmUg
Zm9yIG1lIHNvIGZhci4gRG8gd2UNCj4gaGF2ZSB0aGUgc2FtZSBwcm9ibGVtIGZvciBvdGhlciBp
bnRlcmZhY2VzIHRvbz8gZS5nLiwgU0RJTz8NCj4gSXQgc2VlbXMgdG8gbWUgdGhhdCB0aGUgcm9v
dCBwcm9ibGVtIGlzIGdlbmVyaWMgKGkuZS4sIGRvbid0IHRyeSB0byBTTEVFUCB3aGlsZQ0KPiBw
cm9jZXNzaW5nIFRYIHRyYWZmaWMpIGJ1dCB0aGUgc3ltcHRvbSBqdXN0IGhhcHBlbmVkIHRvIGJl
IGZhdGFsIG9uIGENCj4gcGFydGljdWxhciBQQ0llIGNvbnRyb2xsZXIuDQo+IA0KDQpGb3IgU0RJ
TyBpbnRlcmZhY2UsIGNvbW1hbmQgNTMgd3JpdGUgcmV0dXJuIGluZGljYXRlIHR4IGNvbXBsZXRl
IG9uIHRoZSBidXMsIG5vIGFueSBwZW5kaW5nIHBhY2tldHMgaW4gaGFyZHdhcmUgdGhlbi4NClNv
LCBTRElPIGludGVyZmFjZSBkb24ndCBoYXZlIHRoZSBzYW1lIGlzc3VlIGFzIFBDSUUuIA0KDQo+
IEJyaWFuDQo+IA0KPiA+IGRpZmYgLS1naXQgYS9kcml2ZXJzL25ldC93aXJlbGVzcy9tYXJ2ZWxs
L213aWZpZXgvY21kZXZ0LmMNCj4gPiBiL2RyaXZlcnMvbmV0L3dpcmVsZXNzL21hcnZlbGwvbXdp
ZmlleC9jbWRldnQuYw0KPiA+IGluZGV4IDUzNDc3MjguLjI1YTc0NzUgMTAwNjQ0DQo+ID4gLS0t
IGEvZHJpdmVycy9uZXQvd2lyZWxlc3MvbWFydmVsbC9td2lmaWV4L2NtZGV2dC5jDQo+ID4gKysr
IGIvZHJpdmVycy9uZXQvd2lyZWxlc3MvbWFydmVsbC9td2lmaWV4L2NtZGV2dC5jDQo+ID4gQEAg
LTExMTgsMTMgKzExMTgsMTQgQEAgbXdpZmlleF9jYW5jZWxfcGVuZGluZ19pb2N0bChzdHJ1Y3QN
Cj4gPiBtd2lmaWV4X2FkYXB0ZXIgKmFkYXB0ZXIpICB2b2lkICBtd2lmaWV4X2NoZWNrX3BzX2Nv
bmQoc3RydWN0DQo+ID4gbXdpZmlleF9hZGFwdGVyICphZGFwdGVyKSAgew0KPiA+IC0JaWYgKCFh
ZGFwdGVyLT5jbWRfc2VudCAmJg0KPiA+ICsJaWYgKCFhZGFwdGVyLT5jbWRfc2VudCAmJiAhYXRv
bWljX3JlYWQoJmFkYXB0ZXItPnR4X2h3X3BlbmRpbmcpICYmDQo+ID4gIAkgICAgIWFkYXB0ZXIt
PmN1cnJfY21kICYmICFJU19DQVJEX1JYX1JDVkQoYWRhcHRlcikpDQo+ID4gIAkJbXdpZmlleF9k
bmxkX3NsZWVwX2NvbmZpcm1fY21kKGFkYXB0ZXIpOw0KPiA+ICAJZWxzZQ0KPiA+ICAJCW13aWZp
ZXhfZGJnKGFkYXB0ZXIsIENNRCwNCj4gPiAtCQkJICAgICJjbWQ6IERlbGF5IFNsZWVwIENvbmZp
cm0gKCVzJXMlcylcbiIsDQo+ID4gKwkJCSAgICAiY21kOiBEZWxheSBTbGVlcCBDb25maXJtICgl
cyVzJXMlcylcbiIsDQo+ID4gIAkJCSAgICAoYWRhcHRlci0+Y21kX3NlbnQpID8gIkQiIDogIiIs
DQo+ID4gKwkJCSAgICBhdG9taWNfcmVhZCgmYWRhcHRlci0+dHhfaHdfcGVuZGluZykgPyAiVCIg
OiAiIiwNCj4gPiAgCQkJICAgIChhZGFwdGVyLT5jdXJyX2NtZCkgPyAiQyIgOiAiIiwNCj4gPiAg
CQkJICAgIChJU19DQVJEX1JYX1JDVkQoYWRhcHRlcikpID8gIlIiIDogIiIpOyAgfSBkaWZmIC0t
Z2l0DQo+ID4gYS9kcml2ZXJzL25ldC93aXJlbGVzcy9tYXJ2ZWxsL213aWZpZXgvaW5pdC5jDQo+
ID4gYi9kcml2ZXJzL25ldC93aXJlbGVzcy9tYXJ2ZWxsL213aWZpZXgvaW5pdC5jDQo+ID4gaW5k
ZXggODI4MzlkOS4uYjM2Y2IzZiAxMDA2NDQNCj4gPiAtLS0gYS9kcml2ZXJzL25ldC93aXJlbGVz
cy9tYXJ2ZWxsL213aWZpZXgvaW5pdC5jDQo+ID4gKysrIGIvZHJpdmVycy9uZXQvd2lyZWxlc3Mv
bWFydmVsbC9td2lmaWV4L2luaXQuYw0KPiA+IEBAIC0yNzAsNiArMjcwLDcgQEAgc3RhdGljIHZv
aWQgbXdpZmlleF9pbml0X2FkYXB0ZXIoc3RydWN0DQo+IG13aWZpZXhfYWRhcHRlciAqYWRhcHRl
cikNCj4gPiAgCWFkYXB0ZXItPmFkaG9jXzExbl9lbmFibGVkID0gZmFsc2U7DQo+ID4NCj4gPiAg
CW13aWZpZXhfd21tX2luaXQoYWRhcHRlcik7DQo+ID4gKwlhdG9taWNfc2V0KCZhZGFwdGVyLT50
eF9od19wZW5kaW5nLCAwKTsNCj4gPg0KPiA+ICAJc2xlZXBfY2ZtX2J1ZiA9IChzdHJ1Y3QgbXdp
ZmlleF9vcHRfc2xlZXBfY29uZmlybSAqKQ0KPiA+ICAJCQkJCWFkYXB0ZXItPnNsZWVwX2NmbS0+
ZGF0YTsNCj4gPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9uZXQvd2lyZWxlc3MvbWFydmVsbC9td2lm
aWV4L21haW4uaA0KPiA+IGIvZHJpdmVycy9uZXQvd2lyZWxlc3MvbWFydmVsbC9td2lmaWV4L21h
aW4uaA0KPiA+IGluZGV4IGQ2MWZlM2EuLjdmNjdmMjMgMTAwNjQ0DQo+ID4gLS0tIGEvZHJpdmVy
cy9uZXQvd2lyZWxlc3MvbWFydmVsbC9td2lmaWV4L21haW4uaA0KPiA+ICsrKyBiL2RyaXZlcnMv
bmV0L3dpcmVsZXNzL21hcnZlbGwvbXdpZmlleC9tYWluLmgNCj4gPiBAQCAtODU3LDYgKzg1Nyw3
IEBAIHN0cnVjdCBtd2lmaWV4X2FkYXB0ZXIgew0KPiA+ICAJYXRvbWljX3QgcnhfcGVuZGluZzsN
Cj4gPiAgCWF0b21pY190IHR4X3BlbmRpbmc7DQo+ID4gIAlhdG9taWNfdCBjbWRfcGVuZGluZzsN
Cj4gPiArCWF0b21pY190IHR4X2h3X3BlbmRpbmc7DQo+ID4gIAlzdHJ1Y3Qgd29ya3F1ZXVlX3N0
cnVjdCAqd29ya3F1ZXVlOw0KPiA+ICAJc3RydWN0IHdvcmtfc3RydWN0IG1haW5fd29yazsNCj4g
PiAgCXN0cnVjdCB3b3JrcXVldWVfc3RydWN0ICpyeF93b3JrcXVldWU7IGRpZmYgLS1naXQNCj4g
PiBhL2RyaXZlcnMvbmV0L3dpcmVsZXNzL21hcnZlbGwvbXdpZmlleC9wY2llLmMNCj4gPiBiL2Ry
aXZlcnMvbmV0L3dpcmVsZXNzL21hcnZlbGwvbXdpZmlleC9wY2llLmMNCj4gPiBpbmRleCAwNjNj
NzA3Li40YWE1ZDkxIDEwMDY0NA0KPiA+IC0tLSBhL2RyaXZlcnMvbmV0L3dpcmVsZXNzL21hcnZl
bGwvbXdpZmlleC9wY2llLmMNCj4gPiArKysgYi9kcml2ZXJzL25ldC93aXJlbGVzcy9tYXJ2ZWxs
L213aWZpZXgvcGNpZS5jDQo+ID4gQEAgLTUxNiw2ICs1MTYsNyBAQCBzdGF0aWMgaW50IG13aWZp
ZXhfcGNpZV9lbmFibGVfaG9zdF9pbnQoc3RydWN0DQo+IG13aWZpZXhfYWRhcHRlciAqYWRhcHRl
cikNCj4gPiAgCQl9DQo+ID4gIAl9DQo+ID4NCj4gPiArCWF0b21pY19zZXQoJmFkYXB0ZXItPnR4
X2h3X3BlbmRpbmcsIDApOw0KPiA+ICAJcmV0dXJuIDA7DQo+ID4gIH0NCj4gPg0KPiA+IEBAIC02
ODksNiArNjkwLDcgQEAgc3RhdGljIHZvaWQgbXdpZmlleF9jbGVhbnVwX3R4cV9yaW5nKHN0cnVj
dA0KPiBtd2lmaWV4X2FkYXB0ZXIgKmFkYXB0ZXIpDQo+ID4gIAkJY2FyZC0+dHhfYnVmX2xpc3Rb
aV0gPSBOVUxMOw0KPiA+ICAJfQ0KPiA+DQo+ID4gKwlhdG9taWNfc2V0KCZhZGFwdGVyLT50eF9o
d19wZW5kaW5nLCAwKTsNCj4gPiAgCXJldHVybjsNCj4gPiAgfQ0KPiA+DQo+ID4gQEAgLTExMjYs
NiArMTEyOCw3IEBAIHN0YXRpYyBpbnQgbXdpZmlleF9wY2llX3NlbmRfZGF0YV9jb21wbGV0ZShz
dHJ1Y3QNCj4gbXdpZmlleF9hZGFwdGVyICphZGFwdGVyKQ0KPiA+ICAJCQkJCQkJICAgIC0xKTsN
Cj4gPiAgCQkJZWxzZQ0KPiA+ICAJCQkJbXdpZmlleF93cml0ZV9kYXRhX2NvbXBsZXRlKGFkYXB0
ZXIsIHNrYiwgMCwgMCk7DQo+ID4gKwkJCWF0b21pY19kZWMoJmFkYXB0ZXItPnR4X2h3X3BlbmRp
bmcpOw0KPiA+ICAJCX0NCj4gPg0KPiA+ICAJCWNhcmQtPnR4X2J1Zl9saXN0W3dyZG9uZWlkeF0g
PSBOVUxMOyBAQCAtMTIxOCw2ICsxMjIxLDcgQEANCj4gPiBtd2lmaWV4X3BjaWVfc2VuZF9kYXRh
KHN0cnVjdCBtd2lmaWV4X2FkYXB0ZXIgKmFkYXB0ZXIsIHN0cnVjdCBza19idWZmDQo+ICpza2Is
DQo+ID4gIAkJd3JpbmR4ID0gKGNhcmQtPnR4YmRfd3JwdHIgJiByZWctPnR4X21hc2spID4+IHJl
Zy0+dHhfc3RhcnRfcHRyOw0KPiA+ICAJCWJ1Zl9wYSA9IE1XSUZJRVhfU0tCX0RNQV9BRERSKHNr
Yik7DQo+ID4gIAkJY2FyZC0+dHhfYnVmX2xpc3Rbd3JpbmR4XSA9IHNrYjsNCj4gPiArCQlhdG9t
aWNfaW5jKCZhZGFwdGVyLT50eF9od19wZW5kaW5nKTsNCj4gPg0KPiA+ICAJCWlmIChyZWctPnBm
dV9lbmFibGVkKSB7DQo+ID4gIAkJCWRlc2MyID0gY2FyZC0+dHhiZF9yaW5nW3dyaW5keF07DQo+
ID4gQEAgLTEyOTUsNiArMTI5OSw3IEBAIG13aWZpZXhfcGNpZV9zZW5kX2RhdGEoc3RydWN0IG13
aWZpZXhfYWRhcHRlcg0KPiA+ICphZGFwdGVyLCBzdHJ1Y3Qgc2tfYnVmZiAqc2tiLA0KPiA+ICBk
b25lX3VubWFwOg0KPiA+ICAJbXdpZmlleF91bm1hcF9wY2lfbWVtb3J5KGFkYXB0ZXIsIHNrYiwg
UENJX0RNQV9UT0RFVklDRSk7DQo+ID4gIAljYXJkLT50eF9idWZfbGlzdFt3cmluZHhdID0gTlVM
TDsNCj4gPiArCWF0b21pY19kZWMoJmFkYXB0ZXItPnR4X2h3X3BlbmRpbmcpOw0KPiA+ICAJaWYg
KHJlZy0+cGZ1X2VuYWJsZWQpDQo+ID4gIAkJbWVtc2V0KGRlc2MyLCAwLCBzaXplb2YoKmRlc2My
KSk7DQo+ID4gIAllbHNlDQo+ID4gLS0NCj4gPiAxLjguMS40DQo+ID4NCg==

2016-11-03 16:11:58

by Brian Norris

[permalink] [raw]
Subject: Re: [PATCH v2 01/12] mwifiex: check tx_hw_pending before downloading sleep confirm

+ Wei-Ning

On Tue, Nov 01, 2016 at 08:08:17PM +0800, Xinming Hu wrote:
> From: Shengzhen Li <[email protected]>
>
> We may get SLEEP event from firmware even if TXDone interrupt
> for last Tx packet is still pending. In this case, we may
> end up accessing PCIe memory for handling TXDone after power
> save handshake is completed. This causes kernel crash with
> external abort.
>
> This patch will only allow downloading sleep confirm
> when no tx done interrupt is pending in the hardware.
>
> ---
> v2: address format issues(Brain)
> ---

Nit: typically, it's best if the changelog is placed after the
Sign-offs, so that the first "---" line denotes the beginning of text
that can be discarded. (Typically the changelog doesn't go into the
final git commit, and it also happens that git-am discards everything
between the first "---" line and the actual patch content.)

> Signed-off-by: Cathy Luo <[email protected]>
> Signed-off-by: Shengzhen Li <[email protected]>
> Signed-off-by: Xinming Hu <[email protected]>
> Signed-off-by: Amitkumar Karwar <[email protected]>
> ---
> drivers/net/wireless/marvell/mwifiex/cmdevt.c | 5 +++--
> drivers/net/wireless/marvell/mwifiex/init.c | 1 +
> drivers/net/wireless/marvell/mwifiex/main.h | 1 +
> drivers/net/wireless/marvell/mwifiex/pcie.c | 5 +++++
> 4 files changed, 10 insertions(+), 2 deletions(-)

Overall, I think this change is good, and it tests out fine for me so
far. Do we have the same problem for other interfaces too? e.g., SDIO?
It seems to me that the root problem is generic (i.e., don't try to
SLEEP while processing TX traffic) but the symptom just happened to be
fatal on a particular PCIe controller.

Brian

> diff --git a/drivers/net/wireless/marvell/mwifiex/cmdevt.c b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
> index 5347728..25a7475 100644
> --- a/drivers/net/wireless/marvell/mwifiex/cmdevt.c
> +++ b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
> @@ -1118,13 +1118,14 @@ mwifiex_cancel_pending_ioctl(struct mwifiex_adapter *adapter)
> void
> mwifiex_check_ps_cond(struct mwifiex_adapter *adapter)
> {
> - if (!adapter->cmd_sent &&
> + if (!adapter->cmd_sent && !atomic_read(&adapter->tx_hw_pending) &&
> !adapter->curr_cmd && !IS_CARD_RX_RCVD(adapter))
> mwifiex_dnld_sleep_confirm_cmd(adapter);
> else
> mwifiex_dbg(adapter, CMD,
> - "cmd: Delay Sleep Confirm (%s%s%s)\n",
> + "cmd: Delay Sleep Confirm (%s%s%s%s)\n",
> (adapter->cmd_sent) ? "D" : "",
> + atomic_read(&adapter->tx_hw_pending) ? "T" : "",
> (adapter->curr_cmd) ? "C" : "",
> (IS_CARD_RX_RCVD(adapter)) ? "R" : "");
> }
> diff --git a/drivers/net/wireless/marvell/mwifiex/init.c b/drivers/net/wireless/marvell/mwifiex/init.c
> index 82839d9..b36cb3f 100644
> --- a/drivers/net/wireless/marvell/mwifiex/init.c
> +++ b/drivers/net/wireless/marvell/mwifiex/init.c
> @@ -270,6 +270,7 @@ static void mwifiex_init_adapter(struct mwifiex_adapter *adapter)
> adapter->adhoc_11n_enabled = false;
>
> mwifiex_wmm_init(adapter);
> + atomic_set(&adapter->tx_hw_pending, 0);
>
> sleep_cfm_buf = (struct mwifiex_opt_sleep_confirm *)
> adapter->sleep_cfm->data;
> diff --git a/drivers/net/wireless/marvell/mwifiex/main.h b/drivers/net/wireless/marvell/mwifiex/main.h
> index d61fe3a..7f67f23 100644
> --- a/drivers/net/wireless/marvell/mwifiex/main.h
> +++ b/drivers/net/wireless/marvell/mwifiex/main.h
> @@ -857,6 +857,7 @@ struct mwifiex_adapter {
> atomic_t rx_pending;
> atomic_t tx_pending;
> atomic_t cmd_pending;
> + atomic_t tx_hw_pending;
> struct workqueue_struct *workqueue;
> struct work_struct main_work;
> struct workqueue_struct *rx_workqueue;
> diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c b/drivers/net/wireless/marvell/mwifiex/pcie.c
> index 063c707..4aa5d91 100644
> --- a/drivers/net/wireless/marvell/mwifiex/pcie.c
> +++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
> @@ -516,6 +516,7 @@ static int mwifiex_pcie_enable_host_int(struct mwifiex_adapter *adapter)
> }
> }
>
> + atomic_set(&adapter->tx_hw_pending, 0);
> return 0;
> }
>
> @@ -689,6 +690,7 @@ static void mwifiex_cleanup_txq_ring(struct mwifiex_adapter *adapter)
> card->tx_buf_list[i] = NULL;
> }
>
> + atomic_set(&adapter->tx_hw_pending, 0);
> return;
> }
>
> @@ -1126,6 +1128,7 @@ static int mwifiex_pcie_send_data_complete(struct mwifiex_adapter *adapter)
> -1);
> else
> mwifiex_write_data_complete(adapter, skb, 0, 0);
> + atomic_dec(&adapter->tx_hw_pending);
> }
>
> card->tx_buf_list[wrdoneidx] = NULL;
> @@ -1218,6 +1221,7 @@ mwifiex_pcie_send_data(struct mwifiex_adapter *adapter, struct sk_buff *skb,
> wrindx = (card->txbd_wrptr & reg->tx_mask) >> reg->tx_start_ptr;
> buf_pa = MWIFIEX_SKB_DMA_ADDR(skb);
> card->tx_buf_list[wrindx] = skb;
> + atomic_inc(&adapter->tx_hw_pending);
>
> if (reg->pfu_enabled) {
> desc2 = card->txbd_ring[wrindx];
> @@ -1295,6 +1299,7 @@ mwifiex_pcie_send_data(struct mwifiex_adapter *adapter, struct sk_buff *skb,
> done_unmap:
> mwifiex_unmap_pci_memory(adapter, skb, PCI_DMA_TODEVICE);
> card->tx_buf_list[wrindx] = NULL;
> + atomic_dec(&adapter->tx_hw_pending);
> if (reg->pfu_enabled)
> memset(desc2, 0, sizeof(*desc2));
> else
> --
> 1.8.1.4
>