On Thu, Apr 28, 2011 at 02:59:57AM -0400, Zachary Amsden wrote:
> So I've been going over the new code changes to the TSC related code and
> I don't like one particular set of changes. In particular, here:
>
> kvm_x86_ops->vcpu_load(vcpu, cpu);
> if (unlikely(vcpu->cpu != cpu) || check_tsc_unstable()) {
> /* Make sure TSC doesn't go backwards */
> s64 tsc_delta;
> u64 tsc;
>
> kvm_get_msr(vcpu, MSR_IA32_TSC, &tsc);
> tsc_delta = !vcpu->arch.last_guest_tsc ? 0 :
> tsc - vcpu->arch.last_guest_tsc;
>
> if (tsc_delta < 0)
> mark_tsc_unstable("KVM discovered backwards TSC");
> if (check_tsc_unstable()) {
> kvm_x86_ops->adjust_tsc_offset(vcpu, -tsc_delta);
> vcpu->arch.tsc_catchup = 1;
> }
>
>
> The point of this code fragment is to test the host clock to see if it
> is stable, because we may have just come back from an idle phase which
> stopped the TSC, switched CPUs, or come back from a deep sleep state
> which reset the host TSC.
I see it different. This code wants to check if the _guest_ tsc moves
forwared (or at least not backwards). So it is fully legitimate to just
do this by reading the guest-tsc and compare it to the last one the
guest had.
> I saw a patch floating around that touched this code recently, but I
> think there's a definite issue here that needs addressing.
In fact, this change was done to address one of your concerns. You
mentioned that the values passed to adjust_tsc_offset() were in
unconsistent units in my first version of tsc-scaling. This was a right
objection because one call-site used guest-tsc-units while the other
used host-tsc-units. This change intended to fix that by using
guest-tsc-units always for adjust_tsc_offset().
Not that the guest and the host tsc have the same units on current
machines. But with tsc-scaling these units are different.
Regards,
Joerg
--
AMD Operating System Research Center
Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632
--
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
