> Date: Sat, 10 Aug 2024 21:37:45 +0000
> From: Taylor R Campbell <campbell+netbsd-source-change...@mumble.net>
>
> I suggest doing the same here (and in bge(4) and in vmx(4) and
> wherever else we have an overbroad `core lock'), with the attached
> patch. But I don't have any awge(4) hardware to test. Could I
> trouble you to test this and commit it if it works?
Oops -- missed a spot, need to take sc_mcast_lock around the call to
dwc_gmac_setmulti (and only around that call) in dwc_gmac_init.
diff -r f339ab4bf462 sys/dev/ic/dwc_gmac.c
--- a/sys/dev/ic/dwc_gmac.c Sat Aug 10 15:20:59 2024 +0000
+++ b/sys/dev/ic/dwc_gmac.c Sat Aug 10 21:48:14 2024 +0000
@@ -87,9 +87,7 @@ static void dwc_gmac_reset_tx_ring(struc
static void dwc_gmac_free_tx_ring(struct dwc_gmac_softc *, struct
dwc_gmac_tx_ring *);
static void dwc_gmac_txdesc_sync(struct dwc_gmac_softc *, int, int, int);
static int dwc_gmac_init(struct ifnet *);
-static int dwc_gmac_init_locked(struct ifnet *);
static void dwc_gmac_stop(struct ifnet *, int);
-static void dwc_gmac_stop_locked(struct ifnet *, int);
static void dwc_gmac_start(struct ifnet *);
static void dwc_gmac_start_locked(struct ifnet *);
static int dwc_gmac_queue(struct dwc_gmac_softc *, struct mbuf *);
@@ -297,7 +295,7 @@ dwc_gmac_attach(struct dwc_gmac_softc *s
sc->sc_stopping = false;
sc->sc_txbusy = false;
- sc->sc_core_lock = mutex_obj_alloc(MUTEX_DEFAULT, IPL_NONE);
+ sc->sc_mcast_lock = mutex_obj_alloc(MUTEX_DEFAULT, IPL_SOFTNET);
sc->sc_intr_lock = mutex_obj_alloc(MUTEX_DEFAULT, IPL_NET);
mutex_init(&sc->sc_txq.t_mtx, MUTEX_DEFAULT, IPL_NET);
mutex_init(&sc->sc_rxq.r_mtx, MUTEX_DEFAULT, IPL_NET);
@@ -863,28 +861,13 @@ static int
dwc_gmac_init(struct ifnet *ifp)
{
struct dwc_gmac_softc * const sc = ifp->if_softc;
-
- KASSERT(IFNET_LOCKED(ifp));
-
- mutex_enter(sc->sc_core_lock);
- int ret = dwc_gmac_init_locked(ifp);
- mutex_exit(sc->sc_core_lock);
-
- return ret;
-}
-
-static int
-dwc_gmac_init_locked(struct ifnet *ifp)
-{
- struct dwc_gmac_softc * const sc = ifp->if_softc;
uint32_t ffilt;
ASSERT_SLEEPABLE();
KASSERT(IFNET_LOCKED(ifp));
- KASSERT(mutex_owned(sc->sc_core_lock));
KASSERT(ifp == &sc->sc_ec.ec_if);
- dwc_gmac_stop_locked(ifp, 0);
+ dwc_gmac_stop(ifp, 0);
/*
* Configure DMA burst/transfer mode and RX/TX priorities.
@@ -914,7 +897,9 @@ dwc_gmac_init_locked(struct ifnet *ifp)
/*
* Set up multicast filter
*/
+ mutex_enter(sc->sc_mcast_lock);
dwc_gmac_setmulti(sc);
+ mutex_exit(sc->sc_mcast_lock);
/*
* Set up dma pointer for RX and TX ring
@@ -942,11 +927,11 @@ dwc_gmac_init_locked(struct ifnet *ifp)
mutex_enter(sc->sc_intr_lock);
sc->sc_stopping = false;
+ mutex_exit(sc->sc_intr_lock);
mutex_enter(&sc->sc_txq.t_mtx);
sc->sc_txbusy = false;
mutex_exit(&sc->sc_txq.t_mtx);
- mutex_exit(sc->sc_intr_lock);
return 0;
}
@@ -1015,29 +1000,23 @@ dwc_gmac_stop(struct ifnet *ifp, int dis
{
struct dwc_gmac_softc * const sc = ifp->if_softc;
- mutex_enter(sc->sc_core_lock);
- dwc_gmac_stop_locked(ifp, disable);
- mutex_exit(sc->sc_core_lock);
-}
-
-static void
-dwc_gmac_stop_locked(struct ifnet *ifp, int disable)
-{
- struct dwc_gmac_softc * const sc = ifp->if_softc;
-
ASSERT_SLEEPABLE();
KASSERT(IFNET_LOCKED(ifp));
- KASSERT(mutex_owned(sc->sc_core_lock));
+
+ ifp->if_flags &= ~IFF_RUNNING;
+
+ mutex_enter(sc->sc_mcast_lock);
+ sc->sc_if_flags = ifp->if_flags;
+ mutex_exit(sc->sc_mcast_lock);
mutex_enter(sc->sc_intr_lock);
sc->sc_stopping = true;
+ mutex_exit(sc->sc_intr_lock);
mutex_enter(&sc->sc_txq.t_mtx);
sc->sc_txbusy = false;
mutex_exit(&sc->sc_txq.t_mtx);
- mutex_exit(sc->sc_intr_lock);
-
bus_space_write_4(sc->sc_bst, sc->sc_bsh,
AWIN_GMAC_DMA_OPMODE,
bus_space_read_4(sc->sc_bst, sc->sc_bsh,
@@ -1051,9 +1030,6 @@ dwc_gmac_stop_locked(struct ifnet *ifp,
mii_down(&sc->sc_mii);
dwc_gmac_reset_tx_ring(sc, &sc->sc_txq);
dwc_gmac_reset_rx_ring(sc, &sc->sc_rxq);
-
- ifp->if_flags &= ~IFF_RUNNING;
- sc->sc_if_flags = ifp->if_flags;
}
/*
@@ -1150,7 +1126,7 @@ dwc_gmac_ifflags_cb(struct ethercom *ec)
int ret = 0;
KASSERT(IFNET_LOCKED(ifp));
- mutex_enter(sc->sc_core_lock);
+ mutex_enter(sc->sc_mcast_lock);
u_short change = ifp->if_flags ^ sc->sc_if_flags;
sc->sc_if_flags = ifp->if_flags;
@@ -1161,7 +1137,7 @@ dwc_gmac_ifflags_cb(struct ethercom *ec)
dwc_gmac_setmulti(sc);
}
- mutex_exit(sc->sc_core_lock);
+ mutex_exit(sc->sc_mcast_lock);
return ret;
}
@@ -1187,7 +1163,7 @@ dwc_gmac_ioctl(struct ifnet *ifp, u_long
if (error == ENETRESET) {
error = 0;
if (cmd == SIOCADDMULTI || cmd == SIOCDELMULTI) {
- mutex_enter(sc->sc_core_lock);
+ mutex_enter(sc->sc_mcast_lock);
if (sc->sc_if_flags & IFF_RUNNING) {
/*
* Multicast list has changed; set the hardware
@@ -1195,7 +1171,7 @@ dwc_gmac_ioctl(struct ifnet *ifp, u_long
*/
dwc_gmac_setmulti(sc);
}
- mutex_exit(sc->sc_core_lock);
+ mutex_exit(sc->sc_mcast_lock);
}
}
@@ -1393,7 +1369,6 @@ skip:
static void
dwc_gmac_setmulti(struct dwc_gmac_softc *sc)
{
- struct ifnet * const ifp = &sc->sc_ec.ec_if;
struct ether_multi *enm;
struct ether_multistep step;
struct ethercom *ec = &sc->sc_ec;
@@ -1401,7 +1376,7 @@ dwc_gmac_setmulti(struct dwc_gmac_softc
uint32_t ffilt, h;
int mcnt;
- KASSERT(mutex_owned(sc->sc_core_lock));
+ KASSERT(mutex_owned(sc->sc_mcast_lock));
ffilt = bus_space_read_4(sc->sc_bst, sc->sc_bsh, AWIN_GMAC_MAC_FFILT);
@@ -1463,7 +1438,6 @@ special_filter:
0xffffffff);
bus_space_write_4(sc->sc_bst, sc->sc_bsh, AWIN_GMAC_MAC_HTHIGH,
0xffffffff);
- sc->sc_if_flags = ifp->if_flags;
}
int
diff -r f339ab4bf462 sys/dev/ic/dwc_gmac_var.h
--- a/sys/dev/ic/dwc_gmac_var.h Sat Aug 10 15:20:59 2024 +0000
+++ b/sys/dev/ic/dwc_gmac_var.h Sat Aug 10 21:48:14 2024 +0000
@@ -102,12 +102,12 @@ struct dwc_gmac_softc {
struct dwc_gmac_rx_ring sc_rxq;
struct dwc_gmac_tx_ring sc_txq;
const struct dwc_gmac_desc_methods *sc_descm;
- u_short sc_if_flags; /* shadow of ether flags */
+ u_short sc_if_flags; /* if_flags copy (for mcast) */
uint16_t sc_mii_clk;
bool sc_txbusy;
bool sc_stopping;
krndsource_t rnd_source;
- kmutex_t *sc_core_lock; /* lock for softc operations */
+ kmutex_t *sc_mcast_lock; /* lock for SIOCADD/DELMULTI */
kmutex_t *sc_intr_lock; /* lock for interrupt
operations */
struct if_percpuq *sc_ipq; /* softint-based input queues */
