Re: [PATCH] KVM: avoid taking ioapic mutex for non-ioapic EOIs

[Marcelo Tosatti <mtosatti@redhat.com>]

On Mon, Dec 28, 2009 at 10:47:20PM +0200, Avi Kivity wrote:
> On 12/28/2009 10:37 PM, Marcelo Tosatti wrote:
>> On Mon, Dec 28, 2009 at 02:08:30PM +0200, Avi Kivity wrote:
>>    
>>> When the guest acknowledges an interrupt, it sends an EOI message to the local
>>> apic, which broadcasts it to the ioapic.  To handle the EOI, we need to take
>>> the ioapic mutex.
>>>
>>> On large guests, this causes a lot of contention on this mutex.  Since large
>>> guests usually don't route interrupts via the ioapic (they use msi instead),
>>> this is completely unnecessary.
>>>
>>> Avoid taking the mutex by introducing a handled_vectors bitmap.  Before taking
>>> the mutex, check if the ioapic was actually responsible for the acked vector.
>>> If not, we can return early.
>>>      
>> Can't you skip IOAPIC EOI for edge triggered interrupts (in the LAPIC
>> code), instead?
>>    
>
> That's a lot cleaner, yes.  Indeed there's the TMR which holds this  
> info.  Gleb suggested doing this in the local apic but we didn't think  
> of using the TMR.

Problem with storing in the LAPIC is you have to migrate the bitmap
along (otherwise can't know if EOI is from MSI or IOAPIC). But it sounds
much simpler.

> There's a small race there - the TMR is set after the IRR, so the  
> interrupt can be injected and acked before the TMR is updated, but that  
> can be fixed by switching the order.

Makes sense.

> But what about kvm_notify_acked_irq() in __kvm_ioapic_update_eoi()?

Oops.

The worrying thing about the handled_vectors bitmap in the IOAPIC is
that the update is not atomic wrt to lapic EOI handler.

Unless its certain that races there are the guests problem, which should
have proper locking to never allow things like

kvm_set_ioapic vec
                        update handled bitmap, vec not IOAPIC
                        handled anymore
ack lapic irq vec

to happen.

(with bitmap in LAPIC you avoid those things).