Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934025AbcKWJG3 (ORCPT ); Wed, 23 Nov 2016 04:06:29 -0500 Received: from mx6-phx2.redhat.com ([209.132.183.39]:41278 "EHLO mx6-phx2.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933655AbcKWJGV (ORCPT ); Wed, 23 Nov 2016 04:06:21 -0500 Date: Wed, 23 Nov 2016 04:06:17 -0500 (EST) From: Paolo Bonzini To: David Matlack Cc: kvm@vger.kernel.org, linux-kernel@vger.kernel.org, jmattson@google.com, rkrcmar@redhat.com Message-ID: <1168524783.1374944.1479891977484.JavaMail.zimbra@redhat.com> In-Reply-To: <1479863680-117511-4-git-send-email-dmatlack@google.com> References: <1479863680-117511-1-git-send-email-dmatlack@google.com> <1479863680-117511-4-git-send-email-dmatlack@google.com> Subject: Re: [PATCH 3/4] KVM: nVMX: accurate emulation of MSR_IA32_CR{0,4}_FIXED1 MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-Originating-IP: [10.4.164.1, 10.5.101.130] X-Mailer: Zimbra 8.0.6_GA_5922 (ZimbraWebClient - FF49 (Linux)/8.0.6_GA_5922) Thread-Topic: nVMX: accurate emulation of MSR_IA32_CR{0,4}_FIXED1 Thread-Index: SkkC9XRY9nPj7Zqg5gDGezaWwOS3PQ== Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4502 Lines: 116 > Set MSR_IA32_CR{0,4}_FIXED1 to match the CPU's MSRs. > > In addition, MSR_IA32_CR4_FIXED1 should reflect the available CR4 bits > according to CPUID. Whenever guest CPUID is updated by userspace, > regenerate MSR_IA32_CR4_FIXED1 to match it. > > Signed-off-by: David Matlack Oh, I thought userspace would do that! Doing it in KVM is fine as well, but then do we need to give userspace access to CR{0,4}_FIXED{0,1} at all? Paolo > --- > Note: "x86/cpufeature: Add User-Mode Instruction Prevention definitions" has > not hit kvm/master yet so the macros for X86_CR4_UMIP and X86_FEATURE_UMIP > are not available. > > arch/x86/kvm/vmx.c | 54 > +++++++++++++++++++++++++++++++++++++++++++++++++++--- > 1 file changed, 51 insertions(+), 3 deletions(-) > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > index a2a5ad8..ac5d9c0 100644 > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -2852,16 +2852,18 @@ static void nested_vmx_setup_ctls_msrs(struct > vcpu_vmx *vmx) > vmx->nested.nested_vmx_basic |= VMX_BASIC_INOUT; > > /* > - * These MSRs specify bits which the guest must keep fixed (on or off) > + * These MSRs specify bits which the guest must keep fixed on > * while L1 is in VMXON mode (in L1's root mode, or running an L2). > * We picked the standard core2 setting. > */ > #define VMXON_CR0_ALWAYSON (X86_CR0_PE | X86_CR0_PG | X86_CR0_NE) > #define VMXON_CR4_ALWAYSON X86_CR4_VMXE > vmx->nested.nested_vmx_cr0_fixed0 = VMXON_CR0_ALWAYSON; > - vmx->nested.nested_vmx_cr0_fixed1 = -1ULL; > vmx->nested.nested_vmx_cr4_fixed0 = VMXON_CR4_ALWAYSON; > - vmx->nested.nested_vmx_cr4_fixed1 = -1ULL; > + > + /* These MSRs specify bits which the guest must keep fixed off. */ > + rdmsrl(MSR_IA32_VMX_CR0_FIXED1, vmx->nested.nested_vmx_cr0_fixed1); > + rdmsrl(MSR_IA32_VMX_CR4_FIXED1, vmx->nested.nested_vmx_cr4_fixed1); > > /* highest index: VMX_PREEMPTION_TIMER_VALUE */ > vmx->nested.nested_vmx_vmcs_enum = 0x2e; > @@ -9580,6 +9582,49 @@ static void vmcs_set_secondary_exec_control(u32 > new_ctl) > (new_ctl & ~mask) | (cur_ctl & mask)); > } > > +/* > + * Generate MSR_IA32_VMX_CR4_FIXED1 according to CPUID. Only set bits > + * (indicating "allowed-1") if they are supported in the guest's CPUID. > + */ > +static void nested_vmx_cr4_fixed1_update(struct kvm_vcpu *vcpu) > +{ > + struct vcpu_vmx *vmx = to_vmx(vcpu); > + struct kvm_cpuid_entry2 *entry; > + > +#define update(_cr4_mask, _reg, _cpuid_mask) do { \ > + if (entry && (entry->_reg & (_cpuid_mask))) \ > + vmx->nested.nested_vmx_cr4_fixed1 |= (_cr4_mask); \ > +} while (0) > + > + vmx->nested.nested_vmx_cr4_fixed1 = X86_CR4_PCE; > + > + entry = kvm_find_cpuid_entry(vcpu, 0x1, 0); > + update(X86_CR4_VME, edx, bit(X86_FEATURE_VME)); > + update(X86_CR4_PVI, edx, bit(X86_FEATURE_VME)); > + update(X86_CR4_TSD, edx, bit(X86_FEATURE_TSC)); > + update(X86_CR4_DE, edx, bit(X86_FEATURE_DE)); > + update(X86_CR4_PSE, edx, bit(X86_FEATURE_PSE)); > + update(X86_CR4_PAE, edx, bit(X86_FEATURE_PAE)); > + update(X86_CR4_MCE, edx, bit(X86_FEATURE_MCE)); > + update(X86_CR4_PGE, edx, bit(X86_FEATURE_PGE)); > + update(X86_CR4_OSFXSR, edx, bit(X86_FEATURE_FXSR)); > + update(X86_CR4_OSXMMEXCPT, edx, bit(X86_FEATURE_XMM)); > + update(X86_CR4_VMXE, ecx, bit(X86_FEATURE_VMX)); > + update(X86_CR4_SMXE, ecx, bit(X86_FEATURE_SMX)); > + update(X86_CR4_PCIDE, ecx, bit(X86_FEATURE_PCID)); > + update(X86_CR4_OSXSAVE, ecx, bit(X86_FEATURE_XSAVE)); > + > + entry = kvm_find_cpuid_entry(vcpu, 0x7, 0); > + update(X86_CR4_FSGSBASE, ebx, bit(X86_FEATURE_FSGSBASE)); > + update(X86_CR4_SMEP, ebx, bit(X86_FEATURE_SMEP)); > + update(X86_CR4_SMAP, ebx, bit(X86_FEATURE_SMAP)); > + update(X86_CR4_PKE, ecx, bit(X86_FEATURE_PKU)); > + /* TODO: Use X86_CR4_UMIP and X86_FEATURE_UMIP macros */ > + update(bit(11), ecx, bit(2)); > + > +#undef update > +} > + > static void vmx_cpuid_update(struct kvm_vcpu *vcpu) > { > struct kvm_cpuid_entry2 *best; > @@ -9621,6 +9666,9 @@ static void vmx_cpuid_update(struct kvm_vcpu *vcpu) > else > to_vmx(vcpu)->msr_ia32_feature_control_valid_bits &= > ~FEATURE_CONTROL_VMXON_ENABLED_OUTSIDE_SMX; > + > + if (nested_vmx_allowed(vcpu)) > + nested_vmx_cr4_fixed1_update(vcpu); > } > > static void vmx_set_supported_cpuid(u32 func, struct kvm_cpuid_entry2 > *entry) > -- > 2.8.0.rc3.226.g39d4020 > >