| 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: <6dc0740a095f4a38ae2427169b1d100c@realtek.com>
Date: Wed, 22 Oct 2025 06:21:25 +0000
From: Ping-Ke Shih <pkshih@...ltek.com>
To: Fedor Pchelkin <pchelkin@...ras.ru>,
Bitterblue Smith
<rtl8821cerfe2@...il.com>
CC: Zong-Zhe Yang <kevin_yang@...ltek.com>,
Bernie Huang
<phhuang@...ltek.com>,
"linux-wireless@...r.kernel.org"
<linux-wireless@...r.kernel.org>,
"linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>,
"lvc-project@...uxtesting.org"
<lvc-project@...uxtesting.org>
Subject: RE: [PATCH rtw-next v3 5/9] wifi: rtw89: implement C2H TX report handler
Fedor Pchelkin <pchelkin@...ras.ru> 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 to indicate to the firmware that TX
> report for the frame is expected. This will allow handling TX wait skbs
> and the ones flagged with IEEE80211_TX_CTL_REQ_TX_STATUS correctly.
>
> Do the bulk of the patch according to the vendor driver for RTL8851BU.
> However, there are slight differences in C2H message format between
> different types of chips. RTL885xB ones follow format V0. RTL8852C has
> format V1, and RTL8922AU has format V2.
>
> Found by Linux Verification Center (linuxtesting.org).
>
> Suggested-by: Bitterblue Smith <rtl8821cerfe2@...il.com>
> Signed-off-by: Fedor Pchelkin <pchelkin@...ras.ru>
> ---
[...]
> diff --git a/drivers/net/wireless/realtek/rtw89/fw.h b/drivers/net/wireless/realtek/rtw89/fw.h
> index ddebf7972068..e0e587ec4592 100644
> --- a/drivers/net/wireless/realtek/rtw89/fw.h
> +++ b/drivers/net/wireless/realtek/rtw89/fw.h
> @@ -3747,6 +3747,47 @@ 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;
> +};
__packed
> +
> +#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)
> +#define RTW89_C2H_MAC_TX_RPT_W5_DATA_TX_CNT_V1 GENMASK(15, 10)
> +
> +struct rtw89_c2h_mac_tx_rpt_v2 {
> + struct rtw89_c2h_hdr hdr;
> + __le32 w2;
> + __le32 w3;
> + __le32 w4;
> + __le32 w5;
> + __le32 w6;
> + __le32 w7;
> + __le32 w8;
> + __le32 w9;
> + __le32 w10;
> + __le32 w11;
> + __le32 w12;
> + __le32 w13;
> + __le32 w14;
> + __le32 w15;
> + __le32 w16;
> + __le32 w17;
> + __le32 w18;
> + __le32 w19;
> +};
__packed
> +
> +#define RTW89_C2H_MAC_TX_RPT_W12_TX_STATE_V2 GENMASK(9, 8)
> +#define RTW89_C2H_MAC_TX_RPT_W12_SW_DEFINE_V2 GENMASK(15, 12)
> +#define RTW89_C2H_MAC_TX_RPT_W14_DATA_TX_CNT_V2 GENMASK(15, 10)
> +
> 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..2fe239f18534 100644
> --- a/drivers/net/wireless/realtek/rtw89/mac.c
> +++ b/drivers/net/wireless/realtek/rtw89/mac.c
> @@ -5457,6 +5457,35 @@ 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, tx_status, data_txcnt;
> +
> + if (rtwdev->chip->chip_id == RTL8922A) {
Add a chip_ops c2h_tx_rpt? Then, no need chip_id checking, and reduce line
length (normally we prefer shorter than 80 or 90 characters; over 100 characters
isn't a good idea).
Maybe this is because you want to store the status into local variables.
With a chip_ops, you should define another struct to store them.
Or, you just keep it as was, but wrap lines to be shorter, and give shorter
naming. For example,
- rpt_v2 -> v2
- data_txcnt -> txcnt
if (rtwdev->chip->chip_id == RTL8852C)
txcnt = le32_get_bits(rpt->w5,
RTW89_C2H_MAC_TX_RPT_W5_DATA_TX_CNT_V1);
else
txcnt = le32_get_bits(rpt->w5,
RTW89_C2H_MAC_TX_RPT_W5_DATA_TX_CNT);
> + const struct rtw89_c2h_mac_tx_rpt_v2 *rpt_v2;
> +
> + rpt_v2 = (const struct rtw89_c2h_mac_tx_rpt_v2 *)c2h->data;
> + sw_define = le32_get_bits(rpt_v2->w12, RTW89_C2H_MAC_TX_RPT_W12_SW_DEFINE_V2);
> + tx_status = le32_get_bits(rpt_v2->w12, RTW89_C2H_MAC_TX_RPT_W12_TX_STATE_V2);
> + data_txcnt = le32_get_bits(rpt_v2->w14, RTW89_C2H_MAC_TX_RPT_W14_DATA_TX_CNT_V2);
> + } else {
> + const struct rtw89_c2h_mac_tx_rpt *rpt;
> +
> + rpt = (const struct rtw89_c2h_mac_tx_rpt *)c2h->data;
> + sw_define = le32_get_bits(rpt->w2, RTW89_C2H_MAC_TX_RPT_W2_SW_DEFINE);
> + tx_status = le32_get_bits(rpt->w2, RTW89_C2H_MAC_TX_RPT_W2_TX_STATE);
> + if (rtwdev->chip->chip_id == RTL8852C)
> + data_txcnt = le32_get_bits(rpt->w5, RTW89_C2H_MAC_TX_RPT_W5_DATA_TX_CNT_V1);
> + else
> + 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)
> {
[...]
Powered by blists - more mailing lists