Kernel KVM virtualization development
 help / color / mirror / Atom feed
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.

  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