| Message ID | 0101016eb1903db0-ef7063b4-0f42-4a01-8886-327541e6c1a4-000000@us-west-2.amazonses.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | ath10k: improve throughout of TX of sdio | expand |
Hi, On 11/28/19 6:30 PM, Wen Gong wrote: > ... > diff --git a/drivers/net/wireless/ath/ath10k/htt_tx.c b/drivers/net/wireless/ath/ath10k/htt_tx.c > index a182c0944cc7..c6c4b2a4d20f 100644 > --- a/drivers/net/wireless/ath/ath10k/htt_tx.c > +++ b/drivers/net/wireless/ath/ath10k/htt_tx.c > @@ -543,7 +543,35 @@ void ath10k_htt_tx_free(struct ath10k_htt *htt) > > void ath10k_htt_htc_tx_complete(struct ath10k *ar, struct sk_buff *skb) > { > + struct ath10k_htt *htt = &ar->htt; > + struct htt_tx_done tx_done = {0}; > + struct htt_cmd_hdr *htt_hdr; > + struct htt_data_tx_desc *desc_hdr; > + u16 flags1; > + > dev_kfree_skb_any(skb); > + > + if (!htt->disable_tx_comp) > + return; > + > + htt_hdr = (struct htt_cmd_hdr *)skb->data; skb is already freed on the above line (dev_kfree_skb_any) but is still used here, should the dev_kfree_skb_any be moved to the end of this function? > + if (htt_hdr->msg_type != HTT_H2T_MSG_TYPE_TX_FRM) > + return; > + > + desc_hdr = (struct htt_data_tx_desc *) > + (skb->data + sizeof(*htt_hdr)); > + flags1 = __le16_to_cpu(desc_hdr->flags1); > + > + ath10k_dbg(ar, ATH10K_DBG_HTT, > + "htt tx complete msdu id:%u ,flags1:%x\n", > + __le16_to_cpu(desc_hdr->id), flags1); > + > + if (flags1 & HTT_DATA_TX_DESC_FLAGS1_TX_COMPLETE) > + return; > + > + tx_done.status = HTT_TX_COMPL_STATE_ACK; > + tx_done.msdu_id = __le16_to_cpu(desc_hdr->id); > + ath10k_txrx_tx_unref(&ar->htt, &tx_done); > } >
On 2020-02-11 15:03, Pi-Hsun Shih wrote: > Hi, > > On 11/28/19 6:30 PM, Wen Gong wrote: >> ... >> diff --git a/drivers/net/wireless/ath/ath10k/htt_tx.c >> b/drivers/net/wireless/ath/ath10k/htt_tx.c >> index a182c0944cc7..c6c4b2a4d20f 100644 >> --- a/drivers/net/wireless/ath/ath10k/htt_tx.c >> +++ b/drivers/net/wireless/ath/ath10k/htt_tx.c >> @@ -543,7 +543,35 @@ void ath10k_htt_tx_free(struct ath10k_htt *htt) >> void ath10k_htt_htc_tx_complete(struct ath10k *ar, struct sk_buff >> *skb) >> { >> + struct ath10k_htt *htt = &ar->htt; >> + struct htt_tx_done tx_done = {0}; >> + struct htt_cmd_hdr *htt_hdr; >> + struct htt_data_tx_desc *desc_hdr; >> + u16 flags1; >> + >> dev_kfree_skb_any(skb); >> + >> + if (!htt->disable_tx_comp) >> + return; >> + >> + htt_hdr = (struct htt_cmd_hdr *)skb->data; > > skb is already freed on the above line (dev_kfree_skb_any) but is > still used here, should the dev_kfree_skb_any be moved to the end of > this function? > skb will not freed on the above line, please see this patch https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/drivers/net/wireless/ath/ath10k?h=ath-next&id=30382dd1cf3a141bfaa568ee183c1892090fa79a
On Tue, Feb 11, 2020 at 5:46 PM Wen Gong <wgong@codeaurora.org> wrote: > > On 2020-02-11 15:03, Pi-Hsun Shih wrote: > > Hi, > > > > On 11/28/19 6:30 PM, Wen Gong wrote: > >> ... > >> diff --git a/drivers/net/wireless/ath/ath10k/htt_tx.c > >> b/drivers/net/wireless/ath/ath10k/htt_tx.c > >> index a182c0944cc7..c6c4b2a4d20f 100644 > >> --- a/drivers/net/wireless/ath/ath10k/htt_tx.c > >> +++ b/drivers/net/wireless/ath/ath10k/htt_tx.c > >> @@ -543,7 +543,35 @@ void ath10k_htt_tx_free(struct ath10k_htt *htt) > >> void ath10k_htt_htc_tx_complete(struct ath10k *ar, struct sk_buff > >> *skb) > >> { > >> + struct ath10k_htt *htt = &ar->htt; > >> + struct htt_tx_done tx_done = {0}; > >> + struct htt_cmd_hdr *htt_hdr; > >> + struct htt_data_tx_desc *desc_hdr; > >> + u16 flags1; > >> + > >> dev_kfree_skb_any(skb); > >> + > >> + if (!htt->disable_tx_comp) > >> + return; > >> + > >> + htt_hdr = (struct htt_cmd_hdr *)skb->data; > > > > skb is already freed on the above line (dev_kfree_skb_any) but is > > still used here, should the dev_kfree_skb_any be moved to the end of > > this function? > > > skb will not freed on the above line, please see this patch > https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/drivers/net/wireless/ath/ath10k?h=ath-next&id=30382dd1cf3a141bfaa568ee183c1892090fa79a IIUC the commit only makes the skb not freed in ieee80211_tx_status, but it's still freed in ath10k_htt_htc_tx_complete (by dev_kfree_skb_any)? While booting with this patch (and the 30382dd1cf3a141bfaa568ee183c1892090fa79a commit) with kernel bootargs "slub_debug=FZPUA", I got a kernel panic in ath10k module: [ 16.058676] Unable to handle kernel paging request at virtual address 006b6b6b6b6b6b6b [ 16.066613] Mem abort info: [ 16.069419] ESR = 0x96000004 [ 16.072481] Exception class = DABT (current EL), IL = 32 bits [ 16.078406] SET = 0, FnV = 0 [ 16.081476] EA = 0, S1PTW = 0 [ 16.084624] Data abort info: [ 16.087513] ISV = 0, ISS = 0x00000004 [ 16.091369] CM = 0, WnR = 0 [ 16.094354] [006b6b6b6b6b6b6b] address between user and kernel address ranges [ 16.101503] Internal error: Oops: 96000004 [#1] PREEMPT SMP [ 16.107071] Modules linked in: ath10k_sdio ath10k_core ath mac80211 cfg80211 lzo_rle lzo_compress zram asix usbnet mii joydev [ 16.118380] Process kworker/u16:2 (pid: 142, stack limit = 0x00000000082e3c57) [ 16.125597] CPU: 7 PID: 142 Comm: kworker/u16:2 Not tainted 4.19.102 #48 [ 16.132287] Hardware name: MediaTek krane sku176 board (DT) [ 16.137862] Workqueue: ath10k_sdio_wq ath10k_sdio_write_async_work [ath10k_sdio] [ 16.145251] pstate: 60000005 (nZCv daif -PAN -UAO) [ 16.150051] pc : ath10k_htt_htc_tx_complete+0xe0/0x1a4 [ath10k_core] [ 16.156411] lr : ath10k_htt_htc_tx_complete+0xdc/0x1a4 [ath10k_core] [ 16.162755] sp : ffffff800888bc80 [ 16.166061] x29: ffffff800888bc90 x28: ffffffd892b08c20 [ 16.171363] x27: ffffffd892b173f8 x26: ffffffd892b08c20 [ 16.176666] x25: ffffffd897337240 x24: ffffffd892b16b48 [ 16.181968] x23: 6b6b6b6b6b6b6b6b x22: ffffff970d2a1000 [ 16.187270] x21: ffffff970d2a0000 x20: ffffffd897337240 [ 16.192572] x19: ffffffd892b01960 x18: 0000000000000000 [ 16.197873] x17: 000000000000003c x16: ffffff970edefba0 [ 16.203174] x15: 0000000000000006 x14: ffff001000000600 [ 16.208475] x13: 00000000000064e6 x12: 0000000000000000 [ 16.213777] x11: 0000000000000000 x10: 0000000000000000 [ 16.219079] x9 : b307f4e257a4e000 x8 : b307f4e257a4e000 [ 16.224391] x7 : 0000000000000000 x6 : ffffff970f970e9c [ 16.229712] x5 : 0000000000000027 x4 : 0000000000000000 [ 16.235030] x3 : 000000000002ed25 x2 : ffffffd8bff94fd8 [ 16.240341] x1 : ffffffd8bff8c0c8 x0 : 0000000000000034 [ 16.245644] Call trace: [ 16.248109] ath10k_htt_htc_tx_complete+0xe0/0x1a4 [ath10k_core] [ 16.254123] ath10k_htc_notify_tx_completion+0xe4/0x118 [ath10k_core] [ 16.260559] ath10k_sdio_write_async_work+0x158/0x1f4 [ath10k_sdio] [ 16.266823] process_one_work+0x208/0x408 [ 16.270825] worker_thread+0x23c/0x3e4 [ 16.274566] kthread+0x120/0x130 [ 16.277788] ret_from_fork+0x10/0x18 [ 16.281357] Code: 528046a3 aa1303e0 97ffc028 f9406a97 (394002e8) [ 16.287442] ---[ end trace 3bae4173512bf484 ]--- [ 16.298803] Kernel panic - not syncing: Fatal exception [ 16.304033] SMP: stopping secondary CPUs [ 16.308072] Kernel Offset: 0x1706400000 from 0xffffff8008000000 [ 16.313983] CPU features: 0x0,2188200c [ 16.317721] Memory Limit: none So it seems that the skb is used-after-free in ath10k_htt_htc_tx_complete here.
On 2020-02-11 19:11, Pi-Hsun Shih wrote: > On Tue, Feb 11, 2020 at 5:46 PM Wen Gong <wgong@codeaurora.org> wrote: >> >> On 2020-02-11 15:03, Pi-Hsun Shih wrote: >> > Hi, >> > >> > On 11/28/19 6:30 PM, Wen Gong wrote: >> >> ... >> >> diff --git a/drivers/net/wireless/ath/ath10k/htt_tx.c >> >> b/drivers/net/wireless/ath/ath10k/htt_tx.c >> >> index a182c0944cc7..c6c4b2a4d20f 100644 >> >> --- a/drivers/net/wireless/ath/ath10k/htt_tx.c >> >> +++ b/drivers/net/wireless/ath/ath10k/htt_tx.c >> >> @@ -543,7 +543,35 @@ void ath10k_htt_tx_free(struct ath10k_htt *htt) >> >> void ath10k_htt_htc_tx_complete(struct ath10k *ar, struct sk_buff >> >> *skb) >> >> { >> >> + struct ath10k_htt *htt = &ar->htt; >> >> + struct htt_tx_done tx_done = {0}; >> >> + struct htt_cmd_hdr *htt_hdr; >> >> + struct htt_data_tx_desc *desc_hdr; >> >> + u16 flags1; >> >> + >> >> dev_kfree_skb_any(skb); >> >> + >> >> + if (!htt->disable_tx_comp) >> >> + return; >> >> + >> >> + htt_hdr = (struct htt_cmd_hdr *)skb->data; >> > >> > skb is already freed on the above line (dev_kfree_skb_any) but is >> > still used here, should the dev_kfree_skb_any be moved to the end of >> > this function? >> > >> skb will not freed on the above line, please see this patch >> https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/drivers/net/wireless/ath/ath10k?h=ath-next&id=30382dd1cf3a141bfaa568ee183c1892090fa79a > > IIUC the commit only makes the skb not freed in ieee80211_tx_status, > but it's still freed in ath10k_htt_htc_tx_complete (by > dev_kfree_skb_any)? > > While booting with this patch (and the > 30382dd1cf3a141bfaa568ee183c1892090fa79a commit) with kernel bootargs > "slub_debug=FZPUA", I got a kernel panic in ath10k module: > > [ 16.058676] Unable to handle kernel paging request at virtual > address 006b6b6b6b6b6b6b > [ 16.066613] Mem abort info: > [ 16.069419] ESR = 0x96000004 > [ 16.072481] Exception class = DABT (current EL), IL = 32 bits > [ 16.078406] SET = 0, FnV = 0 > [ 16.081476] EA = 0, S1PTW = 0 > [ 16.084624] Data abort info: > [ 16.087513] ISV = 0, ISS = 0x00000004 > [ 16.091369] CM = 0, WnR = 0 > [ 16.094354] [006b6b6b6b6b6b6b] address between user and kernel > address ranges > [ 16.101503] Internal error: Oops: 96000004 [#1] PREEMPT SMP > [ 16.107071] Modules linked in: ath10k_sdio ath10k_core ath mac80211 > cfg80211 lzo_rle lzo_compress zram asix usbnet mii joydev > [ 16.118380] Process kworker/u16:2 (pid: 142, stack limit = > 0x00000000082e3c57) > [ 16.125597] CPU: 7 PID: 142 Comm: kworker/u16:2 Not tainted 4.19.102 > #48 > [ 16.132287] Hardware name: MediaTek krane sku176 board (DT) > [ 16.137862] Workqueue: ath10k_sdio_wq ath10k_sdio_write_async_work > [ath10k_sdio] > [ 16.145251] pstate: 60000005 (nZCv daif -PAN -UAO) > [ 16.150051] pc : ath10k_htt_htc_tx_complete+0xe0/0x1a4 [ath10k_core] > [ 16.156411] lr : ath10k_htt_htc_tx_complete+0xdc/0x1a4 [ath10k_core] > [ 16.162755] sp : ffffff800888bc80 > [ 16.166061] x29: ffffff800888bc90 x28: ffffffd892b08c20 > [ 16.171363] x27: ffffffd892b173f8 x26: ffffffd892b08c20 > [ 16.176666] x25: ffffffd897337240 x24: ffffffd892b16b48 > [ 16.181968] x23: 6b6b6b6b6b6b6b6b x22: ffffff970d2a1000 > [ 16.187270] x21: ffffff970d2a0000 x20: ffffffd897337240 > [ 16.192572] x19: ffffffd892b01960 x18: 0000000000000000 > [ 16.197873] x17: 000000000000003c x16: ffffff970edefba0 > [ 16.203174] x15: 0000000000000006 x14: ffff001000000600 > [ 16.208475] x13: 00000000000064e6 x12: 0000000000000000 > [ 16.213777] x11: 0000000000000000 x10: 0000000000000000 > [ 16.219079] x9 : b307f4e257a4e000 x8 : b307f4e257a4e000 > [ 16.224391] x7 : 0000000000000000 x6 : ffffff970f970e9c > [ 16.229712] x5 : 0000000000000027 x4 : 0000000000000000 > [ 16.235030] x3 : 000000000002ed25 x2 : ffffffd8bff94fd8 > [ 16.240341] x1 : ffffffd8bff8c0c8 x0 : 0000000000000034 > [ 16.245644] Call trace: > [ 16.248109] ath10k_htt_htc_tx_complete+0xe0/0x1a4 [ath10k_core] > [ 16.254123] ath10k_htc_notify_tx_completion+0xe4/0x118 > [ath10k_core] > [ 16.260559] ath10k_sdio_write_async_work+0x158/0x1f4 [ath10k_sdio] > [ 16.266823] process_one_work+0x208/0x408 > [ 16.270825] worker_thread+0x23c/0x3e4 > [ 16.274566] kthread+0x120/0x130 > [ 16.277788] ret_from_fork+0x10/0x18 > [ 16.281357] Code: 528046a3 aa1303e0 97ffc028 f9406a97 (394002e8) > [ 16.287442] ---[ end trace 3bae4173512bf484 ]--- > [ 16.298803] Kernel panic - not syncing: Fatal exception > [ 16.304033] SMP: stopping secondary CPUs > [ 16.308072] Kernel Offset: 0x1706400000 from 0xffffff8008000000 > [ 16.313983] CPU features: 0x0,2188200c > [ 16.317721] Memory Limit: none > > So it seems that the skb is used-after-free in > ath10k_htt_htc_tx_complete here. I guess the panic is because of a tx mgmt frame. did you connect to an AP? is the panic easy to happen? Could you apply this change and collect message if panic( also apply 30382dd1cf3a141bfaa568ee183c1892090fa79a and ath10k: disable TX complete indication of htt for sdio)? it will print some useful info with ath10k_warn. diff --git a/drivers/net/wireless/ath/ath10k/htt_tx.c b/drivers/net/wireless/ath/ath10k/htt_tx.c index 51f060a00b95..b1f768271331 100644 --- a/drivers/net/wireless/ath/ath10k/htt_tx.c +++ b/drivers/net/wireless/ath/ath10k/htt_tx.c @@ -555,6 +555,21 @@ void ath10k_htt_htc_tx_complete(struct ath10k *ar, struct sk_buff *skb) struct htt_data_tx_desc *desc_hdr; u16 flags1; + htt_hdr = (struct htt_cmd_hdr *)skb->data; + + ath10k_warn(ar, "msg_type: %d\n", htt_hdr->msg_type); + + if (htt_hdr->msg_type == HTT_H2T_MSG_TYPE_TX_FRM) { + desc_hdr = (struct htt_data_tx_desc *) + (skb->data + sizeof(*htt_hdr)); + flags1 = __le16_to_cpu(desc_hdr->flags1); + + if (flags1 & HTT_DATA_TX_DESC_FLAGS1_TX_COMPLETE) + ath10k_warn(ar, "htt tx mgmt\n"); + else + ath10k_warn(ar, "htt tx data\n"); + } + dev_kfree_skb_any(skb); if (!htt->disable_tx_comp)
On Wed, Feb 12, 2020 at 12:58 PM Wen Gong <wgong@codeaurora.org> wrote: > > On 2020-02-11 19:11, Pi-Hsun Shih wrote: > > On Tue, Feb 11, 2020 at 5:46 PM Wen Gong <wgong@codeaurora.org> wrote: > >> > >> On 2020-02-11 15:03, Pi-Hsun Shih wrote: > >> > Hi, > >> > > >> > On 11/28/19 6:30 PM, Wen Gong wrote: > >> >> ... > >> >> diff --git a/drivers/net/wireless/ath/ath10k/htt_tx.c > >> >> b/drivers/net/wireless/ath/ath10k/htt_tx.c > >> >> index a182c0944cc7..c6c4b2a4d20f 100644 > >> >> --- a/drivers/net/wireless/ath/ath10k/htt_tx.c > >> >> +++ b/drivers/net/wireless/ath/ath10k/htt_tx.c > >> >> @@ -543,7 +543,35 @@ void ath10k_htt_tx_free(struct ath10k_htt *htt) > >> >> void ath10k_htt_htc_tx_complete(struct ath10k *ar, struct sk_buff > >> >> *skb) > >> >> { > >> >> + struct ath10k_htt *htt = &ar->htt; > >> >> + struct htt_tx_done tx_done = {0}; > >> >> + struct htt_cmd_hdr *htt_hdr; > >> >> + struct htt_data_tx_desc *desc_hdr; > >> >> + u16 flags1; > >> >> + > >> >> dev_kfree_skb_any(skb); > >> >> + > >> >> + if (!htt->disable_tx_comp) > >> >> + return; > >> >> + > >> >> + htt_hdr = (struct htt_cmd_hdr *)skb->data; > >> > > >> > skb is already freed on the above line (dev_kfree_skb_any) but is > >> > still used here, should the dev_kfree_skb_any be moved to the end of > >> > this function? > >> > > >> skb will not freed on the above line, please see this patch > >> https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/drivers/net/wireless/ath/ath10k?h=ath-next&id=30382dd1cf3a141bfaa568ee183c1892090fa79a > > > > IIUC the commit only makes the skb not freed in ieee80211_tx_status, > > but it's still freed in ath10k_htt_htc_tx_complete (by > > dev_kfree_skb_any)? > > > > While booting with this patch (and the > > 30382dd1cf3a141bfaa568ee183c1892090fa79a commit) with kernel bootargs > > "slub_debug=FZPUA", I got a kernel panic in ath10k module: > > > > [ 16.058676] Unable to handle kernel paging request at virtual > > address 006b6b6b6b6b6b6b > > [ 16.066613] Mem abort info: > > [ 16.069419] ESR = 0x96000004 > > [ 16.072481] Exception class = DABT (current EL), IL = 32 bits > > [ 16.078406] SET = 0, FnV = 0 > > [ 16.081476] EA = 0, S1PTW = 0 > > [ 16.084624] Data abort info: > > [ 16.087513] ISV = 0, ISS = 0x00000004 > > [ 16.091369] CM = 0, WnR = 0 > > [ 16.094354] [006b6b6b6b6b6b6b] address between user and kernel > > address ranges > > [ 16.101503] Internal error: Oops: 96000004 [#1] PREEMPT SMP > > [ 16.107071] Modules linked in: ath10k_sdio ath10k_core ath mac80211 > > cfg80211 lzo_rle lzo_compress zram asix usbnet mii joydev > > [ 16.118380] Process kworker/u16:2 (pid: 142, stack limit = > > 0x00000000082e3c57) > > [ 16.125597] CPU: 7 PID: 142 Comm: kworker/u16:2 Not tainted 4.19.102 > > #48 > > [ 16.132287] Hardware name: MediaTek krane sku176 board (DT) > > [ 16.137862] Workqueue: ath10k_sdio_wq ath10k_sdio_write_async_work > > [ath10k_sdio] > > [ 16.145251] pstate: 60000005 (nZCv daif -PAN -UAO) > > [ 16.150051] pc : ath10k_htt_htc_tx_complete+0xe0/0x1a4 [ath10k_core] > > [ 16.156411] lr : ath10k_htt_htc_tx_complete+0xdc/0x1a4 [ath10k_core] > > [ 16.162755] sp : ffffff800888bc80 > > [ 16.166061] x29: ffffff800888bc90 x28: ffffffd892b08c20 > > [ 16.171363] x27: ffffffd892b173f8 x26: ffffffd892b08c20 > > [ 16.176666] x25: ffffffd897337240 x24: ffffffd892b16b48 > > [ 16.181968] x23: 6b6b6b6b6b6b6b6b x22: ffffff970d2a1000 > > [ 16.187270] x21: ffffff970d2a0000 x20: ffffffd897337240 > > [ 16.192572] x19: ffffffd892b01960 x18: 0000000000000000 > > [ 16.197873] x17: 000000000000003c x16: ffffff970edefba0 > > [ 16.203174] x15: 0000000000000006 x14: ffff001000000600 > > [ 16.208475] x13: 00000000000064e6 x12: 0000000000000000 > > [ 16.213777] x11: 0000000000000000 x10: 0000000000000000 > > [ 16.219079] x9 : b307f4e257a4e000 x8 : b307f4e257a4e000 > > [ 16.224391] x7 : 0000000000000000 x6 : ffffff970f970e9c > > [ 16.229712] x5 : 0000000000000027 x4 : 0000000000000000 > > [ 16.235030] x3 : 000000000002ed25 x2 : ffffffd8bff94fd8 > > [ 16.240341] x1 : ffffffd8bff8c0c8 x0 : 0000000000000034 > > [ 16.245644] Call trace: > > [ 16.248109] ath10k_htt_htc_tx_complete+0xe0/0x1a4 [ath10k_core] > > [ 16.254123] ath10k_htc_notify_tx_completion+0xe4/0x118 > > [ath10k_core] > > [ 16.260559] ath10k_sdio_write_async_work+0x158/0x1f4 [ath10k_sdio] > > [ 16.266823] process_one_work+0x208/0x408 > > [ 16.270825] worker_thread+0x23c/0x3e4 > > [ 16.274566] kthread+0x120/0x130 > > [ 16.277788] ret_from_fork+0x10/0x18 > > [ 16.281357] Code: 528046a3 aa1303e0 97ffc028 f9406a97 (394002e8) > > [ 16.287442] ---[ end trace 3bae4173512bf484 ]--- > > [ 16.298803] Kernel panic - not syncing: Fatal exception > > [ 16.304033] SMP: stopping secondary CPUs > > [ 16.308072] Kernel Offset: 0x1706400000 from 0xffffff8008000000 > > [ 16.313983] CPU features: 0x0,2188200c > > [ 16.317721] Memory Limit: none > > > > So it seems that the skb is used-after-free in > > ath10k_htt_htc_tx_complete here. > I guess the panic is because of a tx mgmt frame. > did you connect to an AP? No, I disabled wifi on UI before adding the kernel bootargs, so it should not be connected to any AP. > is the panic easy to happen? Yes, this happens on every boot (early in boot before entering UI) with kernel bootargs "slub_debug=FZPUA" added. > > Could you apply this change and collect message if panic( > also apply 30382dd1cf3a141bfaa568ee183c1892090fa79a and ath10k: disable > TX complete indication of htt for sdio)? > it will print some useful info with ath10k_warn. > > diff --git a/drivers/net/wireless/ath/ath10k/htt_tx.c > b/drivers/net/wireless/ath/ath10k/htt_tx.c > index 51f060a00b95..b1f768271331 100644 > --- a/drivers/net/wireless/ath/ath10k/htt_tx.c > +++ b/drivers/net/wireless/ath/ath10k/htt_tx.c > @@ -555,6 +555,21 @@ void ath10k_htt_htc_tx_complete(struct ath10k *ar, > struct sk_buff *skb) > struct htt_data_tx_desc *desc_hdr; > u16 flags1; > > + htt_hdr = (struct htt_cmd_hdr *)skb->data; > + > + ath10k_warn(ar, "msg_type: %d\n", htt_hdr->msg_type); > + > + if (htt_hdr->msg_type == HTT_H2T_MSG_TYPE_TX_FRM) { > + desc_hdr = (struct htt_data_tx_desc *) > + (skb->data + sizeof(*htt_hdr)); > + flags1 = __le16_to_cpu(desc_hdr->flags1); > + > + if (flags1 & HTT_DATA_TX_DESC_FLAGS1_TX_COMPLETE) > + ath10k_warn(ar, "htt tx mgmt\n"); > + else > + ath10k_warn(ar, "htt tx data\n"); > + } > + > dev_kfree_skb_any(skb); > > if (!htt->disable_tx_comp) Output as follows: [ 10.747482] ath10k_sdio mmc1:0001:1: msg_type: 0 [ 10.749295] ath10k_sdio mmc1:0001:1: htt-ver 3.73 wmi-op 4 htt-op 3 cal otp max-sta 32 raw 0 hwcrypto 1 [ 10.752243] Unable to handle kernel paging request at virtual address 006b6b6b6b6b6b6b [ 10.769674] Mem abort info: [ 10.772514] ESR = 0x96000004 [ 10.775625] Exception class = DABT (current EL), IL = 32 bits [ 10.781609] SET = 0, FnV = 0 [ 10.784699] EA = 0, S1PTW = 0 [ 10.787889] Data abort info: [ 10.790839] ISV = 0, ISS = 0x00000004 [ 10.794711] CM = 0, WnR = 0 [ 10.797714] [006b6b6b6b6b6b6b] address between user and kernel address ranges [ 10.804911] Internal error: Oops: 96000004 [#1] PREEMPT SMP [ 10.810488] Modules linked in: asix usbnet mii ath10k_sdio ath10k_core ath lzo_rle mac80211 lzo_compress zram cfg80211 joydev [ 10.821800] Process kworker/u16:1 (pid: 140, stack limit = 0x0000000008a1ed57) [ 10.829017] CPU: 4 PID: 140 Comm: kworker/u16:1 Tainted: G W 4.19.102 #49 [ 10.837097] Hardware name: MediaTek krane sku176 board (DT) [ 10.842670] Workqueue: ath10k_sdio_wq ath10k_sdio_write_async_work [ath10k_sdio] [ 10.850059] pstate: 60000005 (nZCv daif -PAN -UAO) [ 10.854860] pc : ath10k_htt_htc_tx_complete+0x9c/0x134 [ath10k_core] [ 10.861225] lr : ath10k_htt_htc_tx_complete+0x8c/0x134 [ath10k_core] [ 10.867568] sp : ffffff800888bc90 [ 10.870874] x29: ffffff800888bca0 x28: fffffffd69548be8 [ 10.876177] x27: fffffffd695573f8 x26: fffffffd69548be8 [ 10.881480] x25: fffffffd742f1e40 x24: fffffffd69556b48 [ 10.886782] x23: fffffffd69556b10 x22: fffffffd695c7e80 [ 10.892084] x21: 6b6b6b6b6b6b6b6b x20: fffffffd742f1e40 [ 10.897386] x19: fffffffd69541960 x18: 0000000000000000 [ 10.902696] x17: 000000000000003c x16: ffffffa964a7d36c [ 10.908004] x15: fffffffd742f3e80 x14: 0000000000000280 [ 10.913306] x13: 0000000000000001 x12: 0000000000000000 [ 10.918607] x11: 0000000000000000 x10: 0000000000000000 [ 10.923908] x9 : 2edc72d89d761200 x8 : 0000000000000001 [ 10.929209] x7 : bbbbbbbbbbbbbbbb x6 : 0000000000000020 [ 10.934511] x5 : 000000000000005a x4 : 0000000000000000 [ 10.939812] x3 : 0000000000000010 x2 : 0000000000000008 [ 10.945122] x1 : 0000000000000000 x0 : 0000000000000000 [ 10.950439] Call trace: [ 10.952904] ath10k_htt_htc_tx_complete+0x9c/0x134 [ath10k_core] [ 10.958918] ath10k_htc_notify_tx_completion+0xe4/0x118 [ath10k_core] [ 10.965366] ath10k_sdio_write_async_work+0x158/0x1f4 [ath10k_sdio] [ 10.971637] process_one_work+0x208/0x408 [ 10.975638] worker_thread+0x23c/0x3e4 [ 10.979379] kthread+0x120/0x130 [ 10.982602] ret_from_fork+0x10/0x18 [ 10.986178] Code: 52820f08 38686a68 340003a8 f9406a95 (394002a8) [ 10.992266] ---[ end trace 3ed9b11cd8f60113 ]--- [ 11.004351] Kernel panic - not syncing: Fatal exception [ 11.009586] SMP: stopping secondary CPUs [ 11.013519] Kernel Offset: 0x295c200000 from 0xffffff8008000000 [ 11.019433] CPU features: 0x0,2188200c [ 11.023172] Memory Limit: none
On 2020-02-12 13:31, Pi-Hsun Shih wrote: > Output as follows: > > [ 10.747482] ath10k_sdio mmc1:0001:1: msg_type: 0 > [ 10.749295] ath10k_sdio mmc1:0001:1: htt-ver 3.73 wmi-op 4 htt-op 3 > cal otp max-sta 32 raw 0 hwcrypto 1 > [ 10.752243] Unable to handle kernel paging request at virtual > address 006b6b6b6b6b6b6b > [ 10.769674] Mem abort info: > [ 10.772514] ESR = 0x96000004 > [ 10.775625] Exception class = DABT (current EL), IL = 32 bits > [ 10.781609] SET = 0, FnV = 0 > [ 10.784699] EA = 0, S1PTW = 0 > [ 10.787889] Data abort info: > [ 10.790839] ISV = 0, ISS = 0x00000004 > [ 10.794711] CM = 0, WnR = 0 > [ 10.797714] [006b6b6b6b6b6b6b] address between user and kernel > address ranges > [ 10.804911] Internal error: Oops: 96000004 [#1] PREEMPT SMP > [ 10.810488] Modules linked in: asix usbnet mii ath10k_sdio > ath10k_core ath lzo_rle mac80211 lzo_compress zram cfg80211 joydev > [ 10.821800] Process kworker/u16:1 (pid: 140, stack limit = > 0x0000000008a1ed57) > [ 10.829017] CPU: 4 PID: 140 Comm: kworker/u16:1 Tainted: G W > 4.19.102 #49 > [ 10.837097] Hardware name: MediaTek krane sku176 board (DT) > [ 10.842670] Workqueue: ath10k_sdio_wq ath10k_sdio_write_async_work > [ath10k_sdio] > [ 10.850059] pstate: 60000005 (nZCv daif -PAN -UAO) > [ 10.854860] pc : ath10k_htt_htc_tx_complete+0x9c/0x134 [ath10k_core] > [ 10.861225] lr : ath10k_htt_htc_tx_complete+0x8c/0x134 [ath10k_core] > [ 10.867568] sp : ffffff800888bc90 > [ 10.870874] x29: ffffff800888bca0 x28: fffffffd69548be8 > [ 10.876177] x27: fffffffd695573f8 x26: fffffffd69548be8 > [ 10.881480] x25: fffffffd742f1e40 x24: fffffffd69556b48 > [ 10.886782] x23: fffffffd69556b10 x22: fffffffd695c7e80 > [ 10.892084] x21: 6b6b6b6b6b6b6b6b x20: fffffffd742f1e40 > [ 10.897386] x19: fffffffd69541960 x18: 0000000000000000 > [ 10.902696] x17: 000000000000003c x16: ffffffa964a7d36c > [ 10.908004] x15: fffffffd742f3e80 x14: 0000000000000280 > [ 10.913306] x13: 0000000000000001 x12: 0000000000000000 > [ 10.918607] x11: 0000000000000000 x10: 0000000000000000 > [ 10.923908] x9 : 2edc72d89d761200 x8 : 0000000000000001 > [ 10.929209] x7 : bbbbbbbbbbbbbbbb x6 : 0000000000000020 > [ 10.934511] x5 : 000000000000005a x4 : 0000000000000000 > [ 10.939812] x3 : 0000000000000010 x2 : 0000000000000008 > [ 10.945122] x1 : 0000000000000000 x0 : 0000000000000000 > [ 10.950439] Call trace: > [ 10.952904] ath10k_htt_htc_tx_complete+0x9c/0x134 [ath10k_core] > [ 10.958918] ath10k_htc_notify_tx_completion+0xe4/0x118 > [ath10k_core] > [ 10.965366] ath10k_sdio_write_async_work+0x158/0x1f4 [ath10k_sdio] > [ 10.971637] process_one_work+0x208/0x408 > [ 10.975638] worker_thread+0x23c/0x3e4 > [ 10.979379] kthread+0x120/0x130 > [ 10.982602] ret_from_fork+0x10/0x18 > [ 10.986178] Code: 52820f08 38686a68 340003a8 f9406a95 (394002a8) > [ 10.992266] ---[ end trace 3ed9b11cd8f60113 ]--- > [ 11.004351] Kernel panic - not syncing: Fatal exception > [ 11.009586] SMP: stopping secondary CPUs > [ 11.013519] Kernel Offset: 0x295c200000 from 0xffffff8008000000 > [ 11.019433] CPU features: 0x0,2188200c > [ 11.023172] Memory Limit: none Thanks. I have made change again to fix the panic. could you try again with this change? (also apply 30382dd1cf3a141bfaa568ee183c1892090fa79a and ath10k: disable TX complete indication of htt for sdio) diff --git a/drivers/net/wireless/ath/ath10k/htt_tx.c b/drivers/net/wireless/ath/ath10k/htt_tx.c index 51f060a00b95..7bfdeb1298a5 100644 --- a/drivers/net/wireless/ath/ath10k/htt_tx.c +++ b/drivers/net/wireless/ath/ath10k/htt_tx.c @@ -554,20 +554,30 @@ void ath10k_htt_htc_tx_complete(struct ath10k *ar, struct sk_buff *skb) struct htt_cmd_hdr *htt_hdr; struct htt_data_tx_desc *desc_hdr; u16 flags1; + u8 msg_type; + + if (htt->disable_tx_comp) { + htt_hdr = (struct htt_cmd_hdr *)skb->data; + msg_type = htt_hdr->msg_type; + ath10k_warn(ar, "msg_type: %d\n", msg_type); + + if (htt_hdr->msg_type == HTT_H2T_MSG_TYPE_TX_FRM) { + desc_hdr = (struct htt_data_tx_desc *) + (skb->data + sizeof(*htt_hdr)); + flags1 = __le16_to_cpu(desc_hdr->flags1); + + if (flags1 & HTT_DATA_TX_DESC_FLAGS1_TX_COMPLETE) + ath10k_warn(ar, "htt tx mgmt\n"); + else + ath10k_warn(ar, "htt tx data\n"); + } + } dev_kfree_skb_any(skb); - if (!htt->disable_tx_comp) + if ((!htt->disable_tx_comp) || (msg_type != HTT_H2T_MSG_TYPE_TX_FRM)) return; - htt_hdr = (struct htt_cmd_hdr *)skb->data; - if (htt_hdr->msg_type != HTT_H2T_MSG_TYPE_TX_FRM) - return; - - desc_hdr = (struct htt_data_tx_desc *) - (skb->data + sizeof(*htt_hdr)); - flags1 = __le16_to_cpu(desc_hdr->flags1); - ath10k_dbg(ar, ATH10K_DBG_HTT, "htt tx complete msdu id:%u ,flags1:%x\n", __le16_to_cpu(desc_hdr->id), flags1);
Tested that the patch fix the kernel panic, thanks. For the fixed version: Tested-by: Pi-Hsun Shih <pihsun@chromium.org> On Wed, Feb 12, 2020 at 2:47 PM Wen Gong <wgong@codeaurora.org> wrote: > > On 2020-02-12 13:31, Pi-Hsun Shih wrote: > > > Output as follows: > > > > [ 10.747482] ath10k_sdio mmc1:0001:1: msg_type: 0 > > [ 10.749295] ath10k_sdio mmc1:0001:1: htt-ver 3.73 wmi-op 4 htt-op 3 > > cal otp max-sta 32 raw 0 hwcrypto 1 > > [ 10.752243] Unable to handle kernel paging request at virtual > > address 006b6b6b6b6b6b6b > > [ 10.769674] Mem abort info: > > [ 10.772514] ESR = 0x96000004 > > [ 10.775625] Exception class = DABT (current EL), IL = 32 bits > > [ 10.781609] SET = 0, FnV = 0 > > [ 10.784699] EA = 0, S1PTW = 0 > > [ 10.787889] Data abort info: > > [ 10.790839] ISV = 0, ISS = 0x00000004 > > [ 10.794711] CM = 0, WnR = 0 > > [ 10.797714] [006b6b6b6b6b6b6b] address between user and kernel > > address ranges > > [ 10.804911] Internal error: Oops: 96000004 [#1] PREEMPT SMP > > [ 10.810488] Modules linked in: asix usbnet mii ath10k_sdio > > ath10k_core ath lzo_rle mac80211 lzo_compress zram cfg80211 joydev > > [ 10.821800] Process kworker/u16:1 (pid: 140, stack limit = > > 0x0000000008a1ed57) > > [ 10.829017] CPU: 4 PID: 140 Comm: kworker/u16:1 Tainted: G W > > 4.19.102 #49 > > [ 10.837097] Hardware name: MediaTek krane sku176 board (DT) > > [ 10.842670] Workqueue: ath10k_sdio_wq ath10k_sdio_write_async_work > > [ath10k_sdio] > > [ 10.850059] pstate: 60000005 (nZCv daif -PAN -UAO) > > [ 10.854860] pc : ath10k_htt_htc_tx_complete+0x9c/0x134 [ath10k_core] > > [ 10.861225] lr : ath10k_htt_htc_tx_complete+0x8c/0x134 [ath10k_core] > > [ 10.867568] sp : ffffff800888bc90 > > [ 10.870874] x29: ffffff800888bca0 x28: fffffffd69548be8 > > [ 10.876177] x27: fffffffd695573f8 x26: fffffffd69548be8 > > [ 10.881480] x25: fffffffd742f1e40 x24: fffffffd69556b48 > > [ 10.886782] x23: fffffffd69556b10 x22: fffffffd695c7e80 > > [ 10.892084] x21: 6b6b6b6b6b6b6b6b x20: fffffffd742f1e40 > > [ 10.897386] x19: fffffffd69541960 x18: 0000000000000000 > > [ 10.902696] x17: 000000000000003c x16: ffffffa964a7d36c > > [ 10.908004] x15: fffffffd742f3e80 x14: 0000000000000280 > > [ 10.913306] x13: 0000000000000001 x12: 0000000000000000 > > [ 10.918607] x11: 0000000000000000 x10: 0000000000000000 > > [ 10.923908] x9 : 2edc72d89d761200 x8 : 0000000000000001 > > [ 10.929209] x7 : bbbbbbbbbbbbbbbb x6 : 0000000000000020 > > [ 10.934511] x5 : 000000000000005a x4 : 0000000000000000 > > [ 10.939812] x3 : 0000000000000010 x2 : 0000000000000008 > > [ 10.945122] x1 : 0000000000000000 x0 : 0000000000000000 > > [ 10.950439] Call trace: > > [ 10.952904] ath10k_htt_htc_tx_complete+0x9c/0x134 [ath10k_core] > > [ 10.958918] ath10k_htc_notify_tx_completion+0xe4/0x118 > > [ath10k_core] > > [ 10.965366] ath10k_sdio_write_async_work+0x158/0x1f4 [ath10k_sdio] > > [ 10.971637] process_one_work+0x208/0x408 > > [ 10.975638] worker_thread+0x23c/0x3e4 > > [ 10.979379] kthread+0x120/0x130 > > [ 10.982602] ret_from_fork+0x10/0x18 > > [ 10.986178] Code: 52820f08 38686a68 340003a8 f9406a95 (394002a8) > > [ 10.992266] ---[ end trace 3ed9b11cd8f60113 ]--- > > [ 11.004351] Kernel panic - not syncing: Fatal exception > > [ 11.009586] SMP: stopping secondary CPUs > > [ 11.013519] Kernel Offset: 0x295c200000 from 0xffffff8008000000 > > [ 11.019433] CPU features: 0x0,2188200c > > [ 11.023172] Memory Limit: none > Thanks. > I have made change again to fix the panic. > could you try again with this change? > (also apply 30382dd1cf3a141bfaa568ee183c1892090fa79a and ath10k: disable > TX complete indication of htt for sdio) > > diff --git a/drivers/net/wireless/ath/ath10k/htt_tx.c > b/drivers/net/wireless/ath/ath10k/htt_tx.c > index 51f060a00b95..7bfdeb1298a5 100644 > --- a/drivers/net/wireless/ath/ath10k/htt_tx.c > +++ b/drivers/net/wireless/ath/ath10k/htt_tx.c > @@ -554,20 +554,30 @@ void ath10k_htt_htc_tx_complete(struct ath10k *ar, > struct sk_buff *skb) > struct htt_cmd_hdr *htt_hdr; > struct htt_data_tx_desc *desc_hdr; > u16 flags1; > + u8 msg_type; > + > + if (htt->disable_tx_comp) { > + htt_hdr = (struct htt_cmd_hdr *)skb->data; > + msg_type = htt_hdr->msg_type; > + ath10k_warn(ar, "msg_type: %d\n", msg_type); > + > + if (htt_hdr->msg_type == HTT_H2T_MSG_TYPE_TX_FRM) { > + desc_hdr = (struct htt_data_tx_desc *) > + (skb->data + sizeof(*htt_hdr)); > + flags1 = __le16_to_cpu(desc_hdr->flags1); > + > + if (flags1 & > HTT_DATA_TX_DESC_FLAGS1_TX_COMPLETE) > + ath10k_warn(ar, "htt tx mgmt\n"); > + else > + ath10k_warn(ar, "htt tx data\n"); > + } > + } > > dev_kfree_skb_any(skb); > > - if (!htt->disable_tx_comp) > + if ((!htt->disable_tx_comp) || (msg_type != > HTT_H2T_MSG_TYPE_TX_FRM)) > return; > > - htt_hdr = (struct htt_cmd_hdr *)skb->data; > - if (htt_hdr->msg_type != HTT_H2T_MSG_TYPE_TX_FRM) > - return; > - > - desc_hdr = (struct htt_data_tx_desc *) > - (skb->data + sizeof(*htt_hdr)); > - flags1 = __le16_to_cpu(desc_hdr->flags1); > - > ath10k_dbg(ar, ATH10K_DBG_HTT, > "htt tx complete msdu id:%u ,flags1:%x\n", > __le16_to_cpu(desc_hdr->id), flags1);
On 2020-02-12 15:08, Pi-Hsun Shih wrote: > Tested that the patch fix the kernel panic, thanks. > > For the fixed version: > Tested-by: Pi-Hsun Shih <pihsun@chromium.org> > Thanks Pi-Hsun's quick test, could you also give me the message log: dmesg | grep ath
On Wed, Feb 12, 2020 at 3:31 PM Wen Gong <wgong@codeaurora.org> wrote: > > On 2020-02-12 15:08, Pi-Hsun Shih wrote: > > Tested that the patch fix the kernel panic, thanks. > > > > For the fixed version: > > Tested-by: Pi-Hsun Shih <pihsun@chromium.org> > > > Thanks Pi-Hsun's quick test, > could you also give me the message log: > dmesg | grep ath [ 11.462398] ath10k_sdio mmc1:0001:1: qca6174 hw3.2 sdio target 0x05030000 chip_id 0x00000000 sub 0000:0000 [ 11.472131] ath10k_sdio mmc1:0001:1: kconfig debug 1 debugfs 1 tracing 1 dfs 0 testmode 1 [ 11.481490] ath10k_sdio mmc1:0001:1: firmware ver WLAN.RMH.4.4.1-00042 api 6 features wowlan,ignore-otp crc32 ac2d4918 [ 11.667020] ath10k_sdio mmc1:0001:1: board_file api 2 bmi_id 0:4 crc32 e74847dc [ 12.035056] ath10k_sdio mmc1:0001:1: msg_type: 0 [ 12.036919] ath10k_sdio mmc1:0001:1: htt-ver 3.73 wmi-op 4 htt-op 3 cal otp max-sta 32 raw 0 hwcrypto 1 [ 12.039878] ath10k_sdio mmc1:0001:1: msg_type: 2 [ 12.053816] ath10k_sdio mmc1:0001:1: msg_type: 5 [ 12.235403] ath: EEPROM regdomain: 0x6c [ 12.239895] ath: EEPROM indicates we should expect a direct regpair map [ 12.247223] ath: Country alpha2 being used: 00 [ 12.251973] ath: Regpair used: 0x6c
On 2020-02-12 15:45, Pi-Hsun Shih wrote: > On Wed, Feb 12, 2020 at 3:31 PM Wen Gong <wgong@codeaurora.org> wrote: >> >> On 2020-02-12 15:08, Pi-Hsun Shih wrote: >> > Tested that the patch fix the kernel panic, thanks. >> > >> > For the fixed version: >> > Tested-by: Pi-Hsun Shih <pihsun@chromium.org> >> > >> Thanks Pi-Hsun's quick test, >> could you also give me the message log: >> dmesg | grep ath > > [ 11.462398] ath10k_sdio mmc1:0001:1: qca6174 hw3.2 sdio target > 0x05030000 chip_id 0x00000000 sub 0000:0000 > [ 11.472131] ath10k_sdio mmc1:0001:1: kconfig debug 1 debugfs 1 > tracing 1 dfs 0 testmode 1 > [ 11.481490] ath10k_sdio mmc1:0001:1: firmware ver > WLAN.RMH.4.4.1-00042 api 6 features wowlan,ignore-otp crc32 ac2d4918 > [ 11.667020] ath10k_sdio mmc1:0001:1: board_file api 2 bmi_id 0:4 > crc32 e74847dc > [ 12.035056] ath10k_sdio mmc1:0001:1: msg_type: 0 > [ 12.036919] ath10k_sdio mmc1:0001:1: htt-ver 3.73 wmi-op 4 htt-op 3 > cal otp max-sta 32 raw 0 hwcrypto 1 > [ 12.039878] ath10k_sdio mmc1:0001:1: msg_type: 2 > [ 12.053816] ath10k_sdio mmc1:0001:1: msg_type: 5 > [ 12.235403] ath: EEPROM regdomain: 0x6c > [ 12.239895] ath: EEPROM indicates we should expect a direct regpair > map > [ 12.247223] ath: Country alpha2 being used: 00 > [ 12.251973] ath: Regpair used: 0x6c Thanks. new patch sent: https://patchwork.kernel.org/patch/11377827/
diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c index 4f76ba5d78a9..5a49edfdf9e6 100644 --- a/drivers/net/wireless/ath/ath10k/core.c +++ b/drivers/net/wireless/ath/ath10k/core.c @@ -695,10 +695,7 @@ static int ath10k_init_sdio(struct ath10k *ar, enum ath10k_firmware_mode mode) if (ret) return ret; - /* Data transfer is not initiated, when reduced Tx completion - * is used for SDIO. disable it until fixed - */ - param &= ~HI_ACS_FLAGS_SDIO_REDUCE_TX_COMPL_SET; + param |= HI_ACS_FLAGS_SDIO_REDUCE_TX_COMPL_SET; /* Alternate credit size of 1544 as used by SDIO firmware is * not big enough for mac80211 / native wifi frames. disable it diff --git a/drivers/net/wireless/ath/ath10k/hif.h b/drivers/net/wireless/ath/ath10k/hif.h index 496ee34a4d78..0dd8973d0acf 100644 --- a/drivers/net/wireless/ath/ath10k/hif.h +++ b/drivers/net/wireless/ath/ath10k/hif.h @@ -56,6 +56,8 @@ struct ath10k_hif_ops { int (*swap_mailbox)(struct ath10k *ar); + int (*get_htt_tx_complete)(struct ath10k *ar); + int (*map_service_to_pipe)(struct ath10k *ar, u16 service_id, u8 *ul_pipe, u8 *dl_pipe); @@ -144,6 +146,13 @@ static inline int ath10k_hif_swap_mailbox(struct ath10k *ar) return 0; } +static inline int ath10k_hif_get_htt_tx_complete(struct ath10k *ar) +{ + if (ar->hif.ops->get_htt_tx_complete) + return ar->hif.ops->get_htt_tx_complete(ar); + return 0; +} + static inline int ath10k_hif_map_service_to_pipe(struct ath10k *ar, u16 service_id, u8 *ul_pipe, u8 *dl_pipe) diff --git a/drivers/net/wireless/ath/ath10k/htc.c b/drivers/net/wireless/ath/ath10k/htc.c index 1d4d1a1992fe..4c6cdc24d282 100644 --- a/drivers/net/wireless/ath/ath10k/htc.c +++ b/drivers/net/wireless/ath/ath10k/htc.c @@ -660,6 +660,16 @@ int ath10k_htc_wait_target(struct ath10k_htc *htc) return 0; } +void ath10k_htc_change_tx_credit_flow(struct ath10k_htc *htc, + enum ath10k_htc_ep_id eid, + bool enable) +{ + struct ath10k *ar = htc->ar; + struct ath10k_htc_ep *ep = &ar->htc.endpoint[eid]; + + ep->tx_credit_flow_enabled = enable; +} + int ath10k_htc_connect_service(struct ath10k_htc *htc, struct ath10k_htc_svc_conn_req *conn_req, struct ath10k_htc_svc_conn_resp *conn_resp) diff --git a/drivers/net/wireless/ath/ath10k/htc.h b/drivers/net/wireless/ath/ath10k/htc.h index f55d3caec61f..d0f37cbdab8b 100644 --- a/drivers/net/wireless/ath/ath10k/htc.h +++ b/drivers/net/wireless/ath/ath10k/htc.h @@ -367,6 +367,9 @@ int ath10k_htc_start(struct ath10k_htc *htc); int ath10k_htc_connect_service(struct ath10k_htc *htc, struct ath10k_htc_svc_conn_req *conn_req, struct ath10k_htc_svc_conn_resp *conn_resp); +void ath10k_htc_change_tx_credit_flow(struct ath10k_htc *htc, + enum ath10k_htc_ep_id eid, + bool enable); int ath10k_htc_send(struct ath10k_htc *htc, enum ath10k_htc_ep_id eid, struct sk_buff *packet); struct sk_buff *ath10k_htc_alloc_skb(struct ath10k *ar, int size); diff --git a/drivers/net/wireless/ath/ath10k/htt.c b/drivers/net/wireless/ath/ath10k/htt.c index 7b75200ceae5..4354bf285ff1 100644 --- a/drivers/net/wireless/ath/ath10k/htt.c +++ b/drivers/net/wireless/ath/ath10k/htt.c @@ -10,6 +10,7 @@ #include "htt.h" #include "core.h" #include "debug.h" +#include "hif.h" static const enum htt_t2h_msg_type htt_main_t2h_msg_types[] = { [HTT_MAIN_T2H_MSG_TYPE_VERSION_CONF] = HTT_T2H_MSG_TYPE_VERSION_CONF, @@ -153,6 +154,10 @@ int ath10k_htt_connect(struct ath10k_htt *htt) htt->eid = conn_resp.eid; + htt->disable_tx_comp = ath10k_hif_get_htt_tx_complete(htt->ar); + if (htt->disable_tx_comp) + ath10k_htc_change_tx_credit_flow(&htt->ar->htc, htt->eid, true); + return 0; } diff --git a/drivers/net/wireless/ath/ath10k/htt.h b/drivers/net/wireless/ath/ath10k/htt.h index 30c080094af1..889bf9fe051a 100644 --- a/drivers/net/wireless/ath/ath10k/htt.h +++ b/drivers/net/wireless/ath/ath10k/htt.h @@ -150,9 +150,19 @@ enum htt_data_tx_desc_flags1 { HTT_DATA_TX_DESC_FLAGS1_MORE_IN_BATCH = 1 << 12, HTT_DATA_TX_DESC_FLAGS1_CKSUM_L3_OFFLOAD = 1 << 13, HTT_DATA_TX_DESC_FLAGS1_CKSUM_L4_OFFLOAD = 1 << 14, - HTT_DATA_TX_DESC_FLAGS1_RSVD1 = 1 << 15 + HTT_DATA_TX_DESC_FLAGS1_TX_COMPLETE = 1 << 15 }; +#define HTT_TX_CREDIT_DELTA_ABS_M 0xffff0000 +#define HTT_TX_CREDIT_DELTA_ABS_S 16 +#define HTT_TX_CREDIT_DELTA_ABS_GET(word) \ + (((word) & HTT_TX_CREDIT_DELTA_ABS_M) >> HTT_TX_CREDIT_DELTA_ABS_S) + +#define HTT_TX_CREDIT_SIGN_BIT_M 0x00000100 +#define HTT_TX_CREDIT_SIGN_BIT_S 8 +#define HTT_TX_CREDIT_SIGN_BIT_GET(word) \ + (((word) & HTT_TX_CREDIT_SIGN_BIT_M) >> HTT_TX_CREDIT_SIGN_BIT_S) + enum htt_data_tx_ext_tid { HTT_DATA_TX_EXT_TID_NON_QOS_MCAST_BCAST = 16, HTT_DATA_TX_EXT_TID_MGMT = 17, @@ -2019,6 +2029,7 @@ struct ath10k_htt { bool tx_mem_allocated; const struct ath10k_htt_tx_ops *tx_ops; const struct ath10k_htt_rx_ops *rx_ops; + bool disable_tx_comp; }; struct ath10k_htt_tx_ops { diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c index d95b63f133ab..049861f38e95 100644 --- a/drivers/net/wireless/ath/ath10k/htt_rx.c +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c @@ -3778,6 +3778,9 @@ bool ath10k_htt_t2h_msg_handler(struct ath10k *ar, struct sk_buff *skb) } case HTT_T2H_MSG_TYPE_MGMT_TX_COMPLETION: { struct htt_tx_done tx_done = {}; + struct ath10k_htt *htt = &ar->htt; + struct ath10k_htc *htc = &ar->htc; + struct ath10k_htc_ep *ep = &ar->htc.endpoint[htt->eid]; int status = __le32_to_cpu(resp->mgmt_tx_completion.status); int info = __le32_to_cpu(resp->mgmt_tx_completion.info); @@ -3803,6 +3806,12 @@ bool ath10k_htt_t2h_msg_handler(struct ath10k *ar, struct sk_buff *skb) break; } + if (htt->disable_tx_comp) { + spin_lock_bh(&htc->tx_lock); + ep->tx_credits++; + spin_unlock_bh(&htc->tx_lock); + } + status = ath10k_txrx_tx_unref(htt, &tx_done); if (!status) { spin_lock_bh(&htt->tx_lock); @@ -3877,8 +3886,31 @@ bool ath10k_htt_t2h_msg_handler(struct ath10k *ar, struct sk_buff *skb) skb_queue_tail(&htt->rx_in_ord_compl_q, skb); return false; } - case HTT_T2H_MSG_TYPE_TX_CREDIT_UPDATE_IND: + case HTT_T2H_MSG_TYPE_TX_CREDIT_UPDATE_IND: { + struct ath10k_htt *htt = &ar->htt; + struct ath10k_htc *htc = &ar->htc; + struct ath10k_htc_ep *ep = &ar->htc.endpoint[htt->eid]; + u32 msg_word = __le32_to_cpu(*(__le32 *)resp); + int htt_credit_delta; + + htt_credit_delta = HTT_TX_CREDIT_DELTA_ABS_GET(msg_word); + if (HTT_TX_CREDIT_SIGN_BIT_GET(msg_word)) + htt_credit_delta = -htt_credit_delta; + + ath10k_dbg(ar, ATH10K_DBG_HTT, + "credit update: delta:%d\n", + htt_credit_delta); + + if (htt->disable_tx_comp) { + spin_lock_bh(&htc->tx_lock); + ep->tx_credits += htt_credit_delta; + spin_unlock_bh(&htc->tx_lock); + ath10k_dbg(ar, ATH10K_DBG_HTT, + "credit total:%d\n", + ep->tx_credits); + } break; + } case HTT_T2H_MSG_TYPE_CHAN_CHANGE: { u32 phymode = __le32_to_cpu(resp->chan_change.phymode); u32 freq = __le32_to_cpu(resp->chan_change.freq); diff --git a/drivers/net/wireless/ath/ath10k/htt_tx.c b/drivers/net/wireless/ath/ath10k/htt_tx.c index a182c0944cc7..c6c4b2a4d20f 100644 --- a/drivers/net/wireless/ath/ath10k/htt_tx.c +++ b/drivers/net/wireless/ath/ath10k/htt_tx.c @@ -543,7 +543,35 @@ void ath10k_htt_tx_free(struct ath10k_htt *htt) void ath10k_htt_htc_tx_complete(struct ath10k *ar, struct sk_buff *skb) { + struct ath10k_htt *htt = &ar->htt; + struct htt_tx_done tx_done = {0}; + struct htt_cmd_hdr *htt_hdr; + struct htt_data_tx_desc *desc_hdr; + u16 flags1; + dev_kfree_skb_any(skb); + + if (!htt->disable_tx_comp) + return; + + htt_hdr = (struct htt_cmd_hdr *)skb->data; + if (htt_hdr->msg_type != HTT_H2T_MSG_TYPE_TX_FRM) + return; + + desc_hdr = (struct htt_data_tx_desc *) + (skb->data + sizeof(*htt_hdr)); + flags1 = __le16_to_cpu(desc_hdr->flags1); + + ath10k_dbg(ar, ATH10K_DBG_HTT, + "htt tx complete msdu id:%u ,flags1:%x\n", + __le16_to_cpu(desc_hdr->id), flags1); + + if (flags1 & HTT_DATA_TX_DESC_FLAGS1_TX_COMPLETE) + return; + + tx_done.status = HTT_TX_COMPL_STATE_ACK; + tx_done.msdu_id = __le16_to_cpu(desc_hdr->id); + ath10k_txrx_tx_unref(&ar->htt, &tx_done); } void ath10k_htt_hif_tx_complete(struct ath10k *ar, struct sk_buff *skb) @@ -1269,6 +1297,9 @@ static int ath10k_htt_tx_hl(struct ath10k_htt *htt, enum ath10k_hw_txrx_mode txm flags0 |= SM(ATH10K_HW_TXRX_MGMT, HTT_DATA_TX_DESC_FLAGS0_PKT_TYPE); flags0 |= HTT_DATA_TX_DESC_FLAGS0_MAC_HDR_PRESENT; + + if (htt->disable_tx_comp) + flags1 |= HTT_DATA_TX_DESC_FLAGS1_TX_COMPLETE; break; } diff --git a/drivers/net/wireless/ath/ath10k/hw.h b/drivers/net/wireless/ath/ath10k/hw.h index 35a362329a4f..2661c6893ca5 100644 --- a/drivers/net/wireless/ath/ath10k/hw.h +++ b/drivers/net/wireless/ath/ath10k/hw.h @@ -762,7 +762,7 @@ ath10k_is_rssi_enable(struct ath10k_hw_params *hw, #define TARGET_TLV_NUM_TDLS_VDEVS 1 #define TARGET_TLV_NUM_TIDS ((TARGET_TLV_NUM_PEERS) * 2) #define TARGET_TLV_NUM_MSDU_DESC (1024 + 32) -#define TARGET_TLV_NUM_MSDU_DESC_HL 64 +#define TARGET_TLV_NUM_MSDU_DESC_HL 1024 #define TARGET_TLV_NUM_WOW_PATTERNS 22 #define TARGET_TLV_MGMT_NUM_MSDU_DESC (50) diff --git a/drivers/net/wireless/ath/ath10k/sdio.c b/drivers/net/wireless/ath/ath10k/sdio.c index 120200a93bcc..84910446df50 100644 --- a/drivers/net/wireless/ath/ath10k/sdio.c +++ b/drivers/net/wireless/ath/ath10k/sdio.c @@ -1674,6 +1674,28 @@ static int ath10k_sdio_hif_swap_mailbox(struct ath10k *ar) return 0; } +static int ath10k_sdio_get_htt_tx_complete(struct ath10k *ar) +{ + u32 addr, val; + int ret; + + addr = host_interest_item_address(HI_ITEM(hi_acs_flags)); + + ret = ath10k_sdio_hif_diag_read32(ar, addr, &val); + if (ret) { + ath10k_warn(ar, + "unable to read hi_acs_flags for htt tx comple : %d\n", ret); + return ret; + } + + ret = (val & HI_ACS_FLAGS_SDIO_REDUCE_TX_COMPL_FW_ACK); + + ath10k_dbg(ar, ATH10K_DBG_SDIO, "sdio reduce tx complete fw%sack\n", + ret ? " " : " not "); + + return ret; +} + /* HIF start/stop */ static int ath10k_sdio_hif_start(struct ath10k *ar) @@ -1943,6 +1965,7 @@ static const struct ath10k_hif_ops ath10k_sdio_hif_ops = { .start = ath10k_sdio_hif_start, .stop = ath10k_sdio_hif_stop, .swap_mailbox = ath10k_sdio_hif_swap_mailbox, + .get_htt_tx_complete = ath10k_sdio_get_htt_tx_complete, .map_service_to_pipe = ath10k_sdio_hif_map_service_to_pipe, .get_default_pipe = ath10k_sdio_hif_get_default_pipe, .send_complete_check = ath10k_sdio_hif_send_complete_check,
For sdio chip, it is high latency bus, all the TX packet's content will be tranferred from HOST memory to firmware memory via sdio bus, then it need much more memory in firmware than low latency bus chip, for low latency chip, such as PCI-E, it only need to transfer the TX descriptor via PCI-E bus to firmware memory. For sdio chip, reduce the complexity of TX logic will help TX efficiency since its memory is limited, and it will reduce the TX circle's time of each packet and then firmware will have more memory for TX since TX complete also need memeory. This patch disable TX complete indication from firmware for htt data packet, it will not have TX complete indication from firmware to ath10k. It will cut the cost of bus bandwidth of TX complete and make the TX logic of firmware simpler, it results in significant performance improvement on TX path. Udp TX throughout is 130Mbps without this patch, and it arrives 400Mbps with this patch. The downside of this patch is the command "iw wlan0 station dump" will show 0 for "tx retries" and "tx failed" since all tx packet's status is success. This patch only effect sdio chip, it will not effect PCI, SNOC etc. Tested with QCA6174 SDIO with firmware WLAN.RMH.4.4.1-00017-QCARMSWPZ-1 Signed-off-by: Wen Gong <wgong@codeaurora.org> --- drivers/net/wireless/ath/ath10k/core.c | 5 +--- drivers/net/wireless/ath/ath10k/hif.h | 9 +++++++ drivers/net/wireless/ath/ath10k/htc.c | 10 +++++++ drivers/net/wireless/ath/ath10k/htc.h | 3 +++ drivers/net/wireless/ath/ath10k/htt.c | 5 ++++ drivers/net/wireless/ath/ath10k/htt.h | 13 ++++++++- drivers/net/wireless/ath/ath10k/htt_rx.c | 34 +++++++++++++++++++++++- drivers/net/wireless/ath/ath10k/htt_tx.c | 31 +++++++++++++++++++++ drivers/net/wireless/ath/ath10k/hw.h | 2 +- drivers/net/wireless/ath/ath10k/sdio.c | 23 ++++++++++++++++ 10 files changed, 128 insertions(+), 7 deletions(-)