public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: David Woodhouse <dwmw2@infradead.org>
Cc: Dongli Zhang <dongli.zhang@oracle.com>,
	Joe Jin <joe.jin@oracle.com>,
	x86@kernel.org, kvm@vger.kernel.org,
	linux-kernel@vger.kernel.org, pbonzini@redhat.com,
	tglx@linutronix.de, mingo@redhat.com, bp@alien8.de,
	dave.hansen@linux.intel.com
Subject: Re: [PATCH RFC 1/1] KVM: x86: add param to update master clock periodically
Date: Mon, 2 Oct 2023 09:37:09 -0700	[thread overview]
Message-ID: <ZRrxtagy7vJO5tgU@google.com> (raw)
In-Reply-To: <884aa233ef46d5209b2d1c92ce992f50a76bd656.camel@infradead.org>

On Mon, Oct 02, 2023, David Woodhouse wrote:
> On Fri, 2023-09-29 at 13:15 -0700, Dongli Zhang wrote:
> > 
> > 
> > We want more frequent KVM_REQ_MASTERCLOCK_UPDATE.
> > 
> > This is because:
> > 
> > 1. The vcpu->hv_clock (kvmclock) is based on its own mult/shift/equation.
> > 
> > 2. The raw monotonic (tsc_clocksource) uses different mult/shift/equation.
> > 
> > 3. As a result, given the same rdtsc, kvmclock and raw monotonic may return
> > different results (this is expected because they have different
> > mult/shift/equation).
> > 
> > 4. However, the base in  kvmclock calculation (tsc_timestamp and system_time)
> > are derived from raw monotonic clock (master clock)
> 
> That just seems wrong. I don't mean that you're incorrect; it seems
> *morally* wrong.
> 
> In a system with X86_FEATURE_CONSTANT_TSC, why would KVM choose to use
> a *different* mult/shift/equation (your #1) to convert TSC ticks to
> nanoseconds than the host CLOCK_MONOTONIC_RAW does (your #2).
> 
> I understand that KVM can't track the host's CLOCK_MONOTONIC, as it's
> adjusted by NTP. But CLOCK_MONOTONIC_RAW is supposed to be consistent.
> 
> Fix that, and the whole problem goes away, doesn't it?
> 
> What am I missing here, that means we can't do that?

I believe the answer is that "struct pvclock_vcpu_time_info" and its math are
ABI between KVM and KVM guests.

Like many of the older bits of KVM, my guess is that KVM's behavior is the product
of making things kinda sorta work with old hardware, i.e. was probably the least
awful solution in the days before constant TSCs, but is completely nonsensical on
modern hardware.

> Alternatively... with X86_FEATURE_CONSTANT_TSC, why do the sync at all?
> If KVM wants to decide that the TSC runs at a different frequency to
> the frequency that the host uses for CLOCK_MONOTONIC_RAW, why can't KVM
> just *stick* to that?

Yeah, bouncing around guest time when the TSC is constant seems counterproductive.

However, why does any of this matter if the host has a constant TSC?  If that's
the case, a sane setup will expose a constant TSC to the guest and the guest will
use the TSC instead of kvmclock for the guest clocksource.

Dongli, is this for long-lived "legacy" guests that were created on hosts without
a constant TSC?  If not, then why is kvmclock being used?  Or heaven forbid, are
you running on hardware without a constant TSC? :-)

Not saying we shouldn't sanitize the kvmclock behavior, but knowing the exact
problematic configuration(s) will help us make a better decision on how to fix
the mess.

  reply	other threads:[~2023-10-02 16:37 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-26 23:06 [PATCH RFC 1/1] KVM: x86: add param to update master clock periodically Dongli Zhang
2023-09-27  0:29 ` Joe Jin
2023-09-27  0:36   ` Dongli Zhang
2023-09-28 16:18     ` Sean Christopherson
2023-09-29 20:15       ` Dongli Zhang
2023-10-02  8:33         ` David Woodhouse
2023-10-02 16:37           ` Sean Christopherson [this message]
2023-10-02 17:17             ` Dongli Zhang
2023-10-02 18:18               ` Sean Christopherson
2023-10-02 21:06                 ` Peter Zijlstra
2023-10-02 21:16                   ` Peter Zijlstra
2023-10-02 18:16             ` David Woodhouse
2023-10-03  0:53               ` Sean Christopherson
2023-10-03  1:32                 ` Dongli Zhang
2023-10-03  1:49                   ` Sean Christopherson
2023-10-03  2:07                     ` Dongli Zhang
2023-10-03 21:00                       ` Sean Christopherson
2023-10-03  5:54                 ` David Woodhouse
2023-10-04  0:04                   ` Sean Christopherson
2023-10-04 10:01                     ` David Woodhouse
2023-10-04 18:06                       ` Sean Christopherson
2023-10-04 19:13                         ` Dongli Zhang
2023-10-11  0:20                           ` Sean Christopherson
2023-10-11  7:18                             ` David Woodhouse
2023-10-13 18:07                               ` Sean Christopherson
2023-10-13 18:21                                 ` David Woodhouse
2023-10-13 19:02                                   ` Sean Christopherson
2023-10-13 19:12                                     ` David Woodhouse
2023-10-13 20:03                                       ` Sean Christopherson
2023-10-13 20:12                                 ` Dongli Zhang
2023-10-13 23:26                                   ` Sean Christopherson
2023-10-14  9:49                                     ` David Woodhouse
2023-10-16 15:47                                       ` Dongli Zhang
2023-10-16 16:25                                         ` David Woodhouse
2023-10-16 17:04                                           ` Dongli Zhang
2023-10-16 18:49                                           ` Sean Christopherson
2023-10-16 22:04                                             ` Dongli Zhang
2023-10-16 22:48                                               ` Sean Christopherson
2023-10-17 16:18                                                 ` Dongli Zhang
2023-10-03  9:12                 ` David Woodhouse
2023-10-04  0:07                   ` Sean Christopherson
2023-10-04  8:06                     ` David Woodhouse
2023-10-03 14:29                 ` David Woodhouse
2023-10-04  0:10                   ` 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=ZRrxtagy7vJO5tgU@google.com \
    --to=seanjc@google.com \
    --cc=bp@alien8.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=dongli.zhang@oracle.com \
    --cc=dwmw2@infradead.org \
    --cc=joe.jin@oracle.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /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