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

["Jan Beulich" <JBeulich@xxxxxxxx>]

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

Jan


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