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

[Jan Kiszka <jan.kiszka@siemens.com>]

On 2013-04-11 16:29, Gleb Natapov wrote:
> 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)).

Hmm, looks like: We leave L2, transfer the real vectoring info into the
queue and then consider injecting something in addition. That should
actually be avoided at higher level. Ok, will drop this test.

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux