All of lore.kernel.org
 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: 33+ 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-07 16:01         ` Sean Christopherson
2026-08-07 17:26           ` David Woodhouse
2026-08-08 15:08             ` David Woodhouse
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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.