Re: [PATCH RFC] x86/time: set AP's TSC scale estimate earlier

[Jan Beulich <jbeulich@xxxxxxxx>]

On 06.05.2026 12:33, Roger Pau Monné wrote:
> On Wed, May 06, 2026 at 11:39:14AM +0200, Jan Beulich wrote:
>> NOW() (in particular) can be used ahead of init_percpu_time(). As the
>> initial scale value set is merely the BSP's, we can as well set it before
>> actually launching the AP. Don't introduce yet another notifier function
>> though; do this from smpboot.c's.
>>
>> Setting the scale alone, however, doesn't work, so the entire struct
>> cpu_time is copied.
>>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>> ---
>> RFC: Copying the entire struct won't work very well when tsc_adjust[] is
>>      in use (and values there differ between sockets).
>>
>> This in particular eliminates an anomaly with log messages issued early
>> while APs are coming up, when "boot" console timestamps are in use.
> 
> Could we consider moving init_percpu_time() earlier in
> start_secondary()?  I think it's main dependency is on
> set_cpu_sibling_map(), which we could also move earlier?
> 
> Looking further, seems like it depends on smp_callin() having parsed
> the CPUID features, plus the socket_cpumask[] also being set.

Right, and CPUID retrieving in turn wants to be after ucode loading. Ucode
loading, however, is the primary source of AP boot log messages that I'm
aware of.

>> --- a/xen/arch/x86/time.c
>> +++ b/xen/arch/x86/time.c
>> @@ -2346,6 +2346,12 @@ void time_latch_stamps(void)
>>      ap_bringup_ref.local_stime = get_s_time_fixed(ap_bringup_ref.local_tsc);
>>  }
>>  
>> +void preinit_percpu_time(unsigned int cpu)
>> +{
>> +    /* Initial estimate for TSC rate etc. */
>> +    per_cpu(cpu_time, cpu) = this_cpu(cpu_time);
>> +}
>> +
>>  void init_percpu_time(void)
>>  {
>>      struct cpu_time *t = &this_cpu(cpu_time);
>> @@ -2353,9 +2359,6 @@ void init_percpu_time(void)
>>      u64 tsc;
>>      s_time_t now;
>>  
>> -    /* Initial estimate for TSC rate. */
>> -    t->tsc_scale = per_cpu(cpu_time, 0).tsc_scale;
> 
> Wouldn't it be simpler to pull this out of init_percpu_time() and do
> it at the start of start_secondary()?

As long as it's only copying memory, that may work fine. Plus, yes, it
would allow accessing actual registers of the CPU being brought up, if
need be. I'm slightly wary, but I guess I'll follow this suggestion.

Jan