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
