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

[Sean Christopherson <seanjc@google.com>]

On Tue, Apr 19, 2022, Vitaly Kuznetsov wrote:
> Sean Christopherson <seanjc@google.com> writes:
> 
> > On Tue, Apr 19, 2022, Vitaly Kuznetsov wrote:
> >> Sean Christopherson <seanjc@google.com> writes:
> >> > 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?

...

> >> (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;
> > +       }
> 
> Looks good for cpu_tsc_khz but I'm not particularly sure about
> kvm_max_guest_tsc_khz.

Doh, ignore that, I got kvm_max_guest_tsc_khz confused with max_tsc_khz.