[Japabu]

Problem

x86-64's clock took the TSC's rate from one 50 ms count against the HPET at boot, then never looked again.

• AMD bench (Ryzen 7 4800U): that count read 41–66 ppm low against the 1796.6248 MHz the TSC counts under Linux, which gains about 5 s a day.
• T14: the kernel kept its own measurement, 285 ppm under or 59 ppm over, beside the 2,419,200,000 Hz CPUID 15H states.
• Unchecked: that the TSC is invariant, that the CPUs agree (an 11 ms bracket at AP start was the only check), and that the rate holds.
• Found while doing this: on the AMD bench the TSC steps about 1.7 ms forward once in every boot (issues/the-amd-benchs-tsc-steps-1-7-ms-forward-once-a-boot.md).

What changes

The rate.

• Stated: where CPUID 15H enumerates its crystal, the clock keeps that rate. Leaf 16H is no longer read as a rate, since the SDM confines it to display.
• Measured: elsewhere the boot counts the TSC against the HPET over 50 ms, in six samples. Each is the narrowest of three reads bracketed by RDTSCP; LFENCE. The rate's bound comes from the brackets and one HPET tick (kernel::clock::reference).
• Calibration steps: a sample off the line between the first and last refuses the calibration. The boot retakes it up to three times, then panics.

The runtime check.

• The machine's sample: once a second, the first CPU whose pass is due takes one HPET read, one ACPI PM timer read and the RTC's latest edge.
  • The HPET is held at the kept rate against the previous sample (a step), and over every held span since boot (a drift).
  • The allowance is the readings' brackets, one tick per segment, the kept rate's bound and the HPET's drift figure. IA-PC HPET 1.0a §2.2 gives ±0.05% over spans of 1 ms or more and ±0.2% over spans of 100 µs or less; spans under 1 ms take the 0.2%.
  • Reports come at 1, 2, 4, … s of held spans. Where the rate was measured, a tighter measurement replaces it.
• Each CPU, once a second: it checks that its IA32_TSC_ADJUST is unchanged, and that its TSC is not behind the latest any CPU's check read before it.
• The RTC as third reference: its update-ended interrupt (IRQ8, vector 0x26, the census's new rtc source on cpu0) times the TSC against each edge. It runs off the RTC's 32.768 kHz crystal; the HPET and the PM timer run off the FCH's (PCH's) 14.318 MHz clock.
  • An edge reads late by its interrupt's latency, never early. So the RTC's reading across a span comes from the least-latency of three edges on each side.
  • The reading is less the RTC crystal's own drift from the TSC, measured from edges at least 4 s earlier. Uncorrected, 60 ppm over the window is about a quarter of a 1.7 ms step.
  • A step the clock takes forgets the edges before it, so neither a drift base nor a later span's side reaches across it.
  • The RTC is armed after the IOMMU, since VT-d blocks a compatibility-format pin routed before it remaps.
• Arbitration replaces the idle guard, which left spans out unseen. Where the TSC and the HPET disagree:
  • The PM timer agrees with the TSC: the HPET moved, and the span is left out.
  • All three disagree: the span is left out.
  • The PM timer agrees with the HPET, or there is no PM timer: the RTC decides once three edges have come after the span. It reads within a quarter of the step of either answer:
    • near zero: the chipset's counters moved together, and the span is left out;
    • near the step: the TSC stepped, logged "confirmed by the RTC";
    • neither, or no edge yet: the TSC stepped, logged "unconfirmed by the RTC".
• A TSC step:
  • The boot's first, forward: it is logged as TSC STEP with its size, and slewed away at no more than 500 ppm. That is RFC 5905's MAXFREQ, the frequency tolerance NTP disciplines a clock within (Appendix A.1.1).
    • Readers see the step until it is found, and the correction never steps back.
    • Each period pays the debt over two periods, so a sample up to a period late cannot overpay. 1.7 ms takes about 12 s, most of it at the full rate.
  • A second, or one backward: the TSC is demoted.
  • Also demoting: a per-CPU check finding IA32_TSC_ADJUST changed, a CPU behind another, or an AP's start proving its offset. Each demotes instead of panicking.
• Demotion: the TSC leaves for the HPET for the rest of the boot, logged as DEMOTED.
  • The clock page is marked demoted. toyos_abi::clock::nanos_since_boot and stamp_ns then take the new SYS_CLOCK_NOW (152).
  • SYS_CLOCK_NOW counts by the HPET from the last held sample's clock, which is the HPET's own.
  • The handover is monotonic across CPUs. demote marks the page, then runs one round (a shootdown::Shootdown). Every online CPU, kicked, floors every later answer at its own clock, along the later of the line in force and the one it replaced. Retunes are held from the mark until the handover ends. A page reader checks the mark after its counter read, so a reader that found the page unmarked read before its own CPU's floor, however far that CPU's TSC leads.
    • SYS_CLOCK_NOW waits for the round, serving its own CPU meanwhile. Every wait panics past time::DEAF_CPU (5 s).
    • Before the APs are released, no other CPU has run a process, so the demoting CPU floors alone and issues no round.
    • The demoter's wait answers every round it is asked meanwhile (the TLB's, SMI_CMD's) through tlb::poll, so the round ends even when other CPUs spin with interrupts closed. tlb::poll serves the handover too, so a Lock spinner answers it.
    • A finding after the demotion is logged once.
  • The line goes back to the kept rate, unslewed. The kernel's own clock, deadlines and log stamps stay on it.
  • The checks and the sampler go on. A later step or check failure is logged, not slewed.
• A drift past both bounds still panics.

Eligibility. A CPU that does not state an invariant TSC is refused, except under a hypervisor: QEMU's TCG filters +invtsc. There, the references alone hold the TSC, and the log says so. RDTSCP is required. The TCG CPU model gains +rdtscp, and the accelerated model gains +invtsc.

Cross-CPU agreement at start. Every AP's TSC is bounded against the BSP's by 256 round trips of a cache line, and the bound is logged. A proven offset demotes the TSC. This replaces the 11 ms bracket.

The clock page. It carries a line (counter, stamp, ns per tick in 32.32) behind a two-copy latch, plus the demoted word (TOYOCLK3). The kernel retunes the rate, and slews, from where the clock stands, without a step.

• Every reader takes the counter inside the latch's read section, so a line replaced while it was preempted sends it round again.
• The ABI counter read is fenced on both sides. On AArch64 it is not nomem, so the compiler cannot move the latch's loads across it.
• The loom model covers the in-section read, and a companion shows the old order fails.

The census. The architecture supplies its own sources (arch::census_sources!). x86-64 adds rtc, and generic code names no architecture's source.

The metal rows. smp_roster_and_tsc_trail, timer_calibration and clock_holds_over_minutes fail on a demotion. They print every TSC STEP, left-out and waiting-on-the-RTC line.

The AMD step

• What it is: the TSC jumps 1.69–1.73 ms ahead once every boot. The HPET, the PM timer and the RTC agree within 37 µs, on two oscillators.
• Where it lands: the r6 scout (not landed) pinned it inside an early spell in which all 16 CPUs sit in port 0x415. In benchcase, spells 0–2 read no step and spell 3 ends 1.70 ms ahead. In testcases, the boot's first spell ends 1.72 ms ahead.
• Open: whether the platform or the state's entry makes the jump. The issue records it, owned by the clock and acpiserver. AMD's PPR for Family 17h Model 60h (55922) is unread, because amd.com refuses the build's user agent.

Decisions

• Owner-accepted:
  • the clock page mutable behind the latch;
  • the hypervisor exception;
  • the step policy: slew the first, demote on a second or a backward one;
  • demotion as a page flag and a syscall;
  • an undecided RTC taken as a TSC step;
  • the 500 ppm bound.
• Loom: kernel/loom/tests/clock_latch.rs models the latch, and clock-latch-fence-off is its control.
• Cost on the wake path: once a second, one CPU's pass spends one HPET read (14.8 µs on the T14, about 1.2 µs on the AMD) and one PM timer read. Every CPU's pass spends one rdmsr and one shared-line read-modify-write a second. IRQ8 adds one interrupt a second on cpu0.

Checks and evidence

Evidence: INDEX.md. The head, 0650c9d, is on main 427f654.

• Host CI at 0650c9d: cargo run -- --ci host exits 0 (108 steps green).
• Image: cargo run -- --build-only exits 0.
• Host tests cover:
  • the math: rate bounds, refusal of steps within the window, refinement, runtime step and drift, wide reads, round-trip offsets, 32-bit wraps and the line;
  • the policy:
    • the RTC's least-latency reading through 700 µs interrupts, and its drift correction (a 60 ppm crystal);
    • a drift base that never reaches across a step: a 1.7 ms step after a 4 ms one reads as itself only once the earlier edges are forgotten (the case asserts both halves);
    • its three verdicts and the response to a step;
    • a slew within 500 ppm that pays its debt in time;
    • a late sample that does not overpay, and a later one paid back;
  • the rows: clock_kept_its_counter fails a demoted boot.
• Loom: the latch, including a counter read inside the section. A should_panic companion shows the old order extrapolates a superseded line. The handover reuses Shootdown, whose protocol loom already models.
• QEMU x86-64 TCG, 2 vCPUs (a_stepped_tsc_is_slewed_then_demoted at 0650c9d, where cpu1 demoted and collected cpu0's floor; both tests at f5a102e):
  • a_stepped_tsc_is_slewed_then_demoted passes. The staged 10 ms step is slewed away. The second step demotes the TSC from cpu0, the round completes, and a process's reading on cpu1 then reaches the kernel.
  • a_stalled_hpet_is_left_out passes.
• Negative controls (earlier rounds, still standing):
  • the stepped test fails with the demotion arm disabled, and again with the page left unmarked;
  • the drift test fails with the correction zeroed.
• Metal r8, at fec4998 (the policy before review r3's fixes), judged at d739b07's harness:
  • T14: 4 rows PASS. No step, nothing left out, no demotion. The HPET over 128 s read 32 ppb from 15H, within 156 ppb.
  • AMD: 4 rows PASS (irq_census_conservation, smp_roster_and_tsc_trail, timer_calibration, clock_holds_over_minutes).
    • benchcase: a 1.701 ms step, confirmed by the RTC (1.710 ms), slewed away 12 s later. Over 128 s the HPET measured 3.9 ppm from the boot's calibration, within its 20.5 ppm bound.
    • testcases: a 1.718 ms step, unconfirmed (no RTC edge before it yet), slewing when the list ended.
    • No demotion, no per-CPU check fired, every AP within 288 cycles of the BSP.
  • Why r8 stands for this head: review r3's fixes change only:
    • a demotion's handover;
    • an AP's proven offset;
    • what a step after the first reads from the RTC;
    • AArch64's counter;
    • one kick-handler check.
      Neither bench demoted, proved an offset or took a second step, and neither is AArch64. With no round in flight, the handover's hint costs a per-CPU read and four loads. It is paid on every kick and, through tlb::poll, on every Lock spin. Neither changes what r8 measured: no bench took a round, and a hint in a spin delays only the spinner. So nothing r8 measured runs differently.
  • Not on metal: the handover itself. No bench demotes, and nothing stages a CPU whose TSC leads in what a process reads.

Risks

• The AMD step is absorbed, not removed. Readers see the clock about 1.7 ms ahead from the step until it is found, within a second plus the RTC's few seconds. It is then slewed back over about 12 s.
• A backward step, recorded in issues/a-demoted-tsc-keeps-the-kernels-clock-apart-from-processes.md:
  1. every reading goes back by the step until a check or the sampler finds it, within a second;
  2. the TSC is demoted, and processes' time jumps forward to the HPET's count from the last held sample;
  3. the kernel's own clock stays behind by the step.
• A forward second step: processes' time holds until the HPET's count reaches it, and the kernel's stays ahead.
• The RTC's verdict rests on the least-latency of three edges each side arriving within a quarter of the step, after its drift correction. It is unsure in the first seconds after it is armed, and an unsure step is taken as a TSC step.
• The slew: a sample more than a period late overpays, and the next pays it back. A machine whose passes stop for long would overpay by the gap times the rate.
• Guests: QEMU's HPET follows the host's slewable monotonic clock, while a TCG guest's TSC follows the raw host counter. A host clock slewed past the HPET tolerance would read as a drift and panic the guest. This is an inference.
• Still panicking: a drift past both bounds.
• The handover's claim rests on the code's argument. No test stages a CPU whose TSC leads and checks a process's readings across the demotion. The QEMU test shows only that the round completes on two vCPUs.
• The handover's residual: a reader of the line in force or the one it replaced is covered exactly. A reader of a line two or more retunes old can read behind. Retunes come a second apart by the sampling CPU's clock, so a CPU leading the previous sampler by λ samples about 1 s − λ later. Coverage then needs a lead under about a second where consecutive samples come from CPUs that far apart. Past that, the reading is behind by at most the rate change times the lead, 0.5 ms per second of lead at 500 ppm. This is recorded in issues/a-demoted-tsc-keeps-the-kernels-clock-apart-from-processes.md.
• A demotion waits for every CPU: a process's SYS_CLOCK_NOW spins until each online CPU takes its kick, which panics past 5 s.
• No real write tested: tsc-step stages its steps in the sampler's readings, because TCG ignores a write to the TSC.
• Syscall number: SYS_CLOCK_NOW is 152, which paged objects: a file's pages are the kernel's, read by its holders without a request (memory stage 7a) #1123 also takes. Whichever lands second renumbers against main at its rebase.
• AArch64: the generic timer is still kept at CNTFRQ_EL0, with nothing to hold it to.

🤖 Generated with Claude Code

[Japabu]

Review of #1104 at 68afed5

Architecture

The ownership is sound. Every process reads the TSC itself, so the kernel keeps it. The HPET is only the reference, and it lives in x86's arch module. The bound math lives in the pure crate (kernel/pure/clock/reference.rs) and is host-tested. Line and the latch belong in toyos-abi because both sides of the boundary read them. I support making the clock page mutable behind a two-copy latch: without it the AMD's measured rate can't be refined without a step, and the latch is the standard NMI-safe way to do that. The owner should still accept the ABI contract change explicitly, as the PR asks.

The hypervisor exception is sound capability selection, and metal stays strict. It does change the exit of the deleted the-system-clock-takes-the-tsc-… issue ("only where every CPU states it invariant"), so that is also the owner's call to accept.

Size: about +900 production and +500 test/model lines, against 700 deleted (hpet.rs, the bracket in smp.rs, three issues). That is proportionate to what is gained: bounds derived from the readings themselves, a runtime check, a measured offset per AP, and the 32.32 line.

What I checked and found correct

• rate() bound. The true ratio lies within (1±r)/(1∓u) of the measured one. With u < 1/4, 1/(1−u) ≤ 1 + 4u/3, so (u+r)(1+6·max(u,r)) covers it. Taking u and r against the measured spans is consistent. The +1/mult term covers the floored mult. TooWide enforces the quarter. Overflow is fine: span_fs << 33 stays under u128 for spans of days.
• agree(). Its terms are the half-widths, ±1 tick (a floor minus a floor), the rated span and +2 ns. The +2 ns covers the three floors, whose sum stays under 2 ns.
• 32.32 arithmetic. One unit of mult is 0.56 ppb at 2.4192 GHz and 1.16 ppb at 5 GHz. A century of ticks times mult fits in u128. The T14 log's 2419200000663 mHz (0.27 ppb) matches.
• Latch ordering. The writer does a Release store of seq, a Release fence, then relaxed word stores. The reader does relaxed word loads, an Acquire fence, then rechecks seq. That fence–fence pair makes any reader that sees a new word also see seq moved. Copy parity is right in both halves. An NMI on the writer's CPU reads the copy nobody is writing, and seq doesn't move under it, so it can't loop. The loom models and the clock-latch-fence-off control (registered in CONTROLS, so $CONTROLS clippy covers it) hold this.
• retune. It is continuous on one CPU. A reader on another CPU that still holds the old line can see at most a sub-ns-per-µs bend, which is negligible.
• Offset. RDTSCP; LFENCE before the Release store and after the Acquire load gives δ ∈ [−least_back, least_out]. The signs of the error branches and their messages match.
• 32-bit HPET extension. near() predicts from the TSC, so long idle gaps don't break it.
• Citations. The 15H/16H description matches the SDM: 15H ECX is the nominal crystal frequency, and 16H is for display. The HPET clause matches r2/hpet-spec-clause.txt (§2.2, 500 ppm over ≥1 ms and 2000 ppm over ≤100 µs; COUNT_SIZE_CAP is §2.3.4).
• Panics. These panic on a genuine platform invariant and are consistent with the issue exits: missing RDTSCP; no invariant TSC on metal; an HPET that doesn't advance; a proven AP offset; an AP that stops answering; an IA32_TSC_ADJUST change; three disturbed calibrations. Blockers 2 and 4 below cover the cases where the step/drift panic can fire on a healthy TSC.

BLOCKERS

1. issues/ deletions and the PR's evidence — the AMD rows have not run, and the T14 evidence is reused without its argument.

   • The three deleted issues' exits name the AMD bench: "Each part is shown on the T14 and the AMD bench", and "against … tsc-drift on the AMD bench".
   • The 50 ms calibration defect is the AMD's. The AMD's 1.7 ms early step could also panic the boot under ToyOS.
   • Landing now records as done what has only been shown on the T14. Either the AMD's smp_roster_and_tsc_trail, timer_calibration and clock_holds_over_minutes rows pass at the head first, or the issues stay until they do.
   • The T14 rows ran at 378995d. The head differs in two ways: a runtime sample is one read, and spans under 1 ms take 0.2%. Both only widen the allowance (the bracket is in allowed_ns), so the pass very likely carries over. The PR must say so, per root's reuse rule. Or rerun the T14 in the same session as the AMD, which is cheap.

2. kernel/src/arch/x86_64/clocksource.rs:332-395 and reference.rs Watch::take assume the HPET counts in every package C-state. On the T14 this is unverified.

   • The T14 now idles in MWAIT 0x60, its C10 (issues/idle-cpus-enter-the-c-states-cst-names.md).
   • Linux force-disables the HPET when PC10 is enabled on recent Intel parts, logging that it is "dysfunctional in PC10" and stops counting. That is another implementation's behaviour, so it is an inference and not an authority, but it is exactly this failure mode.
   • If the HPET stands while the invariant TSC runs, the next sample reads the TSC ahead by the residency and panics "cpuN's TSC stepped" on a healthy laptop. That would happen in an ordinary display-off idle, which none of the bench boots exercised.
   • The evidence holds no package-residency reading, so it doesn't show PC10 was reached.
   • Cheapest sufficient checks, either one:
     • Intel's documentation for the T14's PCH on the HPET in PC10/S0ix.
     • A T14 boot that logs MSR_PKG_C10_RESIDENCY (632H) across a held span, with the package allowed to reach PC10.
   • If the HPET does stop there, holding a 15H-stated TSC to it across idle is a design question for the owner.

3. kernel/src/sched/driver.rs:522 / clocksource.rs:332 — per-CPU HPET sampling costs more and checks less than a simpler split. This answers the question the PR put to review.

   • Cost. The check runs before the pass picks, with preemption off, so once a second every T14 CPU stalls its next thread by a measured 14.8 µs uncached read, on exactly the wake path. The pass then schedules with a now that is 14.8 µs stale.
   • What it buys. Cross-CPU agreement, but only as coarsely as the step allowance: readings + one tick + 0.05% of the gap (about 500 µs at a 1 s gap), or about 15 µs at best on the T14 when two CPUs' samples happen to land close together.
   • The split. One CPU's HPET sample a second covers rate, drift and steps of the whole machine. The per-CPU properties are better served by:
     • the per-CPU IA32_TSC_ADJUST read already there;
     • a shared-line check: load the last-published TSC (Acquire), then rdtsc_ordered, and a reading below it proves this CPU behind by the difference. This is reference::Offset's own argument. Then fetch_max its own reading.
   • Effect. That costs about 100 ns per CPU per second and bounds runtime offsets to cache latency (the round trips showed ≤181 cycles) instead of hundreds of µs. It also removes (N−1) HPET reads per second from wake paths.
   • With the start-time offset bound, per-CPU HPET reads are not needed.

4. clocksource.rs:380-393 — the panic messages blame the TSC.

   • A failed step/drift agreement only shows that the TSC and the HPET disagree. It doesn't show which one moved.
   • On the AMD, which counter moved in the 1.7 ms event is exactly what is unknown. Under a hypervisor, QEMU's HPET follows the host's slewable monotonic clock while the guest TSC follows the raw host counter. This is an inference: macOS's CLOCK_MONOTONIC is documented as frequency-adjusted, and chrony may slew past 500 ppm.
   • "cpuN's TSC stepped" asserts a cause the code doesn't know. It should say the two disagree, give the numbers it already prints, and name both readings.
   • Record the host-slew risk for guests in the PR's risks.

5. Comments.

   • clocksource.rs:54-60 (CHECK_EVERY_NS) carries measurement history: "14.8 µs on the T14, whose HPET read brackets 35,690 TSC cycles", plus the argument for one read instead of three. Root puts measurements and investigations in commits and evidence; they already sit in commit 2's message. Keep only the contract, e.g.: "Once a second: the step allowance at this gap is about 0.5 ms, under the 1.7 ms step. One read a sample: every bound allows its bracket."
   • kernel/loom/tests/clock_latch.rs:17-19 says "Interleavings past this many preemptions add nothing a two-party protocol of this length can do". Nothing establishes that. What is known is that the bound keeps exploration finite and that the fence-off control still reaches a torn read under it. Say that, or say nothing.

SUGGESTIONS

• clocksource.rs:170, stated-rate bound. Kept { bound_ppb: 1 } for a CPUID 15H rate is the mult's rounding, not an accuracy: the SDM calls 15H ECX the crystal's nominal frequency. Yet the log says "the clock keeps CPUID 15H's … mHz, within 1 ppb", beside an HPET measurement "within 118 ppb", which reads as if the stated rate were the better-known one. The drift allowance also gets no term for the TSC crystal's own tolerance. That is harmless where the TSC and HPET share a crystal, as on the T14 (HPET period 52,083,333 fs is 38.4 MHz/2). Drop "within" for a stated rate, or have the field say "as stated".
• reference.rs:162-165 and the PR text, "a step inside the window cannot skew the rate". That holds only for a step the readings can resolve. With 1.2 µs reads, a step of about 1.8 µs at sample 1 sits within every sample's allowance. It still skews a 50 ms rate by about 36 ppm, past its own 24–27 ppm bound. Real steps that small are not expected, but say "a step larger than the readings resolve".
• sdk/abi/src/trace.rs:6. "[crate::clock::ClockPage] turns into nanoseconds" now belongs to Clock.
• build/arch.rs:166, hardware model. A -cpu host KVM guest also lands in the hypervisor exception, because QEMU filters invtsc unless asked. +invtsc on the hardware model would let accelerated guests take the metal path. This is optional.
• PR text (for the orchestrator). "the AMD's rate is ppm-level within seconds and ppb-level within minutes" is a projection from the host model's 1.2 µs reads, not a measurement yet. Label it until the AMD run.

Evidence assessment

• Measured: host CI at the head; the loom model and its red control; three reference mutations, with a test added where one passed; a_stepped_tsc_panics at the head's kernel, which proves the wiring and the verdict but not a real TSC write (the PR says so); the T14's three rows at 378995d.
• Not measured: the AMD; any package-C10 period on the T14; the runtime cost on the AMD.
• The tsc-step actuator is under boot-actuators only, which is acceptable.

SEND BACK

[Japabu]

Review r4 of #1104 at f5a102e (fec4998..f5a102e)

Architecture

The handover is the right shape. It reuses Shootdown and the existing kick, and every CPU floors at its own clock after the mark. That closes the r3 case: a page reader checks the mark after its counter read, and the store is visible before the IPI (SeqCst fence, then wrmsr_fence or the UC ICR write). So a reader that found the page unmarked read its counter before its own CPU's floor. DEMOTED_LATEST's fetch_max happens after HANDED_OVER's Release/Acquire, so every answer comes after every floor in modification order, and answers stay monotonic under any interleaving among themselves. The r3 delta is +145/−34: the round, Edges::forget_before with its host case, and the AP-start arm.

The r3 headlines:

1. Handover: fixed in the main case. Two defects remain in the new round (B1, B2), and the residual is unrecorded (B3).
2. AArch64 nomem: fixed in both the ABI counter and the kernel counter_ordered.
3. Drift base: fixed. settle forgets edges before pending.to.after after computing the current verdict. The host case asserts both halves.
4. AP-start offset: it now demotes, but see B2.
5. QEMU at head: qemu-19.diff matches git diff d739b0743 f5a102ed9 exactly once the index lines are dropped. Both clock tests pass, exit 0, and cpu1's process read through the kernel after cpu0's round.
6. Comments: mostly fixed; B2 covers one false statement.

A userland action does not reach the 5 s panic directly. stamp_now serves its own CPU and holds nothing the demoter needs, and the demoter runs with preemption off. B1 is the exception: concurrent kernel work, some of it started by userland, can wedge the demoter.

Required

B1 kernel/src/arch/x86_64/clocksource.rs:598-602, 622-629: the handover's spin answers no other round, so it can deadlock against a TLB shootdown or an SMI_CMD write.

• Problem: demote runs from check() inside a scheduler pass. The pass can have IF clear: exit_current and leave_user_if_due mask into the exit pass, and shootdown.rs:126 names "the tick's pass" as an IF-clear context. awaiting serves only HANDOVER.
• Scenario A: CPU D demotes with IF clear. CPU T is in tlb::wait_for with IF clear; Shootdown::wait_turn's comment names that state "inside an IrqGuard, a handler or the tick's pass".
  • T waits for D's TLB flush. D never takes vector 0xFE and never polls.
  • D waits for T's HANDOVER answer. T takes no kick, and wait_turn serves only the TLB.
  • Both spin until one panics on DEAF_CPU. The kernel crashes on an interleaving of a hardware event with ordinary unmap or teardown work.
• Scenario B: D is cpu0 while another CPU is in smi_cmd::write with IF clear. Its spin calls tlb::poll but never serves HANDOVER, and D never serves smi_cmd.
• Fix: follow the established rule ("a spinner … has IF clear and takes no IPI", sync.rs:157-161; smi_cmd::write follows it at smi_cmd.rs:170-171). Call super::tlb::poll() in awaiting's loop, which serves both the TLB and smi_cmd.
• Optional companion: have tlb::poll serve the handover too. A Lock spinner with IF clear would then answer it, and the "Never logs" constraint on hand_over would loosen.
• Evidence: inspection is enough for the cycle. A test cannot cheaply stage it.

B2 clocksource.rs:592-596 (doc :586-587): a demotion before the release leaves every AP owing the round, and each AP floors at its own clock whenever it first serves.

• Problem: HANDOVER.issue raises requested to 1 for every CPU. With !answering(), only the demoting CPU serves, so every AP keeps flushed = 0 < requested.
• Scenario:
  • measure_offset proves cpu3's TSC ahead of cpu0's by δ and demotes before the release.
  • After the release, cpu3's first kick, or the first SYS_CLOCK_NOW on cpu3, runs floor_here there. awaiting's closure calls serve_here before it checks HANDED_OVER.
  • That raises DEMOTED_LATEST to cpu3's clock at that later time. Processes' time jumps forward by about δ against the HPET, then holds for δ.
  • No process ever read cpu3's TSC through the page, because the page was marked before any process ran there. So the floor protects nothing.
• Consequences:
  • The doc's "none is asked" is false; the APs are asked, only later.
  • The issue's "holds at the furthest-ahead CPU's clock at the demotion" does not describe this.
  • HANDED_OVER's doc ("every online CPU has answered") is false on this path.
• Fix: when !smp::answering(), call floor_here() and return without issuing a generation.

B3 the handover's residual is real and bounded, but recorded only in a doc comment and the PR body (clocksource.rs:584-586; issues/a-demoted-tsc-keeps-the-kernels-clock-apart-from-processes.md:28-31).

• Mechanism: a reader on X returns v = L.stamp_of(c). A retune publishes L′ at counter a on CPU S after the reader's recheck and before X's floor. Then floor_X − v ≈ mult·(f−c) − (mult−mult′)·(f−a). With f−c small, an answer reads behind v, within one thread, by up to Δmult × (X's lead over S + the publish latency).
• Bound: Δmult is up to 500 ppm during a slew. The lead is unbounded in exactly the cases that demote: IA32_TSC_ADJUST written, or a CPU behind. So the backward step is bounded relative to the lead, not absolutely.
• Scope correction: the PR and commit message say "replaced before the mark". The window also covers retunes after the mark and before X's floor. slew() reads IS_DEMOTED, which is now set at the swap before the mark, and retunes to the kept rate at :779-785 while D runs the round. That retune is redundant with D's own at :571.
• Fix, either:
  • close it: for example, floor at the max over the current line and the one it replaced, which clock::retune can keep under RETUNING, and have slew() not retune while the handover is in flight; or
  • record it in the demoted-TSC issue: the mechanism, the bound, the slew-window condition, owner and exit.
• Why required: r3 asked for this to be fixed, not narrowed. A comment is not the record "No silent debt" requires.

Optional

• S1 clocksource.rs:608-611: serve_here takes an IrqGuard before its hint.
  • Every kick, the wake IPI, pays a flags save/restore and a per-CPU read even with no round in flight. smi_cmd::serve_here checks owes first ("the hint before the mask").
  • The PR body's "the kick handler's check is two loads" is inaccurate as written; the harm is small.
• S2 clocksource.rs:521-526, 550-551: a CPU whose TSC trails stays behind LATEST for the boot. After a demotion, its check() logs DEMOTED TSC: … every second, forever. With an AP-start offset now demoting rather than panicking, that state lasts the whole boot. Log the repeat once, or once per new kind.
• S3 clocksource.rs:622-629: awaiting reads nanos_since_boot on every spin. tlb::wait_for batches the deadline check (SPINS_PER_DEADLINE_CHECK) for the 128-bit divide's cost. It is harmless at one demotion per boot, so this is optional.

Evidence

• Accepted: the host case for headline 3. It is its own control.
• Accepted: QEMU at head (headline 5, verified above).
• Accepted: reusing metal r8. No bench demotes, proves an offset, takes a second step or is AArch64.
• The cross-CPU claim rests on reasoning, which is acceptable here. Loom cannot model an interrupt boundary, and no target can stage a leading TSC in a process's read.
• Needed after the fixes: a compile, --ci host, and a rerun of a_stepped_tsc_is_slewed_then_demoted, which exercises B1's and B2's paths on the round.

SEND BACK (B1, B2, B3).

[Japabu]

Review r5 of #1104 at 0650c9d (f5a102e..0650c9d)

Assessment

The r4 fixes have the right shape. They add 102 lines and remove 42 across four files, and every change sits in the clock or in tlb::poll.

• B1: fixed. awaiting calls tlb::poll on every spin, so the demoter now answers a TLB initiator's round and an SMI_CMD writer's round. Both deadlock cycles from r4 are broken: T's wait_for and the smi_cmd::write loop finish once D has served them. tlb::poll → clocksource::serve_here → floor_here → clock::floor_at takes no lock and does not call tlb::poll, so nothing recurses. A kick that lands between the owes hint and the IrqGuard is harmless: the guarded serve_if_owed checks again, and fetch_max is idempotent.
  • floor_at's expect on REPLACED.snapshot() holds. A CPU owes the handover only between HANDOVER.issue and its own serve. That window lies inside the hold, which clock::demote takes under RETUNING before the mark, so no publish of REPLACED is in progress on any CPU.
  • Lock order: clocksource::HELD is taken before RETUNING, and no path takes them the other way round.
• B2: fixed. Before the release, hand_over floors locally and issues no round. The APs start (main.rs:451) before the supervisor is spawned (:463), and check() runs only in scheduler passes. The HANDED_OVER doc and the issue's "demotes before any process runs" are therefore true.
• B3: fixed for readers of the two latest lines. A reader of the line in force or of the one it replaced read its counter at some c before the floor's at on the same CPU, so max(L₀(at), L₋₁(at)) ≥ Lᵢ(c) by monotonicity. The hold leaves no retune gap: a retune between the IS_DEMOTED swap and HELD lands before the mark and simply becomes L₀.
  • Older lines: my derivation for a line two or more retunes old agrees with the issue's formula. The shortfall is Δ·λ − m·T, where T is the time from the read line's replacement to the floor, and its magnitude is at most Δ·λ. The premise behind the "about 2000 s" figure is wrong, though; see R2.
• Optional changes: S1, S2 and S3 are taken. One comment in S1 is false (R3).
• Evidence: I checked qemu-20.diff and ci-host-12.diff against git diff f5a102ed9 0650c9d08; they are identical once the index lines are dropped.
  • qemu-20.log: a_stepped_tsc_is_slewed_then_demoted PASS, and the demotion ran on cpu1 during a slew. That run exercises the round, the tlb::poll wait and floor_at with a non-zero REPLACED.
  • ci-host-12.log: 108 steps green, including the AArch64 kernel clippy arms.
  • Not exercised: B2's path runs only on a proven offset, which no target stages. Inspection is enough for it.

Required

R1 kernel/src/clock.rs:45-46 and kernel/src/arch/x86_64/clocksource.rs:588-590: two comments claim complete coverage, and the issue records that coverage as incomplete.

• HELD's doc says "the line in force and the one it replaced are what every page reader read". That is false: a process can hold a reading taken along any older line, which is exactly the residual the issue records.
• hand_over's doc says "no answer reads before it however far that TSC leads". That is false for a reader of a line two or more retunes old whose CPU leads, as in R2's scenario.
• Consequence: a maintainer who reads either comment would conclude the handover is exact and would treat the issue as stale.
• Fix: say what is actually held. HELD keeps the two lines floor_at reads unchanged through the round. hand_over's guarantee covers a reader of the line in force or of the one it replaced. Put the rest in the issue only, or point to it.

R2 issues/a-demoted-tsc-keeps-the-kernels-clock-apart-from-processes.md:36-39 (and PR body "covered while its CPU's lead stays under about 2000 s"): the premise "samples a second apart" fails when the CPUs disagree, which is when a demotion happens.

• Mechanism: check() compares NEXT_SAMPLE_NS with each CPU's own now_ns (clocksource.rs:534-535), and the due time is the previous sampler's clock plus 1 s. So a CPU A that leads the previous sampler B by λ takes the next sample about 1 s − λ later in real time, not 1 s later. During a slew, every sample retunes (slew(), :801-804).
• Scenario:
  1. B lags A by λ just under 1 s. The per-CPU check catches a lag only when it exceeds the phase between the CPUs' checks, so this one goes undetected.
  2. A process on A reads L₋₂.
  3. B's sample starts or ends a slew, publishing L₋₁ with Δ = 500 ppm. A samples about 1 − λ later and publishes L₀.
  4. A demotion follows before the next retune.
  5. A's floor exceeds the process's reading only by m·(1 − λ). Against that, the process's L₋₂ reading leads L₋₁ by up to Δ·λ. The answer is behind once λ > ~0.9995 s, by up to about 0.5 ms.
  6. The required lead is therefore about a second, not about 2000 s.
• The bound is still right: "at most the change of rate times the lead" stays true.
• Fix: correct the record and the PR body. The record should say:
  • the spacing between retunes is a second by the sampling CPU's clock, so a leading CPU samples sooner by its lead;
  • the covering condition is then a lead under about a second where consecutive samples come from CPUs that far apart;
  • the bound on magnitude stands.
• Why required: the 2000 s figure is what makes the residual look remote. "No silent debt" needs the record to be accurate.

R3 kernel/src/arch/x86_64/clocksource.rs:608-611: two statements in serve_here's comments are false.

• "so every spinner with interrupts closed answers it":
  • tlb::wait_for's wait_turn serves only SHOOTDOWN (shootdown.rs:138).
  • serial_lock::BackendLock::lock spins without polling (serial_lock.rs:58-72). hand_over's own "Never logs" rationale (:594-595) depends on exactly that spinner being unable to answer.
  • The two comments contradict each other. Deadlock freedom actually rests on the demoter answering every round itself (B1), not on every spinner answering.
• "with no round in flight a kick pays two loads":
  • HANDOVER.owes loads requested, CpuSlice's pointer and length, and flushed, after the per-CPU cpu_id read. That is five loads.
  • It is now also paid on every Lock spin through tlb::poll.
• Fix: name the real answerers, for example "the kick handler's and tlb::poll's, so a Lock spinner answers it". Drop the count, or say "the hint, not the mask".

Optional

• S1 clock.rs:121-122: floor_at can raise the floor above every reading. When the replaced line is faster, its term exceeds the line in force by Δ_last × (time since the last retune). Processes' time then jumps forward by that amount and holds for it. In practice this is small:

  • a slew's first second: up to 0.5 ms;
  • with the doubling reports: about the HPET bracket's width;
  • once the bound reaches its quantization floor, and retunes stop: about 1 ppb × uptime.

  The issue's "holds at the furthest-ahead CPU's clock" could say "or up to that much past it". This is optional because the effect is bounded and monotonic.

• S2 clocksource.rs:633-634: awaiting reads nanos_since_boot before it first checks done. So every SYS_CLOCK_NOW after the handover pays one clock read just to set a deadline it never uses. Checking done() first avoids it.

• S3, PR body, metal reuse: the justification lists only the kick handler. tlb::poll now pays the HANDOVER hint on every Lock spin too. That does not affect anything r8 measured, but the paragraph should say so (orchestrator).

Verdict

SEND BACK (R1, R2, R3). All three are comment and record fixes; nothing needs another run beyond a compile.