From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Subject: Re: [PATCH v3 07/10] qemu-kvm: Cleanup/fix TSC and PV clock writeback Date: Thu, 25 Feb 2010 09:48:47 +0100 Message-ID: <4B86396F.8070208@web.de> References: <20100224231708.GB16246@amt.cnet> <4B85BA33.5080008@web.de> <20100224234935.GA17862@amt.cnet> <4B85BD22.6050209@web.de> <20100225035814.GA470@amt.cnet> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enig1F61A2C3690F42EE58815099" Cc: Avi Kivity , kvm@vger.kernel.org, qemu-devel@nongnu.org, Gleb Natapov , Zachary Amsden To: Marcelo Tosatti Return-path: Received: from fmmailgate03.web.de ([217.72.192.234]:33259 "EHLO fmmailgate03.web.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752594Ab0BYIti (ORCPT ); Thu, 25 Feb 2010 03:49:38 -0500 In-Reply-To: <20100225035814.GA470@amt.cnet> Sender: kvm-owner@vger.kernel.org List-ID: This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enig1F61A2C3690F42EE58815099 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Marcelo Tosatti wrote: > On Thu, Feb 25, 2010 at 12:58:26AM +0100, Jan Kiszka wrote: >> Marcelo Tosatti wrote: >>> 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 le= vel) >>>>>> set_msr_entry(&msrs[n++], MSR_LSTAR , env->lst= ar); >>>>>> } >>>>>> #endif >>>>>> - set_msr_entry(&msrs[n++], MSR_KVM_SYSTEM_TIME, env->system_t= ime_msr); >>>>>> - set_msr_entry(&msrs[n++], MSR_KVM_WALL_CLOCK, env->wall_cloc= k_msr); >>>>>> + if (level =3D=3D KVM_PUT_FULL_STATE) { >>>>>> + set_msr_entry(&msrs[n++], MSR_IA32_TSC, env->tsc); >>>>>> + set_msr_entry(&msrs[n++], MSR_KVM_SYSTEM_TIME, env->syste= m_time_msr); >>>>>> + set_msr_entry(&msrs[n++], MSR_KVM_WALL_CLOCK, env->wall_c= lock_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 (=3D> >>>> 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). >>> >> OK, need to read this more carefully. >> >> I do not yet understand the difference from user space POV: it tries t= o >> transfer the identical TSC values to all currently stopped VCPU thread= s. >=20 > guest tsc =3D host tsc + offset >=20 > So at the time you set_msr(TSC), the guest visible TSC starts ticking. = > For SMP guests, this does not happen exactly at the same time for all > vcpus. Ouch. >=20 >> That should not be different if we are booting a fresh VM or loading a= >> complete state of a migrated image. If it does, it looks like a KVM >> kernel deficit on first glance. >=20 > Yes it is a deficit. After migration TSCs of SMP guests go out of sync.= > Zachary is working on that. >=20 OK, so we need a workaround, ideally without reintroducing hooks. Is this one acceptable? diff --git a/qemu-kvm-x86.c b/qemu-kvm-x86.c index 84fd7fa..285c05a 100644 --- a/qemu-kvm-x86.c +++ b/qemu-kvm-x86.c @@ -966,7 +966,15 @@ void kvm_arch_load_regs(CPUState *env, int level) } #endif if (level =3D=3D KVM_PUT_FULL_STATE) { - set_msr_entry(&msrs[n++], MSR_IA32_TSC, env->tsc); + /* + * KVM is yet unable to synchronize TSC values of multiple VCPUs= on + * writeback. Until this is fixed, we only write the offset to S= MP + * guests after migration, desynchronizing the VCPUs, but avoidi= ng + * huge jump-backs that would occur without any writeback at all= =2E + */ + if (smp_cpus =3D=3D 1 || env->tsc !=3D 0) { + 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_ms= r); } Jan --------------enig1F61A2C3690F42EE58815099 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.9 (GNU/Linux) Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org iEYEARECAAYFAkuGOXUACgkQitSsb3rl5xSFAACdGFMD32oWju+I/+w+sM5WX40V axIAnicnE03ENYizZAKGvKkBczRQ8JTv =njAG -----END PGP SIGNATURE----- --------------enig1F61A2C3690F42EE58815099--