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@xxxxxxxxxxxxxxxxxxx |-----------------------------------------
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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
