From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:39702)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1VobNZ-00040y-H8
	for qemu-devel@nongnu.org; Thu, 05 Dec 2013 11:02:23 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1VobNT-0002UH-I9
	for qemu-devel@nongnu.org; Thu, 05 Dec 2013 11:02:17 -0500
Received: from mx1.redhat.com ([209.132.183.28]:43396)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1VobNT-0002U4-9H
	for qemu-devel@nongnu.org; Thu, 05 Dec 2013 11:02:11 -0500
Message-ID: <52A0A37A.7060204@redhat.com>
Date: Thu, 05 Dec 2013 17:02:02 +0100
From: Paolo Bonzini <pbonzini@redhat.com>
MIME-Version: 1.0
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>
In-Reply-To: <52A09EF4.5080800@lab.ntt.co.jp>
Content-Type: text/plain; charset=Shift_JIS
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH] target-i386: clear guest TSC on reset
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Fernando Luis Vazquez Cao <fernando_b1@lab.ntt.co.jp>
Cc: Gleb Natapov <gleb@kernel.org>, Will Auld <will.auld@intel.com>, Marcelo Tosatti <mtosatti@redhat.com>, qemu-devel@nongnu.org, kvm@vger.kernel.org

Il 05/12/2013 16:42, Fernando Luis Vazquez Cao ha scritto:
> (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?

No, probably not.

> 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.

It does some harm in that it introduces a case where KVM_PUT_RESET_STATE
restores something, but KVM_PUT_FULL_STATE doesn't.

If it really usually works, there shouldn't be a need for this "if"
statement at all.

Marcelo, what do you think?

Paolo