On Tue, Jun 26, 2012 at 9:26 AM, Marc Zyngier <marc.zyngier at arm.com> wrote: > On 25/06/12 22:16, Christoffer Dall wrote: >> On Mon, May 14, 2012 at 9:07 AM, Marc Zyngier <marc.zyngier at arm.com> wrote: >>> 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. >>> >>> Signed-off-by: Marc Zyngier <marc.zyngier at arm.com> >>> --- >>> ?arch/arm/kernel/asm-offsets.c | ? ?8 +++++++ >>> ?arch/arm/kvm/arm.c ? ? ? ? ? ?| ? ?2 + >>> ?arch/arm/kvm/interrupts.S ? ? | ? 43 +++++++++++++++++++++++++++++++++++++++++ >>> ?3 files changed, 53 insertions(+), 0 deletions(-) >>> >>> diff --git a/arch/arm/kernel/asm-offsets.c b/arch/arm/kernel/asm-offsets.c >>> index d5f4ddf..039ef64 100644 >>> --- a/arch/arm/kernel/asm-offsets.c >>> +++ b/arch/arm/kernel/asm-offsets.c >>> @@ -184,6 +184,14 @@ int main(void) >>> ? DEFINE(VCPU_VGIC_APR, ? ? ? ? ? ? ? ?offsetof(struct kvm_vcpu, arch.vgic_cpu.vgic_apr)); >>> ? DEFINE(VCPU_VGIC_LR, ? ? ? ? offsetof(struct kvm_vcpu, arch.vgic_cpu.vgic_lr)); >>> ? DEFINE(VCPU_VGIC_NR_LR, ? ? ?offsetof(struct kvm_vcpu, arch.vgic_cpu.nr_lr)); >>> +#ifdef CONFIG_KVM_ARM_TIMER >>> + ?DEFINE(VCPU_TIMER_CNTV_CTL, ?offsetof(struct kvm_vcpu, arch.timer_cpu.cntv_ctl)); >>> + ?DEFINE(VCPU_TIMER_CNTV_CVALH, ? ? ? ?offsetof(struct kvm_vcpu, arch.timer_cpu.cntv_cval_high)); >>> + ?DEFINE(VCPU_TIMER_CNTV_CVALL, ? ? ? ?offsetof(struct kvm_vcpu, arch.timer_cpu.cntv_cval_low)); >>> + ?DEFINE(KVM_TIMER_CNTVOFF_H, ?offsetof(struct kvm, arch.timer.cntvoff32.cntvoff_high)); >>> + ?DEFINE(KVM_TIMER_CNTVOFF_L, ?offsetof(struct kvm, arch.timer.cntvoff32.cntvoff_low)); >>> + ?DEFINE(KVM_TIMER_ENABLED, ? ?offsetof(struct kvm, arch.timer.enabled)); >>> +#endif >>> ? DEFINE(KVM_VGIC_VCTRL, ? ? ? offsetof(struct kvm, arch.vgic.vctrl_base)); >>> ?#endif >>> ? DEFINE(KVM_VTTBR, ? ? ? ? ? ?offsetof(struct kvm, arch.vttbr)); >>> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c >>> index 86eca66..ccb2c08 100644 >>> --- a/arch/arm/kvm/arm.c >>> +++ b/arch/arm/kvm/arm.c >>> @@ -511,9 +511,11 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run) >>> ? ? ? ? ? ? ? ?kvm_guest_enter(); >>> ? ? ? ? ? ? ? ?vcpu->mode = IN_GUEST_MODE; >>> ? ? ? ? ? ? ? ?kvm_vgic_sync_to_cpu(vcpu); >>> + ? ? ? ? ? ? ? kvm_timer_sync_to_cpu(vcpu); >>> >>> ? ? ? ? ? ? ? ?ret = __kvm_vcpu_run(vcpu); >>> >>> + ? ? ? ? ? ? ? kvm_timer_sync_from_cpu(vcpu); >>> ? ? ? ? ? ? ? ?kvm_vgic_sync_from_cpu(vcpu); >>> ? ? ? ? ? ? ? ?vcpu->mode = OUTSIDE_GUEST_MODE; >>> ? ? ? ? ? ? ? ?vcpu->stat.exits++; >>> diff --git a/arch/arm/kvm/interrupts.S b/arch/arm/kvm/interrupts.S >>> index 56a65c4..1481d44 100644 >>> --- a/arch/arm/kvm/interrupts.S >>> +++ b/arch/arm/kvm/interrupts.S >>> @@ -312,10 +312,33 @@ ENTRY(__kvm_flush_vm_context) >>> ?#endif >>> ?.endm >>> >>> +#ifdef CONFIG_KVM_ARM_TIMER >>> +#define PL1P_ENABLE ? ?3 >>> +#define PL1P_DISABLE ? 2 >>> +#else >>> ?#define PL1P_ENABLE ? ?3 >>> ?#define PL1P_DISABLE ? 3 >>> +#endif >>> >>> ?.macro save_timer_state ? ? ? ?vcpup >>> +#ifdef CONFIG_KVM_ARM_TIMER >>> + ? ? ? ldr ? ? r4, [\vcpup, #VCPU_KVM] >>> + ? ? ? ldr ? ? r2, [r4, #KVM_TIMER_ENABLED] >>> + ? ? ? cmp ? ? r2, #0 >>> + ? ? ? beq ? ? 1f >>> + >>> + ? ? ? mrc ? ? p15, 0, r2, c14, c3, 1 ?@ CNTV_CTL >>> + ? ? ? and ? ? r2, #3 >>> + ? ? ? str ? ? r2, [\vcpup, #VCPU_TIMER_CNTV_CTL] >>> + ? ? ? bic ? ? r2, #1 ? ? ? ? ? ? ? ? ?@ Clear ENABLE >>> + ? ? ? mcr ? ? p15, 0, r2, c14, c3, 1 ?@ CNTV_CTL >>> + >>> + ? ? ? mrrc ? ?p15, 3, r2, r3, c14 ? ? @ CNTV_CVAL >>> + ? ? ? str ? ? r3, [\vcpup, #VCPU_TIMER_CNTV_CVALH] >>> + ? ? ? str ? ? r2, [\vcpup, #VCPU_TIMER_CNTV_CVALL] >>> + >>> +1: >>> +#endif >>> ? ? ? ?@ Allow physical timer access for the host >>> ? ? ? ?mrc ? ? p15, 4, r2, c14, c1, 0 ?@ CNTHCTL >>> ? ? ? ?orr ? ? r2, r2, #PL1P_ENABLE >>> @@ -328,6 +351,26 @@ ENTRY(__kvm_flush_vm_context) >>> ? ? ? ?bic ? ? r2, r2, #PL1P_DISABLE >>> ? ? ? ?mcr ? ? p15, 4, r2, c14, c1, 0 ?@ CNTHCTL >>> >>> +#ifdef CONFIG_KVM_ARM_TIMER >>> + ? ? ? ldr ? ? r4, [\vcpup, #VCPU_KVM] >>> + ? ? ? ldr ? ? r2, [r4, #KVM_TIMER_ENABLED] >>> + ? ? ? cmp ? ? r2, #0 >>> + ? ? ? beq ? ? 1f >>> + >>> + ? ? ? ldr ? ? r3, [\vcpup, #VCPU_TIMER_CNTV_CVALH] >>> + ? ? ? ldr ? ? r2, [\vcpup, #VCPU_TIMER_CNTV_CVALL] >>> + ? ? ? mcrr ? ?p15, 3, r2, r3, c14 ? ? @ CNTV_CVAL >>> + >>> + ? ? ? ldr ? ? r3, [r4, #KVM_TIMER_CNTVOFF_H] >>> + ? ? ? ldr ? ? r2, [r4, #KVM_TIMER_CNTVOFF_L] >>> + ? ? ? mcrr ? ?p15, 4, r2, r3, c14 ? ? @ CNTVOFF >> >> still not quite understanding where we adjust this or why we don't. If >> this is effectively zero do we have to bother with it on every >> world-switch and could this be documented somewhere? > > See my earlier reply. This works around a model bug, hopefully a > temporary situation. > >>> + ? ? ? isb >>> + >>> + ? ? ? ldr ? ? r2, [\vcpup, #VCPU_TIMER_CNTV_CTL] >>> + ? ? ? and ? ? r2, #3 >> >> Are we clearing the enable and mask bits here, because in any case we >> offloaded a remaining timer from last time around to a host timer? > > We're actually clearing the ISTATUS bit. Probably not strictly > necessary, and may well be a debugging leftover. > and, not bic, got it. thanks.
