From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?ISO-8859-1?Q?Fernando_Luis_V=E1zquez_Cao?= Subject: Re: [PATCH] target-i386: clear guest TSC on reset Date: Fri, 06 Dec 2013 17:24:18 +0900 Message-ID: <52A189B2.4060305@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> <52A09EF4.5080800@lab.ntt.co.jp> <20131205161707.GB17277@amt.cnet> <52A0AC09.4090202@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: Gleb Natapov , Will Auld , qemu-devel@nongnu.org, kvm@vger.kernel.org To: Paolo Bonzini , Marcelo Tosatti Return-path: Received: from tama500.ecl.ntt.co.jp ([129.60.39.148]:38100 "EHLO tama500.ecl.ntt.co.jp" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752656Ab3LFIYh (ORCPT ); Fri, 6 Dec 2013 03:24:37 -0500 In-Reply-To: <52A0AC09.4090202@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On 12/06/2013 01:38 AM, Paolo Bonzini wrote: > Il 05/12/2013 17:17, Marcelo Tosatti ha scritto: >>>> 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. When I was writing the original patch I was tempted to do that, but I feared that it could break older kernels that do not have TSC synchronization code. Should we care about such uses (recent QEMU user space + old kernel)? I also wanted to make sure that the initialization that we do in kvm_arch_vcpu_postcreate on power up and the subsequent TSC writeback work well together, but I didn't have time to test it (reading the code, I would say that the TSC generation counter may end up being increased a few times but the TSCs would eventually converge). > If it can be dropped entirely, I certainly have no problem with starting > with a simple patch first. Could we start with the patch that I already sent? It's been tested, it is conservative in the sense that it does the minimum necessary to fix an existing bug, and should be easy to backport. I will be replying to this email with an updated version that has a more appropriate and less scary patch description. I will also be sending a patch that makes the TSC writeback unconditional, but this one should probably be kept on hold until it is properly tested. As a follow-up effort we can work on Paolo's suggestions. Is this an acceptable way forward? Thanks, Fernando