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 n...@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. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html