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:51, Paolo Bonzini wrote:
>
> Liran Alon <LIRAN.ALON@ORACLE.COM> wrote:
>> On 10/11/17 23:37, Paolo Bonzini wrote:
>>> 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?...
>>
>> Consider the case sender sees vcpu->mode==IN_GUEST_MODE and before it
>> sends the physical IPI, dest CPU exits from guest and continues in L0
>> all the way until vcpu_enter_guest() and pass the part it checks for
>> KVM_REQ_EVENT but before it disables interrupts. Then sender sends the
>> physical IPI which is received in host-context and therefore runs
>> smp_kvm_posted_intr_nested_ipi() which sets KVM_REQ_EVENT &
>> pi_pending=true. But without Radim's suggestion of checking pi_pending
>> after interrupts disabled, this is too late as dest CPU will not check
>> these again until next exit from L2 guest.
>
> The destination CPU checks vcpu->requests before entering the guest, so this
> case would be handled fine (see Documentation/virtual/kvm/vcpu-requests.rst
> for an explanation of IN_GUEST_MODE, vcpu->requests, and PIR.ON).
>

Oh I see. You are right. My bad.
So I think your suggestion also works and may be simpler but I would 
further think about it before sending a v2.

Thanks for clarifying this.
-Liran

> Thanks,
>
> Paolo
>
>> I hope I didn't misunderstand something here :)