From: Jonathan Gray <jsg@jsg.id.au>
Subject: Re: [PATCH 3/4] Pick the correct timer interrupt for the running EL
To: Mark Kettenis <mark.kettenis@xs4all.nl>
Cc: Marc Zyngier <maz@kernel.org>, tech@openbsd.org, patrick@openbsd.org
Date: Mon, 27 Apr 2026 15:40:55 +1000

On Sun, Apr 26, 2026 at 12:22:19PM +0200, Mark Kettenis wrote:
> Hi Marc,
> 
> > When running at EL2, the kernel is using the virtual EL2 timer,
> > and therefore uses a different interrupt from the one driven by
> > the virtual EL0 timer which gets used when running at EL1.
> >
> > Pick the correct interrupt depending on the EL the kernel runs at.
> 
> Thanks again for the diff.  I've reworked your diff a bit to use
> READ_SPECIALREG() and bits in <machine/armreg.h> which is the usual
> way we deal with system registers in OpenBSD.
> 
> Tested this on one of my Apple systems.  There it will pick a
> different interrupt now.  Worried a bit about that until I realized
> that the way timer interrupts are FIQs and handled by looking directly
> at the generic timer registers means that the actual interrupt
> descriptor that we use on that platform doesn't matter...
> 
> To my fellow OpenBSD developers: ok?

overdrive 1000 with the original non-acpi firmware still works
doesn't have interrupt-names in the device tree

mainbus0 at root: SoftIron Overdrive 1000 (AMD Seattle (Rev.B1))
...
agtimer0 at mainbus0: 250000 kHz

Node 0x6e0
    name: 'timer'
    compatible: 'arm,armv8-timer'
    interrupts: 00000001.0000000d.0000ff04.00000001.0000000e.0000ff04.000000
01.0000000b.0000ff04.00000001.0000000a.0000ff04

ok jsg@

> 
> 
> Index: arch/arm64/include/armreg.h
> ===================================================================
> RCS file: /cvs/src/sys/arch/arm64/include/armreg.h,v
> diff -u -p -r1.44 armreg.h
> --- arch/arm64/include/armreg.h	22 Jul 2025 09:20:41 -0000	1.44
> +++ arch/arm64/include/armreg.h	26 Apr 2026 09:56:36 -0000
> @@ -119,6 +119,14 @@
>  #define	CTR_ILINE_MASK		(0xf << CTR_ILINE_SHIFT)
>  #define	CTR_ILINE_SIZE(reg)	(((reg) & CTR_ILINE_MASK) >> CTR_ILINE_SHIFT)
>  
> +/* CurrentEL - Current Exception Level */
> +#define	CURRENTEL_EL_SHIFT	2
> +#define	CURRENTEL_EL_MASK	(0x3 << CURRENTEL_EL_SHIFT)
> +#define	 CURRENTEL_EL_EL0	(0x0 << CURRENTEL_EL_SHIFT)
> +#define	 CURRENTEL_EL_EL1	(0x1 << CURRENTEL_EL_SHIFT)
> +#define	 CURRENTEL_EL_EL2	(0x2 << CURRENTEL_EL_SHIFT)
> +#define	 CURRENTEL_EL_EL3	(0x3 << CURRENTEL_EL_SHIFT)
> +
>  /* MPIDR_EL1 - Multiprocessor Affinity Register */
>  #define MPIDR_AFF3		(0xFFULL << 32)
>  #define MPIDR_AFF2		(0xFFULL << 16)
> Index: arch/arm64/dev/agtimer.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/arm64/dev/agtimer.c,v
> diff -u -p -r1.29 agtimer.c
> --- arch/arm64/dev/agtimer.c	31 Jan 2025 16:46:41 -0000	1.29
> +++ arch/arm64/dev/agtimer.c	26 Apr 2026 09:56:36 -0000
> @@ -289,7 +289,9 @@ agtimer_set_clockrate(int32_t new_freque
>  void
>  agtimer_cpu_initclocks(void)
>  {
> -	struct agtimer_softc	*sc = agtimer_cd.cd_devs[0];
> +	struct agtimer_softc *sc = agtimer_cd.cd_devs[0];
> +	uint64_t el;
> +	int idx;
>  
>  	stathz = hz;
>  	profhz = stathz * 10;
> @@ -299,8 +301,20 @@ agtimer_cpu_initclocks(void)
>  		agtimer_set_clockrate(agtimer_frequency);
>  	}
>  
> +	/* Pick the correct PPI depending on the running EL. */
> +	el = READ_SPECIALREG(CurrentEL) & CURRENTEL_EL_MASK;
> +	if (el == CURRENTEL_EL_EL2) {
> +		idx = OF_getindex(sc->sc_node, "hyp-virt", "interrupt-names");
> +		if (idx == -1)
> +			idx = 4;
> +	} else {
> +		idx = OF_getindex(sc->sc_node, "virt", "interrupt-names");
> +		if (idx == -1)
> +			idx = 2;
> +	}
> +
>  	/* configure virtual timer interrupt */
> -	sc->sc_ih = arm_intr_establish_fdt_idx(sc->sc_node, 2,
> +	sc->sc_ih = arm_intr_establish_fdt_idx(sc->sc_node, idx,
>  	    IPL_CLOCK|IPL_MPSAFE, agtimer_intr, NULL, "tick");
>  }
>  
> 
>