On Fri, Feb 01, 2013 at 06:07:53PM +0000, Dave Martin wrote:

On Fri, Feb 01, 2013 at 11:46:41AM +0000, Mark Rutland wrote:

quoted

On Fri, Feb 01, 2013 at 11:13:50AM +0000, Dave Martin wrote:

quoted

On Thu, Jan 31, 2013 at 12:15:38PM +0000, Mark Rutland wrote:

quoted

From: Marc Zyngier <redacted>

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 <redacted>
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Dave Martin <dave.martin@arm.com>
---
 arch/arm/kernel/hyp-stub.S | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/kernel/hyp-stub.S b/arch/arm/kernel/hyp-stub.S
index 65b2417..455603a 100644
--- a/arch/arm/kernel/hyp-stub.S
+++ b/arch/arm/kernel/hyp-stub.S

@@ -152,6 +152,9 @@ 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	r6, #0
+	mov	r7, #0
+	mcrr	p15, 4, r6, r7, c14	@ CNTVOFF

Is this required for safety, or is it more a sanity feature?

This makes more sense with the next patch, which makes the arch_timer
driver always use the virtual counters (to avoid indirection in the fast
path and messy races with the setup of function pointers otherwise).

It's required for safety when hyp mode is enabled, and the arch_timer
driver uses the physical timers in combination with the virtual
counters. Either the driver has to apply CNTVOFF manually when setting
the physical timers, or the physical timers and virtual counters need
the same view of time (i.e. CNTVOFF == 0).

It also brings us in line with arm64, which always uses the virtual
counter for its vDSO.

OK.  This definitely sounds like the correct model.

Good to hear.

quoted

quoted

The architected timer counters are supposed to be monotonic time sources
only, so applying a random offset shouldn't really change anything.

This is true except when we want to use the physical timers as described above.

quoted

The main thing I can think of is that it is easier for the host to
manage guests' virtual counter offsets if the host's offset is 0 (and
we don't really want to be changing the host offset after the host kernel
boots).

That's pretty much it. We don't want to have to further separate the handling
of the timer for host and guest. By having CNTVOFF as zero for the host, we
don't need to duplicate reading of the timers and/or incur an additional
overhead on reading them.

That all sounds sensible.  FWIW:

Reviewed-by: Dave Martin <redacted>

Thanks!

Mark.