Re: [PATCH v4 1/1] printk: fix zero-valued printk timestamps in early boot

On Wed, 15 Apr 2026 at 00:38, Thomas Gleixner <[email protected]> wrote:
>
> On Fri, Apr 10 2026 at 14:37, Tim Bird wrote:
> > +
> > +#include <linux/timekeeping.h>
> > +#ifdef CONFIG_ARM64
> > +#include <asm/sysreg.h>
> > +#endif
> > +
> > +#ifdef CONFIG_EARLY_CYCLES_KHZ
> > +static inline u64 early_unsafe_cycles(void)
> > +{
> > +#if defined(CONFIG_X86_64)
> > +     /*
> > +      * This rdtsc may happen before secure TSC is initialized, and
> > +      * it is unordered. So please don't use this value for cryptography
> > +      * or after SMP is initialized.
> > +      */
> > +     return rdtsc();
> > +#elif defined(CONFIG_ARM64)
> > +     return read_sysreg(cntvct_el0);
> > +#elif defined(CONFIG_RISCV_TIMER)
> > +     u64 val;
> > +
> > +     asm volatile("rdtime %0" : "=r"(val));
> > +     return val;
> > +#else
> > +     return 0;
> > +#endif
> > +}
>
> No. Generic code and generic headers have no business to implement any
> architecture specific code and there is zero justification for
> architecture specific #ifdefs in generic code.

This translates in practice in early_unsafe_cycles() various
instances, one for each architecture supported. Even better a macro
for each architecture like this example below (untested code, just for
exemplification):

  ts_nsec = local_clock();
#ifdef CONFIG_EARLY_CYCLES_KHZ
#if defined(CONFIG_ARM64)
#include <asm/sysreg.h>
#define early_unsafe_cycles read_sysreg(cntvct_el0);
#elif defined(CONFIG_X86_64)
#define early_unsafe_cycles rdtsc()
#elif defined(CONFIG_RISCV_TIMER)
#define early_unsafe_cycles ({ u64 val; asm volatile("rdtime %0" :
"=r"(val)); val; })
#else
#define early_unsafe_cycles 0
#endif
+ if (unlikely(!ts_nsec))
+ ts_nsec = early_times_ns();
 #endif // CONFIG_EARLY_CYCLES_KHZ
  caller_id = printk_caller_id();

While keeping architectures separate is for abstraction layer best
practice, in very specific cases abstraction can be a meaningless
concept or much less important than having a single file for a single
feature (the complementary way of organising stuff). Because in Linux
the abstraction layer approach applies for architectures, the gap
between aesthetic and a specific case like an early boot macros set
hack for debugging is to clarify the exception from the general rule
in the header itself.

On Wed, 15 Apr 2026 at 00:38, Thomas Gleixner <[email protected]> wrote: (to TIM)
>
> Ask your favourite AI assistant for an opinion.

(Roberto maintaining the cold blood in front of "Tim's overlooking a
false positive gcc warning" and check Thomas suggestion just for the
sake of curiosity)

Gemini Thinking AI: Whether or not you can justify this exception
depends entirely on whether this is a permanent feature or a transient
debug hack.

Cheers, R-