Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933045AbeAHWZi (ORCPT + 1 other); Mon, 8 Jan 2018 17:25:38 -0500 Received: from mx1.redhat.com ([209.132.183.28]:41468 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932509AbeAHWZg (ORCPT ); Mon, 8 Jan 2018 17:25:36 -0500 Date: Mon, 8 Jan 2018 17:25:36 -0500 (EST) From: Paolo Bonzini To: ashok raj Cc: linux-kernel@vger.kernel.org, kvm@vger.kernel.org, jmattson@google.com, aliguori@amazon.com, thomas lendacky , dwmw@amazon.co.uk, bp@alien8.de Message-ID: <297841893.31732388.1515450336037.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.19, 10.4.195.7] Thread-Topic: pass MSR_IA32_SPEC_CTRL and MSR_IA32_PRED_CMD down to the guest Thread-Index: hNjQkQr6UTVnK2GwewrX//4XjCQPbA== X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.27]); Mon, 08 Jan 2018 22:25:36 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Return-Path: ----- Original Message ----- > From: "Ashok Raj" > To: "Paolo Bonzini" > Cc: linux-kernel@vger.kernel.org, kvm@vger.kernel.org, jmattson@google.com, aliguori@amazon.com, "thomas lendacky" > , dwmw@amazon.co.uk, bp@alien8.de, "Ashok Raj" > Sent: Monday, January 8, 2018 11:09:53 PM > Subject: Re: [PATCH 3/7] kvm: vmx: pass MSR_IA32_SPEC_CTRL and MSR_IA32_PRED_CMD down to the guest > > Hi Paolo > > Do you assume that host isn't using IBRS and only guest uses it? For now, yes. Patches to add the IBRS and IBPB cpufeatures will have to adjust the MSR writes from this patch. Paolo > > > > On Mon, Jan 8, 2018 at 10:08 AM, Paolo Bonzini wrote: > > Direct access to MSR_IA32_SPEC_CTRL and MSR_IA32_PRED_CMD is important > > for performance. Allow load/store of MSR_IA32_SPEC_CTRL, restore guest > > IBRS on VM entry and set it to 0 on VM exit (because Linux does not use > > it yet). > > > > Signed-off-by: Paolo Bonzini > > --- > > arch/x86/kvm/vmx.c | 32 ++++++++++++++++++++++++++++++++ > > 1 file changed, 32 insertions(+) > > > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > > index 669f5f74857d..d00bcad7336e 100644 > > --- a/arch/x86/kvm/vmx.c > > +++ b/arch/x86/kvm/vmx.c > > @@ -120,6 +120,8 @@ > > module_param_named(preemption_timer, enable_preemption_timer, bool, > > S_IRUGO); > > #endif > > > > +static bool __read_mostly have_spec_ctrl; > > + > > #define KVM_GUEST_CR0_MASK (X86_CR0_NW | X86_CR0_CD) > > #define KVM_VM_CR0_ALWAYS_ON_UNRESTRICTED_GUEST (X86_CR0_WP | X86_CR0_NE) > > #define KVM_VM_CR0_ALWAYS_ON \ > > @@ -609,6 +611,8 @@ struct vcpu_vmx { > > u64 msr_host_kernel_gs_base; > > u64 msr_guest_kernel_gs_base; > > #endif > > + u64 spec_ctrl; > > + > > u32 vm_entry_controls_shadow; > > u32 vm_exit_controls_shadow; > > u32 secondary_exec_control; > > @@ -3361,6 +3365,9 @@ static int vmx_get_msr(struct kvm_vcpu *vcpu, struct > > msr_data *msr_info) > > case MSR_IA32_TSC: > > msr_info->data = guest_read_tsc(vcpu); > > break; > > + case MSR_IA32_SPEC_CTRL: > > + msr_info->data = to_vmx(vcpu)->spec_ctrl; > > + break; > > case MSR_IA32_SYSENTER_CS: > > msr_info->data = vmcs_read32(GUEST_SYSENTER_CS); > > break; > > @@ -3500,6 +3507,9 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct > > msr_data *msr_info) > > case MSR_IA32_TSC: > > kvm_write_tsc(vcpu, msr_info); > > break; > > + case MSR_IA32_SPEC_CTRL: > > + to_vmx(vcpu)->spec_ctrl = msr_info->data; > > + break; > > case MSR_IA32_CR_PAT: > > if (vmcs_config.vmentry_ctrl & VM_ENTRY_LOAD_IA32_PAT) { > > if (!kvm_mtrr_valid(vcpu, MSR_IA32_CR_PAT, data)) > > @@ -7062,6 +7072,17 @@ static __init int hardware_setup(void) > > goto out; > > } > > > > + /* > > + * FIXME: this is only needed until SPEC_CTRL is supported > > + * by upstream Linux in cpufeatures, then it can be replaced > > + * with static_cpu_has. > > + */ > > + have_spec_ctrl = cpu_has_spec_ctrl(); > > + if (have_spec_ctrl) > > + pr_info("kvm: SPEC_CTRL available\n"); > > + else > > + pr_info("kvm: SPEC_CTRL not available\n"); > > + > > if (boot_cpu_has(X86_FEATURE_NX)) > > kvm_enable_efer_bits(EFER_NX); > > > > @@ -7131,6 +7152,8 @@ static __init int hardware_setup(void) > > vmx_disable_intercept_for_msr(MSR_IA32_SYSENTER_CS, false); > > vmx_disable_intercept_for_msr(MSR_IA32_SYSENTER_ESP, false); > > vmx_disable_intercept_for_msr(MSR_IA32_SYSENTER_EIP, false); > > + vmx_disable_intercept_for_msr(MSR_IA32_SPEC_CTRL, false); > > + vmx_disable_intercept_for_msr(MSR_IA32_PRED_CMD, false); > > > > memcpy(vmx_msr_bitmap_legacy_x2apic_apicv, > > vmx_msr_bitmap_legacy, PAGE_SIZE); > > @@ -9597,6 +9620,9 @@ static void __noclone vmx_vcpu_run(struct kvm_vcpu > > *vcpu) > > > > pt_guest_enter(vmx); > > > > + if (have_spec_ctrl && vmx->spec_ctrl != 0) > > + wrmsrl(MSR_IA32_SPEC_CTRL, vmx->spec_ctrl); > > + > > Do we even need to optimize this? what if host Linux enabled IBRS, but > guest has it turned off? > Thought it might be simpler to blindly update it with what > vmx->spec_ctrl value is? > > > 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); > > + } > > + > > Same thing here.. if the host OS has enabled IBRS wouldn't you want to > keep the same value? > > > /* MSR_IA32_DEBUGCTLMSR is zeroed on vmexit. Restore it if needed > > */ > > if (vmx->host_debugctlmsr) > > update_debugctlmsr(vmx->host_debugctlmsr); > > -- > > 1.8.3.1 > > > > >