[PATCH v5 3/4] ARM: KVM: arch_timers: Add timer world switch

[Will Deacon]

On Tue, Jan 08, 2013 at 06:43:27PM +0000, Christoffer Dall wrote:
> From: Marc Zyngier <marc.zyngier at arm.com>
> 
> Do the necessary save/restore dance for the timers in the world
> switch code. In the process, allow the guest to read the physical
> counter, which is useful for its own clock_event_device.

[...]

> @@ -476,6 +513,7 @@ vcpu	.req	r0		@ vcpu pointer always in r0
>   * for the host.
>   *
>   * Assumes vcpu pointer in vcpu reg
> + * Clobbers r2-r4
>   */
>  .macro restore_timer_state
>  	@ Disallow physical timer access for the guest
> @@ -484,6 +522,30 @@ vcpu	.req	r0		@ vcpu pointer always in r0
>  	orr	r2, r2, #CNTHCTL_PL1PCTEN
>  	bic	r2, r2, #CNTHCTL_PL1PCEN
>  	mcr	p15, 4, r2, c14, c1, 0	@ CNTHCTL
> +
> +#ifdef CONFIG_KVM_ARM_TIMER
> +	ldr	r4, [vcpu, #VCPU_KVM]
> +	ldr	r2, [r4, #KVM_TIMER_ENABLED]
> +	cmp	r2, #0
> +	beq	1f
> +
> +	ldr	r2, [r4, #KVM_TIMER_CNTVOFF]
> +	ldr	r3, [r4, #(KVM_TIMER_CNTVOFF + 4)]
> +	mcrr	p15, 4, r2, r3, c14	@ CNTVOFF
> +	isb
> +
> +	ldr	r4, =VCPU_TIMER_CNTV_CVAL
> +	add	vcpu, vcpu, r4
> +	ldrd	r2, r3, [vcpu]
> +	sub	vcpu, vcpu, r4
> +	mcrr	p15, 3, r2, r3, c14	@ CNTV_CVAL
> +
> +	ldr	r2, [vcpu, #VCPU_TIMER_CNTV_CTL]
> +	and	r2, r2, #3
> +	mcr	p15, 0, r2, c14, c3, 1	@ CNTV_CTL
> +	isb

How many of these isbs are actually needed, given that we're going to make
an exception return to the guest? The last one certainly looks redundant and
I can't see the need for ordering CNTVOFF vs CNTV_CVAL. I can see an
argument to putting one *before* CNTV_CTL, but you don't have one there!

Will