Re: [Xen-devel] [PATCH] x86/time: update TSC stamp on restore from deep C-state

["Roger Pau Monné" <roger.pau@citrix.com>]

On Wed, Jan 15, 2020 at 12:40:27PM +0100, Jan Beulich wrote:
> On 15.01.2020 10:47, Roger Pau Monné wrote:
> > On Tue, Jan 14, 2020 at 07:36:21PM +0000, Igor Druzhinin wrote:
> >> --- a/xen/arch/x86/time.c
> >> +++ b/xen/arch/x86/time.c
> >> @@ -955,10 +955,16 @@ u64 stime2tsc(s_time_t stime)
> >>  
> >>  void cstate_restore_tsc(void)
> >>  {
> >> +    struct cpu_time *t = &this_cpu(cpu_time);
> >> +
> >>      if ( boot_cpu_has(X86_FEATURE_NONSTOP_TSC) )
> >>          return;
> >>  
> >> -    write_tsc(stime2tsc(read_platform_stime(NULL)));
> >> +    t->stamp.master_stime = read_platform_stime(NULL);
> >> +    t->stamp.local_tsc = stime2tsc(t->stamp.master_stime);
> >> +    t->stamp.local_stime = t->stamp.master_stime;
> >> +
> >> +    write_tsc(t->stamp.local_tsc);
> > 
> > In order to avoid the TSC write (and the likely associated vmexit),
> > could you instead do:
> > 
> > t->stamp.local_stime = t->stamp.master_stime = read_platform_stime(NULL);
> > t->stamp.local_tsc = rdtsc_ordered();
> > 
> > I think it should achieve the same as it syncs the local TSC stamp and
> > times, would avoid the TSC write and slightly simplifies the logic.
> 
> Wouldn't this result in guests possibly observing the TSC moving
> backwards?

Isn't local_tsc storing a TSC value read from the same CPU always, and
hence could only go backwards if rdtsc actually goes backwards?

Ie: cpu_frequency_change seems to do something similar, together with
a re-adjusting of the time scale, but doesn't perform any TSC write.

Thanks, Roger.

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