Re: [PATCH] KVM: x86: Use current rather than snapshotted TSC frequency if it is constant

[Sean Christopherson <seanjc@xxxxxxxxxx>]

On Tue, Apr 19, 2022, Vitaly Kuznetsov wrote:
> Sean Christopherson <seanjc@xxxxxxxxxx> writes:
> 
> > +Vitaly
> >
> > On Thu, Apr 14, 2022, Anton Romanov wrote:
> 
> ...
> 
> >> @@ -8646,9 +8659,12 @@ static void tsc_khz_changed(void *data)
> >>  	struct cpufreq_freqs *freq = data;
> >>  	unsigned long khz = 0;
> >>  
> >> +	if (boot_cpu_has(X86_FEATURE_CONSTANT_TSC))
> >> +		return;
> >
> > Vitaly,
> >
> > The Hyper-V guest code also sets cpu_tsc_khz, should we WARN if that notifier is
> > invoked and Hyper-V told us there's a constant TSC?
> >
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index ab336f7c82e4..ca8e20f5ffc0 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -8701,6 +8701,8 @@ static void kvm_hyperv_tsc_notifier(void)
> >         struct kvm *kvm;
> >         int cpu;
> >
> > +       WARN_ON_ONCE(boot_cpu_has(X86_FEATURE_CONSTANT_TSC));
> > +
> >         mutex_lock(&kvm_lock);
> >         list_for_each_entry(kvm, &vm_list, vm_list)
> >                 kvm_make_mclock_inprogress_request(kvm);
> >
> 
> (apologies for the delayed reply)
> 
> No, I think Hyper-V's "Reenlightenment" feature overrides (re-defines?)
> X86_FEATURE_CONSTANT_TSC. E.g. I've checked a VM on E5-2667 v4
> (Broadwell) CPU with no TSC scaling. This VM has 'constant_tsc' and will
> certainly get reenlightenment irq on migration.

Ooh, so that a VM with a constant TSC be live migrated to another system with a
constant, but different, TSC.  Does the below look correct as fixup for this patch?

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index ab336f7c82e4..a944e4ba5532 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -8708,10 +8708,18 @@ static void kvm_hyperv_tsc_notifier(void)
        /* no guest entries from this point */
        hyperv_stop_tsc_emulation();

-       /* TSC frequency always matches when on Hyper-V */
-       for_each_present_cpu(cpu)
-               per_cpu(cpu_tsc_khz, cpu) = tsc_khz;
-       kvm_max_guest_tsc_khz = tsc_khz;
+       /*
+        * TSC frequency always matches when on Hyper-V.  Skip the updates if
+        * the TSC is "officially" constant, in which case KVM doesn't use the
+        * per-CPU and max variables.  Note, the notifier can still fire with
+        * a constant TSC, e.g. if this VM (KVM is a Hyper-V guest) is migrated
+        * to a system with a different TSC frequency.
+        */
+       if (!boot_cpu_has(X86_FEATURE_CONSTANT_TSC)) {
+               for_each_present_cpu(cpu)
+                       per_cpu(cpu_tsc_khz, cpu) = tsc_khz;
+               kvm_max_guest_tsc_khz = tsc_khz;
+       }

        list_for_each_entry(kvm, &vm_list, vm_list) {
                __kvm_start_pvclock_update(kvm);