Allow adding rx_skb to rx_free_queue for later reuse on the common error handling path, otherwise free it. Found by Linux Verification Center (linuxtesting.org). Fixes: 2135c28be6a8 ("wifi: rtw89: Add usb.{c,h}") Signed-off-by: Fedor Pchelkin --- v2: - do goto 'free_or_reuse' (Ping-Ke) drivers/net/wireless/realtek/rtw89/usb.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/net/wireless/realtek/rtw89/usb.c b/drivers/net/wireless/realtek/rtw89/usb.c index 6cf89aee252e..e8e064cf7e0a 100644 --- a/drivers/net/wireless/realtek/rtw89/usb.c +++ b/drivers/net/wireless/realtek/rtw89/usb.c @@ -410,8 +410,7 @@ static void rtw89_usb_rx_handler(struct work_struct *work) if (skb_queue_len(&rtwusb->rx_queue) >= RTW89_USB_MAX_RXQ_LEN) { rtw89_warn(rtwdev, "rx_queue overflow\n"); - dev_kfree_skb_any(rx_skb); - continue; + goto free_or_reuse; } memset(&desc_info, 0, sizeof(desc_info)); @@ -422,7 +421,7 @@ static void rtw89_usb_rx_handler(struct work_struct *work) rtw89_debug(rtwdev, RTW89_DBG_HCI, "failed to allocate RX skb of size %u\n", desc_info.pkt_size); - continue; + goto free_or_reuse; } pkt_offset = desc_info.offset + desc_info.rxd_len; @@ -432,6 +431,7 @@ static void rtw89_usb_rx_handler(struct work_struct *work) rtw89_core_rx(rtwdev, &desc_info, skb); +free_or_reuse: if (skb_queue_len(&rtwusb->rx_free_queue) >= RTW89_USB_RX_SKB_NUM) dev_kfree_skb_any(rx_skb); else -- 2.51.0 When there is an attempt to write data and RTW89_FLAG_UNPLUGGED is set, this means device is disconnected and no urb is submitted. Return appropriate error code to the caller to properly free the allocated resources. Found by Linux Verification Center (linuxtesting.org). Fixes: 2135c28be6a8 ("wifi: rtw89: Add usb.{c,h}") Signed-off-by: Fedor Pchelkin Acked-by: Ping-Ke Shih --- drivers/net/wireless/realtek/rtw89/usb.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/net/wireless/realtek/rtw89/usb.c b/drivers/net/wireless/realtek/rtw89/usb.c index e8e064cf7e0a..512a46dd9d06 100644 --- a/drivers/net/wireless/realtek/rtw89/usb.c +++ b/drivers/net/wireless/realtek/rtw89/usb.c @@ -256,7 +256,7 @@ static int rtw89_usb_write_port(struct rtw89_dev *rtwdev, u8 ch_dma, int ret; if (test_bit(RTW89_FLAG_UNPLUGGED, rtwdev->flags)) - return 0; + return -ENODEV; urb = usb_alloc_urb(0, GFP_ATOMIC); if (!urb) @@ -305,8 +305,9 @@ static void rtw89_usb_ops_tx_kick_off(struct rtw89_dev *rtwdev, u8 txch) ret = rtw89_usb_write_port(rtwdev, txch, skb->data, skb->len, txcb); if (ret) { - rtw89_err(rtwdev, "write port txch %d failed: %d\n", - txch, ret); + if (ret != -ENODEV) + rtw89_err(rtwdev, "write port txch %d failed: %d\n", + txch, ret); skb_dequeue(&txcb->tx_ack_queue); kfree(txcb); -- 2.51.0 rtw89_usb_ops_tx_kick_off() may need to release skb if a failure occurs. It operates mainly on skbs coming from the core wireless stack and the ones containing firmware commands. Use ieee80211_free_txskb() for the former case. Suggested-by: Ping-Ke Shih Signed-off-by: Fedor Pchelkin --- drivers/net/wireless/realtek/rtw89/usb.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/drivers/net/wireless/realtek/rtw89/usb.c b/drivers/net/wireless/realtek/rtw89/usb.c index 512a46dd9d06..655e8437d62e 100644 --- a/drivers/net/wireless/realtek/rtw89/usb.c +++ b/drivers/net/wireless/realtek/rtw89/usb.c @@ -278,6 +278,15 @@ static int rtw89_usb_write_port(struct rtw89_dev *rtwdev, u8 ch_dma, return ret; } +static void rtw89_usb_tx_free_skb(struct rtw89_dev *rtwdev, u8 txch, + struct sk_buff *skb) +{ + if (txch == RTW89_TXCH_CH12) + dev_kfree_skb_any(skb); + else + ieee80211_free_txskb(rtwdev->hw, skb); +} + static void rtw89_usb_ops_tx_kick_off(struct rtw89_dev *rtwdev, u8 txch) { struct rtw89_usb *rtwusb = rtw89_usb_priv(rtwdev); @@ -292,7 +301,7 @@ static void rtw89_usb_ops_tx_kick_off(struct rtw89_dev *rtwdev, u8 txch) txcb = kmalloc(sizeof(*txcb), GFP_ATOMIC); if (!txcb) { - dev_kfree_skb_any(skb); + rtw89_usb_tx_free_skb(rtwdev, txch, skb); continue; } @@ -311,7 +320,7 @@ static void rtw89_usb_ops_tx_kick_off(struct rtw89_dev *rtwdev, u8 txch) skb_dequeue(&txcb->tx_ack_queue); kfree(txcb); - dev_kfree_skb_any(skb); + rtw89_usb_tx_free_skb(rtwdev, txch, skb); } } } -- 2.51.0 Pass TX status value directly into rtw89_core_tx_wait_complete(). This will make it a bit in sync with further patches and will give flexibility in future work. Also use scope based RCU locking which simplifies the code of the function. Found by Linux Verification Center (linuxtesting.org). Signed-off-by: Fedor Pchelkin --- drivers/net/wireless/realtek/rtw89/core.h | 20 ++++++++++---------- drivers/net/wireless/realtek/rtw89/pci.c | 2 +- drivers/net/wireless/realtek/rtw89/pci.h | 4 ---- 3 files changed, 11 insertions(+), 15 deletions(-) diff --git a/drivers/net/wireless/realtek/rtw89/core.h b/drivers/net/wireless/realtek/rtw89/core.h index 928c8c84c964..60e32894d8b4 100644 --- a/drivers/net/wireless/realtek/rtw89/core.h +++ b/drivers/net/wireless/realtek/rtw89/core.h @@ -3507,6 +3507,11 @@ struct rtw89_phy_rate_pattern { bool enable; }; +#define RTW89_TX_DONE 0x0 +#define RTW89_TX_RETRY_LIMIT 0x1 +#define RTW89_TX_LIFE_TIME 0x2 +#define RTW89_TX_MACID_DROP 0x3 + #define RTW89_TX_WAIT_WORK_TIMEOUT msecs_to_jiffies(500) struct rtw89_tx_wait_info { struct rcu_head rcu_head; @@ -7374,25 +7379,20 @@ static inline struct sk_buff *rtw89_alloc_skb_for_rx(struct rtw89_dev *rtwdev, static inline bool rtw89_core_tx_wait_complete(struct rtw89_dev *rtwdev, struct rtw89_tx_skb_data *skb_data, - bool tx_done) + u8 tx_status) { struct rtw89_tx_wait_info *wait; - bool ret = false; - rcu_read_lock(); + guard(rcu)(); wait = rcu_dereference(skb_data->wait); if (!wait) - goto out; + return false; - ret = true; - wait->tx_done = tx_done; + wait->tx_done = tx_status == RTW89_TX_DONE; /* Don't access skb anymore after completion */ complete_all(&wait->completion); - -out: - rcu_read_unlock(); - return ret; + return true; } static inline bool rtw89_is_mlo_1_1(struct rtw89_dev *rtwdev) diff --git a/drivers/net/wireless/realtek/rtw89/pci.c b/drivers/net/wireless/realtek/rtw89/pci.c index 0ee5f8579447..b1985193a18f 100644 --- a/drivers/net/wireless/realtek/rtw89/pci.c +++ b/drivers/net/wireless/realtek/rtw89/pci.c @@ -464,7 +464,7 @@ static void rtw89_pci_tx_status(struct rtw89_dev *rtwdev, struct rtw89_tx_skb_data *skb_data = RTW89_TX_SKB_CB(skb); struct ieee80211_tx_info *info; - if (rtw89_core_tx_wait_complete(rtwdev, skb_data, tx_status == RTW89_TX_DONE)) + if (rtw89_core_tx_wait_complete(rtwdev, skb_data, tx_status)) return; info = IEEE80211_SKB_CB(skb); diff --git a/drivers/net/wireless/realtek/rtw89/pci.h b/drivers/net/wireless/realtek/rtw89/pci.h index cb05c83dfd56..16dfb0e79d77 100644 --- a/drivers/net/wireless/realtek/rtw89/pci.h +++ b/drivers/net/wireless/realtek/rtw89/pci.h @@ -1487,10 +1487,6 @@ struct rtw89_pci_tx_addr_info_32_v1 { #define RTW89_PCI_RPP_POLLUTED BIT(31) #define RTW89_PCI_RPP_SEQ GENMASK(30, 16) #define RTW89_PCI_RPP_TX_STATUS GENMASK(15, 13) -#define RTW89_TX_DONE 0x0 -#define RTW89_TX_RETRY_LIMIT 0x1 -#define RTW89_TX_LIFE_TIME 0x2 -#define RTW89_TX_MACID_DROP 0x3 #define RTW89_PCI_RPP_QSEL GENMASK(12, 8) #define RTW89_PCI_RPP_MACID GENMASK(7, 0) -- 2.51.0 rtw89 has several ways of handling TX status report events. The first one is based on RPP feature which is used by PCIe HCI. The other one depends on firmware sending a corresponding C2H message, quite similar to what rtw88 has. Toggle a bit in the TX descriptor and place skb in a queue to wait for a message from the firmware. rtw89 has an extra feature providing TX reports for multiple retry transmission attempts. When there is a failed TX status reported by the firmware, the report is ignored until the limit is reached or success status appears. Do all this according to the vendor driver for RTL8851BU. It seems the only way to implement TX status reporting for rtw89 USB. This will allow handling TX wait skbs and the ones flagged with IEEE80211_TX_CTL_REQ_TX_STATUS correctly. Found by Linux Verification Center (linuxtesting.org). Suggested-by: Bitterblue Smith Signed-off-by: Fedor Pchelkin --- v2: - fix bit masks and consider TX transmission retry limit for TX reporting (Bitterblue) - use newer style for C2H message type definitions and drop unimplemented funcs from 'enum rtw89_mac_c2h_misc_func' (Ping-Ke) - modify rtw89_core_fill_txdesc_v1() and rtw89_core_fill_txdesc_v2() accordingly (Ping-Ke) Well, it took me awhile to figure out how those V0, V1 and V2 parts are structured in rtw89/txrx.h and match them with definitions from the vendor driver. Probably BE_TXD_* section should be marked as V2 by a comment.. Anyway, I've decided to activate TX report bits for V1 and V2 as well though can't experiment with them, firmware on my side follows V0. drivers/net/wireless/realtek/rtw89/core.c | 28 +++++++++++++++++++---- drivers/net/wireless/realtek/rtw89/core.h | 4 ++++ drivers/net/wireless/realtek/rtw89/fw.h | 14 ++++++++++++ drivers/net/wireless/realtek/rtw89/mac.c | 26 +++++++++++++++++++++ drivers/net/wireless/realtek/rtw89/mac.h | 7 ++++++ drivers/net/wireless/realtek/rtw89/txrx.h | 6 ++++- 6 files changed, 79 insertions(+), 6 deletions(-) diff --git a/drivers/net/wireless/realtek/rtw89/core.c b/drivers/net/wireless/realtek/rtw89/core.c index 917b2adede61..49ecc248464b 100644 --- a/drivers/net/wireless/realtek/rtw89/core.c +++ b/drivers/net/wireless/realtek/rtw89/core.c @@ -1396,7 +1396,10 @@ static __le32 rtw89_build_txwd_info1(struct rtw89_tx_desc_info *desc_info) u32 dword = FIELD_PREP(RTW89_TXWD_INFO1_MAX_AGGNUM, desc_info->ampdu_num) | FIELD_PREP(RTW89_TXWD_INFO1_A_CTRL_BSR, desc_info->a_ctrl_bsr) | FIELD_PREP(RTW89_TXWD_INFO1_DATA_RTY_LOWEST_RATE, - desc_info->data_retry_lowest_rate); + desc_info->data_retry_lowest_rate) | + FIELD_PREP(RTW89_TXWD_INFO1_DATA_TXCNT_LMT_SEL, + desc_info->tx_cnt_lmt_en) | + FIELD_PREP(RTW89_TXWD_INFO1_DATA_TXCNT_LMT, desc_info->tx_cnt_lmt); return cpu_to_le32(dword); } @@ -1420,11 +1423,19 @@ static __le32 rtw89_build_txwd_info2_v1(struct rtw89_tx_desc_info *desc_info) return cpu_to_le32(dword); } +static __le32 rtw89_build_txwd_info3(struct rtw89_tx_desc_info *desc_info) +{ + u32 dword = FIELD_PREP(RTW89_TXWD_INFO3_SPE_RPT, desc_info->report); + + return cpu_to_le32(dword); +} + static __le32 rtw89_build_txwd_info4(struct rtw89_tx_desc_info *desc_info) { bool rts_en = !desc_info->is_bmc; u32 dword = FIELD_PREP(RTW89_TXWD_INFO4_RTS_EN, rts_en) | - FIELD_PREP(RTW89_TXWD_INFO4_HW_RTS_EN, 1); + FIELD_PREP(RTW89_TXWD_INFO4_HW_RTS_EN, 1) | + FIELD_PREP(RTW89_TXWD_INFO4_SW_DEFINE, desc_info->sn); return cpu_to_le32(dword); } @@ -1447,6 +1458,7 @@ void rtw89_core_fill_txdesc(struct rtw89_dev *rtwdev, txwd_info->dword0 = rtw89_build_txwd_info0(desc_info); txwd_info->dword1 = rtw89_build_txwd_info1(desc_info); txwd_info->dword2 = rtw89_build_txwd_info2(desc_info); + txwd_info->dword3 = rtw89_build_txwd_info3(desc_info); txwd_info->dword4 = rtw89_build_txwd_info4(desc_info); } @@ -1476,6 +1488,7 @@ void rtw89_core_fill_txdesc_v1(struct rtw89_dev *rtwdev, txwd_info->dword0 = rtw89_build_txwd_info0_v1(desc_info); txwd_info->dword1 = rtw89_build_txwd_info1(desc_info); txwd_info->dword2 = rtw89_build_txwd_info2_v1(desc_info); + txwd_info->dword3 = rtw89_build_txwd_info3(desc_info); txwd_info->dword4 = rtw89_build_txwd_info4(desc_info); } EXPORT_SYMBOL(rtw89_core_fill_txdesc_v1); @@ -1561,7 +1574,10 @@ static __le32 rtw89_build_txwd_info0_v2(struct rtw89_tx_desc_info *desc_info) u32 dword = FIELD_PREP(BE_TXD_INFO0_DATA_STBC, desc_info->stbc) | FIELD_PREP(BE_TXD_INFO0_DATA_LDPC, desc_info->ldpc) | FIELD_PREP(BE_TXD_INFO0_DISDATAFB, desc_info->dis_data_fb) | - FIELD_PREP(BE_TXD_INFO0_MULTIPORT_ID, desc_info->port); + FIELD_PREP(BE_TXD_INFO0_MULTIPORT_ID, desc_info->port) | + FIELD_PREP(BE_TXD_INFO0_DATA_TXCNT_LMT_SEL, + desc_info->tx_cnt_lmt_en) | + FIELD_PREP(BE_TXD_INFO0_DATA_TXCNT_LMT, desc_info->tx_cnt_lmt); return cpu_to_le32(dword); } @@ -1571,7 +1587,8 @@ static __le32 rtw89_build_txwd_info1_v2(struct rtw89_tx_desc_info *desc_info) u32 dword = FIELD_PREP(BE_TXD_INFO1_MAX_AGG_NUM, desc_info->ampdu_num) | FIELD_PREP(BE_TXD_INFO1_A_CTRL_BSR, desc_info->a_ctrl_bsr) | FIELD_PREP(BE_TXD_INFO1_DATA_RTY_LOWEST_RATE, - desc_info->data_retry_lowest_rate); + desc_info->data_retry_lowest_rate) | + FIELD_PREP(BE_TXD_INFO1_SW_DEFINE, desc_info->sn); return cpu_to_le32(dword); } @@ -1580,7 +1597,8 @@ static __le32 rtw89_build_txwd_info2_v2(struct rtw89_tx_desc_info *desc_info) { u32 dword = FIELD_PREP(BE_TXD_INFO2_AMPDU_DENSITY, desc_info->ampdu_density) | FIELD_PREP(BE_TXD_INFO2_FORCE_KEY_EN, desc_info->sec_en) | - FIELD_PREP(BE_TXD_INFO2_SEC_CAM_IDX, desc_info->sec_cam_idx); + FIELD_PREP(BE_TXD_INFO2_SEC_CAM_IDX, desc_info->sec_cam_idx) | + FIELD_PREP(BE_TXD_INFO2_SPE_RPT_V1, desc_info->report); return cpu_to_le32(dword); } diff --git a/drivers/net/wireless/realtek/rtw89/core.h b/drivers/net/wireless/realtek/rtw89/core.h index 60e32894d8b4..66b7bfa5902e 100644 --- a/drivers/net/wireless/realtek/rtw89/core.h +++ b/drivers/net/wireless/realtek/rtw89/core.h @@ -1167,6 +1167,10 @@ struct rtw89_tx_desc_info { u8 ampdu_density; u8 ampdu_num; bool sec_en; + bool report; + bool tx_cnt_lmt_en; + u8 sn: 4; + u8 tx_cnt_lmt: 6; u8 addr_info_nr; u8 sec_keyid; u8 sec_type; diff --git a/drivers/net/wireless/realtek/rtw89/fw.h b/drivers/net/wireless/realtek/rtw89/fw.h index ddebf7972068..6d2f374244c4 100644 --- a/drivers/net/wireless/realtek/rtw89/fw.h +++ b/drivers/net/wireless/realtek/rtw89/fw.h @@ -3747,6 +3747,20 @@ struct rtw89_c2h_scanofld { #define RTW89_GET_MAC_C2H_MCC_REQ_ACK_H2C_FUNC(c2h) \ le32_get_bits(*((const __le32 *)(c2h) + 2), GENMASK(15, 8)) +struct rtw89_c2h_mac_tx_rpt { + struct rtw89_c2h_hdr hdr; + __le32 w2; + __le32 w3; + __le32 w4; + __le32 w5; + __le32 w6; + __le32 w7; +}; + +#define RTW89_C2H_MAC_TX_RPT_W2_TX_STATE GENMASK(7, 6) +#define RTW89_C2H_MAC_TX_RPT_W2_SW_DEFINE GENMASK(11, 8) +#define RTW89_C2H_MAC_TX_RPT_W5_DATA_TX_CNT GENMASK(13, 8) + struct rtw89_mac_mcc_tsf_rpt { u32 macid_x; u32 macid_y; diff --git a/drivers/net/wireless/realtek/rtw89/mac.c b/drivers/net/wireless/realtek/rtw89/mac.c index fd11b8fb3c89..10c2a39e544b 100644 --- a/drivers/net/wireless/realtek/rtw89/mac.c +++ b/drivers/net/wireless/realtek/rtw89/mac.c @@ -5457,6 +5457,20 @@ rtw89_mac_c2h_mcc_status_rpt(struct rtw89_dev *rtwdev, struct sk_buff *c2h, u32 rtw89_complete_cond(&rtwdev->mcc.wait, cond, &data); } +static void +rtw89_mac_c2h_tx_rpt(struct rtw89_dev *rtwdev, struct sk_buff *c2h, u32 len) +{ + const struct rtw89_c2h_mac_tx_rpt *rpt = + (const struct rtw89_c2h_mac_tx_rpt *)c2h->data; + u8 sw_define = le32_get_bits(rpt->w2, RTW89_C2H_MAC_TX_RPT_W2_SW_DEFINE); + u8 tx_status = le32_get_bits(rpt->w2, RTW89_C2H_MAC_TX_RPT_W2_TX_STATE); + u8 data_txcnt = le32_get_bits(rpt->w5, RTW89_C2H_MAC_TX_RPT_W5_DATA_TX_CNT); + + rtw89_debug(rtwdev, RTW89_DBG_TXRX, + "C2H TX RPT: sn %d, tx_status %d, data_txcnt %d\n", + sw_define, tx_status, data_txcnt); +} + static void rtw89_mac_c2h_mrc_tsf_rpt(struct rtw89_dev *rtwdev, struct sk_buff *c2h, u32 len) { @@ -5691,6 +5705,12 @@ void (* const rtw89_mac_c2h_mcc_handler[])(struct rtw89_dev *rtwdev, [RTW89_MAC_C2H_FUNC_MCC_STATUS_RPT] = rtw89_mac_c2h_mcc_status_rpt, }; +static +void (* const rtw89_mac_c2h_misc_handler[])(struct rtw89_dev *rtwdev, + struct sk_buff *c2h, u32 len) = { + [RTW89_MAC_C2H_FUNC_TX_REPORT] = rtw89_mac_c2h_tx_rpt, +}; + static void (* const rtw89_mac_c2h_mlo_handler[])(struct rtw89_dev *rtwdev, struct sk_buff *c2h, u32 len) = { @@ -5777,6 +5797,8 @@ bool rtw89_mac_c2h_chk_atomic(struct rtw89_dev *rtwdev, struct sk_buff *c2h, } case RTW89_MAC_C2H_CLASS_MCC: return true; + case RTW89_MAC_C2H_CLASS_MISC: + return true; case RTW89_MAC_C2H_CLASS_MLO: return true; case RTW89_MAC_C2H_CLASS_MRC: @@ -5812,6 +5834,10 @@ void rtw89_mac_c2h_handle(struct rtw89_dev *rtwdev, struct sk_buff *skb, if (func < NUM_OF_RTW89_MAC_C2H_FUNC_MCC) handler = rtw89_mac_c2h_mcc_handler[func]; break; + case RTW89_MAC_C2H_CLASS_MISC: + if (func < NUM_OF_RTW89_MAC_C2H_FUNC_MISC) + handler = rtw89_mac_c2h_misc_handler[func]; + break; case RTW89_MAC_C2H_CLASS_MLO: if (func < NUM_OF_RTW89_MAC_C2H_FUNC_MLO) handler = rtw89_mac_c2h_mlo_handler[func]; diff --git a/drivers/net/wireless/realtek/rtw89/mac.h b/drivers/net/wireless/realtek/rtw89/mac.h index 25fe5e5c8a97..15c5c7e4033c 100644 --- a/drivers/net/wireless/realtek/rtw89/mac.h +++ b/drivers/net/wireless/realtek/rtw89/mac.h @@ -432,6 +432,12 @@ enum rtw89_mac_c2h_mcc_func { NUM_OF_RTW89_MAC_C2H_FUNC_MCC, }; +enum rtw89_mac_c2h_misc_func { + RTW89_MAC_C2H_FUNC_TX_REPORT = 1, + + NUM_OF_RTW89_MAC_C2H_FUNC_MISC, +}; + enum rtw89_mac_c2h_mlo_func { RTW89_MAC_C2H_FUNC_MLO_GET_TBL = 0x0, RTW89_MAC_C2H_FUNC_MLO_EMLSR_TRANS_DONE = 0x1, @@ -470,6 +476,7 @@ enum rtw89_mac_c2h_class { RTW89_MAC_C2H_CLASS_WOW = 0x3, RTW89_MAC_C2H_CLASS_MCC = 0x4, RTW89_MAC_C2H_CLASS_FWDBG = 0x5, + RTW89_MAC_C2H_CLASS_MISC = 0x9, RTW89_MAC_C2H_CLASS_MLO = 0xc, RTW89_MAC_C2H_CLASS_MRC = 0xe, RTW89_MAC_C2H_CLASS_AP = 0x18, diff --git a/drivers/net/wireless/realtek/rtw89/txrx.h b/drivers/net/wireless/realtek/rtw89/txrx.h index 984c9fdbb018..b37dbac7b790 100644 --- a/drivers/net/wireless/realtek/rtw89/txrx.h +++ b/drivers/net/wireless/realtek/rtw89/txrx.h @@ -127,6 +127,8 @@ static inline u8 rtw89_get_data_nss(struct rtw89_dev *rtwdev, u16 hw_rate) #define RTW89_TXWD_INFO0_MULTIPORT_ID GENMASK(6, 4) /* TX WD INFO DWORD 1 */ +#define RTW89_TXWD_INFO1_DATA_TXCNT_LMT_SEL BIT(31) +#define RTW89_TXWD_INFO1_DATA_TXCNT_LMT GENMASK(30, 25) #define RTW89_TXWD_INFO1_DATA_RTY_LOWEST_RATE GENMASK(24, 16) #define RTW89_TXWD_INFO1_A_CTRL_BSR BIT(14) #define RTW89_TXWD_INFO1_MAX_AGGNUM GENMASK(7, 0) @@ -139,10 +141,12 @@ static inline u8 rtw89_get_data_nss(struct rtw89_dev *rtwdev, u16 hw_rate) #define RTW89_TXWD_INFO2_SEC_CAM_IDX GENMASK(7, 0) /* TX WD INFO DWORD 3 */ +#define RTW89_TXWD_INFO3_SPE_RPT BIT(10) /* TX WD INFO DWORD 4 */ -#define RTW89_TXWD_INFO4_RTS_EN BIT(27) #define RTW89_TXWD_INFO4_HW_RTS_EN BIT(31) +#define RTW89_TXWD_INFO4_RTS_EN BIT(27) +#define RTW89_TXWD_INFO4_SW_DEFINE GENMASK(3, 0) /* TX WD INFO DWORD 5 */ -- 2.51.0 Frames flagged with IEEE80211_TX_CTL_REQ_TX_STATUS mean the driver has to report to mac80211 stack whether AP sent ACK for the null frame/probe request or not. It's not implemented in USB part of the driver yet. PCIe HCI has its own way of getting TX status incorporated into RPP feature, and it's always enabled there. Other HCIs need a different scheme based on processing C2H messages. Thus define a .tx_rpt_enable flag defining which HCIs would need a TX report feature. Currently it is USB only. Do skb handling / queueing part quite similar to what rtw88 has. Store the flagged skbs in a queue for further processing in a C2H handler. Flush the queue on HCI reset and on timeout via delayed work handler used for TX waits - it's convenient since the further changes will pass TX wait skbs management to the same TX report queue. Found by Linux Verification Center (linuxtesting.org). Signed-off-by: Fedor Pchelkin --- v2: - update TX rpt description in rtw89_core_tx_update_desc_info() - add a flag in rtw89_hci_info to determine if we should enable TX report (Ping-Ke) - refine rtw89_tx_rpt_queue_purge() - it's called on USB HCI reset and at rtw89_tx_wait_work. Queueing delayed rtw89_tx_wait_work may be suboptimal but basically it's what rtw88 does with timer stuff. We can drop it and rely only on HCI reset to free remaining buffers in case firmware didn't send any TX status report. I'd like to know your thoughts on this. drivers/net/wireless/realtek/rtw89/core.c | 9 ++++- drivers/net/wireless/realtek/rtw89/core.h | 6 ++++ drivers/net/wireless/realtek/rtw89/mac.c | 19 ++++++++++ drivers/net/wireless/realtek/rtw89/mac.h | 43 +++++++++++++++++++++++ drivers/net/wireless/realtek/rtw89/usb.c | 14 +++++++- 5 files changed, 89 insertions(+), 2 deletions(-) diff --git a/drivers/net/wireless/realtek/rtw89/core.c b/drivers/net/wireless/realtek/rtw89/core.c index 49ecc248464b..214924f8bee0 100644 --- a/drivers/net/wireless/realtek/rtw89/core.c +++ b/drivers/net/wireless/realtek/rtw89/core.c @@ -1107,6 +1107,9 @@ rtw89_core_tx_update_desc_info(struct rtw89_dev *rtwdev, tx_req->rtwsta_link); if (addr_cam->valid && desc_info->mlo) upd_wlan_hdr = true; + + if (info->flags & IEEE80211_TX_CTL_REQ_TX_STATUS) + rtw89_tx_rpt_enable(rtwdev, tx_req); } is_bmc = (is_broadcast_ether_addr(hdr->addr1) || is_multicast_ether_addr(hdr->addr1)); @@ -1140,7 +1143,10 @@ static void rtw89_tx_wait_work(struct wiphy *wiphy, struct wiphy_work *work) struct rtw89_dev *rtwdev = container_of(work, struct rtw89_dev, tx_wait_work.work); - rtw89_tx_wait_list_clear(rtwdev); + if (!rtwdev->hci.tx_rpt_enable) + rtw89_tx_wait_list_clear(rtwdev); + else + rtw89_tx_rpt_queue_purge(rtwdev); } void rtw89_core_tx_kick_off(struct rtw89_dev *rtwdev, u8 qsel) @@ -5847,6 +5853,7 @@ int rtw89_core_init(struct rtw89_dev *rtwdev) wiphy_work_init(&rtwdev->cancel_6ghz_probe_work, rtw89_cancel_6ghz_probe_work); INIT_WORK(&rtwdev->load_firmware_work, rtw89_load_firmware_work); + skb_queue_head_init(&rtwdev->tx_rpt_queue); skb_queue_head_init(&rtwdev->c2h_queue); rtw89_core_ppdu_sts_init(rtwdev); rtw89_traffic_stats_init(rtwdev, &rtwdev->stats); diff --git a/drivers/net/wireless/realtek/rtw89/core.h b/drivers/net/wireless/realtek/rtw89/core.h index 66b7bfa5902e..3940e54353d3 100644 --- a/drivers/net/wireless/realtek/rtw89/core.h +++ b/drivers/net/wireless/realtek/rtw89/core.h @@ -3527,6 +3527,8 @@ struct rtw89_tx_wait_info { struct rtw89_tx_skb_data { struct rtw89_tx_wait_info __rcu *wait; + u8 tx_rpt_sn; + u8 tx_pkt_cnt_lmt; u8 hci_priv[]; }; @@ -3696,6 +3698,7 @@ struct rtw89_hci_info { u32 rpwm_addr; u32 cpwm_addr; bool paused; + bool tx_rpt_enable; }; struct rtw89_chip_ops { @@ -6015,6 +6018,9 @@ struct rtw89_dev { struct list_head tx_waits; struct wiphy_delayed_work tx_wait_work; + atomic_t sn; + struct sk_buff_head tx_rpt_queue; + struct rtw89_cam_info cam_info; struct sk_buff_head c2h_queue; diff --git a/drivers/net/wireless/realtek/rtw89/mac.c b/drivers/net/wireless/realtek/rtw89/mac.c index 10c2a39e544b..75d9efac452b 100644 --- a/drivers/net/wireless/realtek/rtw89/mac.c +++ b/drivers/net/wireless/realtek/rtw89/mac.c @@ -5465,10 +5465,29 @@ rtw89_mac_c2h_tx_rpt(struct rtw89_dev *rtwdev, struct sk_buff *c2h, u32 len) u8 sw_define = le32_get_bits(rpt->w2, RTW89_C2H_MAC_TX_RPT_W2_SW_DEFINE); u8 tx_status = le32_get_bits(rpt->w2, RTW89_C2H_MAC_TX_RPT_W2_TX_STATE); u8 data_txcnt = le32_get_bits(rpt->w5, RTW89_C2H_MAC_TX_RPT_W5_DATA_TX_CNT); + struct rtw89_tx_skb_data *skb_data; + struct sk_buff *cur, *tmp; + unsigned long flags; rtw89_debug(rtwdev, RTW89_DBG_TXRX, "C2H TX RPT: sn %d, tx_status %d, data_txcnt %d\n", sw_define, tx_status, data_txcnt); + + spin_lock_irqsave(&rtwdev->tx_rpt_queue.lock, flags); + skb_queue_walk_safe(&rtwdev->tx_rpt_queue, cur, tmp) { + skb_data = RTW89_TX_SKB_CB(cur); + + if (sw_define != skb_data->tx_rpt_sn) + continue; + if (tx_status != RTW89_TX_DONE && + data_txcnt != skb_data->tx_pkt_cnt_lmt) + continue; + + __skb_unlink(cur, &rtwdev->tx_rpt_queue); + rtw89_tx_rpt_tx_status(rtwdev, cur, tx_status); + break; + } + spin_unlock_irqrestore(&rtwdev->tx_rpt_queue.lock, flags); } static void diff --git a/drivers/net/wireless/realtek/rtw89/mac.h b/drivers/net/wireless/realtek/rtw89/mac.h index 15c5c7e4033c..1f7d3734d15f 100644 --- a/drivers/net/wireless/realtek/rtw89/mac.h +++ b/drivers/net/wireless/realtek/rtw89/mac.h @@ -1616,4 +1616,47 @@ int rtw89_mac_scan_offload(struct rtw89_dev *rtwdev, return ret; } + +static inline +void rtw89_tx_rpt_enable(struct rtw89_dev *rtwdev, + struct rtw89_core_tx_request *tx_req) +{ + if (!rtwdev->hci.tx_rpt_enable) + return; + + tx_req->desc_info.report = true; + tx_req->desc_info.sn = atomic_inc_return(&rtwdev->sn) & 0xF; + tx_req->desc_info.tx_cnt_lmt_en = true; + tx_req->desc_info.tx_cnt_lmt = 8; +} + +static inline +void rtw89_tx_rpt_tx_status(struct rtw89_dev *rtwdev, struct sk_buff *skb, u8 tx_status) +{ + struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb); + + ieee80211_tx_info_clear_status(info); + if (tx_status == RTW89_TX_DONE) + info->flags |= IEEE80211_TX_STAT_ACK; + else + info->flags &= ~IEEE80211_TX_STAT_ACK; + + ieee80211_tx_status_irqsafe(rtwdev->hw, skb); +} + +static inline +void rtw89_tx_rpt_queue_purge(struct rtw89_dev *rtwdev) +{ + struct sk_buff_head q; + struct sk_buff *skb; + unsigned long flags; + + __skb_queue_head_init(&q); + spin_lock_irqsave(&rtwdev->tx_rpt_queue.lock, flags); + skb_queue_splice_init(&rtwdev->tx_rpt_queue, &q); + spin_unlock_irqrestore(&rtwdev->tx_rpt_queue.lock, flags); + + while ((skb = __skb_dequeue(&q))) + rtw89_tx_rpt_tx_status(rtwdev, skb, RTW89_TX_MACID_DROP); +} #endif diff --git a/drivers/net/wireless/realtek/rtw89/usb.c b/drivers/net/wireless/realtek/rtw89/usb.c index 655e8437d62e..f53ab676e9a8 100644 --- a/drivers/net/wireless/realtek/rtw89/usb.c +++ b/drivers/net/wireless/realtek/rtw89/usb.c @@ -216,6 +216,15 @@ static void rtw89_usb_write_port_complete(struct urb *urb) skb_pull(skb, txdesc_size); info = IEEE80211_SKB_CB(skb); + if (info->flags & IEEE80211_TX_CTL_REQ_TX_STATUS) { + /* sn is passed to rtw89_mac_c2h_tx_rpt() via driver data */ + skb_queue_tail(&rtwdev->tx_rpt_queue, skb); + wiphy_delayed_work_queue(rtwdev->hw->wiphy, + &rtwdev->tx_wait_work, + RTW89_TX_WAIT_WORK_TIMEOUT); + continue; + } + ieee80211_tx_info_clear_status(info); if (urb->status == 0) { @@ -396,6 +405,8 @@ static int rtw89_usb_ops_tx_write(struct rtw89_dev *rtwdev, memset(txdesc, 0, txdesc_size); rtw89_chip_fill_txdesc(rtwdev, desc_info, txdesc); + RTW89_TX_SKB_CB(skb)->tx_rpt_sn = tx_req->desc_info.sn; + le32p_replace_bits(&txdesc->dword0, 1, RTW89_TXWD_BODY0_STF_MODE); skb_queue_tail(&rtwusb->tx_queue[desc_info->ch_dma], skb); @@ -678,7 +689,7 @@ static void rtw89_usb_deinit_tx(struct rtw89_dev *rtwdev) static void rtw89_usb_ops_reset(struct rtw89_dev *rtwdev) { - /* TODO: anything to do here? */ + rtw89_tx_rpt_queue_purge(rtwdev); } static int rtw89_usb_ops_start(struct rtw89_dev *rtwdev) @@ -962,6 +973,7 @@ int rtw89_usb_probe(struct usb_interface *intf, rtwdev->hci.ops = &rtw89_usb_ops; rtwdev->hci.type = RTW89_HCI_TYPE_USB; + rtwdev->hci.tx_rpt_enable = true; ret = rtw89_usb_intf_init(rtwdev, intf); if (ret) { -- 2.51.0 TX wait skbs need to be completed when they are done. PCIe part does this inside rtw89_pci_tx_status() during RPP processing. Other HCIs use a mechanism based on C2H firmware messages. Store TX wait skbs inside TX report queue so that it'll be possible to identify completed items inside C2H handler via private driver data of skb. Found by Linux Verification Center (linuxtesting.org). Signed-off-by: Fedor Pchelkin --- v2: store TX wait skbs in tx_rpt_queue (Ping-Ke) drivers/net/wireless/realtek/rtw89/core.c | 6 ++++-- drivers/net/wireless/realtek/rtw89/core.h | 15 +++++++++++++++ drivers/net/wireless/realtek/rtw89/mac.c | 3 ++- drivers/net/wireless/realtek/rtw89/mac.h | 15 +++++++++++++-- drivers/net/wireless/realtek/rtw89/usb.c | 3 ++- 5 files changed, 36 insertions(+), 6 deletions(-) diff --git a/drivers/net/wireless/realtek/rtw89/core.c b/drivers/net/wireless/realtek/rtw89/core.c index 214924f8bee0..1457a5fe7320 100644 --- a/drivers/net/wireless/realtek/rtw89/core.c +++ b/drivers/net/wireless/realtek/rtw89/core.c @@ -1108,7 +1108,7 @@ rtw89_core_tx_update_desc_info(struct rtw89_dev *rtwdev, if (addr_cam->valid && desc_info->mlo) upd_wlan_hdr = true; - if (info->flags & IEEE80211_TX_CTL_REQ_TX_STATUS) + if (tx_req->wait || (info->flags & IEEE80211_TX_CTL_REQ_TX_STATUS)) rtw89_tx_rpt_enable(rtwdev, tx_req); } is_bmc = (is_broadcast_ether_addr(hdr->addr1) || @@ -1173,7 +1173,8 @@ int rtw89_core_tx_kick_off_and_wait(struct rtw89_dev *rtwdev, struct sk_buff *sk if (time_left == 0) { ret = -ETIMEDOUT; - list_add_tail(&wait->list, &rtwdev->tx_waits); + if (!rtwdev->hci.tx_rpt_enable) + list_add_tail(&wait->list, &rtwdev->tx_waits); wiphy_delayed_work_queue(rtwdev->hw->wiphy, &rtwdev->tx_wait_work, RTW89_TX_WAIT_WORK_TIMEOUT); } else { @@ -1242,6 +1243,7 @@ static int rtw89_core_tx_write_link(struct rtw89_dev *rtwdev, tx_req.skb = skb; tx_req.vif = vif; tx_req.sta = sta; + tx_req.wait = wait; tx_req.rtwvif_link = rtwvif_link; tx_req.rtwsta_link = rtwsta_link; tx_req.desc_info.sw_mld = sw_mld; diff --git a/drivers/net/wireless/realtek/rtw89/core.h b/drivers/net/wireless/realtek/rtw89/core.h index 3940e54353d3..c13465e2730a 100644 --- a/drivers/net/wireless/realtek/rtw89/core.h +++ b/drivers/net/wireless/realtek/rtw89/core.h @@ -1201,6 +1201,7 @@ struct rtw89_core_tx_request { struct sk_buff *skb; struct ieee80211_vif *vif; struct ieee80211_sta *sta; + struct rtw89_tx_wait_info *wait; struct rtw89_vif_link *rtwvif_link; struct rtw89_sta_link *rtwsta_link; struct rtw89_tx_desc_info desc_info; @@ -7387,6 +7388,20 @@ static inline struct sk_buff *rtw89_alloc_skb_for_rx(struct rtw89_dev *rtwdev, return dev_alloc_skb(length); } +static inline bool rtw89_core_is_tx_wait(struct rtw89_dev *rtwdev, + struct rtw89_tx_skb_data *skb_data) +{ + struct rtw89_tx_wait_info *wait; + + guard(rcu)(); + + wait = rcu_dereference(skb_data->wait); + if (!wait) + return false; + + return true; +} + static inline bool rtw89_core_tx_wait_complete(struct rtw89_dev *rtwdev, struct rtw89_tx_skb_data *skb_data, u8 tx_status) diff --git a/drivers/net/wireless/realtek/rtw89/mac.c b/drivers/net/wireless/realtek/rtw89/mac.c index 75d9efac452b..3406c8b01eb8 100644 --- a/drivers/net/wireless/realtek/rtw89/mac.c +++ b/drivers/net/wireless/realtek/rtw89/mac.c @@ -5484,7 +5484,8 @@ rtw89_mac_c2h_tx_rpt(struct rtw89_dev *rtwdev, struct sk_buff *c2h, u32 len) continue; __skb_unlink(cur, &rtwdev->tx_rpt_queue); - rtw89_tx_rpt_tx_status(rtwdev, cur, tx_status); + if (!rtw89_core_tx_wait_complete(rtwdev, skb_data, tx_status)) + rtw89_tx_rpt_tx_status(rtwdev, cur, tx_status); break; } spin_unlock_irqrestore(&rtwdev->tx_rpt_queue.lock, flags); diff --git a/drivers/net/wireless/realtek/rtw89/mac.h b/drivers/net/wireless/realtek/rtw89/mac.h index 1f7d3734d15f..2d647d3b0852 100644 --- a/drivers/net/wireless/realtek/rtw89/mac.h +++ b/drivers/net/wireless/realtek/rtw89/mac.h @@ -1647,16 +1647,27 @@ void rtw89_tx_rpt_tx_status(struct rtw89_dev *rtwdev, struct sk_buff *skb, u8 tx static inline void rtw89_tx_rpt_queue_purge(struct rtw89_dev *rtwdev) { + struct rtw89_tx_skb_data *skb_data; + struct rtw89_tx_wait_info *wait; struct sk_buff_head q; struct sk_buff *skb; unsigned long flags; + lockdep_assert_wiphy(rtwdev->hw->wiphy); + __skb_queue_head_init(&q); spin_lock_irqsave(&rtwdev->tx_rpt_queue.lock, flags); skb_queue_splice_init(&rtwdev->tx_rpt_queue, &q); spin_unlock_irqrestore(&rtwdev->tx_rpt_queue.lock, flags); - while ((skb = __skb_dequeue(&q))) - rtw89_tx_rpt_tx_status(rtwdev, skb, RTW89_TX_MACID_DROP); + while ((skb = __skb_dequeue(&q))) { + skb_data = RTW89_TX_SKB_CB(skb); + wait = wiphy_dereference(rtwdev->hw->wiphy, skb_data->wait); + + if (wait) + rtw89_tx_wait_release(wait); + else + rtw89_tx_rpt_tx_status(rtwdev, skb, RTW89_TX_MACID_DROP); + } } #endif diff --git a/drivers/net/wireless/realtek/rtw89/usb.c b/drivers/net/wireless/realtek/rtw89/usb.c index f53ab676e9a8..adbadb2783f0 100644 --- a/drivers/net/wireless/realtek/rtw89/usb.c +++ b/drivers/net/wireless/realtek/rtw89/usb.c @@ -216,7 +216,8 @@ static void rtw89_usb_write_port_complete(struct urb *urb) skb_pull(skb, txdesc_size); info = IEEE80211_SKB_CB(skb); - if (info->flags & IEEE80211_TX_CTL_REQ_TX_STATUS) { + if (rtw89_core_is_tx_wait(rtwdev, RTW89_TX_SKB_CB(skb)) || + (info->flags & IEEE80211_TX_CTL_REQ_TX_STATUS)) { /* sn is passed to rtw89_mac_c2h_tx_rpt() via driver data */ skb_queue_tail(&rtwdev->tx_rpt_queue, skb); wiphy_delayed_work_queue(rtwdev->hw->wiphy, -- 2.51.0