From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43818) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VobcO-0003vu-CE for qemu-devel@nongnu.org; Thu, 05 Dec 2013 11:17:42 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VobcF-0007NQ-G0 for qemu-devel@nongnu.org; Thu, 05 Dec 2013 11:17:36 -0500 Received: from mx1.redhat.com ([209.132.183.28]:20267) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VobcF-0007NF-6p for qemu-devel@nongnu.org; Thu, 05 Dec 2013 11:17:27 -0500 Date: Thu, 5 Dec 2013 14:17:07 -0200 From: Marcelo Tosatti Message-ID: <20131205161707.GB17277@amt.cnet> References: <1386054500.25757.10.camel@nexus> <529D90A6.2080801@lab.ntt.co.jp> <52A0186A.2050207@lab.ntt.co.jp> <1386224104.3091.3.camel@nexus> <52A04732.4040105@redhat.com> <52A07C5A.9090105@lab.ntt.co.jp> <52A08541.6090702@redhat.com> <52A09EF4.5080800@lab.ntt.co.jp> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <52A09EF4.5080800@lab.ntt.co.jp> Subject: Re: [Qemu-devel] [PATCH] target-i386: clear guest TSC on reset List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fernando Luis Vazquez Cao Cc: Gleb Natapov , Paolo Bonzini , Will Auld , qemu-devel@nongnu.org, kvm@vger.kernel.org On Fri, Dec 06, 2013 at 12:42:44AM +0900, Fernando Luis Vazquez Cao wrote: > (2013/12/05 22:53), Paolo Bonzini wrote: > > Il 05/12/2013 14:15, Fernando Luis Vazquez Cao ha scritto: > >> /* > >> * KVM is yet unable to synchronize TSC values of multiple VCPUs on > >> * writeback. Until this is fixed, we only write the offset to SMP > >> * guests after migration, desynchronizing the VCPUs, but avoiding > >> * huge jump-backs that would occur without any writeback at all. > >> */ > >> - if (smp_cpus == 1 || env->tsc != 0) { > >> + if (smp_cpus == 1 || env->tsc != 0 || level == KVM_PUT_RESET_STATE) { > >> kvm_msr_entry_set(&msrs[n++], MSR_IA32_TSC, env->tsc); > >> } > > This is still a bit ugly, and desynchronizes the VCPUs on reset. > > I agree it is a bit ugly, but in my testing QEMU seemed to loop over all > the VCPUS fast enough for the kernel side kvm_write_tsc() to do a > reasonable job of matching the offsets (the Linux guest did not mark > the TSC unstable due to the TSCs being unsynchronized). Am I missing > something? Right, modern kernels (see kvm_write_tsc) perform synchronization, so in theory the "KVM is yet unable to synchronize ..." code is not necessary anymore. I vote for dropping the thing entirely. > > The main point of my outlined solution is that you only have one value > > that is tracked, not one per VCPU (which in the case of migration adds > > unpredictable latencies---for example due to emptying the migration > > buffers). We already save that value; all that's left is to use it > > instead of env->tsc. > > I understand the benefits of what you are proposing but, since it is > wider is scope and it would be more difficult to backport, I would > prefer to implement it as a follow-up patch, unless you think that > the current patch as a standalone fix does more harm than good. > > - Fernando