From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43)
	id 1NasBb-0007Z0-IK
	for qemu-devel@nongnu.org; Fri, 29 Jan 2010 09:51:03 -0500
Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43)
	id 1NasBW-0007S1-Gh
	for qemu-devel@nongnu.org; Fri, 29 Jan 2010 09:51:02 -0500
Received: from [199.232.76.173] (port=33184 helo=monty-python.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1NasBW-0007Ro-Do
	for qemu-devel@nongnu.org; Fri, 29 Jan 2010 09:50:58 -0500
Received: from mail-yw0-f173.google.com ([209.85.211.173]:38794)
	by monty-python.gnu.org with esmtp (Exim 4.60)
	(envelope-from <anthony@codemonkey.ws>) id 1NasBV-00034b-W6
	for qemu-devel@nongnu.org; Fri, 29 Jan 2010 09:50:58 -0500
Received: by ywh3 with SMTP id 3so1723274ywh.22
	for <qemu-devel@nongnu.org>; Fri, 29 Jan 2010 06:50:57 -0800 (PST)
Message-ID: <4B62F5CD.2040709@codemonkey.ws>
Date: Fri, 29 Jan 2010 08:50:53 -0600
From: Anthony Liguori <anthony@codemonkey.ws>
MIME-Version: 1.0
Subject: Re: [Qemu-devel] [RFC][PATCH] KVM: Introduce modification context
	for cpu_synchronize_state
References: <4B605390.9020709@siemens.com>
In-Reply-To: <4B605390.9020709@siemens.com>
Content-Type: text/plain; charset=ISO-8859-15; format=flowed
Content-Transfer-Encoding: 7bit
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.gnu.org/pipermail/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Jan Kiszka <jan.kiszka@siemens.com>
Cc: Anthony Liguori <aliguori@us.ibm.com>, kvm <kvm@vger.kernel.org>, Gleb Natapov <gleb@redhat.com>, Marcelo Tosatti <mtosatti@redhat.com>, Alexander Graf <agraf@suse.de>, qemu-devel <qemu-devel@nongnu.org>, Avi Kivity <avi@redhat.com>

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