Free allocated skb on the error handling path. Found by Linux Verification Center (linuxtesting.org). Fixes: 2135c28be6a8 ("wifi: rtw89: Add usb.{c,h}") Signed-off-by: Fedor Pchelkin --- drivers/net/wireless/realtek/rtw89/usb.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/wireless/realtek/rtw89/usb.c b/drivers/net/wireless/realtek/rtw89/usb.c index 6cf89aee252e..3435599f4740 100644 --- a/drivers/net/wireless/realtek/rtw89/usb.c +++ b/drivers/net/wireless/realtek/rtw89/usb.c @@ -422,6 +422,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); + dev_kfree_skb_any(rx_skb); continue; } -- 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 --- 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 3435599f4740..bc0d5e48d39b 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 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. Do 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 --- drivers/net/wireless/realtek/rtw89/core.c | 12 +++++++++++- drivers/net/wireless/realtek/rtw89/core.h | 2 ++ drivers/net/wireless/realtek/rtw89/fw.h | 5 +++++ drivers/net/wireless/realtek/rtw89/mac.c | 23 +++++++++++++++++++++++ drivers/net/wireless/realtek/rtw89/mac.h | 9 +++++++++ drivers/net/wireless/realtek/rtw89/txrx.h | 2 ++ 6 files changed, 52 insertions(+), 1 deletion(-) diff --git a/drivers/net/wireless/realtek/rtw89/core.c b/drivers/net/wireless/realtek/rtw89/core.c index 917b2adede61..d2a559ddfa2e 100644 --- a/drivers/net/wireless/realtek/rtw89/core.c +++ b/drivers/net/wireless/realtek/rtw89/core.c @@ -1420,11 +1420,20 @@ 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) +{ + bool rpt_en = desc_info->report; + u32 dword = FIELD_PREP(RTW89_TXWD_INFO3_SPE_RPT, rpt_en); + + 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 +1456,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); } diff --git a/drivers/net/wireless/realtek/rtw89/core.h b/drivers/net/wireless/realtek/rtw89/core.h index 928c8c84c964..2362724323a9 100644 --- a/drivers/net/wireless/realtek/rtw89/core.h +++ b/drivers/net/wireless/realtek/rtw89/core.h @@ -1167,6 +1167,8 @@ struct rtw89_tx_desc_info { u8 ampdu_density; u8 ampdu_num; bool sec_en; + bool report; + u8 sn; 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..f196088a8316 100644 --- a/drivers/net/wireless/realtek/rtw89/fw.h +++ b/drivers/net/wireless/realtek/rtw89/fw.h @@ -3747,6 +3747,11 @@ 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)) +#define RTW89_GET_MAC_C2H_TX_RPT_TX_STATE(c2h) \ + le32_get_bits(*((const __le32 *)(c2h) + 2), GENMASK(7, 6)) +#define RTW89_GET_MAC_C2H_TX_RPT_SW_DEFINE(c2h) \ + le32_get_bits(*((const __le32 *)(c2h) + 2), GENMASK(12, 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..01afdcd5f36c 100644 --- a/drivers/net/wireless/realtek/rtw89/mac.c +++ b/drivers/net/wireless/realtek/rtw89/mac.c @@ -5457,6 +5457,17 @@ 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) +{ + u8 sw_define = RTW89_GET_MAC_C2H_TX_RPT_SW_DEFINE(c2h->data); + u8 tx_status = RTW89_GET_MAC_C2H_TX_RPT_TX_STATE(c2h->data); + + rtw89_debug(rtwdev, RTW89_DBG_TXRX, + "C2H TX RPT: sn %d, tx_status %d\n", + sw_define, tx_status); +} + static void rtw89_mac_c2h_mrc_tsf_rpt(struct rtw89_dev *rtwdev, struct sk_buff *c2h, u32 len) { @@ -5691,6 +5702,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 +5794,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 +5831,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..632b85aed032 100644 --- a/drivers/net/wireless/realtek/rtw89/mac.h +++ b/drivers/net/wireless/realtek/rtw89/mac.h @@ -432,6 +432,14 @@ enum rtw89_mac_c2h_mcc_func { NUM_OF_RTW89_MAC_C2H_FUNC_MCC, }; +enum rtw89_mac_c2h_misc_func { + RTW89_MAC_C2H_FUNC_WPS_RPT, + RTW89_MAC_C2H_FUNC_TX_REPORT, + RTW89_MAC_C2H_FUNC_BF_SENS_FEEDBACK = 0x4, + + 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 +478,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..d7259e6d798e 100644 --- a/drivers/net/wireless/realtek/rtw89/txrx.h +++ b/drivers/net/wireless/realtek/rtw89/txrx.h @@ -139,8 +139,10 @@ 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_SW_DEFINE GENMASK(3, 0) #define RTW89_TXWD_INFO4_RTS_EN BIT(27) #define RTW89_TXWD_INFO4_HW_RTS_EN BIT(31) -- 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 handler which will initialize the corresponding values used later when writing to TX descriptor. The handler is a no-op for PCIe, TX status reporting behaviour doesn't change there. 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 (done at core deinitialization phase, too). Found by Linux Verification Center (linuxtesting.org). Signed-off-by: Fedor Pchelkin --- drivers/net/wireless/realtek/rtw89/core.c | 5 ++++ drivers/net/wireless/realtek/rtw89/core.h | 18 ++++++++++++++ drivers/net/wireless/realtek/rtw89/mac.c | 29 +++++++++++++++++++++++ drivers/net/wireless/realtek/rtw89/pci.c | 1 + drivers/net/wireless/realtek/rtw89/pci.h | 4 ---- drivers/net/wireless/realtek/rtw89/usb.c | 18 ++++++++++++++ drivers/net/wireless/realtek/rtw89/usb.h | 15 ++++++++++++ 7 files changed, 86 insertions(+), 4 deletions(-) diff --git a/drivers/net/wireless/realtek/rtw89/core.c b/drivers/net/wireless/realtek/rtw89/core.c index d2a559ddfa2e..3e7bd0cedbdf 100644 --- a/drivers/net/wireless/realtek/rtw89/core.c +++ b/drivers/net/wireless/realtek/rtw89/core.c @@ -1229,6 +1229,7 @@ static int rtw89_core_tx_write_link(struct rtw89_dev *rtwdev, struct ieee80211_sta *sta = rtwsta_link_to_sta_safe(rtwsta_link); struct ieee80211_vif *vif = rtwvif_link_to_vif(rtwvif_link); struct rtw89_tx_skb_data *skb_data = RTW89_TX_SKB_CB(skb); + struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb); struct rtw89_vif *rtwvif = rtwvif_link->rtwvif; struct rtw89_core_tx_request tx_req = {}; int ret; @@ -1240,6 +1241,9 @@ static int rtw89_core_tx_write_link(struct rtw89_dev *rtwdev, tx_req.rtwsta_link = rtwsta_link; tx_req.desc_info.sw_mld = sw_mld; + if (info->flags & IEEE80211_TX_CTL_REQ_TX_STATUS) + rtw89_hci_tx_rpt_enable(rtwdev, &tx_req); + rtw89_traffic_stats_accu(rtwdev, rtwvif, skb, true, true); rtw89_wow_parse_akm(rtwdev, skb); rtw89_core_tx_update_desc_info(rtwdev, &tx_req); @@ -5839,6 +5843,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 2362724323a9..4e597a5df005 100644 --- a/drivers/net/wireless/realtek/rtw89/core.h +++ b/drivers/net/wireless/realtek/rtw89/core.h @@ -3509,6 +3509,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; @@ -3646,6 +3651,8 @@ struct rtw89_hci_ops { void (*pause)(struct rtw89_dev *rtwdev, bool pause); void (*switch_mode)(struct rtw89_dev *rtwdev, bool low_power); void (*recalc_int_mit)(struct rtw89_dev *rtwdev); + void (*tx_rpt_enable)(struct rtw89_dev *rtwdev, + struct rtw89_core_tx_request *tx_req); u8 (*read8)(struct rtw89_dev *rtwdev, u32 addr); u16 (*read16)(struct rtw89_dev *rtwdev, u32 addr); @@ -6008,6 +6015,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; @@ -6294,6 +6304,7 @@ static inline void rtw89_hci_reset(struct rtw89_dev *rtwdev) { rtwdev->hci.ops->reset(rtwdev); rtw89_tx_wait_list_clear(rtwdev); + skb_queue_purge(&rtwdev->tx_rpt_queue); } static inline int rtw89_hci_start(struct rtw89_dev *rtwdev) @@ -6336,6 +6347,13 @@ static inline void rtw89_hci_tx_kick_off(struct rtw89_dev *rtwdev, u8 txch) return rtwdev->hci.ops->tx_kick_off(rtwdev, txch); } +static inline void rtw89_hci_tx_rpt_enable(struct rtw89_dev *rtwdev, + struct rtw89_core_tx_request *tx_req) +{ + if (rtwdev->hci.ops->tx_rpt_enable) + rtwdev->hci.ops->tx_rpt_enable(rtwdev, tx_req); +} + static inline int rtw89_hci_mac_pre_deinit(struct rtw89_dev *rtwdev) { return rtwdev->hci.ops->mac_pre_deinit(rtwdev); diff --git a/drivers/net/wireless/realtek/rtw89/mac.c b/drivers/net/wireless/realtek/rtw89/mac.c index 01afdcd5f36c..831e53aedccc 100644 --- a/drivers/net/wireless/realtek/rtw89/mac.c +++ b/drivers/net/wireless/realtek/rtw89/mac.c @@ -5457,15 +5457,44 @@ 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_tx_rpt_tx_status(struct rtw89_dev *rtwdev, struct sk_buff *skb, bool acked) +{ + struct ieee80211_tx_info *info; + + info = IEEE80211_SKB_CB(skb); + ieee80211_tx_info_clear_status(info); + if (acked) + info->flags |= IEEE80211_TX_STAT_ACK; + else + info->flags &= ~IEEE80211_TX_STAT_ACK; + + ieee80211_tx_status_irqsafe(rtwdev->hw, skb); +} + static void rtw89_mac_c2h_tx_rpt(struct rtw89_dev *rtwdev, struct sk_buff *c2h, u32 len) { u8 sw_define = RTW89_GET_MAC_C2H_TX_RPT_SW_DEFINE(c2h->data); u8 tx_status = RTW89_GET_MAC_C2H_TX_RPT_TX_STATE(c2h->data); + struct sk_buff *cur, *tmp; + unsigned long flags; + u8 *n; rtw89_debug(rtwdev, RTW89_DBG_TXRX, "C2H TX RPT: sn %d, tx_status %d\n", sw_define, tx_status); + + spin_lock_irqsave(&rtwdev->tx_rpt_queue.lock, flags); + skb_queue_walk_safe(&rtwdev->tx_rpt_queue, cur, tmp) { + n = (u8 *)RTW89_TX_SKB_CB(cur)->hci_priv; + if (*n == sw_define) { + __skb_unlink(cur, &rtwdev->tx_rpt_queue); + rtw89_tx_rpt_tx_status(rtwdev, cur, tx_status == RTW89_TX_DONE); + break; + } + } + spin_unlock_irqrestore(&rtwdev->tx_rpt_queue.lock, flags); } static void diff --git a/drivers/net/wireless/realtek/rtw89/pci.c b/drivers/net/wireless/realtek/rtw89/pci.c index 0ee5f8579447..fdf142d77ecc 100644 --- a/drivers/net/wireless/realtek/rtw89/pci.c +++ b/drivers/net/wireless/realtek/rtw89/pci.c @@ -4675,6 +4675,7 @@ static const struct rtw89_hci_ops rtw89_pci_ops = { .pause = rtw89_pci_ops_pause, .switch_mode = rtw89_pci_ops_switch_mode, .recalc_int_mit = rtw89_pci_recalc_int_mit, + .tx_rpt_enable = NULL, /* always enabled */ .read8 = rtw89_pci_ops_read8, .read16 = rtw89_pci_ops_read16, 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) diff --git a/drivers/net/wireless/realtek/rtw89/usb.c b/drivers/net/wireless/realtek/rtw89/usb.c index bc0d5e48d39b..98eff955fc96 100644 --- a/drivers/net/wireless/realtek/rtw89/usb.c +++ b/drivers/net/wireless/realtek/rtw89/usb.c @@ -188,6 +188,13 @@ static u8 rtw89_usb_get_bulkout_id(u8 ch_dma) } } +static void rtw89_usb_ops_tx_rpt_enable(struct rtw89_dev *rtwdev, + struct rtw89_core_tx_request *tx_req) +{ + tx_req->desc_info.report = true; + tx_req->desc_info.sn = atomic_inc_return(&rtwdev->sn) & 0xF; +} + static void rtw89_usb_write_port_complete(struct urb *urb) { struct rtw89_usb_tx_ctrl_block *txcb = urb->context; @@ -216,6 +223,12 @@ 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); + continue; + } + ieee80211_tx_info_clear_status(info); if (urb->status == 0) { @@ -364,6 +377,7 @@ static int rtw89_usb_ops_tx_write(struct rtw89_dev *rtwdev, struct rtw89_tx_desc_info *desc_info = &tx_req->desc_info; struct rtw89_usb *rtwusb = rtw89_usb_priv(rtwdev); struct sk_buff *skb = tx_req->skb; + struct rtw89_usb_tx_data *tx_data; struct rtw89_txwd_body *txdesc; u32 txdesc_size; @@ -387,6 +401,9 @@ static int rtw89_usb_ops_tx_write(struct rtw89_dev *rtwdev, memset(txdesc, 0, txdesc_size); rtw89_chip_fill_txdesc(rtwdev, desc_info, txdesc); + tx_data = RTW89_USB_TX_SKB_CB(skb); + tx_data->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); @@ -811,6 +828,7 @@ static const struct rtw89_hci_ops rtw89_usb_ops = { .pause = rtw89_usb_ops_pause, .switch_mode = rtw89_usb_ops_switch_mode, .recalc_int_mit = rtw89_usb_ops_recalc_int_mit, + .tx_rpt_enable = rtw89_usb_ops_tx_rpt_enable, .read8 = rtw89_usb_ops_read8, .read16 = rtw89_usb_ops_read16, diff --git a/drivers/net/wireless/realtek/rtw89/usb.h b/drivers/net/wireless/realtek/rtw89/usb.h index c1b4bfa20979..c8b2ad2eaa22 100644 --- a/drivers/net/wireless/realtek/rtw89/usb.h +++ b/drivers/net/wireless/realtek/rtw89/usb.h @@ -53,11 +53,26 @@ struct rtw89_usb { struct sk_buff_head tx_queue[RTW89_TXCH_NUM]; }; +struct rtw89_usb_tx_data { + u8 sn; +}; + static inline struct rtw89_usb *rtw89_usb_priv(struct rtw89_dev *rtwdev) { return (struct rtw89_usb *)rtwdev->priv; } +static inline struct rtw89_usb_tx_data *RTW89_USB_TX_SKB_CB(struct sk_buff *skb) +{ + struct rtw89_tx_skb_data *data = RTW89_TX_SKB_CB(skb); + + BUILD_BUG_ON(sizeof(struct rtw89_tx_skb_data) + + sizeof(struct rtw89_usb_tx_data) > + sizeof_field(struct ieee80211_tx_info, driver_data)); + + return (struct rtw89_usb_tx_data *)data->hci_priv; +} + int rtw89_usb_probe(struct usb_interface *intf, const struct usb_device_id *id); void rtw89_usb_disconnect(struct usb_interface *intf); -- 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 a sequence number in a TX wait object so that it'll be possible to identify completed items inside C2H handler. No need to add the corresponding skb to the &txcb->tx_ack_queue on USB part. Found by Linux Verification Center (linuxtesting.org). Signed-off-by: Fedor Pchelkin --- drivers/net/wireless/realtek/rtw89/core.c | 6 ++++-- drivers/net/wireless/realtek/rtw89/core.h | 18 +++++++++++++++++- drivers/net/wireless/realtek/rtw89/mac.c | 11 +++++++++++ drivers/net/wireless/realtek/rtw89/usb.c | 9 ++++++++- 4 files changed, 40 insertions(+), 4 deletions(-) diff --git a/drivers/net/wireless/realtek/rtw89/core.c b/drivers/net/wireless/realtek/rtw89/core.c index 3e7bd0cedbdf..e76f04736502 100644 --- a/drivers/net/wireless/realtek/rtw89/core.c +++ b/drivers/net/wireless/realtek/rtw89/core.c @@ -1161,18 +1161,19 @@ int rtw89_core_tx_kick_off_and_wait(struct rtw89_dev *rtwdev, struct sk_buff *sk lockdep_assert_wiphy(rtwdev->hw->wiphy); + list_add_tail_rcu(&wait->list, &rtwdev->tx_waits); rtw89_core_tx_kick_off(rtwdev, qsel); time_left = wait_for_completion_timeout(&wait->completion, msecs_to_jiffies(timeout)); if (time_left == 0) { ret = -ETIMEDOUT; - 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 { if (!wait->tx_done) ret = -EAGAIN; + list_del_rcu(&wait->list); rtw89_tx_wait_release(wait); } @@ -1237,11 +1238,12 @@ 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; - if (info->flags & IEEE80211_TX_CTL_REQ_TX_STATUS) + if (wait || (info->flags & IEEE80211_TX_CTL_REQ_TX_STATUS)) rtw89_hci_tx_rpt_enable(rtwdev, &tx_req); rtw89_traffic_stats_accu(rtwdev, rtwvif, skb, true, true); diff --git a/drivers/net/wireless/realtek/rtw89/core.h b/drivers/net/wireless/realtek/rtw89/core.h index 4e597a5df005..e7948bd0bdf6 100644 --- a/drivers/net/wireless/realtek/rtw89/core.h +++ b/drivers/net/wireless/realtek/rtw89/core.h @@ -1199,6 +1199,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; @@ -3521,6 +3522,7 @@ struct rtw89_tx_wait_info { struct completion completion; struct sk_buff *skb; bool tx_done; + u8 sn; }; struct rtw89_tx_skb_data { @@ -6289,7 +6291,7 @@ static inline void rtw89_tx_wait_list_clear(struct rtw89_dev *rtwdev) list_for_each_entry_safe(wait, tmp, &rtwdev->tx_waits, list) { if (!completion_done(&wait->completion)) continue; - list_del(&wait->list); + list_del_rcu(&wait->list); rtw89_tx_wait_release(wait); } } @@ -7392,6 +7394,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, bool tx_done) diff --git a/drivers/net/wireless/realtek/rtw89/mac.c b/drivers/net/wireless/realtek/rtw89/mac.c index 831e53aedccc..79409eb4d028 100644 --- a/drivers/net/wireless/realtek/rtw89/mac.c +++ b/drivers/net/wireless/realtek/rtw89/mac.c @@ -5477,6 +5477,7 @@ rtw89_mac_c2h_tx_rpt(struct rtw89_dev *rtwdev, struct sk_buff *c2h, u32 len) { u8 sw_define = RTW89_GET_MAC_C2H_TX_RPT_SW_DEFINE(c2h->data); u8 tx_status = RTW89_GET_MAC_C2H_TX_RPT_TX_STATE(c2h->data); + struct rtw89_tx_wait_info *wait; struct sk_buff *cur, *tmp; unsigned long flags; u8 *n; @@ -5485,6 +5486,16 @@ rtw89_mac_c2h_tx_rpt(struct rtw89_dev *rtwdev, struct sk_buff *c2h, u32 len) "C2H TX RPT: sn %d, tx_status %d\n", sw_define, tx_status); + rcu_read_lock(); + list_for_each_entry_rcu(wait, &rtwdev->tx_waits, list) { + if (wait->sn == sw_define) { + wait->tx_done = tx_status == RTW89_TX_DONE; + complete_all(&wait->completion); + break; + } + } + rcu_read_unlock(); + spin_lock_irqsave(&rtwdev->tx_rpt_queue.lock, flags); skb_queue_walk_safe(&rtwdev->tx_rpt_queue, cur, tmp) { n = (u8 *)RTW89_TX_SKB_CB(cur)->hci_priv; diff --git a/drivers/net/wireless/realtek/rtw89/usb.c b/drivers/net/wireless/realtek/rtw89/usb.c index 98eff955fc96..342c05227191 100644 --- a/drivers/net/wireless/realtek/rtw89/usb.c +++ b/drivers/net/wireless/realtek/rtw89/usb.c @@ -191,8 +191,13 @@ static u8 rtw89_usb_get_bulkout_id(u8 ch_dma) static void rtw89_usb_ops_tx_rpt_enable(struct rtw89_dev *rtwdev, struct rtw89_core_tx_request *tx_req) { + struct rtw89_tx_wait_info *wait = tx_req->wait; + tx_req->desc_info.report = true; tx_req->desc_info.sn = atomic_inc_return(&rtwdev->sn) & 0xF; + + if (wait) + wait->sn = tx_req->desc_info.sn; } static void rtw89_usb_write_port_complete(struct urb *urb) @@ -313,7 +318,9 @@ static void rtw89_usb_ops_tx_kick_off(struct rtw89_dev *rtwdev, u8 txch) txcb->txch = txch; skb_queue_head_init(&txcb->tx_ack_queue); - skb_queue_tail(&txcb->tx_ack_queue, skb); + /* tx_wait skbs are completed in rtw89_mac_c2h_tx_rpt() */ + if (!rtw89_core_is_tx_wait(rtwdev, RTW89_TX_SKB_CB(skb))) + skb_queue_tail(&txcb->tx_ack_queue, skb); ret = rtw89_usb_write_port(rtwdev, txch, skb->data, skb->len, txcb); -- 2.51.0 TX status reporting based on firmware messages does not necessarily happen when an HCI reset occurs, in contrast to RPP based one where pending skbs are forcefully flushed, see rtw89_pci_release_txwd_skb(). So for the former case, if completion from the firmware doesn't happen, TX wait objects are wastefully piled up in the list and not released. Forcefully clear TX wait list on HCI reset then. It's okay since wiphy lock is held during HCI reset. For the RPP case, all pending completions were done just before in ->reset callback and no new ones can appear. For the C2H message case, RCU access to the list helps. Found by Linux Verification Center (linuxtesting.org). Signed-off-by: Fedor Pchelkin --- drivers/net/wireless/realtek/rtw89/core.c | 2 +- drivers/net/wireless/realtek/rtw89/core.h | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/net/wireless/realtek/rtw89/core.c b/drivers/net/wireless/realtek/rtw89/core.c index e76f04736502..3a0388d3acbf 100644 --- a/drivers/net/wireless/realtek/rtw89/core.c +++ b/drivers/net/wireless/realtek/rtw89/core.c @@ -1140,7 +1140,7 @@ 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); + rtw89_tx_wait_list_clear(rtwdev, false); } void rtw89_core_tx_kick_off(struct rtw89_dev *rtwdev, u8 qsel) diff --git a/drivers/net/wireless/realtek/rtw89/core.h b/drivers/net/wireless/realtek/rtw89/core.h index e7948bd0bdf6..0ad871472e79 100644 --- a/drivers/net/wireless/realtek/rtw89/core.h +++ b/drivers/net/wireless/realtek/rtw89/core.h @@ -6282,14 +6282,14 @@ static inline void rtw89_tx_wait_release(struct rtw89_tx_wait_info *wait) kfree_rcu(wait, rcu_head); } -static inline void rtw89_tx_wait_list_clear(struct rtw89_dev *rtwdev) +static inline void rtw89_tx_wait_list_clear(struct rtw89_dev *rtwdev, bool force) { struct rtw89_tx_wait_info *wait, *tmp; lockdep_assert_wiphy(rtwdev->hw->wiphy); list_for_each_entry_safe(wait, tmp, &rtwdev->tx_waits, list) { - if (!completion_done(&wait->completion)) + if (!force && !completion_done(&wait->completion)) continue; list_del_rcu(&wait->list); rtw89_tx_wait_release(wait); @@ -6305,7 +6305,7 @@ static inline int rtw89_hci_tx_write(struct rtw89_dev *rtwdev, static inline void rtw89_hci_reset(struct rtw89_dev *rtwdev) { rtwdev->hci.ops->reset(rtwdev); - rtw89_tx_wait_list_clear(rtwdev); + rtw89_tx_wait_list_clear(rtwdev, true); skb_queue_purge(&rtwdev->tx_rpt_queue); } -- 2.51.0