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.
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 <mtosa...@redhat.com> 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.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
