From mboxrd@z Thu Jan 1 00:00:00 1970 From: Radim =?utf-8?B?S3LEjW3DocWZ?= Subject: Re: PATCH: setup_vmcs_config: disable TSC scaling on unlike processors Date: Fri, 2 Dec 2016 16:07:56 +0100 Message-ID: <20161202150632.GA22204@potion> References: Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: kvm@vger.kernel.org, pbonzini@redhat.com To: Don Bowman Return-path: Received: from mx1.redhat.com ([209.132.183.28]:56670 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756736AbcLBPX7 (ORCPT ); Fri, 2 Dec 2016 10:23:59 -0500 Content-Disposition: inline In-Reply-To: Sender: kvm-owner@vger.kernel.org List-ID: Subjects that touch only arch/x86/kvm/vmx.c are usually prefixed with "[PATCH] KVM: VMX:". 2016-12-01 21:32-0500, Don Bowman: > Add an enable_tsc_scaling parameter to kvm > > Signed-off-by: Don Bowman > --- > > My system has what i thought were two identical processors (same > stepping ID etc). > However, bafflingly, one of them has the ability to do TSC scaling, > and one does not (as reported in the vmcs). Wow, the chip doesn't sound trustworthy. > This in turn causes kvm to give up entirely. > > I feel a better solution is to mask off this capability on the one processor. I like the global toggle better -- it is less code with more uses. > If enable_tsc_scaling is set false, the feature is ignored, and all > processors end up matching each other, enabling acceleration. > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > @@ -103,6 +103,9 @@ module_param_named(enable_shadow_vmcs, > enable_shadow_vmcs, bool, S_IRUGO); > static bool __read_mostly nested = 0; > module_param(nested, bool, S_IRUGO); > > +static bool __read_mostly enable_tsc_scaling = true; > +module_param(enable_tsc_scaling, bool, S_IRUGO); The "enable_" prefix is not needed, please do module_param_named(tsc_scaling, ...) > + > static u64 __read_mostly host_xss; > > static bool __read_mostly enable_pml = 1; > @@ -3449,6 +3452,8 @@ static __init int setup_vmcs_config(struct > vmcs_config *vmcs_conf) > vmcs_conf->pin_based_exec_ctrl = _pin_based_exec_control; > vmcs_conf->cpu_based_exec_ctrl = _cpu_based_exec_control; > vmcs_conf->cpu_based_2nd_exec_ctrl = _cpu_based_2nd_exec_control; > + if (!enable_tsc_scaling) > + vmcs_conf->cpu_based_2nd_exec_ctrl &= ~SECONDARY_EXEC_TSC_SCALING; Move this to vmx_secondary_exec_control(), where we disable all other features. You'll notice kvm_has_tsc_control in hardware_setup() when clearing the flag if it isn't supported by hardware, so I think your change would make sense as a x86 kvm module option based on kvm_has_tsc_control. Thanks.