Re: [PATCH v8 13/17] KVM: x86: Disable preemption, not IRQs, when getting TSC+freq pair

Sean Christopherson <[email protected]> Wed, 5 Aug 2026 09:22:29 -0700
Newsgroups dev.linux.lists.sashiko-reviews,org.kernel.vger.kvm
Message-ID <[email protected]>
On Wed, Aug 05, 2026, David Woodhouse wrote:
> On Wed, 2026-08-05 at 08:16 -0700, Sean Christopherson wrote:
> > 
> > David, any thoughts?  I'm leaning towards keeping IRQs disabled to minimize the
> > chances of introducing a regression, even though I highly doubt disabling IRQs
> > to provide an atomic-ish pair was ever done deliberately.  My main concern with
> > disabling IRQs is that it will further muddy the waters with respect to what is
> > actually necessary, versus weird things KVM does for historical reasons.  Though
> > that can largely be solved with a verbose changelog.
> 
> I'm not sure I'd bother. There are plenty of other places we use an
> "atomic-ish pair", although I've tried to kill most of those by the
> time we get to the end of my series. And we don't disable interrupts
> around them all; why should this one do so just because it accidentally
> inherited it for other reasons?

Ya, after trying to write a changelog and reconcile the new "rule" with the
existing code, I agree.  For kvmclock, the badness is that the guest's view of
time would be off by a smidge until the next kvm_guest_time_update(), but that's
a complete non-issue when considering that a host IRQ at any time immediately
introduces significantly lag into the guest's read of "now".

TSC catchup due to an unstable TSC is a similar story.  The "bad" offset will be
corrected on the next kvm_arch_vcpu_load().

So it's really just the "always catchup" mode for software-based TSC scaling that
would have a persistent flaw, because as Sashiko pointed out, KVM would adjust
the offset by "too much".  But that's a fundamental flaw in the catchup logic:
KVM should compute an guest TSC as an absolute value by using the current time
and a reference time, not by accumulating delta.