[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openbsd-tech
Subject:    Re: Switch agtimer from physical timer to virtual timer.
From:       Patrick Wildt <patrick () blueri ! se>
Date:       2017-02-27 8:34:41
Message-ID: 20170227083441.GA37089 () eris ! local
[Download RAW message or body]

On Sun, Feb 26, 2017 at 10:35:17PM -0500, Dale Rahn wrote:
> Switch agtimer from physical timer to virtual timer.
> 
> This diff makes the arm generic timer for arm64 use the virtual
> timer instead of the physical timer.
> 
> Linux uses the virtual timer in the kernel unless it is operating in
> hypervisor mode.

Specifically, Linux uses the physical timer if hypervisor mode is
available or the kernel is actually running in hypervisor mode.  There
are four different interrupt pins usually provided in the device tree,
but not all of them work depending on which environment you run in:

Physical timer:
 * secure world IRQ
 * non-secure world IRQ
 * hypervisor IRQ

Virtual timer:
 * virt IRQ

Switching to the virtual timer makes sense to me.  We can always add
support for using the physical timer if we ever add hypervisor support.

Diff looks good to me.

> The virtual timer is set up so that a hypervisor can run an operating system
> under it. This allows the hypervisor adjust time, eg to make it look like
> the time spent in a higher level exeption didn't occur.
> 
> Access to the physical timer can be disabled from higher exception level,
> making it required that the operating system use the virtual timer.
> 
> The use of virtual timer vs physical timer came up in a discussion with
> Patrick, I decided it was better to just switch the timer now. 
> 
> diff --git a/sys/arch/arm64/dev/agtimer.c b/sys/arch/arm64/dev/agtimer.c
> index 278ad9a0e19..3c9abc6011a 100644
> --- a/sys/arch/arm64/dev/agtimer.c
> +++ b/sys/arch/arm64/dev/agtimer.c
> @@ -33,9 +33,9 @@
>  #include <dev/ofw/openfirm.h>
>  
>  /* registers */
> -#define GTIMER_CNTP_CTL_ENABLE		(1 << 0)
> -#define GTIMER_CNTP_CTL_IMASK		(1 << 1)
> -#define GTIMER_CNTP_CTL_ISTATUS		(1 << 2)
> +#define GTIMER_CNTV_CTL_ENABLE		(1 << 0)
> +#define GTIMER_CNTV_CTL_IMASK		(1 << 1)
> +#define GTIMER_CNTV_CTL_ISTATUS		(1 << 2)
>  
>  #define TIMER_FREQUENCY		24 * 1000 * 1000 /* ARM core clock */
>  int32_t agtimer_frequency = TIMER_FREQUENCY;
> @@ -96,7 +96,7 @@ agtimer_readcnt64(void)
>  	uint64_t val;
>  
>  	__asm volatile("isb" : : : "memory");
> -	__asm volatile("MRS %x0, CNTPCT_EL0" : "=r" (val));
> +	__asm volatile("MRS %x0, CNTVCT_EL0" : "=r" (val));
>  
>  	return (val);
>  }
> @@ -116,7 +116,7 @@ agtimer_get_ctrl(void)
>  {
>  	uint32_t val;
>  
> -	__asm volatile("MRS %x0, CNTP_CTL_EL0" : "=r" (val));
> +	__asm volatile("MRS %x0, CNTV_CTL_EL0" : "=r" (val));
>  
>  	return (val);
>  }
> @@ -124,7 +124,7 @@ agtimer_get_ctrl(void)
>  static inline int
>  agtimer_set_ctrl(uint32_t val)
>  {
> -	__asm volatile("MSR CNTP_CTL_EL0, %x0" : : "r" (val));
> +	__asm volatile("MSR CNTV_CTL_EL0, %x0" : : "r" (val));
>  	__asm volatile("isb" : : : "memory");
>  
>  	return (0);
> @@ -133,7 +133,7 @@ agtimer_set_ctrl(uint32_t val)
>  static inline int
>  agtimer_set_tval(uint32_t val)
>  {
> -	__asm volatile("MSR CNTP_TVAL_EL0, %x0" : : "r" (val));
> +	__asm volatile("MSR CNTV_TVAL_EL0, %x0" : : "r" (val));
>  	__asm volatile("isb" : : : "memory");
>  
>  	return (0);
> @@ -296,18 +296,16 @@ agtimer_cpu_initclocks()
>  	sc->sc_ticks_err_cnt = sc->sc_ticks_per_second % hz;
>  	pc->pc_ticks_err_sum = 0;
>  
> -	/* Setup secure and non-secure timer IRQs. */
> -	arm_intr_establish_fdt_idx(sc->sc_node, 0, IPL_CLOCK,
> -	    agtimer_intr, NULL, "tick");
> -	arm_intr_establish_fdt_idx(sc->sc_node, 1, IPL_CLOCK,
> +	/* configure virtual timer interupt */
> +	arm_intr_establish_fdt_idx(sc->sc_node, 2, IPL_CLOCK,
>  	    agtimer_intr, NULL, "tick");
>  
>  	next = agtimer_readcnt64() + sc->sc_ticks_per_intr;
>  	pc->pc_nexttickevent = pc->pc_nextstatevent = next;
>  
>  	reg = agtimer_get_ctrl();
> -	reg &= ~GTIMER_CNTP_CTL_IMASK;
> -	reg |= GTIMER_CNTP_CTL_ENABLE;
> +	reg &= ~GTIMER_CNTV_CTL_IMASK;
> +	reg |= GTIMER_CNTV_CTL_ENABLE;
>  	agtimer_set_tval(sc->sc_ticks_per_second);
>  	agtimer_set_ctrl(reg);
>  }
> @@ -381,8 +379,8 @@ agtimer_startclock(void)
>  	pc->pc_nexttickevent = pc->pc_nextstatevent = nextevent;
>  
>  	reg = agtimer_get_ctrl();
> -	reg &= ~GTIMER_CNTP_CTL_IMASK;
> -	reg |= GTIMER_CNTP_CTL_ENABLE;
> +	reg &= ~GTIMER_CNTV_CTL_IMASK;
> +	reg |= GTIMER_CNTV_CTL_ENABLE;
>  	agtimer_set_tval(sc->sc_ticks_per_second);
>  	agtimer_set_ctrl(reg);
>  }
> Dale Rahn				drahn@dalerahn.com
> 

[prev in list] [next in list] [prev in thread] [next in thread]