From patchwork Mon Apr 25 12:19:49 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefano Stabellini X-Patchwork-Id: 8926451 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 6BB479F372 for ; Mon, 25 Apr 2016 12:22:42 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 6F4C3201D3 for ; Mon, 25 Apr 2016 12:22:41 +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 8A8C820142 for ; Mon, 25 Apr 2016 12:22:40 +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 1aufV9-0008Qs-F2; Mon, 25 Apr 2016 12:20:31 +0000 Received: from mail6.bemta6.messagelabs.com ([85.158.143.247]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1aufV8-0008Qk-W9 for xen-devel@lists.xenproject.org; Mon, 25 Apr 2016 12:20:31 +0000 Received: from [85.158.143.35] by server-1.bemta-6.messagelabs.com id BE/45-18833-E8B0E175; Mon, 25 Apr 2016 12:20:30 +0000 X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFtrCIsWRWlGSWpSXmKPExsVybKJsh24vt1y 4we93Bhbft0xmcmD0OPzhCksAYxRrZl5SfkUCa8aMrzMZC9ZJVizqucnUwLhXpIuRi0NIYCqj xML1/9kgnE4mieMfJzJ3MXJysAhoS3z+v58RxGYTMJT4+2QTUBEHhwSQveQzB0hYREBZovfXb xYQm1mgj1Gi4UcpiC0s4CoxYUUrK0g5p4C9RM8NHpAwr4C3xJP2bUwgtpBAncTHh4/ZQGxRAV 2JQ//+sEHUCEqcnPkEaqSWxPLp28BsCYEMiXk9c1ghbC+JRTcuQdlqElfPbWKewCg4C0n7LCT tCxiZVjGqF6cWlaUW6RrpJRVlpmeU5CZm5ugaGpjp5aYWFyemp+YkJhXrJefnbmIEhiYDEOxg XPbX6RCjJAeTkiivwy/ZcCG+pPyUyozE4oz4otKc1OJDjDIcHEoSvDVccuFCgkWp6akVaZk5w CiBSUtw8CiJ8B7hBErzFhck5hZnpkOkTjEqSonzfgdJCIAkMkrz4NpgkXmJUVZKmJcR6BAhno LUotzMElT5V4ziHIxKwrztINt5MvNK4Ka/AlrMBLT48iFZkMUliQgpqQbG2X2+pyZG5W0INFn htnCFxJGqg+7h3z9WONTHdx/srDVvuhep+OrME9mc0y8Wv3yw88/OewFyotuXRVe/f27gd2Wb nk1Rdbd0z57rD0olnzhzaTa/eMmxkueqeC73123WZQE+u/OnbjLV04+9zbQwoN+09+qh4rduN e7Tj0zZ7b1u1uOMownHlViKMxINtZiLihMB/Fb9YscCAAA= X-Env-Sender: sstabellini@kernel.org X-Msg-Ref: server-8.tower-21.messagelabs.com!1461586828!11087573!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.34; banners=-,-,- X-VirusChecked: Checked Received: (qmail 14924 invoked from network); 25 Apr 2016 12:20:29 -0000 Received: from mail.kernel.org (HELO mail.kernel.org) (198.145.29.136) by server-8.tower-21.messagelabs.com with DHE-RSA-AES256-GCM-SHA384 encrypted SMTP; 25 Apr 2016 12:20:29 -0000 Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 0C1D620204; Mon, 25 Apr 2016 12:20:24 +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 E4389201EF; Mon, 25 Apr 2016 12:20:20 +0000 (UTC) Date: Mon, 25 Apr 2016 13:19:49 +0100 (BST) From: Stefano Stabellini X-X-Sender: sstabellini@sstabellini-ThinkPad-X260 To: Jan Beulich In-Reply-To: <571E243E02000078000E550D@prv-mh.provo.novell.com> Message-ID: References: <571E243E02000078000E550D@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 , Wei Liu , xen-devel@lists.xenproject.org Subject: Re: [Xen-devel] [PATCH] xen/time: fix gtime_to_gtsc 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 Mon, 25 Apr 2016, Jan Beulich wrote: > >>> On 25.04.16 at 13:18, wrote: > > From: Jan Beulich > > > > For vtsc=1 PV guests, rdtsc is trapped and calculated from get_s_time() > > using gtime_to_gtsc. Similarly the tsc_timestamp, part of struct > > vcpu_time_info, is calculated from stime_local_stamp using > > gtime_to_gtsc. > > > > However gtime_to_gtsc can return 0, if time < vtsc_offset, which can > > actually happen when gtime_to_gtsc is called passing stime_local_stamp > > (the caller function is __update_vcpu_system_time). > > > > In that case the pvclock protocol doesn't work properly and the guest is > > unable to calculate the system time correctly. As a consequence when the > > guest tries to set a timer event (for example calling the > > VCPUOP_set_singleshot_timer hypercall), the event will be in the past > > causing Linux to hang. > > > > The purpose of the pvclock protocol is to allow the guest to calculate > > the system_time in nanosec correctly. The guest calculates as follow: > > > > from_vtsc_scale(rdtsc - vcpu_time_info.tsc_timestamp) + > > vcpu_time_info.system_time > > > > Given that with vtsc=1: > > rdtsc = to_vtsc_scale(NOW() - vtsc_offset) > > vcpu_time_info.tsc_timestamp = to_vtsc_scale(vcpu_time_info.system_time - > > vtsc_offset) > > > > The expression evaluates to NOW(), which is what we want. However when > > stime_local_stamp < vtsc_offset, vcpu_time_info.tsc_timestamp is > > actually 0. As a consequence the calculated overall system_time is not > > correct. > > > > This patch fixes the issue by letting gtime_to_gtsc return a negative > > integer in the form of a wrapped around unsigned integer, thus when the > > guest subtracts vcpu_time_info.tsc_timestamp from rdtsc will calculate > > the right value. > > > > Signed-off-by: Jan Beulich > > Signed-off-by: Stefano Stabellini > > Assuming you mean for this to go into 4.7, I've added Wei to Cc > (and you should do so in case of re-submission). > > > --- a/xen/arch/x86/time.c > > +++ b/xen/arch/x86/time.c > > @@ -1663,7 +1663,13 @@ 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); > > This line should have been deleted. While I'd be happy to do this > while committing, its presence raises the question of whether > things actually work as expected. A mistake forward-porting the patch from 4.6. Sorry. I tested the code again and works correctly. The patch should be: diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c index 687e39b..6438b47 100644 --- a/xen/arch/x86/time.c +++ b/xen/arch/x86/time.c @@ -1663,7 +1663,12 @@ 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); + { + if ( time < d->arch.vtsc_offset ) + return -scale_delta(d->arch.vtsc_offset - time, + &d->arch.ns_to_vtsc); + time -= d->arch.vtsc_offset; + } return scale_delta(time, &d->arch.ns_to_vtsc); }