Avi Kivity wrote on 2013-02-25:
> I didn't really follow, but is the root cause the need to keep track
> of interrupt coalescing? If so we can recommend that users use
> KVM_IRQ_LINE when coalescing is unneeded, and move interrupt injection
> with irq coalescing support to vcpu context.
So we can hide the capability KVM_CAP_IRQ_INJECT_STATUS when posted interrupt is enabled to force users doesn't to use KVM_IRQ_LINE_STATUS. Does this acceptable?
The only case in KVM that need to know the interrupt injection status is vlapic timer. But since vlapic timer and vcpu are always in same pcpu, so there is no problem.
> It's not pleasant to cause a performance regression, so it should be a
> last resort (or maybe do both if it helps).
>
> On Sun, Feb 24, 2013 at 8:08 PM, Marcelo Tosatti <mtosatti@xxxxxxxxxx> wrote:
>> On Sun, Feb 24, 2013 at 04:19:17PM +0200, Gleb Natapov wrote:
>>> On Sun, Feb 24, 2013 at 01:55:07PM +0000, Zhang, Yang Z wrote:
>>>>> I do not think it fixes it. There is no guaranty that IPI will be
>>>>> processed by remote cpu while sending cpu is still in locked section, so
>>>>> the same race may happen regardless. As you say above there are 3
>>>>> contexts, but only two use locks.
>>>> See following logic, I think the problem you mentioned will not
>>>> happened with current logic.
>>>>
>>>> get lock
>>>> if test_pir (this will ensure there is no in flight IPI for same interrupt. Since
> we are taking the lock now, no IPI will be sent before we release the lock and no
> pir->irr is performed by hardware for same interrupt.)
>>
>> Does the PIR IPI interrupt returns synchronously _after_ PIR->IRR transfer
>> is made? Don't think so.
>>
>> PIR IPI interrupt returns after remote CPU has acked interrupt receival,
>> not after remote CPU has acked _and_ performed PIR->IRR transfer.
>>
>> Right? (yes, right, see step 4. below).
>>
>> Should try to make it easier to drop the lock later, so depend on it as
>> little as possible (also please document what it protects in detail).
>>
>> Also note:
>>
>> "
>> 3. The processor clears the outstanding-notification bit in the
>> posted-interrupt descriptor. This is done atomically
>> so as to leave the remainder of the descriptor unmodified (e.g., with a
>> locked AND operation).
>> 4. The processor writes zero to the EOI register in the local APIC; this
>> dismisses the interrupt with the postedinterrupt
>> notification vector from the local APIC.
>> 5. The logical processor performs a logical-OR of PIR into VIRR and
>> clears PIR. No other agent can read or write a
>> PIR bit (or group of bits) between the time it is read (to determine
>> what to OR into VIRR) and when it is cleared.
>> "
>>
>> So checking the ON bit does not mean the HW has finished performing
>> PIR->VIRR transfer (which means ON bit can only be used as an indication
>> of whether to send interrupt, not whether PIR->VIRR transfer is
>> finished).
>>
>> So its fine to do
>>
>> -> atomic set pir
>> -> if (atomic_test_and_set(PIR ON bit))
>> -> send IPI
>>
>> But HW can transfer two distinct bits, set by different serialized instances
>> of kvm_set_irq (protected with a lock), in a single PIR->IRR pass.
>>
>>> I do not see where those assumptions are coming from. Testing pir does
>>> not guaranty that the IPI is not processed by VCPU right now.
>>>
>>> iothread: vcpu:
>>> send_irq()
>>> lock(pir)
>>> check pir and irr
>>> set pir
>>> send IPI (*)
>>> unlock(pir)
>>>
>>> send_irq()
>>> lock(pir)
>>> receive IPI (*)
>>> atomic {
>>> pir_tmp = pir
>>> pir = 0
>>> } check pir and irr irr &= pir_tmp
>>> set pir
>>> send IPI
>>> unlock(pir)
>>>
>>> At this point both pir and irr are set and interrupt may be coalesced,
>>> but it is reported as delivered.
>>
>> s/"set pir"/"injected = !t_a_s(pir)"/
>>
>>> So what prevents the scenario above from happening?
>>>
>>> --
>>> Gleb.
Best regards,
Yang
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
