From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Tian, Kevin" Subject: RE: [PATCH 21/31] nVMX: vmcs12 checks on nested entry Date: Wed, 25 May 2011 11:01:26 +0800 Message-ID: <625BA99ED14B2D499DC4E29D8138F1505C9BFA377E@shsmsx502.ccr.corp.intel.com> References: <1305575004-nyh@il.ibm.com> <201105161954.p4GJseNX001963@rice.haifa.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 8BIT Cc: "gleb@redhat.com" , "avi@redhat.com" To: Nadav Har'El , "kvm@vger.kernel.org" Return-path: Received: from mga14.intel.com ([143.182.124.37]:22968 "EHLO mga14.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753979Ab1EYDCF convert rfc822-to-8bit (ORCPT ); Tue, 24 May 2011 23:02:05 -0400 In-Reply-To: <201105161954.p4GJseNX001963@rice.haifa.ibm.com> Content-Language: en-US Sender: kvm-owner@vger.kernel.org List-ID: > From: Nadav Har'El > Sent: Tuesday, May 17, 2011 3:55 AM > > This patch adds a bunch of tests of the validity of the vmcs12 fields, > according to what the VMX spec and our implementation allows. If fields > we cannot (or don't want to) honor are discovered, an entry failure is > emulated. > > According to the spec, there are two types of entry failures: If the problem > was in vmcs12's host state or control fields, the VMLAUNCH instruction simply > fails. But a problem is found in the guest state, the behavior is more > similar to that of an exit. > > Signed-off-by: Nadav Har'El > --- > arch/x86/include/asm/vmx.h | 8 ++ > arch/x86/kvm/vmx.c | 94 > +++++++++++++++++++++++++++++++++++ > 2 files changed, 102 insertions(+) > > --- .before/arch/x86/kvm/vmx.c 2011-05-16 22:36:49.000000000 +0300 > +++ .after/arch/x86/kvm/vmx.c 2011-05-16 22:36:49.000000000 +0300 > @@ -870,6 +870,10 @@ static inline bool nested_cpu_has2(struc > (vmcs12->secondary_vm_exec_control & bit); > } > > +static void nested_vmx_entry_failure(struct kvm_vcpu *vcpu, > + struct vmcs12 *vmcs12, > + u32 reason, unsigned long qualification); > + > static int __find_msr_index(struct vcpu_vmx *vmx, u32 msr) > { > int i; > @@ -6160,6 +6164,79 @@ static int nested_vmx_run(struct kvm_vcp > > vmcs12 = get_vmcs12(vcpu); > > + /* > + * The nested entry process starts with enforcing various prerequisites > + * on vmcs12 as required by the Intel SDM, and act appropriately when > + * they fail: As the SDM explains, some conditions should cause the > + * instruction to fail, while others will cause the instruction to seem > + * to succeed, but return an EXIT_REASON_INVALID_STATE. > + * To speed up the normal (success) code path, we should avoid checking > + * for misconfigurations which will anyway be caught by the processor > + * when using the merged vmcs02. > + */ > + if (vmcs12->launch_state == launch) { > + nested_vmx_failValid(vcpu, > + launch ? VMXERR_VMLAUNCH_NONCLEAR_VMCS > + : VMXERR_VMRESUME_NONLAUNCHED_VMCS); > + return 1; > + } from SDM: ELSIF (VMLAUNCH and launch state of current VMCS is not "clear") THEN VMfailValid(VMLAUNCH with non-clear VMCS); ELSIF (VMRESUME and launch state of current VMCS is not "launched") THEN VMfailValid(VMRESUME with non-launched VMCS); So it's legal to use VMLAUNCH on a launched VMCS. However here you changes this behavior. On the other hand, do you want to add a 'clear' state along with L1 VMCLEAR to catch the failure here? Thanks Kevin