Re: [PATCH] KVM: nVMX: Fix races when sending nested PI while dest enters/leaves L2

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

On 11/11/17 00:30, Radim Krčmář wrote:
> 2017-11-10 22:37+0100, Paolo Bonzini:
>> On 10/11/2017 19:06, Radim Krčmář wrote:
>>>> 	/* the PIR and ON have been set by L1. */
>>>> 	if (!kvm_vcpu_trigger_posted_interrupt(vcpu, true)) {
>>> This would still fail on the exiting case.
>>>
>>> If one VCPU was just after a VM exit, then the sender would see it
>>> IN_GUEST_MODE, send the posted notification and return true, but the
>>> notification would do nothing
>>
>> It would cause *something*---a vmexit because the vector doesn't match
>> the L1 posted interrupt.  Then smp_kvm_posted_intr_nested_ipi would be
>> invoked from vmx_handle_external_intr.
>>
>> Could we detect the vector in vmx_handle_external_intr and set
>> pi_pending+KVM_REQ_EVENT?  Or invoke a function in KVM from
>> smp_kvm_posted_intr_nested_ipi?  Or would both be insane?...
>
> I think it is a trade-off.
>
> We could call KVM from smp_kvm_posted_intr_nested_ipi(), which would
> handle the case when the notification arrives after
> vmx_handle_external_intr().
>
> It doesn't performance, because we'd have to avoid a race on VM entry by
> possibly needlessly kicking the guest after seeing that it went from
> OUTSIDE_GUEST_MODE to IN_GUEST_MODE while we were setting the pending
> bit.
>
> But the behavior is slightly better because we can't be scanning PIR
> twice for one notification. (If the notification was handled directly by
> guest and then also by KVM due to the unconditionally set pending bit.)
>
> Well, I better think about it with fresh mind ...
>

If notification was handled directly by guest, the CPU is suppose to 
clear POSTED_INTR_ON bit in pi_desc->control (bit 256 - Outstanding 
Notification).

In that case, even though vmx_complete_nested_posted_interrupt() will be 
called on next VMEntry, it will just set pi_pending=false and do nothing 
because of:
if (!pi_test_and_clear_on(vmx->nested.pi_desc))
     return;

Therefore, there should be no harm in unconditionally setting pi_pending 
bit and I think Radim's original suggestion should still work well.

-Liran