printk_start_of_line() checks for a value of 0 right now. In order to be
able to have NOW() return at least monotonically increasing values, that
needs replacing by an explicit indicator.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
Arm and RISC-V may want to consider whether their initial get_cycles()
can't be moved yet earlier, such that the indicator also can be set
yet earlier.
---
v4: Add barriers.
v3: New.

--- a/xen/arch/arm/time.c
+++ b/xen/arch/arm/time.c
@@ -145,6 +145,8 @@ void __init preinit_xen_time(void)
         panic("Timer: Cannot initialize platform timer\n");
 
     boot_count = get_cycles();
+    smp_wmb();
+    NOW_good = true;
 }
 
 static void __init init_dt_xen_time(void)
--- a/xen/arch/riscv/time.c
+++ b/xen/arch/riscv/time.c
@@ -87,6 +87,8 @@ void __init preinit_xen_time(void)
         panic("%s: ACPI isn't supported\n", __func__);
 
     boot_clock_cycles = get_cycles();
+    smp_wmb();
+    NOW_good = true;
 
     /* set_xen_timer must have been set by sbi_init() already */
     ASSERT(set_xen_timer);
--- a/xen/arch/x86/time.c
+++ b/xen/arch/x86/time.c
@@ -2660,6 +2660,7 @@ void __init early_time_init(void)
 
     set_time_scale(&t->tsc_scale, tmp);
     t->stamp.local_tsc = boot_tsc_stamp;
+    NOW_good = true;
 
     init_percpu_time();
 
--- a/xen/common/time.c
+++ b/xen/common/time.c
@@ -22,6 +22,8 @@
 #include <asm/div64.h>
 #include <asm/domain.h>
 
+bool __ro_after_init NOW_good;
+
 /* Nonzero if YEAR is a leap year (every 4 years,
    except every 100th isn't, and every 400th is).  */
 #define __isleap(year) \
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -975,11 +975,11 @@ static void printk_start_of_line(const c
         }
         /* fall through */
     case TSM_BOOT:
-        sec = NOW();
-        nsec = do_div(sec, 1000000000);
-
-        if ( sec | nsec )
+        if ( NOW_good )
         {
+            smp_rmb();
+            sec = NOW();
+            nsec = do_div(sec, 1000000000);
             snprintf(tstr, sizeof(tstr), "[%5"PRIu64".%06"PRIu64"] ",
                      sec, nsec / 1000);
             break;
--- a/xen/include/xen/time.h
+++ b/xen/include/xen/time.h
@@ -62,6 +62,12 @@ struct tm wallclock_time(uint64_t *ns);
 /* Chosen so (NOW() + delta) wont overflow without an uptime of 200 years */
 #define STIME_DELTA_MAX ((s_time_t)((uint64_t)~0ULL>>2))
 
+/*
+ * Indicator that the value returned by NOW() is good (earlier invocations may
+ * return zero or very small, merely monotonically increasing values).
+ */
+extern bool NOW_good;
+
 /* Explicitly OR with 1 just in case version number gets out of sync. */
 #define version_update_begin(v) (((v) + 1) | 1)
 #define version_update_end(v)   ((v) + 1)

On Tue, Jun 30, 2026 at 04:06:00PM +0200, Jan Beulich wrote:
> printk_start_of_line() checks for a value of 0 right now. In order to be
> able to have NOW() return at least monotonically increasing values, that
> needs replacing by an explicit indicator.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> Arm and RISC-V may want to consider whether their initial get_cycles()
> can't be moved yet earlier, such that the indicator also can be set
> yet earlier.
> ---
> v4: Add barriers.
> v3: New.
> 
> --- a/xen/arch/arm/time.c
> +++ b/xen/arch/arm/time.c
> @@ -145,6 +145,8 @@ void __init preinit_xen_time(void)
>          panic("Timer: Cannot initialize platform timer\n");
>  
>      boot_count = get_cycles();
> +    smp_wmb();
> +    NOW_good = true;
>  }
>  
>  static void __init init_dt_xen_time(void)
> --- a/xen/arch/riscv/time.c
> +++ b/xen/arch/riscv/time.c
> @@ -87,6 +87,8 @@ void __init preinit_xen_time(void)
>          panic("%s: ACPI isn't supported\n", __func__);
>  
>      boot_clock_cycles = get_cycles();
> +    smp_wmb();
> +    NOW_good = true;
>  
>      /* set_xen_timer must have been set by sbi_init() already */
>      ASSERT(set_xen_timer);
> --- a/xen/arch/x86/time.c
> +++ b/xen/arch/x86/time.c
> @@ -2660,6 +2660,7 @@ void __init early_time_init(void)
>  
>      set_time_scale(&t->tsc_scale, tmp);
>      t->stamp.local_tsc = boot_tsc_stamp;
> +    NOW_good = true;

Would you need a barrier here to ensure compiler doesn't re-order the
writes?  Maybe using ACCESS_ONCE(), or a smp_wmb() like it's used in
other arches?

Thanks, Roger.

On 28.07.2026 10:17, Roger Pau Monné wrote:
> On Tue, Jun 30, 2026 at 04:06:00PM +0200, Jan Beulich wrote:
>> --- a/xen/arch/x86/time.c
>> +++ b/xen/arch/x86/time.c
>> @@ -2660,6 +2660,7 @@ void __init early_time_init(void)
>>  
>>      set_time_scale(&t->tsc_scale, tmp);
>>      t->stamp.local_tsc = boot_tsc_stamp;
>> +    NOW_good = true;
> 
> Would you need a barrier here to ensure compiler doesn't re-order the
> writes?  Maybe using ACCESS_ONCE(), or a smp_wmb() like it's used in
> other arches?

Aiui it isn't needed here. The compiler can't re-order the two writes,
due to our use of -fno-strict-aliasing. The barrier is there on other
arch-es to avoid re-ordering in hardware.

Jan

On Tue, Jul 28, 2026 at 10:24:38AM +0200, Jan Beulich wrote:
> On 28.07.2026 10:17, Roger Pau Monné wrote:
> > On Tue, Jun 30, 2026 at 04:06:00PM +0200, Jan Beulich wrote:
> >> --- a/xen/arch/x86/time.c
> >> +++ b/xen/arch/x86/time.c
> >> @@ -2660,6 +2660,7 @@ void __init early_time_init(void)
> >>  
> >>      set_time_scale(&t->tsc_scale, tmp);
> >>      t->stamp.local_tsc = boot_tsc_stamp;
> >> +    NOW_good = true;
> > 
> > Would you need a barrier here to ensure compiler doesn't re-order the
> > writes?  Maybe using ACCESS_ONCE(), or a smp_wmb() like it's used in
> > other arches?
> 
> Aiui it isn't needed here. The compiler can't re-order the two writes,
> due to our use of -fno-strict-aliasing. The barrier is there on other
> arch-es to avoid re-ordering in hardware.

You are the expert in compilers, but it was my understanding that
`no-strict-aliasing` only affects the ordering of pointers accesses,
but not plain variables.  IOW: NOW_good accesses could be reordered
because it's not a pointer.  Does the compiler consider t can point to
&NOW_good and hence it can't be reordered?

Thanks, Roger.

On 28.07.2026 12:32, Roger Pau Monné wrote:
> On Tue, Jul 28, 2026 at 10:24:38AM +0200, Jan Beulich wrote:
>> On 28.07.2026 10:17, Roger Pau Monné wrote:
>>> On Tue, Jun 30, 2026 at 04:06:00PM +0200, Jan Beulich wrote:
>>>> --- a/xen/arch/x86/time.c
>>>> +++ b/xen/arch/x86/time.c
>>>> @@ -2660,6 +2660,7 @@ void __init early_time_init(void)
>>>>  
>>>>      set_time_scale(&t->tsc_scale, tmp);
>>>>      t->stamp.local_tsc = boot_tsc_stamp;
>>>> +    NOW_good = true;
>>>
>>> Would you need a barrier here to ensure compiler doesn't re-order the
>>> writes?  Maybe using ACCESS_ONCE(), or a smp_wmb() like it's used in
>>> other arches?
>>
>> Aiui it isn't needed here. The compiler can't re-order the two writes,
>> due to our use of -fno-strict-aliasing. The barrier is there on other
>> arch-es to avoid re-ordering in hardware.
> 
> You are the expert in compilers, but it was my understanding that
> `no-strict-aliasing` only affects the ordering of pointers accesses,
> but not plain variables.  IOW: NOW_good accesses could be reordered
> because it's not a pointer.  Does the compiler consider t can point to
> &NOW_good and hence it can't be reordered?

Yes, that's my understanding of how this work. (Yet no, "expert" surely
is going too far.) Things would be different if NOW_good was a function-
local variable, I think.

Jan

On Tue, Jul 28, 2026 at 01:37:15PM +0200, Jan Beulich wrote:
> On 28.07.2026 12:32, Roger Pau Monné wrote:
> > On Tue, Jul 28, 2026 at 10:24:38AM +0200, Jan Beulich wrote:
> >> On 28.07.2026 10:17, Roger Pau Monné wrote:
> >>> On Tue, Jun 30, 2026 at 04:06:00PM +0200, Jan Beulich wrote:
> >>>> --- a/xen/arch/x86/time.c
> >>>> +++ b/xen/arch/x86/time.c
> >>>> @@ -2660,6 +2660,7 @@ void __init early_time_init(void)
> >>>>  
> >>>>      set_time_scale(&t->tsc_scale, tmp);
> >>>>      t->stamp.local_tsc = boot_tsc_stamp;
> >>>> +    NOW_good = true;
> >>>
> >>> Would you need a barrier here to ensure compiler doesn't re-order the
> >>> writes?  Maybe using ACCESS_ONCE(), or a smp_wmb() like it's used in
> >>> other arches?
> >>
> >> Aiui it isn't needed here. The compiler can't re-order the two writes,
> >> due to our use of -fno-strict-aliasing. The barrier is there on other
> >> arch-es to avoid re-ordering in hardware.
> > 
> > You are the expert in compilers, but it was my understanding that
> > `no-strict-aliasing` only affects the ordering of pointers accesses,
> > but not plain variables.  IOW: NOW_good accesses could be reordered
> > because it's not a pointer.  Does the compiler consider t can point to
> > &NOW_good and hence it can't be reordered?
> 
> Yes, that's my understanding of how this work. (Yet no, "expert" surely
> is going too far.) Things would be different if NOW_good was a function-
> local variable, I think.

OK, I wouldn't mind adding a compiler barrier to say on the safe side,
but I also won't object if you think that's enough.

Acked-by: Roger Pau Monné <roger@xenproject.org>

Thanks, Roger.

On 29.07.2026 10:46, Roger Pau Monné wrote:
> On Tue, Jul 28, 2026 at 01:37:15PM +0200, Jan Beulich wrote:
>> On 28.07.2026 12:32, Roger Pau Monné wrote:
>>> On Tue, Jul 28, 2026 at 10:24:38AM +0200, Jan Beulich wrote:
>>>> On 28.07.2026 10:17, Roger Pau Monné wrote:
>>>>> On Tue, Jun 30, 2026 at 04:06:00PM +0200, Jan Beulich wrote:
>>>>>> --- a/xen/arch/x86/time.c
>>>>>> +++ b/xen/arch/x86/time.c
>>>>>> @@ -2660,6 +2660,7 @@ void __init early_time_init(void)
>>>>>>  
>>>>>>      set_time_scale(&t->tsc_scale, tmp);
>>>>>>      t->stamp.local_tsc = boot_tsc_stamp;
>>>>>> +    NOW_good = true;
>>>>>
>>>>> Would you need a barrier here to ensure compiler doesn't re-order the
>>>>> writes?  Maybe using ACCESS_ONCE(), or a smp_wmb() like it's used in
>>>>> other arches?
>>>>
>>>> Aiui it isn't needed here. The compiler can't re-order the two writes,
>>>> due to our use of -fno-strict-aliasing. The barrier is there on other
>>>> arch-es to avoid re-ordering in hardware.
>>>
>>> You are the expert in compilers, but it was my understanding that
>>> `no-strict-aliasing` only affects the ordering of pointers accesses,
>>> but not plain variables.  IOW: NOW_good accesses could be reordered
>>> because it's not a pointer.  Does the compiler consider t can point to
>>> &NOW_good and hence it can't be reordered?
>>
>> Yes, that's my understanding of how this work. (Yet no, "expert" surely
>> is going too far.) Things would be different if NOW_good was a function-
>> local variable, I think.
> 
> OK, I wouldn't mind adding a compiler barrier to say on the safe side,
> but I also won't object if you think that's enough.

Okay, I've added one in, even if it feels a little odd.

> Acked-by: Roger Pau Monné <roger@xenproject.org>

Thanks, Jan

On 6/30/26 4:06 PM, Jan Beulich wrote:
> printk_start_of_line() checks for a value of 0 right now. In order to be
> able to have NOW() return at least monotonically increasing values, that
> needs replacing by an explicit indicator.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

LGTM:
  Reviewed-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>

Thanks.

~ Oleksii