Re: [PATCH v3 4/5] KVM: nVMX: Fix conditions for interrupt injection

[Gleb Natapov <gleb@redhat.com>]

On Thu, Apr 11, 2013 at 04:27:23PM +0200, Jan Kiszka wrote:
> On 2013-04-11 13:20, Gleb Natapov wrote:
> > On Sun, Mar 24, 2013 at 07:44:47PM +0100, Jan Kiszka wrote:
> >> From: Jan Kiszka <jan.kiszka@siemens.com>
> >>
> >> If we are in guest mode, L0 can only inject events into L2 if L1 has
> >> nothing pending. Otherwise, L0 would overwrite L1's events and they
> >> would get lost. But even if no injection of L1 is pending, we do not
> >> want L0 to interrupt unnecessarily an on going vmentry with all its side
> >> effects on the vmcs. Therefore, injection shall be disallowed during
> >> L1->L2 transitions. This check is conceptually independent of
> >> nested_exit_on_intr.
> >>
> >> If L1 traps external interrupts, then we also need to look at L1's
> >> idt_vectoring_info_field. If it is empty, we can kick the guest from L2
> >> to L1, just like the previous code worked.
> >>
> >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> >> ---
> >>  arch/x86/kvm/vmx.c |   28 ++++++++++++++++++++--------
> >>  1 files changed, 20 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> >> index d1bc834..30aa198 100644
> >> --- a/arch/x86/kvm/vmx.c
> >> +++ b/arch/x86/kvm/vmx.c
> >> @@ -4325,16 +4325,28 @@ static int vmx_nmi_allowed(struct kvm_vcpu *vcpu)
> >>  
> >>  static int vmx_interrupt_allowed(struct kvm_vcpu *vcpu)
> >>  {
> >> -	if (is_guest_mode(vcpu) && nested_exit_on_intr(vcpu)) {
> >> +	if (is_guest_mode(vcpu)) {
> >>  		struct vmcs12 *vmcs12 = get_vmcs12(vcpu);
> >> -		if (to_vmx(vcpu)->nested.nested_run_pending ||
> >> -		    (vmcs12->idt_vectoring_info_field &
> >> -		     VECTORING_INFO_VALID_MASK))
> >> +
> >> +		if (to_vmx(vcpu)->nested.nested_run_pending)
> >>  			return 0;
> >> -		nested_vmx_vmexit(vcpu);
> >> -		vmcs12->vm_exit_reason = EXIT_REASON_EXTERNAL_INTERRUPT;
> >> -		vmcs12->vm_exit_intr_info = 0;
> >> -		/* fall through to normal code, but now in L1, not L2 */
> >> +		if (nested_exit_on_intr(vcpu)) {
> >> +			/*
> >> +			 * Check if the idt_vectoring_info_field is free. We
> >> +			 * cannot raise EXIT_REASON_EXTERNAL_INTERRUPT if it
> >> +			 * isn't.
> >> +			 */
> >> +			if (vmcs12->idt_vectoring_info_field &
> >> +			    VECTORING_INFO_VALID_MASK)
> >> +				return 0;
> > After patch 2 I do not see how this can be true. Now this case is
> > handled by the common code: since event queue is not empty the code will not
> > get here.
> 
> The event queue is unconditionally cleared (after being migrated to
> vmcs12) in patch 2.
> 
During vmexit, yes. But here we are in if(is_guest_mode(vcpu)).

--
			Gleb.