[v5] clocksource: arch_timer: Fix code to use physical timers when requested
diff mbox

Message ID 1416812564-26465-1-git-send-email-sonnyrao@chromium.org
State New, archived
Headers show

Commit Message

Sonny Rao Nov. 24, 2014, 7:02 a.m. UTC
This is a bug fix for using physical arch timers when
the arch_timer_use_virtual boolean is false.  It restores the
arch_counter_get_cntpct() function after removal in

0d651e4e "clocksource: arch_timer: use virtual counters"

We need this on certain ARMv7 systems which are architected like this:

* The firmware doesn't know and doesn't care about hypervisor mode and
  we don't want to add the complexity of hypervisor there.

* The firmware isn't involved in SMP bringup or resume.

* The ARCH timer come up with an uninitialized offset between the
  virtual and physical counters.  Each core gets a different random
  offset.

* The device boots in "Secure SVC" mode.

* Nothing has touched the reset value of CNTHCTL.PL1PCEN or
  CNTHCTL.PL1PCTEN (both default to 1 at reset)

One example of such as system is RK3288 where it is much simpler to
use the physical counter since there's nobody managing the offset and
each time a core goes down and comes back up it will get reinitialized
to some other random value.

Fixes: 0d651e4e65e9 ("clocksource: arch_timer: use virtual counters")
Cc: stable@vger.kernel.org
Signed-off-by: Sonny Rao <sonnyrao@chromium.org>
Acked-by: Olof Johansson <olof@lixom.net>
---
v2: Add fixes tag to commit message, cc stable, copy Doug's
    description of the systems which need this in commit message.
v3: Don't change the memory-mapped physical timer/counter code
v4: remove the memory-mapped physical counter code since it's not used
v5: rebase and make AArch64 version of arch_counter_get_cntpct call BUG()
---
 arch/arm/include/asm/arch_timer.h    | 9 +++++++++
 arch/arm64/include/asm/arch_timer.h  | 9 +++++++++
 drivers/clocksource/arm_arch_timer.c | 5 ++++-
 3 files changed, 22 insertions(+), 1 deletion(-)

Comments

Daniel Lezcano Nov. 24, 2014, 9:01 a.m. UTC | #1
On 11/24/2014 08:02 AM, Sonny Rao wrote:
> This is a bug fix for using physical arch timers when
> the arch_timer_use_virtual boolean is false.  It restores the
> arch_counter_get_cntpct() function after removal in
>
> 0d651e4e "clocksource: arch_timer: use virtual counters"
>
> We need this on certain ARMv7 systems which are architected like this:
>
> * The firmware doesn't know and doesn't care about hypervisor mode and
>    we don't want to add the complexity of hypervisor there.
>
> * The firmware isn't involved in SMP bringup or resume.
>
> * The ARCH timer come up with an uninitialized offset between the
>    virtual and physical counters.  Each core gets a different random
>    offset.
>
> * The device boots in "Secure SVC" mode.
>
> * Nothing has touched the reset value of CNTHCTL.PL1PCEN or
>    CNTHCTL.PL1PCTEN (both default to 1 at reset)
>
> One example of such as system is RK3288 where it is much simpler to
> use the physical counter since there's nobody managing the offset and
> each time a core goes down and comes back up it will get reinitialized
> to some other random value.
>
> Fixes: 0d651e4e65e9 ("clocksource: arch_timer: use virtual counters")
> Cc: stable@vger.kernel.org
> Signed-off-by: Sonny Rao <sonnyrao@chromium.org>
> Acked-by: Olof Johansson <olof@lixom.net>


Catalin,

are you ok with this patch ?

Thanks
   -- Daniel

> ---
> v2: Add fixes tag to commit message, cc stable, copy Doug's
>      description of the systems which need this in commit message.
> v3: Don't change the memory-mapped physical timer/counter code
> v4: remove the memory-mapped physical counter code since it's not used
> v5: rebase and make AArch64 version of arch_counter_get_cntpct call BUG()
> ---
>   arch/arm/include/asm/arch_timer.h    | 9 +++++++++
>   arch/arm64/include/asm/arch_timer.h  | 9 +++++++++
>   drivers/clocksource/arm_arch_timer.c | 5 ++++-
>   3 files changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/include/asm/arch_timer.h b/arch/arm/include/asm/arch_timer.h
> index 92793ba..d4ebf56 100644
> --- a/arch/arm/include/asm/arch_timer.h
> +++ b/arch/arm/include/asm/arch_timer.h
> @@ -78,6 +78,15 @@ static inline u32 arch_timer_get_cntfrq(void)
>   	return val;
>   }
>
> +static inline u64 arch_counter_get_cntpct(void)
> +{
> +	u64 cval;
> +
> +	isb();
> +	asm volatile("mrrc p15, 0, %Q0, %R0, c14" : "=r" (cval));
> +	return cval;
> +}
> +
>   static inline u64 arch_counter_get_cntvct(void)
>   {
>   	u64 cval;
> diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h
> index f190971..b1fa4e6 100644
> --- a/arch/arm64/include/asm/arch_timer.h
> +++ b/arch/arm64/include/asm/arch_timer.h
> @@ -104,6 +104,15 @@ static inline void arch_timer_set_cntkctl(u32 cntkctl)
>   	asm volatile("msr	cntkctl_el1, %0" : : "r" (cntkctl));
>   }
>
> +static inline u64 arch_counter_get_cntpct(void)
> +{
> +	/*
> +	 * AArch64 kernel and user space mandate the use of CNTVCT.
> +	 */
> +	BUG();
> +	return 0;
> +}
> +
>   static inline u64 arch_counter_get_cntvct(void)
>   {
>   	u64 cval;
> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> index 43005d4..1fa2af9 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
> @@ -462,7 +462,10 @@ static void __init arch_counter_register(unsigned type)
>
>   	/* Register the CP15 based counter if we have one */
>   	if (type & ARCH_CP15_TIMER) {
> -		arch_timer_read_counter = arch_counter_get_cntvct;
> +		if (arch_timer_use_virtual)
> +			arch_timer_read_counter = arch_counter_get_cntvct;
> +		else
> +			arch_timer_read_counter = arch_counter_get_cntpct;
>   	} else {
>   		arch_timer_read_counter = arch_counter_get_cntvct_mem;
>
>
Catalin Marinas Nov. 24, 2014, 2:16 p.m. UTC | #2
On Mon, Nov 24, 2014 at 09:01:36AM +0000, Daniel Lezcano wrote:
> On 11/24/2014 08:02 AM, Sonny Rao wrote:
> > This is a bug fix for using physical arch timers when
> > the arch_timer_use_virtual boolean is false.  It restores the
> > arch_counter_get_cntpct() function after removal in
> >
> > 0d651e4e "clocksource: arch_timer: use virtual counters"
> >
> > We need this on certain ARMv7 systems which are architected like this:
> >
> > * The firmware doesn't know and doesn't care about hypervisor mode and
> >    we don't want to add the complexity of hypervisor there.
> >
> > * The firmware isn't involved in SMP bringup or resume.
> >
> > * The ARCH timer come up with an uninitialized offset between the
> >    virtual and physical counters.  Each core gets a different random
> >    offset.
> >
> > * The device boots in "Secure SVC" mode.
> >
> > * Nothing has touched the reset value of CNTHCTL.PL1PCEN or
> >    CNTHCTL.PL1PCTEN (both default to 1 at reset)
> >
> > One example of such as system is RK3288 where it is much simpler to
> > use the physical counter since there's nobody managing the offset and
> > each time a core goes down and comes back up it will get reinitialized
> > to some other random value.
> >
> > Fixes: 0d651e4e65e9 ("clocksource: arch_timer: use virtual counters")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Sonny Rao <sonnyrao@chromium.org>
> > Acked-by: Olof Johansson <olof@lixom.net>
> 
> 
> Catalin,
> 
> are you ok with this patch ?

Yes.

Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Daniel Lezcano Nov. 24, 2014, 2:19 p.m. UTC | #3
On 11/24/2014 03:16 PM, Catalin Marinas wrote:
> Acked-by: Catalin Marinas<catalin.marinas@arm.com>

Thanks !

   -- Daniel
Daniel Lezcano Nov. 26, 2014, 12:47 p.m. UTC | #4
On 11/24/2014 08:02 AM, Sonny Rao wrote:
> This is a bug fix for using physical arch timers when
> the arch_timer_use_virtual boolean is false.  It restores the
> arch_counter_get_cntpct() function after removal in
>
> 0d651e4e "clocksource: arch_timer: use virtual counters"
>
> We need this on certain ARMv7 systems which are architected like this:
>
> * The firmware doesn't know and doesn't care about hypervisor mode and
>    we don't want to add the complexity of hypervisor there.
>
> * The firmware isn't involved in SMP bringup or resume.
>
> * The ARCH timer come up with an uninitialized offset between the
>    virtual and physical counters.  Each core gets a different random
>    offset.
>
> * The device boots in "Secure SVC" mode.
>
> * Nothing has touched the reset value of CNTHCTL.PL1PCEN or
>    CNTHCTL.PL1PCTEN (both default to 1 at reset)
>
> One example of such as system is RK3288 where it is much simpler to
> use the physical counter since there's nobody managing the offset and
> each time a core goes down and comes back up it will get reinitialized
> to some other random value.
>
> Fixes: 0d651e4e65e9 ("clocksource: arch_timer: use virtual counters")
> Cc: stable@vger.kernel.org
> Signed-off-by: Sonny Rao <sonnyrao@chromium.org>
> Acked-by: Olof Johansson <olof@lixom.net>

Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>



> ---
> v2: Add fixes tag to commit message, cc stable, copy Doug's
>      description of the systems which need this in commit message.
> v3: Don't change the memory-mapped physical timer/counter code
> v4: remove the memory-mapped physical counter code since it's not used
> v5: rebase and make AArch64 version of arch_counter_get_cntpct call BUG()
> ---
>   arch/arm/include/asm/arch_timer.h    | 9 +++++++++
>   arch/arm64/include/asm/arch_timer.h  | 9 +++++++++
>   drivers/clocksource/arm_arch_timer.c | 5 ++++-
>   3 files changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/include/asm/arch_timer.h b/arch/arm/include/asm/arch_timer.h
> index 92793ba..d4ebf56 100644
> --- a/arch/arm/include/asm/arch_timer.h
> +++ b/arch/arm/include/asm/arch_timer.h
> @@ -78,6 +78,15 @@ static inline u32 arch_timer_get_cntfrq(void)
>   	return val;
>   }
>
> +static inline u64 arch_counter_get_cntpct(void)
> +{
> +	u64 cval;
> +
> +	isb();
> +	asm volatile("mrrc p15, 0, %Q0, %R0, c14" : "=r" (cval));
> +	return cval;
> +}
> +
>   static inline u64 arch_counter_get_cntvct(void)
>   {
>   	u64 cval;
> diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h
> index f190971..b1fa4e6 100644
> --- a/arch/arm64/include/asm/arch_timer.h
> +++ b/arch/arm64/include/asm/arch_timer.h
> @@ -104,6 +104,15 @@ static inline void arch_timer_set_cntkctl(u32 cntkctl)
>   	asm volatile("msr	cntkctl_el1, %0" : : "r" (cntkctl));
>   }
>
> +static inline u64 arch_counter_get_cntpct(void)
> +{
> +	/*
> +	 * AArch64 kernel and user space mandate the use of CNTVCT.
> +	 */
> +	BUG();
> +	return 0;
> +}
> +
>   static inline u64 arch_counter_get_cntvct(void)
>   {
>   	u64 cval;
> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> index 43005d4..1fa2af9 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
> @@ -462,7 +462,10 @@ static void __init arch_counter_register(unsigned type)
>
>   	/* Register the CP15 based counter if we have one */
>   	if (type & ARCH_CP15_TIMER) {
> -		arch_timer_read_counter = arch_counter_get_cntvct;
> +		if (arch_timer_use_virtual)
> +			arch_timer_read_counter = arch_counter_get_cntvct;
> +		else
> +			arch_timer_read_counter = arch_counter_get_cntpct;
>   	} else {
>   		arch_timer_read_counter = arch_counter_get_cntvct_mem;
>
>
Olof Johansson Dec. 5, 2014, 7:30 a.m. UTC | #5
On Sun, Nov 23, 2014 at 11:02:44PM -0800, Sonny Rao wrote:
> This is a bug fix for using physical arch timers when
> the arch_timer_use_virtual boolean is false.  It restores the
> arch_counter_get_cntpct() function after removal in
> 
> 0d651e4e "clocksource: arch_timer: use virtual counters"
> 
> We need this on certain ARMv7 systems which are architected like this:
> 
> * The firmware doesn't know and doesn't care about hypervisor mode and
>   we don't want to add the complexity of hypervisor there.
> 
> * The firmware isn't involved in SMP bringup or resume.
> 
> * The ARCH timer come up with an uninitialized offset between the
>   virtual and physical counters.  Each core gets a different random
>   offset.
> 
> * The device boots in "Secure SVC" mode.
> 
> * Nothing has touched the reset value of CNTHCTL.PL1PCEN or
>   CNTHCTL.PL1PCTEN (both default to 1 at reset)
> 
> One example of such as system is RK3288 where it is much simpler to
> use the physical counter since there's nobody managing the offset and
> each time a core goes down and comes back up it will get reinitialized
> to some other random value.
> 
> Fixes: 0d651e4e65e9 ("clocksource: arch_timer: use virtual counters")
> Cc: stable@vger.kernel.org
> Signed-off-by: Sonny Rao <sonnyrao@chromium.org>
> Acked-by: Olof Johansson <olof@lixom.net>

Applied to a topic branch in arm-soc so we can include it in next/dt and
next/drivers as appropriate for rk3288.


Thanks, all!


-Olof
Yingjoe Chen Dec. 5, 2014, 10:41 a.m. UTC | #6
On Sun, 2014-11-23 at 23:02 -0800, Sonny Rao wrote: 
> This is a bug fix for using physical arch timers when
> the arch_timer_use_virtual boolean is false.  It restores the
> arch_counter_get_cntpct() function after removal in
> 
> 0d651e4e "clocksource: arch_timer: use virtual counters"
> 
> We need this on certain ARMv7 systems which are architected like this:
> diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h
> index f190971..b1fa4e6 100644
> --- a/arch/arm64/include/asm/arch_timer.h
> +++ b/arch/arm64/include/asm/arch_timer.h
> @@ -104,6 +104,15 @@ static inline void arch_timer_set_cntkctl(u32 cntkctl)
>  	asm volatile("msr	cntkctl_el1, %0" : : "r" (cntkctl));
>  }
>  
> +static inline u64 arch_counter_get_cntpct(void)
> +{
> +	/*
> +	 * AArch64 kernel and user space mandate the use of CNTVCT.
> +	 */
> +	BUG();
> +	return 0;
> +}
> +
>  static inline u64 arch_counter_get_cntvct(void)
>  {
>  	u64 cval;

I tested this on an arm64 platform and system fail to boot when apply
this patch.

The boot loader start kernel at EL2, so is_hyp_mode_available() will be
true and we will use physical timer. Without this patch,
arch_timer_read_counter set to arch_counter_get_cntvct even when we use
physical timer which is incorrect but at least system will boot.

I think we still need this function on arm64. We should add BUG() to
arch_timer_init instead, maybe something like this:


@@ -708,9 +708,12 @@ static void __init arch_timer_init(struct device_node *np)
 	 * If we cannot rely on firmware initializing the timer registers then
 	 * we should use the physical timers instead.
 	 */
-	if (IS_ENABLED(CONFIG_ARM) &&
-	    of_property_read_bool(np, "arm,cpu-registers-not-fw-configured"))
+	if (of_property_read_bool(np, "arm,cpu-registers-not-fw-configured")) {
+		if (IS_ENABLED(CONFIG_ARM64))
+			BUG();
+		else
 			arch_timer_use_virtual = false;
+	}

Joe.C
Catalin Marinas Dec. 8, 2014, 4:21 p.m. UTC | #7
On Fri, Dec 05, 2014 at 10:41:04AM +0000, Yingjoe Chen wrote:
> On Sun, 2014-11-23 at 23:02 -0800, Sonny Rao wrote: 
> > This is a bug fix for using physical arch timers when
> > the arch_timer_use_virtual boolean is false.  It restores the
> > arch_counter_get_cntpct() function after removal in
> > 
> > 0d651e4e "clocksource: arch_timer: use virtual counters"
> > 
> > We need this on certain ARMv7 systems which are architected like this:
> > diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h
> > index f190971..b1fa4e6 100644
> > --- a/arch/arm64/include/asm/arch_timer.h
> > +++ b/arch/arm64/include/asm/arch_timer.h
> > @@ -104,6 +104,15 @@ static inline void arch_timer_set_cntkctl(u32 cntkctl)
> >  	asm volatile("msr	cntkctl_el1, %0" : : "r" (cntkctl));
> >  }
> >  
> > +static inline u64 arch_counter_get_cntpct(void)
> > +{
> > +	/*
> > +	 * AArch64 kernel and user space mandate the use of CNTVCT.
> > +	 */
> > +	BUG();
> > +	return 0;
> > +}
> > +
> >  static inline u64 arch_counter_get_cntvct(void)
> >  {
> >  	u64 cval;
> 
> I tested this on an arm64 platform and system fail to boot when apply
> this patch.
> 
> The boot loader start kernel at EL2, so is_hyp_mode_available() will be
> true and we will use physical timer. Without this patch,
> arch_timer_read_counter set to arch_counter_get_cntvct even when we use
> physical timer which is incorrect but at least system will boot.

So on arm64 we want to use CNTVCT all the time, even if we start the
kernel at EL2. This is the counter that gets exposed to user via vdso.
When the kernel boots at EL2, we initialise CNTVOFF to 0, so we know
that CNTVCT == CNTPCT. However, I would still prefer to use CNTVCT even
in such case to spot possible firmware issues with not restoring CNTVOFF
when coming out of suspend (one particular case of suspend which does
not require a different kernel entry point).

> I think we still need this function on arm64. We should add BUG() to
> arch_timer_init instead, maybe something like this:
> 
> @@ -708,9 +708,12 @@ static void __init arch_timer_init(struct device_node *np)
>  	 * If we cannot rely on firmware initializing the timer registers then
>  	 * we should use the physical timers instead.
>  	 */
> -	if (IS_ENABLED(CONFIG_ARM) &&
> -	    of_property_read_bool(np, "arm,cpu-registers-not-fw-configured"))
> +	if (of_property_read_bool(np, "arm,cpu-registers-not-fw-configured")) {
> +		if (IS_ENABLED(CONFIG_ARM64))
> +			BUG();
> +		else
>  			arch_timer_use_virtual = false;
> +	}

We could be more tolerant (give people a chance to check their DT):

	if (!WARN_ON(IS_ENABLED(CONFIG_ARM64)))
		arch_timer_use_virtual = false;

In addition, we can define arch_counter_get_cntpct to always read CNTVCT
(not that nice but maybe it looks better with a comment):

/*
 * AArch64 kernel and user space mandate the use of CNTVCT.
 */
#define arch_counter_get_cntpct	arch_counter_get_cntvct

(or any better suggestion to avoid reading CNTPCT on arm64)
Yingjoe Chen Dec. 9, 2014, 6:31 a.m. UTC | #8
On Mon, 2014-12-08 at 16:21 +0000, Catalin Marinas wrote:
> On Fri, Dec 05, 2014 at 10:41:04AM +0000, Yingjoe Chen wrote:
> > On Sun, 2014-11-23 at 23:02 -0800, Sonny Rao wrote: 
> > > This is a bug fix for using physical arch timers when
> > > the arch_timer_use_virtual boolean is false.  It restores the
> > > arch_counter_get_cntpct() function after removal in
> > > 
> > > 0d651e4e "clocksource: arch_timer: use virtual counters"
> > > 
> > > We need this on certain ARMv7 systems which are architected like this:
> > > diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h
> > > index f190971..b1fa4e6 100644
> > > --- a/arch/arm64/include/asm/arch_timer.h
> > > +++ b/arch/arm64/include/asm/arch_timer.h
> > > @@ -104,6 +104,15 @@ static inline void arch_timer_set_cntkctl(u32 cntkctl)
> > >  	asm volatile("msr	cntkctl_el1, %0" : : "r" (cntkctl));
> > >  }
> > >  
> > > +static inline u64 arch_counter_get_cntpct(void)
> > > +{
> > > +	/*
> > > +	 * AArch64 kernel and user space mandate the use of CNTVCT.
> > > +	 */
> > > +	BUG();
> > > +	return 0;
> > > +}
> > > +
> > >  static inline u64 arch_counter_get_cntvct(void)
> > >  {
> > >  	u64 cval;
> > 
> > I tested this on an arm64 platform and system fail to boot when apply
> > this patch.
> > 
> > The boot loader start kernel at EL2, so is_hyp_mode_available() will be
> > true and we will use physical timer. Without this patch,
> > arch_timer_read_counter set to arch_counter_get_cntvct even when we use
> > physical timer which is incorrect but at least system will boot.
> 
> So on arm64 we want to use CNTVCT all the time, even if we start the
> kernel at EL2. This is the counter that gets exposed to user via vdso.
> When the kernel boots at EL2, we initialise CNTVOFF to 0, so we know
> that CNTVCT == CNTPCT. However, I would still prefer to use CNTVCT even
> in such case to spot possible firmware issues with not restoring CNTVOFF
> when coming out of suspend (one particular case of suspend which does
> not require a different kernel entry point).
> 
> > I think we still need this function on arm64. We should add BUG() to
> > arch_timer_init instead, maybe something like this:
> > 
> > @@ -708,9 +708,12 @@ static void __init arch_timer_init(struct device_node *np)
> >  	 * If we cannot rely on firmware initializing the timer registers then
> >  	 * we should use the physical timers instead.
> >  	 */
> > -	if (IS_ENABLED(CONFIG_ARM) &&
> > -	    of_property_read_bool(np, "arm,cpu-registers-not-fw-configured"))
> > +	if (of_property_read_bool(np, "arm,cpu-registers-not-fw-configured")) {
> > +		if (IS_ENABLED(CONFIG_ARM64))
> > +			BUG();
> > +		else
> >  			arch_timer_use_virtual = false;
> > +	}
> 
> We could be more tolerant (give people a chance to check their DT):
> 
> 	if (!WARN_ON(IS_ENABLED(CONFIG_ARM64)))
> 		arch_timer_use_virtual = false;
> 
> In addition, we can define arch_counter_get_cntpct to always read CNTVCT
> (not that nice but maybe it looks better with a comment):
> 
> /*
>  * AArch64 kernel and user space mandate the use of CNTVCT.
>  */
> #define arch_counter_get_cntpct	arch_counter_get_cntvct
> 
> (or any better suggestion to avoid reading CNTPCT on arm64)
> 

I'm not sure about this. arch_timer_use_virtual is false, indicate we
are using physical timer, the function name suggest it will read
physical timer, but actually it is reading virtual timer. It sure will
create confusion when one need to debug.

We are using physical timer because arch_timer_init() will check
is_hyp_mode_available(), and use physical timer if it is avaliable. For
arm64, if we want to use virtual timer even in HYP mode, how about this
change:

@@ -720,7 +723,8 @@ static void __init arch_timer_init(struct device_node *np)
         * If no interrupt provided for virtual timer, we'll have to
         * stick to the physical timer. It'd better be accessible...
         */
-       if (is_hyp_mode_available() || !arch_timer_ppi[VIRT_PPI]) {
+       if ((IS_ENABLED(CONFIG_ARM) && is_hyp_mode_available()) ||
+           !arch_timer_ppi[VIRT_PPI]) {
                arch_timer_use_virtual = false;

                if (!arch_timer_ppi[PHYS_SECURE_PPI] ||

Maybe also add some WARN_ON for arm64 here to make sure we never set
arch_timer_use_virtual to false for ARM64.

Joe.C

Patch
diff mbox

diff --git a/arch/arm/include/asm/arch_timer.h b/arch/arm/include/asm/arch_timer.h
index 92793ba..d4ebf56 100644
--- a/arch/arm/include/asm/arch_timer.h
+++ b/arch/arm/include/asm/arch_timer.h
@@ -78,6 +78,15 @@  static inline u32 arch_timer_get_cntfrq(void)
 	return val;
 }
 
+static inline u64 arch_counter_get_cntpct(void)
+{
+	u64 cval;
+
+	isb();
+	asm volatile("mrrc p15, 0, %Q0, %R0, c14" : "=r" (cval));
+	return cval;
+}
+
 static inline u64 arch_counter_get_cntvct(void)
 {
 	u64 cval;
diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h
index f190971..b1fa4e6 100644
--- a/arch/arm64/include/asm/arch_timer.h
+++ b/arch/arm64/include/asm/arch_timer.h
@@ -104,6 +104,15 @@  static inline void arch_timer_set_cntkctl(u32 cntkctl)
 	asm volatile("msr	cntkctl_el1, %0" : : "r" (cntkctl));
 }
 
+static inline u64 arch_counter_get_cntpct(void)
+{
+	/*
+	 * AArch64 kernel and user space mandate the use of CNTVCT.
+	 */
+	BUG();
+	return 0;
+}
+
 static inline u64 arch_counter_get_cntvct(void)
 {
 	u64 cval;
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 43005d4..1fa2af9 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -462,7 +462,10 @@  static void __init arch_counter_register(unsigned type)
 
 	/* Register the CP15 based counter if we have one */
 	if (type & ARCH_CP15_TIMER) {
-		arch_timer_read_counter = arch_counter_get_cntvct;
+		if (arch_timer_use_virtual)
+			arch_timer_read_counter = arch_counter_get_cntvct;
+		else
+			arch_timer_read_counter = arch_counter_get_cntpct;
 	} else {
 		arch_timer_read_counter = arch_counter_get_cntvct_mem;