Re: [Xen-devel] [PATCH v2 31/62] x86: xen pv clock time source

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Fri, Jan 12, 2018 at 08:45:17PM +0000, Joao Martins wrote:
> On 01/12/2018 11:28 AM, Wei Liu wrote:
> > It is a variant of TSC clock source.
> > 
> > Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>
> > Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> > Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> > ---
> > Changes since v1:
> >  - Use the mapped vcpu_info.
> > ---
> >  xen/arch/x86/time.c | 89 
> > +++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 89 insertions(+)
> > 
> > diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
> > index 3b654d7b7d..4cbd068d17 100644
> > --- a/xen/arch/x86/time.c
> > +++ b/xen/arch/x86/time.c
> > @@ -29,6 +29,7 @@
> >  #include <asm/mpspec.h>
> >  #include <asm/processor.h>
> >  #include <asm/fixmap.h>
> > +#include <asm/guest.h>
> >  #include <asm/mc146818rtc.h>
> >  #include <asm/div64.h>
> >  #include <asm/acpi.h>
> > @@ -525,6 +526,91 @@ static struct platform_timesource __initdata plt_tsc =
> >      .init = init_tsc,
> >  };
> >  
> > +#ifdef CONFIG_XEN_GUEST
> > +/************************************************************
> > + * PLATFORM TIMER 5: XEN PV CLOCK SOURCE
> > + *
> > + * Xen clock source is a variant of TSC source.
> > + */
> > +
> > +static uint64_t xen_timer_cpu_frequency(void)
> > +{
> > +    struct vcpu_time_info *info = &this_cpu(vcpu_info)->time;
> > +    uint64_t freq;
> > +
> > +    freq = 1000000000ULL << 32;
> > +    do_div(freq, info->tsc_to_system_mul);
> > +    if ( info->tsc_shift < 0 )
> > +        freq <<= -info->tsc_shift;
> > +    else
> > +        freq >>= info->tsc_shift;
> > +
> > +    return freq;
> > +}
> > +
> > +static int64_t __init init_xen_timer(struct platform_timesource *pts)
> > +{
> > +    if ( !xen_guest )
> > +        return 0;
> > +
> > +    pts->frequency = xen_timer_cpu_frequency();
> > +
> > +    return pts->frequency;
> > +}
> > +
> Guests that don't have TSC_RELIABLE set in verify_tsc_reliability() you end up
> evaluating this condition to true:
> 
> if ( boot_cpu_has(X86_FEATURE_CONSTANT_TSC) &&
>      !boot_cpu_has(X86_FEATURE_TSC_RELIABLE) )
>     time_calibration_rendezvous_fn = time_calibration_tsc_rendezvous;
> 
> And that's possibly not necessary to keep sync the TSC in sync with pvclock?
> 
> Meaning the default rendezvous function (time_calibration_std_rendezvous) 
> might
> be enough.

Yes, it is possible that the default function is enough. But we haven't
tested that yet.

> 
> > +static always_inline uint64_t read_cycle(const struct vcpu_time_info *info,
> > +                                         uint64_t tsc)
> > +{
> > +    uint64_t delta = tsc - info->tsc_timestamp;
> > +    struct time_scale ts = {
> > +        .shift    = info->tsc_shift,
> > +        .mul_frac = info->tsc_to_system_mul,
> > +    };
> > +    uint64_t offset = scale_delta(delta, &ts);
> > +
> > +    return info->system_time + offset;
> > +}
> > +
> > +static uint64_t read_xen_timer(void)
> > +{
> > +    struct vcpu_time_info *info = &this_cpu(vcpu_info)->time;
> > +    uint32_t version;
> > +    uint64_t ret;
> > +    uint64_t last;
> > +    static uint64_t last_value;
> > +
> > +    do {
> > +        version = info->version & ~1;
> > +        /* Make sure version is read before the data */
> > +        smp_rmb();
> > +
> > +        ret = read_cycle(info, rdtsc_ordered());
> > +        /* Ignore fancy flags for now */
> > +
> 
> Heh :)
> 
> When the dust settles around the security issues, perhaps this patch could 
> help
> (I can formally submit it after more proper testing, but should give an
> expectation on the changes).
> 

I skimmed through it. It looks like it is mostly an optimisation
for this patch. That is, properly detecting the stable bit and using it
where appropriate. We can definitely improve in that regard. But as you
said, let's handle the security issues at hand first.

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel