[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 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?

-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@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


_______________________________________________
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®.