From patchwork Fri Apr 22 10:08:43 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefano Stabellini X-Patchwork-Id: 8908671 Return-Path: X-Original-To: patchwork-xen-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id D46619F39A for ; Fri, 22 Apr 2016 10:11:17 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 03C2A201C8 for ; Fri, 22 Apr 2016 10:11:17 +0000 (UTC) Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 1DE3C201C7 for ; Fri, 22 Apr 2016 10:11:15 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1atY18-0006km-MC; Fri, 22 Apr 2016 10:08:54 +0000 Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1atY16-0006kN-M5 for xen-devel@lists.xenproject.org; Fri, 22 Apr 2016 10:08:52 +0000 Received: from [193.109.254.147] by server-10.bemta-14.messagelabs.com id F8/A9-02972-338F9175; Fri, 22 Apr 2016 10:08:51 +0000 X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFtrMIsWRWlGSWpSXmKPExsVybKJsh67xD8l wg6W7JS2+b5nM5MDocfjDFZYAxijWzLyk/IoE1oyXa6+xF/wRqbix+Q9zA+MOgS5GLg4hgamM EpPPn2CEcDqZJE5efszWxcjBwSKgLTG9JbCLkZODTcBQ4u+TTWBhCSB7yWcOkLCIgLJE76/fL CA2s0C+xIW321lBSoQFnCWOz9QFCXMK2Eu0fHjLDmLzCnhJnN/cyAZiCwnUSVy+sh4sLiqgK3 Ho3x82iBpBiZMzn0CN1JJYPn0bmC0hkCExr2cOK4TtJbHoxiUoW03i6rlNzBMYBWchaZ+FpH0 BI9MqRvXi1KKy1CJdY72kosz0jJLcxMwcXUNDE73c1OLixPTUnMSkYr3k/NxNjMDAZACCHYx3 +5wPMUpyMCmJ8p5/IBkuxJeUn1KZkVicEV9UmpNafIhRhoNDSYI38TtQTrAoNT21Ii0zBxgjM GkJDh4lEV4ZkDRvcUFibnFmOkTqFKOilDhvGkhCACSRUZoH1waLy0uMslLCvIxAhwjxFKQW5W aWoMq/YhTnYFQS5mUCmcKTmVcCN/0V0GImoMX/LoAtLklESEk1MDql3QrY3PQi/eSrnkyu97W zbLkDYyOv7cvi+tbDxDqfQS47+tipvP59VbFZE7+ttCoOu+DLrm3+31T19e/79zc0Xmh48G1q zNfDU0K6N9ewyPKYe92XYTz18pxdQb3ez3thBRsUZhat3BHHxtIjpqA/1yj0rZbFRac1Z724V uibnXlnOS1ooRJLcUaioRZzUXEiAJfTVi/GAgAA X-Env-Sender: sstabellini@kernel.org X-Msg-Ref: server-15.tower-27.messagelabs.com!1461319729!37168446!1 X-Originating-IP: [198.145.29.136] X-SpamReason: No, hits=0.0 required=7.0 tests= X-StarScan-Received: X-StarScan-Version: 8.28; banners=-,-,- X-VirusChecked: Checked Received: (qmail 43830 invoked from network); 22 Apr 2016 10:08:51 -0000 Received: from mail.kernel.org (HELO mail.kernel.org) (198.145.29.136) by server-15.tower-27.messagelabs.com with DHE-RSA-AES256-GCM-SHA384 encrypted SMTP; 22 Apr 2016 10:08:51 -0000 Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 92E44201B4; Fri, 22 Apr 2016 10:08:48 +0000 (UTC) Received: from [10.0.0.5] (107.238.189.80.dyn.plus.net [80.189.238.107]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id BDB7C20160; Fri, 22 Apr 2016 10:08:46 +0000 (UTC) Date: Fri, 22 Apr 2016 11:08:43 +0100 (BST) From: Stefano Stabellini X-X-Sender: sstabellini@sstabellini-ThinkPad-X260 To: Jan Beulich In-Reply-To: <571A092902000078000E4984@prv-mh.provo.novell.com> Message-ID: References: <571A092902000078000E4984@prv-mh.provo.novell.com> User-Agent: Alpine 2.10 (DEB 1266 2009-07-14) MIME-Version: 1.0 X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Cc: andrew.cooper3@citrix.com, Stefano Stabellini , xen-devel@lists.xenproject.org Subject: Re: [Xen-devel] [PATCH] xen/time: fix system_time for vtsc=1 PV guests X-BeenThere: xen-devel@lists.xen.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" X-Virus-Scanned: ClamAV using ClamSMTP On Fri, 22 Apr 2016, Jan Beulich wrote: > >>> On 21.04.16 at 15:29, wrote: > > --- a/xen/arch/x86/time.c > > +++ b/xen/arch/x86/time.c > > @@ -784,7 +784,7 @@ static void __update_vcpu_system_time(struct vcpu *v, int force) > > struct cpu_time *t; > > struct vcpu_time_info *u, _u = {}; > > struct domain *d = v->domain; > > - s_time_t tsc_stamp; > > + s_time_t stime_stamp, tsc_stamp = 0; > > I don't see why the initializer needs adding here. Ops, sorry, I developed the patch against 4.6, the useless initialization derives from it. > > @@ -807,7 +808,11 @@ static void __update_vcpu_system_time(struct vcpu *v, int force) > > tsc_stamp = -gtime_to_gtsc(d, -stime); > > } > > else > > + { > > tsc_stamp = gtime_to_gtsc(d, stime); > > + if (!tsc_stamp) > > Coding style. > > > + stime_stamp = d->arch.vtsc_offset; > > + } > > While I can see this being the right thing for getting the two stamps > in sync, is that really helping the guest? Time ought to be not moving > forward until getting past vtsc_offset afaict, and that can't be good. It helps a lot in my test case: without this Linux hangs due to lost timer interrupts (because they are set in the past). > I.e. it would seem to me that it's gtime_to_gtsc() that needs > adjustment to properly deal with time < d->arch.vtsc_offset. I agree that it would be nice to fix gtime_to_gtsc, but how do you suggest to do it? > Plus I can't see why, in the worst case, the gTSC value can't be > wrapped through zero into negative (or really huge positive) range: > Such TSC values are certainly not invalid, and guests shouldn't really > make assumptions on TSC values being in the small positive range > when they boot. Am I understanding correctly that you are suggesting to let the subtraction in gtime_to_gtsc return a negative -- actually a wrapped around positive? Something like: > Also, looking at all the involved code, I once again wonder whether > all the is_hvm_*() there shouldn't be has_hvm_container_*(). diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c index 7a01c90..896fd9f 100644 --- a/xen/arch/x86/time.c +++ b/xen/arch/x86/time.c @@ -1757,8 +1757,8 @@ custom_param("tsc", tsc_parse); u64 gtime_to_gtsc(struct domain *d, u64 time) { if ( !is_hvm_domain(d) ) - time = max_t(s64, time - d->arch.vtsc_offset, 0); - return scale_delta(time, &d->arch.ns_to_vtsc); + time = time - d->arch.vtsc_offset; + return scale_delta(time2, &d->arch.ns_to_vtsc); } Unfortunately that wouldn't solve the problem because of the scaling.