The recent mii_tick diff made me ponder whether the mii tick timeout
could be put in the mii_data struct rather than each driver softc (not
that a good idea, actually).

While looking at this, I noticed that two drivers may end up invoking
mii_tick() while not being at IPL_NET.

bnx: bnx_tick() can get invoked either from the interrupt handler, or
from a timeout.

sk: on some boards, sk_tick() gets invoked from timeout, but may end up
invoking sk_intr_bcom() which is intended to run at IPL_NET.

This diff has only been compile-tested, for lack of such hardware.

Index: if_bnx.c
===================================================================
RCS file: /OpenBSD/src/sys/dev/pci/if_bnx.c,v
retrieving revision 1.125
diff -u -p -r1.125 if_bnx.c
--- if_bnx.c    10 Mar 2018 10:51:46 -0000      1.125
+++ if_bnx.c    3 Jul 2019 06:40:47 -0000
@@ -5457,6 +5457,9 @@ bnx_tick(void *xsc)
        struct ifnet            *ifp = &sc->arpcom.ac_if;
        struct mii_data         *mii = NULL;
        u_int32_t               msg;
+       int                     s;
+
+       s = splnet();
 
        /* Tell the firmware that the driver is still running. */
 #ifdef BNX_DEBUG
@@ -5489,7 +5492,7 @@ bnx_tick(void *xsc)
        }
 
 bnx_tick_exit:
-       return;
+       splx(s);
 }
 
 /****************************************************************************/
Index: if_sk.c
===================================================================
RCS file: /OpenBSD/src/sys/dev/pci/if_sk.c,v
retrieving revision 1.189
diff -u -p -r1.189 if_sk.c
--- if_sk.c     4 Jun 2017 04:29:23 -0000       1.189
+++ if_sk.c     3 Jul 2019 06:40:47 -0000
@@ -1703,15 +1703,20 @@ sk_tick(void *xsc_if)
        struct sk_if_softc *sc_if = xsc_if;
        struct mii_data *mii = &sc_if->sk_mii;
        struct ifnet *ifp = &sc_if->arpcom.ac_if;
-       int i;
+       int i, s;
 
        DPRINTFN(2, ("sk_tick\n"));
 
-       if (!(ifp->if_flags & IFF_UP))
+       s = splnet();
+
+       if (!(ifp->if_flags & IFF_UP)) {
+               splx(s);
                return;
+       }
 
        if (sc_if->sk_phytype == SK_PHYTYPE_BCOM) {
                sk_intr_bcom(sc_if);
+               splx(s);
                return;
        }
 
@@ -1729,6 +1734,7 @@ sk_tick(void *xsc_if)
 
        if (i != 3) {
                timeout_add_sec(&sc_if->sk_tick_ch, 1);
+               splx(s);
                return;
        }
 
@@ -1736,6 +1742,7 @@ sk_tick(void *xsc_if)
        SK_XM_CLRBIT_2(sc_if, XM_IMR, XM_IMR_GP0_SET);
        SK_XM_READ_2(sc_if, XM_ISR);
        mii_tick(mii);
+       splx(s);
        timeout_del(&sc_if->sk_tick_ch);
 }
 

Reply via email to