[PATCHv3 0/5] arm: arch_timer cleanups and niceties

[PATCHv3 0/5] arm: arch_timer cleanups and niceties

[Mark Rutland]

The following patches clean up some (harmless) fallout from the changes
in 3.9, and make the driver always use the virtual counter, eliminating
problems relating to choosing which counters to use (racy
initialisation, branches in hot paths).

The patches can also be found in my public git tree:
git://linux-arm.org/git/linux-mr.git clocks/arch-timer-cleanups

Russell, are you happy to take these for 3.11 if I send a pull request or drop
these in your patch system?

Since v1 [1]:
* Rebased to v3.10-rc1
* Fix up mach-shmobile's selection of ARCH_ARM_TIMER
Since v2: [2]:
* Added Simon Horman's ack for shmobile
* Clarified virtual timer CNTVOFF requirement per Christopher Covington's
  comments.

Thanks,
Mark.

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2013-March/158604.html
[2] http://lists.infradead.org/pipermail/linux-arm-kernel/2013-May/168536.html

Marc Zyngier (2):
  ARM: hyp: initialize CNTVOFF to zero
  ARM: arch_timer: stop virtual timer when booted in HYP mode

Mark Rutland (3):
  ARM: KVM: arch_timers: zero CNTVOFF upon return to host
  clocksource: arch_timer: use virtual counters
  arm: fix up ARM_ARCH_TIMER selects

 arch/arm/include/asm/arch_timer.h    |  9 ---------
 arch/arm/kernel/hyp-stub.S           |  7 +++++++
 arch/arm/kvm/interrupts_head.S       |  4 ++++
 arch/arm/mach-exynos/Kconfig         |  2 +-
 arch/arm/mach-shmobile/Kconfig       |  4 ++--
 arch/arm/mach-tegra/Kconfig          |  2 +-
 arch/arm/mach-virt/Kconfig           |  2 +-
 arch/arm64/include/asm/arch_timer.h  | 10 ----------
 drivers/clocksource/arm_arch_timer.c | 23 +++++------------------
 include/clocksource/arm_arch_timer.h |  2 +-
 10 files changed, 22 insertions(+), 43 deletions(-)

-- 
1.8.1.1

[PATCHv3 1/5] ARM: hyp: initialize CNTVOFF to zero

[Mark Rutland]

From: Marc Zyngier <Marc.Zyngier@arm.com>

In order to be able to use the virtual counter in a safe way,
make sure it is initialized to zero before dropping to SVC.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Cc: Dave Martin <dave.martin@linaro.org>
---
 arch/arm/kernel/hyp-stub.S | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/kernel/hyp-stub.S b/arch/arm/kernel/hyp-stub.S
index 1315c4c..dbe2110 100644
--- a/arch/arm/kernel/hyp-stub.S
+++ b/arch/arm/kernel/hyp-stub.S
@@ -153,6 +153,8 @@ THUMB(	orr	r7, #(1 << 30)	)	@ HSCTLR.TE
 	mrc	p15, 4, r7, c14, c1, 0	@ CNTHCTL
 	orr	r7, r7, #3		@ PL1PCEN | PL1PCTEN
 	mcr	p15, 4, r7, c14, c1, 0	@ CNTHCTL
+	mov	r7, #0
+	mcrr	p15, 4, r7, r7, c14	@ CNTVOFF
 1:
 #endif
 
-- 
1.8.1.1

[PATCHv3 2/5] ARM: KVM: arch_timers: zero CNTVOFF upon return to host

[Mark Rutland]

To use the virtual counters from the host, we need to ensure that
CNTVOFF doesn't change unexpectedly. When we change to a guest, we
replace the host's CNTVOFF, but we don't restore it when returning to
the host.

As the host sets CNTVOFF to zero, and never changes it, we can simply
zero CNTVOFF when returning to the host. This patch adds said zeroing to
the return to host path.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Acked-by: Marc Zyngier <marc.zyngier@arm.com>
Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Acked-by: Christoffer Dall <cdall@cs.columbia.edu>
---
 arch/arm/kvm/interrupts_head.S | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/kvm/interrupts_head.S b/arch/arm/kvm/interrupts_head.S
index 3c8f2f0..d43cfb5 100644
--- a/arch/arm/kvm/interrupts_head.S
+++ b/arch/arm/kvm/interrupts_head.S
@@ -497,6 +497,10 @@ vcpu	.req	r0		@ vcpu pointer always in r0
 	add	r5, vcpu, r4
 	strd	r2, r3, [r5]
 
+	@ Ensure host CNTVCT == CNTPCT
+	mov	r2, #0
+	mcrr	p15, 4, r2, r2, c14	@ CNTVOFF
+
 1:
 #endif
 	@ Allow physical timer/counter access for the host
-- 
1.8.1.1

[PATCHv3 3/5] clocksource: arch_timer: use virtual counters

[Mark Rutland]

Switching between reading the virtual or physical counters is
problematic, as some core code wants a view of time before we're fully
set up. Using a function pointer and switching the source after the
first read can make time appear to go backwards, and having a check in
the read function is an unfortunate block on what we want to be a fast
path.

Instead, this patch makes us always use the virtual counters. If we're a
guest, or don't have hyp mode, we'll use the virtual timers, and as such
don't care about CNTVOFF as long as it doesn't change in such a way as
to make time appear to travel backwards. As the guest will use the
virtual timers, a (potential) KVM host must use the physical timers
(which can wake up the host even if they fire while a guest is
executing), and hence a host must have CNTVOFF set to zero so as to have
a consistent view of time between the physical timers and virtual
counters.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Acked-by: Marc Zyngier <marc.zyngier@arm.com>
Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Cc: Rob Herring <rob.herring@calxeda.com>
---
 arch/arm/include/asm/arch_timer.h    |  9 ---------
 arch/arm64/include/asm/arch_timer.h  | 10 ----------
 drivers/clocksource/arm_arch_timer.c | 23 +++++------------------
 include/clocksource/arm_arch_timer.h |  2 +-
 4 files changed, 6 insertions(+), 38 deletions(-)

diff --git a/arch/arm/include/asm/arch_timer.h b/arch/arm/include/asm/arch_timer.h
index 7c1bfc0..accefe0 100644
--- a/arch/arm/include/asm/arch_timer.h
+++ b/arch/arm/include/asm/arch_timer.h
@@ -80,15 +80,6 @@ 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 bf6ab242..d56ed11 100644
--- a/arch/arm64/include/asm/arch_timer.h
+++ b/arch/arm64/include/asm/arch_timer.h
@@ -110,16 +110,6 @@ static inline void __cpuinit arch_counter_set_user_access(void)
 	asm volatile("msr	cntkctl_el1, %0" : : "r" (cntkctl));
 }
 
-static inline u64 arch_counter_get_cntpct(void)
-{
-	u64 cval;
-
-	isb();
-	asm volatile("mrs %0, cntpct_el0" : "=r" (cval));
-
-	return cval;
-}
-
 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 a2b2541..053d846 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -186,27 +186,19 @@ u32 arch_timer_get_rate(void)
 	return arch_timer_rate;
 }
 
-/*
- * Some external users of arch_timer_read_counter (e.g. sched_clock) may try to
- * call it before it has been initialised. Rather than incur a performance
- * penalty checking for initialisation, provide a default implementation that
- * won't lead to time appearing to jump backwards.
- */
-static u64 arch_timer_read_zero(void)
+u64 arch_timer_read_counter(void)
 {
-	return 0;
+	return arch_counter_get_cntvct();
 }
 
-u64 (*arch_timer_read_counter)(void) = arch_timer_read_zero;
-
 static cycle_t arch_counter_read(struct clocksource *cs)
 {
-	return arch_timer_read_counter();
+	return arch_counter_get_cntvct();
 }
 
 static cycle_t arch_counter_read_cc(const struct cyclecounter *cc)
 {
-	return arch_timer_read_counter();
+	return arch_counter_get_cntvct();
 }
 
 static struct clocksource clocksource_counter = {
@@ -287,7 +279,7 @@ static int __init arch_timer_register(void)
 	cyclecounter.mult = clocksource_counter.mult;
 	cyclecounter.shift = clocksource_counter.shift;
 	timecounter_init(&timecounter, &cyclecounter,
-			 arch_counter_get_cntpct());
+			 arch_counter_get_cntvct());
 
 	if (arch_timer_use_virtual) {
 		ppi = arch_timer_ppi[VIRT_PPI];
@@ -376,11 +368,6 @@ static void __init arch_timer_init(struct device_node *np)
 		}
 	}
 
-	if (arch_timer_use_virtual)
-		arch_timer_read_counter = arch_counter_get_cntvct;
-	else
-		arch_timer_read_counter = arch_counter_get_cntpct;
-
 	arch_timer_register();
 	arch_timer_arch_init();
 }
diff --git a/include/clocksource/arm_arch_timer.h b/include/clocksource/arm_arch_timer.h
index e6c9c4c..c463ce9 100644
--- a/include/clocksource/arm_arch_timer.h
+++ b/include/clocksource/arm_arch_timer.h
@@ -32,7 +32,7 @@
 #ifdef CONFIG_ARM_ARCH_TIMER
 
 extern u32 arch_timer_get_rate(void);
-extern u64 (*arch_timer_read_counter)(void);
+extern u64 arch_timer_read_counter(void);
 extern struct timecounter *arch_timer_get_timecounter(void);
 
 #else
-- 
1.8.1.1

[PATCHv3 4/5] arm: fix up ARM_ARCH_TIMER selects

[Mark Rutland]

In 8a4da6e: "arm: arch_timer: move core to drivers/clocksource", the
selection of ARM_ARCH_TIMER was indirected via HAVE_ARM_ARCH_TIMER,
though mach-exynos's selection of ARM_ARCH_TIMER was missed, and since
then mach-shmobile, mach-tegra, and mach-virt have begun selecting
ARM_ARCH_TIMER. This can lead to architected timer support erroneously
appearing to not be selected in menuconfig.

This patch fixes up the Kconfigs for those platforms to select
HAVE_ARM_ARCH_TIMER.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Acked-by: Stephen Warren <swarren@nvidia.com>
Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Acked-by: Simon Horman <horms+renesas@verge.net.au>
Cc: Kukjin Kim <kgene.kim@samsung.com>
Cc: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/mach-exynos/Kconfig   | 2 +-
 arch/arm/mach-shmobile/Kconfig | 4 ++--
 arch/arm/mach-tegra/Kconfig    | 2 +-
 arch/arm/mach-virt/Kconfig     | 2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/arm/mach-exynos/Kconfig b/arch/arm/mach-exynos/Kconfig
index d19edff..3c3f36a 100644
--- a/arch/arm/mach-exynos/Kconfig
+++ b/arch/arm/mach-exynos/Kconfig
@@ -76,7 +76,7 @@ config SOC_EXYNOS5440
 	default y
 	depends on ARCH_EXYNOS5
 	select ARCH_HAS_OPP
-	select ARM_ARCH_TIMER
+	select HAVE_ARM_ARCH_TIMER
 	select AUTO_ZRELADDR
 	select PINCTRL
 	select PINCTRL_EXYNOS5440
diff --git a/arch/arm/mach-shmobile/Kconfig b/arch/arm/mach-shmobile/Kconfig
index 1a517e2..767a6c3 100644
--- a/arch/arm/mach-shmobile/Kconfig
+++ b/arch/arm/mach-shmobile/Kconfig
@@ -23,7 +23,7 @@ config ARCH_R8A73A4
 	select ARCH_WANT_OPTIONAL_GPIOLIB
 	select ARM_GIC
 	select CPU_V7
-	select ARM_ARCH_TIMER
+	select HAVE_ARM_ARCH_TIMER
 	select SH_CLK_CPG
 	select RENESAS_IRQC
 
@@ -56,7 +56,7 @@ config ARCH_R8A7790
 	select ARCH_WANT_OPTIONAL_GPIOLIB
 	select ARM_GIC
 	select CPU_V7
-	select ARM_ARCH_TIMER
+	select HAVE_ARM_ARCH_TIMER
 	select SH_CLK_CPG
 	select RENESAS_IRQC
 
diff --git a/arch/arm/mach-tegra/Kconfig b/arch/arm/mach-tegra/Kconfig
index 84d72fc..65c5ae6 100644
--- a/arch/arm/mach-tegra/Kconfig
+++ b/arch/arm/mach-tegra/Kconfig
@@ -60,7 +60,7 @@ config ARCH_TEGRA_3x_SOC
 
 config ARCH_TEGRA_114_SOC
 	bool "Enable support for Tegra114 family"
-	select ARM_ARCH_TIMER
+	select HAVE_ARM_ARCH_TIMER
 	select ARM_GIC
 	select ARM_L1_CACHE_SHIFT_6
 	select CPU_FREQ_TABLE if CPU_FREQ
diff --git a/arch/arm/mach-virt/Kconfig b/arch/arm/mach-virt/Kconfig
index 8958f0d..081d469 100644
--- a/arch/arm/mach-virt/Kconfig
+++ b/arch/arm/mach-virt/Kconfig
@@ -2,7 +2,7 @@ config ARCH_VIRT
 	bool "Dummy Virtual Machine" if ARCH_MULTI_V7
 	select ARCH_WANT_OPTIONAL_GPIOLIB
 	select ARM_GIC
-	select ARM_ARCH_TIMER
+	select HAVE_ARM_ARCH_TIMER
 	select ARM_PSCI
 	select HAVE_SMP
 	select CPU_V7
-- 
1.8.1.1

[PATCHv3 4/5] arm: fix up ARM_ARCH_TIMER selects

[Kukjin Kim]

Mark Rutland wrote:
> 
> In 8a4da6e: "arm: arch_timer: move core to drivers/clocksource", the
> selection of ARM_ARCH_TIMER was indirected via HAVE_ARM_ARCH_TIMER,
> though mach-exynos's selection of ARM_ARCH_TIMER was missed, and since
> then mach-shmobile, mach-tegra, and mach-virt have begun selecting
> ARM_ARCH_TIMER. This can lead to architected timer support erroneously
> appearing to not be selected in menuconfig.
> 
> This patch fixes up the Kconfigs for those platforms to select
> HAVE_ARM_ARCH_TIMER.
> 
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Acked-by: Stephen Warren <swarren@nvidia.com>
> Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Acked-by: Simon Horman <horms+renesas@verge.net.au>
> Cc: Kukjin Kim <kgene.kim@samsung.com>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  arch/arm/mach-exynos/Kconfig   | 2 +-
>  arch/arm/mach-shmobile/Kconfig | 4 ++--
>  arch/arm/mach-tegra/Kconfig    | 2 +-
>  arch/arm/mach-virt/Kconfig     | 2 +-
>  4 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm/mach-exynos/Kconfig b/arch/arm/mach-exynos/Kconfig
> index d19edff..3c3f36a 100644
> --- a/arch/arm/mach-exynos/Kconfig
> +++ b/arch/arm/mach-exynos/Kconfig
> @@ -76,7 +76,7 @@ config SOC_EXYNOS5440
>  	default y
>  	depends on ARCH_EXYNOS5
>  	select ARCH_HAS_OPP
> -	select ARM_ARCH_TIMER
> +	select HAVE_ARM_ARCH_TIMER

Sorry for late response :(

BTW, above should be put after AUTO_ZRELADDR because of alphabetical
ordering...

>  	select AUTO_ZRELADDR
>  	select PINCTRL
>  	select PINCTRL_EXYNOS5440

Others look good to me,

Acked-by: Kukjin Kim <kgene.kim@samsung.com>

Thanks,
- Kukjin

[PATCHv3 5/5] ARM: arch_timer: stop virtual timer when booted in HYP mode

[Mark Rutland]

From: Marc Zyngier <Marc.Zyngier@arm.com>

When booting the kernel, a bootloader could have left the virtual
timer ticking away, potentially generating interrupts. This could
be troublesome if the user of the virtual timer is not careful
when enabling the interrupt.

In order to avoid any surprise, stop the virtual timer from
interrupting us when booted in HYP mode, as we'll use the physical
timer in this case.

Reported-by: Giridhar Maruthy <giridhar.m@samsung.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Cc: Dave Martin <dave.martin@linaro.org>
---
 arch/arm/kernel/hyp-stub.S | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/kernel/hyp-stub.S b/arch/arm/kernel/hyp-stub.S
index dbe2110..4910232 100644
--- a/arch/arm/kernel/hyp-stub.S
+++ b/arch/arm/kernel/hyp-stub.S
@@ -155,6 +155,11 @@ THUMB(	orr	r7, #(1 << 30)	)	@ HSCTLR.TE
 	mcr	p15, 4, r7, c14, c1, 0	@ CNTHCTL
 	mov	r7, #0
 	mcrr	p15, 4, r7, r7, c14	@ CNTVOFF
+
+	@ Disable virtual timer in case it was counting
+	mrc	p15, 0, r7, c14, c3, 1	@ CNTV_CTL
+	bic	r7, #1			@ Clear ENABLE
+	mcr	p15, 0, r7, c14, c3, 1	@ CNTV_CTL
 1:
 #endif
 
-- 
1.8.1.1