Re: [PATCH] x86/time: make do_settime() uses more accurate

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

On Wed, May 06, 2026 at 01:45:36PM +0200, Jan Beulich wrote:
> On 06.05.2026 13:15, Roger Pau Monné wrote:
> > On Wed, May 06, 2026 at 11:35:45AM +0200, Jan Beulich wrote:
> >> As a comment next to one of the invocations states, get_wallclock_time()
> >> can take over a second. The order of evaluation of function arguments is
> >> in principle unspecified; in practice at least gcc looks to be evaluating
> >> them from last to first. Hence with NOW() invoked first, the respective
> >> value passed to do_settime() can be off by over a second (which is in
> >> contrast to __get_cmos_time() attempting to get the time exactly after an
> >> update, i.e. [pretty] precisely at a seconds boundary).
> >>
> >> This also addresses a Misra C:2012 rule 13.2 ("The value of an expression
> >> and its persistent side-effects shall be the same under all permitted
> >> evaluation orders") violation each.
> >>
> >> Fixes: f64134cdb81c ("x86: Fix time_resume() to notify all domains of 
> >> wallclock change")
> >> Fixes: 0bfcf984b727 ("x86: Reintroduce clocksource=tsc")
> >> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> >> ---
> >> Of course the time it takes to do all the CMOS reads (or whichever else
> >> wallclock time source is in use) also results in an inaccuracy. For
> >> __get_cmos_time() this might be solvable by having it latch NOW() before
> >> doing the 6 reads, but in particular for efi_get_time() there's hardly
> >> anything we can do.
> >>
> >> As to Misra rule 13.2: tagging.ecl lists the rule as clean. I also can't
> >> find any deviation for the two instances fixed here. What am I missing?
> >>
> >> For __get_cmos_time(), tangentially: Wouldn't we better use the
> >> century byte if available? As it stands, things will break in 2070. Which
> >> is a long way out, yes, but still. (Of course this would mean a 7th slow
> >> I/O port write/read pair.)
> > 
> > Seems fine to me.
> 
> I'll make a patch, provided I can figure out under what conditions the byte
> is present / valid (hopefully that won't involve ACPI AML).

It's in the ACPI FADT table AFAIK.

> > One further note: in __get_cmos_time() I think we want to read
> > backwards, so start with century then year and so on, so the last read
> > is seconds, as to avoid extra skew.
> 
> I don't think this matters. It would be awkward if those reads took over a
> second. Plus if it did, the UIP flag would transiently be set, in which
> case doing the reads wouldn't be valid in the first place. Arguably this
> can in principle happen if a SMI hits in the middle, and its handling
> takes excessively long.

Oh, I see, indeed.

Thanks, Roger.