Index | Thread | Search

From:
Jonathan Matthew <jonathan@d14n.org>
Subject:
Re: bse: mp-safe interrupts
To:
Vitaliy Makkoveev <mvs@openbsd.org>
Cc:
tech@openbsd.org
Date:
Sat, 14 Feb 2026 13:38:44 +1000

Download raw body.

Thread
On Fri, Feb 13, 2026 at 08:35:54PM +0300, Vitaliy Makkoveev wrote:
> Make the interrupt handler mp-asfe. Serialize the reception part of
> genet_intr() interrupt handler and genet_rxtick() timeout handler with
> `sc_mtx' mutex. The transmission part has nothing to do.

The timeout handler already has exclusive access to the receive ring
without adding a mutex here, so all you need to do is add IPL_MPSAFE.

The timeout is only armed when the interrupt handler has emptied the
ring and has not been able to refill it.  Once the ring is empty,
the interface can't receive any more packets until the timeout runs,
so the interrupt handler won't touch the receive ring.
We use this pattern in almost every mpsafe network driver.


> 
> Running this on couple RPI4s without issues for a couple of days.
> 
> Index: sys/dev/acpi/if_bse_acpi.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/acpi/if_bse_acpi.c,v
> diff -u -p -r1.6 if_bse_acpi.c
> --- sys/dev/acpi/if_bse_acpi.c	6 Apr 2022 18:59:27 -0000	1.6
> +++ sys/dev/acpi/if_bse_acpi.c	13 Feb 2026 17:15:12 -0000
> @@ -90,7 +90,7 @@ bse_acpi_attach(struct device *parent, s
>  	}
>  
>  	sc->sc.sc_ih = acpi_intr_establish(aaa->aaa_irq[0],
> -	    aaa->aaa_irq_flags[0], IPL_NET, genet_intr,
> +	    aaa->aaa_irq_flags[0], IPL_NET | IPL_MPSAFE, genet_intr,
>  	    sc, sc->sc.sc_dev.dv_xname);
>  	if (sc->sc.sc_ih == NULL) {
>  		printf(": can't establish interrupt\n");
> Index: sys/dev/fdt/if_bse_fdt.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/fdt/if_bse_fdt.c,v
> diff -u -p -r1.2 if_bse_fdt.c
> --- sys/dev/fdt/if_bse_fdt.c	6 Apr 2022 18:59:28 -0000	1.2
> +++ sys/dev/fdt/if_bse_fdt.c	13 Feb 2026 17:15:12 -0000
> @@ -72,7 +72,7 @@ bse_fdt_attach(struct device *parent, st
>  		return;
>  	}
>  
> -	sc->sc_ih = fdt_intr_establish(faa->fa_node, IPL_NET,
> +	sc->sc_ih = fdt_intr_establish(faa->fa_node, IPL_NET | IPL_MPSAFE,
>  	    genet_intr, sc, sc->sc_dev.dv_xname);
>  	if (sc->sc_ih == NULL) {
>  		printf(": can't establish interrupt\n");
> Index: sys/dev/ic/bcmgenet.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/ic/bcmgenet.c,v
> diff -u -p -r1.12 bcmgenet.c
> --- sys/dev/ic/bcmgenet.c	24 Jan 2026 05:33:11 -0000	1.12
> +++ sys/dev/ic/bcmgenet.c	13 Feb 2026 17:15:12 -0000
> @@ -345,7 +345,11 @@ genet_fill_rx_ring(struct genet_softc *s
>  void
>  genet_rxtick(void *arg)
>  {
> -	genet_fill_rx_ring(arg, GENET_DMA_DEFAULT_QUEUE);
> +	struct genet_softc *sc = arg;
> +
> +	mtx_enter(&sc->sc_mtx);
> +	genet_fill_rx_ring(sc, GENET_DMA_DEFAULT_QUEUE);
> +	mtx_leave(&sc->sc_mtx);
>  }
>  
>  void
> @@ -822,8 +826,11 @@ genet_intr(void *arg)
>  	val &= ~RD4(sc, GENET_INTRL2_CPU_STAT_MASK);
>  	WR4(sc, GENET_INTRL2_CPU_CLEAR, val);
>  
> -	if (val & GENET_IRQ_RXDMA_DONE)
> +	if (val & GENET_IRQ_RXDMA_DONE) {
> +		mtx_enter(&sc->sc_mtx);
>  		genet_rxintr(sc, GENET_DMA_DEFAULT_QUEUE);
> +		mtx_leave(&sc->sc_mtx);
> +	}
>  
>  	if (val & GENET_IRQ_TXDMA_DONE) {
>  		genet_txintr(sc, GENET_DMA_DEFAULT_QUEUE);
> @@ -952,6 +959,7 @@ genet_attach(struct genet_softc *sc)
>  		return EINVAL;
>  	}
>  
> +	mtx_init(&sc->sc_mtx, IPL_NET);
>  	timeout_set(&sc->sc_stat_ch, genet_tick, sc);
>  	timeout_set(&sc->sc_rxto, genet_rxtick, sc);
>  
> Index: sys/dev/ic/bcmgenetvar.h
> ===================================================================
> RCS file: /cvs/src/sys/dev/ic/bcmgenetvar.h,v
> diff -u -p -r1.2 bcmgenetvar.h
> --- sys/dev/ic/bcmgenetvar.h	15 Jan 2026 03:12:49 -0000	1.2
> +++ sys/dev/ic/bcmgenetvar.h	13 Feb 2026 17:15:12 -0000
> @@ -74,6 +74,7 @@ struct genet_softc {
>  	struct genet_ring	sc_rx;
>  	struct if_rxring	sc_rx_ring;
>  	struct timeout		sc_rxto;
> +	struct mutex		sc_mtx;
>  };
>  
>  int	genet_attach(struct genet_softc *);