Re: [PATCH v2] KVM: x86: Extend KVM_SET_VCPU_EVENTS with selective updates

[Jan Kiszka <jan.kiszka@web.de>]

[-- Attachment #1: Type: text/plain, Size: 1985 bytes --]

Marcelo Tosatti wrote:
> On Tue, Dec 08, 2009 at 04:07:32PM +0200, Avi Kivity wrote:
>> On 12/08/2009 04:02 PM, Marcelo Tosatti wrote:
>>> On Sun, Dec 06, 2009 at 06:24:15PM +0100, Jan Kiszka wrote:
>>>    
>>>> User space may not want to overwrite asynchronously changing VCPU event
>>>> states on write-back. So allow to skip nmi.pending and sipi_vector by
>>>> setting corresponding bits in the flags field of kvm_vcpu_events.
>>>>
>>>> Signed-off-by: Jan Kiszka<jan.kiszka@siemens.com>
>>>>      
>>> Can't you handle this in userspace entirely, only updating vcpu_events
>>> state when appropriate?
>>>    
>> For what we do now I think you're right, it can be handled in userspace.
>>
>> But in general, there's currently no way to update vcpu_events without  
>> overwriting nmi and sipi_vector, which can also be written concurrently  
>> by other vcpus.  So there's a hole in the interface.
>>
>>> Shouldnt the vcpu be stopped in the first place, when its state is
>>> updated?
>>>    
>> It is stopped, but other vcpus are not.
> 
> I don't see the need for setting any state in kvm_vcpu_events
> automatically, on kernel entry (apparently there was consensus that
> saving similar state explicitly in qemu was the way to go).

(I don't think so. IMHO the cleaner way is to avoid loading critical
states unless we are resetting or vmloading.)

> 
> kvm_arch_put_registers in qemu saves mpstate now that way,
> and the same problem is present.
> 
> The sites to load vcpu_events would be machine reset and cpu_load
> only, right?

That is how qemu use it, currently. But this interface should be
designed with more flexibility. For the (yet theoretical) case you want
to update RIP of a single VCPU, you also have to reset all the
context-related states but maybe not the asynchronously changing ones
like nmi.pending. We have no such use case yet, but KVM should not
prevent them by design (if the change is so trivial).

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]