[PATCH net v6 3/4] net: wwan: t7xx: fix race between TX/RX data path and system PM suspend

[Tim JH Chen <tim770802@gmail.com>]

Several DPMAIF data-plane contexts call pm_runtime_resume_and_get() and
then access hardware registers. System suspend ignores the runtime PM
reference they hold, so with ASPM L1 enabled and repeated suspend/resume
cycles they can touch the device while the suspend callback tears it
down, ending in a CPU soft lockup:

  watchdog: BUG: soft lockup - CPU#N stuck for 26s! [dpmaif_tx_hw_pu]
    __pm_runtime_resume+0x5b/0x80
    t7xx_dpmaif_tx_hw_push_thread+0xc4 [mtk_t7xx]

Runtime suspend is already safe: while any of these contexts holds its PM
reference the runtime suspend callback cannot run. Only system suspend,
which ignores that reference, is exposed.

Quiesce the DPMAIF data-plane contexts across system suspend:

 - Make the TX push kthread freezable (set_freezable(),
   wait_event_freezable(), kthread_freezable_should_stop()) so the PM
   freezer parks it before dpm_suspend() runs the device suspend
   callbacks. kthread_freezable_should_stop() also lets a concurrent
   kthread_stop() proceed while the thread is frozen, and a
   freezing(current) bail-out in the DRB-ring-full retry loop keeps the
   thread from looping there under sustained TX.

 - The suspend callback masks interrupts and drains the TX-done workers
   (cancel_work_sync(); their producer irq_tx_done is masked and
   cancel_work_sync() also blocks a self-requeue). It then calls
   t7xx_dpmaif_rx_stop(), which clears que_started and waits for the
   in-flight NAPI RX poll to finish, so that poll -- which writes
   registers via t7xx_dpmaif_clr_ip_busy_sts() /
   t7xx_dpmaif_dlq_unmask_rx_done() -- cannot run after the hardware is
   torn down. bat_release_work is cancelled only after rx_stop(), since
   its sole producer is that NAPI poll.

The data-plane workqueues are intentionally left non-freezable: marking
them WQ_FREEZABLE would let a flush_work()/cancel_work_sync() from a
context the freezer does not freeze (the FSM kthread, or an unbind
holding device_lock) block until thaw_workqueues(), which can hang the
suspend. Draining them from the suspend callback avoids that.

This covers the DPMAIF data path that produces the observed soft lockup;
the CLDMA control path uses a different mechanism and is out of scope.

Tested with 500+ suspend/resume cycles, SIM registered and ASPM L1
enabled.

Fixes: 46e8f49ed7b3 ("net: wwan: t7xx: Introduce power management")
Signed-off-by: Tim JH Chen <tim770802@gmail.com>
---
 drivers/net/wwan/t7xx/t7xx_hif_dpmaif.c    | 24 +++++++++++++++--
 drivers/net/wwan/t7xx/t7xx_hif_dpmaif_tx.c | 30 ++++++++++++++++++----
 2 files changed, 47 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wwan/t7xx/t7xx_hif_dpmaif.c b/drivers/net/wwan/t7xx/t7xx_hif_dpmaif.c
index 7ff33c1d6ac7..b5a857e940b7 100644
--- a/drivers/net/wwan/t7xx/t7xx_hif_dpmaif.c
+++ b/drivers/net/wwan/t7xx/t7xx_hif_dpmaif.c
@@ -410,12 +410,32 @@ static int t7xx_dpmaif_stop(struct dpmaif_ctrl *dpmaif_ctrl)
 static int t7xx_dpmaif_suspend(struct t7xx_pci_dev *t7xx_dev, void *param)
 {
 	struct dpmaif_ctrl *dpmaif_ctrl = param;
+	unsigned int i;
 
+	/* Stop new TX and mask interrupts first, so nothing re-arms the
+	 * contexts drained below.
+	 */
 	t7xx_dpmaif_tx_stop(dpmaif_ctrl);
-	t7xx_dpmaif_hw_stop_all_txq(&dpmaif_ctrl->hw_info);
-	t7xx_dpmaif_hw_stop_all_rxq(&dpmaif_ctrl->hw_info);
 	t7xx_dpmaif_disable_irq(dpmaif_ctrl);
+
+	/* irq_tx_done is masked now and cancel_work_sync() also blocks a
+	 * self-requeue, so the TX-done workers can be drained here.
+	 */
+	for (i = 0; i < DPMAIF_TXQ_NUM; i++)
+		cancel_work_sync(&dpmaif_ctrl->txq[i].dpmaif_tx_work);
+
+	/* t7xx_dpmaif_rx_stop() clears que_started and waits for the
+	 * in-flight NAPI poll (rx_processing) to finish, so no poll issues
+	 * MMIO after this point. It is also the sole producer of
+	 * bat_release_work, so cancel that work only after rx_stop();
+	 * otherwise a residual poll re-queues it and it runs against
+	 * torn-down hardware.
+	 */
 	t7xx_dpmaif_rx_stop(dpmaif_ctrl);
+	cancel_work_sync(&dpmaif_ctrl->bat_release_work);
+
+	t7xx_dpmaif_hw_stop_all_txq(&dpmaif_ctrl->hw_info);
+	t7xx_dpmaif_hw_stop_all_rxq(&dpmaif_ctrl->hw_info);
 	return 0;
 }
 
diff --git a/drivers/net/wwan/t7xx/t7xx_hif_dpmaif_tx.c b/drivers/net/wwan/t7xx/t7xx_hif_dpmaif_tx.c
index 2a405bc74312..450e030fc696 100644
--- a/drivers/net/wwan/t7xx/t7xx_hif_dpmaif_tx.c
+++ b/drivers/net/wwan/t7xx/t7xx_hif_dpmaif_tx.c
@@ -22,6 +22,7 @@
 #include <linux/dma-direction.h>
 #include <linux/dma-mapping.h>
 #include <linux/err.h>
+#include <linux/freezer.h>
 #include <linux/gfp.h>
 #include <linux/kernel.h>
 #include <linux/kthread.h>
@@ -426,6 +427,12 @@ static void t7xx_do_tx_hw_push(struct dpmaif_ctrl *dpmaif_ctrl)
 
 		drb_send_cnt = t7xx_txq_burst_send_skb(txq);
 		if (drb_send_cnt <= 0) {
+			/* Bail out promptly on a pending freeze so the caller can
+			 * drop its runtime-PM reference and this thread can reach
+			 * the freeze point instead of looping here under load.
+			 */
+			if (freezing(current))
+				return;
 			usleep_range(10, 20);
 			cond_resched();
 			continue;
@@ -457,19 +464,30 @@ static int t7xx_dpmaif_tx_hw_push_thread(void *arg)
 	struct dpmaif_ctrl *dpmaif_ctrl = arg;
 	int ret;
 
+	set_freezable();
+
 	while (!kthread_should_stop()) {
 		if (t7xx_tx_lists_are_all_empty(dpmaif_ctrl) ||
 		    dpmaif_ctrl->state != DPMAIF_STATE_PWRON) {
-			if (wait_event_interruptible(dpmaif_ctrl->tx_wq,
-						     (!t7xx_tx_lists_are_all_empty(dpmaif_ctrl) &&
-						     dpmaif_ctrl->state == DPMAIF_STATE_PWRON) ||
-						     kthread_should_stop()))
+			if (wait_event_freezable(dpmaif_ctrl->tx_wq,
+						 (!t7xx_tx_lists_are_all_empty(dpmaif_ctrl) &&
+						  dpmaif_ctrl->state == DPMAIF_STATE_PWRON) ||
+						 kthread_should_stop()))
 				continue;
 
 			if (kthread_should_stop())
 				break;
 		}
 
+		/* Park on a pending freeze here, outside the runtime-PM and MMIO
+		 * section below, so the PM freezer quiesces this thread before
+		 * dpm_suspend() runs the device suspend callbacks.
+		 * kthread_freezable_should_stop() also honours a concurrent
+		 * kthread_stop() while the thread is frozen.
+		 */
+		if (kthread_freezable_should_stop(NULL))
+			break;
+
 		ret = pm_runtime_resume_and_get(dpmaif_ctrl->dev);
 		if (ret < 0 && ret != -EACCES) {
 			/* Do not exit the thread: dpmaif_ctrl->tx_thread still
@@ -478,7 +496,9 @@ static int t7xx_dpmaif_tx_hw_push_thread(void *arg)
 			 */
 			dev_err_ratelimited(dpmaif_ctrl->dev,
 					    "Failed to resume for TX push: %d\n", ret);
-			msleep_interruptible(DPMAIF_TX_RESUME_RETRY_MS);
+			wait_event_freezable_timeout(dpmaif_ctrl->tx_wq,
+						     kthread_should_stop(),
+						     msecs_to_jiffies(DPMAIF_TX_RESUME_RETRY_MS));
 			continue;
 		}
 
-- 
2.43.0