[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 01/13] x86/time.c: Use system time to calculate elapsed_nsec in tsc_get_info()



On 10/09/2015 11:11 AM, Jan Beulich wrote:
On 09.10.15 at 16:00, <haozhong.zhang@xxxxxxxxx> wrote:
On Fri, Oct 09, 2015 at 09:41:36AM -0400, Boris Ostrovsky wrote:
On 10/09/2015 02:51 AM, Jan Beulich wrote:
On 28.09.15 at 09:13, <haozhong.zhang@xxxxxxxxx> 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?

Yes, I should minus d->arch.vtsc_offset here.
In which case - afaict - the code becomes identical to that of the
TSC_MODE_ALWAYS_EMULATE case as well as the
TSC_MODE_DEFAULT w/ d->arch.vtsc true. Which seems quite
unlikely to be correct.

*elapsed_nsec = *gtsc_khz = 0; ? Because we are effectively in TSC_MODE_NEVER.

That can't be right...


-boris


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.