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

Avi Kivity <[email protected]> Sun, 16 May 2010 09:47:46 +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:35 PM, Alexander Graf wrote:
>
>>
>>> 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.
>>      
> It's not?

With s/signals/IPIs/.

> On signals we always exit to userspace, no?
>    

Yes (if the signal isn't blocked).

>>> * 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.
>>      
> Please elaborate.
>    

1: vcpu_run
2: KVM_INTERRUPT
2k: sets flag, if msr.ee IPIs 1 or wakes up 1 if halted
1k: notices flag, if msr.ee injects interrupt
...
1g: acks
1k: forwards ack to userspace
1: completes interrupt


>>> 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.
>>      
> For everyone except for the vcpu thread executing the interrupt, it's asynchronous, right?

For everyone other than the vcpu thread, it's off limits.  kvm_run is 
only read on KVM_RUN entries and written on KVM_RUN exits.

> The same applies to an in-kernel pic.
>    

The in-kernel pic doesn't use kvm_run.


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