Re: [PATCH 0/7] Consolidate vcpu ioctl locking

Avi Kivity <[email protected]> Sun, 16 May 2010 09:09:58 +0000
Newsgroups org.kernel.vger.kvm-ia64,org.kernel.vger.kvm,org.kernel.vger.kvm-ppc
Message-ID <[email protected]>
On 05/16/2010 12:01 PM, Alexander Graf wrote:
>
>> That's what the world looked like in 2006.
>>
>> We could change it, but there's not much point, since having the local apic in the kernel is pretty much a requirement for reasonable performance.
>>      
> Well, I'm not convinced yet that's the case for PPC as well. The timer is in-cpu anyways and I don't see why IPIs should be slow with a userspace pic - if we keep the overhead low.
>    

If it's at all possible keep the mpic out.  I am _not_ advocating 
pushing ppc's mpic into the kernel.

> So let me think this through. With remote interrupt injection we have.
>
> * thread 1 does vcpu_run
> * thread 2 triggers KVM_INTERRUPT on fd
> * thread 2 signals thread 1 so we're sure the interrupt gets injected
> * thread 1 exits into qemu
>    

This doesn't seem necessary.  The kernel can own the interrupt line, so 
it remembers it from the last KVM_INTERRUPT.

> * thread 1 goes back into the vcpu, triggering an interrupt
>
> Without we have:
>
> * thread 1 does vcpu_run
> * thread 2 wants to trigger an an interrupt, sets the qemu internal bit
> * thread 2 signals thread 1 so we're sure the interrupt gets processed
> * thread 1 exits into qemu
> * thread 1 triggers KVM_INTERRUPT on fd
> * thread 1 goes into the vcpu
>
> So we don't really buy anything from doing the remote injection. Hrm.
>    

Not if you make interrupt injection a lightweight exit.

> What's somewhat striking me here though is - why do we need KVM_INTERRUPT when there's all those kvm_run fields? Can't we just do interrupt injection by setting run->trigger_interrupt? There's only a single "interrupt line" on the CPU anyways. That way we'd save the ioctl and get rid of the locking problem altogether.
>    

That's what x86 does.  However, it's synchronous.

-- 
error compiling committee.c: too many arguments to function