This is a note to let you know that I've just added the patch titled
ath10k: Add interrupt summary based CE processing
to the 5.4-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
ath10k-add-interrupt-summary-based-ce-processing.patch
and it can be found in the queue-5.4 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@xxxxxxxxxxxxxxx> know about it.
>From stable+bounces-10188-greg=kroah.com@xxxxxxxxxxxxxxx Mon Jan 8 16:37:56 2024
From: Amit Pundir <amit.pundir@xxxxxxxxxx>
Date: Mon, 8 Jan 2024 21:07:35 +0530
Subject: ath10k: Add interrupt summary based CE processing
To: Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx>, Sasha Levin <sashal@xxxxxxxxxx>, Douglas Anderson <dianders@xxxxxxxxxxxx>, Rakesh Pillai <pillair@xxxxxxxxxxxxxx>
Cc: Yongqin Liu <yongqin.liu@xxxxxxxxxx>, Stable <stable@xxxxxxxxxxxxxxx>, Kalle Valo <kvalo@xxxxxxxxxxxxxx>
Message-ID: <20240108153737.3538218-3-amit.pundir@xxxxxxxxxx>
From: Rakesh Pillai <pillair@xxxxxxxxxxxxxx>
[ Upstream commit b92aba35d39d10d8a6bdf2495172fd490c598b4a ]
Currently the NAPI processing loops through all
the copy engines and processes a particular copy
engine is the copy completion is set for that copy
engine. The host driver is not supposed to access
any copy engine register after clearing the interrupt
status register.
This might result in kernel crash like the one below
[ 1159.220143] Call trace:
[ 1159.220170] ath10k_snoc_read32+0x20/0x40 [ath10k_snoc]
[ 1159.220193] ath10k_ce_per_engine_service_any+0x78/0x130 [ath10k_core]
[ 1159.220203] ath10k_snoc_napi_poll+0x38/0x8c [ath10k_snoc]
[ 1159.220270] net_rx_action+0x100/0x3b0
[ 1159.220312] __do_softirq+0x164/0x30c
[ 1159.220345] run_ksoftirqd+0x2c/0x64
[ 1159.220380] smpboot_thread_fn+0x1b0/0x288
[ 1159.220405] kthread+0x11c/0x12c
[ 1159.220423] ret_from_fork+0x10/0x18
To avoid such a scenario, we generate an interrupt
summary by reading the copy completion for all the
copy engine before actually processing any of them.
This will avoid reading the interrupt status register
for any CE after the interrupt status is cleared.
Tested-on: WCN3990 hw1.0 SNOC WLAN.HL.3.1-01040-QCAHLSWMTPLZ-1
Signed-off-by: Rakesh Pillai <pillair@xxxxxxxxxxxxxx>
Reviewed-by: Douglas Anderson <dianders@xxxxxxxxxxxx>
Tested-by: Douglas Anderson <dianders@xxxxxxxxxxxx>
Signed-off-by: Kalle Valo <kvalo@xxxxxxxxxxxxxx>
Link: https://lore.kernel.org/r/1593193967-29897-1-git-send-email-pillair@xxxxxxxxxxxxxx
Stable-dep-of: 170c75d43a77 ("ath10k: Don't touch the CE interrupt registers after power up")
Signed-off-by: Amit Pundir <amit.pundir@xxxxxxxxxx>
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
---
drivers/net/wireless/ath/ath10k/ce.c | 63 +++++++++++++++++++++--------------
drivers/net/wireless/ath/ath10k/ce.h | 5 +-
2 files changed, 42 insertions(+), 26 deletions(-)
--- a/drivers/net/wireless/ath/ath10k/ce.c
+++ b/drivers/net/wireless/ath/ath10k/ce.c
@@ -481,15 +481,38 @@ static inline void ath10k_ce_engine_int_
ath10k_ce_write32(ar, ce_ctrl_addr + wm_regs->addr, mask);
}
-static inline bool ath10k_ce_engine_int_status_check(struct ath10k *ar,
- u32 ce_ctrl_addr,
- unsigned int mask)
+static bool ath10k_ce_engine_int_status_check(struct ath10k *ar, u32 ce_ctrl_addr,
+ unsigned int mask)
{
struct ath10k_hw_ce_host_wm_regs *wm_regs = ar->hw_ce_regs->wm_regs;
return ath10k_ce_read32(ar, ce_ctrl_addr + wm_regs->addr) & mask;
}
+u32 ath10k_ce_gen_interrupt_summary(struct ath10k *ar)
+{
+ struct ath10k_hw_ce_host_wm_regs *wm_regs = ar->hw_ce_regs->wm_regs;
+ struct ath10k_ce_pipe *ce_state;
+ struct ath10k_ce *ce;
+ u32 irq_summary = 0;
+ u32 ctrl_addr;
+ u32 ce_id;
+
+ ce = ath10k_ce_priv(ar);
+
+ for (ce_id = 0; ce_id < CE_COUNT; ce_id++) {
+ ce_state = &ce->ce_states[ce_id];
+ ctrl_addr = ce_state->ctrl_addr;
+ if (ath10k_ce_engine_int_status_check(ar, ctrl_addr,
+ wm_regs->cc_mask)) {
+ irq_summary |= BIT(ce_id);
+ }
+ }
+
+ return irq_summary;
+}
+EXPORT_SYMBOL(ath10k_ce_gen_interrupt_summary);
+
/*
* Guts of ath10k_ce_send.
* The caller takes responsibility for any needed locking.
@@ -1308,32 +1331,24 @@ void ath10k_ce_per_engine_service(struct
struct ath10k_hw_ce_host_wm_regs *wm_regs = ar->hw_ce_regs->wm_regs;
u32 ctrl_addr = ce_state->ctrl_addr;
- spin_lock_bh(&ce->ce_lock);
-
- if (ath10k_ce_engine_int_status_check(ar, ctrl_addr,
- wm_regs->cc_mask)) {
- /* Clear before handling */
- ath10k_ce_engine_int_status_clear(ar, ctrl_addr,
- wm_regs->cc_mask);
-
- spin_unlock_bh(&ce->ce_lock);
-
- if (ce_state->recv_cb)
- ce_state->recv_cb(ce_state);
-
- if (ce_state->send_cb)
- ce_state->send_cb(ce_state);
-
- spin_lock_bh(&ce->ce_lock);
- }
-
/*
+ * Clear before handling
+ *
* Misc CE interrupts are not being handled, but still need
* to be cleared.
+ *
+ * NOTE: When the last copy engine interrupt is cleared the
+ * hardware will go to sleep. Once this happens any access to
+ * the CE registers can cause a hardware fault.
*/
- ath10k_ce_engine_int_status_clear(ar, ctrl_addr, wm_regs->wm_mask);
+ ath10k_ce_engine_int_status_clear(ar, ctrl_addr,
+ wm_regs->cc_mask | wm_regs->wm_mask);
- spin_unlock_bh(&ce->ce_lock);
+ if (ce_state->recv_cb)
+ ce_state->recv_cb(ce_state);
+
+ if (ce_state->send_cb)
+ ce_state->send_cb(ce_state);
}
EXPORT_SYMBOL(ath10k_ce_per_engine_service);
--- a/drivers/net/wireless/ath/ath10k/ce.h
+++ b/drivers/net/wireless/ath/ath10k/ce.h
@@ -259,6 +259,8 @@ int ath10k_ce_disable_interrupts(struct
void ath10k_ce_enable_interrupts(struct ath10k *ar);
void ath10k_ce_dump_registers(struct ath10k *ar,
struct ath10k_fw_crash_data *crash_data);
+
+u32 ath10k_ce_gen_interrupt_summary(struct ath10k *ar);
void ath10k_ce_alloc_rri(struct ath10k *ar);
void ath10k_ce_free_rri(struct ath10k *ar);
@@ -369,7 +371,6 @@ static inline u32 ath10k_ce_base_address
(((x) & CE_WRAPPER_INTERRUPT_SUMMARY_HOST_MSI_MASK) >> \
CE_WRAPPER_INTERRUPT_SUMMARY_HOST_MSI_LSB)
#define CE_WRAPPER_INTERRUPT_SUMMARY_ADDRESS 0x0000
-#define CE_INTERRUPT_SUMMARY (GENMASK(CE_COUNT_MAX - 1, 0))
static inline u32 ath10k_ce_interrupt_summary(struct ath10k *ar)
{
@@ -380,7 +381,7 @@ static inline u32 ath10k_ce_interrupt_su
ce->bus_ops->read32((ar), CE_WRAPPER_BASE_ADDRESS +
CE_WRAPPER_INTERRUPT_SUMMARY_ADDRESS));
else
- return CE_INTERRUPT_SUMMARY;
+ return ath10k_ce_gen_interrupt_summary(ar);
}
/* Host software's Copy Engine configuration. */
Patches currently in stable-queue which might be from kroah.com@xxxxxxxxxxxxxxx are
queue-5.4/ath10k-get-rid-of-per_ce_irq-hw-param.patch
queue-5.4/ath10k-wait-until-copy-complete-is-actually-done-before-completing.patch
queue-5.4/ath10k-keep-track-of-which-interrupts-fired-don-t-poll-them.patch
queue-5.4/ath10k-add-interrupt-summary-based-ce-processing.patch
