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: Tue, 13 Dec 2016 16:43:34 +0100 Message-ID: <20161213154334.GC2293@potion> References: <20161202150632.GA22204@potion> <59a3f1a2-5b9e-2ca7-5285-1469fef40f42@redhat.com> <20161206110858.GC8660@potion> <22b615ad-9161-2fef-4d17-885c33b0ac76@redhat.com> <20161207152520.GA15611@potion> <23190243-47f6-ebb9-279d-c4db9f58c6ef@redhat.com> <20161208143206.GA22892@potion> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: David Hildenbrand , Paolo Bonzini , kvm@vger.kernel.org To: Don Bowman Return-path: Received: from mx1.redhat.com ([209.132.183.28]:51554 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933726AbcLMPnt (ORCPT ); Tue, 13 Dec 2016 10:43:49 -0500 Content-Disposition: inline In-Reply-To: Sender: kvm-owner@vger.kernel.org List-ID: 2016-12-09 10:12-0500, Don Bowman: > OK, based on previous feedback, this patch version simply ignores any > inconsistency if a knowing and trusting user wishes. > > In my case, two identical processors in stepping and version and all > others have 1 flag missing (retail vs tray version of chip), but the > next person might have another flag. > > Comments? > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > index 5382b82..264870e 100644 > --- 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 ignore_inconsistency = false; > +module_param(ignore_inconsistency, bool, S_IRUGO); > + > static u64 __read_mostly host_xss; > > static bool __read_mostly enable_pml = 1; > @@ -3449,6 +3452,7 @@ 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; > + vmcs_conf->cpu_based_2nd_exec_ctrl &= ~SECONDARY_EXEC_TSC_SCALING; This would disable TSC_SCALING for everyone. You need a second patch that makes tsc_scaling optional. > vmcs_conf->vmexit_ctrl = _vmexit_control; > vmcs_conf->vmentry_ctrl = _vmentry_control; > > @@ -9202,9 +9206,11 @@ static void __init vmx_check_processor_compat(void *rtn) > if (setup_vmcs_config(&vmcs_conf) < 0) > *(int *)rtn = -EIO; > if (memcmp(&vmcs_config, &vmcs_conf, sizeof(struct vmcs_config)) != 0) { > - printk(KERN_ERR "kvm: CPU %d feature inconsistency!\n", > - smp_processor_id()); > - *(int *)rtn = -EIO; > + printk(KERN_ERR "kvm: CPU %d feature inconsistency%s!\n", > + smp_processor_id(), > + ignore_inconsistency ? " -- ignored" : ""); > + if (!ignore_inconsistency) > + *(int *)rtn = -EIO; Please add a note in the spirit of: "KVM is going to fail unless you explicitly disable features that are not present on all CPUs." If this becomes a normal feature, we'd remove the toggle, check that all enabled features are supported, and pass if KVM will work with selected features ... the check seems like a waste of code for this rarity of machines. Paolo, are you ok with the toggle? (We can just make it the default without any significant issues.) Thanks.