Re: [Qemu-devel] [RFC][PATCH] KVM: Introduce modification context for cpu_synchronize_state

[Anthony Liguori <anthony@codemonkey.ws>]

On 01/27/2010 08:54 AM, Jan Kiszka wrote:
> This patch originates in the mp_state writeback issue: During runtime
> and even on reset, we must not write the previously saved VCPU state
> back into the kernel in an uncontrolled fashion. E.g mp_state should
> only written on reset or on VCPU setup. Certain clocks (e.g. the TSC)
> may only be written on setup or after vmload.
>
> By introducing additional information about the context of the planned
> vcpu state manipulation, we can simply skip sensitive states like
> mp_state when updating the in-kernel state. The planned modifications
> are defined when calling cpu_synchronize_state. They accumulate, ie.
> once a full writeback was requested, it will stick until it was
> performed.
>
> This patch already fixes existing writeback issues in upstream KVM by
> only selectively writing MSR_IA32_TSC, MSR_KVM_SYSTEM_TIME,
> MSR_KVM_WALL_CLOCK, the mp_state and the vcpu_events.
>
> Signed-off-by: Jan Kiszka<jan.kiszka@siemens.com>
>    

I think the context argument makes the function very difficult to call 
correctly.

I'd suggest making CPU_MODIFY_RUNTIME the behaviour of 
cpu_synchronize_state.  I'd suggest adding another function to handle 
init like cpu_init_state().  Likewise, if an explicit reset state is 
needed, I think a cpu_init_state_after_reset() makes sense.

I don't quite understand the context that NONE should be used in.

I think the key point though is to handle RUNTIME mostly transparently 
since it's the most common case.

Regards,

Anthony Liguori