eth: tg3: fix calling napi_enable() in atomic context

tg3 has a spin lock protecting most of the config,
switch to taking netdev_lock() explicitly on enable/start
paths. Disable/stop paths seem to not be under the spin
lock (since napi_disable() already needs to sleep),
so leave that side as is.

tg3_restart_hw() releases and re-takes the spin lock,
we need to do the same because dev_close() needs to
take netdev_lock().

Fixes: 413f0271f396 ("net: protect NAPI enablement with netdev_lock()")
Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Link: https://lore.kernel.org/dcfd56bc-de32-4b11-9e19-d8bd1543745d@stanley.mountain
Reviewed-by: Eric Dumazet <edumazet@google.com>
Reviewed-by: Michael Chan <michael.chan@broadcom.com>
Link: https://patch.msgid.link/20250124031841.1179756-2-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index 9cc8db10a8d6043fe48ede6ae4a45f268c83d60e..1c94bf1db718651bf9e6f54b9b84339b2edca45a 100644 (file)
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -7424,7 +7424,7 @@ static void tg3_napi_enable(struct tg3 *tp)
 
        for (i = 0; i < tp->irq_cnt; i++) {
                tnapi = &tp->napi[i];
-               napi_enable(&tnapi->napi);
+               napi_enable_locked(&tnapi->napi);
                if (tnapi->tx_buffers) {
                        netif_queue_set_napi(tp->dev, txq_idx,
                                             NETDEV_QUEUE_TYPE_TX,
@@ -7445,9 +7445,10 @@ static void tg3_napi_init(struct tg3 *tp)
        int i;
 
        for (i = 0; i < tp->irq_cnt; i++) {
-               netif_napi_add(tp->dev, &tp->napi[i].napi,
-                              i ? tg3_poll_msix : tg3_poll);
-               netif_napi_set_irq(&tp->napi[i].napi, tp->napi[i].irq_vec);
+               netif_napi_add_locked(tp->dev, &tp->napi[i].napi,
+                                     i ? tg3_poll_msix : tg3_poll);
+               netif_napi_set_irq_locked(&tp->napi[i].napi,
+                                         tp->napi[i].irq_vec);
        }
 }
 
@@ -11259,6 +11260,8 @@ static void tg3_timer_stop(struct tg3 *tp)
 static int tg3_restart_hw(struct tg3 *tp, bool reset_phy)
        __releases(tp->lock)
        __acquires(tp->lock)
+       __releases(tp->dev->lock)
+       __acquires(tp->dev->lock)
 {
        int err;
 
@@ -11271,7 +11274,9 @@ static int tg3_restart_hw(struct tg3 *tp, bool reset_phy)
                tg3_timer_stop(tp);
                tp->irq_sync = 0;
                tg3_napi_enable(tp);
+               netdev_unlock(tp->dev);
                dev_close(tp->dev);
+               netdev_lock(tp->dev);
                tg3_full_lock(tp, 0);
        }
        return err;
@@ -11299,6 +11304,7 @@ static void tg3_reset_task(struct work_struct *work)
 
        tg3_netif_stop(tp);
 
+       netdev_lock(tp->dev);
        tg3_full_lock(tp, 1);
 
        if (tg3_flag(tp, TX_RECOVERY_PENDING)) {
@@ -11318,12 +11324,14 @@ static void tg3_reset_task(struct work_struct *work)
                 * call cancel_work_sync() and wait forever.
                 */
                tg3_flag_clear(tp, RESET_TASK_PENDING);
+               netdev_unlock(tp->dev);
                dev_close(tp->dev);
                goto out;
        }
 
        tg3_netif_start(tp);
        tg3_full_unlock(tp);
+       netdev_unlock(tp->dev);
        tg3_phy_start(tp);
        tg3_flag_clear(tp, RESET_TASK_PENDING);
 out:
@@ -11683,9 +11691,11 @@ static int tg3_start(struct tg3 *tp, bool reset_phy, bool test_irq,
        if (err)
                goto out_ints_fini;
 
+       netdev_lock(dev);
        tg3_napi_init(tp);
 
        tg3_napi_enable(tp);
+       netdev_unlock(dev);
 
        for (i = 0; i < tp->irq_cnt; i++) {
                err = tg3_request_irq(tp, i);
@@ -12569,6 +12579,7 @@ static int tg3_set_ringparam(struct net_device *dev,
                irq_sync = 1;
        }
 
+       netdev_lock(dev);
        tg3_full_lock(tp, irq_sync);
 
        tp->rx_pending = ering->rx_pending;
@@ -12597,6 +12608,7 @@ static int tg3_set_ringparam(struct net_device *dev,
        }
 
        tg3_full_unlock(tp);
+       netdev_unlock(dev);
 
        if (irq_sync && !err)
                tg3_phy_start(tp);
@@ -12678,6 +12690,7 @@ static int tg3_set_pauseparam(struct net_device *dev, struct ethtool_pauseparam
                        irq_sync = 1;
                }
 
+               netdev_lock(dev);
                tg3_full_lock(tp, irq_sync);
 
                if (epause->autoneg)
@@ -12707,6 +12720,7 @@ static int tg3_set_pauseparam(struct net_device *dev, struct ethtool_pauseparam
                }
 
                tg3_full_unlock(tp);
+               netdev_unlock(dev);
        }
 
        tp->phy_flags |= TG3_PHYFLG_USER_CONFIGURED;
@@ -13911,6 +13925,7 @@ static void tg3_self_test(struct net_device *dev, struct ethtool_test *etest,
                        data[TG3_INTERRUPT_TEST] = 1;
                }
 
+               netdev_lock(dev);
                tg3_full_lock(tp, 0);
 
                tg3_halt(tp, RESET_KIND_SHUTDOWN, 1);
@@ -13922,6 +13937,7 @@ static void tg3_self_test(struct net_device *dev, struct ethtool_test *etest,
                }
 
                tg3_full_unlock(tp);
+               netdev_unlock(dev);
 
                if (irq_sync && !err2)
                        tg3_phy_start(tp);
@@ -14365,6 +14381,7 @@ static int tg3_change_mtu(struct net_device *dev, int new_mtu)
 
        tg3_set_mtu(dev, tp, new_mtu);
 
+       netdev_lock(dev);
        tg3_full_lock(tp, 1);
 
        tg3_halt(tp, RESET_KIND_SHUTDOWN, 1);
@@ -14384,6 +14401,7 @@ static int tg3_change_mtu(struct net_device *dev, int new_mtu)
                tg3_netif_start(tp);
 
        tg3_full_unlock(tp);
+       netdev_unlock(dev);
 
        if (!err)
                tg3_phy_start(tp);
@@ -18164,6 +18182,7 @@ static int tg3_resume(struct device *device)
 
        netif_device_attach(dev);
 
+       netdev_lock(dev);
        tg3_full_lock(tp, 0);
 
        tg3_ape_driver_state_change(tp, RESET_KIND_INIT);
@@ -18180,6 +18199,7 @@ static int tg3_resume(struct device *device)
 
 out:
        tg3_full_unlock(tp);
+       netdev_unlock(dev);
 
        if (!err)
                tg3_phy_start(tp);
@@ -18260,7 +18280,9 @@ static pci_ers_result_t tg3_io_error_detected(struct pci_dev *pdev,
 done:
        if (state == pci_channel_io_perm_failure) {
                if (netdev) {
+                       netdev_lock(netdev);
                        tg3_napi_enable(tp);
+                       netdev_unlock(netdev);
                        dev_close(netdev);
                }
                err = PCI_ERS_RESULT_DISCONNECT;
@@ -18314,7 +18336,9 @@ static pci_ers_result_t tg3_io_slot_reset(struct pci_dev *pdev)
 
 done:
        if (rc != PCI_ERS_RESULT_RECOVERED && netdev && netif_running(netdev)) {
+               netdev_lock(netdev);
                tg3_napi_enable(tp);
+               netdev_unlock(netdev);
                dev_close(netdev);
        }
        rtnl_unlock();
@@ -18340,12 +18364,14 @@ static void tg3_io_resume(struct pci_dev *pdev)
        if (!netdev || !netif_running(netdev))
                goto done;
 
+       netdev_lock(netdev);
        tg3_full_lock(tp, 0);
        tg3_ape_driver_state_change(tp, RESET_KIND_INIT);
        tg3_flag_set(tp, INIT_COMPLETE);
        err = tg3_restart_hw(tp, true);
        if (err) {
                tg3_full_unlock(tp);
+               netdev_unlock(netdev);
                netdev_err(netdev, "Cannot restart hardware after reset.\n");
                goto done;
        }
@@ -18357,6 +18383,7 @@ static void tg3_io_resume(struct pci_dev *pdev)
        tg3_netif_start(tp);
 
        tg3_full_unlock(tp);
+       netdev_unlock(netdev);
 
        tg3_phy_start(tp);