> From: Marcelo Tosatti
> Sent: Tuesday, August 23, 2011 6:47 PM
>
> > >+ if (!apic->lapic_timer.tscdeadline)
> > >+ return;
> > >+
> > >+ tsc_target = kvm_x86_ops->
> > >+ guest_to_host_tsc(apic->lapic_timer.tscdeadline);
> > >+ rdtscll(tsc_now);
> > >+ tsc_delta = tsc_target - tsc_now;
> >
> > This only works if we have a constant tsc, that's not true for large
that type of machine exposes tricky issues to time virtualization in
general.
> > multiboard machines. Need to do this with irqs disabled as well
> > (reading both 'now' and 'tsc_now' in the same critical section).
>
> Should look like this:
>
> local_irq_disable();
> u64 guest_tsc = kvm_x86_ops->read_l1_tsc(vcpu);
> if (guest_tsc <= tscdeadline)
> hrtimer_start(now);
> else {
> ns = convert_to_ns(guest_tsc - tscdeadline);
> hrtimer_start(now + ns);
> }
> local_irq_enable();
Above is an overkill. only calculating guest tsc delta needs be protected.
On the other hand, I don't think masking irq here adds obvious benefit.
Virtualization doesn't ensure micro-level time accuracy, since there're
many events delaying virtual time interrupt injection even when timer
emulation logic triggers it timely. That's even the case on bare metal
though with smaller scope, and thus most guests are able to handle
such situation. masking irq in non-critical regions simply slow down the
system response on other events.
>
> Note the vcpus tsc can have different frequency than the hosts, so
> vcpu_tsc_khz() should be used to convert to nanoseconds, not tsc_khz.
>
yes, that's a good suggestion.
btw, Jinsong is on vacation this week. He will update the patch according
to you comment when back.
Thanks
Kevin
--
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
