On 14/01/13 15:21, Will Deacon wrote: > On Tue, Jan 08, 2013 at 06:43:27PM +0000, Christoffer Dall wrote: >> From: Marc Zyngier <marc.zyngier@xxxxxxx> >> >> 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! CNTVOFF directly influences whether or not CNTV_CVAL will trigger or not. Maybe I'm just being paranoid and moving the isb after CNTV_CVAL is enough. The last one is definitively superfluous. M. -- Jazz is not dead. It just smells funny... -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html