[Android-virt] [PATCH 09/10] ARM: KVM: arch_timers: Add timer world switch

[marc.zyngier at arm.com (Marc Zyngier)]

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.

>> +       mcr     p15, 0, r2, c14, c3, 1  @ CNTV_CTL
>> +1:
>> +#endif
>>        isb
>>  .endm

	M.
-- 
Jazz is not dead. It just smells funny...