| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <07ee4e8c-bcb8-4349-afd3-59bc58899116@gmail.com>
Date: Wed, 24 Sep 2025 22:16:43 +0300
From: Bitterblue Smith <rtl8821cerfe2@...il.com>
To: Fedor Pchelkin <pchelkin@...ras.ru>, Ping-Ke Shih <pkshih@...ltek.com>
Cc: Zong-Zhe Yang <kevin_yang@...ltek.com>, Po-Hao Huang
<phhuang@...ltek.com>, linux-wireless@...r.kernel.org,
linux-kernel@...r.kernel.org, lvc-project@...uxtesting.org
Subject: Re: [PATCH rtw-next 3/6] wifi: rtw89: implement C2H TX report handler
On 24/09/2025 01:12, Bitterblue Smith wrote:
> On 20/09/2025 16:26, Fedor Pchelkin wrote:
>> 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 <rtl8821cerfe2@...il.com>
>> Signed-off-by: Fedor Pchelkin <pchelkin@...ras.ru>
>> ---
>> 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))
>
> This is only 4 bits:
>
> #define TXCCXRPT_SW_DEFINE_SH 8
> #define TXCCXRPT_SW_DEFINE_MSK 0xf
>
>
> The rest of the series looks good to me. (I don't know much about
> the RCU stuff.) I will test this tomorrow.
>
Actually, I found this in my notes:
"how to get just one tx report for each request? currently it seems
to provide a report for each transmission attempt. how is the vendor
driver coping with that?"
I think your code doesn't account for this.
Sorry I forgot about this detail. This behaviour is new in rtw89.
The chips supported by rtw88 provide only one report for each request.
>> +
>> 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)
>>
>
Powered by blists - more mailing lists