| Message ID | 1443418691-24050-5-git-send-email-haozhong.zhang@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@intel.com> > 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@intel.com> > > --- > > 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@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
2015-09-28 13:38+0800, Haozhong Zhang: > 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@intel.com> > --- > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > +static void set_tsc_khz(struct kvm_vcpu *vcpu, u32 user_tsc_khz, bool scale) > +{ > + u64 ratio, khz; | [...] > + khz = user_tsc_khz; I'd use "user_tsc_khz" directly. > + /* 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); VMX is losing 16 bits by this operation; normal fixed point division could get us a smaller drift (and an one-liner here) ... at 4.3 GHz, 32 instead of 48 bits after decimal point translate to one "lost" TSC tick per second, in the worst case. Please mention that we are truncating on purpose :) -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Oct 5, 2015 at 12:53 PM, Radim Kr?má? <rkrcmar@redhat.com> wrote: > 2015-09-28 13:38+0800, Haozhong Zhang: >> 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@intel.com> >> --- >> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c >> +static void set_tsc_khz(struct kvm_vcpu *vcpu, u32 user_tsc_khz, bool scale) >> +{ >> + u64 ratio, khz; > | [...] >> + khz = user_tsc_khz; > > I'd use "user_tsc_khz" directly. > >> + /* 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); > > VMX is losing 16 bits by this operation; normal fixed point division > could get us a smaller drift (and an one-liner here) ... > at 4.3 GHz, 32 instead of 48 bits after decimal point translate to one > "lost" TSC tick per second, in the worst case. We can easily avoid losing precision on x86_64 (divq allows a 128-bit dividend). 32-bit can just lose the 16 bits of precision (TSC scaling is only available on SkyLake, and I'd be surprised if there were many hosts running KVM in protected mode on SkyLake :)). > > Please mention that we are truncating on purpose :) > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Oct 05, 2015 at 09:53:26PM +0200, Radim Kr?má? wrote: > 2015-09-28 13:38+0800, Haozhong Zhang: > > 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@intel.com> > > --- > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > > +static void set_tsc_khz(struct kvm_vcpu *vcpu, u32 user_tsc_khz, bool scale) > > +{ > > + u64 ratio, khz; > | [...] > > + khz = user_tsc_khz; > > I'd use "user_tsc_khz" directly. > I'll do so. > > + /* 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); > > VMX is losing 16 bits by this operation; normal fixed point division > could get us a smaller drift (and an one-liner here) ... > at 4.3 GHz, 32 instead of 48 bits after decimal point translate to one > "lost" TSC tick per second, in the worst case. > > Please mention that we are truncating on purpose :) It's intentional to avoid the potential overflow in khz << kvm_tsc_scaling_ratio_frac_bits. For VMX where kvm_tsc_scaling_ratio_frac_bits == 48, the above expression is only safe to left shift a pretty small khz (< 2^16 KHz or 65.5 MHz). Thus, I decided to sacrifice the precision for safety. I chose to truncate at the boundary of 32 bits which can handle khz as large as 4294 GHz. Though this truncation results in losing TSC ticks when khz is larger than 4.3 GHz, the lost is however pretty small compared with the large khz. Alternatively, it's also possible to follow David's comment to use divq on x86_64 to keep both precision and safety. On i386, it just falls back to above truncating approach. - Haozhong -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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"); + 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)
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@intel.com> --- 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(-)