From cdeccacaee7660679e6474edafaea3d52e6816b6 Mon Sep 17 00:00:00 2001 From: Luis R. Rodriguez Date: Tue, 19 Oct 2010 12:50:53 -0700 Subject: [PATCH 01/15] cfg80211: pass the reg hint initiator to helpers This is required later. Cc: Easwar Krishnan Cc: stable@kernel.org signed-off-by: Luis R. Rodriguez --- net/wireless/reg.c | 12 ++++++++---- 1 files changed, 8 insertions(+), 4 deletions(-) diff --git a/net/wireless/reg.c b/net/wireless/reg.c index 4b9f891..b64596f 100644 --- a/net/wireless/reg.c +++ b/net/wireless/reg.c @@ -720,7 +720,9 @@ EXPORT_SYMBOL(freq_reg_info); * on the wiphy with the target_bw specified. Then we can simply use * that below for the desired_bw_khz below. */ -static void handle_channel(struct wiphy *wiphy, enum ieee80211_band band, +static void handle_channel(struct wiphy *wiphy, + enum nl80211_reg_initiator initiator, + enum ieee80211_band band, unsigned int chan_idx) { int r; @@ -784,7 +786,9 @@ static void handle_channel(struct wiphy *wiphy, enum ieee80211_band band, chan->max_power = (int) MBM_TO_DBM(power_rule->max_eirp); } -static void handle_band(struct wiphy *wiphy, enum ieee80211_band band) +static void handle_band(struct wiphy *wiphy, + enum ieee80211_band band, + enum nl80211_reg_initiator initiator) { unsigned int i; struct ieee80211_supported_band *sband; @@ -793,7 +797,7 @@ static void handle_band(struct wiphy *wiphy, enum ieee80211_band band) sband = wiphy->bands[band]; for (i = 0; i < sband->n_channels; i++) - handle_channel(wiphy, band, i); + handle_channel(wiphy, initiator, band, i); } static bool ignore_reg_update(struct wiphy *wiphy, @@ -1030,7 +1034,7 @@ void wiphy_update_regulatory(struct wiphy *wiphy, goto out; for (band = 0; band < IEEE80211_NUM_BANDS; band++) { if (wiphy->bands[band]) - handle_band(wiphy, band); + handle_band(wiphy, band, initiator); } out: reg_process_beacons(wiphy); -- 1.7.1 From b41f9b3fc5631289f8272fb1d3638f0229843371 Mon Sep 17 00:00:00 2001 From: Luis R. Rodriguez Date: Tue, 19 Oct 2010 12:53:42 -0700 Subject: [PATCH 02/15] cfg80211: fix allowing country IEs for WIPHY_FLAG_STRICT_REGULATORY We should be enabling country IE hints for WIPHY_FLAG_STRICT_REGULATORY even if we haven't yet recieved regulatory domain hint for the driver if it needed one. Without this Country IEs are not passed on to drivers that have set WIPHY_FLAG_STRICT_REGULATORY, today this is just all Atheros chipset drivers: ath5k, ath9k, ar9170, carl9170. This was part of the original design, however it was completely overlooked... Cc: Easwar Krishnan Cc: stable@kernel.org Signed-off-by: Luis R. Rodriguez --- include/net/cfg80211.h | 15 ++++++++------- net/wireless/reg.c | 1 + 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h index 2a7936d..352ebc5 100644 --- a/include/net/cfg80211.h +++ b/include/net/cfg80211.h @@ -1321,13 +1321,14 @@ struct cfg80211_ops { * initiator is %REGDOM_SET_BY_CORE). * @WIPHY_FLAG_STRICT_REGULATORY: tells us the driver for this device will * ignore regulatory domain settings until it gets its own regulatory - * domain via its regulatory_hint(). After its gets its own regulatory - * domain it will only allow further regulatory domain settings to - * further enhance compliance. For example if channel 13 and 14 are - * disabled by this regulatory domain no user regulatory domain can - * enable these channels at a later time. This can be used for devices - * which do not have calibration information gauranteed for frequencies - * or settings outside of its regulatory domain. + * domain via its regulatory_hint() unless the regulatory hint is + * from a country IE. After its gets its own regulatory domain it will + * only allow further regulatory domain settings to further enhance + * compliance. For example if channel 13 and 14 are disabled by this + * regulatory domain no user regulatory domain can enable these channels + * at a later time. This can be used for devices which do not have + * calibration information gauranteed for frequencies or settings + * outside of its regulatory domain. * @WIPHY_FLAG_DISABLE_BEACON_HINTS: enable this if your driver needs to ensure * that passive scan flags and beaconing flags may not be lifted by * cfg80211 due to regulatory beacon hints. For more information on beacon diff --git a/net/wireless/reg.c b/net/wireless/reg.c index b64596f..1bc8131 100644 --- a/net/wireless/reg.c +++ b/net/wireless/reg.c @@ -813,6 +813,7 @@ static bool ignore_reg_update(struct wiphy *wiphy, * desired regulatory domain set */ if (wiphy->flags & WIPHY_FLAG_STRICT_REGULATORY && !wiphy->regd && + initiator != NL80211_REGDOM_SET_BY_COUNTRY_IE && !is_world_regdom(last_request->alpha2)) return true; return false; -- 1.7.1 From fde58905a1f459d0cdef164ea7c75256a0dfc635 Mon Sep 17 00:00:00 2001 From: Luis R. Rodriguez Date: Tue, 19 Oct 2010 12:53:42 -0700 Subject: [PATCH 03/15] cfg80211: fix disabling channels based on hints After a module loads you will have loaded the world roaming regulatory domain or a custom regulatory domain. Further regulatory hints are welcomed and should be respected unless the regulatory hint is coming from a country IE as the IEEE spec allows for a country IE to be a subset of what is allowed by the local regulatory agencies. So disable all channels that do not fit a regulatory domain sent from a unless the hint is from a country IE and the country IE had no information about the band we are currently processing. This fixes a few regulatory issues, for example for drivers that depend on CRDA and had no 5 GHz freqencies allowed were not properly disabling 5 GHz at all, furthermore it also allows users to restrict devices further as was intended. If you recieve a country IE upon association we will also disable the channels that are not allowed if the country IE had at least one channel on the respective band we are procesing. This was the original intention behind this design but it was completely overlooked... Cc: David Quan Cc: Jouni Malinen cc: Easwar Krishnan Cc: stable@kernel.org Signed-off-by: Luis R. Rodriguez --- include/linux/nl80211.h | 6 +++++- net/wireless/reg.c | 20 +++++++++++++++++++- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/include/linux/nl80211.h b/include/linux/nl80211.h index 0edb256..fb877b5 100644 --- a/include/linux/nl80211.h +++ b/include/linux/nl80211.h @@ -1307,7 +1307,11 @@ enum nl80211_bitrate_attr { * wireless core it thinks its knows the regulatory domain we should be in. * @NL80211_REGDOM_SET_BY_COUNTRY_IE: the wireless core has received an * 802.11 country information element with regulatory information it - * thinks we should consider. + * thinks we should consider. cfg80211 only processes the country + * code from the IE, and relies on the regulatory domain information + * structure pased by userspace (CRDA) from our wireless-regdb. + * If a channel is enabled but the country code indicates it should + * be disabled we disable the channel and re-enable it upon disassociation. */ enum nl80211_reg_initiator { NL80211_REGDOM_SET_BY_CORE, diff --git a/net/wireless/reg.c b/net/wireless/reg.c index 1bc8131..8ab65f2 100644 --- a/net/wireless/reg.c +++ b/net/wireless/reg.c @@ -750,8 +750,26 @@ static void handle_channel(struct wiphy *wiphy, desired_bw_khz, ®_rule); - if (r) + if (r) { + /* + * We will disable all channels that do not match our + * recieved regulatory rule unless the hint is coming + * from a Country IE and the Country IE had no information + * about a band. The IEEE 802.11 spec allows for an AP + * to send only a subset of the regulatory rules allowed, + * so an AP in the US that only supports 2.4 GHz may only send + * a country IE with information for the 2.4 GHz band + * while 5 GHz is still supported. + */ + if (initiator == NL80211_REGDOM_SET_BY_COUNTRY_IE && + r == -ERANGE) + return; + + REG_DBG_PRINT("cfg80211: Disabling freq %d MHz\n", + chan->center_freq); + chan->flags = IEEE80211_CHAN_DISABLED; return; + } power_rule = ®_rule->power_rule; freq_range = ®_rule->freq_range; -- 1.7.1 From b02573a76dc98875b4bc16d5f5c66d9ef8e4aa26 Mon Sep 17 00:00:00 2001 From: Luis R. Rodriguez Date: Tue, 19 Oct 2010 14:39:58 -0700 Subject: [PATCH 04/15] cfg80211: add debug prints for when we ignore regulatory hints This can help with debugging issues. You will only see these with CONFIG_CFG80211_REG_DEBUG enabled. Cc: Easwar Krishnan Signed-off-by: Luis R. Rodriguez --- net/wireless/reg.c | 42 +++++++++++++++++++++++++++++++++++++++--- 1 files changed, 39 insertions(+), 3 deletions(-) diff --git a/net/wireless/reg.c b/net/wireless/reg.c index 8ab65f2..7bff1c1 100644 --- a/net/wireless/reg.c +++ b/net/wireless/reg.c @@ -711,6 +711,25 @@ int freq_reg_info(struct wiphy *wiphy, } EXPORT_SYMBOL(freq_reg_info); +#ifdef CONFIG_CFG80211_REG_DEBUG +static const char *reg_initiator_name(enum nl80211_reg_initiator initiator) +{ + switch (initiator) { + case NL80211_REGDOM_SET_BY_CORE: + return "Set by core"; + case NL80211_REGDOM_SET_BY_USER: + return "Set by user"; + case NL80211_REGDOM_SET_BY_DRIVER: + return "Set by driver"; + case NL80211_REGDOM_SET_BY_COUNTRY_IE: + return "Set by country IE"; + default: + WARN_ON(1); + return "Set by bug"; + } +} +#endif + /* * Note that right now we assume the desired channel bandwidth * is always 20 MHz for each individual channel (HT40 uses 20 MHz @@ -821,19 +840,36 @@ static void handle_band(struct wiphy *wiphy, static bool ignore_reg_update(struct wiphy *wiphy, enum nl80211_reg_initiator initiator) { - if (!last_request) + if (!last_request) { + REG_DBG_PRINT("cfg80211: Ignoring regulatory request %s since " + "last_request is not set\n", + reg_initiator_name(initiator)); return true; + } + if (initiator == NL80211_REGDOM_SET_BY_CORE && - wiphy->flags & WIPHY_FLAG_CUSTOM_REGULATORY) + wiphy->flags & WIPHY_FLAG_CUSTOM_REGULATORY) { + REG_DBG_PRINT("cfg80211: Ignoring regulatory request %s " + "since the driver uses its own custom " + "regulatory domain ", + reg_initiator_name(initiator)); return true; + } + /* * wiphy->regd will be set once the device has its own * desired regulatory domain set */ if (wiphy->flags & WIPHY_FLAG_STRICT_REGULATORY && !wiphy->regd && initiator != NL80211_REGDOM_SET_BY_COUNTRY_IE && - !is_world_regdom(last_request->alpha2)) + !is_world_regdom(last_request->alpha2)) { + REG_DBG_PRINT("cfg80211: Ignoring regulatory request %s " + "since the driver requires its own regulaotry " + "domain to be set first", + reg_initiator_name(initiator)); return true; + } + return false; } -- 1.7.1 From 5c5e4ff39a8adf4d756dbd1949cd4ecd5f0092bf Mon Sep 17 00:00:00 2001 From: Luis R. Rodriguez Date: Tue, 19 Oct 2010 17:01:01 -0700 Subject: [PATCH 05/15] cfg80211: add debug print when disabling a channel on a custom regd Cc: Easwar Krishnan Signed-off-by: Luis R. Rodriguez --- net/wireless/reg.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/net/wireless/reg.c b/net/wireless/reg.c index 7bff1c1..6e7a9d8 100644 --- a/net/wireless/reg.c +++ b/net/wireless/reg.c @@ -1125,6 +1125,11 @@ static void handle_channel_custom(struct wiphy *wiphy, regd); if (r) { + REG_DBG_PRINT("cfg80211: Disabling freq %d MHz as custom " + "regd has no rule that fits a %d MHz " + "wide channel\n", + chan->center_freq, + KHZ_TO_MHZ(desired_bw_khz)); chan->flags = IEEE80211_CHAN_DISABLED; return; } -- 1.7.1 From be9c8feda9701b1a5cf1afb8db34c0b00d87d0e4 Mon Sep 17 00:00:00 2001 From: Luis R. Rodriguez Date: Tue, 19 Oct 2010 17:48:34 -0700 Subject: [PATCH 06/15] cfg80211: add debug print when processing a channel In the worst case you are seeing really odd things you want more information than what is provided right now, for those that insist and want debug info through CONFIG_CFG80211_REG_DEBUG provide a print of when we are processing a channel and with what regulatory rule. Cc: Easwar Krishnan Signed-off-by: Luis R. Rodriguez --- net/wireless/reg.c | 39 +++++++++++++++++++++++++++++++++++++++ 1 files changed, 39 insertions(+), 0 deletions(-) diff --git a/net/wireless/reg.c b/net/wireless/reg.c index 6e7a9d8..5556c5f 100644 --- a/net/wireless/reg.c +++ b/net/wireless/reg.c @@ -728,6 +728,41 @@ static const char *reg_initiator_name(enum nl80211_reg_initiator initiator) return "Set by bug"; } } + +static void chan_reg_rule_print_dbg(struct ieee80211_channel *chan, + u32 desired_bw_khz, + const struct ieee80211_reg_rule *reg_rule) +{ + const struct ieee80211_power_rule *power_rule; + const struct ieee80211_freq_range *freq_range; + char max_antenna_gain[32]; + + power_rule = ®_rule->power_rule; + freq_range = ®_rule->freq_range; + + if (!power_rule->max_antenna_gain) + snprintf(max_antenna_gain, 32, "N/A"); + else + snprintf(max_antenna_gain, 32, "%d", power_rule->max_antenna_gain); + + REG_DBG_PRINT("cfg80211: Updating information on frequency %d MHz " + "for %d a MHz width channel with regulatory rule:\n", + chan->center_freq, + KHZ_TO_MHZ(desired_bw_khz)); + + REG_DBG_PRINT("cfg80211: %d KHz - %d KHz @ KHz), (%s mBi, %d mBm)\n", + freq_range->start_freq_khz, + freq_range->end_freq_khz, + max_antenna_gain, + power_rule->max_eirp); +} +#else +static void chan_reg_rule_print_dbg(struct ieee80211_channel *chan, + u32 desired_bw_khz, + const struct ieee80211_reg_rule *reg_rule) +{ + return; +} #endif /* @@ -790,6 +825,8 @@ static void handle_channel(struct wiphy *wiphy, return; } + chan_reg_rule_print_dbg(chan, desired_bw_khz, reg_rule); + power_rule = ®_rule->power_rule; freq_range = ®_rule->freq_range; @@ -1134,6 +1171,8 @@ static void handle_channel_custom(struct wiphy *wiphy, return; } + chan_reg_rule_print_dbg(chan, desired_bw_khz, reg_rule); + power_rule = ®_rule->power_rule; freq_range = ®_rule->freq_range; -- 1.7.1 From 66ab06294afb9d54090e39e7f13a5241bdf4facf Mon Sep 17 00:00:00 2001 From: Luis R. Rodriguez Date: Tue, 19 Oct 2010 17:05:04 -0700 Subject: [PATCH 07/15] cfg80211: prefix REG_DBG_PRINT() with cfg80211 Everyone's doing it, its the cool thing. Cc: Easwar Krishnan Signed-off-by: Luis R. Rodriguez --- net/wireless/reg.c | 31 +++++++++++++++---------------- 1 files changed, 15 insertions(+), 16 deletions(-) diff --git a/net/wireless/reg.c b/net/wireless/reg.c index 5556c5f..3be18d9 100644 --- a/net/wireless/reg.c +++ b/net/wireless/reg.c @@ -48,7 +48,7 @@ #ifdef CONFIG_CFG80211_REG_DEBUG #define REG_DBG_PRINT(format, args...) \ do { \ - printk(KERN_DEBUG format , ## args); \ + printk(KERN_DEBUG "cfg80211: " format , ## args); \ } while (0) #else #define REG_DBG_PRINT(args...) @@ -745,12 +745,12 @@ static void chan_reg_rule_print_dbg(struct ieee80211_channel *chan, else snprintf(max_antenna_gain, 32, "%d", power_rule->max_antenna_gain); - REG_DBG_PRINT("cfg80211: Updating information on frequency %d MHz " + REG_DBG_PRINT("Updating information on frequency %d MHz " "for %d a MHz width channel with regulatory rule:\n", chan->center_freq, KHZ_TO_MHZ(desired_bw_khz)); - REG_DBG_PRINT("cfg80211: %d KHz - %d KHz @ KHz), (%s mBi, %d mBm)\n", + REG_DBG_PRINT("%d KHz - %d KHz @ KHz), (%s mBi, %d mBm)\n", freq_range->start_freq_khz, freq_range->end_freq_khz, max_antenna_gain, @@ -819,8 +819,7 @@ static void handle_channel(struct wiphy *wiphy, r == -ERANGE) return; - REG_DBG_PRINT("cfg80211: Disabling freq %d MHz\n", - chan->center_freq); + REG_DBG_PRINT("Disabling freq %d MHz\n", chan->center_freq); chan->flags = IEEE80211_CHAN_DISABLED; return; } @@ -878,7 +877,7 @@ static bool ignore_reg_update(struct wiphy *wiphy, enum nl80211_reg_initiator initiator) { if (!last_request) { - REG_DBG_PRINT("cfg80211: Ignoring regulatory request %s since " + REG_DBG_PRINT("Ignoring regulatory request %s since " "last_request is not set\n", reg_initiator_name(initiator)); return true; @@ -886,7 +885,7 @@ static bool ignore_reg_update(struct wiphy *wiphy, if (initiator == NL80211_REGDOM_SET_BY_CORE && wiphy->flags & WIPHY_FLAG_CUSTOM_REGULATORY) { - REG_DBG_PRINT("cfg80211: Ignoring regulatory request %s " + REG_DBG_PRINT("Ignoring regulatory request %s " "since the driver uses its own custom " "regulatory domain ", reg_initiator_name(initiator)); @@ -900,7 +899,7 @@ static bool ignore_reg_update(struct wiphy *wiphy, if (wiphy->flags & WIPHY_FLAG_STRICT_REGULATORY && !wiphy->regd && initiator != NL80211_REGDOM_SET_BY_COUNTRY_IE && !is_world_regdom(last_request->alpha2)) { - REG_DBG_PRINT("cfg80211: Ignoring regulatory request %s " + REG_DBG_PRINT("Ignoring regulatory request %s " "since the driver requires its own regulaotry " "domain to be set first", reg_initiator_name(initiator)); @@ -1162,7 +1161,7 @@ static void handle_channel_custom(struct wiphy *wiphy, regd); if (r) { - REG_DBG_PRINT("cfg80211: Disabling freq %d MHz as custom " + REG_DBG_PRINT("Disabling freq %d MHz as custom " "regd has no rule that fits a %d MHz " "wide channel\n", chan->center_freq, @@ -1662,7 +1661,7 @@ static void restore_alpha2(char *alpha2, bool reset_user) if (is_user_regdom_saved()) { /* Unless we're asked to ignore it and reset it */ if (reset_user) { - REG_DBG_PRINT("cfg80211: Restoring regulatory settings " + REG_DBG_PRINT("Restoring regulatory settings " "including user preference\n"); user_alpha2[0] = '9'; user_alpha2[1] = '7'; @@ -1673,7 +1672,7 @@ static void restore_alpha2(char *alpha2, bool reset_user) * back as they were for a full restore. */ if (!is_world_regdom(ieee80211_regdom)) { - REG_DBG_PRINT("cfg80211: Keeping preference on " + REG_DBG_PRINT("Keeping preference on " "module parameter ieee80211_regdom: %c%c\n", ieee80211_regdom[0], ieee80211_regdom[1]); @@ -1681,7 +1680,7 @@ static void restore_alpha2(char *alpha2, bool reset_user) alpha2[1] = ieee80211_regdom[1]; } } else { - REG_DBG_PRINT("cfg80211: Restoring regulatory settings " + REG_DBG_PRINT("Restoring regulatory settings " "while preserving user preference for: %c%c\n", user_alpha2[0], user_alpha2[1]); @@ -1689,14 +1688,14 @@ static void restore_alpha2(char *alpha2, bool reset_user) alpha2[1] = user_alpha2[1]; } } else if (!is_world_regdom(ieee80211_regdom)) { - REG_DBG_PRINT("cfg80211: Keeping preference on " + REG_DBG_PRINT("Keeping preference on " "module parameter ieee80211_regdom: %c%c\n", ieee80211_regdom[0], ieee80211_regdom[1]); alpha2[0] = ieee80211_regdom[0]; alpha2[1] = ieee80211_regdom[1]; } else - REG_DBG_PRINT("cfg80211: Restoring regulatory settings\n"); + REG_DBG_PRINT("Restoring regulatory settings\n"); } /* @@ -1764,7 +1763,7 @@ static void restore_regulatory_settings(bool reset_user) void regulatory_hint_disconnect(void) { - REG_DBG_PRINT("cfg80211: All devices are disconnected, going to " + REG_DBG_PRINT("All devices are disconnected, going to " "restore regulatory settings\n"); restore_regulatory_settings(false); } @@ -1794,7 +1793,7 @@ int regulatory_hint_found_beacon(struct wiphy *wiphy, if (!reg_beacon) return -ENOMEM; - REG_DBG_PRINT("cfg80211: Found new beacon on " + REG_DBG_PRINT("Found new beacon on " "frequency: %d MHz (Ch %d) on %s\n", beacon_chan->center_freq, ieee80211_frequency_to_channel(beacon_chan->center_freq), -- 1.7.1 From f61580751fe4181b81357f62e85152f4d5c4b970 Mon Sep 17 00:00:00 2001 From: Luis R. Rodriguez Date: Fri, 15 Oct 2010 17:02:08 -0700 Subject: [PATCH 08/15] ath9k: add locking for stopping RX ath9k locks for starting RX but not for stopping RX. We could potentially run into a situation where tried to stop RX but immediately started RX. This allows for races on the the RX engine deciding what buffer we last left off on and could potentially cause ath9k to DMA into already free'd memory or in the worst case at a later time to already given memory to other drivers. Fix this by locking stopping RX. This is part of a series that will help resolve the bug: https://bugzilla.kernel.org/show_bug.cgi?id=14624 For more details about this issue refer to: http://marc.info/?l=linux-wireless&m=128629803703756&w=2 Cc: stable@kernel.org Cc: Ben Greear Cc: Kyungwan Nam Signed-off-by: Luis R. Rodriguez --- drivers/net/wireless/ath/ath9k/recv.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/wireless/ath/ath9k/recv.c b/drivers/net/wireless/ath/ath9k/recv.c index 2b2c318..3b19bbb 100644 --- a/drivers/net/wireless/ath/ath9k/recv.c +++ b/drivers/net/wireless/ath/ath9k/recv.c @@ -306,10 +306,8 @@ static void ath_edma_start_recv(struct ath_softc *sc) static void ath_edma_stop_recv(struct ath_softc *sc) { - spin_lock_bh(&sc->rx.rxbuflock); ath_rx_remove_buffer(sc, ATH9K_RX_QUEUE_HP); ath_rx_remove_buffer(sc, ATH9K_RX_QUEUE_LP); - spin_unlock_bh(&sc->rx.rxbuflock); } int ath_rx_init(struct ath_softc *sc, int nbufs) @@ -518,6 +516,7 @@ bool ath_stoprecv(struct ath_softc *sc) struct ath_hw *ah = sc->sc_ah; bool stopped; + spin_lock_bh(&sc->rx.rxbuflock); ath9k_hw_stoppcurecv(ah); ath9k_hw_setrxfilter(ah, 0); stopped = ath9k_hw_stopdmarecv(ah); @@ -526,6 +525,7 @@ bool ath_stoprecv(struct ath_softc *sc) ath_edma_stop_recv(sc); else sc->rx.rxlink = NULL; + spin_unlock_bh(&sc->rx.rxbuflock); return stopped; } -- 1.7.1 From 6f53e3ab69ffbf4e465b96f756fe8399945b4e35 Mon Sep 17 00:00:00 2001 From: Luis R. Rodriguez Date: Wed, 20 Oct 2010 10:57:56 -0700 Subject: [PATCH 09/15] ath9k: add locking for starting the PCU on RX There was some locking for starting some parts of RX but not for starting the PCU. Include this otherwise we can content against stopping the PCU. This can potentially lead to races against different buffers on the PCU which can lead to to the DMA RX engine writing to buffers which are already freed. This is part of a series that will help resolve the bug: https://bugzilla.kernel.org/show_bug.cgi?id=14624 For more details about this issue refer to: http://marc.info/?l=linux-wireless&m=128629803703756&w=2 Cc: stable@kernel.org Cc: Ben Greear Cc: Kyungwan Nam Signed-off-by: Luis R. Rodriguez --- drivers/net/wireless/ath/ath9k/recv.c | 7 ++++--- 1 files changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/net/wireless/ath/ath9k/recv.c b/drivers/net/wireless/ath/ath9k/recv.c index 3b19bbb..944fb59 100644 --- a/drivers/net/wireless/ath/ath9k/recv.c +++ b/drivers/net/wireless/ath/ath9k/recv.c @@ -297,11 +297,11 @@ static void ath_edma_start_recv(struct ath_softc *sc) ath_rx_addbuffer_edma(sc, ATH9K_RX_QUEUE_LP, sc->rx.rx_edma[ATH9K_RX_QUEUE_LP].rx_fifo_hwsize); - spin_unlock_bh(&sc->rx.rxbuflock); - ath_opmode_init(sc); ath9k_hw_startpcureceive(sc->sc_ah, (sc->sc_flags & SC_OP_OFFCHANNEL)); + + spin_unlock_bh(&sc->rx.rxbuflock); } static void ath_edma_stop_recv(struct ath_softc *sc) @@ -504,10 +504,11 @@ int ath_startrecv(struct ath_softc *sc) ath9k_hw_rxena(ah); start_recv: - spin_unlock_bh(&sc->rx.rxbuflock); ath_opmode_init(sc); ath9k_hw_startpcureceive(ah, (sc->sc_flags & SC_OP_OFFCHANNEL)); + spin_unlock_bh(&sc->rx.rxbuflock); + return 0; } -- 1.7.1 From 5133c5a0feee1c072563bcf7eba1e5b2409d2dfe Mon Sep 17 00:00:00 2001 From: Luis R. Rodriguez Date: Wed, 20 Oct 2010 13:38:09 -0700 Subject: [PATCH 10/15] ath9k: rename rxflushlock to pcu_lock The real way to lock RX is to contend on the PCU and reset, this will be fixed in the next patch but for now just do the renames so that the next patch which changes the locking order is crystal clear. This is part of a series that will help resolve the bug: https://bugzilla.kernel.org/show_bug.cgi?id=14624 For more details about this issue refer to: http://marc.info/?l=linux-wireless&m=128629803703756&w=2 Cc: stable@kernel.org Cc: Ben Greear Cc: Kyungwan Nam Signed-off-by: Luis R. Rodriguez --- drivers/net/wireless/ath/ath9k/ath9k.h | 2 +- drivers/net/wireless/ath/ath9k/main.c | 4 ++-- drivers/net/wireless/ath/ath9k/recv.c | 6 +++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h index 0f0bc54..81fed83 100644 --- a/drivers/net/wireless/ath/ath9k/ath9k.h +++ b/drivers/net/wireless/ath/ath9k/ath9k.h @@ -309,7 +309,7 @@ struct ath_rx { u8 rxotherant; u32 *rxlink; unsigned int rxfilter; - spinlock_t rxflushlock; + spinlock_t pcu_lock; spinlock_t rxbuflock; struct list_head rxbuf; struct ath_descdma rxdma; diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c index da6578a..3c66eda 100644 --- a/drivers/net/wireless/ath/ath9k/main.c +++ b/drivers/net/wireless/ath/ath9k/main.c @@ -612,7 +612,7 @@ void ath9k_tasklet(unsigned long data) rxmask = (ATH9K_INT_RX | ATH9K_INT_RXEOL | ATH9K_INT_RXORN); if (status & rxmask) { - spin_lock_bh(&sc->rx.rxflushlock); + spin_lock_bh(&sc->rx.pcu_lock); /* Check for high priority Rx first */ if ((ah->caps.hw_caps & ATH9K_HW_CAP_EDMA) && @@ -620,7 +620,7 @@ void ath9k_tasklet(unsigned long data) ath_rx_tasklet(sc, 0, true); ath_rx_tasklet(sc, 0, false); - spin_unlock_bh(&sc->rx.rxflushlock); + spin_unlock_bh(&sc->rx.pcu_lock); } if (status & ATH9K_INT_TX) { diff --git a/drivers/net/wireless/ath/ath9k/recv.c b/drivers/net/wireless/ath/ath9k/recv.c index 944fb59..b099827 100644 --- a/drivers/net/wireless/ath/ath9k/recv.c +++ b/drivers/net/wireless/ath/ath9k/recv.c @@ -317,7 +317,7 @@ int ath_rx_init(struct ath_softc *sc, int nbufs) struct ath_buf *bf; int error = 0; - spin_lock_init(&sc->rx.rxflushlock); + spin_lock_init(&sc->rx.pcu_lock); sc->sc_flags &= ~SC_OP_RXFLUSH; spin_lock_init(&sc->rx.rxbuflock); @@ -533,13 +533,13 @@ bool ath_stoprecv(struct ath_softc *sc) void ath_flushrecv(struct ath_softc *sc) { - spin_lock_bh(&sc->rx.rxflushlock); + spin_lock_bh(&sc->rx.pcu_lock); sc->sc_flags |= SC_OP_RXFLUSH; if (sc->sc_ah->caps.hw_caps & ATH9K_HW_CAP_EDMA) ath_rx_tasklet(sc, 1, true); ath_rx_tasklet(sc, 1, false); sc->sc_flags &= ~SC_OP_RXFLUSH; - spin_unlock_bh(&sc->rx.rxflushlock); + spin_unlock_bh(&sc->rx.pcu_lock); } static bool ath_beacon_dtim_pending_cab(struct sk_buff *skb) -- 1.7.1 From 78c7d4eadbccc2f9b8188ff9dcddaba9cb33ea2a Mon Sep 17 00:00:00 2001 From: Luis R. Rodriguez Date: Wed, 20 Oct 2010 13:25:10 -0700 Subject: [PATCH 11/15] ath9k: lock reset and PCU start/stopping Apart from locking the start and stop PCU we need to ensure we also content starting and stopping the PCU between hardware resets. This is part of a series that will help resolve the bug: https://bugzilla.kernel.org/show_bug.cgi?id=14624 For more details about this issue refer to: http://marc.info/?l=linux-wireless&m=128629803703756&w=2 Cc: stable@kernel.org Cc: Ben Greear Cc: Kyungwan Nam Signed-off-by: Luis R. Rodriguez --- drivers/net/wireless/ath/ath9k/main.c | 27 +++++++++++++++++++++++++++ drivers/net/wireless/ath/ath9k/recv.c | 2 -- 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c index 3c66eda..81e5f6d 100644 --- a/drivers/net/wireless/ath/ath9k/main.c +++ b/drivers/net/wireless/ath/ath9k/main.c @@ -241,6 +241,9 @@ int ath_set_channel(struct ath_softc *sc, struct ieee80211_hw *hw, */ ath9k_hw_disable_interrupts(ah); ath_drain_all_txq(sc, false); + + spin_lock_bh(&sc->rx.pcu_lock); + stopped = ath_stoprecv(sc); /* XXX: do not flush receive queue here. We don't want @@ -268,6 +271,7 @@ int ath_set_channel(struct ath_softc *sc, struct ieee80211_hw *hw, "reset status %d\n", channel->center_freq, r); spin_unlock_bh(&sc->sc_resetlock); + spin_unlock_bh(&sc->rx.pcu_lock); goto ps_restore; } spin_unlock_bh(&sc->sc_resetlock); @@ -276,9 +280,12 @@ int ath_set_channel(struct ath_softc *sc, struct ieee80211_hw *hw, ath_print(common, ATH_DBG_FATAL, "Unable to restart recv logic\n"); r = -EIO; + spin_unlock_bh(&sc->rx.pcu_lock); goto ps_restore; } + spin_unlock_bh(&sc->rx.pcu_lock); + ath_update_txpow(sc); ath9k_hw_set_interrupts(ah, ah->imask); @@ -878,6 +885,7 @@ void ath_radio_enable(struct ath_softc *sc, struct ieee80211_hw *hw) if (!ah->curchan) ah->curchan = ath_get_curchannel(sc, sc->hw); + spin_lock_bh(&sc->rx.pcu_lock); spin_lock_bh(&sc->sc_resetlock); r = ath9k_hw_reset(ah, ah->curchan, ah->caldata, false); if (r) { @@ -892,8 +900,10 @@ void ath_radio_enable(struct ath_softc *sc, struct ieee80211_hw *hw) if (ath_startrecv(sc) != 0) { ath_print(common, ATH_DBG_FATAL, "Unable to restart recv logic\n"); + spin_unlock_bh(&sc->rx.pcu_lock); return; } + spin_unlock_bh(&sc->rx.pcu_lock); if (sc->sc_flags & SC_OP_BEACONS) ath_beacon_config(sc, NULL); /* restart beacons */ @@ -932,6 +942,9 @@ void ath_radio_disable(struct ath_softc *sc, struct ieee80211_hw *hw) ath9k_hw_disable_interrupts(ah); ath_drain_all_txq(sc, false); /* clear pending tx frames */ + + spin_lock_bh(&sc->rx.pcu_lock); + ath_stoprecv(sc); /* turn off frame recv */ ath_flushrecv(sc); /* flush recv queue */ @@ -949,6 +962,9 @@ void ath_radio_disable(struct ath_softc *sc, struct ieee80211_hw *hw) spin_unlock_bh(&sc->sc_resetlock); ath9k_hw_phy_disable(ah); + + spin_unlock_bh(&sc->rx.pcu_lock); + ath9k_hw_configpcipowersave(ah, 1, 1); ath9k_ps_restore(sc); ath9k_setpower(sc, ATH9K_PM_FULL_SLEEP); @@ -968,6 +984,9 @@ int ath_reset(struct ath_softc *sc, bool retry_tx) ath9k_hw_disable_interrupts(ah); ath_drain_all_txq(sc, retry_tx); + + spin_lock_bh(&sc->rx.pcu_lock); + ath_stoprecv(sc); ath_flushrecv(sc); @@ -982,6 +1001,8 @@ int ath_reset(struct ath_softc *sc, bool retry_tx) ath_print(common, ATH_DBG_FATAL, "Unable to start recv logic\n"); + spin_unlock_bh(&sc->rx.pcu_lock); + /* * We may be doing a reset in response to a request * that changes the channel so update any state that @@ -1144,6 +1165,7 @@ static int ath9k_start(struct ieee80211_hw *hw) * be followed by initialization of the appropriate bits * and then setup of the interrupt mask. */ + spin_lock_bh(&sc->rx.pcu_lock); spin_lock_bh(&sc->sc_resetlock); r = ath9k_hw_reset(ah, init_channel, ah->caldata, false); if (r) { @@ -1152,6 +1174,7 @@ static int ath9k_start(struct ieee80211_hw *hw) "(freq %u MHz)\n", r, curchan->center_freq); spin_unlock_bh(&sc->sc_resetlock); + spin_unlock_bh(&sc->rx.pcu_lock); goto mutex_unlock; } spin_unlock_bh(&sc->sc_resetlock); @@ -1173,8 +1196,10 @@ static int ath9k_start(struct ieee80211_hw *hw) ath_print(common, ATH_DBG_FATAL, "Unable to start recv logic\n"); r = -EIO; + spin_unlock_bh(&sc->rx.pcu_lock); goto mutex_unlock; } + spin_unlock_bh(&sc->rx.pcu_lock); /* Setup our intr mask. */ ah->imask = ATH9K_INT_TX | ATH9K_INT_RXEOL | @@ -1373,12 +1398,14 @@ static void ath9k_stop(struct ieee80211_hw *hw) * before setting the invalid flag. */ ath9k_hw_disable_interrupts(ah); + spin_lock_bh(&sc->rx.pcu_lock); if (!(sc->sc_flags & SC_OP_INVALID)) { ath_drain_all_txq(sc, false); ath_stoprecv(sc); ath9k_hw_phy_disable(ah); } else sc->rx.rxlink = NULL; + spin_unlock_bh(&sc->rx.pcu_lock); /* disable HAL and put h/w to sleep */ ath9k_hw_disable(ah); diff --git a/drivers/net/wireless/ath/ath9k/recv.c b/drivers/net/wireless/ath/ath9k/recv.c index b099827..c04a940 100644 --- a/drivers/net/wireless/ath/ath9k/recv.c +++ b/drivers/net/wireless/ath/ath9k/recv.c @@ -533,13 +533,11 @@ bool ath_stoprecv(struct ath_softc *sc) void ath_flushrecv(struct ath_softc *sc) { - spin_lock_bh(&sc->rx.pcu_lock); sc->sc_flags |= SC_OP_RXFLUSH; if (sc->sc_ah->caps.hw_caps & ATH9K_HW_CAP_EDMA) ath_rx_tasklet(sc, 1, true); ath_rx_tasklet(sc, 1, false); sc->sc_flags &= ~SC_OP_RXFLUSH; - spin_unlock_bh(&sc->rx.pcu_lock); } static bool ath_beacon_dtim_pending_cab(struct sk_buff *skb) -- 1.7.1 From f3bef34d01cb1657d1b9dd85d242258d84f94171 Mon Sep 17 00:00:00 2001 From: Luis R. Rodriguez Date: Wed, 20 Oct 2010 13:56:43 -0700 Subject: [PATCH 12/15] ath: add a ATH_DBG_WARN() To be used to throw out warnings only for developers. This can be used by some corner cases that developers already know can be hit but developers want to address so to avoid spewing out a warning this can only be enabled with CONFIG_ATH_DEBUG enabled. Cc: Ben Greear Cc: Kyungwan Nam Signed-off-by: Luis R. Rodriguez --- drivers/net/wireless/ath/debug.h | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/drivers/net/wireless/ath/debug.h b/drivers/net/wireless/ath/debug.h index 64e4af2..15fc8ff 100644 --- a/drivers/net/wireless/ath/debug.h +++ b/drivers/net/wireless/ath/debug.h @@ -70,11 +70,13 @@ enum ATH_DEBUG { #ifdef CONFIG_ATH_DEBUG void ath_print(struct ath_common *common, int dbg_mask, const char *fmt, ...) __attribute__ ((format (printf, 3, 4))); +#define ATH_DBG_WARN(foo, arg...) WARN(foo, arg) #else static inline void __attribute__ ((format (printf, 3, 4))) ath_print(struct ath_common *common, int dbg_mask, const char *fmt, ...) { } +#define ATH_DBG_WARN(foo) #endif /* CONFIG_ATH_DEBUG */ /** Returns string describing opmode, or NULL if unknown mode. */ -- 1.7.1 From 6ad6ba943cb213315b2d34eb61a329873d95727e Mon Sep 17 00:00:00 2001 From: Luis R. Rodriguez Date: Wed, 20 Oct 2010 15:15:13 -0700 Subject: [PATCH 13/15] ath9k: add a debug warning when we cannot stop RX We have seen several DMA races when we race against stopping and starting the PCU. I suspect that when we cannot stop the PCU we may hit some of these same races so warn against them for now but only when debugging (CONFIG_ATH_DEBUG) is enabled. If you run into this warning and are a developer, please fix the cause of the warning. The potential here, although I cannot prove yet, is that the DMA engine can be confused and start writing to a buffer that was already DMA'd before and at least the kernel assumes is not being accessed by hardware anymore. Cc: Ben Greear Cc: Kyungwan Nam Signed-off-by: Luis R. Rodriguez --- drivers/net/wireless/ath/ath9k/recv.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/drivers/net/wireless/ath/ath9k/recv.c b/drivers/net/wireless/ath/ath9k/recv.c index c04a940..87fabf8 100644 --- a/drivers/net/wireless/ath/ath9k/recv.c +++ b/drivers/net/wireless/ath/ath9k/recv.c @@ -528,6 +528,8 @@ bool ath_stoprecv(struct ath_softc *sc) sc->rx.rxlink = NULL; spin_unlock_bh(&sc->rx.rxbuflock); + ATH_DBG_WARN(!stopped, "Could not stop RX, we could be " + "confusing the DMA engine when we start RX up\n"); return stopped; } -- 1.7.1 From bfe1eeff3d2194f81a1a0b6498fc3b84ba7d28e2 Mon Sep 17 00:00:00 2001 From: Felix Fietkau Date: Thu, 21 Oct 2010 02:47:23 +0200 Subject: [PATCH 14/15] ath9k_hw: fix potential spurious tx error bit interpretation According to documentation, AR_ExcessiveRetries, AR_Filtered and AR_FIFOUnderrun are only valid if AR_FrmXmitOK is clear. Not checking this might result in suboptimal FIFO settings, unnecessary retransmissions, or other connectivity issues. Signed-off-by: Felix Fietkau Cc: stable@kernel.org --- drivers/net/wireless/ath/ath9k/ar9002_mac.c | 16 +++++++++------- 1 files changed, 9 insertions(+), 7 deletions(-) diff --git a/drivers/net/wireless/ath/ath9k/ar9002_mac.c b/drivers/net/wireless/ath/ath9k/ar9002_mac.c index 3b4c52c..f0268e5 100644 --- a/drivers/net/wireless/ath/ath9k/ar9002_mac.c +++ b/drivers/net/wireless/ath/ath9k/ar9002_mac.c @@ -237,13 +237,15 @@ static int ar9002_hw_proc_txdesc(struct ath_hw *ah, void *ds, status = ACCESS_ONCE(ads->ds_txstatus1); if (status & AR_FrmXmitOK) ts->ts_status |= ATH9K_TX_ACKED; - if (status & AR_ExcessiveRetries) - ts->ts_status |= ATH9K_TXERR_XRETRY; - if (status & AR_Filtered) - ts->ts_status |= ATH9K_TXERR_FILT; - if (status & AR_FIFOUnderrun) { - ts->ts_status |= ATH9K_TXERR_FIFO; - ath9k_hw_updatetxtriglevel(ah, true); + else { + if (status & AR_ExcessiveRetries) + ts->ts_status |= ATH9K_TXERR_XRETRY; + if (status & AR_Filtered) + ts->ts_status |= ATH9K_TXERR_FILT; + if (status & AR_FIFOUnderrun) { + ts->ts_status |= ATH9K_TXERR_FIFO; + ath9k_hw_updatetxtriglevel(ah, true); + } } if (status & AR_TxTimerExpired) ts->ts_status |= ATH9K_TXERR_TIMER_EXPIRED; -- 1.7.1 From c83aac482e280ae7825787f3aabd71ab4b84fa5e Mon Sep 17 00:00:00 2001 From: Felix Fietkau Date: Thu, 21 Oct 2010 02:47:24 +0200 Subject: [PATCH 15/15] ath9k: fix handling of rate control probe frames The ath9k aggregation code was already checking the rate control probe flag to prevent starting an aggregate frame with a sampling rate. What was missing was closing an aggregate before adding a probing frame to it. Without that, rate control cannot have precise control over probing, which delays using faster rates when the channel conditions improve. Signed-off-by: Felix Fietkau --- drivers/net/wireless/ath/ath9k/xmit.c | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c index d077186..089b05e 100644 --- a/drivers/net/wireless/ath/ath9k/xmit.c +++ b/drivers/net/wireless/ath/ath9k/xmit.c @@ -673,6 +673,7 @@ static enum ATH_AGGR_STATUS ath_tx_form_aggr(struct ath_softc *sc, u16 aggr_limit = 0, al = 0, bpad = 0, al_delta, h_baw = tid->baw_size / 2; enum ATH_AGGR_STATUS status = ATH_AGGR_DONE; + struct ieee80211_tx_info *tx_info; bf_first = list_first_entry(&tid->buf_q, struct ath_buf, list); @@ -699,6 +700,11 @@ static enum ATH_AGGR_STATUS ath_tx_form_aggr(struct ath_softc *sc, break; } + tx_info = IEEE80211_SKB_CB(bf->bf_mpdu); + if (nframes && ((tx_info->flags & IEEE80211_TX_CTL_RATE_CTRL_PROBE) || + !(tx_info->control.rates[0].flags & IEEE80211_TX_RC_MCS))) + break; + /* do not exceed subframe limit */ if (nframes >= min((int)h_baw, ATH_AMPDU_SUBFRAME_DEFAULT)) { status = ATH_AGGR_LIMITED; -- 1.7.1