From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH 05/29] nVMX: Implement reading and writing of VMX MSRs Date: Sun, 30 Jan 2011 11:52:32 +0200 Message-ID: <4D4534E0.7010500@redhat.com> References: <1296116987-nyh@il.ibm.com> <201101270832.p0R8WOxe002432@rice.haifa.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org, gleb@redhat.com To: "Nadav Har'El" Return-path: Received: from mx1.redhat.com ([209.132.183.28]:41620 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751339Ab1A3Jwi (ORCPT ); Sun, 30 Jan 2011 04:52:38 -0500 In-Reply-To: <201101270832.p0R8WOxe002432@rice.haifa.ibm.com> Sender: kvm-owner@vger.kernel.org List-ID: On 01/27/2011 10:32 AM, Nadav Har'El wrote: > When the guest can use VMX instructions (when the "nested" module option is > on), it should also be able to read and write VMX MSRs, e.g., to query about > VMX capabilities. This patch adds this support. > > > +#define CORE2_PINBASED_CTLS_MUST_BE_ONE 0x00000016 > + case MSR_IA32_VMX_TRUE_PINBASED_CTLS: > + case MSR_IA32_VMX_PINBASED_CTLS: > + vmx_msr_low = CORE2_PINBASED_CTLS_MUST_BE_ONE; > + vmx_msr_high = CORE2_PINBASED_CTLS_MUST_BE_ONE | > + PIN_BASED_EXT_INTR_MASK | > + PIN_BASED_NMI_EXITING | > + PIN_BASED_VIRTUAL_NMIS; Can we actually support PIN_BASED_VIRTUAL_NMIs on hosts which don't support them? Maybe better to drop for the initial version. > + rdmsr(MSR_IA32_VMX_PROCBASED_CTLS, vmx_msr_low, vmx_msr_high); > + > + vmx_msr_low = 0; /* allow disabling any feature */ > + vmx_msr_high&= /* do not expose new untested features */ > + CPU_BASED_HLT_EXITING | CPU_BASED_CR3_LOAD_EXITING | > + CPU_BASED_CR3_STORE_EXITING | CPU_BASED_USE_IO_BITMAPS | > + CPU_BASED_MOV_DR_EXITING | CPU_BASED_USE_TSC_OFFSETING | > + CPU_BASED_MWAIT_EXITING | CPU_BASED_MONITOR_EXITING | > + CPU_BASED_INVLPG_EXITING | CPU_BASED_TPR_SHADOW | > +#ifdef CONFIG_X86_64 > + CPU_BASED_CR8_LOAD_EXITING | > + CPU_BASED_CR8_STORE_EXITING | > +#endif > + CPU_BASED_ACTIVATE_SECONDARY_CONTROLS; Similarly, I think CPU_BASED_TPR_SHADOW and CPU_BASED_ACTIVATE_SECONDARY_CONTROLS are not available on all models. > + case MSR_IA32_VMX_PROCBASED_CTLS2: > + *pdata = 0; > + if (vm_need_virtualize_apic_accesses(vcpu->kvm)) > + *pdata |= SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES; > + break; Here, at least you qualify support by checking the host. > + case MSR_IA32_VMX_EPT_VPID_CAP: > + /* Currently, no nested ept or nested vpid */ > + *pdata = 0; > + break; > + default: > + return 0; > + } > + > + return 1; > +} > + > +static int vmx_set_vmx_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 data) > +{ > + if (!nested_vmx_allowed(vcpu)) > + return 0; > + > + /* > + * according to the spec, "VMX capability MSRs are read-only; an > + * attempt to write them (with WRMSR) produces a #GP(0). > + */ > + if (msr_index>= MSR_IA32_VMX_BASIC&& > + msr_index<= MSR_IA32_VMX_TRUE_ENTRY_CTLS) { > + kvm_queue_exception_e(vcpu, GP_VECTOR, 0); > + return 1; Can just drop this part, #GP is the default response. > + } else if (msr_index == MSR_IA32_FEATURE_CONTROL) > + /* TODO: the right thing. */ > + return 1; > + else > + return 0; > +} -- error compiling committee.c: too many arguments to function