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

[Haozhong Zhang <haozhong.zhang@xxxxxxxxx>]

On Wed, Oct 14, 2015 at 03:40:01AM -0600, Jan Beulich wrote:
> >>> On 14.10.15 at 04:45, <haozhong.zhang@xxxxxxxxx> wrote:
> > However, patch 2 is still necessary. The existing tsc_get_info() uses
> > the host TSC frequency as the guest TSC frequency for a domain in
> > TSC_MODE_DEFAULT, which could cause errors in the following example:
> >  - A domain d using TSC_MODE_DEFAULT is created on host A, then
> >    migrated to host B, and finally migrated to host C.
> >  - The host TSC frequencies of three hosts are f_a, f_b and f_c
> >    respectively and f_a != f_b and f_b != f_c.
> >  - Both host B and host C support TSC scaling (either VMX TSC scaling
> >    or SVM TSC ratio).
> > 
> > In above example w/o patch 2,
> >  1. Initially, d->arch.tsc_khz == f_a.
> >  
> >  2. In the first migration, tsc_get_info() on host A passes f_a as the
> >     guest TSC frequency to tsc_set_info() on host B, so that after the
> >     migration it's still that d->arch.tsc_khz == f_a. As TSC scaling
> >     takes effect, guest programs can still observe TSC in frequency f_a.
> >     So far so good.
> >     
> >  3. However, in the second migration, f_b (!= f_a) is passed as the
> >     guest TSC frequency to tsc_set_info() on host C so that after the
> >     migration d->arch.tsc_khz is not f_a any more.
> 
> Hmm, yes, looks like you're right. But I don't think the current use of
> cpu_khz should be blindly replaced - instead this should be done only
> when cpu_has_tsc_ratio (mirroring what tsc_set_info() does). Unless
> of course it can be proven that d->arch.tsc_khz == cpu_khz in all
> relevant cases without use of TSC ratio (i.e. the use of cpu_khz here
> just served as kind of a shorthand).
>
> Jan
> 

Sounds reasonable. A better patch 2 may look like

- *gtsc_khz = cpu_khz;
+ if ( is_hvm_domain(d) && cpu_has_tsc_ratio )
+     *gtsc_khz = d->arch.tsc_khz;
+ else
+     *gtsc_khz = cpu_khz;

which gets d->arch.tsc_khz only if TSC ratio is used, otherwise it
follows the existing behavior.

- Haozhong

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