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

Sean Christopherson <[email protected]>
Newsgroups org.kernel.vger.kvm,dev.linux.lists.sashiko-reviews
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.
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.