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@xxxxxxxxxx>
> > ---
> > 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;
> > }
> >
> >
>
> 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
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
