From 82749f5d8793b9d55210416e4a83b1625e991efb Mon Sep 17 00:00:00 2001
From: Taylor R Campbell <riastradh@NetBSD.org>
Date: Thu, 1 Sep 2022 15:04:40 +0000
Subject: [PATCH] bge(4): Narrow scope of `core' lock somewhat.

This avoids holding it across sleeps where it is forbidden, such as
callout_halt, adaptive mutex_enter, and memory allocation in
bge_init.

Note: The `core' lock is still too broad at IPL_NET.  It should be
narrowed further so that the only parts that run with interrupts
blocked are the parts that actually synchronize with the interrupt
handler or touch the same registers.  For example, I don't think the
mii tick has to do any of that.
---
 sys/dev/pci/if_bge.c | 55 +++++++++++++++-----------------------------
 1 file changed, 18 insertions(+), 37 deletions(-)

diff --git a/sys/dev/pci/if_bge.c b/sys/dev/pci/if_bge.c
index eccf043efaf2..93a839c72758 100644
--- a/sys/dev/pci/if_bge.c
+++ b/sys/dev/pci/if_bge.c
@@ -210,9 +210,7 @@ static void bge_start_locked(struct ifnet *);
 static int bge_ifflags_cb(struct ethercom *);
 static int bge_ioctl(struct ifnet *, u_long, void *);
 static int bge_init(struct ifnet *);
-static int bge_init_locked(struct ifnet *);
 static void bge_stop(struct ifnet *, int);
-static void bge_stop_locked(struct ifnet *, int);
 static bool bge_watchdog(struct ifnet *);
 static int bge_ifmedia_upd(struct ifnet *);
 static void bge_ifmedia_sts(struct ifnet *, struct ifmediareq *);
@@ -4095,7 +4093,9 @@ bge_detach(device_t self, int flags __unused)
 	struct ifnet * const ifp = &sc->ethercom.ec_if;
 
 	/* Stop the interface. Callouts are stopped in it. */
+	IFNET_LOCK(ifp);
 	bge_stop(ifp, 1);
+	IFNET_UNLOCK(ifp);
 
 	mii_detach(&sc->bge_mii, MII_PHY_ANY, MII_OFFSET_ANY);
 
@@ -5553,31 +5553,18 @@ bge_start_locked(struct ifnet *ifp)
 
 static int
 bge_init(struct ifnet *ifp)
-{
-	struct bge_softc * const sc = ifp->if_softc;
-
-	mutex_enter(sc->sc_core_lock);
-	int ret = bge_init_locked(ifp);
-	mutex_exit(sc->sc_core_lock);
-
-	return ret;
-}
-
-
-static int
-bge_init_locked(struct ifnet *ifp)
 {
 	struct bge_softc * const sc = ifp->if_softc;
 	const uint16_t *m;
 	uint32_t mode, reg;
 	int error = 0;
 
+	ASSERT_SLEEPABLE();
 	KASSERT(IFNET_LOCKED(ifp));
-	KASSERT(mutex_owned(sc->sc_core_lock));
 	KASSERT(ifp == &sc->ethercom.ec_if);
 
 	/* Cancel pending I/O and flush buffers. */
-	bge_stop_locked(ifp, 0);
+	bge_stop(ifp, 0);
 
 	bge_stop_fw(sc);
 	bge_sig_pre_reset(sc, BGE_RESET_START);
@@ -5666,7 +5653,9 @@ bge_init_locked(struct ifnet *ifp)
 		BGE_CLRBIT(sc, BGE_RX_MODE, BGE_RXMODE_RX_PROMISC);
 
 	/* Program multicast filter. */
+	mutex_enter(sc->sc_core_lock);
 	bge_setmulti(sc);
+	mutex_exit(sc->sc_core_lock);
 
 	/* Init RX ring. */
 	bge_init_rx_ring_std(sc);
@@ -5753,10 +5742,15 @@ bge_init_locked(struct ifnet *ifp)
 	/* IFNET_LOCKED asserted above */
 	ifp->if_flags |= IFF_RUNNING;
 
+	mutex_enter(sc->sc_core_lock);
+	sc->bge_detaching = false;
 	callout_schedule(&sc->bge_timeout, hz);
+	mutex_exit(sc->sc_core_lock);
 
 out:
+	mutex_enter(sc->sc_core_lock);
 	sc->bge_if_flags = ifp->if_flags;
+	mutex_exit(sc->sc_core_lock);
 
 	return error;
 }
@@ -6100,35 +6094,22 @@ bge_stop_block(struct bge_softc *sc, bus_addr_t reg, uint32_t bit)
 		    (u_long)reg, bit);
 }
 
-
-static void
-bge_stop(struct ifnet *ifp, int disable)
-{
-	struct bge_softc * const sc = ifp->if_softc;
-
-	ASSERT_SLEEPABLE();
-
-	mutex_enter(sc->sc_core_lock);
-	bge_stop_locked(ifp, disable);
-	mutex_exit(sc->sc_core_lock);
-}
-
 /*
  * Stop the adapter and free any mbufs allocated to the
  * RX and TX lists.
  */
 static void
-bge_stop_locked(struct ifnet *ifp, int disable)
+bge_stop(struct ifnet *ifp, int disable)
 {
 	struct bge_softc * const sc = ifp->if_softc;
 
-	KASSERT(mutex_owned(sc->sc_core_lock));
+	ASSERT_SLEEPABLE();
+	KASSERT(IFNET_LOCKED(ifp));
 
-	if (disable) {
-		sc->bge_detaching = true;
-		callout_halt(&sc->bge_timeout, NULL);
-	} else
-		callout_stop(&sc->bge_timeout);
+	mutex_enter(sc->sc_core_lock);
+	sc->bge_detaching = true;
+	mutex_exit(sc->sc_core_lock);
+	callout_halt(&sc->bge_timeout, NULL);
 
 	/* Disable host interrupts. */
 	BGE_SETBIT(sc, BGE_PCI_MISC_CTL, BGE_PCIMISCCTL_MASK_PCI_INTR);