Re: [PATCH 2/4] KVM: nVMX: Require immediate-exit when event reinjected to L2 and L1 event pending

[Liran Alon <LIRAN.ALON@ORACLE.COM>]

On 11/11/17 01:26, Paolo Bonzini wrote:
> On 05/11/2017 15:07, Liran Alon wrote:
>> +	/*
>> +	 * If we reinjected a previous event,
>> +	 * don't consider a new pending event
>> +	 */
>> +	if (kvm_event_needs_reinjection(vcpu))
>> +		return 0;
>> +
>
> Could you end up with
>
>                          WARN_ON(vcpu->arch.exception.pending);
>
> in vcpu_enter_guest after returning 0 here?
>
> Maybe it would be safer to return a non-zero value so that the caller
> sets req_immediate_exit = true.  But I haven't really thought through
> the consequences.

The only difference before and after this patch *should* have been that 
now if L1 has pending event (as specified by vmx_check_nested_events()), 
a value of -EBUSY will be returned and an immediate-exit will be 
requested. Even if a re-injection had occurred.
If that is not the case, previous code and this code should return 0 on 
the exact same cases.

*However*, if exception.pending=true and 
nmi_injected/interrupt.pending=true, then previous code would have 
continued inject_pending_event() while this code would return too-soon.
Indeed triggering the above mentioned warning.

Therefore I think you have found here a bug that was missed in the 
review-chain from some reason and wasn't observed in tests... Will 
investigate how tests didn't caught this. Sorry for this.
It seems that this patch approach would have worked on version before 
commit 664f8e26b00c ("KVM: X86: Fix loss of exception which has not yet 
been injected") but afterwards will break...

Will fix and re-run all tests.

Thanks,

-Liran

>
> Thanks,
>
> Paolo
>