Initialize the cntvoff at vcpu_init time, not before running the VCPUs at the first time because that will overwrite any potentially restored values from user space.
Signed-off-by: Christoffer Dall christoffer.dall@linaro.org --- virt/kvm/arm/arch_timer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c index 8168437..2f6b5fe 100644 --- a/virt/kvm/arm/arch_timer.c +++ b/virt/kvm/arm/arch_timer.c @@ -181,6 +181,7 @@ void kvm_timer_vcpu_init(struct kvm_vcpu *vcpu) INIT_WORK(&timer->expired, kvm_timer_inject_irq_work); hrtimer_init(&timer->timer, CLOCK_MONOTONIC, HRTIMER_MODE_ABS); timer->timer.function = kvm_timer_expire; + vcpu->kvm->arch.timer.cntvoff = kvm_phys_timer_read(); }
static void kvm_timer_init_interrupt(void *info) @@ -282,7 +283,6 @@ void kvm_timer_vcpu_terminate(struct kvm_vcpu *vcpu) int kvm_timer_init(struct kvm *kvm) { if (timecounter && wqueue) { - kvm->arch.timer.cntvoff = kvm_phys_timer_read(); kvm->arch.timer.enabled = 1; }
Hi Christoffer,
On 28/09/13 15:05, Christoffer Dall wrote:
Initialize the cntvoff at vcpu_init time, not before running the VCPUs at the first time because that will overwrite any potentially restored values from user space.
Signed-off-by: Christoffer Dall christoffer.dall@linaro.org
virt/kvm/arm/arch_timer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c index 8168437..2f6b5fe 100644 --- a/virt/kvm/arm/arch_timer.c +++ b/virt/kvm/arm/arch_timer.c @@ -181,6 +181,7 @@ void kvm_timer_vcpu_init(struct kvm_vcpu *vcpu) INIT_WORK(&timer->expired, kvm_timer_inject_irq_work); hrtimer_init(&timer->timer, CLOCK_MONOTONIC, HRTIMER_MODE_ABS); timer->timer.function = kvm_timer_expire;
- vcpu->kvm->arch.timer.cntvoff = kvm_phys_timer_read();
} static void kvm_timer_init_interrupt(void *info) @@ -282,7 +283,6 @@ void kvm_timer_vcpu_terminate(struct kvm_vcpu *vcpu) int kvm_timer_init(struct kvm *kvm) { if (timecounter && wqueue) {
kvm->arch.timer.cntvoff = kvm_phys_timer_read();
I'm not exactly fond of this change. CNTVOFF is a per-VM property. Here, we end-up recomputing it as we initialize VCPUs, which is very counter-intuitive.
If anything, I'd move the call of kvm_timer_init out of kvm_vgic_init into kvm_arch_init_vm.
What do you think?
M.
On Mon, Sep 30, 2013 at 08:41:06AM +0100, Marc Zyngier wrote:
Hi Christoffer,
On 28/09/13 15:05, Christoffer Dall wrote:
Initialize the cntvoff at vcpu_init time, not before running the VCPUs at the first time because that will overwrite any potentially restored values from user space.
Signed-off-by: Christoffer Dall christoffer.dall@linaro.org
virt/kvm/arm/arch_timer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c index 8168437..2f6b5fe 100644 --- a/virt/kvm/arm/arch_timer.c +++ b/virt/kvm/arm/arch_timer.c @@ -181,6 +181,7 @@ void kvm_timer_vcpu_init(struct kvm_vcpu *vcpu) INIT_WORK(&timer->expired, kvm_timer_inject_irq_work); hrtimer_init(&timer->timer, CLOCK_MONOTONIC, HRTIMER_MODE_ABS); timer->timer.function = kvm_timer_expire;
- vcpu->kvm->arch.timer.cntvoff = kvm_phys_timer_read();
} static void kvm_timer_init_interrupt(void *info) @@ -282,7 +283,6 @@ void kvm_timer_vcpu_terminate(struct kvm_vcpu *vcpu) int kvm_timer_init(struct kvm *kvm) { if (timecounter && wqueue) {
kvm->arch.timer.cntvoff = kvm_phys_timer_read();I'm not exactly fond of this change. CNTVOFF is a per-VM property. Here, we end-up recomputing it as we initialize VCPUs, which is very counter-intuitive.
If anything, I'd move the call of kvm_timer_init out of kvm_vgic_init into kvm_arch_init_vm.
What do you think?
I think as for the anything part, we need to move it, otherwise migration won't work.
Calling this from kvm_arch_init_vm sounds much cleaner actually, so I'll do that.
Thanks, -Christoffer
linaro-kernel@lists.linaro.org
-
Christoffer Dall -
Marc Zyngier