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

[Boris Ostrovsky <boris.ostrovsky@oracle.com>]

On 10/09/2015 02:51 AM, Jan Beulich wrote:
>>>> On 28.09.15 at 09:13, <haozhong.zhang@intel.com> 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