From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paolo Bonzini Subject: Re: [PATCH v2 00/23] MIPS: KVM: Fixes and guest timer rewrite Date: Fri, 30 May 2014 09:57:18 +0200 Message-ID: <538839DE.3000804@redhat.com> References: <1401355005-20370-1-git-send-email-james.hogan@imgtec.com> <53870DAD.7050900@redhat.com> <53874719.5070604@imgtec.com> <538750F8.7040202@redhat.com> <53875FEE.1020607@imgtec.com> <53876850.20600@redhat.com> <53879C3E.3040102@imgtec.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-15; format=flowed Content-Transfer-Encoding: 7bit Cc: Andreas Herrmann , Gleb Natapov , kvm@vger.kernel.org, Ralf Baechle , linux-mips@linux-mips.org, David Daney , Sanjay Lal To: James Hogan Return-path: Received: from mail-wi0-f176.google.com ([209.85.212.176]:41933 "EHLO mail-wi0-f176.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754865AbaE3H5Y (ORCPT ); Fri, 30 May 2014 03:57:24 -0400 Received: by mail-wi0-f176.google.com with SMTP id n15so612829wiw.3 for ; Fri, 30 May 2014 00:57:22 -0700 (PDT) In-Reply-To: <53879C3E.3040102@imgtec.com> Sender: kvm-owner@vger.kernel.org List-ID: Il 29/05/2014 22:44, James Hogan ha scritto: > Yes, I agree with your analysis and had considered something like this, > although it doesn't particularly appeal to my sense of perfectionism :). I can see that. But I think the simplification of the code is worth it. It is hard to explain why the invalid times-goes-backwards case can happen if env->count_save_time is overwritten with data from another machine. I think the explanation is that (due to timers_state.cpu_ticks_enabled) the value of "cpu_get_clock_at(now) - env->count_save_time" does not depend on get_clock(), but the code doesn't have any comment for that. Rather than adding comments, we might as well force it to be always zero and just write get_clock() to COUNT_RESUME. Finally, having to serialize env->count_save_time makes harder to support migration from TCG to KVM and back. > It would be race free though, and if you're stopping the VM at all you > expect to lose some time anyway. Since you mentioned perfectionism, :) your code also loses some time; COUNT_RESUME is written based on when the CPU state becomes clean, not on when the CPU was restarted. Paolo