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.