Re: [PATCH 04/12] KVM: x86: Replace call-back set_tsc_khz() with a common function

[Haozhong Zhang]

On Mon, Sep 28, 2015 at 08:27:02PM -0700, Eric Northup wrote:
> On Sun, Sep 27, 2015 at 10:38 PM, Haozhong Zhang <haozhong.zhang@xxxxxxxxx>
> wrote:
> 
> > Both VMX and SVM propagate virtual_tsc_khz in the same way, so this
> > patch removes the call-back set_tsc_khz() and replaces it with a common
> > function.
> >
> > Signed-off-by: Haozhong Zhang <haozhong.zhang@xxxxxxxxx>
> > ---
> >  arch/x86/include/asm/kvm_host.h |  1 -
> >  arch/x86/kvm/svm.c              | 36 ------------------------------------
> >  arch/x86/kvm/vmx.c              | 17 -----------------
> >  arch/x86/kvm/x86.c              | 41
> > ++++++++++++++++++++++++++++++++++++++++-
> >  4 files changed, 40 insertions(+), 55 deletions(-)
> >
> > diff --git a/arch/x86/include/asm/kvm_host.h
> > b/arch/x86/include/asm/kvm_host.h
> > index 4f32c68..5a0c435 100644
> > --- a/arch/x86/include/asm/kvm_host.h
> > +++ b/arch/x86/include/asm/kvm_host.h
> > @@ -842,7 +842,6 @@ struct kvm_x86_ops {
> >
> >         bool (*has_wbinvd_exit)(void);
> >
> > -       void (*set_tsc_khz)(struct kvm_vcpu *vcpu, u32 user_tsc_khz, bool
> > scale);
> >         u64 (*read_tsc_offset)(struct kvm_vcpu *vcpu);
> >         void (*write_tsc_offset)(struct kvm_vcpu *vcpu, u64 offset);
> >
> > diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> > index 1a333bd..d46dcf3 100644
> > --- a/arch/x86/kvm/svm.c
> > +++ b/arch/x86/kvm/svm.c
> > @@ -1015,41 +1015,6 @@ static void init_sys_seg(struct vmcb_seg *seg,
> > uint32_t type)
> >         seg->base = 0;
> >  }
> >
> > -static void svm_set_tsc_khz(struct kvm_vcpu *vcpu, u32 user_tsc_khz, bool
> > scale)
> > -{
> > -       u64 ratio;
> > -       u64 khz;
> > -
> > -       /* Guest TSC same frequency as host TSC? */
> > -       if (!scale) {
> > -               vcpu->arch.tsc_scaling_ratio = TSC_RATIO_DEFAULT;
> > -               return;
> > -       }
> > -
> > -       /* TSC scaling supported? */
> > -       if (!boot_cpu_has(X86_FEATURE_TSCRATEMSR)) {
> > -               if (user_tsc_khz > tsc_khz) {
> > -                       vcpu->arch.tsc_catchup = 1;
> > -                       vcpu->arch.tsc_always_catchup = 1;
> > -               } else
> > -                       WARN(1, "user requested TSC rate below hardware
> > speed\n");
> > -               return;
> > -       }
> > -
> > -       khz = user_tsc_khz;
> > -
> > -       /* TSC scaling required  - calculate ratio */
> > -       ratio = khz << 32;
> > -       do_div(ratio, tsc_khz);
> > -
> > -       if (ratio == 0 || ratio & TSC_RATIO_RSVD) {
> > -               WARN_ONCE(1, "Invalid TSC ratio - virtual-tsc-khz=%u\n",
> > -                               user_tsc_khz);
> > -               return;
> > -       }
> > -       vcpu->arch.tsc_scaling_ratio = ratio;
> > -}
> > -
> >  static u64 svm_read_tsc_offset(struct kvm_vcpu *vcpu)
> >  {
> >         struct vcpu_svm *svm = to_svm(vcpu);
> > @@ -4507,7 +4472,6 @@ static struct kvm_x86_ops svm_x86_ops = {
> >
> >         .has_wbinvd_exit = svm_has_wbinvd_exit,
> >
> > -       .set_tsc_khz = svm_set_tsc_khz,
> >         .read_tsc_offset = svm_read_tsc_offset,
> >         .write_tsc_offset = svm_write_tsc_offset,
> >         .adjust_tsc_offset = svm_adjust_tsc_offset,
> > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> > index 6407674..1751537 100644
> > --- a/arch/x86/kvm/vmx.c
> > +++ b/arch/x86/kvm/vmx.c
> > @@ -2255,22 +2255,6 @@ static u64 vmx_read_l1_tsc(struct kvm_vcpu *vcpu,
> > u64 host_tsc)
> >         return host_tsc + tsc_offset;
> >  }
> >
> > -/*
> > - * Engage any workarounds for mis-matched TSC rates.  Currently limited to
> > - * software catchup for faster rates on slower CPUs.
> > - */
> > -static void vmx_set_tsc_khz(struct kvm_vcpu *vcpu, u32 user_tsc_khz, bool
> > scale)
> > -{
> > -       if (!scale)
> > -               return;
> > -
> > -       if (user_tsc_khz > tsc_khz) {
> > -               vcpu->arch.tsc_catchup = 1;
> > -               vcpu->arch.tsc_always_catchup = 1;
> > -       } else
> > -               WARN(1, "user requested TSC rate below hardware speed\n");
> > -}
> > -
> >  static u64 vmx_read_tsc_offset(struct kvm_vcpu *vcpu)
> >  {
> >         return vmcs_read64(TSC_OFFSET);
> > @@ -10380,7 +10364,6 @@ static struct kvm_x86_ops vmx_x86_ops = {
> >
> >         .has_wbinvd_exit = cpu_has_vmx_wbinvd_exit,
> >
> > -       .set_tsc_khz = vmx_set_tsc_khz,
> >         .read_tsc_offset = vmx_read_tsc_offset,
> >         .write_tsc_offset = vmx_write_tsc_offset,
> >         .adjust_tsc_offset = vmx_adjust_tsc_offset,
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index 920c302..e2e1fdb 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -1248,6 +1248,45 @@ static u32 adjust_tsc_khz(u32 khz, s32 ppm)
> >         return v;
> >  }
> >
> > +static void set_tsc_khz(struct kvm_vcpu *vcpu, u32 user_tsc_khz, bool
> > scale)
> > +{
> > +       u64 ratio, khz;
> > +       s8 shift;
> > +
> > +       /* Guest TSC same frequency as host TSC? */
> > +       if (!scale) {
> > +               vcpu->arch.tsc_scaling_ratio =
> > kvm_default_tsc_scaling_ratio;
> > +               return;
> > +       }
> > +
> > +       /* TSC scaling supported? */
> > +       if (!kvm_has_tsc_control) {
> > +               if (user_tsc_khz > tsc_khz) {
> > +                       vcpu->arch.tsc_catchup = 1;
> > +                       vcpu->arch.tsc_always_catchup = 1;
> > +               } else
> > +                       WARN(1, "user requested TSC rate below hardware
> > speed\n");
> >
> 
> It was like this before, but why should KVM_SET_TSC_KHZ ioctl have return
> value of 0 in this case?  Failing the request would be better than kernel
> log spew (and below).
> 
>

Yes, failing the request is a better way. I'll change this and below.

> > +               return;
> > +       }
> > +
> > +       khz = user_tsc_khz;
> > +
> > +       /* TSC scaling required  - calculate ratio */
> > +       shift = (kvm_tsc_scaling_ratio_frac_bits <= 32) ?
> > +               kvm_tsc_scaling_ratio_frac_bits : 32;
> > +       ratio = khz << shift;
> > +       do_div(ratio, tsc_khz);
> > +       ratio <<= (kvm_tsc_scaling_ratio_frac_bits - shift);
> > +
> > +       if (ratio == 0 || ratio & kvm_tsc_scaling_ratio_rsvd) {
> > +               WARN_ONCE(1, "Invalid TSC scaling ratio -
> > virtual-tsc-khz=%u\n",
> > +                         user_tsc_khz);
> > +               return;
> > +       }
> > +
> > +       vcpu->arch.tsc_scaling_ratio = ratio;
> > +}
> > +
> >  static void kvm_set_tsc_khz(struct kvm_vcpu *vcpu, u32 this_tsc_khz)
> >  {
> >         u32 thresh_lo, thresh_hi;
> > @@ -1275,7 +1314,7 @@ static void kvm_set_tsc_khz(struct kvm_vcpu *vcpu,
> > u32 this_tsc_khz)
> >                 pr_debug("kvm: requested TSC rate %u falls outside
> > tolerance [%u,%u]\n", this_tsc_khz, thresh_lo, thresh_hi);
> >                 use_scaling = 1;
> >         }
> > -       kvm_x86_ops->set_tsc_khz(vcpu, this_tsc_khz, use_scaling);
> > +       set_tsc_khz(vcpu, this_tsc_khz, use_scaling);
> >  }
> >
> >  static u64 compute_guest_tsc(struct kvm_vcpu *vcpu, s64 kernel_ns)
> > --
> > 2.4.8
> >
> > --
> > 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
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/