From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marcelo Tosatti Subject: Re: [PATCH v3 07/10] qemu-kvm: Cleanup/fix TSC and PV clock writeback Date: Wed, 24 Feb 2010 20:49:35 -0300 Message-ID: <20100224234935.GA17862@amt.cnet> References: <20100224231708.GB16246@amt.cnet> <4B85BA33.5080008@web.de> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Avi Kivity , kvm@vger.kernel.org, qemu-devel@nongnu.org, Gleb Natapov , Zachary Amsden To: Jan Kiszka Return-path: Received: from mx1.redhat.com ([209.132.183.28]:59137 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758420Ab0BXXtr (ORCPT ); Wed, 24 Feb 2010 18:49:47 -0500 Content-Disposition: inline In-Reply-To: <4B85BA33.5080008@web.de> Sender: kvm-owner@vger.kernel.org List-ID: On Thu, Feb 25, 2010 at 12:45:55AM +0100, Jan Kiszka wrote: > Marcelo Tosatti wrote: > > On Wed, Feb 24, 2010 at 03:17:55PM +0100, Jan Kiszka wrote: > >> Drop kvm_load_tsc in favor of level-dependent writeback in > >> kvm_arch_load_regs. KVM's PV clock MSRs fall in the same category and > >> should therefore only be written back on full sync. > >> > >> Signed-off-by: Jan Kiszka > >> --- > >> qemu-kvm-x86.c | 19 +++++-------------- > >> qemu-kvm.h | 4 ---- > >> target-i386/machine.c | 5 ----- > >> 3 files changed, 5 insertions(+), 23 deletions(-) > >> > >> diff --git a/qemu-kvm-x86.c b/qemu-kvm-x86.c > >> index 840c1c9..84fd7fa 100644 > >> --- a/qemu-kvm-x86.c > >> +++ b/qemu-kvm-x86.c > >> @@ -965,8 +965,11 @@ void kvm_arch_load_regs(CPUState *env, int level) > >> set_msr_entry(&msrs[n++], MSR_LSTAR , env->lstar); > >> } > >> #endif > >> - set_msr_entry(&msrs[n++], MSR_KVM_SYSTEM_TIME, env->system_time_msr); > >> - set_msr_entry(&msrs[n++], MSR_KVM_WALL_CLOCK, env->wall_clock_msr); > >> + if (level == KVM_PUT_FULL_STATE) { > >> + set_msr_entry(&msrs[n++], MSR_IA32_TSC, env->tsc); > >> + set_msr_entry(&msrs[n++], MSR_KVM_SYSTEM_TIME, env->system_time_msr); > >> + set_msr_entry(&msrs[n++], MSR_KVM_WALL_CLOCK, env->wall_clock_msr); > >> + } > > > > As things stand today, the TSC should only be written on migration. See > > 53f658b3c33616a4997ee254311b335e59063289 in the kernel. > > Migration and power-up - that's what this patch ensures (=> > KVM_PUT_FULL_STATE). Or where do you see any problem? > > Jan > The problem is it should not write on power up (the kernel attempts to synchronize the TSCs in that case, see the commit).