From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751736AbdJCJmZ (ORCPT ); Tue, 3 Oct 2017 05:42:25 -0400 Received: from mail-wm0-f41.google.com ([74.125.82.41]:55181 "EHLO mail-wm0-f41.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751450AbdJCJmX (ORCPT ); Tue, 3 Oct 2017 05:42:23 -0400 X-Google-Smtp-Source: AOwi7QAlPla/l4STuoOAx/yDsS4Ihpjn+z6BMAKTHv2XSdHuQDgVqo+WFQTjElw0oD4pbePGkb5iww== Subject: Re: [PATCH] KVM: VMX: check match table To: Nick Desaulniers Cc: Josh Triplett , kay@vrfy.org, avi@redhat.com, Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" , x86@kernel.org, =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= , linux-kernel@vger.kernel.org, kvm@vger.kernel.org References: <20170926042540.10100-1-nick.desaulniers@gmail.com> <20170926171219.ulnknakiohm2whpd@x> <498ed7dc-41b2-a075-cdce-e63a80842d39@redhat.com> <20170930232234.ue665ojgjkegjpag@lostoracle.net> From: Paolo Bonzini Message-ID: <28a852ce-9fa4-0b41-6867-1659849a8037@redhat.com> Date: Tue, 3 Oct 2017 11:42:18 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 MIME-Version: 1.0 In-Reply-To: <20170930232234.ue665ojgjkegjpag@lostoracle.net> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 01/10/2017 01:22, Nick Desaulniers wrote: > On Wed, Sep 27, 2017 at 02:36:03PM +0200, Paolo Bonzini wrote: >> On 26/09/2017 19:12, Josh Triplett wrote: >>> Does this make any other checks redundant and removable? >> >> It would make sense to place it in cpu_has_kvm_support instead > > cpu_has_kvm_support() or cpu_has_vmx()? > >> , and the same in svm.c's has_svm. > > I don't follow (but I also don't know what any of these three letter > acryonyms acronyms stand for), does svm depend on vmx or vice-versa? Neither, one is Intel (VMX), the other is AMD (SVM). Would this work for you? And again, is this only with clang? ---------- 8< ------------ From: Paolo Bonzini Subject: [PATCH] KVM: clean up virtext.h virtext.h does a lot of complicated checks where it could just use boot_cpu_has instead. Remove all the cruft and stop using it in KVM even, because KVM can just use x86_match_cpu. As a side effect, this fixes arch/x86/kvm/vmx.c:64:32: warning: variable vmx_cpu_id is not needed and will not be emitted [-Wunneeded-internal-declaration] Reported-by: Nick Desaulniers Signed-off-by: Paolo Bonzini diff --git a/arch/x86/include/asm/virtext.h b/arch/x86/include/asm/virtext.h index 0116b2ee9e64..c7f7d5008d75 100644 --- a/arch/x86/include/asm/virtext.h +++ b/arch/x86/include/asm/virtext.h @@ -25,13 +25,6 @@ * VMX functions: */ -static inline int cpu_has_vmx(void) -{ - unsigned long ecx = cpuid_ecx(1); - return test_bit(5, &ecx); /* CPUID.1:ECX.VMX[bit 5] -> VT */ -} - - /** Disable VMX on the current CPU * * vmxoff causes a undefined-opcode exception if vmxon was not run @@ -49,22 +42,13 @@ static inline int cpu_vmx_enabled(void) return __read_cr4() & X86_CR4_VMXE; } -/** Disable VMX if it is enabled on the current CPU - * - * You shouldn't call this if cpu_has_vmx() returns 0. - */ -static inline void __cpu_emergency_vmxoff(void) -{ - if (cpu_vmx_enabled()) - cpu_vmxoff(); -} - /** Disable VMX if it is supported and enabled on the current CPU */ static inline void cpu_emergency_vmxoff(void) { - if (cpu_has_vmx()) - __cpu_emergency_vmxoff(); + if (boot_cpu_has(X86_FEATURE_VMX) && + cpu_vmx_enabled()) + cpu_vmxoff(); } @@ -74,36 +58,6 @@ static inline void cpu_emergency_vmxoff(void) * SVM functions: */ -/** Check if the CPU has SVM support - * - * You can use the 'msg' arg to get a message describing the problem, - * if the function returns zero. Simply pass NULL if you are not interested - * on the messages; gcc should take care of not generating code for - * the messages on this case. - */ -static inline int cpu_has_svm(const char **msg) -{ - if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD) { - if (msg) - *msg = "not amd"; - return 0; - } - - if (boot_cpu_data.extended_cpuid_level < SVM_CPUID_FUNC) { - if (msg) - *msg = "can't execute cpuid_8000000a"; - return 0; - } - - if (!boot_cpu_has(X86_FEATURE_SVM)) { - if (msg) - *msg = "svm not available"; - return 0; - } - return 1; -} - - /** Disable SVM on the current CPU * * You should call this only if cpu_has_svm() returned true. @@ -117,11 +71,17 @@ static inline void cpu_svm_disable(void) wrmsrl(MSR_EFER, efer & ~EFER_SVME); } +static inline int cpu_svm_enabled(void) +{ + return rdmsrl(MSR_EFER) & EFER_SVME; +} + /** Makes sure SVM is disabled, if it is supported on the CPU */ static inline void cpu_emergency_svm_disable(void) { - if (cpu_has_svm(NULL)) + if (boot_cpu_has(X86_FEATURE_SVM) && + cpu_svm_enabled()) cpu_svm_disable(); } diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c index 0e68f0b3cbf7..4ad26be59327 100644 --- a/arch/x86/kvm/svm.c +++ b/arch/x86/kvm/svm.c @@ -736,14 +736,7 @@ static void svm_init_osvw(struct kvm_vcpu *vcpu) static int has_svm(void) { - const char *msg; - - if (!cpu_has_svm(&msg)) { - printk(KERN_INFO "has_svm: %s\n", msg); - return 0; - } - - return 1; + return x86_match_cpu(svm_cpu_id); } static void svm_hardware_disable(void) @@ -769,10 +762,6 @@ static int svm_hardware_enable(void) if (efer & EFER_SVME) return -EBUSY; - if (!has_svm()) { - pr_err("%s: err EOPNOTSUPP on %d\n", __func__, me); - return -EINVAL; - } sd = per_cpu(svm_data, me); if (!sd) { pr_err("%s: svm_data is NULL on %d\n", __func__, me); diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index a2b804e10c95..9f6bcd9dd3c6 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -3468,7 +3468,7 @@ static void vmx_cache_reg(struct kvm_vcpu *vcpu, enum kvm_reg reg) static __init int cpu_has_kvm_support(void) { - return cpu_has_vmx(); + return x86_match_cpu(vmx_cpu_id); } static __init int vmx_disabled_by_bios(void)