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@suse.com>
---
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.

--- a/xen/arch/x86/include/asm/time.h
+++ b/xen/arch/x86/include/asm/time.h
@@ -21,6 +21,7 @@ mktime (unsigned int year, unsigned int
 int time_suspend(void);
 int time_resume(void);
 
+void preinit_percpu_time(unsigned int cpu);
 void init_percpu_time(void);
 void time_latch_stamps(void);
 
--- a/xen/arch/x86/smpboot.c
+++ b/xen/arch/x86/smpboot.c
@@ -1139,6 +1139,7 @@ static int cf_check cpu_smpboot_callback
     {
     case CPU_UP_PREPARE:
         rc = cpu_smpboot_alloc(cpu);
+        preinit_percpu_time(cpu);
         break;
     case CPU_UP_CANCELED:
     case CPU_DEAD:
--- 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;
-
     if ( tsc_adjust )
     {
         unsigned int socket = cpu_to_socket(smp_processor_id());

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@suse.com>
> ---
> 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.

> --- a/xen/arch/x86/include/asm/time.h
> +++ b/xen/arch/x86/include/asm/time.h
> @@ -21,6 +21,7 @@ mktime (unsigned int year, unsigned int
>  int time_suspend(void);
>  int time_resume(void);
>  
> +void preinit_percpu_time(unsigned int cpu);
>  void init_percpu_time(void);
>  void time_latch_stamps(void);
>  
> --- a/xen/arch/x86/smpboot.c
> +++ b/xen/arch/x86/smpboot.c
> @@ -1139,6 +1139,7 @@ static int cf_check cpu_smpboot_callback
>      {
>      case CPU_UP_PREPARE:
>          rc = cpu_smpboot_alloc(cpu);
> +        preinit_percpu_time(cpu);
>          break;
>      case CPU_UP_CANCELED:
>      case CPU_DEAD:
> --- 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()?

Thanks, Roger.

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@suse.com>
>> ---
>> 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