Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752033AbeAPNAg (ORCPT + 1 other); Tue, 16 Jan 2018 08:00:36 -0500 Received: from mx1.redhat.com ([209.132.183.28]:47676 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750852AbeAPNAf (ORCPT ); Tue, 16 Jan 2018 08:00:35 -0500 Subject: Re: [PATCH 9/8] KVM: x86: limit MSR_IA32_SPEC_CTRL access based on CPUID availability To: Eric Wheeler Cc: linux-kernel@vger.kernel.org, kvm@vger.kernel.org, rkrcmar@redhat.com, liran.alon@oracle.com, jmattson@google.com, aliguori@amazon.com, thomas.lendacky@amd.com, dwmw@amazon.co.uk, bp@alien8.de, x86@kernel.org References: <20180109120311.27565-1-pbonzini@redhat.com> <20180109120311.27565-10-pbonzini@redhat.com> From: Paolo Bonzini Message-ID: Date: Tue, 16 Jan 2018 13:59:57 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Tue, 16 Jan 2018 13:00:34 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Return-Path: On 16/01/2018 01:55, Eric Wheeler wrote: > On Tue, 9 Jan 2018, Paolo Bonzini wrote: >> MSR_IA32_SPEC_CTRL is not available unless CPU[7,0].EDX[26] is 1. >> Check that against host CPUID or guest CPUID, respectively for >> host-initiated and guest-initiated accesses. > > Hi Radim, Paolo: > > In porting this patch series to v4.14 Just don't apply this patch. Paolo > In file included from arch/x86/kvm/vmx.c:21:0: > In function 'x86_feature_cpuid', > inlined from 'guest_cpuid_get_register' at arch/x86/kvm/cpuid.h:72:25, > inlined from 'vmx_get_msr' at arch/x86/kvm/cpuid.h:101:6: > arch/x86/kvm/cpuid.h:64:232: error: call to '__compiletime_assert_64' > declared with attribute error: BUILD_BUG_ON failed: > reverse_cpuid[x86_leaf].function == 0 > BUILD_BUG_ON(reverse_cpuid[x86_leaf].function == 0); > > ^ > In function 'x86_feature_cpuid', > inlined from 'guest_cpuid_get_register' at arch/x86/kvm/cpuid.h:72:25, > inlined from 'vmx_set_msr' at arch/x86/kvm/cpuid.h:101:6: > arch/x86/kvm/cpuid.h:64:232: error: call to '__compiletime_assert_64' > declared with attribute error: BUILD_BUG_ON failed: > reverse_cpuid[x86_leaf].function == 0 > BUILD_BUG_ON(reverse_cpuid[x86_leaf].function == 0); > > > I think this is caused by the following call stack for > X86_FEATURE_SPEC_CTRL, but if not please correct me here: > > arch/x86/kvm/vmx.c: > vmx_get_msr/vmx_set_msr() > guest_cpuid_has(vcpu, X86_FEATURE_SPEC_CTRL) > guest_cpuid_get_register(vcpu, x86_feature); > x86_feature_cpuid(x86_feature); > x86_feature_cpuid(unsigned x86_feature) > BUILD_BUG_ON(reverse_cpuid[x86_leaf].function == 0); > > > It looks like I need to add something to reverse_cpuid[] but I'm not sure > what. > > Do you know what needs to be added here? > > -Eric > > -- > Eric Wheeler > > > >> >> Suggested-by: Jim Mattson >> Signed-off-by: Paolo Bonzini >> --- >> This is for after X86_FEATURE_SPEC_CTRL is added to Linux, but >> I still wanted to ack Jim's improvement. >> >> arch/x86/kvm/svm.c | 8 ++++++++ >> arch/x86/kvm/vmx.c | 8 ++++++++ >> 2 files changed, 16 insertions(+) >> >> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c >> index 97126c2bd663..3a646580d7c5 100644 >> --- a/arch/x86/kvm/svm.c >> +++ b/arch/x86/kvm/svm.c >> @@ -3648,6 +3648,10 @@ static int svm_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info) >> msr_info->data = svm->nested.vm_cr_msr; >> break; >> case MSR_IA32_SPEC_CTRL: >> + if (!static_cpu_has(X86_FEATURE_SPEC_CTRL) || >> + (!msr_info->host_initiated && >> + !guest_cpuid_has(vcpu, X86_FEATURE_SPEC_CTRL))) >> + return 1; >> msr_info->data = svm->spec_ctrl; >> break; >> case MSR_IA32_UCODE_REV: >> @@ -3806,6 +3810,10 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr) >> vcpu_unimpl(vcpu, "unimplemented wrmsr: 0x%x data 0x%llx\n", ecx, data); >> break; >> case MSR_IA32_SPEC_CTRL: >> + if (!static_cpu_has(X86_FEATURE_SPEC_CTRL) || >> + (!msr_info->host_initiated && >> + !guest_cpuid_has(vcpu, X86_FEATURE_SPEC_CTRL))) >> + return 1; >> svm->spec_ctrl = data; >> break; >> case MSR_IA32_APICBASE: >> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c >> index 49b4a2d61603..42bc7ee293e4 100644 >> --- a/arch/x86/kvm/vmx.c >> +++ b/arch/x86/kvm/vmx.c >> @@ -3368,6 +3368,10 @@ static int vmx_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info) >> msr_info->data = guest_read_tsc(vcpu); >> break; >> case MSR_IA32_SPEC_CTRL: >> + if (!static_cpu_has(X86_FEATURE_SPEC_CTRL) || >> + (!msr_info->host_initiated && >> + !guest_cpuid_has(vcpu, X86_FEATURE_SPEC_CTRL))) >> + return 1; >> msr_info->data = to_vmx(vcpu)->spec_ctrl; >> break; >> case MSR_IA32_SYSENTER_CS: >> @@ -3510,6 +3514,10 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info) >> kvm_write_tsc(vcpu, msr_info); >> break; >> case MSR_IA32_SPEC_CTRL: >> + if (!static_cpu_has(X86_FEATURE_SPEC_CTRL) || >> + (!msr_info->host_initiated && >> + !guest_cpuid_has(vcpu, X86_FEATURE_SPEC_CTRL))) >> + return 1; >> to_vmx(vcpu)->spec_ctrl = data; >> break; >> case MSR_IA32_CR_PAT: >> -- >> 1.8.3.1 >> >>