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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 15.01.2020 12:53, Roger Pau Monné wrote:
> 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?

For one I have to admit I was (mistakenly) thinking of wakeup
from S states more than that from C states. So assuming the
TSC indeed only stops (but won't get e.g. restarted), backwards
moves ought to be excluded. What I'm then worried about is too
little progress observable by guests. The PV time protocol
ought to be fine in this regard (and consumers of raw TSC values
are on their own anyway), but wouldn't you need to update TSC
offsets of HVM guests in order to compensate for the elapsed
time?

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

A P-state change at most alters the the tick rate, but wouldn't
stop or even reset the TSC (afaict).

Jan

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