Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758028AbeAHWc3 (ORCPT + 1 other); Mon, 8 Jan 2018 17:32:29 -0500 Received: from mx1.redhat.com ([209.132.183.28]:42594 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753851AbeAHWcZ (ORCPT ); Mon, 8 Jan 2018 17:32:25 -0500 Date: Mon, 8 Jan 2018 17:32:24 -0500 (EST) From: Paolo Bonzini To: Jim Mattson Cc: LKML , kvm list , aliguori@amazon.com, Tom Lendacky , dwmw@amazon.co.uk, bp@alien8.de Message-ID: <996327789.31733050.1515450744981.JavaMail.zimbra@redhat.com> In-Reply-To: References: <1515434925-10250-1-git-send-email-pbonzini@redhat.com> <1515434925-10250-4-git-send-email-pbonzini@redhat.com> Subject: Re: [PATCH 3/7] kvm: vmx: pass MSR_IA32_SPEC_CTRL and MSR_IA32_PRED_CMD down to the guest MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-Originating-IP: [94.39.196.37, 10.4.196.30, 10.4.195.17] Thread-Topic: pass MSR_IA32_SPEC_CTRL and MSR_IA32_PRED_CMD down to the guest Thread-Index: DSplVwBnn+Oo2vMr9W88wacLtmEl5g== X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Mon, 08 Jan 2018 22:32:25 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Return-Path: > I have: > > if (!have_spec_ctrl || > (!msr_info->host_initiated && > !guest_cpuid_has(vcpu, X86_FEATURE_SPEC_CTRL))) > return 1; > msr_info->data = to_vmx(vcpu)->msr_ia32_spec_ctrl; > break; > I have: > > if (!have_spec_ctrl || > (!msr_info->host_initiated && > !guest_cpuid_has(vcpu, X86_FEATURE_SPEC_CTRL))) > return 1; > to_vmx(vcpu)->msr_ia32_spec_ctrl = data; > break; Both better than mine. > > + vmx_disable_intercept_for_msr(MSR_IA32_SPEC_CTRL, false); > > + vmx_disable_intercept_for_msr(MSR_IA32_PRED_CMD, false); > > I have a lot of changes to MSR permission bitmap handling, but these > intercepts should only be disabled when guest_cpuid_has(vcpu, > X86_FEATURE_SPEC_CTRL). That's harder to backport and not strictly necessary (just like e.g. we don't check guest CPUID bits before emulation). I agree that your version is better, but I think the above is fine as a minimal fix. > Here, I have: > > /* > * If the guest is allowed to write to MSR_IA32_SPEC_CTRL, > * store it on VM-exit. > */ > if (guest_cpuid_has(vcpu, X86_FEATURE_SPEC_CTRL)) > add_autostore_msr(vmx, MSR_IA32_SPEC_CTRL); > else > clear_autostore_msr(vmx, MSR_IA32_SPEC_CTRL); > > /* > * If the guest's IA32_SPEC_CTRL MSR doesn't match the host's > * IA32_SPEC_CTRL MSR, then add the MSR to the atomic switch > * MSRs, so that the guest value will be loaded on VM-entry > * and the host value will be loaded on VM-exit. > */ > if (vmx->msr_ia32_spec_ctrl != spec_ctrl_enabled()) > add_atomic_switch_msr(vmx, MSR_IA32_SPEC_CTRL, > vmx->msr_ia32_spec_ctrl, > spec_ctrl_enabled()); > else > clear_atomic_switch_msr(vmx, MSR_IA32_SPEC_CTRL); > > > atomic_switch_perf_msrs(vmx); > > > > vmx_arm_hv_timer(vcpu); > > @@ -9707,6 +9733,12 @@ static void __noclone vmx_vcpu_run(struct kvm_vcpu > > *vcpu) > > #endif > > ); > > > > + if (have_spec_ctrl) { > > + rdmsrl(MSR_IA32_SPEC_CTRL, vmx->spec_ctrl); > > + if (vmx->spec_ctrl) > > + wrmsrl(MSR_IA32_SPEC_CTRL, 0); > > + } > > + > > I know the VM-exit MSR load and store lists are probably slower, but > I'm a little uncomfortable restoring the host's IA32_SPEC_CTRL MSR > late if the guest has it clear and the host has it set. There is no indirect branch before though, isn't it? Paolo > > /* MSR_IA32_DEBUGCTLMSR is zeroed on vmexit. Restore it if needed > > */ > > if (vmx->host_debugctlmsr) > > update_debugctlmsr(vmx->host_debugctlmsr); > > -- > > 1.8.3.1 > > > > >