From: Sean Christopherson <seanjc@google.com>
To: David Woodhouse <dwmw2@infradead.org>
Cc: sashiko-reviews@lists.linux.dev, kvm@vger.kernel.org
Subject: Re: [PATCH v8 13/17] KVM: x86: Disable preemption, not IRQs, when getting TSC+freq pair
Date: Wed, 5 Aug 2026 09:22:29 -0700 [thread overview]
Message-ID: <anNjRbdR1P0yZZa2@google.com> (raw)
In-Reply-To: <04c395bb7d7a63e6b38a8d5684cffde1a1ebcf3f.camel@infradead.org>
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.
next prev parent reply other threads:[~2026-08-05 16:22 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-04 23:39 [PATCH v8 00/17] KVM: x86: Cleaning up the KVM clock mess, part 1 Sean Christopherson
2026-08-04 23:39 ` [PATCH v8 01/17] KVM: x86: Update "last guest TSC" snapshot prior to enabling IRQs/preemption Sean Christopherson
2026-08-04 23:39 ` [PATCH v8 02/17] KVM: x86: Improve accuracy of KVM clock when TSC scaling is in force Sean Christopherson
2026-08-04 23:39 ` [PATCH v8 03/17] KVM: x86: Explicitly disable TSC scaling without CONSTANT_TSC Sean Christopherson
2026-08-04 23:39 ` [PATCH v8 04/17] KVM: x86: Activate master clock immediately on vCPU creation Sean Christopherson
2026-08-05 0:06 ` sashiko-bot
2026-08-05 9:11 ` David Woodhouse
2026-08-05 15:02 ` Sean Christopherson
2026-08-04 23:39 ` [PATCH v8 05/17] KVM: x86: Avoid NTP frequency skew for KVM clock on 32-bit host Sean Christopherson
2026-08-05 0:02 ` sashiko-bot
2026-08-05 18:21 ` Sean Christopherson
2026-08-07 0:27 ` Sean Christopherson
2026-08-04 23:39 ` [PATCH v8 06/17] KVM: x86: Drop unnecessary CPU pinning when computing/getting kvmclock Sean Christopherson
2026-08-04 23:39 ` [PATCH v8 07/17] KVM: x86: Move "no master clock" fallback from __get_kvmclock() to get_kvmclock() Sean Christopherson
2026-08-04 23:52 ` sashiko-bot
2026-08-05 15:17 ` Sean Christopherson
2026-08-04 23:39 ` [PATCH v8 08/17] KVM: x86: Wrap all of __get_kvmclock_master_clock() with CONFIG_X86_64=y Sean Christopherson
2026-08-04 23:39 ` [PATCH v8 09/17] KVM: x86: Fall back to non-master-clock if clockread fails in get_kvmclock() Sean Christopherson
2026-08-04 23:39 ` [PATCH v8 10/17] KVM: x86: Fix KVM clock precision in get_kvmclock() with TSC scaling Sean Christopherson
2026-08-04 23:39 ` [PATCH v8 11/17] KVM: x86: Use get_kvmclock() in kvm_get_wall_clock_epoch() Sean Christopherson
2026-08-04 23:39 ` [PATCH v8 12/17] KVM: x86: Fix compute_guest_tsc() to handle negative time deltas Sean Christopherson
2026-08-04 23:39 ` [PATCH v8 13/17] KVM: x86: Disable preemption, not IRQs, when getting TSC+freq pair Sean Christopherson
2026-08-04 23:56 ` sashiko-bot
2026-08-05 15:16 ` Sean Christopherson
2026-08-05 15:55 ` David Woodhouse
2026-08-05 16:22 ` Sean Christopherson [this message]
2026-08-04 23:39 ` [PATCH v8 14/17] KVM: x86: Make master clock logic in guest PV clock updates 64-bit only Sean Christopherson
2026-08-04 23:39 ` [PATCH v8 15/17] KVM: x86: Upscale TSC to "now", not master clock when updating PV clocks Sean Christopherson
2026-08-04 23:39 ` [PATCH v8 16/17] KVM: x86: Simplify and comment kvm_get_time_scale() Sean Christopherson
2026-08-04 23:39 ` [PATCH v8 17/17] KVM: x86: Remove implicit rdtsc() from kvm_compute_l1_tsc_offset() Sean Christopherson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=anNjRbdR1P0yZZa2@google.com \
--to=seanjc@google.com \
--cc=dwmw2@infradead.org \
--cc=kvm@vger.kernel.org \
--cc=sashiko-reviews@lists.linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox