From: Anthony Liguori <anthony@codemonkey.ws>
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>
Subject: Re: [RFC][PATCH] KVM: Introduce modification context for cpu_synchronize_state
Date: Fri, 29 Jan 2010 08:50:53 -0600 [thread overview]
Message-ID: <4B62F5CD.2040709@codemonkey.ws> (raw)
In-Reply-To: <4B605390.9020709@siemens.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
WARNING: multiple messages have this Message-ID (diff)
From: Anthony Liguori <anthony@codemonkey.ws>
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>
Subject: Re: [Qemu-devel] [RFC][PATCH] KVM: Introduce modification context for cpu_synchronize_state
Date: Fri, 29 Jan 2010 08:50:53 -0600 [thread overview]
Message-ID: <4B62F5CD.2040709@codemonkey.ws> (raw)
In-Reply-To: <4B605390.9020709@siemens.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
next prev parent reply other threads:[~2010-01-29 14:50 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-01-27 14:54 [RFC][PATCH] KVM: Introduce modification context for cpu_synchronize_state Jan Kiszka
2010-01-27 14:54 ` [Qemu-devel] " Jan Kiszka
2010-01-27 20:53 ` Marcelo Tosatti
2010-01-27 20:53 ` [Qemu-devel] " Marcelo Tosatti
2010-01-28 8:52 ` Jan Kiszka
2010-01-28 8:52 ` [Qemu-devel] " Jan Kiszka
2010-01-29 14:50 ` Anthony Liguori [this message]
2010-01-29 14:50 ` [Qemu-devel] " Anthony Liguori
2010-01-29 15:25 ` Jan Kiszka
2010-01-29 15:25 ` Jan Kiszka
2010-01-29 15:44 ` Anthony Liguori
2010-01-29 15:44 ` Anthony Liguori
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4B62F5CD.2040709@codemonkey.ws \
--to=anthony@codemonkey.ws \
--cc=agraf@suse.de \
--cc=aliguori@us.ibm.com \
--cc=avi@redhat.com \
--cc=gleb@redhat.com \
--cc=jan.kiszka@siemens.com \
--cc=kvm@vger.kernel.org \
--cc=mtosatti@redhat.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.