From mboxrd@z Thu Jan 1 00:00:00 1970 From: Fernando Luis Vazquez Cao Subject: Re: [PATCH] target-i386: clear guest TSC on reset Date: Fri, 06 Dec 2013 00:42:44 +0900 Message-ID: <52A09EF4.5080800@lab.ntt.co.jp> 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> Mime-Version: 1.0 Content-Type: text/plain; charset=Shift_JIS Content-Transfer-Encoding: 7bit Cc: Gleb Natapov , Will Auld , qemu-devel@nongnu.org, kvm@vger.kernel.org, Marcelo Tosatti To: Paolo Bonzini Return-path: Received: from tama500.ecl.ntt.co.jp ([129.60.39.148]:59065 "EHLO tama500.ecl.ntt.co.jp" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932718Ab3LEPnD (ORCPT ); Thu, 5 Dec 2013 10:43:03 -0500 In-Reply-To: <52A08541.6090702@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: (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? > 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