From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Nadav Har'El" Subject: Re: [PATCH 22/31] nVMX: Deciding if L0 or L1 should handle an L2 exit Date: Wed, 25 May 2011 16:45:07 +0300 Message-ID: <20110525134507.GF16418@fermat.math.technion.ac.il> References: <1305575004-nyh@il.ibm.com> <201105161955.p4GJtB4O001985@rice.haifa.ibm.com> <625BA99ED14B2D499DC4E29D8138F1505C9BFA39E8@shsmsx502.ccr.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: "kvm@vger.kernel.org" , "gleb@redhat.com" , "avi@redhat.com" To: "Tian, Kevin" Return-path: Received: from mailgw12.technion.ac.il ([132.68.225.12]:39096 "EHLO mailgw12.technion.ac.il" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757717Ab1EYNpM (ORCPT ); Wed, 25 May 2011 09:45:12 -0400 Content-Disposition: inline In-Reply-To: <625BA99ED14B2D499DC4E29D8138F1505C9BFA39E8@shsmsx502.ccr.corp.intel.com> Sender: kvm-owner@vger.kernel.org List-ID: On Wed, May 25, 2011, Tian, Kevin wrote about "RE: [PATCH 22/31] nVMX: Deciding if L0 or L1 should handle an L2 exit": > > +static inline bool nested_cpu_has_virtual_nmis(struct kvm_vcpu *vcpu) > > +{ > > + return is_guest_mode(vcpu) && > > + (get_vmcs12(vcpu)->pin_based_vm_exec_control & > > + PIN_BASED_VIRTUAL_NMIS); > > +} > > any reason to add guest mode check here? I didn't see such check in your > earlier nested_cpu_has_xxx. It would be clearer to use existing nested_cpu_has_xxx > along with is_guest_mode explicitly which makes such usage consistent. The nested_cpu_has function is for procbased controls, not pinbased controls... But you're right, it's strange that only this function has is_guest_mode() in it. Moving it outside. The call site is now really ugly ;-) > > + case EXIT_REASON_INVLPG: > > + return vmcs12->cpu_based_vm_exec_control & > > + CPU_BASED_INVLPG_EXITING; > > use nested_cpu_has. Right ;-) > > + if (exit_reason == EXIT_REASON_VMLAUNCH || > > + exit_reason == EXIT_REASON_VMRESUME) > > + vmx->nested.nested_run_pending = 1; > > + else > > + vmx->nested.nested_run_pending = 0; > > what about VMLAUNCH invoked from L2? In such case I think you expect > L1 to run instead of L2. Wow, a good catch! According to the theory (see our paper :-)), our implementations should work with any number of nesting levels, not just two. But in practice, we've always had a bug in running L3, and we never had the time to figure out why. This is a good lead - I'll have to investigate. But not now. > On the other hand, isn't just guest mode check enough to differentiate > pending nested run? When L1 invokes VMLAUNCH/VMRESUME, guest mode > hasn't been set yet, and below check will fail. All other operations will then > be checked by nested_vmx_exit_handled... > > Do I miss anything here? As we discussed in another thread, nested_run_pending is important later, at the injection path. > > - if (unlikely(!cpu_has_virtual_nmis() && vmx->soft_vnmi_blocked)) { > > + if (unlikely(!cpu_has_virtual_nmis() && vmx->soft_vnmi_blocked && > > + !nested_cpu_has_virtual_nmis(vcpu))) { > > Would L0 want to control vNMI for L2 guest? Otherwise we just use is_guest_mode > here for the condition check? I don't remember the details here, but this if() used to be different, until Avi Kivity asked me to change it to this state. Nadav. -- Nadav Har'El | Wednesday, May 25 2011, 21 Iyyar 5771 nyh@math.technion.ac.il |----------------------------------------- Phone +972-523-790466, ICQ 13349191 |The person who knows how to laugh at http://nadav.harel.org.il |himself will never cease to be amused.