Re: [Xen-devel] [PATCH v2 04/14] x86/time.c: Use correct guest TSC frequency in tsc_get_info()

[Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>]

On 12/07/2015 08:08 PM, Haozhong Zhang wrote:
> On 12/07/15 12:53, Boris Ostrovsky wrote:
> > On 12/06/2015 03:58 PM, Haozhong Zhang wrote:
> > > When the TSC mode of a HVM container is TSC_MODE_DEFAULT or
> > > TSC_MODE_PVRDTSCP and no TSC emulation is used, the existing
> > > tsc_get_info() uses the host TSC frequency (cpu_khz) as the guest TSC
> > > frequency. However, tsc_set_info() may set the guest TSC frequency to a
> > > value different than the host. In order to keep consistent to
> > > tsc_set_info(), this patch makes tsc_get_info() use the value set by
> > > tsc_set_info() as the guest TSC frequency.
> > >
> > > Signed-off-by: Haozhong Zhang <haozhong.zhang@xxxxxxxxx>
> > > ---
> > >   xen/arch/x86/time.c | 12 +++++++++---
> > >   1 file changed, 9 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
> > > index 1091e69..95df4f1 100644
> > > --- a/xen/arch/x86/time.c
> > > +++ b/xen/arch/x86/time.c
> > > @@ -1749,6 +1749,9 @@ void tsc_get_info(struct domain *d, uint32_t *tsc_mode,
> > >                     uint64_t *elapsed_nsec, uint32_t *gtsc_khz,
> > >                     uint32_t *incarnation)
> > >   {
> > > +    bool_t enable_tsc_scaling = has_hvm_container_domain(d) &&
> > > +                                cpu_has_tsc_ratio;
> > && !d->arch.vtsc ?
> >
> > (assuming my comment to the previous patch is correct).
> enable_tsc_scaling in tsc_get_info() is always used in the condition
> !d->arch.vtsc, so it's not necessary to include it again in
> enable_tsc_scaling.
>
> > > +
> > >       *incarnation = d->arch.incarnation;
> > >       *tsc_mode = d->arch.tsc_mode;
> > > @@ -1769,7 +1772,7 @@ void tsc_get_info(struct domain *d, uint32_t *tsc_mode,
> > >           }
> > >           tsc = rdtsc();
> > >           *elapsed_nsec = scale_delta(tsc, &d->arch.vtsc_to_ns);
> > > -        *gtsc_khz = cpu_khz;
> > > +        *gtsc_khz = enable_tsc_scaling ? d->arch.tsc_khz : cpu_khz;
> > >           break;
> > >       case TSC_MODE_PVRDTSCP:
> > >           if ( d->arch.vtsc )
> > > @@ -1779,10 +1782,13 @@ void tsc_get_info(struct domain *d, uint32_t *tsc_mode,
> > >           }
> > >           else
> > >           {
> > > +            struct time_scale *scale = enable_tsc_scaling ?
> > > +                                       &this_cpu(cpu_time).tsc_scale :
> > > +                                       &d->arch.vtsc_to_ns;
> > IIUIC tsc_scale is host property and so why would it be used if TSC scaling
> > is available to guests?
> scale is used below to convert a host TSC to nanosec. When TSC scaling is used,
> d->arch.vtsc_to_ns may not base on the host TSC frequency, so I turn to the host
> tsc_scale instead.
OK. Can we then use tsc_scale for both with and without TSC scaling? (on 
the set side too).
(And as a side note, I think that the comment in 
include/asm-x86/domain.h describing vtsc_to_ns as being used for 
emulated cases is rather misleading. We use it for both emulated and native)
-boris


> Haozhong
>
> > -boris
> >
> >
> > >               tsc = rdtsc();
> > > -            *elapsed_nsec = scale_delta(tsc, &d->arch.vtsc_to_ns) -
> > > +            *elapsed_nsec = scale_delta(tsc, scale) -
> > >                               d->arch.vtsc_offset;
> > > -            *gtsc_khz = 0; /* ignored by tsc_set_info */
> > > +            *gtsc_khz = enable_tsc_scaling ? d->arch.tsc_khz : 0;
> > >           }
> > >           break;
> > >       }

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