On 20/11/2017 23:46, Liran Alon wrote:
>>>
>>
>> But this is buggy as well, because the #GP is lost, isn't it?
>
> I don't think so.
>
> Since commit 664f8e26b00c ("KVM: X86: Fix loss of exception which has
> not yet been injected"), there is a fundamental difference between a
> pending exception and an injected exception.
> A pending exception means that no side-effects of the exception have
> been applied yet. Including incrementing the RIP after the instruction
> which cause exception. In our case for example, handle_wrmsr() calls
> kvm_inject_gp() and returns without calling
> kvm_skip_emulated_instruction() which increments the RIP.
Ok, I was almost sure this was going to be the case if the #GP wasn't
lost (but couldn't convince myself 100%).
> Therefore, when we exit from L2 to L1 on vmx-preemption-timer, we can
> safely clear exception.pending because when L1 will resume L2, the
> exception will be raised again (the same WRMSR instruction will be run
> again which will raise #GP again).
> This is also why vmcs12_save_pending_event() only makes sure to save in
> VMCS12 idt-vectoring-info the "injected" events and not the "pending"
> events (interrupt.pending is misleading name and I would rename it in
> upcoming patch to interrupt.injected. See explanation below).
Indeed. And then kvm_event_needs_reinjection starts making sense.
> I can confirm this patch works because I have wrote a kvm-unit-test
> which reproduce this issue. And after the fix the #GP is not lost and
> raised to L2 directly correctly.
> (I haven't posted the unit-test yet because it is very dependent on
> correct vmx-preemption-timer timer config that varies between
> environments).
Can you post it anyway? Tests always help understanding the code.
Paolo
