2023-01-17 09:46:41

by Deren Wu

[permalink] [raw]
Subject: [PATCH] wifi: mt76: mt7921: fix rx filter incorrect by drv/fw inconsistent

From: Neil Chen <[email protected]>

The rx filter, in mt7921 series, may be changed in fw operation. There is
a racing problem if rx filter controlled by both driver and firmware at
the same time. To avoid this issue, let mt7921 driver set rx filter by new
command MCU_CE_CMD_SET_RX_FILTER and allow the firmware controlling it
only.

Reviewed-by: Lorenzo Bianconi <[email protected]>
Co-developed-by: Deren Wu <[email protected]>
Signed-off-by: Deren Wu <[email protected]>
Signed-off-by: Neil Chen <[email protected]>
---
.../wireless/mediatek/mt76/mt76_connac_mcu.h | 1 +
.../net/wireless/mediatek/mt76/mt7921/init.c | 2 -
.../net/wireless/mediatek/mt76/mt7921/main.c | 48 ++-----------------
.../net/wireless/mediatek/mt76/mt7921/mcu.c | 36 +++++++++++++-
.../wireless/mediatek/mt76/mt7921/mt7921.h | 2 +
.../wireless/mediatek/mt76/mt7921/testmode.c | 1 -
6 files changed, 40 insertions(+), 50 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt76_connac_mcu.h b/drivers/net/wireless/mediatek/mt76/mt76_connac_mcu.h
index f1e942b9a887..1baea5d5bdb1 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76_connac_mcu.h
+++ b/drivers/net/wireless/mediatek/mt76/mt76_connac_mcu.h
@@ -1238,6 +1238,7 @@ enum {
MCU_CE_CMD_TEST_CTRL = 0x01,
MCU_CE_CMD_START_HW_SCAN = 0x03,
MCU_CE_CMD_SET_PS_PROFILE = 0x05,
+ MCU_CE_CMD_SET_RX_FILTER = 0x0a,
MCU_CE_CMD_SET_CHAN_DOMAIN = 0x0f,
MCU_CE_CMD_SET_BSS_CONNECTED = 0x16,
MCU_CE_CMD_SET_BSS_ABORT = 0x17,
diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/init.c b/drivers/net/wireless/mediatek/mt76/mt7921/init.c
index d4b681d7e1d2..4b935867c2b2 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7921/init.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7921/init.c
@@ -229,8 +229,6 @@ int mt7921_mac_init(struct mt7921_dev *dev)
for (i = 0; i < 2; i++)
mt7921_mac_init_band(dev, i);

- dev->mt76.rxfilter = mt76_rr(dev, MT_WF_RFCR(0));
-
return mt76_connac_mcu_set_rts_thresh(&dev->mt76, 0x92b, 0);
}
EXPORT_SYMBOL_GPL(mt7921_mac_init);
diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/main.c b/drivers/net/wireless/mediatek/mt76/mt7921/main.c
index 76ac5069638f..df7bd84b28d2 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7921/main.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7921/main.c
@@ -681,7 +681,6 @@ static int mt7921_config(struct ieee80211_hw *hw, u32 changed)
ieee80211_iterate_active_interfaces(hw,
IEEE80211_IFACE_ITER_RESUME_ALL,
mt7921_sniffer_interface_iter, dev);
- dev->mt76.rxfilter = mt76_rr(dev, MT_WF_RFCR(0));
}

out:
@@ -710,53 +709,12 @@ static void mt7921_configure_filter(struct ieee80211_hw *hw,
u64 multicast)
{
struct mt7921_dev *dev = mt7921_hw_dev(hw);
- u32 ctl_flags = MT_WF_RFCR1_DROP_ACK |
- MT_WF_RFCR1_DROP_BF_POLL |
- MT_WF_RFCR1_DROP_BA |
- MT_WF_RFCR1_DROP_CFEND |
- MT_WF_RFCR1_DROP_CFACK;
- u32 flags = 0;
-
-#define MT76_FILTER(_flag, _hw) do { \
- flags |= *total_flags & FIF_##_flag; \
- dev->mt76.rxfilter &= ~(_hw); \
- dev->mt76.rxfilter |= !(flags & FIF_##_flag) * (_hw); \
- } while (0)

mt7921_mutex_acquire(dev);
-
- dev->mt76.rxfilter &= ~(MT_WF_RFCR_DROP_OTHER_BSS |
- MT_WF_RFCR_DROP_OTHER_BEACON |
- MT_WF_RFCR_DROP_FRAME_REPORT |
- MT_WF_RFCR_DROP_PROBEREQ |
- MT_WF_RFCR_DROP_MCAST_FILTERED |
- MT_WF_RFCR_DROP_MCAST |
- MT_WF_RFCR_DROP_BCAST |
- MT_WF_RFCR_DROP_DUPLICATE |
- MT_WF_RFCR_DROP_A2_BSSID |
- MT_WF_RFCR_DROP_UNWANTED_CTL |
- MT_WF_RFCR_DROP_STBC_MULTI);
-
- MT76_FILTER(OTHER_BSS, MT_WF_RFCR_DROP_OTHER_TIM |
- MT_WF_RFCR_DROP_A3_MAC |
- MT_WF_RFCR_DROP_A3_BSSID);
-
- MT76_FILTER(FCSFAIL, MT_WF_RFCR_DROP_FCSFAIL);
-
- MT76_FILTER(CONTROL, MT_WF_RFCR_DROP_CTS |
- MT_WF_RFCR_DROP_RTS |
- MT_WF_RFCR_DROP_CTL_RSV |
- MT_WF_RFCR_DROP_NDPA);
-
- *total_flags = flags;
- mt76_wr(dev, MT_WF_RFCR(0), dev->mt76.rxfilter);
-
- if (*total_flags & FIF_CONTROL)
- mt76_clear(dev, MT_WF_RFCR1(0), ctl_flags);
- else
- mt76_set(dev, MT_WF_RFCR1(0), ctl_flags);
-
+ mt7921_mcu_set_rxfilter(dev, *total_flags, 0, 0);
mt7921_mutex_release(dev);
+
+ *total_flags &= (FIF_OTHER_BSS | FIF_FCSFAIL | FIF_CONTROL);
}

static void mt7921_bss_info_changed(struct ieee80211_hw *hw,
diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/mcu.c b/drivers/net/wireless/mediatek/mt76/mt7921/mcu.c
index 8930b5a4467c..8aeb030fb763 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7921/mcu.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7921/mcu.c
@@ -1019,6 +1019,8 @@ int mt7921_mcu_set_beacon_filter(struct mt7921_dev *dev,
struct ieee80211_vif *vif,
bool enable)
{
+#define MT7921_FIF_BIT_CLR BIT(1)
+#define MT7921_FIF_BIT_SET BIT(0)
int err;

if (enable) {
@@ -1026,7 +1028,11 @@ int mt7921_mcu_set_beacon_filter(struct mt7921_dev *dev,
if (err)
return err;

- mt76_set(dev, MT_WF_RFCR(0), MT_WF_RFCR_DROP_OTHER_BEACON);
+ err = mt7921_mcu_set_rxfilter(dev, 0,
+ MT7921_FIF_BIT_SET,
+ MT_WF_RFCR_DROP_OTHER_BEACON);
+ if (err)
+ return err;

return 0;
}
@@ -1035,7 +1041,11 @@ int mt7921_mcu_set_beacon_filter(struct mt7921_dev *dev,
if (err)
return err;

- mt76_clear(dev, MT_WF_RFCR(0), MT_WF_RFCR_DROP_OTHER_BEACON);
+ err = mt7921_mcu_set_rxfilter(dev, 0,
+ MT7921_FIF_BIT_CLR,
+ MT_WF_RFCR_DROP_OTHER_BEACON);
+ if (err)
+ return err;

return 0;
}
@@ -1255,3 +1265,25 @@ int mt7921_mcu_set_clc(struct mt7921_dev *dev, u8 *alpha2,
}
return 0;
}
+
+int mt7921_mcu_set_rxfilter(struct mt7921_dev *dev, u32 fif,
+ u8 bit_op, u32 bit_map)
+{
+ struct {
+ u8 rsv[4];
+ u8 mode;
+ u8 rsv2[3];
+ __le32 fif;
+ __le32 bit_map; /* bit_* for bitmap update */
+ u8 bit_op;
+ u8 pad[51];
+ } __packed data = {
+ .mode = fif ? 1 : 2,
+ .fif = cpu_to_le32(fif),
+ .bit_map = cpu_to_le32(bit_map),
+ .bit_op = bit_op,
+ };
+
+ return mt76_mcu_send_msg(&dev->mt76, MCU_CE_CMD(SET_RX_FILTER),
+ &data, sizeof(data), false);
+}
diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/mt7921.h b/drivers/net/wireless/mediatek/mt76/mt7921/mt7921.h
index efff4d43d796..248a4bc30955 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7921/mt7921.h
+++ b/drivers/net/wireless/mediatek/mt76/mt7921/mt7921.h
@@ -383,6 +383,8 @@ int mt7921_mcu_get_rx_rate(struct mt7921_phy *phy, struct ieee80211_vif *vif,
struct ieee80211_sta *sta, struct rate_info *rate);
int mt7921_mcu_fw_log_2_host(struct mt7921_dev *dev, u8 ctrl);
void mt7921_mcu_rx_event(struct mt7921_dev *dev, struct sk_buff *skb);
+int mt7921_mcu_set_rxfilter(struct mt7921_dev *dev, u32 fif,
+ u8 bit_op, u32 bit_map);

static inline void mt7921_irq_enable(struct mt7921_dev *dev, u32 mask)
{
diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/testmode.c b/drivers/net/wireless/mediatek/mt76/mt7921/testmode.c
index bdec8684ce94..7f408212e716 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7921/testmode.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7921/testmode.c
@@ -59,7 +59,6 @@ mt7921_tm_set(struct mt7921_dev *dev, struct mt7921_tm_cmd *req)
cancel_work_sync(&pm->wake_work);
__mt7921_mcu_drv_pmctrl(dev);

- mt76_wr(dev, MT_WF_RFCR(0), dev->mt76.rxfilter);
phy->test.state = MT76_TM_STATE_ON;
}

--
2.18.0


Subject: Re: [PATCH] wifi: mt76: mt7921: fix rx filter incorrect by drv/fw inconsistent

Il 17/01/23 10:30, Deren Wu ha scritto:
> From: Neil Chen <[email protected]>
>
> The rx filter, in mt7921 series, may be changed in fw operation. There is
> a racing problem if rx filter controlled by both driver and firmware at
> the same time. To avoid this issue, let mt7921 driver set rx filter by new
> command MCU_CE_CMD_SET_RX_FILTER and allow the firmware controlling it
> only.
>
> Reviewed-by: Lorenzo Bianconi <[email protected]>
> Co-developed-by: Deren Wu <[email protected]>
> Signed-off-by: Deren Wu <[email protected]>
> Signed-off-by: Neil Chen <[email protected]>

For MT7921E:

Tested-by: AngeloGioacchino Del Regno <[email protected]>


2023-02-10 09:09:27

by Felix Fietkau

[permalink] [raw]
Subject: Re: [PATCH] wifi: mt76: mt7921: fix rx filter incorrect by drv/fw inconsistent

On 17.01.23 10:30, Deren Wu wrote:
> From: Neil Chen <[email protected]>
>
> The rx filter, in mt7921 series, may be changed in fw operation. There is
> a racing problem if rx filter controlled by both driver and firmware at
> the same time. To avoid this issue, let mt7921 driver set rx filter by new
> command MCU_CE_CMD_SET_RX_FILTER and allow the firmware controlling it
> only.
>
> Reviewed-by: Lorenzo Bianconi <[email protected]>
> Co-developed-by: Deren Wu <[email protected]>
> Signed-off-by: Deren Wu <[email protected]>
> Signed-off-by: Neil Chen <[email protected]>
Somebody on github reported that this patch breaks scanning. After
looking over it once more, I noticed something very odd:
It seems that you are dropping the mac80211 FIF_* -> mt76 filter flags
conversion and passing mac80211 flags to the firmware by using
*total_flags as an argument to mt7921_mcu_set_rxfilter.
This does not make any sense to me.

Shouldn't you leave the existing conversion logic in place, and just
pass the new dev->mt76.rxfilter value to the firmware?

- Felix