From: Visa Hankala <visa@hankala.org>
Subject: Re: octeon: interrupts and barier cleanup
To: "Kirill A. Korinsky" <kirill@korins.ky>
Cc: tech@openbsd.org
Date: Thu, 2 Apr 2026 13:48:01 +0000

On Wed, Apr 01, 2026 at 09:44:40PM +0200, Kirill A. Korinsky wrote:
> Visa, tech@,
> 
> in my work to bring support queues in cnmac I think I discovered a weird
> issue in interrupts and barrier in octeon.
> 
> octciu_intr_establish() has:
> 
>    201          int cpuid = cpu_number();
> ...
>    208  #ifdef MULTIPROCESSOR
>    209          /* Span work queue interrupts across CPUs. */
>    210          if (IS_WORKQ_IRQ(irq))
>    211                  cpuid = irq % ncpus;
>    212  #endif
> ...
>    222          ih->ih_irq = irq;
> 
> my understanding is that octciu_intr_establish() happens on autoconf where
> only CPU0 exists, and on octeon we increase ncpus in cpu hatch, and not
> attach, which means that ncpus is 1 at octciu_intr_establish() time.
> 
> What makes cpuid always 1 for that code and schedules all interrupts on
> CPU0.

Hmm, the code used ncpusfound originally. But it was later changed to
use ncpus, which apparently broke the logic.

> Next, octciu_intr_barrier():
> 
>    290  void
>    291  octciu_intr_barrier(void *_ih)
>    292  {
>    293          struct cpu_info *ci = NULL;
>    294  #ifdef MULTIPROCESSOR
>    295          struct octciu_intrhand *ih = _ih;
>       
>    296          if (IS_WORKQ_IRQ(ih->ih_irq))
>    297                  ci = get_cpu_info(ih->ih_irq % ncpus);
>    298  #endif
>       
>    299          sched_barrier(ci);
>    300  }
> 
> because ih->ih_irq is 0 here, it picks cpu0 only.
> 
> kn@'s diff https://marc.info/?l=openbsd-bugs&m=177171071006897&w=2
> introduced a regression on my ER-4:
> https://marc.info/?l=openbsd-bugs&m=177177669428825&w=2 and this regression
> can be avoided by removing the block in octciu_intr_establish().
> 
> Here is the diff where I remove this block and % ncpus.
> 
> This diff keeps interrupts on CPU0 which is, actually, current behaviour and
> unblocks kn@'s diff to enable softnet threads on octeon which unblocks my
> work of making cnmac with RX queue:
> https://marc.info/?l=openbsd-tech&m=177506394121597&w=2
> 
> So far I see this diff as safe because it doesn't change current behaviour
> but I may miss some details.
> 
> OK to commit it?

I suggest you surround the #ifdef MULTIPROCESSOR blocks in
octciu_intr_establish() and octciu_intr_barrier() with #if 0, because
you will probably need them later for distributing the POW group
interrupts and testing your multi-queue work.

> Index: sys/arch/octeon/dev/octciu.c
> ===================================================================
> RCS file: /home/cvs/src/sys/arch/octeon/dev/octciu.c,v
> diff -u -p -r1.19 octciu.c
> --- sys/arch/octeon/dev/octciu.c	11 Dec 2022 05:31:05 -0000	1.19
> +++ sys/arch/octeon/dev/octciu.c	1 Apr 2026 19:39:40 -0000
> @@ -250,12 +250,6 @@ octciu_intr_establish(int irq, int level
>  		panic("%s: illegal irq %d", __func__, irq);
>  #endif
>  
> -#ifdef MULTIPROCESSOR
> -	/* Span work queue interrupts across CPUs. */
> -	if (IS_WORKQ_IRQ(irq))
> -		cpuid = irq % ncpus;
> -#endif
> -
>  	flags = (level & IPL_MPSAFE) ? CIH_MPSAFE : 0;
>  	level &= ~IPL_MPSAFE;
>  
> @@ -363,7 +357,7 @@ octciu_intr_barrier(void *_ih)
>  	struct octciu_intrhand *ih = _ih;
>  
>  	if (IS_WORKQ_IRQ(ih->ih_irq))
> -		ci = get_cpu_info(ih->ih_irq % ncpus);
> +		ci = get_cpu_info(ih->ih_irq);
>  #endif
>  
>  	sched_barrier(ci);