Re: [PATCH 1/5] x86/time: deal with negative deltas in get_s_time_fixed()

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Thu, Jan 15, 2026 at 11:38:10AM +0100, Jan Beulich wrote:
> On 15.01.2026 09:24, Roger Pau Monné wrote:
> > On Thu, Jan 15, 2026 at 09:00:07AM +0100, Jan Beulich wrote:
> >> On 14.01.2026 18:49, Roger Pau Monné wrote:
> >>> On Tue, Jan 06, 2026 at 02:58:11PM +0100, Jan Beulich wrote:
> >>>> amd_check_erratum_1474() (next to its call to tsc_ticks2ns()) has a
> >>>> comment towards the TSC being "sane", but is that correct? Due to
> >>>> TSC_ADJUST, rdtsc() may well return a huge value (and the TSC would then
> >>>> wrap through 0 at some point). Shouldn't we subtract boot_tsc_stamp 
> >>>> before
> >>>> calling tsc_ticks2ns()?
> >>>
> >>> amd_check_erratum_1474() runs after early_time_init(), which would
> >>> have cleared any TSC_ADJUST offset AFAICT.  There's a note in the
> >>> initcall to that regard:
> >>>
> >>> /*
> >>>  * Must be executed after early_time_init() for tsc_ticks2ns() to have 
> >>> been
> >>>  * calibrated.  That prevents us doing the check in init_amd().
> >>>  */
> >>> presmp_initcall(amd_check_erratum_1474);
> >>
> >> Hmm, I should have written "Due to e.g. TSC_ADJUST". Firmware may also
> >> have played other games with MSR_TSC.
> > 
> > For amd_check_erratum_1474() we don't want to subtract boot_tsc_stamp,
> > otherwise when kexec'ed we won't be accounting properly for the time
> > since host startup, as subtracting boot_tsc_stamp would remove any
> > time consumed by a previously run OS.
> 
> For both this and ...
> 
> >>>> A similar issue looks to exist in tsc_get_info(), again when rdtsc()
> >>>> possibly returns a huge value due to TSC_ADJUST. Once again I wonder
> >>>> whether we shouldn't subtract boot_tsc_stamp.
> >>>
> >>> I would expect tsc_get_info() to also get called exclusively after
> >>> early_time_init()?
> >>
> >> Same here then (obviously).
> > 
> > For tsc_get_info() I think you are worried that the TSC might
> > overflow, and hence the calculation in scale_delta() would then be
> > skewed.  We must have other instances of this pattern however, what
> > about get_s_time_fixed(), I think it would also be affected?
> > 
> > Or maybe I'm not understanding the concern.  Given the proposed
> > scale_delta() logic, it won't be possible to distinguish rdtsc
> > overflowing from a value in the past.
> 
> ... this, my main point really is that scale_delta() (as its name says),
> and hence also tsc_ticks2ns(), shouldn't be used on absolute counts, but
> only deltas. (Yes, an absolute count can be viewed as delta from 0, but
> that's correct only if we know the TSC started counting from 0 and was
> never adjusted by some bias.)

Well amd_check_erratum_1474() does want the delta from 0 to the
current TSC, because that's the best? way to see when C6 needs to be
disabled.  Otherwise we just straight disable C6 on boot on affected
systems.

IMO the problem is not about using tsc_ticks2ns() or scale_delta()
with absolute or delta values, the issue if how the "delta" passed to
those functions is calculated.

Thanks, Roger.