From mboxrd@z Thu Jan 1 00:00:00 1970 From: Boris Ostrovsky Subject: Re: [PATCH 01/13] x86/time.c: Use system time to calculate elapsed_nsec in tsc_get_info() Date: Fri, 9 Oct 2015 12:44:29 -0400 Message-ID: <5617EEED.60101@oracle.com> References: <1443424438-13404-1-git-send-email-haozhong.zhang@intel.com> <1443424438-13404-2-git-send-email-haozhong.zhang@intel.com> <5617801402000078000A992C@prv-mh.provo.novell.com> <5617C410.2090208@oracle.com> <5617EDDD02000078000A9C65@prv-mh.provo.novell.com> <5617DF22.8070602@oracle.com> <20151009163900.GA23023@hzzhang-OptiPlex-9020.sh.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20151009163900.GA23023@hzzhang-OptiPlex-9020.sh.intel.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich , Aravind Gopalakrishnan , Suravee Suthikulpanit , Andrew Cooper , Jun Nakajima , Kevin Tian , xen-devel@lists.xen.org, Keir Fraser List-Id: xen-devel@lists.xenproject.org On 10/09/2015 12:39 PM, Haozhong Zhang wrote: > On Fri, Oct 09, 2015 at 11:37:06AM -0400, Boris Ostrovsky wrote: >> On 10/09/2015 10:39 AM, Jan Beulich wrote: >>>>>> On 09.10.15 at 15:41, wrote: >>>> On 10/09/2015 02:51 AM, Jan Beulich wrote: >>>>>>>> On 28.09.15 at 09:13, wrote: >>>>>> When the TSC mode of a domain is TSC_MODE_DEFAULT and no TSC emulation >>>>>> is used, the existing tsc_get_info() calculates elapsed_nsec by scaling >>>>>> the host TSC with a ratio between guest TSC rate and >>>>>> nanoseconds. However, the result will be incorrect if the guest TSC rate >>>>>> differs from the host TSC rate. This patch fixes this problem by using >>>>>> the system time as elapsed_nsec. >>>>> For both this and patch 2, while at a first glance (and taking into >>>>> account just the visible patch context) what you say seems to >>>>> make sense, the explanation is far from sufficient namely when >>>>> looking at the function as a whole. For one, effects on existing >>>>> cases need to be explicitly described, in particular why SVM's TSC >>>>> ratio code works without that change (or whether it has been >>>>> broken all along, in which case these would become backporting >>>>> candidates; input from SVM maintainers would be appreciated >>>>> too). That may in particular mean being more specific about >>>>> what is actually wrong with scaling the host TSC here (i.e. in >>>>> which way both results differ), when supposedly that matches >>>>> what the hardware does when TSC ratio is supported. >>>> If elapsed_nsec is the time that guest has been running then how can >>>> get_s_time(), which is system time, be the right answer here? But what >>>> confuses me even more is that existing code is not doing that neither. >>>> >>>> Shouldn't elapsed_nsec be offset by d->arch.vtsc_offset on the get side? >>>> I.e. >>>> >>>> *elapsed_nsec = get_s_time() - d->arch.vtsc_offset? >>> Doesn't whether or not to adjust be the offset depend on d-arch.vtsc? >> We only use elapsed_nsec when vtsc is set, I think. In native case (vtsc=0) >> elapsed_nsec and d->arch.vtsc_offset are ignored. >> > But it is used in tsc_set_info() if a HVM domain in TSC_MODE_DEFAULT > is migrated to a machine and the following if condition in > tsc_set_info() is false. > > if ( tsc_mode == TSC_MODE_DEFAULT && host_tsc_is_safe() && > (has_hvm_container_domain(d) ? > d->arch.tsc_khz == cpu_khz || cpu_has_tsc_ratio : > incarnation == 0) ) Ah, yes, then we do need to save it. -boris