Re: [PATCH v5 2/2] x86/time: avoid early uses of NOW() to return zero

[Jan Beulich <jbeulich@xxxxxxxx>]

On 02.09.2026 09:54, Roger Pau Monné wrote:
> On Wed, Aug 19, 2026 at 01:45:56PM +0200, Jan Beulich wrote:
>> Waiting loops like the one in flush_command_buffer() will degenerate to
>> infinite ones when used early enough for NOW() to still return constant
>> zero. Make sure the returned value at least monotonically increases. When
>> available, use nominal frequency values as initial approximation.
>>
>> Do this only in get_s_time(), as producing a sane value in
>> get_s_time_fixed() for non-zero inputs won't be reasonably possible.
>> Put an assertion there.
>>
>> Reported-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> Acked-by: Roger Pau Monné <roger@xxxxxxxxxxxxxx>

I assume you won't mind if I correct the typo in the domain name.

>> --- a/xen/arch/x86/time.c
>> +++ b/xen/arch/x86/time.c
>> @@ -1664,6 +1664,9 @@ s_time_t get_s_time_fixed(uint64_t at_ts
>>      const struct cpu_time *t = &this_cpu(cpu_time);
>>      uint64_t tsc, delta;
>>  
>> +    /* scale_delta() degenerates when the scale wasn't set yet. */
>> +    ASSERT(t->tsc_scale.mul_frac);
>> +
>>      if ( at_tsc )
>>          tsc = at_tsc;
>>      else
>> @@ -1679,6 +1682,20 @@ s_time_t get_s_time_fixed(uint64_t at_ts
>>  
>>  s_time_t get_s_time(void)
>>  {
>> +    /*
>> +     * Before the TSC scale is set, avoid returning constant 0 (or whatever
>> +     * this_cpu(cpu_time).stamp.local_stime is set to).  While the returned
>> +     * value is in no way representing time, it at least increases
>> +     * monotonically, thus avoiding e.g. waiting loops to degenerate to
>> +     * entirely infinite ones.
>> +     */
>> +    if ( unlikely(!this_cpu(cpu_time).tsc_scale.mul_frac) )
>> +    {
>> +        static s_time_t counter;
> 
> Not that it matters much, but counter can probably be __initdata?

I wanted to play safe here: In get_s_time_fixed(), release builds won't
crash if the assertion wasn't there, but would trigger in a
corresponding debug build. In that (unexpected) situation, we'd crash
here (when .init.* was unmapped) if __initdata was used.

Jan