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 09:41:36 -0400 Message-ID: <5617C410.2090208@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> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <5617801402000078000A992C@prv-mh.provo.novell.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 , Haozhong Zhang Cc: Kevin Tian , Keir Fraser , Jun Nakajima , Andrew Cooper , xen-devel@lists.xen.org, Aravind Gopalakrishnan , Suravee Suthikulpanit List-Id: xen-devel@lists.xenproject.org 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? -boris > > Then a reason needs to be given why the similar logic in the > PVRDTSCP case does not also get adjusted. > > Plus, looking at the respective code in tsc_set_info(), I'm > getting the impression that what you're trying to do is not in line > with what is intended so far: Especially the comment there > suggests that the intention is for the guest TSC to be made > match the host one. Considering migration this indeed looks > suspicious, but then that would need changing too. > > Jan > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel